Skip to content

refactor(webapp): move version history and diagram loading to TanStack Query - #821

Merged
FelixTJDietrich merged 14 commits into
mainfrom
687-tanstack-query-adoption-review
Jul 18, 2026
Merged

refactor(webapp): move version history and diagram loading to TanStack Query#821
FelixTJDietrich merged 14 commits into
mainfrom
687-tanstack-query-adoption-review

Conversation

@FelixTJDietrich

@FelixTJDietrich FelixTJDietrich commented Jul 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

The web app fetched HTTP data (version history, version snapshots, shared diagrams) with hand-written code: fetch calls wired through optional AbortSignal parameters, cancelled flags inside effects, and a slice of server data living in a Zustand store next to genuine client state. This PR moves version history onto TanStack Query — the list, the immutable snapshot bodies, and the create/rename/delete/restore mutations — so pagination, optimistic updates with rollback, and request deduplication come from one well-tested library instead of bespoke scaffolding.

It is behaviour-preserving by design — the editor looks and works exactly as before (matching the non-goals in #687).

On size, stated honestly: useVersionStore drops from 563 to ~170 lines and now holds only client state, and the five files the issue calls out shrink by a net ~336. But the app's shipped code grows by roughly 670 lines, because the ~730-line src/queries/ layer is new. This PR does not make the codebase smaller. What it buys is a single shared cache for version bodies (fewer requests — thumbnails, preview entry and the dirty-check baseline now share one fetch), real cursor pagination, and a rollback primitive replacing hand-written splice-restore logic. Bundle cost is +10 kB gzipped against a 1300 kB budget.

One framing in the original issue does not survive contact with the code, and is worth correcting: the "delete duplicated fetch boilerplate" argument is thin here. origin/main had exactly one hand-rolled cancelled flag (VersionThumbnail) and two AbortControllers (ApollonShared); loading/error handling was already centralised in one store rather than duplicated across call sites.

The collaborative document itself is untouched: Yjs over the WebSocket stays the single source of truth for the live canvas. TanStack Query owns request/response HTTP only, and that boundary is documented at the top of queryClient.ts. The debounced autosaver and the pagehide flush are deliberately left out of Query (a keepalive beacon can't run through a cancellable mutation) — see the implementation notes.

Closes #687.

Release note

No visible change to the editor. Under the hood, the web app now loads and caches version history, version snapshots, and shared diagrams through one shared data layer instead of hand-written fetching, so the version panel refreshes and reconciles more consistently across tabs and collaborators.

Implementation notes

A few decisions are load-bearing and worth a reviewer's attention. Each is documented next to the code it governs.

  • Query hooks sit on the existing VersionRepository port, not on the REST client. Version history runs against two backends — REST for shared diagrams, IndexedDB for local ones — behind one port. The hooks resolve the adapter by a kind ("local" | "remote") that the editor route declares via a small React context. Modals and toast bodies render at the app root, outside that route subtree, so they take kind as a prop instead. (An earlier revision used a mutable module global for this; a review found it could re-key the outgoing page's queries onto the wrong backend during a /local → /shared navigation, so it was replaced.)

  • The editor seed is deliberately NOT a query. The body the editor mounts with is an initialisation input, not server state: Yjs owns the document from mount onwards, so it must never be refetched or replayed from a cache. Expressing that through Query meant disabling caching in both directions (staleTime: Infinity, gcTime: 0) — a plain fetch wearing a cache's clothes — so it is now useDiagramSeed, one abortable fetch per diagram. The two imperative "give me the latest HEAD" reads (peer restore, preview exit) each own an AbortController, so one caller's teardown cannot abort another's request.

  • No optimistic rows are forged into the cache. The version-create mutation returns its settle-time invalidation promise, so it stays "pending" until the refreshed list lands; the drawer renders the in-flight row straight from that pending state. (An isSuccess-derived row turned out to be sticky and could resurrect a just-deleted version.)

  • Autosave and flush-on-unload stay outside Query. They keep their own retry/rebase and keepalive semantics, which mutations can't express; this is stated in queryClient.ts so the boundary is explicit.

While migrating, the review pass also surfaced and fixed a handful of edge cases in the same code: an undo-restore toast that read its backend from a context it renders outside of (it now takes the value as a prop); a peer deleting the version you're previewing no longer leaves a stale snapshot on the canvas; and a failed history load now shows a "couldn't load" surface instead of the "no versions yet" empty state. Each has a regression test that fails when the fix is reverted.

Not done here / follow-ups (non-blocking):

  • The version-list query uses refetchOnWindowFocus, and v5 refetches all loaded pages on focus. For very long histories a maxPages bound would cap that; left out until a profile shows it matters.
  • A peer's restore can still momentarily clobber an active local preview overlay. This predates the PR and isn't touched.

Steps for testing

Everything below is behaviour that should be unchanged — the point is to confirm the migration didn't regress anything.

  1. Collaborative load & share-again. Open a shared diagram — the loading overlay shows then clears, the canvas mounts, peers connect. From inside it, "Share again" to a new URL: the overlay re-shows, no blank screen, no stale model.
  2. Version panel. Open the version drawer, save a version (the "Saving…" row should hand over to the real row with no gap), rename one, delete one — the deleted row must not reappear. Scroll a long history and hover thumbnails: only visible rows fetch (check the Network tab).
  3. Undo restore. Restore a version in a collaborative session, then click Undo on the toast — it must complete without error. (This path threw before the fix in this PR; the regression test is UndoRestoreToast.test.tsx.)
  4. Peer delete during preview. While previewing a version, have a second client delete that version — your canvas must exit preview rather than keep showing the deleted snapshot.
  5. Local (offline) mode. Repeat the version-panel steps at /local/:id to confirm the IndexedDB backend shares the same code path.

On collaborative coverage, stated plainly: the webapp's e2e stack serves the bundled frontend only — no API server, no Redis, and no existing spec touches /shared — so a genuine two-browser collaboration test cannot run in CI without adding server infrastructure to every PR. Instead, ApollonShared.collab.test.tsx drives the exact ControlEvent sequence the server publishes to a peer through the real page, store, query cache and event bridge, doubling only the WebSocket, HTTP and editor. That covers this client's reaction to a collaborator — where every defect found while building this layer lived — and not the server's behaviour, which the server suite owns. Steps 3–4 above are still worth doing by hand once.

Automated, on this branch:

  • pnpm --filter @tumaet/webapp test → 291 passing (44 files; 287 on main). New: the query layer, the collaborator-actions spec, and a regression test for each defect fixed here.
  • pnpm --filter @tumaet/webapp exec tsc -b --noEmit, pnpm lint, pnpm build → clean.

Screenshots / screencasts

No UI change — this is behaviour-preserving plumbing, so there is nothing new to show.

Checklist

  • Linked to a related issue — closes Adopt TanStack Query as the webapp's HTTP data layer #687
  • Added a changeset in the user's voice (@tumaet/webapp, patch)
  • PR title's Conventional Commit type (refactor) matches the kind of change
  • Tests added or updated — 5 new query-layer test files + regression tests for each fix
  • Ran pnpm lint && pnpm build && pnpm test locally — green (pnpm format:check is clean for all committed files)
  • Documentation updated — n/a (no public API or docs change)
  • Screenshots or screencasts — n/a (no UI change)

FelixTJDietrich and others added 7 commits July 17, 2026 20:34
@FelixTJDietrich
feat(webapp): adopt TanStack Query as the HTTP data layer
695706b 
Replaces the hand-rolled fetch scaffolding in React with TanStack Query
5.101.2: ad-hoc AbortController plumbing, `cancelled` flags inside effects,
and the server state that had accumulated inside `useVersionStore` all go
away. Closes #687.

Boundary: Query owns request/response server state (diagram seed, version
list, version bodies). Yjs over WebSocketManager stays the source of truth
for the mounted editor, and the debounced autosaver + `useFlushOnUnload`
stay outside Query — v5 mutations have no AbortSignal, and the
If-Match/REVISION_MISMATCH rebase is bespoke either way.

Query hooks sit ON the existing `VersionRepository` port rather than on
`VersionApiClient`, so local (IndexedDB) mode shares one implementation
with collab mode; keys embed `repo.kind`.

Load-bearing decisions, each documented at its definition:

- Two diagram keys. `diagramKeys.seed` is the one-shot editor seed
  (staleTime Infinity, gcTime 0). Imperative HEAD reads use
  `diagramKeys.head` so a fresh body can never change the seed's data
  identity and rebuild the editor + WS + autosaver from the mount effect.
- `head` keys carry a `reason` ("peer-restore" | "preview-exit").
  `cancelQueries` matches by key, so a shared key let the preview effect's
  cleanup abort the WS handler's in-flight peer-restore refresh — leaving
  the user with a "X restored a version" toast and a stale canvas.
- The create mutation's `onSettled` returns its invalidation promise, so
  the mutation stays pending until the refreshed list lands and the
  drawer's optimistic row can derive from `isPending` alone. Deriving it
  from `isSuccess` made it sticky mutation state that resurrected a
  version after it was deleted.
- `useBoundRepository()` snapshots the adapter at mount: the editor routes
  rebind the registry in `beforeLoad`, which runs while the outgoing page
  is still mounted, so a render-time read let a /local -> /shared hop
  re-key the outgoing page's queries onto the wrong backend.
- Mutation side effects live on mutation options, not `mutate()` call
  sites: call-site callbacks are skipped when the caller unmounts, which
  would leak `pendingRestoreFromId` and misclassify a peer's restore.
- `fetchNextPage({ throwOnError: true })` — without it the promise never
  rejects and the "Failed to load more versions." toast is unreachable.

`useVersionStore` holds only synchronous client state now (drawer, preview,
undo window, in-flight-restore marker) — 563 lines to 141
(`git show origin/main:standalone/webapp/src/stores/useVersionStore.ts | wc -l`
vs `wc -l` on the branch).
`versionStoreBootstrap`'s visibilitychange loop is replaced by
`refetchOnWindowFocus` on the version-list query, which covers exactly the
mounted-consumer set the loop iterated.

Net -328 LOC across the five files the issue names (ApollonShared.tsx,
useVersionStore.ts, VersionThumbnail.tsx, VersionDrawer.tsx,
DiagramApiClient.ts) per `git diff --stat origin/main -- <those five>`:
447 insertions, 775 deletions. Bundle 1.13 -> 1.14 MB gzipped against a
1.3 MB limit, both from `pnpm run size` on each ref; devtools self-strip
from production.

Tests: 264 pass (+3). New coverage at the query layer (seed contract and
gcTime 0, head-key cancellation scopes, pagination cursors, body `enabled`
gate, mutation rollback/eviction/restore-window, the control-event bridge)
plus the two drawer failure surfaces that had none (REDIS_UNAVAILABLE copy,
load-more toast). Four of the five ApollonShared revert-proofs still bind;
the unmount-race one is now structural rather than guard-based.
@FelixTJDietrich @claude
refactor(webapp): resolve version adapters by kind from route context
b83522b 
Replaces the mutable module-global repository registry and its mount-time
snapshot with a backend kind supplied by the editor routes:

- getVersionRepository(kind) resolves from a static map, so a queryFn reads
  only values that are already in its query key. Both
  @tanstack/query/exhaustive-deps suppressions are gone and the rule keeps
  protecting the cache. The kind was never honestly derivable from a global
  the routes rebind while the outgoing page is still rendering.
- Routes declare the backend via VersionRepositoryProvider. Modals mount
  under ModalProvider at the app root, outside the page subtree, so they
  take kind as a prop from whoever opened them.
- Deletes useBoundRepository and the /local -> /shared re-key race it
  existed to defend against.

Tests: cut the theatre an audit surfaced (assertions that survive deleting
the code under test) and replace it with revert-proof coverage — WS
self-restore suppression driven through a real control event, create's
awaited settle-invalidation, the mid-flight optimistic patch, and
kind-keyed cache isolation between the two backends. 260 tests, down from
264, each new one verified to fail when its code is reverted.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@FelixTJDietrich @claude
fix(webapp): resolve the undo toast's backend outside the provider
449eb33 
react-toastify renders toast content at its ToastContainer, which __root.tsx
mounts as a sibling of the routed page — outside the editor route's
VersionRepositoryProvider. UndoRestoreToastBody read the kind from context
there, so every completed collab restore threw
"useVersionRepositoryKind must be used within a VersionRepositoryProvider".

Nothing caught it: ApollonShared.test.tsx mocks the toast to null, and
UndoRestoreToast.stories.tsx mounts its own container inside the story
subtree — where the global decorator does provide the context.

The driver does render inside the route, so it resolves the kind and passes
it down, the same escape hatch DeleteVersionModal already uses. The new test
mirrors __root.tsx's shape (container as a sibling of the provider) and
reproduces the throw when reverted.

Also from the same review pass:

- A failed background refetch no longer wipes a loaded version list. The
  drawer derived its error surface from `isError`, which v5 also sets when a
  refetch fails while data survives — and that query has
  refetchOnWindowFocus, so a blip on tab-focus replaced the user's rows with
  the REDIS_UNAVAILABLE banner. Only an initial-load failure does that now.
- Query failures log once per request via QueryCache.onError rather than
  once per observer.
- Cover the cancelQueries step: an in-flight list refetch that resolves after
  an optimistic edit must not revert the row. Removing cancelQueries
  previously broke no test.
- Rewrite the control-event tests to assert what a reader of the list sees
  instead of that invalidateQueries was called; they now fail on revert.
- One stubVersionRepository factory replaces four ad-hoc stubs, and its
  unstubbed methods reject rather than returning undefined.
- Pages read the kind their route declared instead of re-asserting a literal.
- Correct stale comments the refactor invalidated (the context's "modals are
  the only escape hatch" claim, the story helpers' store-seeding header, and
  two references to adapters the files no longer touch).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@FelixTJDietrich @claude
fix(webapp): exit preview on peer delete; replace the legal resolver …
0ddaeb6 
…prop with a module seam

Third-review pass. Two defects and a pile of stale comments.

- A peer deleting the version you are previewing left the canvas rendering
  a phantom snapshot: the WS VERSION_DELETED branch dropped the row and its
  body but never exited preview. It now does, matching the local
  BroadcastChannel path.

- LegalPage keyed its query on (page, profile) while reading `resolver`
  from a prop the key omitted — two <LegalPage> with different resolvers
  and the same key cross-read for the session, and the eslint suppression
  hid it. Replaced the prop with a `setLegalResolver` module seam (the same
  shape as setVersionRepository); the key is now honest and the last
  query/exhaustive-deps suppression is gone.

- A non-Redis initial-load failure rendered the "No versions yet" empty
  state — telling the user their history is empty when it only failed to
  load. It now shows a generic failure surface; the Redis branch is kept.

Comments: correct the ones that lied — DIAGRAM_DELETED named a handler that
was never written; versionQueries' "a closed drawer costs nothing" is false
for the page-level local subscription; versionListCache named a consumer
that doesn't import it; the stub's "fails loudly" claim (rejections are
caught and logged, not thrown). Delete duplicated rationale (body-dedup
stated 6x, create-pending 4x) and type-system-enforced warnings.

Dead code: merge the one-interface types/versioning.ts into types/api.ts;
inline versionKeys.bodies (no prefix consumer); drop WithQueryOptions and
the wrapWithQueryClient tuple (no caller passed opts or read the client);
un-export makeStoryRepository; remove the story-support restore dance that
contradicted its own comment. stubVersionRepository now reports the correct
local cap.

Tests: cover peer-delete-exits-preview and the non-Redis load surface;
delete the trivial "forwards the abort signal" test (library contract,
already covered by the cancellation-scope test); rename the stale
"infinite render loop" describe (that regression died with selectVersions).

264 tests pass; tsc and eslint clean; bundle unchanged at 1.14 MB.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@FelixTJDietrich @claude
fix(webapp): key the thumbnail SVG cache by backend kind
aed1828 
The module-level rendered-SVG cache keyed on diagramId/versionId only, so
a local and a remote diagram sharing an id would serve each other's
thumbnail. Include the kind, matching the version query keys.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@FelixTJDietrich @claude
refactor(webapp): drop the redundant importDiagram cast in the thumbnail
500482b 
`importDiagram` already returns `UMLModel`, so the `as UMLModel` on the
body-query data was noise. Removing it keeps the query data path cast-free.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@FelixTJDietrich @claude
docs(webapp): add changeset for the TanStack Query migration
a4af6b9 
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claudia-Anthropica
Claudia-Anthropica requested changes Jul 17, 2026
View reviewed changes

@Claudia-Anthropica Claudia-Anthropica left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FelixTJDietrich The query migration is thoughtfully structured and well tested overall. One mutation-lifecycle regression can leave the collaborative autosaver with a stale HEAD revision when the version panel closes during a save; please see the inline comment.

Comment thread standalone/webapp/src/components/versioning/VersionDrawer.tsx Outdated
@github-project-automation github-project-automation Bot moved this from Backlog to In progress in Apollon Development Jul 17, 2026
FelixTJDietrich and others added 2 commits July 18, 2026 22:21
@FelixTJDietrich @claude
Merge origin/main into the TanStack Query migration
ad6ca4a 
Resolves three conflicts and the semantic fallout of #817's save shortcut:

- useVersionStore: keep main's `saveRequestByDiagram` nonce and its
  `requestSave`/`clearSaveRequest` actions — those are client state and
  belong in the pruned store — while the server-state fields stay removed.
- VersionDrawer's baseline effect: keep main's `stale` out-of-order guard
  and `baselineVersionId`, but fetch the body through the query cache.
- `initialListLoaded` came from `versions[id] !== undefined || error[id]`,
  both of which now live in the query cache; it reads the list query's own
  pending state instead.
- VersionDrawer.saveShortcut.test: migrated onto the query cache — the peer
  save it simulates is now a `setQueryData` on the list key rather than a
  store write.

Two real issues the merge surfaced, both fixed here:

- The shared body cache meant a thumbnail could run its expensive SVG export
  while still off-screen, because another consumer (preview entry, or the
  drawer's dirty-check baseline) had already populated that cache entry. The
  export is now gated on visibility, not just the fetch.
- The save shortcut could fire in the same effect flush where `hasChanges`
  was still a render stale, writing a duplicate version. It now compares the
  structural fingerprint directly instead of the lagging state.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@FelixTJDietrich @claude
fix(webapp): keep the headRev handoff alive when the version panel cl…
edd71ee 
…oses

Review catch on #821. `onVersionSaved` ran from a `mutate()` call-site
callback, which TanStack Query skips when the calling component unmounts —
and closing the version rail or sheet does exactly that. The create endpoint
still bumps `headRev`, so the page's autosaver was left dirty on a stale
revision and the next save paid a REVISION_MISMATCH round-trip.

`useCreateVersionMutation` now takes an `onCommitted` option and invokes it
from the mutation's own `onSuccess`, which runs regardless of the caller's
lifecycle. The panel keeps only genuinely panel-local UX (clearing the
composer) at the call site — the split the module header already documents.

The regression test unmounts the hook's consumer before the create resolves
and asserts the callback still fires with the committed headRev; moving the
handoff back to the call site turns it red.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claudia-Anthropica
Claudia-Anthropica approved these changes Jul 18, 2026
View reviewed changes

@Claudia-Anthropica Claudia-Anthropica left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FelixTJDietrich The mutation lifecycle issue is fixed and covered by the unmount regression test. All feedback addressed, nice work.

@FelixTJDietrich @claude
refactor(webapp): make the Query devtools opt-in instead of always on
a2ab404 
The devtools floated a toggle button over the editor's own floating chrome
in every dev session, where it was in the way far more often than it was
useful. They now render only when a developer opts in per browser with
`localStorage.setItem("apollon:query-devtools", "1")`.

Production is unchanged: the package already swapped itself for a no-op
export outside development, and the built bundle still contains no devtools
runtime.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claudia-Anthropica
Claudia-Anthropica approved these changes Jul 18, 2026
View reviewed changes

@Claudia-Anthropica Claudia-Anthropica left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FelixTJDietrich The devtools opt-in change is clean, and the TanStack Query migration remains well structured and thoroughly covered. Approving the new head.

@FelixTJDietrich @claude
docs(webapp): document the Query devtools flag in the README
fd892b3 
The opt-in was only discoverable by grepping AppProviders.tsx, which
excludes anyone who doesn't already know the devtools exist. The README now
names the flag next to the rest of the webapp's dev setup, and says why the
tool is off by default.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claudia-Anthropica
Claudia-Anthropica approved these changes Jul 18, 2026
View reviewed changes

@Claudia-Anthropica Claudia-Anthropica left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FelixTJDietrich The README now makes the opt-in Query devtools flag discoverable and accurately documents why it stays disabled by default. The TanStack Query migration remains clean and well covered; approving the new head.

@FelixTJDietrich @claude
fix(webapp): stop the version list refetching for users who never ope…
1cddd68 
…n it

An adversarial audit of this PR caught a real regression it introduced.

Before this PR, `versionStoreBootstrap` refetched version history on
`visibilitychange` and iterated open drawers only — `if (!open) continue` —
reloading the first page. This PR had replaced that with
`refetchOnWindowFocus: true` on the shared list options, which is broader on
three axes at once: window focus fires more often than visibilitychange,
every mounted observer refetches (the local editor page subscribes for the
whole session, drawer shut, to label its restore dialog), and v5 refetches
all loaded pages. A user who never opens the panel went from 0 requests per
focus to 1; one who had paged back four times went from 1 to 5.

Focus refetching is now opt-in and the open panel is the only caller that
opts in — it is mounted only while open, which reproduces the old
open-drawers-only bargain. Two tests pin both halves; flipping the default
back on turns the first red.

Also drops the LegalPage changes from this PR entirely. Moving legal content
into a query was unrelated to version history, and the module-level resolver
seam it needed was a testability regression traded for a problem this PR had
created. Reverted to main; it can be revisited on its own merits.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claudia-Anthropica
Claudia-Anthropica approved these changes Jul 18, 2026
View reviewed changes

@Claudia-Anthropica Claudia-Anthropica left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FelixTJDietrich The focus-refetch opt-in restores the previous open-panel-only behavior and the added tests cover both observer modes. The unrelated legal-page changes are cleanly removed; approving the new head.

@FelixTJDietrich @claude
refactor(webapp): make the editor seed a plain fetch, and cover peer …
4840de7 
…actions

Two changes that share `ApollonShared.tsx`, so they land together.

1. The editor seed is no longer a query. The diagram body the editor mounts
   with is an initialisation input, not server state: after mount Yjs owns
   the document, so it must never be refetched, revalidated, or replayed
   from a cache on remount. Getting that from TanStack Query meant
   `staleTime: Infinity, gcTime: 0` — caching disabled in both directions,
   a plain fetch wearing a cache's clothes — plus a two-key
   `seed`/`head(id, reason)` split with cancellation-scope discriminators.

   `useDiagramSeed` replaces it: one abortable fetch per diagram, pending
   derived from whether the settled result matches the requested id, so
   switching diagrams reports pending on the first render rather than
   briefly offering the previous body. The two imperative HEAD reads get
   their own `AbortController`s again, which structurally removes the bug
   class the `reason` discriminator existed to prevent. Deletes
   `queries/diagramQueries.ts` and the diagram half of the key factory.
   Query now covers only what it is good at here: the version list, version
   bodies, and version mutations.

2. New coverage for what a collaborator's actions do to this client. The
   webapp e2e stack serves the bundled frontend only — no API server, no
   Redis, no spec touches /shared — so a genuine two-browser test cannot run
   in CI. `ApollonShared.collab.test.tsx` is the closest honest substitute:
   the real page, store, query cache and control-event bridge, driven by the
   exact ControlEvent sequence the server publishes to a peer, with only the
   WebSocket, HTTP and editor doubled.

   Writing it surfaced two defects, both fixed here. A peer's restore
   assigned the fetched HEAD onto `editor.model` while this client was
   previewing — but that is the read-only overlay in preview mode, so the
   snapshot under inspection silently became something else and the write
   was discarded on exit anyway. And a peer deleting the version you were
   previewing cleared the store but not `?version=`; since the URL is the
   source of truth, the sync effect put you straight back into previewing a
   version the server no longer has. Clearing the param is the page's job,
   so the cache bridge — which cannot reach the router — no longer half-does
   it. Both turn red when their guard is reverted.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@FelixTJDietrich
FelixTJDietrich force-pushed the 687-tanstack-query-adoption-review branch from a43df81 to 4840de7 Compare July 18, 2026 22:15
Claudia-Anthropica
Claudia-Anthropica requested changes Jul 18, 2026
View reviewed changes

@Claudia-Anthropica Claudia-Anthropica left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FelixTJDietrich The Query migration remains thoughtfully structured and well tested, but the local cross-tab deletion path can keep a deleted snapshot preview alive through the immutable body cache. Please fix that reconciliation issue; I also noted two stale descriptions from the latest seed refactor.

Comment thread standalone/webapp/src/stores/versionStoreBootstrap.ts Outdated
Comment thread standalone/webapp/README.md Outdated
@FelixTJDietrich @claude
fix(webapp): reconcile a cross-tab delete of the previewed version
495c5a5 
Review catch on #821. When another tab deleted the version this tab was
previewing, the local BroadcastChannel path cleared only the Zustand
preview. But `?version=` is the source of truth: `useVersionPreviewUrlSync`
immediately re-entered, and because the snapshot body query is
`staleTime: Infinity` and this path never removed the deleted body key, it
served the cached snapshot straight back without touching IndexedDB — so the
deleted version stayed previewed indefinitely.

The path now evicts that body query before exiting preview. Its re-entry then
misses the cache, reads IndexedDB, gets a 404, and the URL sync's existing
error path strips `?version=` — which keeps this non-React singleton out of
routing. The bootstrap test is extended through the body-cache path: it warms
the body, then asserts it is evicted and a re-read 404s. Reverting the
eviction turns it red.

Also corrects the README and changeset, which still claimed diagram/shared
loads go through TanStack Query after the editor seed moved to
`useDiagramSeed`. Both now scope Query to version history and note the
deliberate seed exception.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claudia-Anthropica
Claudia-Anthropica approved these changes Jul 18, 2026
View reviewed changes

@Claudia-Anthropica Claudia-Anthropica left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FelixTJDietrich Both requested changes are fixed and covered by the updated tests. All feedback addressed, nice work.

@FelixTJDietrich
FelixTJDietrich merged commit d04150a into main Jul 18, 2026
26 checks passed
@FelixTJDietrich
FelixTJDietrich deleted the 687-tanstack-query-adoption-review branch July 18, 2026 22:44
@github-project-automation github-project-automation Bot moved this from In progress to Done in Apollon Development Jul 18, 2026
@github-actions github-actions Bot mentioned this pull request Jul 18, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Claudia-Anthropica Claudia-Anthropica Claudia-Anthropica approved these changes

Assignees

No one assigned

Labels

None yet

Projects

Apollon Development
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Adopt TanStack Query as the webapp's HTTP data layer

2 participants

@FelixTJDietrich @Claudia-Anthropica