fix(compile): drop same-batch duplicate page titles before approval#503
fix(compile): drop same-batch duplicate page titles before approval#503galuis116 wants to merge 1 commit into
Conversation
compile_kb() computed taken_names once before validating drafts and never folded accepted survivors back in, so two drafts in one llm response with the same (or same-slugifying) title both passed the collision guard. proposals.approve() exempts PAGE proposals from the generic existing-artifact guard to support the vault-edit flow, so approving the second duplicate silently routed through update_page and overwrote the first-approved page with no error and no reviewer signal. fold each survivor's title/slug into taken_names as soon as it's accepted, mirroring what phase 2 already does for wikilink resolution. new regression files two same-titled drafts in one batch, approves the surviving proposal, and asserts the first page's body is untouched. Fixes vouchdev#439
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
Pull request was closed
|
closing — this duplicates #453 (merged into |
What changed
compile_kb()now folds each accepted draft's title/slug intotaken_namesas soon as it survives validation, instead of computing
taken_namesonceup front and never updating it. A new regression test files two same-titled
drafts in one batch, approves the surviving proposal, and asserts the first
page's body is untouched.
Why
Two drafts in a single llm response sharing a title (or slugifying to the
same id) both passed
_draft_problem's collision guard, becausetaken_namesnever saw the first draft before the second was checked.proposals.approve()deliberately exemptsPAGEproposals from the genericexisting-artifact guard to support the vault-edit flow (
update_pageratherthan
put_page), so approving the second duplicate proposal silentlyoverwrote the first-approved page — no error, no diff, no "already exists"
signal, and an audit-log entry indistinguishable from a legitimate edit. The
collision-guard comment in
compile.pyalready states this exact invariant("a colliding 'new' draft would silently overwrite the page on approval");
it just wasn't enforced against siblings in the same batch, unlike phase 2's
wikilink resolution just below it, which does fold survivors back in
incrementally.
Fixes #439
What might break
Nothing for users with an existing
.vouch/directory — no on-disk shape,kb.*method, or object model changes. Behaviorally: acompile_kb()batchcontaining duplicate-titled drafts will now drop the second (and any further)
duplicate instead of filing it, with
report.droppedcarrying the same"page for ... already exists or is pending review"reason already used forcross-batch collisions. No previously-succeeding compile with distinct titles
is affected.
VEP
Not a surface change — no object model,
kb.*method, on-disk layout,bundle format, or audit-log shape change. Purely a validation-order fix
inside
compile_kb.Tests
make checklocally: ruff clean; mypy clean (pre-existing, unrelatedos.getuid/os.getgidWindows-only stub gap insandbox.pyuntouched by this change);
tests/test_compile.pypasses in fullexcept one pre-existing, unrelated failure
(
test_jsonl_kb_compile_files_proposals, a Windows-only stub-LLMshell-quoting issue — confirmed present on
mainbefore this changevia
git stashcomparison, not something this PR touches)(
test_collision_within_same_batch_second_draft_dropped)CHANGELOG.mdupdated under## [Unreleased]