docs(chat): explain event sequence ordering invariant - #4265
huangruiteng merged 1 commit into
Conversation
Signed-off-by: Lihua <1017343802@qq.com>
huangruiteng
left a comment
There was a problem hiding this comment.
动机
这个 PR 针对的是一个很小但值得长期保留的维护性风险:events_after 使用 bisect_right,前提是 JSONL 事件行按 numeric sequence 严格递增;而 compaction 可以删除 replay-only 行并留下 gaps。过去这个不变量只存在于实现细节里,维护者修改 writer 或 compaction 时不容易意识到 binary search 的前提。当前 head 只在两个实际 owner 附近补充说明,没有改变事件分配、持久化、读取或用户体验。
我按 exact head 1cc2218cf04833476481c9fcdb38d026d5319b5f 读取了完整 diff 和未改动的相邻调用者。tests/test_chat_event_cursor.py 与 tests/test_attached_session_broker.py 共 17 个测试通过,git diff --check 和 Python 编译检查也通过;远端 25 个检查全部成功。PR 处于 MERGEABLE,只是相对当前 main 显示 BEHIND,这不构成本五行 comment-only 变更的语义阻断。
改动思路
append_event 将 pending rows 交给 flush_events;后者在同一个 per-key flush lock 和文件锁内读取最后一个 persisted sequence、逐行递增分配 ID 并 append。events_after flush 后读取缓存/文件,以 bisect_right(..., key=sequence) 找到 cursor 后缀。compact_completed_events 只过滤 delta 类型并原序写回,因此允许 gaps 但不能重排 retained rows。PR 在 flush_events 解释“分配与 append 共享文件锁、顺序严格递增”,在 events_after 解释“compaction 的 gaps 合法但顺序必须保留”。
这里没有新增 state owner、协议字段、runtime assertion 或 abstraction;说明文字紧贴既有 writer/reader 边界,且没有把“注释”误称为 machine-enforced guarantee。真正的执行保证仍来自现有锁、append 和 compaction 实现,测试继续是可执行回归边界。
具体改动
关键代码讲解
loopx/chat_store.py:1309的ChatSessionStore.flush_events新增两行注释,明确 sequence 从最后一个持久化行之后分配,并与 append 处在同一文件锁内;这解释了为什么并发 writer 不会在正常路径上产生倒序。loopx/chat_store.py:1353的ChatSessionStore.events_after新增说明,指出bisect_right依赖 sequence 排序,compaction 删除行造成的 gaps 不改变排序前提。- 未修改的
compact_completed_events仍保留行的原顺序后执行_replace_jsonl;未修改的_event_rows_locked、cache revision 和 lock 路径共同构成 reader 的真实输入。把这两个 unchanged owner 一起检查,是为了确认注释没有描述一个不存在的行为。
对主干的风险
本次没有发现可操作的阻断问题:
- 这不是只看 diff 的“文档通过”。我沿着
append_event -> flush_events -> events_after以及compact_completed_events -> _replace_jsonl的真实文件持久化路径核对了锁和排序关系;17 个 focused tests 覆盖 cursor seeking、跨实例 event reconciliation 和 attached-session consumer,exact head 上通过。 - 注释准确地区分“严格递增”与“允许 gaps”:它没有错误地声称 sequence 连续,也没有暗示 compaction 会重新编号。invalid cursor 的既有归一化和 file/JSONL 错误路径没有改变。
- 这是 product_runtime 区域,所以我检查了 typed-state、authority、default-off 与 guidance/obligation 维度:没有新增状态分类、权限、feature gate、调度副作用或硬 obligation;文案仍是模块局部的 explanatory guidance,未污染 generic control-plane contract。
- 远端 25 个检查(包括 DCO、Python checks、Node compatibility、test shards、stage2c、pytest 与 merge-gate)均成功。
BEHIND只表示可在合并前同步最新 main;当前 diff 不修改代码,因此没有因它制造新的 review blocker。
剩余风险是注释不会在运行时强制未来维护者保持排序;这正是现有测试与代码 owner 应继续承担的职责,不需要在一个五行文档 PR 中增加 assertion 或新框架。
我的整体评价
这是一个范围准确、位置正确、成本很低的维护性改进。它复用 ChatSessionStore 既有 owner,补上了 binary-search 前提和 compaction gaps 语义,未引入重复知识或静默行为变化;17 个 focused tests、编译/diff 检查与 exact-head 远端 required checks 都支持其安全性。建议 APPROVE。合并前可按仓库常规把 branch 更新到最新 main,但不需要为此扩大 PR 或添加运行时结构。
English verdict: APPROVE — this exact head adds only five precise comments at the existing event writer and cursor-reader owners. The comments correctly document locked monotonic sequence allocation and compaction gaps without changing executable behavior; focused tests and all 25 reported checks pass. The branch is merely behind main, so it can be synchronized during normal merge preparation.
Summary
Document the strictly increasing sequence order that
events_afterrelies on for binary search. The writer comment explains locked sequence allocation and ordered append; the reader comment records that compaction may leave gaps while preserving order. Executable behavior is unchanged.Closes #4192.
Validation
1cc2218cf04833476481c9fcdb38d026d5319b5fpython3 -m pytest -q tests/test_chat_event_cursor.py tests/test_attached_session_broker.py: 17 passed, including cursor seeking and cross-instance event reconciliation.git diff --checkandloopx check --scan-path loopx/chat_store.pypassed with no warnings.Coverage and gaps: this is a five-line code-comment change. No CLI, Dashboard, Lark, provider, or runtime behavior changes; existing tests and AST parity are sufficient for this scope. Full canary and live-service tests were not run. Hosted CI remains separate validation.
Future-facing pass: the invariant is documented at its existing writer and reader owners; no helper, runtime assertion, or additional abstraction is needed.
Scope
main.Boundary Checklist