Skip to content

fix(qwen3_5): remove the duplicated TestSanitizeConvWeight; main does not build - #182

Merged
glennneuber merged 1 commit into
mainfrom
fix/qwen35-duplicate-test
Aug 17, 2026
Merged

glennneuber merged 1 commit into
mainfrom
fix/qwen35-duplicate-test

Conversation

@glennneuber

Copy link
Copy Markdown

main is red and has been all day

x/models/qwen3_5/qwen3_5_test.go:43:6: TestSanitizeConvWeight redeclared in this block
    x/models/qwen3_5/qwen3_5_test.go:21:6: other declaration of TestSanitizeConvWeight

The package fails to build, so go test ./... fails on ubuntu, macos and windows, and main's last three CI runs are all red — 9594f81e, ade6362a, bc11eab5. It came in with the upstream merge (#165 touched this file).

The two copies are byte-identical

Verified by diffing the ranges before deleting anything:

diff <(sed -n '21,41p' file) <(sed -n '43,63p' file)   ->   no differences

That distinction decides the fix. A duplicate is resolved by deleting one; two different tests colliding on a name must be resolved by renaming, or coverage is lost. This is the first, so the second copy is removed and nothing is renamed.

Why it sat unnoticed

The test job is gated behind the changes filter. Today's work was mostly documentation and harness changes, which reported CLEAN while main was broken — the Go tests never ran for them. The first two PRs to touch Go (#180, #181) inherited the failure and looked like they had caused it.

Worth considering separately: a red main that only surfaces on Go-touching PRs is a gap the changes gate creates. A scheduled run of test against main would catch it regardless of what a given PR touches.

Verification

go vet  ./x/models/qwen3_5   clean
go test ./x/models/qwen3_5   ok

Unblocks #180 and #181, whose only failing check is this.

🤖 Generated with Claude Code

… not build

x/models/qwen3_5/qwen3_5_test.go declares TestSanitizeConvWeight twice, at
line 21 and again at line 43:

    x/models/qwen3_5/qwen3_5_test.go:43:6: TestSanitizeConvWeight redeclared in this block
        x/models/qwen3_5/qwen3_5_test.go:21:6: other declaration of TestSanitizeConvWeight

The package therefore fails to build, `go test ./...` fails on ubuntu, macos and
windows, and main's last three CI runs are all red (9594f81, ade6362,
bc11eab). It arrived with the upstream merge in #165, which touched this file.

The two copies are BYTE-IDENTICAL -- verified by diffing the ranges before
deleting anything, because "duplicate name" and "two different tests that
collide" need opposite fixes and only the first can be resolved by deletion. So
the second copy is removed and nothing is renamed: no coverage is lost.

WHY THIS SAT UNNOTICED ALL DAY. The `test` job is gated behind the `changes`
filter, so the documentation and harness PRs that made up most of today's work
reported CLEAN while main was broken. Only a PR touching Go triggers it, and the
first two that did (#180, #181) inherited the failure and looked like they had
caused it.

go vet ./x/models/qwen3_5: clean. go test ./x/models/qwen3_5: ok.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@glennneuber

Copy link
Copy Markdown
Author

Confirmed and reproduced — this is correct and should land now.

vet: x/models/qwen3_5/qwen3_5_test.go:43:6: TestSanitizeConvWeight redeclared in this block

TestSanitizeConvWeight is declared at both line 21 and line 43 on main, so the package does not compile and go test ./... fails for the whole tree, not just this package.

Where it came from, and the part worth fixing

e8b7178c chore: merge upstream v0.32.14 added 22 lines to that file — a merge resolution that kept both copies of a test upstream had moved.

CI caught it. The PR was merged anyway:

#165 merged 2026-08-17T05:24:30Z
  test (ubuntu-latest):  FAILURE
  test (macos-latest):   CANCELLED
  test (windows-latest): CANCELLED
  patches / patches-ggml / go_mod_tidy / expectations: SUCCESS

The test job is one of the few that is NOT gated behind vars.SELF_HOSTED_RUNNERS — it runs on hosted runners and did its job. It was overridden.

So main has been unbuildable for roughly fourteen hours, across a dozen merged PRs. Everything merged since — #168, #172, #173, #176, #177, #178 — was reviewed and merged against a base whose tests could not run. Their own green checks were green on the checks that did run, not on go test.

What I would add on top of this PR

This is the second time today a control existed and did not bind. #137 landed patches-ggml after finding that 903 "was application-checked but never compiled", with the reasoning: "Coverage that only exists when a runner happens to be present is not coverage." The same sentence applies to a required check that can be clicked past.

Two cheap follow-ups, neither belonging in this PR:

  1. Make test a required status check on main in branch protection, so a FAILURE blocks rather than warns. Nothing in the repo can enforce this; it is a settings change.
  2. A merge-resolution guard for upstream syncs. This class — a sync keeping both copies of a moved symbol — is invisible to review because the diff looks like an addition. go vet ./... on the sync branch would have caught it in seconds.

Happy to open either. Meanwhile I would merge this ahead of every other open PR, including mine, since all of them are currently being evaluated against a base that does not build.

@glennneuber
glennneuber merged commit 7d776b8 into main Aug 17, 2026
11 checks passed
@glennneuber
glennneuber deleted the fix/qwen35-duplicate-test branch August 17, 2026 14:01
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