fix: address PR review — decision-gate skip, gerund exceptions, dead branch, store prune

[slittycode]

Summary

Addresses all four findings from the PR review on the audit/grammar-fix branch.

CI blocker

• decision_gate.{multi,real,stems}.live.test.ts — these three files unconditionally asserted expect(ok.length).toBeGreaterThan(0), failing on any machine without /tmp/decision_gate_*.json snapshots. Added a describe-level existsSync guard that registers it.skip when no snapshots are present (mirrors the existing pattern in the sibling decision_gate.live.test.ts). The assertion inside the test body is preserved — it remains meaningful when at least one snapshot exists.

Worth-considering

• _to_gerund consonant doubling (server_phase2.py:977) — added _GERUND_IRREGULARS module-level dict covering the music-production verbs Gemini emits after "by" (controls→controlling, submits→submitting, runs→running, commits→committing, begins→beginning, etc., 10 entries). English doubling is stress-conditional and not worth implementing algorithmically. New test ToGerundTests.test_consonant_doubling_exceptions locks it in; the stale comment in test_simple_consonant_stem now points at the exception map.
• Dead or True branch (server_phase2.py:1027) — collapsed return record if updated or True else record to return record and dropped the unused updated flag.
• appliedRecommendations unbounded growth — added MAX_TRACKED_FILES = 50 constant; saveAppliedIds now evicts the least-recently-updated records over the cap. Three new tests cover the boundary (at-limit no-op, over-limit eviction, recency-not-insertion ordering). Docstring updated to document the bound.

Test plan

• python -m unittest tests.test_phase2_grammar_fix — 17 tests pass (including new test_consonant_doubling_exceptions)
• python -m unittest tests.test_server — 184 tests pass
• npx vitest run tests/services/appliedRecommendations.test.ts — 21 tests pass (18 existing + 3 new prune tests)
• npx vitest run over the full UI suite — 543 tests pass, 0 failures
• npx vitest run tests/decision_gate.*.live.test.ts — all 4 skip cleanly when /tmp snapshots are absent; verified the multi-model test runs and passes when at least one snapshot is dropped into /tmp/decision_gate_<model>.json
• npm run lint — clean

---

Generated by Claude Code

[slittycode]

Verdict: APPROVE (self-review — posted as COMMENT)

Summary

Addresses all four prior-review findings: the CI-blocking decision-gate skip logic, _to_gerund consonant-doubling via _GERUND_IRREGULARS, the or True dead branch, and appliedRecommendations unbounded growth. The PR also bundles the broader UX work from the audit (CitationBlock, applied-recommendations tracker, input-panel collapse, MeasurementDashboard reorder, streaming-reveal status panel). Every new logic path has meaningful test coverage. Phase boundaries are clean throughout.

Test tooling was not available in this environment (no venv, no node_modules), so I'm relying on the author's reported results and a full code read. The new test files are substantive — the appliedRecommendations suite covers corrupted JSON, private-mode storage, and the MAX_TRACKED_FILES recency-ordering boundary condition correctly.

Findings

Should fix

• formatTrackDuration rounding edge case (App.tsx:483): Math.round(seconds % 60) returns 60 for inputs like seconds = 59.5, producing "0:60" instead of "1:00". DSP durations are usually integer-ish so blast radius is small, but the function is exported with no unit tests and the fix is one line: clamp secs to Math.min(..., 59) and carry into mins. Worth closing before this gets imported elsewhere.

• No unit tests for getInterpretationSubtitle / getPhase2NavDisabledReason (AnalysisResults.tsx:814–873): both are exported and contain a multi-branch switch. The analysisResultsUi.test.ts covers them indirectly via HTML matching, but a focused unit test would catch a future bad-case addition without a full render. Low overhead to add.

Worth considering

• IdleSignalMonitor is now dead code — App.tsx replaced the import with IdleValuePropPanel. The comment says "kept for a potential future state" but it's imported nowhere. Per CLAUDE.md: delete rather than leave a speculative corpse.

Phase boundary check

Clean.

• _apply_phase2_grammar_fixes mutates normalized in-place — that's the freshly-built Phase 2 output dict inside _normalize_and_salvage_phase2_result, not a stored Phase 1 row. No measurement value is touched.
• CitationBlock / phase1Picker.ts are pure read-only traversals of Phase1Result via dotted-path descent. pickWorstConfidence returns a derived scalar; it never writes back.
• appliedIds tracker writes only to localStorage keys (asa:applied-recommendations:v1), keyed by content hash. No Phase 1 field is mutated or re-derived.
• buildPatchGroups operates on PatchCardViewModel[] produced by buildPatchCards; both source inputs are treated read-only.
• CONFIDENCE_PAIRS export from phase2Validator.ts is a read-only Record<string, string> now shared between the validator and the citation picker — single source of truth, no mutation risk.

Phase 1 contract check

No changes to analyze.py, EXPECTED_TOP_LEVEL_KEYS, or JSON_SCHEMA.md. The BPM display change in MeasurementDashboard.tsx (formatNumber(phase1.bpm, 1) → Math.round(phase1.bpm)) is display-only and correctly aligns the hero tile with the executive-summary card. Not a contract change.

Test results

Not runnable locally. Author reports: 543 UI tests pass / 0 failures, 184 backend tests pass, 3 decision-gate tests skip cleanly without fixtures, npm run lint clean. Consistent with the code.

---

Generated by Claude Code