Skip to content

test: add session memory summary replay consistency harness - #264

Open
Hugo-Chu-HY wants to merge 2 commits into
trpc-group:mainfrom
Hugo-Chu-HY:feat/session-replay-consistency
Open

test: add session memory summary replay consistency harness#264
Hugo-Chu-HY wants to merge 2 commits into
trpc-group:mainfrom
Hugo-Chu-HY:feat/session-replay-consistency

Conversation

@Hugo-Chu-HY

@Hugo-Chu-HY Hugo-Chu-HY commented Jul 30, 2026

Copy link
Copy Markdown

No description provided.

@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@Hugo-Chu-HY

Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

Rook1ex added a commit to trpc-group/cla-database that referenced this pull request Jul 30, 2026
@helloopenworld

Copy link
Copy Markdown
Contributor

AI Code Review

这确认了设计:summary_update/summary_truncation 应检测到 SQLite 漂移。根报告(pr.diff 中的 session_memory_summary_diff_report.json)显示这两个用例均为 differences:[] —— 这与框架实际行为及设计相矛盾。这是一个陈旧/误导性的已提交制品。

现在,该测试框架(harness)已失效(未被使用),因此其内部 Bug 无法在 CI 中体现。但根据审查指南,测试代码必须按实际执行风险进行评估。由于此测试框架从未被导入,其内部 Bug 属于维护性/死代码问题。影响最大且可验证的问题是已提交制品具有误导性,以及存在死代码的测试框架。

我来验证一下 pr.diff 中的测试框架是否存在 summary 在后端之间不可比的问题——实际上 backend.summaries 是一个仅在测试框架层面设置的字典(并未持久化到任何后端)。因此 summary_create/summary_update 用例根本不测试后端持久化 —— 摘要完全是在测试框架中伪造的,在两个后端中完全相同。这意味着这些用例无法检测到摘要持久化中的任何漂移(与设计文档实际意图相比,这是一个测试有效性漏洞)。这是一个真正的测试有效性问题:摘要“测试”没有涉及后端,因此毫无意义。

我来确认一下:在 replay_case 中,op == "summary" 只是设置了 backend.summaries[session_id] —— 没有调用 session_service。快照返回的是 backend.summaries.get(session_id)。因此,在所有后端中,摘要是相同的。summary_create/summary_update/summary_truncation 用例无法测试后端摘要的一致性。这是一个真实的测试有效性漏洞(即“测试没有覆盖实际风险路径”),即使代码是死代码,这也符合审查准则(测试有效性)。

现在我来完成审查。我还要再仔细检查一下 state_overwrite:用例设置 {language:zh, theme:light},然后是 {theme:dark}。测试框架对两者都使用了 EventActions(state_delta=...) 进行追加。在 _append_event_to_session 之后,session.state 将包含合并后的结果:language=zh, theme=dark。冒烟测试(重写后的版本)断言 counter==3,但这个测试框架的根用例使用的是 language/theme。由于这个测试框架是死代码,并没有这样的断言。没问题。

现在我来给出审查结论。我来精确获取报告和用例文件的行号。

pr.diff 的行号映射:

我再重新考虑一下报告 differences:[] 的问题是否真的是一个 Bug。它是硬编码的预期输出。但已知漂移的摘要用例应该存在差异。对于 summary_update/summary_truncation,报告显示为空 -> 误导。我将把报告 + 死掉的测试框架归为一组,合并为一个关于陈旧/孤立制品的警告。

我再考虑一下 summary 的测试有效性问题是否成立且可验证。在测试框架中,摘要从未触及任何后端。已验证。

现在我来写审查意见。我已经充分理解了。让我确认一下 harness 中的一个最终细节——即 summary 是否涉及任何后端,以验证测试有效性的发现。

replay_case 中的 summary 操作(第 74-79 行)仅设置 backend.summaries[session_id](一个 harness-local 字典),没有 session_service 调用。快照在 tests/sessions/replay_harness.py:101 处返回 backend.summaries.get(session_id)。因此,summary case 无法检测到任何后端差异。已确认。

发现的问题

🚨 Critical

(无)

⚠️ Warning

  • tests/sessions/replay_harness.py:74-79tests/sessions/replay_harness.py:101:summary 用例不经过任何后端,无法检测摘要持久化漂移

    • op == "summary" 仅写入 harness 本地的 backend.summaries 字典,未调用 session_service/summarizer_manager;快照又直接返回该字典。因此 summary_create/summary_update/summary_truncation 在 InMemory 与 SQLite 两端取到的 summary 必然逐字节相同,根本不覆盖后端摘要持久化这一声称的核心风险路径。建议改为走真实的 create_session_summary + get_session 读回,或明确删除这些 case。
    • elif op == "summary":
          backend.summaries[session_id] = {...}  # 仅本地字典,不落库
  • session_memory_summary_diff_report.json:44-46:提交的“全绿”报告与框架实际行为/设计相矛盾

    • 该静态报告对 summary_updatesummary_truncation 均记录 "differences":[],但设计文档(tests/sessions/replay/session-memory-replay-consistency-design.md:403)与真实测试(test_replay_consistency.pyKNOWN_DRIFT)明确要求这两个 case 应被检出 SQLite 漂移。提交一份误导性的“零差异”产物会让后续阅读者误判一致性状态;建议不要把运行时报告作为静态文件提交,或至少与实际输出对齐。
  • tests/sessions/replay_harness.pyreplay_cases/session_memory_summary.jsonsession_memory_summary_diff_report.json:新增文件未被任何测试路径引用,属孤立死代码

    • 全仓库检索无任何 .py/配置导入 replay_harness 或引用根目录下的 replay_cases/session_memory_summary.json(真实回放用的是 tests/sessions/replay/ 包及 replay/replay_cases/cases.jsonl)。pytest 也不会收集 replay_harness.py(非 test_*.py)。这些文件既不进入 CI 也不可复用,建议删除以免与 tests/sessions/replay/ 重写版本长期并存造成维护歧义。

💡 Suggestion

  • tests/sessions/replay_harness.py:83stored.events[:-operation["keep"] or None] 依赖 -0 == 0 的 falsy 特性把 keep=0 转成 [:None],可读性差且易被后续维护者误改。建议显式处理 keep == 0 分支(如 events[: len(events)-keep]),避免隐式 or None 技巧。

总结

整体无阻塞性安全/逻辑错误,但本 PR 引入的四个文件均为孤立产物:harness 未被任何测试引用,且 summary 用例因不经过任何后端而无法覆盖其声称的摘要一致性风险;提交的“零差异”报告还与设计期望的已知漂移相矛盾。建议删除这些冗余文件,或将其 summary 路径接入真实后端并对齐报告内容。

测试建议

  • 若保留 harness:补一个 InMemory vs SQLite 的 summary_update 端到端断言,确认能复现 KNOWN_DRIFT 中描述的 SQLite 摘要漂移,否则该 case 形同虚设。
  • 暂无其他额外测试建议。

@codecov

codecov Bot commented Jul 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@86b5f7d). Learn more about missing BASE report.

Additional details and impacted files
@@            Coverage Diff             @@
##             main        #264   +/-   ##
==========================================
  Coverage        ?   87.85545%           
==========================================
  Files           ?         482           
  Lines           ?       45189           
  Branches        ?           0           
==========================================
  Hits            ?       39701           
  Misses          ?        5488           
  Partials        ?           0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

2 participants