Clarify the child-ordering invariant (Codex review follow-up)
The createdAt+recordedId child sort comment over-claimed "tie-safe". Codex flagged that a same-millisecond sibling tie would be broken by random session id, which does not recover first-call order. In the current synchronous cut that tie is unreachable — the subagent tool awaits one child's result and disposes it before the parent starts the next, so siblings' createdAt values are strictly ordered and match first-call order. Restate the comment to that real invariant (at both the replay sort and the harvest sort), note that the id tiebreak only makes a degenerate collision deterministic, and flag the concurrent-subagent cut that would need a real first-call ordinal with XXX(concurrent-subagents). The RFC records the same limitation. Comment/doc only — no behavior change.
This commit is contained in:
@@ -29,6 +29,8 @@ Live session ids are freshly random every run and never equal the recorded ones,
|
||||
|
||||
This keys by WHO calls, not by global call order — so it stays correct even if subagents ever run concurrently or in the background (a global cursor would interleave them). A call carrying no `sessionId` (a direct unit-test `stream()`) is treated as one anonymous session bound to the primary script, so the single-session path is byte-for-byte the old behavior. More distinct live sessions than recorded scripts is a fail-loud error (an unrecorded subagent appeared), never a silent mis-route.
|
||||
|
||||
The ordering key is the session header `createdAt`. In the current synchronous cut this is sound because sibling children are created **strictly sequentially** — the subagent tool awaits one child's result and disposes it before the parent's next tool call starts the next child — so their `createdAt` values are strictly ordered and match first-call order exactly. A same-millisecond sibling tie is therefore unreachable; the `recordedId` tiebreak only keeps such a degenerate collision deterministic, it does not recover first-call order. A future cut that runs siblings concurrently/backgrounded WOULD be able to create two children in the same millisecond, and must then thread a real first-call ordinal (the order live sessions first stream) rather than leaning on `createdAt` — flagged with `XXX(concurrent-subagents)` at the sort site.
|
||||
|
||||
The alternative considered and rejected was a **call-ordered merge of the parent and child logs** into one global script (sound only because in-process subagent execution is strictly nested — the parent blocks on the child). It is simpler for today's synchronous cut but bakes in the parent-blocks-on-child invariant that a future backgrounded/concurrent subagent would break; per-session keying does not.
|
||||
|
||||
### 3. The harness harvests every log, primary-first
|
||||
|
||||
@@ -369,8 +369,13 @@ async function harvestSessionLogs(root: string): Promise<HarvestedLog[]> {
|
||||
}
|
||||
}
|
||||
// Primary (no parentSession) first, then children by ascending createdAt. A
|
||||
// scenario has exactly one top-level session; ties among children fall back to
|
||||
// recorded id for a stable order.
|
||||
// scenario has exactly one top-level session. In the synchronous cut sibling
|
||||
// children are created strictly sequentially, so their createdAt values are
|
||||
// strictly ordered; the recordedId tiebreak only keeps a degenerate
|
||||
// same-millisecond collision (unreachable here) deterministic. This harvest
|
||||
// order must match the replay load order in dsh-llm-replay's loadSessionScripts
|
||||
// so session.<n>.jsonl maps to the same child on record and replay — replay
|
||||
// re-sorts childFiles by the same key, so the two stay consistent.
|
||||
logs.sort((a, b) => {
|
||||
const ap = a.parentSession === undefined ? 0 : 1
|
||||
const bp = b.parentSession === undefined ? 0 : 1
|
||||
|
||||
@@ -266,10 +266,18 @@ export function loadSessionScripts(config: ReplayConfig): SessionScript[] {
|
||||
}
|
||||
// The primary (parent) always binds first — it issues the first model call,
|
||||
// because it must run a turn before it can delegate. Children follow in
|
||||
// createdAt order (the order they were spawned in the synchronous nested cut),
|
||||
// ties broken by recorded id for determinism. Keeping the primary at the head
|
||||
// rather than sorting it among the children means a sub-millisecond
|
||||
// parent/child createdAt collision can never reorder it behind a child.
|
||||
// createdAt order. In the current synchronous cut sibling children are created
|
||||
// STRICTLY SEQUENTIALLY — the subagent tool awaits one child's result and
|
||||
// disposes it before the parent's next tool call can start the next — so their
|
||||
// createdAt values are strictly ordered and match first-call order exactly.
|
||||
// The recordedId tiebreak only makes a degenerate same-millisecond collision
|
||||
// (unreachable in this cut) deterministic; it does NOT recover first-call
|
||||
// order, so it is arbitrary if such a tie ever occurs.
|
||||
// XXX(concurrent-subagents): a future cut that runs siblings concurrently or
|
||||
// backgrounded could create two children in the same millisecond, where this
|
||||
// createdAt+id order may diverge from first-call order. That cut must thread a
|
||||
// real first-call ordinal (the order live sessions first stream) instead of
|
||||
// leaning on createdAt — see the per-session-replay RFC.
|
||||
children.sort((a, b) => a.createdAt - b.createdAt || a.recordedId.localeCompare(b.recordedId))
|
||||
return [primary, ...children]
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user