Skip to content

Instantly share code, notes, and snippets.

@Ovid

Ovid/architecture-review.md

Last active July 20, 2026 17:51
Show Gist options

  • Select an option

    Learn more about clone URLs
  • Save Ovid/7938d58e58558dd553e18b83e7add975 to your computer and use it in GitHub Desktop.

Select an option

Learn more about clone URLs
Save Ovid/7938d58e58558dd553e18b83e7add975 to your computer and use it in GitHub Desktop.
Download ZIP
Fable Architecture Review
Raw
architecture-review.md

Human Note

The following is an architecture report on experimental software built with Fable. Parts of this report have been slightly altered to obscure what is being built, but no material findings have been changed or omitted.

The codebase is small (roughly 10K lines of code) and Fable had no significant problems in creating the software. To create the original spec and plan, we used Superpowers and PAAD to ensure that both were of the highest quality. In other words, we used spec-driven development with an extremely detailed, high-quality specification.

For the implementation, we deliberately did not use the PAAD methodology. We wanted to assess the quality of the software that Fable can natively build without engineering assistance.

After the implementation, we manually verified that the software behaved as intended. It mostly did, with a few quirks, but it was suitable. Then we ran PAAD's /agentic-architecture tool to generate the following report. This spawns multiple subagents, each specializing in a particular kind of architectural area, and a final validator to deduplicate and verify the findings. We then used /pushback to validate. It's clean and correct.

The final verdict is damning. Fable can brute force past architectural challenges, but without clear and explicit guidance, it creates the same technical debt that less powerful models create. Thus, Fable does not produce production ready code. You can jump straight to the flaws, if desired (reading full report goes into detail on them).

Counts: 17 verified strengths (7 High, 9 Medium, 1 Low) and 33 verified flaws (4 High, 17 Medium, 12 Low), distilled from 79 raw findings by 5 specialist agents plus an adversarial verification pass. For a small, 10K LOC codebase that's built in a few hours this is terrible. The PAAD methodology avoids these flaws and builds production-ready code, but at a higher unit cost (tokens+time). This is offset by lower TCO — less rework, and fewer bugs or security holes.

Architecture Report — [REDACTED]

Date: 2026-07-18
Commit: 06a5cf9663ba6be71c99606dc6dd5ceaadb891e3
Languages: [REDACTED]
Key directories: src/sim/ (deterministic core), src/state/ (sim↔React bridge), src/ui/, data-raw/, scripts/, docs/adr/
Scope: full repository

Repo Overview

[REDACTED]. ~65 source files, ~10.5k lines, 31 commits, 16 ADRs. Architecture is three layers with a declared strict dependency direction sim → state → ui: a headless deterministic sim core (event-sourced — the save file is the [REDACTED] action log, rewalked via rewalkScenario()), a thin state bridge (module-level SimScenario + Zustand mirror + fixed-timestep rAF loop), and a [REDACTED]. Ephemeris data is baked at build time from committed JPL Horizons JSON.

Note: all 33 flaws below survived verification, slightly exceeding the template's 25-item guideline; none were cut.

Strengths

[S1] Event-sourced domain model with the action log as the single narrow seam

  • Category: S13 Domain modeling strength (also S3)
  • Impact: High
  • Explanation: One discriminated ConsumerAction union and one apply() are the entire cross-layer command surface; the save format shares the same seam; lagged knowledge is encoded in the types (e.g. alive vs knownDead), and the Ascension verdict is decided at send time but revealed at return time.
  • Evidence: src/sim/scenario.ts:78-94 (ConsumerAction), :306-313 validate-before-log, :249-261 (CastStatus), :529-544 (feed verdict); src/state/save.ts:13-27; src/state/simStore.ts:68-85 (AscensionMirror deliberately withholds the verdict)
  • Found by: Structure & Boundaries, Coupling & Dependencies, Error Handling & Observability

[S2] Three-layer dependency direction holds in production code

  • Category: S1 Clear modular boundaries
  • Impact: High
  • Explanation: Grep-verified zero React imports in src/sim/ and zero ui imports in src/state/; the sole exception is a test file (F28). Authored content is quarantined in sim/data/; three.js scenes are lazy-loaded so the boundary is also a bundle boundary.
  • Evidence: src/App.tsx:17-19 (lazy scene imports); full-tree grep of src/sim/ for react/state/ui imports: none
  • Found by: Structure & Boundaries

[S3] Single-writer write-path discipline enforced end to end

  • Category: S4 Dependency direction is stable (also S6)
  • Impact: High
  • Explanation: Zero scenario.apply/scenario.advance calls or state assignments anywhere in src/ui/; all 16 action types route through store wrappers, and every sim apply case throws before its first mutation, so a save can never contain an unrewalkable action.
  • Evidence: src/sim/scenario.ts:306-313, single eventLog.push at :712; grep of src/ui/ for scenario.apply: zero hits
  • Found by: Coupling & Dependencies, Integration & Data

[S4] Sim core is a textbook DAG with pure leaf finding-modules

  • Category: S3 Loose coupling
  • Impact: High
  • Explanation: Finding modules never import scenario.ts back — orchestrator.ts and deeptime.ts have zero imports, curator.ts imports geometry/data only — so every feature module stays headlessly testable and no scenario↔feature cycle exists.
  • Evidence: src/sim/orchestrator.ts (zero imports), src/sim/deeptime.ts (zero imports), src/sim/curator.ts imports (bodies/constants/evidence/ephemeris/vec3); no circular imports found in the sim graph
  • Found by: Coupling & Dependencies

[S5] Torn saves are structurally impossible

  • Category: S12 Resilience patterns
  • Impact: High
  • Explanation: serialize() emits only {version, epoch, seed, ConsumerDeltas} — never mutable derived state — and apply() is synchronous with autosave running between frames, so no instant exists at which a half-applied mutation can be captured.
  • Evidence: src/state/save.ts:20-27 (serialize); src/sim/scenario.ts:718-755 (idempotent settle()), :296-300 (negative-dt guard), :1218-1220 (non-decreasing rewalk guard)
  • Found by: Integration & Data

[S6] Deterministic headless test seam with rewalk-hash assertions

  • Category: S11 Testability & coverage (also S8)
  • Impact: High
  • Explanation: 154 tests across 15 files run the whole walk headless in under a second, including full endng walk; stateHash() is asserted in 13 places to prove bit-for-bit rewalk equivalence, giving the determinism pillar a direct cheap oracle. Sim layer sits at ~92% statement coverage.
  • Evidence: src/sim/travel.test.ts:92-106 (rewalk hash + mid-flight save round-trip), src/sim/scenario.ts:1180-1199 (stateHash)
  • Found by: Security & Code Quality, Error Handling & Observability

[S7] Subsystem cohesion: one mechanic, one file, one test

  • Category: S2 High cohesion
  • Impact: High
  • Explanation: Each mechanic is a small pure module with a colocated test and a header docstring naming its ADR and determinism argument.
  • Evidence: src/sim/chorus.ts (102 lines), credibility.ts (67), curator.ts (347, single pure curatorView), deeptime.ts (44), orchestrator.ts (58)
  • Found by: Structure & Boundaries

[S8] Randomness discipline: no wall-clock or unseeded randomness in sim/state

  • Category: S6/S9/S12
  • Impact: Medium-High
  • Explanation: Math.random/Date.now appear only in prohibition comments; transmitter lifetimes are a closed-form FNV hash of (seed, bodyId) — savescum-proof and immune to call-count effects; the Prng field is never even consumed, so no call-order randomness exists.
  • Evidence: src/sim/scenario.ts:922-929 (transmitterLifetimeS), :265 and src/sim/prng.ts:5 (prohibitions)
  • Found by: Integration & Data, Error Handling & Observability, Security & Code Quality

[S9] Data-pipeline provenance and reference-vector testing against external ground truth

  • Category: S6 (also S11)
  • Impact: Medium
  • Explanation: Raw JPL Horizons JSON is committed as provenance; the generated file carries source/fetch-date/units headers; ephemeris tests assert against Horizons vectors at three epochs — external ground truth, not self-referential snapshots. (Caveats: F21, F30.)
  • Evidence: data-raw/, src/sim/data/elements.gen.ts:1-10 (provenance header), scripts/bake-elements.mjs:26-31 (field/unit validation), src/sim/ephemeris.test.ts
  • Found by: Integration & Data, Security & Code Quality

[S10] Consistent diegetic error surfacing; hint and rejection text cannot disagree

  • Category: S7 Robust error handling
  • Impact: Medium
  • Explanation: The sim throws walk-readable sentences captured into 7 per-domain error slots rendered beside the causing control; introductionShortfall/eraseShortfall return the same sentences the sim throws, so button hints and rejections share one source. (Caveat: F10.)
  • Evidence: src/state/simStore.ts:154-165 (error slots), src/sim/orchestrator.ts:42-58
  • Found by: Error Handling & Observability

[S11] Fail-fast guards and explicit corruption detection at sim edges

  • Category: S7 (also S8)
  • Impact: Medium
  • Explanation: Negative dt, non-monotonic delta logs, unknown save versions, invalid gamma/distances/ids all throw with labeled messages rather than propagating NaN; load never crashes the app. (Major caveat: the load-path diagnostics are then swallowed — F3.)
  • Evidence: src/sim/scenario.ts:296-300, :1217-1220 ("corrupt save: delta log epochs must be non-decreasing"); src/state/save.ts:45; src/sim/travel.ts:50, ephemeris.ts:32,40, evidence.ts:149-151
  • Found by: Error Handling & Observability, Security & Code Quality

[S12] No injection or network surfaces

  • Category: S10 Security built-in
  • Impact: Medium
  • Explanation: Zero dangerouslySetInnerHTML/innerHTML/eval/new Function/fetch/XMLHttpRequest/WebSocket in src/; CLAUDE.md's "no runtime network calls" claim verified true in code.
  • Evidence: full-tree greps, all zero hits; index.html loads only the local module script
  • Found by: Security & Code Quality

[S13] Pragmatic, ceiling-documented abstractions; no speculative machinery

  • Category: S14 Simple, pragmatic abstractions
  • Impact: Medium
  • Explanation: 70-line versioned save module, 54-line loop with background-tab clamp, FNV-1a hash chosen for cheapness, one enforcement point for panel exclusivity; approximations are stated where made with revisit conditions. No single-implementation interfaces, factories, or DI machinery anywhere.
  • Evidence: src/state/simLoop.ts:23-25 (tab clamp), src/state/simStore.ts:203-214 (PANELS_CLOSED), src/sim/messages.ts:9-16 and constants.ts:31-35 (documented approximations)
  • Found by: Structure & Boundaries, Coupling & Dependencies

[S14] Lean dependency footprint

  • Category: S5 Dependency management hygiene
  • Impact: Medium
  • Explanation: Six runtime dependencies, no utility-library bloat, no runtime data-fetching; three.js weight lazy-loaded off the initial path. (Caveat: one of the six is unused — F26.)
  • Evidence: package.json dependencies; src/App.tsx:17-19
  • Found by: Coupling & Dependencies

[S15] constants.ts as documented single source; UI imports sim constants rather than copying values

  • Category: S9 Configuration discipline
  • Impact: Medium
  • Explanation: Physical constants carry citations (IAU 2012 AU, DE440 GM_sun); UI panels import CHORUS_MIN_GAMMA, DEEP_TIME_MIN_GAMMA, ORCHESTRATOR_* etc. from sim modules; tuning surfaces are labeled provisional. (Caveat: formulas and epsilons are still copied — F11/F12/F31.)
  • Evidence: src/sim/constants.ts; src/ui/Hud.tsx:12-30, EvidencePanel.tsx:8-12, ConcordatPanel.tsx:10-15; src/sim/scenario.ts:226-236 (ENDSCENARIO_*)
  • Found by: Error Handling & Observability

[S16] Versioned save with migration switch and non-crashing load since M1

  • Category: S12 Resilience patterns
  • Impact: Medium (capped by F2 — the version covers the envelope only)
  • Explanation: Version field, migration point, and defensive load existed from the first save commit — unusually disciplined scaffolding. The recovery policy is flawed (F3), but the scaffolding is right.
  • Evidence: src/state/save.ts:11,39-47,58-66; present since commit 4e4a8ee
  • Found by: Integration & Data

[S17] Coverage tooling wired and gitignored

  • Category: S11
  • Impact: Low
  • Explanation: make cover runs vitest coverage; the coverage/ output directory is properly gitignored, not committed cruft.
  • Evidence: Makefile cover: target; @vitest/coverage-v8 in devDependencies
  • Found by: Security & Code Quality

Flaws/Risks

[F1] Finding resolution order depends on tick size — rewalk/skip can produce a different finding than live walk

  • Category: 19 Lack of idempotency (determinism-invariant violation)
  • Impact: High
  • Explanation: settle() resolves concurrent findings in fixed code order (deep-time block before ascension-feed block), each guarded first-finding-wins — not in epoch order as ADR 0003 promises. A pending ascension feed (resolves in hours) and a lit deep-time leap (arrives years later) can legally coexist; fine-tick live walk crosses the feed first → ascension finding, while loading a save past the flare (or pressing "Skip to the flare", a single epoch jump) runs the deep-time block first → deep-time finding, with divergent finding id, epoch, tauOffset, and conveyor state.
  • Evidence: src/sim/scenario.ts:716-756 (settle(), deepTime at :736-745 before feed at :749-755); :603-637 (deeptime.dilate never checks a pending feed); src/state/save.ts:29-37 (deserialize jumps epoch then advance(0)); src/state/simStore.ts:524-533 (skipToFlare)
  • Found by: Integration & Data; verifier CONFIRMED by walking the scenario end-to-end

[F2] Save version covers only the envelope; any rebalance silently invalidates and destroys old saves

  • Category: 24 Inconsistent API contracts
  • Impact: High
  • Explanation: Rewalk re-validates every logged action against current tuning and content (driver checks, availableFrom, inbox membership, gamma minima), so the real save contract includes every tuning constant — yet SAVE_VERSION = 1 has never been bumped while the action union grew from 8 to 16 types. CLAUDE.md blesses rebalancing as "a data change", but any such change can make rewalk throw → load returns null → fresh scenario → autosave overwrites the old save within 30 s.
  • Evidence: src/state/save.ts:11 (SAVE_VERSION = 1); src/sim/scenario.ts:345,570,624 (driver), :456 (availableFrom), :381 (inbox membership); src/state/simLoop.ts:37-40 (autosave)
  • Found by: Integration & Data

[F3] Corrupt/unknown-version saves are silently discarded, then overwritten

  • Category: 26 Poor transactional boundaries (also 20)
  • Impact: High
  • Explanation: loadFromStorage's blanket catch { return null } swallows the deliberately good downstream diagnostics ("unknown save version", "corrupt save: …epochs must be non-decreasing"); nothing is quarantined, backed up, or logged, and the 30 s autosave clobbers the possibly recoverable save under the same key. Downgrading after a future version bump destroys the save by design.
  • Evidence: src/state/save.ts:58-66, excerpt: catch { return null; }; src/state/simStore.ts:18; src/sim/scenario.ts:1219
  • Found by: Integration & Data, Error Handling & Observability

[F4] State layer (simStore/simLoop/save) has zero test coverage

  • Category: 32 Missing test coverage for critical paths
  • Impact: High
  • Explanation: All 15 test files live in src/sim/; nothing imports simStore.ts (619 lines) or simLoop.ts, yet simStore contains real logic — skip sequences driving the sim via debug.setEpoch, arrival detection, error capture, the module boot path. This is exactly the layer where F1's skip trigger and F3's load path live. save.ts:45-69 (localStorage/migration edges) is also uncovered.
  • Evidence: src/state/simStore.ts:448-457,524-564 (skip sequences), :221-235 (arrivalFrom); coverage run: 154 tests, state layer absent from report
  • Found by: Security & Code Quality

[F5] No React error boundary or global error handlers; render throws blank the app

  • Category: 21 No observability plan
  • Impact: Medium
  • Explanation: No error boundary, no window.onerror/unhandledrejection, zero console.* in production code; render-path non-null assertions can white-screen the app with no message. A refresh recovers via the autosave boot path, so the cost is a blank page, up to 30 s of lost progress, and zero diagnostics.
  • Evidence: src/main.tsx/src/App.tsx (nothing); src/ui/Hud.tsx:670 (useSimStore((s) => s.transit)!), :548; src/ui/ConcordatPanel.tsx:53 (joinedEpoch!)
  • Found by: Error Handling & Observability

[F6] Six UI components bypass the Zustand mirror and read the live SimScenario during render

  • Category: 13 Inconsistent boundaries (also 6, 17)
  • Impact: Medium
  • Explanation: The store's own docs promise a "thin Zustand mirror", and AscensionMirror deliberately withholds the feed verdict — yet FindingOverlay reads scenario.state.ascension?.feed?.flaws directly, and five other components read live scenario state in render, relying on no-op subscriptions (useSimStore((s) => s.epoch);) to force re-renders — tearing-prone under React 19 concurrent rendering. Writes remain fully disciplined; the erosion is read-side only.
  • Evidence: src/ui/Inbox.tsx:28,33; Hud.tsx:41,605 (planTrip(..., scenario.state.conveyor.aKmS2)); ConcordatPanel.tsx:85,90; EvidencePanel.tsx:34,184,186; FindingOverlay.tsx:72; NavMap.tsx:152; src/state/simStore.ts:68-85
  • Found by: Structure & Boundaries, Coupling & Dependencies, Integration & Data, Error Handling & Observability (4-specialist agreement)

[F7] Shotgun surgery: every finding/mechanic touches 6–10 files across three parallel dispatch sites

  • Category: 9 Shotgun surgery
  • Impact: Medium
  • Explanation: Each feature must be threaded through the action union, the lockout set, an apply() case, settle(), nextEventEpoch(), a mirror field, a hand-written wrapper, a dedicated error slot, an finding card, and a help section. Git confirms the pattern is mechanical: the last three finding commits touched 8, 8, and 10 files respectively.
  • Evidence: src/sim/scenario.ts:78-94,189-203,331-711,716-756,1067-1113; src/state/simStore.ts:154-165,385-618; commits b069937, cb11ae6, ec33a6c (git show --stat)
  • Found by: Structure & Boundaries

[F8] scenario.ts is the unstable hub / emerging god object

  • Category: 2 God object (also 4)
  • Impact: Medium
  • Explanation: 1225 lines importing 21 sim modules + 5 data modules, owning the action union, apply/advance/settle, and every derived query — highest fan-in × fan-out in the repo, modified by every feature commit. Mitigated by real delegation to pure subsystems, but apply() alone is a ~380-line 16-way switch and the file grows monotonically per feature.
  • Evidence: src/sim/scenario.ts:263-1200; transmitterLifetimeS (:922-929) duplicates the FNV-1a loop in stateHash (:1193-1197); direct test file scenario.test.ts is 57 lines
  • Found by: Structure & Boundaries, Coupling & Dependencies

[F9] Full mirror rebuilt every rAF frame, even paused; O(content) derived state with no memoization

  • Category: 3 Tight coupling (chatty-recomputation analog of 15)
  • Impact: Medium
  • Explanation: syncFromSim() runs unconditionally each animation frame and mirror() recomputes unreadCount() (full inbox: every letter × lag × scenarioline walk), nextEventEpoch(), curator(), cast(), chorusTranscripts(), credibility() — all returning fresh references, so subscribed panels re-render at display refresh rate regardless of change. Inbox then computes scenario.inbox() again in render.
  • Evidence: src/state/simLoop.ts:36; src/state/simStore.ts:237-318 (mirror()); src/ui/Inbox.tsx:33
  • Found by: Coupling & Dependencies, Integration & Data

[F10] Inconsistent catch discipline: three store actions let sim throws escape into React handlers

  • Category: 34 Inconsistent error conventions (also 20)
  • Impact: Medium
  • Explanation: 12+ actions wrap scenario.apply in try/catch with a diegetic error slot, but markRead, collectEvidence, and redriver call it bare; the sim throws for all three, so a stale-mirror click throws uncaught in a React event handler — action lost, nothing surfaced.
  • Evidence: src/state/simStore.ts:566-570,572-576,590-594; src/sim/scenario.ts:382,445-461,354-357
  • Found by: Error Handling & Observability

[F11] Driver formula duplicated in the HUD

  • Category: 25 Business logic in the UI
  • Impact: Medium
  • Explanation: Hud.tsx re-derives const cost = gamma - 1; instead of calling deepTimeDriver() — which it could import from a module it already imports from. A rebalance of the blessed tuning surface silently desyncs the planner's cost display and afford-check from what scenario.apply charges.
  • Evidence: src/ui/Hud.tsx:494 vs src/sim/deeptime.ts:37-39 (return gamma - 1;), charged at src/sim/scenario.ts:623
  • Found by: Error Handling & Observability

[F12] Orchestrator readiness rule re-implemented in the HUD

  • Category: 25 Business logic in the UI
  • Impact: Medium
  • Explanation: The HUD forks the comparison-and-epsilon logic of introductionShortfall(), whose own docstring designates it as the shared source ("scenario.ts throws these and the UI can show them"). Constants are shared; the rule logic is not.
  • Evidence: src/ui/Hud.tsx:361-363 (listenedYears >= AMBASSADOR_COORD_YEARS - 1e-9 …) vs src/sim/orchestrator.ts:42-58
  • Found by: Error Handling & Observability

[F13] stateHash() omits finding (and transmitters/purges), so hash equality does not certify finding equality

  • Category: 19/24 Verification gap
  • Impact: Medium
  • Explanation: The hash covers [epoch, seed, conveyor, readMessageIds, evidence, ascension, eventLog]. Transmitters/purges are pure folds over the hashed eventLog (redundant-safe), but finding is the one settle-derived field — exactly the F1 divergence class — and no test compares fine-tick live vs loaded scenarios at all.
  • Evidence: src/sim/scenario.ts:1183-1191; only scenario-level rewalk test scenario.test.ts:36-51 uses debug.setEpoch alone
  • Found by: Integration & Data

[F14] stateHash documented for save verification but never used on the save/load path

  • Category: 21 No observability plan
  • Impact: Medium
  • Explanation: The save file stores no hash and deserialize never verifies the rewalked scenario against anything; a determinism regression that slips past the suite corrupts saves invisibly at every load, with no dev-mode assertion either.
  • Evidence: src/sim/scenario.ts:1180 (doc: "save/rewalk verification"); src/state/save.ts:13-18 (SaveFile has no hash field)
  • Found by: Error Handling & Observability

[F15] Save parsed with a blind type cast; no runtime shape validation

  • Category: 24 (also 30 — the app's only trust boundary)
  • Impact: Medium
  • Explanation: Only version is inspected; seed/epoch/consumerDeltas shapes are never validated, so a string epoch survives advance(0) into a loaded-but-NaN scenario instead of a clean reset. Severity bounded by the boundary being the consumers's own localStorage.
  • Evidence: src/state/save.ts:62, excerpt: deserialize(JSON.parse(rawJson) as SaveFile)
  • Found by: Integration & Data, Security & Code Quality

[F16] A throw inside the rAF frame kills the sim loop permanently and silently

  • Category: 21
  • Impact: Medium
  • Explanation: frame() has no try/catch, so an exception prevents the trailing requestAnimationFrame; rafId keeps its stale value, so startSimLoop()'s re-entry guard blocks any restart. Clocks freeze, autosave stops, no signal.
  • Evidence: src/state/simLoop.ts:20-42, guard at :45 (if (rafId !== null) return)
  • Found by: Error Handling & Observability

[F17] apply→save→mirror ritual hand-copied ~15×; two skips bypass arrival detection

  • Category: 27 Temporal coupling
  • Impact: Medium
  • Explanation: Every action wrapper repeats scenario.apply(...); saveToStorage(scenario); set({...mirror()}) with nothing enforcing order or completeness. Separately, skipToFlare and skipToSessionEnd jump the epoch without routing through arrivalFrom(), safe only via an unstated transit/session/leap mutual-exclusion invariant.
  • Evidence: src/state/simStore.ts:385-618 (wrappers), :221 (arrivalFrom), :448,524
  • Found by: Coupling & Dependencies

[F18] Hud.tsx low cohesion; SimUiState mixes mirror, chrome, and 7 error slots

  • Category: 11 Low cohesion
  • Impact: Medium
  • Explanation: Hud.tsx (708 lines) is "every eventside interaction in the scenario" — BodyPanel is a hardcoded ladder of 7 conditional feature sections, growing +151/+65 lines per finding commit. SimUiState interleaves ~50 fields of sim mirror data, panel chrome, badge bookkeeping, error slots, and ~30 actions.
  • Evidence: src/ui/Hud.tsx:133-141 (BodyPanel); src/state/simStore.ts:100-201
  • Found by: Structure & Boundaries

[F19] Scenario constructed and localStorage rewalked at module import time; const binding; hardcoded seed

  • Category: 1 Global mutable state (also 12, 27)
  • Impact: Medium
  • Explanation: Importing simStore touches localStorage and rewalks the full action log as a side effect; the const binding means "new walk" is only clearStorage() + window.location.reload(); the seed is hardcoded so "pure function of (epoch, seed, log)" has exactly one seed in practice. This import side effect is a direct cause of F4's untestability.
  • Evidence: src/state/simStore.ts:18, excerpt: export const scenario: SimScenario = loadFromStorage() ?? new SimScenario(0xc0ffee);; src/ui/HelpPanel.tsx:111-114; src/state/simLoop.ts:13-16
  • Found by: Structure & Boundaries, Coupling & Dependencies, Error Handling & Observability (3-specialist agreement)

[F20] Save writes fail fully silently while the eventLog grows without bound toward the quota that makes them fail

  • Category: 26 (also 20)
  • Impact: Medium
  • Explanation: The empty catch on saveToStorage emits nothing, not even a console line — Safari private mode means hours of walk with zero persistence and zero indication. Every skip/scrub appends a debug.setEpoch delta, so the save grows monotonically toward the ~5 MB quota, after which every save fails silently forever.
  • Evidence: src/state/save.ts:49-56 (empty catch {}); src/state/simStore.ts:354-364,448-457,524-564
  • Found by: Integration & Data, Error Handling & Observability

[F21] Pluto barycenter branch and orbitalPeriodS untested despite CLAUDE.md's ephemeris-testing claim

  • Category: 32 (steering-file contradiction)
  • Impact: Medium
  • Explanation: CLAUDE.md says moons/dwarfs/Pluto are tested against Horizons reference vectors, but ephemeris.test.ts contains zero pluto/charon assertions and coverage confirms the branch never runs under test; orbitalPeriodS (UI orbit drawing depends on it) is also untested.
  • Evidence: src/sim/ephemeris.ts:55-57 (Pluto = −Charon × CHARON_BARY_FACTOR), :78-86; ephemeris.ts at 66.7% statements
  • Found by: Security & Code Quality

[F22] Transmitter-placement availability diverges between UI and sim

  • Category: 25
  • Impact: Low
  • Explanation: The panel computes canPlaceHere from aliveTransmitters() (excludes expired caches) while the sim blocks on all ever-placed non-purged caches — so a discovered-but-not-dismantled cache vanishes from the list, the button enables, and the click yields "a transmitter is already hidden at this body" for an invisible cache. The throw is caught and surfaced, so no crash.
  • Evidence: src/ui/EvidencePanel.tsx:211-212 vs src/sim/scenario.ts:393-399; alive filter at :951-960
  • Found by: Error Handling & Observability

[F23] Chorus exchange ids as bare string literals gate two findings

  • Category: 28 Magic strings
  • Impact: Low
  • Explanation: "chorus-06" gates the Orchestrator section render and "chorus-05" gates refit + Time, both matching data rows with no named constants. The sim-side literal is indirectly test-covered; the uncovered risk is the Hud copy, where a data rename silently hides an finding's UI.
  • Evidence: src/ui/Hud.tsx:357; src/sim/scenario.ts:1006 (knowsFlareDate); src/sim/data/chorus.ts:102,112
  • Found by: Error Handling & Observability

[F24] First-order lag formula reimplemented ≥4×, once in UI

  • Category: 9/28
  • Impact: Low
  • Explanation: The distance(...)/C_KM_S idiom is copy-implemented per site; the ConcordatPanel copy must stay numerically identical to the sim's feed math or its "her reply, at the earliest" preview diverges from the actual resolveEpoch.
  • Evidence: src/sim/scenario.ts:531-535,841-845,1123-1124; src/sim/curator.ts:122-126; src/ui/ConcordatPanel.tsx:87-91,101 (reply = 2 * lagS)
  • Found by: Structure & Boundaries, Error Handling & Observability

[F25] save.ts mutates sim internals directly on load

  • Category: 6 Leaky abstraction
  • Impact: Low
  • Explanation: deserialize() assigns scenario.state.epoch from outside the sim then calls advance(0) to settle; readonly state guards only reassignment, and no sim API exists for "restore to epoch without logging". Single documented site, but an encapsulation hole other callers could copy (its behavioral consequence is F1).
  • Evidence: src/state/save.ts:34, excerpt: scenario.state.epoch = migrated.epoch;
  • Found by: Coupling & Dependencies

[F26] @react-three/drei is an unused runtime dependency

  • Category: 31 Unused dependencies
  • Impact: Low
  • Explanation: Zero imports anywhere in src/; ADR 0016 confirms the drei Stars usage was replaced. Tree-shaken from the bundle, so the cost is install weight and audit surface.
  • Evidence: package.json ("@react-three/drei": "^10.7.7"); docs/adr/0016-aberration-doppler-shaders.md:23
  • Found by: Coupling & Dependencies, Security & Code Quality

[F27] playwright is an unused devDependency

  • Category: 31
  • Impact: Low
  • Explanation: No config, no e2e directory, no script or Makefile target references it — one of the heaviest possible dev deps (downloads browser binaries), likely used ad hoc during autonomous development and never wired in.
  • Evidence: package.json devDependencies ("playwright": "^1.61.1")
  • Found by: Coupling & Dependencies, Security & Code Quality

[F28] Sim-layer test imports the state layer, contradicting the documented dependency direction

  • Category: 13 (steering-file contradiction)
  • Impact: Low
  • Explanation: CLAUDE.md's strict sim → state → ui direction and its all-tests-in-src/sim/ convention collide: save round-trip tests can only live in sim by importing state. Production sim code is clean; this is the sole violation.
  • Evidence: src/sim/travel.test.ts:6, excerpt: import { deserialize, serialize } from "../state/save";
  • Found by: Structure & Boundaries, Coupling & Dependencies, Security & Code Quality (3-specialist agreement)

[F29] Zero tests for src/ui/, including pure format.ts and projection.ts

  • Category: 32
  • Impact: Low
  • Explanation: Expected for an r3f walk overall, but format.ts (imported in 9 places) and projection.ts are pure functions — free test wins left on the table.
  • Evidence: all 15 test files under src/sim/; src/ui/format.ts, src/ui/projection.ts
  • Found by: Security & Code Quality

[F30] No automated check that elements.gen.ts matches data-raw/

  • Category: 24 (pipeline contract)
  • Impact: Low
  • Explanation: The bake is a manual step and both artifacts are committed; nothing asserts bake(data-raw) === elements.gen.ts, and moon tolerances (up to ~15°/6%) could let a stale gen file pass the indirect ephemeris tests. Mitigated by provenance headers and bake-time validation.
  • Evidence: scripts/bake-elements.mjs:1-9 ("Run manually after refreshing data"); no CI/test rebake-and-diff
  • Found by: Integration & Data

[F31] Epsilon 1e-9 duplicated ~10× across sim and UI

  • Category: 28
  • Impact: Low
  • Explanation: The same undocumented tolerance literal appears in driver/threshold checks on both sides of the sim/UI boundary; the UI copies must match the sim's by hand for buttons to agree with apply().
  • Evidence: src/sim/scenario.ts:345,570,624; src/ui/Hud.tsx:85,282,361-362,495,608; src/sim/chorus.ts:91; src/sim/orchestrator.ts:47,52
  • Found by: Error Handling & Observability

[F32] Walk caps enforced only in UI sliders

  • Category: 25/28
  • Impact: Low
  • Explanation: The racetrack gamma ceiling (refit ? 40 : 7) and Time slider max (100) exist only as widget attributes; the sim validates only minimums plus driver, so the effective ceilings are business rules living as unnamed literals in a component.
  • Evidence: src/ui/Hud.tsx:293,510; src/sim/scenario.ts:556-572,618-625
  • Found by: Error Handling & Observability

[F33] Untracked _to_delete/ directory containing a single empty stale HEAD.lock

  • Category: 31
  • Impact: Low
  • Explanation: Pure clutter; one 0-byte git lock file, nothing references it.
  • Evidence: _to_delete/HEAD.lock (0 bytes, untracked)
  • Found by: Security & Code Quality

Coverage Checklist

Flaw/Risk Types 1–34

# Type Status Finding
1 Global mutable state Observed #F19
2 God object Observed #F8
3 Tight coupling Observed #F9
4 High/unstable dependencies Observed #F8, #F26
5 Circular dependencies Not observed (sim graph verified as a DAG) —
6 Leaky abstractions Observed #F6, #F25
7 Over-abstraction Not observed —
8 Premature optimization Not observed —
9 Shotgun surgery Observed #F7, #F24
10 Feature envy / anemic domain model Not observed (plain-data state + pure functions is required by the rewalk/hash design) —
11 Low cohesion Observed #F18
12 Hidden side effects Observed #F19
13 Inconsistent boundaries Observed #F6, #F28
14 Distributed monolith Not applicable (single-process SPA) —
15 Chatty service calls Not applicable (nearest analog: #F9) —
16 Synchronous-only integration Not applicable —
17 No clear ownership of data Observed (read-side only) #F6
18 Shared database across services Not applicable (one localStorage key, one writer) —
19 Lack of idempotency Observed #F1, #F13
20 Weak error handling strategy Observed #F3, #F10, #F20
21 No observability plan Observed #F5, #F14, #F16
22 Configuration sprawl Not observed (countered by S15) —
23 Dependency injection misuse Not observed —
24 Inconsistent API contracts Observed #F2, #F15, #F30
25 Business logic in the UI Observed #F11, #F12, #F22, #F32
26 Poor transactional boundaries Observed #F3, #F20
27 Temporal coupling Observed #F17, #F19
28 Magic numbers/strings everywhere Observed #F23, #F31
29 "Utility" dumping ground Not observed (format.ts, vec3.ts, constants.ts all small and single-purpose) —
30 Security as an afterthought Observed (minor; trust boundary is consumer's own localStorage) #F15
31 Dead code / unused dependencies Observed #F26, #F27, #F33
32 Missing test coverage for critical paths Observed #F4, #F21, #F29
33 Hard-coded credentials or secrets Not observed (full grep; only narrative prose hits) —
34 Inconsistent error/logging conventions Observed #F10

Strength Categories S1–S14

# Category Status Finding
S1 Clear modular boundaries Observed #S2
S2 High cohesion Observed #S7
S3 Loose coupling Observed #S1, #S4
S4 Dependency direction is stable Observed #S3
S5 Dependency management hygiene Observed #S14
S6 Consistent API contracts Observed #S1, #S8, #S9
S7 Robust error handling Observed #S10, #S11
S8 Observability present Observed (determinism tests as the observability plan; runtime side weak — see F5/F14) #S6, #S11
S9 Configuration discipline Observed #S15
S10 Security built-in Observed #S12
S11 Testability & coverage Observed (sim layer; state/ui gaps in F4/F29) #S6, #S9, #S17
S12 Resilience patterns Observed #S5, #S16
S13 Domain modeling strength Observed #S1
S14 Simple, pragmatic abstractions Observed #S13

Hotspots

  1. src/state/ — the highest-risk, least-tested code in the repo: zero test coverage (F4) over the save/load data-loss paths (F2, F3, F15, F20), the import-time boot side effect (F19), and the skip logic that triggers F1.
  2. src/sim/scenario.ts — the hub everything routes through (F8): settle()'s code-order finding resolution (F1), the stateHash blind spot on finding (F13), and the per-feature growth that drives shotgun surgery (F7).
  3. src/ui/Hud.tsx (with EvidencePanel/ConcordatPanel) — the accretion point for forked sim rules (F11, F12, F22, F24, F32), mirror-bypass reads (F6), and per-finding growth (F18).

Next Questions

  1. Is the F1 scenario (pending ascension feed + lit time drive) reachable in a normal walkthrough, and is the intended contract that settle() resolve events in epoch order per ADR 0003 — or should the ADR document a first-finding-wins code order?
  2. What is the intended save-compatibility policy across rebalances, given CLAUDE.md declares them "a data change" while rewalk re-validates every logged action against current tuning (F2)?
  3. Is the Zustand mirror meant to be the sole read contract for the UI, or is direct read-only access to the live scenario sanctioned — and if sanctioned, where should that be documented so AscensionMirror-style information hiding isn't silently bypassed (F6)?
  4. Which state-layer behaviors (skip sequences, arrivalFrom, load/migrate/quota paths) count as critical paths that need tests before more content lands (F4)?
  5. Was Pluto's barycenter offset intended to be covered by the Horizons reference-vector tests, per CLAUDE.md's claim (F21)?

Analysis Metadata

  • Agents dispatched: 5 specialists (Structure & Boundaries; Coupling & Dependencies; Integration & Data; Error Handling & Observability; Security & Code Quality) + 1 verifier
  • Scope: full repository (~65 source files, ~10.5k lines) at commit 06a5cf9
  • Raw findings: 79 (51 flaws, 28 strengths)
  • Verified findings: 50 (33 flaws, 17 strengths)
  • Filtered out: 29 (18 cross-specialist duplicates merged, 2 dropped as false positives/subsumed, 1 folded into another finding, plus merge consolidation)
  • By impact (flaws): 4 High, 17 Medium, 12 Low
  • By impact (strengths): 7 High, 9 Medium, 1 Low
  • Steering files consulted: CLAUDE.md, docs/design.md, docs/adr/0001–0016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment