feat: enhance subagent event visualization with multi-source ownership, filtering, and timeline analysis#8
Conversation
There was a problem hiding this comment.
Pull request overview
Enhances the session detail subagent event visualization by introducing a unified “subagent ownership” model across sources, adding a subagent filter UI, and surfacing per-subagent activity stats.
Changes:
- Add subagent selector dropdown + activity badge, and make subagent owner tags clickable to filter events.
- Implement multi-source subagent ownership attribution (Copilot CLI, VS Code, Claude) including temporal attribution between subagent boundaries.
- Add unit + e2e tests intended to validate subagent ownership and filtering behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| views/session-vue.ejs | Adds styling for the new subagent selector UI and interactive subagent owner tags. |
| src/frontend/session-detail.js | Implements subagent ownership attribution, filtering, selector state, and activity badge computation. |
| tests/subagent-view.test.js | Adds unit tests for subagent ownership/filtering logic (currently re-implemented in tests). |
| tests/e2e/subagent-view.spec.js | Adds Playwright coverage for dropdown visibility, filtering, and badge behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Token usage for the currently selected subagent (computed from events) | ||
| const subagentTokenUsage = computed(() => { | ||
| if (!selectedSubagent.value) return null; | ||
| const { ownerMap, subagentInfo } = subagentOwnership.value; | ||
| const tcid = selectedSubagent.value; | ||
| if (!subagentInfo.has(tcid)) return null; |
There was a problem hiding this comment.
subagentTokenUsage is named as though it reports tokens, but it currently returns { eventCount, durationMs } and the UI badge renders “X events · duration”. Either compute actual token usage for the selected subagent or rename this computed value (and related UI copy) to reflect what it really represents (activity/coverage stats).
__tests__/e2e/subagent-view.spec.js
Outdated
| const data = await eventsResponse.json(); | ||
| const events = Array.isArray(data) ? data : (data.events || []); | ||
| const hasSubagentStarted = events.some(e => e.type === 'subagent.started'); | ||
| const hasVsCodeSubagent = events.some(e => e.type === 'assistant.message' && e.data?.subAgentId); |
There was a problem hiding this comment.
The VS Code session detection in beforeAll checks e.data?.subAgentId only. But the dropdown is populated from subAgentName + subAgentId, so this can select a session that won’t actually render the dropdown, reducing test signal. Consider requiring subAgentName as well when picking SUBAGENT_SESSION_ID for the VS Code path.
| const hasVsCodeSubagent = events.some(e => e.type === 'assistant.message' && e.data?.subAgentId); | |
| const hasVsCodeSubagent = events.some( | |
| e => e.type === 'assistant.message' && e.data?.subAgentId && e.data?.subAgentName | |
| ); |
| test('should not show subagent dropdown for sessions without subagents', async ({ page }) => { | ||
| // Use a session that likely has no subagents (first session as fallback) | ||
| const testId = SUBAGENT_SESSION_ID ? SESSION_ID : SESSION_ID; | ||
| test.skip(SUBAGENT_SESSION_ID === SESSION_ID, 'Cannot test - first session has subagents'); | ||
|
|
There was a problem hiding this comment.
The “should not show subagent dropdown for sessions without subagents” test doesn’t reliably validate the negative case: testId is always SESSION_ID, and if the dropdown exists the test makes no assertion (effectively a no-op). To make this meaningful, select a session that is verified to have no subagent markers (no subagent.*, no _subagent, and no subAgentName/subAgentId) and then assert the dropdown count is 0.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/frontend/session-detail.js
Outdated
| let activeSubagent = null; | ||
| for (const ev of sorted) { | ||
| if (ev.type === 'subagent.started' && ev.data?.toolCallId && subagentInfo.has(ev.data.toolCallId)) { | ||
| activeSubagent = ev.data.toolCallId; | ||
| } else if ((ev.type === 'subagent.completed' || ev.type === 'subagent.failed') && ev.data?.toolCallId === activeSubagent) { | ||
| activeSubagent = null; | ||
| } else if (activeSubagent && !ownerMap.has(ev.stableId)) { | ||
| ownerMap.set(ev.stableId, activeSubagent); |
There was a problem hiding this comment.
Temporal attribution uses a single activeSubagent and will mis-attribute events if subagent spans overlap or nest (e.g., a second subagent.started occurs before the first completes). The time analysis view already uses a stack to match started/completed/failed by toolCallId; consider switching this attribution to a stack (top-of-stack owner) and popping by matching toolCallId to avoid incorrect ownership in nested/overlapping cases.
| let activeSubagent = null; | |
| for (const ev of sorted) { | |
| if (ev.type === 'subagent.started' && ev.data?.toolCallId && subagentInfo.has(ev.data.toolCallId)) { | |
| activeSubagent = ev.data.toolCallId; | |
| } else if ((ev.type === 'subagent.completed' || ev.type === 'subagent.failed') && ev.data?.toolCallId === activeSubagent) { | |
| activeSubagent = null; | |
| } else if (activeSubagent && !ownerMap.has(ev.stableId)) { | |
| ownerMap.set(ev.stableId, activeSubagent); | |
| // Use a stack to correctly handle nested or overlapping subagent spans. | |
| const subagentStack = []; | |
| for (const ev of sorted) { | |
| if (ev.type === 'subagent.started' && ev.data?.toolCallId && subagentInfo.has(ev.data.toolCallId)) { | |
| subagentStack.push(ev.data.toolCallId); | |
| } else if (ev.type === 'subagent.completed' || ev.type === 'subagent.failed') { | |
| const tcid = ev.data?.toolCallId; | |
| if (tcid) { | |
| const idx = subagentStack.lastIndexOf(tcid); | |
| if (idx !== -1) { | |
| subagentStack.splice(idx, 1); | |
| } | |
| } | |
| } else if (subagentStack.length > 0 && !ownerMap.has(ev.stableId)) { | |
| const activeSubagent = subagentStack[subagentStack.length - 1]; | |
| if (activeSubagent) { | |
| ownerMap.set(ev.stableId, activeSubagent); | |
| } |
| test('should not show subagent dropdown for sessions without subagents', async ({ page }) => { | ||
| // Use a session that likely has no subagents (first session as fallback) | ||
| const testId = SUBAGENT_SESSION_ID ? SESSION_ID : SESSION_ID; | ||
| test.skip(SUBAGENT_SESSION_ID === SESSION_ID, 'Cannot test - first session has subagents'); | ||
|
|
||
| await page.goto(`/session/${testId}`); | ||
| await page.waitForSelector('.main-layout', { timeout: 10000 }); | ||
|
|
||
| await page.waitForFunction(() => { | ||
| const loadingEl = document.querySelector('.loading-message'); | ||
| return loadingEl === null || window.getComputedStyle(loadingEl).display === 'none'; | ||
| }, { timeout: 30000 }); | ||
|
|
||
| // Wait for events to render | ||
| await page.waitForTimeout(2000); | ||
|
|
||
| // If no subagents, dropdown should not be visible | ||
| const dropdown = page.locator('.subagent-dropdown'); | ||
| const count = await dropdown.count(); | ||
| // Either not present, or present because session has subagents (both ok) | ||
| if (count === 0) { | ||
| expect(count).toBe(0); | ||
| } | ||
| }); |
There was a problem hiding this comment.
This test doesn’t actually verify the "no subagents" behavior: testId is always SESSION_ID, and when count > 0 the test makes no assertion (so it will pass even if the dropdown is shown). Consider explicitly selecting a session that is verified (via /events) to have no subagent markers, and assert dropdown.count() is 0 (or toBeHidden) for that session.
| for (const ev of eventsWithTimestamps) { | ||
| const isSubagentDivider = (ev.type === 'subagent.started' || ev.type === 'subagent.completed') && ev.data?.toolCallId === tcid; | ||
| const isOwned = ownerMap.get(ev.stableId) === tcid; | ||
| if (isSubagentDivider || isOwned) { | ||
| eventCount++; | ||
| const t = new Date(ev.timestamp).getTime(); | ||
| if (!startTime || t < startTime) startTime = t; | ||
| if (!endTime || t > endTime) endTime = t; | ||
| } |
There was a problem hiding this comment.
The simulated token-usage loop diverges from the real implementation (it ignores subagent.failed dividers and doesn’t include _subagent / VS Code subAgentId ownership). This makes the test less effective at catching regressions in the actual computation; align the simulation with the production conditions you added in subagentTokenUsage.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // 1b. VS Code source: collect subagent names from assistant.message data.subAgentName | ||
| // (VS Code does not emit subagent.started events; subagent identity is on the message) | ||
| for (const ev of events) { | ||
| if (ev.type === 'assistant.message' && ev.data?.subAgentName && ev.data?.subAgentId) { | ||
| const sid = ev.data.subAgentId; | ||
| if (!subagentInfo.has(sid)) { | ||
| subagentInfo.set(sid, { | ||
| name: ev.data.subAgentName, | ||
| colorIndex: colorIdx++ | ||
| }); | ||
| } | ||
| // Directly map this event to its subagent (vscode has no parentToolCallId) | ||
| ownerMap.set(ev.stableId, sid); | ||
| } |
There was a problem hiding this comment.
VS Code subagent detection currently requires both data.subAgentName and data.subAgentId. The backend can emit events with a subAgentId but no name (e.g., synthetic/grouped events), which would prevent the subagent from appearing in the dropdown and from being attributed in ownerMap. Consider collecting subagents whenever subAgentId is present, using subAgentName || subAgentId || 'SubAgent' as the display name, and still mapping ownerMap for those events.
| <span | ||
| v-if="getSubagentInfo(item)" | ||
| class="subagent-owner-tag" | ||
| :style="{ color: getSubagentColor(item), borderColor: getSubagentColor(item) }" | ||
| >{{ getSubagentInfo(item).name }}</span> | ||
| :style="{ '--subagent-color': getSubagentColor(item) || '#58a6ff', '--subagent-hover-bg': ((getSubagentColor(item) || '#58a6ff') + '26') }" | ||
| :title="'Filter to ' + getSubagentInfo(item).name" | ||
| @click.stop="selectSubagent(getSubagentInfo(item).toolCallId)" | ||
| >🤖 {{ getSubagentInfo(item).name }}</span> |
There was a problem hiding this comment.
.subagent-owner-tag is a clickable <span> (it changes filters) but it is not keyboard-focusable and has no semantic role. This makes the filter action inaccessible to keyboard/screen-reader users. Use a <button type="button"> (or add role="button", tabindex="0", and Enter/Space key handlers) and keep the existing styling via CSS classes.
| const { test, expect } = require('./fixtures'); | ||
|
|
||
| test.describe('Subagent View', () => { | ||
| let SESSION_ID; | ||
| let SUBAGENT_SESSION_ID; | ||
|
|
||
| const getWithRetry = async (request, url, attempts = 3) => { | ||
| let lastError; | ||
| for (let attempt = 1; attempt <= attempts; attempt++) { | ||
| try { | ||
| return await request.get(url); | ||
| } catch (error) { | ||
| lastError = error; | ||
| if (attempt < attempts) { | ||
| await new Promise(resolve => setTimeout(resolve, 500 * attempt)); | ||
| } | ||
| } | ||
| } | ||
| throw lastError; | ||
| }; | ||
|
|
||
| test.beforeAll(async ({ request }) => { | ||
| const response = await getWithRetry(request, '/api/sessions'); | ||
| const sessions = await response.json(); | ||
| if (sessions.length === 0) { |
There was a problem hiding this comment.
This spec re-implements a getWithRetry helper instead of using the shared getJsonWithRetry/getSessionsWithRetry utilities already added to __tests__/e2e/fixtures.js, and it also doesn't check response.ok() before calling .json(). Reusing the shared helper would reduce duplication and make failures clearer/consistent (status/validation errors).
| // Suppress harmless virtual scroller errors | ||
| test.beforeEach(async ({ page }) => { | ||
| page.on('pageerror', error => { | ||
| const message = error.message; | ||
| if (message.includes('ResizeObserver') || | ||
| message.includes("Cannot read properties of undefined (reading 'has')")) { | ||
| return; | ||
| } | ||
| throw error; | ||
| }); |
There was a problem hiding this comment.
The pageerror handler suppresses any error whose message contains Cannot read properties of undefined (reading 'has'). This pattern is broad and can hide real regressions in the subagent ownership logic (Map/Set usage) or other runtime issues. Consider narrowing the suppression to a specific known stack/source (or asserting it never happens) so the test suite still catches unexpected frontend errors.
| const { test, expect, getSessionsWithRetry } = require('./fixtures'); | ||
|
|
||
| test.describe('API Endpoints', () => { | ||
| test('GET /api/sessions should return JSON array', async ({ request }) => { |
There was a problem hiding this comment.
Switching to getSessionsWithRetry() removed the explicit assertions on /api/sessions response metadata (status/content-type). getSessionsWithRetry does validate response.ok(), but it doesn’t verify JSON content-type; if you still want that contract covered, consider adding an optional content-type check in the helper or keeping a lightweight header assertion in this test.
| test('GET /api/sessions should return JSON array', async ({ request }) => { | |
| test('GET /api/sessions should return JSON array', async ({ request }) => { | |
| const response = await request.get('/api/sessions'); | |
| expect(response.ok()).toBeTruthy(); | |
| expect(response.headers()['content-type']).toContain('application/json'); |
There was a problem hiding this comment.
@haochenghuang
For now, we have:
- three types of events filtering:
a. by type
b. by subagent name (which is added by this pr)
c. by keyword search - one type of quick jump:
a. jump between turns
I think it's better to let LLM to help on the design to consolidate all the elements into one box, this is quick design by my agent:
┌───────────────────────────┐
│ ☰ │ 🔍 [搜索/命令________________________] │ ▼▲ │
│ │ │ │
│ │ 快捷 chips: [All ✓] [user] [assistant] [+3] │ │
│ │ [Agent: main ▾] [Turn 3/12 ▾] │ │
└───────────────────────────┘
Description
Enhance subagent event visualization across the session detail and time analysis views. This adds a unified subagent ownership model that works across all three session sources (Copilot CLI, VS Code, Claude), with color-coded event attribution, a subagent filter dropdown, usage badges, and visual dividers. Backend support includes automatic merging of sub-agent event files from
subagents/directories and VS CoderunSubagenttool invocation parsing.Type of Change
Checklist
npm run lint:fixand fixed all issuesnpm run test:alland all tests passRelated Issues
Screenshots (if applicable)
Additional Notes