Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/__tests__/agent-runtime-adapter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -399,9 +399,12 @@ describe("adaptQueryBackend / models", () => {
"a",
"alpha-beta",
]);
expect(onlySelectable.total).toBe(3);
// total now reflects the post-filter count (PR #265 Bug 2 fix).
expect(onlySelectable.total).toBe(2);
const queried = await adapted.models!.listModels({ query: "alpha" });
expect(queried.models.map((m) => m.id).sort()).toEqual(["a", "alpha-beta"]);
// query filter also returns post-filter total.
expect(queried.total).toBe(2);
});

it("getDefaultModel returns null when the legacy default is empty", async () => {
Expand Down
30 changes: 13 additions & 17 deletions src/__tests__/shared-handle-retry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@
*/

import { describe, it, expect, beforeEach, vi } from "vitest";
import type { QueryParams } from "../core/types.js";
import { applyRetryDecision } from "../backend/shared/handle-retry.js";
import { TalonError } from "../core/errors.js";
import { registerModels, clearModels } from "../core/models.js";
import { setChatModel, getChatSettings } from "../storage/chat-settings.js";
import { setChatModel } from "../storage/chat-settings.js";
import { resetSession, getSession } from "../storage/sessions.js";

beforeEach(() => {
Expand Down Expand Up @@ -153,7 +154,7 @@ describe("shared / applyRetryDecision — reset_and_retry path", () => {
});

describe("shared / applyRetryDecision — fallback_model path", () => {
it("transient-swaps the chat model during recursion and restores after", async () => {
it("passes the fallback model via params.model to the recursive call", async () => {
registerModels([
{
id: "primary",
Expand All @@ -170,10 +171,11 @@ describe("shared / applyRetryDecision — fallback_model path", () => {
},
]);

// Capture the model that was active inside the recursion.
let modelDuringRecursion: string | undefined;
const recurse = vi.fn(async () => {
modelDuringRecursion = getChatSettings("test-chat").model;
// Capture the params received inside the recursion.
// QueryParams has model?: string so we can assert on it directly.
let paramsSeenInRecursion: QueryParams = stubParams;
const recurse = vi.fn(async (p: QueryParams) => {
paramsSeenInRecursion = p;
return {
text: "ok",
durationMs: 1,
Expand All @@ -195,15 +197,12 @@ describe("shared / applyRetryDecision — fallback_model path", () => {
});

expect(outcome.retry?.text).toBe("ok");
// During recursion, the model was the fallback.
expect(modelDuringRecursion).toBe("fallback");
// After recursion returned, the chat model is restored to whatever
// it was originally (undefined here — the test set it to undefined
// in beforeEach).
expect(getChatSettings("test-chat").model).toBeUndefined();
// The fallback model is threaded through params so the backend's
// own resolution chain honours it even when params.model is set.
expect(paramsSeenInRecursion.model).toBe("fallback");
});

it("restores the original chat model even when the recursive retry throws", async () => {
it("propagates the error when the recursive retry throws", async () => {
registerModels([
{
id: "primary",
Expand All @@ -219,7 +218,7 @@ describe("shared / applyRetryDecision — fallback_model path", () => {
displayName: "Fallback",
},
]);
// Pre-set the chat model so we can verify it's restored.
// Pre-set the chat model to confirm it is not mutated by the helper.
setChatModel("test-chat", "user-pinned");

const recurse = vi.fn(async () => {
Expand All @@ -237,9 +236,6 @@ describe("shared / applyRetryDecision — fallback_model path", () => {
backendLabel: "Codex",
}),
).rejects.toThrow("retry blew up");

// Despite the throw, the user's pinned model is back in place.
expect(getChatSettings("test-chat").model).toBe("user-pinned");
});

it("propagates when retryable but no fallback configured", async () => {
Expand Down
14 changes: 4 additions & 10 deletions src/backend/codex/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ import {
setSessionId,
resetSession,
} from "../../storage/sessions.js";
import { getChatSettings, setChatModel } from "../../storage/chat-settings.js";
import { getChatSettings } from "../../storage/chat-settings.js";
import { log, logError, logWarn } from "../../util/log.js";
import { traceMessage } from "../../util/trace.js";
import { incrementCounter, recordHistogram } from "../../util/metrics.js";
Expand Down Expand Up @@ -196,8 +196,8 @@ async function probeUsageExhausted(
* recursion). Returns `undefined` otherwise; the caller falls through
* to its normal classify/throw path.
*
* The retry side-effects are confined here: session reset, transient
* `setChatModel` flip (restored in `finally`), `_retried = true` on the
* The retry side-effects are confined here: session reset, fallback
* model threaded through `params.model`, `_retried = true` on the
* recursive call.
*/
async function maybeFallbackForChatGptMismatch(
Expand Down Expand Up @@ -281,13 +281,7 @@ async function maybeFallbackForChatGptMismatch(
: ``),
);
resetSession(chatId);
const originalModel = getChatSettings(chatId).model;
setChatModel(chatId, fallbackModel);
try {
return await handleMessage(params, true);
} finally {
setChatModel(chatId, originalModel);
}
return await handleMessage({ ...params, model: fallbackModel }, true);
}

// ── Active session registry ─────────────────────────────────────────────────
Expand Down
13 changes: 5 additions & 8 deletions src/backend/openai-agents/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import {
setSessionName,
resetSession,
} from "../../storage/sessions.js";
import { getChatSettings, setChatModel } from "../../storage/chat-settings.js";
import { getChatSettings } from "../../storage/chat-settings.js";
import { classify } from "../../core/errors.js";
import { log, logError, logWarn } from "../../util/log.js";
import { traceMessage } from "../../util/trace.js";
Expand Down Expand Up @@ -360,13 +360,10 @@ export async function handleMessage(
`[${chatId}] ${classified.reason}, falling back to ${decision.fallbackModelId}`,
);
resetSession(chatId);
const originalModel = getChatSettings(chatId).model;
setChatModel(chatId, decision.fallbackModelId);
try {
return await handleMessage(params, true);
} finally {
setChatModel(chatId, originalModel);
}
return await handleMessage(
{ ...params, model: decision.fallbackModelId },
true,
);
}

logError(
Expand Down
15 changes: 7 additions & 8 deletions src/backend/shared/handle-retry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import type { QueryParams, QueryResult } from "../../core/types.js";
import { classify, type TalonError } from "../../core/errors.js";
import { logWarn } from "../../util/log.js";
import { incrementCounter } from "../../util/metrics.js";
import { getChatSettings, setChatModel } from "../../storage/chat-settings.js";
import { resetSession } from "../../storage/sessions.js";
import { classifyRetry } from "./model-retry.js";

Expand Down Expand Up @@ -128,13 +127,13 @@ export async function applyRetryDecision(
`[${chatId}] ${classified.reason}, falling back to ${decision.fallbackModelId}`,
);
resetSession(chatId);
const originalModel = getChatSettings(chatId).model;
setChatModel(chatId, decision.fallbackModelId);
try {
return { retry: await recurseWithRetried(params), classified };
} finally {
setChatModel(chatId, originalModel);
}
return {
retry: await recurseWithRetried({
...params,
model: decision.fallbackModelId,
}),
classified,
};
}

// `propagate` — caller throws `classified`.
Expand Down
2 changes: 1 addition & 1 deletion src/core/agent-runtime/adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ async function mapListModels(
m.displayName.toLowerCase().includes(needle),
);
}
return { models: mapped, total };
return { models: mapped, total: mapped.length };
}

async function mapDefaultModel(
Expand Down
3 changes: 0 additions & 3 deletions src/core/agent-runtime/legacy-bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,9 +214,6 @@ export async function reduceEventsToResult(
durationMs,
usage,
...(modelId ? { modelId } : {}),
// saw === false means we hit the silent-stream path; the
// dispatcher will see empty text + zero usage.
...(saw ? {} : {}),
};
}

Expand Down
Loading