feat(console): reactive traces + session-events live streams - #224
Merged
Conversation
Subscribe each chat session directly to its `agent::events` stream via a scoped engine stream trigger (group_id = session_id), consumed in the console (session-events-live.ts), replacing the harness fanout hop (`ui::subscribe` → per-browser `ui::session::event` push). Drops the harness agent-events pump. The handler is `iii::`-prefixed so its delivery spans are engine-internal — hidden from the traces view and skipped by the trigger loop-break, matching `traces-live.ts`. Also collapse trace search results to one row per trace (`dedupeToTraceRoots`): an operation search uses `search_all_spans`, which returns every span of a matching trace, and the flat list is one-row-per- trace.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR transitions the console from harness-fanned agent events to direct per-session ChangesFrontend Agent Events Subscription and Traces Live Refresh
Harness Agent Events Pump Removal
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
console/web/src/lib/backend/session-events-live.test.ts (1)
65-104: ⚡ Quick winAdd a regression test for trigger-registration failure cleanup.
Please add a case where
on()succeeds butregisterTrigger()throws, and assert the handler unregister (offHandler) is called. This locks in rollback behavior and prevents future listener leaks.Also applies to: 146-154
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@console/web/src/lib/backend/session-events-live.test.ts` around lines 65 - 104, Add a test to cover the regression where on() registers a handler but registerTrigger() throws: modify the fakeClient test helper or a new test case to have registerTrigger reject/throw (e.g., make registerTrigger = vi.fn(() => { throw new Error(...) }) ), call the code path that invokes client.on and client.registerTrigger, then assert offHandler (the function returned by on) is invoked and triggerUnregister is not leaked; reference the fakeClient helper, its on/registerTrigger/offHandler/triggerUnregister symbols and the code path that calls client.registerTrigger to ensure cleanup is performed when registration fails (also add the same test around the other location mentioned).console/web/src/lib/traces-live.test.ts (1)
93-152: 💤 Low valueConsider adding a test for the hidden-tab check in
makeTracesChangedHandler.The handler skips invalidation when
document.visibilityState === 'hidden', but this branch isn't directly tested. WhilestartTracesSubscriptiontests cover visibility indirectly, a direct unit test would improve coverage.it('does nothing while the tab is hidden', () => { const originalDoc = globalThis.document try { Object.defineProperty(globalThis, 'document', { value: { visibilityState: 'hidden' }, configurable: true, }) const { client, invalidateQueries } = fakeQueryClient() const handler = makeTracesChangedHandler(client, { current: false }) handler() expect(invalidateQueries).not.toHaveBeenCalled() } finally { Object.defineProperty(globalThis, 'document', { value: originalDoc, configurable: true, }) } })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@console/web/src/lib/traces-live.test.ts` around lines 93 - 152, Add a unit test for makeTracesChangedHandler to cover the branch where document.visibilityState === 'hidden': create a test that temporarily stubs globalThis.document to an object with visibilityState: 'hidden', call makeTracesChangedHandler(client, { current: false }) using fakeQueryClient(), invoke the handler, assert invalidateQueries is not called (and onExtra isn't invoked if provided), and finally restore the original document in a finally block to avoid test pollution; reference makeTracesChangedHandler and fakeQueryClient and assert against the fake invalidateQueries mock to locate where to add the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@console/web/src/lib/backend/session-events-live.ts`:
- Around line 78-90: The handler registered via client.on(SESSION_EVENT_FN, ...)
can leak if client.registerTrigger(...) throws; wrap the call to
client.registerTrigger(...) in a try/catch and, on any error, call the
unregister function returned by client.on (off) to remove the listener before
rethrowing the error; reference the existing symbols SESSION_EVENT_FN,
client.on, off, functionId, client.registerTrigger, extractSessionEvent and
onEvent so you locate the registration block and ensure off() is invoked in the
catch path to roll back the partial subscription.
---
Nitpick comments:
In `@console/web/src/lib/backend/session-events-live.test.ts`:
- Around line 65-104: Add a test to cover the regression where on() registers a
handler but registerTrigger() throws: modify the fakeClient test helper or a new
test case to have registerTrigger reject/throw (e.g., make registerTrigger =
vi.fn(() => { throw new Error(...) }) ), call the code path that invokes
client.on and client.registerTrigger, then assert offHandler (the function
returned by on) is invoked and triggerUnregister is not leaked; reference the
fakeClient helper, its on/registerTrigger/offHandler/triggerUnregister symbols
and the code path that calls client.registerTrigger to ensure cleanup is
performed when registration fails (also add the same test around the other
location mentioned).
In `@console/web/src/lib/traces-live.test.ts`:
- Around line 93-152: Add a unit test for makeTracesChangedHandler to cover the
branch where document.visibilityState === 'hidden': create a test that
temporarily stubs globalThis.document to an object with visibilityState:
'hidden', call makeTracesChangedHandler(client, { current: false }) using
fakeQueryClient(), invoke the handler, assert invalidateQueries is not called
(and onExtra isn't invoked if provided), and finally restore the original
document in a finally block to avoid test pollution; reference
makeTracesChangedHandler and fakeQueryClient and assert against the fake
invalidateQueries mock to locate where to add the test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ca17ce6b-3353-436f-86a3-0d6c7d747ffd
📒 Files selected for processing (24)
console/web/src/lib/backend/real.tsconsole/web/src/lib/backend/session-events-live.test.tsconsole/web/src/lib/backend/session-events-live.tsconsole/web/src/lib/backend/types.tsconsole/web/src/lib/iii-client.tsconsole/web/src/lib/traces-live.test.tsconsole/web/src/lib/traces-live.tsconsole/web/src/pages/Traces/components/TraceGroupsView.tsxconsole/web/src/pages/Traces/hooks/useTraceData.tsconsole/web/src/pages/Traces/hooks/useTraceGroups.tsconsole/web/src/pages/Traces/index.tsxconsole/web/src/pages/Traces/lib/spanLabel.test.tsconsole/web/src/pages/Traces/lib/spanLabel.tsconsole/web/src/pages/Traces/lib/spanTree.test.tsconsole/web/src/pages/Traces/lib/traceListItem.test.tsconsole/web/src/pages/Traces/lib/traceListItem.tsconsole/web/src/types/iii-agent-event.tsharness/README.mdharness/docs/architecture.mdharness/docs/workers/harness.mdharness/src/harness/fanout/agent-events.tsharness/src/harness/fanout/index.tsharness/src/harness/main.tsharness/src/index.ts
💤 Files with no reviewable changes (4)
- harness/src/harness/fanout/agent-events.ts
- harness/src/harness/fanout/index.ts
- console/web/src/pages/Traces/components/TraceGroupsView.tsx
- console/web/src/types/iii-agent-event.ts
Comment on lines
+78
to
+90
| const off = client.on(SESSION_EVENT_FN, (frame: unknown) => { | ||
| const event = extractSessionEvent(frame, sessionId) | ||
| if (event) onEvent(event) | ||
| }) | ||
|
|
||
| // `on()` registers under `<fn>::<browserId>`; the trigger must target that id. | ||
| const functionId = `${SESSION_EVENT_FN}::${client.browserId}` | ||
| const offTrigger = client.registerTrigger({ | ||
| type: 'stream', | ||
| function_id: functionId, | ||
| config: { stream_name: EVENTS_STREAM, group_id: sessionId }, | ||
| }) | ||
|
|
There was a problem hiding this comment.
Handle partial subscription failure rollback.
If client.registerTrigger(...) throws after client.on(...) succeeds (Line 78 → Line 85), the handler remains registered. This leaks listeners and can cause duplicate delivery behavior on later retries. Unregister off in a catch before rethrowing.
Suggested fix
export function startSessionEventsSubscription(
client: Pick<IiiClient, 'browserId' | 'on' | 'registerTrigger'>,
sessionId: string,
onEvent: (event: AgentEvent) => void,
): () => void {
const off = client.on(SESSION_EVENT_FN, (frame: unknown) => {
const event = extractSessionEvent(frame, sessionId)
if (event) onEvent(event)
})
// `on()` registers under `<fn>::<browserId>`; the trigger must target that id.
const functionId = `${SESSION_EVENT_FN}::${client.browserId}`
- const offTrigger = client.registerTrigger({
- type: 'stream',
- function_id: functionId,
- config: { stream_name: EVENTS_STREAM, group_id: sessionId },
- })
+ let offTrigger: (() => void) | null = null
+ try {
+ offTrigger = client.registerTrigger({
+ type: 'stream',
+ function_id: functionId,
+ config: { stream_name: EVENTS_STREAM, group_id: sessionId },
+ })
+ } catch (err) {
+ off()
+ throw err
+ }
return () => {
off()
try {
- offTrigger()
+ offTrigger?.()
} catch {
// SDK already disposed; nothing to do.
}
}
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@console/web/src/lib/backend/session-events-live.ts` around lines 78 - 90, The
handler registered via client.on(SESSION_EVENT_FN, ...) can leak if
client.registerTrigger(...) throws; wrap the call to client.registerTrigger(...)
in a try/catch and, on any error, call the unregister function returned by
client.on (off) to remove the listener before rethrowing the error; reference
the existing symbols SESSION_EVENT_FN, client.on, off, functionId,
client.registerTrigger, extractSessionEvent and onEvent so you locate the
registration block and ensure off() is invoked in the catch path to roll back
the partial subscription.
Resolve conflicts: - harness/README.md: take main's rewritten README (our one-line edit to the old worker table is obsoleted by the rewrite; harness 'UI fanout' stays accurate after dropping the agent-events pump). - harness/src/harness/fanout/index.ts: combine both sides — keep this branch's removal of the agent-events pump and add main's new models-changed pump.
Contributor
skill-check — worker0 verified, 14 skipped (no docs/).
Four for four. Nicely done. |
andersonleal
approved these changes
Jun 4, 2026
This was referenced Jul 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Makes the console reactive by subscribing directly to engine streams/triggers, removing the harness fanout relays:
tracetrigger instead of 3 s polling.agent::events(a session-scoped stream trigger) instead of the harnessui::session::eventfanout.Details
traces-live.tsregisters aniii::console::traces_changedhandler bound to atracetrigger;useTraceData/useTraceGroupsdroprefetchInterval; the open trace's detail tree reloads silently on each tick (no flicker, no reselect); auto-pause-on-select removed so the list stays live.session-events-live.tsbinds agroup_id-scopedagent::eventsstream trigger per session, delivered straight to the browser; the harnessagent-eventsfanout pump (spawnAgentEventsPump,ui::subscribe→ per-browser push) is removed.iii::prefix — both handlers areiii::-prefixed, so their delivery spans are taggediii.function.kind=internal(hidden by the defaultinclude_internal:falsequery) and skipped by the engine trigger loop-break — notraces_changed/session_eventspam in the trace list.dedupeToTraceRootscollapsessearch_all_spansresults to one row per trace, so an operation search (e.g.harness::trigger) returns the trace, not its whole span dump.isEngineRoutingSpannow requires afunction_idattribute, so a worker's owncall <fn>span (nofunction_id) is no longer swept up with the engine'scall <fn>routing spans.Depends on
tracetrigger type iii#1753 — thetracetrigger that drives the traces live-refresh. (The session-events stream uses existing scoped stream triggers, no engine change.)Test plan
pnpm test(console) — 519 pass, incl.traces-live,session-events-live,dedupeToTraceRoots, engine-routingpnpm typecheck— clean (excl. pre-existing storybook resolution errors)Summary by CodeRabbit
New Features
Bug Fixes
Chores