Skip to content

fix(ui): hide empty tool details and render summary bodies - #5999

Open
Colafornia wants to merge 2 commits into
apache:mainfrom
Colafornia:fix/tool-activity-detail-availability
Open

Colafornia wants to merge 2 commits into
apache:mainfrom
Colafornia:fix/tool-activity-detail-availability

Conversation

@Colafornia

@Colafornia Colafornia commented Oct 8, 2026 •

Copy link
Copy Markdown
Member
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

  • Rendering and plugin suites: 42 passed, including 9 regression tests. Against the original implementation, the regression suite reports 7 failures and 2 passes.
  • Full UI suite: 698 passed.
  • npm run lint, npm run format:check, npm run build, and npm run typecheck: passed.
  • npx knip --workspace apps/desktop --workspace packages/ui and npm run check:locale-hygiene: passed.
  • Full cross-workspace tests and live application/provider testing were not run.

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.

Before After
Before: six expandable rows and a summary placeholder After: two expandable rows and the summary body

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

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

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No
中文

概述

展开工具调用行时,详情可能为空、重复收起行上的命令,或只显示原始结果类型。现在只有详情能提供新信息时,工具行才可展开。摘要结果显示摘要正文,并沿用现有的脱敏、本地化和行数上限;归档结果在工具行上显示本地化状态。

匹配该工具的详情插件、实时输出,以及沙箱拦截或权限绕过恢复横幅,仍会让工具行保持可展开。详情比较沿用行上显示意图或调用命令时的格式化方式,确保被截断的参数和不同的意图后缀仍可查看。

Fixes #5997

验证

  • 渲染与插件测试:42 项通过,其中包含 9 项回归测试。在原始实现上,回归测试有 7 项失败、2 项通过。
  • 完整 UI 测试:698 项通过。
  • 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 是否改变行为?

  • 是——已在上述概述中说明
  • 否

@github-actions github-actions Bot added the effort/M Under 500 readable lines label Oct 8, 2026
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 Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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: the quietText comparison caps both sides with boundedToolTarget. 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 at tool-activity-detail-availability.test.tsx:105 currently asserts this. Issue #5997 only asks to hide text that equals the target, and argsOnly already keeps over-cap text.

P3

  • tool-activity.tsx:666: the archive status replaces outcomeWord, 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: empty text/summary bodies still open to an empty code block.
  • packages/ui/stories/tool-activity.stories.tsx has no summary, archived or non-expandable rows. A story would be more durable than the PNG and README evidence under docs/images/pr/.

CI: test and label pass. Mergeable; awaiting review.

Comment thread packages/ui/src/tool-activity.tsx Outdated
Comment thread packages/ui/src/__tests__/tool-activity-detail-availability.test.tsx Outdated
Comment thread packages/ui/src/tool-activity.tsx Outdated
Comment thread packages/ui/src/client-plugin-slots.tsx
Comment thread packages/ui/src/tool-activity.tsx Outdated
@Colafornia
Colafornia force-pushed the fix/tool-activity-detail-availability branch from 5d164eb to 76e8da5 Compare October 9, 2026 01:12
Colafornia added a commit to Colafornia/maka-agent that referenced this pull request Oct 9, 2026
… 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 Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
    • quietText now compares the uncapped first line, or normalizeToolIntent for 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 ToolTrow via the shared slotEntriesRenderForHost, 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 (describeToolCall runs 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.
  • ChatView calls that hook for the session-scoped conversation.header.actions slot at chat-view.tsx:687. That call sits outside the <MakaClientSessionScope sessionId={activeSession.id}> the same component renders at :916.
  • MakaClientSlotProvider always provides sessionId: 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 e0291438 with 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.tsx the header renders the fallback.
    • With 76e8da51's version it renders the plugin action.
  • 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.footer caller in chat-turn.tsx:1050 is unaffected because it renders inside the scope.
  • Fix: leave useMakaClientSlotOccupied unchanged and gate only useMakaClientSlotOccupiedKeys. Alternatively, give the hook an explicit sessionId argument, 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 standardToolCall resultDetail gating in tool-activity.tsx.
  • It adds a parallel useMakaClientSlotEntryKeys hook to client-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.

Comment thread packages/ui/src/client-plugin-slots.tsx Outdated
… 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>
@Colafornia
Colafornia force-pushed the fix/tool-activity-detail-availability branch from e029143 to 507a294 Compare October 9, 2026 05:40

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.actions slot never rendered plugin buttons): fixed.
    • The hook at chat-view.tsx:687 runs outside the MakaClientSessionScope. useMakaClientSlotOccupied no longer applies the scope gate, so that call reports the registered action again.
    • useMakaClientSlotOccupiedKeys keeps the gate through slotEntriesRenderForHost. That hook is the one the ToolTrow detail fix needed.
    • Reproduced at 507a2941: the new ChatView test registers a header action under MakaClientSlotProvider with no outer session scope. The test passes and checks that the action receives sessionName/sessionId from ChatView's own scope.
    • Negative control: I re-inserted e0291438's gate into the built useMakaClientSlotOccupied. The same test then fails with "the registered header action is visible without an outer session scope". So the test guards this regression.
  • P3 (no Storybook coverage) and P3 (describeToolCall runs 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-tree reports content conflicts in packages/ui/src/client-plugin-slots.tsx, packages/ui/src/tool-activity.tsx and packages/ui/src/tool-activity/tool-result-preview.tsx.
  • #6015 adds useMakaClientSlotEntryKeys, which does the same scope-guarded keyed lookup as this PR's useMakaClientSlotOccupiedKeys.
  • #6015 also rewrites the same resultDetail gating.
  • 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-actions
  • tool-activity-detail-availability
  • client-plugin*
  • chat-view*
  • chat-turn*

Status: MERGEABLE, mergeStateStatus BLOCKED (review required). The branch merges cleanly with current main. CI test passes on 507a2941.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(desktop): tool rows expand to empty, repeated, or placeholder details

2 participants