Skip to content

fix(opencode): reject empty or length-cut compaction summaries - #52786

Open
pauldupuyjr wants to merge 2 commits into
anomalyco:devfrom
pauldupuyjr:fix/compaction-empty-summary
Open

fix(opencode): reject empty or length-cut compaction summaries#52786
pauldupuyjr wants to merge 2 commits into
anomalyco:devfrom
pauldupuyjr:fix/compaction-empty-summary

Conversation

@pauldupuyjr

@pauldupuyjr pauldupuyjr commented Oct 2, 2026

Copy link
Copy Markdown

Issue for this PR

Closes #44080
Closes #41571

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

Compaction could succeed with a summary that has no text (the model spent its whole output budget on reasoning) or that was cut off by the output length limit. That empty or partial summary still became the compaction boundary, so everything before it was hidden and the session carried on with no context.

After the summary call, if nothing has errored yet and the summary has no text or finished with length, the summary message is now marked as an error and compaction stops. completedCompactions() already skips errored summaries, so the earlier history stays visible and Compacted is not published.

This re-derives the guard from #42063 by @vladislav-miroshnikov (closed for age, not rejected), which covered empty text only; this also rejects summaries cut off by finish: "length".

How did you verify your code works?

Three tests in packages/opencode/test/session/compaction.test.ts: a reasoning-only summary is rejected, a length-finished summary (with or without partial text) is rejected, and a normal text summary still compacts. The two rejection tests fail on dev without the fix.

I also ran a real session against a reasoning model (GLM 5.3 Flash via OpenRouter) with the compaction model's output capped at 64 tokens and tail turns off. On dev the cut-off summary was accepted and the next turn had lost the earlier messages; with this change the summary is marked as an error and the next turn still sees the full history.

Screenshots / recordings

Not a UI change.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

pauldupuyjr and others added 2 commits October 1, 2026 19:13
@pauldupuyjr @claude GLM-5.3
fix(opencode): reject empty or length-cut compaction summaries
ebf58df 
A compaction whose summary message has no non-empty text part, or whose
summary call ended on finish reason "length", no longer becomes the
history boundary. The summary message is errored and the process stops,
so the original history stays visible instead of being silently dropped
behind an empty or truncated summary.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: GLM-5.3 <[email protected]>
@pauldupuyjr @claude GLM-5.3
fix(opencode): key compaction guard error message off finish reason
a40dccd 
The empty-summary guard picked its error text from summary presence,
so a length-cut run that produced no text reported "Compaction
produced no summary" instead of the length-limit message. Select the
message from the finish reason first so both failure shapes report
accurately.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: GLM-5.3 <[email protected]>
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

The following comment was made by an LLM, it may be inaccurate:

@wluisdev

wluisdev commented Oct 7, 2026

Copy link
Copy Markdown

I hit the same empty-summary issue independently and ended up testing a very similar guard locally.

One thing I’m not completely sure about is rejecting every finish === "length" case.
I tested this with Qwen3.8-27B through llama.cpp, using an 8K max output for compaction:

  • The bad case was a compaction that used all 8192 output tokens on reasoning, produced 34K chars of reasoning, but no text summary at all. OpenCode accepted it as a valid boundary, and the previous task context effectively disappeared.
  • In another 55 minute agentic run, though, 4 out of 14 completed compactions ended with finish: "length" and still had a useful text summary. They were truncated near the end, usually around the file list or the last next step, but they still kept the objective, all task requirements, and the current work state.

Those summaries were also carried forward as <prior-summary> into the next compaction and kept the task alive across the chain.
I also replayed the original empty response byte-for-byte through the real TUI, after a valid first /compact. With the text-only guard, the empty compaction was rejected and the previous valid summary stayed as the boundary.

My concern with || finish === "length" is that it also rejects summaries that are truncated but still useful. In my run, the first one would have stopped the session about 5 minutes in. Retrying with the same output budget can hit length again, so this can make compaction hard to use with reasoning models that regularly consume most of the output budget.

I think a narrower rule would be less restrictive while still protecting against the zero-text failure:

  • reject when the persisted summary has no non-whitespace text
  • keep length + non-empty text accepted
  • optionally log or warn when the summary was truncated

I’d also suggest covering these cases in tests:

  • reasoning-only + length + no text → reject, previous boundary preserved
  • whitespace-only text → reject
  • length + non-empty text → accept
  • valid previous summary + new empty compaction → previous summary remains the boundary and is reused as <prior-summary>

One small extra: publishing Session.Event.Error on rejection makes the stop visible in the TUI instead of looking like a silent halt.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

2 participants

@pauldupuyjr @wluisdev