Skip to content

fix(dispatch): Tier-3 pool sort oldest -> hybrid — honour P-level for fresh work (vc-zv4y) - #96

Merged
bourgois merged 1 commit into
mainfrom
gc/vc-zv4y
Jul 27, 2026
Merged

fix(dispatch): Tier-3 pool sort oldest -> hybrid — honour P-level for fresh work (vc-zv4y)#96
bourgois merged 1 commit into
mainfrom
gc/vc-zv4y

Conversation

@voxist-merge-bot

Copy link
Copy Markdown

What & why

The generic pool-worker work_query Tier 3 (routedReadyTierCommand, internal/config/workquery.go) ordered routed pool work strictly by created_at ASC (--sort oldest) and never read a bead's priority, so a freshly-routed P1 starved behind every older P2/P3 until they closed — priority was decorative for the pool tier. Measured live on the city store (2026-07-17): a P1 sat at rank 49/51, outside the 20-row gc hook window, so gc hook --claim deterministically claimed a P3 filed 2026-07-01 while the P1 waited.

Switch the sole canonical Tier-3 sort site to bd's hybrid policy: recent work (< 48h) is ordered by priority, older work (>= 48h) drains by age. This honors priority for fresh work and preserves the age-based anti-starvation drain that --sort oldest was chosen for. Live effect: the P1 moves rank 49 → 1.

Decision: ADR-0035 (docs/decisions/ADR-0035-pool-dispatch-priority-blind-fifo.md, in voxist-city). Track 1 (fork-local); resync-debt stamps onto the bead when Track 2 (vc-ivi3, upstream PR) opens.

Scope (deliberately narrow)

  • Only the canonical routedReadyTierCommand flips to --sort hybrid.
  • The retiring migration probe (workquery.go:54, ga-dhf44) intentionally stays --sort oldest.
  • poolDemandCountShell (reconciler count-form) passes no --sort and is byte-identical ⇒ spawn-decision parity preserved (the 6 *_PoolDemand_* goldens are unchanged).
  • --limit=20 (anti-self-block lookahead, not a priority window) preserved.

Tests & docs

  • Canonical-tier assertions moved to hybrid; the 12 Work+RoutedPool goldens regenerated (PoolDemand goldens untouched — parity).
  • The two tests that pinned the old FIFO were rewritten to pin the hybrid intent with a negative regression guard (--sort oldest must be ABSENT on the canonical tier): ...UsesHybridSortHonoringPriority, ...UsesNativeHybridSortAcrossReadyTiers. Migration-probe assertions left as oldest.
  • engdocs/architecture/dispatch.md updated: its pool-tier description said "FIFO before priority" (the exact opposite of the shipped behavior) and is linked directly from the changed code; also corrected a pre-existing --limit=1--limit=20 drift. (ADR-0035 AC 4.)
  • go build ./..., go vet, full internal/config package: green. The Convoy|WorkflowServe|ControlReady|WorkQuery tests in cmd/gc: green.

Not in this PR (filed separately)

  • AC-1's literal P1 rank (49 → 1) is verified by the live post-deploy re-measure (ADR-0035 AC 3 / va-urb0), not a unit test: a mock bd cannot reproduce bd's ORDER BY; gascity's contract here is selecting the flag, which the unit tests pin.
  • A second priority-blind routed dispatcher exists at cmd/gc/dispatch_runtime.go:768-769 (control-dispatcher / workflow-serve) — a genuinely distinct consumer (different bd --readonly --sandbox invocation, control-plane only). Filed as vc-8poto for an explicit hybrid-vs-FIFO decision rather than silently widening this diff.
  • Merge ≠ live. gc is a self-built binary; this fix reaches the fleet only after gc is rebuilt/redeployed (va-urb0). Do not treat merge as deploy — the tracking bead vc-zv4y stays open until the live re-measure.

@bourgois bourgois left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Architect validation gate — APPROVE (vc-zv4y / ADR-0035 Fix 1)

Validated independently against the live source + CI, not against the PR summary (va-6sx discipline).

Code — CORRECT and exactly scoped

  • internal/config/workquery.go routedReadyTierCommand: single-token --sort oldest--sort hybrid at the sole canonical Tier-3 site (was :133). This is ADR-0035 Fix 1 verbatim.
  • --limit=20 preserved (anti-self-block lookahead, not a priority window).
  • The retiring migration probe (bdReadyPoolDemandMigrationShell, :54) and the legacy gc.run_target/gc.kind=workflow fallback branch are left on oldest — correct; only the primary routed pool-demand probe flips. Verified in the golden files.
  • Reconciler parity held: the count-form (poolDemandCountShell) still passes no --sort (order-irrelevant to a length), so spawn decisions are unperturbed. Documented in the new comment and unchanged in the count-form goldens.

Tests — sound and honest

  • internal/config package PASSES in CI (ok github.com/gastownhall/gascity/internal/config 3.731s).
  • The FIFO-pinning test ...UsesOldestBeforePriority is correctly replaced by ...UsesHybridSortHonoringPriority, and ...UsesNativeHybridSortAcrossReadyTiers pins the native-sort composition + first-row-only contract.
  • The test comment correctly scopes gascity's contract to selecting hybrid; bd owns the ORDER BY, and the end-to-end rank re-measure (P1 49→1) is deferred to ADR-0035 AC3 (live, post-deploy) — a mock bd cannot reproduce bd 1.1.0's comparator. Right seam.

Red CI is PRE-EXISTING and UNRELATED — not a blocker here

  • All cmd/gc process shards fail only on TestClassifyProductMetricsCommandCanonicalMatrix/*; packages-core-4-of-4 fails only on TestRepositoryLedgerMatchesCensusAndDocumentation. Both are command-census/ledger drift tests, untouched by this PR.
  • Identical failure signature appears on sibling PRs #95 and #97 (neither touches workquery.go) ⇒ fleet-wide on Voxist/main. PR #98 ("regenerate command census") is the dedicated remedy.
  • Voxist/gascity main protection has required_status_checks: none, so this red does not gate merge.

Branch hygiene

  • head 97ceb06a is exactly 1 commit atop current Voxist/main (behind_by=0, ahead_by=1) — no rebase, no upstream-lineage drag (the vp-zq8h trap avoided; base is the fork's main).

Merge note (NOT deploy)

gc is self-built: merge does not reach the fleet until gc is rebuilt/redeployed (va-urb0). Per ADR-0035 AC3, do not close vc-zv4y at merge — it stays open until the live P1-rank re-measure on a rebuilt gc.

Verdict: APPROVE on merits. gascity main requires 1 review, require_last_push_approval=false — this satisfies the gate.

… fresh work (vc-zv4y)

Pool dispatch (work_query Tier 3) ordered routed work strictly by created_at ASC (FIFO) and never read priority: a fresh P1 starved behind every older P2/P3 until they closed. Switch the sole canonical Tier-3 sort site (routedReadyTierCommand) from --sort oldest to --sort hybrid. bd's hybrid policy orders fresh work (<48h) by priority while draining aged work (>=48h) oldest-first, so a fresh P1 jumps ahead AND the age-based anti-starvation property that --sort oldest was chosen for is preserved.

Reconciler parity held: the shared pool-demand predicate (poolDemandCountShell) passes no --sort (order-irrelevant to a length), so the reconciler spawn decision is unaffected. The retiring migration probe (workquery.go:54, ga-dhf44) deliberately stays --sort oldest.

Tests: replace TestEffectiveWorkQueryRoutedQueueUsesOldestBeforePriority (which pinned the priority-blind FIFO this removes) with ...UsesHybridSortHonouringPriority; rename the native-sort composition test; regenerate the 12 workquery goldens. Verified: full internal/config package passes; bd 1.1.0 accepts --sort hybrid (negative control: invalid sort errors).

ADR-0035 (docs/decisions/ADR-0035-pool-dispatch-priority-blind-fifo.md). Refs vc-zv4y.
@bourgois
bourgois merged commit 485678d into main Jul 27, 2026
73 of 80 checks passed
@bourgois
bourgois deleted the gc/vc-zv4y branch July 27, 2026 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant