Skip to content

fix(upload): time-consistent replay so delegated sessions don't strand at status=running - #94

Open
Diego Colombo (colombod) wants to merge 3 commits into
mainfrom
fix/upload-time-consistent-replay
Open

fix(upload): time-consistent replay so delegated sessions don't strand at status=running#94
Diego Colombo (colombod) wants to merge 3 commits into
mainfrom
fix/upload-time-consistent-replay

Conversation

@colombod

Copy link
Copy Markdown
Collaborator

Problem

When a recorded session is uploaded, run_upload feeds events one whole session at a time, parents before children, in strict file order, with nothing relating that feed to the server's own drain order. The server drains each session's events on an independent, concurrent queue. So for a session that used delegated sub-agents, a cross-session write that (re)opens a node — the parent's delegate:agent_spawned opening the child, or the child's session:start opening the parent — can be processed after that node's own session:end. The node is then left stranded at status="running" in the graph even though the upload completed with no error.

This is an upload-ordering artifact only: it does not occur for live, real-time capture (the events are naturally spaced), and it is invisible at upload time (every POST returns 2xx).

Fix (client-side only)

Make the uploader replay time-consistently, so a spawned sub-session drains before its parent resumes — reproducing the real capture timing that is race-free:

  1. Feed events in true global timestamp order across the session-tree closure, via a lazy heapq.merge over per-line generators. A spawned sub-session's events therefore interleave before its parent's later-timestamped resume, exactly as they occurred.
  2. Pace by the events' own inter-event gaps, capped at max_gap_s (default 2.0s), derived purely from the recorded timestamp fields.

Everything else is reused verbatim — parsing, build_payload, the POST + bounded-retry loop, per-attempt auth, reconciliation, and progress. Only the feed order + pacing + the tracker's start/complete triggers changed.

Explicitly NOT in this change

  • No server change of any kind.
  • No new server calls — the only request remains the existing POST /events. No /status polling, no drain/barrier signal. (Enforced in tests: the client mock is spec=["post"], so any non-POST call raises.)

What this gives user data uploads

  • Delegated sessions land correctly. Sub-agent (and parent) nodes end completed, so the uploaded graph faithfully represents what happened instead of showing phantom "still running" sessions.
  • Bounded memory — safe on large sessions. The merge is a lazy generator: memory is O(number of sessions), not O(events); no events.jsonl is ever read whole. Measured: a 74.9 MB single-session file uploads inside a ~41 MB process; 430× more events / ~1960× more bytes cost +944 KB peak RSS.
  • Not a real-time replay. Idle gaps are capped, so a 19-minute session uploads in ~2 minutes (~80% of events sleep ~0); the cost is bounded per gap, not wall-clock fidelity.
  • No regression for ordinary sessions. Non-delegated uploads are unchanged; event accounting stays exact (accepted == written, 0 dead-letter/residual).

Evidence

Validated against an isolated server instance with small real sessions:

Check Result
Delegated session correctness root + all sub-sessions completed 20/20 trials
Non-delegated regression clean upload, exact event accounting, 0 loss/dup
Memory / streaming peak RSS flat (~41 MB) uploading a 74.9 MB single events.jsonl; RSS independent of event count
No /status proxy log shows only POST /events
Real-time? 19.4-min session replays in ~2 min
Tests 564 existing + 10 new pass; ruff/pyright clean

Scope / not claimed

  • Does not claim race-freedom under concurrent external writers to the same server — the ordering guarantee is for offline, sole-writer uploads (the upload model). The pacing is bounded by max_gap_s, never a wrong result.
  • Does not address the separate IncompleteSession co-labelling (its own PR).

Test plan

  • uv run pytest (module) — green (574).
  • uv run ruff check / pyright — clean.
  • New tests/test_replay_ordering.py: global timestamp ordering, parent/child interleave (child session:end before parent's; parent spawn before child session:end), capped-gap pacing with event_delay_s floor, and a mechanical assertion that no non-/events request is ever made.

…sions

run_upload previously fed events whole-session, parent-first, with no ordering
relative to the server's concurrent per-session drain -- so a delegated
sub-session (or its parent) could be left status=running when a cross-session
reopen was processed after that node's own session:end.

Feed events in true global timestamp order across the session-tree closure
(lazy heapq.merge over per-line generators -- O(sessions) memory, never loads a
file whole), and pace by the events' own inter-event gaps capped at max_gap_s
(default 2.0s) so a spawned sub-session drains before its parent resumes. No
server change and no /status/drain signal -- ordering is derived purely from the
recorded timestamps. Reuses the existing parse/build_payload/POST/retry/auth path
verbatim; only the feed order + pacing + tracker triggers changed.

Validated on an isolated server: child+parent completed 20/20; non-delegated
upload unaffected; peak RSS flat (~41MB) uploading a 74.9MB single-session file;
zero /status calls. 564 existing + 10 new tests green.

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…sion)

test_project_version asserted the pyproject version equals a hard-coded literal
("0.1.3"). It catches no defect -- it only mirrors one hand-edited string
against another -- and breaks on every routine version bump and every in-flight
PR based on a pre-bump commit (as #93's 0.1.4 bump just did, leaving main red).
The useful pyproject contracts (name, requires-python, license, deps, no
amplifier.modules entry point) are kept.

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Match the lock's editable self-package version to pyproject (bumped to 0.1.4 by
#93); no dependency changes.

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
@colombod
Diego Colombo (colombod) force-pushed the fix/upload-time-consistent-replay branch from d205b7b to f4c8dfb Compare August 17, 2026 11:14
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.

1 participant