Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .github/workflows/claude-code-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,14 @@ jobs:

- name: Run Claude Code Review
id: claude-review
# Advisory, never merge-blocking: the review runs on the subscription
# OAuth token (CLAUDE_CODE_OAUTH_TOKEN secret), which expires roughly
# yearly — an expired token must not hold PRs hostage. Step-level
# continue-on-error keeps the job (and the PR check) green when the
# step fails; job-level continue-on-error would still surface a red
# check. Refresh the token with `claude setup-token` → repo
# Settings → Secrets and variables → Actions.
continue-on-error: true
uses: anthropics/claude-code-action@v1
with:
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
Expand Down
4 changes: 2 additions & 2 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -400,8 +400,8 @@ Things that look like normal code changes but silently break the contract. Most

1. **`print(...)` in [analyze.py](apps/backend/analyze.py) without `file=sys.stderr`.** Stdout is the JSON contract. Any stray print corrupts it and the server reports a parse error with no useful trace. The existing code is consistent about this — match the pattern (`print(f"[warn] ...", file=sys.stderr)`).
2. **Calling `analyze.py` as a subprocess without `--yes`.** The CLI prompts for confirmation when stdin is a TTY. Subprocess invocations must pass `--yes` or hang waiting for input. `server.py` already does; new callers must too.
3. **Renaming a field on only one side.** Python emits *camelCase* JSON directly (`bpmConfidence`, not `bpm_confidence`) — there is no conversion layer. A rename in [analyze.py](apps/backend/analyze.py) without a matching update in [src/types.ts](apps/ui/src/types.ts) is undetectable by either type system; the field just disappears from the UI.
4. **Adding a top-level key without updating `EXPECTED_TOP_LEVEL_KEYS`.** [tests/test_analyze.py](apps/backend/tests/test_analyze.py) holds a snapshot of every root key. New fields require updating that set *and* [JSON_SCHEMA.md](apps/backend/JSON_SCHEMA.md). The same test enforces that `--fast` only populates `FAST_MODE_POPULATED_FIELDS`. A change to *measured values* (not just keys) can also trip the golden-snapshot regression gate in [tests/test_phase1_golden.py](apps/backend/tests/test_phase1_golden.py) (fixture `tests/fixtures/golden/phase1_default.json`) — re-baseline it deliberately, never blindly (from `apps/backend/`): `UPDATE_PHASE1_GOLDEN=1 ./venv/bin/python -m unittest tests.test_phase1_golden`
3. **Renaming a field on only one side.** Python emits *camelCase* JSON directly (`bpmConfidence`, not `bpm_confidence`) — there is no conversion layer. A rename in [analyze.py](apps/backend/analyze.py) without a matching update in [src/types.ts](apps/ui/src/types.ts) is undetectable by either type system; the field just disappears from the UI. Executable defense: the cross-boundary parity gate [apps/ui/tests/services/phase1ContractParity.test.ts](apps/ui/tests/services/phase1ContractParity.test.ts) walks the backend golden snapshot through the real frontend parser and fails on any silent drop.
4. **Adding a top-level key without updating `EXPECTED_TOP_LEVEL_KEYS`.** [tests/test_analyze.py](apps/backend/tests/test_analyze.py) holds a snapshot of every root key. New fields require updating that set *and* [JSON_SCHEMA.md](apps/backend/JSON_SCHEMA.md). The same test enforces that `--fast` only populates `FAST_MODE_POPULATED_FIELDS`. A change to *measured values* (not just keys) can also trip the golden-snapshot regression gate in [tests/test_phase1_golden.py](apps/backend/tests/test_phase1_golden.py) (fixture `tests/fixtures/golden/phase1_default.json`) — re-baseline it deliberately, never blindly (from `apps/backend/`): `UPDATE_PHASE1_GOLDEN=1 ./venv/bin/python -m unittest tests.test_phase1_golden` The golden also carries a recursive `keyTree` (nested key structure); re-baselining it is what arms/updates the frontend parity gate's nested check, so expect `apps/ui/tests/services/phase1ContractParity.test.ts` to demand the matching frontend sync.
5. **Using `document` or `window` in `tests/services/`.** Vitest runs in `node`, not `jsdom`. Service-layer tests are pure logic; if you need DOM, it's a Playwright test in `tests/smoke/` instead.
6. **Hard-coding `Path(...)` for artifacts in new code.** Artifact access must go through [artifact_storage.py](apps/backend/artifact_storage.py). Direct paths work in `local` profile and break silently in `hosted`.
7. **Editing `apps/ui/.env` and expecting `dev.sh` to honor it.** `dev.sh` reads `apps/ui/.env` but *overrides* `VITE_API_BASE_URL` for the spawned UI process so stale `.env` files don't break the stack. To point the UI at a non-canonical backend, edit `dev.sh` or run the UI directly with the env var on the command line.
Expand Down
2 changes: 1 addition & 1 deletion apps/backend/JSON_SCHEMA.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ Top-level keys:

`phase1Version`, `bpm`, `bpmConfidence`, `bpmPercival`, `bpmAgreement`, `bpmDoubletime`, `bpmSource`, `bpmRawOriginal`, `key`, `keyConfidence`, `keyProfile`, `tuningFrequency`, `tuningCents`, `timeSignature`, `timeSignatureSource`, `timeSignatureConfidence`, `durationSeconds`, `sampleRate`, `lufsIntegrated`, `lufsRange`, `lufsMomentaryMax`, `lufsShortTermMax`, `lufsCurve`, `truePeak`, `crestFactor`, `dynamicSpread`, `monoCompatible`, `plr`, `dynamicCharacter`, `textureCharacter`, `stereoDetail`, `spectralBalance`, `spectralBalanceTimeSeries`, `spectralDetail`, `stemAnalysis`, `transientDensityDetail`, `saturationDetail`, `snareDetail`, `hihatDetail`, `rhythmDetail`, `melodyDetail`, `transcriptionDetail`, `pitchDetail`, `grooveDetail`, `beatsLoudness`, `rhythmTimeline`, `sidechainDetail`, `reverbDetail`, `vocalDetail`, `acidDetail`, `supersawDetail`, `bassDetail`, `kickDetail`, `genreDetail`, `effectsDetail`, `synthesisCharacter`, `danceability`, `structure`, `arrangementDetail`, `segmentLoudness`, `segmentSpectral`, `segmentStereo`, `segmentKey`, `chordDetail`, `perceptual`, `essentiaFeatures`.

**Shared (fast + full) vs full-only.** Fast-mode output is asserted byte-for-byte against the `EXPECTED_TOP_LEVEL_KEYS` set in [`tests/test_analyze.py`](tests/test_analyze.py). Full mode emits those keys *plus* a handful of detail-only fields that are deliberately absent from the shared snapshot: `keyProfile`, `tuningFrequency`, `tuningCents`, `lufsMomentaryMax`, `lufsShortTermMax`, and `pitchDetail`. When changing the schema, update both this list and `EXPECTED_TOP_LEVEL_KEYS`; full-only fields stay out of that set on purpose. See CLAUDE.md tripwire #4.
**Shared (fast + full) vs full-only.** Fast-mode output is asserted byte-for-byte against the `EXPECTED_TOP_LEVEL_KEYS` set in [`tests/test_analyze.py`](tests/test_analyze.py). Full mode emits those keys *plus* a handful of detail-only fields that are deliberately absent from the shared snapshot: `keyProfile`, `tuningFrequency`, `tuningCents`, `lufsMomentaryMax`, `lufsShortTermMax`, and `pitchDetail`. When changing the schema, update both this list and `EXPECTED_TOP_LEVEL_KEYS`; full-only fields stay out of that set on purpose. See CLAUDE.md tripwire #4. Schema changes are also cross-checked executably against the frontend fixture and parser by `apps/ui/tests/services/phase1ContractParity.test.ts`, driven by the golden snapshot's `topLevelKeys`/`keyTree` — regenerating the golden (`UPDATE_PHASE1_GOLDEN=1`) is what arms the nested half of that gate.

## Relationship To `POST /api/analyze`

Expand Down
Loading
Loading