Repository navigation
Conversation
…mment Review of ranxianglei#5055 (in passing, no behavior change): - src/nudge-decide.ts: DECIDE_TIMEOUT_MS back-compat alias had zero importers (server.ts imports DEFAULT_DECIDE_TIMEOUT_MS) and its doc comment claimed it was imported by server.ts — dead code + wrong comment, removed. - tests/issue3004-nudge-decide-selfheal.test.ts: detector comment still said "default 200" while its own threshold is 1024 since this PR (the sibling e2e file's comment was updated; this one was missed).
(ai: vllm-qwen/qwen3.8-27b) 查重: 不重复 — open+closed issue/PR 检索均无第二个 #5030 decide-lane 修复;同家族近亲是已关闭但修复未合入的 #2666/#4463(summary lane 的同类 population gap),lane 不同、不冲突。本 PR 是 #5030 option A 的唯一在途实现。 方向关(独立审核,不复读 issue):方向成立——#5030 的两个失败模式(200 token 预算被隐藏思考吃空 / 硬编码 15s < 全上下文 prefill 25–60s)都是「常量与默认思考/慢 prefill provider 不匹配」,option A 把常量变成配置+调对默认值,打到了根因层而不是在 ladder 里打补丁。两点与 PR body 不符、需 owner 知悉:① A2 实际走 回归审查(逐既有行为过):
顺手小修(已推送到 PR 分支,commit
结构化八问:
收工判定:代码质量过关、可合状态——但按 Configuration Surface Discipline 与 §7.4,新配置键 + 默认值变更属 owner 拍板面,合并前需要 owner 确认 option A 的配置面形状(含上面两点:A2 三级级联 vs body 的 "global";枚举 strip/follow/off vs floor-8 的 off/template-off)。作者本人也声明 "happy to adjust to B/C or close at the owner's call"。owner 确认后此 PR 即可由人工合并。 |
(ai: vllm-qwen/qwen3.8-27b) 状态确认(进程曾重启一次,工作已全部完成,无遗留):
|
…not a sideBody transport key (ranxianglei#5055) Review of ranxianglei#5055: the resolved nudgeDecisionTimeoutMs was smuggled through the outbound body under a magic key that runNudgeDecision had to peel off before serialization. The args object already exists for exactly this purpose; passing it there removes the whole forgot-to-strip wire-leak class.
…irections (ranxianglei#5055) The A1 knob was the load-bearing half of the fix with zero coverage. Slow side-call upstream + budget below latency -> decision aborts, nothing injected this round; same latency with a larger budget -> decision lands and the directive is injected.
|
(ai: vllm-qwen/qwen3.8-27b) Independent review of #5055 at ① Direction (issue first)The PR implements option A from the #5030 decision request: A1 ② Regression audit — existing behaviors of the touched paths
③ Code — findings, fixed directly (pushed)
④ Test validityUnit tri-state pins ⑤ Diff cleanliness13 files, every line purpose-relevant; no whitespace churn; docs mirrored en+zh; config-reference entries present. Clean. Config surface (per AGENTS.md discipline)
结构化八问
待 owner 拍板(配置面属 owner 设计区,我不越权)
判定: 代码与测试质量达到可合标准;因触碰配置面 + 默认值变更,按 §7.4 必须人工合入,等 owner 对 D1–D3 拍板。CI 已在推送头运行。 |
(ai: vllm-qwen/qwen3.8-27b) 交叉核验(针对新增的两个 review commit,当前 head 我独立验证了什么(不是复读上面那条审核):
八问增量(相对我上轮结论,仅列变化项): 剩余待办(全部在人工侧):
判定:head |
|
(ai: vllm-qwen/qwen3.8-27b) Synced: merged origin/master |
|
(ai: vllm-qwen/qwen3.8-27b) Re-synced: master moved again between guard wakes ( |
|
(ai: vllm-qwen/qwen3.8-27b) Re-synced (round 4): master moved to |
|
(ai: vllm-qwen/qwen3.8-27b) Re-synced (round 5): master at |
[bot] 🏷 Merge-guard sync(第 5 轮)完成:master 前进 4 个提交到 当前状态:分支与 master 同步、无冲突;剩余动作在 owner——fork PR 的全部 CI lane 停在 "Approve and run" 审批门(Actions 标签页),批准后 CI 才会跑。 |
[bot] 🏷 这条转发的是我自己刚发的同步结果评论(镜像回显),无新输入。状态不变:分支已同步至 master( |
|
(ai: deepseek/deepseek-v41-flash) 与 #5339 同型的 alert(改 YAML 种子却不重新生成),但这次不用等合并 —— head 自己就红了。 事实([实测])head = 我在它的 head 上单独建 worktree(不合并任何东西),跑 required check 的那条命令: 根因它改了 另外 但四个生成物一个都没重新生成。实测
⇒ 为什么 CI 没告诉你
查 所以这个红属于「本地一条命令就能看见、CI 永远不会通知你」的类型。 修法(作者一条命令)node tools/gen-config-docs.mjs generate
git add CONFIGURATION.md CONFIGURATION.zh-CN.md website/main.js website/docs/index.html
git commit -m "docs: regenerate after the decide-lane config keys (#5030)"
我为什么不代改该 head 在别人 fork( (同类先例: |
[bot] 🏷 @Louie0806 发现已独立复现并修复。我在新 head 上单独跑 按本仓规则(外部 PR 质量缺口 → 直接在贡献者分支上补 additive commit,不 rebase/不替换、不动作者署名),我没有等作者,已代为推送:
推送后状态:head = 行为变更披露(非本次引入): |
… round 7) # Conflicts: # src/config.ts # src/server/prepare-anthropic.ts
…warn flags Review nits pushed per owner rule (小问题直接修), PR ranxianglei#5659: - src/agent/pi.ts: align the four pi.commands gates with their block (9->10 spaces) - src/agent/pi-subagents.ts: move the new exports below the imports; drop the "(ranxianglei#5055-adjacent)" reference on DEFAULT_PI_COMMANDS (the two-command default is this PR's design decision, not ranxianglei#5055's); split the one-shot stderr flag into warnedUnknownPiCommands / warnedWrongTypePiCommands so each warning class fires once independently instead of cross-suppressing - tests/pi-commands.test.ts: wrong-type case now asserts its own warning (one-shot, own flag) instead of skipping it as unassertable
…lt-thinking models Implements option A from ranxianglei#5030. - A1 nudgeDecisionTimeoutMs (default 15000): the decide side call used a hardcoded 15s idle timeout. Full-context side calls prefill 25-60s at 200-450k tokens even on a cache hit, so every arm on slow providers became an AbortError while the upstream still completed and billed. - A2 nudgeDecisionReasoning: "strip" (default, ranxianglei#3004 behavior) / "follow" (keep host thinking-effort knobs verbatim) / "off" (strip + protocol- native disable: reasoning_effort none, thinking disabled, thinkingBudget 0, reasoning effort none). - A3 DEFAULT_DECIDE_MAX_TOKENS 200 -> 1024: hybrid-thinking models (glm-5.3) spend hidden reasoning inside the same output budget and answer with empty content + finish_reason length at 200. - A4 failure detail now distinguishes budget exhaustion: 'empty response (finish_reason "length" ...)' so operators can tell it from provider glitches in logs. Tests: unit tri-state parity/keep/disable per protocol; e2e reasoning off/strip/follow on the wire; detector threshold aligned to 1024. Docs: compress.yaml entries for both new keys, default update.
…mment Review of ranxianglei#5055 (in passing, no behavior change): - src/nudge-decide.ts: DECIDE_TIMEOUT_MS back-compat alias had zero importers (server.ts imports DEFAULT_DECIDE_TIMEOUT_MS) and its doc comment claimed it was imported by server.ts — dead code + wrong comment, removed. - tests/issue3004-nudge-decide-selfheal.test.ts: detector comment still said "default 200" while its own threshold is 1024 since this PR (the sibling e2e file's comment was updated; this one was missed).
…not a sideBody transport key (ranxianglei#5055) Review of ranxianglei#5055: the resolved nudgeDecisionTimeoutMs was smuggled through the outbound body under a magic key that runNudgeDecision had to peel off before serialization. The args object already exists for exactly this purpose; passing it there removes the whole forgot-to-strip wire-leak class.
…irections (ranxianglei#5055) The A1 knob was the load-bearing half of the fix with zero coverage. Slow side-call upstream + budget below latency -> decision aborts, nothing injected this round; same latency with a larger budget -> decision lands and the directive is injected.
8cb96fc to
9e8fb6f
Compare
|
Rebased onto current master (head |
[bot] 🏷 @john08281 收到你的 rebase(
另外做了 round-7 sync:你 rebase 之后 master 又走了 8 个提交到 当前状态:head = 提醒:后续如再 force-push,请带上 docs 重生成(推送前跑一次 |
… round 8) Master advanced 298 commits (69327e3 -> 33104b1). Six conflicted files, all resolved by union of intent: - src/server.ts: import union — master's preflight additions (estimateRawBodyBreakdown/PreflightDeps/RawBodyBreakdown) + this PR's DEFAULT_DECIDE_TIMEOUT_MS rename (old DECIDE_TIMEOUT_MS name no longer exported by nudge-decide.ts). - src/server/prepare-anthropic.ts: import union — master's dedupeAdjacentOutputConfig + recordNudgeObservation kept, this PR's applyDecisionReasoningPolicy kept, makeFoldPointReasoningDroppable dropped (removed from server.ts by ranxianglei#6132; zero remaining refs). - CONFIGURATION.md / CONFIGURATION.zh-CN.md: take master's redirect stubs — the full reference moved to docs/configuration[.zh-CN].md in ranxianglei#5920; this PR's regeneration of the root files is superseded. - website/main.js + website/docs/index.html: take master, then regenerate from the merged compress.yaml seed (node tools/gen-config-docs.mjs generate; i18n ok: 255 keys). Resulting delta vs master = exactly the decide-lane additions (nudgeDecisionTimeoutMs, nudgeDecisionReasoning, nudgeDecisionMaxTokens 200 -> 1024) in both languages. Verified: tsc --noEmit clean; targeted suites green — nudge-decide + issue3004 selfheal + config-surface-guard 24/24, nudge-model-decided e2e 9/9. Full-suite verdict = remote CI.
The unused-exports gate (check-unused-exports.mjs) flags the type alias as INTERNAL_ONLY: it is referenced only inside src/nudge-decide.ts (DecideConfig.reasoning and the applyDecisionReasoningPolicy parameter), never imported by any other file. Drop the leading export per the gate's own fix hint. Type-only, zero runtime change; config.ts keeps its own inline literal union for the parsed config value.
… round 9) Master advanced 33104b1 -> c26c31a (ranxianglei#6260, ranxianglei#5441, ranxianglei#6186, ranxianglei#5774, ranxianglei#6166 et al.). Clean auto-merge, no conflicts: the new window's blockOps work (ranxianglei#5765/ranxianglei#6166) and this PR's decide-lane keys land in different regions of every shared file. Also completes the round-8 docs regeneration that was left unstaged: the committed head carried the decide-lane keys in the compress.yaml seed but not in the generated output. Re-ran node tools/gen-config-docs.mjs generate on the merged seed (341 entries; i18n ok: 256 keys, EN/ZH equal) — generated output now matches the seed in docs/configuration[.zh-CN].md, website/main.js and website/docs/index.html (blockOps + decide-lane keys, both languages). Verified: tsc --noEmit clean; unused-exports gate clean; targeted suites green (nudge-decide + issue3004 selfheal + config-surface-guard + nudge-model-decided e2e). Full-suite verdict = remote CI.
(ai: vllm-qwen/qwen3.8-27b) CI-lane run complete — conflicts resolved (two sync rounds), red check fixed, docs inconsistency repaired. PR head is now What happened this run, in order:
Live state (verified this run, not from memory): mirror cache on head 七问:
|
[ework] CI fix verified green ✓ |
…opic The ranxianglei#5055 refactor replaced the direct stripDecisionReasoningFields call with applyDecisionReasoningPolicy across all four prepare-* files. prepare-anthropic.ts kept the now-unused import specifier (the other three files dropped it). noUnusedLocals is off so tsc/CI never flagged it. Removal-only; no behavior change.
ranxianglei
left a comment
There was a problem hiding this comment.
(ai: vllm-qwen/qwen3.8-27b)
APPROVE — option A from #5030, implemented cleanly. Independent pass against true master c26c31ace4 (not a re-read of the prior floors).
Correctness (verified by me):
- A1 timeout —
compress.nudgeDecisionTimeoutMs(default 15000, = old hardcoded) threadshandle.ts→prepare-*→runNudgeDecision→fetchWithTimeout(..., timeoutMs ?? DEFAULT_DECIDE_TIMEOUT_MS)(src/server.ts:3252). The renameDECIDE_TIMEOUT_MS→DEFAULT_DECIDE_TIMEOUT_MSis clean: zero dangling refs insrc/. Fixes the AbortError on 25–60s prefills. - A2 reasoning —
applyDecisionReasoningPolicy(src/nudge-decide.ts:206):follow= verbatim (host knobs kept); else strip;offwrites the protocol-native disable for all four wires. I checked each protocol composes correctly with the strip (google keepsmaxOutputTokens). - A3 —
DEFAULT_DECIDE_MAX_TOKENS200→1024 (disclosed). A4 —openaiFinishReasonis OpenAI-wire only;parseDecision("")→{failed,"empty response"}, and the enrichment only replaces the detail when a finish_reason is present (other protocols keep the original string). Additive, log-only. - Config — both keys in
CompressSettings/COMPRESS_SETTING_FIELDS/parseCompressSettings(timeoutMs finite>0 floored; reasoning enum strip/follow/off);mergeCompresspicks both; yaml + docs en/zh + generated website in sync. - Tests have teeth — tri-state parity (strip == legacy #3004 deepEqual per protocol), follow/off wire-byte asserts; e2e both-directions A1 (100ms→abort / 5000ms→lands) + A2 (off→"none" / unset→undefined / follow→"low"); detector 200→1024 pinned.
Regression: default path byte-identical except 200→1024 (disclosed); feature-off path untouched; ladder/fallback (recordDecision/consumeFallback, 3-strike) untouched; cache prefix unchanged (reasoning fields are top-level body fields, outside the prefix-cache key; main request never mutated); both compression modes reasoned (shared tier-1 arm, ephemeral side call only). No kernel/, no src/update.ts, no main-request wire change.
已顺手修: src/server/prepare-anthropic.ts:12 残留未使用 import stripDecisionReasoningFields(重构后四个 prepare-* 都改用 applyDecisionReasoningPolicy,其余三个已删、anthropic 漏删;noUnusedLocals 关闭故 CI 不报)→ 已删,commit 9fa674a5f4(已推 PR 分支,CI 重跑中)。
Process (not a code blocker): config-surface change (2 new keys + 1 disclosed default 200→1024) → per §7.4 the merge stays human (no auto-merge); the product decisions are already flagged in the PR body.
8问(中文,供人工评审):
- 是什么问题:
nudgeModelDecided判定车道在「默认开 thinking 的模型」上不可用——side call 的 reasoning 字段处理一刀切(只 strip),且 15s 硬超时在慢 prefill(25–60s)下必然 AbortError。 - 什么原因导致的:side call 硬编码 15s 超时(无法覆盖慢 prefill)+ reasoning 字段只有单一 strip 语义(#3004),表达不了「保留宿主 thinking 旋钮」或「显式关 thinking 省 token」。
- 影响范围与严重程度:仅影响开了
nudgeModelDecided且模型默认开 thinking 的会话(特定条件);默认路径(feature 关)零影响。特定条件下的可用性/体验缺陷,非数据丢失。 - 怎么修复的:三档 reasoning 策略(strip/follow/off)+ 可配置超时 + max tokens 200→1024 给足输出预算 + finish_reason 诊断(OpenAI-wire)。
- 是否有回归问题:无。默认路径字节等价(除已披露 200→1024);feature-off 未触碰;ladder/fallback 未触碰;缓存前缀不变;两种压缩模式均已推理。
- 是否破坏缓存:否。reasoning 字段是 body 顶层字段,不在 prefix-cache key 内;主请求字节不变。
- 是否改了设计方向:否。仍在 tier-1「模型自决」臂内,只改 ephemeral side call,不动主请求 wire。
- 是否引入了新的配置:是。
compress.nudgeDecisionTimeoutMs(默认 15000)、compress.nudgeDecisionReasoning(默认 "strip"=legacy #3004)、compress.nudgeDecisionMaxTokens默认 200→1024(已披露)。均在compress.*下走三级 cascade。
判定:可合——代码、测试、披露齐备;因属 config-surface 变更,合并留人工(§7.4),不 auto-merge。
[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b) APPROVE — posted as a formal GitHub review on this PR (review id 5869982551). Independent pass against true master What I verified (my own pass, not a re-read of prior floors):
Regression: none. Default path byte-identical except the disclosed 200→1024; feature-off path untouched; ladder/fallback untouched; cache prefix unchanged (reasoning fields are top-level, outside the prefix-cache key; main request never mutated); both compression modes reasoned. 已顺手修: Process (not a code blocker): config-surface change (2 new keys + 1 disclosed default 200→1024) → per §7.4 the merge stays human (no auto-merge); the product decisions are already flagged in the PR body. 8问(中文,供人工评审):
判定:可合——代码、测试、披露齐备;因属 config-surface 变更,合并留人工(§7.4),不 auto-merge。 |
Implements option A from #5030 — reference implementation, happy to adjust to B/C or close at the owner's call.
What this changes
A1 —
compress.nudgeDecisionTimeoutMs(default15000, the old hardcoded value)The decide side call was wired to a hardcoded 15s idle timeout (
DECIDE_TIMEOUT_MS). Full-context side calls prefill 25–60s at 200–450k tokens even on a cache hit (measured on z.ai glm-5.3; one decide call completed upstream in 25.8s and was billed, while bili had already aborted at 15s), so on slow providers every arm degraded into an AbortError. The new setting is threaded throughDecideConfigon an internal transport key and consumed byrunNudgeDecision'sfetchWithTimeout.A2 —
compress.nudgeDecisionReasoning:"strip"|"follow"|"off"(default"strip"= current #3004 behavior)Global (not per-model), per the discussion in #5030:
"strip"— current behavior: delete host reasoning knobs before the side call (byte-identical, covered by a parity unit test)."follow"— keep the host's knobs verbatim, so the side call obeys the thinking effort configured on the host for that model. Also fixes the dsh上下文700K不触发压缩 #3004 side effect where stripping an intentionally low effort re-enabled a default-thinking model's heavy reasoning."off"— strip, then write the protocol-native disable: OpenAIreasoning_effort: "none", Anthropicthinking: {type: "disabled"}, GooglethinkingConfig: {includeThoughts: false, thinkingBudget: 0}, Responsesreasoning: {effort: "none"}.A3 —
DEFAULT_DECIDE_MAX_TOKENS200 → 1024Hybrid-thinking models (glm-5.3) spend hidden reasoning inside the same output budget and answer with empty
content+finish_reason: "length"at 200 (reasoning_tokens140–199, billed, zero visible text). 1024 leaves room for short reasoning plus the one-line JSON verdict. Configurable before, but the default was a trap.A4 — budget-exhaustion empties are now distinguishable in logs
empty response (finish_reason "length" — output budget exhausted before any visible text; raise nudgeDecisionMaxTokens or set nudgeDecisionReasoning "off")instead of a bareempty response.Validation
tests/nudge-decide.test.ts: tri-state parity (strip == legacy dsh上下文700K不触发压缩 #3004 output, per protocol, deepEqual), follow keeps knobs, off writes native disables; constants test updated.tests/nudge-model-decided.test.ts: e2e — side-call detector threshold aligned to 1024; new scenarios assert the wire seesreasoning_effortnone(off) / absent (strip) / the host'slow(follow).tests/issue3004-nudge-decide-selfheal.test.ts: detector threshold aligned; 7/7.config-surface-guard5/5 (both new keys documented inwebsite/config-reference/compress.yaml, bilingual).tsc --noEmitclean;npm run buildOK.nudgeDecisionTimeoutMs: 45000the decision survived (old default would have aborted at 15s) and logged a cleanno; side call on the wire carriedmax_tokens: 1024+reasoning_effort: "none"withnudgeDecisionReasoning: "off".Closes #5030 (option A).