feat(observability): #108 residuals — always-on subagent registry feed + resume user bubble + crashed-turn replay#118
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc89a4a3e8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| id: `resume_user_${turnId}`, | ||
| turnId, | ||
| role: "user", | ||
| content: userPreview, |
There was a problem hiding this comment.
Keep synthetic resume previews out of request history
When a resumed turn settles, the synthetic user bubble remains in messages until the transcript refetch/hydrate completes, and runTurn builds the next chat request from that same messages array. If the original prompt was longer than user_text_preview's 200-character truncation and the user sends another message immediately after streaming stops (or the refetch fails), the next request includes this truncated preview as prior history, corrupting the model context. This bubble needs to be marked UI-only/excluded from request payloads or replaced before input is re-enabled.
Useful? React with 👍 / 👎.
| logger.warning( | ||
| "agent.journal.latest_event_rowid_failed", error=str(exc) | ||
| ) | ||
| return 0 |
There was a problem hiding this comment.
Avoid replaying history when the seed read fails
When the initial high-water read hits a transient SQLite error on an existing journal, returning 0 is indistinguishable from an empty DB to run_journal_subagent_tail, so the tail starts at row 0 and replays historical SubagentSpawned rows. That violates the forward-only contract described for this feature and can resurrect old orphaned subagents as running in /admin/subagents; errors should leave the tail unseeded/retry instead of rewinding to the beginning.
Useful? React with 👍 / 👎.
| # rows are real history now; replay them. | ||
| if ( | ||
| str(turn_row.get("status") or "") == "in_progress" | ||
| and raw_turn_id == newest_raw_turn_id |
There was a problem hiding this comment.
Do not replay concurrently live older turns
When the same session has two simultaneous in_progress turns (for example two browser tabs or API clients; begin_turn does not enforce a per-session singleton), resumeInFlight can only attach to the latest turn, but this condition now treats the older still-running turn as settled history and replays its partial user/assistant/tool rows. That brings back the double-render/stale-transcript problem for concurrent runs, and the later completion refetch can replace those partial rows again; skip all live in_progress turns unless they are first marked abandoned/errored.
Useful? React with 👍 / 👎.
…d + resume user bubble + crashed-turn replay (v1.29.1) Closes the last three #108 items: - C1 (item 2): process-wide journal tail. New gateway background task run_journal_subagent_tail() feeds Subagent* lifecycle events from the journal into LiveSubagentRegistry continuously — previously the only cross-process feed was the per-session SSE poll, so /admin/subagents stayed empty in grpc_agent mode unless that session's chat page was open. Journal gains latest_event_rowid() + load_subagent_events_since() (rowid-cursored, bounded against a snapshotted MAX(rowid) so a mid-scan commit can't be skipped; Postgres stubs alongside the rest of the event plane). Forward-only from the boot high-water mark; idempotent with the SSE-poll feed; cancelled via the lifespan background-task teardown. - C2 (item 3): resume user bubble. resumeInFlight now renders the user's message (turn user_text_preview) above the live streaming bubble — the settled transcript excludes the in-progress turn wholesale, so the assistant previously streamed a reply to nothing. Washed out by the authoritative transcript refetch on finalize; guarded against double-insert. - C3 (L-103): crashed-turn replay. _replay_from_journal skipped EVERY in_progress turn; a turn that crashed mid-run and was followed by newer turns lost its user message + partial answer from the thread forever. Skip is now scoped to the newest turn of the first page (the only turn that can legitimately be live); older in_progress rows replay as history. Tests: 4 backend tail tests, 3 tail-loop tests (incl. error-survival + legacy-journal no-op), 1 replay regression, 2 UI resume tests. Full gate green: ruff, mypy (594 files), pytest 6679 passed, UI tsc/lint/build, lint-imports, proto gen clean.
dc89a4a to
93593dc
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Wave C of the gap-close campaign — closes the last three #108 items. Independent branch off
main(parallel to #114–#117; the only expected merge conflict is the trivial version/CHANGELOG bump).C1 (#108 item 2) — process-wide subagent journal tail
Problem: in
grpc_agentmode subagent lifecycle events reach the gateway only through the journal, and the only journal→LiveSubagentRegistryfeed point was the per-session SSE poll. So/admin/subagentsstayed empty unless someone happened to have that exact session's chat page open.Fix: a gateway background task (
run_journal_subagent_tail) tailsSubagent*events into the registry continuously.latest_event_rowid()+load_subagent_events_since()(Protocol + SQLite impl + facade; Postgres stubs alongside the rest of the event plane, same degradation contract).MAX(rowid)snapshotted first, so an event committed between the two statements can never be skipped. A full page advances only to its last row; a short page jumps to the snapshot so trailing non-subagent rows are never rescanned.backgroundteardown.C2 (#108 item 3) — resume user bubble
Problem: returning to a chat with an in-flight turn,
resumeInFlightrebuilt only the assistant bubble; the settled transcript excludes the in-progress turn wholesale, so the assistant visibly streamed a reply to… nothing.Fix: the resume path renders a user bubble from the turn's
user_text_preview(already returned byGET /admin/sessions/:key/turns). Guarded against double-insert; washed out by the authoritative transcript refetch when the turn finalizes.C3 (L-103) — crashed in-progress turns vanished from the transcript
Problem:
_replay_from_journalskipped everyin_progressturn. For a turn that crashed mid-run (never completed) and was followed by newer turns, the user's message and any partial answer disappeared from the thread forever.Fix: the skip is scoped to the one turn that can legitimately still be live — the newest turn of the first page (a session runs one turn at a time;
before_turn_idpages are by construction older). Olderin_progressrows are crash artifacts and replay as real history. The newest-turn skip + the existing double-render rationale are unchanged, and with C2 the newest-stale case now at least renders via the resume bubble instead of vanishing.Tests
test_agent_journal_subagent_tail.py— 4 backend tests: fresh-DB rowid, event-type filtering + cursor advance past non-subagent rows, mid-stream resume, limit paging.gateway/observability/test_journal_tail.py— 3 loop tests: feed + cancel, error survival, legacy-journal (no tail surface) clean exit.test_sessions_replay_in_progress.py— new regression: older crashed turn included in chronological order; both existing exclusion tests still green.use-chat-stream-resume.test.ts— user bubble rendered from preview (+ no-dup on double resume), skipped when preview empty.Gate
ruff ✅ · mypy (594 files) ✅ · pytest 6679 passed, 4 skipped ✅ · UI tsc/lint/build ✅ · lint-imports ✅ · proto gen clean ✅
v1.29.1 + bilingual CHANGELOG entry.