Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 25 additions & 2 deletions CONTEXT.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ A directed relationship between two vertices (source → target), with a type an
_Avoid_: Relationship, link

**Graph Database**:
The external graph database a user connects to and explores — the source of all vertices and edges, reached over HTTP via a Connection. It is the user's own data, brought along and queried live; distinct from the local persisted app state (connections, schema cache, styles, sessions, layout) that Graph Explorer keeps in the browser's IndexedDB.
The external graph database a user connects to and explores — the source of all vertices and edges, reached over HTTP via a Connection. It is the user's own data, brought along and queried live; distinct from the local app state (connections, schema cache, styles, sessions) that Graph Explorer keeps in the browser's IndexedDB, plus per-tab layout in sessionStorage.
_Avoid_: Database (ambiguous — clarify remote graph database vs. local persisted state)

**Connection**:
Expand Down Expand Up @@ -74,7 +74,7 @@ The set of vertices and edges a user has loaded through exploration for a given
_Avoid_: State, workspace

**Graph View**:
The interactive canvas where vertices and edges are visualized using Cytoscape.js. Users explore the graph here by expanding neighbors and applying layouts. Nav label: "Graph".
The interactive canvas where vertices and edges are visualized using Cytoscape.js. Users explore the graph here by expanding neighbors and applying a Layout. Nav label: "Graph".
_Avoid_: Graph Explorer (ambiguous with the product name)

**Data Table View**:
Expand All @@ -85,6 +85,26 @@ _Avoid_: Data Explorer (legacy route name)
Visual representation of the Schema — shows vertex types and their edge connections as a graph.
_Avoid_: Schema Explorer (legacy route name)

**Layout**:
The algorithm that positions vertices on the Graph View canvas, chosen from the layout picker and run by Cytoscape (`LayoutName`). The unqualified word always means this.
_Avoid_: View Layout (a different concept, below), graph arrangement

**View Layout**:
The per-tab UI state of a view: which sidebar panel is active, how wide the sidebar is, and which content is toggled on. A per-tab Storage Scope concept, so it survives a tab's reload but not its close, and a fresh tab starts from the View Layout most recently used. The two are Graph View Layout and Schema View Layout. Never shortened to Layout, which is the positioning algorithm.
_Avoid_: Layout (means the algorithm), preferences, settings

**Graph View Layout**:
The View Layout for the Graph View — active sidebar panel, sidebar width, active content toggles, table-view height, and the details-auto-open preference.
_Avoid_: Graph preferences, graph settings

**Schema View Layout**:
The View Layout for the Schema View — active sidebar panel, sidebar width, and the details-auto-open preference.
_Avoid_: Schema preferences, schema settings

**Storage Scope**:
The cross-tab behavior a persisted atom picks at creation, so scope is a visible decision rather than a side effect of which factory was reached for. Three named scopes: **per-tab**, where tabs diverge and a fresh tab starts from the value most recently used; **shared-reconciled**, where a Map-keyed collection is merged per key across tabs; and **shared-blind-write**, where each write is the whole value. See the `per-tab-session-scoped-storage-primitive` ADR for which atoms use which, and `per-key-diff-merge-cross-tab-reconciliation` for the merge rule.
_Avoid_: Persistence mode, storage strategy
Comment on lines +104 to +106

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor, and the term itself clearly earns its place — it names a decision the code really does make at atom creation.

The length is what I'd push back on. CONTEXT-FORMAT.md asks for "one or two sentences max" and "define what it IS, not what it does," and docs/agents/domain.md splits what into the glossary and why into ADRs. This entry enumerates all three scopes with their merge and divergence mechanics and points at two ADRs, which reads closer to an ADR abstract than a definition. The neighbouring Layout and View Layout entries are tight by comparison.

Something like: "The cross-tab persistence behavior a persisted atom picks at creation — per-tab (tabs diverge), shared-reconciled (merged per key), or shared-blind-write. See the storage ADRs for which atoms use which." The per-scope mechanics are already in the new ADR, so restating them here is the part that could go.

Entirely your call on which detail an agent needs inline versus behind a link — that is the judgement the glossary exists to encode, so I'm not going to be confident about it from outside.


**Edge Connection**:
A schema-level pattern describing how two vertex types can be related via an edge type: sourceVertexType --[edgeType]--> targetVertexType. What the Schema View visualizes. Not an actual edge instance.
_Avoid_: Relationship (Gremlin UI term), Object Property (SPARQL UI term)
Expand Down Expand Up @@ -149,6 +169,9 @@ _Avoid_: Save-status indicator
- **Neighbors** are **Vertices** one hop away from a given **Vertex**
- **Styles** are scoped per **Vertex Type** (**Vertex Styles**) and **Edge Type** (**Edge Styles**)
- The **Graph View**, **Data Table View**, and **Schema View** all render from the same **Session** and **Schema**
- Each browser tab has its own **View Layout** per view, the same divergence as **Active Connection**
- A **Layout** positions **Vertices** on the **Graph View** canvas and is not part of any **View Layout**

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A question about how absolute you want this, because my in-flight #2144 collides with it and I am not assuming the glossary is the side that should bend.

This invariant, plus the Layout entry's _Avoid_: View Layout (a different concept) and the closed enumeration in Schema View Layout ("active sidebar panel, sidebar width, and the details-auto-open preference"), together say a Layout is never part of a View Layout. #2144 adds a required layoutAlgorithm: LayoutName field directly to SchemaViewLayout and defaults it in transformSchemaViewLayout. The moment it lands, a Layout is part of a View Layout and that enumeration is incomplete. Since this PR is sequenced first, the text would be merged as settled fact and then falsified.

I can see two resolutions and I think the choice is yours as the owner of the domain model:

  1. The invariant is what you meant, and Persist schema view layout selection #2144 is wrong. Then layoutAlgorithm does not belong in SchemaViewLayout at all and I move it to its own atom in Persist schema view layout selection #2144. Costs me an atom and keeps the concept boundary sharp, which is a real benefit: it would mean "View Layout" stays purely UI chrome.
  2. The invariant is over-stated. Then scoping it to the Graph View ("it is the algorithm, not part of the Graph View Layout's UI state") and opening the Schema View Layout list with "such as" costs one line here and saves a doc-correcting commit in Persist schema view layout selection #2144.

For what it is worth, #2145 does not conflict — its Graph Arrangement entry also treats Layout as the algorithm only, carrying _Avoid_: Layout (only the algorithm). It does add a sixth Layout-adjacent term to this region of the glossary, so whichever way this goes, the six read better if they agree on where the algorithm lives.

Happy to take the work in #2144 either way. I just want the decision made here rather than discovered later.

- Every persisted atom picks one of the three **Storage Scopes** at creation

## Example dialogue

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ The unit of reconciliation is the **key** (collection entry — e.g. one **Verte
Reconciliation is an **optional `reconcile` parameter on `atomWithLocalForage`**, not separate machinery. An atom opts in by being created with a reconciler function; without one the atom does a blind whole-value write (the parameter selects the flush at creation — a plain `storage.setItem`, or the re-read-merge-write built by `createReconcilingFlush`). This is deliberately **opt-in rather than required** because reconciliation is correct for only a minority of atoms:

- **Map-keyed shared collections reconcile.** The five `Map`-keyed atoms — **Connections** (`configuration`), **Schema**, **User Preferences** (`user-vertex-styles`, `user-edge-styles`, since #1867 split styling into type-keyed maps), and **Sessions** (`graph-sessions`) — all pass the one generic reconciler, `reconcileMapByKey`.
- **Scalars must not.** Layout and the boolean/number settings have no sibling entries to preserve — each write is the whole intended value — so a per-key merge is meaningless for them.
- **Scalars must not.** Layout and the boolean/number settings have no sibling entries to preserve — each write is the whole intended value — so a per-key merge is meaningless for them. (Layout has since moved from shared to per-tab session scope — see ADR `per-tab-session-scoped-storage-primitive` — so it is no longer a shared scalar at all; the boolean/number settings remain the standing examples here.)
- **`activeConfiguration` must not.** It deliberately _diverges_ per tab (the #1788 inverse); reconciling it would reintroduce the very behaviour that decision avoids.

There is also no universal default reconciler: a reconciler must know the value is key-addressable (`reconcileMapByKey` only works on `Map`s). Making reconciliation _required_ would force every scalar atom to pass a nonsensical reconciler, so "required" would in practice still be "choose a reconciler" with worse ergonomics. The cost of opt-in is that a **new** `Map`-keyed shared collection added with plain `atomWithLocalForage` would silently clobber across tabs until someone notices — mitigated by the rule below rather than by flipping the default.
Expand Down
2 changes: 1 addition & 1 deletion docs/adr/20260618-per-tab-active-connection.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ Split the concept into two roles:

A fresh tab's Active Connection is seeded with `sessionStorage value ?? persisted breadcrumb value`, resolved before the atom is created (the breadcrumb read is async, in the factory; the atom itself is then created with an already-resolved seed, so there is no post-mount flash or ordering race). On a cold start — no sessionStorage value — the tab does not merely read the breadcrumb, it **claims** it, writing the seeded value into its own sessionStorage. This is the load-bearing property for per-tab stability: a later reload of that tab reads its own value back rather than re-seeding from a breadcrumb another tab may have since overwritten. (Without the claim, two tabs that both cold-started would each re-seed from the shared breadcrumb on every reload, so one tab switching connections would silently change the other on its next reload.)

Writing the per-tab value and the breadcrumb is funneled through the `activeConfigurationAtom` setter (in `activeConnectionStorage.ts`), so every existing `set(activeConfigurationAtom, …)` call site updates both backings with no change. Session-state reset is not part of this atom; each activation call site still pairs the set with `useResetState()` — a known duplication that a single `useActivateConnection` hook could centralize as a clean additive follow-up.
Writing the per-tab value and the breadcrumb is funneled through the `activeConfigurationAtom` setter (in `activeConnectionStorage.ts`), so every existing `set(activeConfigurationAtom, …)` call site updates both backings with no change. (**Updated 2026-09-21:** the seed-and-claim logic described here was extracted to the generic `createSessionScopedAtom` in `sessionScopedStorage.ts`, which now backs three concepts; `activeConnectionStorage.ts` is a codec plus one call. See ADR `per-tab-session-scoped-storage-primitive`. Behavior is unchanged.) Session-state reset is not part of this atom; each activation call site still pairs the set with `useResetState()` — a known duplication that a single `useActivateConnection` hook could centralize as a clean additive follow-up.

## Considered Options

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,5 @@ The depth is hidden behind composition, not crammed into one file: `classifyStor
- **Status is strictly per-tab.** Consistent with the substrate ADR's "no cross-tab sync primitives," a failed write in one tab shows `failed` there regardless of other tabs, and clears only when that tab itself successfully flushes the key. Honest about whose edit is at risk.
- **Recovery is retry + backup, not a write guarantee.** The failure indicator opens a detail dialog; for terminal-quota failures the dialog offers a full configuration backup (`saveLocalForageToFile`), which is read-mostly and so remains viable under quota pressure. Terminal-access failures (private mode, blocked) offer no backup — IndexedDB never opened, so there is nothing to read. We do **not** block reload (`beforeunload`).
- **This effort and the cross-tab merge are orthogonal**, meeting only at the `flush` seam — either can ship first without blocking the other.
- **Every IndexedDB write routes through one shared queue, including the Active Connection breadcrumb.** The queue/status are not embedded in `atomWithLocalForage`; they live in a shared `persistThroughQueue` helper that wraps the localForage write itself. Both `atomWithLocalForage` and the per-tab active-connection path (whose synchronous sessionStorage write stays outside the queue) call it, so all IndexedDB writes feed one global status. This dropped the special-case that would have excluded the breadcrumb.
- **Every IndexedDB write routes through one shared queue, including the Active Connection breadcrumb.** The queue/status are not embedded in `atomWithLocalForage`; they live in a shared `persistThroughQueue` helper that wraps the localForage write itself. Both `atomWithLocalForage` and the per-tab path (whose synchronous sessionStorage write stays outside the queue) call it, so all IndexedDB writes feed one global status. (**Updated 2026-09-21:** that per-tab path is now the generic `createSessionScopedAtom`, backing the active connection plus both view layouts. A failed sessionStorage write is logged and swallowed rather than reported, since the breadcrumb write still goes through the queue; see ADR `per-tab-session-scoped-storage-primitive`.) This dropped the special-case that would have excluded the breadcrumb.
- **The `void`-setter change lives in the shared `createWriteThroughAtom`**, which the active-connection path also uses. No production call site awaited the old promise (only tests did), so the migration was mechanical. The test seam moved to a layer-level `waitForIdle()` on the status store, replacing per-write promise awaits across the persistence test helpers and the active-connection tests.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ Persisted state in IndexedDB (via `atomWithLocalForage`) reloads in its stored s

Reshape the value **on read**, via a `transform` option on `atomWithLocalForage`: `transform: (loaded: T) => T` runs on the preloaded value before it seeds the atom. The transform lives beside its type (`transformGraphViewLayout` / `transformSchemaViewLayout`, sharing `transformLegacySidebarItem`) and is wired onto the atom in `storageAtoms.ts`.

- **Updated 2026-09-21:** `transformGraphViewLayout` and `transformSchemaViewLayout` now ride `createSessionScopedAtom` instead, since both layouts moved to per-tab scope (see ADR `per-tab-session-scoped-storage-primitive`). That factory takes the same `ReadTransform<T>` but runs it on the shared localForage breadcrumb only, not on the per-tab value (which its codec validates) or on `defaultValue`. The decision here is unchanged; only the wiring moved.
- **Updated 2026-07-10:** `transformVertexStyles` (in `vertexStylesTransform.ts`) is a second consumer, applied to `user-vertex-styles`. It coerces retired round-polygon shapes to their non-round counterpart (see ADR `coerce-retired-round-polygon-shapes`). Values arriving through file import are stored verbatim — the same ReadTransform coerces them on the next load, so both entry points (persisted storage and imported files) converge on the same coercion without the import path needing its own transform.

Two decisions here are not obvious from the code:
Expand Down
42 changes: 42 additions & 0 deletions docs/adr/20260921-per-tab-session-scoped-storage-primitive.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# Per-tab session-scoped storage as a reusable primitive

## Status

accepted

## Context

The per-tab Active Connection decision (`per-tab-active-connection`) solved one concept by hand: hold the live value in `sessionStorage`, keep the existing localForage key as a shared last-writer-wins breadcrumb, and **claim** the breadcrumb into `sessionStorage` on cold start so a reload reads the tab's own value back. That logic lived inline in `activeConnectionStorage.ts`.

Graph-view and schema-view layout (active sidebar tab, sidebar width, view toggles, the details-auto-open preference) had the same shape of problem as Active Connection, not the shape the reconciliation ADR addresses. Layout is a property of **what this tab is looking at**, not a global user preference: two tabs exploring different connections each want their own sidebar and toggle state. As shared `atomWithLocalForage` scalars they were last-writer-wins across tabs — a second tab's layout silently became the first tab's on its next cold start. Reconciliation (`per-key-diff-merge`) is the wrong tool: layout has no sibling entries to preserve, so a per-key merge is meaningless; what it wants is per-tab **divergence**, exactly like Active Connection.

That made three distinct cross-tab storage behaviors in the codebase, only two of them named, and the third (per-tab) implemented once as a one-off. Adding a second and third per-tab concept by copy-pasting the subtle seed-and-claim logic was the wrong move.

## Decision

Extract the per-tab + breadcrumb mechanism into one primitive, `createSessionScopedAtom<T>` (`core/StateProvider/sessionScopedStorage.ts`), and route every per-tab concept through it. There are now **three named cross-tab storage scopes**, each a deliberate choice at the atom's creation:

- **Per-tab** — `createSessionScopedAtom`. Live value in `sessionStorage`; shared localForage breadcrumb read once as the cold-start seed and claimed into `sessionStorage`. Tabs diverge. Backs Active Connection, graph-view layout, schema-view layout.
- **Shared-reconciled** — `atomWithLocalForage` with `reconcileMapByKey`. Map-keyed collections genuinely shared across tabs, merged per key (`per-key-diff-merge`). Backs Connections, Schema, Vertex and Edge Styles, Sessions.
- **Shared-blind-write** — `atomWithLocalForage` with no reconciler. Scalars where each write is the whole intended value and tabs need not diverge. Backs the boolean/number settings (e.g. `showDebugActions`).

`createActiveConfigurationAtom` is refactored onto the primitive rather than left as a parallel implementation, so the seed-and-claim logic lives in exactly one place. Its own tests stay, narrowed to what the wrapper still owns: the empty-string-is-a-miss codec rule, the bare-string round trip, and the `resolveSessionStorage` fallback. They also stand as behavior-preservation evidence for the refactor.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Small factual correction: nothing was narrowed. git diff bfc06442 HEAD -- packages/graph-explorer/src/core/StateProvider/activeConnectionStorage.test.ts is empty — all ten tests are unchanged, including the multi-tab cases.

Worth fixing because the reality is stronger than the claim. Retaining the full pre-existing suite unmodified is better behavior-preservation evidence for the refactor than retaining a narrowed subset would be, and it is the thing that made me comfortable with the createActiveConfigurationAtom rewrite. Suggest rewording to say the tests were kept intact as behavior-preservation evidence.


A per-tab value crosses two backings with different serialization needs, so the primitive takes a **`SessionValueCodec<T>`**: the breadcrumb keeps the native value (structured clone preserves a `Set`), while `sessionStorage` holds only strings. The codec's `deserialize` returns `null` only for an absent value (a legitimate miss) and validates a present value with **zod**, _throwing_ on an unparseable or wrong-shape value rather than swallowing it. Detecting corruption is thus separate from deciding what to do about it: the seam (`createSessionScopedAtom`) catches the throw, logs it, and treats it as a miss so a stale or hand-edited per-tab value falls through to the breadcrumb instead of seeding a bad shape or crashing startup. Graph-view layout's codec serializes its `activeToggles` `Set` as an array and rebuilds it on read via a zod `.transform`; schema-view layout is plain JSON. Active Connection's value is a bare id string, so its codec passes the string through and skips zod.

## Considered Options

- **Copy the seed-and-claim logic into each layout atom.** Rejected: duplicates the load-bearing, easy-to-get-subtly-wrong claim logic across three atoms, with three sets of near-identical tests.
- **Leave layout as shared (status quo).** Rejected: the clobber that motivated the per-tab Active Connection decision applies to layout for the same reason.
- **Reconcile layout per key (`atomWithLocalForage` + a reconciler).** Rejected: layout has no sibling entries; it wants per-tab divergence, not a merge. Reconciling it would reintroduce cross-tab coupling.
- **Per-tab + breadcrumb extracted to a primitive (chosen).** Names the third scope, removes the duplication, and unifies Active Connection onto it.

## Consequences

- **No migration.** The breadcrumb keeps each concept's existing localForage key and native shape; the codec only governs the per-tab `sessionStorage` round-trip. Existing stored layouts seed a fresh tab unchanged.
- **Read-time transforms apply to the breadcrumb only.** A concept whose stored shape an older app version wrote differently passes a `ReadTransform` (`read-time-transform-for-persisted-values`), and the primitive runs it on the breadcrumb before claiming it — that is the only path a retired shape can arrive by. The per-tab value is zod-validated on read, so it cannot carry one, and `defaultValue` is already current. Both layouts use this to remap the retired `nodes-styling`/`edges-styling` sidebar items onto the combined `styles` panel.
- **Cold start can resume a layout set in a different tab** (last-writer-wins breadcrumb), the same honest "resume the most recent" semantics Active Connection accepts. Per the storage model — read once at creation, never re-read (`per-key-diff-merge` context) — no scope here has ever had live cross-tab sync; an already-open tab does not reflect another tab's change until it reloads.
- **A corrupt or stale per-tab value self-heals.** `deserialize` throws on a present-but-invalid value (and reading a blocked `sessionStorage` can throw a `SecurityError`); the seam catches either, logs a warning, and falls through to the breadcrumb then the default rather than crashing startup. Breadcrumb **write** failures still surface through the persistence-status path (`storage-layer-owns-persistence-failure`). Appropriate for non-critical view preferences.
- **Rule for new atoms.** A concept that should diverge per tab uses `createSessionScopedAtom` with a codec; a shared Map-keyed collection uses `reconcileMapByKey` (`per-key-diff-merge`); a shared scalar uses plain `atomWithLocalForage`. Scope is now a visible choice at the atom's creation, not an implicit consequence of which factory was reached for.
- The `sessionStorage` backing stays injectable, so per-tab isolation is tested directly with separate stores and separate `sessionStorage` mocks over one shared mock localForage.
- This primitive is the `perTab` adapter that spike #1876 proposes to formalize alongside the other two scopes. This ADR records the **scope** decision; #1876 may later relocate where the logic lives without re-deciding it.
Loading
Loading