Skip to content

fix(desktop): open usage trace from composer - #4689

Draft
Astro-Han wants to merge 6 commits into
apache:mainfrom
Astro-Han:fix/composer-usage-inspector-trigger
Draft

fix(desktop): open usage trace from composer#4689
Astro-Han wants to merge 6 commits into
apache:mainfrom
Astro-Han:fix/composer-usage-inspector-trigger

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

The composer exposed context usage through a hand-written span and dedicated CSS. It looked like a control but had no action, used provider-facing failure language as its visible label, and duplicated information already owned by the existing Inspector.

This PR:

  • replaces the custom readout with the Astryx Button primitive (ghost, sm) and its built-in Tooltip;
  • keeps the trigger scannable: it shows the measured percentage when a context limit is known, otherwise the localized Usage label;
  • makes the Tooltip state the request-level fact in plain language: used tokens, context limit and percentage when available, or a neutral absence message without exposing provider internals;
  • opens the existing right-side Trace tool on activation instead of introducing another panel or state authority;
  • removes the replaced control CSS;
  • makes the Astryx admission check reject new raw controls and button-like span/div implementations with an actionable use Astryx diagnostic.

There is no migration or compatibility path: the old presentation-only control is removed.

Behavior

  • Known usage and context limit: trigger 10%; Tooltip This request used 12,345 / 128,000 tokens (10%).
  • Known usage without a context limit: trigger Usage; Tooltip This request used 12,345 tokens; no context limit is available for this model.
  • No usage data: trigger Usage; Tooltip No usage data is available for this request.
  • Activating any state opens the existing Task trace in the right workbar.

Visual verification

The comparisons use the same chat-prompt-rail fixture, viewport, theme, and interaction. BEFORE is the independently installed and built PR base b0255edc; AFTER is the current PR head 8f070590.

Tooltip and trigger

Light:

Context usage Tooltip and trigger before and after in light mode

Dark:

Context usage Tooltip and trigger before and after in dark mode

Interaction outcome

Light:

Usage action opening the right Trace tool in light mode

Dark:

Usage action opening the right Trace tool in dark mode

Verification

  • npm run format
  • npm run lint
  • npm run build -w @maka/ui
  • npm run test:dist -w @maka/ui — 357 passed
  • npm run astryx:surface-inventory
  • npm run check:renderer-architecture -- --base b0255edcb35588b2f24efd386447d9a494c1e395
  • npm run build:renderer -w @maka/desktop
  • npx playwright test --config e2e/playwright.config.ts e2e/session-workbar.spec.ts --grep "composer usage action" — 1 passed

AI use

Select exactly one:

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

Tool(s) and scope: Codex traced the existing Astryx and workbar ownership seams, implemented the control, Tooltip copy and routing changes, added focused UI/Electron regressions and the Astryx admission guard, and captured exact-base/head visual evidence.

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 and Behavior above
  • No

@github-actions github-actions Bot added the effort/M Under 500 readable lines label Sep 3, 2026

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I reviewed exact head 8f0705901de2a0b3166e61e99f656ebdee73328b. One P3, no P0/P1/P2.

Replacing the hand-written span with an Astryx Button is a straight accessibility improvement: what was a decorative, unfocusable readout is now a real control a keyboard reaches, and it opens the Inspector that already existed rather than adding a second panel or a second state authority.

The accessible naming is right, and it is the part that is easy to get wrong. Because children !== label, the button takes its accessible name from aria-label — "open usage trace" — while the visible text stays the percentage or the Usage label. The measured detail lives in the tooltip, which Astryx wires through useTooltip with aria-describedby pointing at the tooltip layer (Button.tsx:728-737, useTooltip.tsx:608). So the name says what activating it does, and the description carries the numbers. That is the correct split; naming the control after its data would have made the accessible name change every time the percentage moved.

The provider-facing wording is genuinely gone, not just reworded in one place. The three usage strings are now "no usage data yet" / "context limit unknown" and their English equivalents; the remaining "provider" phrasing in conversation-copy.ts belongs to the overflow and refusal banners, which are different controls.

The new Astryx gate does not appear to over-reach. Scanning the current product surface with the same pattern, the only match is the known legacyBlockerBaseline fingerprint in packages/ui/src/astryx-chat-reasoning.tsx — one div with role="button" and an onClick. Ordinary layout span/div without those do not enter admissionGaps. A newly added control of that shape fails because the occurrence count no longer matches the baseline, a raw <button> still trips the blocker severity, and the diagnostic names the path, the count and the remedy. Both routes fail closed.

P3 — activating the button opens the Inspector without moving or announcing focus

app-shell.tsx:2995 calls workbar.commands.openTool('inspector'), and the singleton path does openWorkbarTab plus revealPlacement (use-workbar-controller.ts:322-325) with no focus(). The panel appears; focus stays on the button.

This is not the failure mode where a control unmounts and focus lands on document.body — the button is still there, and a keyboard user can Tab onward into the right-hand rail. But a screen reader user gets no signal that anything opened. The panel does carry an aria-label (session-inspector-panel.tsx:95), so the content is reachable once found; the gap is the transition. The e2e case asserts the panel becomes visible (session-workbar.spec.ts:54-67) and does not assert focus, so it would not catch a regression here either.

Either moving focus into the inspector region on open, or giving the button aria-expanded / aria-controls, would close it. Not blocking.

Evidence boundary: static review against the source plus the Astryx Button and useTooltip implementations. No browser or Playwright run, so the tooltip's announced behaviour is derived from the wiring rather than observed. test was still running when I posted.

简体中文

我审的是 8f0705901de2a0b3166e61e99f656ebdee73328b一条 P3,没有 P0/P1/P2。

把手写的 span 换成 Astryx Button 是一次实打实的可及性改善:原来那是一个装饰性的、无法聚焦的读数,现在是键盘能到达的真控件;而且它打开的是已经存在的 Inspector,没有新增第二个面板或第二份状态权威。

无障碍命名做对了,而这恰恰是最容易做错的地方。 由于 children !== label,按钮的无障碍名取自 aria-label——「打开用量追踪」——而可见文字仍是百分比或「用量」。测量出来的细节放在 tooltip 里,Astryx 通过 useTooltiparia-describedby 指向 tooltip 层(Button.tsx:728-737useTooltip.tsx:608)。所以名称说的是「激活它会做什么」,描述承载数字。这个分工是对的;要是用数据给控件命名,无障碍名就会随百分比每次变化而变。

面向提供方的措辞是真的清掉了,不是只改了一处。三条用量文案现在是「暂无用量数据」/「上下文上限未知」及其英文;conversation-copy.ts 里剩下的「供应商」属于溢出与拒绝横幅,那是另外的控件。

新增的 Astryx 门禁看起来没有误伤。 用同一套模式扫描当前产品面,唯一命中的是 packages/ui/src/astryx-chat-reasoning.tsx 里那个已知的 legacyBlockerBaseline 指纹——一个带 role="button"onClickdiv普通布局用的 span/div,只要不带这两样,不会进入 admissionGaps 新增同类控件会因为出现次数与 baseline 对不上而失败;原始 <button> 仍然触发 blocker 级别;诊断信息写明了路径、次数和修法。两条路径都是 fail-closed。

P3:激活按钮会打开 Inspector,但不移动焦点、也不播报。

app-shell.tsx:2995 调用 workbar.commands.openTool('inspector'),而 singleton 路径只做了 openWorkbarTabrevealPlacement(use-workbar-controller.ts:322-325),没有 focus()。面板出现了,焦点仍停在按钮上。

这不是那种「控件卸载、焦点掉到 document.body」的失效方式——按钮还在,键盘用户可以继续 Tab 进右侧栏。但屏幕阅读器用户得不到「有东西打开了」的任何信号。面板本身带 aria-label(session-inspector-panel.tsx:95),所以内容一旦找到是可达的;缺的是这个转换过程。e2e 只断言面板变为可见(session-workbar.spec.ts:54-67),不断言焦点,所以它也抓不到这里的回归。

打开后把焦点送入 inspector 区域,或者给按钮加 aria-expanded / aria-controls,都能关闭它。不构成阻塞。

证据边界:针对源码以及 Astryx ButtonuseTooltip 实现的静态审查。没有跑浏览器或 Playwright,所以 tooltip 的实际播报行为是从接线推出来的,不是观测到的。我发布时 test 仍在运行。


Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving at exact head 8f0705901de2a0b3166e61e99f656ebdee73328b. No P0 or P1 — the single P3 from my earlier comment stands and does not block.

To restate the one open item: activating the button opens the Inspector without moving focus or announcing it (app-shell.tsx:2995use-workbar-controller.ts:322-325, no focus()). The button does not unmount, so nothing is lost — a keyboard user simply gets no signal that a panel appeared. Moving focus into the inspector region, or giving the button aria-expanded / aria-controls, closes it whenever you want to.

Everything else checked out: the accessible name says what activation does while the measured detail lives in the tooltip description, the provider-facing wording is gone from all three usage strings, and the new Astryx gate matches only the known baseline fingerprint rather than ordinary layout elements.

This approval covers the review gate only. The PR is still a draft and checks had not reached a terminal state when I posted, so branch protection still applies.

简体中文

8f0705901de2a0b3166e61e99f656ebdee73328b 上批准。没有 P0/P1——先前评论里那条 P3 仍然成立,但不构成阻塞。

唯一未决项重述一下:激活按钮会打开 Inspector,但不移动焦点、也不播报(app-shell.tsx:2995use-workbar-controller.ts:322-325,没有 focus())。按钮不会卸载,所以没有东西丢失——只是键盘用户得不到「有面板出现了」这个信号。把焦点移入 inspector 区域,或给按钮加 aria-expanded / aria-controls,随时可以关闭它。

其余都核过了:无障碍名说的是「激活会做什么」,测量细节放在 tooltip 描述里;三条用量文案里面向提供方的措辞都已清除;新增的 Astryx 门禁只命中已知 baseline 指纹,不会误伤普通布局元素。

本批准只覆盖审查门禁。 PR 仍是草稿,我发布时检查尚未终态,分支保护依然生效。


Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

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.

2 participants