feat(corpus): v10 canonical song identity + fail-closed song holdout (PR D)

[PhysShell]

What

Implement schema v10 (ADR-0031, Accepted in #164) — the canonical Work identity that makes song-level holdout implementable fail-closed. Implementation-only, RED→GREEN.

• SongId(String) newtype — curator-assigned, opaque, above the sha256 Manifestation level.
• SourceRef.song_id: Option<SongId> — optional/skip-if-none; pre-v10 records load as None and round-trip byte-identically.
• CorpusManifest.songs: Option<BTreeMap<SongId, Vec<String>>> — presence-preserving: absent (None) skips the cross-check, while present (Some, even {}) triggers the bidirectional agreement check, so a present-empty manifest must still account for every labelled source.
• SCHEMA_VERSION 9 → 10.
• song_holdout_preflight + SongHoldoutRefusal typed refusals — corpus-wide, fail-closed:
  • unidentified source (no sha256) → refuse;
  • uncurated source (no song_id, strict-refusal default) → refuse;
  • a sha256 split across ≥2 SongIds (chunk-level leakage) → refuse;
  • present songs manifest must agree with per-source labels both ways.

Tests (RED → GREEN, genuine tests-only RED)

• 301e8bf RED — the schema/preflight tests + the cockpit browser assertion only; introduces no production API and fails to compile in isolation (the v10 types/fields are absent), per the corpus_schema.rs "fail to compile until implemented" precedent.
• 7268b4d GREEN — the v10 types, preflight, and construction-site ripple.

core/tests/corpus_schema.rs covers: schema-version bump, song_id + songs-map round-trip, both pre-v10 lossless characterizations (SPEC hard rule 5), a present-empty-manifest regression ("songs": {} + labelled source ⇒ ManifestLabelMissing), and six preflight cases (accept-clean; refuse uncurated / unidentified / file-split; both manifest bidirectional checks). 40 tests, all green.

Ripple

18 struct-literal construction sites across core/cli/ui-core carry the additive fields (song_id: None / songs: None). Audio crates (cockpit/plugin/preview) construct neither struct, so the change is source-compatible for them; the cockpit web-test producer now emits schema v10.

Verification

Exact-head CI fully green (check, cockpit-web-test, msrv, fuzz matrix, CodeRabbit). Locally: clippy (all/pedantic/nursery/cargo denied) + rustfmt clean on core/cli/ui-core; griff-core full suite green; pattern/swang build clean.

Out of scope (per the PR D contract)

song_id curation/backfill · production corpus cutover · track_index recovery · title-based grouping · any generation/scoring change.

🤖 Generated with Claude Code

https://claude.ai/code/session_012T7SRMiXmZe5v1imtrKMPi

[coderabbitai]

📝 Walkthrough

Walkthrough

Corpus schema v10 adds canonical song identities to source references and manifests, introduces fail-closed song holdout validation, updates CLI/UI construction paths, and expands schema round-trip and refusal tests.

Changes

Song identity schema and holdout validation

Layer / File(s)	Summary
Schema v10 identity contracts core/src/corpus.rs	Adds SongId, optional SourceRef.song_id, the manifest songs mapping, and schema version 10 with legacy serialization defaults.
Song holdout preflight and schema tests core/src/corpus.rs, core/tests/corpus_schema.rs	Validates hash-to-song assignments and manifest completeness, with round-trip, compatibility, acceptance, and refusal tests.
CLI, UI, and fixture propagation cli/src/*, ui-core/src/*, core/src/{curation.rs,curation_store.rs,generation_input.rs}, core/tests/similarity.rs, ui-core/tests/*	Initializes song_id and empty songs maps across production constructors and test fixtures.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CorpusManifest
  participant song_holdout_preflight
  participant SourceRef
  participant SongHoldoutRefusal
  CorpusManifest->>song_holdout_preflight: validate songs and source mappings
  song_holdout_preflight->>SourceRef: inspect sha256 and song_id labels
  song_holdout_preflight->>SongHoldoutRefusal: collect validation refusals

Loading

Possibly related PRs

• PhysShell/griff#134: Earlier corpus schema evolution also modified SourceRef and round-trip expectations.
• PhysShell/griff#157: Documents the schema v10 song identity model implemented by this change.

Suggested reviewers: claude

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: corpus schema v10 with canonical song identity and fail-closed song holdout preflight.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/v10-song-id

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 764f2fa526

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[PhysShell]

REQUEST CHANGES addressed (force-pushed, head 7268b4d)

1. Exact-head CI / cockpit-web-test. The browser assertion cockpit.capture.test.js:149 (schema_version, 9) is updated to 10, matching the v10 producer. Placed in the RED commit so it fails against the still-v9 producer and passes once GREEN bumps the schema — real red→green for that assertion too.

2. Genuine tests-only RED → GREEN. History rewritten (not a corrective commit on top):

• 301e8bf RED — test files only; fails to compile in isolation (verified: unresolved SongId / song_holdout_preflight / SongHoldoutRefusal; SourceRef/CorpusManifest have no song_id/songs), per the corpus_schema.rs "fail to compile until implemented" precedent.
• 7268b4d GREEN — the v10 types, preflight, and construction-site ripple.

No new public type/function shares a commit with its tests. Codex P1 thread replied to and resolved.

3. Absent vs present-empty manifest. songs is now Option<BTreeMap<SongId, Vec<String>>>: absent → None (skips the cross-check, pre-v10 byte-identical); explicitly present (even {}) → Some (triggers the bidirectional check). New regression preflight_present_empty_manifest_still_demands_every_source asserts "songs": {} + a labelled source ⇒ ManifestLabelMissing. The two JSON inputs now deserialize distinguishably.

Local verification (nix): 40 corpus_schema tests + full griff-core suite green; clippy (all/pedantic/nursery/cargo denied) + rustfmt clean on core/cli/ui-core; pattern/swang build clean. Awaiting exact-head CI + cockpit-web-test.