Merge PR #8068: fix(agent): fit-check retry fallback before switching models (@roboomp)
This commit is contained in:
@@ -94,6 +94,9 @@
|
||||
### Fixed
|
||||
|
||||
- Fixed custom commands losing the documented `api.arktype.type(...)` compatibility surface while retaining the callable `api.arktype(...)` builder. ([#7968](https://github.com/can1357/oh-my-pi/issues/7968))
|
||||
### Fixed
|
||||
|
||||
- Fixed retry-fallback selection switching a live session from a large-context primary onto a smaller-context fallback and immediately sending a predictably oversized request; candidate selection now skips any fallback whose usable window cannot hold the current context and advances to the first configured candidate that fits ([#8065](https://github.com/can1357/oh-my-pi/issues/8065)).
|
||||
|
||||
## [17.2.12] - 2026-08-08
|
||||
|
||||
|
||||
@@ -1037,6 +1037,7 @@ export class AgentSession {
|
||||
modelRegistry: this.#modelRegistry,
|
||||
configWarnings: this.configWarnings,
|
||||
model: () => this.model,
|
||||
contextFitsModel: model => this.#maintenance.contextFitsModel(model),
|
||||
textOutputCommitted: () => this.#textOutputCommitted,
|
||||
thinkingLevel: () => this.thinkingLevel,
|
||||
configuredThinkingLevel: () => this.configuredThinkingLevel(),
|
||||
|
||||
@@ -1817,30 +1817,26 @@ export class SessionMaintenance {
|
||||
}
|
||||
|
||||
/**
|
||||
* Retry-side counterpart to {@link #compactionCreatedHeadroom}. An
|
||||
* overflow/incomplete recovery only needs the rebuilt prompt to *fit* the
|
||||
* window again — it does not have to land under the compaction threshold, let
|
||||
* alone the stricter `COMPACTION_RECOVERY_BAND × threshold` hysteresis the
|
||||
* auto-continue thrash guard uses. Reusing the band here turned recoverable
|
||||
* overflows into manual dead-ends: a 200k-window prompt compacted from
|
||||
* overflow down to ~150k is comfortably retryable, but sits above
|
||||
* `0.8 × 170k = 136k` and was wrongly refused (PR #3412 review).
|
||||
* Whether the current stored context fits `model`'s usable window
|
||||
* (`contextWindow - reserve`), using the same reserve resolution as
|
||||
* compaction. This is deliberately independent of `compaction.enabled`: an
|
||||
* oversized request overflows the provider whether or not compaction would
|
||||
* have run, so a fit check must judge the raw budget.
|
||||
*
|
||||
* Measures residual context against the usable budget (`contextWindow - reserve`).
|
||||
* The default absolute reserve can exceed bundled small-context windows, or
|
||||
* nearly consume a 16k-class window; those known-impossible defaults fall
|
||||
* back to the proportional 15% reserve. Explicit valid reserves still define
|
||||
* the usable prompt budget so retries do not enter headroom the user
|
||||
* intentionally reserved. Callers MUST
|
||||
* invoke this AFTER dropping the failed assistant from `this.#host.messages()`, so
|
||||
* the just-failed turn (which the retry prompt will not include) is excluded
|
||||
* from the estimate.
|
||||
* the usable prompt budget so callers do not enter headroom the user
|
||||
* intentionally reserved.
|
||||
*
|
||||
* When the model/window is unknown we cannot evaluate the budget, so we
|
||||
* optimistically allow the retry (preserving prior behavior).
|
||||
* Used by the retry-fallback selector to skip a candidate whose window cannot
|
||||
* hold the live context before switching onto it, and (via
|
||||
* {@link #compactionCreatedRetryFit}) to decide whether an overflow recovery
|
||||
* produced a retryable prompt. When the window is unknown we cannot evaluate
|
||||
* the budget, so we optimistically report a fit (preserving prior behavior).
|
||||
*/
|
||||
#compactionCreatedRetryFit(): boolean {
|
||||
const contextWindow = this.#model?.contextWindow ?? 0;
|
||||
contextFitsModel(model: Model): boolean {
|
||||
const contextWindow = model.contextWindow ?? 0;
|
||||
if (contextWindow <= 0) return true;
|
||||
const compactionSettings = this.#host.settings.getGroup("compaction");
|
||||
const residualTokens = compactionContextTokens(
|
||||
@@ -1851,6 +1847,20 @@ export class SessionMaintenance {
|
||||
return residualTokens <= fitBudget;
|
||||
}
|
||||
|
||||
/**
|
||||
* Retry-side check: whether an overflow/incomplete recovery rebuilt a prompt
|
||||
* that fits the active model's window again. Callers MUST invoke this AFTER
|
||||
* dropping the failed assistant from `this.#host.messages()` so the just-failed
|
||||
* turn (absent from the retry prompt) is excluded from the estimate. Unlike
|
||||
* the `COMPACTION_RECOVERY_BAND × threshold` hysteresis the auto-continue
|
||||
* thrash guard uses, a retry only needs to *fit* — a 200k-window prompt
|
||||
* compacted from overflow down to ~150k is retryable even though it sits above
|
||||
* `0.8 × 170k` (PR #3412 review).
|
||||
*/
|
||||
#compactionCreatedRetryFit(): boolean {
|
||||
return this.#model ? this.contextFitsModel(this.#model) : true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Last-resort tiered reducer when {@link runAutoCompaction} would otherwise
|
||||
* dead-end. The summarizer cut at the only available turn boundary, but the
|
||||
|
||||
@@ -111,6 +111,12 @@ export interface TurnRecoveryHost {
|
||||
modelRegistry: ModelRegistry;
|
||||
configWarnings: string[];
|
||||
model(): Model | undefined;
|
||||
/**
|
||||
* Whether the live context fits `model`'s usable window. Retry-fallback
|
||||
* selection uses it to skip a candidate whose window cannot hold the current
|
||||
* context before switching onto it. See `SessionMaintenance.contextFitsModel`.
|
||||
*/
|
||||
contextFitsModel(model: Model): boolean;
|
||||
/** Whether streamed text has already been committed to the active output sink. */
|
||||
textOutputCommitted(): boolean;
|
||||
thinkingLevel(): ThinkingLevel | undefined;
|
||||
@@ -1339,6 +1345,10 @@ export class TurnRecovery {
|
||||
const candidateModel = resolved.model ?? this.#host.modelRegistry.find(candidate.provider, candidate.id);
|
||||
if (!candidateModel || !this.#host.modelRegistry.hasConfiguredAuth(candidateModel)) continue;
|
||||
if (ceiling !== undefined && !modelSupportsEffortCeiling(candidateModel, ceiling)) continue;
|
||||
// A usage fallback must also fit: skip a candidate whose window cannot
|
||||
// hold the live context so we never switch onto an oversized request
|
||||
// (issue #8065).
|
||||
if (!this.#host.contextFitsModel(candidateModel)) continue;
|
||||
try {
|
||||
const candidateHealth = await this.#host.modelRegistry.authStorage.getModelUsageHealth(
|
||||
candidateModel.provider,
|
||||
@@ -1523,6 +1533,10 @@ export class TurnRecovery {
|
||||
// A candidate whose effort floor exceeds the per-spawn ceiling would be
|
||||
// clamped UP past the cap by its model floor — skip it entirely.
|
||||
if (ceiling !== undefined && !modelSupportsEffortCeiling(candidate, ceiling)) continue;
|
||||
// Skip a candidate whose window cannot hold the live context: switching
|
||||
// onto it would send a predictably oversized request. Advance to the
|
||||
// first candidate whose usable window fits (issue #8065).
|
||||
if (!this.#host.contextFitsModel(candidate)) continue;
|
||||
const apiKey = await this.#host.modelRegistry.getApiKey(candidate, this.#host.sessionId());
|
||||
if (!apiKey) continue;
|
||||
return this.applyRetryFallbackCandidate(role, selector, currentSelector, options);
|
||||
|
||||
@@ -3662,6 +3662,105 @@ describe("AgentSession retry fallback", () => {
|
||||
expect(requestedModels.filter(id => id === `${primaryModel.provider}/${primaryModel.id}`)).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("does not send oversized context to a smaller retry fallback model", async () => {
|
||||
// Regression for #8065: the forward counterpart of #7952. A retryable
|
||||
// error on a large-window primary switches to a retry-fallback candidate,
|
||||
// but candidate selection never compared the candidate's window with the
|
||||
// live context. A 1M-window primary could fall onto a 4000-window fallback
|
||||
// and immediately send a predictably oversized request. The fit gate must
|
||||
// skip the undersized candidate and advance to the first configured
|
||||
// candidate whose window can hold the accumulated context.
|
||||
const modelsConfigPath = path.join(tempDir.path(), "fallback-overflow-models.json");
|
||||
await Bun.write(
|
||||
modelsConfigPath,
|
||||
JSON.stringify({
|
||||
providers: {
|
||||
anthropic: {
|
||||
modelOverrides: {
|
||||
"claude-sonnet-4-5": { contextWindow: 1_000_000 },
|
||||
},
|
||||
},
|
||||
openai: {
|
||||
modelOverrides: {
|
||||
"gpt-4o-mini": { contextWindow: 4000, contextPromotionTarget: "openai/gpt-4o" },
|
||||
"gpt-4o": { contextWindow: 1_000_000 },
|
||||
},
|
||||
},
|
||||
},
|
||||
}),
|
||||
);
|
||||
modelRegistry = new ModelRegistry(authStorage, modelsConfigPath);
|
||||
|
||||
const primaryModel = modelRegistry.find("anthropic", "claude-sonnet-4-5");
|
||||
const smallFallback = modelRegistry.find("openai", "gpt-4o-mini");
|
||||
const largeFallback = modelRegistry.find("openai", "gpt-4o");
|
||||
if (!primaryModel || !smallFallback || !largeFallback) {
|
||||
throw new Error("Expected override models to resolve");
|
||||
}
|
||||
expect(primaryModel.contextWindow).toBe(1_000_000);
|
||||
expect(smallFallback.contextWindow).toBe(4000);
|
||||
expect(largeFallback.contextWindow).toBe(1_000_000);
|
||||
|
||||
// ~15k estimated tokens in the initial prompt: fits the 1M primary and the
|
||||
// 1M large fallback, but far exceeds the 4000-window small fallback
|
||||
// (80% => 3200), so the small fallback cannot legally receive the request.
|
||||
const bigText = "lorem ipsum ".repeat(5000);
|
||||
const requestedModels: string[] = [];
|
||||
const mock = createMockModel();
|
||||
let primaryAttempts = 0;
|
||||
const agent = new Agent({
|
||||
getApiKey: model => `${model.provider}-test-key`,
|
||||
initialState: {
|
||||
model: primaryModel,
|
||||
systemPrompt: ["Test"],
|
||||
tools: [],
|
||||
messages: [],
|
||||
},
|
||||
streamFn: (model, context, options) => {
|
||||
requestedModels.push(`${model.provider}/${model.id}`);
|
||||
if (model.id === primaryModel.id && primaryAttempts === 0) {
|
||||
primaryAttempts += 1;
|
||||
mock.push({ throw: "rate limit exceeded retry-after-ms=200" });
|
||||
} else {
|
||||
mock.push({ content: ["ok"] });
|
||||
}
|
||||
return mock.stream(model, context, options);
|
||||
},
|
||||
});
|
||||
|
||||
const settings = Settings.isolated({
|
||||
"compaction.enabled": true,
|
||||
"compaction.strategy": "context-full",
|
||||
"compaction.thresholdPercent": 80,
|
||||
"compaction.thresholdTokens": -1,
|
||||
"contextPromotion.enabled": true,
|
||||
"retry.baseDelayMs": 5,
|
||||
"retry.fallbackChains": {
|
||||
default: [`${smallFallback.provider}/${smallFallback.id}`, `${largeFallback.provider}/${largeFallback.id}`],
|
||||
},
|
||||
});
|
||||
settings.setModelRole("default", `${primaryModel.provider}/${primaryModel.id}`);
|
||||
|
||||
session = new AgentSession({
|
||||
agent,
|
||||
sessionManager: SessionManager.inMemory(),
|
||||
settings,
|
||||
modelRegistry,
|
||||
});
|
||||
const now = Date.now();
|
||||
vi.spyOn(Date, "now").mockImplementation(() => now);
|
||||
|
||||
// Primary rate-limits with a live ~15k context; the retry-fallback path
|
||||
// must skip the 4000-window candidate and land on the 1M-window one.
|
||||
await session.prompt(bigText);
|
||||
await session.waitForIdle();
|
||||
|
||||
expect(requestedModels).not.toContain(`${smallFallback.provider}/${smallFallback.id}`);
|
||||
expect(requestedModels).toContain(`${largeFallback.provider}/${largeFallback.id}`);
|
||||
expect(session.model?.id).toBe(largeFallback.id);
|
||||
expect(requestedModels.at(-1)).toBe(`${largeFallback.provider}/${largeFallback.id}`);
|
||||
});
|
||||
|
||||
it("restores routed fallback primaries after cooldown expiry", async () => {
|
||||
const openRouterModel = getBundledModel("openrouter", "z-ai/glm-4.7");
|
||||
const fallbackModel = getBundledModel("openai", "gpt-4o-mini");
|
||||
|
||||
@@ -56,6 +56,7 @@ function createHost(
|
||||
modelRegistry,
|
||||
configWarnings: [],
|
||||
model: () => model,
|
||||
contextFitsModel: () => true,
|
||||
textOutputCommitted: () => options.textOutputCommitted !== false,
|
||||
thinkingLevel: () => undefined,
|
||||
configuredThinkingLevel: () => undefined,
|
||||
|
||||
Reference in New Issue
Block a user