🪜 feat: Title an Open Activity Fold and Reach Failed Calls in One Click - #16390
Conversation
An open phase or tool group now reads as the title of the rows under it: the header steps up to primary semibold, pins the stable label instead of the live line, and sticks to the top while its rows scroll. Rows step in 24px under a hairline rail hung from the header glyph, one level per fold. A failure is one click away from anywhere. A "N failed" pill beside the header opens the fold and every failed row's output; a collapsed card peeks its first failed call and error line under the header; the failed row carries a red stripe, the error's first line as its subtitle, and names its subject by the model's intent as the live header and peek already do. The reveal travels down a FailedRevealContext counter that a group relays to its own rows only once they have mounted, so a request made on a closed card reaches a row three lazy disclosures below it.
…Showed A mock-lane spec runs a phase whose middle batch holds a call that failed schema validation beside a slow one, watches the fold stream, then reaches the error from the peek, from the pill, and again after a reload, in both themes, with no console or page errors. Three things the real app showed that the harness had not: - The client counted only two of the server's three failed-output shapes, so a schema-validation failure that the server labelled `partial` rendered as a completed card with no pill. `isError` now mirrors `completedToolExecutionStatus`. - A row's label lost its last letters to an ellipsis whenever its subtitle overflowed: the weighted shrink left the label a fraction of a pixel short. The subtitle now takes all of the shrink and the label caps at the row. - The peek named an MCP call by its server while the row named the tool. Both now name it the way the row does, and the error line drops its bare `Error:` prefix the red glyph already speaks for.
1739475 to
cb023ec
Compare
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb023ec251
ℹ️ 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".
The reveal was consumed by the generic ToolCall row alone, so a failed bash, code, memory, file or skill card, each rendered through its own component, opened its group and stopped. The consumer now lives in ProgressText, the disclosure control every card renders: a failed, openable, closed row opens through the card's own toggle, and the one row the request lets take focus lands the reader on its labeled button rather than on an unnamed wrapper. The request carries a one-shot focus claim, relayed through nested providers, so a phase with several failures opens all of them and focuses only the first in document order, the one its peek named. Also from the same review: an error line is derived only from output that is an error, so a step failed by its run status or a status attachment no longer shows ordinary output in red; a standalone group shows its failure count once, on the pill, keeping the text for the accessible name; and the label and subtitle share one shrinking box so the verdict, duration and chevron after them never lose width.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: adfa8d2e8a
ℹ️ 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".
|
Completion pass for head |
|
Current remote head |
2 participants
Summary
An expanded activity phase or tool group is hard to read as a group. Its header keeps the same weight and colour open or closed, while the run is live the header shows the newest row's text instead of the group's name, and a phase header, a sub-group header and a tool row all share one left edge and one type size, so the reader cannot tell what sits under what. A failed call is a
· failedsuffix on one row and a· 1 failednote on the header: reaching the error takes opening the fold, scanning for the suffix, and opening that row.This PR keeps the phase → tool group → row → output hierarchy and changes how an open fold and a failure present. An open header steps up to primary semibold, is titled by its stable label instead of the live line, and sticks to the top of the viewport while its rows scroll under it. The rows under an open header step in 24px beneath a hairline rail hung from the header's glyph, one level per fold. A failure is one click away from anywhere: a
N failedpill beside the header opens the fold and every failed row's output panel, a collapsed card peeks its first failed call and error line on a row under the header, and the failed row itself carries a red stripe at its left edge, the error's first line as its subtitle, and names its subject by the model's intent, the way the live header and the peek already do.Driving the change through the real app on the mock lane surfaced three defects that the component harness could not: the client recognised only two of the server's three failed-output shapes, so a schema-validation failure the server labelled
partialrendered as a completed card with no pill; a row's label lost its last letters to an ellipsis whenever its subtitle overflowed, because the weighted shrink left it a fraction of a pixel short of its text; and the peek named an MCP call by its server while the row named the tool. All three are fixed here.Related to the Discord thread with Berry: make the tool group name more visible when expanding, a quick way to expand failed calls, and be clearer which tools sit under the main group.
Failed question calls now give their shared disclosure the failed phase. Searches and image calls with failed output or run status use the regular tool-error disclosure, so their error is present, expandable and reachable from the pill rather than hidden or marked successful. Failed subagents open their existing details panel when available. Cancelled calls remain cancelled; older search errors that were not counted as failures stay hidden rather than newly reading as successful. Long failed-peek labels yield space to the fixed count and Show error action.
How it works
The open-fold treatment is classes only:
FOLD_RAIL_CLASSESinrows.tsgoes on each panel's clipping wrapper (the inset has to sit inside theoverflow-hiddenelement, or the failed row's stripe drawn 12px left of the row is clipped away), and the phase header's wrapper takessticky top-0while open.The reveal is a counter that travels down the fold tree, since expansion otherwise only flows up through
onToolExpand:Deferring the increment until
readyis what makes one click on a closed card work: a collapsed card mounts its rows in the same commit that opens it, and a row that mounts under an already-advanced counter would take it as the resting value. Each group re-issues the request to its own rows for the same reason, so it reaches a row three lazy disclosures down.The main reveal consumer lives in
ProgressText, the disclosure for generic, bash, code, memory, file, skill and question cards. A failed search or image call routes through the genericToolCalldisclosure; a failed subagent claims the request and opens its own activity panel. An openable row uses its own toggle and puts focus on its labeled button. The request carries a one-shot focus claim, relayed through nested providers, so a phase with several failures opens all of them and focuses only the first in document order, the one its peek previewed.Visible text stays deduplicated: while live, the header's failure count is spoken as part of the header's name (
sr-only) and shown by the pill; a standalone group's text detail drops the failed note when its pill is showing. A stop count has no pill and stays in text.The client's
isErrornow matches the server'scompletedToolExecutionStatus: the tool-call error prefix,Error processing tool, and a bareError:block closed by "Please fix your mistakes." (the SDK's schema-validation feedback, which leaves the run stepcompleted). InProgressTextthe label takesshrink-0 max-w-fullwhenever a subtitle is present and the subtitle takes all of the shrink; before, any overflowing subtitle cost the label its last letters.Type of change
Testing
Ran the real app on the mock e2e lane with a new fake-model scenario,
E2E_ACTIVITY_FAILED_REPLY, whose middle batch callsslow_echowithout its requiredtext(fails schema validation) beside a 4-secondslow_echo. Watched the fold stream (live header ticks "Running e2e-memory ×2"), then on the settled card: pill and peek present, peek click opens the phase, the group holding the failure and that row's output with focus on the row, header click closes it and brings the peek back, pill click reveals again, reload keeps pill and peek and the reveal still works, dark theme rendered. No console errors or page errors across the run. A second scenario without failures shows neither pill nor peek and opens its nested groups as before.Also mounted the real
ActivityPhaseGroup→ToolCallGroup→ToolCallunder the realstyle.cssin Chromium (vite harness, both themes) and confirmed the rail is a 1pxborder-mediumline at a 24px inset on both levels, the failed row's stripe renders beside the rail, and scrolling leaves the open header pinned at the top.Tested environments/configuration:
playwright.config.mock.ts, in-memory Mongo, fake model, fake MCP server), not a live providerAutomated tests:
Completion pass: 208 passed across 8 focused frontend suites on
d734f7d3f(Part,WebSearch,SubagentCall,AskUserQuestionCall,ActivityPhaseGroup,ProgressText,ToolCallGroup,ToolCall). On final head87e7b1f2a, 21WebSearchtests passed, including the legacy-output compatibility case.npm run build:data-providerand clienttsc --noEmitpassed, including the final head.npx playwright test --config=e2e/playwright.config.mock.ts activity-fold activity-phases— 3 passed (newe2e/specs/mock/activity-fold.spec.ts)cd client && npx jest src/components/Chat/Messages/Content— 77 suites, 1210 testsProgressText.test.tsx: reveal opens through the row's own toggle and focuses its button, opens without focus when another row holds the claim, never toggles an open panel, ignores rows that did not fail or cannot open; fixed siblings stay out of the shrinking label box.ToolCallGroup.test.tsx: a standalone group shows its count once, on the pill; nested groups keep it in text.failed.test.ts: benign output on a failed step yields no error line.Added
__tests__/failed.test.ts(failed-line helper), cases inActivityPhaseGroup.test.tsx,ToolCallGroup.test.tsx,ToolCall.test.tsx,ProgressText.test.tsx,OutputRenderer.test.tsx; three selectors inLiveParity.test.tsxupdated for the pill and the header wrappernpx tsc --noEmitinclient(no errors in touched files; the worktree has pre-existing sandpack module-resolution errors unrelated to this change)Before / after screenshots
Revision-pinned captures from the real mock app in desktop dark mode, using synthetic local fixtures.
Collapsed failed phase
Open phase hierarchy
84e3ea9ed99d(before) →cb023ec251c3(after).