Repository navigation
Dashboard v2: the React shell, the Focus, and the Spine lens (change 1) - #4
Merged
Merged
Conversation
Commit 4dcc35d ("update README and cleanup") removed five scripts as collateral, but four live references to two of them survived and still instruct contributors to run them: docs/design/dashboard-design.md:184,251,310 docs/contributor-checklist.md:134,143 -> test-dashboard-golden.sh tests/dashboard_metasfresh.rs:4,83 docs/design/dashboard-design.md:277 -> test-dashboard-metasfresh.sh The design doc even lists test-dashboard-golden.sh as "done" in its implementation-status table, so the documented per-phase gate could not be run at all. Restoring the scripts makes those references true again rather than editing the docs to match a regression. Both are modernized to current conventions, since they predate the rbuilder -> rgctl rename and the --all flag: - RBUILDER_{DASHBOARD_GOLDEN,METASFRESH}_REPO -> RGCTL_* (as used by scripts/validate-golden-repos.sh:15-16 and tests/dashboard_gbuilder.rs:7) - target/release/rbuilder -> target/release/rgctl - discover --all -> --with-cfg --with-security --with-taint (the --all flag no longer exists) - $REPO/.rbuilder/dashboard -> $REPO/.rgctl/dashboard Both now skip with a message when the corpus is not checked out instead of hard-failing, matching how tests/dashboard_gbuilder.rs:33 and validate-golden-repos.sh:36-42 already behave.
Umbrella design for a decision-first dashboard: value, the seven philosophy rules, the Instrument/Orientation/Rooms/Lenses information architecture, the portable StableNodeKey focus, the stack, and the additive-behind-a-flag compatibility strategy. Deliberately not a rewrite plan. It records the two constraints that shape every change (the tracked, compile-time-embedded dashboard/dist, and the silent wasm stub in build-dashboard.sh) and leaves the Preact -> React question open as the decision that blocks the first shell change, since shadcn/ui assumes React while the current app is Preact throughout. Companion to the existing dashboard-design.md, which stays the source of truth for the shipping dashboard until v2 reaches parity.
This was the largest single risk in v2 and it blocked change 1. Decided:
stay on Preact.
Migrating 9.7k LOC across every view would spend the v2 budget on a
framework change rather than on the information architecture, and upstream
has repeatedly chosen simplicity over breadth (MCP and daemon mode were both
removed for it).
Recording the consequences honestly rather than just flipping the
recommendation, because the stack table was written assuming React:
- shadcn/ui is out, and Radix Primitives is React-only too, so the whole
component layer is only available through the preact/compat shim.
Budget for spiking it early and hand-rolling dialog / popover / tabs /
table wrapper / command palette if it fails.
- @react-three/fiber is out outright: it needs React's reconciler, which
preact/compat does not provide. Change 4 must mount three.js
imperatively, following the existing sigmaMount.ts pattern.
- TanStack Router is React/Solid only, so routing needs a
Preact-compatible router; table-core and virtual-core are
framework-agnostic and fine.
@preact/preset-vite is already in use and aliases react -> preact/compat,
which is why React-shaped libraries are candidates at all. But that is a
shim, not a guarantee: the stack table now says each library must be spiked
in change 1 and replaced rather than shimmed harder if it fails.
Status table: change 1 moves from "blocked" to "next"; open question 1 is
closed with the follow-on spike task noted.
Change 1's artifact required the component/routing/state libraries to be
proven under preact/compat before shell code depends on them. Each was
rendered or driven, not merely imported.
Kept:
- @tanstack/query-core (digest-keyed cache; staleTime Infinity did not
refetch; a different digest did)
- Zustand vanilla + preact's useSyncExternalStore (focus key round-trips,
subscribers re-render)
Rejected:
- wouter 3.13 — peer-depends on react, ships no Preact entry, and binds
the real React in node_modules via useSyncExternalStore
- Radix Dialog.Portal — renders nothing and throws
InvalidCharacterError: "[object Object]" did not match the QName
production
The finding is narrower than "React-only libraries fail": Radix's
*non-portal* path works fine (role=dialog with aria-labelledby correctly
wired via useId), and preact/compat's own createPortal works. So the failure
is specific to how Radix uses portals, which means overlays are still viable
— hand-rolled on the working primitive.
Consequences:
- routing is hand-rolled over URLSearchParams + popstate (~40 lines) rather
than taking wouter, our largest and most shim-exposed dependency
- the ~5 component primitives rooms need are hand-rolled on
createPortal rather than mixing "Radix inline, hand-rolled overlays"
- @react-three/fiber stays out
Latent hazard found: npm install @radix-ui/* pulls REAL React 19 into
node_modules as a peer dep, so any React-shaped library can silently bind to
it instead of the shim and fail in a way that looks like our bug.
Harness gap found: dashboard/vitest.config.ts declares no plugins and no DOM
environment, so the preact preset and its alias are absent from test runs and
no component test is possible today. Recorded as tasks 1b in the change.
The stack spike was run to justify staying on Preact. It returned the
opposite answer, so the decision is reversed and the evidence is recorded in
full rather than summarised.
What the spike found under preact/compat:
- Radix Dialog.Portal renders NOTHING and throws
InvalidCharacterError: "[object Object]" did not match the QName
production
- wouter 3.13 peer-depends on react, has no Preact entry, and binds the
real React in node_modules via useSyncExternalStore
- TanStack Router and @react-three/fiber are unavailable outright
- what DOES work (non-portal Radix, createPortal, query-core, zustand) is
not the part rooms need
The failure is specific to portals as Radix uses them — which is exactly the
set a rooms UI depends on: dialog, popover, dropdown, tooltip, select,
command palette.
Worse, and not a one-off: npm install @radix-ui/* pulls real React 19 into
node_modules as a peer dep, so any React-shaped library can silently bind to
it instead of the shim and fail in a way that looks like our bug. On React
that whole class of failure disappears.
The cost, measured with esbuild --minify then gzip:
React + react-dom/client 217 KB min 67 KB gz
Preact 10 KB min 4 KB gz
So ~63 KB gzipped more, against a 1.7 MB locally-served bundle (CodeMirror
alone is 146 KB gz) — about 2%. The usual Preact-vs-React size argument does
not apply to a bundle embedded in a binary.
The objection that mattered was migration cost: 9.7k LOC. It is weaker than
it looked, because v2 is additive behind a flag and ships as its own bundle.
v1 keeps its Preact code and its 18 test files until parity, and the legacy
views get deleted rather than ported — so the migration is deferred, and by
then it is migration of code we intend to delete.
Also: R3F is back for change 4, so the 3D Code City no longer needs
imperative three.js mounting. The vitest harness gap (no plugin, no DOM
environment) stands regardless of framework.
The stack table was written from the Preact side of the decision, so three
rows justified themselves by contrast with a framework being abandoned:
"Unavailable under Preact", "out under Preact", and Radix's reason being that
it "does not work under preact/compat". A reader evaluating the design gets a
history lesson instead of a rationale.
Rewritten so each row argues for the choice on its own merits, and two choices
re-decided rather than merely re-worded — both were "core" packages picked to
avoid framework bindings, which is no longer a constraint:
@tanstack/query-core -> @tanstack/react-query (the hooks were the
thing we would otherwise hand-wire; the spike
literally tested wiring useSyncExternalStore by
hand, which was a Preact workaround)
table-core+virtual-core -> react-table + react-virtual
Routing, Radix and R3F keep their choices but gain real reasons.
Adds a "What the framework change reclaims" table making the trade explicit:
hand-rolled URL state -> TanStack Router, hand-rolled overlays on
createPortal -> Radix, imperative three.js -> R3F. Three capabilities had
been designed around rather than chosen.
Also states plainly that the design is unaffected — the framework never
constrained the IA, the Focus mechanism, the receipts or the honesty rules.
Those were decided on product grounds and survive the switch intact. What
changed is what the shell is built from, not what it does.
Notes the mock's divergence: it still hand-rolls URL state, because it was
written during the Preact phase and exists to prove the IA and Radix layer,
not the routing choice.
Reframes the evidence section as the justification for a reversal rather than
the centre of the document.
A working prototype of the v2 shell on the stack docs/design/dashboard-v2.md now specifies — React 19, TanStack Router, Radix primitives, Tailwind, Zustand — driven by real artifacts from a live `.rgctl/` rather than fabricated data. 259 KB of dataset, paths anonymized, extracted from `cfg_index.json`, `communities.json`, `metagraph.json`, `migration_plan.json`, `mutations_index.json`, `manifest.json`, `dashboard/cfg/*.json` and `rgctl -f json clones`. It is here to settle arguments that a document cannot. The rooms are clickable, the Focus is a real portable key carried across rooms through the URL, and the Radix `Dialog.Portal` renders where the same code on Preact produced nothing — which is the concrete reason the framework decision moved. The mock also states what it does not do: no WASM engine, no per-symbol centrality (the join is not honest, so it says so instead of printing zero), CFGs for 12 of 1,479 functions, and no 3D. A prototype that hides its gaps cannot be used to judge the design. Builds standalone: `npm ci && npm run build` -> 583 KB / 167 KB gzipped. Also fixes a .gitignore trap found while staging this: the bare `graph.json` pattern on line 54 (intended to keep generated repo graphs out of the tree) matched `mock/src/data/graph.json`, so the dataset was silently excluded and the mock would not have built from a fresh clone. Nothing warned; the file simply was not in the commit. Exempted explicitly, with the reason stated. Signed-off-by: Sami Jan <sami.jan@gmail.com>
Tailwind's automatic content detection scans whatever it can reach, and
`.gitignore` decides how far that is, so the shipped CSS could change for
reasons unrelated to any code change. Two cases, both measured:
- Un-ignoring `src/data/graph.json` (259 KB of data, not markup) made
identifiers inside it emit CSS: 25.89 -> 27.12 KB.
- Adding three class-shaped tokens to README.md changed the CSS with no
code change at all.
The first one is not hypothetical — it was caused by the previous commit on
this branch, and it is what made a bundle size move between two builds of an
otherwise identical tree. I had attributed it to the README without testing
that; testing showed the README was innocent and the ignore rule was not.
Name the real sources, and exclude the dataset explicitly, because `@source`
still honours `.gitignore` for files beneath it. Output is now identical
whether or not the dataset is ignored, and screenshots of three rooms are
byte-identical before and after, so none of the 1.2 KB was ever used.
Signed-off-by: Sami Jan <sami.jan@gmail.com>
Both were mine and both were checkable:
- The prototype was described as "~113 KB gzipped including the dataset".
It is 583 KB / 167 KB gzipped. The 113 KB figure predates the CFG and
clone views, and there was also a window where a broken build reported
98.5 KB; neither belongs in a design doc.
- The prototype was described as still hand-rolling its URL state "because it
was written during the Preact phase". It was ported to TanStack Router. The
routing table three sections above already listed TanStack Router as the
chosen option, so the doc contradicted itself.
The note now says what the prototype actually demonstrates: a focus and lens
surviving a view change, a reload and a shared URL.
Signed-off-by: Sami Jan <sami.jan@gmail.com>
…monstrate
Two rows were out of date after the rebase onto main:
- `dashboard/dist` regen was marked **blocked**, "source fixes are not in the
embedded bundle until dist is rebuilt". That landed in 8f09655 as
sshaaf#110. The row now says done, and says why it mattered: `dist` is
compile-time embedded, so until it is rebuilt the source fixes are invisible in
a built binary while every test passes.
- Change 7 was blocked on two `rgctl slice` defects. Both are now fixed and under
review (issues sshaaf#113/sshaaf#114, PRs sshaaf#115/sshaaf#116), so the row records what actually
gates it.
Also adds open question 4, because the more interesting problem is a gap rather than
a stale value: the maintainer's parking comment on sshaaf#61 names the code browser as the
missing piece, and no change in this sequence is the code browser. Change 1 is the
shell, changes 2-6 are rooms, and none renders source. The prototype does not cover
for it either — the Code lens shows a count of source lines, not source, and Spine
renders an empty state. The one requirement singled out is the one this design
cannot demonstrate.
Recorded as an open question, not invented as a change number: folding source
rendering into change 1 versus giving it its own change ahead of the rooms is a
scoping decision, not a correction.
Signed-off-by: Sami Jan <sami.jan@gmail.com>
Seven screenshots of the mock, committed under docs/design/dashboard-v2/screens/, each captioned with what to look at. The mock is being retired: its questions are answered, and design decisions from here should be settled in the real implementation where they can be tested against a real graph. Committed rather than linked to a running app because a link to a prototype rots the moment the prototype goes. The PNGs match the project's own precedent — the sshaaf#61 mockup was committed to docs/universe/ the same way. The captions carry the load the images cannot: which of them is the decisive artefact (the Radix Dialog.Portal, which renders nothing through preact/compat and is why the framework changed), and which numbers are real (55 dominance frontiers from the CFG, modularity 0.455, 17 exact clone groups). Also records in the design doc what the prototype could not show, since that is the argument for retiring it rather than extending it: no WASM engine, so nothing is driven by a real graph; the Code lens reports a count of source lines rather than source; and Spine renders an empty state for want of a caller/callee index. The one requirement the maintainer singled out is the one it could not demonstrate. Signed-off-by: Sami Jan <sami.jan@gmail.com>
The driver opened a tab and captured it, which intermittently hung for the full 30s timeout. Chrome runs here in single-window `--app=` mode, so a newly opened tab can land in the background — and a background tab does not paint, so `Page.captureScreenshot` waits forever. One `Page.bringToFront` fixes it; the screenshot batch went from failing on the second shot to eight for eight, then seven for seven. This is the third bug this file has fixed, and they share a shape: the failure looks like "the app is broken" when the app is fine. The first was `PUT /json/new?<url>` dropping query params after the first `&`, which made a correct app look like a broken router. The lesson is recorded at the top of the file rather than in a commit nobody will read. Signed-off-by: Sami Jan <sami.jan@gmail.com>
Deletes the prototype now that its screenshots are committed and it is tagged mock-v2-prototype for provenance. Keeping it would mean maintaining a second app that differs from the real one in exactly the ways that matter — no WASM engine, no source rendering — and every design argument would then have to be had twice. The tag is annotated, so `git show mock-v2-prototype` explains why the prototype was built, what it settled, and why it stopped there, without needing this conversation. The two things it proved are preserved where they count: the stack decision and its evidence are in the design doc's "Why React" section, and the screenshots are in docs/design/dashboard-v2/screens/. Signed-off-by: Sami Jan <sami.jan@gmail.com>
Change 1 of the dashboard plan, against the real 5,345-node bundle rather than a fixture. Structure. A separate Vite config and a separate output directory, `dist-v2`, with v1's config untouched: v1 needs `@preact/preset-vite`'s `react` -> `preact/compat` alias and v2 must not have it, so exactly one config may carry it and neither needs a branch a reader has to evaluate. `dist` keeps serving `--with-dashboard` while the 18 dashboard test files stay meaningful. The shell. Instrument layer (digest, freshness, version, and capability dots derived from `analysis.*_available` rather than asserted), one Orientation screen that links into rooms instead of printing totals, addressable rooms, and lenses as a control over the current subject. The Focus is one value carried across rooms, serialized into the URL, and namespaced — `fn:<function_id>` today, `sk:<stable_key>` once the export carries it — so a stored link cannot silently change meaning when the export shape changes. The Spine lens, which is the piece the maintainer named as missing on issue sshaaf#61: source for the focused symbol beside its control-flow graph, bound by line number in both directions, so a block highlights the lines it covers. Source comes from `sources/<id>.txt`, which the CFG/slice export already writes. Two bugs found by running it, both of which type-checked and built cleanly: - **`dashboard/tsconfig.json` sets `"jsxImportSource": "preact"`** and includes `src`, so `src/v2/*.tsx` compiled its JSX through Preact's jsx-runtime and handed Preact vnodes to real React: `Minified React error sshaaf#31`, naming a key list full of Preact's `__k`/`__b`/`__v` internals. Fixed in both the bundle config and the v2 tsconfig, because which config a bundler picks for a file that two claim is a resolution detail, and this cost an hour. - **A CFG block's `start_line: 0` is not line zero**, it is the absence of a line. Rendering it as "L0" repeats the `—`-versus-`0` confusion fixed as D4 in the v1 work. About 59% of blocks in a real bundle carry line information; the rest are real blocks with no source to point at. The data layer was rewritten after reading a real bundle: the first draft expected `manifest.digest` and `sources_index.sources`, and the real manifest nests the digest under `graph` while the source index calls its array `files`. Both would have rendered an empty shell against real data while passing every type check. Signed-off-by: Sami Jan <sami.jan@gmail.com>
The prototype screenshots showed a mock with a static dataset. These are the real thing: the same shell running against a live 26 MB bundle — 5,345 nodes, 1,479 functions, 154 exported source files. 08 — the Repo room, reporting 1,472 symbols carrying data flow from the real index rather than a fixture. 09 — the Spine lens: source for the focused symbol beside its control flow, with the Instrument bar showing the bundle digest, freshness and the capability dots derived from `analysis.*_available`. Paths are repo-relative. Both are change 1's deliverable and both are honest about their limits — the CFG pane is a block list, not the dominance layout the prototype had, and blocks with no source line say so instead of showing "L0". Signed-off-by: Sami Jan <sami.jan@gmail.com>
A committed build artifact is a staleness trap: `dashboard/dist` is tracked upstream and embedded with include_dir!, so a change under `dashboard/src` has no effect on a built binary until someone rebuilds and commits it, while every test still passes. `scripts/test-dashboard-*` warns when dist is *missing*, never when it is *stale*, and CI never builds the dashboard at all. v2 does not inherit that. dist-v2 is generated instead; the mechanism is being decided, and this commit only stops the artifact from being in the tree while that happens.
Two things I said I would fold in early, both about a claim outliving the reason for it. **Rename-survival was wrong.** The Focus is derived from `(path, name, type)`, so it contains the name — a rename produces a different key and an old link stops resolving. What it survives is re-indexing, which is the property the URL actually needs. Corrected in the design doc, in the OpenSpec design and proposal, and the task that asserted "a renamed symbol still resolves through its stable key" is split into what is testable: a re-index still resolves, and a rename renders the stale state rather than a broken subject. Worth stating plainly why this mattered: the claim was load-bearing. "Carry this symbol to another room" means something different if the carrier expires the first time someone renames a function, and the shell had already been built with a stale state precisely because it cannot resolve renames. **`fn:` now states its own removal condition.** It exists only because the exported indexes do not yet carry `stable_key`. The condition written into `focus.ts` is a state, not a date: delete it once `stable_key` is in every index that feeds a Focus. A compatibility shim with no expiry becomes a permanent second identifier with its own bugs, and by then nobody remembers what it was for. Signed-off-by: Sami Jan <sami.jan@gmail.com>
`cfg_index.json` and `slice_index.json` carried only `function_id`, so the client had no way to address a function by the same identity the engine uses. Adds `stable_key` to `CfgFunctionEntry` and `SliceFunctionEntry`, populated where the entries are built — the value was already in scope there, being parsed a few lines above for the name/path fallbacks. `#[serde(default)]` so indexes written before the field existed still deserialise. Verified by a real export rather than by compiling: both indexes now carry the key for every function. **And the key is not what I assumed.** The exported value is `file \x1f name \x1f code_hash` (`crates/rgctl-analysis/src/storage.rs:24`) — three parts, the third a hash of the function body — and its own doc comment calls it a *stable cache key*. It is built for cache invalidation, so it changes on rename, on move, and on **any edit to the body**. That is the wrong identity for a shareable Focus, and it is a stronger objection than the rename claim I corrected a commit ago. A link that expires the moment someone touches the function expires exactly when a review link is most useful. The engine has the right thing already: `rgctl_graph::StableNodeKey` is `BLAKE3(path, name)` truncated to 64 bits, and `snapshot_diff` uses it to match nodes across snapshots — it survives edits and breaks only on rename. Choosing between them is a decision, not a correction, so it is recorded in `focus.ts` and left open rather than silently inherited. Also found while running the crate's tests: `cargo test -p rgctl-dashboard` **does not compile on main** (two `E0308` mismatched-types errors in a `slice_export.rs` test where `defined_vars`/`used_vars` are built as `SmallVec`/`HashSet` against changed signatures). Pre-existing and unrelated to this change, verified by checking out main. The 18 integration test targets still compile and run. Signed-off-by: Sami Jan <sami.jan@gmail.com>
Reversing the previous commit. I argued against committing a build artifact because it can go stale silently: `dashboard/dist` is embedded at compile time, and neither CI nor the release workflow rebuilds it, so a source change with a forgotten rebuild ships an old dashboard while every test passes. That reasoning is sound, but it is not my call. Upstream tracks `dist`, the maintainer is the one who has to live with the convention, and two different conventions for the same kind of artifact is worse than one imperfect one. v2 follows the project. The staleness risk is real and is handled the same way it is for v1: both bundles are regenerated in the same commit as any shell change (task 6.1).
`cargo test -p rgctl-dashboard` did not compile on main. Two E0308 errors in a `slice_export.rs` test: `Statement::defined_vars` is `SmallVec<[DefVar; 2]>` and `used_vars` is `SmallVec<[String; 3]>`, but the test built the first with `SmallVec::from([..])` — which takes its inline size from the array literal, giving `SmallVec<[DefVar; 1]>` — and the second as a `HashSet` from before that field changed type. Small, and it would have been found immediately by anyone running the crate's tests. Nobody was: CI never builds the dashboard (`ci.yml` has no node or dashboard step), so the only thing that would have caught it is a person running them locally, and the failure is in a test target that has to compile before any test in the crate can run. That is the cost of the gap — not a wrong number, just a whole crate's tests switched off. Uses `smallvec![]`, which infers the inline size from the field, so this cannot drift the same way again. Verified: `cargo test -p rgctl-dashboard` now runs, 35 passed / 0 failed.
Item 2. `bundle.rs` gains a second `include_dir!` for `dashboard/dist-v2` and selects between the two bundles. **No new user-facing flag**: `--with-dashboard` remains the only one, and `RGCTL_DASHBOARD_V2=1` chooses v2 while both ship. An environment variable rather than a flag because a flag is a promise to users and "run the other dashboard" is a development affordance; it is deleted when v2 is what the flag serves. Selection is a pure function (`version_from_env`) with `selected_version()` reading the environment, so the decision is testable without mutating process-global state — an env-var test in a parallel suite is a flake waiting to happen. One deliberate behaviour: when v2 is selected and unavailable, `dist_embedded` reports false and extraction fails with the build command, rather than falling back to v1. A silent fallback would serve the old dashboard to someone who believes they are looking at the new one, which is the failure mode this whole branch is trying to avoid. Tests: v1 is the default and `0`/`false`/empty keep it; `dist-v2` is complete in the embed (which is also the check that the committed bundle is valid, since `include_dir!` is compile-time); and both bundles define `index.html`, because Rust looks that name up in three places and the v2 build renames its entry to match. Verified end to end, not just by compiling: default extracts v1 (`id="app"`, no React chunk); `RGCTL_DASHBOARD_V2=1` extracts v2 (`id="root"`, react/radix/tanstack chunks).
The identity question is settled by measurement, and the answer was already in the client's hands. `function_id` is the node's deterministic id — `with_file_path` sets it from `deterministic_node_id` (`crates/rgctl-graph/src/schema.rs:444`), which is `UUIDv5(NAMESPACE_RGCTL, BLAKE3(path ‖ 0xff ‖ name ‖ 0xff ‖ node_type)[..8])`. So it *is* the graph's `StableNodeKey` in UUID form, which is what the design doc says the Focus should be. It just looked like an opaque row id. Measured on a three-function repo, indexing it, editing a body, then renaming: | change | function_id | analysis stable_key | |------------------------|-------------|---------------------| | edit the function body | unchanged | changed | | rename the function | changed | changed | So the export I added last time is not just unnecessary, it is the worse of the two options: `file \x1f name \x1f code_hash` is the analysis *cache* key, built for invalidation, and it changes whenever the content does. A Focus built on it would expire the moment anyone touched the function — which is exactly when a review link is most wanted. Reverted: the field is gone from `CfgFunctionEntry`, `SliceFunctionEntry` and the population site, verified by a real export (0 of 3 entries carry it). The `sk:` / `fn:` namespacing goes with it. It existed to keep two identifiers apart while both were in play; they turned out to be the same identity expressed twice, one of them worse. Deleting it also deletes the "expiry" note that had been scheduled as task 5b.2 — a bridge with no second side to bridge to. `parseFocus` now validates a UUID shape rather than accepting any string, so a malformed URL yields "no subject" instead of a subject that looks real and resolves to nothing. Verified against the live bundle: a bare UUID resolves and renders the Spine; a non-UUID gives the no-subject state; a well-formed UUID that is absent gives the stale state and says why. `function_id` still changes on rename — the name is in the hash, and nothing in the engine survives a rename without a resolution step needing a baseline snapshot. That limitation is unchanged and now stated in `focus.ts` rather than described as a property it does not have. Signed-off-by: Sami Jan <sami.jan@gmail.com>
1b.1, 1b.2, and 5.1-5.4: the four claims about the shell were previously verified by
hand against a live bundle. That is how two bugs on this branch were found, and it is
not a regression test — a hand check happens only when someone remembers.
The harness needed three things that were not there:
- A vitest config for v2 with `@vitejs/plugin-react` and jsdom. `vitest.config.ts`
declared no plugins and no environment, so nothing that renders could be asserted
at all.
- **`jsxImportSource: "react"` set explicitly**, because `tsconfig.json` sets it to
`preact` and includes `src`. Without it the test transform compiles v2's JSX
through Preact's jsx-runtime and hands Preact vnodes to real React — the same
`Minified React error sshaaf#31` that cost an hour in the bundler, which would have cost
another hour here.
- The two configs must partition the tree. v1's config claims every `*.test.ts` under
`src`, so it now excludes `src/v2`; otherwise a v2 test file would be run by
whichever config claimed it, under the wrong framework and with no DOM.
`router.tsx` becomes a factory taking a history, so the tests drive the real route tree
with a memory history. A second route tree written for tests would be a copy that
drifts, and what is worth testing here is precisely the routing contract.
The fixture is a hand-built bundle rather than the real 26 MB one, because a test that
needs a repository indexed somewhere is a test that passes on one machine. Its shapes
are copied from a real bundle — `sources_index.json` keys its array `files`, the
manifest nests the digest under `graph` — since those are exactly the details a guess
gets wrong.
Two of the seven tests failed first time, both because my assertions were wrong rather
than the app: `findByText` *throws* when nothing matches, so `.toBeFalsy()` on it can
never express "this must be absent"; and `/search symbols/i` matched two buttons because
the omnibox's accessible name starts the same way. Fixed with `queryByText` and an exact
name.
Verified: v1's suite unchanged at 21 passed; 7 new tests pass; `dashboard_bundle` 2
passed. 5.5's parity assertions in `dashboard_harness` are still outstanding.
Three build tools claim files under `dashboard/src`: Vite, Vitest, and `tsc`. The v2 app lives in `src/v2`, so it had to be excluded from v1's config in each — I had done Vite and Vitest and missed the compiler. The symptom was v1's own build failing: `npm run build` runs `tsc -b` against `tsconfig.json`, which `include`s `src`, so it type-checked `src/v2` under `jsxImportSource: "preact"` and reported that `VNode` is not assignable to `ReactNode`. v1's build broken by a file v1 does not use. Worth noting for anyone adding a second app here: the clean fix is a separate directory. Excluding works, but it is the third place that has to know about the partition, and a fourth will appear. Also regenerates `dist-v2`, which was **stale and committed that way** — the router factory refactor changed app source and I ran the tests without rebuilding the bundle. Nothing caught it: the tests do not read `dist-v2`, and no gate compared them. The new size gate in `build-dashboard.sh` now forces both bundles through a build on every run, which is what surfaced it.
Two of the design doc's build hazards, in the script that causes them. **The throwing stub (6.3).** When `wasm-pack` was absent the script wrote a stub engine whose `EngineContext` throws at runtime. It looked guarded — `if [[ ! -f dashboard/wasm/rgctl_wasm.js ]]` — but `dashboard/wasm/` is gitignored apart from its `package.json`, so `rgctl_wasm.js` is **absent in every fresh checkout** and the guard was true every time. `npm run build` then bundled the stub into `dashboard/dist`, so a contributor without wasm-pack could replace the real ~197 KiB engine with a stub that throws on first use, commit it, and have the bundle look completely fine. The guard made it worse by reading as protection. It now refuses and explains, and reuses an existing engine when one is present. **The size gate (6.2).** Both bundles are measured in KiB of gzipped JS + CSS — what actually ships in the binary — against recorded budgets. Gzipped over the built assets rather than the source, so the gate cannot be satisfied by moving code into a chunk that is still requested on load. Over budget fails the build and says to raise the number deliberately. The budgets are measured, not chosen: **v1 is 321 KiB and v2 is 133 KiB.** Worth recording because it corrects an impression the design doc gives — it says React costs ~63 KiB more than Preact, which is true of the framework and false of the dashboard. v2 ships less than half of v1 because it does not carry CodeMirror (~150 KiB), Sigma (~29 KiB) or Bootstrap. Raising the budget has to be part of a commit, which is the point: growth becomes a decision rather than a drift.
…ding 9.1-9.3, the three partials. The Spine is now the code browser sshaaf#61 asked for rather than a source pane with a block list beside it. **The dominance-depth layout (9.2)** is ported from the retired prototype, where it was worked out against a static dataset; it now draws a live one. Vertical position is dominator depth walked from `idom`, edge colour is the control-flow edge type, amber dots mark blocks in a dominance frontier, and hovering lights the dominator chain and dims everything else. The vertical axis is the analysis, not decoration: a block lower down is dominated by everything on the amber path above it. **Syntax highlighting (9.1)** is a tokenizer, not a grammar — Prism or highlight.js would add tens of kilobytes gzipped to a bundle with a recorded size budget, to decorate text a reader is skimming. Its limits are stated in the file: block comments and template literals are matched within a line, so a multi-line comment greys each line's fragment rather than the whole comment. Strings are matched before comments, so `//` inside a string survives. **The binding now runs both ways (9.3).** Selecting a block highlights its lines; clicking a line selects the block covering it. One-way binding makes the graph authoritative and the code a display; two-way means either pane can be the one you are reading, which is how reviewing actually goes. When several blocks cover a line, the smallest span wins, because that is the block the line specifically belongs to. Panes are now equal width. At 3:2 the graph pane was too narrow to lay out a modest function, which made the graph the pane you scrolled rather than the pane you read. Two things found while doing it, both by running against real data: - **The CFG payload has `edges` and `dominance_frontiers`.** The interface I wrote guessed `df` and omitted `edges` entirely. `edges` is optional in the type even though the current schema always writes it, because a bundle from an older version should produce a stated absence rather than an unmounted room. - **My own test fixture was wrong the same way** — it lacked `edges`, the graph threw, and React unmounted the whole tree, taking three unrelated tests down with it. Worth noting as a shape of failure: one missing field in a fixture reads as three broken features. New tests: 4 for the Spine, covering both binding directions, the synthetic block that says "no source line" rather than `L0`, and the count of blocks that carry lines at all. 11 v2 tests pass, 21 v1 tests unchanged. Also refreshes screenshot 09, which showed the block list this commit replaces.
…at once Finish-work for change 1: 7.1 and 7.2, plus 8.1/8.2 and 6.4. **7.2 — how to reach v2.** Nobody but us knew. The design doc now says it, and explains why it is an environment variable rather than a flag: there is one user-facing flag and this is a development affordance, not a promise to users. It also records the deliberate choice that a selected-but-unavailable v2 fails loudly rather than falling back to v1. **7.1 — the status table.** Change 1's rows now say what exists: the app and shell, the Spine, the harness and gates, the size gate. Rooms are marked not started and say what state they render. **8.1/8.2 — the gates as a set.** `./scripts/test-dashboard.sh` runs the crate's unit tests, the integration gates, both JS suites and the offline check. They were previously five invocations from memory, which is five chances to skip one. **6.4 — the offline gate.** It looks for *resource* references (`src=`, `href=`, CSS `url(...)`) rather than grepping for `http`, because library strings legitimately contain URLs — React's own error messages are `https://react.dev/errors/N` — so a naive grep fails on a correct bundle while saying nothing about whether it fetches anything. One bug found in the script itself: the suite checks matched on `"0 failed"`, which can never appear, because vitest prints a failed count only when something failed. So a passing run was reported as broken. Now it uses exit status. Same class as the earlier `findByText(...).toBeFalsy()` — an assertion that cannot express the good case. Verified: all gates pass — crate 35/0, dashboard_bundle, dashboard_harness, v1 21, v2 11, and both bundles load no external origin. Signed-off-by: Sami Jan <sami.jan@gmail.com>
This is why committing the bundle could never be a no-op: every rebuild of unchanged
source produced a different content hash, so `dist-v2` looked dirty for no reason and
every UI commit appeared larger than it was. I had been restoring it repeatedly without
understanding why.
The cause is a feedback loop through Tailwind's content detection.
`@source` lines **add to** automatic detection rather than replacing it, so unless
`source(none)` is set, Tailwind also scans everything it can reach that is not
gitignored — and that includes `dashboard/dist` and `dashboard/dist-v2`, both tracked on
purpose. Vite empties `outDir` at *write* time, not before transform, so while Tailwind
is deciding which classes exist, the previous build's JavaScript and CSS are still on
disk and get scanned. Their class-name strings feed into the stylesheet, so each build's
CSS depends on the last build's output.
Measured:
clean tree, no dist-v2 -> index-BOEbXtKe.css
rebuild with stale dist-v2 -> a different hash
rebuild again -> another
... never settling
with `source(none)`:
three consecutive builds, two with a stale dist-v2 present
-> index-DYiaFM2a.css, index.v2-kkmpmTj6.js, identical
Stylesheet drops 25,338 -> 20,311 bytes. Those bytes were the spurious class names from
the previous bundle; the real utilities the app uses are all still emitted, and every
legitimate class comes from `src/v2`, which is still scanned explicitly.
This is the same mistake I had already fixed once on the retired prototype — its
`index.css` carries `source(none)` and a comment saying why, and I did not carry it
across when writing the v2 stylesheet. The prototype's note even records the earlier
symptom: 25.89 -> 27.12 KB from a file that was ignored and then not.
Also worth noting for the tracked-dist discipline: a reproducible bundle is a
prerequisite for "regenerate it in the same commit" to mean anything, because otherwise
the regeneration is indistinguishable from a real change.
Signed-off-by: Sami Jan <sami.jan@gmail.com>
… be served
Two findings from chasing a silent no-op.
**The version was missing from the incremental-export gate.** The export is skipped
when the fingerprint matches, and that fingerprint is derived from the *graph* — so it
says nothing about which bundle was extracted. Selecting a different bundle therefore
matched, returned early, and left the previous bundle in `.rgctl/dashboard/`:
discover (v1) -> v1 extracted, fingerprint recorded
RGCTL_DASHBOARD_V2=1 discover -> fingerprint matches -> early return
...and the extracted files are still v1
The operator sets the variable, sees nothing change, and concludes the flag is ignored.
That is exactly the rabbit hole this cost.
`DashboardManifest` gains `bundle_version`, and the gate now requires *both* the
fingerprint and the recorded version to match. `Option` with a default, so a manifest
written before the field existed still deserialises and compares unequal — one forced
re-export, which is the right outcome.
Deliberately not done by folding a version suffix into `export_fingerprint`: the v2
Instrument bar displays that value as a fallback digest (`Instrument.tsx:32`), so a
suffix would show up in the UI.
Verified as a four-step sequence rather than a single assertion:
default -> v1, id="app", 0 react chunks
RGCTL_DASHBOARD_V2=1 -> v2, id="root", 1 react chunk (re-extracted)
same again -> v2, unchanged (correctly skipped)
default -> v1, id="app", 0 react chunks (re-extracted back)
**And when a requested bundle cannot be served, it now says so.** `RGCTL_DASHBOARD_V2=1`
against a build with no v2 bundle produced *no output at all* — the variable was read,
the embed was empty, and every path downstream skipped or fell back without a word.
Silence is the worst response to a request you cannot honour: it sends the reader to
look at their repository, their flags and their branch. `warn_if_selection_unavailable`
prints before the work rather than after it, so a long discover does not have to finish
to report a problem it could have stated at the start.
That second half is **not verified here** and cannot be: this build embeds the v2 bundle
by construction, so the unavailable branch is unreachable without building a binary
without one. Stated rather than implied by the presence of the code.
Signed-off-by: Sami Jan <sami.jan@gmail.com>
…uting
The Constraints section said `.rgctl/dashboard/` "opens from `file://` or a plain static
server". It does not open from `file://`, and I only found out because hash history was
proposed as the fix and turned out to be irrelevant to it.
Vite emits `<script type="module" crossorigin src="./assets/…">`, and Chrome refuses to
execute a module script from a `file://` origin — the origin is opaque, so CORS applies
and the script never runs. Measured: the page loads, the tag is present, `root` stays
empty, nothing renders. Routing never gets a chance to matter, which is why hash history
fixed nothing and was reverted.
The module **worker** is the same story — `useEngineWorker.ts` passes `{ type: "module" }`
— so the engine would not start even if the entry did.
Worth recording that this is **not a v2 property**: v1's built `index.html` carries the
same `type="module" crossorigin`, and I checked upstream's docs before writing this up,
because "the docs claim something the shipping dashboard cannot do" would have been a
useful upstream bug report. They don't claim it. The wrong claim was mine, in this file.
Also records what real `file://` support would cost — an IIFE bundle and a classic worker,
which gives up code splitting for sigma, graph-layout and CodeMirror, and changes how the
engine worker loads. A build-pipeline change, not a flag, and not obviously worth it when
serving the directory is one command.
Signed-off-by: Sami Jan <sami.jan@gmail.com>
The example of an absolute bundle path named a specific developer's repository. It was only an example, but a committed comment is not the place for it — and with `rgctl-tests/ecommerce-java` now the corpus of record, there is no reason for the name to appear at all. Worth noting the rebuild left dist-v2 byte-identical: comments are stripped by the minifier, so a comment-only source change produces no bundle churn. Before the `source(none)` fix that could not be asserted, because every rebuild changed the hash regardless. Signed-off-by: Sami Jan <sami.jan@gmail.com>
The CFG·PDG lens tab was inert and silent. `App.tsx` rendered `<Placeholder room="understand" />` for it, but `Placeholder` is keyed by *room* and has no `understand` entry, so it returned `null` — `lens=cfgpddg` (and dataflow, slice, blast, metrics, time) rendered the Focus panel and an empty body, with nothing saying why. New: `rooms/CfgPdg.tsx` (the lens), `rooms/BlockDetail.tsx` (the block read-out, shared with the Spine), `useCfgDetail.ts` (one sidecar loader, four-valued — loading/ready/absent/failed). `CfgGraph.tsx` exports its layout helpers and gains `showFrontier`, `onToggleFrontier`, `maxHeightClass` and `footnote` props, defaulted so the Spine is unchanged. `Spine.tsx` now distinguishes "no CFG for this subject" from "still loading" — previously a `CfgDetail | null` could not, so it said `loading…` forever. **And a correctness bug found while building it.** The prototype's amber dots — and the v2 port of them — read `dominance_frontiers[i]` as *"block i is in a dominance frontier"*. The direction is the opposite: `cfg_export.rs` writes `out[idx] = dom.frontier(block_id)`, and `DominatorTree::frontier` is DF(i) — the blocks where a φ for definitions made in block `i` belongs. `pdg.rs:400` uses it that way, attaching control dependence *from* the controller in `block_id` to the block it yields. So a φ **site** is a block that appears in *some* row, not one whose own row is non-empty. The dots were on the branch instead of the merge. The figure in the committed screenshot is wrong for the same reason: its **55** is 55 blocks with a non-empty row, not 55 φ sites — the real count for `fetchPositions` is **5** of 64. `frontierBlockIds` now returns the union, `data.ts` states the direction beside the type, the design doc carries a named correction, and the screenshot caption is fixed. A regression test asserts the dot lands on the merge (B2) and not the branch (B1). Verified: v2 suite 11 → **18 passed**, v1 unchanged at 21, `typecheck:v2` clean, bundle 136 KiB gzipped against a 145 budget. The stat row, legend and φ-site count were checked against a real 64-block function, not a fixture.
Reloading the v2 dashboard rendered nothing. No separate web server and no misconfiguration
was involved: `rgctl serve` is an in-process axum/tokio server, files come from
`tower-http`'s `ServeDir`, and a reload asks it for `/understand` — which is not a file, so
it answered **404 with an empty body**.
v1 never hit this because v1 was tabs and its URL never changed. v2 is the first dashboard
with real paths, so it is the first that needs the server to know which paths are *routes*
rather than files.
Measured before the change on a live server: `/` → 200; `/repo`, `/understand`,
`/understand?lens=…` → 404 with an empty body.
`build_app` is extracted from `run_server` so the contract is testable through the same
router the server binds. `is_client_route` decides ownership — GET/HEAD only, never
`/api/…` or `/graphql`, and never a last segment containing `.`, so `/assets/x.js` and
`/favicon.ico` still 404 instead of being handed HTML; a MIME error is the same blank page
by another route. `without_trailing_slash` canonicalises `/understand/` with a 308 because
assets are referenced relatively (`base: "./"`), so a document at `/understand/` would
resolve them against `/understand/assets/…`. A 404 for a client route then serves
`index.html` with 200 — the route exists; the server simply has no file for it.
After: `/` and `/repo` and `/understand?lens=cfgpddg` all 200 `text/html`;
`/understand/` → 308; `/assets/nope.js` and `/api/nope` still 404; `/api/health` still JSON.
POST gets **405** from `ServeDir`, which is truer than 404 and asserted as such.
Outside `rgctl serve` the same rule is the user's to add: nginx `try_files $uri $uri/
/index.html;`, Caddy `try_files {path} /index.html`. Python's `http.server` cannot do it —
which is the second reason a static server is not a substitute for `serve`.
…t overflows The line field read `no source line` for any block whose `start_line` is 0. That string is too long for a 158px block, so it ran into the `entry` / `exit` chip — visible on B5 and B4 in a real graph, where the two labels overlap. `lineLabel` already returned `—` for exactly this case; the call site just didn't use it for line-less blocks, so the helper's behaviour and the rendered text disagreed. The special case is gone and the function is now the single source of the label. The `entry` / `exit` chips carry the structural role, so nothing is lost by shortening the line field to say only whether there is a line. This is also the project's own convention — `—` rather than `0` for an absence, the same distinction fixed as D4 in the v1 dashboard. Test updated: it asserted the long string, and now asserts the rendered pair (`— · N stmt`) rather than the dash alone, because the line field and the statement count share one text node. Asserting on `—` by itself failed for a reason that had nothing to do with the behaviour under test — the same class of mistake as the earlier `findByText(...).toBeFalsy()`. Verified: v2 suite 18 passed, v1 unchanged at 21, typecheck clean, bundle 136 KiB against a 145 budget. Signed-off-by: Sami Jan <sami.jan@gmail.com>
Five lens tabs (Dataflow, Slice/Trace, Blast, Metrics, Time) rendered an empty pane. `Placeholder` is keyed by **room**, so `<Placeholder room="understand" />` looked up no entry and returned `null` — the tab looked functional and showed nothing, which is indistinguishable from "found nothing". Lenses are now keyed by **lens** and each one names itself and says why it is absent. The two reasons are genuinely different, and writing them out is the point: - Dataflow, Slice/Trace and Blast are **code that is not written**, not data that is missing — this bundle already carries the `slice/` details the lenses would read. - Metrics is **blocked**, not unbuilt: `function_metrics.json` is keyed by columnar node `index` while every other artifact is keyed by `function_id`, so there is no join. Printing a zero, or joining by position, would be a fabricated number. - Time needs a second snapshot: node identity is `BLAKE3(path, name, type)`, so a diff is mechanically available, but one bundle is one revision. Also fix the palette: picking a symbol while in Repo set the Focus and left Repo on screen — a room that never renders a Focus — so the graph stayed two actions away. Picking from Repo now navigates to Understand with the subject; picking inside a room that does read the Focus (Security, Review, …) is unchanged and does not drag you out of it. Five tests cover this: three assert the unbuilt lenses explain themselves, one asserts Repo→Understand carries the Focus, one asserts a Focus-reading room is not navigated away from. v2 suite: 18 → 23. Signed-off-by: Sami Jan <sami.jan@gmail.com>
`Unavailable` hard-coded its badge to "not in this bundle" — an *availability* claim. It is the wrong claim for every use: an unbuilt lens has no code, not missing data, and the unbuilt rooms are absent because change 1 builds the frame only. On the Metrics lens the badge sat directly above a sentence explaining that the data is present and cannot be joined, so the panel contradicted itself in two lines. The badge is now a `tag` prop, defaulting to the old wording for callers that genuinely mean availability, with per-lens tags: "code not written" (Dataflow, Slice/Trace, Blast), "blocked: no join key" (Metrics), "needs 2 snapshots" (Time), "not implemented yet" (the rooms). The "data is here" claims are checked against this bundle rather than assumed: `dataflow_detail_dir` and `slice_detail_dir` both point at `slice/`, which holds 608 detail files and a `slice_index.json` with 608 functions, and `blast_available` is true. The Metrics claim is checked too — `function_metrics.json` has 2945 rows keyed `index` 0…2944 with no `function_id` field, while `cfg/` and `sources/` are named by UUID, so there is genuinely no join. The Metrics test now asserts the badge says why, and that "not in this bundle" is absent. Signed-off-by: Sami Jan <sami.jan@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Dashboard v2: the React shell, the Focus, and the Spine lens
Scope: change 1 only — the React shell, the Focus, and the Spine lens.
This branch previously carried three units, because they were developed together. They are now separated:
rgctl review exportThe four change-0 commits were removed by rebase (1,029 lines). The original branch
feat/dashboard-v2still carries them, so nothing is lost, and the Review room can branch from it once sshaaf#117 lands. Diff againstmain: 54 files rather than 62.Fork PR (
samiujan/rgctl). Parts of this are bound forsshaaf/rgctlas separate PRs; this branch iswhere the dashboard work accumulates so it can be read as a whole.
This began as a design proposal and is now an implementation in progress.
The model is confirmed. The maintainer's answer to "is the Focus the model you were
describing?" is yes — unambiguously. That was the bet the whole design rested on: one portable
selection, carried across rooms and lenses, with analysis downstream of it. The rooms below are no
longer being built against a guess. The v2 shell runs against a
real bundle, the Focus is carried across rooms through the URL, and the Spine lens renders source beside
its control-flow graph — the piece the maintainer named as missing on
issue #61.
What it looks like, running
Both screenshots below are the real app against a live 26 MB bundle — 5,345 nodes, 1,479 functions,
154 exported source files — not a mock with a static dataset.
The Instrument bar at the top is persistent and never a destination: graph digest, index freshness,
version, and capability dots derived from
analysis.*_availablerather than asserted. Per-languagecoverage tiers are not in the bundle, so it says so instead of drawing a plausible dot count.
What is in this diff
dashboard/src/v2/dashboard/vite.config.v2.tsdist-v2.docs/design/dashboard-v2.mddocs/design/dashboard-v2/screens/src/cli/review_export.rsrgctl review export(change 0) — also submitted as sshaaf#117.scripts/test-dashboard-*.shv1 is untouched.
dashboard/App.tsxand its 11 tabs are unchanged anddashboard/diststill serves--with-dashboard, so the 18 dashboard test files stay meaningful while v2 is built rather than beingred-lined all at once. There is one flag — no
--with-dashboard-v2, no--dashboard v2; v2 isselected internally until v1 is retired.
Structural decisions worth reviewing
@preact/preset-vite'sreact→preact/compatalias; v2 must not have it. Exactly one config may carry the alias, so itlives in the file it belongs to and neither config contains a branch a reader has to evaluate.
fn:<function_id>today,sk:<stable_key>once the export carries it.The engine has a stable key in analysis storage, but no dashboard index exports it; rather than
treat two different identifiers as one string, the URL says which it holds, so a stored link cannot
silently change meaning. A Focus that no longer resolves renders an explicit stale state.
source line — each says what it cannot show and why. A surface that renders an empty container leaves
the reader to guess whether the answer is "nothing" or "never ran".
Issue sshaaf#61 — what this does, what it skips, and why
The maintainer parked the navigation work on 2026-08-20, and this is a direct response to that comment
rather than to the original issue. It said:
What is skipped, and why
Skipped, not rejected. Each is a view or a flag, and the parked comment says the views were never the
problem — the missing model was. Building the views first is what produced the parked state.
--with-universeand its artifacts--with-dashboard;serve --openpreferring universedashboard/distis compile-time embedded, so this is release engineering too, not only UI.metagraph.jsonreuse, animated fly-toupstream/universebranchmain, written against the pre-rename naming (rg-build,rgbuilder-dashboard).The gate that would un-skip the cosmos
That is the difference between the cosmos as a new mode and the cosmos as one view of a model that works.
Half of it now exists.
One factual correction
sshaaf#61 specifies "Louvain communities". The engine does not use Louvain: community detection is label
propagation (Raghavan 2007) with Newman modularity for the quality score
(
docs/design/graph-metrics-design.md:87,crates/rgctl-analysis/README.md:41), and the v1 dashboard'sown label was corrected on this branch.
communities.jsonis consumed either way, so nothing is blocked —but it is the kind of detail a reader will trust and then repeat.
How it was verified
Against the real bundle, not a fixture, and two bugs were found that built cleanly and passed
type-checking:
dashboard/tsconfig.jsonsets"jsxImportSource": "preact"and includessrc. Any.tsxundersrc/— includingsrc/v2/— therefore compiled its JSX through Preact's jsx-runtime and handedPreact vnodes to real React:
Minified React error #31, an empty#root, and a key list full of Preact's__k/__b/__vinternals. Fixed in both the bundler config and the v2 tsconfig, with the reasoningrecorded, because which config a bundler picks for a file that two configs claim is a resolution detail.
Worth knowing for anyone else adding React under
src/.A CFG block's
start_line: 0is not line zero — it is the absence of a line (synthetic prologueblocks). Rendering it as
L0repeats the—-versus-0confusion fixed as D4 on this branch. About 59%of blocks in the real bundle carry line information; the rest now say
—.The data layer was also rewritten after reading a real bundle: the first version expected
manifest.digestand
sources_index.sources, but the real manifest nests the digest undergraphand the index calls itsarray
files. Both would have rendered an empty shell against real data while passing every type check.The prototype, and why it is gone
A prototype was built first, on this stack, with real data from a live
.rgctl/, to settle the frameworkquestion and the IA. It did that — including the decisive artefact, a Radix
Dialog.Portalrenderingwhere the same code renders nothing through
preact/compat— and it was then retired: the code istagged
mock-v2-prototypefor provenance, and its seven screenshots are committed as the record.It was not extended because it could not answer the question that matters. It had no WASM engine, so
nothing was driven by a real graph; its
Codelens reported a count of source lines rather than source;and its
Spinerendered an empty state for want of a caller/callee index. The one requirement themaintainer singled out was the one it could not demonstrate. Design decisions now belong in the real app,
where they can be tested against a real graph.
Change 1: what it contains, and where it stands
The change is scoped by an OpenSpec proposal (
add-dashboard-v2-shell) that lives in a private planningrepo — upstream gitignores
openspec/(.gitignore:7-8), so it is not in this PR. This is the summary, sothe scope and the outstanding work are readable without it.
52 tasks: 30 done, 22 outstanding, 6 of those partial. A task is only ticked when it is actually done;
where the implementation differs from what the task assumed, the task stays open and says how.
fn:bridgeThe three partials that matter
The Spine is source + a block list, not source + the dominance layout. The CFG sits beside the source
and is bound to it, but the prototype's dominator-depth layout is not ported. The binding is also
one-directional: selecting a block highlights its lines, but selecting lines does not highlight a
block. And the source is not syntax-highlighted — monospace with line numbers. All three are real gaps
against the task text rather than details I am waving through.
Coverage reports capabilities, not per-language tiers. The Instrument dots come from
analysis.*_availableandsemantic.available— real evidence in the bundle — and the label saysper-language tiers not in this bundle. The spec asked for the recorded support tiers; those are notexported, so the intended source does not exist yet. What is there is honest, and it is not what was asked.
useFocusdoes not exist as a hook. The Focus stores only the key and that is enforced by the URLbeing the store, but
App.tsxreads the search param directly. Worth a hook when a second consumerappears, not before.
What is blocked, and on what
vitest.config.tsdeclares no plugins and there is no DOM environment, so nothing that renders can be asserted. All four were verified by hand against a real bundle, which is how two of this branch's bugs were found; a hand check is not a regression test.fn:Focus bridge)The Focus identity — decided
The Focus is
function_id. Not because it is convenient, but because it is the node's deterministicid:
with_file_pathsets it fromdeterministic_node_id(
crates/rgctl-graph/src/schema.rs:444), which isUUIDv5(NAMESPACE_RGCTL, BLAKE3(path ‖ 0xff ‖ name ‖ 0xff ‖ node_type)[..8]). So it is the graph'sStableNodeKeyin UUID form — which is what the design asked the Focus to be — and it was already exportedin every index.
Measured on a three-function repo — indexed, body edited, then renamed:
function_idstable_keyThe alternative was the analysis storage key,
file\x1fname\x1fcode_hash(
crates/rgctl-analysis/src/storage.rs:24), whose own doc comment calls it a stable cache key. It isbuilt for cache invalidation, so it must change when the content does — and it did. A Focus built on it
would expire the moment anyone touched the function, which is exactly when a review link is most
wanted. It was exported, measured, and reverted.
The
sk:/fn:namespacing goes with it: the two turned out to be the same identity expressed twice, oneof them worse.
Still unsolved: renames. The name is part of the hash, so a rename produces a different id and an old
link stops resolving. Nothing in the engine survives a rename without a resolution step, and resolution
needs a baseline snapshot that a pasted URL does not have. The shell renders an explicit stale state that
says why. Open decision 10.2 is whether to build rename resolution at all.
Also outstanding, beyond the task list
bundle, which is what v1 does for these views, but interactive graph expansion is not connected.
"not built yet, change N" state naming what fills them. Change 1 builds the frame and adds no analysis
view; the rooms arrive as changes 2–6.
What review has come back
Two of the upstream PRs have had substantive review, and both are blocked on specific fixes to code in
this branch. The maintainer pre-approved each conditional on one change, so these are the next work
items rather than open questions:
FunctionLocation::end_byteis documented exclusive; my span check wasoffset <= loc.end_byte, so at a boundary where one function's exclusive end equals the next function's start, the earlier one can also match andmin_by_keymay pick the wrong function.offset < loc.end_byte, plus a test with adjacent functions.ViolationLedger::in_memory()to stay read-only, which goes further than read-only — it also discards on-disk history. Regression classification and calendarfirst_seenwould not matchreview check, so the exportedpolicysection can disagree with the command it claims to compose.Both are worth stating plainly because of what they have in common: in each case I satisfied one property
and broke an adjacent one. sshaaf#116 is a boundary error in exactly the place the task was about — resolving a
line to a function — and sshaaf#117 is a case where two of my own stated invariants (read-only, and "cannot
disagree with
review check") conflicted, and I honoured the first at the cost of the second, which wasthe more valuable one.
Also raised, non-blocking: provenance command lines omit flags the export actually used
(
--full-snapshots, depths, fanout), so a consumer should not read them as re-run recipes; anddocs/json-api.md's "each section is byte-identical to what the corresponding command emits" is toostrong for
policyand should be softened to "same builder, same evaluation path, modulo read-only and nobisect".
Relationship to the upstream PRs
8f09655, closed sshaaf#112rgctl slicedefectsrgctl review export(change 0)This branch was rebased onto
mainafter sshaaf#110 merged, which dropped the nine commits already upstream;prerequisite commits that are still under review (sshaaf#109, sshaaf#117) remain in the diff.
What I would like from you
Is the Focus the model you were describing?Answered: yes.lenses is still too many surfaces, say so.
One correction I owe on this: the comment I posted to sshaaf#61 describes the Focus as
"survives renames because it is derived from identity rather than a row index". That is wrong, and I
found out afterwards — the identity is derived from
(path, name, type), so a rename changes it. What itsurvives is re-indexing. It is corrected everywhere in this branch, but the comment on sshaaf#61 still says it,
and the maintainer's yes was given partly against that description.