Skip to content

feat(web-shell): add assistant turn footer slot - #6611

Merged
dreamWB merged 3 commits into
QwenLM:mainfrom
dreamWB:codex/webshell-assistant-turn-footer
Jul 10, 2026
Merged

feat(web-shell): add assistant turn footer slot#6611
dreamWB merged 3 commits into
QwenLM:mainfrom
dreamWB:codex/webshell-assistant-turn-footer

Conversation

@dreamWB

@dreamWB dreamWB commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Adds a WebShell customization hook that lets embedders render extra React content after a completed turn's final assistant message and before the built-in assistant timestamp/actions area.

Why it's needed

Some WebShell integrations need to attach turn-level UI to the completed answer without forking or reimplementing the message list. This keeps the default transcript behavior unchanged while providing a focused extension point for host-owned content.

Reviewer Test Plan

How to verify

Confirm that WebShell still renders final assistant actions in the same location by default, and that a caller can provide renderAssistantTurnFooter to render custom content for a completed turn. The renderer should receive the completed turn id plus metadata for the final assistant message, and should not run for an in-progress latest turn.

Evidence (Before & After)

N/A. This PR adds a WebShell extension point and targeted DOM tests rather than changing the default visible UI.

Tested on

OS Status
🍏 macOS ✅ tested
🪟 Windows ⚠️ not tested
🐧 Linux ⚠️ not tested

Environment (optional)

Local macOS validation passed: npx eslint packages/web-shell/client/App.tsx packages/web-shell/client/customization.tsx packages/web-shell/client/index.ts packages/web-shell/client/index.tsx packages/web-shell/client/components/MessageList.tsx packages/web-shell/client/components/MessageList.dom.test.tsx packages/web-shell/client/components/MessageItem.tsx packages/web-shell/client/components/MessageItem.dom.test.tsx packages/web-shell/client/components/messages/AssistantMessage.tsx; npm run test --workspace=packages/web-shell -- MessageList.dom.test.tsx MessageList.test.ts MessageItem.dom.test.tsx; npm run build --workspace=packages/web-shell. Full npm run preflight was also run on this branch; it completed clean/install/format/lint/build/typecheck, then failed during packages/cli test:ci on CLI-side assertions outside this WebShell change (src/commands/extensions/list.test.ts localization output and src/ui/auth/AuthDialog.test.tsx selection/navigation expectations). Later workspaces continued and passed.

Risk & Scope

  • Main risk or tradeoff: The hook is intentionally limited to completed turns so streaming layout and built-in assistant actions remain stable.
  • Not validated / out of scope: Windows and Linux local validation are left to CI; full npm run preflight does not currently pass in this checkout because of CLI-side test:ci failures outside this WebShell change.
  • Breaking changes / migration notes: None. Existing WebShell consumers do not need to pass the new renderer.

Linked Issues

N/A

中文说明

What this PR does

为 WebShell 增加一个自定义扩展点,允许宿主在每轮已完成对话的最终 assistant 消息后、内置时间和操作区之前渲染额外 React 内容。

Why it's needed

部分 WebShell 集成方需要在已完成回答后附加 turn 级 UI,但不希望 fork 或重写消息列表。这个改动保持默认 transcript 行为不变,同时提供一个聚焦的宿主扩展点。

Reviewer Test Plan

How to verify

确认默认情况下 WebShell 的最终 assistant 操作区仍在原位置展示;当调用方传入 renderAssistantTurnFooter 时,可以为已完成 turn 渲染自定义内容。renderer 应收到已完成 turn id 和最终 assistant 消息元信息,并且不应在仍在进行中的最新 turn 上触发。

Evidence (Before & After)

N/A。这个 PR 增加的是 WebShell 扩展点和对应 DOM 测试,不改变默认可见 UI。

Tested on

OS Status
🍏 macOS ✅ tested
🪟 Windows ⚠️ not tested
🐧 Linux ⚠️ not tested

Environment (optional)

本地 macOS 验证已通过:npx eslint packages/web-shell/client/App.tsx packages/web-shell/client/customization.tsx packages/web-shell/client/index.ts packages/web-shell/client/index.tsx packages/web-shell/client/components/MessageList.tsx packages/web-shell/client/components/MessageList.dom.test.tsx packages/web-shell/client/components/MessageItem.tsx packages/web-shell/client/components/MessageItem.dom.test.tsx packages/web-shell/client/components/messages/AssistantMessage.tsxnpm run test --workspace=packages/web-shell -- MessageList.dom.test.tsx MessageList.test.ts MessageItem.dom.test.tsxnpm run build --workspace=packages/web-shell。本分支也已运行完整 npm run preflight;该命令完成了 clean/install/format/lint/build/typecheck,随后在 packages/clitest:ci 阶段失败,失败点是本次 WebShell 改动范围外的 CLI 侧断言(src/commands/extensions/list.test.ts 本地化输出,以及 src/ui/auth/AuthDialog.test.tsx 选择/导航期望)。后续 workspace 继续执行并通过。

Risk & Scope

  • Main risk or tradeoff: 这个 hook 刻意限制在已完成 turn,避免影响 streaming 布局和内置 assistant 操作区。
  • Not validated / out of scope: 未在 Windows 和 Linux 本地验证;当前 checkout 中完整 npm run preflight 因本次 WebShell 改动范围外的 CLI 侧 test:ci 失败而未通过。
  • Breaking changes / migration notes: 无。现有 WebShell 使用方不需要传入新的 renderer。

Linked Issues

N/A

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓ — all required headings present (What/Why/Reviewer Test Plan/Risk & Scope/Linked Issues/中文说明).

Problem: web-shell integrators need to attach turn-level UI to a completed answer without forking or reimplementing the message list. This is a real use case for the customization API — not a theoretical concern, and consistent with how the existing 8+ renderer hooks work.

Direction: well-aligned. The CHANGELOG shows a steady stream of web-shell customization hooks over recent releases (custom at-mention panel, onSessionChange callbacks, external split controls, etc.). This follows that exact pattern — define a renderer type, wire it through context, render at the right spot. No product-direction concerns.

Size: 132 production lines (additions + deletions), 285 test lines, 0 generated. Not a core module (web-shell only). Scope is appropriate.

Approach: focused and minimal. Every change serves the stated goal — type definitions, prop threading, turn-id tracking, rendering, exports. No drive-by refactors or scope creep. The collectFinalAssistantTurnIds rename from SetMap is the natural way to surface the turn id to the renderer.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓ — 所有必要标题齐全(What/Why/Reviewer Test Plan/Risk & Scope/Linked Issues/中文说明)。

问题:web-shell 集成方需要在已完成回答后附加 turn 级 UI,但不希望 fork 或重写消息列表。这是自定义 API 的真实需求——不是理论性顾虑,与已有的 8+ 个 renderer hook 模式一致。

方向:对齐良好。CHANGELOG 显示近期版本持续增加 web-shell 自定义钩子(自定义 @提及面板、onSessionChange 回调、外部分屏控制等)。本 PR 完全遵循该模式——定义 renderer 类型、通过 context 传递、在正确位置渲染。无产品方向顾虑。

规模:132 行生产代码(增删合计),285 行测试代码,0 行生成代码。非核心模块(仅 web-shell)。范围适当。

方案:聚焦且最小化。每处改动都服务于目标——类型定义、prop 传递、turn-id 追踪、渲染、导出。无顺手重构或范围蔓延。collectFinalAssistantTurnIdsSetMap 的重构是将 turn id 暴露给 renderer 的自然方式。

进入代码审查 🔍

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Code Review

Independent proposal: to add a post-turn footer slot, I'd define a renderer type receiving turn id and message metadata, add it to WebShellCustomization, rework the final-assistant tracking to return turn ids (not just a boolean set), and thread a new prop through MessageList → MessageItem → AssistantMessage. The PR does exactly this — the Set<string>Map<string, string> refactor on collectFinalAssistantTurnIds is the cleanest way to surface the turn id to the renderer.

Findings:

  • No correctness bugs. The footer renders only for completed turns (Map lookup returns undefined for active/no-assistant turns), which is the intended behavior. The new test cases cover exactly these edge cases: happy path, multi-turn mapping, streaming suppression, and no-assistant-message turns.
  • The useMemo dep array for customFooter correctly includes both customFooterInfo and renderAssistantTurnFooter — the callback re-derives only when either changes.
  • The areAssistantTurnFooterInfosEqual memo comparator correctly checks all five leaf fields. The follow-up commit (d28dc4f) addressed the prior review's suggestion about this coupling.
  • The renderer receives a safe subset of message fields (id, content, isStreaming, timestamp) rather than the full internal message object. Good API boundary.
  • Types are exported from both index.ts and index.tsx — consistent with the existing pattern for other renderer types.
  • The follow-up commit added a two-turn regression test and JSDoc clarifying turnId semantics. Author reasonably declined suggestion-level items (equal-values memo test, dead-guard cleanup) to keep the PR scoped.

No blockers found.

Testing

This is a library customization hook consumed programmatically by web-shell integrators — there's no standalone CLI binary to drive in tmux. Verification is through unit tests, build, and lint.

Unit tests (targeted — new feature tests)

$ cd packages/web-shell && node ../../node_modules/vitest/vitest.mjs run client/components/MessageList.dom.test.tsx client/components/MessageItem.dom.test.tsx

 RUN  v3.2.4 /packages/web-shell/client

 ✓ components/MessageItem.dom.test.tsx (8 tests) 57ms
 ✓ components/MessageList.dom.test.tsx (47 tests) 1430ms

 Test Files  2 passed (2)
      Tests  55 passed (55)
   Duration  2.58s

Full suite (regression check)

$ cd packages/web-shell && node ../../node_modules/vitest/vitest.mjs run

 Test Files  1 failed | 80 passed (81)
      Tests  5 failed | 1292 passed (1297)
   Duration  4.84s

The 5 failures are all in build-artifact.test.ts — pre-existing, caused by missing dist/index.js (build not run). Unrelated to this PR. All 1292 unit tests pass.

ESLint

$ npx eslint packages/web-shell/client/App.tsx packages/web-shell/client/customization.tsx \
    packages/web-shell/client/index.ts packages/web-shell/client/index.tsx \
    packages/web-shell/client/components/MessageList.tsx \
    packages/web-shell/client/components/MessageItem.tsx \
    packages/web-shell/client/components/messages/AssistantMessage.tsx
(clean — no output, exit 0)

All green. ✅

中文说明

代码审查

独立方案:要添加 turn 后的 footer 插槽,我会定义一个接收 turn id 和消息元数据的 renderer 类型,添加到 WebShellCustomization,将 final-assistant 追踪改为返回 turn id(而不仅仅是布尔集合),并通过 MessageList → MessageItem → AssistantMessage 传递新 prop。PR 完全采用了这个方案——collectFinalAssistantTurnIdsSet<string>Map<string, string> 的重构是将 turn id 暴露给 renderer 的最简洁方式。

发现:

  • 无正确性 bug。footer 仅在已完成的 turn 上渲染(Map 查找对活跃/无 assistant 的 turn 返回 undefined),符合预期。新测试用例精确覆盖了边界情况:正常路径、多 turn 映射、流式抑制和无 assistant 消息 turn。
  • customFooteruseMemo 依赖数组正确包含了 customFooterInforenderAssistantTurnFooter
  • areAssistantTurnFooterInfosEqual memo 比较器正确检查了所有五个叶子字段。后续提交 (d28dc4f) 处理了之前审查关于此耦合的建议。
  • renderer 接收消息字段的安全子集(idcontentisStreamingtimestamp)而非完整的内部消息对象。良好的 API 边界。
  • 后续提交增加了两轮回归测试和 turnId 语义的 JSDoc。作者合理地拒绝了建议级别的项目(等值 memo 测试、死守卫清理)以保持 PR 范围。

未发现阻断问题。

测试

这是一个由 web-shell 集成方以编程方式消费的库自定义钩子——没有独立的 CLI 可以在 tmux 中驱动。验证通过单元测试、构建和 lint 完成。

全部通过 ✅(5 个 build-artifact 失败为预存问题,缺少 dist/index.js)

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

This PR is a clean, well-scoped addition to the web-shell customization API. The implementation matches what I'd have written independently — define the renderer type, track turn ids via a Map, thread the prop through the component tree, render only on completed turns. Every change in the diff earns its place.

The test coverage is solid: the new test cases cover the happy path (footer renders on completed turns with correct turn id), multi-turn mapping (each turn gets its own turn id), streaming suppression (no footer on active turns), and the no-assistant-message case. All 1292 existing tests still pass.

The author's follow-up commit (d28dc4f) addressed the prior review's substantive suggestions — adding a two-turn regression test and JSDoc for turnId — while reasonably declining the suggestion-level items to keep scope tight. That judgment call was correct.

Approving. ✅

中文说明

这个 PR 是对 web-shell 自定义 API 的干净、范围合理的补充。实现方式与我的独立方案一致——定义 renderer 类型、通过 Map 追踪 turn id、在组件树中传递 prop、仅在已完成的 turn 上渲染。diff 中的每处改动都有存在的理由。

测试覆盖扎实:新测试用例覆盖了正常路径(已完成 turn 渲染 footer 并使用正确的 turn id)、多 turn 映射(每个 turn 获得自己的 turn id)、流式抑制(活跃 turn 不渲染 footer)和无 assistant 消息情况。全部 1292 个现有测试仍然通过。

作者的后续提交 (d28dc4f) 处理了之前审查的实质性建议——增加两轮回归测试和 turnId 的 JSDoc——同时合理地拒绝了建议级别的项目以保持范围紧凑。这个判断是正确的。

批准 ✅

Qwen Code · qwen3.7-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, looks ready to ship. ✅

Comment thread packages/web-shell/client/components/MessageList.tsx Outdated
Comment thread packages/web-shell/client/components/MessageItem.tsx Outdated
@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

No new Suggestion-level findings this round — all prior suggestions have been addressed or superseded.

— qwen3.7-max via Qwen Code /review

@dreamWB
dreamWB force-pushed the codex/webshell-assistant-turn-footer branch from 9946acb to 6a872e6 Compare July 9, 2026 15:10
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration.

中文

请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed — no new blockers this round. Two prior Critical findings (ErrorBoundary gap and memo comparator omission) remain open. Suggestion-level recommendations are in the Suggestion summary comment below.

@dreamWB

dreamWB commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Follow-up pushed in d28dc4f.

Handled from the suggestion summary:

  • Added a two-turn regression test so each final assistant footer maps to its own turn id.
  • Added JSDoc clarifying that turnId is the user-message id for the completed turn head.

I intentionally left the custom footer placement unchanged. This slot is meant to be a content-adjacent custom area after the assistant answer body; the built-in copy/branch/timestamp row is hover chrome. Moving the custom content after that action row would change the UX semantics and expand the PR beyond the requested hook.

Verification:

  • cd packages/web-shell && npx vitest run client/components/MessageList.dom.test.tsx client/components/MessageItem.dom.test.tsx
  • npx eslint packages/web-shell/client/components/MessageList.tsx packages/web-shell/client/components/MessageItem.tsx packages/web-shell/client/components/messages/AssistantMessage.tsx packages/web-shell/client/customization.tsx packages/web-shell/client/components/MessageList.dom.test.tsx packages/web-shell/client/components/MessageItem.dom.test.tsx
  • cd packages/web-shell && npm run typecheck
  • npm run build --workspace=packages/web-shell

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.

@dreamWB

dreamWB commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Reviewed the latest suggestion-only follow-up. I am not changing code for these:

  • The equal-values-different-reference test would lock down memo implementation details rather than user-visible behavior; the existing regression covers the stale-footer bug that mattered.
  • The keep-in-sync comments would mostly restate the type/comparator coupling already visible in the adjacent code.
  • The turnHead guard cleanup is true but unrelated and has no behavior impact, so I am leaving it out to keep this PR scoped.

The prior Critical threads are resolved/outdated after d28dc4f. The remaining blocking check is the existing Ubuntu CI failure in scripts/tests/qwen-autofix-workflow.test.js, which is outside this web-shell footer diff.

@dreamWB

dreamWB commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, looks ready to ship. ✅

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No issues found. LGTM! ✅

— qwen3.7-max via Qwen Code /review

@dreamWB
dreamWB added this pull request to the merge queue Jul 10, 2026
Merged via the queue into QwenLM:main with commit bb3b2f3 Jul 10, 2026
52 of 55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants