The question
With several substantial features recently landed or in flight in explorer.qmd
— heatmap mode (#241, #242, #246), the collection facet (#244), cross-filter
facets (#156), URL-state contract (#163/#164) — is the explorer codebase messy
enough that we should refactor before adding the next big feature, or keep
shipping incrementally?
This issue is the analysis to decide that, not a proposal to rewrite.
Diagnosis (measured)
explorer.qmd is ~4,580 lines across 9 OJS cells. The complexity is not
uniform — it's concentrated in one cell:
| Block |
~Lines |
Health |
~40 helper functions (facetFilterSQL, readHash/buildHash, card renderers, bbox math, …) |
730 |
✅ small, single-purpose |
viewer cell (Cesium setup + click/hover handlers) |
270 |
✅ cohesive |
facetFilters cell |
105 |
✅ fine |
tableView cell |
455 |
⚠️ large but cohesive |
zoomWatcher cell |
~2,200 |
❌ god-closure |
perfPanel cell |
95 |
✅ fine |
~48% of the file is a single closure. zoomWatcher co-mingles: H3 cluster
loading (loadRes), point-mode sample loading (loadViewportSamples), mode
transitions (enterPointMode/exitPointMode/applyLayerVisibility), the
heatmap subsystem (refreshHeatmap/renderHeatmap/clearHeatmap + imagery
layer), camera (moveEnd/camera.changed) handlers, facet-count refresh, and
several stale-request guards. Recent features (heatmap, collection facet) mostly
landed in or adjacent to this cell.
Why it's monolithic (it's the platform, not just neglect)
Quarto OJS has no ergonomic way to share mutable state across cells, so
stateful, tightly-coupled logic tends to collapse into one closure (the shared
loading, requestId, loadResGen, heatmapReqId, currentRes, etc.).
EXPLORER_STATE.md already documents the escape hatch — hoisting state onto
viewer._globeState / viewer._heatmap* so logic can be split across cells.
The risk / tension
explorer.qmd is dense with hard-won, issue-specific fixes. A non-exhaustive
list visible in the comments: #185 (terrain depth-test bypass), #221 (viewport
padding so map/table/count agree), #233 (heatmap mutual-exclusivity + self-
refresh race), #156 (cross-filter pid IN (...) predicate), _suppressHashWrite
(camera↔hash echo prevention), loadResGen/requestId/freshSelectionToken
(stale-result guards), #127 (TOC overlap intercepting clicks).
A big-bang rewrite is precisely the move that silently drops one of these guards
and reintroduces a solved bug. The cost of a regression here is high (the page is
the project's flagship demo).
What we already have for safe decomposition
Two of the three prerequisites for a low-risk refactor are already in place:
- Test net: ~24 test files under
tests/ (Playwright + pytest), plus a
search-perf baseline.
- State contract:
EXPLORER_STATE.md defines URL params, hash channel,
DOM-as-state, and viewer.* widget state — the seams along which logic can be
separated.
- Module-extraction precedent: the explorer already imports an ES module at
runtime — await import('assets/js/source-palette.js'). Pure logic can move
into assets/js/*.js the same way and become independently unit-testable.
Options
A — Refactor-first (restructure explorer.qmd before the next feature).
Cleaner base for future work. Cons: high regression risk against the dense
fix-set above; a large, hard-to-review diff; delays features; and it collides
with any concurrent work in this same file. Not recommended now.
B — Don't refactor; keep shipping. Fastest short term, lowest immediate
risk. Cons: zoomWatcher keeps accreting; each subsequent change gets slower and
riskier. Defers the problem rather than addressing it.
C — Strangler / refactor-as-you-go (recommended). Decompose incrementally
behind the existing test net:
- Strengthen tests around whatever the next feature touches (cheap, compounding).
- Extract pure logic into
assets/js/*.js ES modules — SQL builders
(facetFilterSQL, sourceFilterSQL, textSearchWhere/Score), the hash
codec (readHash/buildHash), bbox math (paddedViewportBounds/
viewerBboxSQL), card HTML renderers, escapers. Pure functions → easy unit
tests, ~500 lines out of the qmd, near-zero risk.
- Later, last, one seam at a time: carve the heatmap subsystem out of
zoomWatcher into its own cell/module (its state is already namespaced
viewer._heatmap* / heatmapImageryLayer). Highest-risk seam — only behind
green tests.
Recommendation
Don't refactor-first; adopt C. The codebase hasn't crossed the
"stop-and-rewrite" line — it's one bloated closure plus otherwise-clean code,
with a real test net and a state contract. Highest-leverage, lowest-risk path is
opportunistic module extraction that lets zoomWatcher shrink across a few small
PRs, rather than blocking features on a big restructure.
Proposed next step
Before committing to any option, produce a refactor map: enumerate every
extractable seam in zoomWatcher (and the pure helpers), risk-rated and in
dependency order, with the target module/cell for each. Low commitment; doubles
as the execution plan if we pick C.
Open questions for the team
- Is the single-file Quarto-OJS approach a long-term fit, or should the explorer
eventually become a bundled app (e.g. a Vite entry embedded in the page) with
real modules + a test harness? (Bigger fork; out of scope for C but worth
naming.)
- How do we coordinate refactor PRs with concurrent feature work in
explorer.qmd to avoid merge churn? (Refactor windows? Feature freeze on the
touched seam?)
- Is
EXPLORER_STATE.md the right home to also document the module boundaries
as they're extracted?
Analysis generated while landing #244 / #246; line counts are against main at
the time of writing.
The question
With several substantial features recently landed or in flight in
explorer.qmd— heatmap mode (#241, #242, #246), the collection facet (#244), cross-filter
facets (#156), URL-state contract (#163/#164) — is the explorer codebase messy
enough that we should refactor before adding the next big feature, or keep
shipping incrementally?
This issue is the analysis to decide that, not a proposal to rewrite.
Diagnosis (measured)
explorer.qmdis ~4,580 lines across 9 OJS cells. The complexity is notuniform — it's concentrated in one cell:
facetFilterSQL,readHash/buildHash, card renderers, bbox math, …)viewercell (Cesium setup + click/hover handlers)facetFilterscelltableViewcellzoomWatchercellperfPanelcell~48% of the file is a single closure.
zoomWatcherco-mingles: H3 clusterloading (
loadRes), point-mode sample loading (loadViewportSamples), modetransitions (
enterPointMode/exitPointMode/applyLayerVisibility), theheatmap subsystem (
refreshHeatmap/renderHeatmap/clearHeatmap+ imagerylayer), camera (
moveEnd/camera.changed) handlers, facet-count refresh, andseveral stale-request guards. Recent features (heatmap, collection facet) mostly
landed in or adjacent to this cell.
Why it's monolithic (it's the platform, not just neglect)
Quarto OJS has no ergonomic way to share mutable state across cells, so
stateful, tightly-coupled logic tends to collapse into one closure (the shared
loading,requestId,loadResGen,heatmapReqId,currentRes, etc.).EXPLORER_STATE.mdalready documents the escape hatch — hoisting state ontoviewer._globeState/viewer._heatmap*so logic can be split across cells.The risk / tension
explorer.qmdis dense with hard-won, issue-specific fixes. A non-exhaustivelist visible in the comments: #185 (terrain depth-test bypass), #221 (viewport
padding so map/table/count agree), #233 (heatmap mutual-exclusivity + self-
refresh race), #156 (cross-filter
pid IN (...)predicate),_suppressHashWrite(camera↔hash echo prevention),
loadResGen/requestId/freshSelectionToken(stale-result guards), #127 (TOC overlap intercepting clicks).
A big-bang rewrite is precisely the move that silently drops one of these guards
and reintroduces a solved bug. The cost of a regression here is high (the page is
the project's flagship demo).
What we already have for safe decomposition
Two of the three prerequisites for a low-risk refactor are already in place:
tests/(Playwright + pytest), plus asearch-perf baseline.
EXPLORER_STATE.mddefines URL params, hash channel,DOM-as-state, and
viewer.*widget state — the seams along which logic can beseparated.
runtime —
await import('assets/js/source-palette.js'). Pure logic can moveinto
assets/js/*.jsthe same way and become independently unit-testable.Options
A — Refactor-first (restructure
explorer.qmdbefore the next feature).Cleaner base for future work. Cons: high regression risk against the dense
fix-set above; a large, hard-to-review diff; delays features; and it collides
with any concurrent work in this same file. Not recommended now.
B — Don't refactor; keep shipping. Fastest short term, lowest immediate
risk. Cons:
zoomWatcherkeeps accreting; each subsequent change gets slower andriskier. Defers the problem rather than addressing it.
C — Strangler / refactor-as-you-go (recommended). Decompose incrementally
behind the existing test net:
assets/js/*.jsES modules — SQL builders(
facetFilterSQL,sourceFilterSQL,textSearchWhere/Score), the hashcodec (
readHash/buildHash), bbox math (paddedViewportBounds/viewerBboxSQL), card HTML renderers, escapers. Pure functions → easy unittests, ~500 lines out of the qmd, near-zero risk.
zoomWatcherinto its own cell/module (its state is already namespacedviewer._heatmap*/heatmapImageryLayer). Highest-risk seam — only behindgreen tests.
Recommendation
Don't refactor-first; adopt C. The codebase hasn't crossed the
"stop-and-rewrite" line — it's one bloated closure plus otherwise-clean code,
with a real test net and a state contract. Highest-leverage, lowest-risk path is
opportunistic module extraction that lets
zoomWatchershrink across a few smallPRs, rather than blocking features on a big restructure.
Proposed next step
Before committing to any option, produce a refactor map: enumerate every
extractable seam in
zoomWatcher(and the pure helpers), risk-rated and independency order, with the target module/cell for each. Low commitment; doubles
as the execution plan if we pick C.
Open questions for the team
eventually become a bundled app (e.g. a Vite entry embedded in the page) with
real modules + a test harness? (Bigger fork; out of scope for C but worth
naming.)
explorer.qmdto avoid merge churn? (Refactor windows? Feature freeze on thetouched seam?)
EXPLORER_STATE.mdthe right home to also document the module boundariesas they're extracted?
Analysis generated while landing #244 / #246; line counts are against
mainatthe time of writing.