session/replaytest: add multi-backend replay consistency harness#2293
session/replaytest: add multi-backend replay consistency harness#2293lai-long wants to merge 15 commits into
Conversation
Add a reusable replay-consistency test harness for Session/Memory backends (refs trpc-group#2001). A deterministic case suite is replayed against multiple backends, five-dimension snapshots (events, state, memory, summary, tracks) are normalized and diffed field by field, and a located JSON diff report is produced. - 11 public replay cases covering single/multi-turn dialog, tool calls, state overwrite/delete/clear, memory CRUD and scope isolation, summary generate/update/filter-key/truncation, tracks, concurrent interleaved appends, and failure/retry recovery. - Backends: InMemory reference + real SQLite (session and memory); Redis binding gated by TRPC_REPLAYTEST_REDIS_URL with a miniredis smoke test running by default. - Normalization: symbolic IDs with reference rewriting, canonical JSON, duration-key scrubbing, big.Rat float tolerance. - Executable acceptance evidence: 9-class mutation matrix across all cases, four summary fault classes, service-boundary fault injection (silent drop, dirty half-write, cross-session summary leak), explicit false-positive-rate (<=5%) and lightweight-mode (<30s) assertions. - Ships a regenerable example session_memory_summary_track_diff_report.json.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughEnglishOverviewAdds a new standalone Operationally, it provides:
Public API and compatibilityNew exported API surface (leaf module) is intended for backend owners to implement a Case model (package
Backend abstraction and execution (package
Canonical snapshots / normalization / diffs (package
Extension/testing utilities (package
Backend target constructors (subpackages)
Questions / export-naming/semantics to review
Behavioral and operational risks
Recommended validation
中文变更概述新增独立的 公共 API 与兼容性该 PR 以“叶子模块”形式新增面向后端接入方的导出 API，用于实现 回放模型（package
后端抽象与执行（package
快照/规范化/差异（package
扩展/测试工具（package
后端 Target 构造（子包）
API 评审要点（建议重点确认）
行为与运行风险
建议验证
WalkthroughIntroduces a standalone Go replay-consistency module with declarative cases, deterministic execution, snapshot normalization, canonical diffing, backend targets, JSON reporting, fault injection, and SQLite/Redis acceptance coverage. ChangesSession Replaytest Harness
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
| func (t *Target) Reset(ctx context.Context) error { | ||
| t.closeServices() | ||
| t.seq++ | ||
| prefix := fmt.Sprintf("replaytest:%s:%d", t.name, t.seq) |
There was a problem hiding this comment.
Redis prefixes repeat across test processes because they only use name and seq, so old memory keys are reused on the next run. Add a per-target random run ID to the prefix.
中文
Redis 前缀只使用 `name` 和 `seq`，跨测试进程会重复，所以下次运行会复用旧 memory key。给前缀增加每个 target 独立的随机 run ID。
| continue | ||
| } | ||
| ms := &MemorySnap{ | ||
| UserID: userID, |
There was a problem hiding this comment.
Wrong memory scope is masked because toMemorySnaps replaces returned Entry.UserID with the queried user. Use e.UserID so stored scope mismatches fail.
中文
`toMemorySnaps` 用查询用户替换返回的 `Entry.UserID`，会掩盖错误的 memory 归属。改用 `e.UserID`，让存储归属不一致时报错。
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (8)
session/replaytest/case.go (2)
82-174: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDefine the public step-field contracts.
Step.Opdoes not define required versus ignored fields or nil behavior.SummarySpec.FilterKey,ToolCallSpec.ID,TrackSpec.Payload, andWriterSpecalso leave defaults and constraints undefined. Document and validate these combinations so third-party backend authors can construct cases reliably.中文
请明确公开步骤字段的契约。
Step.Op未说明哪些字段必填、哪些会被忽略以及 nil 的行为。SummarySpec.FilterKey、ToolCallSpec.ID、TrackSpec.Payload和WriterSpec也没有定义默认值与约束。请补充文档并在 runner 中校验这些组合，避免第三方后端接入方错误构造 case。As per path instructions, public API defaults, nil inputs, constraints, and errors must be defined.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@session/replaytest/case.go` around lines 82 - 174, Define and document the public contracts for Step and its operation-specific fields, including required versus ignored fields, nil behavior, defaults, and invalid combinations. Add runner validation for each Op, including SummarySpec.FilterKey, ToolCallSpec.ID, TrackSpec.Payload, and WriterSpec constraints, with clear errors and consistent handling of omitted values. Ensure raw JSON fields are validated and concurrent writer settings have explicit non-negative/count and required-field rules.Source: Path instructions
10-175: 📐 Maintainability & Code Quality | 🔵 TrivialRun
golangci-lintwith the repo-level config
session/replaytestshould use../../.golangci.yml; then rungo test ./....中文
使用仓库级配置运行
golangci-lint
session/replaytest应使用../../.golangci.yml；然后再运行go test ./...。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@session/replaytest/case.go` around lines 10 - 175, Run golangci-lint for the session/replaytest package using the repository-level ../../.golangci.yml configuration, address any reported issues in the affected symbols, then run go test ./... to verify the changes.Source: Coding guidelines
session/replaytest/diff.go (1)
490-495: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the local
minhelper.
This module targets Go 1.21, whereminis already a predeclared builtin, so the package-level helper is redundant and may triggerpredeclaredlint.中文
删除本地的
min辅助函数。
该模块目标是 Go 1.21，min已是预声明内建函数，因此这个包级辅助函数是多余的，也可能触发predeclared规则。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@session/replaytest/diff.go` around lines 490 - 495, Remove the package-level min helper function and rely on Go 1.21’s predeclared min builtin at its existing call sites; do not alter the surrounding diff logic.session/replaytest/report.go (1)
46-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConfusable public field names:
NotesvsNote. These two exported fields differ only by a trailingsyet carry unrelated meanings —Notes []Diffcollects allowed-diff entries, whileNote stringexplains an unsupported status. In a brand-new public schema this is the cheapest moment to disambiguate (e.g. renameNotetoReason/UnsupportedNote) before consumers depend on the JSON key.♻️ Proposed rename
Unsupported []string `json:"unsupported,omitempty"` - // Note explains an unsupported status: a capability gap is an - // allowed_diff by definition and never counts as a failure. - Note string `json:"note,omitempty"` + // Reason explains an unsupported status: a capability gap is an + // allowed_diff by definition and never counts as a failure. + Reason string `json:"reason,omitempty"` }中文
公开字段名易混淆：
Notes与Note。 两个导出字段仅差一个s，含义却完全不同：Notes []Diff收集 allowed-diff 条目，Note string解释 unsupported 状态。作为全新的公开 schema，现在是消费者尚未依赖该 JSON key 时最低成本的消歧时机（例如把Note改名为Reason/UnsupportedNote）。依据 path instructions：导出命名影响可发现性时属于框架设计问题。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@session/replaytest/report.go` around lines 46 - 52, Rename the exported Note field in the report schema to a distinct name such as UnsupportedNote or Reason, while preserving its string type, JSON behavior, and unsupported-status explanation. Update all references to Note consistently; leave the Notes []Diff field unchanged.Source: Path instructions
session/replaytest/sqlite/replay_test.go (1)
54-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider failing (not just logging) on unexpected
Unsupportedcases.
sqlite.Target.Caps()returnsCapAll, so a nonzerorep.Totals.Unsupportedhere would indicate a genuine capability-wiring regression, not an expected condition. Currently it's only logged, so this acceptance test wouldn't fail on that regression.♻️ Proposed strengthening
- if rep.Totals.Unsupported > 0 { - t.Logf("unsupported cases: %d (see report)", rep.Totals.Unsupported) - } + require.Zero(t, rep.Totals.Unsupported, + "sqlite target claims full capability; unsupported cases indicate a capability-wiring regression")中文
sqlite.Target.Caps()返回CapAll，因此rep.Totals.Unsupported非零意味着能力接入出现了真实回归，而当前只是记录日志，不会导致测试失败。建议改为断言其为零。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@session/replaytest/sqlite/replay_test.go` around lines 54 - 56, Update the replay test’s handling of rep.Totals.Unsupported to fail the test when it is nonzero, using the test’s assertion or failure mechanism instead of only t.Logf. Preserve the existing diagnostic context by including the unsupported count and report reference in the failure message.session/replaytest/runner.go (1)
400-411: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant
rs.seq++beforebuildEvent(which increments again internally).Each concurrent event consumes two sequence slots instead of one, since
buildEvent(line 435) already incrementsrs.seq. Harmless (IDs stay unique/monotonic and both backends see the same script), but the pre-increment at line 402 does nothing useful and makes sequence math inconsistent with the single-increment convention used everywhere else in this file.♻️ Proposed cleanup
for i := 0; i < w.Count; i++ { - rs.seq++ specs = append(specs, r.buildEvent(rs, st.SessionID, &EventSpec{中文
第402行在调用
buildEvent之前多做了一次rs.seq++，而buildEvent内部（第435行）自身也会自增，导致并发事件每个消耗两个序号而非一个。功能上无害（序号仍单调唯一，且两端后端回放同一脚本），但与本文件其余位置“每个事件自增一次”的约定不一致，建议移除多余的自增。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@session/replaytest/runner.go` around lines 400 - 411, Remove the redundant rs.seq++ inside the loop that builds concurrent events, while keeping buildEvent responsible for the single sequence increment per event. Leave the event construction and expected-count updates unchanged.session/replaytest/snapshot.go (1)
121-133: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winState capture is gated by Session capability, contradicting the independent-capability contract.
CapabilitydocumentsStateandSessionas independent fields, andCapability.Missingtreats them independently. But here, App/User state capture (lines 121-133) only runs insideif svc != nil && t.Caps().Session. A future target withState: true, Session: falsewould pass theMissing()check (state "supported") yet silently produce an empty state snapshot instead of being reportedUnsupported, violating the documented "unsupported instead of failing" contract. Currently latent since all targets returnCapAll, but worth tightening for extensibility.中文
Capability将State与Session定义为独立字段，但此处状态读取被嵌套在Session能力判断内部。若未来出现State: true, Session: false的后端，Missing()检查会通过，但状态快照会被静默留空，而不是按文档约定标记为Unsupported。目前因所有后端都返回CapAll而未触发，但建议为扩展性提前修正。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@session/replaytest/snapshot.go` around lines 121 - 133, Update the state-capture logic in the snapshot-building flow so AppState and UserState are gated by t.Caps().State independently of t.Caps().Session. Preserve the existing service availability and error handling, and ensure targets with State unsupported produce the documented Unsupported result rather than an empty state snapshot.session/replaytest/target.go (1)
89-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding
Close() error(or requiringio.Closer) to theTargetinterface.All three concrete implementations reviewed in this cohort (
InMemoryTarget,sqlite.Target,redis.Target) already implement an identicalClose() errormethod, but it isn't part of theTargetcontract, so generic code holding aTargetvalue can't clean it up without a type assertion. Given every current implementer already complies, this is low cost to add now versus later once more backends exist. Worth checking whetherreport.go'sRunPair/RunPairT(not in this cohort) currently type-switches or leaks target cleanup responsibility to callers only.中文
本队列审查到的三个具体实现（
InMemoryTarget、sqlite.Target、redis.Target）都已经实现了同样签名的Close() error，但Target接口本身并未声明该方法，导致持有Target接口值的通用代码无法直接调用Close。由于现有实现均已满足该签名，现在补充成本很低。建议核实report.go的RunPair/RunPairT（不在本次审查范围）是否需要统一的生命周期管理。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@session/replaytest/target.go` around lines 89 - 100, Update the Target interface to require Close() error, matching the existing implementations InMemoryTarget, sqlite.Target, and redis.Target, so generic Target values can be cleaned up directly. Check RunPair and RunPairT for target lifecycle handling and use the interface method where cleanup is responsible there.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@session/replaytest/cases/cases.go`:
- Around line 21-35: Update the case ordering returned by All() to match the
documented public numbering, placing MemoryScopeIsolation at its designated
position 11 and preserving the documented order of subsequent cases. Keep
RunPair’s slice-order behavior unchanged and do not alter case implementations.
In `@session/replaytest/cases/memory.go`:
- Around line 101-116: Adjust the replay sequence around OpClearMemories so the
final snapshot still includes the updated preference and metadata-bearing
records, or add an intermediate snapshot before clearing them. Preserve clear
behavior coverage through the existing OpClearMemories step, while ensuring
update, topics, metadata, and serialization fidelity are observable in the
captured snapshot; MemoryScopeIsolation already covers clear semantics.
- Around line 75-78: Remove the MemorySearch requirement from the
memory/write_read case’s NeedCaps configuration, retaining only the baseline
Memory capability so backends without search still run add/update/delete/list
validation. Keep SearchQuery unchanged so search results remain conditionally
captured when supported.
In `@session/replaytest/cases/track.go`:
- Around line 21-25: Update the Description in the replaytest.Case named
"track/tool_and_subtask" to state that duration fields are normalized/scrubbed,
replacing the inaccurate allowed_diff wording while preserving the rest of the
description.
In `@session/replaytest/go.mod`:
- Line 47: Upgrade the root module’s indirect OpenTelemetry SDK and gRPC
dependencies so replaytest resolves go.opentelemetry.io/otel/sdk to at least
v1.43.0 and google.golang.org/grpc to at least v1.79.3, then regenerate the
session/replaytest go.mod dependency pins and associated sums.
In `@session/replaytest/normalize.go`:
- Around line 237-240: Update the LongRunningToolIDs loop in normalizeEvent to
append callSym.sym(id) instead of the raw id, while preserving the existing
sorting of ce.LongRunning so long-running tool-call IDs are normalized
consistently with ToolID and ToolCalls[*].ID.
In `@session/replaytest/snapshot.go`:
- Around line 95-118: Protect both sess.Summaries reads in the primary snapshot
path and the WindowEventNum snapshot block with the same SummariesMu
RLock/RUnlock pattern used by verifySummaryIsolation in runner.go. Keep the
existing snapshot construction unchanged while ensuring each summary access
occurs under the read lock.
---
Nitpick comments:
In `@session/replaytest/case.go`:
- Around line 82-174: Define and document the public contracts for Step and its
operation-specific fields, including required versus ignored fields, nil
behavior, defaults, and invalid combinations. Add runner validation for each Op,
including SummarySpec.FilterKey, ToolCallSpec.ID, TrackSpec.Payload, and
WriterSpec constraints, with clear errors and consistent handling of omitted
values. Ensure raw JSON fields are validated and concurrent writer settings have
explicit non-negative/count and required-field rules.
- Around line 10-175: Run golangci-lint for the session/replaytest package using
the repository-level ../../.golangci.yml configuration, address any reported
issues in the affected symbols, then run go test ./... to verify the changes.
In `@session/replaytest/diff.go`:
- Around line 490-495: Remove the package-level min helper function and rely on
Go 1.21’s predeclared min builtin at its existing call sites; do not alter the
surrounding diff logic.
In `@session/replaytest/report.go`:
- Around line 46-52: Rename the exported Note field in the report schema to a
distinct name such as UnsupportedNote or Reason, while preserving its string
type, JSON behavior, and unsupported-status explanation. Update all references
to Note consistently; leave the Notes []Diff field unchanged.
In `@session/replaytest/runner.go`:
- Around line 400-411: Remove the redundant rs.seq++ inside the loop that builds
concurrent events, while keeping buildEvent responsible for the single sequence
increment per event. Leave the event construction and expected-count updates
unchanged.
In `@session/replaytest/snapshot.go`:
- Around line 121-133: Update the state-capture logic in the snapshot-building
flow so AppState and UserState are gated by t.Caps().State independently of
t.Caps().Session. Preserve the existing service availability and error handling,
and ensure targets with State unsupported produce the documented Unsupported
result rather than an empty state snapshot.
In `@session/replaytest/sqlite/replay_test.go`:
- Around line 54-56: Update the replay test’s handling of rep.Totals.Unsupported
to fail the test when it is nonzero, using the test’s assertion or failure
mechanism instead of only t.Logf. Preserve the existing diagnostic context by
including the unsupported count and report reference in the failure message.
In `@session/replaytest/target.go`:
- Around line 89-100: Update the Target interface to require Close() error,
matching the existing implementations InMemoryTarget, sqlite.Target, and
redis.Target, so generic Target values can be cleaned up directly. Check RunPair
and RunPairT for target lifecycle handling and use the interface method where
cleanup is responsible there.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 99e1c2bc-d01b-4855-a435-1e932aab3579
⛔ Files ignored due to path filters (1)
session/replaytest/go.sumis excluded by!**/*.sum
📒 Files selected for processing (38)
session/replaytest/README.mdsession/replaytest/case.gosession/replaytest/cases/basic.gosession/replaytest/cases/cases.gosession/replaytest/cases/concurrency.gosession/replaytest/cases/memory.gosession/replaytest/cases/recovery.gosession/replaytest/cases/state.gosession/replaytest/cases/summary.gosession/replaytest/cases/toolcall.gosession/replaytest/cases/track.gosession/replaytest/delta.gosession/replaytest/delta_test.gosession/replaytest/diff.gosession/replaytest/diff_test.gosession/replaytest/doc.gosession/replaytest/e2e_fault_test.gosession/replaytest/example_report_test.gosession/replaytest/fakesum.gosession/replaytest/go.modsession/replaytest/inmemory.gosession/replaytest/mutate.gosession/replaytest/mutate_test.gosession/replaytest/normalize.gosession/replaytest/normalize_test.gosession/replaytest/redis/miniredis_test.gosession/replaytest/redis/replay_test.gosession/replaytest/redis/target.gosession/replaytest/report.gosession/replaytest/report_test.gosession/replaytest/runner.gosession/replaytest/selftest_test.gosession/replaytest/session_memory_summary_track_diff_report.jsonsession/replaytest/snapshot.gosession/replaytest/sqlite/replay_test.gosession/replaytest/sqlite/target.gosession/replaytest/target.gosession/replaytest/testing.go
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2293 +/- ##
===================================================
+ Coverage 89.86563% 89.91635% +0.05071%
===================================================
Files 1144 1172 +28
Lines 198483 205382 +6899
===================================================
+ Hits 178368 184672 +6304
- Misses 12602 12963 +361
- Partials 7513 7747 +234
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@session/replaytest/e2e_fault_test.go`:
- Around line 254-266: Update the test around replaytest.RunPair to execute only
cases.MemoryScopeIsolation() instead of cases.All(), then assert that the
specific case has StatusFail and includes a replaytest.DimMemory diff. Preserve
the existing failure validation while ensuring unrelated cases cannot satisfy
the assertions.
In `@session/replaytest/internal_test.go`:
- Around line 293-295: Update the over-length input setup in the
parseBoundedDecimal test to use valid numeric content via strings.Repeat("1",
maxExactNumberCharacters+1), ensuring the rejection specifically exercises the
maximum-length boundary rather than malformed NUL bytes.
- Around line 12-27: Update the validation setup to use the supported structured
form for .golangci.yml output.formats. In session/replaytest/internal_test.go
(12-27) and session/replaytest/snapshot.go (73-79), isolate Redis- and
CGO-dependent SQLite replay tests behind appropriate integration/build
constraints so go test ./... remains self-contained without external services or
CGO.
In `@session/replaytest/redis/target_test.go`:
- Around line 44-47: Strengthen the second Reset test around tgt.Reset by
retaining the pre-reset SessionService and MemoryService, writing representative
data before reset, then asserting the post-reset accessors are replaced and the
prior data is not visible. Verify the externally observable isolation and prefix
rotation without relying solely on non-nil checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 671c0059-ae5b-43a0-b8d2-eeef1b08ee59
⛔ Files ignored due to path filters (1)
session/replaytest/go.sumis excluded by!**/*.sum
📒 Files selected for processing (25)
session/replaytest/README.mdsession/replaytest/case.gosession/replaytest/cases/cases.gosession/replaytest/cases/cases_test.gosession/replaytest/cases/memory.gosession/replaytest/cases/track.gosession/replaytest/diff.gosession/replaytest/e2e_fault_test.gosession/replaytest/example_report_test.gosession/replaytest/fakesum_test.gosession/replaytest/go.modsession/replaytest/internal_test.gosession/replaytest/normalize.gosession/replaytest/normalize_test.gosession/replaytest/redis/target.gosession/replaytest/redis/target_test.gosession/replaytest/report.gosession/replaytest/report_test.gosession/replaytest/runner.gosession/replaytest/runner_errors_test.gosession/replaytest/session_memory_summary_track_diff_report.jsonsession/replaytest/snapshot.gosession/replaytest/sqlite/replay_test.gosession/replaytest/target.gosession/replaytest/target_test.go
💤 Files with no reviewable changes (2)
- session/replaytest/diff.go
- session/replaytest/runner.go
🚧 Files skipped from review as they are similar to previous changes (12)
- session/replaytest/cases/track.go
- session/replaytest/sqlite/replay_test.go
- session/replaytest/target.go
- session/replaytest/cases/cases.go
- session/replaytest/redis/target.go
- session/replaytest/report.go
- session/replaytest/example_report_test.go
- session/replaytest/README.md
- session/replaytest/case.go
- session/replaytest/normalize_test.go
- session/replaytest/session_memory_summary_track_diff_report.json
- session/replaytest/normalize.go
3 participants
Refs #2001.
What changed
Backend authors and CI now have a reusable way to verify that Session and
Memory backends behave identically under the same agent trajectory. A
deterministic case suite is replayed into each backend, and any behavioral
drift — lost or reordered events, state divergence, polluted memories,
missing or mis-attributed summaries, wrong filter-keys, dropped tracks —
is reported with an exact location (session id, event index, summary
filter-key, track name, memory id, field path, and both backends' values)
instead of surfacing as replay corruption or context loss in production.
New backends onboard in three documented steps and are compared against
the InMemory reference with no changes to existing code.
Why
Teams routinely prototype on InMemory and later switch to SQLite, Redis,
or other persistent backends. If event order, state, memory, or summary
semantics diverge silently across backends, long conversations replay
incorrectly and summaries get lost or overwritten — issues #2001 calls
out specifically, including the Go-specific filter-key summary and track
dimensions.
Non-obvious design decisions:
and backend modules via
replace, so it can importsession/sqlite,session/redis, etc. without creating import cycles, and touches zeroexisting files.
rewriting, canonical JSON, scrubbed timing fields, exact-decimal float
tolerance) so that legitimate representation differences never become
false failures, while everything else fails strictly.
unsupported(allowed_diff) rather than letting them fake-pass or hard-fail.
cross-session summary leaks that per-case snapshots cannot see.
Testing
All commands run from
session/replaytest/(CGO enabled):go build ./... && go vet ./...— clean.go test ./...— all green (~12s): comparator/normalizer/reporter unittests, mutation detection matrix, e2e fault injection, miniredis smoke
test.
go test ./sqlite/ -run TestReplayConsistencySQLite— 11/11 public cases pass InMemory vs real SQLite, timed ~5s with an
explicit
< 30sassertion.mutate_test.goapplies 9 mutationclasses across every public case — including all four summary fault
classes (loss, stale overwrite, wrong session, wrong filter-key) — and
asserts located detection;
e2e_fault_test.goinjects silent drop,dirty half-write, and cross-session summary leak at the real
session.Serviceboundary.TestFalsePositiveRateWithinBudgetreplays the fullsuite 10x on dual InMemory targets and asserts FP rate <= 5%
(observed 0/110); the concurrency case is repeated 100x without flakes.
session_memory_summary_track_diff_report.jsonregenerates byte-identically via
TestGenerateExampleReport.gofmt/goimports, license headers, typos,
go mod tidy(all 77modules), check-go-mod-version, external-consumer (CGO off), sumdb,
file-size, LFS.
Not run: a real Redis server (the binding is verified on miniredis only);
Postgres/MySQL/ClickHouse are not wired, which the issue lists as
optional.
Notes for reviewers
sibling backend modules at their real release tags (v1.9.0) with local
replacedirectives, since the placeholderv0.0.0-00010101000000-000000000000is forbidden bycheck-go-mod-version.
(proving the differ), while
e2e_fault_test.gocovers three faultclasses at the real service boundary; it does not simulate real backend
non-atomic writes — see the README "Limitations" section, which also
documents the InMemory-reference caveat (consistency != correctness).
compares multiset equality plus per-branch partial order; please check
this matches expected backend semantics.
keys carry a 1h TTL; memory-side keys have no TTL (documented).