perf(chat): skip redundant transcript scans - #4092
Conversation
Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion
精确审阅 head:d5279c81ff295e3a7a5be7b1bb9842ff8a43ff21。结论为 APPROVE,未发现阻塞或非阻塞问题。
1. 变更与架构判断
本 PR 将 ChatSessionStore.append_message() 的 transcript 去重扫描限定到调用方显式提供 message_id 的路径。普通消息由 store 生成全新 UUID,可在既有 exclusive file lock 下直接 append;managed/attached completion 等显式稳定 ID 仍走原有扫描和幂等 replay。变更仅涉及 4 行生产代码和一个 46 行回归测试,没有引入索引、缓存、schema 或第二个状态 owner,范围与热路径问题成比例。
2. 语义与调用链复核
我检查了所有 append_message callers、_read_jsonl/_append_jsonl、managed/attached completion writers、Chat runtime consumers 和 PR #3981 引入的 caller-supplied idempotency contract。分支依据是原始可选参数是否存在,而不是生成后的 payload ID,因此显式 ID 始终保留锁内扫描;无 ID 调用才生成 UUID 并跳过无意义的 O(n) 预读。invalid ID 校验、JSONL row shape、fsync、排序和 readback 均未改变。
3. 验证结果
tests/test_chat_session_active_turn.py:31 passed in 3.59s。- 全部
tests/test_chat*.py:88 passed in 23.37s。 - Ruff、
git diff --check和 17 项 hosted checks 通过。 - 使用真实 filesystem
ChatSessionStore做 base/head poisoned-read counterfactual:base 会触发 transcript scan,当前 head 在不扫描时成功追加 32 字符 generated ID。 - 同一回归覆盖显式 ID replay,确认重复 completion 返回原 durable row 而不追加第二条。
4. 失败与兼容性判断
最强反例是把优化错误扩大到显式 completion ID,导致重试重复写 terminal message。当前实现由 message_id is not None 精确保护该路径,并在既有 exclusive lock 内保持 dedup;回归同时对 generated path 的“不得读”和 explicit path 的“必须幂等”敏感。无持久化迁移、权限、quota、scheduler 或外部副作用变化,回滚也只是恢复原三行扫描逻辑。
5. Future-facing pass
相邻 owner 内没有更有价值的小型重构:把这一条件抽成 helper 会稀释 caller-intent 语义,新增 transcript index/cache 则明显超出当前问题。保留局部条件和双向语义回归是更可读、可逆的边界。
English verdict: APPROVE exact head d5279c81ff295e3a7a5be7b1bb9842ff8a43ff21. The change safely removes the transcript pre-read only for store-generated message IDs while preserving caller-supplied idempotent replay under the existing lock. Base/head counterfactuals, 88 Chat tests, Ruff, diff hygiene, and 17 hosted checks support the intended behavior; no unresolved finding remains.
Summary
Performance evidence
Seven-run median for one generated-ID append, with fsync stubbed to isolate transcript scan cost:
The common generated-ID path no longer scales with transcript length. No cache, index, schema, or migration was added.
Validation
Scope
A broader transcript index was considered and deferred: deterministic completion messages still need durable replay deduplication, and this change removes the unnecessary scan without adding state or recovery complexity.