Skip to content

fix: keep Responses assistant history valid after compression (#564) - #565

Merged
ranxianglei merged 1 commit into
masterfrom
2026-09-06_responses-assistant-ordering
Sep 6, 2026
Merged

ranxianglei merged 1 commit into
masterfrom
2026-09-06_responses-assistant-ordering

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

What

Fixes #564: after a successful compression, bili could send an invalid Responses assistant history upstream, producing repeated 400 invalid_assistant_history:

input Item 9 reasoning cannot follow another reasoning Item; supported assistant Item order is reasoning, message content, then function calls
input Item 56 reasoning cannot follow assistant message content

Root cause

Responses item order equals CoreMessage order (coreToResponses maps each core message to an item in sequence). Two original assistant turns were being merged into one run by two steps:

  1. folding removes the tool calls/results that separated two turns, and
  2. stripKernelSummaries() then deletes the acp_summary_<block> that was the last remaining separator (the block's compress call is present in history, so the summary is redundant and gets stripped).

Result: [reasoningA][messageA][reasoningB][messageB] — a single run violating the Responses invariant (within a run the order must be reasoning* -> message* -> function_call*, with at most one reasoning item). This is Responses-specific: OpenAI chat merges consecutive assistant messages into one object, so it never surfaces there.

Distinct from #539 (which dropped the reasoning_content field on re-request): here the existing reasoning items survive, but their order relative to assistant text/tool-calls is corrupted.

Fix

New repairResponsesAssistantOrdering(folded, original) in src/server.ts, applied after summary dedup at all three Responses wire-build sites:

  • prepareResponses (initial request / plugin-mode replay)
  • prepareResponsesCompact (forged compacted history handed back to the client)
  • refreshFolded (post-compress internal re-request)

It rebuilds turn boundaries from the original client history:

  • drops a reasoning whose turn's non-reasoning body was wholly folded away (its content now lives in the block summary), while keeping originally-reasoning-only turns' reasoning;
  • otherwise inserts a neutral user-role marker between the merged runs.

Signed reasoning items are preserved verbatim (encrypted_content untouched); the original history and compression blocks are never mutated.

Tests

tests/issue564-responses-ordering.test.ts:

  • end-to-end: drives the real runCompressLoop + responses adapter through a live compress and asserts on the actual re-request body (captured via a fetch probe) — not just the pre-dedup fold output;
  • unit: orphaned-reasoning drop, reasoning-only-turn retention (runs stay separated), and surviving-acp_summary-carrier preservation (no spurious marker injected).

Pre-flight

  • npm run typecheck — clean
  • npm test — 1051 pass / 1 fail. The single failure (launcher.test.ts: resolveClientCommand: codex/claude resolve to themselves) is pre-existing and environmental: this checkout has /usr/bin/codex installed, so resolution returns the full path where the test expects the bare name. Reproduced identically on pristine master; unrelated to this change.
  • npm run build — clean

Note: the real-codex-through-bili E2E suite (tests/e2e/e2e-codex.test.ts) requires a local Responses-compatible upstream plus the codex binary and could not be executed in this sandbox (loopback upstreams are blocked by network policy); the new regression pins the final wire ordering directly instead.

Folding removes the tool calls/results that separated two assistant turns, and stripKernelSummaries can then delete the acp_summary that was the last separator - merging two turns into one run that Responses rejects with invalid_assistant_history.

Add repairResponsesAssistantOrdering(), applied AFTER summary dedup at all three Responses wire-build sites (initial prepare, compact forge, compress re-request). It rebuilds turn boundaries from the ORIGINAL client history: drop a reasoning whose turn body was wholly folded away (keep originally-reasoning-only turns), otherwise separate merged runs with a neutral user marker. Signed reasoning items are preserved verbatim (encrypted_content untouched).

Adds tests/issue564-responses-ordering.test.ts: an end-to-end case capturing the actual re-request body after a live compress, plus unit cases for orphaned-reasoning drop, reasoning-only-turn retention, and summary-carrier preservation.
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

📦 Built Package Artifact

Branch: 2026-09-06_responses-assistant-ordering (3a5df63)

Option A — Install from npm PR tag (recommended)

npm install -g billion-context@pr-565

Each push to this PR publishes a new version under the pr-565 npm tag.

Option B — Download artifact

  1. Download the artifact from the Actions run
  2. Extract the tarball and install:
tar xzf billion-context-pr565.tgz
npm install -g package

This comment is automatically updated on each push.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 ework agent · qwen3.8-27b

[bot] 🏷 Review of PR #565 (fixes #564)

Reviewed the diff (src/server.ts + tests/issue564-responses-ordering.test.ts) and independently reproduced the pre-flight on the PR commit (3a5df63). Verdict: LGTM to merge — correct root-cause fix, well-tested, clean build. One non-blocking hardening note at the end.

Duplicate screen: no other open/closed issue or PR targets this (searched by symptom + fix). Confirmed distinct from #539 as you framed it — #539 dropped the reasoning_content field on replay; here the reasoning items survive but their order relative to assistant text/tool-calls is corrupted.

Root cause — verified. Responses enforces per-run order reasoning* -> message* -> function_call* with ≤1 reasoning item. coreToResponses maps CoreMessage→item in sequence, so item order == core order. Folding removes the tool calls/results that separated two assistant turns; stripKernelSummaries() then deletes the acp_summary_<block> that was the last remaining separator (its compress call is present in history → redundant). Net: one run [reasoningA][messageA][reasoningB][messageB] → repeated 400 invalid_assistant_history. OpenAI chat never surfaces it because it merges consecutive assistant messages into one object. Diagnosis is accurate.

Fix — sound. repairResponsesAssistantOrdering(folded, original) rebuilds turn boundaries from the original (pre-fold) client history, applied after dedup:

  • drops a reasoning whose turn's non-reasoning body was wholly folded away (content now lives in the block summary);
  • keeps originally-reasoning-only turns' reasoning;
  • otherwise inserts a neutral user-role marker between the merged runs.

Signed reasoning is preserved verbatim (encrypted_content untouched); inputs are never mutated. Applied after stripKernelSummaries at all three wire-build sites:

  • prepareResponses — src/server.ts:2083
  • prepareResponsesCompact — src/server.ts:2330
  • refreshFolded in forward — src/server.ts:3133

I hand-traced the drop/separator logic against the orphaned-reasoning, reasoning-only-turn, and surviving-carrier cases — all consistent. Importantly, a separator is only ever inserted directly after assistant content (the phase/seenReasoning flags reset on every non-assistant item), so it cannot create two consecutive same-role items.

Both compression modes: the repair is carrier-independent (pure CoreMessage transform) and covers both plugin-mode replay and proxy-mode re-request. ✓

Pre-flight (reproduced independently on 3a5df63):

  • npm run typecheck — clean
  • npm run build — clean
  • npm test — 1051 pass / 1 fail. The single failure is launcher.test.ts › resolveClientCommand: codex/claude resolve to themselves; I reproduced it identically on pristine master (2cbbeab). It's pre-existing/environmental (a CLI binary resolving to its full path where the test expects the bare name) and has zero code-path overlap with this change.

Tests: strong. The e2e case drives the real runCompressLoop + responses adapter through a live compress and asserts on the actual re-request body (captured via a fetch probe), not just the pre-dedup fold — plus unit coverage for orphan-drop, reasoning-only retention, and surviving-acp_summary no-spurious-marker. This pins the final wire ordering directly.

Non-blocking observation (optional hardening): refreshFolded is the shared loop path (src/loop/core.ts calls it for every protocol after a successful compress), so repairResponsesAssistantOrdering also runs for OpenAI/Anthropic re-requests. I verified it stays structurally safe there (separator only lands after assistant content → assistant/user/assistant, valid alternation, never two consecutive users), but it injects a spurious user turn and breaks the prefix cache for protocols that never had this bug. Two options:

  1. Gate the refresh-path call to protocol === "responses" for minimal blast radius, or
  2. Keep it universal if you want stricter turn-fidelity across all protocols (also defensible — presenting two distinct turns as one is arguably wrong everywhere).

My lean is option 1: it matches the actual bug scope and avoids introducing untested behavior against real codex (the E2E suite couldn't run here). Not a blocker either way.

E2E note: the real-codex-through-bili suite needs a local Responses-compatible upstream + the codex binary and could not be executed in this sandbox (loopback upstreams blocked); the fetch-probe regression pins the final ordering instead, which is adequate. Recommend running the manual-dispatch ci-e2e.yml post-merge for full confidence.

Per repo policy I won't merge (human-only). Happy to implement the responses-gating follow-up if you'd like it.

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.

Compression breaks Responses assistant history ordering, causing repeated invalid_assistant_history 400s

1 participant