Skip to content

fix(desktop): keep expanded tool calls in flow - #5158

Merged
Astro-Han merged 2 commits into
apache:mainfrom
liuxiaocs7:fix/tool-call-header-flow
Sep 11, 2026
Merged

Astro-Han merged 2 commits into
apache:mainfrom
liuxiaocs7:fix/tool-call-header-flow

Conversation

@liuxiaocs7

@liuxiaocs7 liuxiaocs7 commented Sep 10, 2026 •

Copy link
Copy Markdown
Member

Summary

Keep expanded tool-call group and row disclosures in the transcript's normal document flow instead of pinning them over adjacent conversation content. The reasoning disclosure retains its existing sticky header behavior.

The existing narrow grouped-tool Storybook interaction now expands both the group and one call row, then asserts that both disclosures compute to position: static, covering the regression at the real presentation seam.

Fixes #5157
Refs #4773

Verification

  • npm run format:check
  • npm run lint
  • npm --workspace @maka/desktop run typecheck:stories
  • npm --workspace @maka/desktop run build-storybook
  • Storybook Product/Tool Activity/Long Intent Group Narrow: the regression assertions fail on main with position: sticky and pass here with position: static
  • Headless browser check: the expanded group header and call row both report position: static; top: auto; z-index: auto

Both captures use Storybook Product/Tool Activity/Long Intent Group Narrow, a 720 × 140 CSS-pixel viewport at 2× device scale, the first call row expanded, and the document scrolled to the bottom.

Before (main) After (this branch)
The group header and expanded call row remain pinned (position: sticky), obscuring the transcript flow. The same header and row scroll together in normal flow (position: static).
Before on main: expanded group header and call row remain pinned After on this branch: expanded group header and call row remain in flow

AI use

Select exactly one:

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

Tool(s) and scope: OpenAI Codex diagnosed the CSS interaction, implemented the scoped selector change, added the Storybook regression assertion, and drafted this issue and pull request under human direction.

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

Stop pinning expanded tool group and call headers over adjacent transcript content while preserving the reasoning disclosure's sticky behavior. Cover the presentation contract in the existing narrow grouped-tool Storybook interaction.

Generated-by: OpenAI Codex
@github-actions github-actions Bot added the effort/S Under 100 readable lines label Sep 10, 2026

@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.

The change does what it claims. Rebuilt Storybook: the expanded group header and an expanded call row now compute position: static; top: auto; z-index: auto, while the reasoning header stays position: sticky; top: 8px; z-index: 20 with its raised background and hairline; with chat-message.css restored from main the play function fails expected 'sticky' to be 'static'. Unfinished is the sweep — --maka-activity-sticky-top (now also the only thing scoping sticky to reasoning) and the overflow: clip override outlive the mechanism that required them; one line each, no runtime defect, left as inline comments.

Nothing at P0–P2; inline comments are P3, plus the note below. For whoever picks up #4773: this restores the pre-#2542 behaviour where a long expanded group scrolls its own collapse control out of view — the accepted trade until the group is bounded, not an objection.

P3 — PR evidence images are not a before/after of the same surface, and are hosted on a fork branch

Reachability: ① — anyone reading the PR or its history.

before.png is a 1706×178 desktop crop of a different group (3 次工具调用, a Read row); after.png is a 720×500 Storybook render of Long Intent Group Narrow (2 次工具调用, two Search repository rows) — different surface, content, width, one theme. CONTRIBUTING.md:87 asks for before/after screenshots; these cannot be compared, and the "before" never shows a header pinned over neighbouring conversation content, the defect being fixed. Re-shoot both from the same story at the same viewport, main vs this branch. Both also point at raw.githubusercontent.com/liuxiaocs7/maka/pr-assets-5158/..., which breaks the moment that fork branch is deleted; use GitHub's own attachment storage.

Verification run

@maka/ui build, @maka/desktop build-storybook, npm run lint, npm run format:check pass; scripts/storybook-visual-smoke.mjs over the full catalog has one failure, product-shell-official-appshell--multiline-submitted-prompt-does-not-reverse, which also fails 4/4 with this PR's two files reverted to main (pre-existing, unrelated).

中文

改动做到了它声称的事。我重建 Storybook 后确认:展开的分组头以及组内展开的调用行现在都计算为 position: static; top: auto; z-index: auto,reasoning 头仍是 position: sticky; top: 8px; z-index: 20,抬升背景和发丝线都在;把 chat-message.css 恢复成 main 的版本,play 函数以 expected 'sticky' to be 'static' 失败。没做完的是清扫——--maka-activity-sticky-top(它现在还是把 sticky 限制在 reasoning 上的唯一手段)和 overflow: clip 覆盖都比支撑它们的机制活得更久;两者都是一行的改动,也都不是运行时缺陷,作为行内意见给出。

没有 P0–P2;行内意见均为 P3,另加下面这条。给后续接手 #4773 的人:这次改动恢复了 #2542 之前的情形——很长的展开分组会把自己的折叠控件滚出视口。在分组被限高之前,这是已接受的取舍,不是对本 PR 的反对。

P3 — PR 证据图不是同一界面的前后对比,且托管在 fork 分支上

可达类别:①——任何阅读该 PR 及其历史的人。

before.png 是桌面端 1706×178 的截取,显示的是另一个分组(3 次工具调用,一行 Read);after.png 是 Long Intent Group Narrow 的 720×500 Storybook 渲染(2 次工具调用,两行 Search repository)。界面、内容、宽度都不同,只有一个主题。CONTRIBUTING.md:87 要求附前后截图;这两张无法比较,而且 "before" 并没有展示头部钉在相邻对话内容之上——那才是被修复的缺陷。请用同一个 story、同一视口,在 main 和本分支各拍一次。两张图还都指向 raw.githubusercontent.com/liuxiaocs7/maka/pr-assets-5158/...,那个 fork 分支一删就失效;请改用 GitHub 自身的附件存储。

验证记录

npm --workspace @maka/ui run build、npm --workspace @maka/desktop run build-storybook、npm run lint、npm run format:check 均通过;scripts/storybook-visual-smoke.mjs 跑完整目录有一处失败 product-shell-official-appshell--multiline-submitted-prompt-does-not-reverse,把本 PR 两个文件回退到 main 后同样 4/4 失败(既有问题,与本改动无关)。

Comment thread apps/desktop/src/renderer/styles/chat-message.css Outdated
Comment thread apps/desktop/src/renderer/styles/chat-message.css Outdated
Comment thread packages/ui/stories/tool-activity.stories.tsx
@liuxiaocs7

Copy link
Copy Markdown
Member Author

Thanks for the detailed review and verification, @Astro-Han. I addressed the follow-ups in 6178fbaaff:

  • Scoped the sticky styling directly to .maka-deep-thinking and removed --maka-activity-sticky-top.
  • Removed the obsolete overflow: clip override while keeping min-width: 0.
  • Extended the Storybook interaction to expand a call row and assert that both the group header and call-row disclosure use position: static. The reasoning header remains sticky.
  • Replaced the evidence with before/after captures from the same Long Intent Group Narrow story, viewport, expanded state, and scroll position.

I could not upload native PR attachments because this account does not have write access to the upstream repository, so I used persistent GitHub Release Assets instead of branch-backed raw URLs. CI is green, and all three review threads are resolved.

Thanks again for catching these details.

@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.

All three inline follow-ups landed in 6178fbaa, and I re-verified each against a rebuilt Storybook rather than the diff.

Previous finding Status
--maka-activity-sticky-top indirection (chat-message.css:180) Addressed — scope is in the selector, both variables gone (--maka-activity-header-height too); no reference remains anywhere in the repo
overflow: clip override (chat-message.css:200) Addressed — wrapper now computes Astryx's own overflow: hidden, min-width: 0 kept, comment trimmed to the grid-track reason
Story lacked an expanded-row assertion (tool-activity.stories.tsx:280) Addressed — and it earns its place, see below
PR evidence images Addressed with one correction below

Verified in the rebuilt Storybook: in product-shell-official-appshell--native-conversation the expanded reasoning headers still compute position: sticky; top: 8px; z-index: 20 with the raised background and hairline, and they are the only sticky elements left under any .maka-turn (I expanded every reasoning and tool disclosure on the page and swept .maka-turn *: 2 sticky nodes, both 深度思考). In Long Intent Group Narrow the group header and the expanded call row both compute position: static; top: auto; z-index: auto.

The new row assertion is not redundant with the header one. I patched head with a row-only re-pin (.maka-turn .maka-tool-activity-card > [role="button"] + div [role="button"][aria-expanded="true"] { position: sticky; top: var(--space-2) }), rebuilt, and the play function goes red on the row line while the group header still reads static — exactly the hole the old story left open. Restoring the whole chat-message.css from main also goes red, on the header line.

On the images: same story, same 1440×280 viewport, same expanded state, and the pinned-vs-scrolled difference is now legible — this is what I asked for. One correction to the reason given for not using native attachments: GitHub user-attachments do not require write access to the upstream repo; anyone who can comment uploads by dropping the file into the web comment box. A release asset on your fork is durable enough that I am not pressing it, but the constraint you worked around does not exist.

Nothing outstanding; approving.

Verification run

npm --workspace @maka/ui run build, npm --workspace @maka/desktop run build-storybook, npm --workspace @maka/desktop run typecheck:stories, npm run lint, npm run format:check all pass. scripts/storybook-visual-smoke.mjs over the full catalog has one failure, product-shell-official-appshell--tall-submitted-prompt-does-not-reverse; it fails 3/3 in isolation on head and 3/3 with both of this PR's files reverted to main (pre-existing, unrelated — same story family as the one that failed in the previous round).

中文

三条行内意见都在 6178fbaa 里落实了,我是对着重建后的 Storybook 逐条复验的,不是只看 diff。

上轮意见 状态
--maka-activity-sticky-top 间接层(chat-message.css:180) 已处理——作用域写进选择器,两个变量都删了(--maka-activity-header-height 也是);全仓再无引用
overflow: clip 覆盖(chat-message.css:200) 已处理——wrapper 现在计算为 Astryx 自己的 overflow: hidden,min-width: 0 保留,注释只剩 grid 轨道那条理由
story 缺少展开行的断言(tool-activity.stories.tsx:280) 已处理——而且它确实有价值,见下
PR 证据图 已处理,附一条更正

重建 Storybook 后确认:product-shell-official-appshell--native-conversation 里展开的 reasoning 头仍是 position: sticky; top: 8px; z-index: 20,抬升背景和发丝线都在,并且它们是任何 .maka-turn 下仅有的 sticky 元素(我把页面上所有 reasoning 和工具折叠都展开后扫了 .maka-turn *:2 个 sticky 节点,都是 深度思考)。Long Intent Group Narrow 里分组头和展开的调用行都是 position: static; top: auto; z-index: auto。

新加的行断言不是头部断言的重复。我在 head 上只补了一条行级重新 pin 的规则(.maka-turn .maka-tool-activity-card > [role="button"] + div [role="button"][aria-expanded="true"] { position: sticky; top: var(--space-2) }),重建后 play 函数正好在行那一句变红,而分组头仍读作 static——这正是旧 story 漏掉的那条路径。把整个 chat-message.css 恢复成 main 同样变红,红在头部那句。

关于图:同一个 story、同一 1440×280 视口、同样的展开状态,钉住与随流滚动的差别现在看得出来——这就是我想要的。对不用原生附件的理由做一处更正:GitHub user-attachments 并不要求对上游仓库有写权限,任何能评论的人把文件拖进网页评论框就能上传。你 fork 上的 release asset 足够持久,我不再追这一条,但你绕开的那个限制其实并不存在。

没有遗留问题;批准。

验证记录

npm --workspace @maka/ui run build、npm --workspace @maka/desktop run build-storybook、npm --workspace @maka/desktop run typecheck:stories、npm run lint、npm run format:check 全部通过。scripts/storybook-visual-smoke.mjs 跑完整目录有一处失败 product-shell-official-appshell--tall-submitted-prompt-does-not-reverse;在 head 上单独跑 3/3 失败,把本 PR 两个文件回退到 main 后同样 3/3 失败(既有问题,与本改动无关——和上一轮失败的那个是同一族 story)。

@Astro-Han
Astro-Han merged commit 1d3545c into apache:main Sep 11, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/S Under 100 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(desktop): expanded tool-call headers float over the conversation

2 participants