From b1ae782876920154cbd2d3834aa2ff8fe38a8633 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 5 Apr 2026 00:48:50 -0400 Subject: [PATCH] fix: dashboard model label shows dispatched model, not stale previous unit (#3320) * fix: dashboard model label shows dispatched model, not stale previous unit model Move updateProgressWidget and ensurePreconditions after selectAndApplyModel in phases.ts so the widget's first render tick reads the correct model. Store currentDispatchedModelId in session state after model selection + hook overrides, expose it via widgetStateAccessors, and update auto-dashboard.ts to prefer the dispatched model ID over cmdCtx.model which can be stale. Closes #2899 Co-Authored-By: Claude Opus 4.6 * fix(test): widen runUnitPhase slice in ordering test to accommodate grown function body The structural test for selectAndApplyModel ordering sliced only 8000 chars of runUnitPhase, but the function grew past that limit after rebase, causing updateProgressWidget to fall outside the window. Co-Authored-By: Claude Opus 4.6 * chore: retrigger CI --------- Co-authored-by: Claude Opus 4.6 Co-authored-by: trek-e --- .../extensions/gsd/auto-dashboard.ts | 14 ++- src/resources/extensions/gsd/auto.ts | 1 + src/resources/extensions/gsd/auto/phases.ts | 50 ++++---- src/resources/extensions/gsd/auto/session.ts | 3 + .../extensions/gsd/tests/auto-loop.test.ts | 2 +- .../dashboard-model-label-ordering.test.ts | 107 ++++++++++++++++++ 6 files changed, 152 insertions(+), 25 deletions(-) create mode 100644 src/resources/extensions/gsd/tests/dashboard-model-label-ordering.test.ts diff --git a/src/resources/extensions/gsd/auto-dashboard.ts b/src/resources/extensions/gsd/auto-dashboard.ts index 688b6bdbc..a1a072dc3 100644 --- a/src/resources/extensions/gsd/auto-dashboard.ts +++ b/src/resources/extensions/gsd/auto-dashboard.ts @@ -438,6 +438,8 @@ export interface WidgetStateAccessors { isVerbose(): boolean; /** True while newSession() is in-flight — render must not access session state. */ isSessionSwitching(): boolean; + /** Fully-qualified dispatched model ID (provider/id) set after model selection + hook overrides (#2899). */ + getCurrentDispatchedModelId(): string | null; } export function updateProgressWidget( @@ -629,9 +631,15 @@ export function updateProgressWidget( const cxPctVal = cxUsage?.percent ?? 0; const cxPct = cxUsage?.percent !== null ? cxPctVal.toFixed(1) : "?"; - // Model display — shown in context section, not stats - const modelId = cmdCtx?.model?.id ?? ""; - const modelProvider = cmdCtx?.model?.provider ?? ""; + // Model display — prefer dispatched model ID (set after selectAndApplyModel + // + hook overrides) over cmdCtx?.model which can be stale (#2899). + const dispatchedModelId = accessors.getCurrentDispatchedModelId(); + const modelId = dispatchedModelId + ? dispatchedModelId.split("/").slice(1).join("/") || dispatchedModelId + : (cmdCtx?.model?.id ?? ""); + const modelProvider = dispatchedModelId + ? dispatchedModelId.split("/")[0] || "" + : (cmdCtx?.model?.provider ?? ""); const tierIcon = resolveServiceTierIcon(effectiveServiceTier, modelId); const modelDisplay = (modelProvider && modelId ? `${modelProvider}/${modelId}` diff --git a/src/resources/extensions/gsd/auto.ts b/src/resources/extensions/gsd/auto.ts index a7cceab81..0f67fbb65 100644 --- a/src/resources/extensions/gsd/auto.ts +++ b/src/resources/extensions/gsd/auto.ts @@ -1351,6 +1351,7 @@ const widgetStateAccessors: WidgetStateAccessors = { getBasePath: () => s.basePath, isVerbose: () => s.verbose, isSessionSwitching: isSessionSwitchInFlight, + getCurrentDispatchedModelId: () => s.currentDispatchedModelId, }; // ─── Preconditions ──────────────────────────────────────────────────────────── diff --git a/src/resources/extensions/gsd/auto/phases.ts b/src/resources/extensions/gsd/auto/phases.ts index 430cde624..63c59f9d6 100644 --- a/src/resources/extensions/gsd/auto/phases.ts +++ b/src/resources/extensions/gsd/auto/phases.ts @@ -985,30 +985,10 @@ export async function runUnitPhase( }, ); - // Select and apply model (with tier escalation on retry — normal units only) - const modelResult = await deps.selectAndApplyModel( - ctx, - pi, - unitType, - unitId, - s.basePath, - prefs, - s.verbose, - s.autoModeStartModel, - sidecarItem ? undefined : { isRetry, previousTier }, - ); - s.currentUnitRouting = - modelResult.routing as AutoSession["currentUnitRouting"]; - s.currentUnitModel = - modelResult.appliedModel as AutoSession["currentUnitModel"]; - - // Status bar + progress widget + // Status bar (widget + preconditions deferred until after model selection — see #2899) ctx.ui.setStatus("gsd-auto", "auto"); if (mid) deps.updateSliceProgressCache(s.basePath, mid, state.activeSlice?.id); - deps.updateProgressWidget(ctx, unitType, unitId, state); - - deps.ensurePreconditions(unitType, unitId, s.basePath, state); // Prompt injection let finalPrompt = prompt; @@ -1074,6 +1054,23 @@ export async function runUnitPhase( logWarning("engine", "Prompt reorder failed", { error: msg }); } + // Select and apply model (with tier escalation on retry — normal units only) + const modelResult = await deps.selectAndApplyModel( + ctx, + pi, + unitType, + unitId, + s.basePath, + prefs, + s.verbose, + s.autoModeStartModel, + sidecarItem ? undefined : { isRetry, previousTier }, + ); + s.currentUnitRouting = + modelResult.routing as AutoSession["currentUnitRouting"]; + s.currentUnitModel = + modelResult.appliedModel as AutoSession["currentUnitModel"]; + // Apply sidecar/pre-dispatch hook model override (takes priority over standard model selection) const hookModelOverride = sidecarItem?.model ?? iterData.hookModelOverride; if (hookModelOverride) { @@ -1099,6 +1096,17 @@ export async function runUnitPhase( } } + // Store the final dispatched model ID so the dashboard can read it (#2899). + // This accounts for hook model overrides applied after selectAndApplyModel. + s.currentDispatchedModelId = s.currentUnitModel + ? `${(s.currentUnitModel as any).provider ?? ""}/${(s.currentUnitModel as any).id ?? ""}` + : null; + + // Progress widget + preconditions — deferred to after model selection so the + // widget's first render tick shows the correct model (#2899). + deps.updateProgressWidget(ctx, unitType, unitId, state); + deps.ensurePreconditions(unitType, unitId, s.basePath, state); + // Start unit supervision deps.clearUnitTimeout(); deps.startUnitSupervision({ diff --git a/src/resources/extensions/gsd/auto/session.ts b/src/resources/extensions/gsd/auto/session.ts index 9d11545e3..f2e690a33 100644 --- a/src/resources/extensions/gsd/auto/session.ts +++ b/src/resources/extensions/gsd/auto/session.ts @@ -105,6 +105,8 @@ export class AutoSession { // ── Model state ────────────────────────────────────────────────────────── autoModeStartModel: StartModel | null = null; currentUnitModel: Model | null = null; + /** Fully-qualified model ID (provider/id) set after selectAndApplyModel + hook overrides (#2899). */ + currentDispatchedModelId: string | null = null; originalModelId: string | null = null; originalModelProvider: string | null = null; lastBudgetAlertLevel: BudgetAlertLevel = 0; @@ -193,6 +195,7 @@ export class AutoSession { // Model this.autoModeStartModel = null; this.currentUnitModel = null; + this.currentDispatchedModelId = null; this.originalModelId = null; this.originalModelProvider = null; this.lastBudgetAlertLevel = 0; diff --git a/src/resources/extensions/gsd/tests/auto-loop.test.ts b/src/resources/extensions/gsd/tests/auto-loop.test.ts index 3a548f326..2414d280a 100644 --- a/src/resources/extensions/gsd/tests/auto-loop.test.ts +++ b/src/resources/extensions/gsd/tests/auto-loop.test.ts @@ -325,7 +325,7 @@ test("auto/phases.ts: selectAndApplyModel called exactly once and before updateP // Extract the runUnitPhase function body const fnStart = src.indexOf("export async function runUnitPhase"); assert.ok(fnStart > 0, "runUnitPhase should exist in phases.ts"); - const fnBody = src.slice(fnStart, fnStart + 8000); + const fnBody = src.slice(fnStart, fnStart + 12000); // selectAndApplyModel must appear exactly once const allOccurrences = [...fnBody.matchAll(/selectAndApplyModel\(/g)]; diff --git a/src/resources/extensions/gsd/tests/dashboard-model-label-ordering.test.ts b/src/resources/extensions/gsd/tests/dashboard-model-label-ordering.test.ts new file mode 100644 index 000000000..ad28398a2 --- /dev/null +++ b/src/resources/extensions/gsd/tests/dashboard-model-label-ordering.test.ts @@ -0,0 +1,107 @@ +/** + * dashboard-model-label-ordering.test.ts — Regression test for #2899. + * + * The dashboard model label was showing the previous unit's model because + * updateProgressWidget was called before selectAndApplyModel in phases.ts. + * This test verifies: + * 1. updateProgressWidget is called AFTER selectAndApplyModel in phases.ts + * 2. session.ts has a currentDispatchedModelId field + * 3. auto.ts exposes getCurrentDispatchedModelId in widgetStateAccessors + * 4. auto-dashboard.ts reads from a dispatched model accessor, not cmdCtx?.model + */ + +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { createTestContext } from "./test-helpers.ts"; + +const { assertTrue, assertMatch, report } = createTestContext(); + +const phasesPath = join(import.meta.dirname, "..", "auto", "phases.ts"); +const sessionPath = join(import.meta.dirname, "..", "auto", "session.ts"); +const autoPath = join(import.meta.dirname, "..", "auto.ts"); +const dashboardPath = join(import.meta.dirname, "..", "auto-dashboard.ts"); + +const phasesSrc = readFileSync(phasesPath, "utf-8"); +const sessionSrc = readFileSync(sessionPath, "utf-8"); +const autoSrc = readFileSync(autoPath, "utf-8"); +const dashboardSrc = readFileSync(dashboardPath, "utf-8"); + +console.log("\n=== #2899: Dashboard model label shows correct (dispatched) model ==="); + +// ── Test 1: updateProgressWidget is called AFTER selectAndApplyModel ────── + +// Find the positions of the calls in the dispatch function body. +// selectAndApplyModel must appear BEFORE updateProgressWidget. +const selectModelPos = phasesSrc.indexOf("deps.selectAndApplyModel("); +const updateWidgetPos = phasesSrc.indexOf("deps.updateProgressWidget("); + +assertTrue( + selectModelPos > 0, + "phases.ts contains deps.selectAndApplyModel call", +); + +assertTrue( + updateWidgetPos > 0, + "phases.ts contains deps.updateProgressWidget call", +); + +assertTrue( + selectModelPos < updateWidgetPos, + `selectAndApplyModel (pos ${selectModelPos}) must be called BEFORE updateProgressWidget (pos ${updateWidgetPos}) — widget needs fresh model`, +); + +// ── Test 2: session.ts declares currentDispatchedModelId ────────────────── + +assertTrue( + sessionSrc.includes("currentDispatchedModelId"), + "session.ts has currentDispatchedModelId field", +); + +// ── Test 3: auto.ts exposes getCurrentDispatchedModelId in widgetStateAccessors ── + +assertTrue( + autoSrc.includes("getCurrentDispatchedModelId"), + "auto.ts exposes getCurrentDispatchedModelId accessor", +); + +// Verify it's in the widgetStateAccessors object +const accessorsBlock = autoSrc.slice( + autoSrc.indexOf("const widgetStateAccessors"), + autoSrc.indexOf("};", autoSrc.indexOf("const widgetStateAccessors")) + 2, +); + +assertTrue( + accessorsBlock.includes("getCurrentDispatchedModelId"), + "getCurrentDispatchedModelId is in the widgetStateAccessors object", +); + +// ── Test 4: WidgetStateAccessors interface has getCurrentDispatchedModelId ── + +assertTrue( + dashboardSrc.includes("getCurrentDispatchedModelId"), + "auto-dashboard.ts references getCurrentDispatchedModelId", +); + +// The dashboard render closure should NOT read model from cmdCtx?.model for display. +// It should use the accessor for the dispatched model ID. +// Check that the "Model display" section uses the accessor, not cmdCtx?.model directly. +const modelDisplaySection = dashboardSrc.slice( + dashboardSrc.indexOf("// Model display"), + dashboardSrc.indexOf("// Model display") + 500, +); + +assertTrue( + modelDisplaySection.includes("getCurrentDispatchedModelId") || + modelDisplaySection.includes("getDispatchedModelId"), + "Model display section reads from dispatched model accessor, not cmdCtx?.model alone", +); + +// ── Test 5: currentDispatchedModelId is set after selectAndApplyModel in phases.ts ── + +// After selectAndApplyModel returns, phases.ts should store the dispatched model ID +assertTrue( + phasesSrc.includes("currentDispatchedModelId"), + "phases.ts stores currentDispatchedModelId after model selection", +); + +report();