-
Notifications
You must be signed in to change notification settings - Fork 0
fix(opencode): stop overflow compaction from replaying a message that cannot fit #39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -380,6 +380,38 @@ const layer = Layer.effect( | |||||||||||||||||||||||
| }, | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| yield* session.updateMessage(msg) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // NOTE (Yuxin, 2026-09-03, REPL-31509): an overflow compaction whose previous | ||||||||||||||||||||||||
| // summary produced no finished assistant step is a loop, not progress — the | ||||||||||||||||||||||||
| // replayed message itself does not fit. Fail the turn on the summary row so | ||||||||||||||||||||||||
| // the pending compaction task is consumed instead of re-firing next prompt. | ||||||||||||||||||||||||
| // Checked against the full transcript: with a replay, `history` already | ||||||||||||||||||||||||
| // dropped the replayed turn, which is exactly where progress would show. | ||||||||||||||||||||||||
| const previous = prior.at(-1) | ||||||||||||||||||||||||
| const stalled = | ||||||||||||||||||||||||
| input.overflow === true && | ||||||||||||||||||||||||
| previous !== undefined && | ||||||||||||||||||||||||
| !input.messages | ||||||||||||||||||||||||
| .slice(previous.assistantIndex + 1) | ||||||||||||||||||||||||
| .some((m) => m.info.role === "assistant" && m.info.finish && !m.info.error) | ||||||||||||||||||||||||
|
Comment on lines
+394
to
+396
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Retained history defeats loop guard When a prior compaction preserves completed history, Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback. |
||||||||||||||||||||||||
| const replayTooLarge = | ||||||||||||||||||||||||
| replay !== undefined && Token.estimate(JSON.stringify(replay.parts)) >= usable({ cfg, model }) | ||||||||||||||||||||||||
|
Comment on lines
+397
to
+398
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Replay size uses wrong request
Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: When an overflow replay includes a data-URL media attachment, this check counts the full base64 URL and stops before replay's media stripping runs. Measure the transformed replay payload so large attachments retain the existing replay-with-placeholder path. Prompt for AI agents
Suggested change
|
||||||||||||||||||||||||
| if (stalled || replayTooLarge) { | ||||||||||||||||||||||||
| msg.error = new SessionV1.ContextOverflowError({ | ||||||||||||||||||||||||
| message: | ||||||||||||||||||||||||
| "The last message is too large for the model's context window even after compaction. Remove or shorten the large attachment, or start a new chat.", | ||||||||||||||||||||||||
| }).toObject() | ||||||||||||||||||||||||
| msg.finish = "error" | ||||||||||||||||||||||||
| msg.time.completed = Date.now() | ||||||||||||||||||||||||
| yield* session.updateMessage(msg) | ||||||||||||||||||||||||
| yield* Effect.logWarning("compaction stalled on oversized message", { | ||||||||||||||||||||||||
| sessionID: input.sessionID, | ||||||||||||||||||||||||
| stalled, | ||||||||||||||||||||||||
| replayTooLarge, | ||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||
| return "stop" | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| const processor = yield* processors.create({ | ||||||||||||||||||||||||
| assistantMessage: msg, | ||||||||||||||||||||||||
| sessionID: input.sessionID, | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -895,6 +895,116 @@ describe("session.compaction.process", () => { | |
| }).pipe(withCompaction({ result: "compact" })), | ||
| ) | ||
|
|
||
| // REPL-31509: an oversized message overflowed, was compacted, replayed, and | ||
| // overflowed again ~2,145 times. The second overflow compaction after a | ||
| // summary that produced no finished step must fail the turn instead. | ||
| itCompaction.instance( | ||
| "stops an overflow compaction when the previous summary produced no finished step", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: Neither new test exercises the replay-size guard. Both use small replayed text parts, so the branch Prompt for AI agents |
||
| Effect.gen(function* () { | ||
| const test = yield* TestInstance | ||
| const ssn = yield* SessionNs.Service | ||
| const session = yield* ssn.create({}) | ||
| yield* createUserMessage(session.id, "make this page") | ||
| const firstMarker = yield* createUserMessage(session.id, "") | ||
| yield* ssn.updatePart({ | ||
| id: PartID.ascending(), | ||
| messageID: firstMarker.id, | ||
| sessionID: session.id, | ||
| type: "compaction", | ||
| auto: true, | ||
| overflow: true, | ||
| }) | ||
| yield* createSummaryAssistantMessage(session.id, firstMarker.id, test.directory, "summary") | ||
| const replayed = yield* createUserMessage(session.id, "make this page") | ||
| yield* ssn.updateMessage({ | ||
| id: MessageID.ascending(), | ||
| role: "assistant", | ||
| sessionID: session.id, | ||
| mode: "build", | ||
| agent: "build", | ||
| path: { cwd: test.directory, root: test.directory }, | ||
| cost: 0, | ||
| tokens: { output: 0, input: 0, reasoning: 0, cache: { read: 0, write: 0 } }, | ||
| modelID: ref.modelID, | ||
| providerID: ref.providerID, | ||
| parentID: replayed.id, | ||
| time: { created: Date.now() }, | ||
| error: new SessionV1.ContextOverflowError({ message: "prompt is too long" }).toObject(), | ||
| }) | ||
| const secondMarker = yield* createUserMessage(session.id, "") | ||
| yield* ssn.updatePart({ | ||
| id: PartID.ascending(), | ||
| messageID: secondMarker.id, | ||
| sessionID: session.id, | ||
| type: "compaction", | ||
| auto: true, | ||
| overflow: true, | ||
| }) | ||
| const msgs = yield* ssn.messages({ sessionID: session.id }) | ||
|
|
||
| const result = yield* SessionCompaction.use.process({ | ||
| parentID: secondMarker.id, | ||
| messages: msgs, | ||
| sessionID: session.id, | ||
| auto: true, | ||
| overflow: true, | ||
| }) | ||
|
|
||
| const summaries = (yield* ssn.messages({ sessionID: session.id })).filter( | ||
| (msg) => msg.info.role === "assistant" && msg.info.summary, | ||
| ) | ||
| const last = summaries.at(-1) | ||
| expect(result).toBe("stop") | ||
| expect(last?.info.role).toBe("assistant") | ||
| if (last?.info.role === "assistant") { | ||
| expect(last.info.finish).toBe("error") | ||
| expect(JSON.stringify(last.info.error)).toContain("too large for the model's context window") | ||
| } | ||
| }).pipe(withCompaction({ result: "continue" })), | ||
| ) | ||
|
|
||
| itCompaction.instance( | ||
| "still compacts on overflow when a finished step followed the previous summary", | ||
| Effect.gen(function* () { | ||
| const test = yield* TestInstance | ||
| const ssn = yield* SessionNs.Service | ||
| const session = yield* ssn.create({}) | ||
| yield* createUserMessage(session.id, "make this page") | ||
| const firstMarker = yield* createUserMessage(session.id, "") | ||
| yield* ssn.updatePart({ | ||
| id: PartID.ascending(), | ||
| messageID: firstMarker.id, | ||
| sessionID: session.id, | ||
| type: "compaction", | ||
| auto: true, | ||
| overflow: true, | ||
| }) | ||
| yield* createSummaryAssistantMessage(session.id, firstMarker.id, test.directory, "summary") | ||
| const next = yield* createUserMessage(session.id, "now tweak the header") | ||
| yield* createAssistantMessage(session.id, next.id, test.directory) | ||
| const secondMarker = yield* createUserMessage(session.id, "") | ||
| yield* ssn.updatePart({ | ||
| id: PartID.ascending(), | ||
| messageID: secondMarker.id, | ||
| sessionID: session.id, | ||
| type: "compaction", | ||
| auto: true, | ||
| overflow: true, | ||
| }) | ||
| const msgs = yield* ssn.messages({ sessionID: session.id }) | ||
|
|
||
| const result = yield* SessionCompaction.use.process({ | ||
| parentID: secondMarker.id, | ||
| messages: msgs, | ||
| sessionID: session.id, | ||
| auto: true, | ||
| overflow: true, | ||
| }) | ||
|
|
||
| expect(result).toBe("continue") | ||
| }).pipe(withCompaction({ result: "continue" })), | ||
| ) | ||
|
|
||
| it.instance( | ||
| "adds synthetic continue prompt when auto is enabled", | ||
| Effect.gen(function* () { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Identify assistant progress by message identity or chronology instead of the array index. Retained pre-compaction messages can be reordered after the summary, allowing this slice to count an old finished assistant as new progress and continue the oversized-message compaction loop.
Prompt for AI agents