Repository navigation
fix(core): compact when a reply stops early at the output limit - #58994
rekram1-node wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
The change works as described. On the base, the new scenario continues without compacting. With this PR it compacts once and then continues. The runner test file passes, and tsc -b passes for packages/core.
Public API: unchanged. OutputLimit.early is internal to the core runner.
Two things are worth fixing before merge (details inline):
- The compaction summarizes the context loaded before the step, so the cut-off reply is neither summarized nor kept. The continue nudge then refers to output the model can no longer see.
- "Early" is measured against the catalog
limit.outputinstead of the limit actually requested. A plugin or config that lowersmaxTokens, or the OpenAI plugin removing it, turns every ordinary length stop into an overflow compaction.
Related: #52015
| // Stopping short of the catalog limit means the context window ran out, so continuing alone cannot help. | ||
| if (outcome.early && recoverOverflow) { | ||
| recoverOverflow = false | ||
| yield* compaction.compact({ reason: "overflow", context: loaded }) |
There was a problem hiding this comment.
loaded was taken before this step ran, so it doesn't include the reply that just stopped with length. The summary leaves that reply out, and afterwards it's no longer in the model's context either: the new test ends with context [compaction, synthetic, assistant], and the partial reply only exists in the stored messages. CONTINUE_AFTER_OUTPUT_LIMIT then asks the model to carry on from output it can't see, so whatever the cut-off reply had produced is lost. Could you reload the context (prepareContext(sessionID).pipe(Effect.flatMap(context.load))) before compacting, so the cut-off reply is summarized or kept?
| if (record.finish?.finish === "length") return Outcome.OutputLimit({ needsContinuation }) | ||
| if (record.finish?.finish === "length") { | ||
| const output = record.finish.usage?.outputTokens | ||
| const early = output !== undefined && input.model.limit.output > 0 && output < input.model.limit.output |
There was a problem hiding this comment.
This compares output tokens with the catalog limit.output, not with the limit actually sent. The two can differ for reasons that have nothing to do with the context window: a context hook or the model options can lower maxTokens, and the OpenAI plugin removes maxTokens altogether, so the provider's default applies. In those cases a normal stop at the full allowance is treated as early and triggers an expensive overflow compaction. Comparing with input.prepared.request.generation?.maxTokens would avoid that. The reply would count as early when it stops below that value, or when that value was cut below the catalog limit to fit the context window.
190e309 to
7c58b70
Compare
There was a problem hiding this comment.
The new commit rebases the change onto the latest dev. The fix itself is unchanged, so both earlier points are still open: compaction still summarizes the context from before the step, which leaves out the cut-off reply, and "stopped early" is still measured against the catalog limit.output rather than the requested limit. On the rebased head, test/session-runner.test.ts passes (234 tests) and tsc -b passes for packages/core. The base now passes an estimate to compaction, but it's optional, so the new overflow call without it is still correct. Public API: unchanged. Nothing new beyond that.
A reply can end with
lengthfor two different reasons:limit.output). Asking it to continue is the right call.max_tokensdown to whatever room is left in the context window, some servers stop early once the window fills, and Anthropic'smodel_context_window_exceededalso maps tolength. Here "continue" doesn't help, since the next request has even less room.Today both cases get the same continue nudge. With this change, when a length stop reports fewer output tokens than the catalog limit, I run one overflow compaction first (same path and same once-per-step guard as the "too long" recovery), then send the usual continue message.
Before:
After:
A full-allowance length stop, or one without reported usage, works exactly as before. If compaction fails or is skipped, it just falls through to the normal continue. The 3-in-a-row output-limit check is unchanged, and the cut-off reply stays in history.
Added by the opencode bot (AI-generated).
Related: #52015