Files

T

Hypatia May 828c3f85c9 fix review findings: skip collided SCHEMA_VERSION 3; reject marker-less surface events

P1: both merge parents shipped SCHEMA_VERSION=3 for different layouts (surface
columns vs seed_length), so an on-disk 3 was ambiguous and wrongly accepted.
Bump to 4 (merged layout) so the version check rejects both sibling v3s.

P2: a surface-eligible event with no surfaceOp lands in the log but vanishes
from deriveMessages() (surface is the sole derivation path). The typed append
overload enforces the marker only when the type arg is a literal; it collapses
to optional when widened to the union (a caller iterating raw events). Guard at
runtime in both append() and the seed constructor — no backward-compat for
surface-less logs. Shared seed fixtures carry surfaceOp explicitly and the
appendLog helper forwards it verbatim (no synthesized default). Exports
isSurfaceEligibleType. Regression tests for all three, each verified to fail
on the unfixed code.

Gates: typecheck, test (1115), snapshot (14), doc-sync, lint, build, hygiene green.

2026-06-24 17:45:48 +08:00

6.0 KiB

Raw Blame History

RFC: Persist the seed boundary so fork-child replay routes correctly

Status: implemented

Problem

The per-session snapshot replay RFC made the snapshot tier express a nested-agent shape: a parent plus one recorded log per in-process subagent, each replayed as its own script keyed by calling session. It noted (§ Scope, final bullet) that a fork snapshot was "a trivial future addition, not a gap in the keying." That was wrong about a fork child specifically — not the keying, but the script derivation.

A subagent script is derived from a recorded session log by deriveReplayScript: it groups the log's assistant/chunk events by (turn, step) into one replay entry per stream() call. This is correct for a spawn child, whose log contains only its own model calls.

A fork child is different. The fork backend seeds the child session with a balanced completed-turn prefix of the parent's log (dsh-subagent-inprocess), and that seed becomes the child session's persisted log (Session's constructor copies the seed into this.log). So a fork child's .jsonl begins with the parent's events — including the parent's assistant/chunk events — and only then carries the child's own turn.

Deriving the child script from the whole fork-child log therefore replays the parent's recorded responses as the child's model calls: the live fork child's first stream() would receive the parent's first recorded chunk sequence instead of its own. The recorded scenarios are all spawn today, so this never fired — but a fork snapshot would have mis-routed silently, exactly the class of bug the snapshot tier exists to catch.

Decision

Record where a session's inherited prefix ends, persist it, and have the replay harness derive a child's script from its own events only.

1. `seedLength` on the session header

SessionHeader gains an optional seedLength: number — how many leading events were inherited via a seed rather than produced by this session. The fork backend stamps it (= the seeded-prefix length) when it creates the child; a fresh spawn leaves it absent (≡ 0). It is threaded through CreateSessionOptions.meta (and CreateAgentOptions.meta), set in SessionStore.prepare.

seedLength is explicit, never inferred from seed.length. A reconstruction (resume/load) seeds the session with its WHOLE stored log, so seed.length there is the full length, not the original boundary — the resume path passes the persisted seedLength back from the loaded header instead. (Same shape as createdAt, which is also explicitly preserved on reconstruction rather than re-defaulted to now.)

2. Both persistence backends round-trip it

JSONL: a seedLength field on the header line (toHeaderLine/fromHeaderLine).
SQLite: a seed_length column on the sessions table.

The SQLite change is a breaking table-layout change, so SCHEMA_VERSION bumps. This branch added seed_length under version 3; it later merged with the session-surface branch, which had independently shipped its OWN version-3 layout (the source_event_seqs/surface_op columns). Because an on-disk 3 is ambiguous between the two sibling layouts, the merged build is version 4 (every column), and an on-disk 3 is rejected like any other non-current version. Per the repo's pre-release stance (§ "Pre-release stance" in AGENTS.md) the backend rejects a non-current user_version on open rather than migrating it — there is no persisted user data to preserve, so no migration code is written (the existing reject-not-migrate path at openDatabase already enforces this; v1, v2, and the collided v3 are all rejected).

3. Replay derives a child script after the boundary

dsh-llm-replay's parseSessionHeader now also reads seedLength (absent ⇒ 0), and loadSessionScripts derives a child's entries from parseSessionLog(text).slice(seedLength) — the events at or after the boundary, i.e. the child's own model calls. For a spawn child seedLength is 0 and this is a no-op, so spawn scenarios are byte-for-byte unchanged.

This closes the routing correctness gap, and two recorded fork scenarios exercise it end to end — see Record fork and mixed spawn+fork snapshot scenarios.

Alternatives considered

Derive the boundary heuristically in llm-replay (the seeded prefix is contiguous parent events ending at the last turn/end before the child's first user/message). Rejected: a brittle heuristic in the test harness that re-derives a fact the producer already knows. Persisting the boundary at its source (the fork backend) is the "explicit > implicit at package seams" rule applied across the persistence boundary — the reader of a child fixture never has to reconstruct where the inheritance ended.
Pin the format version instead of bumping (the SESSION_FORMAT_VERSION = 0 "unstable" stance the event log uses). Rejected for the SQLite table layout: SCHEMA_VERSION is the monotonic bump-and-reject knob (a small enumerable set of revisions worth telling apart), distinct from the event-vocabulary version. Adding a column is precisely the breaking table change it versions, so it bumps.

Consequences

A new persisted header field across core + both backends; the core-data-structures catalog (persistence.md) is updated in the same change (its SessionHeader / CreateSessionOptions type-equiv blocks).
Existing SQLite databases at schema v2 are rejected on open (no user data pre-release).
Spawn replay is unchanged (seedLength 0). Fork replay now routes a child to its own script; covered by a regression in llm-replay's tests (a child fixture whose seeded prefix carries a parent chunk — the derived child script must exclude it, proven red without the slice) and a persistence round-trip test (both backends, via the shared coordinator contract).

6.0 KiB Raw Blame History