fix(core): stop repeated truncated write_file/edit retries from looping - #5934
Merged
Conversation
yiliang114
force-pushed
the
codex/issue-5756-plan
branch
from
June 27, 2026 09:13
26d840f to
a6f46ad
Compare
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
… cap The 8K CAPPED_DEFAULT_MAX_TOKENS made normal large responses (esp. file writes) truncate, forcing a truncate->escalate round-trip and, worst case, a retry loop. Default to the model's declared output limit instead; the existing escalation + multi-turn recovery stay as the truncation backstop. The 8K cap was a slot-reservation optimization. Claude Code keeps the same cap but gates it behind a feature flag that defaults OFF for third-party providers; qwen-code's providers are all third-party / OpenAI-compatible / self-hosted, so matching that default-off behavior is the safe choice. The capacity tradeoff stays opt-in via QWEN_CODE_MAX_OUTPUT_TOKENS. Refs #5756
yiliang114
marked this pull request as ready for review
June 27, 2026 10:09
Collaborator
|
Thanks for the PR! Re-run review below. Template looks good ✓ — all required sections present, bilingual, clear test plan. On direction: this is a solid fix. The 8K default cap causing a truncate → reject → retry loop is a real correctness bug, not just a performance concern. The reasoning about Claude Code's On approach: the two-layer fix is clean and minimal. (1) Remove Moving on to code review. 🔍 中文说明感谢贡献！重新审查如下。 模板完整 ✓ 方向：这是一个实在的修复。8K 默认 cap 导致截断 → 拒绝 → 重试死循环是真正的正确性问题，不只是性能。关于 Claude Code 的 方案：两层修复干净且最小化。(1) 删除 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Collaborator
Code ReviewIndependent proposal: to fix the truncated-write loop, I would (1) raise the default Comparison: the PR's approach matches my proposal exactly — and executes it well. The Reuse check: the PR correctly reuses the existing No issues found. The diff is focused, every change serves the stated goal, and the design doc is updated consistently. TestsVerified: the new loop-detector test does not exist on Real-Scenario Testing (tmux)Before (installed build)After (this PR via dev build)Both builds write files correctly. The behavioral change (higher default 中文说明代码审查PR 方案与独立方案完全一致：(1) 默认 测试全部通过：scheduler 1/222（新回归测试），token/provider/geminiChat 416/416。新测试在 main 上不存在，确认是真正的回归测试。 真实场景测试两个版本都能正常写文件。行为变化（更高的默认 — Qwen Code · qwen3.7-max |
Collaborator
|
This PR fixes a real correctness bug with a clean, minimal change. The 8K default cap was causing truncated writes to loop, and the fix addresses both the root cause (use model limit) and the residual case (loop detection backstop). The design rationale — matching Claude Code's default-off behavior for third-party providers — is sound and well-documented. My independent proposal matched the PR's approach exactly, which is a good sign: there's no simpler path being missed. The Tests are solid: 416 unit tests pass, the new regression test genuinely fails on The only tradeoff is higher per-request slot reservation on capacity-constrained backends, which is honestly acknowledged and mitigated by the Approving. ✅ 中文说明本 PR 用干净、最小化的改动修复了一个真正的正确性 bug。8K 默认 cap 导致截断写循环，修复同时处理了根因（用模型上限）和残余情况（循环检测兜底）。设计理由——对齐 Claude Code 对第三方 provider 默认关闭的行为——合理且有据。 独立方案与 PR 方案完全一致，没有遗漏更简路径。 测试可靠：416 单元测试通过，新回归测试在 main 上确实失败，tmux 冒烟测试确认基础写文件无回归。行为变化在 API 层内部，单元测试是主要验证手段——覆盖充分。 唯一的代价是容量受限后端的 slot 预留增加，已通过 批准 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
previously approved these changes
Jun 27, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
added
category/core
wenshao
reviewed
Jun 27, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
[Suggestion] Escalation no-op for high-output models (geminiChat.ts:~2375)
After removing the 8K cap, the provider now sends max_tokens = modelLimit by default. For models with limits ≥ 64K (e.g., Claude Sonnet 64K, GPT-5 128K, DeepSeek V4 384K), escalatedLimit = Math.max(64000, modelLimit) equals the initial limit, so the escalation retry fires at the same limit — a wasted full API round-trip that generates up to the same limit again, hits MAX_TOKENS again, and discards the partial response.
The guard shouldEscalateMaxOutputTokens only checks whether the user configured maxOutputTokens, not what the provider actually sent:
const shouldEscalateMaxOutputTokens =
requestedMaxOutputTokens === undefined ||
requestedMaxOutputTokens < escalatedLimit;Suggested fix — compare against the effective initial limit:
const effectiveInitialLimit = requestedMaxOutputTokens ?? tokenLimit(model, 'output');
const shouldEscalateMaxOutputTokens = effectiveInitialLimit < escalatedLimit;This preserves escalation for models below 64K (e.g., GPT-4 at 16K → escalates to 64K) while skipping the no-op retry for high-output models.
— qwen3.7-max via Qwen Code /review
| // while using ESCALATED_MAX_TOKENS (64K) as a floor for unknown | ||
| // models. | ||
| // Max output tokens escalation: if the retry loop succeeded but hit | ||
| // MAX_TOKENS, retry once at the model's full output limit. This ensures |
Collaborator
There was a problem hiding this comment.
[Suggestion] After removing the 8K default cap, this escalation path is now a no-op for all known models. Since content generators now default to the model's declared output limit (e.g., 65536 for Claude, 131072 for GPT-5), and escalatedLimit = max(64K, modelLimit):
- Models with limit >= 64K (Claude, GPT-5, o-series):
escalatedLimit == modelLimit, which is already the initialmax_tokens. The retry sends an identical request. - Models with limit < 64K (GPT-4 at 16K, Gemini fallback at 8K):
escalatedLimit = 64K, butapplyOutputTokenLimitin the provider caps it back tomin(64K, modelLimit) = modelLimit. Still a no-op.
This comment's claim that "models with large output limits (e.g., 128K for Claude Opus, GPT-5) are fully utilized" no longer holds — these are exactly the models for which escalation does nothing.
Before this PR, escalation from 8K → 64K was meaningful. Now it wastes a full API round-trip per truncation event.
Suggested change
| // MAX_TOKENS, retry once at the model's full output limit. This ensures | |
| // Max output tokens escalation: if the retry loop succeeded but hit | |
| // MAX_TOKENS, retry once at the model's full output limit. Currently | |
| // only effective for models whose declared output limit is below 64K | |
| // (e.g., Gemini fallback at 8K). For models at or above 64K, the | |
| // default already equals the escalated limit, making this a no-op. |
Consider adding a guard to skip the wasted retry:
const currentEffectiveLimit = requestedMaxOutputTokens ?? tokenLimit(model, 'output');
const shouldEscalateMaxOutputTokens = currentEffectiveLimit < escalatedLimit;— qwen3.7-max via Qwen Code /review
Collaborator
Author
There was a problem hiding this comment.
Same finding as wenshao's — real no-op for ≥64K models. The catch: the suggested guard would also disable the recovery loop, which is nested inside this escalation if block (the recovery while at ~L2441), so ≥64K models would lose recovery entirely — 24 recovery tests drop out when the guard is applied. It's an efficiency issue, not a correctness bug; recovery still fires today. Decoupling it properly (skip the no-op escalation call but still recover) in a follow-up: #5939.
wenshao
approved these changes
Jun 27, 2026
Collaborator
Author
|
@wenshao on the escalation point: you're right that escalating 64K→64K is a wasted same-limit round-trip now that the default is the model limit. One catch on the guard though — the multi-turn recovery loop is nested inside the escalation So it's a real inefficiency but not a correctness bug — recovery still fires today. The proper fix decouples the two (skip the no-op escalation call but still recover on the initial partial) and needs the recovery tests moved off the 64K model. Tracking in #5939 rather than widening this PR. The directive comment is addressed in |
| /** Directive injected when a truncated file-modifying call repeats. */ | ||
| const TRUNCATION_RETRY_LOOP_DIRECTIVE = | ||
| '\n\n⚠️ RETRY LOOP DETECTED: The same truncated file write has been rejected multiple times. ' + | ||
| 'STOP resending the same large content. Either split it into smaller write_file + incremental edit calls, ' + |
Collaborator
There was a problem hiding this comment.
[Suggestion] This directive text hardcodes write_file-specific advice ("split it into smaller write_file + incremental edit calls"), but it's injected for every tool with Kind.Edit — which includes edit and notebook_edit. For notebook_edit, the advice to use write_file + edit is inapplicable (notebooks operate on cells, not raw file content). For edit, it's confusing because the failed tool IS the edit tool.
Suggested change
| 'STOP resending the same large content. Either split it into smaller write_file + incremental edit calls, ' + | |
| 'STOP resending the same large content. Break the operation into smaller incremental steps, ' + | |
| 'or explain to the user that the content is too large to produce safely in one call.'; |
— qwen3.7-max via Qwen Code /review
| expect(messages[0]).toContain('truncated due to max_tokens limit'); | ||
| expect(messages[0]).not.toContain('RETRY LOOP DETECTED'); | ||
| expect(messages[1]).not.toContain('RETRY LOOP DETECTED'); | ||
| expect(messages[2]).toContain('RETRY LOOP DETECTED'); |
Collaborator
There was a problem hiding this comment.
[Suggestion] toContain('RETRY LOOP DETECTED') passes for both TRUNCATION_RETRY_LOOP_DIRECTIVE ("RETRY LOOP DETECTED: The same truncated file write…") and RETRY_LOOP_STOP_DIRECTIVE ("RETRY LOOP DETECTED: This tool call has failed validation…"). A regression that accidentally injects the wrong directive would go undetected.
Suggested change
| expect(messages[2]).toContain('RETRY LOOP DETECTED'); | |
| expect(messages[2]).toContain('truncated file write'); |
— qwen3.7-max via Qwen Code /review
Collaborator
|
@qwen-code /triage |
qwen-code-ci-bot
approved these changes
Jun 27, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Collaborator
Author
Real tmux verification caseI verified the fixed branch with a live tmux-driven Qwen Code run against the direct Case setup
The prompt forced one large Command executed inside tmux: npm run dev -- \
-p "$PROMPT" \
--model qwen3.7-max \
--yolo \
--max-session-turns 4 \
--max-tool-calls 3 \
--max-wall-time 15m \
--openai-logging \
--openai-logging-dir /private/tmp/qwen-issue-5756-directmax-lines-openai-logs \
--output-format stream-jsonObserved stream-json eventsThe local worktree path is redacted below, but the model, token counts, tool call, and result are copied from the real stream-json run: Generated file verificationWhy this covers #5756The key signal is that the successful This is the class of response that would have exceeded the old default cap. On this branch, because the default output limit now follows the model limit when no user/env One note: the tmux process later exited with code 55 because the model continued with unrelated extra tool calls after the successful write and hit the local |
doudouOUC
reviewed
Jun 28, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
[Suggestion] docs/users/configuration/settings.md lines 192–200 still describe the removed 8K default behavior:
Requests start with a default limit of 8K output tokens
99% of responses are under 5K tokens, so the retry happens rarely (<1% of requests)
This directly contradicts the updated environment variable table in the same file, which correctly says Qwen Code now defaults to the model's declared output limit. Consider rewriting the prose section to match.
— qwen3.7-max via Qwen Code /review
| } | ||
| }); | ||
|
|
||
| it('should inject retry loop directive after repeated truncated write_file rejections', async () => { |
Collaborator
There was a problem hiding this comment.
[Suggestion] The new truncation loop-detection test covers the threshold boundary (3 consecutive truncated writes), but there is no test verifying that the counter resets after a successful (non-truncated) invocation of the same Edit tool.
The validation path has an equivalent test (should reset retry counter after a successful invocation of the same tool), but the truncation path doesn't. If clearRetryCountsForTool were ever scoped differently (e.g., only clearing validation keys), the truncation counter would silently accumulate across successes and produce a false-positive RETRY LOOP DETECTED.
Consider adding a test: send 2 truncated write_file calls → 1 successful write_file → 2 more truncated calls, and assert the 5th call's error does NOT contain RETRY LOOP DETECTED.
— qwen3.7-max via Qwen Code /review
This was referenced Jun 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TLDR
#5756 is a failed-retry loop: the default 8K output cap truncates normal large responses (especially
write_file), the scheduler rejects the partial write, and the model retries the same oversized call. This PR fixes it at two levels: (1) the root cause — default to the model's real output limit instead of the 8K cap, and (2) a backstop — route repeated truncated edit rejections through the existing retry-loop detector so a stuck model gets a stop directive instead of looping forever.What this changes
1. Root cause — drop the 8K default cap (
packages/coretoken-limit resolution).Before: with no user/env
max_tokens, requests defaulted tomin(modelLimit, CAPPED_DEFAULT_MAX_TOKENS=8K), so any response over ~8K truncated and had to escalate (a wasted round-trip), and edge cases looped. After: the default is the model's declared output limit. The already-present escalation (to a 64K floor / model limit) and multi-turn recovery remain as the truncation backstop.CAPPED_DEFAULT_MAX_TOKENSis removed and the OpenAI-compatible, DashScope, and Anthropic generators all default to the model limit.2. Backstop — loop detection for truncated edits (
coreToolScheduler).The scheduler already rejects truncated
write_file/edit calls to avoid writing partial content, but that rejection bypassed the retry-loop detector, so a model resending the same truncated call only ever got the same plain hint. It now feeds the same per-(tool, error)counter as schema-validation failures; the third identical truncated rejection carries the existingRETRY LOOP DETECTEDstop directive.Why it's needed
8K is too small for legitimate large generation and file edits, so the previous design turned a normal request into a truncate → escalate round-trip and, in the worst case, an infinite retry loop. Raising the default removes the truncation in the first place; the loop detector covers the residual case where even the model limit can't fit the content.
Design Consideration — the 8K cap was a slot-reservation optimization
The 8K cap was not arbitrary: a request reserves a GPU slot proportional to
max_tokens, so a low default over-reserves less on capacity-constrained backends. Claude Code keeps the same cap but gates it behind a feature flag (tengu_otk_slot_v1) that defaults to off for third-party providers ("not validated on Bedrock/Vertex") — i.e. its default behavior for non-first-party serving is exactly "use the model's declared limit." qwen-code's providers are all third-party / OpenAI-compatible / self-hosted, so matching that default-off behavior is the right call rather than assuming the low default is safe everywhere. The tradeoff is made opt-in, not lost: operators on a capacity-constrained self-hosted backend can setQWEN_CODE_MAX_OUTPUT_TOKENS=8000to restore the lower reservation. A GrowthBook-style flag is intentionally not reintroduced — qwen-code has no such infra and the env var already covers it. Full rationale indocs/design/adaptive-output-token-escalation/.Reviewer Test Plan
How to verify
Expected: all green. The new scheduler regression test fails on
main(3rd rejection lacksRETRY LOOP DETECTED) and passes here; provider/token tests assert the new model-limit default.Evidence (Before & After)
Non-user-visible (request
max_tokens+ tool-response text). Verified locally:tsc --noEmit(core) clean, eslint on changed files clean,coreToolScheduler.test.ts222/222, and the five token/provider/geminiChat suites 416/416. The loop-detector behavior was also reproduced against the built bundle — three truncatedwrite_filerejections, only the 3rd carryingRETRY LOOP DETECTED.Tested on
Risk & Scope
QWEN_CODE_MAX_OUTPUT_TOKENS. No effect on hosted APIs that bill on actual tokens.max_tokensstill wins and still disables escalation.QWEN_CODE_MAX_OUTPUT_TOKENS=8000.Linked Issues
Refs #5756
中文说明
摘要
#5756 是一个失败重试死循环：默认 8K 输出上限会截断正常的大响应（尤其
write_file），scheduler 拒绝写入残缺内容，模型又重发同一个超大调用。本 PR 从两个层面修复：(1) 根因——默认改用模型真实的输出上限，而不是 8K cap；(2) 兜底——把重复的截断 edit 拒绝接入已有的重试循环检测器，让卡住的模型拿到停止指令而不是无限循环。改了什么
min(模型上限, 8K)，超过 ~8K 就截断、要 escalate（一次无谓往返），极端情况会循环。现在默认用模型声明的输出上限；已有的 escalation（64K floor / 模型上限）和多轮 recovery 仍作为截断兜底。删除CAPPED_DEFAULT_MAX_TOKENS，三个 provider 生成器都改为默认模型上限。write_file/edit，但这条拒绝绕过了循环检测器。现在它走和 schema 校验失败相同的(工具,错误)计数器，第 3 次相同截断拒绝带上RETRY LOOP DETECTED。设计权衡
8K cap 是 slot 预留优化（请求按
max_tokens预留 GPU 槽位）。Claude Code 保留了同样的 cap，但用一个 feature flag 门控，且对第三方 provider 默认关闭（"未在 Bedrock/Vertex 验证"）——也就是说它对非一方服务的默认行为就是"用模型声明上限"。qwen-code 的 provider 全是第三方 / OpenAI 兼容 / 自建，所以对齐这个"默认关闭"才是对的。权衡没丢只是改成 opt-in：容量受限的自建后端可设QWEN_CODE_MAX_OUTPUT_TOKENS=8000恢复低预留。不重新引入 GrowthBook 式 flag（qwen-code 没这套基建，env var 已够）。详见docs/design/adaptive-output-token-escalation/。验证
见上方英文 Reviewer Test Plan：全绿。scheduler 新回归测试在
main上 fail、本分支 pass；provider/token 测试断言新的模型上限默认。本地tsc/eslint clean，coreToolScheduler222/222，token/provider/geminiChat 五套 416/416，并用已打包产物复现了第 3 次出现RETRY LOOP DETECTED。