feat(core,cli,web): persist near-duplicate link in ChunkMeta (schema v8) (#76) - #96
Conversation
The split flags near-duplicate phrases and shows them in the UI/CLI, but the signal lives only in the split envelope / PhraseChunk wrapper — a downloaded chunk.json or manifest loses which phrases are repeats. Failing tests for a persisted `ChunkMeta.duplicate`: a serde round-trip that skips the key when absent (pre-v8 byte-identical) and carries it when present, and CLI parity asserting the link is mirrored onto the record. Implemented in the green commit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
…(schema v8) (#76) `ChunkMeta` gains an optional `duplicate: Option<PhraseDuplicate>` under the established additive pattern (serde default + skip-if-none), bumping `SCHEMA_VERSION` to 8 — pre-v8 records round-trip byte-identically. Both split paths now mirror the computed near-duplicate flag onto the record after building it (web `split_segments_to_json`, CLI `chunks_for_segments`), so a downloaded chunk.json / manifest keeps which earlier phrase a later one repeats — not just the live UI/CLI. `PhraseDuplicate` gains serde derives; the round-trip proptest fuzzes the new field too. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
Y-statement for the ChunkMeta.duplicate field + SCHEMA_VERSION bump, noting why a structural field bumps the schema where the #75 tag additions did not. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds an optional ChangesChunkMeta duplicate field — schema v8
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18fd624649
ℹ️ 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".
| /// Skipped — not written as `null` — when absent, so pre-v8 files round-trip | ||
| /// byte-identically. | ||
| #[serde(default, skip_serializing_if = "Option::is_none")] | ||
| pub duplicate: Option<PhraseDuplicate>, |
There was a problem hiding this comment.
Split this schema change into red/green commits
The /workspace/griff AGENTS.md TDD workflow says “Never commit new pub fn / pub struct implementation in the same commit as the tests that cover it” and that reviewers must judge the commit sequence. This single commit adds the public ChunkMeta.duplicate schema field and its covering tests together, so the required failing-test commit is absent; please split the history into a red test commit followed by the implementation commit.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This reads the flattened PR diff rather than the commit sequence — the history is already red→green, as AGENTS.md asks reviewers to judge:
3b09acctest(core,cli): red— adds the covering testchunk_meta_persists_and_skips_the_near_duplicate_link(and the CLI persistence assertion). It does not compile:error[E0609]: no field 'duplicate' on type 'ChunkMeta'.54f70c5feat: green— addspub duplicate: Option<PhraseDuplicate>and makes that test pass.
Evidence:
git log -G 'pub duplicate: Option' -- core/src/corpus.rs→ only54f70c5git log -G 'chunk_meta_persists_and_skips' -- core/tests/corpus_schema.rs→ only3b09acc
The failing-test-first commit is present and precedes the implementation; the "single commit" is the squashed-diff view, not the branch history.
Generated by Claude Code
| /// Skipped — not written as `null` — when absent, so pre-v8 files round-trip | ||
| /// byte-identically. | ||
| #[serde(default, skip_serializing_if = "Option::is_none")] | ||
| pub duplicate: Option<PhraseDuplicate>, |
There was a problem hiding this comment.
Clear stale duplicate links on extent edits
When a split-generated chunk carrying duplicate is later re-curated with preview split/merge, preview/src/curation.rs uses fresh_extent to reset invalidated whole-extent state but will now preserve this new field. Because duplicate.of is only meaningful for the original split sibling index, both halves or the merged record can keep a stale duplicate pointer/share after their source.bar_range and ids change; clear it in the extent-changing paths alongside the other reset metadata.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 62739f6. fresh_extent is the single reset shared by both split halves (split_record) and merge_records, so clearing duplicate there covers every extent-changing path: a re-cut extent no longer keeps a stale sibling pointer, alongside the reviewer/structure/gesture/complexity resets. Covered by split_record_clears_the_near_duplicate_link (red 1a310e3 → green 62739f6).
Generated by Claude Code
…76) Codex P2 on #96: a split chunk's `duplicate.of` indexes the original split's sibling phrases, so once `fresh_extent` re-cuts the extent (split halves or a merge) the pointer is stale. Failing test asserting both split halves drop the link, mirroring the ensemble-link reset. Green adds the reset. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
) `fresh_extent` now clears `duplicate` alongside the reviewer decision and the whole-extent measurements, so a split half or a merged record no longer carries a stale sibling pointer (Codex P2 on #96). One chokepoint covers both the split and merge paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
Closes the curation gap surfaced by a DGD playtest export: the split flags near-duplicate phrases (#76) and shows them in the UI/CLI, but the signal lived only in the split envelope — a downloaded
chunk.jsonor a built manifest lost which phrases are repeats. On that export, phrases p2≡p6, p3≡p7, p4≡p8 are byte-identical repeats, yet nothing in the chunk recorded it.What
ChunkMetagains an optionalduplicate: Option<PhraseDuplicate>— the earlier split phrase a later one verbatim-quotes (transposition-aware) and the quote share. Both split paths now mirror the already-computed flag onto the persisted record:Schema
Bumps
SCHEMA_VERSION7 → 8 under the established additive pattern (serdedefault+skip_serializing_if), exactly likestructure(v2) /gesture(v3) /complexity(v6) /rights(v7). Pre-v8 records have noduplicatekey, parse asNone, and re-serialize byte-identically.PhraseDuplicategains serde derives. Unlike the #75 tag additions — data within an existing field, deliberately unversioned — this is a structural field, so it bumps the schema (recorded indecisions.log).Wiring
core/src/corpus.rs— field + version + doc;core/src/novelty.rs— serde derives onPhraseDuplicate.web/src/lib.rssplit_segments_to_jsonandcli/src/main.rschunks_for_segmentssetmeta.duplicatefrom the per-phraseflag_phrase_duplicatesresult, right where each already setsbar_range. The single-capture path staysNone(no phrase-sequence context).TDD
red (
3b09acc) → green (54f70c5) → docs (18fd624). The serde round-trip (skip-when-absent / carry-when-present) and CLI persistence parity were red first; the round-trip proptest fuzzes the new field too via a newarb_duplicate()strategy.Verification
cargo test --workspacegreen;cargo clippy --workspace --all-targets -- -D warningsclean;cargo fmt --all --checkcleancargo check+cargo clippycleanduplicate_of/duplicate_sharefor the pager; this only adds the same signal to the persisted record.🤖 Generated with Claude Code
https://claude.ai/code/session_01TTUbGjzD8ysnVnCJnZJE95
Generated by Claude Code
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Tests