From 9b01da195a915b0a271b8bc23d2f7daa929b8a69 Mon Sep 17 00:00:00 2001 From: Chinesezjc Date: Thu, 6 Aug 2026 13:56:39 +0800 Subject: [PATCH] fix(ui-conversation): keep the summary suffix on one line `flex: none` stops the `+n` box shrinking but not its text wrapping, so a row too narrow for title + separator + suffix broke the one-line summary in the exact case the slot exists for. Add `white-space: nowrap` and pin the declaration as CSS text, since jsdom has no layout. Also align the README's row illustration with the English rendering (the same sentence's plan-strip clause already used it), and record the two deferred review findings in the Agent Note so they survive merge. --- ...-07-26-todo-parallel-in-progress.i18n.yaml | 4 +- .../2026-07-26-todo-parallel-in-progress.md | 4 ++ ...2026-07-26-todo-parallel-in-progress.zh.md | 4 ++ .../client/ui-conversation/README.i18n.yaml | 2 +- packages/client/ui-conversation/README.md | 2 +- .../src/client/chat/ToolRow.module.css | 5 ++- .../tests/tool-row-styles.spec.ts | 42 +++++++++++++++++++ 7 files changed, 58 insertions(+), 5 deletions(-) create mode 100644 packages/client/ui-conversation/tests/tool-row-styles.spec.ts diff --git a/.agents/notes/implemented/feature/2026-07-26-todo-parallel-in-progress.i18n.yaml b/.agents/notes/implemented/feature/2026-07-26-todo-parallel-in-progress.i18n.yaml index d227299170..65c10eb347 100644 --- a/.agents/notes/implemented/feature/2026-07-26-todo-parallel-in-progress.i18n.yaml +++ b/.agents/notes/implemented/feature/2026-07-26-todo-parallel-in-progress.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/feature/2026-07-26-todo-parallel-in-progress.md -2026-07-26-todo-parallel-in-progress.md: 8480107920ace22b6f79b96145bb9d2103455f5a -2026-07-26-todo-parallel-in-progress.zh.md: 81d5411e374daa64c7e112d0269ea733b15a133b +2026-07-26-todo-parallel-in-progress.md: 558dd6dda1452515ea1f1c173941a5e51e9653ef +2026-07-26-todo-parallel-in-progress.zh.md: 8aabef3e058f7dfd02ac3dcbae0214ed930440a4 diff --git a/.agents/notes/implemented/feature/2026-07-26-todo-parallel-in-progress.md b/.agents/notes/implemented/feature/2026-07-26-todo-parallel-in-progress.md index 8480107920..558dd6dda1 100644 --- a/.agents/notes/implemented/feature/2026-07-26-todo-parallel-in-progress.md +++ b/.agents/notes/implemented/feature/2026-07-26-todo-parallel-in-progress.md @@ -45,6 +45,10 @@ The row takes `planSummary` in `toolviews/plan-summary.ts`. It names the first a `summarySuffix` is a slot on `ToolRow` rather than markup owned by the todo row: every toolview renders through that shared component, whose `summary` is a plain ellipsized string with no place for a fragment that must survive the clip. Sitting outside the `.summary` rule, the suffix repeats that rule's `font-size` and `line-height` — the web shell leaves body text at the browser default rather than the row's 14px, so an unstyled span renders visibly larger than the text beside it on a 24px row. An error row drops the suffix, because its collapsed summary is the failure line rather than anything derived from the call args. +## Deferred + +Two review findings are recorded here rather than fixed on this branch. The `summarySuffix` span carries no accessible name, so a screen reader reads the count without its noun (`… 实现 fixture 样本 +1`); naming it introduces localized copy with its own test contract, which belongs to an accessibility pass over the whole `ToolRow` summary line rather than to one row. And when the *first* active item's content is unusable — missing, mistyped, or blank once trimmed — the row drops the active clause and the count with it, so a parallel plan renders as bare counts; skipping forward to the first usable active item was rejected because call args are an explicitly unvalidated boundary where model order is the only ordering the row can honour, and dropping the unusable clause alone keeps the `done`/`total` counts, which are trustworthy regardless. + ## Consequences A todo list can now faithfully mirror parallel execution, and every UI renders several active markers at once: the TUI's per-status prefix needed no change, the plan strip's header counts the active items, and the row needed the derivation above. A composition that sets `allowParallelInProgress: true` no longer rejects a formerly-invalid snapshot shape; one that sets `false` keeps the old rejection, and the durable-log invariant accepts both. The model-facing description changed, which re-recorded the tool-catalog page and every snapshot sidecar carrying the todo schema. No count is recorded here: the set grows with every pinning scenario that lands, and the two point-in-time censuses this note previously carried were both stale within days. The operative rule is that a branch changing the tool description must refresh whichever sidecars landed after it branched — including the numbered `tool-schemas..expected.json` files pinning a subagent class, whose schemas the parent scenario does not cover — and `pnpm run test:snapshot:refresh` does it keylessly over the whole corpus. The web fixture's todo sample now runs two items `in_progress`, so both fixture-driven surfaces render a parallel plan. `packages/client/ui-conversation/tests/todo-panel.spec.tsx` pins the row summary and the plan strip over src, the ACP `todo-write` scenario records a three-todo plan with two active, and `apps/web/tests/todo-row.snapshot.ts` pins both surfaces in the assembled application — booted from the built `packages/client/*/lib/client.js` bundles, so it is the one place the keyed registration and the bundled wiring are under test. That last file records `summary`, `suffix`, and the strip's header as separate fields, so folding the `+N` count back into the summary string changes the expected output even though the concatenated text would read the same. diff --git a/.agents/notes/implemented/feature/2026-07-26-todo-parallel-in-progress.zh.md b/.agents/notes/implemented/feature/2026-07-26-todo-parallel-in-progress.zh.md index 81d5411e37..8aabef3e05 100644 --- a/.agents/notes/implemented/feature/2026-07-26-todo-parallel-in-progress.zh.md +++ b/.agents/notes/implemented/feature/2026-07-26-todo-parallel-in-progress.zh.md @@ -45,6 +45,10 @@ Status: implemented `summarySuffix` 是 `ToolRow` 上的槽位,而不是 todo 工具行自有的标记:每个 toolview 都经由这个共享组件渲染,而它的 `summary` 是一个会被省略号截断的普通字符串,容不下一个必须挺过截断的片段。该后缀落在 `.summary` 规则之外,因此重复了该规则的 `font-size` 与 `line-height`——Web 外壳把正文字号留在浏览器默认值而非该行的 14px,所以未加样式的 span 会明显大于同一 24px 行内与之并列的文本。错误行会丢弃该后缀,因为它折叠态的摘要是失败行,而非任何由调用 args 推导出的内容。 +## 暂缓项 + +两条 review 结论在此记录而非在本分支修复。`summarySuffix` 这个 span 没有无障碍名称,屏幕阅读器读出的数量缺少它所修饰的名词(`… 实现 fixture 样本 +1`);为它命名会引入带自身测试契约的本地化文案,这属于对整条 `ToolRow` 摘要行做的无障碍专项,而不属于某一行。以及,当*第一个*活跃条目的 content 不可用时——缺失、类型不对、或 trim 后为空——行会连同数量一起丢掉活跃子句,于是并行计划渲染成裸计数;向后跳到第一个可用活跃条目的方案被否决,因为调用 args 是一处明确未经校验的边界,模型给出的顺序是该行唯一能遵循的顺序,而只丢掉不可用的那个子句可以保住 `done`/`total` 计数——这两个数无论如何都是可信的。 + ## 后果 现在 todo 列表可以忠实反映并行执行,并且每个 UI 都能一次渲染多个活跃标记:TUI 按状态区分的前缀无需改动,计划横条的表头会计数活跃条目,工具行则需要上述推导。设置 `allowParallelInProgress: true` 的组合不再拒绝一种此前无效的快照形状;设置为 `false` 的组合仍保留旧的拒绝行为,而持久日志不变式两者都接受。面向模型的描述发生了变化,这重新记录了 tool-catalog 页面以及每个带有 todo schema 的快照 sidecar。此处不记录数量:该集合会随每个新落地的 pin 场景增长,而本 Note 先前记过的两次点时刻计数都在几天内失实。有效规则是:改动工具描述的分支必须刷新它分叉之后落地的那些 sidecar —— 包括固定 subagent 类工具的编号文件 `tool-schemas..expected.json`,其 schema 不被父场景覆盖 —— `pnpm run test:snapshot:refresh` 可以无 key 地对整个语料完成刷新。web fixture 的 todo 样本现在有两个条目处于 `in_progress`,因此两个由 fixture 驱动的展示面渲染的都是并行计划。`packages/client/ui-conversation/tests/todo-panel.spec.tsx` 在 src 上固定工具行摘要与计划横条,ACP `todo-write` 场景录制的是三条目、两个活跃的计划,而 `apps/web/tests/todo-row.snapshot.ts` 在组装后的应用中固定这两个面——它从构建产物 `packages/client/*/lib/client.js` 启动,因此是唯一覆盖 keyed 注册与打包接线的地方。该文件把 `summary`、`suffix` 与横条表头记录为独立字段,因此即便拼接后的文本读起来一样,把 `+N` 计数折回摘要字符串也会改变预期输出。 diff --git a/packages/client/ui-conversation/README.i18n.yaml b/packages/client/ui-conversation/README.i18n.yaml index 171e6967bc..0e49265ca1 100644 --- a/packages/client/ui-conversation/README.i18n.yaml +++ b/packages/client/ui-conversation/README.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write packages/client/ui-conversation/README.md -README.md: b4cbfc9c9fde730ff44fc63bd87194b37d518919 +README.md: 0c9ea8af211826e86412504674b0ca07536c822c README.zh.md: f11fb1133655ce8ee73507bc926a38e1e5274e4e diff --git a/packages/client/ui-conversation/README.md b/packages/client/ui-conversation/README.md index b4cbfc9c9f..0c9ea8af21 100644 --- a/packages/client/ui-conversation/README.md +++ b/packages/client/ui-conversation/README.md @@ -34,7 +34,7 @@ A `grep`/`glob` call declaring the `search` render intent renders its result inl Tool rows use the keyed, session-scoped `'conversation.chat.toolview'` slot; its render site dispatches via `entryKey: toolName` with `GenericToolCard` as the call-site fallback. The owner payload is the uniform `ToolRowOwnerProps` (`callId`/`toolName`/`block`/`openFile`), and `ToolRowProps` composes it with the session standard kit. A registrant is a plain plugin with only the slot service edge: `ctx.slots.inject('conversation.chat.toolview', () => ctx.slots.register({ name: 'conversation.chat.toolview', key: '', inject? }, Row))`. The declaration is the activation and reload dependency; `ConversationService` is required only by registrations that call its actions. Trajectory and waterfall toolview slots share this shape and use their own render sites; RendersCheck rejects a declaration nobody renders. -The todo surfaces are two registrations over that shape, both using slot declaration injection without a `ConversationService` edge. `TodoRow` takes the `'conversation.chat.toolview'` key `todo_write` and summarizes what the call attempted (`/ 已完成 · ` plus a `+` count of the other active ones, parsed from its args through `toolviews/plan-summary.ts` `planSummary`, falling back to the generic summary on malformed or wrongly-shaped model JSON, and keeping the generic dot for non-ok execution states so a cancelled call never reads as a completed update). When the deployment permits parallel work, several items may be `in_progress` at once, so `planSummary` names the first and counts the rest, and deliberately returns the two unjoined: the row ellipsizes its summary text, so a count concatenated onto the end of the task name would be the first thing a narrow row clips. The row hands the count to `ToolRow`'s `summarySuffix`, the shared row's non-shrinking slot beside that ellipsized text (an error row drops it, since its collapsed summary is the failure line). `TodoDock` takes the `'conversation.input.dock'` list slot at `order: 0` — before Goal and Queue — and is the plan strip: it reads the host-computed `todos` projection via `useProjection` (standing plan: latest `todo/write` with no later `turn/start`) and renders `TodoPanel`, which takes the plain list, hides itself while the list is empty, and starts collapsed as a header of title plus its own `·`-joined per-status counts (localized, `1 completed · 2 in progress · 1 pending`, zero-count segments omitted; status glyphs are the figma check / progress / dashed-pending set), so it reports the parallel count without needing a name to truncate. The dock adapter owns the selection so the panel stays a pure function of its props; the standing list lives here rather than in the row so the row stays one line. Anything the input-zone composer chain hides (a `conversation.composer` takeover such as ui-question's) hides the whole dock, this strip included. +The todo surfaces are two registrations over that shape, both using slot declaration injection without a `ConversationService` edge. `TodoRow` takes the `'conversation.chat.toolview'` key `todo_write` and summarizes what the call attempted (`/ completed · ` plus a `+` count of the other active ones, parsed from its args through `toolviews/plan-summary.ts` `planSummary`, falling back to the generic summary on malformed or wrongly-shaped model JSON, and keeping the generic dot for non-ok execution states so a cancelled call never reads as a completed update). When the deployment permits parallel work, several items may be `in_progress` at once, so `planSummary` names the first and counts the rest, and deliberately returns the two unjoined: the row ellipsizes its summary text, so a count concatenated onto the end of the task name would be the first thing a narrow row clips. The row hands the count to `ToolRow`'s `summarySuffix`, the shared row's non-shrinking slot beside that ellipsized text (an error row drops it, since its collapsed summary is the failure line). `TodoDock` takes the `'conversation.input.dock'` list slot at `order: 0` — before Goal and Queue — and is the plan strip: it reads the host-computed `todos` projection via `useProjection` (standing plan: latest `todo/write` with no later `turn/start`) and renders `TodoPanel`, which takes the plain list, hides itself while the list is empty, and starts collapsed as a header of title plus its own `·`-joined per-status counts (localized, `1 completed · 2 in progress · 1 pending`, zero-count segments omitted; status glyphs are the figma check / progress / dashed-pending set), so it reports the parallel count without needing a name to truncate. The dock adapter owns the selection so the panel stays a pure function of its props; the standing list lives here rather than in the row so the row stays one line. Anything the input-zone composer chain hides (a `conversation.composer` takeover such as ui-question's) hides the whole dock, this strip included. `QueueDock` is the terminal input-dock entry at `order: 20`. It hides while empty, renders one pending row directly, and defaults two or more rows to a collapsed `" 条排队消息"` header whose button expands or collapses the complete list. The header exposes `aria-expanded` and `aria-controls`; the expanded list scrolls within a 180px height bound. An active edit or mutation keeps its rows visible, and emptying the queue restores the collapsed default for the next queue. Each visible ordinary-session row remains a single-line preview with its exact-occurrence edit, delete, and strict-steer actions; addressed subagents retain the rows as a read-only projection because their continuation transport does not expose queue mutation. If strict steer loses to a closed window, the original occurrence remains queued for normal delivery; if the driver already claimed it, normal delivery is already underway. Neither converged race displays a failure, while transport and unknown failures do. diff --git a/packages/client/ui-conversation/src/client/chat/ToolRow.module.css b/packages/client/ui-conversation/src/client/chat/ToolRow.module.css index c62949b0f3..78dfdca877 100644 --- a/packages/client/ui-conversation/src/client/chat/ToolRow.module.css +++ b/packages/client/ui-conversation/src/client/chat/ToolRow.module.css @@ -91,10 +91,13 @@ /* Trailing summary fragment kept out of .summary's ellipsis, for a count whose whole value is that it survives a narrow row (the todo row's parallel-active - `+n`). Repeats .summary's type because it sits beside that text. */ + `+n`). Repeats .summary's type because it sits beside that text, and its + `nowrap` too: `flex: none` stops the box shrinking but not the text wrapping, + which would break the one-line row in the narrow case the slot exists for. */ .summarySuffix { flex: none; margin-left: 4px; + white-space: nowrap; font-size: 14px; line-height: 24px; color: var(--dsw-alias-label-tertiary); diff --git a/packages/client/ui-conversation/tests/tool-row-styles.spec.ts b/packages/client/ui-conversation/tests/tool-row-styles.spec.ts new file mode 100644 index 0000000000..50ac6b7886 --- /dev/null +++ b/packages/client/ui-conversation/tests/tool-row-styles.spec.ts @@ -0,0 +1,42 @@ +/** + * The one-line contract of the ToolRow summary line as CSS text. jsdom has no + * layout, so the rendering specs (chat-tool-row.spec.tsx) can pin which spans + * exist but not whether a narrow row still fits on one line; these read the + * declarations the layout depends on. + */ +import { readFileSync } from 'node:fs' +import { fileURLToPath } from 'node:url' +import { describe, expect, it } from 'vitest' + +const css = readFileSync(fileURLToPath(new URL('../src/client/chat/ToolRow.module.css', import.meta.url)), 'utf8') +/** Declarations only: the sheet's prose names the properties it explains. */ +const declarationText = css.replace(/\/\*[\s\S]*?\*\//g, ' ') + +function declarations(selector: string): string[] { + const rule = new RegExp(`\\${selector}\\s*\\{([^{}]*)\\}`).exec(declarationText) + if (rule === null) throw new Error(`ToolRow.module.css has no \`${selector}\` rule`) + return (rule[1] ?? '').split(';').map(part => part.trim()).filter(Boolean) +} + +describe('ToolRow.module.css summary line', () => { + it('keeps the summary suffix on one line and unshrunk', () => { + // `flex: none` stops the box shrinking, not the text wrapping: without + // `nowrap`, a row too narrow for title + separator + suffix wraps the `+n` + // onto a second line — the exact case the slot exists to survive. + expect(declarations('.summarySuffix')).toEqual(expect.arrayContaining([ + 'flex: none', + 'white-space: nowrap', + ])) + }) + + it('leaves the truncation to the summary text alone', () => { + // The suffix must never ellipsize: a clipped count reads as a smaller + // number rather than as missing information. + expect(declarations('.summary')).toEqual(expect.arrayContaining([ + 'overflow: hidden', + 'text-overflow: ellipsis', + 'white-space: nowrap', + ])) + expect(declarations('.summarySuffix')).not.toEqual(expect.arrayContaining(['text-overflow: ellipsis'])) + }) +})