feat(api): thread AbortSignal through querySparql wrappers#606
feat(api): thread AbortSignal through querySparql wrappers#606HexaField wants to merge 1 commit into
Conversation
Adds opt-in cancellation to every public API method that wraps a
querySparql call. Callers pass `{ signal }` from an AbortController and
the executor receives a `request.cancel` over the WebSocket, short-
circuiting the JSON reply. Aborted calls reject with `DOMException
('Aborted', 'AbortError')` — wrappers re-throw this so callers can
distinguish cancellation from real failures (non-AbortError exceptions
are still logged + swallowed for graceful degradation).
Wired through:
- Channel: allItems, unprocessedItems, totalItemCount,
recentConversations, pinnedConversations
- Conversation: stats, topics, subgroups, subgroupsData
- ConversationSubgroup: stats, topics, itemsData, topicsWithRelevance
- SemanticRelationship: itemEmbedding, allConversationEmbeddings,
allSubgroupEmbeddings, allItemEmbeddings, allItemEmbeddingsByType
- Topic: linkedConversations, linkedSubgroups
Structural `AbortOptions { signal?: AbortSignal }` type lives in
`packages/api/src/shared/abort.ts` and is re-exported from the package
index. Structurally compatible with ad4m's `CallOptions` — no version
bump required to consume the underlying executor cancellation support
shipped in coasys/ad4m#855.
Tests
- Forwarding: Channel.allItems passes the signal to
perspective.querySparql.
- Re-throw: Channel.allItems propagates DOMException AbortError instead
of swallowing it.
Backwards compatible: all new parameters are optional, existing call
sites are unchanged.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
✅ Deploy Preview for fluxsocial-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThis PR adds cooperative request cancellation support across the Flux API package by introducing an ChangesRequest Cancellation Pattern Across API Models
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/api/src/conversation/index.ts`:
- Around line 135-141: The call in Conversation.subgroups is passing
AbortOptions as a third arg to ConversationSubgroup.findAll which the
mocked/real API treats as a single options object; fix by passing the
abort/cancellation options inside the same options object passed as the second
parameter—i.e. call ConversationSubgroup.findAll(this.perspective, { parent: {
model: Conversation, id: this.id }, ...(options ?? {}) }) so the abort signal is
propagated; update the subgroups method accordingly (referencing
ConversationSubgroup.findAll and the subgroups method).
🪄 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: 167606f0-efdd-46a0-8e7d-2160a4cef550
📒 Files selected for processing (8)
packages/api/src/channel/channel.test.tspackages/api/src/channel/index.tspackages/api/src/conversation-subgroup/index.tspackages/api/src/conversation/index.tspackages/api/src/index.tspackages/api/src/semantic-relationship/index.tspackages/api/src/shared/abort.tspackages/api/src/topic/index.ts
| async subgroups(options?: AbortOptions): Promise<ConversationSubgroup[]> { | ||
| // find the conversations subgroup entities — use parent-scoped query | ||
| // instead of this.get() which fetches all Conversation triples | ||
| return ConversationSubgroup.findAll(this.perspective, { | ||
| parent: { model: Conversation, id: this.id }, | ||
| }) as unknown as Promise<ConversationSubgroup[]>; | ||
| }, options) as unknown as Promise<ConversationSubgroup[]>; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for the findAll method definition in Ad4mModel or its implementation
# to confirm the signature accepts options as third parameter
# Check local Ad4m model implementations
rg -n -A 10 'static\s+(async\s+)?findAll' --type=ts
# Check if there's any documentation or type definition for findAll
rg -n -B 2 -A 5 'findAll.*perspective.*where' --type=tsRepository: coasys/flux
Length of output: 3622
Fix ConversationSubgroup.findAll options placement (AbortOptions as 3rd arg likely ignored)
- In
packages/api/src/conversation/conversation.test.ts,findAllis mocked asstatic async findAll(perspective, opts?)(2 params). The call inpackages/api/src/conversation/index.tspassesAbortOptionsas a 3rd argument (this.perspective, { parent: ... }, options), which won’t match that API and will likely not propagate abort/cancellation. - Adjust the
ConversationSubgroup.findAllcall to passAbortOptionsin the parameter position actually supported by the realAd4mModel.findAllimplementation (or through its expected cancellation mechanism).
🤖 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/conversation/index.ts` around lines 135 - 141, The call in
Conversation.subgroups is passing AbortOptions as a third arg to
ConversationSubgroup.findAll which the mocked/real API treats as a single
options object; fix by passing the abort/cancellation options inside the same
options object passed as the second parameter—i.e. call
ConversationSubgroup.findAll(this.perspective, { parent: { model: Conversation,
id: this.id }, ...(options ?? {}) }) so the abort signal is propagated; update
the subgroups method accordingly (referencing ConversationSubgroup.findAll and
the subgroups method).
Summary
Threads an optional
AbortController.signalthrough every public API method inpackages/api/that wraps aquerySparqlcall. Callers can now cancel long-running queries (timeline loads, embedding fetches, topic lookups) when a view unmounts or a newer query supersedes the in-flight one — the executor short-circuits the JSON reply and the wrappers re-throw the cancellation asDOMException('Aborted', 'AbortError')instead of swallowing it.Backwards compatible — every new parameter is optional.
Why
Long-running SPARQL queries (think
Channel.allItemsover a chatty channel, orSemanticRelationship.allItemEmbeddingswhile embeddings are still being indexed) can ship megabytes of JSON back over the WebSocket. Without cancellation, a Synergy view that unmounts mid-fetch still pays the serialise + transit + deserialise tax, even though nothing renders the result. Now the wrappers honour anAbortSignaland forward it to the executor.Caveat: Oxigraph itself can't be interrupted mid-evaluation, so the blocking thread keeps grinding until the query returns; what's saved is the JSON serialise + WebSocket reply + client deserialise tax, which is the dominant cost for large result sets.
Wire protocol
Built on the executor-side support shipped in coasys/ad4m#855:
The
CallOptionsshape is structurally compatible with ad4m's — Flux uses a localAbortOptions { signal?: AbortSignal }interface (inpackages/api/src/shared/abort.ts, re-exported from the package root) so it doesn't need an ad4m version bump to consume the new executor surface. Anyone consuming the SDK can already pass{ signal }if they're running against an executor with cancellation support; against older executors the signal is harmlessly forwarded and ignored.Methods wired
ChannelallItems,unprocessedItems,totalItemCount,recentConversations,pinnedConversationsConversationstats,topics,subgroups,subgroupsDataConversationSubgroupstats,topics,itemsData,topicsWithRelevanceSemanticRelationshipitemEmbedding,allConversationEmbeddings,allSubgroupEmbeddings,allItemEmbeddings,allItemEmbeddingsByTypeTopiclinkedConversations,linkedSubgroupsEach wrapper:
options?: AbortOptionsas the last parameter.optionsto everyperspective.querySparql<T>(query, options)(andperspective.get(query, options)where applicable) inside the method body.DOMException('Aborted', 'AbortError')instead of swallowing it — so callers can distinguish cancellation from a real query failure. Other errors are still logged + a sensible default returned (matches pre-existing behaviour).conversation/util.tsis an internal helper called only byremoveEmbedding; left alone to keep the diff focused on user-facing API.Reviewer workflow
To verify end-to-end against the companion ad4m PR, use the existing branch-aware build script:
# From the Flux checkout root: BRANCH=feat/query-abort scripts/build-with-ad4m-link.shThat script detects the branch from
BRANCH(or CI env vars), checks whether coasys/ad4m has a matching branch, clones it, builds ad4m/core + ad4m/connect + hooks, rewrites Flux'spnpm.overridestofile:./ad4m/..., clears all caches, and builds Flux.The companion WE PR coasys/we#72 adds a developer-facing equivalent at
scripts/sync-ad4m-branch.shwith explicit--ad4mand--branchflags for local dev outside CI.Test plan
Channel.allItemsforwards the signal toperspective.querySparql.Channel.allItemsre-throwsDOMException('Aborted', 'AbortError').packages/apivitest suite).request.cancelis sent (DevTools → Network → WS frames) — easiest viaBRANCH=feat/query-abort scripts/build-with-ad4m-link.shto pick up the companion executor support.Coordination
Ad4mModel.findAll/modelQuerysignal threading.SchemaRendererand ships ascripts/sync-ad4m-branch.shfor the same workflow.feat/query-abort.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests