Skip to content

refactor: rewrite reasoning per step through the engine before its first resend - #721

Merged
LeXwDeX merged 6 commits into
mainfrom
refactor/reasoning-rewrite-engine
Oct 8, 2026
Merged

LeXwDeX merged 6 commits into
mainfrom
refactor/reasoning-rewrite-engine

Conversation

@LeXwDeX

@LeXwDeX LeXwDeX commented Oct 8, 2026

Copy link
Copy Markdown
Owner

Closes #720

Why

Reasoning distillation organized a completed turn after it ended. A step's reasoning is first resent by the next step of
the same turn, so the rewrite landed after earlier requests had already sent the original: the provider prompt cache broke
from that point, within-turn resends kept the original, and a quick follow-up message made adoption fail for the whole
turn. The two hosts used different organizer transports (AI SDK with an openaiCompatible-only effort option vs the
@opencode-ai/llm engine), core decided editability by sniffing SDK metadata key names, and about 11.7k lines of
native-wire projector code had no production caller.

What changed

  • Per-step jobs with a send barrier. A job starts at reasoning-end, overlapping the rest of the step and its tools.
    Before every provider request the session barrier waits for in-flight jobs up to
    OPENCODE_REASONING_DISTILLATION_SETTLE_MS (default 3000 ms), then seals and interrupts the rest; sealed jobs never
    adopt, so a part is rewritten before its first resend or never. Adoption is validated per part (not sealed, session not
    reverted, persisted part unchanged).
  • One engine organizer. Both hosts organize through @opencode-ai/llm (no tools, temperature 0, no retries,
    none/low effort per protocol); the opencode host keeps AI SDK only as a fallback transport for packages the engine
    does not route. Parts are organized concurrently and independently. The budget counts reported or estimated tokens
    instead of pausing on missing usage or capping calls.
  • Engine-owned carrier classification. ReasoningCarrier.classify in @opencode-ai/llm decides plain / plaintext
    mirror / opaque (signed, encrypted, referenced) / unknown.
  • Real savings comparison. A script-calibrated estimate (measured DeepSeek and GLM tokenizers) replaces the
    conservative reserve estimator for "is the rewrite smaller", which had judged Chinese rewrites of English reasoning as
    larger.
  • Field preservation. All-noise output becomes a non-empty placeholder so reasoning_content stays present.
  • Removed. The unreachable native-wire projector and propose/judge/claims/gates pipeline, the native-runtime hook, and
    the core runner's unused per-turn history conversion and llm.prepare.
  • Lint warnings on touched lines fixed; ratchet lowered 4850 → 4750.

No persisted-format, HTTP or event shape change; no migration. The compatibility config entry is accepted and ignored.
Details and measurements: docs/reasoning-rewrite-engine-2026-10-08.md.

Evidence

  • Workspace typecheck 31/31; lint 0 errors, 4,738 warnings.
  • packages/core 1,459 pass; packages/llm 325 pass; packages/opencode 5,241 pass (full suite before the final review
    fixes; the touched files were rerun after them: 337 pass); packages/tui 282 pass; DAG-core gate passed.
  • New coverage: barrier adopt/seal semantics and early cleanup, per-part adoption after later messages, placeholder keeps
    reasoning_content, carrier classification, calibrated estimator, organizer engine transport (effort, no retry,
    abort), and integration tests in both hosts showing the next step's request carries the rewrite.
  • Isolated smoke on the built host binary with the configured DeepSeek models (organizer deepseek at none, default
    window): the next step's and next turn's requests carried the rewrites (organizer ~0.5 s, adoption ~5 ms); with the
    feature off, originals were resent; no server errors. The installed-binary check over /usr/local/bin/opencode was
    not run (it requires sudo).

Checklist

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

🤖 Generated with Claude Code

…re its first resend

Reasoning distillation organized a completed turn after it ended, so a step's
reasoning was rewritten only after later requests had already sent it: the
provider prompt cache broke from that point, within-turn resends kept the
original, and a quick follow-up made adoption fail for the whole turn.

- Start one organizer call per reasoning part at reasoning-end, overlapping the
  rest of the step and its tools. Before each provider request a per-session
  barrier waits for in-flight jobs up to OPENCODE_REASONING_DISTILLATION_SETTLE_MS
  (default 3000) and seals the rest; sealed jobs never adopt, so a part is
  rewritten before its first resend or never. Adoption is validated per part.
- Organize through the @opencode-ai/llm engine in both hosts (no tools,
  temperature 0, no retries, none/low effort per protocol); the opencode host
  keeps AI SDK only as a fallback transport for packages the engine does not
  route. Budget counts reported or estimated tokens instead of pausing on
  missing usage or capping calls.
- Decide replay carriers in the engine (ReasoningCarrier.classify) instead of
  sniffing SDK metadata keys in core.
- Compare savings with a script-calibrated token estimate so a Chinese rewrite
  of English reasoning is judged by its real cost.
- Replace all-noise output with a non-empty placeholder so provider reasoning
  fields stay present.
- Remove the dead native-wire projector, propose/judge/claims/gates pipeline,
  native-runtime hook and the core runner's unused history conversion and
  llm.prepare.
- Fix warnings on touched lines and lower the lint ratchet to 4750.
@LeXwDeX
LeXwDeX marked this pull request as ready for review October 8, 2026 14:58
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-08T16:48:29.372816Z 3e007d4 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@LeXwDeX
LeXwDeX enabled auto-merge October 8, 2026 15:05
@LeXwDeX
LeXwDeX disabled auto-merge October 8, 2026 15:05
Sealed rewrites left no log line, so a part that stayed original because its
organizer missed the settle window was only inferable. Each barrier now logs
sealed work at info level and other barriers at debug level, with job, adopted
and sealed counts and the wait time.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8e71295eb9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/opencode/src/session/prompt.ts Outdated
Comment thread packages/opencode/src/session/reasoning-adoption.ts
Review follow-ups:

- Keep the opencode rewrite scheduler and budget in InstanceState so
  disposing a directory closes its scope and cancels pending organizer
  calls; the job owns an AbortController aborted on interruption so the
  provider request is cancelled on every path.
- Refuse adoption once the session has a newer assistant message in both
  runtimes. The in-process send barrier only covers one process; this
  durable fence keeps another process's already-sent attempt from being
  followed by a late rewrite.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ea69fdb23d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/opencode/src/session/reasoning-adoption.ts Outdated
Comment thread packages/opencode/src/session/prompt.ts
…eleted sessions

The opencode loop now treats its persisted assistant message as the claim and
re-reads the previous assistant message's settled reasoning after it, so a
rewrite another process commits between the history read and the claim is in
the request; the durable fence refuses any later one. The fence is a direct
query for later assistant rows. Deleting a session cancels its pending rewrite
jobs and releases its budget in both runtimes.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 049d8983d5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +482 to +484
// Send barrier: adopt finished reasoning rewrites before this request reads history; seal the rest so a part is
// rewritten before its first resend or never. Adoption changes earlier messages, so drop the incremental cursor.
if ((yield* rewrites.settle(session.id)).length > 0) cursors.delete(session.id)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Claim core-runner attempts before reading history

When two SessionRunner instances service the same session, this barrier seals only local rewrite jobs. A competing runner can pass it, snapshot the original history, and start llm.stream(request) before publisher.startAssistant() persists its attempt; during that interval an organizer in this runner sees no later assistant and adopts the rewrite. The competing request then sends the original reasoning while durable history contains the replacement. Persist/claim the assistant attempt before building the request (and reread afterward), as the opencode loop does.

AGENTS.md reference: AGENTS.md:L18-L18

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not changed in this PR, deliberately. Claiming first in the core runner means publishing Step.Started before the request, which leaves an empty assistant message on every overflow recovery (the runner relies on hasAssistantStarted() being false there) and changes step event order; a compensating rollback cannot clear distillation provenance through Reasoning.Ended. The residual window needs two runner processes on one session, a foreign adoption landing between history read and first stream event, and costs one prompt-cache miss, not history consistency. It is documented in adopt.ts and the design doc; leaving this thread open for the maintainer.

🤖 Reviewed by Claude Code

Comment thread packages/core/src/session/reasoning-distillation/engine.ts Outdated
Both organizer transports sent max_tokens 24576 regardless of the selected
small model, so providers that enforce a lower output cap rejected every
rewrite. The engine call and the AI-SDK fallback now clamp to the model's
declared output limit.
@LeXwDeX
LeXwDeX merged commit cb27632 into main Oct 8, 2026
13 checks passed
@LeXwDeX
LeXwDeX deleted the refactor/reasoning-rewrite-engine branch October 8, 2026 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor: rewrite reasoning per step through the engine before its first resend

1 participant