feat(census): read-only real-corpus health census (PR A) - #158
Conversation
|
Warning Review limit reached
Next review available in: 29 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 (6)
✨ 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: b004b42448
ℹ️ 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".
| for marker in ["_ver__by_", "_ver_by_", " ver by ", " ver. by "] { | ||
| if let Some(i) = w.find(marker) { | ||
| w.truncate(i); | ||
| } | ||
| } |
There was a problem hiding this comment.
For filenames such as Connector (ver 2 by LPFzCS_LMS).gp5, none of these markers matches because the actual suffix contains parentheses and a version number. The checked report contains 140 names in this form, including multiple versions of the same songs, but reports only seven multi-transcription works; this materially overstates distinct_works_estimated and undermines the song-level leakage migration this census is intended to guide.
Useful? React with 👍 / 👎.
|
|
||
| // ── census ──────────────────────────────────────────────────────────────────── | ||
|
|
||
| fn census(corpus_dir: &Path, tabs_dir: &Path) -> Result<Health, String> { |
There was a problem hiding this comment.
This is the first commit containing the new census module, and it adds the complete implementation without any tests or a preceding commit containing failing census tests. That bypasses the repository's mandatory per-commit red-green workflow for new modules and leaves central heuristics such as work grouping and source reconciliation unverified; rebuild the sequence with failing tests committed before the implementation.
AGENTS.md reference: AGENTS.md:L78-L91
Useful? React with 👍 / 👎.
|
|
||
| fn walk(dir: &Path) -> Vec<PathBuf> { | ||
| let mut out = Vec::new(); | ||
| if let Ok(entries) = fs::read_dir(dir) { |
There was a problem hiding this comment.
If either input path is misspelled or unreadable—or a nested subtree becomes unreadable—read_dir failure is converted into an empty result, so census still returns success with a digest and zero/partial counts. Because main then writes the default tracked JSON and Markdown outputs, an operator can silently replace the health audit with a report over an incomplete corpus; propagate traversal errors instead.
Useful? React with 👍 / 👎.
| let name = basename(&path); | ||
| by_basename | ||
| .entry(name.clone()) | ||
| .or_default() | ||
| .push(name.clone()); | ||
| by_stem.entry(stem(&name)).or_default().push(name.clone()); |
There was a problem hiding this comment.
When recursive tab directories contain the same basename, both physical files are stored as identical strings, so the by_basename key and later unreferenced listing collapse them. The committed artifact already exposes this undercount: it records 410 tab files, 400 matched sources, no ambiguities, but only 9 unreferenced tabs; retaining relative paths while indexing by basename is necessary to classify every physical file.
Useful? React with 👍 / 👎.
| `census/` tool (`cargo run -p griff-corpus-census -- <corpus_dir> \ | ||
| <tabs_dir>`). Rendered from `corpus-health.json`; re-running on the same \ |
There was a problem hiding this comment.
The documented invocation cannot run from the repository root: checking it produces package(s) griff-corpus-census not found in workspace because census is explicitly excluded from the workspace, while Cargo's -p option only selects a package in the active workspace. Render a command using --manifest-path census/Cargo.toml so readers can actually reproduce the report.
Useful? React with 👍 / 👎.
…ristics Establishes the census tool and its test contract with the central heuristics red: strip_version_suffix is a stub, so work_key does not group transcriptions of one composition and two tests fail (version-suffix stripping; work_key grouping). The other heuristics (stem, resolve, walk error) are green. Isolated census/ crate (excluded from the workspace, griff-core read-only, ADR-0010). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012T7SRMiXmZe5v1imtrKMPi
… +Codex fixes Green: strip_version_suffix strips a trailing `(ver N by X)` / `(N)` suffix (only a real version marker — a title's own parentheses like `(Reprise)` are preserved), so work_key groups the transcriptions of one composition; all 5 tests pass. Also folds in the other Codex review fixes over the initial census: - walk() propagates directory-read errors (no silent report over a partial corpus); - tabs are indexed by relative path, so files sharing a basename in different directories are counted/classified separately; - the reproduce command uses `--manifest-path census/Cargo.toml` (works from the repo root; `-p` cannot select a workspace-excluded crate). Verified in nix: cargo test 5 pass, clippy -D warnings clean, fmt clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012T7SRMiXmZe5v1imtrKMPi
corpus-health.json + docs/audit/2026-07-real-corpus-health.md, produced by the census tool over ~/griff_data (digest d87c530c…, deterministic). Real findings: 9907 chunk files / 0 parse errors; two disjoint manifest layers (ingest 9687/220 curated) reconcile exactly; fully measured (structure/gesture/complexity/rights 100%; 0 broken/orphan ensemble refs); but schema v8 → sha256 0% / track_index 0% (the fail-closed holdout contracts block on the whole corpus, so the v9 sha256 backfill is the immediate prerequisite); 400 referenced sources all matched, 9 unreferenced tabs, 1 content-duplicate pair; 257 works and 94 multi-transcription works (237 files) — the concrete song-level leakage ADR-0031 closes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012T7SRMiXmZe5v1imtrKMPi
b004b42 to
56c9913
Compare
|
All five findings fixed; the branch is rebuilt as a clean red→green→report history (force-pushed, head
Verified in nix: |
… path (arbiter) Two acceptance-evidence defects the arbiter found: 1. corpus_digest hashed only chunk bytes (no relpath, no manifests, no tabs), so a manifest edit / tab rename / path swap changed the report but not the digest. 2. source↔tab reconciliation was not a partition: resolve() returned a count and unreferenced_tabs used a basename/stem heuristic, so 410 tabs / 400 unique matches reported 9 unreferenced (must be >= 10). Red step: introduce input_digest (bind all inputs) and a Resolution enum returning concrete paths, with resolve() and input_digest_of() stubbed — the new tests fail (resolve concrete-path, physical partition regression, digest order-independence + change-sensitivity). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012T7SRMiXmZe5v1imtrKMPi
…on is a partition
Green step for the arbiter's two evidence defects:
- input_digest = SHA-256 over sorted domain-tagged `<kind>:<relpath>:<sha256>`
lines for every chunk, manifest, and tab (input_digest_of). Order-independent;
any content change, rename, or path swap changes it.
- resolve() returns Resolution::{Unique(path)|Missing|Ambiguous(paths)}; the
source↔tab reconciliation collects concrete referenced paths and computes
unreferenced_tabs as a true set difference over physical paths, with a
debug_assert that referenced ∪ unreferenced partitions the tabs. The
basename/stem `referenced_hits` heuristic is removed.
All 7 tests pass; clippy -D warnings + fmt clean (nix).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012T7SRMiXmZe5v1imtrKMPi
Regenerated over ~/griff_data with the fixed tool. Corrected numbers: unreferenced tabs 9 -> 10 (410 tabs - 400 unique matches; now a real partition: 0 missing, 0 ambiguous), and input_digest now binds all chunks + manifests + tabs (was the chunk-only corpus_digest). All other findings unchanged (9907 chunks, v8/sha256 0%, 257 works / 94 multi-transcription). Deterministic. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012T7SRMiXmZe5v1imtrKMPi
|
Both evidence defects fixed as a focused red→green→report follow-up ( 1. Digest binds all inputs. Right — the old 2. Physical-tab reconciliation is now a partition. Correct, and the 9 was arithmetically impossible (410 tabs, ≤400 unique matches ⟹ ≥10 unreferenced). Root cause was exactly as you diagnosed: Also updated the PR body (was 393/7 — now 257 works / 94 multi-transcription / 237 files, and unreferenced 10). Verified in nix: 7 tests pass, |
1 participant
What
PR A of the corpus-migration arc (A: census → B: v9 contract+RED → C: v9 backfill → D: v10 song_id). The first artifact connecting the contract series (benchmark #153, reachability lab #154, ADR-0031 song identity #157) to the real 9 907-record corpus instead of synthetic data.
A read-only census: isolated
census/crate (excluded from the workspace,griff-coreread-only; not a CI gate). Parses every*.chunk.jsonwith the productionChunkMeta, hashes tabs with the productionsource_sha256, and emitscensus/corpus-health.json+docs/audit/2026-07-real-corpus-health.md(rendered from the same struct). Changes no corpus data. Deterministic —input_digest(SHA-256 over sorted<kind>:<relpath>:<sha256>for every chunk, manifest, and tab) ties the report to an exact input set (verified: identical across re-runs).Findings on the real corpus
ingest/manifest.json(9 687 / 400 groups) +manifest.json(220 / 5) = 9 907; 0 uncovered, 0 orphan ids.sha2560 %,track_index0 % — the fail-closed holdout contracts (Phase-0 audit; ADR-0031) currently block on the whole corpus; the v9sha256backfill is the immediate prerequisite (PR B/C).SongIds).Review history
Codex round (all 5 fixed; branch rebuilt red→green→report): parenthesized
(ver N by X)normalization (→ 257/94, was 393/7), red→green sequence,walkerror propagation, relpath tab indexing,--manifest-pathcommand. Arbiter round (this follow-up, red→green→reportbd3e00d/adb49ce/257cb67):input_digestnow binds all inputs (was chunk-bytes only), and tab reconciliation is a true partition (resolvereturns concreteResolution::{Unique|Missing|Ambiguous}; unreferenced = physical set difference; the impossible 9 → correct 10).Verified in nix:
cargo test7 pass,clippy -D warningsclean,fmtclean, deterministic digest. Only workspace change is addingcensustoexclude. Not merging without an explicit human GO.🤖 Generated with Claude Code
https://claude.ai/code/session_012T7SRMiXmZe5v1imtrKMPi