Skip to content

[codex] Stabilize backend CI subprocess assertions - #7

Merged
slittycode merged 2 commits into
mainfrom
codex/plan-public-hosting-readiness
Mar 31, 2026
Merged

[codex] Stabilize backend CI subprocess assertions#7
slittycode merged 2 commits into
mainfrom
codex/plan-public-hosting-readiness

Conversation

@slittycode

@slittycode slittycode commented Mar 31, 2026

Copy link
Copy Markdown
Owner

Summary

This follow-up change stabilizes two backend CI tests that became flaky on GitHub Actions.

In plain English: the app code was fine, but the tests assumed the last subprocess call was always the analyze.py call. On GitHub's Ubuntu runner, an extra font-related subprocess call can happen, so the tests were checking the wrong command and failing CI even though the backend behavior was correct.

Root cause

The tests used run_mock.call_args, which only captures the most recent subprocess call. In CI, an extra fc-list --help call can occur through the runtime environment, so the assertions sometimes inspected that call instead of the actual analyzer subprocess.

Fix

The tests now scan run_mock.call_args_list and select the real ./venv/bin/python analyze.py ... call before asserting on flags and timeout behavior.

I updated:

  • apps/backend/tests/test_server.py

Verification

  • cd apps/backend && ./venv/bin/python -m unittest tests.test_server.ServerContractTests.test_analyze_endpoint_combines_separate_and_transcribe_in_subprocess tests.test_server.ServerContractTests.test_analyze_endpoint_passes_fast_flag_to_subprocess tests.test_server.ServerContractTests.test_analyze_endpoint_does_not_pass_fast_when_false
  • cd apps/backend && ./venv/bin/python -m unittest tests.test_server tests.test_cleanup tests.test_runtime_profile

@slittycode
slittycode merged commit d02f890 into main Mar 31, 2026
1 of 2 checks passed
@slittycode
slittycode deleted the codex/plan-public-hosting-readiness branch March 31, 2026 23:41
slittycode added a commit that referenced this pull request May 7, 2026
@slittycode
Codex/backend upload hardening (#11)
5450723 
* prepare hosted runtime foundation (#6)

* [codex] Stabilize backend CI subprocess assertions (#7)

* prepare hosted runtime foundation

* stabilize backend ci subprocess assertions
@slittycode slittycode mentioned this pull request May 14, 2026
Merged
slittycode added a commit that referenced this pull request May 16, 2026
@slittycode @claude
fix: address PR review — decision-gate skip, gerund exceptions, dead …
e9e97e1 
…branch, store prune (#51)

* audit: chain-of-custody, recommendations-first IA, applied tracker, grammar post-process

Implements the design audit's prescription end-to-end (findings #1–#15 + N1/N2/N7/N9/N10
+ follow-ups). The product's chain-of-custody promise — every Phase 2 recommendation
traces back to the Phase 1 measurement that justifies it — is now visually first-class
on every card, not a 9px monospace footnote.

What ships:

* Chain-of-custody (findings #2 + #3). New CitationBlock primitive renders a structured
  "GROUNDED IN" block above every Mix Chain / Patches / Sonic Element card with
  humanized labels (FIELD_LABELS map, ~50 entries + humanizeFieldPath fallback) and a
  ConfidenceBandBadge pill computed from the worst confidence among cited fields.
  Also retires GroundingBadgeList at the Track Layout site; segmentIndexes ride
  through a new extraRows prop.

* Recommendations-first IA (#1). MeasurementDashboard moved to the bottom of the
  results scroll; StickyNav's 9 measurement pills collapse to one trailing
  "Measurements" entry. Producers hit Style → Sonic → Mix Chain → Patches → Session
  before the measurement evidence.

* Header polish (#7, #9, #11). CPU meter removed (browser-tab CPU is misleading
  during backend analysis), "Local DSP Engine v1.6.0" eyebrow removed (resolves
  mobile 3-line wrap), Dense DAW Lab demoted from accent chip to quiet text link.

* Engineering vocab cleanup (#8 + N3/N4/N5/N8). New userLabels.ts service translates
  field paths to producer-readable labels at every render. Button labels renamed
  (Download data / Download report). FAMILY: NATIVE chip dropped from meta-badge rows.
  workflowStage prettified at the view-model layer ("Sound design" not "SOUND_DESIGN").
  AI Interpretation gated copy reworded ("AI interpretation isn't configured…" not
  "Developer kill-switch is off").

* Applied-recommendations tracker (#14 + #15). Per-card checkbox affordance + section-
  header "N of M applied" chip + localStorage persistence keyed by audio content
  SHA256. Producer can rename their file without losing their progress.

* Idle value-prop panel (#5). Replaces the 200px "NO SIGNAL DETECTED" canvas with
  a producer-readable explanation of what ASA does, with honest pacing copy
  (4–5 min Phase 2 wait, not 2–5 min).

* Patches group structure. Mirrors Mix Chain's emoji-eyebrow grouping (Drums / Bass /
  Synth / Master) so producers can jump to the bass patch without scanning 8 cards.

* Input Source collapse (N9). Post-analysis, the Input Source panel collapses to a
  compact summary with filename + duration + "Analyze new file" + "Adjust settings".
  Frees the top of the page for the results the user came for.

* AnalysisStatusPanel primary readout (#6). Stage diagnostic message promoted from
  9px footnote to the visual focus of the progress card. Pre-existing tone-aware
  fill (running / success / failed) preserved.

* Phase 2 failure mode (N1). Header subtitle derives from interpretation stage status
  (no more "PHASE COMPLETE" while INTERPRET still RUNNING/FAILED). StickyNav Phase 2
  pills render disabled with hover-reason when sections didn't populate. Retry button
  gated on error.retryable; non-retryable failures surface the error code inline.

* Misc audit follow-ups: BPM reconciled across exec card + Core Metrics tile (N2);
  Signal Monitor STANDBY canvas hidden when audio isn't playing, freeing ~160px (N7);
  StickyNav label "Device Chain" → "Sections" (N10, less ambiguous with Ableton's own
  effects-routing meaning); BASS group icon swapped from 🫧 → Lucide AudioWaveform
  (#13); toggle helper paragraphs switched from all-caps mono walls to sans-serif
  sentence case (#4 revised).

* Phase 2 grammar post-process (audit final round). The prompt instruction added
  earlier didn't take — Gemini still emits "by recreates / by absorbs / by shapes"
  3rd-person singular forms after "by" in role/reason text. Server-side
  _apply_phase2_grammar_fixes rewrites these to gerunds in-place on
  mixAndMasterChain[].reason, abletonRecommendations[].{reason,advancedTip}, and
  secretSauce.workflowSteps[].{instruction,measurementJustification}. Conservative
  regex (\bby \w{4,}s\b) + denylist guards against plural-noun false positives.

Test coverage:

* UI: 46 test files / 540 tests pass (was 39/422 before the audit). New service tests:
  userLabels (21), phase1Picker (25), citationBlock (12), appliedRecommendations (16),
  formatTrackDuration (12), interpretationSubtitle (10), workflowStagePrettifier (8),
  analysisStatusProgress (6), idleValuePropPanel + phase2NavReason. New DOM tests in
  analysisResultsUi.test.ts cover Track Layout citation, applied-checkbox flow, mix-
  chain citation rendering.

* Backend: 16 new unit tests in test_phase2_grammar_fix.py cover _to_gerund,
  _fix_by_gerund_in_text, _apply_phase2_grammar_fixes including the actual
  screenshot corpus (recreates → recreating, shapes → shaping, matches → matching,
  etc.). Full suite: 463 of 463 ASA tests pass; 1 unrelated pre-existing failure
  in tests.test_url_ingest predates this branch.

Visual verification: Playwright capture pass against a real 126s track confirmed
every surface (15 screenshots in /tmp/asa-shots-audit-final/). Phase 2 returned in
220s; localStorage round-trip verified on applied-checkbox toggles.

Documented limitations:

* The gerund rule is algorithmic — verbs requiring consonant doubling (control →
  controlling, submit → submitting) degrade to "controling" / "submiting". Still
  better than "by controls". Drop a hand-mapped exception into the module if
  observed in real output.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* Fix smoke tests for retired/renamed surfaces

CI surfaced 7 smoke test failures from this branch's audit changes that
hadn't been propagated to the smoke spec assertions:

* `tests/smoke/ui-details.spec.ts` × 3
  - `NO SIGNAL DETECTED` → `IdleValuePropPanel` (audit #5)
  - `JSON_DATA` / `REPORT_MD` button labels → `Download data` / `Download report`

* `tests/smoke/responsive-layout.spec.ts` × 4
  - `NO SIGNAL DETECTED` (×2) → `IdleValuePropPanel`
  - `CPU` text-presence checks removed; the two viewport-shape tests now
    assert just the model-selector responsive behavior (audit #11 retired
    the CPU meter; there's no element to assert)

* `tests/smoke/file-validation.spec.ts` × 1
  - `re-upload after results resets to file-selected state`: the test
    used `Remove File` (FileUpload component's affordance) to clear after
    results were visible. Post-N9 collapse, the Input Source panel
    replaces FileUpload with a compact summary card whose "↺ Analyze new
    file" button calls the same handleFileClear. Switched the test to
    target the new affordance.

Also updated the e2e exports spec for label consistency (not in the
failing CI job, but the same renames apply):

* `tests/e2e/phase1-exports.spec.ts`
  - `downloadTextArtifact(page, /JSON_DATA/i)` → `/Download data/i`
  - `downloadTextArtifact(page, /REPORT_MD/i)` → `/Download report/i`

Verified locally against the live stack: 45 of 46 smoke tests pass, 1
skipped (was unrelated). The previously-failing 7 are all green.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* audit: nightly Phase-1 invariant + test guardrail (2026-05-14)

3 unit tests failing — all fixture-dependent live decision-gate
comparators that assert instead of skipping when Gemini snapshots
are absent. No Phase-boundary violations found.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

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

- Skip decision_gate.{multi,real,stems}.live.test.ts when no /tmp snapshots
  are present (matches the existing decision_gate.live.test.ts pattern).
  Unblocks npm test on CI.
- _to_gerund: add _GERUND_IRREGULARS map for consonant-doubling verbs
  (controls→controlling, submits→submitting, runs→running, etc.) — English
  doubling is stress-conditional, not worth implementing algorithmically.
- _fix_grammar_in_record: drop the always-true conditional return and the
  unused `updated` flag.
- appliedRecommendations: bound the localStorage store to MAX_TRACKED_FILES=50
  (least-recently-updated wins eviction) so it can't grow without bound.

* fix: formatTrackDuration carries seconds boundary; drop dead IdleSignalMonitor

- formatTrackDuration: round seconds to total first, then derive mins/secs.
  Previously Math.round(seconds % 60) could yield 60, producing "0:60" for
  inputs like 59.5. New tests cover 59.5, 59.9, 119.5, 3599.7.
- Delete IdleSignalMonitor.tsx (replaced by IdleValuePropPanel; not imported
  anywhere). Strip the "kept for future use" comments per CLAUDE.md.

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
@slittycode slittycode mentioned this pull request May 30, 2026
Merged
2 tasks
slittycode added a commit that referenced this pull request May 31, 2026
@slittycode @claude
test: lock the measurement + interpretation setup-failure terminalizers
10b5b61 
Follow-up to the 2026-05-30 review fixes (PR #125). The resurrection guards
and the #12 cache path already had regressions, but the two terminalizers this
branch ADDED — #7 (_execute_measurement_run source-artifact guard) and #3/#6
(_execute_interpretation_attempt setup wrapper) — rode on the pre-existing MT3
sibling test (test_source_artifact_failure_terminalizes_attempt) with no
regression of their own. A reverted guard would have stayed green.

Add StageSetupFailureTerminalizationTests, one test per stage, mirroring the MT3
sibling: reserve the stage, make source/grounding resolution raise, and assert
the stage terminalizes to 'failed' with the expected error code instead of
wedging in 'running'. Verified meaningful — both error out when the
terminalizer's except is neutered.

Backend 1071 green (+2).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
slittycode added a commit that referenced this pull request May 31, 2026
@slittycode @claude
test: lock the measurement + interpretation setup-failure terminalizers
821175d 
Follow-up to the 2026-05-30 review fixes (PR #125). The resurrection guards
and the #12 cache path already had regressions, but the two terminalizers this
branch ADDED — #7 (_execute_measurement_run source-artifact guard) and #3/#6
(_execute_interpretation_attempt setup wrapper) — rode on the pre-existing MT3
sibling test (test_source_artifact_failure_terminalizes_attempt) with no
regression of their own. A reverted guard would have stayed green.

Add StageSetupFailureTerminalizationTests, one test per stage, mirroring the MT3
sibling: reserve the stage, make source/grounding resolution raise, and assert
the stage terminalizes to 'failed' with the expected error code instead of
wedging in 'running'. Verified meaningful — both error out when the
terminalizer's except is neutered.

Backend 1071 green (+2).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
slittycode added a commit that referenced this pull request May 31, 2026
@slittycode @claude
fix: 2026-05-30 full-review fixes — staged-run lifecycle, contract dr…
13f73b1 
…ift, error boundary (#125)

* fix: staged-run lifecycle reliability + chain-of-custody/contract fixes

From the 2026-05-30 full review (audits/full-review-2026-05-30.md).

Staged-run lifecycle (P1/P2 reliability cluster):
- Register pitch-note + MT3 subprocesses for interruption so interrupt/delete
  actually terminate them instead of orphaning multi-GB children up to their
  600s/1800s timeout; route both through the proven _run_streamed_subprocess.
- Terminalize interpretation and measurement attempts on setup failure (were
  left 'running' forever with no reaper).
- Guard every attempt/measurement transition against resurrecting an
  interrupted row, and gate the measurement follow-up enqueue on the guarded
  update so a cancelled run can't spawn a fresh pipeline.
- Frontend monitorAnalysisRun: overall wall-clock budget + per-poll deadline
  (was an unbounded poll loop with no per-request timeout).

Chain-of-custody + quick wins:
- Phase 1 parser now carries reverbDetail.perBandRt60 / preDelayMs and
  vocalDetail.stemEnergyRatio / stemOtherCorrelation -- prompt-citable fields it
  silently dropped, which broke the citation-existence check.
- normalizeKey no longer rewrites "major" into "majoror" (false key
  contradiction on a valid "Maj" abbreviation).
- Define _FILE_CACHE_LOCK / _FILE_CACHE_TTL_SECONDS and import timedelta --
  latent NameError in the (currently unwired) temp-file cache path.
- Add a React ErrorBoundary around the lazy AnalysisResults Suspense so one
  render or chunk-load failure can't blank the whole app.

Tests: backend resurrection + temp-file-cache regressions (+ caught a second
undefined name, timedelta); frontend parser/validator/citation contract,
normalizeKey, and polling-timeout regressions. Backend 1068 green, frontend 713
green, lint + production build clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: address review follow-ups + ErrorBoundary fallback smoke test

Bundles the P3/LOW findings from the sequential review of e3e0f69 (verdict:
ship-as-is) plus the ErrorBoundary fallback-render coverage.

1. Backend orphan guard (analysis_runtime.py): _enqueue_requested_followups now
   re-asserts measurement.status == 'completed' before creating follow-up rows.
   Closes the cross-transaction window where an interrupt committing between
   complete_measurement's guarded update and the (separate) enqueue transaction
   would leave inert 'queued' rows on a cancelled run.
2. Faithful MT3 mock-timeout (server.py): the mock branch of
   _run_streamed_subprocess now coerces bytes streams to text via _stream_text,
   matching the real branch's text=True contract — so a TimeoutExpired carrying
   bytes reaches the timeout-specific branch instead of a str-in-bytes TypeError
   that the broad except masked. Test tightened to assert the timeout message.
3. Bound the mid-loop interpretation POST (analyzer.ts): extracted the
   race-with-neutralized-fetch logic into an exported raceWithDeadline<T> helper
   and wrapped the stem-summary createInterpretationAttempt POST in it, closing
   the last unbounded await in the poll loop.
4. ErrorBoundary docstring: corrected the "Try again" claim — React.lazy caches
   a rejected import, so it recovers a render-throw but not a chunk-load failure
   ("Reload page" does). No behavior change.
5. ErrorBoundary fallback smoke test (tests/smoke/error-boundary.spec.ts): drives
   a mocked run to the results view, aborts the lazy AnalysisResults chunk, and
   asserts the fallback renders (alert + Try again/Reload). Verified meaningful:
   it fails when the boundary is removed. Updates the unit-test caveat comment.

Tests: backend 1069 green (+1 orphan-race regression), frontend 716 green (+3
raceWithDeadline) + lint clean + the new smoke spec passes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test: lock the measurement + interpretation setup-failure terminalizers

Follow-up to the 2026-05-30 review fixes (PR #125). The resurrection guards
and the #12 cache path already had regressions, but the two terminalizers this
branch ADDED — #7 (_execute_measurement_run source-artifact guard) and #3/#6
(_execute_interpretation_attempt setup wrapper) — rode on the pre-existing MT3
sibling test (test_source_artifact_failure_terminalizes_attempt) with no
regression of their own. A reverted guard would have stayed green.

Add StageSetupFailureTerminalizationTests, one test per stage, mirroring the MT3
sibling: reserve the stage, make source/grounding resolution raise, and assert
the stage terminalizes to 'failed' with the expected error code instead of
wedging in 'running'. Verified meaningful — both error out when the
terminalizer's except is neutered.

Backend 1071 green (+2).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@slittycode