Skip to content

fix(chat): reuse completed replay snapshots within bounded retention - #4648

Merged
huangruiteng merged 2 commits into
mainfrom
codex/chat-replay-budget-20260917
Sep 17, 2026
Merged

huangruiteng merged 2 commits into
mainfrom
codex/chat-replay-budget-20260917

Conversation

@huangruiteng

Copy link
Copy Markdown
Collaborator

Completed Chat turns were evicted after every replay, so 20 reconnects reread the same JSONL log 20 times. Keep recently replayed terminal snapshots within aggregate limits of 8 turns, 4,096 rows and 2 MiB of encoded log data. Oversized logs remain replayable without being retained.

Replaces the runtime cache portion of #4620; its credential/runtime smoke fixes move to #4647. A small ChatEventCache owns snapshot revisions and retention, while ChatSessionStore keeps persistence, sequence allocation and file locks. There are no mutable-map compatibility aliases. External appends and compaction invalidate revisions; an older replay cannot retain a newer snapshot by mistake. This intentionally changes terminal cache retention; active history and peak decode memory are outside these retention limits.

Validation: 14 event retention/buffer/cursor/input tests pass, including count/byte/aggregate limits, LRU, external append and concurrent snapshot replacement. The real runtime throughput smoke passes with one read across 20 replays; the unchanged baseline fails with 20 reads. Ruff, maintainability and semantic-vocabulary checks, exact-scope quality qualification and standard premerge pass. The initial direct edit exceeded the large store's maintenance limit; extraction fixed that without increasing the ceiling.

The change serves existing Chat/SSE consumers and adds no frontend configuration. Refactor pass: reduce the oversized store and keep native I/O concerns in Python; no control-plane state rule moves or duplicates.

Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Approval conclusion (author-owned PR; GitHub blocks formal self-approval)

Reviewed exact head: 0d32e0c77b34a66ae858a55720242bd6d40fca6f (refactor(chat): encapsulate revision-bound replay snapshots).

动机

Chat 的已完成 turn 每次 replay 后就把快照丢掉,于是 20 次重连会把同一份 JSONL 读 20 次。这不是理论问题:仓库自己的 examples/loopx-chat-stream-throughput-smoke.py 就断言 read_calls <= 1,而它在当前 main 上是红的。我在 base 9118568dd 上复现了失败原文:AssertionError: SSE replay reread the event log 20 times。

改动思路

把「快照修订与保留策略」抽成一个小的 ChatEventCache,把持久化、序号分配与文件锁继续留在 ChatSessionStore;保留策略是有界的聚合上限(8 个 terminal turn / 4096 行 / 2 MiB 编码日志),超限按最近最少 replay 的顺序淘汰,超大的单份日志仍可 replay 但不保留。不做可变 map 的兼容别名,外部 append 与 compaction 会让修订失效。

具体改动

4 个文件、+137/-48:

  • loopx/chat_event_cache.py(新增 52 行):get/put/drop/retain_terminal;retain_terminal 用对象同一性检查(entry[1] is not rows)防止「旧 replay 保留了已被替换的快照」,并在同一把 RLock 下做淘汰。
  • loopx/chat_store.py:删掉 _event_cache/_event_cache_revision 两个并行 dict 与 _drop_event_cache,改为调用 cache;events_after 的 terminal 分支由 drop 改为 retain_terminal;compaction 与 append 路径改用 drop/put。
  • tests/test_chat_event_retention.py(+93/-24)、tests/test_chat_event_cursor.py:覆盖计数/字节/聚合三项上限、LRU 淘汰、超大日志可读不保留、外部 append 失效、替换快照竞态。

我复核的关键点(都在这个 head 上自己跑过):

  • 核心主张的对照实验:examples/loopx-chat-stream-throughput-smoke.py 在 head 上 43 fsync, 0.043s 后 ok;在 base 9118568dd 上同一个 smoke 直接 AssertionError: SSE replay reread the event log 20 times。也就是这次改动修的是当前 main 上已经红的公共 smoke。
  • pytest -q tests/test_chat_event_retention.py tests/test_chat_event_cursor.py → 12 passed;tests/test_chat_event_buffer.py → 1 passed;examples/loopx-chat-store-smoke.py → ok。
  • 修订语义未退步:_event_revision 仍是 (st_ino, st_size, st_mtime_ns),读路径先比修订、未命中才在排他文件锁下重读,append 后重算修订、terminal append 直接 drop。
  • 有界性:字节上限用的是文件编码大小(revision[1] = st_size),与正文「2 MiB encoded log data」一致;单个超大快照会在淘汰循环里把自己淘汰掉,即「可读但不保留」。

遗留问题(非阻塞,P3)

PR body 说「最初的直接改法超出 large store 的维护上限,抽取后没有提高上限」。我在 base 与 head 上都查了 loopx/canary/module_metric_baseline.json:没有 loopx/chat_store.py 的 module_metric_ceilings 条目,树里也没找到针对它的 per-file large-module 上限。抽取本身站得住(该文件 1487 行,且同时持有 I/O、锁与序号分配;本次 diff 在 store 侧还是净删除),所以不阻塞;但这句话对评审者不可复现。最小修法:点名触发该上限的检查或 baseline 条目,或删掉这句、只保留体量与归属理由。

对主干的风险

最强回归是缓存返回陈旧快照,导致重连漏掉外部新写入的事件。防护有三层:读时比 (inode, size, mtime_ns)、append 后重算修订、compaction 显式 drop,再加 retain_terminal 的对象同一性检查(旧 replay 不能把被替换过的快照留在缓存里)。这些都写进了测试名。改动只影响 Chat replay:持久化日志、序号分配、文件锁与公开会话投影未动;内存占用有 8 个 turn / 2 MiB 的显式上限,超过就回落到「再读一次」的旧路径。未验证维度:进程粒度的保留意味着合法地反复访问 8 个以上 terminal turn 时仍会重读(正文已说明这是取舍),以及上面那条不可复现的上限说法。回退成本一个 commit。

我的整体评价

结论 APPROVE。这是我这几轮里最干净的一个改动:动机由当前 main 上真实的红 smoke 支撑,我把 base/head 的读次数对照亲手跑了一遍;抽取后 store 变小、保留策略有明确上限、失效路径由修订与对象同一性双重保证。唯一 P3 是正文引用的维护上限在树里查不到。

English verdict: APPROVE - exact head 0d32e0c; completed Chat turns now retain revision-bound replay snapshots under 8 turns / 4096 rows / 2 MiB with least-recently-replayed eviction, while persistence, sequence allocation and file locks stay in the store. The motivating failure is real and I reproduced both sides: examples/loopx-chat-stream-throughput-smoke.py fails on base 9118568 with "SSE replay reread the event log 20 times" and passes at this head (43 fsync, 0.043s), with 12 retention/cursor tests plus the buffer test and the store smoke passing. Staleness is guarded by the (inode, size, mtime_ns) revision, append-time recomputation, compaction drops and an identity check in retain_terminal. One non-blocking P3: the PR body's "large store maintenance limit" is not visible in loopx/canary/module_metric_baseline.json at either revision.

@huangruiteng
huangruiteng merged commit b6ee8c7 into main Sep 17, 2026
26 checks passed
@huangruiteng
huangruiteng deleted the codex/chat-replay-budget-20260917 branch September 17, 2026 15:28
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.

1 participant