docs(proposal): human-confirmed song-id curation workflow - #171
Conversation
Docs-only. Adds docs/proposals/song-id-curation-workflow.md (+ README index). Status: for discussion — binds nothing, not Accepted. No Rust/Python/corpus/ manifest/fixture/generated changes; no corpus labels written. Treats ADR-0031 and ADR-0032 as fixed law (10 invariants). Defines an offline, human-confirmed pipeline: inventory sources by sha256 -> non-authoritative metadata-only suggestions -> explicit curator decisions (no default = acceptance) -> immutable fingerprinted plan -> transactional apply to a fresh copy -> deterministic songs manifest -> validation via the existing core song_holdout_preflight. Curation unit is the sha256 source (all its chunks share one SongId, indivisible). Suggestions never write song_id; artist is a reported heuristic, not structured provenance. Makes the selections the brief required rather than deferring them: owner = standalone isolated song-curation/ tool (compared against griff curate / manifest); manifest strategy = tool owns song-aware generation first, extend griff manifest later; SongId = opaque ledger-issued song-<monotonic counter> (title-slug and membership-hash rejected); a domain-tagged sorted-line SHA-256 corpus fingerprint; an 11-category typed refusal taxonomy; and a four-slice RED->GREEN implementation sequence gated behind a new ADR (prov. ADR-0033) and a controlled pilot. Partial curation is holdout_ready:false and never a valid song-holdout corpus. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012T7SRMiXmZe5v1imtrKMPi
📝 WalkthroughWalkthroughAdds a discussion-only proposal and index entry for an offline, human-confirmed SongId curation workflow covering inventory, suggestions, decisions, deterministic application, manifest generation, validation, artifacts, refusals, and governance. ChangesSong ID curation workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@docs/proposals/song-id-curation-workflow.md`:
- Line 37: Add explicit language tags to every fenced Markdown block in
song-id-curation-workflow.md, including the pseudocode and field-list blocks at
the referenced sections; use text where no more specific language applies so
Markdown lint passes.
- Around line 338-341: Update the corpus fingerprint encoding to emit a distinct
presence marker for CorpusManifest.songs when it is None versus when it is an
explicitly present empty map. Ensure the marker participates in the UTF-8 byte
sorting, concatenation, and hash alongside chunk and song lines, while
preserving existing song-entry encoding.
- Around line 338-341: Define an injective encoding for every fingerprint field
used by the chunk and song lines, such as escaping or length-prefixing values,
and apply it consistently before sorting and hashing. Alternatively, explicitly
constrain and validate chunk_id and manifest-derived fields to reject tabs and
newlines, while preserving the documented line format and fingerprint behavior.
- Around line 144-156: Define the immutable serialized plan artifact referenced
by §5.4 and make it the required input to Apply in §8. Specify its schema,
including plan digest, corpus fingerprint, decisions-artifact identity/version,
tool-policy version, and complete source-level assignment and affected-chunk
lists, along with validation rules proving the artifact is the plan derived from
those inputs.
- Around line 191-198: Update the curated manifest workflow to add a hard
refusal-to-overwrite guard before the controlled pilot: store the curated
manifest at a distinct path and make ordinary `griff manifest` refuse to
overwrite it. Do not rely solely on the existing documentation warning in the
“Interim hazard” section; preserve curated `songs` data whenever standard
manifest generation is invoked.
- Around line 134-142: Update the decisions artifact schema to include reject in
the action enum and explicit references for the affected source groups. Add a
supersedes_song_ids-equivalent field to correct and merge decisions, documenting
when it is populated so superseded SongId values are auditable and replayable.
Keep the schema aligned with the confirmation workflow and apply-report
requirements.
🪄 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: 01ae917e-b515-42d0-8bd5-ae5844c66c9d
📒 Files selected for processing (2)
docs/proposals/README.mddocs/proposals/song-id-curation-workflow.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 455d0327a5
ℹ️ 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".
Address the five blocking findings; still docs-only, Status: for discussion.
1. Decisions ledger is now an append-only per-event JSON document following
the CurationStoreV1 precedent: each event carries its own event_id,
curator, occurred_at, and corpus_fingerprint (per-event fingerprints let a
later event bind to its own post-apply snapshot honestly). `action` is a
tagged union — accept / reject / manual_define / split / merge / correct —
each with exact source hashes and supersedes_song_ids; a reject records the
reviewed candidate + hashes so "reviewed and rejected" stays distinct from
"never reviewed".
2. Added the serialized, digest-verified plan artifact
(song-curation.plan.v1) as the only thing Apply consumes: assignments with
expected_existing_song_id and affected_chunk_ids, generated_songs_map, and a
plan_digest Apply recomputes (plan digest, corpus fingerprint, decisions
digest, bindings, prior labels).
3. Fingerprint is now injective canonical compact-JSON records with a
manifest_songs absent/present marker, so None vs Some({}) differ and
tab/newline in ChunkId/SongId cannot collide; same scheme defines
plan_digest / decisions_digest. Notes the existing core corpus_fingerprint
and why a label-aware one is needed.
4. Manifest guard is now hard: the tool writes a distinct curated path and
refuses ordinary <corpus>/manifest.json; ordinary griff manifest cannot
overwrite it; strategy 2 later. Corrected the builder citation to
cli/src/main.rs:1998 (shared seam ui-core/src/corpus.rs:18).
5. Resolved the self-contradictions: §14 lists closed v1 choices only —
census strip_version_suffix reused exactly, monotonic single-writer SongId
(concurrent issuance out of scope, not open), JSON per-event ledger.
Also tagged the untyped fenced blocks with `text`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012T7SRMiXmZe5v1imtrKMPi
REQUEST CHANGES addressed — docs-only repair (head
|
Close the three proof-chain contradictions exposed by the hardened schemas. Still docs-only, Status: for discussion. 1. The plan now EMBEDS the complete ordered decision batch (not just a digest), so Apply can recompute decisions_digest, replay the events, DERIVE the assignments itself, and compare them to the plan's — a digest cannot be recomputed from its own field, and plan_digest alone proves only internal integrity, not derivation from the decisions (DecisionProjectionMismatch on divergence). 2. Incremental curation now has an explicit batch/apply chain: the ledger is append-only BATCHES, each binding its events to one input_corpus_fingerprint; one plan embeds exactly one unapplied batch; historical batches are audit history and are never replayed; batch -> report -> next batch is linked by previous_application_report_digest with input==previous-output fingerprints; reuse of an applied batch refuses (DecisionBatchAlreadyApplied, ApplicationChainMismatch, DecisionBatchFingerprintMismatch). 3. The digests are now three distinct canonicalizations, not "the same scheme": order-insensitive corpus_fingerprint (set-like), order-SENSITIVE decisions_digest (events in exact append order, ordinal included, no sorting), and order-aware plan_digest (embedded batch order preserved, assignments/chunk-ids/map sorted). Updated §5.4/§5.5/§6/§8.2/§8.3/§8.4/§9/§10/§14 accordingly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012T7SRMiXmZe5v1imtrKMPi
Close four local contradictions + a taxonomy gap inside the accepted chain model. Still docs-only, Status: for discussion. 1. merge/split are now authorized replacement actions alongside correct: a non-None label may be replaced by exactly the action whose supersession set covers it (correct: supersedes_song_ids; merge: from_song_ids; split: from_song_id). Removed "correct is the only variant permitted to change a non-None label" — all six actions are first-class. 2. report_digest now has a canonicalization contract: §9 defines one shared canonical JSON encoding (sorted object keys, compact UTF-8) under FOUR ordering contracts — corpus (set-like), decisions (append order), plan (order-aware), and report (applied_event_ids in batch order, refusals sorted by (kind, source_sha256)). 3. Event order has one source of truth: the events array order is authoritative, ordinal MUST equal array position, ordinals contiguous+unique, event_id unique — else InvalidDecisionBatchOrder / DuplicateDecisionEventId before digest/replay. 4. DecisionBatchAlreadyApplied is now provable: a new append-only application index (§8.5, song-curation.applications.v1) is an explicit Apply input, written transactionally with the report; Apply checks batch_id absence and chains via the index's last record (catches rejection-only / fingerprint- neutral batches and re-apply to a fresh copy). 5. Added DecisionDigestMismatch, distinct from PlanDigestMismatch and DecisionProjectionMismatch. Changed §5.5, §6, §8.2, §8.4 (+ new §8.5), §9, §10, §14. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012T7SRMiXmZe5v1imtrKMPi
What
Docs-only proposal: an offline, human-confirmed workflow for assigning
SongId(ADR-0031) to corpus sources, so song-level holdout (ADR-0032) becomes usable without heuristics becoming provenance by clerical accident.docs/proposals/song-id-curation-workflow.md+ the README index bullet.df2978a144bc2549f848149228635b8f922e7a3b.Verifiable chain: ledger → plan → apply → report → index
CurationStoreV1precedent). Theeventsarray order is authoritative (ordinal == position, contiguous/unique;event_idunique), so replay is unambiguous.actionis a tagged union;correct/merge/splitare all first-class authorized replacements, each replacing exactly the labels in its supersession set.plan_digestanddecisions_digest(PlanDigestMismatch/DecisionDigestMismatch), replays the events, derives assignments itself, and compares (DecisionProjectionMismatch).song-curation.applications.v1) — an append-only registry that is an explicit Apply input, written transactionally with the report; makesDecisionBatchAlreadyAppliedand the chain provable (catches rejection-only / fingerprint-neutral batches, and re-apply to a fresh copy), not inferred from corpus side-effects.corpus_fingerprint(with amanifest_songsabsent/present marker), order-sensitivedecisions_digest, order-awareplan_digest, and order-awarereport_digest.<corpus>/manifest.json(buildercli/src/main.rs:1998).Closed v1 choices
Owner = standalone
song-curation/; ledger = batched JSON perCurationStoreV1; six first-class actions (three authorized replacements);SongId= opaque ledger-issuedsong-<monotonic counter>, single-writer; suggestion normalization = censusstrip_version_suffixreused exactly; typed refusal taxonomy (now incl.DecisionDigestMismatch,InvalidDecisionBatchOrder,DuplicateDecisionEventId); 4 RED→GREEN slices gated behind a new ADR (prov. ADR-0033) + controlled pilot.Review status
Three REQUEST-CHANGES rounds addressed (five artifact-contract findings → three proof-chain contradictions → four edge contracts + a taxonomy gap); all review threads resolved. Still no code or corpus-label changes.
Governance
Discussion only → on acceptance, durable decisions move to a new ADR (prov. ADR-0033); implementation from the ADR; corpus labeling stays prohibited until the implementation + controlled-pilot gates open.
🤖 Generated with Claude Code
https://claude.ai/code/session_012T7SRMiXmZe5v1imtrKMPi