Skip to content

fix(goal): count catalog previews in the unit their budget is written in - #9835

Merged
qqqys merged 1 commit into
QwenLM:mainfrom
qqqys:goal/c2-salvage-checkpoint
Aug 24, 2026
Merged

fix(goal): count catalog previews in the unit their budget is written in#9835
qqqys merged 1 commit into
QwenLM:mainfrom
qqqys:goal/c2-salvage-checkpoint

Conversation

@qqqys

@qqqys qqqys commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Evidence catalog previews are now capped to 240 UTF-8 bytes on a code point boundary, rather than to 240 characters, at the two points a catalog entry is built. And a truncated evidence window now compresses what it captured instead of stopping the Goal — only a window that captured nothing at all still fails.

Why it's needed

The catalog budget is 24,000 bytes. The preview limit that feeds it was 240 characters. In UTF-8 those units differ by up to four times, so the guard held only for ASCII.

The consequence is not a tuning problem, it is a Goal that cannot run in some languages. A checkpoint may carry 32 claims; each is projected into the catalog as a preview plus a small envelope. With ASCII that is about 430 bytes per entry, so a full checkpoint occupies 13,760 bytes — 57% of the budget, leaving room for roughly 23 more records. With Chinese the same legal checkpoint serializes to about 29,000 bytes, over the cap on its own, before a single new record has been scanned. The window is then marked truncated, and shouldCheckpoint required !truncated — so the one state compaction exists to resolve was the one state it refused to run in. The runtime read that as unrecoverable and stopped the Goal as usage_limited, which reduceGoalResume refuses to resume, and there was nothing left to salvage because no candidate evidence had been collected yet.

An English Goal never reached that state. A Chinese one could not avoid it: the failure arrives on the first checkpoint that fills its claim budget, regardless of how the run is going.

The second half follows from the first. A window that overflows its budget is precisely the state compaction is for — the newest evidence that did fit is exactly what a checkpoint folds into claims, and the older evidence left behind is already covered by the previous checkpoint's claims. Refusing to compress there and stopping the Goal instead is strictly worse than compressing what is available.

Reviewer Test Plan

How to verify

Run a Goal whose objective and work are in a non-Latin script — Chinese is the reported case — long enough for it to take a checkpoint whose claims fill the claim budget. Before this change the next window is truncated on arrival and the Goal stops as usage_limited with limitKind: 'evidence_catalog', and /goal resume is refused. After it, the checkpoint stays comfortably inside the catalog budget and the run continues. Then force an overflow deliberately (more than 100 post-cursor records) and confirm the Goal now takes a checkpoint and stays active rather than stopping.

Two tests carry the change. does not start truncated under a full checkpoint of multi-byte claims builds a goal carrying 32 claims of 2,000 Chinese characters — the maximum a checkpoint may hold — and asserts the window is not truncated. compresses a truncated window instead of stopping the Goal drives 101 post-cursor records through the runtime and asserts the checkpoint verifier is called, the Goal stays active, and the journal records a checkpoint rather than a usage_limited cause.

Both were mutation-checked rather than merely observed to pass. Reverting the byte cap on checkpoint claims to the old character slice fails the multi-byte test and leaves the other 29 in that file green. Restoring the !truncated gate on shouldCheckpoint fails the salvage test and leaves the other 108 in the runtime file green. Both mutations were reverted and the suites re-run clean.

The existing moves to usage_limited when checkpoint %s fails case was split: flush and read still assert usage_limited, because a transient I/O failure is unrelated to this change, while truncated became the salvage test above. One existing multi-byte fixture was rescaled from 26 records to 55 — its intent is that a window crossing the 19,200-byte threshold arms a checkpoint, and a correctly capped CJK entry now costs about 364 bytes instead of about 910, so the same record count no longer reaches the threshold. The assertion is unchanged; only the fixture size moved.

npx vitest run packages/core/src/goals/ passes 388 tests across 15 files. npx tsc --noEmit in packages/core is clean apart from pre-existing @types/node skew in src/utils/tool-result-boundary-diagnostics.ts, a file this PR does not touch. prettier and eslint are clean on all four changed files.

Evidence (Before & After)

Before, from the reported session: a Chinese Goal ran 27 turns and stopped as usage_limited with The current Goal revision exceeded the bounded evidence catalog. Automatic retries cannot recover. Edit or replace the Goal before resuming it. — 27 of 60 files written, no output, and /goal resume refused.

After: a full 32-claim Chinese checkpoint occupies roughly 13,000 bytes rather than 29,000, so the window is not truncated on arrival; and when a window does overflow, the runtime compresses it and the Goal stays active.

Tested on

OS Status
🍏 macOS N/A
🪟 Windows ⚠️
🐧 Linux

Environment (optional)

Linux, Node.js 22, unit tests only.

Risk & Scope

  • Main risk or tradeoff: non-Latin previews get shorter — a Chinese preview now holds about 80 characters where it previously held 240. That is the cost of the cap actually holding, and it is the same information density ASCII has always had. Previews are a catalog index, not the evidence itself: the terminal verifier reads full content through evidenceContent, which is capped separately and unchanged here. ASCII previews are byte-for-byte unchanged, since the two units already agreed there.
  • Not validated / out of scope: this does not change CATALOG_BYTE_LIMIT, CHECKPOINT_BYTE_THRESHOLD or the claim budget — the units are reconciled at the values already in the tree rather than retuned. It does not add compaction-effectiveness detection: a checkpoint that compresses but does not shrink enough will still overflow again, which is a separate change. Windows and macOS were not exercised locally and remain covered by CI.
  • Breaking changes / migration notes: none. Goals persisted with longer previews are re-projected through the cap when their window is next built, so an existing transcript is not invalidated. Scope for the core triage gate: 49 added and 7 deleted production lines across two files in packages/core/src/goals, with no cross-package change.

Linked Issues

None — the failure is reported here rather than in an existing issue.

中文说明

本 PR 做了什么

证据目录的 preview 现在按 240 个 UTF-8 字节、在码点边界上截断,而不是按 240 个字符;截断施加在构造 catalog entry 的两个汇聚点上。另外,被截断的证据窗口现在会压缩它已经捕获的内容,而不是停掉 Goal —— 只有「什么都没捕获到」的窗口才仍然失败。

为什么需要

catalog 预算是 24,000 字节,而喂给它的 preview 上限是 240 个字符。UTF-8 下这两个单位最多相差四倍,因此这道防线只在 ASCII 下成立。

后果不是调参问题,而是 Goal 在某些语言下根本跑不了。一个 checkpoint 最多可以携带 32 条 claim,每条会被投影成 catalog 中的一条 preview 加一个小信封。ASCII 下每条约 430 字节,满载 checkpoint 占 13,760 字节 —— 预算的 57%,之后还能装大约 23 条记录。中文下同样合法的 checkpoint 序列化后约 29,000 字节,在还没扫描任何一条新记录之前,光它自己就越过了上限。窗口于是被标记为 truncated,而 shouldCheckpoint 要求 !truncated —— 压缩本该解决的那个状态,恰恰是它拒绝运行的那个状态。运行时把这读作不可恢复,将 Goal 停为 usage_limitedreduceGoalResume 拒绝恢复的那个状态),而且此时没有任何可抢救的东西,因为一条候选证据都还没被收集。

英文 Goal 永远到不了这个状态;中文 Goal 则无法避免它:故障在第一个填满 claim 预算的 checkpoint 上就会到来,与运行得好不好无关。

第二半是第一半的推论。一个越过预算的窗口,正是压缩存在的意义 —— 装得下的那些最新证据,恰恰是 checkpoint 要折叠成 claim 的东西,而被留下的更旧证据早已被上一个 checkpoint 的 claim 覆盖。在那里拒绝压缩、转而停掉 Goal,严格劣于压缩手头能压的部分。

评审者测试计划

如何验证

用非拉丁文字(报告的案例是中文)的 objective 跑一个 Goal,跑到它做出一个 claim 填满预算的 checkpoint。改动之前,下一个窗口在构造时就是 truncated,Goal 以 usage_limitedlimitKind: 'evidence_catalog' 停止,/goal resume 被拒绝。改动之后,checkpoint 稳稳落在 catalog 预算之内,运行继续。然后刻意制造一次溢出(游标之后超过 100 条记录),确认 Goal 现在会做一次 checkpoint 并保持 active,而不是停止。

两个测试承载这次改动。does not start truncated under a full checkpoint of multi-byte claims 构造一个携带 32 条、每条 2,000 个中文字符 claim 的 goal(checkpoint 可容纳的上限),断言窗口未被截断。compresses a truncated window instead of stopping the Goal 把 101 条游标后记录送进运行时,断言 checkpoint verifier 被调用、Goal 保持 active、journal 记录的是 checkpoint 而不是 usage_limited

两者都做了变异检验而不只是「跑通了」。把 checkpoint claim 的字节封顶改回旧的字符切片,恰好让多字节那个测试失败,同文件其余 29 个仍绿。把 shouldCheckpoint 上的 !truncated 门恢复,恰好让抢救那个测试失败,runtime 文件其余 108 个仍绿。两次变异都已还原并重跑干净。

既有的 moves to usage_limited when checkpoint %s fails 用例被拆开:flushread 仍断言 usage_limited,因为瞬时 I/O 失败与本次改动无关;truncated 则变成了上面那个抢救测试。另有一个既有的多字节 fixture 从 26 条记录扩到 55 条 —— 它的意图是「跨过 19,200 字节阈值的窗口会武装 checkpoint」,而正确封顶后的 CJK entry 现在约 364 字节而非约 910,同样的记录数不再够到阈值。断言未变,只是 fixture 规模改了。

npx vitest run packages/core/src/goals/ 通过 15 个文件共 388 个测试。packages/corenpx tsc --noEmit 干净,仅剩 src/utils/tool-result-boundary-diagnostics.ts 中预先存在的 @types/node 偏斜,本 PR 未触及该文件。四个改动文件的 prettiereslint 均干净。

证据(修复前后)

修复前,来自已报告的 session:一个中文 Goal 跑了 27 轮后以 usage_limited 停止,原因是 The current Goal revision exceeded the bounded evidence catalog. Automatic retries cannot recover. Edit or replace the Goal before resuming it. —— 60 个文件写了 27 个、没有产出,且 /goal resume 被拒绝。

修复后:满载 32 条 claim 的中文 checkpoint 约占 13,000 字节而非 29,000,窗口在构造时不再是 truncated;而当窗口确实溢出时,运行时会压缩它,Goal 保持 active。

测试平台

操作系统 状态
🍏 macOS N/A
🪟 Windows ⚠️
🐧 Linux

环境(可选)

Linux、Node.js 22,仅单元测试。

风险与范围

  • 主要风险或取舍:非拉丁文字的 preview 会变短 —— 中文 preview 现在约容纳 80 个字符,此前是 240 个。这是「上限真正生效」的代价,而且这正是 ASCII 一直以来的信息密度。preview 是目录索引而不是证据本身:终审 verifier 通过 evidenceContent 读取完整内容,那一路有独立的封顶,本 PR 未改动。ASCII 的 preview 逐字节不变,因为那里两个单位本来就一致。
  • 未验证/不在范围:本 PR 不改动 CATALOG_BYTE_LIMITCHECKPOINT_BYTE_THRESHOLD 或 claim 预算 —— 是在树上已有的数值下把单位对齐,而不是重新调参。它也不加入「压缩是否有效」的检测:一个压缩了但没缩够的 checkpoint 仍会再次溢出,那是另一次改动。Windows 与 macOS 未在本地验证,仍由 CI 覆盖。
  • 破坏性变更/迁移说明:无。以更长 preview 持久化的 Goal,在下次构造窗口时会重新经过封顶投影,因此既有转录不会失效。供 core triage gate 参考的规模:packages/core/src/goals 下两个文件,新增 49 行、删除 7 行生产代码,无跨包改动。

关联 Issue

无 —— 该故障在此处首次记录,没有对应的既有 issue。

Evidence catalog previews were cut to 240 characters while the budget they
feed is 24,000 bytes. In UTF-8 those units differ by up to four times, so the
guard held only for ASCII. A legal 32-claim checkpoint of Chinese claims
serialized to roughly 29kB — over the cap on its own, before a single new
record had been scanned — which marked the window truncated, and `truncated`
switched compaction off. The one state compaction exists to resolve was the
one state it refused to run in, so the Goal was stopped as `usage_limited`,
the only status the reducer refuses to resume, with nothing left to salvage.
An English Goal never reached that state; a Chinese one could not avoid it.

Previews are now capped to 240 UTF-8 bytes on a code point boundary at the two
points a catalog entry is built. Nothing changes for ASCII, where the two units
already agreed; a CJK preview is shorter than before, which is the cost of the
cap actually holding. With it, a full checkpoint is bounded well inside the
catalog budget for every script, so a window can no longer start out truncated.

A truncated window now compresses rather than stopping the Goal. Overflow means
the budget is full and the newest evidence that did fit is exactly what a
checkpoint folds into claims; the older evidence left behind is already covered
by the previous checkpoint. Only a window that captured nothing at all has
nothing to salvage, and that is the sole remaining path to `usage_limited` here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finished — CI landed green on 6b43e1c and the deferred approval was posted. finalize run

Qwen Triage 已完成 —— 6b43e1c 的 CI 全绿,延迟审批已提交。查看 finalize 运行

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: observed bug, and I verified the mechanism in the base tree. Catalog previews are cut with slice(0, 240)characters — while the budget they feed (CATALOG_BYTE_LIMIT = 24_000) counts UTF-8 bytes. The PR cites a reported Chinese-language Goal session that stopped as usage_limited with the catalog-exhausted message, and the code path checks out: shouldCheckpoint requires !truncated, and the runtime turns window.truncated straight into recordCheckpointFailure(..., 'evidence_catalog') — so a 32-claim CJK checkpoint (~29kB serialized against a 24kB cap) structurally wedges compaction off and stops the Goal. Real failure, not theoretical hardening.

Direction: in scope — a correctness fix inside the Goal evidence runtime, no product-direction questions.

Size: core paths (packages/core/src/goals). Production logic: 56 lines (goal-evidence.ts 44+/6−, goal-runtime.ts 5+/1−); tests: 109 lines; no generated/schema files. Well under the 500-line maintainer-awareness bar.

Approach: the scope feels right, and I checked whether the byte-cap half alone would do. It fixes the reported CJK deadlock, but not the entry-count truncation path (candidateUuids >= rawEntryLimit also sets truncated), which the old !truncated gate turned into the same dead end — so the salvage half (compress a truncated-but-nonempty window instead of stopping) belongs in the same fix. Every edit is needed for the stated goal; no drive-by changes spotted.

Risk: no elevated risk signals — neither changed file matches the revert-correlated paths.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:已观测到的 bug,且机制已在基线代码中核实。catalog preview 用 slice(0, 240)字符截断,而其喂给的预算(CATALOG_BYTE_LIMIT = 24_000)按 UTF-8 字节计。PR 引述了一个报告的中文 Goal session 以 usage_limited 与 catalog 耗尽信息停止,代码路径核实无误:shouldCheckpoint 要求 !truncated,而 runtime 将 window.truncated 直接转为 recordCheckpointFailure(..., 'evidence_catalog') —— 因此一个 32 条 claim 的 CJK checkpoint(序列化约 29kB,上限 24kB)在结构上必然卡死压缩并停掉 Goal。是真实故障,不是理论性加固。

方向:在范围内——Goal evidence 运行时内部的正确性修复,无产品方向问题。

规模:核心路径(packages/core/src/goals)。生产代码 56 行(goal-evidence.ts 44+/6−,goal-runtime.ts 5+/1−);测试 109 行;无生成/schema 文件。远低于 500 行维护者关注线。

方案:范围合理,且核实过"只做字节封顶"是否足够。字节能修复报告的 CJK 死锁,但条目数截断路径(candidateUuids >= rawEntryLimit 同样会置 truncated)在旧的 !truncated 门下会落入同样的死局——所以抢救那一半(对 truncated 但非空的窗口做压缩而不是停掉 Goal)应与本修复合并。每处改动都为所述目标所需,未发现夹带改动。

风险:无升级风险信号——改动文件均未命中与 revert 相关的路径。

进入代码审查 🔍

Qwen Code · qwen3.8-max

Reviewed at 6b43e1ce40fb411da4ecbf86ec38f0a2c142acec · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Code review

Read against 6b43e1ce (worktree on main + the PR diff). My independent read of the problem before opening the diff: cap previews in UTF-8 bytes at the entry-construction points, and stop gating compaction on !truncated, because that gate refuses to run exactly when compaction is needed. The PR does precisely that, and does it cleanly.

What I verified:

  • Cap coverage is complete. Every path that builds a catalog entry preview goes through the new byte cap: the record-index accumulator's finish() (cap applied before catalogEntryBytes is measured, so budget accounting and actual entries agree), catalogEvidence() and the checkpoint accumulator's capture() via evidencePreview() (both return paths capped), and the claim projection in checkpointCatalogEntries(). ASCII previews are byte-for-byte unchanged; only multi-byte previews shrink.
  • The deadlock is closed structurally, not just for the reported case. With GOAL_CHECKPOINT_CLAIM_LIMIT = 32 enforced by the reducer and previews ≤ 240 bytes, a full claim checkpoint fits well inside the 24,000-byte catalog budget even under worst-case JSON escaping — so the "checkpoint alone overflows the budget" state can no longer arise.
  • capPreviewBytes is sound: code-point iteration via for...of never splits a surrogate pair, it mirrors the existing capCheckpointContent idiom, and it always receives ≤ 240 characters, so there's no cost concern.
  • The salvage path keeps its safety net. A truncated window that captured nothing still stops the Goal as usage_limited with limitKind: 'evidence_catalog', and flush/read checkpoint failures still assert usage_limited in the split test. The does not accept catalog exhaustion as an external blocker case (101 records, entry-cap truncation through the proposal path) is untouched by the diff.
  • Conventions: ESM imports, colocated tests, no any, comments explain the non-obvious why. No AGENTS.md violations.

One honest reservation, non-blocking: the PR description says the older evidence left behind by a salvaged checkpoint "is already covered by the previous checkpoint's claims" — strictly, those dropped records are post-cursor, so they are not in the previous claims, and once the salvaged checkpoint lands, the cursor moves past them and they lose citability. That is still a strictly better outcome than the old behavior (unrecoverable usage_limited death of the whole Goal), and with the byte cap in place this path is only reachable after repeated checkpoint failures — but a maintainer merging this should know the salvage tradeoff is "newest evidence survives, oldest post-cursor records are sacrificed", not lossless.

Also for the record: the mutation-testing results described in the PR body are the author's own report, not independently re-run here — CI below is what this review counts as test evidence.

Test evidence — the PR's own CI

The unit suite on the reviewed commit is still running, so the table below is a live snapshot; the finalize job updates it once CI settles. The macOS/Windows/integration rows show skipped by workflow design on pull_request events (those jobs are merge-queue-gated in ci.yml), not a gap introduced by this PR. No red checks so far; precheck (lint/typecheck/format), dependency CVE audit, secret scan, and desktop-shell builds all passed.

Final CI results for 6b43e1c (auto-updated by the triage finalize job after CI completed):

Check Conclusion
Classify PR ✅ success
Dependency CVE audit ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
Secret scan (TruffleHog) ✅ success
Test (ubuntu-latest, Node 22.x) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

Sandboxed verification would settle the one thing CI cannot: @qwen-code /verify — a green suite shows the two new tests pass, but the claim that they pin the change (reverting either half fails the matching test) is the author's own mutation report; an A/B run against the base build would prove the tests fail without the diff. The author has write access and can trigger it directly; there is no TUI surface here, so /tmux does not apply.

中文说明

代码审查

6b43e1ce 上阅读(main worktree + PR diff)。我在看 diff 之前对问题的独立判断是:在条目构造点按 UTF-8 字节封顶 preview,并移除 !truncated 对压缩的门控——因为该门控恰恰在压缩最需要运行时拒绝运行。PR 正是这么做的,而且做得干净。

已核实:

  • 封顶覆盖完整。 每条构造 catalog entry preview 的路径都经过新的字节封顶:记录索引累加器的 finish()(封顶在 catalogEntryBytes 度量之前应用,因此预算计量与实际条目一致)、经 evidencePreview()catalogEvidence() 与 checkpoint 累加器的 capture()(两个返回路径都已封顶)、以及 checkpointCatalogEntries() 中的 claim 投影。ASCII preview 逐字节不变,只有多字节 preview 变短。
  • 死锁在结构上被消除,而不只是报告的案例。 在 reducer 强制 GOAL_CHECKPOINT_CLAIM_LIMIT = 32 且 preview ≤ 240 字节的前提下,即使按最坏 JSON 转义,满载 claim 的 checkpoint 也远在 24,000 字节预算之内——"仅 checkpoint 就超预算"的状态不再可能出现。
  • capPreviewBytes 可靠for...of 按码点迭代,不会拆散代理对;与既有 capCheckpointContent 惯用法一致;入参恒 ≤ 240 字符,无性能顾虑。
  • 抢救路径保留了安全网。 什么都没捕获到的 truncated 窗口仍会以 usage_limitedlimitKind: 'evidence_catalog')停掉 Goal;flush/read checkpoint 失败在拆分后的测试中仍断言 usage_limiteddoes not accept catalog exhaustion as an external blocker(101 条记录、经 proposal 路径的条目上限截断)不受本 diff 影响。
  • 约定:ESM 导入、测试同目录、无 any、注释解释非显而易见的为什么。无 AGENTS.md 违规。

一个如实的保留意见(不阻塞):PR 描述称被抢救的 checkpoint 留下的更旧证据"已被上一个 checkpoint 的 claim 覆盖"——严格来说,被丢下的记录在游标之后,并不在先前 claim 中;抢救 checkpoint 落地后游标越过它们,它们失去可引用性。这仍严格优于旧行为(整个 Goal 以不可恢复的 usage_limited 死亡),且字节封顶就位后该路径只在 checkpoint 反复失败时可达——但合并此 PR 的维护者应知道:抢救的取舍是"最新证据存活、最旧的游标后记录被牺牲",而非无损。

另外说明:PR 正文中的变异测试结果系作者自述,本审查未独立重跑——下方 CI 才是本审查计为测试证据的部分。

测试证据——PR 自身的 CI

被审提交上的单元测试仍在运行,下表为实时快照;finalize 任务会在 CI 结束后更新它。macOS/Windows/integration 行显示 skippedpull_request 事件下的工作流设计(这些任务在 ci.yml 中仅在 merge queue 运行),不是本 PR 引入的缺口。目前无红色检查;precheck(lint/类型检查/格式)、依赖 CVE 审计、密钥扫描与 desktop-shell 构建均已通过。

(CI 表格见上方标记区域,此处不重复。)

沙箱验证可以解决 CI 无法解决的一点:@qwen-code /verify —— 绿色 CI 表明两个新测试通过,但它们钉住改动的论断(还原任一半则对应测试失败)来自作者自己的变异报告;对 base 构建做一次 A/B 运行即可证明测试在缺少该 diff 时失败。作者有写权限,可直接触发;本改动无 TUI 界面,/tmux 不适用。

Qwen Code · qwen3.8-max

Reviewed at 6b43e1ce40fb411da4ecbf86ec38f0a2c142acec · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — a clean, minimal fix for a deadlock I verified mechanically in the base tree; the two reservations below are about framing, not about the code.

Stepping back over the whole thing:

  • The problem is real and nasty — the unit mismatch between the 240-character preview cap and the 24,000-byte catalog budget made non-Latin Goals structurally unrunnable, and the !truncated gate then refused compaction in exactly the state compaction exists to resolve. The PR's account of the failure matches the code.
  • The approach matches the one I'd have proposed independently: a code-point-safe byte cap applied where catalog entries are built, plus letting a truncated-but-nonempty window compress. The implementation mirrors the existing capCheckpointContent idiom rather than inventing a new pattern, and the tests encode both regressions directly against the real runtime — one builds the exact 32-claim × 2,000-CJK-char worst case, the other drives 101 records through the runtime and asserts the Goal stays active with a checkpoint journal entry.
  • Every edit in the diff is needed; there is no drive-by churn. ASCII behavior is byte-for-byte unchanged, and persisted Goals re-project through the cap on the next window build, so there is no migration story to worry about.
  • If I were maintaining this in six months, the comments would thank me — they explain the unit mismatch and the salvage rationale at exactly the spots where the invariants are subtle.

The two reservations, neither blocking:

  1. The salvage tradeoff is slightly misdescribed in the PR body: the dropped records are post-cursor, so they are not covered by the previous checkpoint's claims, and the salvaged checkpoint's cursor moves past them. The honest description is "newest evidence survives, oldest post-cursor records lose citability" — still strictly better than the old unrecoverable stop, and only reachable after repeated checkpoint failures once the byte cap lands.
  2. The mutation-testing results are the author's own report; what this review counts is CI, still running on this commit.

Because CI has not settled on the reviewed commit, approval is deferred rather than posted now: this comment carries the deferred-approval marker, and the finalize job posts the commit-pinned approval once every check on that SHA completes green — withholding it if anything lands red or the head moves.

中文说明

置信度:4/5 —— 对一个我在基线代码中从机制上核实过的死锁,给出了干净且最小的修复;下面两点保留意见针对的是表述,不是代码。

整体回顾:

  • 问题真实且棘手——240 字符的 preview 上限与 24,000 字节 catalog 预算之间的单位不匹配,使非拉丁文字的 Goal 在结构上无法运行;而 !truncated 门控又恰恰在压缩最需要运行时拒绝压缩。PR 对故障的描述与代码一致。
  • 方案与我独立得出的方案一致:在 catalog entry 构造点施加码点安全的字节封顶,并允许 truncated 但非空的窗口做压缩。实现沿用了既有 capCheckpointContent 惯用法,而非另造模式;两个测试直接基于真实 runtime 编码了两条回归——一个构造 32 条 × 2,000 个 CJK 字符 claim 的最坏情形,另一个把 101 条记录送进 runtime,断言 Goal 保持 active 且 journal 记录 checkpoint
  • diff 中每处改动都是必要的,无夹带改动。ASCII 行为逐字节不变;已持久化的 Goal 会在下次构造窗口时重新经过封顶投影,因此没有迁移负担。
  • 六个月后维护这段代码时,注释会令人感激——它们在不变量微妙之处恰当地解释了单位不匹配与抢救逻辑的缘由。

两点保留意见,均不阻塞:

  1. 抢救取舍在 PR 正文中的表述略有偏差:被丢下的记录在游标之后,并未被上一个 checkpoint 的 claim 覆盖,且抢救 checkpoint 落地后游标会越过它们。如实的说法是"最新证据存活、最旧的游标后记录失去可引用性"——仍严格优于旧的不可恢复停止,且字节封顶就位后只在 checkpoint 反复失败时才可达。
  2. 变异测试结果为作者自述;本审查计为证据的是 CI,该提交上的 CI 仍在运行。

由于被审提交上的 CI 尚未结束,本次不直接批准、改为延迟批准:本评论携带延迟批准标记,finalize 任务会在该 SHA 上所有检查变绿后发布钉住该提交的批准——若有检查变红或 head 移动,则扣下不发。

(标记行见正文,此处不重复其文本。)

Qwen Code · qwen3.8-max

Reviewed at 6b43e1ce40fb411da4ecbf86ec38f0a2c142acec · re-run with @qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, looks ready to ship — CI landed green after the review. ✅

@qqqys
qqqys enabled auto-merge August 24, 2026 03:26
@qqqys
qqqys added this pull request to the merge queue Aug 24, 2026
Merged via the queue into QwenLM:main with commit 5cff52c Aug 24, 2026
90 of 91 checks passed
qqqys added a commit to qqqys/qwen-code that referenced this pull request Aug 25, 2026
QwenLM#9835 made a truncated evidence window compact instead of stopping the
Goal. That removed the only terminator on the path where compaction
runs and gives no relief: a Goal whose evidence rate outruns the
catalog pays a checkpoint verifier call every turn, loses evidence every
turn, and never converges on its own.

A checkpoint is counted as stalled when it comes back holding the
maximum number of claims while the window it compacted was already
truncated. Compaction has two levers, folding evidence into claims and
moving the cursor past what was folded; that combination means the
first is exhausted (the next checkpoint can only merge) and the second
is not keeping up (eligible evidence was left behind). A busy turn that
truncates with room in the claims is not a stall, and a full claim list
on a quiet Goal is not either.

The streak is persisted as `GoalRecord.checkpointStalls` (absent means
zero) so a restart or resume cannot launder it. Any check that finds
room resets it, and so do edit and replace. After three consecutive
stalls the Goal settles as `usage_limited` with the existing
`limitKind: 'evidence_catalog'` and a reason naming what stalled, how
many times, and what to do. No new limit kind, no branch in
`goalLimitKindForReason`, nothing crosses the wire.

Mutation probes (goal-runtime + goal-reducer + goal-checkpoint, 192
tests): no increment -> 3 fail; no reset on an effective checkpoint ->
1; no reset on a quiet check -> 1; threshold >= to > -> 1; parse drops
the key -> 1; parse never restores it -> 1; predicate ignores
truncation -> 2; predicate ignores the claim cap -> 1; edit stops
resetting -> 1. Every other test green in every run.
euntaek-hong pushed a commit to wrongbutworks/qwen-code that referenced this pull request Aug 25, 2026
…wenLM#9975)

* feat(goal): stop a Goal whose checkpoints stall three times in a row

QwenLM#9835 made a truncated evidence window compact instead of stopping the
Goal. That removed the only terminator on the path where compaction
runs and gives no relief: a Goal whose evidence rate outruns the
catalog pays a checkpoint verifier call every turn, loses evidence every
turn, and never converges on its own.

A checkpoint is counted as stalled when it comes back holding the
maximum number of claims while the window it compacted was already
truncated. Compaction has two levers, folding evidence into claims and
moving the cursor past what was folded; that combination means the
first is exhausted (the next checkpoint can only merge) and the second
is not keeping up (eligible evidence was left behind). A busy turn that
truncates with room in the claims is not a stall, and a full claim list
on a quiet Goal is not either.

The streak is persisted as `GoalRecord.checkpointStalls` (absent means
zero) so a restart or resume cannot launder it. Any check that finds
room resets it, and so do edit and replace. After three consecutive
stalls the Goal settles as `usage_limited` with the existing
`limitKind: 'evidence_catalog'` and a reason naming what stalled, how
many times, and what to do. No new limit kind, no branch in
`goalLimitKindForReason`, nothing crosses the wire.

Mutation probes (goal-runtime + goal-reducer + goal-checkpoint, 192
tests): no increment -> 3 fail; no reset on an effective checkpoint ->
1; no reset on a quiet check -> 1; threshold >= to > -> 1; parse drops
the key -> 1; parse never restores it -> 1; predicate ignores
truncation -> 2; predicate ignores the claim cap -> 1; edit stops
resetting -> 1. Every other test green in every run.

* fix(goal): preserve the stall streak when a check proves no room (QwenLM#9975)

finishCheckpointCheck reset checkpointStalls on all three call sites, but
only the room branch proved the window had relief. A transient verifier
failure or an empty turn now preserves the streak, so intermittent
checkpoint-verifier errors cannot launder the count and keep the stall
breaker from firing. Also moves withCheckpointStalls out from between
takeTurnTokens and its JSDoc.

* fix(goal): count unusable checkpoint results toward the stall limit (QwenLM#9975)

* fix(goal): surface unusable checkpoint verifier output to the stall breaker (QwenLM#9975)

* fix(goal): reset the stall streak when a resume restarts the evidence window

QwenLM#9840 landed after this branch opened: an evidence-limited Goal now
resumes by repointing the cursor and dropping the checkpoint, which is
a different evidence window from the one the streak was counted
against. Carrying the count across it spends the new window's
allowance on the old window's failures -- a Goal resumed at two stalls
would stop again after a single stalled checkpoint.

A resume that does NOT restart the window (paused, blocked) keeps the
streak: that Goal comes back to the same window, so what it learned
about that window is still true.

Mutation probe: removing the reset fails exactly the new resume test
(75 others green).

---------

Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.22.2.

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.

3 participants