Skip to content

feat(core): add configurable auto-compact threshold and Stop hook context usage (#4025) - #5868

Merged
wenshao merged 12 commits into
QwenLM:mainfrom
ZijianZhang989:feat/configurable-compact-threshold
Jun 28, 2026
Merged

feat(core): add configurable auto-compact threshold and Stop hook context usage (#4025)#5868
wenshao merged 12 commits into
QwenLM:mainfrom
ZijianZhang989:feat/configurable-compact-threshold

Conversation

@ZijianZhang989

Copy link
Copy Markdown
Collaborator

What this PR does

Implements two features requested in #4025 (comment by @kaisersong):

1. Configurable auto-compact threshold

Adds a new context.autoCompactThreshold setting in settings.json that lets users customize when auto-compaction triggers, instead of the hardcoded 70%.

{
  "context": {
    "autoCompactThreshold": 0.5
  }
}

The setting is a number between 0 and 1 (exclusive of 0, inclusive of 1). Invalid values silently fall back to the default 0.7. The threshold is passed to computeThresholds(window, pct?) which now accepts an optional pct parameter. All 4 production call sites are wired: chatCompressionService.ts (auto-compaction gate), geminiChat.ts (pre-send hard-cap rescue), contextCommand.ts (/context display), and useContextualTips.ts (tip thresholds).

Important design note: The three-tier threshold system uses max(proportional, absolute) where the absolute branch is effectiveWindow - 13K. For large context windows (>110K tokens), the absolute branch always dominates, so a custom threshold below ~0.7 has no visible effect on large-window models. Custom thresholds primarily affect small-window models (≤128K) where the proportional branch dominates.

2. Stop hook stdin includes context usage data

Adds three new optional fields to the Stop hook stdin payload: context_usage (0-1 ratio), context_limit (context window size in tokens), and input_tokens (current prompt token count). This enables hook scripts to observe context usage and implement custom compact strategies — for example, a script that prints a reminder to run /compact when usage exceeds a custom threshold.

{
  "session_id": "...",
  "stop_hook_active": true,
  "last_assistant_message": "...",
  "context_usage": 0.72,
  "context_limit": 200000,
  "input_tokens": 144000
}

A new buildContextUsage(windowSize, inputTokens) helper in packages/core/src/hooks/context-usage.ts constructs the ContextUsageData object (or returns undefined when inputs are invalid). Three callers are wired: Session.ts, client.ts, and the messageBus Stop handler in config.ts.

What this PR does NOT do

  • Does not add a hook-driven compact trigger. Stop hooks can observe context usage and suggest compaction via additionalContext, but cannot directly trigger /compact. A future PR could add a triggerCompact: boolean field to StopOutput for this.
  • Does not redesign the three-tier threshold system. The existing max(proportional, absolute) formula is unchanged; this PR only makes the proportional component configurable.
  • Does not fix the original cxt percentage display. The original issue report about inaccurate cxt display was investigated and found to be working correctly — lastPromptTokenCount is only updated on API responses, which is a fundamental API constraint, not a bug.

Why it's needed

Users with small-window models (32K-128K) want to control when auto-compaction triggers — earlier to preserve more headroom, or later to retain more conversation context. The hardcoded 70% is a reasonable default but doesn't fit all workflows.

Hook script authors need visibility into context usage to implement custom strategies. Without context_usage in the Stop hook payload, there's no way for external scripts to know how full the context window is.

Reviewer Test Plan

How to verify

# Feature 1: Configurable threshold
# Add to ~/.qwen/settings.json:
# { "context": { "autoCompactThreshold": 0.99 } }
# Restart CLI, run /context, verify Auto threshold changes
# (On 1M window: default=967K, custom 0.99=990K)

# Feature 2: Stop hook context data
# Add a Stop hook that dumps stdin:
# { "hooks": { "Stop": [{ "matcher": "", "hooks": [{ "type": "command", "command": "cat > /tmp/stop-hook.json" }] }] } }
# Restart CLI, have one conversation turn, check:
# cat /tmp/stop-hook.json | jq '{context_usage, context_limit, input_tokens}'

Evidence (Before & After)

Before: /context always shows Auto threshold at ~70% of window (or absolute branch for large windows). Stop hook stdin has only stop_hook_active and last_assistant_message.

After: /context Auto threshold reflects settings.json value when it exceeds the absolute branch. Stop hook stdin includes context_usage, context_limit, input_tokens.

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

Environment

Node v26.2.0, macOS arm64, dev build from worktree.

Risk & Scope

  • Schema nesting sensitivity: autoCompactThreshold must be at context.properties level in settingsSchema.ts — placing it inside a nested sub-object (like fileFiltering.properties) causes a silent TypeScript error that only appears on clean build (incremental tsc --build cache hides it).
  • Large window no-op: Users with 1M+ context models won't see a visible change unless they set threshold > ~0.97. This is by design, not a bug.
  • Hook observation only: Stop hooks can observe but not trigger compaction. This is a deliberate scope boundary.
  • Breaking changes: None. All changes are additive — existing behavior is preserved when settings are not configured.

Linked Issues

Closes #4025

中文说明

这个 PR 做了什么

实现了 #4025@kaisersong 评论里提出的两个功能:

1. 可配置的 auto-compact 阈值

settings.json 中新增 context.autoCompactThreshold 配置项,允许用户自定义 auto-compact 触发比例(默认 0.7)。配置值通过 computeThresholds(window, pct?) 传递给所有 4 个调用点。注意:三级阈值系统使用 max(比例, 绝对值) 公式,大窗口(>110K)时绝对分支始终主导,自定义阈值主要影响小窗口模型。

2. Stop hook stdin 包含 context 使用率

Stop hook 的 stdin payload 新增 context_usagecontext_limitinput_tokens 三个字段,让用户可以通过 hook 脚本观察上下文使用情况并自定义 compact 策略。

这个 PR 没有做什么

  • 不支持 hook 直接触发 compact(Stop hook 只能观察和建议,不能执行 /compact
  • 不改变三级阈值系统的设计公式
  • 不修复原始 issue 中 cxt 百分比显示问题(经调查该行为正确,不是 bug)

为什么需要

小窗口模型用户(32K-128K)需要控制 auto-compact 的触发时机。Hook 脚本作者需要感知上下文使用率来实现自定义策略。

风险与范围

  • Schema 嵌套敏感:autoCompactThreshold 必须在 context.properties 层级
  • 大窗口无变化:1M+ 模型除非设 >0.97 否则看不到变化(这是设计意图)
  • Hook 只能观察不能触发 compact(明确的 scope boundary)
  • 无 breaking changes

关联 Issues

Closes #4025

…text usage (QwenLM#4025)

Add two features requested in issue QwenLM#4025:

1. Configurable auto-compact threshold via settings.json
   - Add context.autoCompactThreshold setting (0-1, default 0.7)
   - Extend computeThresholds(window, pct?) to accept optional pct parameter
   - Wire all 4 call sites (chatCompressionService, geminiChat, contextCommand, useContextualTips)
   - Large windows (>110K) dominated by absolute branch, custom threshold mainly affects small windows

2. Stop hook stdin payload includes context usage data
   - Add ContextUsageData interface and buildContextUsage helper
   - Extend StopInput with context_usage, context_limit, input_tokens fields
   - Wire 3 callers (Session.ts, client.ts, config.ts)
   - Enables hook scripts to observe context usage and suggest compact strategies
@qwen-code-ci-bot

qwen-code-ci-bot commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the PR, @ZijianZhang989!

Template looks good ✓

On direction: both features are well-aligned with the project's scope. Configurable auto-compact threshold directly addresses a real pain point for small-window model users (#4025), and exposing context usage data in Stop hooks is a natural extension of the hook observation model. No direct CHANGELOG reference in comparable tools, but the area (context management, hook extensibility) is squarely within qwen-code's core mission.

On approach: the scope feels right — two complementary features that share the context-management domain, with a focused diff (+492/-32 across 24 files). The computeThresholds(window, pct?) signature with an optional parameter is minimal and non-breaking. Validation lives in the right place (getAutoCompactThreshold() returns undefined for out-of-range values, letting computeThresholds fall back to DEFAULT_PCT). The buildContextUsage helper is clean and reusable. All four call sites wired correctly.

Moving on to code review and testing. 🔍

中文说明

感谢贡献,@ZijianZhang989

模板完整 ✓

方向:两个功能都与项目范围高度对齐。可配置的 auto-compact 阈值直接解决了小窗口模型用户的真实痛点(#4025),Stop hook 中暴露 context 使用率是 hook 观察模型的自然延伸。没有直接的 CHANGELOG 参考,但方向(上下文管理、hook 扩展性)属于 qwen-code 核心使命。

方案:范围合理——两个互补的功能共享上下文管理领域,diff 聚焦(+492/-32,24 个文件)。computeThresholds(window, pct?) 的可选参数签名最小化且无破坏性变更。验证放在了正确的位置(getAutoCompactThreshold() 对越界值返回 undefined,让 computeThresholds 回退到 DEFAULT_PCT)。buildContextUsage 工具函数简洁可复用。四个调用点全部正确接入。

进入代码审查和测试 🔍

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

qwen-code-ci-bot commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Code Review

Independent proposal (before reading the diff): To add a configurable auto-compact threshold, I would: add context.autoCompactThreshold to the settings schema, add a getAutoCompactThreshold() getter with range validation, thread an optional pct parameter through computeThresholds, and wire all call sites. For Stop hook context usage, I'd create a small buildContextUsage helper that constructs the data object (or returns undefined when inputs are invalid), add optional fields to StopInput, and spread at the three call sites.

Comparison: The PR's implementation matches this approach exactly. Specific findings:

  • computeThresholds(window, pct?) — Clean extension. The effectivePct = Math.min(1, Math.max(0, ...)) clamping is correct and defensive. The warn <= auto < hard invariant holds for valid pct values (tested at 0.3–0.9). Edge cases (pct=0, pct=1, NaN, negative) are handled with appropriate test expectations.
  • getAutoCompactThreshold() — Validation in the getter (> 0 && <= 1) is the right place. Returns undefined for invalid values, letting computeThresholds fall back to default 0.7. Matches the PR's claim of "silently falls back".
  • buildContextUsage(windowSize, inputTokens) — Minimal helper (21 lines), correct guards (!contextWindowSize, !Number.isFinite, <= 0 for both params), properly typed with ContextUsageData. Spread pattern (...contextUsage) gracefully handles undefined.
  • config.ts messageBus handler — Uses buildContextUsage() for runtime validation of serialized input. Correct.
  • Schema integrationautoCompactThreshold correctly placed at context.properties level with jsonSchemaOverride: { minimum: 0.01, maximum: 1 }. The vscode schema JSON also updated consistently.
  • Test coverage — Thorough: computeThresholds custom pct tests (11 new tests including clamping/NaN), buildContextUsage (8 new tests), hook event forwarding (4 new tests), compression service integration (2 new tests), MCP Stop dispatch validation. All mock configs updated across existing tests.

No critical bugs, no security concerns, no AGENTS.md violations. The diff is focused — every edit serves the stated goals. No drive-by refactors or scope creep.

Reuse Check

  • buildContextUsage is a new helper, but it's minimal (21 lines) and used by three callers. No existing utility covers this.
  • The ContextUsageData type is correctly placed in hooks/types.ts alongside other hook types.

Unit Test Results

All PR-touched test files pass:

Test File Tests Status
context-usage.test.ts (new) 8
hookEventHandler.test.ts 127
hookSystem.test.ts 89
chatCompressionService.test.ts 84
config.test.ts 261
geminiChat.test.ts 192
contextCommand.test.ts 10
Total 771 All pass

Build: 0 errors, 0 new warnings. Typecheck: 0 errors.

Real-Scenario Testing (tmux)

Tested the Stop hook context usage feature via npm run dev -- -p with a Stop hook configured in $QWEN_HOME/settings.json that captures stdin to a file.

Tmux session output (PR code, headless -p mode)

$ npm run dev -- -p 'What is 3+3? Answer briefly.'

> @qwen-code/qwen-code@0.19.2 dev
> node scripts/dev.js -p What is 3+3? Answer briefly.

DEV is set to true, but the React DevTools server is not running. Start it with:

$ npx react-devtools

6

Stop hook stdin captured by the hook:

{
  "session_id": "7bb59801-9874-4cd8-aa47-7e366bb350c5",
  "transcript_path": "/home/github-runner/actions-runner-25/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-25--work-qwen-code-qwen-code/chats/7bb59801-9874-4cd8-aa47-7e366bb350c5.jsonl",
  "cwd": "/home/github-runner/actions-runner-25/_work/qwen-code/qwen-code",
  "hook_event_name": "Stop",
  "timestamp": "2026-06-28T02:15:49.821Z",
  "stop_hook_active": true,
  "last_assistant_message": "The user asked \"What is 3+3? Answer briefly.\" This is a simple arithmetic question. Let me just answer it directly.\n\n3 + 3 = 6",
  "context_usage": 0.1661834716796875,
  "context_limit": 131072,
  "input_tokens": 21782
}

Observation: The Stop hook fires correctly with stop_hook_active: true, last_assistant_message populated, and the new context_usage (0.166), context_limit (131072), and input_tokens (21782) fields all present and populated with valid values. This confirms both features work end-to-end in a real session.

中文说明

代码审查

独立方案(阅读 diff 前): 添加可配置的 auto-compact 阈值:在 settings schema 中添加 context.autoCompactThreshold,添加带范围验证的 getter,给 computeThresholds 传递可选 pct 参数,接入所有调用点。Stop hook context 使用率:创建小的 buildContextUsage helper,在 StopInput 添加可选字段,在三个调用点展开。

对比: PR 实现与此方案完全一致。具体发现:

  • computeThresholds(window, pct?) — 干净的扩展。effectivePct 的 clamping 正确且有防御性。warn <= auto < hard 不变式在有效 pct 值下成立。
  • getAutoCompactThreshold() — 验证放在 getter 中是正确的位置。无效值返回 undefined
  • buildContextUsage — 最小化 helper(21 行),正确的守卫。展开模式优雅处理 undefined。
  • 测试覆盖 — 全面:computeThresholds 自定义 pct 测试(11 个新测试含 clamping/NaN)、buildContextUsage(8 个新测试)、hook 事件转发(4 个新测试)、压缩服务集成(2 个新测试)。

无关键 bug、无安全问题、无 AGENTS.md 违规。 diff 聚焦。

单测结果

所有 PR 涉及的测试文件通过:771 测试全部通过。Build 0 errors,typecheck 0 errors。

真实场景测试(tmux)

通过 npm run dev -- -p 测试 Stop hook context 使用率功能。

观察: Stop hook 正确触发,stop_hook_active: truelast_assistant_message 已填充,新增的 context_usage(0.166)、context_limit(131072)、input_tokens(21782)字段全部存在且有有效值。确认两个功能在真实会话中端到端工作正常。

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

qwen-code-ci-bot commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Stepping back: this is a clean, well-thought-out PR that ships exactly what it promises. The author clearly understands the codebase — the PR description anticipates edge cases (large-window no-op, schema nesting sensitivity, hook observation-only scope), the code is minimal and focused, and the test coverage is thorough.

The implementation matches my independent proposal. computeThresholds(window, pct?) with an optional parameter is the right extension — non-breaking, no new abstractions. Validation in the getter (> 0 && <= 1) rather than at call sites is the right layering decision. The buildContextUsage helper is exactly the right size — 21 lines, one job, three callers. Every change in the diff serves one of the two stated goals.

The real-scenario tmux test confirmed the Stop hook context usage feature works end-to-end: context_usage, context_limit, and input_tokens all appeared in the hook stdin payload with valid values after a simple conversation turn. This is a meaningful improvement over the previous triage run where headless mode timing prevented the fields from populating — the fix commits (NaN guard, Number.isFinite checks, ?? DEFAULT_TOKEN_LIMIT fallbacks) have made the feature more robust.

The author has been exceptionally responsive to review feedback — 8 fix commits addressing CI failures, NaN guards, schema constraints, DRY improvements, documentation updates, pct clamping, and TypeScript type narrowing. Each commit is focused and well-described. The review history shows a collaborative process, not a contentious one.

771 unit tests pass across 7 test files, build passes with 0 errors, typecheck passes with 0 errors. The diff is focused — no drive-by refactors, no scope creep, no unnecessary abstractions.

If I had to maintain this in six months, I'd thank the author — the validation logic, the spread pattern for optional fields, the clear boundary between "configurable threshold" and "redesigning the three-tier system", and the thorough test coverage make the intent obvious.

Looks good, shipping it. ✅

中文说明

退一步看:这是一个干净、考虑周全的 PR,准确交付了承诺的功能。作者对代码库的理解清晰——PR 描述预判了边界情况(大窗口无变化、schema 嵌套敏感性、hook 只观察不触发),代码最小化且聚焦,测试覆盖充分。

实现与我的独立方案一致。computeThresholds(window, pct?) 的可选参数是正确的扩展方式——无破坏性变更,无新抽象。验证放在 getter 中而非调用点,是正确的分层决策。buildContextUsage helper 大小恰到好处——21 行,一个职责,三个调用者。

真实场景 tmux 测试确认 Stop hook context 使用率功能端到端工作正常:简单对话轮次后,context_usagecontext_limitinput_tokens 全部出现在 hook stdin payload 中且有有效值。相比之前的 triage(headless 模式时间约束导致字段未填充),修复提交(NaN 守卫、Number.isFinite 检查、?? DEFAULT_TOKEN_LIMIT 回退)使功能更加健壮。

作者对 review 反馈积极响应——8 个修复提交涵盖 CI 修复、NaN 守卫、schema 约束、DRY 改进、文档更新、pct clamping 和 TypeScript 类型收窄。每个提交聚焦且描述清晰。

771 个单测通过(7 个测试文件),build 0 errors,typecheck 0 errors。diff 聚焦——无顺手重构、无范围蔓延、无不必要的抽象。

如果六个月后需要维护这个代码,我会感谢作者。看起来不错,可以合并。✅

Qwen Code · qwen3.7-max

@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. ✅

@qwen-code-ci-bot qwen-code-ci-bot added category/core Core engine and logic scope/token-management Token handling and limits scope/settings Settings and preferences type/enhancement Non-bug improvement or optimization labels Jun 25, 2026

@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.

⚠️ Downgraded from Request changes to Comment: CI failing: Test (ubuntu-latest, Node 22.x).

Overall: feature implementation is clean, build + 313 unit tests pass locally in the worktree, deterministic checks (tsc, eslint) report 0 findings. Two minor robustness suggestions and one documentation gap inline.

Also worth noting: docs/users/features/hooks.md (lines 562-571) documents the Stop hook's input schema with only stop_hook_active and last_assistant_message, but this PR adds three new optional fields (context_usage, context_limit, input_tokens) to the Stop hook stdin payload. Hook authors reading the docs will not discover these fields without reading the source.

— qwen3.7-max via Qwen Code /review

Comment thread packages/core/src/hooks/context-usage.ts Outdated
Comment thread packages/core/src/config/config.ts
@ZijianZhang989

ZijianZhang989 commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator Author

Review fixes

Pushed commit 4fa164b7f addressing CI failure and review feedback:

  1. CI fix: Added getAutoCompactThreshold: vi.fn().mockReturnValue(undefined) to geminiChat.test.ts mock config — 128 tests were failing because this.config.getAutoCompactThreshold() was called but the mock didn't have it.

  2. NaN guard (context-usage.ts:7): Added !Number.isFinite(inputTokens) check to buildContextUsage. Without this, buildContextUsage(200_000, NaN) bypassed all guards and returned { context_usage: NaN }. Added test case for NaN input.

  3. getAutoCompactThreshold unit tests (config.ts:4589): Deferred — validation logic is already tested indirectly through computeThresholds tests and the integration test with custom threshold. Adding dedicated unit tests for this getter is a follow-up.

Comment thread packages/core/src/core/geminiChat.ts
Comment thread packages/core/src/services/chatCompressionService.ts
Comment thread packages/cli/src/config/settingsSchema.ts
Comment thread packages/core/src/hooks/types.ts
Comment thread packages/cli/src/config/settingsSchema.ts
Comment thread packages/core/src/config/config.ts Outdated
Comment thread packages/core/src/config/config.ts
Comment thread packages/core/src/config/config.ts
…ontextUsageData>, buildContextUsage validation
@ZijianZhang989

ZijianZhang989 commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator Author

Review fixes

Pushed commit 6122695fc addressing review feedback:

Fixed:

  1. geminiChat.test.ts mock (Critical) — Added getAutoCompactThreshold: vi.fn().mockReturnValue(undefined) to mock config. All 128 tests now pass.
  2. Schema minimum/maximum constraints — Added minimum: 0.01, maximum: 1 via jsonSchemaOverride (SettingDefinition type doesn't support these directly). Description updated to 'greater than 0, up to 1'.
  3. StopInput DRY — Changed to extends HookInput, Partial<ContextUsageData> to derive optional fields from the named type.
  4. config.ts ContextUsageData validation — Replaced manual as number casts with buildContextUsage() which validates via Number.isFinite() and positivity checks.

Deferred:

  • getAutoCompactThreshold unit tests (config.ts:4589) — validation logic already tested indirectly through computeThresholds custom-pct tests and compress integration test.

Rejected:

  • pct=1 invariant violation (chatCompressionService.ts:173) — The formula hard = Math.min(window, Math.max(rawHard, auto + HARD_BUFFER)) ensures hard >= auto. With pct=1 on 200K: auto=200K, hard=Math.min(200K, max(rawHard, 203K))=200K. Both collapse to window size — this is the documented degenerate case, not a safety gap. The hard-rescue never triggers when tokens already exceed the window (API has already failed at that point).
  • pct=1 rejection in getter (config.ts:4591) — Same reasoning: pct=1 is a valid but degenerate setting. Users choosing this accept the tradeoff. The behavior is documented in test cases and comments.

@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.

[Suggestion] Stop hook documentation not updated

The Stop hook section in docs/users/features/hooks.md (lines 560-579) documents only stop_hook_active and last_assistant_message as event-specific input fields. The three new optional fields added by this PR (context_usage, context_limit, input_tokens) are not mentioned. Hook authors have no way to discover these fields without reading source code.

Consider adding the new fields to the docs with their optionality semantics — they are absent on the first turn and after context resets (when lastPromptTokenCount is 0), so hook scripts should handle missing values gracefully.

— qwen3.7-max via Qwen Code /review

@wenshao wenshao 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.

Documentation gaps (not in the diff, but load-bearing for this feature)

The code changes look clean (build + 506 unit tests pass in the worktree; tsc/eslint 0 findings). The two findings below are documentation pages that need updating for this feature to be discoverable and usable by end users.

[Suggestion] docs/users/configuration/settings.md:152 — the page still documents model.chatCompression.contextPercentageThreshold as REMOVED with "no longer user-configurable" and no replacement mentioned. This PR reintroduces the capability as context.autoCompactThreshold, but no new row is added to the context.* table. Users reading the settings reference will believe the feature is permanently gone and will never discover the new setting. Add a context.autoCompactThreshold row (type number, range (0, 1], default undefined (0.7)) near the other context.* entries, and update line 152's REMOVED note to point to the new setting name.

[Suggestion] docs/users/features/hooks.md:566 — the Stop hook "Event-specific fields" JSON block documents only stop_hook_active and last_assistant_message, but this PR adds three optional fields (context_usage, context_limit, input_tokens) to the stdin payload via Partial<ContextUsageData>. Hook authors — the primary audience of this feature — won't know the data is available. Add the three optional fields to the JSON block with type annotations, e.g. "context_usage": "number (ratio 0-1+, optional)", "context_limit": "number (context window tokens, optional)", "input_tokens": "number (prompt token count, optional)".

— qwen3.7-max via Qwen Code /review (second opinion pass; inline findings from the prior automated review rounds are unchanged and tracked in the 10 existing inline comments)

Comment thread packages/cli/src/config/settingsSchema.ts
Comment thread packages/core/src/hooks/types.ts
@ZijianZhang989

ZijianZhang989 commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator Author

Review fixes

Pushed commit e5a2bbf19 addressing documentation requests:

  1. settings.md — Added context.autoCompactThreshold to the #### context section with full description (valid range, default, large-window behavior note). Updated the old model.chatCompression.contextPercentageThreshold entry to reference the new setting as its replacement.

  2. hooks.md — Added context_usage, context_limit, and input_tokens to the Stop hook event-specific fields documentation, with a note explaining their purpose for custom compact strategies.

Comment thread packages/cli/src/acp-integration/session/Session.ts Outdated
Comment thread packages/core/src/core/client.ts Outdated
Comment thread docs/users/features/hooks.md
Comment thread packages/cli/src/ui/commands/contextCommand.test.ts
Comment thread packages/core/src/services/chatCompressionService.ts
@ZijianZhang989

ZijianZhang989 commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator Author

Review fixes

Pushed commit b15b23448 addressing 5 bot review findings:

  1. Session.ts:1679 + client.ts:2286 (Critical) — Added ?? DEFAULT_TOKEN_LIMIT fallback to buildContextUsage calls. Without this, users who don't set generationConfig.contextWindowSize get undefinedbuildContextUsage(undefined, ...) returns undefined → Stop hook stdin carries no context fields. Now consistent with all other computeThresholds call sites.

  2. hooks.md:568 (Suggestion) — Fixed field descriptions: context_usage now notes it may exceed 1 (verified by test); input_tokens now notes it may include output tokens depending on provider fallback.

  3. contextCommand.test.ts (Suggestion) — Added test that mocks getAutoCompactThreshold returning 0.5 and asserts the /context thresholds match computeThresholds(32000, 0.5).

  4. chatCompressionService.ts:162 (Suggestion) — Added Math.min(1, Math.max(0, ...)) clamp to effectivePct so computeThresholds is safe against out-of-range values from external callers. Existing tests unaffected since all valid inputs produce identical outputs.

@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.

⚠️ Downgraded from Approve to Comment: CI still running.

Overall the implementation is clean — build passes (315 tests), tsc/eslint report 0 findings. One minor Suggestion below.

Comment thread packages/cli/src/ui/commands/contextCommand.test.ts Outdated
@ZijianZhang989

ZijianZhang989 commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator Author

Review fix

Fixed documentation error in test comment (contextCommand.test.ts:256):

The comment incorrectly stated warn: 9,600 for computeThresholds(32000, 0.5). Corrected to warn: 16,000 which matches the actual computation: warn = max((0.5 - 0.1) * 32000, 16000 - 20000) = max(12800, -4000) = 12800, then max(12800, absWarn) → 16000 (matching auto).

Commit: 7234694

@doudouOUC doudouOUC 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.

[Suggestion] Test coverage gaps for context usage propagation at call sites

The new context usage fields (context_usage, context_limit, input_tokens) added to the Stop hook stdin are not fully verified at two call sites:

  1. packages/cli/src/acp-integration/session/Session.test.ts — The existing Stop hook test uses objectContaining but does not assert the new context usage fields. The test would pass identically if the ...contextUsage spread were removed from Session.ts.

  2. packages/core/src/config/config.ts:1751-1765 — The messageBus Stop handler reconstructs ContextUsageData from raw input fields via buildContextUsage, but has no test verifying that context_limit and input_tokens are correctly reconstructed and forwarded to fireStopEvent.

Suggested fix: Add explicit assertions for context usage fields in the Session.ts Stop hook test (with a controlled lastPromptTokenCount). Add a test for the config.ts messageBus Stop handler verifying context usage reconstruction.

— qwen3.7-max via Qwen Code /review

Comment thread packages/core/src/services/chatCompressionService.test.ts Outdated
@ZijianZhang989

ZijianZhang989 commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator Author

Review fix

Fixed misleading test in chatCompressionService.test.ts:2190:

Test name: Changed from "auto=window but hard capped below window" to "auto and hard both equal window"

Actual computation:

  • auto = max(1 × 200000, 180000 - 13000) = max(200000, 167000) = 200000
  • hard = min(200000, max(177000, 200000 + 3000)) = min(200000, 203000) = 200000

So hard = window = 200000, not "capped below". Added assertion for hard and corrected the test name.

Commit: c047c9a

@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.

Implementation is clean — build passes (315 tests), tsc/eslint report 0 findings. Both features (configurable auto-compact threshold and Stop hook context usage) are well-wired across all call sites with proper validation layers.

— qwen3.7-max via Qwen Code /review

Comment thread packages/core/src/services/chatCompressionService.ts Outdated
@ZijianZhang989

ZijianZhang989 commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator Author

修复了一个新的 review 意见:

问题: 当 设置得很低时(schema 允许 minimum: 0.01), 可能产生负数的 阈值。例如 会产生 ,导致:

  1. 显示负数的 token 数量给 Warn 阈值
  2. 会将每个非负 token 数量分类为至少 "warn"

修复: 在 计算外层添加 确保不会产生负数阈值

Commit: b66a71339

Comment thread packages/core/src/services/chatCompressionService.ts Outdated
Comment thread packages/core/src/services/chatCompressionService.ts Outdated
Comment thread packages/core/src/config/config.ts
俊良 added 3 commits June 26, 2026 19:14
…ling

Add tests for out-of-range pct values (-0.5, 1.5, NaN) to verify
computeThresholds clamping behavior. Fix implementation to use
Number.isFinite() check so NaN falls back to DEFAULT_PCT instead
of propagating through Math.max(0, NaN) which yields NaN.
Add tests for buildContextUsage runtime validation in MCP Stop dispatch path:
- Valid numeric inputs produce correct ContextUsageData
- Missing/undefined fields return undefined
- String values rejected by Number.isFinite validation
- Negative values return undefined

Also add Number.isFinite check for contextWindowSize in buildContextUsage
to properly validate MCP input types at runtime.
…rameter

Use explicit undefined check before Number.isFinite to properly narrow
the number | undefined type in the ternary expression.
@ZijianZhang989

ZijianZhang989 commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator Author

Issue: The recent Number.isFinite(pct) check in chatCompressionService.ts:166 didn't properly narrow the number | undefined type in the ternary expression, causing:

error TS2345: Argument of type 'number | undefined' is not assignable to parameter of type 'number'.
  Type 'undefined' is not assignable to type 'number'.

Fix: Changed from Number.isFinite(pct) ? pct : DEFAULT_PCT to pct !== undefined && Number.isFinite(pct) ? pct : DEFAULT_PCT to explicitly check for undefined first, allowing TypeScript to properly narrow the type.

Commit: e75ada9a7 - fix(chatCompressionService): fix TypeScript type narrowing for pct parameter

@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.

⚠️ Downgraded from Approve to Comment: CI still running.

Overall the implementation is clean — build passes (579 tests), tsc/eslint report 0 PR-related findings. Both features (configurable auto-compact threshold and Stop hook context usage) are well-wired across all call sites with proper validation layers. One minor suggestion below.

— qwen3.7-max via Qwen Code /review

// 0 * 32000 = 0, absolute branch is negative → auto = 0
expect(t.auto).toBe(0);
// warn = max((0 - 0.1) * 32000, absWarn) = -3200
expect(t.warn).toBeLessThanOrEqual(t.auto);

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.

[Suggestion] The Math.max(0, ...) wrapper was specifically added to prevent negative warn values, but this test only asserts t.warn <= t.auto — which passes even if warn were -3200 (since -3200 <= 0). The non-negative invariant the wrapper establishes is not pinned.

Suggested change
expect(t.warn).toBeLessThanOrEqual(t.auto);
expect(t.warn).toBe(0);

— qwen3.7-max via Qwen Code /review

@wenshao
wenshao requested a review from LaZzyMan June 26, 2026 15:33
@wenshao

wenshao commented Jun 28, 2026

Copy link
Copy Markdown
Collaborator

@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. ✅

@doudouOUC doudouOUC 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 ✅ — both features (configurable auto-compact threshold and Stop hook context usage) are clean, well-wired across all call sites, and thoroughly tested. The implementation handles edge cases (NaN, negative, >1 pct values, zero tokens) correctly through layered validation.

— qwen3.7-max via Qwen Code /review

@wenshao
wenshao added this pull request to the merge queue Jun 28, 2026
Merged via the queue into QwenLM:main with commit 8daeb5b Jun 28, 2026
48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category/core Core engine and logic scope/settings Settings and preferences scope/token-management Token handling and limits type/enhancement Non-bug improvement or optimization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Statusline context (cxt) percentage is inaccurate

5 participants