Skip to content

session: optional stripReasoning for compaction and cross-model safety#25184

Closed
jackmazac wants to merge 1 commit intoanomalyco:devfrom
jackmazac:pr/upstream-1-reasoning-plumbing
Closed

session: optional stripReasoning for compaction and cross-model safety#25184
jackmazac wants to merge 1 commit intoanomalyco:devfrom
jackmazac:pr/upstream-1-reasoning-plumbing

Conversation

@jackmazac
Copy link
Copy Markdown

Summary

When the app summarizes a long chat (compaction) or you switch models mid-session, hidden “reasoning” data from the old model can leak into the next request. Providers then reject the call or behave inconsistently. This change gives the session layer an explicit switch to drop that reasoning for compaction, while keeping normal chat behavior unchanged.

Problem (plain language)

Long conversations are periodically summarized so they fit the model’s context window. That summary step should read like a normal transcript: goals, decisions, and tool outcomes—not low-level reasoning blobs tied to a specific model. If those blobs are forwarded anyway, the next model may error (for example missing signatures or wrong format) or users see confusing internal content in places it was never meant to go.

Separately, when you change which model is answering, reasoning attached to an earlier model is often not valid for the new one. The UI already treats “different model” as a special case; the conversion path to provider messages should align with that.

Technical breakdown

  1. MessageV2.toModelMessagesEffect converts stored session parts into the wire format the AI SDK sends to providers. It already supports stripping media and truncating large tool output (toolOutputMaxChars). It did not support omitting reasoning parts for specific call sites (notably compaction).

  2. Compaction builds a shorter context for the compaction model. That path should not include assistant reasoning parts from the pre-compaction history, because they are not part of the user-visible story and can break or bloat the compaction request.

  3. Cross-model turns: when the assistant message was produced with a different providerID/modelID than the target model, we already omit some provider metadata on text tools. Reasoning parts should follow the same rule: skip them when the model changed, instead of sending partial or incompatible reasoning metadata.

  4. When we keep reasoning (same model, normal chat), reasoning parts should carry providerMetadata consistently so providers that need it still receive it.

Solution

  • Extend toModelMessagesEffect / toModelMessages options with optional stripReasoning?: boolean (alongside existing stripMedia and toolOutputMaxChars).
  • In the reasoning branch: if stripReasoning or differentModel, omit the reasoning part; otherwise emit reasoning with providerMetadata from the stored part.
  • In compaction, call toModelMessagesEffect with stripReasoning: true in addition to existing stripMedia and toolOutputMaxChars.

Files

  • packages/opencode/src/session/message-v2.ts
  • packages/opencode/src/session/compaction.ts

How to test

From packages/opencode:

bun typecheck
bun test test/session/message-v2.test.ts

Merge order

None. This PR is safe to merge first. PR 2–4 in this series do not depend on this branch for compilation (they touch other files), but landing this first keeps the story and review surface clear.

Notes for reviewers

  • No behavior change for default toModelMessagesEffect calls: stripReasoning is opt-in.
  • Compaction is the only call site that sets stripReasoning: true in this PR.

Opt-in stripReasoning on toModelMessagesEffect; compaction enables it
alongside stripMedia and toolOutputMaxChars. Skip reasoning when model
changed; keep providerMetadata when same model.
@github-actions github-actions Bot added the needs:compliance This means the issue will auto-close after 2 hours. label May 1, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

This PR doesn't fully meet our contributing guidelines and PR template.

What needs to be fixed:

  • PR description is missing required template sections. Please use the PR template.

Please edit this PR description to address the above within 2 hours, or it will be automatically closed.

If you believe this was flagged incorrectly, please let a maintainer know.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

Hey! Your PR title session: optional stripReasoning for compaction and cross-model safety doesn't follow conventional commit format.

Please update it to start with one of:

  • feat: or feat(scope): new feature
  • fix: or fix(scope): bug fix
  • docs: or docs(scope): documentation changes
  • chore: or chore(scope): maintenance tasks
  • refactor: or refactor(scope): code refactoring
  • test: or test(scope): adding or updating tests

Where scope is the package name (e.g., app, desktop, opencode).

See CONTRIBUTING.md for details.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 1, 2026

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

Based on the search results, I found a potentially related PR:

Related PR Found:

Why it's related: This PR also addresses reasoning handling across model switches and provider metadata preservation. It appears to cover similar territory around preserving/managing reasoning data when models change, which is directly mentioned in the current PR's "Cross-model turns" section (point 3 of the technical breakdown).

The current PR (#25184) builds upon these concerns by adding explicit stripReasoning control for compaction and cross-model scenarios, so these may be related work or this PR might supersede/complement that earlier fix.

@jackmazac
Copy link
Copy Markdown
Author

Closing to reopen with a linked issue and the repo PR template per CONTRIBUTING.

@jackmazac jackmazac closed this May 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs:compliance This means the issue will auto-close after 2 hours. needs:title

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant