Skip to content

fix(gateway): ContextFill 仅从控制通道获取,消除多步 turn 累计膨胀#477

Merged
hrygo merged 3 commits into
hrygo:mainfrom
aaronwong1989:fix/context-fill-accuracy
May 22, 2026
Merged

fix(gateway): ContextFill 仅从控制通道获取,消除多步 turn 累计膨胀#477
hrygo merged 3 commits into
hrygo:mainfrom
aaronwong1989:fix/context-fill-accuracy

Conversation

@aaronwong1989
Copy link
Copy Markdown
Contributor

@aaronwong1989 aaronwong1989 commented May 22, 2026

Closes #478

Summary

  • mergePerTurnStats 不再从 Done 事件 usage 设置 ContextFill(该值是 turn 内所有 API 调用的累计 token,多步 agentic turn 下膨胀 2-3x)
  • ContextFill 现在仅由 get_context_usage 控制通道设置,resetPerTurn 清零防止跨 turn 残留
  • OCS queryContextUsagetotalTokens 改为最后一条 assistant 消息的 input + cache_read + cache_write,不再包含 output/reasoning
  • 控制通道失败时 context 不显示(ContextFill=0),而非显示膨胀值
  • 修正 3 处过期注释

Test plan

  • make check 全通过(lint + test + build)
  • TestSessionAccumulator_MergePerTurnStats 4 个子测试更新,验证 ContextFill 不从 Done 事件设置
  • 新增 mergeContextUsageresetPerTurn 生命周期测试
  • OCS TestServerCommanderQueryContextUsage / TestServerCommanderQueryContextUsageMultipleMessages 更新

🤖 Generated with Claude Code

mergePerTurnStats 不再从 Done 事件 usage 设置 ContextFill(该值为
turn 内所有 API 调用的累计 token,多步 agentic turn 下膨胀 2-3x)。
ContextFill 现在仅由 get_context_usage 控制通道设置,resetPerTurn
清零防止跨 turn 残留。OCS queryContextUsage 改为取最后一条 assistant
消息的 input+cache_read+cache_write,不再包含 output/reasoning。

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
hotplex-ai
hotplex-ai previously approved these changes May 22, 2026
Copy link
Copy Markdown
Owner

@hotplex-ai hotplex-ai left a comment

Choose a reason for hiding this comment

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

PR #477 Review — fix(gateway): ContextFill 仅从控制通道获取

三维度审查结论

维度 结论
代码质量 ✅ PASS
非功能性 ✅ PASS
集成与防腐 ✅ PASS

代码质量

  • DRY: PASS — ContextFill 赋值从 mergePerTurnStats 两个分支中移除,收敛到 mergeContextUsage 单一来源
  • SOLID: PASS — 职责分离正确,mergePerTurnStats 仅处理累计 token/cost,mergeContextUsage 独占 ContextFill/ContextWindow 赋值
  • 命名规范: PASS — contextFilllastInput/lastCacheRead/lastCacheWrite 遵循 Go 惯例
  • 错误处理: PASS — fmt.Errorf("opencode context query: %w", err) 正确包装;控制通道失败静默忽略符合预期
  • 代码风格: PASS — tab 缩进、testify/require、测试命名模式均一致

非功能性

  • 性能: PASS — 无新增分配,移除两处冗余写入为微优化
  • 安全: PASS — 无外部输入处理,不引入注入或凭证风险
  • 并发安全: PASS — sessionAccumulator 单 goroutine 顺序访问,生命周期(设置→消费→清零)时序正确

集成与防腐

  • 测试覆盖: PASS — 4 个子测试全部更新,新增 mergeContextUsageresetPerTurn 生命周期测试
  • API 兼容性: PASS — context_fill JSON key 不变,下游 ContextFill=0 时优雅隐藏
  • 依赖变更: PASS — 无新增依赖
  • 变更影响范围: PASS — 精准限定在 session stats 的 context_fill/context_pct 准确性

⚠️ WARN(非阻塞,供参考)

  1. bridge_forward.go:665 — 注释 the caller falls back to aggregated Done stats 与新行为矛盾。mergePerTurnStats 已不再设置 ContextFill,控制通道失败时 ContextFill 保持 0 而非回退。建议改为:// Errors are silently ignored; ContextFill stays 0 when control channel is unavailable.
  2. docs/archive/specs/Session-Stats-Spec.md:446ContextFill 注释仍为旧语义 本轮 input tokens,建议更新为 context window fill from control channel (0 if unavailable)

Reviewed by hotplex-ai — 3-agent parallel review (code quality × non-functional × integration)

- 新增 source 列('' 或 'cron')及复合索引
- GC 重新启用 DeleteTerminated DB 清理(此前已移除)
- cron session 保留 24h,普通 session 保留 7d,不级联删除 events
- CreateWithBot 从 platformKey 自动推断 source
- config 新增 TermRetention/CronTermRetention 配置项

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
hotplex-ai
hotplex-ai previously approved these changes May 22, 2026
Copy link
Copy Markdown
Owner

@hotplex-ai hotplex-ai left a comment

Choose a reason for hiding this comment

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

PR #477 审查报告 — hotplex-ai

三维度并行审查完成,结论:APPROVE(附 WARN 项供参考)

代码质量 ✅ PASS

核心逻辑变更精准:mergePerTurnStats 移除两行 ContextFill = input 赋值,resetPerTurn 添加清零,控制通道成为唯一数据源。OCS queryContextUsage 改为 last assistant message 的 input+cache 汇总,语义正确。

WARN 项:

  1. opencodeserver/commands.go totalTokens 字段名偏移 — 字段名暗示"总和",实际值现在是"最后一条 assistant 消息的 context fill"。建议在 ContextUsageData.TotalTokens 上添加注释说明 OCS worker 下的新语义。
  2. 011_session_source.sql Down migration 不完整 — 仅 DROP INDEX,未 DROP COLUMN source(SQLite 限制)。建议添加注释说明,或实现 table-rebuild 策略。

非功能性 ✅ PASS

  • 性能:无回退。delete_terminated SQL 使用新建的复合索引 idx_sessions_source_state_updated,查询高效。
  • 安全source 字段有 CHECK(source IN ('', 'cron')) 约束;来源通过 platformKey["cron_job_id"] 服务端检测,非用户输入;参数化查询无注入风险。
  • 并发:锁顺序 m.mu → ms.mu 正确;sessionAccumulatorforwardEvents 单 goroutine 内访问无竞争;无新 goroutine 泄漏。

集成防腐 ⚠️ WARN

  • 测试覆盖:良好。5 个测试文件更新,DeleteTerminated 接口签名变更传播至所有 mock;store_test.go 4 场景差分清理验证完备。
  • API 兼容Store 接口为 internal,签名变更已全量传播;SessionInfo.Source 增量字段 omitempty 向后兼容。
  • 依赖:无新增。

文档过时项(建议补充更新):
3. docs/explanation/session-lifecycle.md:187 — 原文"GC 不执行物理删除",现已恢复 DeleteTerminated,需更新为 source 差分清理描述。
4. docs/reference/configuration.md:178-187 — 缺少 term_retention(默认 7d)和 cron_term_retention(默认 24h)两个字段的文档。
5. docs/reference/glossary.md:100 — 保留期清理描述需更新为 source 相关。
6. docs/specs/Turn-Summary-Spec.md:220ContextFill 注释"本轮 input tokens"需更新为控制通道语义。
7. docs/architecture/Worker-Gateway-Design.md:244 — 需标注 source 差分。

总结

核心逻辑变更正确且充分测试。ContextFill 修复干净地消除了膨胀问题,差分保留是合理的 cron 优化。主要关注点是文档同步(6 处)和 Down migration 完整性,建议合并前处理。

Copy link
Copy Markdown
Owner

@hotplex-ai hotplex-ai left a comment

Choose a reason for hiding this comment

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

PR #477 审查报告

三维度审查结论:APPROVE

变更摘要

两类修复打包在一个分支中:

  1. ContextFill 精度修复(主变更):mergePerTurnStats 不再从 Done 事件累计 token 设置 ContextFill,改为仅由控制通道的 mergeContextUsage 设置,resetPerTurn 中清零防止跨 turn 残留。消除了多步 agentic turn 中 2-3x 的膨胀问题。OCS queryContextUsage 改为报告最后一条消息的 token 而非累计值。

  2. TERMINATED session 差异化清理(次变更):Cron session 24h 清理 vs 普通 session 7d 清理。新增 source 列(SQL 迁移 011)、配置字段、Store 接口变更、GC 逻辑更新。

代码质量 ✅ PASS

  • 错误处理:全部使用 fmt.Errorf("%w", err) 包装,GC 错误 log 不传播(符合现有 GC 容错模式)
  • Mutex 使用:无嵌入、无指针传递,锁顺序 m.mu → ms.mu 保持不变
  • 命名:SourceCron 常量、TermRetention/CronTermRetention 清晰规范
  • DRY/SOLID:职责分离清晰,source != 'cron' SQL 条件优雅覆盖空字符串和未来 source 值
  • SQL 迁移:NOT NULL DEFAULT '' + CHECK 约束,复合索引支撑删除查询

非功能性 ✅ PASS

  • 性能:delete_terminated 查询有复合索引 idx_sessions_source_state_updated 覆盖,GC 60s 间隔低频操作无性能问题
  • 安全:无路径穿越、注入、硬编码凭证等 OWASP 风险
  • 并发安全:sessionAccumulator 仅在单个 forwardEvents goroutine 中使用,DeleteTerminated 在释放 m.mu 后调用 store,不违反锁顺序

集成与防腐 ✅ PASS

  • 文档一致性:5 个文档文件同步更新(架构设计、session 生命周期、配置参考、术语表、turn summary spec)
  • 测试覆盖:session_stats_test.go 4 个子测试更新 + 生命周期测试;store_test.go 4 场景覆盖;所有 mock 签名一致更新
  • API 兼容:DeleteTerminated 签名变更为内部接口,所有调用方已更新;SessionInfo.Source 字段 omitempty 向后兼容
  • 无新增依赖

审查总结

PR # 标题 代码质量 非功能 集成 裁决
#477 fix(gateway): ContextFill 仅从控制通道获取 ✅ PASS ✅ PASS ✅ PASS APPROVE

@hrygo hrygo merged commit a5b834c into hrygo:main May 22, 2026
6 checks passed
@aaronwong1989 aaronwong1989 deleted the fix/context-fill-accuracy branch May 22, 2026 10:59
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.

ContextFill 从 Done 事件累计 token 导致多步 turn 膨胀 2-3x

3 participants