docs: design — video notes view as a navigation-native, pane-scoped view mode (#335)

[spolakh]

Design doc for #335 — video notes view as a navigation-native, pane-scoped view mode.

Read it rendered: docs/video-notes-navigation-native.html (self-contained HTML, dark-mode aware, two SVG figures).

Summary of what it proposes (details, file:line grounding, alternatives, and test plan in the doc):

• Leaf annotations in the slot grammar — #ws/a/b~video-notes/c: a plugin-owned pane view mode projected from a new panel:viewMode panel-row prop through parseSlot/buildLayoutSlot/layoutSlotsFromRows/sameLayoutSlots/reconcilePanelRows (row reused on annotation-only diffs; LCS plan-matching needs no change — it's id-only). History entries, Back/Forward, session restore, and layout-serializing consumers then work via the existing projection machinery.
• Video plugin rides on it: pane takeover (absolute inset-0) instead of the viewport-fixed overlay; video:playerView retired; mode cleared on in-pane navigation with VisitState capture (slot annotation authoritative on URL-driven reconcile); note auto-creation stays gesture-only (navigation never writes the document); a composed navigateInPanel(…, {viewMode}) so nested-video enter is one tx/one history entry; per-pane player identity (panel discriminator in the top-level render scope — fixes the existing registry collision when two panes show the same video).
• PR 2: generic maximize-pane gesture — reserved ~max annotation + panel:maximized, rendered via a desktop analog of LayoutRenderer's mobile single-pane path, with the maximized row coerced to be the active panel.
• Playback continuity: Stage A in-memory position map (ships with PR 1; also fixes today's position loss on the plain notes toggle, which remounts the player); Stage B parked-player keep-alive as a clearly-scoped follow-up (moveBefore/native-<video> limits documented).

The doc went through an adversarial review pass against the current code — every cited mechanism (pushHash on slot change, LCS row reuse, reconcileUrlNavigation blockId matching, mobile single-pane path, UiState sync semantics) was verified at the cited lines as of fad09f9.

Happy to iterate on any of it — grammar char and the max reservation especially need your sign-off before I start on the implementation PR.

Closes nothing on merge (design only); implementation will follow as PR 1 (annotations + video plugin) and PR 2 (maximize) per §6 of the doc.

🤖 Generated with Claude Code

[spolakh]

@codex pls review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1976352675

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Update every top-level render-scope producer

Threading the panel id only at the PanelRenderer call site leaves several non-rendering producers of top-level scope IDs on the old outline:<blockId> format: writePanelContent seeds focusedBlockLocationProp with outlineRenderScopeId(blockId) (src/utils/panelHistory.ts:226-229), new panel rows do the same (src/utils/panelLayoutProjection.ts:341-345), and global shortcuts target the active panel with outlineRenderScopeId(activeTopLevelBlockId) (src/shortcuts/defaultShortcuts.ts:188-191). If the rendered video registers under a panel-qualified scope but these callers keep writing/resolving the block-only scope, focus restoration and scoped video actions in duplicate-video panes will still miss the intended pane or fall back to an arbitrary player; the implementation plan should explicitly update these producers too.

Useful? React with 👍 / 👎.

[spolakh]

Good catch — folded in. The doc now specifies the change as one shared derivation moved wholesale: a panel-qualified panelRenderScopeId(panelId, blockId) adopted by every top-level scope producer (PanelRenderer, writePanelContent/createPanelRowInTx focus seeding, defaultShortcuts.ts:190, the focusBlock default at properties.ts:384, and the selection.ts:447 fallback — your list plus the two you didn't hit), with embed scopes explicitly out of scope and a test for focus restoration + scoped actions with the same video in two panes. §4.3 + §6 updated.

[Stvad]

proposed url format feels arbitrary, and suggests that we potentially may want future extensions, what if we afford for that by allowing encoding arbitrary key-value context after block uid?

[Stvad]

• we achieve the current custom view by supplying a custom block layout does that stay the case/what's expected way of consuming the view?
• at the end of this I want there still be one shortcut that gives me a similar experience to current one (video maximized to full screen ish)

[Stvad]

also curious: any interactions with #332 ?

[spolakh]

kv context: agreed — the positional tokens forced a pile of disambiguation rules (reserved set, first-token-wins, drop-extras) that exist only because the tokens are anonymous. Proposed shape: <blockId>(~<key>(=<value>)?)*, e.g. b~view=video-notes~max (flag keys valueless; ~ separator, = fragment-legal per RFC 3986; canonical key order on build; value charset [A-Za-z0-9._-]+ until something needs escaping). Known keys map to the same typed panel props (view ↔ panel:viewMode, max ↔ panel:maximized); unknown keys round-trip opaquely through parse→build, so older clients preserve rather than strip keys they predate. Full fidelity through row-reconcile would additionally need a residual panel:urlContext prop — deferred until a consumer needs it.

Layout consumption: unchanged — the custom view stays a blockLayoutFacet contribution; only its gate moves from the block prop (video:playerView) to the panel's view key (+ "block is the pane's top-level block"). That's the expected way to consume a view mode.

One shortcut: preserved — view and max are keys on the same slot, so the notes toggle sets both in one tx (b~view=video-notes~max): one shortcut, one history entry, Back exits the whole thing. Full-screen minus the header (today's overlay paints over the header too — that's the one delta). I'll make view+max the default for the main shortcut, with an un-maximized variant for split layouts.

#332: no substrate overlap (threads are normal blocks; this is UiState panel-row props + the URL). Two real touchpoints: (1) terminology — I'll rename our "leaf annotations" to slot context so "annotations" stays #332's word; (2) if comments ever want per-pane state (comments panel visible, deep-link to a thread), this kv channel is exactly where it'd live — likely the "second consumer" that would justify graduating the opaque view value to a registered facet. (Speculative, not a dependency: timestamp-anchored video notes could later reuse #332's block-local anchor-payload pattern.)

Updating the doc to the kv shape + rename + one-shortcut default now.

[Stvad]

I'll rename our "leaf annotations" to slot context so "annotations" stays #332 word;

uh no need

is ~max shortcut for ~max=true in kv schema?

[Stvad]

what if value needs to contain ~

[spolakh]

~max ≡ ~max=true — yes. A valueless key is boolean-true shorthand, and the bare flag is the canonical form: parse accepts max=true and normalizes it to max on build; max=false normalizes to absence. (Only known boolean keys get this treatment — an unknown key can't be known to be boolean, so it round-trips verbatim, =true and all.)

Value containing ~ — excluded by construction for now: the value charset is [A-Za-z0-9._-]+, so ~ (and =, /, %) simply can't appear, and no escaping machinery exists to get wrong. The escape hatch is deliberately reserved: % is outside the charset, so if a future consumer needs arbitrary values we can enable percent-encoding inside values fully backward-compatibly — no currently-valid value contains %, so turning on decoding can't reinterpret any existing URL.

Rename — heard, no collision-driven need; I kept "slot context" anyway since it describes the kv shape better than "annotation" described the positional tokens (that name got weird once tokens stopped being anonymous). Zero-cost to flip back if you have a preference.

Folding the max=true normalization + charset/escaping rules into §4.2 now.

[Stvad]

hmm, we're inventing this whole new grammar, do we have to, can we just use an existing well-known format?

[spolakh]

Fair challenge — and there is a well-known format for exactly this: matrix URI parameters (;key=value;flag attached to a path segment — TBL's Matrix URIs note, later idiomatic in JAX-RS etc.). Our slots are path segments, and matrix params are precisely "key-value context on one segment rather than the whole URI" — which is also why slot-level ?query is the wrong tool: it scopes to the URI (and would collide with the hash-level ? channel that already carries the agent pairing secret).

So: #ws/a/b;view=video-notes;max/c. What we inherit for free:

• familiar, named syntax — nothing invented;
• valueless flag keys are already idiomatic (;max, with ;max=true accepted and normalized);
• standard percent-encoding for values — which retro-answers your "what if a value contains the delimiter" question with a standard (decode on parse, encode on build) instead of my reserved-%-escape-hatch construction.

What still needs pinning regardless of syntax (any format needs these, so they're semantics, not grammar invention): duplicate keys first-wins, unknown keys preserved verbatim through parse→build, boolean normalization, canonical key order for idempotent rebuilds.

One honest trade-off: ; is a sub-delim — fragment-legal and browsers keep it intact (verified: new URL(...) preserves it; we write the hash raw via pushState), but defensive third-party encoders escape sub-delims more readily than the unreserved ~. If bulletproof copy-paste-through-anything matters more than familiarity, the same matrix shape with ~ as delimiter is the fallback. My recommendation: standard ; matrix params — familiarity plus standard escaping beats the marginal encoder paranoia. Say the word and I'll flip the doc (it's a delimiter + encoding-paragraph change; all the semantics carry over unchanged).

[Stvad]

ok, better. remind me why we're not using most starndard/boring query params format ?key=val&key2=val ?

[spolakh]

Two reasons, one conceptual, one mechanical-to-this-codebase:

1. Query params scope to the whole URI; this state scopes to one slot. #ws/a/b/c?view=video-notes can't say which pane is in notes view. You'd have to invent slot addressing inside the params — by position (?2.view=…, breaks on every pane reorder) or by block id (?view[b]=…, breaks on the explicit goal of the same block open in two panes with different modes). Matrix params sit inside the slot token, so identity is structural: survives reorders, duplicates, stack nesting — and the existing slot differ (sameLayoutSlots) and row reconciler see the change exactly where they already look.

2. The hash's one ? channel is already taken, with opposite semantics. splitHashRouteAndParams splits the hash at the first ?, and that query channel deliberately carries non-layout, cross-cutting state (e.g. the agent pairing secret) that preserveHashQueryParams passes through rewrites while the layout differ ignores it. Putting layout state there inverts its contract — the projection/history machinery would need to know which params are layout-bearing vs pass-through, and a per-slot ? mid-route would truncate the route parse.

So: query params = right format for URI-scoped state (and that's exactly what the existing ? channel does); matrix params = the standard format for segment-scoped state, which is what a pane's view mode is.

Proceeding with ; matrix params in the doc — flag if you'd rather the ~-delimited variant from the encoder-paranoia note.

[spolakh]

Doc updated to matrix params: #ws/a/b;view=video-notes;max/c — ; delimiter, values percent-encoded (standard component encoding), semantics unchanged, grammar open-question closed. Rendered: https://htmlpreview.github.io/?https://github.com/spolakh/km-upstream/blob/video-notes-navigation-native/docs/video-notes-navigation-native.html

[Stvad]

I see so we can't be like ws/a?k=v/b/c?k2=v ?

[spolakh]

Right — that shape isn't available, for one standard reason and one local one:

• In URI grammar the first ? terminates the path: after it, / (and even a second ?) are just query-string characters with no structure. So ws/a?k=v/b/c?k2=v doesn't parse as "slot a with k=v, then slots b, c" — anything applying URI conventions reads it as path ws/a + one opaque query k=v/b/c?k2=v, and the rest of the layout is swallowed. Making it work would mean redefining ? to be per-segment and letting / stay structural after it — i.e. inventing a nonstandard grammar that merely looks like query params. Matrix params (;k=v per segment) are the convention that exists precisely to fill this gap — TBL's note describes them as the segment-scoped counterpart to ?.
• Locally, the first ? in the hash is already load-bearing: splitHashRouteAndParams (routing.ts) splits route from params at the first ?, and that channel carries pass-through non-layout state (agent pairing secret). #ws/a?k=v/b/c would today parse as route ws/a + params k=v/b/c — the layout truncates at the first annotated slot.

So: ?k=v&k2=v stays what it already is (one URI-scoped channel, already in use), and ;k=v is the standard per-segment form — #ws/a;k=v/b/c;k2=v is exactly your example, one character different and parser-safe.

[Stvad]

btw we don't care about backward compat on this for now - with that in mind, should we consider refactoring our url format to be better - come up with a few potential designs and lmk what you think overall

[spolakh]

Good frame. Current format's actual warts, judged with compat off the table: the bespoke stack syntax (s:a,b) (parens + s: tag + a depth-aware splitter — the one part of the grammar that's invented rather than borrowed), raw-UUID length, and nothing else structural — the /-separated columns and the single pass-through ? channel are already the right shapes. Candidates:

A. Status quo + matrix params (what the doc says today)
#ws/a;view=video-notes/(s:b,c)/d — keeps (s:…). Cheapest, but the parens splitter stays, and the grammar is half-standard half-invented.

B. Three separators, all standard — / columns, , stacked rows, ;k=v slot context
#ws/a;view=video-notes;max/b,c/d — a stack is just a comma-list inside a column (, is already a reserved char; the matrix-URIs note itself uses commas for sibling values). Every slot in a stack can carry its own matrix params (b;view=x,c — unambiguous because values are percent-encoded, so split order is strict: / → , → ;). This deletes the parens, the s: tag, and the depth-tracking splitter entirely — parseLayout becomes three split()s in precedence order. The data model is already exactly this shape (stacks hold panels, one level — verified against PANEL_STACK_TYPE usage and LayoutRenderer's RenderSlot), so the format encodes precisely what can exist, no more. If nesting ever deepens, parens can return as an extension rather than a founding piece.

C. Layout as one structured query param
#ws?layout=a;view=x|b,c|d — rejected for the reasons rehearsed above: it inverts the ? channel's pass-through contract, and the differ/projection would need to learn which params are layout. Listing for completeness.

D. Encoded-blob (base64 JSON tree)
#ws/z:eyJjb2xz… — maximally expressive and versionable, but kills readability, hand-editing, and URL diffing; sharing/debugging gets strictly worse. Reject unless layouts ever become genuinely tree-shaped.

Overall take: B. It's the same refactor PR 1 already touches (parseSlot/buildLayoutSlot/splitTopLevel), so with compat free it costs nearly nothing extra now and would cost a real migration later. Two optional riders, both cheap under B and both deferrable:

• ;active flag key — the active pane is currently not in the URL at all, so a shared link can't say "…with this pane focused"; it's just another core-owned boolean key on a slot now that the kv channel exists.
• Short ids — the remaining wart is UUID length, and that's orthogonal to format: git-style prefix shortening needs an ambiguity/resolution story that's dicey in a local-first app (a prefix that's unique today may not be after sync), and aliases rot on rename. Worth its own think, not worth coupling to this.

No version marker while compat stays free; if the format changes again post-adoption, a one-char version prefix can be introduced then. If B works for you I'll fold it into the doc (it shrinks §4.2 — the stack-parsing special cases go away).

[Stvad]

b if you look at original docs/git history I do think parens are there to enable depth/sub-splits. is there a version of this that preserves that option?

;active flag key — the active pane is currently not in the URL at all, so a shared link can't say "…with this pane focused"; it's just another core-owned boolean key on a slot now that the kv channel exists.

sure let's add this

[spolakh]

Yes — B generalizes to a recursive grammar where parens return as pure grouping, exactly preserving the depth option while never appearing at today's depth:

layout := column ('/' column)*
column := cell (',' cell)*
cell   := slot | '(' layout ')'
slot   := blockId (';' key ('=' value)?)*

• Axis alternates per nesting level: top-level / = side-by-side columns, , = stacked rows within a column, and a parenthesized cell re-enters the grammar (its / splits the cross axis at that depth). That's the classic tiling encoding.
• Today's model (stacks hold panels, one level) never emits parens — #ws/a;view=video-notes/b,c/d — but the parser accepts them from day one, so deeper splits are a data-model change later, not a format change. The s: tag goes away (redundant once , means stack); the depth-aware splitter stays (it already exists for (s:…)).
• No ambiguity with values: parens are sub-delims, so they're percent-encoded inside values like ; and ,.

;active — added, with one semantic worth pinning: if active-pane changes were projected like any slot change, every click into another pane would push a history entry and Back would un-focus-switch — noisy. Proposal: active is part of the slot context (so shared/restored URLs honor it, and copied links carry it), but the projection treats an active-only diff as replaceState, not pushState — URL stays truthful, no history spam, Back never merely re-focuses. Inbound it coerces activePanelIdProp (same mechanism the maximize coercion uses).

Folding B'+active into the doc now — §4.2 gets simpler (the s: special case disappears) and §6 gains the replaceState-on-active-only rule.

[Stvad]

make sense. should we fold mobile front/focused tab situation too, or is it just "max" ?

[spolakh]

It's active, not max — and it's already folded with no extra concept: mobile's front pane is the active panel (LayoutRenderer renders activePanelSlot ?? fallback when narrow), and that's exactly the prop ;active projects. So:

• A shared #ws/a/b;active/c opened on mobile fronts pane b; on desktop it renders all three with b focused. Same key, form-factor-appropriate meaning: "front tab" and "focused pane" are the same state rendered by different policies.
• max stays a desktop-explicit gesture; on mobile the flag itself is moot (already single-pane) but its active-coercion still runs, so a maximized shared URL opened on mobile fronts the intended pane anyway — max degrades to front-tab through the same path.
• Mobile tab switches are pane activations → active-only diffs → replaceState, so no history spam there either. One consequence worth noting: hardware/browser Back on mobile won't undo a tab switch (it goes to the previous real layout entry). If mobile UX ever wants Back-pops-tab-switch, that's a per-form-factor choice in the projection's push/replace classification — possible later, not proposed now.

So: no new key, one sentence in the doc making the mobile equivalence explicit.