Skip to content

feat(reachability-lab): exact-sha256 source-file holdout mode (ADR-0032) - #169

Merged
PhysShell merged 2 commits into
mainfrom
claude/holdout-source-file-mode
Jul 29, 2026
Merged

feat(reachability-lab): exact-sha256 source-file holdout mode (ADR-0032)#169
PhysShell merged 2 commits into
mainfrom
claude/holdout-source-file-mode

Conversation

@PhysShell

@PhysShell PhysShell commented Jul 29, 2026

Copy link
Copy Markdown
Owner

What

The next ADR-0032 slice: HoldoutTargetSourceFile — holdout by exact sha256 source-file identity, independent of song curation. RED→GREEN, in the isolated reachability-lab/ crate. File mode only (fragment mode is a separate later slice).

Contract

  • TargetIdentity gains source_sha256 (kept per-mode explicit alongside song_id).
  • CorpusMode::HoldoutTargetSourceFile.
  • Execution order: check_bindingsource-file preflight → require target source_sha256 → partition by exact sha256typed-refuse if nothing was excludedcorpus_material(survivors).
  • Fail-closed identity: source_file_preflight rejects any participating manifest chunk without sha256 (SourceHoldoutRefusal::UnidentifiedSource) — complete hash identity, no basename fallback.
  • Not blocked on curation: file mode does not run song_holdout_preflight and does not require song_id — a hash-identified but song-uncurated corpus is a valid file-mode experiment.
  • New typed refusals: MissingTargetSourceSha256, SourcePreflight(Vec<SourceHoldoutRefusal>), TargetSourceAbsent(String).

Tests (RED → GREEN, 28 total)

The 10-point file-mode matrix: missing target sha256; a sha256-less manifest chunk refuses before material; target hash absent; target present only in a skipped/unloaded record; every chunk of the target hash excluded regardless of bar_range/track; a song-uncurated corpus with complete hashes succeeds; material equals keeper-only across references, rhythms, gesture, and skipped; full-ChunkMeta binding still refuses first; deterministic. (Song / NoCorpus / LeakyDiagnostic tests unchanged.)

Also (per review guidance)

Fixed the two stale rustdoc comments so ProvenanceMismatch and check_binding describe full ChunkMeta equality, not song_id/sha256. Module/README/Cargo scope updated to song + source-file modes.

Verification

cargo test -p …reachability-lab28/28 green; clippy --all-targets (all=deny) + rustfmt clean. Isolated crate → not in CI (ADR-0010 precedent), verified locally under nix.

Out of scope

HoldoutTargetFragment, range overlap, bar_range == None semantics, song-id curation, measurement axes, production CLI/cockpit wiring, scoring/generation changes.

🤖 Generated with Claude Code

https://claude.ai/code/session_012T7SRMiXmZe5v1imtrKMPi

Summary by CodeRabbit

  • New Features
    • Added source-file holdout mode using exact source hashes.
    • Excludes all chunks from the selected source file, regardless of track or bar range.
    • Preserves support for song-based holdouts.
  • Bug Fixes
    • Prevents silent success when the target song or source file is unavailable.
    • Rejects source-file holdouts when participating chunks lack verifiable file identity.
  • Documentation
    • Clarified holdout behavior, refusal conditions, and fragment-mode limitations.

PhysShell and others added 2 commits July 29, 2026 07:32
Tests-only; the crate fails to compile until the file-mode API lands in
GREEN (TargetIdentity.source_sha256, CorpusMode::HoldoutTargetSourceFile,
HoldoutError::{MissingTargetSourceSha256, TargetSourceAbsent, SourcePreflight},
SourceHoldoutRefusal::UnidentifiedSource).

HoldoutTargetSourceFile drives on exact sha256 identity, independent of song
curation:
- missing target source_sha256 refuses;
- any participating manifest chunk without sha256 refuses before material;
- a target hash carried by no loaded chunk refuses;
- a target present only via a skipped/unloaded chunk refuses;
- every loaded chunk sharing the target hash is excluded, regardless of
  bar_range or track;
- a song-uncurated corpus with complete hashes SUCCEEDS in file mode (file
  mode must not run song_holdout_preflight or require song_id);
- material equals keeper-only across references, rhythms, gesture, and
  skipped reporting;
- the full-ChunkMeta binding still refuses first;
- deterministic.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012T7SRMiXmZe5v1imtrKMPi
Implement HoldoutTargetSourceFile:
- TargetIdentity gains source_sha256 (song_id kept, per-mode explicit);
- CorpusMode::HoldoutTargetSourceFile;
- source_file_preflight rejects any manifest chunk without sha256
  (SourceHoldoutRefusal::UnidentifiedSource) — complete hash identity, no
  basename fallback;
- HoldoutError::{MissingTargetSourceSha256, SourcePreflight, TargetSourceAbsent};
- order: check_binding -> source_file_preflight -> require target sha256 ->
  partition by exact sha256 -> refuse if nothing excluded -> corpus_material.
  File mode does NOT run song_holdout_preflight and does NOT require song_id,
  so a hash-identified but song-uncurated corpus succeeds.

Also fixes the two stale rustdoc comments so ProvenanceMismatch and
check_binding describe full ChunkMeta equality (not song_id/sha256), and
updates the module/README/Cargo scope to song + source-file modes.

28/28 tests green; clippy (all=deny, pedantic=warn) and rustfmt clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012T7SRMiXmZe5v1imtrKMPi

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b8cfe83d1f

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

Comment thread reachability-lab/src/lib.rs
@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The reachability lab adds fail-closed source-file holdout mode using exact source_sha256 identity, new typed refusal cases, binding and preflight validation, matching-chunk exclusion, documentation updates, and comprehensive tests.

Changes

Source-file holdout

Layer / File(s) Summary
Holdout contracts and refusal types
reachability-lab/src/lib.rs
Adds HoldoutTargetSourceFile, source-hash target identity, source-file refusal types, and full-ChunkMeta binding semantics.
Source-file preparation flow
reachability-lab/src/lib.rs
Binds loaded records, rejects unidentified manifest chunks, validates the target hash, and excludes every matching chunk regardless of range or track.
Source-file scenarios and documentation
reachability-lab/src/lib.rs, reachability-lab/README.md, reachability-lab/Cargo.toml
Adds refusal, exclusion, skipped-reporting, ordering, uncurated-song, and determinism tests, and updates ADR-0032 documentation.

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

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant prepare_corpus_for_mode
  participant source_file_preflight
  participant LoadedCorpus
  Caller->>prepare_corpus_for_mode: request HoldoutTargetSourceFile
  prepare_corpus_for_mode->>LoadedCorpus: validate full ChunkMeta binding
  prepare_corpus_for_mode->>source_file_preflight: validate source_sha256 identities
  source_file_preflight-->>prepare_corpus_for_mode: return preflight result
  prepare_corpus_for_mode->>LoadedCorpus: exclude chunks matching target source_sha256
  prepare_corpus_for_mode-->>Caller: return prepared corpus or typed refusal
Loading

Possibly related PRs

  • PhysShell/griff#166: Defines the ADR-0032 holdout boundary and typed refusal structure extended by this change.
  • PhysShell/griff#168: Provides the preceding song-mode implementation expanded here with source-file targeting.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding an exact-sha256 source-file holdout mode under ADR-0032.
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.
✨ 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-source-file-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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
reachability-lab/src/lib.rs (1)

139-183: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider extracting the shared partition-and-build pattern.

The HoldoutTargetSong and HoldoutTargetSourceFile arms both follow the identical shape: bind → preflight → require target → partition loaded chunks by a predicate → refuse if nothing excluded → build material from survivors. With fragment mode slated as the next slice built on this same identity path (per the stack context), this duplication is likely to grow to a third near-identical arm.

♻️ Possible extraction sketch
fn exclude_by<F>(
    loaded: Vec<LoadedChunk>,
    skipped: Vec<String>,
    matches_target: F,
    absent_err: impl FnOnce() -> HoldoutError,
) -> Result<Option<CorpusMaterial>, HoldoutError>
where
    F: Fn(&LoadedChunk) -> bool,
{
    let (excluded, kept): (Vec<_>, Vec<_>) = loaded.into_iter().partition(matches_target);
    if excluded.is_empty() {
        return Err(absent_err());
    }
    Ok(Some(corpus_material(kept, skipped)))
}

Optional now; worth doing before fragment mode adds a third copy of this shape.

Also applies to: 188-200

🤖 Prompt for 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.

In `@reachability-lab/src/lib.rs` around lines 139 - 183, Extract the duplicated
partition, absent-target validation, and survivor material construction from the
HoldoutTargetSong and HoldoutTargetSourceFile arms into a shared helper such as
exclude_by. Keep each arm responsible for binding, mode-specific preflight,
target extraction, and supplying its matching predicate and HoldoutError;
preserve the existing corpus_material and skipped-data behavior.
🤖 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.

Nitpick comments:
In `@reachability-lab/src/lib.rs`:
- Around line 139-183: Extract the duplicated partition, absent-target
validation, and survivor material construction from the HoldoutTargetSong and
HoldoutTargetSourceFile arms into a shared helper such as exclude_by. Keep each
arm responsible for binding, mode-specific preflight, target extraction, and
supplying its matching predicate and HoldoutError; preserve the existing
corpus_material and skipped-data behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 783fd72b-6456-4a85-8788-74d071e62c82

📥 Commits

Reviewing files that changed from the base of the PR and between e86560a and b8cfe83.

📒 Files selected for processing (3)
  • reachability-lab/Cargo.toml
  • reachability-lab/README.md
  • reachability-lab/src/lib.rs

@PhysShell
PhysShell merged commit 2ff8172 into main Jul 29, 2026
15 checks passed
@PhysShell
PhysShell deleted the claude/holdout-source-file-mode branch July 29, 2026 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant