fix(agent): clock the AG-UI encoder so headless runs carry elapsedMs - #3502
Conversation
Persisted run events for hosted and scheduled runs have never carried `elapsedMs`. The emitter is `AgUiBrowserEncoderState`: the runtime streams its AG-UI events as SSE to veryfront-api, which persists them into `agent_run_event` verbatim (the payload schemas are `.passthrough()`, so added fields survive). That state holds run-scoped `stepCount` and `activeStepName` but had no clock at all. The two symptoms fingerprint the encoder, and they are independent: step names advance + no elapsedMs -> one shared encoder, unclocked step names repeat step-1 -> a fresh encoder per chunk Production shows the first. `run-chunk-mirror` has been clocked since #3483, so it would stamp; the hosted adapter fixed in #3497 rebuilds its encoder per chunk, so it would repeat `step-1`. Neither matched, which is what identified this third emitter. Despite its name it is the durable path for headless runs -- there is no browser attached to a scheduled run, which is what veryfront-issue-inbox#413 already flags. That name misled both #3483 and #3497. Clocked by default, rather than an opt-in clock. This state is built at four composition roots (`ag-ui-sse`, `runtime-event-encoder`, `chunk-encoder-bridge`, and through `browser-chunk-encoder`), and an opt-in clock only has to be missed at one of them to lose `elapsedMs` for every run -- which is precisely how it was missed twice. Callers that assert exact payloads pass `nowMs: null`. Tests cover both halves: a deterministic clock stamps run-relative elapsed that accrues across events without disturbing the rest of the payload, and a separate test pins that the default is clocked, which fails if the default ever reverts to opt-in.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughAG-UI encoders now support optional run-relative ChangesAG-UI timing support
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant EncoderState
participant RuntimeEventMapper
participant AGUIEvent
EncoderState->>RuntimeEventMapper: provide run start and clock
RuntimeEventMapper->>AGUIEvent: emit payload with elapsedMs
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 395dcf7768
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export function createStreamTransformState( | ||
| options: AgUiBrowserEncoderStateOptions = {}, | ||
| ): StreamTransformState { | ||
| return createAgUiBrowserEncoderState(options); |
There was a problem hiding this comment.
In the headless path checked in src/internal-agents/run-stream.ts, this state feeds mapped events into formatAgUiEvent, but that formatter parses known events through the closed object schemas in buildAgUiEventPayloadSchemas. Those schemas do not declare elapsedMs, and the registered schema validator strips unknown object keys by default, so the newly stamped field is removed before the SSE reaches persistence. Hosted and scheduled runs therefore still store events without elapsedMs; add the field to the wire schemas or make them passthrough, and cover the formatted SSE output.
Useful? React with 👍 / 👎.
Root cause
Persisted run events for hosted and scheduled runs have never carried
elapsedMs. The emitter isAgUiBrowserEncoderState(src/agent/ag-ui/browser-encoder.ts): the runtime streams its AG-UI events as SSE to veryfront-api, which persists them intoagent_run_eventverbatim. That state holds run-scopedstepCountandactiveStepNamebut had no clock at all.Confirmed the field survives ingest: the API's AG-UI payload schemas are
.passthrough(), andappend-external-agent-run-events.tsstorespayload: event. So a framework-side stamp reaches the column with no API change.How it was identified
The two symptoms are independent, and together they fingerprint the encoder:
elapsedMsstep-1Production (runs at 10:26, 10:31, 11:00 on 2026-08-09, read from the prod primary) shows advance + absent:
payload ? 'elapsedMs'false on every event,stepNamerunning step-1 through step-5, each once asSTEP_STARTEDand once asSTEP_FINISHED.run-chunk-mirrorhas been clocked since #3483, so it would stamp. The hosted adapter fixed in #3497 rebuilds its encoder per chunk, so it would repeatstep-1. Neither matched — which is what pointed at a third emitter.Ruled out, so this is not a stale deploy: v0.1.1220 genuinely contains the clocked chunk mirror, and the pod that executed the 11:00 run (
10.192.6.132,veryfront-server-6f45c6d8c6-pct7c) reportsVERYFRONT_VERSION 0.1.1220.Despite its name, this encoder is the durable path for headless runs — there is no browser attached to a scheduled run. That is exactly what veryfront-issue-inbox#413 flags, and the name misled two prior fixes: #3483 and #3497.
Why clocked by default
This state is built at four composition roots —
ag-ui-sse,runtime-event-encoder,chunk-encoder-bridge, and throughbrowser-chunk-encoder. An opt-in clock only has to be missed at one of them to loseelapsedMsfor every run, which is precisely how it was missed twice. So the default stamps, and callers asserting exact payloads passnowMs: nullexplicitly.Tests
the default state must stamp elapsedMs— verified.Verification
src/agent,src/internal-agents,src/chat,src/runtime,src/server: 1407 passed. The singleagent-stream.handlerfailure is pre-existing — confirmed by stashing this change and reproducing it identically on a clean tree.deno task typecheckclean;docs/api-referenceregenerated with the CI-pinned Deno 2.7.7.Follow-up
#3497 remains a valid fix for a real per-chunk encoder defect on the top-level hosted adapter; it is simply not the path production takes. Its description has been corrected on the record.
Summary by CodeRabbit
New Features
elapsedMsvalues.Bug Fixes
Documentation