Conversation
The primary tab strip now renders exactly two tabs, stdout and stderr. Execution events move to a low-emphasis text button beside the strip, so the panel is still one click away but no longer competes with the streams the console was opened for. The events unread indicator is gone: the unread state, its counter ref, its reset and the effect that set it are deleted. For a running task the events arrive over SSE, so the badge fired on every pushed event and pulled attention away from stdout and stderr. The stdout, stderr and per-step unread dots are untouched, including clearing a stream's dot when its tab is selected. `topTab` keeps 'events' as a value, so the shared active step, the step strip swapping to the execution-event steps, the panel render and the download guard all keep working unchanged. Tabs receives `false` while the events view is open, which leaves the strip unselected and avoids MUI's out-of-range warning. The events data path is untouched: useExecutionEvents is called with the same arguments and the SSE and REST paths behave as before.
MUI gives every tab tabIndex -1 when Tabs has no selected value, so an open events view left the primary strip unreachable by keyboard. Keep stdout as a tab-key entry point while the strip is unselected, and cover it with a test. Swap aria-pressed for aria-current on the events control: it is not a toggle, a second click is a no-op, and the way back is a primary tab. Route the button through handleTopTab so one function still owns a top-view change.
There was a problem hiding this comment.
Pull request overview
Updates TaskLogViewer’s log console UI so the primary navigation focuses on the log streams (stdout/stderr) while keeping execution events accessible via a lower-emphasis control, aligning with the framework’s task log viewer UX direction.
Changes:
- Removes the execution-events
Tab(and its unread/badge state) and replaces it with a subordinate textButton(“Execution events”) beside the stdout/stderr tab strip. - Keeps the existing
'events'view/state plumbing (step swapping, panel render, download-disable guard), but renders the primaryTabsas unselected (value={false}) while events are open to avoid MUI out-of-range warnings. - Updates and expands component tests to cover the new entry point, absence of unread indicators for events, keyboard reachability while the strip is unselected, and per-step behavior in the events view.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| frontend/packages/framework/src/components/TaskLogViewer/TaskLogViewer.tsx | Demotes execution events from a primary tab to a subordinate button; removes unread-event badge state; ensures MUI Tabs remain warning-free and keyboard reachable when events view is active. |
| frontend/packages/framework/src/components/TaskLogViewer/tests/TaskLogViewer.test.tsx | Adjusts and adds tests for the new control, verifies no events unread badge, validates unselected primary tabs behavior (no MUI warning + keyboard entry point), and confirms events step grouping/search remain functional. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
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 changed
The task log console's primary tab strip now renders exactly two tabs, stdout and stderr. Execution events move to a low-emphasis text button beside the strip, so the panel is still one click away but no longer competes with the streams the console was opened for.
Tab, theunreadEventsstate, theprevEventCountRefcounter, their resets, and the effect that set the badge. For a running task the events arrive over SSE (fetchEventSource), so the badge fired on every pushed event rather than on a fixed interval, which made it noisier than the ticket's "polling" description suggests.Button("Execution events") next to the tab strip. It is text-labelled rather than icon-only so events stay discoverable.topTabkeeps'events'as a value, so the sharedactiveStep,stepsForTabsswapping toeventStepOrder, the panel render, and the download guard all keep working with no edit.Tabsreceivesvalue={false}while the events view is open: the strip shows no active tab and MUI logs no out-of-range warning.Acceptance criteria covered
useExecutionEventsis called with the same arguments, SSE for running and REST for finished behave as before, and both empty states still render.Review feedback folded in
tabIndex -1whenTabshas no selected value, sovalue={false}would have stranded keyboard users outside the strip. stdout now keeps an explicittabIndex 0entry point while the strip is unselected; arrow keys move from there. Covered by a test.aria-currentinstead ofaria-pressed. The control is not a toggle: a second click is a no-op and the way back is a primary tab, so it marks the current view rather than a pressed state.handleTopTabrather than callingsetTopTabdirectly.Out of scope
No backend change: the execution-events endpoints, the SSE stream, and the payload shape are untouched.
useExecutionEventsandExecutionEventsPanelare unchanged — only how the panel is reached. Demoting the UI does not defer or lazy-start the event stream.Noted but not changed: the
taskHistoryIdreset effect does not resettopTab, so an open events view survives a task switch. That behaviour is identical onmainand is outside this ticket.Test plan
pnpm --filter @sep/framework test— 750 passed across 61 files.pnpm --filter @sep/framework type-check— clean.oxlint/oxfmt --check— clean (one pre-existing warning inExecutionEventsPanel.tsx, untouched here).New cases: exactly two primary tabs; one-interaction access to the panel and a stable second click; no visible badge anywhere while a running task pushes events (then opening the panel proves the events did arrive); unselected strip with no MUI warning, a keyboard-focusable tab, and a one-click return; search and per-step grouping from the demoted entry point.
Merge note
mainis the base.SEP-1670(#1314) edits the line-cap select inside the second toolbarStack, which this branch does not touch, but it is built on the three-tab structure and the removedunreadEventsstate, so whichever lands second needs a rebase.TopTabis internal and not exported, so no consumer of@sep/frameworkneeds an edit.Code was reviewed before opening this PR and the feedback is folded into the second commit.