-
Notifications
You must be signed in to change notification settings - Fork 113
Scope graph-view and schema-view layouts to the browser tab #1894
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
b9e3ee4
d76ef52
8ad67b0
a47a44e
ea899db
3c1fb70
e6e4cc7
10dce52
f42f0e7
f959849
1bce1e9
1e3b15a
b328b55
ff175ba
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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**: | ||
|
|
@@ -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**: | ||
|
|
@@ -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 | ||
|
|
||
| **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) | ||
|
|
@@ -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** | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 I can see two resolutions and I think the choice is yours as the owner of the domain model:
For what it is worth, #2145 does not conflict — its 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 | ||
|
|
||
|
|
||
| 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. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Small factual correction: nothing was narrowed. 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 |
||
|
|
||
| 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. | ||
There was a problem hiding this comment.
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.mdasks for "one or two sentences max" and "define what it IS, not what it does," anddocs/agents/domain.mdsplits 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 neighbouringLayoutandView Layoutentries 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.