Tracking: architecture review candidates (docs/architecture-review.html)

[turner]

Umbrella for the 11 refactor candidates in docs/architecture-review.html (committed 2026-07-31, 0e40761).

No release until every candidate is done. Each candidate lands on master as its own PR; the version bump, tag and gh release create happen once at the end. Consumers pin to a tag (juicebox-web and spacewalk → v3.6.2), so a moving master costs them nothing in the meantime.

Where facts live

This issue is the status surface and the only one. It is edited when a candidate lands, not continuously.

Question	Answered in
Is the candidate done?	this table
What do I do next?	docs/juicebox-punch-list.md — the ordered working list
Why was it decided that way?	docs/adr/ — append-only
What does the term mean?	CONTEXT.md
Which skill do I reach for?	docs/agents/triage-labels.md
What did the review originally claim?	docs/architecture-review.html — frozen; cards are not edited, only Outcome boxes are added

Note: an older 7-candidate copy of this review exists outside the repo and renumbers the candidates after 3. docs/architecture-review.html is canonical.

Candidates

7 of 11 done.

#	Candidate	Issues	Status
1	Lift the tile pipeline out of the contact matrix view	#428	✅ done (ecd44d9)
2	Delete the event bus; keep the coordinator	#414	✅ done (66e68ec..3528717) — bus kept, both buses are consumer API; internal subscriptions removed. ADR-0002
3	Collapse the pass-through modules around HICBrowser	#467, #468	✅ done (99c297b, 18ac88b, d535e64) — member-count target retired; delivered 80 → 65
4	Give the browser registry an owner	#476, #478–#483	✅ done (78a5d0b..9fde9a2) — ADR-0004. Closes #384 (open since 2023) and #475. Caveats below
8	Give the browser a teardown that matches its construction	#491–#496	✅ done (000a43a..8d50359) — ADR-0005. dispose() at both levels, reset() keeps identity; four teardown verbs → two. Not breaking. Tests 439 → 497. One release note, below
5	One decoder for session and URL	#499–#509	✅ done (7d93be4..683ed54) — ADR-0006. One decodeSession behind four adapters, encodeSession + round-trip property test, golden snapshot as the gate. Tests 497 → 731. One deliberate break and eight follow-ups, below
9	Give the config schema one reader	#531–#536	✅ done (f08ece8..9bc9583) — ADR-0008. One normalizeSession, run once at the entry; the readers below it read fields; the schema is written down in CONTEXT.md. Tests 731 → 881. One deliberate behaviour change, below
11	Give the track tile one owner	—	⚠️ breaking as scoped
6	Fold StateManager into State, and make restore use the chokepoint	—	not filed
7	Move the gesture state machines behind InteractionHandler	—	not filed — largest remaining deepening
10	One dataset-load path behind one interface	—	not filed — this is the live-map seam

Adjacent, landed alongside: #470 — the intended public surface is now marked in code (js/publicApi.js + test/testPublicApi.js, PR #472).

Still open and not candidate work: #473, #474, #471, #438. #477 (page-scoped viewport CSS variables) is done — 461535a, PR #548, no ADR.

Open follow-ups filed by candidate 5, none blocking: #510, #514, #515, #518, #519, #521, #525, #528.

Candidate 9 as filed

Six tickets, gate first, following candidate 5's shape. GitHub-native blocking edges are set, so the frontier is queryable rather than read off this table.

#	Ticket	Blocked by
#531	Gate: snapshot the resolved config every entry path produces today	— frontier
#532	Extract normalizeSession: a pure, session-shaped normalize stage	#531
#533	Move the remaining normalization across the decode/normalize seam	#532
#534	Delete the duplicate URL-shortcut expansion	#533
#535	Normalize once, at the entry	#533
#536	Downstream readers stop defaulting, and the schema is written down	#535 — landed

All six landed. #534 and #535 are parallel once #533 lands. #533 was expected to be the only ticket that deliberately moves snapshots — it closes three divergences at once: track-default fixing skipped by restoreSession, the selectedGene reconciliation (#481), and syncDatasets honoured on one browser-creation path and ignored on the other.

Two card corrections found while decomposing, both from candidate 5's landing: normalizeConfig already exists in js/createBrowser.js, and fixDefaults now lives in js/sessionCodec.js, not js/urlUtils.js. The card in docs/architecture-review.html still names the old locations.

The consumer-impact block became acceptance criteria rather than prose: normalize defaults and coerces, never rejects (tightening validation would break configs that work today), and browser.config is read back off the instance by juicebox-web, so the resolved config is snapshotted as an observable surface in #531.

Candidate 9's outcome, in four lines

1. "May not need its own ADR" held until the last ticket, and then did not. Candidate 9 gate: snapshot the resolved config every entry path produces today #531–Normalize once, at the entry #535 were all moves against ADR-0006 decision 8. Downstream readers stop defaulting, and the config schema is written down #536 hit the one question decision 8 does not answer: HICBrowser read miniMode as a figure mode and the normalize stage did not, so browser.figureMode was true while the three display flags defaulted on — every entry path agreeing with the others and none with itself. Picking a winner is consumer-visible, so it got ADR-0008: figureMode wins by absorbing miniMode, and a mini map is a figure. ADR-0007 was reserved for --hic-viewport-width/height are page-scoped, so two embeds cannot have different viewport sizes #477, which has since landed without one, so 0007 is unclaimed.
2. The gate's acceptance criterion was wrong on the last ticket, for an instructive reason. Every ticket carried "Candidate 9 gate: snapshot the resolved config every entry path produces today #531's snapshots come back byte-identical". Downstream readers stop defaulting, and the config schema is written down #536 moved 63 of them and could not have done otherwise: a default moved up into the stage becomes a field of the resolved config, and the resolved config is what is snapshotted. Additive movement is forced by the work; the diff was tallied line by line and is three kinds — synchable and backgroundColor appearing everywhere with no behaviour change, the mini-mode fixture moving behaviourally, and one query fixture's displayMode moving to where it was already going. Move the remaining normalization across the decode/normalize seam #533's lesson repeated: collapsing duplicated rules into a shared stage necessarily widens them, and widening is visible.
3. The two "different questions" turned out to be one. Move the remaining normalization across the decode/normalize seam #533 kept DataLoader.loadTracks's annotation-conditioned defaults beside the normalize stage's, arguing the loader owned what the load discovers. It did not: the type those rules keyed on is a field of the config. The real gap was that a track added at runtime through browser.loadTracks met no stage at all. Giving tracks their own door — HICBrowser.loadTracks resolves through normalizeTrackConfigs — let the second copy go.
4. What the schema cost, and what it bought. Three fields turn out to be honoured on one path only and cannot be resolved by the stage: queryParametersSupported is read before it, width/height are read by the constructor before the stage (they were page-scoped until --hic-viewport-width/height are page-scoped, so two embeds cannot have different viewport sizes #477 landed), and config.normalization is checked against a set that does not exist until a dataset is loaded. Naming them in CONTEXT.md is most of the value of writing the schema down — the exceptions were the part nobody could have enumerated from the code.

Candidate 5's outcome, in four lines

1. The card said internal duplication; the contract turned out to be with users. ADR-0006's first fact reframed it: State.stringify() had zero callers in all three repos, and every format except session=blob: is read-only legacy inbound. So one encoder shipped, for the session-JSON form only — writing four would have resurrected a live encoder for formats nothing has written in years.
2. The gate is the reusable part. Golden-file snapshot of the current decoder — the gate for all decoder work #503 snapshotted the whole wire-format corpus before one line of decoder code moved, and Lift format sniffing and the state-decode ladder out as pure functions #504–encodeSession and the round-trip property test #507 passed by coming back byte-identical. Exactly one snapshot moved deliberately across the candidate, logged with the decision that authorised it. Two live bugs had to land first because they legitimately move decoded output: chr1 ≤ chr2 in setView (Enforce chr1 ≤ chr2 in the state chokepoint #499) and the "{}" an empty browser wrote into a session (An empty browser leaves the session instead of corrupting it #500).
3. One deliberate break, and it was not the dead path the ADR assumed. juiceboxURL= was dropped (Drop legacy bit.ly juiceboxURL= support #506) partly on the claim that its bearer token would draw a 401. Measured 9 August: bit.ly expanded it and the session decoded in full. The drop stands on other grounds; the ADR's consequences section was corrected to say it removed live behaviour.
4. What the corpus proves, and what it does not. It pins the decoder's accepted set. Harvest session URLs from external citations to widen the fixture corpus #509 sampled the sent set from our own issue trackers and found one new shape (A numeric seventh state token decodes as a normalization name that no .hic file has #528), then deliberately stopped short of papers, GEO and 4DN — that search was written to run before the collapse, and the window shut at One decodeSession with an injected loader; network I/O leaves the decoder #505. The external population beyond our trackers has never been sampled, and CONTEXT.md now says so.

Candidate 8's outcome, in three lines

1. It grew one member the card never named. contactMatrixView installs gesture handlers on the document, which removing an element cannot take with it (reset() becomes dispose-then-construct on the same instance #494). The card's actual mechanism — the constructor records what it installs outside rootElement, dispose() walks that record — is what caught it. InputDialog was the first instance, not the point.
2. registry.dispose() evicts the registry from the container WeakMap, so dispose-then-init() on the same element is supported rather than accidental. Eviction is identity-checked: a host disposing a stale registry after re-initializing the same container would otherwise delete the live embed's map entry, silently.
3. The review gate is now a test, not a warning. reset() installs a new State, and test/testRepaintDuringReset.js (reset() must install a new State object, not mutate the existing one #495) drives a repaint across a reset and asserts the pass is abandoned on state identity, per reset() during an in-flight render throws: Cannot destructure property 'x' of 'this.browser.state' #469.

Candidate 4's two caveats

1. The runtime click-through against a running juicebox-web was never run. Run 2026-08-11, all boxes passed — Runtime click-through of the browser registry against a running juicebox-web #549. Key browser registries by container element #479's acceptance criteria carried a manual step no skill covers, and it had been verified statically only: 13 call sites read, all resolving one registry, no headless browser available. Candidate 4 now has no unverified claim.
2. --hic-viewport-width/height are page-scoped, so two embeds cannot have different viewport sizes #477 is now done, so "candidate 4 is done" does mean "multi-embed works." --hic-viewport-width/height are written to each browser's rootElement rather than to the page (461535a). The scope unit turned out to be the browser, not the container this candidate keys registries by — juicebox-web clones a second browser into one container, so container scoping would have left last-writer-wins intact inside an embed. ADR-0004's consequences section still says --hic-viewport-width/height are page-scoped, so two embeds cannot have different viewport sizes #477 is open; its appended resolution note is the current word.

Scoping gate

A candidate is not ready to be filed as a Refactor: issue until its card carries a Consumer impact block. All 11 cards now have one.

This replaced a release-time check, because verifying at release time catches breakage after the candidate has been scoped, the design chosen and the code written — by then the wrong verdict is baked into the card. That is what happened to candidates 2 and 3, both reasoned from grep js/, which for an embeddable component is the wrong denominator: HICBrowser is not exported from js/index.js, so the entire browser-instance surface is public in practice and was declared nowhere. ADR-0003 now records the measured surface.

Pre-release consumer verification

Run before the release, not per candidate. Re-measure rather than trust — this list under-counted once already (#474 proposes making the measurement re-runnable).

Spacewalk (13 members + browser.config):

• browser.coordinator.addCallback('onMapLoaded' | 'onBackgroundColorChange' | 'onForegroundColorChange', …) still registers and fires
• browser.activeDataset / browser.contactMatrixView still resolve
• browser.eventBus.subscribe('DidHideCrosshairs', …) still fires
• browser.loadLiveContactMap, browser.parseGotoInput, browser.setCustomCrosshairsHandler, browser.genome, browser.id, browser.rootElement, browser.layoutController, browser.config

juicebox-web (7 members):

• browser.dataset, browser.loadTracks, browser.loadHicFile, browser.loadHicControlFile, browser.reset, browser.layoutController, browser.eventBus

Carried over from candidate 4:

• Runtime click-through of the registry against a running juicebox-web — browser panel add/delete/select, and session save. Filed as Runtime click-through of the browser registry against a running juicebox-web #549; the four registry boxes are checked there.
• The two --hic-viewport-width/height are page-scoped, so two embeds cannot have different viewport sizes #477 sizing boxes on Runtime click-through of the browser registry against a running juicebox-web #549, which the registry click-through cannot cover: clone sizing (the cloned panel comes up the same size as the original and the original does not change) and per-browser scoping (two browsers built with different width/height each render at their own size; one built with neither falls back to the 640 stylesheet default). The second needs a dev page or the console — juicebox-web's clone copies the source's dimensions, so it cannot observe the difference. Run 2026-08-11 on dev/issue-477-per-browser-viewport-size.html, which builds four browsers at three sizes plus one unsized, the unsized one last so the old last-writer-wins behaviour would show; it also asserts nothing writes --hic-viewport-* to the page root. Both passed.

New published members to re-measure into ADR-0003 (candidate 8, #493 and #496 — both now shipped):

• browser.dispose() and registry.dispose() — declared in js/publicApi.js and in ADR-0003 as prose rather than table rows, because the tables measure what hosts use and no host can be measured using a method that did not exist when they were measured
• Release note: a disposed browser now throws DisposedBrowserError rather than silently no-op'ing. Neither known host can hit it today; a third-party embedder might.

Release notes owed by candidate 5 — two more, and the first is the only one an end user can hit:

• ?juiceboxURL= links no longer work (Drop legacy bit.ly juiceboxURL= support #506). The one deliberate break in the wire format, and not the dead path ADR-0006 assumed: measured 9 August, bit.ly still expanded these and the session still decoded. Any published link in that form is now broken; how many exist was deliberately not measured beyond our own trackers (Harvest session URLs from external citations to widen the fixture corpus #509).
• A session saved while a panel was empty now restores with one fewer panel (An empty browser leaves the session instead of corrupting it #500), where before it could not be reloaded at all. An improvement, but browser count no longer survives that round trip.
• Re-measure whether decodeSession/encodeSession stayed internal as intended — they are deliberately not in js/publicApi.js, because a public decoder would be a second, weaker contract we would then owe compatibility to.

Two pre-existing regressions this pass surfaced

Both fixed; recorded because neither threw, both failed as a silently wrong branch, and no test in any of the three repos caught them.

What	Where	Since
juicebox-web subscribed MapLoad, which juicebox.js no longer posts	initializationHelper.js:570	PR #406, shipped v3.1.0, Dec 2025 — 8 months
Spacewalk guarded on dataset.isLiveContactMapDataSet, which no longer exists	sessionServices.js:158, :229	8e1f041, Sept 2024

[turner]

The consumer verification here is in the wrong place in the process

The Pre-release consumer verification section below is good, and its member lists were roughly right (12 Spacewalk / 7 juicebox-web; re-measured as 13 and 7, plus browser.config which was missed). The problem is when it runs.

Verifying at release time catches breakage after the candidate has been scoped, the design chosen and the code written. By then the wrong verdict is already baked into the card. That is what happened:

• Candidate 2 — card says the per-browser bus has "zero subscribers". Spacewalk subscribes DidHideCrosshairs; juicebox-web subscribes four global-bus events. Re-verdicted on Refactor: clean up event system — dead events, hybrid subscriptions, missing unsubscribe #414, where four of the six "dead events" turned out to be live consumer API — and the one event the issue said to preserve (UpdateContactMapMousePosition) is the only genuinely dead subscription in the set.
• Candidate 3 — card counts 31 members carrying "zero behaviour". Five are consumer API, four with no internal callers at all. Target retired on Refactor: collapse the pass-through modules around HICBrowser #467; delivered 80 → 65.

Both were reasoned from grep js/, which for an embeddable component is the wrong denominator. HICBrowser is not exported from js/index.js — hosts get instances from init() — so the entire browser-instance surface is public in practice and declared nowhere.

Two regressions this surfaced, both pre-existing

What	Where	Since
juicebox-web subscribes MapLoad, which juicebox.js no longer posts	initializationHelper.js:570	PR #406, shipped v3.1.0, Dec 2025 — 8 months
Spacewalk guards on dataset.isLiveContactMapDataSet, which no longer exists	sessionServices.js:158, :229	8e1f041, Sept 2024

Neither threw. Both fail as a silently wrong branch. Neither was caught by any test in any of the three repos.

What changed

• ADR-0003 records the measured surface: 12 namespace exports (10 live), 17 browser-instance members, the sub-surfaces reached through them, all 7 posted events with their subscribers, and the event payload shapes that are contract. Documented, not frozen — no code changes, and marking the intended surface in code is deferred to its own candidate rather than smuggled into a docs ADR.
• All 11 cards in docs/architecture-review.html now carry a Consumer impact block, with a correction banner at the top. Five are breaking as currently scoped: 2, 4, 5, 8, 11.

The five that need rescoping before they are picked up

• 4 (browser registry) — highest exposure on the list. juicebox-web uses every registry export, getCurrentBrowser at 10 sites, and reads getCurrentBrowser().config. "An object returned by init()" changes what init resolves to. Must be additive or it is a major version.
• 5 (session/URL decoder) — the encoded format is a contract with users, not just the host apps: sessions are shared as URLs, so old links must still decode. The report already calls this the strongest ADR opportunity; the reason is the external consumer, not the internal duplication.
• 8 (teardown) — juicebox-web calls browser.reset() and then loadHicFile() on the same instance, next line. "reset becomes dispose-then-construct" breaks that unless identity survives.
• 11 (track tile) — the TrackXYPairLoad / TrackXYPairRemoval payload is the track pair, and juicebox-web reads data.track.config.format off it then hands it back to layoutController.removeTrackXYPair. The pair's shape is published.
• 2 (event bus) — above.

Proposed process change

Move the consumer check from a release gate to a scoping gate: a candidate is not ready to be filed as a Refactor: issue until its card has a Consumer impact line. Keep the pre-release checklist as the backstop — it is still needed, it just cannot be the only pass.

The checklist below should also be re-measured rather than trusted: it under-counted Spacewalk by one and missed browser.config entirely.

Tracker update

#	Candidate	Issue	Status
3	Collapse the pass-through modules around HICBrowser	#467	done (99c297b, 18ac88b) — member-count target retired
3b	One accessor vocabulary for dataset and state	#468	open — unblocked by #467; re-checked, decision holds
2	Delete the event bus; keep the coordinator	#414	open — scope revised, see comment

Also open and unrelated to this pass: #469 (reset() during an in-flight render throws), found while smoke-testing #467 and confirmed pre-existing.