Skip to content

One data declaration authored twice: main's floor lane is red for every PR - #9500

Closed
gunbai-bot[bot] wants to merge 1 commit into
mainfrom
session/merry-bear-547-dup-decl
Closed

gunbai-bot[bot] wants to merge 1 commit into
mainfrom
session/merry-bear-547-dup-decl

Conversation

@gunbai-bot

@gunbai-bot gunbai-bot Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

dag/gunbc/fleet_fan_wiring_witness.dag declares data srv3_wiring_with_a_duplicated_header at both line 447 and line 754. The two blocks are byte-identical, seven lines each — a copy, not two intents needing reconciliation.

error: duplicate declaration 'srv3_wiring_with_a_duplicated_header'
       in module 'gunbc.fleet_fan_wiring_witness'

Discriminator, both directions, same entry

blocking advisory
main as it stands 1 283
with this deletion 0 283

Identical advisory count, so the diff removes the duplicate and nothing else. The three remaining references resolve against the surviving declaration.

Why this is not one PR's problem

The required floor resolves the whole corpus, so a duplicate declaration anywhere refuses the phase for every open PR. Measured on #9456: floor lane failed after 34m51s on this error plus the product.fabric.contention break — neither of which that PR touches. Every lane is currently paying ~35 minutes to fail on someone else's break.

The other break is separately owned and does not cover this one

Main is red on two independent errors:

  1. product.fabric.contention imports grant_duration_seconds, which dag/product/fabric/supply.dag stopped declaring in b20ac7ad44f and now mentions only in a // comment — plus two non-exhaustive matches (UnobservedGrantDuration, QuoteNotPriceableWithoutDuration). The unobserved-duration trigger fires: contention refuses to rank rather than substituting a worst case #9488 fixes this, verified: 0 references remain on its head.
  2. This duplicate declaration. The unobserved-duration trigger fires: contention refuses to rank rather than substituting a worst case #9488 does not touch this file — still 2 declarations on its head — so main stays red on this error after it merges.

Both are needed. Verified by compiling dag/product/fabric/contention.dag directly: 5 blocking errors from main's own content.

Provenance

Introduced by 868a509a80e — "Consolidate eight approved fabric/fleet PRs into one, and repair the two breaks the merge made". The consolidation repaired two breaks and introduced a third. That is the ordinary hazard of hand-resolving a large multi-PR merge, and the reason this fix states its discriminator rather than asserting the deletion is self-evidently safe.

…ry PR

dag/gunbc/fleet_fan_wiring_witness.dag declares
`data srv3_wiring_with_a_duplicated_header` at BOTH line 447 and line 754. The
two blocks are byte-identical, seven lines each, so this is a copy rather than
two intents that need reconciling. The compiler refuses:

  duplicate declaration 'srv3_wiring_with_a_duplicated_header' in module
  'gunbc.fleet_fan_wiring_witness'

MEASURED DISCRIMINATOR, both directions, on the same entry:

  main as it stands   1 blocking error, 283 advisories
  with this deletion  0 blocking errors, 283 advisories

Same advisory count both ways, so the diff removes the duplicate and nothing
else. The three remaining references still resolve against the surviving
declaration.

WHY IT IS WORTH A DEDICATED CUT: this is not one PR's problem. The required
floor resolves the whole corpus, so a duplicate declaration anywhere refuses
the phase for EVERY open PR -- measured on gunbc#9456, whose floor lane failed
after 34m51s on this error and on the product.fabric.contention break, neither
of which that PR touches. Every lane is currently paying ~35 minutes to fail on
someone else's break.

The second break is separately owned and already in flight: gunbc#9488 removes
contention's reference to `grant_duration_seconds`, which
`dag/product/fabric/supply.dag` stopped declaring in b20ac7a and now
mentions only in a comment. That PR does NOT touch this file, so main stays red
on this error after it merges. Both are needed.

PROVENANCE: introduced by 868a509, "Consolidate eight approved fabric/fleet
PRs into one, and repair the two breaks the merge made" -- the consolidation
repaired two breaks and introduced a third. That is the ordinary hazard of
hand-resolving a large multi-PR merge, and the reason this fix states its
discriminator rather than asserting the deletion is obviously safe.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gunbai-bot

gunbai-bot Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Predicted result, recorded before the run finishes so it is a prediction and not an explanation afterwards: this PR's own floor lane will FAIL, and that is expected.

Main is red on two independent breaks. This PR fixes one of them. Until #9488 merges, the floor phase will still refuse on the other:

dag/product/fabric/contention.dag:23   name 'grant_duration_seconds' not found in module 'product.fabric.supply'
dag/product/fabric/contention.dag:280  non-exhaustive match: missing variant(s) UnobservedGrantDuration
dag/product/fabric/contention.dag:421  non-exhaustive match: missing variant(s) QuoteNotPriceableWithoutDuration
dag/product/fabric/contention.dag:463  function 'grant_duration_seconds' not found in scope
dag/product/fabric/contention.dag:469  function 'grant_duration_seconds' not found in scope

What should change is that fleet_fan_wiring_witness.dag:754 duplicate declaration disappears from the floor output. That single line is this PR's entire discriminating claim.

What should not change is the five contention.dag errors above — they are #9488's, and if they vanish here something is wrong with my reading, not with main.

So the honest reading of this PR's CI is: red, with exactly five remaining errors and the duplicate-declaration line gone. A green run would be the surprising outcome and would mean #9488 landed in between.

I am recording this now because a red check on a PR whose stated purpose is "unbreak main" invites either a retry loop or the conclusion that the fix does not work. Neither is right. The evidence for the fix is the two-directional compile discriminator in the description — 1 blocking error on main, 0 with the deletion, 283 advisories both ways — not this run's exit status.

— sent from merry-bear-547

@gunbai-bot

gunbai-bot Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Closing. #9497 landed the same repair while this was open, and did it better — and merging this now would actively remove content.

9d9c12c04b5 — "Delete the byte-identical second declaration of srv3_wiring_with_a_duplicated_header, keeping both authored rationales" — is on main. The duplicate is gone; that file now has exactly one declaration.

Why this must be closed rather than merged

This PR is still MERGEABLE and carries an APPROVE (review 56890), and it is now wrong. Its remaining diff against the new main is a 3-line deletion:

-// SRV3 HAS NO FAN2 ROW AT ALL, so prepending one there would have made an UNEXPECTED header
-// rather than a duplicated one -- a distinction the coverage query reports correctly and which a
-// first draft of this control got wrong. Duplicating an EXISTING header is what reaches the arm.

That is authored rationale #9497 deliberately preserved by folding both copies' comments together. My version deleted the duplicate's comment along with its data block. #9497's resolution is strictly better — it kept the reasoning from both copies and I discarded half of it.

The part worth keeping from this

An approval attaches to a diff, but a diff's effect is relative to a base that moves. Review 56890 correctly assessed "surgical delete of a duplicated data declaration" — accurate when written. Once main gained the same repair, the identical branch became a rationale deletion without a single byte changing on it. The tally (approved, mergeable, no request-changes) would have been fully satisfied by a merge that lost content.

So the check that mattered was not the review state but re-reading the diff against the current base before acting on it.

Main is still red on the other break

Break 1 is unfixed: compiling dag/product/fabric/contention.dag against current main still gives 5 blocking errors (grant_duration_seconds absent from product.fabric.supply, two non-exhaustive matches, two out-of-scope calls). #9488 is the fix and is open and mergeable. Every open PR's floor lane stays red until it lands.

— sent from merry-bear-547

@gunbai-bot gunbai-bot Bot closed this Aug 27, 2026
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.

0 participants