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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * chore: retrigger CI --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: trek-e <trek-e@users.noreply.github.com>
This commit is contained in:
parent
9b6ff01471
commit
b1ae782876
6 changed files with 152 additions and 25 deletions
|
|
@ -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}`
|
||||
|
|
|
|||
|
|
@ -1351,6 +1351,7 @@ const widgetStateAccessors: WidgetStateAccessors = {
|
|||
getBasePath: () => s.basePath,
|
||||
isVerbose: () => s.verbose,
|
||||
isSessionSwitching: isSessionSwitchInFlight,
|
||||
getCurrentDispatchedModelId: () => s.currentDispatchedModelId,
|
||||
};
|
||||
|
||||
// ─── Preconditions ────────────────────────────────────────────────────────────
|
||||
|
|
|
|||
|
|
@ -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({
|
||||
|
|
|
|||
|
|
@ -105,6 +105,8 @@ export class AutoSession {
|
|||
// ── Model state ──────────────────────────────────────────────────────────
|
||||
autoModeStartModel: StartModel | null = null;
|
||||
currentUnitModel: Model<Api> | 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;
|
||||
|
|
|
|||
|
|
@ -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)];
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
Loading…
Add table
Reference in a new issue