Repository navigation
refactor: rewrite reasoning per step through the engine before its first resend - #721
Conversation
…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.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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.
There was a problem hiding this comment.
💡 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".
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.
There was a problem hiding this comment.
💡 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".
…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.
There was a problem hiding this comment.
💡 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".
| // 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) |
There was a problem hiding this comment.
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 👍 / 👎.
There was a problem hiding this comment.
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
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.
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/llmengine), core decided editability by sniffing SDK metadata key names, and about 11.7k lines ofnative-wire projector code had no production caller.
What changed
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 neveradopt, so a part is rewritten before its first resend or never. Adoption is validated per part (not sealed, session not
reverted, persisted part unchanged).
@opencode-ai/llm(no tools, temperature 0, no retries,none/loweffort per protocol); the opencode host keeps AI SDK only as a fallback transport for packages the enginedoes not route. Parts are organized concurrently and independently. The budget counts reported or estimated tokens
instead of pausing on missing usage or capping calls.
ReasoningCarrier.classifyin@opencode-ai/llmdecides plain / plaintextmirror / opaque (signed, encrypted, referenced) / unknown.
conservative reserve estimator for "is the rewrite smaller", which had judged Chinese rewrites of English reasoning as
larger.
reasoning_contentstays present.the core runner's unused per-turn history conversion and
llm.prepare.No persisted-format, HTTP or event shape change; no migration. The
compatibilityconfig entry is accepted and ignored.Details and measurements:
docs/reasoning-rewrite-engine-2026-10-08.md.Evidence
packages/core1,459 pass;packages/llm325 pass;packages/opencode5,241 pass (full suite before the final reviewfixes; the touched files were rerun after them: 337 pass);
packages/tui282 pass; DAG-core gate passed.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.
deepseekatnone, defaultwindow): 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/opencodewasnot run (it requires sudo).
Checklist
🤖 Generated with Claude Code