Flux deploy preview for use in WE#596
Open
jhweir wants to merge 78 commits into
Open
Conversation
Add Channel.recentConversations() and Channel.pinnedConversations() static methods that use single SPARQL queries instead of iterative channel.get() loops. - Channel.recentConversations(perspective, limit): single query with ORDER BY DESC(?lastActivity) LIMIT — replaces N×M×K iterative walk - Channel.pinnedConversations(perspective): single query for pinned channels with their conversation IDs - Both methods deduplicate by channelId and handle errors gracefully Tests prove single SPARQL call per invocation (not N+1).
…dels - ChannelSummary: Channel without @hasmany relations — no hidden graph exploration during sidebar/list hydration - MessageSummary: Message without SPARQL getter properties (replyingTo, isPopular) — retains simple @hasmany (reactions, thread, replies) - Both exported from @coasys/flux-api barrel Tests verify no relation/getter queries fire on lightweight models.
- useCommunityService: use ChannelSummary instead of Channel for allChannels live query — eliminates @hasmany relation hydration on every link change - Replace N+1 getPinnedConversations with Channel.pinnedConversations() - Replace N×M×K getRecentConversations with Channel.recentConversations() - Replace iterative getChannelsWithConversations with Conversation.findOne() - Update handleParticipantTracking for ChannelSummary (no participants prop) - MessageList.tsx: add lazy evaluateGetters for visible messages (replyingTo) — works with WS-2 deepQuery inversion where getters are skipped by default Tests verify scoped queries and no iterative patterns remain.
- Remove perspective.addListener('link-added', handleLinkAdded) from
TimelineColumn.vue — was firing on EVERY link in the perspective
- Remove handleLinkAdded, getDataFull, getDataIncremental, debounce state
- Replace with useLiveQuery(Conversation, perspective, { parent: channel })
— only fires when Conversation instances under this channel change
- Watch conversationInstances for reactive refreshAllData() calls
- Import useLiveQuery, remove onUnmounted (handled by useLiveQuery)
Tests verify no raw listeners, no old handler functions, and scoped
useLiveQuery usage.
All WS-3/4/5/6 tests now strip // and /* */ comments before pattern matching, preventing false failures from comment text like 'No @hasmany relations' or 'replaces perspective.addListener'. All 72 tests pass.
Code comments now describe what the code does rather than referencing internal planning workstream identifiers. Renamed test files for clarity.
The previous query applied LIMIT before deduplication, so a busy channel with many items could consume all LIMIT rows, starving other channels from the result set. Switch to GROUP BY ?channelId with SAMPLE/MAX aggregation so the LIMIT applies after per-channel grouping. Oxigraph supports full SPARQL 1.1 aggregation (already used elsewhere in this file via COUNT(DISTINCT ...)), so remove the misleading comment about GROUP BY availability. The client-side seen Map is kept as a safety net but should no longer be needed.
Rapid watch(conversationInstances, ...) triggers could fire refreshAllData() concurrently. Without a guard, a slow earlier call could resolve after a newer one, overwriting fresher state. Add an in-flight promise guard with a pending flag: concurrent calls coalesce onto the running promise, and if a call arrived while in-flight, one additional refresh runs after the current one completes.
The getPinnedConversations and getRecentConversations tests silently passed when the regex failed to match (the if-block was skipped). - Add expect(match).not.toBeNull() so a regex miss is a real failure - Apply comment-stripping in both tests for consistency
…subscriptions - Remove refreshAllData(), getConversations(), getUnprocessedItems(), refreshInFlight, refreshPending, and refreshTrigger from TimelineColumn - Replace with reactive watchEffect that maps conversationInstances directly from the existing useLiveQuery subscription - Add separate watchEffect for unprocessed items, triggered by conversationInstances changes - Extract sidebar refresh into a dedicated watch on conversation name - Extract AI task check into a dedicated watch on unprocessedItems - Remove onMounted — reactive watchEffect handles initial load - Remove refreshTrigger prop from TimelineBlock (and recursive children) - TimelineBlock now watches props.data instead of refreshTrigger - Add structural tests verifying removal of imperative patterns - All 20 structural tests pass, pnpm build succeeds
… proper unit tests - Remove jest, ts-jest, @types/jest from packages/api - Add vitest with config (globals, node environment) - Delete regex/structural test files that loaded source as strings: - community-service-scoping.test.ts - timeline-column-scoping.test.ts - channel-sparql-methods.test.ts - Rewrite ChannelSummary.test.ts: uses getModelMetadata() to verify no relations, expected properties, and flag metadata - Rewrite MessageSummary.test.ts: uses getModelMetadata() to verify no getter properties, expected relations, and flag metadata - Rewrite channel-query.test.ts: tests set-difference logic and three-query SPARQL integration pattern with mocked perspectives - Add channel.test.ts: tests Channel.recentConversations() and Channel.pinnedConversations() with mocked perspectives — verifies SPARQL query structure, result mapping, deduplication, error handling, and limit parameter - Existing parseLit.test.ts works unchanged (Vitest globals) All 38 tests pass across 5 test files.
The build script temporarily sets file: overrides for local linking. These should not be committed.
…ives
- Channel.recentConversations/pinnedConversations SPARQL queries used
string literal 'true' to match channel_is_conversation/channel_is_pinned,
but AD4M stores these as literal:json URIs. Use ad4m://fn/parse_literal
SPARQL function (consistent with buildSPARQLWhereFilters).
- markRaw(perspective) in useCommunityService to prevent Vue reactive Proxy
from wrapping PerspectiveProxy, which breaks TypeScript #private fields
(WeakMap lookup fails when 'this' is a Proxy instead of raw instance).
- Add { immediate: true } to sidebar watchers so data loads on first render
instead of waiting for a change event that may never fire.
- Null-safe signallingService for imported perspectives without a
neighbourhood (LinkLanguageFailedToInstall).
- Private perspective fallback: try private://UUID when neighbourhood://
URL lookup fails in createCommunityService.
- Temporarily skip Conversation.findOne cache population — hangs on
perspectives without a link language. Sidebar still renders with
channelId + lastActivity timestamps.
…terns
- Convert all GRAPH ?g { s p o } patterns to direct default-graph triples
- Use rdf:reifies <<( s p o )>> for link metadata (author, timestamp)
- Updated files: channel/index.ts, conversation/index.ts,
conversation-subgroup/index.ts, registerMobileNotifications.ts
- Add PREFIX rdf: where reifier patterns are used
- Companion to ad4m/core Oxigraph 0.5.7 upgrade
- restoreNeighbourhoodPrefix now passes through URLs that already contain :// - Router guard matches perspectives by raw UUID after stripping private:// prefix
…mplates - SidebarList: use item.channel?.id with channelId fallback for v-for key - SidebarItem: guard c.channel before accessing .id in nested channel check - CommunityView: optional chain on channelData.channel in channel card grid - aiStore: replace non-null assertions with optional chains on channel.id - routeUtils: guard restoreNeighbourhoodPrefix against undefined input
…on ordering Channel.allItems() and Channel.unprocessedItems() now query flux://transcript_started_at and use it (when present) instead of the link-creation timestamp. This ensures transcription messages sort by when speech occurred rather than when Whisper processing completed. Matches the existing pattern in ConversationSubgroup.itemsData(). Also consolidates channel test files into a single channel.test.ts.
# Conflicts: # pnpm-lock.yaml
… feat/sparql-1.2
…tibility
Root cause: ad4m/package.json uses 'overrides.core: { hoist: false }' format
which pnpm v10 rejects (requires string format). Netlify likely upgraded
to pnpm v10, breaking feat/sparql-1.2-cleanup branch builds.
Solution: Skip AD4M linking for deploy previews (pull/*/head), use
published npm packages instead. This matches how origin/dev works.
- Remove broken pipe issue (| tail -5 caused SIGPIPE exit code 1) - Simplify deploy preview logic: check for matching AD4M branch first - Pin pnpm to v9.15.0 for AD4M builds (v10 incompatible with object-format workspace overrides) - Add @coasys/hooks-helpers override when building AD4M from source - Restore package.json to dev baseline for deploy previews without matching AD4M branches - Build now completes successfully for feat/sparql-1.2-cleanup branch Fixes: Netlify build failures preventing deploy preview validation checks
- Always set all 5 @coasys package overrides when building AD4M from source - Removes unnecessary file existence check (if AD4M cloned, files exist) - Now matches the overrides in build.yaml and tests.yaml exactly: * @coasys/ad4m → file:./ad4m/core * @coasys/ad4m-connect → file:./ad4m/connect * @coasys/hooks-helpers → file:./ad4m/ad4m-hooks/helpers * @coasys/ad4m-react-hooks → file:./ad4m/ad4m-hooks/react * @coasys/ad4m-vue-hooks → file:./ad4m/ad4m-hooks/vue Ensures consistency between Netlify deploy script and GitHub Actions CI.
✅ Deploy Preview for fluxsocial-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughReplaces infer/prolog and global link listeners with SPARQL queries and targeted subscribeQuery subscriptions; adds ChannelSummary/MessageSummary models; rewrites Channel/Conversation/Subgroup queries; migrates tests to Vitest; updates CI to add lint/typecheck and local AD4M/hooks pnpm overrides; refactors services, stores, and many UI components to use new APIs and safe async/subscription patterns. ChangesComplete refactor to SPARQL-driven reactive architecture
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
jhweir
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
views/kanban-view/src/components/Board/Board.tsx (2)
186-188:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard
setTasksagainst an undefinedentries.
useLiveQuery'sdatacan beundefinedbefore the first result arrives (or after a query error). CallingsetTasks(undefined)then makestransformData'stasks.forEach(...)(Line 112) throwCannot read properties of undefined (reading 'forEach'). The state is initialized as[](Line 41), so preserve that contract.🛡️ Suggested fix
useEffect(() => { - setTasks(entries); + setTasks(entries ?? []); }, [JSON.stringify(entries), perspective.uuid]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@views/kanban-view/src/components/Board/Board.tsx` around lines 186 - 188, The useEffect that calls setTasks(entries) should guard against entries being undefined (useLiveQuery can return undefined); update the effect in Board.tsx so it only calls setTasks when entries !== undefined (or falls back to an empty array) to preserve the state contract initialized as [] and avoid transformData's tasks.forEach throwing; reference the useEffect that sets tasks, the entries variable from useLiveQuery, and the setTasks state updater when making the change.
168-180:⚠️ Potential issue | 🔴 Critical | 💤 Low valueUse correct method name
subjectClasses()instead oflistRegisteredClasses().The code calls
listRegisteredClasses()which doesn't exist in@coasys/ad4m0.11.1. The correct API issubjectClasses(), which returnsPromise<string[]>containing plain class names like'Task'.Additionally, since
ensureSDNASubjectClass()is idempotent (it checks for existing classes before registering), the outerifguard is redundant and can be removed. This also eliminates the issue whereclassesremains unpopulated when the condition is false.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@views/kanban-view/src/components/Board/Board.tsx` around lines 168 - 180, The effect currently calls a non-existent method listRegisteredClasses() and conditionally registers the Task class which leaves classes unset when the guard prevents registration; update the effect to call perspective.subjectClasses() (which returns Promise<string[]>) or, per the comment, simply remove the outer if and always call perspective.ensureSDNASubjectClass(Task). After ensureSDNASubjectClass(Task) resolves, call getClasses(perspective).then(setClasses) so classes are populated; keep the rest of the effect (including checkNeighbourhoodAuthor() and the dependency on perspective.uuid) unchanged and reference useEffect, checkNeighbourhoodAuthor, perspective.subjectClasses(), perspective.ensureSDNASubjectClass(Task), getClasses, and setClasses when making the change.views/poll-view/src/components/PollCard/PollCard.tsx (1)
110-112:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix useEffect dependency to trigger when votes change.
The current dependency
answers.map((a) => a.id).join(',')only changes when answer IDs change (added/removed/reordered). It does NOT trigger when votes within existing answers change.Problem flow:
- User votes →
vote()persists the change and callsbuildAnswerData()at line 107 with potentially stale subscription data- Subscription updates with fresh votes → component re-renders
- useEffect sees identical answer IDs → doesn't run →
processedAnswersremains stale- UI shows incorrect vote counts despite subscription having fresh data
This breaks vote count updates after voting.
🔧 Proposed fix
useEffect(() => { buildAnswerData(); - }, [answers.map((a) => a.id).join(',')]); + }, [answers]);This will correctly trigger whenever the
answersarray updates (including vote changes), ensuring vote counts stay in sync with the subscription data.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@views/poll-view/src/components/PollCard/PollCard.tsx` around lines 110 - 112, The useEffect dependency currently only watches answer IDs so processedAnswers isn't rebuilt when vote counts change; update the useEffect dependency to watch the entire answers array (e.g., change the dependency from answers.map(a => a.id).join(',') to simply answers) so that useEffect(() => { buildAnswerData(); }, [answers]) re-runs whenever answers (including votes) update; ensure this touches the useEffect surrounding buildAnswerData and references the answers and processedAnswers variables so vote-driven subscription updates correctly refresh the UI.views/table-view/src/components/Header/Header.tsx (1)
35-77:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
onUpdate(andproperties) are unreferenced in the rendered JSX — likely leftover from the refactor.The new
onUpdatecallback (Lines 35–53) and thepropertiescomputed at Line 56 are not wired to anything in the returned JSX: the component now only renders the header title, theCurrent Entrylabel, and the expand button. None of them reachDisplayValueor any other element that callsonUpdate. Either the JSX is missing the property list / editable display that was intended to use this update path, or this code is dead and should be removed.Also,
entryis initialized viauseState({}), soif (entry)is always truthy and the finalreturn <span>{source}</span>;(Line 82) is unreachable.Same async-staleness caveat applies as in
Entry.tsx/History.tsx: acancelledguard in the effect and after the update's refresh would prevent stalesetEntrycalls whensourcechanges mid-flight.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@views/table-view/src/components/Header/Header.tsx` around lines 35 - 77, The onUpdate function and the properties variable are dead code after the refactor: onUpdate (async function onUpdate) and properties (computed from entry) are never used in the returned JSX, and entry is initialized as useState({}) making the `if (entry)` check always truthy so the fallback return is unreachable. Fix by either reintroducing the intended property list / editable DisplayValue components that consume properties and call onUpdate (e.g., render DisplayValue rows with onclick/update handlers) or remove onUpdate and properties entirely if editing was intentionally removed; also change entry initialization to null (useState(null)) and add a cancelled guard in the effect and after the async refresh (where setEntry is called, e.g., after perspective.getSubjectData and in onUpdate) to avoid stale setState when source changes mid-flight. Ensure you update or remove references to setEntry/getSubjectData/getClassShape to keep semantics consistent.views/table-view/src/components/TableView/TableView.tsx (1)
250-259:⚠️ Potential issue | 🟠 Major | ⚡ Quick winChange predicate to
'ad4m://has_child'to match the codebase convention.The PR objective calls for explicit
through: 'ad4m://has_child'for HasMany relations. Throughout the codebase, all parent–child links use the fully-qualified'ad4m://has_child'predicate. HerecreateEntryuses a bare'has_child'string, which is inconsistent with every other link creation in the repo and with the SHACL/HasMany relation definitions. Entries created here will not query correctly against the standardized predicate.Diff
await perspective.add({ source: source || 'ad4m://self', - predicate: 'has_child', + predicate: 'ad4m://has_child', target: uuid, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@views/table-view/src/components/TableView/TableView.tsx` around lines 250 - 259, The createEntry function is using the bare predicate 'has_child' when adding the parent→child link; update the perspective.add call inside createEntry to use the fully-qualified predicate 'ad4m://has_child' (matching the repo convention and SHACL/HasMany relation definitions) so entries will be queryable; ensure the call in createEntry({ perspective, subjectClass, source }) replaces predicate: 'has_child' with predicate: 'ad4m://has_child' (no other logic changes).views/table-view/src/components/History/History.tsx (1)
41-57:⚠️ Potential issue | 🟠 Major | ⚡ Quick winVerify the new AD4M API contract and fix race condition in async state updates.
API verification:
perspective.getInstanceClasses(source)andperspective.getSubjectData(className, source)are both available in@coasys/ad4m0.13.0. They returnPromise<string[]>andPromise<T>respectively, matching the code's assumptions.Race condition—requires fixing: If
sourceorperspective.uuidchanges faster thangetSubjectDataresolves, an older async response can overwrite newer state, displaying the wrong entry. No cancellation guard exists. This pattern is repeated identically in Header.tsx and Entry.tsx.Implement a cancellation flag or AbortController in the effect cleanup:
♻️ Suggested staleness guard
useEffect(() => { - fetchSourceClasses(source); + let cancelled = false; + (async () => { + const classResults = await perspective.getInstanceClasses(source); + if (cancelled) return; + if (classResults?.length > 0) { + const className = classResults[0]; + const data = await perspective.getSubjectData(className, source); + if (cancelled) return; + setClasses(classResults); + setEntry({ id: source, ...data }); + } else { + setClasses([]); + setEntry({ id: source }); + } + })(); + return () => { cancelled = true; }; }, [source, perspective.uuid]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@views/table-view/src/components/History/History.tsx` around lines 41 - 57, The effect calling fetchSourceClasses (which calls perspective.getInstanceClasses and perspective.getSubjectData) can suffer a race where an earlier async response overwrites newer state; update the useEffect/fetchSourceClasses pattern to use a staleness guard (e.g., a local cancelled flag or AbortController) that is set in the effect cleanup and checked before calling setClasses and setEntry, and apply the same fix to the identical patterns in Header.tsx and Entry.tsx; keep the same API calls but ensure you only set state when the request is not cancelled and the current perspective.uuid/source still match.app/src/composables/useCommunityService.ts (2)
41-50:⚠️ Potential issue | 🟠 Major | ⚡ Quick winType contract violation when
signallingServiceisnull.The interface declares
agentsInChannel: AgentData[]andagentsInCall: AgentData[]as non-optional arrays, but withsignallingService?.getAgentsInChannel(...)?.value, both fields areundefinedwhensignallingServiceisnull(the newprivate://perspective path). Consumers iterating these (e.g., sidebar/agent badges) will hitundefined.length/undefined.map.Either widen the type to
AgentData[] | undefinedand update consumers, or provide a stable[]fallback here.🛡️ Proposed fix
pinnedConversationsWithAgents = computed((): ChannelDataWithAgents[] => { return pinnedConversations.value.map((data) => ({ ...data, channel: allChannels.value.find((c) => c.id === data.channelId), conversation: data.conversationId ? conversationCache.get(data.conversationId) : undefined, - agentsInChannel: signallingService?.getAgentsInChannel(data.channelId).value, - agentsInCall: signallingService?.getAgentsInCall(data.channelId).value, + agentsInChannel: signallingService?.getAgentsInChannel(data.channelId).value ?? [], + agentsInCall: signallingService?.getAgentsInCall(data.channelId).value ?? [], children: undefined, })); });(Apply the same
?? []fallback at lines 182-183, 192-193, and 199-200.)Also applies to: 167-204
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/composables/useCommunityService.ts` around lines 41 - 50, The ChannelDataWithAgents interface currently requires agentsInChannel and agentsInCall but the code that builds ChannelDataWithAgents uses signallingService?.getAgentsInChannel(...)?.value (and similar calls) which yields undefined when signallingService is null; update the builder to provide a stable empty-array fallback (use the nullish-coalescing fallback like ?? []) for each place that assigns agentsInChannel and agentsInCall (e.g., the expressions using signallingService?.getAgentsInChannel(...).value and signallingService?.getAgentsInCall(...).value) so the constructed ChannelDataWithAgents always has AgentData[] values, or alternatively change the interface fields to AgentData[] | undefined and update consumers—prefer adding the ?? [] fallback to keep the interface unchanged.
476-512:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSync state listener registered but never removed in cleanup().
perspective.addSyncStateChangeListener(syncStateListener)(line 482) is added during service creation, but thecleanup()function (lines 509-512) only removes thelink-addedlistener and does not unsubscribe from sync state changes. Even thoughcleanup()is called on component unmount, the sync state callback will continue to fire against a staleisSyncedref, causing memory leaks and ref mutations.The proposed fix assumes
removeSyncStateChangeListener()exists, but a codebase search found no such API. Verify with the@coasys/ad4mlibrary documentation (v0.11.1) whether this removal method is available, or consider alternative cleanup approaches if the API does not exist.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/composables/useCommunityService.ts` around lines 476 - 512, The sync state listener added via perspective.addSyncStateChangeListener(syncStateListener) is never removed in cleanup(), causing stale updates to isSynced; locate the syncStateListener registration and either call the corresponding removal API (e.g., perspective.removeSyncStateChangeListener(syncStateListener) or an unsubscribe function returned by addSyncStateChangeListener) inside cleanup(), or—if the `@coasys/ad4m` v0.11.1 API lacks a removal method—replace the raw listener with a guarded handler that becomes a no-op on teardown (e.g., set a local "mounted" flag false in cleanup() and have syncStateListener return immediately when mounted is false) so that isSynced is not mutated after cleanup. Ensure changes touch the syncStateListener, addSyncStateChangeListener call, and cleanup() so the listener is properly unsubscribed or neutralized.scripts/build-with-ad4m-link.sh (1)
42-44:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
A && B || Cswallows build failures.The pattern
[ -f X ] && (cd ... && pnpm exec tsc) || echo "Skipping ..."is not equivalent to if/then/else: when the file exists butpnpm exec tscfails, the||branch runs and the script proceeds as if hooks built successfully. Combined withset -e, this also unexpectedly suppresses theset -epropagation inside the parentheses. If hooks fail to compile, the deploy preview will silently link against stale/missing builds.🛡️ Proposed fix using explicit `if`
- [ -f ad4m-hooks/helpers/tsconfig.json ] && (cd ad4m-hooks/helpers && pnpm exec tsc) || echo "Skipping ad4m-hooks/helpers" - [ -f ad4m-hooks/react/tsconfig.json ] && (cd ad4m-hooks/react && pnpm exec tsc) || echo "Skipping ad4m-hooks/react" - [ -f ad4m-hooks/vue/tsconfig.json ] && (cd ad4m-hooks/vue && pnpm exec tsc) || echo "Skipping ad4m-hooks/vue" + if [ -f ad4m-hooks/helpers/tsconfig.json ]; then (cd ad4m-hooks/helpers && pnpm exec tsc); else echo "Skipping ad4m-hooks/helpers"; fi + if [ -f ad4m-hooks/react/tsconfig.json ]; then (cd ad4m-hooks/react && pnpm exec tsc); else echo "Skipping ad4m-hooks/react"; fi + if [ -f ad4m-hooks/vue/tsconfig.json ]; then (cd ad4m-hooks/vue && pnpm exec tsc); else echo "Skipping ad4m-hooks/vue"; fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/build-with-ad4m-link.sh` around lines 42 - 44, The three one-liners using the pattern "[ -f X ] && (cd ... && pnpm exec tsc) || echo 'Skipping ...'" incorrectly treat a failing "pnpm exec tsc" as success and run the echo branch; replace each with an explicit if block that checks -f, then runs the subshell "cd ... && pnpm exec tsc" and exits non-zero (or re-throw) if tsc fails, else echo the "Skipping ad4m-hooks/..." message; ensure you do not wrap the tsc call in a construct that swallows errors (avoid the "A && B || C" pattern) so that set -e will propagate failures from the subshell.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/package.json`:
- Line 19: Confirm that the "typecheck" script which uses vue-tsc@^3.2.7 is
actually compatible with the resolved TypeScript version in pnpm-lock.yaml:
inspect pnpm-lock.yaml for the resolved vue-tsc and typescript versions (vue-tsc
should be 3.2.7 range and typescript should satisfy >=5.0.0, e.g., ^5.0.4), run
the typecheck script locally to reproduce any issues, and if you find a mismatch
or known minor-version edge case pin or bump the devDependency (typescript or
vue-tsc) in package.json to a compatible version or add an explicit
devDependency entry to avoid an incompatible resolved version.
In `@app/src/components/conversation/timeline/TimelineColumn.vue`:
- Around line 203-219: The TimelineColumn component currently sets up a
long-lived SPARQL subscription using the captured channelUrl inside an IIFE (see
the subscribeQuery block and channelItemsSub), which won’t reset when Vue Router
reuses the component for a different channel; fix by either adding a route-based
key on the parent (e.g., set :key="route.params.channelId" where Conversation
renders TimelineColumn) or by adding a watcher inside TimelineColumn for
route.params.channelId that disposes channelItemsSub, clears any related
watchers (AI/sidebar watchers, unprocessed items logic), and re-runs the
subscription setup (the IIFE logic) when the channel changes; also sanitize or
assert channelUrl before interpolating into the SPARQL string (or use
restoreChannelPrefix consistently) to avoid injecting malformed IRIs into
subscribeQuery.
In `@app/src/views/main/modals/CreateCommunityModal.vue`:
- Around line 210-211: The key used to check membership is built incorrectly:
change the key derivation used by perspectiveIsCommunity to match the
myCommunities store format (which uses perspective.sharedUrl ||
`private://${perspective.uuid}` in appStore.getMyCommunities()). Update the code
that defines key (used by perspectiveIsCommunity) to use perspective.sharedUrl
|| `private://${perspective.uuid}` so perspectives without a sharedUrl will
correctly match entries in myCommunities.value.
In `@packages/api/src/channel/index.ts`:
- Around line 167-168: The interpolation of unprocessedIds into valuesClause
(`unprocessedIds.map(id => `<${id}>`)`) is unsafe; validate or escape each id
before building the SPARQL VALUES clause (e.g., reject or log and skip ids
containing '>', whitespace, or other IRI-breaking characters) to avoid
constructing a malformed query that causes unprocessedItems() to fail silently;
update the code that produces valuesClause to validate each id against a safe
pattern (or explicitly escape/URN-encode it), and ensure failures are handled
deterministically (throw or return a clear error/log) rather than falling back
to an empty array.
In `@packages/api/src/conversation/conversation.test.ts`:
- Around line 15-17: Update the stale comment above the vi.mock('@coasys/ad4m',
...) block: either change the hardcoded version string
`@coasys/ad4m@0.13.0-test-2` to `0.13.0-test-4` or replace it with a generalized
note like "published 0.13.0 test releases" and add a short TODO to verify
exports; then check whether the current published package actually exports
Model/HasMany and if so remove the mock layer in the vi.mock('@coasys/ad4m',
...) setup to avoid unnecessary no-op decorators.
In `@packages/flux-container/package.json`:
- Around line 22-23: The package.json for `@coasys/flux-container` currently pins
transitive dependencies "@coasys/ad4m" and "@coasys/ad4m-connect" to the
prerelease "0.13.0-test-4"; create a tracking issue to restore these to the
stable GA tag (e.g., "0.13.0" or the eventual release tag) before publishing a
non-preview release of flux-container, and update the package.json entries for
"@coasys/ad4m" and "@coasys/ad4m-connect" to use the stable version (or a semver
range) at release time to avoid leaking test prerelease pins to downstream
consumers.
In `@scripts/build-with-ad4m-link.sh`:
- Line 5: The BRANCH assignment can fail under set -u if none of BRANCH, HEAD,
GITHUB_HEAD_REF, or GITHUB_REF are set; update the assignment so each fallback
uses a safe default before the # operator and the overall default is an empty
string. Specifically, change the BRANCH assignment that references BRANCH, HEAD,
GITHUB_HEAD_REF and the parameter expansion on GITHUB_REF so each symbol is
expanded with a safe default (e.g., use the ":-" or "-" style fallback for HEAD,
GITHUB_HEAD_REF and GITHUB_REF before applying the `#refs/heads/` substring
operation) and ensure the final BRANCH default is empty to avoid unbound
variable errors when running with set -u.
In `@views/graph-view/src/components/CommunityGraph.tsx`:
- Around line 105-122: The effect can leak subscriptions because
sourceSub/targetSub are assigned only inside subscribeQuery.then callbacks; add
a local boolean flag (e.g., cancelled = false) at the top of the useEffect and
in the cleanup set cancelled = true and dispose any already-assigned
subscriptions, and in each subscribeQuery.then handler check the flag: if
cancelled then immediately dispose the newly-arrived subscription
(sub.dispose()) and return; otherwise assign to sourceSub/targetSub and wire
sub.onResult(() => refresh()). Also remove the unnecessary optional chaining on
perspective (or early-return if perspective is falsy) so you're not calling
perspective?.subscribeQuery with a non-nullable PerspectiveProxy.
In `@views/kanban-view/src/components/Entry/Entry.tsx`:
- Around line 15-19: The useEffect calling
perspective.getNamedOptions(selectedClass) can resolve after selectedClass
changes or component unmounts, causing stale setNamedOptions calls and it also
lacks error handling; update the effect to create a cancellation flag (e.g., let
cancelled = false) and in the cleanup set cancelled = true, then after the
promise resolves only call setNamedOptions if not cancelled (and optionally
clear named options on cancel), and add a .catch handler to the promise to
handle/log errors instead of letting rejections be silent; reference the
existing useEffect, perspective.getNamedOptions, setNamedOptions, selectedClass
and perspective.uuid when applying this change.
In `@views/kanban-view/src/hooks/useAssociations.tsx`:
- Around line 33-43: The effect in useAssociations builds a SPARQL query using
predicate but the dependency array only tracks perspective.uuid and source, so
changes to predicate won't resubscribe; update the effect's dependency array to
include predicate (i.e., [perspective.uuid, source, predicate]) so
perspective.subscribeQuery(spql) is re-run when predicate changes and the
previous subscription (sub) is disposed and replaced correctly.
- Around line 35-42: The async subscription created via
perspective.subscribeQuery must be guarded so we don't register callbacks or
leak handles after unmount: when calling perspective.subscribeQuery(sparql)
capture the returned promise and track a local "cancelled" (or "mounted") flag;
after the promise resolves, check the flag before assigning sub or calling
handle.onResult(() => fetchLinks()); if cancelled, call handle.dispose()
immediately instead of attaching listeners; also add a .catch(...) to the
promise to log/handle subscription errors. Ensure the cleanup function sets the
cancelled flag and disposes sub if already assigned (sub?.dispose()).
In `@views/poll-view/src/components/PollCard/PollCard.tsx`:
- Around line 31-34: The project is using the new include query syntax with
useLiveQuery(Answer, perspective, { parent: { model: Poll, id: poll.id }, query:
{ include: { votes: true } } }) but views/poll-view/package.json is pinned to
`@coasys/ad4m-react-hooks` v0.11.1 which lacks that syntax; update
views/poll-view/package.json to use `@coasys/ad4m-react-hooks` version
0.13.0-test-4 so the include option works, and modify the render code that
currently calls Vote.findAll() (referenced as Vote.findAll) to instead use the
eager-loaded votes from the subscription (access votes on the Answer objects
returned by useLiveQuery, e.g., answers -> answer.votes) so you reuse the loaded
data rather than refetching.
In `@views/table-view/src/components/NewClass/NewClass.tsx`:
- Around line 40-44: In addClass(), validate and sanitize inputs before calling
buildSHACLShape/perspective.addShacl: ensure the class name (variable name) is
non-empty and URL/IRI-safe (no spaces, '#' or '/'; escape or reject invalid
chars), ensure every column in columns (added via addNewColumn) has non-empty
name and predicate, and enforce uniqueness of column predicates (reject or mark
duplicates), then surface a clear validation error to the user (do not call
perspective.addShacl or onSaved until validation passes); run these checks
immediately in addClass and only call buildSHACLShape(passedName, passedColumns,
perspective) and perspective.addShacl when all validations succeed.
- Around line 232-233: The code builds an IRI using the raw user-provided name
(const ns = `recipe://${name}`; new SHACLShape(`${ns}Shape`, `${ns}#${name}`))
which can produce invalid IRIs; sanitize/percent-encode the name before
embedding it (or validate/restrict to a safe char class) and use the encoded
value when constructing ns and the shape IRI (i.e., apply the same encoding
helper to name for both the namespace and the `${ns}#${name}` target so both
IRIs are valid).
- Around line 227-276: The SHACL property datatype is using a non-standard URI
'xsd://string' inside buildSHACLShape; change the propShape.datatype assignment
to use the CURIE 'xsd:string' (or full XSD URI) so the SHACL parser accepts it;
update the line that sets datatype in the propShape object (where propShape is
constructed in buildSHACLShape) to 'xsd:string', keep the rest of the propShape
construction, addProperty calls, and constructorActions logic unchanged, and
then re-run tests to confirm parser accepts the shape.
In `@views/table-view/src/components/TableView/TableView.tsx`:
- Around line 61-66: When perspective.listRegisteredClasses() yields an empty
array the component sets selected to '' but the header "+" button still renders
and can call createEntry({ subjectClass: '' }); update the header button
rendering logic in the TableView component to guard against no selection: check
classes.length and selected and either disable the "+" button when !selected or
hide the button entirely when classes.length === 0 so createEntry is never
invoked with an empty subjectClass; locate the UI that renders the "+" button
(references: selected, classes, createEntry, createSubject) and add the
conditional rendering/disabled-state there.
---
Outside diff comments:
In `@app/src/composables/useCommunityService.ts`:
- Around line 41-50: The ChannelDataWithAgents interface currently requires
agentsInChannel and agentsInCall but the code that builds ChannelDataWithAgents
uses signallingService?.getAgentsInChannel(...)?.value (and similar calls) which
yields undefined when signallingService is null; update the builder to provide a
stable empty-array fallback (use the nullish-coalescing fallback like ?? []) for
each place that assigns agentsInChannel and agentsInCall (e.g., the expressions
using signallingService?.getAgentsInChannel(...).value and
signallingService?.getAgentsInCall(...).value) so the constructed
ChannelDataWithAgents always has AgentData[] values, or alternatively change the
interface fields to AgentData[] | undefined and update consumers—prefer adding
the ?? [] fallback to keep the interface unchanged.
- Around line 476-512: The sync state listener added via
perspective.addSyncStateChangeListener(syncStateListener) is never removed in
cleanup(), causing stale updates to isSynced; locate the syncStateListener
registration and either call the corresponding removal API (e.g.,
perspective.removeSyncStateChangeListener(syncStateListener) or an unsubscribe
function returned by addSyncStateChangeListener) inside cleanup(), or—if the
`@coasys/ad4m` v0.11.1 API lacks a removal method—replace the raw listener with a
guarded handler that becomes a no-op on teardown (e.g., set a local "mounted"
flag false in cleanup() and have syncStateListener return immediately when
mounted is false) so that isSynced is not mutated after cleanup. Ensure changes
touch the syncStateListener, addSyncStateChangeListener call, and cleanup() so
the listener is properly unsubscribed or neutralized.
In `@scripts/build-with-ad4m-link.sh`:
- Around line 42-44: The three one-liners using the pattern "[ -f X ] && (cd ...
&& pnpm exec tsc) || echo 'Skipping ...'" incorrectly treat a failing "pnpm exec
tsc" as success and run the echo branch; replace each with an explicit if block
that checks -f, then runs the subshell "cd ... && pnpm exec tsc" and exits
non-zero (or re-throw) if tsc fails, else echo the "Skipping ad4m-hooks/..."
message; ensure you do not wrap the tsc call in a construct that swallows errors
(avoid the "A && B || C" pattern) so that set -e will propagate failures from
the subshell.
In `@views/kanban-view/src/components/Board/Board.tsx`:
- Around line 186-188: The useEffect that calls setTasks(entries) should guard
against entries being undefined (useLiveQuery can return undefined); update the
effect in Board.tsx so it only calls setTasks when entries !== undefined (or
falls back to an empty array) to preserve the state contract initialized as []
and avoid transformData's tasks.forEach throwing; reference the useEffect that
sets tasks, the entries variable from useLiveQuery, and the setTasks state
updater when making the change.
- Around line 168-180: The effect currently calls a non-existent method
listRegisteredClasses() and conditionally registers the Task class which leaves
classes unset when the guard prevents registration; update the effect to call
perspective.subjectClasses() (which returns Promise<string[]>) or, per the
comment, simply remove the outer if and always call
perspective.ensureSDNASubjectClass(Task). After ensureSDNASubjectClass(Task)
resolves, call getClasses(perspective).then(setClasses) so classes are
populated; keep the rest of the effect (including checkNeighbourhoodAuthor() and
the dependency on perspective.uuid) unchanged and reference useEffect,
checkNeighbourhoodAuthor, perspective.subjectClasses(),
perspective.ensureSDNASubjectClass(Task), getClasses, and setClasses when making
the change.
In `@views/poll-view/src/components/PollCard/PollCard.tsx`:
- Around line 110-112: The useEffect dependency currently only watches answer
IDs so processedAnswers isn't rebuilt when vote counts change; update the
useEffect dependency to watch the entire answers array (e.g., change the
dependency from answers.map(a => a.id).join(',') to simply answers) so that
useEffect(() => { buildAnswerData(); }, [answers]) re-runs whenever answers
(including votes) update; ensure this touches the useEffect surrounding
buildAnswerData and references the answers and processedAnswers variables so
vote-driven subscription updates correctly refresh the UI.
In `@views/table-view/src/components/Header/Header.tsx`:
- Around line 35-77: The onUpdate function and the properties variable are dead
code after the refactor: onUpdate (async function onUpdate) and properties
(computed from entry) are never used in the returned JSX, and entry is
initialized as useState({}) making the `if (entry)` check always truthy so the
fallback return is unreachable. Fix by either reintroducing the intended
property list / editable DisplayValue components that consume properties and
call onUpdate (e.g., render DisplayValue rows with onclick/update handlers) or
remove onUpdate and properties entirely if editing was intentionally removed;
also change entry initialization to null (useState(null)) and add a cancelled
guard in the effect and after the async refresh (where setEntry is called, e.g.,
after perspective.getSubjectData and in onUpdate) to avoid stale setState when
source changes mid-flight. Ensure you update or remove references to
setEntry/getSubjectData/getClassShape to keep semantics consistent.
In `@views/table-view/src/components/History/History.tsx`:
- Around line 41-57: The effect calling fetchSourceClasses (which calls
perspective.getInstanceClasses and perspective.getSubjectData) can suffer a race
where an earlier async response overwrites newer state; update the
useEffect/fetchSourceClasses pattern to use a staleness guard (e.g., a local
cancelled flag or AbortController) that is set in the effect cleanup and checked
before calling setClasses and setEntry, and apply the same fix to the identical
patterns in Header.tsx and Entry.tsx; keep the same API calls but ensure you
only set state when the request is not cancelled and the current
perspective.uuid/source still match.
In `@views/table-view/src/components/TableView/TableView.tsx`:
- Around line 250-259: The createEntry function is using the bare predicate
'has_child' when adding the parent→child link; update the perspective.add call
inside createEntry to use the fully-qualified predicate 'ad4m://has_child'
(matching the repo convention and SHACL/HasMany relation definitions) so entries
will be queryable; ensure the call in createEntry({ perspective, subjectClass,
source }) replaces predicate: 'has_child' with predicate: 'ad4m://has_child' (no
other logic changes).
---
Nitpick comments:
In `@app/.eslintrc.js`:
- Line 6: The project removed the Prettier preset from the ESLint extends array
(see the extends entry in .eslintrc.js) but left Prettier-related packages
installed; update package.json by removing `@vue/eslint-config-prettier`,
eslint-config-prettier, and eslint-plugin-prettier from
dependencies/devDependencies so they no longer install, and run pnpm install to
update the lockfile; ensure no other config files reference these plugins
(search for "prettier" and the specific package names) before committing.
In `@app/package.json`:
- Around line 18-21: The package.json "test" script still invokes Jest ("test":
"jest --env=jsdom"); update it to use Vitest to match the migration in
packages/api by replacing the script value with a Vitest invocation (e.g.,
"test": "vitest" and include the appropriate environment flag such as
--environment=jsdom if browser-like DOM is needed) and then open a follow-up
task to remove Jest-related devDependencies (`@types/jest`, jest, ts-jest,
babel-jest, jest-transform-stub) once all app tests are converted; target the
"test" script entry in package.json when making this change.
In
`@app/src/components/conversation/timeline/__tests__/timeline-subscription.test.ts`:
- Around line 83-189: Switch the tests to fake timers to make debounce waits
deterministic: call vi.useFakeTimers() before running debounce-related code
(e.g., in the describe's beforeEach or at the start of individual it blocks),
replace all new Promise(r => setTimeout(r, N)) waits with await
vi.advanceTimersByTimeAsync(N) (or await
vi.runAllTimersAsync()/advanceTimersByTimeAsync where appropriate), and restore
real timers with vi.useRealTimers() in afterEach; update tests referencing
createDebouncedRefreshHandler and handler.scheduleRefresh() to use these
fake-timer helpers so the debounce window is driven deterministically.
In `@app/src/components/conversation/timeline/TimelineColumn.vue`:
- Around line 355-362: The comment above the watchEffect is stale (refers to the
removed "link-added listener") and the watchEffect uses a non-obvious reactive
dependency hack; update the comment to mention that unprocessed items are
re-fetched when the SPARQL subscribeQuery block triggers conversation updates
and simplify the reactive registration by replacing the watchEffect(async () =>
{ const _ = conversationInstances.value; await refreshUnprocessedItems(); })
with an explicit watch on conversationInstances that calls
refreshUnprocessedItems (i.e., use watch(conversationInstances,
refreshUnprocessedItems, { immediate: true }) or equivalent) and remove the
outdated "link-added listener" text; keep references to subscribeQuery,
conversationInstances, watchEffect/watch, and refreshUnprocessedItems to find
the code to modify.
In `@app/src/composables/useCommunityService.ts`:
- Around line 524-530: The watcher on recentConversations needs to trigger when
AI becomes enabled after load; update the watch so it either (A) uses {
immediate: true } on recentConversations or (B) watches both recentConversations
and aiEnabled together (e.g., watch([recentConversations, aiEnabled], ...)) so
that when aiEnabled flips to true the handler runs; inside the handler keep the
existing guard using processingStateChecked.value and call
aiStore.findProcessingTasksInCommunity(perspective.sharedUrl || '') after the
HEARTBEAT_INTERVAL as before to preserve behavior.
In `@app/src/router/index.ts`:
- Around line 96-97: The membership check is redundant: remove the separate
p.uuid === communityId test and only compare against the normalized id (rawId)
that strips the "private://" prefix; update the isMember expression (which uses
appStore.myPerspectives, neighbourhoodUrl, communityId, rawId) to use p.uuid ===
rawId (and keep the neighbourhoodUrl check) so a single normalized comparison
covers both prefixed and unprefixed communityId cases.
In `@app/src/stores/aiStore.ts`:
- Around line 259-263: The code incorrectly casts
processingQueue.value[0].channel to Channel using toRaw(channel) as Channel
despite the ProcessingQueueItem typing the field as ChannelSummary; stop lying
to the type system and either treat it as ChannelSummary or just read the id
directly. Replace the cast with a ChannelSummary-typed value (or avoid toRaw
entirely) and use that to access rawChannel.id when calling
communityServiceStore.getCommunityService(...).getConversation(rawChannel.id!)
and getParentChannel(rawChannel.id!), ensuring the variable is declared as
ChannelSummary (or simply const id = processingQueue.value[0].channel.id) so no
Channel-only properties are assumed.
- Line 273: The verbose console.log in aiStore.ts (the line with console.log('🤖
Checking for unprocessed items in channel:', rawChannel.id)) spams logs on every
dequeued task; change it to a lower-severity debug call or gate it behind a
runtime/dev flag. Locate the log inside the AI processing function in the
aiStore (where rawChannel is used) and either replace console.log with
console.debug or wrap the console call in a check (e.g., if
(process.env.DEBUG_AI || isDev) ...) so the message only appears when debugging
is enabled. Ensure the new approach uses the same message and rawChannel.id for
context.
In `@app/src/stores/appStore.ts`:
- Line 105: Document the fallback community ID convention used when computing
key (const key = perspective.sharedUrl || `private://${perspective.uuid}`): add
a JSDoc comment near the key derivation (and/or extract a named constant like
PRIVATE_COMMUNITY_PREFIX or make a helper getCommunityKey(perspective)) that
explains the "private://{uuid}" format and intended semantics, then update
usages (references to perspective.sharedUrl, perspective.uuid, and
router/CreateCommunityModal usage) to reference the constant/helper so the
convention is explicit and consistent across the codebase.
In `@app/src/utils/routeUtils.ts`:
- Around line 7-21: restoreNeighbourhoodPrefix uses communityId.includes('://')
which can false-positive on embedded strings; change the check in
restoreNeighbourhoodPrefix to only treat a leading scheme as already-prefixed
(e.g. test communityId against a scheme-at-start regex like
/^[a-z][a-z0-9+\-.]*:\/\// or use startsWith checks for known schemes) so that
only true protocol-prefixed IDs are returned unchanged; also ensure
stripNeighbourhoodPrefix (and any callers) keep handling both 'neighbourhood://'
and 'private://' as before.
In `@app/src/views/main/community/channel/modals/ChangeChannelModal.vue`:
- Around line 39-58: The watcher currently observes only channel, but its
callback also depends on perspective (used in the guard and to construct new
Channel), so change the watch to observe both channel and perspective (e.g.,
watch([channel, perspective], ...)) so the callback re-runs when perspective
becomes available; keep the existing logic using channelLoadSeq, the
Channel(perspective, newChannel.id) construction, fullChannel.get({ views: true
}), and assignments to views.value and error handling intact, and preserve the {
immediate: true } option.
In `@app/src/views/main/community/channel/modals/ManageChannelPluginsModal.vue`:
- Around line 125-149: The watcher currently only watches channel but also reads
perspective, so when perspective initializes later the handler won't run; update
the watch call that wraps channel (the watch(...) block) to watch both channel
and perspective (e.g., watch([channel, perspective], ...)) and adjust the
handler to accept the new source tuple (or read current values inside) so the
guard using perspective and the logic that constructs new Channel, calls
fullChannel.get, and updates views, selectedPlugins and viewsLoadSeq behaves
correctly; ensure seq checks and the existing try/catch logic remain intact.
In `@app/src/views/main/community/sidebar/SidebarItem.vue`:
- Line 131: The callback for item.children in the inNestedChannel computation
currently uses (c: any); replace the any with the proper ChannelDataWithAgents
type (or remove explicit typing so TypeScript infers it from item.children) so
that the predicate becomes typed correctly; update the reference in the
inNestedChannel expression (which calls stripChannelPrefix(c.channel.id) ===
currentChannelId) to use (c: ChannelDataWithAgents) to restore type-safety for
item.children and its channel property.
In `@packages/api/package.json`:
- Around line 8-33: The package.json currently uses a broad vitest range
("devDependencies" entry "vitest": "^2"); narrow this to a specific minor to
reduce cross-machine drift (e.g., change the "vitest" devDependency to "^2.1" or
another chosen minor), keeping the existing scripts ("test" and "test:watch")
and config intact; update the "devDependencies" vitest entry accordingly so the
workspace consistently installs the pinned minor version.
In `@packages/api/src/channel/channel.test.ts`:
- Around line 298-322: Update the misleading block comment above the helper
runUnprocessedItems used in the Channel.unprocessedItems() test to reflect that
it only simulates the set-difference portion (two-query portion) rather than the
full three-query pattern; locate the comment immediately above the
runUnprocessedItems function and replace "the three-query pattern from
Channel.unprocessedItems()" with wording like "the set-difference portion of the
three-query pattern from Channel.unprocessedItems()" so the test's intent and
the assertion that querySparql is called twice are unambiguous.
In `@packages/api/src/channel/index.ts`:
- Around line 278-282: The OPTIONAL reifier block allows ?_parent to match any
previous parent, causing multiple ?chanCreatedTs per channel; restrict it by
scoping ?_parent to the current/community parent or collapse multiple timestamps
inside the subquery. Update the OPTIONAL block around ?_chanReifier rdf:reifies
<<( ?_parent <flux://has_channel> ?channelId )>> to either add a FILTER that
ties ?_parent to the known community parent variable used elsewhere or replace
the unconstrained timestamp selection with an aggregation (e.g., SELECT ...
(MAX(?chanCreatedTs) AS ?chanCreatedTs) or SAMPLE-like isolation) or otherwise
limit to a single chosen ?chanCreatedTs (LIMIT 1) so the outer COALESCE and
final MAX(?ts) no longer receive duplicated rows.
In `@packages/api/src/conversation/conversation.test.ts`:
- Around line 622-624: The test uses vi.restoreAllMocks() in the beforeEach
which removes hoisted vi.mock implementations and forces manual re-mocking of
ensureLLMTasks, LLMTaskWithExpectedOutputs, and getProfile; replace
vi.restoreAllMocks() with vi.clearAllMocks() (or remove the global restore and
scope overrides to individual it blocks) in the beforeEach so hoisted module
mocks remain intact and only call history is reset, ensuring the existing mocked
implementations for ensureLLMTasks, LLMTaskWithExpectedOutputs, and getProfile
are preserved and future tests don't silently break.
- Around line 411-447: The tests currently swallow errors from
Conversation.processNewExpressions which can mask regressions; update each test
in the processNewExpressions describe block (e.g., "creates a batch...", "calls
updateProcessingState...", etc.) to either fully mock the external dependencies
so processNewExpressions resolves (mock the LLM/task client returned by
createMockClient, any async methods used inside processNewExpressions such as
detectNewGroup/updateGroupTopics/addLinks/save/get) or remove the empty
try/catch and assert the failure explicitly with await
expect(conv.processNewExpressions(...)).rejects.toThrow(/specific reason/) so
the thrown error becomes part of the test contract; ensure you reference the
Conversation.processNewExpressions invocation and updateState assertions
(updateState({ step: ... })) when adjusting each test.
In `@packages/api/src/message/MessageSummary.ts`:
- Around line 28-35: Add an isInstance filter to the `@HasMany` decorators in
MessageSummary for the self-referential relations so they only traverse Message
entities: update the decorators on reactions, thread, and replies (the `@HasMany`
entries in MessageSummary) to include isInstance: 'Message' (use the string
literal form for self-references) to ensure only Message-type targets are
returned.
In `@packages/api/src/semantic-relationship/index.ts`:
- Around line 166-173: The plural-to-singular logic in allItemEmbeddingsByType
is brittle — replace the naive itemType.slice(0, -1) approach by normalizing and
validating the incoming type against known keys: check TYPE_MAP for the provided
itemType first (accepting both plural and singular forms), then try a
deterministic conversion (e.g., strip trailing "s" only if present) or use an
explicit lookup map from common plural forms to singulars before falling back to
error; update references to the local variable singular and the TYPE_MAP lookup
so the function robustly handles inputs like "Messages", "Message", or
unexpected casing and logs a clear error if no mapping exists.
In `@scripts/build-with-ad4m-link.sh`:
- Around line 29-30: The script currently suppresses pnpm install errors with
`npm i -g pnpm@9.15.0 2>/dev/null || true`, which can leave an incompatible pnpm
on PATH; after attempting to install/update pnpm, run `pnpm --version` and
validate it matches 9.x (or explicitly equals 9.15.0) and if not, print a clear
error message and exit non‑zero before continuing to the `pnpm install` step so
the failure is visible and the build doesn't proceed with pnpm v10; update the
shell block containing the install command to include this verification and a
failing exit when the version check fails.
In `@turbo.json`:
- Around line 27-30: The typecheck pipeline entry ("typecheck") currently omits
an explicit "inputs" array which causes Turbo to hash the entire package
contents and over-invalidate; update the "typecheck" task to add an "inputs"
field (alongside the existing "dependsOn": ["^build"] and "outputs": []) and
restrict it to only relevant files such as "src/**/*.{ts,tsx,vue}",
"tsconfig*.json", and any shared type-related configs (e.g.,
"packages/**/tsconfig*.json") so cache keys are stable and changes to unrelated
files (like README.md) don’t bust the cache.
In `@views/graph-view/src/components/CommunityGraph.tsx`:
- Around line 88-100: In CommunityGraph's useEffect, avoid calling fetchSnapShot
twice by removing the initial fetchSnapShot(perspective, source) call and
instead invoke the existing refresh() function once at the start; keep refresh()
as the single place that calls fetchSnapShot(perspective, source) and updates
state via setNodes and setLinks (and consolidate any error handling/logging
there), and drop the stray console.log to keep behavior DRY and maintain one
error path.
In `@views/kanban-view/src/components/Board/Board.tsx`:
- Around line 150-161: getClasses currently performs N sequential
perspective.getNamedOptions calls causing N+1 round trips; change it to fetch
options in parallel by mapping allClasses to an array of getNamedOptions
promises, await Promise.all, then filter those results for non-empty option
objects and return the corresponding class names. Update the getClasses function
(which calls perspective.listRegisteredClasses and perspective.getNamedOptions)
to use Promise.all and preserve the existing return type so callers (e.g., the
selectedClass effect) receive the same string[] result.
- Around line 190-193: Current useEffect mixes class discovery and column
loading causing unnecessary re-runs of getClasses when selectedClass changes and
a race with ensureSDNASubjectClass; split into two effects: one effect whose
dependency is [perspective.uuid] only to call
getClasses(perspective).then(setClasses) (move discovery out), and a separate
effect that depends on [perspective.uuid, selectedClass] to call loadColumns()
and perform the auto-selection logic; remove the duplicate
getClasses(perspective).then(setClasses) from the init/ensureSDNASubjectClass
flow and instead either await/chain ensureSDNASubjectClass to complete before
running the discovery effect or trigger a manual re-discovery (call getClasses
and setClasses) after ensureSDNASubjectClass finishes so setClasses ordering is
deterministic.
In `@views/table-view/src/components/Entry/Entry.tsx`:
- Around line 16-49: The async flows in fetchSourceClasses and onUpdate can
write stale state: add a cancellation guard (e.g., a local "cancelled" boolean
or AbortController token created inside the useEffect and passed into
fetchSourceClasses/onUpdate) so that after any await you check the token before
calling setClasses or setEntry (and clean up by toggling cancelled in the effect
cleanup). Also fix the dead render branch by initializing entry to null
(useState(null)) and guarding rendering on entry (or remove the unreachable
return) so the component only renders the fallback when entry is actually null;
update references to entry, setEntry, fetchSourceClasses, onUpdate and the
useEffect accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9dcf9676-ddd4-403e-9102-2c75ff9a607e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (73)
.github/workflows/build.yaml.github/workflows/tests.yamlapp/.eslintignoreapp/.eslintrc.jsapp/package.jsonapp/src/__tests__/audio-processor.test.tsapp/src/components/conversation/timeline/TimelineBlock.vueapp/src/components/conversation/timeline/TimelineColumn.vueapp/src/components/conversation/timeline/__tests__/timeline-subscription.test.tsapp/src/composables/useCommunityService.tsapp/src/composables/useSignallingService.tsapp/src/containers/Conversation.vueapp/src/containers/Conversations.vueapp/src/router/index.tsapp/src/stores/aiStore.tsapp/src/stores/appStore.tsapp/src/utils/registerMobileNotifications.tsapp/src/utils/routeUtils.tsapp/src/views/main/MainView.vueapp/src/views/main/community/CommunityView.vueapp/src/views/main/community/channel/modals/ChangeChannelModal.vueapp/src/views/main/community/channel/modals/ManageChannelPluginsModal.vueapp/src/views/main/community/sidebar/SidebarItem.vueapp/src/views/main/community/sidebar/SidebarList.vueapp/src/views/main/modals/CreateCommunityModal.vueapp/src/views/signup/Orb.vuepackage.jsonpackages/api/jest.config.cjspackages/api/package.jsonpackages/api/src/channel/ChannelSummary.test.tspackages/api/src/channel/ChannelSummary.tspackages/api/src/channel/channel-query.test.tspackages/api/src/channel/channel.test.tspackages/api/src/channel/index.tspackages/api/src/conversation-subgroup/index.tspackages/api/src/conversation/conversation.test.tspackages/api/src/conversation/index.tspackages/api/src/conversation/util.tspackages/api/src/createCommunity.tspackages/api/src/index.tspackages/api/src/message/MessageSummary.test.tspackages/api/src/message/MessageSummary.tspackages/api/src/post/index.tspackages/api/src/semantic-relationship/index.tspackages/api/src/subscribeToLinks.tspackages/api/src/task/index.tspackages/api/src/topic/index.tspackages/api/vitest.config.tspackages/comment-section/src/Components/CommentSection/CommentSection.tsxpackages/flux-container/package.jsonpackages/flux-container/src/flux-container.tspackages/react-web/src/cache.tsxpackages/ui/lib/components/input/Input.jspackages/ui/lib/components/tab-item/TabItem.tspackages/ui/meta.jsonpackages/vue/src/usePerspectives.tsscripts/build-with-ad4m-link.shturbo.jsonviews/chat-view/src/components/MessageList/MessageList.tsxviews/graph-view/src/components/CommunityGraph.tsxviews/kanban-view/src/components/Board/Board.tsxviews/kanban-view/src/components/Board/Task.plviews/kanban-view/src/components/Board/TaskModel.tsviews/kanban-view/src/components/Entry/Entry.tsxviews/kanban-view/src/hooks/useAssociations.tsxviews/poll-view/src/components/PollCard/PollCard.tsxviews/synergy-demo-view/src/components/TimelineColumn/TimelineColumn.tsxviews/table-view/src/components/Entry/Entry.tsxviews/table-view/src/components/Header/Header.tsxviews/table-view/src/components/History/History.tsxviews/table-view/src/components/NewClass/NewClass.tsxviews/table-view/src/components/Table/Table.tsxviews/table-view/src/components/TableView/TableView.tsx
💤 Files with no reviewable changes (4)
- packages/api/jest.config.cjs
- views/kanban-view/src/components/Board/Task.pl
- packages/react-web/src/cache.tsx
- packages/api/src/channel/channel-query.test.ts
Comment on lines
+203
to
+219
| (async () => { | ||
| try { | ||
| const sub = await perspective.subscribeQuery(` | ||
| SELECT ?id WHERE { <${channelUrl}> <ad4m://has_child> ?id . } | ||
| `); | ||
| if (isUnmounted) { | ||
| sub.dispose(); | ||
| return; | ||
| } | ||
| channelItemsSub = sub; | ||
| sub.onResult(() => { | ||
| scheduleUnprocessedItemsRefresh(); | ||
| }); | ||
| } catch (error) { | ||
| console.error('Failed to subscribe to channel items:', error); | ||
| } | ||
| })(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether TimelineColumn.vue is mounted with a :key forcing remount on channelId change
rg -nP --type=vue -C2 '<TimelineColumn\b' || rg -nP -g '*.vue' -C2 '<TimelineColumn\b'Repository: coasys/flux
Length of output: 454
🏁 Script executed:
cd app && find . -name "TimelineColumn.vue" -type fRepository: coasys/flux
Length of output: 113
🏁 Script executed:
cat -n app/src/components/conversation/timeline/TimelineColumn.vueRepository: coasys/flux
Length of output: 20073
🏁 Script executed:
# Check how channelUrl is derived from route params
rg -n "channelUrl|restoreChannelPrefix" app/src/components/conversation/timeline/TimelineColumn.vueRepository: coasys/flux
Length of output: 734
🏁 Script executed:
# Check for route watchers or computed properties in TimelineColumn.vue
rg -n "watch|computed.*route" app/src/components/conversation/timeline/TimelineColumn.vueRepository: coasys/flux
Length of output: 242
🏁 Script executed:
# Check the parent Conversation.vue for route context and whether it forces remount
cat -n app/src/containers/Conversation.vue | head -150Repository: coasys/flux
Length of output: 7345
🏁 Script executed:
# Check if Conversation.vue has route watching or is remounted on route change
rg -n "watch.*route|channelId" app/src/containers/Conversation.vue | head -20Repository: coasys/flux
Length of output: 639
🏁 Script executed:
# Find restoreChannelPrefix implementation
fd -name "routeUtils*" app/srcRepository: coasys/flux
Length of output: 286
🏁 Script executed:
# Check for Vue Router configuration to understand navigation behavior
fd -name "*.ts" -o -name "*.js" app/src | xargs rg -l "createRouter\|beforeEach" | head -5Repository: coasys/flux
Length of output: 286
🏁 Script executed:
# Check if there's a route watcher that forces component updates
rg -n "onBeforeRouteUpdate|onBeforeRouteLeave|watch.*route" app/src/containers/Conversation.vueRepository: coasys/flux
Length of output: 37
🏁 Script executed:
# Find routeUtils file
fd routeUtils app/srcRepository: coasys/flux
Length of output: 83
🏁 Script executed:
# Find and display restoreChannelPrefix implementation
rg -n "restoreChannelPrefix" app/src/utils -A 10Repository: coasys/flux
Length of output: 256
🏁 Script executed:
# Check router configuration
rg -n "createRouter\|beforeEach" app/srcRepository: coasys/flux
Length of output: 37
🏁 Script executed:
# Check Conversation.vue for route watching to see if component remounts on channel change
cat -n app/src/containers/Conversation.vue | sed -n '140,250p'Repository: coasys/flux
Length of output: 5245
🏁 Script executed:
# Check if Conversation.vue uses useRoute with watchers or lifecycle hooks to handle route changes
rg -n "onBeforeRouteUpdate\|watch.*route\|route\." app/src/containers/Conversation.vue | head -20Repository: coasys/flux
Length of output: 37
🏁 Script executed:
# Check the full Conversation component setup to see if there's navigation handling
cat -n app/src/containers/Conversation.vue | sed -n '156,300p'Repository: coasys/flux
Length of output: 7016
🏁 Script executed:
# Look at router configuration to understand navigation behavior
find app/src -name "*router*" -o -name "*routes*" | head -10Repository: coasys/flux
Length of output: 70
🏁 Script executed:
# Check router configuration files
ls -la app/src/router/Repository: coasys/flux
Length of output: 227
🏁 Script executed:
# Check the main router file to understand navigation behavior
cat -n app/src/router/index.ts | head -100Repository: coasys/flux
Length of output: 4028
🏁 Script executed:
# Check if Conversation.vue route is defined with any special configuration
rg -n "Conversation\|conversation" app/src/router/Repository: coasys/flux
Length of output: 37
🏁 Script executed:
# Check for vue-router beforeEach guards that might affect component lifecycle
rg -n "beforeEach\|beforeResolve" app/src/Repository: coasys/flux
Length of output: 37
🏁 Script executed:
# Check ChannelView.vue to see if it handles route param changes
cat -n app/src/views/main/community/channel/ChannelView.vue | head -200Repository: coasys/flux
Length of output: 2804
🏁 Script executed:
# Look for how Conversation container is used
rg -n "Conversation" app/src/views/main/community/channel/ChannelView.vueRepository: coasys/flux
Length of output: 137
🏁 Script executed:
# Find ViewView.vue to see what it renders
fd ViewView.vue app/srcRepository: coasys/flux
Length of output: 110
🏁 Script executed:
# Check ViewView.vue to see if it renders Conversation or something else
cat -n app/src/views/main/community/channel/view/ViewView.vue | head -150Repository: coasys/flux
Length of output: 6385
🏁 Script executed:
# Check if there are any route watchers in ViewView or other components in the hierarchy
rg -n "watch.*route\|onBeforeRouteUpdate" app/src/views/main/community/channel/view/Repository: coasys/flux
Length of output: 37
🏁 Script executed:
# Check where route.params.channelId is used and what values it can have
rg -n "route.params.channelId\|channelId" app/src/utils/routeUtils.tsRepository: coasys/flux
Length of output: 37
🏁 Script executed:
# Look at the full routeUtils.ts to see all route helpers
cat -n app/src/utils/routeUtils.tsRepository: coasys/flux
Length of output: 1571
🏁 Script executed:
# Check if there's any validation or sanitization of route params
rg -n "stripChannelPrefix\|sanitize" app/src/utils/Repository: coasys/flux
Length of output: 37
🏁 Script executed:
# Check the full Conversation.vue file to see if there are any route watchers we missed
wc -l app/src/containers/Conversation.vueRepository: coasys/flux
Length of output: 95
🏁 Script executed:
# Search for any route watchers in Conversation.vue around line 300+
cat -n app/src/containers/Conversation.vue | tail -100Repository: coasys/flux
Length of output: 3148
🏁 Script executed:
# Check if channelId is validated anywhere in the router or component setup
rg -n "channelId.*validate\|channelId.*test\|channelId.*regex" app/src/Repository: coasys/flux
Length of output: 37
🏁 Script executed:
# Verify: does the KeepAlive in ChannelView affect remounting behavior?
rg -n "KeepAlive" app/src/views/main/community/channel/ChannelView.vue -B2 -A2Repository: coasys/flux
Length of output: 404
Add a route watcher or component :key to reset subscriptions when navigating between channels.
The IIFE at lines 203–219 runs once during component setup and captures channelUrl. If Vue Router reuses the TimelineColumn component instance across channel navigations (which occurs without a :key on the component in Conversation.vue:131), the SPARQL subscription, the unprocessed items refresh, and the AI/sidebar watchers all continue pointing at the original channel.
To fix this, either:
- Add
:key="route.params.channelId"to TimelineColumn in Conversation.vue, or - Add a watcher for
route.params.channelIdin TimelineColumn.vue to reset and re-issue subscriptions.
Additionally, channelUrl is interpolated directly into the SPARQL query at line 206. While restoreChannelPrefix simply prepends literal:string:, channel IRIs derived from route params are typically safe, but an explicit sanitization or assertion would harden the query against malformed URIs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/components/conversation/timeline/TimelineColumn.vue` around lines 203
- 219, The TimelineColumn component currently sets up a long-lived SPARQL
subscription using the captured channelUrl inside an IIFE (see the
subscribeQuery block and channelItemsSub), which won’t reset when Vue Router
reuses the component for a different channel; fix by either adding a route-based
key on the parent (e.g., set :key="route.params.channelId" where Conversation
renders TimelineColumn) or by adding a watcher inside TimelineColumn for
route.params.channelId that disposes channelItemsSub, clears any related
watchers (AI/sidebar watchers, unprocessed items logic), and re-runs the
subscription setup (the IIFE logic) when the channel changes; also sanitize or
assert channelUrl before interpolating into the SPARQL string (or use
restoreChannelPrefix consistently) to avoid injecting malformed IRIs into
subscribeQuery.
Comment on lines
+210
to
+211
| const key = perspective.sharedUrl || perspective.uuid; | ||
| const perspectiveIsCommunity = Object.keys(myCommunities.value).some((id) => key === id); |
There was a problem hiding this comment.
Fix key format mismatch with myCommunities store.
The key derivation here uses perspective.sharedUrl || perspective.uuid, but appStore.getMyCommunities() (line 105 in appStore.ts) stores keys as perspective.sharedUrl || 'private://${perspective.uuid}'. This mismatch means perspectives without sharedUrl will never match against myCommunities, incorrectly showing them as "not a flux community".
🐛 Proposed fix to align key format
- const key = perspective.sharedUrl || perspective.uuid;
+ const key = perspective.sharedUrl || `private://${perspective.uuid}`;
const perspectiveIsCommunity = Object.keys(myCommunities.value).some((id) => key === id);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const key = perspective.sharedUrl || perspective.uuid; | |
| const perspectiveIsCommunity = Object.keys(myCommunities.value).some((id) => key === id); | |
| const key = perspective.sharedUrl || `private://${perspective.uuid}`; | |
| const perspectiveIsCommunity = Object.keys(myCommunities.value).some((id) => key === id); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/views/main/modals/CreateCommunityModal.vue` around lines 210 - 211,
The key used to check membership is built incorrectly: change the key derivation
used by perspectiveIsCommunity to match the myCommunities store format (which
uses perspective.sharedUrl || `private://${perspective.uuid}` in
appStore.getMyCommunities()). Update the code that defines key (used by
perspectiveIsCommunity) to use perspective.sharedUrl ||
`private://${perspective.uuid}` so perspectives without a sharedUrl will
correctly match entries in myCommunities.value.
Comment on lines
167
to
168
| const valuesClause = unprocessedIds.map((id: string) => `<${id}>`).join(' '); | ||
| const dataQuery = ` |
There was a problem hiding this comment.
Escape/validate IDs interpolated into the VALUES clause.
unprocessedIds is produced from a prior SPARQL result and then concatenated directly into the data query as <${id}>. If any item id ever contains a > (or other IRI-breaking character) this will produce a malformed query and the whole unprocessedItems() call will fall back to [] (the catch block), causing the AI pipeline to silently stop enqueuing. A simple guard (regex-validate or reject ids containing >/whitespace before interpolation) avoids a hard-to-diagnose outage.
🛡️ Suggested defensive validation
- const valuesClause = unprocessedIds.map((id: string) => `<${id}>`).join(' ');
+ const safeIds = unprocessedIds.filter(
+ (id: string) => typeof id === 'string' && !/[\s<>"{}|^`\\]/.test(id),
+ );
+ if (safeIds.length === 0) return [];
+ const valuesClause = safeIds.map((id: string) => `<${id}>`).join(' ');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const valuesClause = unprocessedIds.map((id: string) => `<${id}>`).join(' '); | |
| const dataQuery = ` | |
| const safeIds = unprocessedIds.filter( | |
| (id: string) => typeof id === 'string' && !/[\s<>"{}|^`\\]/.test(id), | |
| ); | |
| if (safeIds.length === 0) return []; | |
| const valuesClause = safeIds.map((id: string) => `<${id}>`).join(' '); | |
| const dataQuery = ` |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/api/src/channel/index.ts` around lines 167 - 168, The interpolation
of unprocessedIds into valuesClause (`unprocessedIds.map(id => `<${id}>`)`) is
unsafe; validate or escape each id before building the SPARQL VALUES clause
(e.g., reject or log and skip ids containing '>', whitespace, or other
IRI-breaking characters) to avoid constructing a malformed query that causes
unprocessedItems() to fail silently; update the code that produces valuesClause
to validate each id against a safe pattern (or explicitly escape/URN-encode it),
and ensure failures are handled deterministically (throw or return a clear
error/log) rather than falling back to an empty array.
Comment on lines
+261
to
+308
| const sparql = ` | ||
| PREFIX rdf: <http://www.w3.org/1999/02/22-rdf-syntax-ns#> | ||
| SELECT ?channelId (SAMPLE(?cId) AS ?conversationId) (MAX(?ts) AS ?lastActivity) WHERE { | ||
| ?channelId <${ENTRY_TYPE}> <${EntryType.Channel}> . | ||
| ?channelId <${CHANNEL_IS_CONVERSATION}> ?_isConv . | ||
| FILTER(STR(<ad4m://fn/parse_literal>(?_isConv)) = "true") | ||
| OPTIONAL { | ||
| ?channelId <ad4m://has_child> ?cId . | ||
| ?cId <flux://entry_type> <flux://conversation> . | ||
| } | ||
| OPTIONAL { | ||
| ?channelId <ad4m://has_child> ?item . | ||
| ?_itemReifier rdf:reifies <<( ?channelId <ad4m://has_child> ?item )>> . | ||
| ?_itemReifier <ad4m://ontology/timestamp> ?itemTs . | ||
| ?item <${ENTRY_TYPE}> ?itemType . | ||
| FILTER(?itemType IN (<${EntryType.Message}>, <${EntryType.Post}>)) | ||
| } | ||
| OPTIONAL { | ||
| ?_chanReifier rdf:reifies <<( ?_parent <flux://has_channel> ?channelId )>> . | ||
| ?_chanReifier <ad4m://ontology/timestamp> ?chanCreatedTs . | ||
| } | ||
| BIND(COALESCE(?itemTs, ?chanCreatedTs, "1970-01-01T00:00:00Z"^^<http://www.w3.org/2001/XMLSchema#dateTime>) AS ?ts) | ||
| } | ||
| GROUP BY ?channelId | ||
| ORDER BY DESC(?lastActivity) | ||
| LIMIT ${limit} | ||
| `; | ||
|
|
||
| try { | ||
| const results = await perspective.querySparql(sparql); | ||
| // Safety-net dedup by channelId — the SPARQL GROUP BY should already | ||
| // return one row per channel, but guard against engine quirks. | ||
| const seen = new Map<string, { channelId: string; conversationId?: string; lastActivity?: string }>(); | ||
| for (const r of results || []) { | ||
| const cid = r.channelId; | ||
| if (!cid || seen.has(cid)) continue; | ||
| seen.set(cid, { | ||
| channelId: cid, | ||
| conversationId: r.conversationId || undefined, | ||
| lastActivity: r.lastActivity || undefined, | ||
| }); | ||
| } | ||
| return Array.from(seen.values()); | ||
| } catch (error) { | ||
| console.error('Error in Channel.recentConversations():', error); | ||
| return []; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does @coasys/ad4m0.13.x Oxigraph expose a custom SPARQL functionad4m://fn/parse_literal for use in FILTER expressions?
💡 Result:
No, @coasys/ad4m 0.13.x with Oxigraph does not expose a custom SPARQL function ad4m://fn/parse_literal for use in FILTER expressions. Oxigraph implements standard SPARQL 1.1 with some extensions, but no custom AD4M functions like ad4m://fn/parse_literal are documented in its SPARQL support [1][2]. AD4M primarily uses SurrealDB for querying, where a similar fn::parse_literal() function exists for parsing AD4M literal URIs (e.g., literal://string:Hello → "Hello") [3]. This SurrealQL function is used in AD4mModel queries and direct perspective.querySurrealDB() calls, not SPARQL [4][5][6]. No evidence in AD4M GitHub repo [7][8][9], changelog [9], releases up to v0.13.0-test-1 [8], or docs mentions Oxigraph integration or custom SPARQL functions [6]. AD4M's query engine defaults to SurrealDB (10-100x faster than Prolog), with Prolog as fallback [3][4]. If Oxigraph is used, it would support standard SPARQL functions only [1].
Citations:
- 1: https://github.com/oxigraph/oxigraph/wiki/SPARQL
- 2: https://github.com/oxigraph/oxigraph
- 3: https://docs.ad4m.dev/developer-guides/surreal-queries
- 4: https://docs.ad4m.dev/developer-guides/model-classes
- 5: https://docs.ad4m.dev/jsdoc/classes/Ad4mModel
- 6: https://docs.ad4m.dev/
- 7: https://github.com/coasys/ad4m
- 8: https://github.com/coasys/ad4m/tags
- 9: https://github.com/coasys/ad4m/blob/dev/CHANGELOG
Remove or replace the unsupported ad4m://fn/parse_literal SPARQL function.
The STR(<ad4m://fn/parse_literal>(?_isConv)) filter will fail at runtime because ad4m://fn/parse_literal is not a supported SPARQL function in Oxigraph. AD4M uses SurrealDB by default and only supports standard SPARQL 1.1 functions when Oxigraph is used. The function exists as fn::parse_literal() in SurrealQL, not SPARQL. This will cause both recentConversations and pinnedConversations to throw an exception, caught silently and returning [], hiding all pinned/recent conversations from the UI.
Additionally, the OPTIONAL block (lines 271–277) expands the join to every channel item before GROUP BY collapses it; correctness is preserved by MAX(?ts)/SAMPLE(?cId), but result-set size grows linearly with item count per channel.
Comment on lines
+31
to
+34
| const { data: answers } = useLiveQuery(Answer, perspective, { | ||
| parent: { model: Poll, id: poll.id }, | ||
| query: { include: { votes: true } }, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify useLiveQuery API and include syntax in ad4m-react-hooks
# Check the actual version being used
cat package.json | jq '.dependencies["@coasys/ad4m-react-hooks"], .devDependencies["@coasys/ad4m-react-hooks"]'
# Search for other usages of include in queries to see if pattern is used elsewhere
rg -n "useLiveQuery.*include" --type=ts --type=tsx -C2Repository: coasys/flux
Length of output: 111
🌐 Web query:
@coasys/ad4m-react-hooks useLiveQuery include option documentation version 0.13.0-test-4
💡 Result:
There isn’t any documentation I can find (from the official AD4M docs/pages that describe useLiveQuery) that mentions an include option for @coasys/ad4m-react-hooks useLiveQuery—so I can’t provide “include option documentation” for version 0.13.0-test-4 specifically. What the official docs do say about useLiveQuery: - useLiveQuery is documented in AD4M’s “AD4M React Hooks” page, describing collection mode (default), single-instance mode (pass an id in options), and other features like parent-scoped queries, pagination, and return types. [1] - The docs page shows useLiveQuery(Todo, perspective, { query: { where: ... }, pageSize: ... }), but the visible options shown there do not include an include field. [1] Why this matters / where “include” likely fits instead: - AD4M’s query system supports eager-loading via an include field in the underlying model/query type (include?: IncludeMap), and IncludeMap is documented as controlling relation hydration (e.g., { comments: true }, or scoped sub-queries). [2] - So, if you want “include” behavior with useLiveQuery, the most likely path is that useLiveQuery forwards a query object down to the model query builder, and therefore query.include (not a top-level useLiveQuery option named include) is the correct place. However, I did not find a source that explicitly states this mapping for 0.13.0-test-4. If you want, paste the exact useLiveQuery call you’re using and I’ll map which option shape should contain include based on AD4M’s Query/IncludeMap types. [2]
Citations:
- 1: https://docs.ad4m.dev/developer-guides/hooks
- 2: https://github.com/coasys/ad4m/blob/58a48e968/core/src/model/types.ts
🏁 Script executed:
#!/bin/bash
# Find package.json files and check ad4m-react-hooks version
fd -n "package.json" -x cat {} \; | grep -A2 -B2 "ad4m-react-hooks"Repository: coasys/flux
Length of output: 286
🏁 Script executed:
#!/bin/bash
# Search for useLiveQuery usage patterns (fix file type issue)
rg -n "useLiveQuery.*include" -A2 -B2Repository: coasys/flux
Length of output: 37
🏁 Script executed:
#!/bin/bash
# Check for type definitions or docs in the repo about useLiveQuery
find . -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.d.ts" \) | xargs grep -l "useLiveQuery" | head -10Repository: coasys/flux
Length of output: 635
🏁 Script executed:
#!/bin/bash
# Find and display package.json with ad4m-react-hooks dependency
find . -name "package.json" -type f -exec grep -l "ad4m-react-hooks" {} \;Repository: coasys/flux
Length of output: 462
🏁 Script executed:
#!/bin/bash
# Look at actual useLiveQuery calls in PollCard and other files
rg "useLiveQuery" -A4 -B1 views/poll-view/src/components/PollCard/PollCard.tsx views/chat-view/src/components/MessageList/MessageList.tsx views/kanban-view/src/components/Card/Card.tsxRepository: coasys/flux
Length of output: 3336
🏁 Script executed:
#!/bin/bash
# Check package.json in the root and views directories
cat package.json 2>/dev/null | grep -A1 -B1 "ad4m-react-hooks" || echo "Not found in root"
for dir in views/*/; do
[ -f "$dir/package.json" ] && echo "=== $dir ===" && grep -A1 -B1 "ad4m-react-hooks" "$dir/package.json"
doneRepository: coasys/flux
Length of output: 1602
🏁 Script executed:
#!/bin/bash
# Check if poll-view package.json should be updated
# Look at root package.json and see if this might be part of the PR
cat package.json | grep -A5 -B5 "ad4m-react-hooks"Repository: coasys/flux
Length of output: 337
🏁 Script executed:
#!/bin/bash
# Get full PollCard.tsx to understand the complete implementation
wc -l views/poll-view/src/components/PollCard/PollCard.tsxRepository: coasys/flux
Length of output: 112
🏁 Script executed:
#!/bin/bash
# Check if poll-view has a separate yarn lock or if it uses root
cat views/poll-view/package.json | head -20Repository: coasys/flux
Length of output: 506
🏁 Script executed:
#!/bin/bash
# Read full PollCard.tsx to see the implementation
cat -n views/poll-view/src/components/PollCard/PollCard.tsx | head -50Repository: coasys/flux
Length of output: 2156
🏁 Script executed:
#!/bin/bash
# Check if there's a pnpm-workspace or monorepo config
ls -la | grep -E "pnpm-workspace|package-lock|yarn.lock"
cat pnpm-workspace.yaml 2>/dev/null || echo "No pnpm-workspace.yaml found"Repository: coasys/flux
Length of output: 206
🏁 Script executed:
#!/bin/bash
# Check git status to see if package.json updates are part of this change
git diff HEAD -- views/poll-view/package.json 2>/dev/null || echo "Not a git repo or no diff"
git log --oneline -5 2>/dev/null || echo "Cannot access git log"Repository: coasys/flux
Length of output: 106
🏁 Script executed:
#!/bin/bash
# Read more of PollCard.tsx to see how include is used
cat -n views/poll-view/src/components/PollCard/PollCard.tsx | sed -n '47,120p'Repository: coasys/flux
Length of output: 3401
🏁 Script executed:
#!/bin/bash
# Check views/poll-view/package.json dependencies more thoroughly
cat views/poll-view/package.jsonRepository: coasys/flux
Length of output: 1239
🏁 Script executed:
#!/bin/bash
# Check if there are any type errors or TypeScript config issues
find views/poll-view -name "tsconfig.json" -o -name "tsconfig.app.json" | xargs cat 2>/dev/nullRepository: coasys/flux
Length of output: 466
Update views/poll-view/package.json to use @coasys/ad4m-react-hooks version 0.13.0-test-4 to support the include query syntax.
The code at lines 31-34 uses query: { include: { votes: true } } to eager-load votes with the subscription, but views/poll-view/package.json is locked to version 0.11.1 while the root package.json has been bumped to 0.13.0-test-4. In a pnpm monorepo, this version mismatch will cause the include syntax to fail at runtime.
Additionally, line 98 should use the already-loaded votes from the subscription instead of calling Vote.findAll():
Suggested fix for line 98
async function vote(answerId: string, value?: number) {
- const votes = await Vote.findAll(perspective, { parent: { model: Answer, id: answerId } });
+ const answer = answers.find(a => a.id === answerId);
+ const votes = answer?.votes || [];
const previousVote = votes.find((vote: any) => vote.author === myDid) as any;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@views/poll-view/src/components/PollCard/PollCard.tsx` around lines 31 - 34,
The project is using the new include query syntax with useLiveQuery(Answer,
perspective, { parent: { model: Poll, id: poll.id }, query: { include: { votes:
true } } }) but views/poll-view/package.json is pinned to
`@coasys/ad4m-react-hooks` v0.11.1 which lacks that syntax; update
views/poll-view/package.json to use `@coasys/ad4m-react-hooks` version
0.13.0-test-4 so the include option works, and modify the render code that
currently calls Vote.findAll() (referenced as Vote.findAll) to instead use the
eager-loaded votes from the subscription (access votes on the Answer objects
returned by useLiveQuery, e.g., answers -> answer.votes) so you reuse the loaded
data rather than refetching.
Comment on lines
40
to
44
| async function addClass() { | ||
| const sdna = await generateSDNA(name, columns, perspective); | ||
| await perspective.addSdna(sdna); | ||
| const shape = await buildSHACLShape(name, columns, perspective); | ||
| await perspective.addShacl(name, shape); | ||
| onSaved(); | ||
| } |
There was a problem hiding this comment.
Validate and sanitize before persisting the SHACL shape.
addClass() currently saves whatever is in state. There are several reachable bad states:
nameis not validated; it can be empty, or contain spaces /#//, which produces a malformed IRI inrecipe://${name}and a broken class id${ns}#${name}further down.- New columns are added with empty
nameand emptypredicate(seeaddNewColumn). Persisting a property shape whosepathis''is almost certainly invalid SHACL. - Two columns with the same predicate would overwrite each other semantically.
Recommend at minimum validating that name is non-empty and IRI-safe, every column has a non-empty name and predicate, and predicates are unique — and surface the error before calling perspective.addShacl.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@views/table-view/src/components/NewClass/NewClass.tsx` around lines 40 - 44,
In addClass(), validate and sanitize inputs before calling
buildSHACLShape/perspective.addShacl: ensure the class name (variable name) is
non-empty and URL/IRI-safe (no spaces, '#' or '/'; escape or reject invalid
chars), ensure every column in columns (added via addNewColumn) has non-empty
name and predicate, and enforce uniqueness of column predicates (reject or mark
duplicates), then surface a clear validation error to the user (do not call
perspective.addShacl or onSaved until validation passes); run these checks
immediately in addClass and only call buildSHACLShape(passedName, passedColumns,
perspective) and perspective.addShacl when all validations succeed.
Comment on lines
+227
to
276
| async function buildSHACLShape( | ||
| name: string, | ||
| columns: Record<string, Column>, | ||
| perspective: PerspectiveProxy, | ||
| ): Promise<string> { | ||
| const atom = makeRandomPrologAtom(6); | ||
| let constructorActions = ''; | ||
| let instancePredicates = ''; | ||
| let propertiesString = ''; | ||
|
|
||
| for await (const column of Object.values(columns)) { | ||
| const { name: columnName, required, predicate, defaultValue, language } = column; | ||
|
|
||
| const expression = await perspective.createExpression(defaultValue, language); | ||
|
|
||
| if (defaultValue) { | ||
| constructorActions += `{action: "addLink", source: "this", predicate: "${predicate}", target: "${expression}"},`; | ||
| } | ||
|
|
||
| if (required) { | ||
| instancePredicates += `triple(Base, "${predicate}", _),`; | ||
| ): Promise<SHACLShape> { | ||
| const ns = `recipe://${name}`; | ||
| const shape = new SHACLShape(`${ns}Shape`, `${ns}#${name}`); | ||
| const constructorActions: Array<{ action: string; source: string; predicate: string; target: string }> = []; | ||
|
|
||
| for (const column of Object.values(columns)) { | ||
| const { name: columnName, required, predicate, defaultValue, language, options } = column; | ||
|
|
||
| const expression = defaultValue ? await perspective.createExpression(defaultValue, language) : undefined; | ||
|
|
||
| const propShape: any = { | ||
| name: columnName, | ||
| path: predicate, | ||
| datatype: 'xsd://string', | ||
| minCount: required ? 1 : 0, | ||
| maxCount: 1, | ||
| writable: true, | ||
| }; | ||
|
|
||
| if (language) { | ||
| propShape.resolveLanguage = language; | ||
| } | ||
|
|
||
| /* | ||
| if (required && column.options.length > 0) { | ||
| for (const option of column.options) { | ||
| instancePredicates += `triple(Base, "${predicate}", "${option.value}");`; | ||
| } | ||
| if (options.length > 0) { | ||
| propShape.in = options.map((opt) => ({ value: opt.value, label: opt.name })); | ||
| } | ||
| */ | ||
|
|
||
| let namedOptionsString = ''; | ||
| shape.addProperty(propShape); | ||
|
|
||
| for (const option of column.options) { | ||
| namedOptionsString += `property_named_option(${atom}, "${columnName}", "${option.value}", "${option.name}").\n`; | ||
| // Build constructor action for default value | ||
| if (expression) { | ||
| constructorActions.push({ | ||
| action: 'addLink', | ||
| source: 'this', | ||
| predicate, | ||
| target: expression, | ||
| }); | ||
| } | ||
|
|
||
| propertiesString += ` | ||
| property(${atom}, "${columnName}"). | ||
| property_resolve(${atom}, "${columnName}"). | ||
| property_resolve_language(${atom}, "${columnName}", "${language}"). | ||
| property_getter(${atom}, Base, "${columnName}", Value) :- triple(Base, "${predicate}", Value). | ||
| property_setter(${atom}, "${columnName}", '[{action: "setSingleTarget", source: "this", predicate: "${predicate}", target: "value"}]'). | ||
| ${namedOptionsString} | ||
| `; | ||
| } | ||
|
|
||
| // Replace the last comma with a space to format the Prolog string properly | ||
| constructorActions = constructorActions.replace(/,*$/, ''); | ||
| instancePredicates = instancePredicates.replace(/,*$/, ''); | ||
| // Replace the or operator (;) | ||
| instancePredicates = instancePredicates.replace(/;*$/, ''); | ||
|
|
||
| const prologString = ` | ||
| subject_class("${name}", ${atom}). | ||
| constructor(${atom}, '[${constructorActions}]'). | ||
| instance(${atom}, Base) :- ${instancePredicates}. | ||
| ${propertiesString} | ||
| `; | ||
| if (constructorActions.length > 0) { | ||
| shape.setConstructorActions(constructorActions); | ||
| } | ||
|
|
||
| return prologString; | ||
| return shape; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In @coasys/ad4m 0.13.0, what is the SHACLShape API (constructor args, addProperty schema, setConstructorActions placeholders), and what datatype URI is expected for strings?
💡 Result:
In @coasys/ad4m 0.13.0, the SHACLShape API is: 1) Constructor args - new SHACLShape(targetClassOrShapeUri, targetClass?) [1] - If you pass one argument, it is the target class; the shape URI is auto-derived as {class}Shape [1] - If you pass two arguments: first is the shape URI and second is the target class [1] 2) addProperty schema - Method signature: addProperty(prop: SHACLPropertyShape): void [1] - You add a property constraint by pushing the SHACLPropertyShape into the shape’s properties array [2]. 3) setConstructorActions placeholders - Method signature: setConstructorActions(actions: AD4MAction[]): void [1] - It sets the AD4M-specific constructor_actions field on the shape object [2]. 4) Datatype URI expected for strings - For string values, the generated SHACL uses sh:datatype xsd:string, and AD4M stores those as literal://string:... URIs [3]. So, for “strings”, use xsd:string (SHACL datatype) / the corresponding AD4M literal string encoding literal://string:... [3].
Citations:
- 1: https://docs.ad4m.dev/jsdoc/classes/SHACLShape
- 2: https://github.com/coasys/ad4m/blob/58a48e968/core/src/shacl/SHACLShape.ts
- 3: https://docs.ad4m.dev/social-dna
🏁 Script executed:
# Search for AD4M SHACLShape and related SHACL implementations
curl -s "https://raw.githubusercontent.com/coasys/ad4m/main/core/src/shacl/SHACLShape.ts" | head -200Repository: coasys/flux
Length of output: 6102
🏁 Script executed:
# Search for AD4M constructor actions documentation
curl -s "https://raw.githubusercontent.com/coasys/ad4m/main/core/src/types.ts" | grep -A 20 "constructor_actions\|AD4MAction" | head -50Repository: coasys/flux
Length of output: 37
🏁 Script executed:
# Search for datatype handling in AD4M SHACL
curl -s "https://raw.githubusercontent.com/coasys/ad4m/main/core/src/shacl/SHACLPropertyShape.ts" | head -150Repository: coasys/flux
Length of output: 69
🏁 Script executed:
# Search for 'in' field in SHACL property shape definitions
rg "in\?" views/table-view/src/components --type ts -A 2 -B 2 | head -100Repository: coasys/flux
Length of output: 37
🏁 Script executed:
# Search for propShape definitions in the file to understand the structure
rg "propShape\." views/table-view/src/components/NewClass/NewClass.tsx -A 1 -B 1Repository: coasys/flux
Length of output: 248
🏁 Script executed:
# Search for perspective.addShacl calls to understand the method signature
rg "addShacl|setConstructorActions" views/table-view/src --type ts -B 2 -A 2Repository: coasys/flux
Length of output: 859
🏁 Script executed:
# Search for how AD4M handles datatype URIs in examples or documentation
rg "datatype.*string|xsd:" views/table-view/src --type ts | head -20Repository: coasys/flux
Length of output: 141
🏁 Script executed:
# Search for datatype examples in the codebase
rg "datatype.*:" views/table-view/src --type ts -B 1 -A 1Repository: coasys/flux
Length of output: 306
🏁 Script executed:
# Search for constructor actions in the codebase to understand 'this' usage
rg "action.*addLink|source.*this" views/table-view/src --type ts -B 2 -A 2Repository: coasys/flux
Length of output: 531
🏁 Script executed:
# Look for any documentation or type definitions of perspective methods
fd -e ts -e tsx views/table-view/src | xargs grep -l "perspective\." | head -3Repository: coasys/flux
Length of output: 441
🏁 Script executed:
# Search for PerspectiveProxy type definition
fd . views/table-view/src --full-path -type f -name "*.ts" -o -name "*.tsx" | xargs grep -l "PerspectiveProxy\|perspective\.addShacl" | head -5Repository: coasys/flux
Length of output: 226
🏁 Script executed:
# Search for any imports or type definitions related to perspective
rg "import.*Perspective|type.*Perspective" views/table-view/src --type ts -A 2 | head -50Repository: coasys/flux
Length of output: 1976
🏁 Script executed:
# Look for example SHACL shapes in the codebase
rg "SHACLShape\|addProperty" views/table-view/src --type ts -B 1 -A 3 | head -80Repository: coasys/flux
Length of output: 37
🏁 Script executed:
# Search for any existing SHACL shape examples in the codebase
rg "new SHACLShape|addProperty|setConstructorActions" . --type ts --type tsx -B 2 -A 5 | head -100Repository: coasys/flux
Length of output: 83
🏁 Script executed:
# Look for how datatype is handled elsewhere in the codebase
rg "datatype" . --type ts --type tsx -B 1 -A 1 | head -50Repository: coasys/flux
Length of output: 83
🏁 Script executed:
# Search for any test files that might show expected formats
fd -type f -name "*.test.ts" -o -name "*.spec.ts" | xargs grep -l "SHACL\|datatype" 2>/dev/null | head -5Repository: coasys/flux
Length of output: 226
🏁 Script executed:
# Search for SHACL shape examples in the entire repo
rg "new SHACLShape" -A 5 -B 1 | head -80Repository: coasys/flux
Length of output: 786
🏁 Script executed:
# Search for datatype usage patterns
rg "datatype:" -A 1 -B 1 | head -60Repository: coasys/flux
Length of output: 306
🏁 Script executed:
# Search for any test or example files with SHACL
rg "SHACLShape\|addProperty" -A 3 -B 1 | head -100Repository: coasys/flux
Length of output: 37
🏁 Script executed:
# Look for any documentation or comments about the expected format
rg "xsd:|datatype|datatype.*string" -B 2 -A 1 | head -80Repository: coasys/flux
Length of output: 384
🏁 Script executed:
# Search for perspective method definitions or type definitions
rg "addShacl.*function|addShacl.*method" . -B 2 -A 5Repository: coasys/flux
Length of output: 37
🏁 Script executed:
# Look for any imports from ad4m to understand the API surface
rg "from.*@coasys/ad4m" . -A 3 | head -100Repository: coasys/flux
Length of output: 8070
🏁 Script executed:
# Search for any comments or documentation about SHACL in the codebase
rg "SHACL|datatype|constructor" . -B 1 -A 1 | grep -i "comment\|doc\|//" | head -50Repository: coasys/flux
Length of output: 31631
Fix datatype URI format from xsd://string to xsd:string.
Line 244 uses datatype: 'xsd://string', but AD4M 0.13.0's SHACL parser expects standard CURIE notation: datatype: 'xsd:string'. The xsd:// scheme is non-standard; the correct format for XSD datatypes in SHACL is the CURIE prefix xsd: (or the full URI http://www.w3.org/2001/XMLSchema#string).
The other concerns remain valid and worth confirming:
- Constructor action
source: 'this'(Line 264) — verify AD4M's constructor-action engine correctly substitutes'this'with the new subject. propShape.informat (Line 255) using{ value, label }objects — confirm this AD4M extension is supported.perspective.addShacl(name, shape)signature — verify the arity and parameter order match AD4M's API.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@views/table-view/src/components/NewClass/NewClass.tsx` around lines 227 -
276, The SHACL property datatype is using a non-standard URI 'xsd://string'
inside buildSHACLShape; change the propShape.datatype assignment to use the
CURIE 'xsd:string' (or full XSD URI) so the SHACL parser accepts it; update the
line that sets datatype in the propShape object (where propShape is constructed
in buildSHACLShape) to 'xsd:string', keep the rest of the propShape
construction, addProperty calls, and constructorActions logic unchanged, and
then re-run tests to confirm parser accepts the shape.
Comment on lines
+232
to
+233
| const ns = `recipe://${name}`; | ||
| const shape = new SHACLShape(`${ns}Shape`, `${ns}#${name}`); |
There was a problem hiding this comment.
Escape name before embedding it in the recipe IRI.
recipe://${name} is built by raw string concatenation. Any whitespace, #, /, or other reserved IRI character that a user types will produce an invalid IRI and a malformed targetClass (${ns}#${name}). Sanitize/percent-encode the name (or restrict the input to a safe character class) before constructing the namespace.
- const ns = `recipe://${name}`;
- const shape = new SHACLShape(`${ns}Shape`, `${ns}#${name}`);
+ const safeName = encodeURIComponent(name.trim());
+ if (!safeName) throw new Error('Class name is required');
+ const ns = `recipe://${safeName}`;
+ const shape = new SHACLShape(`${ns}Shape`, `${ns}#${safeName}`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const ns = `recipe://${name}`; | |
| const shape = new SHACLShape(`${ns}Shape`, `${ns}#${name}`); | |
| const safeName = encodeURIComponent(name.trim()); | |
| if (!safeName) throw new Error('Class name is required'); | |
| const ns = `recipe://${safeName}`; | |
| const shape = new SHACLShape(`${ns}Shape`, `${ns}#${safeName}`); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@views/table-view/src/components/NewClass/NewClass.tsx` around lines 232 -
233, The code builds an IRI using the raw user-provided name (const ns =
`recipe://${name}`; new SHACLShape(`${ns}Shape`, `${ns}#${name}`)) which can
produce invalid IRIs; sanitize/percent-encode the name before embedding it (or
validate/restrict to a safe char class) and use the encoded value when
constructing ns and the shape IRI (i.e., apply the same encoding helper to name
for both the namespace and the `${ns}#${name}` target so both IRIs are valid).
Comment on lines
61
to
66
| useEffect(() => { | ||
| perspective.infer(`subject_class(ClassName, C)`).then((result) => { | ||
| if (Array.isArray(result)) { | ||
| const uniqueClasses = [...new Set(result.map((c) => c.ClassName))]; | ||
| setClasses(uniqueClasses); | ||
| setSelected(uniqueClasses[0] || ''); | ||
| } else { | ||
| setClasses([]); | ||
| setSelected(''); | ||
| } | ||
| perspective.listRegisteredClasses().then((result) => { | ||
| setClasses(result); | ||
| setSelected(result[0] || ''); | ||
| }); | ||
| }, [perspective.uuid]); |
There was a problem hiding this comment.
Guard the "+" create button when no class is selected.
If listRegisteredClasses() returns an empty array, selected is set to ''. The header's + {selected} button still renders and clicking it calls createEntry({ subjectClass: '' }), which will either fail in createSubject or create a subject without a class. Either disable the button when !selected or hide it when classes.length === 0.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@views/table-view/src/components/TableView/TableView.tsx` around lines 61 -
66, When perspective.listRegisteredClasses() yields an empty array the component
sets selected to '' but the header "+" button still renders and can call
createEntry({ subjectClass: '' }); update the header button rendering logic in
the TableView component to guard against no selection: check classes.length and
selected and either disable the "+" button when !selected or hide the button
entirely when classes.length === 0 so createEntry is never invoked with an empty
subjectClass; locate the UI that renders the "+" button (references: selected,
classes, createEntry, createSubject) and add the conditional
rendering/disabled-state there.
Picks up the embedded mode URL fix — when WE is connected to a remote node the correct host URL is now forwarded in AD4M_CONFIG so Flux's WebSocket connects to the right endpoint instead of localhost.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Deploy preview for use in WE
feat/sparql-1.2-cleanup+ ad4m version bump to0.13.0-test-4+ explicit through: 'ad4m://has_child' on HasMany relationsSummary by CodeRabbit
New Features
Bug Fixes
Tests
Chores