Repository navigation
fix(ui): hide empty tool details and render summary bodies - #5999
Colafornia wants to merge 2 commits into
Conversation
Omit empty and repeated tool details while preserving formatted intent, live output, recovery decorations, and matching plugin contributions. Render summary bodies and add regression tests. Generated-by: OpenAI Codex
Astro-Han
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.
Thanks, this is a nice cleanup. Summary bodies now render through the same plain-text redactSecrets → formatUserVisibleToolText → capLines → ToolCodeBlock path as text, so there is no Markdown or HTML interpretation and no new XSS surface. The keyed plugin lookup mirrors the outlet's exact-key rule, and the i18n strings are complete.
P2
tool-activity.tsx:363: thequietTextcomparison caps both sides withboundedToolTarget. A single-line invocation longer than 120 characters therefore compares equal to its truncated row target, and the row loses its only view of the full command. This is reached when there is no displayable result: a running call before its first output, or an interrupted or cancelled call. The test attool-activity-detail-availability.test.tsx:105currently asserts this. Issue #5997 only asks to hide text that equals the target, andargsOnlyalready keeps over-cap text.
P3
tool-activity.tsx:666: the archive status replacesoutcomeWord, so "Interrupted" / "Sandbox blocked" disappear from archived rows.client-plugin-slots.tsx:772: the hook doesn't mirror the outlet's session-scope gate, so a chevron can appear over an empty panel outside a session scope.tool-activity.tsx:370: emptytext/summarybodies still open to an empty code block.packages/ui/stories/tool-activity.stories.tsxhas no summary, archived or non-expandable rows. A story would be more durable than the PNG and README evidence underdocs/images/pr/.
CI: test and label pass. Mergeable; awaiting review.
5d164eb to
76e8da5
Compare
… scope Address review on apache#5999: compare the uncapped detail body against the capped row target so a truncated command or intent stays expandable, keep the outcome word alongside the archived-result status, share the outlet session-scope gate with the slot occupancy hooks, and treat whitespace-only text or summary bodies as no detail. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Astro-Han
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.
Incremental review of 5d164ebd..e0291438 (merge-base unchanged at 5735554b). Per git range-diff, the first commit (76e8da51) matches the previously reviewed commit except that it drops docs/images/pr/tool-activity-detail/ (README and two PNGs). All code changes in this range are in the new commit e0291438. Protocol and runtime-host files are untouched, so there is no epoch impact.
Prior findings
- P2 (a long single-line command or intent lost its only full view): fixed.
quietTextnow compares the uncapped first line, ornormalizeToolIntentfor intents, against the capped target.- Under the cap the two forms are identical, so equal rows still hide. Over the cap they differ, so the row stays expandable.
- The tests at both the 120-character and 240-character caps were flipped to
true.
- P3 (the archive status replaced the outcome word): fixed. The row now shows
outcome · archived, and a test covers it. - P3 (the occupied-keys hook ignored the session-scope gate): fixed for
ToolTrowvia the sharedslotEntriesRenderForHost, with a scoped/unscoped test. However, the same change introduces the regression below. - P3 (whitespace-only text or summary bodies stayed expandable): fixed, with tests.
- P3 (no Storybook coverage) and P3 (
describeToolCallruns twice per expanded row): still open, non-blocking. The docs PNG/README concern is moot now that they are removed.
New finding
P2: plugin conversation header actions never render. (packages/ui/src/client-plugin-slots.tsx:780)
- The scope gate was also added to the existing
useMakaClientSlotOccupied. ChatViewcalls that hook for the session-scopedconversation.header.actionsslot atchat-view.tsx:687. That call sits outside the<MakaClientSessionScope sessionId={activeSession.id}>the same component renders at :916.MakaClientSlotProvideralways providessessionId: undefined, so the hook now always returns false.actions={hasConversationHeaderActions ? <MakaClientSlotOutlet …/> : undefined}then never mounts the outlet, even though the outlet itself would render inside the scope.- Reproduced at
e0291438with a ChatView-shaped tree: provider, then the hook, then a session scope, then the outlet, with one registered header action.- With the head's
client-plugin-slots.tsxthe header renders the fallback. - With
76e8da51's version it renders the plugin action.
- With the head's
- No in-tree plugin registers this slot, which is why CI and the existing chat-view/chat-turn tests (64 pass) stay green. It is still a silent break of a public client-plugin slot.
- The
conversation.turn.footercaller inchat-turn.tsx:1050is unaffected because it renders inside the scope. - Fix: leave
useMakaClientSlotOccupiedunchanged and gate onlyuseMakaClientSlotOccupiedKeys. Alternatively, give the hook an explicitsessionIdargument, or move the ChatView header check inside the scope. Please also add a ChatView-level test that registers a header action.
Overlap: #6015 (ggbdpq, "only expand tool rows whose detail adds information") overlaps substantially and will conflict.
- It rewrites the same
standardToolCallresultDetailgating intool-activity.tsx. - It adds a parallel
useMakaClientSlotEntryKeyshook toclient-plugin-slots.tsx. - The two should be reconciled so that one supersedes the other.
Verification: I built @maka/core and @maka/ui at the head. tool-activity-detail-availability.test passes 10/10, and the client-plugin* tests pass 19/19.
Status: MERGEABLE, mergeStateStatus BLOCKED (review required). CI test passes on e0291438.
Severity note: graded P2 rather than P1 because no plugin in this repository registers the header-actions slot today, so no shipped surface regresses. It is still a reproduced regression of the plugin slot contract and should be fixed before merge.
… scope Compare uncapped detail text against capped row targets, retain outcome words alongside archived-result status, and hide whitespace-only result bodies. Share the outlet session-scope gate with keyed detail occupancy while preserving the generic occupancy hook used outside ChatView's child session scope. Add a ChatView regression test for registered header actions and their session props. Generated with [Devin](https://devin.ai) Generated-by: OpenAI Codex Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
e029143 to
507a294
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.
Incremental review of e0291438..507a2941. The merge-base is unchanged at 5735554b. Per git range-diff, the first commit 76e8da51 is identical, and the second commit was amended from e0291438 to 507a2941. The amended commit changes two files relative to the previously reviewed head:
- It reverts the session-scope gate in
useMakaClientSlotOccupied(client-plugin-slots.tsx, now byte-identical to main). - It adds
packages/ui/src/__tests__/chat-view-header-actions.test.tsx.
Protocol and runtime-host files are untouched, so there is no epoch impact.
Prior findings
- P2 (ChatView's
conversation.header.actionsslot never rendered plugin buttons): fixed.- The hook at
chat-view.tsx:687runs outside theMakaClientSessionScope.useMakaClientSlotOccupiedno longer applies the scope gate, so that call reports the registered action again. useMakaClientSlotOccupiedKeyskeeps the gate throughslotEntriesRenderForHost. That hook is the one theToolTrowdetail fix needed.- Reproduced at
507a2941: the new ChatView test registers a header action underMakaClientSlotProviderwith no outer session scope. The test passes and checks that the action receivessessionName/sessionIdfrom ChatView's own scope. - Negative control: I re-inserted
e0291438's gate into the builtuseMakaClientSlotOccupied. The same test then fails with "the registered header action is visible without an outer session scope". So the test guards this regression.
- The hook at
- P3 (no Storybook coverage) and P3 (
describeToolCallruns twice per expanded row): still open, non-blocking. - All other earlier findings stay fixed. Nothing else changed in this range.
New findings: none.
Overlap with #6015 (ggbdpq, head 33cc4bed, "only expand tool rows whose detail adds information"): still a substantial overlap.
git merge-treereports content conflicts inpackages/ui/src/client-plugin-slots.tsx,packages/ui/src/tool-activity.tsxandpackages/ui/src/tool-activity/tool-result-preview.tsx.- #6015 adds
useMakaClientSlotEntryKeys, which does the same scope-guarded keyed lookup as this PR'suseMakaClientSlotOccupiedKeys. - #6015 also rewrites the same
resultDetailgating. - Whichever PR lands second needs a rebase that keeps a single keyed hook and a single detail-gating path. The maintainers should pick one PR as the base and not merge both as-is.
Verification: I built @maka/core and @maka/ui at 507a2941. 94 of 94 tests pass across:
chat-view-header-actionstool-activity-detail-availabilityclient-plugin*chat-view*chat-turn*
Status: MERGEABLE, mergeStateStatus BLOCKED (review required). The branch merges cleanly with current main. CI test passes on 507a2941.
English
Summary
Expanding a tool row could show an empty panel, repeat the collapsed command, or display a raw result type. Rows now offer expansion only when their details add information. Summary results display their summarized text with the existing redaction, localization, and line cap; archived results show a localized status on the row.
Matching tool-detail plugins, live output, and sandbox/bypass recovery banners keep rows expandable. Detail comparisons use the displayed intent or invocation formatting, so truncated arguments and distinct intent suffixes remain accessible.
Fixes #5997
Verification
npm run lint,npm run format:check,npm run build, andnpm run typecheck: passed.npx knip --workspace apps/desktop --workspace packages/uiandnpm run check:locale-hygiene: passed.The screenshots render Maka/Astryx components with fixed fixtures in Chromium. The same six rows have six expansion chevrons before and two after; the summary row is expanded in both. Images link to the earlier evidence snapshot; this PR includes no
docs/files.AI use
Tool(s) and scope: OpenAI Codex implemented the UI changes, regression tests, and review evidence, and drafted this description. The commit includes
Generated-by: OpenAI Codex.Checklist
Does this PR entail a change in behavior?
中文
概述
展开工具调用行时,详情可能为空、重复收起行上的命令,或只显示原始结果类型。现在只有详情能提供新信息时,工具行才可展开。摘要结果显示摘要正文,并沿用现有的脱敏、本地化和行数上限;归档结果在工具行上显示本地化状态。
匹配该工具的详情插件、实时输出,以及沙箱拦截或权限绕过恢复横幅,仍会让工具行保持可展开。详情比较沿用行上显示意图或调用命令时的格式化方式,确保被截断的参数和不同的意图后缀仍可查看。
Fixes #5997
验证
npm run lint、npm run format:check、npm run build和npm run typecheck:通过。npx knip --workspace apps/desktop --workspace packages/ui和npm run check:locale-hygiene:通过。截图使用固定测试数据,在 Chromium 中渲染 Maka/Astryx 组件。同样的六行在修改前有六个展开箭头,修改后有两个;两张截图中的摘要行均已展开。图片链接指向之前的证据快照;本 PR 不包含
docs/文件。AI 使用情况
工具及范围:OpenAI Codex 实现了 UI 改动、回归测试和审查证据,并起草了本说明。提交包含
Generated-by: OpenAI Codex。检查清单
本 PR 是否改变行为?