# Independent Audit — U-A1: split `js/ui-core.js` into six files **Commit audited:** `f746675` on `dev` — "refactor: split ui-core.js by responsibility into 6 files (2.8.5 U-A1)" **Baseline:** `99782d4` (U-A0) **Auditor:** independent session (Protocol 8 stage 3). No access to the implementer's task list or session transcript. Findings below were formed from the code first; the commit-message body was read only afterward, to compare against my own findings. **Method:** mechanical line-multiset reconstruction of the pre-split file vs. the six post-split files; per-file function inventory; boundary/coupling analysis via the eslint cross-file globals delta; full re-sync verification against all seven required touch-points; targeted read of every test hunk in both runners. --- ## VERDICT: **ACCEPT WITH FOLLOW-UPS** The split is genuinely well-executed and behavior-preserving. I independently **proved it is a pure move** (see §1). The gate was **not weakened in the assertion sense** — no asserted literal changed, no matcher was loosened, no test was deleted, skipped, or made vacuous (see §6). The load-order contract is sound and deliberately order-independent (see §4). The re-sync is complete on all seven required surfaces (see §5). Two real follow-ups, **both the same root cause** and both SHOULD-FIX (not blockers): two hardcoded "served-JS-file" lists in the test suite were not widened to include the five new sibling files, so two guards silently now cover only the 2,480-line hub instead of the whole former 8,082-line file. Nothing is broken today (the moved code is clean), but the guards' *coverage* regressed as a side effect of the split. Details in §6 (Finding A/B). Honest note on this audit stage's value: Finding B (the UTF-8/mojibake guard now scanning only the hub) is exactly the class of defect a green build cannot see — the guard passes *because* it stopped looking at the files that matter. That is a real, non-obvious finding. It is a SHOULD-FIX, not a disaster, and I present it proportionately. --- ## 1. Was it a pure move? (the invariant the unit must not violate) — **PROVEN YES** I reconstructed the check independently rather than trusting the "pure code move" claim: - Extracted `js/ui-core.js` at `99782d4` (8,082 lines) and all six post-split files (combined 8,108 lines). - Normalized both sides (trim whitespace, drop blank lines) and compared line **multisets** with `comm` (which honors duplicate counts). - **Lines only in the original: 0.** No code line was removed or altered. - **Lines only in the new files: 26 — every one a comment.** They are the file-header banners on the five new files (responsibility descriptions + the "static `<script>` tag, see index.html load order" note). Because `comm` accounts for duplicate counts, this simultaneously rules out **removal/alteration** (zero only-in-original) *and* **accidental duplication** (a code line copied into two files would appear in only-in-new; none did). The post-split code is the exact same multiset of code lines as the pre-split file, plus 26 comment lines. Function count is preserved exactly: 49 + 21 + 33 + 37 + 55 + 53 = **248**, matching the original. This also transitively confirms no `window.X = X` exposure was dropped (a dropped exposure would break an inline `onclick` in index.html at runtime — but every such line is present, since none appear in the only-in-original set). **Nothing was changed rather than moved.** The "no rewrites, no drive-by fixes" invariant holds. --- ## 2. Split-boundary quality — **GOOD, with one loose bucket** Per-file responsibility, judged from the actual function inventory: | File | Lines | Fns | Verdict | |---|---|---|---| | `ui-core.js` (hub) | 2,480 | 49 | **Coherent.** Boot orchestration (`window.onload` + `_hydrate*`/`_restore*`/`_wire*` boot-phase fns), core primitives (`loadUI`, `updateMath`, `appendToChat`, `escapeHtml`, `_updatePanelBadges`, `expandPanelForCategory`), shared modal/error/badge infra, and the standby/shutdown power lifecycle. Genuine spine, **not a leftovers bin.** Two borderline residents: the changelog viewer (`_parseChangelog`/`showFullChangelog`/…) and `seedNewCampaignInventory` — defensible as "misc shared UI with no better sibling home," but they are the least hub-like things in it. | | `ui-core-nav.js` | 501 | 21 | **Clean single responsibility** — bezel subsystem nav, tabs, scroll persistence, hotkeys, SWEEP/SEAT motion verbs. | | `ui-core-overseer.js` | 780 | 33 | **Clean** — Director Uplink scope state machine, composer wiring, Tool Deck launcher, help modals. | | `ui-core-chassis.js` | 914 | 37 | **Clean** — the Living Core, System Status, Service & Fault Console, overseer log. | | `ui-core-modulebay.js` | 1,133 | 55 | **Clean** — Module Bay, wake-lock/high-lumen/immersion clusters, campaign-config board, tempo dial. | | `ui-core-cmd.js` | 2,300 | 53 | **Defensible but loose — the weakest boundary.** See §3. | The one placement the commit flagged as mechanically motivated — `_powerOnFromShutdown` in the hub "to satisfy Suite 152.7's adjacency assertion" — I checked directly and it is **conceptually correct, not convenient**. The `shutdown-crt` observer it recovers from is registered in `_wireAmbientExperiences()` in the hub (line 534), alongside `enterStandby`/`exitStandby`. The whole power-lifecycle cluster lives in the hub; the recovery handler belongs next to its observer (Protocol 22 co-location). Suite 152.7 enforces a genuine conceptual invariant (keep recovery beside the state it recovers from), not an incidental textual adjacency. The commit message slightly over-dramatizes this as a test forcing a suboptimal home; in reality the hub is the right home. **Not a finding.** --- ## 3. Is `ui-core-cmd.js` one responsibility? — **NO, but acceptable for now (NOTE)** Its stated charter is "command layer (native stat setters / COMMAND_REGISTRY / core event-bus subscribers)." In practice it bundles ~4 semi-distinct subsystems: - The native-command stat layer (`_nativeSet*`, `commitStat`, `nativeLevelUp`, `capStatMax`) + the HP/XP/Rad draggable-bar interactions — this *is* the command layer. - **VATS** (`showVATSOverlay`, `recomputeVATS`, `_vatsResolveSkill`, ~250 lines) — a self-contained feature. - **The feedback-echo system** (`_echoPush`/`_echoRenderCurrent`/`_echoAdvance`/`_echoJump`/`_wireFeedbackEchoSubscribers`) — a self-contained subsystem. - Live-vitals wiring: limbs/bio-harness, skills/karma rendering, fader drag, location card, operator telemetry, the 400-line `_wireCoreEventBusSubscribers`. There is a loose unifying theme ("the operator manipulating live character stats/vitals and the reactive UI that responds"), so it is not incoherent. But at 2,300 lines it is the second grab-bag the phase set out to eliminate, and calling it "one responsibility" is generous. **VATS and the feedback-echo system are the natural extraction candidates** if/when this file is revisited. Flagging, not blocking — it is still a large improvement over the 8,082-line monolith, and further splitting is out of this unit's scope. --- ## 4. Load order / global-scope contract — **CORRECT, not order-fragile** - index.html loads the hub **first** (line 4277), then the five siblings (4282–4286), then `test-console.js` and `api.js`. - The order is safe by construction, not by luck. Every sibling is pure function declarations plus a handful of `window.X = X` exposures (same-file references, hoisted). The **only** parse-time execution in the siblings is two `document.addEventListener('visibilitychange', …)` registrations (chassis, modulebay) — these only *register* handlers; the callbacks fire at runtime when all files are parsed. - All cross-file orchestration is deferred to `window.onload` (assigned in the hub, line 1034), which fires after all six scripts parse. The siblings' own comments state this explicitly ("Wiring is deferred to a function called from window.onload, NOT run at this time" — the "U7 boot-order lesson"). index.html documents intra-group order as flexible. - Consequence: even though the hub loads *first*, its `onload` body can safely call sibling functions and read `const COMMAND_REGISTRY` (cmd.js) because those resolve at event time. I verified the hub has no top-level statement that invokes a sibling at parse time. A future change would have to introduce brand-new top-level cross-file execution to break this — the current structure is not balanced on a timing coincidence. --- ## 5. The seven re-sync surfaces — **ALL PRESENT** | Surface | Status | |---|---| | index.html script tags | ✅ hub + 5 siblings, before `test-console.js`/`api.js` | | sw.js precache `ASSETS` | ✅ all six `./js/ui-core*.js` listed (offline app intact) | | sw.js `CACHE_NAME` bump | ✅ `robco-terminal-v2.8.0-r5` (served files changed → r-bump, Protocol 1) | | CLAUDE.md LOAD-ORDER-GUARD | ✅ items 11–16 enumerate all six | | ARCHITECTURE.md LOAD-ORDER-GUARD | ✅ file tree + numbered load order include all six | | tests/test.html boot chain | ✅ correctly untouched — test.html never loaded any `ui-*` file (loads only idb/db/state/reg/registry-core/api/runtime/test-console), so the split does not affect it | | repomix.config.json | ✅ uses `js/**` glob — new files auto-included, no per-file entry needed | | library/CODE_MAP.md | ✅ present locally, reflects the split (gitignored; absent on CI by design) | | eslint.config.mjs globals | ✅ 61 cross-file names added (see §7 on coupling) | No re-sync miss here. A miss on sw.js/index.html would have been a black screen or broken offline app; both are correct. --- ## 6. ⚠ Was the gate weakened? — **NO (assertion integrity intact); two guards' COVERAGE narrowed as a side effect** **Assertion integrity: clean.** I read every hunk in both runners. Every test change is one of exactly two mechanical kinds: 1. **Read-source retarget** — a suite that read `js/ui-core.js` now reads the whole `ui-core` family (`readGroup('ui-core')` in Node, an equivalent `rdGroup`/`Read-Group` in the PowerShell-embedded Node here-strings). The Node runner needed only 2 such hunks because U-A0 had already migrated most of its suites to `readGroup`; the PowerShell runner needed ~20 because its embedded here-strings each hardcoded the path and were never migrated by U-A0. **This asymmetry (14 vs 83 lines) is a structural artifact of how the two runners were built, not a coverage difference** — I confirmed the Node runner's equivalent high-numbered suites (137, 152, 162, 182, 190, 200–216) already read the family. 2. **File-list widening** — `uiSource`/`$uiSrc` and Suite 59's file list gained the five siblings. **No asserted literal changed. No `Check(...)`/`assert(...)` boolean was loosened. No regex was broadened. No test was deleted, skipped, or made vacuous.** Where the split moved a function out of `ui-core.js`, the test still asserts the identical string/regex about the identical function body — sourced from wherever it now lives. This is the correct way to keep a content-location suite green across a move, and it is precisely what U-A0 existed to enable. **But two guards silently narrowed their coverage** — two *other* hardcoded served-JS-file lists were not widened, so they now scan only the hub: ### Finding A — SHOULD-FIX — Suite 62 runtime-fetched-asset guard no longer covers the siblings - **What:** Suite 62's publish/precache guard scans a hardcoded list (`jsFiles62` in PowerShell line 3718; `jsAll62` in Node ~line 6467) for `fetch('…')` calls, to prove every runtime-fetched local asset is published to staging + prod + precache. The list includes `js/ui-core.js` but **not** the five siblings. - **Why it matters:** Before the split, all `fetch()` calls in the ui-core code were scanned (one file, in the list). After the split, a `fetch()` added to any sibling (e.g. an AI asset fetch in `ui-core-overseer.js`) would escape the guard, and a "CHANGELOG NOT FOUND"-class 404 could recur — the exact failure class the suite was built (Protocol 36b/42) to prevent. - **Current impact:** latent only. Both existing `fetch('CHANGELOG.md')` calls stayed in `js/ui-core.js` (lines 912, 1541); no sibling contains a `fetch(`. So the sentinel (62.7) still finds CHANGELOG.md and the guard passes today. - **Fix:** add `js/ui-core-nav.js`, `-overseer.js`, `-chassis.js`, `-modulebay.js`, `-cmd.js` to `jsFiles62`/`jsAll62` in both runners (parity, Protocol 15). ~2 lines each. ### Finding B — SHOULD-FIX — Suite 90 UTF-8/mojibake guard (Protocol 39) no longer covers the siblings - **What:** Suite 90 scans a hardcoded list (`srcFiles90` in Node ~line 10506; `srcPairs90` in PowerShell line 6039) for U+FFFD / `â€` / `â–` double-encoding signatures. The list includes `js/ui-core.js` but **not** the five siblings. - **Why it matters:** This is the enforcement arm of Protocol 39 (the named guard against PowerShell double-encoding corruption). The split moved ~70% of ui-core's non-ASCII content (box-drawing banners, glyphs, em-dashes, the VATS template literals) into the five siblings, but the guard's list still names only the 2,480-line hub. A future PowerShell write to any sibling would double-encode silently and Suite 90 would not catch it — and these five files are prime edit targets. This is the higher-value of the two misses because it degrades a protocol-level safety guarantee. - **Current impact:** latent only. I confirmed all five siblings are mojibake-clean right now (the split was done with the Edit tool, no corruption), so the guard passes today. - **Fix:** add the five siblings to `srcFiles90`/`srcPairs90` in both runners (parity). ~5 lines each. **Root cause of both:** the re-sync widened the content-location reads (via `readGroup`) and two file lists (`uiSource`, Suite 59) but missed the two *other* hardcoded served-file enumerations. I swept every multi-file list in both runners (signature: contains both `ui-render.js` and `ui-audio.js`) and confirmed these two are the **only** misses — Suite 137's `servedJsFiles137` reads the family via `readGroup` (coverage fine; only its `'js/ui-core.js'` label is cosmetically stale). Neither miss is declared in the commit message's re-sync list, so both are unknown gaps rather than known-and-accepted ones. --- ## 7. Coupling: reduced, or relocated? — **mostly reduced; real interconnection is honest hub-and-spoke** The eslint globals delta (61 names now cross the new boundaries) is the objective coupling signal. In a global-script codebase every function was already global; these entries mark which symbols are now referenced across the new file lines. 61 cross-boundary names against 248 functions is a reasonable ratio — most functions are called within their own new file. The cross-boundary traffic is dominated by: - **Boot orchestration** — the hub's `onload`/`loadUI` calling `init*` and `_wire*` into each sibling (`initChassisCore`, `initWakeLock`, `initImmersion`, `initOverseerScope`, `_wireCoreEventBusSubscribers`, …). This is legitimate hub-and-spoke: the hub is the one orchestrator, spokes expose an init/wire entry point. Good design, not tangling. - A handful of genuinely shared primitives the hub owns (`setOverseerState`, `selectSubsystem`, `COMMAND_REGISTRY`) read by siblings. No sibling is merely an extension of another (no A↔B reach-through pair dominates). The split reduced coupling into a directed hub→spoke shape rather than a mesh. The one caveat is `ui-core-cmd.js`'s size (§3), not its coupling. --- ## 8. NOTES (non-blocking) - **N1 — stale test-label text.** Many assertion *messages* in both runners now say "…is defined in ui-core.js" for functions that moved to a sibling (e.g. Suites 57, 58, 64, 137.8/137.9, 152.7). The assertions themselves read the whole family and are correct; only the human-readable label is imprecise. Cosmetic. Worth a cleanup pass but not urgent. - **N2 — Suite 137 label.** `servedJsFiles137`'s `['js/ui-core.js', uiCore137]` pair labels the source `js/ui-core.js` while `uiCore137` is actually `readGroup('ui-core')` (all six files). Coverage is correct (the alert/confirm scan does cover the siblings); the label is misleading. Same class as N1. - **N3 — hub borderline residents.** The changelog viewer and `seedNewCampaignInventory` sit in the hub for lack of a better sibling. Fine for now; if a future unit adds a "misc modals" or "changelog" file they are the first candidates to move. --- ## Bottom line A clean, disciplined split. The behavior-identical invariant is *proven*, not asserted. The gate's assertions are intact — nothing was softened to make the move pass. The two follow-ups are latent coverage regressions in the test suite (two guards stopped watching the five new files), share one root cause, and are ~10 lines total to fix across both runners. Recommend landing the split as-is and folding Findings A/B into the next test-sync commit (or a fast follow-up), with A/B added to whichever guard-completeness checklist future file-splits run against — the escape-ratchet lesson here is "when a file splits, every hardcoded served-file list must be widened, not just the content-location reads."