cmd/gc fails golangci-lint on main with 3 issues, so lint-affected reds every PR that touches the package (gc-pypo9) by zook-bot · Pull Request #172 · zookanalytics/gascity

zook-bot · 2026-09-03T23:21:19Z

Summary

golangci-lint run ./cmd/gc reported three issues on main, so every PR
touching the package inherited a red lint-affected job that had nothing to do
with its own diff. The pre-commit hook did not surface them: .githooks/pre-commit
runs make lint-changed with LINT_FLAGS="--new-from-rev=HEAD --whole-files --fix",
and --new-from-rev=HEAD reports only issues new relative to HEAD, so findings
already committed at HEAD stay invisible. CI's lint-affected passes the empty
default LINT_FLAGS (Makefile:309) and so reports every issue in each affected
package. A green hook was therefore not evidence the package was green, and the
findings first appeared in the PR's CI job.

This clears the three findings at their source rather than adding exclusions.
.golangci.yml's exclusion list is entirely revive var-naming entries for
established package names, which are structural naming facts; an unparam
exclusion keyed to two signatures would be new precedent and would hide the
next genuinely dead parameter in those files.

The three fixes:

misspell — session_beads.go doc comment: favour to favor.
unparam — workAssignmentStores's variadic extra ...beads.Store leg was
never passed by any caller. The variadic is dropped. That leg carried the
binding store before S2; upstream 27e2f670c moved the responsibility to the
storeref resolver, and assigned_work_residency_test.go pins the resolver as
the thing that adds the binding. Dropping it also retires
workAssignmentStoresHave, whose only caller was the extra-dedup loop.
unparam — orderRunsStoreForScoped's count parameter was 3 at both
call sites. It is now the named constant orderScopedTestRunCount, which the
assertions also read, so the seeded count and the expected totals can no
longer drift apart.

A doc comment in assigned_work_residency_test.go cited the old
workAssignmentStores(rigStore, nil, binding) arity. It now states the leg
set and order in present tense against assignedWorkPlanForSessionInfo, the
resolver those tests exercise, naming no removed function.

Verification

Same pinned invocation that reproduced the baseline, on the same host:

GOTOOLCHAIN=go1.26.6+auto GOFLAGS=-mod=readonly golangci-lint run ./cmd/gc

origin/main (f8f4ee8): 3 issues (misspell 1, unparam 2).
This branch: 0 issues.

Non-regression checks on the two removals: storePointerKey keeps three live
callers in api_state.go after workAssignmentStoresHave was retired, so no
new unused finding; and every workAssignmentStores call site now passes
exactly two arguments (session_work_guard.go twice, session_beads.go four
times, plus the release-store tests).

Dispatch — what this work was asked to do

What

golangci-lint run ./cmd/gc reports three issues against an unmodified
origin/main checkout:

cmd/gc/session_beads.go:3254:36: `favour` is a misspelling of `favor` (misspell)
cmd/gc/session_beads.go:3268:80: workAssignmentStores - extra always receives nil (unparam)
cmd/gc/cmd_order_history_store_completeness_test.go:25:77: orderRunsStoreForScoped - count always receives 3 (unparam)

Why it reds unrelated PRs

CI's PR path runs make lint-affected (ci.yml, Lint affected packages,
gated on scope == 'changed'). scripts/ci-static-select resolves changed
files to whole packages and runs the linter on those package arguments — see
lint_affected calling run_static_checks(..., packages). There is no
--new-from-merge-base filter on that path, unlike the separate lint-new
target.

So any PR that changes a single file under cmd/gc, including a test-only
change, lints the whole package and inherits all three findings. The job goes
red for code the PR never touched, and the natural reading is that the PR
broke lint.

Provenance

All three are on origin/main today and arrived with ordinary feature work:

favour and the extra ...beads.Store variadic came in 4687504b2
("fix(session): release work in every reachable store when a session bead
closes (gc-d9qnh)").
orderRunsStoreForScoped came in 64253325e ("fix(order): make a bounded
order-history read store-complete (gc-6a6vz) (gc order history: a bounded read is not store-complete — any positive --limit answers for the city store alone while printing a RIG column, and it cost a P1 on the refinery merge cadence (gc-6a6vz) #155)").

.golangci.yml carries no exclusion covering any of them; misspell is set
to locale: US and unparam is in the enabled set.

Why it was not caught at the time

Reproducing it locally needs a GOROOT workaround, because the pinned linter
cannot typecheck the host stdlib on this box — that blocker is [[gc-m7rp0]].
With GOROOT pointed at the go.mod toolchain the three issues are stable and
reproducible.

Fix candidates

Fix the three findings directly. The misspell is a comment word. The two
unparam findings are signatures whose extra parameter has exactly one
caller-supplied value; workAssignmentStores is a fork-local seam carrying
a residency:allow annotation, so dropping its variadic wants a look at
whether the fork intends to grow a second caller.
Add narrowly scoped .golangci.yml exclusions if either signature is
deliberately wider than its current callers.

Either way the package should be green before the next cmd/gc PR, otherwise
every one of them arrives with a red lint job that has to be explained away.

Found while self-reviewing gc-82y1l, a test-only change under cmd/gc.

Refinery handoff

Issue: gc-pypo9
Source branch: polecat/gc-pypo9
Target: main
Gates codex signed off pre-open at 4a2bce2e; PR opened green.

…(gc-pypo9) `golangci-lint run ./cmd/gc` reported three issues against an unmodified origin/main, so every PR touching the package inherited them. CI's PR path runs `make lint-affected`, which resolves changed files to whole packages and lints those packages with no `--new-from-merge-base` filter, so a one-file change red the job for code it never touched. The pre-commit hook missed it because it passes `--new-from-rev=HEAD`, which reports only what the staged diff introduces. misspell: `favour` -> `favor` in a workAssignmentStores doc comment; the linter is configured `locale: US`. unparam, workAssignmentStores: the `extra ...beads.Store` variadic has no caller. It carried the binding store before upstream 27e2f67 moved that responsibility to the storeref resolver, which assigned_work_residency_test.go pins as the thing that adds the binding leg (TestAssignedWorkSweepPlanAddsTheRealBindingToAWorkLedScan). Every call site in the fork passes two arguments, so the parameter goes, and with it workAssignmentStoresHave, whose only caller was the extra-dedup loop. Rig-store legs were never deduped against the leading store, so the legs that remain are unchanged. unparam, orderRunsStoreForScoped: `count` always receives 3. Named as a package constant, following orderHistoryRunsStore in the sibling cmd_order_history_bounds_test.go, which seeds from orderHistoryTestRunCount and asserts against the same constant. The fixture-count assertions now read orderScopedTestRunCount instead of repeating 3 and 6. Fixed the findings rather than adding .golangci.yml exclusions: the exclusion list is entirely revive var-naming/stutters entries for established package names, and a unparam exclusion keyed to two signatures would hide the next genuinely dead parameter in those files. Validation: `golangci-lint run ./cmd/gc` under the pinned toolchain goes from `3 issues: misspell 1, unparam 2` to `0 issues.`; `go build`, `go vet ./cmd/gc`, and `scripts/check-residency-boundary.sh` pass (the deleted lines carried `residency:allow` markers the census skips, so the baseline is unchanged); TestCloseBead*, TestReleaseWorkFromClosedSessionBead*, TestCloseSessionBeadIfReachableStoreUnassigned*, TestAssignedWorkSweepPlan*, TestSessionAssignedWorkPlan*, TestOrderHistoryBoundedRead* and TestResidencyResolverBoundary all pass. The reworked store-completeness assertion was checked against an inverted fixture (rig-b resolving to no store): it fails with `entries = 3, want 6`, so coupling the expectation to the seed constant did not make it vacuous. Claude-Session: https://claude.ai/code/session_01W7BYK3WcDaBtDtr3t8Gyfs

…res arity (gc-pypo9) Self-review catch: dropping the `extra ...beads.Store` variadic in the parent commit left assigned_work_residency_test.go:159 describing the session-scoped plan as "byte-identical to the pre-S2 workAssignmentStores(rigStore, nil, binding)" — a three-argument call that no longer exists in any form. A git grep for `workAssignmentStores(` against origin/main returns exactly one three-argument occurrence, and it is that comment; every real call site already passed two. So the line was the last reference to the retired signature rather than evidence of a caller the parent commit missed. The replacement states the same claim — the leg set and the ordering the plan must reproduce — without naming a signature. It leans on the framing the section header two lines above already establishes, so it introduces no reference the file did not already carry. Comment-only; no behaviour change. golangci-lint run ./cmd/gc ./test/tmuxtest reports `0 issues.`, go vet ./cmd/gc, scripts/check-residency-boundary.sh and go test ./scripts -run TestResidencyResolverBoundary all pass, and the 69 targeted cmd/gc tests covering the touched helpers pass with no failures. Claude-Session: https://claude.ai/code/session_01RWkzRawZZckeY1KqWd3a7z

…r (gc-pypo9) The self-review follow-up in caa8963 cleared one dead reference and introduced another. The section header and body comment above TestSessionAssignedWorkPlanStaysRigScopedAndAddsTheBinding both named reachableStoresForSessionInfo, a function that no longer exists anywhere in the tree. A living comment states what is true now, so it names the resolver the tests actually exercise. assigned_work_residency_test.go now describes the session-scoped plan in present tense against assignedWorkPlanForSessionInfo (assigned_work_residency.go:84): a rig-bound session's plan reads its rig store and the binding, in that order, and the city work store does not appear. Comment-only; no behaviour change. Claude-Session: https://claude.ai/code/session_019VKHLFj6EXHLRPpjqn3RnY

zook-bot · 2026-09-03T23:21:32Z

Pre-open signoff (comment-only — not an approval):

VERDICT: approve
Reviewed branch: polecat/gc-pypo9
Reviewed base: main
Reviewed commit: 4a2bce2

Scope checked: Read the full three-dot diff from origin/main to the reviewed commit across cmd/gc/assigned_work_residency_test.go, cmd/gc/cmd_order_history_store_completeness_test.go, and cmd/gc/session_beads.go. Checked anchor bead gc-pypo9, review bead gc-qf74t, prior review bead gc-zc8uw, rework bead gc-rdi57, the three branch commit messages, workAssignmentStores and all its call sites, workAssignmentStoresHave references, orderRunsStoreForScoped references, storePointerKey callers, assignedWorkPlanForSessionInfo, the nearby close-release tests, scripts/check-residency-boundary.sh, the residency boundary pattern file, and the pack work-quality and learning-exemplar fragments. This is pre-open, so no PR page exists to review.

Findings: none.

Filed: gc-ni9y4 for an out-of-scope stale comment in cmd/gc/session_beads_close_release_store_test.go:243. That file is not changed by this branch.

Verification: In a detached worktree at the reviewed commit, gofmt -l cmd/gc/assigned_work_residency_test.go cmd/gc/cmd_order_history_store_completeness_test.go cmd/gc/session_beads.go printed no files, and git diff --check origin/main...HEAD passed. go vet ./cmd/gc passed. ./scripts/check-residency-boundary.sh passed. The targeted go test -count=1 -v ./cmd/gc -run 'Test(OrderHistoryBoundedRead(IsStoreCompleteAcrossRigs|KeepsNewestAcrossRigs)|SessionAssignedWorkPlan(StaysRigScopedAndAddsTheBinding|StaysSingleLeggedOnASingleStoreCity)|AssignedWorkSweepPlan(IsByteIdenticalOnASingleStoreCity|CollapsesALeadingBindingLeg|AddsTheRealBindingToAWorkLedScan|FailsLoudOnARefusedCity)|CloseBeadReleases(InProgressWorkInRigStore|WorkAcrossEveryRigStore|SameBeadIDInTwoStores)|CloseBeadWithoutRigStoresReleasesCityWorkUnchanged|ReleaseWorkFromClosedSessionBead(SkipsRigSessionBeads|ToleratesNilRigStoreEntry))$' ran 14 tests, all passed. Targeted golangci-lint run --allow-parallel-runners --enable-only=misspell,unparam ./cmd/gc with isolated caches reported 0 issues. A full golangci-lint run ./cmd/gc did not complete in this session: the first attempt failed on shared Go-cache export-data churn after quota warnings, and the isolated retry stayed in go list for more than eight minutes before I stopped it.

Anchor: gc-pypo9 — check.codex @ 4a2bce2

zook-bot

VERDICT: approve
Reviewed branch: polecat/gc-pypo9
Reviewed base: main
Reviewed commit: e257d90

Scope checked: Read the full three-dot diff from origin/main to the reviewed commit across cmd/gc/assigned_work_residency_test.go, cmd/gc/cmd_order_history_store_completeness_test.go, and cmd/gc/session_beads.go. Checked anchor bead gc-pypo9, review bead gc-tpae8, PR #172 title, body, comments, commits, files, current head OID, and status rollup. Checked workAssignmentStores and all its call sites at the pinned commit, absence of workAssignmentStoresHave, live storePointerKey callers, orderRunsStoreForScoped references, orderScopedTestRunCount assertions, assignedWorkPlanForSessionInfo references, the nearby close-release tests, and the pack work-quality and learning-exemplar fragments. Read /home/zook/loomington/rigs/gc-toolkit/assets/scripts/pr-open.sh where the published PR body looked malformed.

Findings: none.

Filed: gc-ni9y4 for an existing out-of-scope stale comment in cmd/gc/session_beads_close_release_store_test.go:243. gc-yb75x for the PR #172 CI failure in examples/bd/dolt sync concurrency tests where sync exits before reaching the expected push point. gc-lvi11 for the PR #172 CI failure in internal/bdflags where the bd flag manifest is stale for the installed bd v1.2.1 flags. gc-xd9ak for the PR #172 CI failure in TestManagedBdRigProviderStoreRecoversAfterHardKillPortRebind where bd init rejects a legacy Dolt workspace before the rebind behavior runs. gc-gjy04 for the duplicated PR #172 Summary heading; pr-open.sh writes a Summary heading and this anchor's pr_summary already contained one. All five are outside the reviewed diff.

Verification: In a detached worktree at the reviewed commit, git diff --check origin/main...e257d90610c90bd0f73d2eaa66a327e6d491de3c passed, and gofmt -l cmd/gc/assigned_work_residency_test.go cmd/gc/cmd_order_history_store_completeness_test.go cmd/gc/session_beads.go printed no files. go vet ./cmd/gc passed. golangci-lint run ./cmd/gc with isolated temp and lint cache reported 0 issues. Targeted go test -count=1 -v -timeout 10m ./cmd/gc -run 'Test(SessionAssignedWorkPlan|RetiredSessionSweep|CloseBead|ReleaseWorkFromClosedSessionBead|CloseSessionBeadIfReachableStoreUnassigned|OrderHistoryBoundedRead)' ran 34 tests and all passed. The first two local test attempts failed before reaching cmd/gc because tmpfs was quota-exhausted under /tmp; the passing run set TMPDIR and GOTMPDIR under /var/tmp/gotmp. I did not rerun the full project test matrix locally. Current PR CI at the reviewed head has failed required jobs, and the failure lines I inspected are the filed out-of-scope issues above.

Anchor: gc-pypo9 — check.codex @ e257d90

…ad closes (gc-d9qnh) closeSessionBeadIfReachableStoreUnassigned proved "no assigned work" across the session's reachable store set but then handed closeBead a single store, so work assigned in a rig store was never released when the session bead closed. The assignee stayed stamped and the bead stayed unclaimable. Carry the exact scope the gate proved into the release: closeBead now takes the reachable store slice rather than one store. Releasing beyond that slice would mutate work the gate never judged — beads in unreachable stores may be unrelated and merely share an assignment token. Merged with upstream gastownhall#4765's excludeOwnDrainStep: the flag now selects the per-store probe inside reachableAssignedWorkScope, so the proof and the release read one store slice. Only the per-store probe varies; the shared sessionHasOpenAssignedWorkForTier / sessionHasOpenAssignedWispWork helpers are untouched, so the awake-work chain still counts the drain step. Rebased 2026-09-15 onto upstream 83c8270: upstream 6ab009c narrows the pool-freeable gate with !isNamedSessionInfo (kept alongside the reachable scope carried to the close), and upstream 3a09f0d (gastownhall#5456) added reapPreBootSessionBeads with a single-store closeBead call; it now takes the rig-store map and fans out through workAssignmentStores like the sibling stale-session and dead-runtime reapers. Claude-Session: https://claude.ai/code/session_01XfZfLDuwcBfs3TXnn1udn9 chore(residency): annotate the fork's release-union sites for the boundary guard (gc-p6xab) Upstream retired `workAssignmentStores` in favour of the internal/storeref resolver, and added a SHRINK-ONLY residency ratchet that refuses new store-enumeration sites. The fork carries gc-d9qnh, whose whole point is that a closing session's work must be released in every reachable store -- so the sync lands eleven sites the ratchet reads as new blind readers. They are not blind readers, and the baseline is deliberately not touched (growing a SHRINK-ONLY census would bank this as unreviewed). Each site is annotated `residency:allow` with its own reason, in two families: * `assignedWorkExistsForSession` / `reachableAssignedWorkScope` DO consume the resolver -- they build the plan and walk it fail-closed. They additionally RETURN the legs walked, which is what lets the close gate release in exactly the scope it proved empty. The hit is on the signature, not on a bypass. * `workAssignmentStores` and its six callers hold a RAW BEAD rather than a session Info, so there is no plan to resolve, and they close for reasons (stale-session, dead-runtime, stranded repair) that are not gated on a proven-empty scope. With no proven scope to release into, they release across the whole reachable union. The distinction matters: releasing beyond a scope the gate proved would mutate work the check never covered, which is the failure gc-d9qnh exists to prevent. Co-authored-by: refinery costing <refinery@local> Lint (gc-pypo9, #172): drop the unused `extra ...beads.Store` variadic and the unused `workAssignmentStoresHave` helper from the release-union site, and fix the "favour" spelling in its doc comment, so cmd/gc passes golangci-lint on main.

…z) (#155) `gc order history <name>` was store-complete only with `--limit 0`. Any positive `--limit` -- including the default 50, and including a limit far larger than the number of retained runs -- answered from a single store while still rendering a RIG column, so a one-rig answer was indistinguishable from a city-wide one. Root cause is in the routing, not in the read. routeOrderHistory sends a single-order bounded query to the supervisor API, and that request carries one `scoped_name`. But a rig-scoped order is registered once per importing rig, so a bare name with no `--rig` names N registrations at once. orderScopedName resolves it through findOrder, which returns the FIRST match -- so the API was asked about one rig and the other N-1 were dropped silently. Because the city store is the mayor rig's store, the answer was always that rig, which reads as "the one rig that is working" rather than "the only rig I looked at". That under-report has already cost a P1: gc-toolkit bead tk-fdstg was filed at severity 1 reporting that the refinery-reconcile order had never fired on gc-toolkit, when it had in fact been firing in lockstep with the other three rigs since the order's first tick. The reporter explicitly flagged `gc order history` as unusable there -- it returned only gascity rows even at `--limit 40` -- and still reached the opposite of the truth, because no reachable surface would answer per-rig. The fix stays on the local iterator whenever the name resolves to more than one registration, alongside the existing multi-order and unlimited fallbacks and using the same `logRoute(... "fallback", ...)` idiom. That iterator was already correct: it walks every matching registration, merges newest-first, and only then applies the bound, so `--limit N` means "the N most recent runs in the city" rather than "the N most recent runs in whichever store I read first". No change was needed in the read path itself. Deliberately narrow. A rig-qualified read (`--rig <name>`) still resolves to exactly one registration and keeps the API route, so this does not push every bounded read back onto the slower local scan; a city-scoped order has a single registration and is likewise unaffected. The per-store reads stay bounded by `--limit`, so the fan-out costs N bounded reads, not the unbounded scan the help text warns about. The routing tests pass a nil API client and assert on the route= line that logRoute emits before any request is built, rather than standing up an httptest server. The decision is fully observable from that line, and the untagged http_test_server census is a "cannot grow" ratchet (TESTING.md, Small and Source debt ratchets, ga-80po0c.2.2) -- an earlier draft using two real listeners pushed it to 319 calls / 67 files against a 317 / 66 baseline. Spending that ceiling to observe what stderr already reports would have been a poor trade, so the baseline is left untouched rather than raised. Validation: four new tests in cmd_order_history_store_completeness_test.go. TestRouteOrderHistoryBoundedStaysLocalWhenNameSpansRigs is the regression -- it fails before this change with "bounded read of a name spanning 2 rigs was routed to the API". TestRouteOrderHistoryBoundedUsesAPIWhenRigQualified guards the other side so the API route is not lost. The remaining two pin the bead's stated acceptance: with a limit larger than the total row count every rig is represented, and with a limit smaller than it the rows kept are the newest across all stores rather than the newest of one. Existing `-run Order` (11.5s) and `-run Doctor` (67.2s) suites in cmd/gc stay green, as do go build ./..., go vet, and ./internal/testpolicy/... (the census). Not changed, noted for follow-up: `--since` is applied after the fetch rather than pushed into the per-store query (`--rig` genuinely is pushed down, by filtering registrations before any store is opened). With the fan-out corrected, pushing `--since` down is what would keep an unbounded-shaped read cheap; that is a separate performance change and carries its own risk, so it is left out of this fix. Co-authored-by: refinery costing <refinery@local> Lint (gc-pypo9, #172): rename the unparam `count` helper to `orderScopedTestRunCount` in the store-completeness test so cmd/gc passes golangci-lint on main.

…ad closes (gc-d9qnh) closeSessionBeadIfReachableStoreUnassigned proved "no assigned work" across the session's reachable store set but then handed closeBead a single store, so work assigned in a rig store was never released when the session bead closed. The assignee stayed stamped and the bead stayed unclaimable. Carry the exact scope the gate proved into the release: closeBead now takes the reachable store slice rather than one store. Releasing beyond that slice would mutate work the gate never judged — beads in unreachable stores may be unrelated and merely share an assignment token. Merged with upstream gastownhall#4765's excludeOwnDrainStep: the flag now selects the per-store probe inside reachableAssignedWorkScope, so the proof and the release read one store slice. Only the per-store probe varies; the shared sessionHasOpenAssignedWorkForTier / sessionHasOpenAssignedWispWork helpers are untouched, so the awake-work chain still counts the drain step. Rebased 2026-09-15 onto upstream 83c8270: upstream 6ab009c narrows the pool-freeable gate with !isNamedSessionInfo (kept alongside the reachable scope carried to the close), and upstream 3a09f0d (gastownhall#5456) added reapPreBootSessionBeads with a single-store closeBead call; it now takes the rig-store map and fans out through workAssignmentStores like the sibling stale-session and dead-runtime reapers. Claude-Session: https://claude.ai/code/session_01XfZfLDuwcBfs3TXnn1udn9 chore(residency): annotate the fork's release-union sites for the boundary guard (gc-p6xab) Upstream retired `workAssignmentStores` in favour of the internal/storeref resolver, and added a SHRINK-ONLY residency ratchet that refuses new store-enumeration sites. The fork carries gc-d9qnh, whose whole point is that a closing session's work must be released in every reachable store -- so the sync lands eleven sites the ratchet reads as new blind readers. They are not blind readers, and the baseline is deliberately not touched (growing a SHRINK-ONLY census would bank this as unreviewed). Each site is annotated `residency:allow` with its own reason, in two families: * `assignedWorkExistsForSession` / `reachableAssignedWorkScope` DO consume the resolver -- they build the plan and walk it fail-closed. They additionally RETURN the legs walked, which is what lets the close gate release in exactly the scope it proved empty. The hit is on the signature, not on a bypass. * `workAssignmentStores` and its six callers hold a RAW BEAD rather than a session Info, so there is no plan to resolve, and they close for reasons (stale-session, dead-runtime, stranded repair) that are not gated on a proven-empty scope. With no proven scope to release into, they release across the whole reachable union. The distinction matters: releasing beyond a scope the gate proved would mutate work the check never covered, which is the failure gc-d9qnh exists to prevent. Co-authored-by: refinery costing <refinery@local> Lint (gc-pypo9, #172): drop the unused `extra ...beads.Store` variadic and the unused `workAssignmentStoresHave` helper from the release-union site, and fix the "favour" spelling in its doc comment, so cmd/gc passes golangci-lint on main.

…z) (#155) `gc order history <name>` was store-complete only with `--limit 0`. Any positive `--limit` -- including the default 50, and including a limit far larger than the number of retained runs -- answered from a single store while still rendering a RIG column, so a one-rig answer was indistinguishable from a city-wide one. Root cause is in the routing, not in the read. routeOrderHistory sends a single-order bounded query to the supervisor API, and that request carries one `scoped_name`. But a rig-scoped order is registered once per importing rig, so a bare name with no `--rig` names N registrations at once. orderScopedName resolves it through findOrder, which returns the FIRST match -- so the API was asked about one rig and the other N-1 were dropped silently. Because the city store is the mayor rig's store, the answer was always that rig, which reads as "the one rig that is working" rather than "the only rig I looked at". That under-report has already cost a P1: gc-toolkit bead tk-fdstg was filed at severity 1 reporting that the refinery-reconcile order had never fired on gc-toolkit, when it had in fact been firing in lockstep with the other three rigs since the order's first tick. The reporter explicitly flagged `gc order history` as unusable there -- it returned only gascity rows even at `--limit 40` -- and still reached the opposite of the truth, because no reachable surface would answer per-rig. The fix stays on the local iterator whenever the name resolves to more than one registration, alongside the existing multi-order and unlimited fallbacks and using the same `logRoute(... "fallback", ...)` idiom. That iterator was already correct: it walks every matching registration, merges newest-first, and only then applies the bound, so `--limit N` means "the N most recent runs in the city" rather than "the N most recent runs in whichever store I read first". No change was needed in the read path itself. Deliberately narrow. A rig-qualified read (`--rig <name>`) still resolves to exactly one registration and keeps the API route, so this does not push every bounded read back onto the slower local scan; a city-scoped order has a single registration and is likewise unaffected. The per-store reads stay bounded by `--limit`, so the fan-out costs N bounded reads, not the unbounded scan the help text warns about. The routing tests pass a nil API client and assert on the route= line that logRoute emits before any request is built, rather than standing up an httptest server. The decision is fully observable from that line, and the untagged http_test_server census is a "cannot grow" ratchet (TESTING.md, Small and Source debt ratchets, ga-80po0c.2.2) -- an earlier draft using two real listeners pushed it to 319 calls / 67 files against a 317 / 66 baseline. Spending that ceiling to observe what stderr already reports would have been a poor trade, so the baseline is left untouched rather than raised. Validation: four new tests in cmd_order_history_store_completeness_test.go. TestRouteOrderHistoryBoundedStaysLocalWhenNameSpansRigs is the regression -- it fails before this change with "bounded read of a name spanning 2 rigs was routed to the API". TestRouteOrderHistoryBoundedUsesAPIWhenRigQualified guards the other side so the API route is not lost. The remaining two pin the bead's stated acceptance: with a limit larger than the total row count every rig is represented, and with a limit smaller than it the rows kept are the newest across all stores rather than the newest of one. Existing `-run Order` (11.5s) and `-run Doctor` (67.2s) suites in cmd/gc stay green, as do go build ./..., go vet, and ./internal/testpolicy/... (the census). Not changed, noted for follow-up: `--since` is applied after the fetch rather than pushed into the per-store query (`--rig` genuinely is pushed down, by filtering registrations before any store is opened). With the fan-out corrected, pushing `--since` down is what would keep an unbounded-shaped read cheap; that is a separate performance change and carries its own risk, so it is left out of this fix. Co-authored-by: refinery costing <refinery@local> Lint (gc-pypo9, #172): rename the unparam `count` helper to `orderScopedTestRunCount` in the store-completeness test so cmd/gc passes golangci-lint on main.

refinery costing added 3 commits September 3, 2026 22:44

github-actions Bot added the status/needs-triage label Sep 3, 2026

zook-bot force-pushed the polecat/gc-pypo9 branch from 4a2bce2 to e257d90 Compare September 3, 2026 23:36

zook-bot commented Sep 4, 2026

View reviewed changes

zook-bot merged commit 298fb20 into main Sep 4, 2026
89 of 97 checks passed

Navigation Menu

cmd/gc fails golangci-lint on main with 3 issues, so lint-affected reds every PR that touches the package (gc-pypo9) - #172

cmd/gc fails golangci-lint on main with 3 issues, so lint-affected reds every PR that touches the package (gc-pypo9)#172
zook-bot merged 3 commits into
mainfrom
polecat/gc-pypo9

zook-bot commented Sep 3, 2026

Uh oh!

zook-bot commented Sep 3, 2026

Uh oh!

zook-bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

zook-bot commented Sep 3, 2026

Summary

Summary

Verification

What

Why it reds unrelated PRs

Provenance

Why it was not caught at the time

Fix candidates

Refinery handoff

Uh oh!

zook-bot commented Sep 3, 2026

Uh oh!

zook-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant