Skip to content

fix(panels): correct roster ids and bound reviewer dispatch time - #274

Open
neilwashere wants to merge 3 commits into
mainfrom
feat/panel-dispatch-hygiene
Open

fix(panels): correct roster ids and bound reviewer dispatch time#274
neilwashere wants to merge 3 commits into
mainfrom
feat/panel-dispatch-hygiene

Conversation

@neilwashere

Copy link
Copy Markdown
Contributor
track: reversible
slug: panel-dispatch-hygiene

Closes#268
Closes#270

What and why

A panel wastes a dispatch cycle on something we already knew would fail. #270 knew the model was dead; #268 knew the diff was huge. Neither fact reached the dispatch decision.

Two files. No frozen surfaces, no code, no new scripts.

The premise of #270 was wrong

#270 asks to demote two "dead" models. A live probe answered on both:

PONG-OK google-vertex/gemini-3.1-pro-preview
PONG-OK anthropic/claude-opus-5
PONG-OK amazon-bedrock/eu.anthropic.claude-opus-5
PONG-OK amazon-bedrock/eu.anthropic.claude-sonnet-5

Gemini was never credit-exhausted in the way recorded — the entry named google/, and no google provider exists; the credentialled one is google-vertex, so it could not resolve at all. Opus was an AWS IAM gap since fixed, plus a moved inference region and a superseded generation.

So the defect is not "pools rank dead models above live ones". It is the roster rots silently and nothing notices until dispatch. Demoting either model would have fixed neither. That evidence is recorded on #141, whose --pong-by-default mechanism it strengthens.

Changes

.pi/sdlc/sdlc.config.json — every id now resolves and names the current generation of its family. No entry changed position.

WasNow
google/gemini-3.1-pro-previewgoogle-vertex/gemini-3.1-pro-preview
anthropic/claude-fable-5 (incl. authorDefault)anthropic/claude-fable-5-1
anthropic/claude-opus-4-8anthropic/claude-opus-5
amazon-bedrock/global.anthropic.claude-opus-4-8anthropic/claude-opus-5 + amazon-bedrock/eu.anthropic.claude-opus-5
zai/glm-5.2zai/glm-5.3

pr_review now pairs direct claude-opus-5 with the Bedrock eu route behind it. modelIdentity() folds the Bedrock alias onto the direct identity, so the pair adds no distinct-model slot and is skipped at resolution. It exists for dispatch-time recovery. The $comment says so explicitly, because otherwise it reads as a duplicate and invites deletion.

skills/sdlc/references/phase-pr-review.md §5 — three edits:

  1. Time budget. Every dispatch passes an explicit timeoutMs. Floor 45 minutes; 90 above 30 changed files or 2,000 changed lines.
  2. A timeout is deterministic, not transient. The ladder classed timeouts with transient failures, so the sanctioned response was retry-then-replace. Both are wrong on a large diff: the retry fails identically and the replacement inherits the same diff. Now: one retry of the same model at double the budget, inside the original logical wave, before any replacement.
  3. One sentence naming a provider-route twin as the first replacement for a provider-side failure.

Assumptions & discretionary calls

  • A1 — reversible track, no Spec, so no scenario ids. Each task still carries a "full"-tagged check.
  • A2 — 45/90 minutes rest on one observation (a 70-file, ~5,200-line panel: died at 30, completed at 90). Written into §5 as a revisable floor that says so, not as a derived model.
  • A3 — two tasks split by file, so an unfalsifiable doctrine change does not hide behind a data change's mechanical check.
  • A4deviates from shape.publishToTracker. Threshold is 2 and this build has 2 tasks, so the default is an epic plus two sub-issues. Not done: Retro(S2): scale PR-panel reviewer time budgets with review size (phase-pr-review §5) #268 and Retro(S2): prefer pools rank known-dead models above live ones (opus IAM, gemini credits) #270 already exist, are labelled, and partition the work identically. Four tracker objects for a two-file change is disproportionate. Flag it if you want the projection.
  • A5.pi/sdlc/CONFIG.md is a generated companion and became a T1 surface at implement; regenerated via config-doc.sh write, never hand-edited.
  • A6the local test baseline is not green, and this change did not do it. See Verification.

Deliberately not done

The route-twin rule is held to one sentence on purpose. A lens/seat table with per-seat fallbacks, a vendor-diversity floor, and seat efficacy measurement is a known successor pattern that would replace bespoke fallback prose wholesale. Building a full recovery rung now means deleting it next round. Also out of scope: --pong by default (#141), re-ordering the roster, restoring gpt-5.6-terra, and any change to resolve-panel.mjs or sdlc.config.example.json (both frozen).

Verification

  • All 25 configured ids resolve against pi --list-models; no google/ prefix remains; no entry has a live newer sibling in its family.
  • sdlc-statusstate: ready, exit 0.
  • config-doc.sh checkcurrent.
  • §5's five asserting suites: 96/96 pass. check-references.mjs exit 0.
  • No file from test/frozen-surfaces.test.js's FROZEN list is in the diff.
  • biome findings repo-wide unchanged at 3, all pre-existing and outside this diff.

On the test baseline (A6):npm test fails 29 of 616 on main on macOS, and this branch's failure set is byte-identical to it — zero new failures. All 29 share one cause: macOS mktemp -d returns /var/folders/…, a symlink to /private/var/folders/…; git rev-parse --show-toplevel resolves it, the containment check compares the two literally, and reports git.repository :: resolved root escapes its git top-level. CI is Linux, so CI is green. Filed as #273. CI on this PR is the authoritative signal.

Plan and build plan for correcting the panel roster and bounding reviewer
dispatch time.
Records that the originating premise was falsified during brainstorm: a live
probe answered on every model the roster was said to have lost, so the defect
is a rotting roster (a stale provider prefix and three superseded model
generations), not depleted credits or a missing IAM grant.
Scopes the slice around a known successor reviewer pattern rather than
pre-empting it, so the bespoke fallback prose that pattern would replace is
held to one sentence.
Every configured id now resolves against `pi --list-models` and names the
current generation of its family. No entry changed position.
The gemini entry named a provider prefix no credentialled provider answers
to, so it could never resolve; the recorded API-credit reason for its
demotion is superseded, and the correction moves it to `google-vertex/`.
pr_review pairs direct claude-opus-5 with the Bedrock eu route immediately
behind it. modelIdentity() folds the Bedrock alias onto the direct identity,
so the pair adds no distinct-model slot and is skipped at resolution; it
exists as the route twin the PR-review reference names for a provider-side
failure. The panels comment says so, because otherwise the entry reads as a
duplicate and invites deletion.
Regenerates the CONFIG.md companion from the edited manifest.
…overy
Panel dispatch inherited the subagent tool's 30-minute default because no
budget was ever stated. A 70-file review lost every reviewer to it and
completed only after the budget was raised by hand.
States an explicit time budget at dispatch with a 45-minute floor and a
90-minute start above 30 changed files or 2,000 changed lines, marked as a
floor open to revision since it rests on one observation.
Corrects the recovery ladder, which classed a timeout with transient infra
failures and so prescribed retry-then-replace. A timeout is deterministic:
the same budget fails again and a replacement inherits the same diff. The
sanctioned move is one retry of the same model at double the budget, inside
the original logical wave, before any replacement.
Adds one sentence naming a provider-route twin as the first replacement for
a provider-side failure.
@neilwashereneilwashere changed the title Panel dispatch hygiene: roster rot and reviewer time budgetsfix(panels): correct roster ids and bound reviewer dispatch timeSep 4, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant

@neilwashere