Skip to content

fix(session): route hydrate_from_event_log through bounded replay#5861

Merged
bug-ops merged 2 commits into
mainfrom
fix/5851-hydrate-chunked-replay-bypass
Jul 10, 2026
Merged

fix(session): route hydrate_from_event_log through bounded replay#5861
bug-ops merged 2 commits into
mainfrom
fix/5851-hydrate-chunked-replay-bypass

Conversation

@bug-ops

@bug-ops bug-ops commented Jul 10, 2026

Copy link
Copy Markdown
Owner

Summary

  • hydrate_from_event_log (crates/zeph-agent-persistence/src/hydrate.rs) is the single choke point every session-resume path (ACP resume/load/fork, CLI sessions resume, /conv resume) routes through, but it never called the bounded/chunked ReplayEngine::replay added by perf(session): bound replay memory via chunked event-log read #5844 — it still did ReplayEngine::fold(events.clone(), up_to), doubling peak memory (the clone plus the events Vec already held for reconcile_projection/Hydrated::events) and defeating perf(session): bound replay memory via chunked event-log read #5844's memory bound on the production resume path.
  • Fixed by folding messages via ReplayEngine::replay(session_path, up_to) (bounded/chunked reader, ≤ 100 envelopes in memory at once). events is still fully read via log.read_all() — a deliberate second sequential I/O pass, not a second in-memory copy — since reconcile_projection and maybe_condense_on_resume need the full owned event list.
  • Also deduplicated the identical torn-tail warn+repair epilogue between read_events/read_events_chunked (crates/zeph-session/src/log.rs) into a shared finish_torn_tail helper.

Closes #5851, closes #5852.

Design notes

  • Verified ReplayEngine::replay's internal lockless SessionEventLog::open cannot deadlock against hydrate_from_event_log's already-open exclusive-locked log handle — open() never acquires the flock.
  • up_to semantics are shared between fold and replay via the same fold_step helper; a new regression test (up_to_bounds_messages_but_not_events) exercises up_to = Some(n) through hydrate_from_event_log for the first time and pins down that Hydrated::events is always the full, unbounded log (pre-existing, unchanged behavior — reconcile_projection/condensation need the whole list regardless of the fold's cutoff).
  • Known, accepted minor side effect (flagged by adversarial review): a torn-tailed session's log now triggers the "dropped torn tail" warning twice per resume instead of once, since the fix performs two independent reads of events.jsonl (one lockless inside replay, one exclusive via read_all). Both reads observe identical bytes (no concurrent writer during resume, per D-10), so this is a duplicate log line, not a correctness issue.

Test plan

  • cargo +nightly fmt --check — clean
  • cargo clippy --profile ci -p zeph-session -p zeph-agent-persistence --all-targets --features sqlite -- -D warnings — clean
  • cargo nextest run --config-file .github/nextest.toml -p zeph-session -p zeph-agent-persistence — 101/101 pass (includes new up_to_bounds_messages_but_not_events regression test)
  • RUSTDOCFLAGS="--deny rustdoc::broken_intra_doc_links" cargo doc --no-deps -p zeph-session -p zeph-agent-persistence — clean
  • All 4 pre-existing hydrate.rs tests pass unchanged (behavioral equivalence confirmed)
  • Full workspace cargo clippy --profile ci --workspace ... gate — currently blocked by an unrelated, pre-existing failure on main HEAD in crates/zeph-scheduler/src/durable.rs:239 (clippy::manual_assert_eq), filed separately as workspace clippy gate fails on main HEAD: manual_assert_eq in zeph-scheduler durable.rs test #5860; not introduced by this PR (scoped clippy on both touched crates is clean)
  • Live-tester pass on the resume path (tracked in .local/testing/coverage-status.md, marked "Untested (live)" pending next CI cycle)

hydrate_from_event_log called ReplayEngine::fold(events.clone(), up_to)
instead of the bounded/chunked ReplayEngine::replay added by #5844,
doubling peak memory on every session-resume path (ACP resume/load/fork,
CLI sessions resume, /conv resume) and defeating #5844's memory bound
where it matters most. The messages fold now goes through
ReplayEngine::replay; events is still read once via read_all for
reconcile_projection and Hydrated::events, which need the full owned
list — a deliberate second sequential read, not a second in-memory copy.

Also extract a shared finish_torn_tail helper in zeph-session/src/log.rs
for the torn-tail warn+repair epilogue duplicated between read_events
and read_events_chunked.

Closes #5851, closes #5852
@github-actions github-actions Bot added documentation Improvements or additions to documentation rust Rust code changes bug Something isn't working size/M Medium PR (51-200 lines) labels Jul 10, 2026
@bug-ops
bug-ops enabled auto-merge (squash) July 10, 2026 13:40
The rustc/clippy 1.97.0 (2026-07-07) toolchain bump tightened several
lints (manual_assert_eq, useless_borrows_in_formatting, question_mark),
newly flagging 10 pre-existing call sites across the workspace and
blocking the full clippy gate for every open PR, including this one.

Trivial, mechanical, behavior-preserving fixes, unrelated to #5851/#5852:

- crates/zeph-scheduler/src/durable.rs: assert! -> assert_ne!
- crates/zeph-orchestration/src/adaptorch.rs: assert! -> assert_eq!
- crates/zeph-core/src/agent/experiment_cmd.rs: drop redundant & in writeln!
- crates/zeph-core/src/agent/learning/background.rs: drop redundant & in format!
- crates/zeph-tui/src/widgets/chat.rs: assert! -> assert_eq!
- crates/zeph-tui/src/widgets/subagents.rs: drop redundant & (x2)
- src/runner.rs: drop redundant & in println!
- src/scheduler_executor.rs: use ? instead of an explicit else/return None
- src/tests.rs: assert! -> assert_eq!

Full CI-matching clippy (both the desktop/ide/server/chat/pdf/scheduler
feature set and the bench matrix), fmt, and nextest on every touched
crate all verified clean.

See #5860
@github-actions github-actions Bot added the core zeph-core crate label Jul 10, 2026
@bug-ops
bug-ops merged commit cae6442 into main Jul 10, 2026
43 checks passed
@bug-ops
bug-ops deleted the fix/5851-hydrate-chunked-replay-bypass branch July 10, 2026 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working core zeph-core crate documentation Improvements or additions to documentation rust Rust code changes size/M Medium PR (51-200 lines)

Projects

None yet

1 participant