Skip to content

fix(fleet): scope the sweep duplicate-check to the paths this round touched - #693

Merged
swackhamer merged 1 commit into
mainfrom
fix/dedupe-sweep-prs-against-open-prs
Aug 11, 2026
Merged

fix(fleet): scope the sweep duplicate-check to the paths this round touched#693
swackhamer merged 1 commit into
mainfrom
fix/dedupe-sweep-prs-against-open-prs

Conversation

@swackhamer

Copy link
Copy Markdown
Collaborator

Problem

Follow-up to #691. Measured 2026-08-11 in production, immediately
after #691 merged and the dispatcher restarted: PR #692 duplicated
#690
anyway (byte-identical diffs, md5-verified) — the exact bug #691
was supposed to prevent.

Root cause: #691's duplicate check did a raw full-tree diff
(git diff --quiet <candidate>..HEAD). PR #690's branch was cut
before #691 itself landed on origin/main, so it legitimately
lacks the scripts/ changes #691 added. PR #692's branch, cut
after #691 merged, carries those changes as part of its own base.
A full-tree diff sees that unrelated drift as "different" forever —
and the same defeat recurs for every future round whenever any
unrelated commit lands on main while a sweep PR sits open, which
given this repo's "the fleet publishes, it never merges" design is
routine, not an edge case.

Fix

Scope the comparison to git diff --name-only origin_ref..HEAD — the
paths THIS round's own squad merges actually touched — instead of the
whole tree. That isolates the tag-fix content from incidental history
the two branches don't share.

Testing

New regression test reproduces the exact #692-vs-#690 shape: an
unrelated commit lands on main between when the open PR's branch was
cut and when the new round's branch is cut, with the identical tag fix
present on both. Confirmed the test fails against the pre-fix
(#691-only) code and passes with this fix — verified by stashing
just this change and re-running the test before restoring it.

Full suite: test_overlord_sweep.py (102 tests) and
test_parallel_model_fix_loop.py (240 tests), all green.

Cleanup

While verifying this, closed 8 confirmed-duplicate PRs
(#680, #684-#690) as md5-identical to a newer surviving PR
(#682 and #692 respectively) — see their close comments.

…ouched

Follow-up to #691. Measured 2026-08-11 in production: PR #692 duplicated
#690 (byte-identical diffs, md5-verified) -- the #691 fix's own
full-tree comparison missed it, because #690's branch was cut BEFORE
#691 itself landed on origin/main, so it legitimately lacks the
scripts/ changes #691 added. #692's branch, cut after #691 merged,
carries those changes. A raw `diff --quiet <candidate>..HEAD` sees
that unrelated drift as "different" and can never again match #690,
no matter how many more times the same tag gap gets re-solved
identically -- and the same defeat recurs for every future round
whenever ANY unrelated commit lands on main while a sweep PR sits
open, which given "the fleet publishes, it never merges" is routine.

Fix: scope the comparison to `git diff --name-only origin_ref..HEAD`
-- the paths THIS round's own squad merges actually touched -- instead
of a full-tree diff. That isolates the tag-fix content from incidental
history the two branches don't share.

New regression test reproduces the exact #692-vs-#690 shape: an
unrelated commit lands on main between when the open PR's branch was
cut and when the new round's branch is cut, with the identical tag fix
on both. Confirmed the test fails against the unversioned (pre-fix)
code and passes with it. Full suite: 102 + 240 tests green.
@swackhamer
swackhamer force-pushed the fix/dedupe-sweep-prs-against-open-prs branch from e2178d4 to 5c7e1f1 Compare August 11, 2026 19:29
@swackhamer
swackhamer merged commit 7c2ad6e into main Aug 11, 2026
10 checks passed
@swackhamer
swackhamer deleted the fix/dedupe-sweep-prs-against-open-prs branch August 11, 2026 19:32
swackhamer added a commit that referenced this pull request Aug 11, 2026
…695)

Second follow-up to #691/#693. Measured 2026-08-11 in production: PR
#694 duplicated #692 (byte-identical diffs, md5-verified) despite
#693's path-scoped duplicate check. The scoped diff DID find a
difference -- but it was pure rustfmt whitespace (a 3-line match arm
collapsed to one line), nothing semantic. #692's branch had already
been through format_sweep_branch's fmt-and-commit step in its own
round; this round's comparison ran on raw, pre-fmt worker output,
because both idempotency checks (origin_ref and the open-PR one) are
commit-to-commit diffs that only ever compare what THIS round has
committed so far -- which, at that point in run_sweep, has never been
through fmt. Unformatted content can never tree-match a PR that
already went through fmt, no matter how many times the same gap gets
re-solved with functionally identical code.

Fix: run format_sweep_branch once, early -- right before the
origin_ref zero-delta check -- so both idempotency checks compare
already-formatted content. The later format_sweep_branch call (after
the evidence table / judgment queue are built, where it has always
lived) is usually now a no-op ("already cargo-fmt clean"); kept in
place because commits_contributed resolves each squad's contribution
from merge_infos boundaries recorded before any fmt call, so an early
fmt commit can never leak into the evidence table regardless of when
it happens -- but the evidence table's own construction still must not
move earlier than it already is.

New regression test reproduces the exact #694-vs-#692 shape: the
already-open PR's branch carries the fmt-normalized form of a fix,
this round's squad commit carries the identical fix pre-fmt. Confirmed
the test fails against the pre-fix code and passes with this fix.
One existing assertion (which of the two format_sweep_branch calls
reports "committed") updated to match: the commit now happens at the
earlier call, so the later one correctly reports false.

Full suite: 103 tests green.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant