docs: add Generator Reachability Lab Phase 0 metric-inventory audit - #154
Conversation
Delivers the Phase 0 audit named in generator-reachability-lab.md §5 (docs-only; no production code; binds nothing until the proposal is accepted). Six required answers, every code claim verified by file:line: 1. Inventory of the metric layer (scoring/similarity/novelty/syncopation/ feature/rerank/structure/gesture/closure) with versioned policies (similarity v3, novelty v1, generation_rerank v1, structure v1, closure v1). 2. Per-axis triage (reuse/extend/new) — the debt is privacy, not absence: the reusable primitives are private helpers. 3. Holdout feasibility: PASS — enforceable on Vec<LoadedChunk> before corpus_material() (generation_input.rs:137). Identity is dropped at CorpusMaterial construction — a blocker for post-hoc leak attribution only, not for holdout. Caveat: bar_range is Option (None = whole-file overlap). 4. Target eligibility: honest correction — no ExactVoice type exists; the monophonic (onset,duration,pitch) projection is a private gesture::top_line. Generator output is provably monophonic, so poly/chord/technique targets are correctly ineligible. 5. Cost benchmark: no bench infra exists (no criterion/benches). Protocol + dev-only harness specified; numbers TBD (Rust toolchain unavailable in the authoring env) — measurement is a mechanical follow-up. 6. Roadmap placement: recommend an ADR for the measurement/holdout boundary + an isolated lab crate (constraint-lab/ADR-0010 precedent), not a new stage; no SN number invented (glossary §0). Also flags three private "melodic line" extractors (consolidate before a fourth). Back-references the audit from the proposal §5. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012T7SRMiXmZe5v1imtrKMPi
|
Warning Review limit reached
Next review available in: 35 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 084fbd5bfe
ℹ️ 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".
| song identity (`title`, `filename`, `sha256`) are **required**; the source range | ||
| `bar_range` is **`Option`**. |
There was a problem hiding this comment.
For pre-v9 records, SourceRef::sha256 is explicitly optional (core/src/corpus.rs:45-50,114-120), and load_chunk falls back to the basename when it is absent (cli/src/generation_input.rs:83-101); the code itself notes that a filename is not an identity. Therefore a corpus containing such a record cannot reliably implement HoldoutTargetSong by source identity, so declaring PASS here can label a leaky run as a valid holdout. These records need to be rejected/migrated in holdout mode, or the missing identity must remain a Phase-1 blocker.
Useful? React with 👍 / 👎.
| No production code. Deliverable (**delivered** — see | ||
| [`../audit/2026-07-generator-reachability-metric-inventory.md`](../audit/2026-07-generator-reachability-metric-inventory.md); | ||
| a docs-only draft that binds nothing until this proposal is accepted): |
There was a problem hiding this comment.
The Phase-0 contract still requires measured costs at 1k/10k/100k trials (item 5), but the linked audit explicitly leaves every value as TBD (unmeasured) and supplies only a harness sketch. Marking the deliverable as completed can let later work treat the evidence gate as satisfied even though the benchmark result that gates directed-search discussion does not exist; keep Phase 0 incomplete until those measurements are recorded.
Useful? React with 👍 / 👎.
|
|
||
| | Proposed fact | Best existing source | Decision | Reason (grounded) | | ||
| | --- | --- | --- | --- | | ||
| | Exact onset set | `syncopation::track_onsets` (`syncopation.rs:59`, private); `structure::track_notes` (`:127`) | **new** | No public onset-set projection; extractors are private and on different shapes. | |
There was a problem hiding this comment.
The table defines extend as lifting an existing private helper, then labels this row new solely because the onset extractor is private. For eligible monophonic inputs, syncopation::track_onsets already computes the exact onset set, and novelty::top_line also contains the required onsets, so following this triage would encourage another projection despite the audit's extraction-first goal. Mark this as extend and select the shared line/onset primitive to lift.
Useful? React with 👍 / 👎.
…red, onset triage) Three Codex findings on 084fbd5, all accepted: - P1 holdout content identity: "PASS" was too strong. SourceRef.sha256 is Option (pre-v9) and the loader falls back to the basename (cli/src/generation_input.rs:87-101; "a filename is not an identity"), so a corpus with sha256-less records cannot reliably do HoldoutTargetSong by content identity. Verdict qualified to fail-closed: reject/migrate records without sha256 in holdout mode (the proposal's own §6 fail-closed law), or the missing identity is a Phase-1 blocker. Records WITH sha256 stay fully reliable. - P2 "delivered" overstated: item 5 (measured cost) is still TBD, so Phase 0 is not satisfied as an evidence gate. Proposal back-reference changed to "drafted, not yet complete"; audit §0 says so explicitly. - P2 onset-set triage: reclassified new -> extend to match the audit's own extend definition (syncopation::track_onsets already computes the onset set privately; lift the shared line/onset primitive rather than author a new one). Docs-only; binds nothing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012T7SRMiXmZe5v1imtrKMPi
|
@codex — all three findings accepted and fixed in
CI green on the new head; docs-only. Please re-review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2583e20b5a
ℹ️ 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".
| identity is a Phase-1 blocker, not a footnote. (Records *with* `sha256` are | ||
| fully reliable — the content check runs and a mismatch is a hard failure.) |
There was a problem hiding this comment.
When the same song is represented by two different MIDI/Guitar Pro files, both records can have valid but different SHA-256 values, so the claim that hash-bearing records are “fully reliable” still allows HoldoutTargetSong to retain target-song material. SourceRef has only a basename and file-content hash (core/src/corpus.rs:98-120), while EnsembleRef.group_id identifies parts from one source span rather than the song (core/src/corpus.rs:271-279); there is no stable song ID in the model. Phase 1 therefore needs a canonical song identity (or must redefine this mode as exact-source-file holdout) before this can be a conditional PASS.
Useful? React with 👍 / 👎.
The audit conflated source-file identity with song identity. The model (schema v9) has sha256 (one file's content hash), filename, a free-form title, and EnsembleRef.group_id (siblings of ONE source span) — but NO canonical song_id. So two tabs of one composition (MIDI vs GP5) hash differently and one leaks while the other is held out: a source-file holdout mislabelled as a song holdout. Verdict split into three modes: - HoldoutTargetSourceFile: CONDITIONAL PASS (needs sha256; sha-less fail closed). - HoldoutTargetFragment: CONDITIONAL PASS within one source file (sha256 + bar_range; None = whole-source overlap). - HoldoutTargetSong: BLOCKED today — needs a canonical song_id (or a versioned files->song manifest) as a Phase-1 prerequisite; EnsembleRef.group_id is one source span, not a song, and cannot substitute. Also: dropped the "title/filename/sha256 = required song identity" overstatement (title is free-form; sha256/track_index/bar_range are Option); qualified "fully reliable" to "reliable as a source-FILE identity". Proposal §6 keeps HoldoutTargetSong but records it is not fail-closed-implementable today. PR body updated to match (was still "PASS" / "delivered"). Docs-only; binds nothing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012T7SRMiXmZe5v1imtrKMPi
|
Accepted — the file-vs-song identity conflation was the real defect. Fixed in Holdout verdict split into three modes (§3):
Also dropped the " Proposal §6 keeps PR body updated to match the head — was still "Holdout: PASS" / "delivered audit"; now states the three-mode split and Phase-0-incomplete. Standing accepted (per your note): onset |
… CHANGES) 1. CorpusMode enum: add HoldoutTargetSourceFile — the audited file-level mode that PASSes today existed only in prose; the proposal enum now enumerates NoCorpus / HoldoutTargetSourceFile / HoldoutTargetFragment / HoldoutTargetSong / LeakyDiagnostic, with HoldoutTargetSong marked BLOCKED until a song_id. 2. Proposal §5 starter table: onset row none/new -> track_onsets/extend, to match the audit triage (else a future agent builds a fourth extractor). 3. Eligibility record: carry per-mode identity explicitly — source_sha256 (required, file/fragment modes typed-refuse without it), canonical_song_id (Option; HoldoutTargetSong typed-refuses without it), bar_range, track_index, projection, eligibility. No basename fallback. Docs-only; binds nothing. Still an honest Phase 0 DRAFT (item 5 cost run outstanding), not a completed Phase 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012T7SRMiXmZe5v1imtrKMPi
|
Thanks — three consistency fixes pushed in
This is one consistency commit with exactly the three requested changes; nothing else touched. Still an honest Phase 0 draft — item 5 (cost run) remains a separate mandatory closure, not satisfied here. CI running on |
|
@codex — the 10:14 P1 ("Require a song ID before trusting hash-bearing records") is fully addressed in |
Review ResultConfirmed — the P1 finding is fully resolved. No further changes requested. The updated documentation now:
The original “fully reliable” song-level implication is gone. The proposal and audit are now internally consistent about what the current corpus model can and cannot guarantee. No repository changes were necessary, so I did not create a commit or a follow-up pull request. The original PR is already present in the current history as merge commit Checks
|
1 participant
What
New audit
docs/audit/2026-07-generator-reachability-metric-inventory.md— the Phase 0 deliverable named ingenerator-reachability-lab.md§5.Docs-only research artifact; binds nothing (the proposal binds nothing until accepted). Phase 0 adds no production code. Every code claim is verified against the tree and cited by
file:line.Phase 0 is a drafted, honest, incomplete deliverable: items 1–4 & 6 are answered; item 5 (measured cost) is outstanding (no Rust toolchain was available to run the specified harness), so Phase 0 is not yet satisfied as an evidence gate.
The six required answers (§5)
scoring/similarity/novelty/syncopation/feature/rerank/structure/gesture/closure), with the versioned policies present (similarityv3,noveltyv1,generation_rerankv1,structurev1,closurev1).reuse/extend/new). Headline: the debt is privacy, not absence — reusable primitives are private helpers awaiting disciplined extraction.HoldoutTargetSourceFile— CONDITIONAL PASS (requiressha256;sha256-less records fall back to a basename and must fail closed).HoldoutTargetFragment— CONDITIONAL PASS within one source file (sha256+bar_range;None= whole-source overlap).HoldoutTargetSong— BLOCKED today: no canonicalsong_idgroups the several tabs of one composition (MIDI vs GP5 → differentsha256), so a song holdout would silently keep one and exclude the other. Needs a canonical song identity (or a files→song manifest) as a Phase-1 prerequisite.EnsembleRef.group_idis one source span, not a song, so it can't substitute.CorpusMaterialconstruction — a blocker for post-hoc leak attribution only.ExactVoicetype; the monophonic(onset,duration,pitch)projection is a privategesture::top_line. The generator's output is provably monophonic, so poly/chord/technique targets are correctly ineligible.criterion, nobenches/). Protocol + a zero-production-code dev-only harness given; numbersTBD (unmeasured)— Phase 0 stays incomplete until they are recorded.SNinvented (glossary §0).Also flags the three private "melodic line" extractors — consolidate before a fourth.
Also
generator-reachability-lab.md— back-reference to the drafted (not-complete) audit; §6 records thatHoldoutTargetSongneeds a canonical song identity as a Phase-1 prerequisite.Review
Addressed two Codex rounds + arbiter REQUEST CHANGES: fail-closed content identity,
new→extendonset triage, "delivered"→"drafted", and the file-vs-song holdout split (HoldoutTargetSongBLOCKED).Docs-only → CI (Rust-only) green. Not a completed Phase 0 — an honest Phase 0 draft. Not merging without an explicit human GO.
🤖 Generated with Claude Code
https://claude.ai/code/session_012T7SRMiXmZe5v1imtrKMPi