feat(compaction): clearer summary prompt structure for smaller models - #58
Merged
Conversation
Hand-merged from anomalyco/opencode@dab2637217 ("fix(compaction): adjust instructions and structure to be more clear to smaller models like dsv4 flash", anomalyco#42045). Their patch didn't cherry-pick cleanly -- it conflicts with TKT-379 (the compaction summary must NOT hide that context was compacted; hiding it measurably drops history_search's call rate from 100% to 20%) and with our own splitPrefix/splitSuffix/ retainedCount extension to select(). Reapplied by hand, taking the genuinely good parts and explicitly not the rest: TAKEN: - New SUMMARY_UPDATE_INSTRUCTIONS block: explicit guidance for combining a <prior-summary> with new <conversation> content (carry forward objectives/constraints/decisions, conversation wins on conflict, move Active->Completed, update Objective/Next Move). - buildPrompt() restructured to wrap the actual conversation content in explicit <conversation> tags up front, before the instructions, instead of spreading context items loose at the end of the prompt array. This is the actual "clearer to smaller models" improvement. - Tag renamed <previous-summary> -> <prior-summary> to match. Fixed every place that tag name is asserted or referenced, including in the SECOND compaction pipeline (packages/opencode/src/session/ compaction.ts) that turns out to share this same buildPrompt() -- and its own agent system prompt (packages/core/src/plugin/agent.ts PROMPT_COMPACTION / packages/opencode/src/agent/prompt/ compaction.txt), which told the model to look for the old tag name. NOT TAKEN, deliberately (Sean: "don't take their side"): - Upstream's version of this same instruction says "Do not mention the summary process or that context was compacted" -- the exact opposite of TKT-379's fix. Kept our version unchanged. Added a test pinning this so a future upstream sync can't silently reintroduce it via a cleaner-looking cherry-pick. - Upstream's select() also drops splitPrefix/splitSuffix (character- level splitting of a message straddling the retention boundary) in favor of whole-message-only slicing. Our fork extended that same logic with retainedCount (feeds SessionEvent.Compaction.Ended.retainedTailMessages) -- left entirely untouched. NOT TOUCHED, flagged to feedback instead (anomalyco#227): PROMPT_COMPACTION's own "do not mention... compacting" sentence (a second, separate instance of the same class of bug TKT-379 fixed) and the packages/core/src/v1/config/config.ts tail_turns docstring clarity change -- both belong to the second compaction pipeline, whose liveness relative to the first is TKT-377's still-open question, not something to resolve as a side effect of this hand-merge. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (2)
packages/opencode/test/session/compaction.test.ts:1491
- This test currently only asserts that the prior summary tag/template sections appear, but it doesn’t pin the new
<conversation>...</conversation>prompt structure thatbuildPrompt()now requires. That leaves room for regressions where the real conversation is appended outside the<conversation>block (or the block is empty), which smaller models are especially sensitive to.
expect(captured).toContain("<prior-summary>")
expect(captured).toContain("summary one")
expect(captured.match(/summary one/g)?.length).toBe(1)
expect(captured).toContain("## Important Details")
expect(captured).toContain("## Work State")
packages/core/src/session/compaction.ts:266
buildPrompt()now embedsinput.contextinside<conversation>...</conversation>and the returned instructions explicitly tell the model to summarize the conversation in those tags. Howeverpackages/opencode/src/session/compaction.tsimports this helper but still appends the real serialized conversation separately ("The following is the conversation history:") while passing plugin-providedcontext(documented as additional context appended to the default prompt) intobuildPrompt. When no plugin setscontext, the<conversation>block is empty and the prompt’s instructions become misleading for that pipeline, likely degrading compaction quality.
export const buildPrompt = (input: { readonly previousSummary?: string; readonly context: readonly string[] }) => {
const conversation = `Here is the conversation so far:\n\n<conversation>\n${input.context.join("\n\n")}\n</conversation>`
if (!input.previousSummary)
return [
conversation,
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue for this PR
Closes #
Type of change
What does this PR do?
Hand-merged from
anomalyco/opencode@dab2637217("fix(compaction): adjust instructions and structure to be more clear to smaller models like dsv4 flash", anomalyco#42045) — one of the 35 commits upstream is ahead by. It did not cherry-pick cleanly: it conflicts with two things this fork deliberately did on purpose, so this required a careful hand-merge, not a blind take.Conflict 1 — TKT-379. The compaction summary prompt must not hide that context was compacted: TKT-379 (diary 2610) measured that hiding it drops
history_search's call rate from 100% to 20%, because the model gets no cue that older detail might still exist. Upstream's version of this same prompt instructs the opposite — "Do not mention the summary process or that context was compacted." Sean's explicit call on this: keep our version, don't take theirs.Conflict 2 — our fork's
select()independently extended message-splitting withsplitPrefix/splitSuffix(character-level splitting of a message straddling the retention boundary) plusretainedCount(feedsSessionEvent.Compaction.Ended.retainedTailMessages). Upstream's patch removes that logic entirely in favor of whole-message-only slicing. Left ours untouched.What I took, and what I didn't
Taken — the actual "clearer to smaller models" improvement:
SUMMARY_UPDATE_INSTRUCTIONSblock: explicit guidance for combining a prior summary with new conversation content (carry forward objectives/constraints/decisions even if unmentioned in the new turns, newer conversation wins on conflict, moveActive→Completed, keepObjective/Next Movecurrent).buildPrompt()restructured to wrap the actual conversation content in explicit<conversation>tags up front, before the instructions, instead of spreading context items loose at the end of a joined array. This is the real clarity win for a smaller model reading the prompt.<previous-summary>→<prior-summary>to match upstream. This turned out to have a real, non-optional follow-on:buildPrompt()is shared with a second compaction pipeline (packages/opencode/src/session/compaction.ts, which invokes a hiddencompactionagent viaagents.get("compaction")) — its own test asserted the old tag literal, and its agent's own system prompt (packages/core/src/plugin/agent.tsPROMPT_COMPACTION, mirrored inpackages/opencode/src/agent/prompt/compaction.txt) told the model to look for<previous-summary>by name. Fixed the tag name everywhere it's referenced so the two stay consistent — this is a direct consequence of the rename, not scope creep.Not taken, deliberately:
session-compaction.test.ts) that pins our version and fails if a future sync silently reintroduces theirs — a cleaner-looking future cherry-pick shouldn't be able to slip this past review again.splitPrefix/splitSuffix/retainedCount(see Conflict 2). Untouched.Not touched at all, flagged to feedback instead (anomalyco#227):
PROMPT_COMPACTION's own "do not mention...compacting" sentence is a second, separate instance of the exact class of bug TKT-379 fixed — but it belongs to the second compaction pipeline, and whether that pipeline is actually live relative to the first is TKT-377's own still-open question. Fixing it as a side effect of this hand-merge would be resolving that question by accident rather than on purpose. Also leftpackages/core/src/v1/config/config.ts'stail_turnsdocstring clarity change out — it documents behavior in that same second pipeline, which this PR doesn't touch.How did you verify your code works?
EXPECTATION: the new prompt structure wraps conversation content in
<conversation>tags before the instructions, and the update-instructions block appears correctly for the prior-summary case.HOW EXERCISED:
bun test --cwd packages/core test/session-compaction.test.ts— ported upstream's two new test cases (structure ordering, update-instructions content) adapted to our function signature.OBSERVED: pass.
VERDICT: pass.
EXPECTATION: the compaction prompt still requires the compaction marker to stay visible — i.e., we did NOT take upstream's opposite instruction.
HOW EXERCISED: new test,
compaction prompt still requires the compaction marker to stay visible (TKT-379 -- do not adopt upstream's opposite instruction)— asserts the TKT-379 sentence is present and upstream's exact sentence is absent.OBSERVED: pass.
VERDICT: pass.
EXPECTATION: the tag rename doesn't silently break the second, buildPrompt-sharing compaction pipeline.
HOW EXERCISED:
bun test --cwd packages/opencode test/session/compaction.test.ts(updated its one<previous-summary>assertion to<prior-summary>) andbun test --cwd packages/core test/session-runner.test.ts(same fix, one assertion).OBSERVED: opencode:
56 pass, 1 skip, 0 fail. core session-runner: included in the 95-pass run below.VERDICT: pass.
EXPECTATION: nothing else in the compaction/anchor/history-search area regressed.
HOW EXERCISED:
bun test --cwd packages/core test/session-compaction.test.ts test/session-runner.test.ts(95 pass),test/session-history-search.test.ts test/session-compaction-versioning.test.ts(7 pass).OBSERVED:
95 pass, 0 fail;7 pass, 0 fail.VERDICT: pass.
EXPECTATION: typecheck clean.
HOW EXERCISED:
bun run --cwd packages/core typecheck,bun run --cwd packages/opencode typecheck, plus the repo's pre-push hook (bun turbo typecheck, all 38 packages).OBSERVED: all clean; pre-push:
32 successful, 32 total.VERDICT: pass.
Screenshots / recordings
Not applicable — prompt/text-only change, no UI.
Checklist