Skip to content

fix(fleet): normalize formatting before the sweep idempotency checks - #695

Merged
swackhamer merged 1 commit into
mainfrom
fix/normalize-formatting-before-sweep-idempotency-checks
Aug 11, 2026
Merged

fix(fleet): normalize formatting before the sweep idempotency checks#695
swackhamer merged 1 commit into
mainfrom
fix/normalize-formatting-before-sweep-idempotency-checks

Conversation

@swackhamer

Copy link
Copy Markdown
Collaborator

Problem

Second follow-up to #691/#693. Measured 2026-08-11 in production, right
after #693's path-scoped fix deployed: PR #694 duplicated #692
anyway (byte-identical diffs, md5-verified).

The scoped diff #693 added did find a difference — but it was pure
rustfmt whitespace (a 3-line match arm collapsed to one line),
nothing semantic:

-            "note" => metadata.insert("DjVu:Note".to_string(), TagValue::new_string(value.clone())),
+            "note" => metadata.insert(
+                "DjVu:Note".to_string(),
+                TagValue::new_string(value.clone()),
+            ),

Root cause: #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 (the origin_ref one and #693's open-PR one) are
commit-to-commit diffs, and at the point they run, this round's branch
has never been through cargo fmt yet — that historically only
happened much later, right before the push. 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 existing late call (after the evidence
table / judgment queue are built) is now usually 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 — but building the evidence table itself must not move
earlier than it already is.

Testing

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 (verified via stash-and-rerun). One existing assertion (which of
the two format_sweep_branch calls reports "committed") updated to
match the new timing.

Full suite: test_overlord_sweep.py, 103 tests, all green.

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.
@swackhamer
swackhamer merged commit c874e65 into main Aug 11, 2026
10 checks passed
@swackhamer
swackhamer deleted the fix/normalize-formatting-before-sweep-idempotency-checks branch August 11, 2026 21:50
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