Skip to content

fix(compile): drop same-batch duplicate page titles before approval#503

Closed
galuis116 wants to merge 1 commit into
vouchdev:testfrom
galuis116:fix/compile-batch-duplicate-title-collision
Closed

fix(compile): drop same-batch duplicate page titles before approval#503
galuis116 wants to merge 1 commit into
vouchdev:testfrom
galuis116:fix/compile-batch-duplicate-title-collision

Conversation

@galuis116

Copy link
Copy Markdown
Contributor

What changed

compile_kb() now folds each accepted draft's title/slug into taken_names
as soon as it survives validation, instead of computing taken_names once
up 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, because
taken_names never saw the first draft before the second was checked.
proposals.approve() deliberately exempts PAGE proposals from the generic
existing-artifact guard to support the vault-edit flow (update_page rather
than put_page), so approving the second duplicate proposal silently
overwrote 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.py already 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: a compile_kb() batch
containing duplicate-titled drafts will now drop the second (and any further)
duplicate instead of filing it, with report.dropped carrying the same
"page for ... already exists or is pending review" reason already used for
cross-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 check locally: ruff clean; mypy clean (pre-existing, unrelated
    os.getuid/os.getgid Windows-only stub gap in sandbox.py
    untouched by this change); tests/test_compile.py passes in full
    except one pre-existing, unrelated failure
    (test_jsonl_kb_compile_files_proposals, a Windows-only stub-LLM
    shell-quoting issue — confirmed present on main before this change
    via git stash comparison, not something this PR touches)
  • New / changed behaviour has a test
    (test_collision_within_same_batch_second_draft_dropped)
  • CHANGELOG.md updated under ## [Unreleased]

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
@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ca106829-816a-4c8a-a00a-65dc7b133742

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added docs documentation, specs, examples, and repo guidance tests tests and fixtures size: XS less than 50 changed non-doc lines labels Jul 16, 2026
@plind-junior plind-junior enabled auto-merge July 16, 2026 03:36
@galuis116 galuis116 closed this Jul 16, 2026
auto-merge was automatically disabled July 16, 2026 03:38

Pull request was closed

@galuis116

Copy link
Copy Markdown
Contributor Author

closing — this duplicates #453 (merged into test on 2026-07-13, ahead of main at the time I branched). my dedup pass checked main and existing issue titles but not test's already-merged state, so I independently re-found and re-fixed the same taken_names gap in compile_kb. sorry for the noise; re-basing my bug search against test going forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs documentation, specs, examples, and repo guidance size: XS less than 50 changed non-doc lines tests tests and fixtures

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant