fix(agent): let elapsedMs reach the wire through the AG-UI SSE schemas - #3509
Conversation
`elapsedMs` has been stamped since #3502 but never reached storage. The AG-UI SSE payload schemas in `ag-ui-sse.ts` are an allow-list, and `formatAgUiEvent` serializes `schema.parse(payload)` -- the parse result, not the input. Any field the schema does not declare is dropped between the encoder and the wire: formatAgUiEvent("StepStarted", { stepName: "step-1", elapsedMs: 42 }) -> 'event: StepStarted\ndata: {"stepName":"step-1"}\n\n' That also explains the confusing evidence this produced: `stepName` survived because it is on the allow-list, `elapsedMs` never was. Measured on production v0.1.1222, the 14:00 tick still showed no `elapsedMs` on any event despite running on pods that report 0.1.1222. Declare `elapsedMs` once via a `withElapsed` helper applied to all 18 event schemas, rather than widening them to passthrough. The allow-list is a deliberate wire contract -- this stream is persisted verbatim into `agent_run_event` -- so it stays closed to everything else. Two tests pin both halves, and both fail without this change: the field reaches the wire, and an undeclared field still does not. The second drives the real production encoder root (`createStreamTransformState`) through `formatAgUiEvent`, which is the boundary nothing covered before -- `browser-encoder` and `ag-ui-sse` were each tested in isolation, so the field being dropped between them was invisible. Follow-up agreed separately: add an absolute `emittedAt` as the durable primitive and derive elapsed on read. `elapsedMs` is measured from encoder construction, so its meaning depends on which encoder instance produced it -- the anchor ambiguity behind #3483, #3497 and #3502 all landing in the wrong place. An absolute timestamp has no anchor, and additionally makes ingest lag measurable as `created_at - emittedAt`.
|
Warning Review limit reached
Next review available in: 4 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Comment |
elapsedMshas been stamped since #3502 but still never reached storage. This is why.Root cause, with a reproduction
The AG-UI SSE payload schemas in
src/internal-agents/ag-ui-sse.tsare an allow-list, andformatAgUiEventserializesschema.parse(payload)— the parse result, not the input. Anything the schema does not declare is dropped between the encoder and the wire:That also explains the evidence that made this so confusing to chase:
stepNamesurvived because it is on the allow-list;elapsedMsnever was. Both symptoms had the same single cause.Measured on production v0.1.1222: the 14:00 tick still showed no
elapsedMson any event, on pods verified to report0.1.1222(10.192.9.132,10.192.6.137, artifact20260809132722-48ee1f3b77bb). So this was not a rollout-timing artifact.Fix
Declare
elapsedMsonce through awithElapsedhelper applied to all 18 event schemas, rather than widening them to passthrough. The allow-list is a deliberate wire contract — this stream is persisted verbatim intoagent_run_event— so it stays closed to everything else.Tests
Two, both verified to fail without the change:
elapsedMsreaches the wire, and an undeclared field still does not — pinning both halves, so a future "fix" that just makes the schemas passthrough would not satisfy it.createStreamTransformState()→mapRuntimeEventToAgUi→formatAgUiEvent.Reverting the fix reproduces the exact production symptom:
The gap that let this through:
browser-encoderandag-ui-ssewere each well covered in isolation, so a field vanishing between them was invisible to both suites. The second test exists specifically to cover that boundary.Follow-up, agreed separately
Add an absolute
emittedAtas the durable primitive and derive elapsed on read.elapsedMsis measured from encoder construction, so its meaning depends on which encoder instance produced it and when — the anchor ambiguity behind #3483, #3497 and #3502 each landing in the wrong place. An absolute timestamp has no anchor to get wrong, and additionally makes ingest lag measurable ascreated_at − emittedAt, which is the gap that misled the original trace analysis.Note on the pre-push gate
Two pushes were rejected by timing-sensitive tests before this landed —
string-renderer(5ms SSR deadline) andproject-env/fetcher(timeout fallback). Both pass in isolation and are unrelated to SSE schemas; the machine was loaded by concurrent suites. Flagging rather than hiding it.