Skip to content

fix(#684): reasoning-drop self-disables on strict-echo upstreams; split-turn exit sentinels - #690

Merged
ranxianglei merged 2 commits into
masterfrom
2026-09-11_turn-integrity-684
Sep 10, 2026
Merged

fix(#684): reasoning-drop self-disables on strict-echo upstreams; split-turn exit sentinels#690
ranxianglei merged 2 commits into
masterfrom
2026-09-11_turn-integrity-684

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Fixes #689 (reported as #684). Depends on the kernel-side atomic-turn fix: acp-kernel#245 (issue acp-kernel#244) — release acp-kernel first, then bump the pinned version here before releasing this.

What this adds

  • isStrictReasoningEcho(): static deepseek host check + session-learned flag gate withReasoningDrop at all three wire prepares (prepareAnthropic/prepareOpenai now receive upstreamOrigin).
  • Learn-on-400: loop UpstreamHttpError handler sets session.metadata.strictReasoningEcho = true on first 400 mentioning reasoning_content (persisted, self-healing — billion-context: 400 Bad Request when codex sends role: "developer" messages — request a request-body compat transform hook #552 pattern).
  • Exit sentinels on all three wires: [reasoning-pair-violated] / [thinking-pair-violated] — with the kernel gate these are unreachable, so a hit = new regression path.
  • Docs: CONFIGURATION.md / .zh-CN.md reasoning section updated (auto-disable + manual drop:false escape hatch); CHANGELOG entry.

Tests

tests/reasoning-echo.test.ts (static + learned gate, all three sentinels fire/silent). 1320/1320 green, typecheck clean.

Merge is human-only (AGENTS.md §4).

…it-turn exit sentinels

DeepSeek thinking mode documents reasoning_content as mandatory
round-trip with tool_calls; #651's drop could strip it from a closed
compress turn and the kernel's half-turn bug (fixed in acp-kernel #684
twin) shipped tool_calls without it. Proxy-side closure:

- isStrictReasoningEcho(): static deepseek host check + session-learned
  flag; disables the #651 reasoning-drop (withReasoningDrop gate at all
  three wire prepares, which now receive upstreamOrigin).
- loop UpstreamHttpError handler: first 400 whose body mentions
  reasoning_content learns session.metadata.strictReasoningEcho=true
  (self-healing, persisted — same pattern as #552), with ops log.
- Exit sentinels on all three wires (warnReasoningPairs /
  warnAnthropicThinkingPairs / warnResponsesReasoningPairs): log
  reasoning-pair-violated / thinking-pair-violated when a tool-call
  message lacks reasoning while siblings carry it — the turn gate makes
  this unreachable, so a hit means a new regression path.

Tests: tests/reasoning-echo.test.ts (static + learned gate, all three
sentinels fire/silent). 1320/1320.
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

📦 Built Package Artifact

Branch: 2026-09-11_turn-integrity-684 (664184c)

Option A — Install from npm PR tag (recommended)

npm install -g billion-context@pr-690

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

Option B — Download artifact

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

This comment is automatically updated on each push.

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 PR #690 reviewed. The code is clean, correct, and green — but it is blocked on the kernel dependency and should not be released yet. Details below.

Duplicate screening: No duplication. #690 is the proxy-side implementation of #689 (twin of kernel acp-kernel#245 / issue #244). It is distinct from the already-merged #540 (loop re-request drops fresh reasoning) and #667/#651 (compress-reasoning drop) — this PR builds on those rather than overlapping them.

Diff cleanliness: ✅ Clean. Single commit ed8c840 sitting directly on latest origin/master (v0.1.101 base). 6 files, +192/−7. Every line is on-topic for #684. No unrelated/generated churn; the PR correctly does not touch package.json or the lockfile.

Correctness (read the diff):

Independent verification (ran at ed8c840): typecheck clean; full suite 1327 tests, 0 fail, 2 skipped (the e2e-gated ones); new tests/reasoning-echo.test.ts 7/7. (Body said 1320 — count drifted, but zero failures is what matters.)

🔴 Blocker — kernel dependency not yet satisfied. The PR states it depends on acp-kernel#245 (issue #244) being released first, then the pin bumped here. Current state:

  • acp-kernel#245 is still OPEN (unmerged, unreleased).
  • Latest stable acp-kernel is 0.0.62 (published Sep 10 05:08Z), which predates release v0.1.54 #245 (opened 14:55Z) → no stable release contains the turn-integrity gate.
  • This PR pins 0.0.61 (the base's pin) and does not bump it.

Consequences of cutting a release from this branch today:

  1. The primary half-turn-fold root cause ([bug] compress 保护区过滤在完整性调整之后挖洞,导致 assistant tool-call 消息缺 reasoning_content(DeepSeek 思考模式 400) #684) lives in the kernel and stays unfixed.
  2. The exit sentinels assume the kernel gate makes split-turns unreachable — without the gate they fire in prod (false-alarm log noise).
  3. The proxy-side pieces (static/learned gate disabling Protected-message exemption inflates the incompressible context floor: per-compression reasoning is never reclaimable (~83.5% of measured residual) #651's drop) are independently correct and do close the secondary Protected-message exemption inflates the incompressible context floor: per-compression reasoning is never reclaimable (~83.5% of measured residual) #651 path for DeepSeek on their own — but full [bug] compress 保护区过滤在完整性调整之后挖洞,导致 assistant tool-call 消息缺 reasoning_content(DeepSeek 思考模式 400) #684 closure needs the kernel.

Recommendation (one path, not a menu): Hold the merge. Land acp-kernel#245 first (already written there, 644/644 green — just needs its human merge + CI publish), verify it is live on npm, then add a follow-up commit to this branch bumping acp-kernel 0.0.61 → that version (+ lockfile refresh), re-run preflight, and only then merge + release. That keeps the release atomic and avoids prod sentinel noise. (If you specifically want the #651-secondary-path protection for DeepSeek users sooner, the proxy side is safe to land early since the sentinels are warn-only — but I'd still prefer the atomic path.)

Minor nits (non-blocking, left as-is):

中文摘要: 代码本身干净、正确、测试全绿(我独立复跑:typecheck 通过、1327 用例 0 失败、新测试 7/7),可以合入;但它依赖的 acp-kernel#245 内核"原子轮次"修复目前仍是未合并未发布状态(最新稳定版 0.0.62 也不含该修复,本 PR 固定的是更旧的 0.0.61),所以现在不能发布——需等内核先发布、在本分支把 acp-kernel 升到含该修复的版本并刷新 lockfile、重跑预检后再合并发布,否则根因未修且三处哨兵会在生产环境误报。

Merge stays human-only per AGENTS.md §4 — I won't merge.

@ranxianglei

Copy link
Copy Markdown
Owner Author

acp-kernel v0.0.63 is live on npm (includes #245 + directional refinement #247). Pinned 0.0.61 → 0.0.63 in 664184c; full suite re-verified against the real published package: 1327 tests / 0 fail, typecheck clean, build clean. Ready for merge (human-only).

@ranxianglei

Copy link
Copy Markdown
Owner Author

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 Re-verified independently against the real published package — every claim checks out. Ready for merge.

I didn't take the claim at face value: pulled acp-kernel@0.0.63 from npm into a clean worktree at 664184c and ran the full gate.

Check Result
acp-kernel@0.0.63 live on npm ✅ latest stable, published Sep 10 15:14Z
#245 actually shipped in 0.0.63 dist/turn-integrity.d.ts + computeTurnGroups present in the installed kernel
Bump commit 664184c ✅ touches only package.json + package-lock.json; lockfile fully in sync (dep spec and resolved entry both → 0.0.63, matching tarball) so CI npm ci succeeds
Installed version after npm ci ✅ genuinely 0.0.63 from the registry
typecheck ✅ exit 0
Full suite 1327 tests / 0 fail / 2 skipped (the e2e-gated ones)
Build (tsup, inlines kernel) ✅ success; bundled dist/index.js carries both the kernel turn-gate and the proxy strict-echo/sentinel logic

The dependency I flagged last round is now satisfied: the atomic-turn gate (#245) that makes the three exit sentinels unreachable is present in the pinned kernel, so a release cut from this branch ships the root-cause fix and the proxy-side closure together. (#247 directional refinement rides along inside 0.0.63 — a kernel-side change covered by its own release process; here it's exercised by the bundled suite passing.)

Verdict: code correct, diff clean, dependency satisfied, green across typecheck/test/build. Ready to merge. Per AGENTS.md §4 the merge itself is human-only — I won't merge.

中文摘要: 已独立复验(非照单全收)——从 npm 拉取真实的 acp-kernel@0.0.63 装入干净 worktree 跑完整门禁:0.0.63 确已发布且含 #245(computeTurnGroups/turn-integrity 均在)、664184c 仅改 package.json+lock 且锁文件完全同步、typecheck 通过、1327 用例 0 失败、build 成功且产物内联了内核轮次门与代理侧哨兵逻辑;上轮的内核依赖阻塞已解除,可以合并(合并按 §4 由人工执行)。

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.

fix: #684 half-turn on DeepSeek thinking mode — reasoning-drop self-disable + split-turn sentinels (proxy side)

1 participant