feat(reachability-lab): song-mode holdout filtering boundary (ADR-0032 impl)

[PhysShell]

What

The ADR-0032 implementation slice (authorized after #167): the holdout-filtering boundary over a bound LoadedCorpus, in a new isolated reachability-lab/ crate. Two RED→GREEN pairs. Song mode only.

Excluded from the workspace (ADR-0010/0032 isolation) — production build, CI, --workspace clippy, CLI, and cockpit acquire no holdout policy.

Boundary

prepare_corpus_for_mode(corpus: LoadedCorpus, mode: CorpusMode, target: &TargetIdentity)
    -> Result<Option<CorpusMaterial>, HoldoutError>

• NoCorpus → Ok(None) (genuinely corpus-free); LeakyDiagnostic → Ok(Some(_)) (deliberately unfiltered, named); HoldoutTargetSong → bind → preflight → require target song_id → partition & refuse if nothing is excluded → compile survivors.
• Single authority: check_binding maps every loaded record by ChunkId to one manifest chunk whose complete ChunkMeta matches (not just song_id/sha256), collects duplicate manifest ids, and rejects duplicate loaded ids — closing the stale-manifest leak with typed BindingRefusals.
• A holdout that would exclude nothing typed-refuses (TargetSongAbsent) rather than returning the corpus unchanged. Every failure is a typed HoldoutError, never a silent pick.

Tests (RED → GREEN, 19)

• mode distinctions (NoCorpus None vs LeakyDiagnostic Some(all); distinct);
• fail-closed refusals propagate the full SongHoldoutRefusal set (uncurated, unidentified/no-sha256, manifest disagreement, inconsistent-sha256) + MissingTargetSongId;
• target must actually be held out: absent from corpus, present only in a skipped/unloaded record, and a successful holdout proves ≥1 chunk excluded — all typed-refuse or assert exclusion;
• single authority: duplicate manifest id, loaded-not-in-manifest, duplicate loaded id, and full-metadata mismatch (differing bar_range, differing gesture under matching source identity);
• zero leakage: a curated corpus excludes both source files of the target song, and the material equals what the keeper alone produces across references, rhythm templates, and gesture;
• deterministic.

Verification

cargo test -p …reachability-lab → 19/19 green; clippy --all-targets (all=deny, pedantic=warn) and rustfmt clean. Isolated crate → not in CI (ADR-0010 precedent) — verified locally under nix.

Out of scope

File/fragment modes (separate slices), measurement/eligibility/projection axes, curation, production wiring, generation/scoring changes.

🤖 Generated with Claude Code

https://claude.ai/code/session_012T7SRMiXmZe5v1imtrKMPi

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds an isolated, non-publishable reachability-lab crate implementing fail-closed song holdout filtering, corpus binding validation, target exclusion, deterministic compilation, documentation, and comprehensive tests.

Changes

Reachability Lab holdout boundary

Layer / File(s)	Summary
Crate boundary and holdout contract Cargo.toml, reachability-lab/Cargo.toml, reachability-lab/README.md, reachability-lab/src/lib.rs	Excludes the lab from the workspace, defines its isolated manifest and lint policies, documents the ADR contract, and adds the public corpus, mode, target, and error types.
Fail-closed holdout execution reachability-lab/src/lib.rs	Implements mode-specific preparation, validates manifest-to-loaded-chunk binding, runs song preflight checks, removes chunks matching the target song, and compiles survivors.
Holdout behavior validation reachability-lab/src/lib.rs	Tests mode semantics, refusal propagation, binding failures, zero-leakage exclusion, and deterministic output.

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

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant prepare_corpus_for_mode
  participant check_binding
  participant song_holdout_preflight
  participant corpus_material
  Caller->>prepare_corpus_for_mode: Submit LoadedCorpus and target
  prepare_corpus_for_mode->>check_binding: Validate manifest and loaded chunks
  check_binding-->>prepare_corpus_for_mode: Return binding result
  prepare_corpus_for_mode->>song_holdout_preflight: Validate song-mode sources
  song_holdout_preflight-->>prepare_corpus_for_mode: Return preflight result
  prepare_corpus_for_mode->>corpus_material: Compile filtered survivor chunks
  corpus_material-->>Caller: Return CorpusMaterial or HoldoutError

Loading

Possibly related PRs

• PhysShell/griff#166: Documents and implements the related ADR-0032 reachability-lab boundary and song-based holdout exclusion contract.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 clearly reflects the main change: an ADR-0032 song-mode holdout-filtering boundary in reachability-lab.

✨ 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/holdout-wiring-song-mode

---

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: 5b4f9d0a96

ℹ️ 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".

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@reachability-lab/src/lib.rs`:
- Around line 131-152: Update check_binding to detect duplicate ChunkId values
while constructing by_id from corpus.manifest.chunks, recording the appropriate
binding refusal instead of silently overwriting entries. Preserve the existing
loaded-duplicate and provenance checks, and ensure duplicate manifest IDs cause
validation to fail rather than selecting one manifest entry.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4bebd189-c918-4981-afc7-724e0eb43566

📥 Commits

Reviewing files that changed from the base of the PR and between 5a6a0f1 and 5b4f9d0.

⛔ Files ignored due to path filters (1)

• reachability-lab/Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (4)

• Cargo.toml
• reachability-lab/Cargo.toml
• reachability-lab/README.md
• reachability-lab/src/lib.rs

[PhysShell]

REQUEST CHANGES addressed — second RED→GREEN pair (head 40aeef6)

Both contract gaps closed; the original two commits are preserved, with a genuine tests-only RED (2f2159b) → GREEN (40aeef6) added. Both Codex P2 threads + the CodeRabbit note resolved.

1. A nonexistent target is no longer a "successful" holdout. HoldoutTargetSong now partitions the loaded records by the target song_id and typed-refuses HoldoutError::TargetSongAbsent(song_id) when the excluded set is empty. Tests:

• target absent from manifest + loaded → refuse;
• target present in the manifest but only via a skipped/unloaded record → refuse (an import failure can't silently become the holdout);
• a successful holdout asserts ≥1 chunk was actually excluded.

2. check_binding now enforces single authority.

• Duplicate ChunkIds in manifest.chunks are collected as BindingRefusal::DuplicateManifest (built incrementally, not silently overwritten).
• A loaded record must match its manifest entry on the complete ChunkMeta (**manifest_chunk != chunk.meta) — bar_range, track_index, gesture, every material-defining field — not a two-field imitation of equality. Tests cover differing bar_range and differing gesture under matching source identity.

Verification (nix): cargo test → 19/19 green (was 13); clippy --all-targets (all=deny, pedantic=warn) + rustfmt clean. README error semantics updated. Isolated crate → not in CI (ADR-0010 precedent), verified locally.

Still song mode only; no file/fragment modes, curation, production wiring, measurement axes, or scoring changes.