feat(web-shell): render composer references in user messages - #6537
Conversation
|
Thanks for the PR! (Re-run after split-pane fix.) Template looks good ✓ Problem: Observed UI bug — issue #6536 documents that composer Direction: Aligned. The fix improves transcript readability by keeping UI rendering consistent between the composer and sent messages. The annotation-based approach is the right call — text parsing alone can't reliably distinguish Size: ~1,085 production logic lines across 4 packages ( Approach: The scope expanded from the initial text-parsing version to a full annotation pipeline, driven by review feedback that exposed fundamental problems with text-only parsing. The consolidation of All previously flagged issues resolved. Moving to verification. ✅ 中文说明感谢贡献!(split-pane 修复后的 re-run。) 模板完整 ✓ 问题: 已观测到的 UI bug — issue #6536 记录了 composer 方向: 对齐。修复通过保持 chip 渲染一致性来提升会话记录可读性。基于 annotation 的方案是正确的选择。 规模: 跨 4 个包约 1,085 行生产代码,加上 396 行设计文档和约 929 行测试。未触及核心路径。1000+ 行大 PR 建议适用。 方案: 从纯文本解析扩展为完整 annotation 管道,由 review 反馈驱动。 所有之前标记的问题均已解决。进入验证阶段 ✅ — Qwen Code · qwen3.7-max |
Code ReviewThe implementation is clean and follows the design doc faithfully. The annotation pipeline threads My independent proposal before reading the diff: add a Previously flagged critical — now resolved:
Remaining suggestion-level item (non-blocking):
Test ResultsReal-Scenario VerificationThis is a browser-rendered WebShell feature — CLI-level tmux testing is not applicable (chip rendering only appears in the browser). Maintainer
中文说明代码审查实现清晰,忠实遵循设计文档。annotation 管道将 独立方案与 PR 方案一致。PR 超额覆盖了 queued prompt、bridge echo、split-pane 转发、宿主渲染器覆盖和旧 transcript 向后兼容。 之前标记的 Critical 问题——已解决:
剩余 Suggestion 级项目(不阻塞):
测试结果全部通过:web-shell 177 个测试、sdk-typescript 261 个测试、acp-bridge 386 个测试。TypeScript 和 ESLint 均无错误。 真实场景验证这是浏览器渲染的 WebShell 功能——CLI 级别的 tmux 测试不适用。维护者 — Qwen Code · qwen3.7-max |
|
This PR has been through multiple review rounds and every flagged issue is now resolved. The split-pane annotation gap — the last Critical — was fixed in My independent proposal matched the PR's approach, and the implementation exceeds it. The annotation pipeline is well-layered: each step validates defensively, so corrupted metadata degrades to plain text rather than hiding content. The 824 tests pass across 3 packages. TypeScript and ESLint are clean. Maintainer Approving. ✅ 中文说明这个 PR 经过多轮 review,所有标记的问题均已解决。最后一个 Critical(split-pane annotation 缺口)在 独立方案与 PR 方案一致,实现超额覆盖。annotation 管道分层良好,每一步都有防御性校验。 3 个包 824 个测试全部通过。TypeScript 和 ESLint 均无错误。维护者 通过 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
MessageItem.tsx:292-297 |
areMessagesEqual checks content and images for user messages but omits inputAnnotations. Line 60 passes message.inputAnnotations to UserMessage, but the memo comparator won't detect annotation changes, causing stale plain-text renders instead of chips. |
Add annotation comparison (length + per-entry reference.id check) to the 'user' branch, similar to stableImagesEqual. |
App.tsx:3429-3431 |
The /plan slash command's active-session path calls sendPrompt(prompt, images, { clearComposerOnPromptStart: true }) but drops inputAnnotations. The no-session path at lines 3414-3418 correctly forwards them. |
Add inputAnnotations: metadata?.inputAnnotations to the options object. |
actions.ts:289-305, 372-388 |
The inputAnnotations normalization block (~12 lines) is copy-pasted identically between sendPrompt and submitPrompt. |
Extract a shared helper like buildPromptRequestWithAnnotations(text, normalizedImages, options). (carried from prior review) |
— qwen3.7-max via Qwen Code /review
| serialized: raw, | ||
| }; | ||
| } | ||
| if (isCustomProviderReference(raw)) return null; |
There was a problem hiding this comment.
[Critical] This fallback turns every boundary @... token that was not already handled into a file reference. Normal text such as @types/node or @alice will be displayed as a file chip and lose the visible @; selected MCP resources are also emitted by the composer as escaped @server\:uri text, for example @docs\:res\://doc, so they become file chips instead of MCP chips. The transcript no longer faithfully shows what the user sent, and the PR's MCP-resource case is still misrendered.
Please only chipify tokens the renderer can confidently identify as built-in references. Either preserve the accepted composer tag metadata/ranges, or explicitly recognize the existing MCP resource serialization and leave other ambiguous @ tokens as text. Add regressions for both a plain package/handle token like @types/node and an escaped MCP resource like @docs\\:res\\://doc.
— GPT-5 Codex via Qwen Code /review
| let index = start + 1; | ||
| while (index < content.length) { | ||
| const char = content[index]; | ||
| if (char === '\\' && index + 1 < content.length) { |
There was a problem hiding this comment.
[Suggestion] The escape branch advances by only two UTF-16 code units total. For an escaped non-BMP character, such as an emoji in a file or resource name, that skips the backslash and the high surrogate but leaves the low surrogate behind; the next regex check then stops the reference early and renders a truncated chip.
Advance over the full escaped code point instead of one code unit after the backslash, or iterate the reference text by code point, and add a regression for an escaped emoji path/resource.
— GPT-5 Codex via Qwen Code /review
wenshao
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
UserMessage.tsx:46-58 |
React UserMessageReferenceChip and CodeMirror ComposerTagWidget.toDOM have divergent fallback logic for custom tags with a label but empty value. React shows "Dataset" + "custom:1" while CodeMirror shows only "Dataset". |
Align the React chip with CodeMirror's logic: only show fallback when both tagLabel and tagValue are empty. Or update CodeMirror to match React. |
UserMessage.module.css:118-120 |
Dead CSS rule: .referenceIcon + .referenceValue { padding-left: 0.5ch } sets the same value already in the base .referenceValue shorthand. No visual effect. |
Remove the dead rule. |
— qwen3.7-max via Qwen Code /review
|
Addressed the latest review feedback in the pushed commits:
I intentionally left @skill: out of scope because WebShell currently references skills through / commands rather than @. Could you please take another look once the workflow is approved? |
|
|
||
| // User messages persist only text, so ambiguous mentions like @alice or | ||
| // @types/node must remain text. Only chipify file references with path cues. | ||
| function isBuiltInFileReference(raw: string): boolean { |
There was a problem hiding this comment.
[Critical] This heuristic still drops valid built-in file references that do not have one of these path cues. The built-in file provider serializes selected files directly as @${path} , so common files such as @Makefile, @LICENSE, or @src/Makefile reach this function but fail every check here and remain raw text after the message is sent. That means a file chip can be visible in the composer and then disappear from the transcript. Please either serialize built-in file references with an unambiguous marker (for example @./Makefile / @./src/Makefile) or preserve enough tag metadata for user-message rendering, and add coverage for extensionless root and nested files.
— GPT-5 Codex via Qwen Code /review
| } | ||
| const end = readAtReferenceEnd(content, index); | ||
| if (end === index + 1) continue; | ||
| const referenceEnd = trimReferenceTrailingPunctuation(content, end); |
There was a problem hiding this comment.
[Critical] referenceEnd is trimmed before escaped MCP resources are classified, so a real trailing . or : in the resource URI gets moved outside the chip. The composer can serialize MCP resources with trailing punctuation, for example @docs\:res\://doc., but this path slices it to @docs\:res\://doc, producing a chip for docs:res://doc plus a separate . text node. That makes the transcript show a different resource identity than the one selected/sent. Please detect escaped MCP resource references before trimming, or make the trimming step skip escaped resource candidates, and add a regression test for a resource URI ending in punctuation.
— GPT-5 Codex via Qwen Code /review
| let nextEnd = end; | ||
| while (nextEnd > 0) { | ||
| const last = content[nextEnd - 1]; | ||
| if ( |
There was a problem hiding this comment.
[Critical] trimReferenceTrailingPunctuation is not escape-aware. When a reference ends with an escaped . or : (e.g. @path\: or @file\.), the trimmer strips the literal . or : but leaves the preceding \ dangling, corrupting the chip.
For @path\:: readAtReferenceEnd correctly consumes \: as an escape pair (end=7), but the trimmer sees raw : at index 6 and strips it → raw becomes @path\ (dangling backslash).
For @file\.: the trimmer strips ., leaving @file\ which then fails isBuiltInFileReference, causing the reference to disappear entirely from the rendered chip.
| if ( | |
| function trimReferenceTrailingPunctuation( | |
| content: string, | |
| end: number, | |
| ): number { | |
| let nextEnd = end; | |
| while (nextEnd > 0) { | |
| const last = content[nextEnd - 1]; | |
| if ( | |
| (last !== '.' && last !== ':') || | |
| !isAtReferenceBoundary(content[nextEnd]) | |
| ) { | |
| break; | |
| } | |
| let backslashes = 0; | |
| let i = nextEnd - 2; | |
| while (i >= 0 && content[i] === '\\') { | |
| backslashes += 1; | |
| i -= 1; | |
| } | |
| if (backslashes % 2 === 1) break; | |
| nextEnd -= 1; | |
| } | |
| return nextEnd; | |
| } |
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running.
— qwen3.7-max via Qwen Code /review
|
Update after latest push (5bb47de) Background: the PR now relies on structured input annotations instead of reconstructing user-message chips from serialized prompt text. During local verification I found one missing handoff in the submit path: CodeMirror inline chips selected through the composer @ flow were visible in the editor, but the submit logic only used the outer composer tag list when building inputAnnotations. What changed: the latest commit includes inline CodeMirror tags when creating inputAnnotations for the submitted prompt. This keeps the prompt text unchanged while making the local user-message echo and the replayed transcript carry the same structured reference metadata. Verification:
Scope note: this remains focused on built-in WebShell @ references. Prompt text format is unchanged, and custom-provider rendering still requires either structured metadata or a host-provided renderer. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
…-reference-chips # Conflicts: # packages/web-shell/client/components/ChatEditor.tsx # packages/web-shell/client/components/messages/UserMessage.module.css # packages/web-shell/client/components/messages/UserMessage.tsx # packages/web-shell/client/hooks/useComposerCore.ts
| ) => readonly string[] | undefined | null; | ||
|
|
||
| export interface WebShellCustomization { | ||
| composerTagIcons?: WebShellComposerTagIconMap; |
There was a problem hiding this comment.
[Critical] Duplicate composerTagIcons property causes TS2300 build failure. This declaration was added at line 392 alongside the new renderComposerTag/renderComposerTagTooltip/onComposerTagClick fields, but the pre-existing composerTagIcons declaration at line 398 (from upstream/main) was not removed. After merge, both copies are present.
| composerTagIcons?: WebShellComposerTagIconMap; | |
| renderToolHeaderExtra?: ToolHeaderExtraRenderer; |
Remove the duplicate at line 392 (keep the original at line 398), or vice versa — only one declaration should remain.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no new blockers. The previously reported Critical (duplicate composerTagIcons) still stands. New Suggestion-level findings are in the Suggestion summary comment below.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
[Critical] /plan session-exists branch drops inputAnnotations. In handleSubmit, when connectionRef.current.sessionId is truthy, the /plan handler calls sendPrompt(prompt, images, { clearComposerOnPromptStart: true }) without forwarding metadata?.inputAnnotations. Every other prompt-bearing slash-command path (including /plan's own no-session branch via submitPromptFromEditor) passes annotations. The fix is to add inputAnnotations: metadata?.inputAnnotations to the opts at the session-exists sendPrompt call (around line 3429).
— qwen3.7-max via Qwen Code /review
| @@ -415,7 +425,12 @@ export function useQueuedPrompts({ | |||
| ); | |||
There was a problem hiding this comment.
[Suggestion] insertQueuedPrompt calls sessionActions.enqueueMidTurnMessage(prompt.text, { signal }) without passing prompt.inputAnnotations, and restoreTextToEditor(text, images, sessionId) does not accept annotations. Queued prompts with @ references lose their annotation metadata when inserted mid-turn or restored to the editor for editing.
| ); | |
| // Consider extending enqueueMidTurnMessage and restoreTextToEditor | |
| // to accept and forward inputAnnotations |
— qwen3.7-max via Qwen Code /review
| mimeType: | ||
| img.mimeType || img.mediaType || img.media_type || 'image/*', | ||
| })); | ||
| const inputAnnotations = |
There was a problem hiding this comment.
[Suggestion] The inputAnnotations extraction, validation, appendLocalUserMessage forwarding, and _meta injection are copy-pasted verbatim between sendPrompt and submitPrompt. This follows the pre-existing image normalization duplication. A future fix to the annotation-forwarding logic must be applied in both places.
| const inputAnnotations = | |
| // Consider extracting a shared helper, e.g.: | |
| // preparePromptWithAnnotations(text, images, options) |
— qwen3.7-max via Qwen Code /review
| role: 'user', | ||
| content: textBlock.text, | ||
| timestamp: blockTime, | ||
| ...(source ? { source } : {}), |
There was a problem hiding this comment.
[Suggestion] This only checks Array.isArray(meta?.inputAnnotations) and casts the entire array without per-element validation. The design doc specifies per-annotation sanitization at this boundary: non-empty string id/type/text, finite numeric start/end within bounds, and reference payload restricted to known fields. The renderer's splitComposerTagContentByAnnotations does perform bounds checks, but malformed annotations from a corrupt transcript still propagate to the message layer.
| ...(source ? { source } : {}), | |
| const rawAnnotations = Array.isArray(meta?.inputAnnotations) | |
| ? meta.inputAnnotations | |
| : undefined; | |
| const inputAnnotations = rawAnnotations?.filter( | |
| (a: unknown) => | |
| a && typeof a === 'object' && | |
| typeof (a as Record<string, unknown>).type === 'string' && | |
| typeof (a as Record<string, unknown>).text === 'string' && | |
| typeof (a as Record<string, unknown>).start === 'number' && | |
| typeof (a as Record<string, unknown>).end === 'number' | |
| ) as DaemonInputAnnotation[] | undefined; |
— qwen3.7-max via Qwen Code /review
| for (const tag of tags) { | ||
| const serialized = getComposerTagSerialized(tag); | ||
| if (!serialized) continue; | ||
| const start = content.indexOf(serialized, cursor); |
There was a problem hiding this comment.
[Suggestion] Missing test coverage for: (1) createInputAnnotationsFromComposerTags with empty tags array, (2) tag whose serialized text is not found in content (indexOf returns -1), (3) splitComposerTagContentByAnnotations with overlapping annotations. The if (start < 0) continue skip path is untested, and the start < cursor overlap skip is never exercised.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
| tags: readonly WebShellComposerTag[], | ||
| ): DaemonInputAnnotation[] { | ||
| const annotations: DaemonInputAnnotation[] = []; | ||
| let cursor = 0; |
There was a problem hiding this comment.
[Suggestion] Test
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
| (textBlock as ExtendedDaemonTextTranscriptBlock).meta, | ||
| ); | ||
| const source = getString(meta, 'source'); | ||
| const inputAnnotations = Array.isArray(meta?.inputAnnotations) |
There was a problem hiding this comment.
[Suggestion] Array.isArray(meta?.inputAnnotations) validates the container, then casts with as DaemonInputAnnotation[] without per-item checks. While splitComposerTagContentByAnnotations in the renderer has its own defensive validation (bounds, overlap, text-match), a corrupted transcript block could still pass malformed items through. Consider adding a lightweight filter:
| const inputAnnotations = Array.isArray(meta?.inputAnnotations) | |
| const inputAnnotations = Array.isArray(meta?.inputAnnotations) | |
| ? (meta.inputAnnotations as unknown[]).filter( | |
| (a): a is DaemonInputAnnotation => | |
| typeof a === 'object' && a !== null && | |
| 'type' in a && typeof (a as Record<string, unknown>).type === 'string', | |
| ) as DaemonInputAnnotation[] | |
| : undefined; |
— qwen3.7-max via Qwen Code /review
| mimeType: | ||
| img.mimeType || img.mediaType || img.media_type || 'image/*', | ||
| })); | ||
| const inputAnnotations = |
There was a problem hiding this comment.
[Suggestion] The inputAnnotations extraction, appendLocalUserMessage forwarding, and _meta injection (lines ~290–305) are duplicated verbatim in submitPrompt (~373–388). This extends the pre-existing image normalization duplication. If a future change updates one path but misses the other, the two entry points will silently diverge.
Consider extracting the shared logic into a helper, e.g. preparePromptRequest(text, normalizedImages, options).
— qwen3.7-max via Qwen Code /review
| tags: readonly WebShellComposerTag[], | ||
| ): DaemonInputAnnotation[] { | ||
| const annotations: DaemonInputAnnotation[] = []; | ||
| let cursor = 0; |
There was a problem hiding this comment.
[Suggestion] createInputAnnotationsFromComposerTags uses a forward-only cursor (content.indexOf(serialized, cursor)) which implicitly assumes tags is ordered by position of appearance in content. Out-of-order tags would be silently dropped. In practice the composer inserts tags left-to-right, but this invariant is undocumented.
Consider either sorting the input by position before iterating, or adding a unit test with out-of-order tags to codify the expected behavior.
— qwen3.7-max via Qwen Code /review
| @@ -428,6 +443,7 @@ export function useQueuedPrompts({ | |||
| sessionId: targetSessionId, | |||
There was a problem hiding this comment.
[Suggestion] inputAnnotations is stored by reference in the QueuedPrompt object, while images is defensively copied via const queuedImages = images ? [...images] : undefined a few lines above. If a caller mutates the annotations array after calling enqueuePrompt, the stored copy would be silently corrupted. Mirror the images pattern:
| sessionId: targetSessionId, | |
| const queuedAnnotations = inputAnnotations ? [...inputAnnotations] : undefined; |
and use queuedAnnotations in the QueuedPrompt object and downstream calls.
— qwen3.7-max via Qwen Code /review
| expect( | ||
| getComposerTagViewModel({ | ||
| id: 'custom:1', | ||
| label: ' Dataset ', |
There was a problem hiding this comment.
[Suggestion] createInputAnnotationsFromComposerTags is only tested with positive cases (1 tag, 3 tags). Consider adding tests for:
- Empty
tagsarray → should return[] - Tag whose serialized text is not found in
content(indexOfreturns -1) → should skip that tag - Two tags with identical serialized text → cursor-based dedup behavior
— qwen3.7-max via Qwen Code /review
✅ Maintainer verification — built & tested locallyI pulled this PR at its head ( Environment: macOS (Darwin 24.6), Node v22.23.1, isolated git worktree at the PR head. What the PR does (as verified)Built‑in composer I traced and confirmed the full data path: I confirmed this both with an isolated Node repro against the built SDK ( Automated tests (all green, run in the isolated worktree)
The 5 End‑to‑end visual verification (real render, not a mock)I replayed a transcript through the real normalizer → store →
Dark Light Notes
Verdict: functionally verified — LGTM from a verification standpoint. 👍 中文说明(点击展开)✅ 维护者本地验证 —— 已构建并实测我把该 PR 在其 head( 环境:macOS(Darwin 24.6)、Node v22.23.1、检出在 PR head 的独立 git worktree。 PR 的作用(已验证)通过 composer 选择的内置 我完整跟踪并确认了数据链路: 我用两种方式确认:针对已构建 SDK 的独立 Node 复现( 自动化测试(均通过,运行于独立 worktree)
端到端可视化验证(真实渲染,非 mock)我在 headless Chromium 中,让一段 transcript 走真实的 normalizer → store →
截图见上方英文部分(Dark / Light)。 备注
结论: 功能验证通过 —— 从验证角度 LGTM。👍 Verification performed locally by a maintainer; screenshots hosted on the |
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
[Critical] ChatPane.tsx handleSubmit (line ~152) still declares only 3 parameters (text, images, commitAccepted), so the 4th metadata argument from useComposerCore — carrying inputAnnotations — is silently dropped. Split-view panes (via SplitView.tsx line 324) never forward annotations to actions.sendPrompt or enqueuePrompt, so @ references submitted from a split view render as plain serialized text instead of chips. App.tsx's handleSubmit was updated to accept and forward metadata, but ChatPane was not. Fix: add metadata?: ComposerSubmitMetadata as the 4th parameter and pass metadata?.inputAnnotations to both sendPrompt and enqueuePrompt call sites.
| @@ -3391,6 +3415,7 @@ export function App({ | |||
| prompt, | |||
There was a problem hiding this comment.
[Suggestion] This forwards metadata?.inputAnnotations alongside the stripped prompt, but the annotation start/end offsets reference the original text (e.g., /plan fix @file → annotation at start:12), while the stripped prompt is shorter (fix @file → @file at position 4). splitComposerTagContentByAnnotations defensively skips annotations whose content.slice(start, end) doesn't match, so chips silently degrade to plain text.
Consider adjusting annotation offsets after stripping the slash-command prefix, or filtering out annotations that fall outside the stripped text's bounds.
— qwen3.7-max via Qwen Code /review
| prompt.isEditing === other.isEditing && | ||
| prompt.isRemoving === other.isRemoving && | ||
| (prompt.images?.length ?? 0) === (other.images?.length ?? 0) | ||
| (prompt.images?.length ?? 0) === (other.images?.length ?? 0) && |
There was a problem hiding this comment.
[Suggestion] areQueuedPromptsEqual compares inputAnnotations only by array length. Two prompts with the same annotation count but different references (different files, offsets, or text) are treated as equal, which can cause the queued-prompt list to skip re-rendering when annotation content changes.
| (prompt.images?.length ?? 0) === (other.images?.length ?? 0) && | |
| (prompt.images?.length ?? 0) === (other.images?.length ?? 0) && | |
| (prompt.inputAnnotations?.length ?? 0) === | |
| (other.inputAnnotations?.length ?? 0) && | |
| (prompt.inputAnnotations ?? []).every( | |
| (a, i) => | |
| a.text === other.inputAnnotations?.[i]?.text && | |
| a.start === other.inputAnnotations?.[i]?.start, | |
| ) |
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
| } | ||
| segments.push({ | ||
| type: 'reference', | ||
| tag: { |
There was a problem hiding this comment.
[Critical] reference.id (and reference.kind, reference.label, etc.) is accessed after destructuring without a null guard on reference itself. This function receives data from the daemon replay path (transcriptToMessages.ts → UserMessage), where Array.isArray + as DaemonInputAnnotation[] is the only boundary check. A malformed annotation missing the reference field would produce a TypeError and crash the entire message list rendering.
| tag: { | |
| if (annotation.type !== 'reference') continue; | |
| const { start, end, reference, text } = annotation; | |
| if ( | |
| !reference || | |
| typeof reference.id !== 'string' || | |
| start < cursor || | |
| end <= start || | |
| end > content.length || | |
| content.slice(start, end) !== text | |
| ) { | |
| continue; | |
| } |
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
✅ Maintainer re-verification at head
|
| Commit | Change | How I verified it |
|---|---|---|
b4295bb85 |
test(web-shell): cover composer annotation edge cases | +125 lines of composerTag tests now run & pass |
6c4e142d4 |
fix(web-shell): forward split pane prompt annotations | ChatPane now threads metadata.inputAnnotations into both sendPrompt and enqueuePrompt; covered by the new ChatPane.test.tsx cases (all green) |
29fa00100 |
fix(web-shell): guard malformed input annotations | splitComposerTagContentByAnnotations now drops any annotation with no reference or a non-string id and degrades to plain text instead of mis-rendering — the new leaves malformed reference annotations as text case passes |
Automated tests — all green in the isolated worktree
| Suite | Result |
|---|---|
web-shell full unit suite (83 files) |
1383 / 1383 ✅ |
acp-bridge bridge |
386 / 386 ✅ |
sdk-typescript daemonUi |
261 / 261 ✅ |
webui daemon/session (7 files) |
200 / 200 ✅ |
Focused PR tests (composerTag + UserMessage) |
40 / 40 ✅ |
| ESLint (PR-touched web-shell files) | clean ✅ |
tsc --noEmit (web-shell) |
clean ✅ |
npm run build (web-shell prod + lib bundle + tsc -p tsconfig.lib.json) |
success ✅ |
The web-shell count rose 1376 → 1383 (+7) since my last verification — consistent with the new composer-annotation edge-case tests added in b4295bb85 / 29fa00100.
End-to-end visual verification (real render, not a mock)
I drove the real web-shell against the mock daemon and replayed a user_message_chunk transcript carrying _meta.inputAnnotations (the same shape the composer emits at submit time), then screenshotted the actual UserMessage output in both themes. One transcript demonstrates the chips, the guardrails, and the pre-PR fallback side by side:
- File reference → 📄
.qwen/settings.jsonchip - Extension reference → chip
- MCP reference → chip
- Multiple built-in references in one message → 3 chips
- Custom-provider
@dataset:users(no annotation) → stays plain text by design - Before this PR — the same
@.qwen/settings.jsonwith no annotations → plain serialized text (the old behavior)
Dark
Light
Data path re-confirmed
composer submit
→ createInputAnnotationsFromComposerTags (web-shell)
→ prompt request _meta.inputAnnotations (webui actions)
→ bridge echo user_message_chunk._meta (multi-client round-trip)
→ normalizer → transcript block .meta (sdk-typescript)
→ transcriptBlocksToDaemonMessages (web-shell adapter)
→ UserMessage / splitComposerTagContentByAnnotations → chips
Chip reconstruction is purely annotation-driven — the serialized prompt text sent to the daemon is unchanged, and any invalid / misaligned / malformed annotation degrades to plain text rather than mangling the message.
Verdict: re-verified at 29fa00100 — functionally correct, new commits behave as intended, no regressions. LGTM from a verification standpoint. 👍
中文说明(点击展开)
✅ 维护者在 head 29fa00100 的复验 —— 已本地构建并实测
我把该 PR 在当前 head(29fa00100)重新检出到一个全新的独立 worktree,执行了干净的 npm ci,并做了端到端的重新构建 + 全量测试(真实浏览器渲染 + 全量测试),而不仅仅是看 diff。我上一次验证是在 32708b41e;本次复验重新确认了此后新增的 3 个 commit,全部仍为绿色,无回归。
环境:macOS(Darwin 24.6)、Node v22.23.1、检出在 PR head 的独立 git worktree。
上次验证之后的新增(均已重新检查)
| Commit | 改动 | 我如何验证 |
|---|---|---|
b4295bb85 |
test(web-shell):补充 composer 标注的边界用例 | 新增 +125 行 composerTag 测试,均运行并通过 |
6c4e142d4 |
fix(web-shell):转发 split pane 的 prompt 标注 | ChatPane 现在把 metadata.inputAnnotations 同时传入 sendPrompt 和 enqueuePrompt;由新增的 ChatPane.test.tsx 用例覆盖(全绿) |
29fa00100 |
fix(web-shell):防护畸形的 input 标注 | splitComposerTagContentByAnnotations 现在会丢弃缺少 reference 或 id 非字符串的标注,退化为纯文本而不是错误渲染 —— 新增用例 leaves malformed reference annotations as text 通过 |
自动化测试 —— 均在独立 worktree 中通过
| 测试套件 | 结果 |
|---|---|
web-shell 全量单测(83 个文件) |
1383 / 1383 ✅ |
acp-bridge bridge |
386 / 386 ✅ |
sdk-typescript daemonUi |
261 / 261 ✅ |
webui daemon/session(7 个文件) |
200 / 200 ✅ |
PR 聚焦测试(composerTag + UserMessage) |
40 / 40 ✅ |
| ESLint(PR 改动的 web-shell 文件) | 通过 ✅ |
tsc --noEmit(web-shell) |
通过 ✅ |
npm run build(web-shell 生产包 + lib 包 + tsc -p tsconfig.lib.json) |
成功 ✅ |
web-shell 数量从上次的 1376 升到 1383(+7)—— 与 b4295bb85 / 29fa00100 中新增的 composer 标注边界用例一致。
端到端可视化验证(真实渲染,非 mock)
我让真实的 web-shell 对接 mock daemon,回放一段带 _meta.inputAnnotations 的 user_message_chunk transcript(与 composer 提交时产生的结构一致),并在明暗两种主题下截取实际的 UserMessage 输出。同一段 transcript 同时展示标签、边界防护与 PR 前的回退:
- File 引用 → 📄
.qwen/settings.json标签 - Extension 引用 → 标签
- MCP 引用 → 标签
- 单条消息内多个内置引用 → 3 个标签
- custom-provider
@dataset:users(无标注)→ 按设计保持纯文本 - PR 之前 —— 相同的
@.qwen/settings.json在无标注时 → 纯序列化文本(旧行为)
截图见上方英文部分(Dark / Light)。
数据链路已重新确认
composer 提交
→ createInputAnnotationsFromComposerTags (web-shell)
→ prompt 请求 _meta.inputAnnotations (webui actions)
→ bridge 回传 user_message_chunk._meta (多客户端往返)
→ normalizer → transcript block .meta (sdk-typescript)
→ transcriptBlocksToDaemonMessages (web-shell 适配层)
→ UserMessage / splitComposerTagContentByAnnotations → 标签
标签重建完全由标注驱动 —— 发送给 daemon 的序列化 prompt 文本保持不变,任何无效 / 错位 / 畸形的标注都会退化为纯文本,而不会破坏消息内容。
结论: 已在 29fa00100 复验 —— 功能正确,新增 commit 行为符合预期,无回归。从验证角度 LGTM。👍
Verification performed locally by a maintainer; screenshots hosted on the pr-assets/pr-6537-verify branch.
…ksDialog (QwenLM#6748) PR QwenLM#6537 consolidated composerTagIcons.ts into utils/composerTag.ts but PR QwenLM#6589 was branched before that and re-introduced the old import path, breaking the Vite build.




What this PR does
This PR renders serialized built-in composer references as compact chips in user message bubbles. It covers file, extension, and MCP references selected through the composer, and it keeps unknown provider-prefixed references as their original text so custom providers can continue to own their own message rendering behavior. It also consolidates the shared tag display helpers used by composer-related surfaces, so built-in reference labels and icons stay consistent without duplicating display rules.
Why it's needed
Composer references already appear as chips before sending, but after sending the user message previously showed serialized text such as
@.qwen/. That made the transcript less consistent with the composer and harder to scan. Rendering built-in references as chips in the transcript improves readability while preserving the prompt text sent to the daemon. Custom provider references need structured metadata to be rendered accurately after send, so this PR intentionally leaves those references as text instead of guessing a provider type.Reviewer Test Plan
How to verify
Create or open a WebShell session, select a file reference through the composer @ flow, send the prompt, and confirm the resulting user message renders that reference as a chip rather than raw serialized text. Repeat with an extension or MCP reference if available. Confirm ordinary text such as email-like content is not converted into a chip, and confirm custom-provider-shaped text such as
@dataset:usersremains plain text unless the host provides its own user-message renderer.Focused local verification was run with
npx vitest run client/utils/composerTag.test.ts client/components/messages/UserMessage.test.tsxandnpx eslint client/utils/composerTag.ts client/utils/composerTag.test.ts client/components/messages/UserMessage.tsx client/components/messages/UserMessage.test.tsxfrompackages/web-shell. The commit hook also ran Prettier and ESLint on the staged WebShell files during commit.Evidence (Before & After)
Before: sent @ references appeared as serialized text such as
@.qwen/in the user message bubble. After: the same built-in references render as compact file, extension, or MCP chips in user messages while the submitted prompt content remains serialized for the daemon.Before:

After:

Tested on
Environment (optional)
Local focused WebShell unit tests and lint checks only.
Risk & Scope
npm run preflight, Windows and Linux manual verification, and structured metadata support for custom provider references. Custom provider references remain text unless a host customizes user message rendering.Linked Issues
Fixes #6536
中文说明
What this PR does
这个 PR 会在用户消息气泡中将序列化的内置 composer 引用渲染为紧凑标签。它覆盖通过 composer 选择的文件、扩展和 MCP 引用;对于未知 provider 前缀的引用,会保留原始文本,让 custom provider 继续由宿主自行控制消息渲染行为。同时,这个 PR 也整理了 composer 相关展示面共用的 tag 展示辅助逻辑,避免重复维护内置引用的 label 和 icon 展示规则。
Why it's needed
composer 引用在发送前已经会显示为标签,但发送后用户消息之前会显示
@.qwen/这类序列化文本,导致会话记录和 composer 展示不一致,也不便于快速浏览。将内置引用在会话记录中渲染为标签,可以在保留提交给 daemon 的 prompt 文本的同时提升可读性。custom provider 引用如果要在发送后准确渲染,需要结构化 metadata,因此这个 PR 会刻意保留这类引用的原始文本,而不是猜测 provider 类型。Reviewer Test Plan
How to verify
创建或打开一个 WebShell 会话,通过 composer 的 @ 流程选择一个文件引用并发送 prompt,确认生成的用户消息中该引用显示为标签,而不是原始序列化文本。如果环境中有扩展或 MCP 引用,也可以重复验证。确认类似邮箱的普通文本不会被转换为标签,并确认
@dataset:users这类 custom-provider 形态的文本会保持普通文本,除非宿主自行提供用户消息渲染逻辑。本地已在
packages/web-shell下执行聚焦验证:npx vitest run client/utils/composerTag.test.ts client/components/messages/UserMessage.test.tsx和npx eslint client/utils/composerTag.ts client/utils/composerTag.test.ts client/components/messages/UserMessage.tsx client/components/messages/UserMessage.test.tsx。提交时的 hook 也对暂存的 WebShell 文件执行了 Prettier 和 ESLint。Evidence (Before & After)
Before:发送后的 @ 引用会在用户消息气泡中显示为
@.qwen/这类序列化文本。After:相同的内置引用会在用户消息中渲染为紧凑的文件、扩展或 MCP 标签,同时提交给 daemon 的 prompt 内容仍保持序列化形式。Before:

After:

Tested on
Environment (optional)
仅执行了本地聚焦 WebShell 单元测试和 lint 检查。
Risk & Scope
npm run preflight、Windows 和 Linux 手动验证,以及 custom provider 引用的结构化 metadata 支持。custom provider 引用会保持文本,除非宿主自定义用户消息渲染。Linked Issues
Fixes #6536