Repository navigation
fix(web-shell): keep inline chip annotations on the chip's real range - #12992
Conversation
createInputAnnotationsFromComposerTags re-located every tag in the final prompt with indexOf, so plain text spelled like an inline chip's serialized text stole the annotation and the bubble rendered the typed look-alike as the chip (#12980). The composer already knows each inline chip's editor range; thread those placements through submit (shifted past the prompt prefix) and emit their annotations directly, keeping the forward indexOf scan for tags without a known placement. Co-authored-by: Qwen-Coder <qwen-coder @alibabacloud.com> Patrol-Run: qwen-issue-patrol/jmum9v0vg30
Every case added for #12980 ran with promptPrefixLength === 0, so the one line of new arithmetic (promptText.length - text.length, and the shifts that use it) was unpinned. Adds a dom case with a top tag attached (prefix 6, chip must land on [24, 28) instead of the plain-text look-alike at [14, 18)) and one in shell mode (prefix 1, chip must land on [19, 23)). Forcing the prefix to 0 makes each lose its chip annotation entirely. Also applies Prettier to this PR's two test files, which was the only thing `Lint & Static` objected to. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmumcpwj7eg
|
Both items from the review are done at 1. Prettier — 2. The prefix shift is pinned — two dom cases asserting the chip's absolute prompt offset:
Both were mutation-checked individually with 39/39 across the two touched test files, I left the two non-blocking items alone as you framed them: the One line on the red 中文评审提的两件事都已在
两个非阻塞项按你的定性保留不动：
|
|
Correction to my previous comment: re-running CI does not clear that The job checks out
The re-run at So this is branch staleness, not a defect in this PR — the branch carries #12545's fixture without #12996's repair, and nothing under
中文更正上一条：重跑 CI 不会清掉那个 该 job 检出的是
因此这是分支陈旧，不是本 PR 的缺陷——分支带着 #12545 的 fixture 却没有 #12996 的修复，
|
Corrected CI note on head
|
Picks up #12996 (e44e45e), which registers the Skill tool in the resume-listing fixture. Without it the branch still carries the pre-#12545 fixture, so 7 BackgroundAgentResumeService "launch-time skill listing" cases fail on `expect(initialMessages.includes('auto-skill-demo')).toBe(true)`. This branch does not touch packages/core/src/agents/background-agent-resume.*, so the merge takes main's version wholesale. Co-authored-by: Qwen-Coder <[email protected]>
…acements Known placements are pre-substitution editor offsets, so once the document text under a chip drifts from its serialized form every later chip's range goes stale too and the slice guard dropped all of their annotations. On a mismatch, fall back to the text when the serialized form occurs exactly once — a unique match cannot annotate the wrong span — and keep dropping to plain text when it is repeated or absent. Addresses review thread R1-1.
A stale known placement recovers through content.indexOf(serialized), which searches from the prompt start. Nothing pinned that origin: every existing placement either starts at 0 or names a serialized form absent from the content, so anchoring the search at the recorded start was behaviourally indistinguishable. Add a placement whose recorded start sits past its true occurrence; it fails under an indexOf(serialized, start) mutant and passes without it. Co-authored-by: Qwen-Coder <[email protected]> Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmumw0cjpfe
…ertions R3-1: the two multi-annotation assertions in composerTag.test.ts projected each annotation down to [start, end], so nothing in the repo pinned that a recovered span is emitted with its own placement's tag. Offsets come from the span's serialized form while identity comes from placement.tag, and only the placement loop joins them, so a mis-zip that preserves annotation order was invisible. A wrong pairing renders a chip whose reference.id points at a different file, and that id is embedded in the submit-path dedupe key, so it propagates into replay reconciliation and is persisted with the transcript. Both assertions now project [start, end, reference.id]: - 'combines searched tags with known inline placements' - 'recovers a stale placement whose true occurrence is behind its recorded start' (the only test where two placements both take the stale fallback) The expectations assert the ids the fixtures actually declare (file:@ctx / file:@foo and file:@Alpha / file:@beta) and deliberately do not gain a serialized key: these fixtures set only id/kind/value, and createReferenceAnnotation spreads serialized conditionally. Discriminating power verified with a temporary mis-zip mutant in the stale fallback branch (collect spans and tags separately, emit spans[i] with tags[n-1-i].tag, annotation order preserved): the strengthened assertion reds with "expected [ [ +0, 6, 'file:@beta' ], ...(1) ] to deeply equal [ [ +0, 6, 'file:@Alpha' ], ...(1) ]" and it is the only one of the 36 tests in the file that does. The mutant was reverted; production code is unchanged. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmun8vb7vg3
Real-environment verification: PR #12992 @
|
| Scenario (UI path) | Before fa4a4c92 |
After 6ad9468d |
|
|---|---|---|---|
| S1 | Issue repro: paste literal @foo.md then , then add @foo.md with + → Reference file |
[8,15) #1 (the typed text); bubble literal [chip] then @foo.md |
[21,28) #2 (the chip); bubble literal @foo.md then [chip] |
| S1 | Same session after a page reload (bubble replayed from the daemon; chats/<id>.jsonl) |
still swapped; JSONL start 8, end 15 |
correct; JSONL start 21, end 28 |
| S2 | Chip inserted from the typed @ mention menu |
[8,15) #1 |
[21,28) #2 |
| S4 | chip · look-alike · chip | [0,7) + [11,18) #2: the second chip renders as text and the look-alike renders as a chip |
[0,7) + [22,29) #3 |
| S7 | First send fails (HTTP 400; separately, a connection reset) → Restore local copy → resend | [8,15) on both sends |
[21,28) on both sends |
| S6 | Prompt sent while a turn is running (mid-turn queue) | the model receives literal then @foo.md + attachment |
the model receives literal @foo.md then + attachment |
| S3 | Control: chip placed before the look-alike | [0,7) |
[0,7) (identical) |
| S5 | Shell mode ! |
no annotations sent (POST /session/:id/shell, plain command) |
identical |
| — | Production bundle served by the daemon, S1 and S4 | — | same ranges as "After" |
Worth knowing: before this PR, the bug also changed what the model received
The PR description says the submitted prompt string was always correct. That is true for the normal path: in S1 both arms send literal @foo.md then @foo.md. It is not true for the mid-turn queue. There, annotatedFiles() (packages/web-shell/client/hooks/useQueuedPrompts.ts:172) removes each annotated file range from the text and sends the file as an attachment. With the pre-PR range it removed the @foo.md the user typed, so the model received literal then @foo.md@attachment:///foo.md (Figure 3). This PR fixes that path too, without changing it. None of the PR's tests cover this consumer. A queue-level regression test would be a reasonable follow-up, but it should not block the merge.
Tests and static checks (macOS, Node 24.18.1)
- The PR's 4 test files on the head: 149/149 pass.
- The same test files against the merge-base source: 7 of 41 fail (4 in
composerTag.test.tsand 3 in the new DOM test, for exampleexpected [[8, 12]] to deeply equal [[18, 22]]). Both control cases pass. So the new tests catch the bug. - Full
packages/web-shellvitest suite on the head: 394 files / 10410 tests pass (3 m 04 s). The PR's "Tested on" table leaves macOS unchecked; this run covers it. tsc --noEmitforpackages/web-shell, plus eslint and prettier on the 4 touched files: all clean.- I ran 8 mutation probes against the 4 test files. Each mutant was reverted and the tree checked clean afterwards.
- 7 killed:
- prefix shift forced to 0
- shift that ignores the shell
! - caller reverted to the pre-PR search
- slice-match guard removed
- stale-placement fallback removed
- uniqueness check removed
- fallback searching from the recorded start
- 1 survived: deleting
start >= 0(composerTag.ts:221). This is the bot's R1-2, which the author declined. It cannot be reached today: the only caller clamps each start withMath.max(0, …)(useComposerCore.ts:2833), and the prefix shift is never negative.
- 7 killed:
Merge readiness
- The head stayed at
6ad9468dthroughout. These CI checks are green at this head: Test (ubuntu), Lint & Static, web-shell E2E Smoke, Integration (no-AK), Desktop Shell (ubuntu + windows) and web-shell visuals. All 5 review threads are resolved, and the last bot round (R4) posted no new finding. git merge-treeagainst currentmain(afb911a3) is clean.mainis 45 commits past the merge-base and changed 65 web-shell client files in that span, but none of those changes touchinputAnnotations,createInputAnnotationsFromComposerTags, inline tag placements orannotatedFiles.
Same on both arms (FYI, not caused by this PR)
- Up-arrow history recall restores the prompt as plain text with no chip, and resending it sends no annotation.
- After a mid-turn delivery, the bubble shows neither a chip nor an attachment marker for the reference.
- Stock Web Shell sends
!commands toPOST /session/:id/shelland dropsinputAnnotations(App.tsx:17204). So the PR's shell-mode shift is only visible to hosts that read theonSubmitmetadata. The unit test covers it (the "ignores the shell!" mutant was killed). Top tags (placement: 'top') are likewise host-API only, and the unit tests cover them too (the "prefix shift forced to 0" mutant was killed).
Not covered here: Windows and Linux browsers, the touch/textarea (mobile) backend, and custom host providers whose composerTag.serialized differs from the inserted text. That last case is the stale-placement fallback, which only the unit tests exercise.
中文版
真实环境验证：PR #12992 @ 6ad9468d
结论：修复在真实浏览器 + 真实 daemon 下端到端成立，未发现回归，建议合入。 该 head 上 CI 全绿，5 个评审 thread 已全部解决；仍需一个 approving review。
装置
- 在隔离 worktree 中从源码构建 PR head
6ad9468d（npm run build+npm run bundle）。daemon 用该构建的node dist/cli.js serve，QWEN_HOME/QWEN_RUNTIME_DIR隔离，模型用仓库自带的假 OpenAI 兼容服务（integration-tests/fake-openai-server.ts），夹具工作区含foo.md。 - 同一个 daemon 上挂两个 Web Shell 客户端：merge-base
fa4a4c92的 vite dev（修复前）与 PR head 的 vite dev（修复后），两棵树只差 PR 的 4 个文件。daemon 自己托管的生产构建（dist/web-shell）作为第三组。 - 环境：Playwright 1.61.1 / Chromium，macOS（Darwin 25.6，arm64），Node 24.18.1。每次运行都用全新浏览器上下文和全新会话，每次记录四项：发送前编辑器结构（文字 vs chip）、浏览器自己发出的
POST /session/:id/prompt请求体中的inputAnnotations、渲染出的气泡、截图。 - 终轮：18/18 次运行全部完成（8 个场景 × 前/后两组，外加生产构建 2 个场景）。
结果
「#n」表示该区间覆盖 prompt 中第 n 次出现的 @foo.md。
| 场景（UI 路径） | 修复前 fa4a4c92 |
修复后 6ad9468d |
|
|---|---|---|---|
| S1 | issue 复现：粘贴 literal @foo.md then ，再用 + → Reference file 添加 @foo.md |
[8,15) #1（手打文字）；气泡 literal [chip] then @foo.md |
[21,28) #2（chip）；气泡 literal @foo.md then [chip] |
| S1 | 同一会话刷新页面后（气泡由 daemon 回放；chats/<id>.jsonl） |
仍然颠倒；JSONL start 8, end 15 |
正确；JSONL start 21, end 28 |
| S2 | 通过手打 @ 提及菜单插入 chip |
[8,15) #1 |
[21,28) #2 |
| S4 | chip · 同形文字 · chip | [0,7) + [11,18) #2：第二个 chip 被渲染成文字，同形文字被渲染成 chip |
[0,7) + [22,29) #3 |
| S7 | 首次发送失败（HTTP 400；另测连接重置）→ Restore local copy → 重发 | 两次发送都是 [8,15) |
两次发送都是 [21,28) |
| S6 | 在一轮运行中发送（mid-turn 排队） | 模型收到 literal then @foo.md + 附件 |
模型收到 literal @foo.md then + 附件 |
| S3 | 对照：chip 在同形文字之前 | [0,7) |
[0,7)（一致） |
| S5 | shell 模式 ! |
不发送注解（POST /session/:id/shell，纯文本命令） |
一致 |
| — | daemon 托管的生产构建，S1 与 S4 | — | 区间与「修复后」相同 |
（图 1–3 见上方英文部分。）
值得注意：修复前这个 bug 也会改变模型收到的内容
PR 描述说提交的 prompt 字符串一直是对的。普通路径确实如此：S1 两组发出的都是 literal @foo.md then @foo.md。但 mid-turn 排队路径不是这样：annotatedFiles()（packages/web-shell/client/hooks/useQueuedPrompts.ts:172）会把每个带注解的文件区间从文本中删掉，改成附件发送。修复前的区间删掉的是用户手打的 @foo.md，于是模型收到的是 literal then @foo.md@attachment:///foo.md（图 3）。本 PR 没有改动这条路径，但也把它修好了。PR 的测试都没有覆盖这个消费方；补一个排队层面的回归测试可以作为后续工作，不应阻塞合入。
测试与静态检查（macOS，Node 24.18.1）
- PR 的 4 个测试文件在 head 上：149/149 通过。
- 同样的测试文件跑在 merge-base 源码上：41 个中 7 个失败（
composerTag.test.ts4 个，新 DOM 测试 3 个，例如expected [[8, 12]] to deeply equal [[18, 22]]），两个对照用例通过。说明新测试确实能抓住这个 bug。 - head 上
packages/web-shell全量 vitest：394 个文件 / 10410 个用例全部通过（3 分 04 秒）。PR 的「Tested on」表里 macOS 未勾选，这次运行补上了。 packages/web-shell的tsc --noEmit，以及 4 个改动文件的 eslint 和 prettier：全部干净。- 针对这 4 个测试文件跑了 8 个变异体，每个都在事后还原并确认树干净。
- 杀死 7 个：
- 前缀位移强制为 0
- 位移忽略 shell 的
! - 调用方回退到 PR 前的搜索方式
- 删除切片校验
- 删除陈旧区间回退
- 删除唯一性检查
- 回退改为从记录的起点开始搜索
- 存活 1 个： 删除
start >= 0（composerTag.ts:221）。这就是机器人的 R1-2，作者已拒绝修改。目前这条路径不可达：唯一的调用方会用Math.max(0, …)钳住起点（useComposerCore.ts:2833），前缀位移也不会为负。
- 杀死 7 个：
合入就绪情况
- 验证全程 head 都停在
6ad9468d。该 head 上这些 CI 检查全绿：Test（ubuntu）、Lint & Static、web-shell E2E Smoke、Integration（no-AK）、Desktop Shell（ubuntu + windows）、web-shell visuals。5 个评审 thread 全部已解决，机器人最后一轮（R4）没有新发现。 - 对当前
main（afb911a3）的git merge-tree干净。main比 merge-base 多 45 个提交，期间改了 65 个 web-shell 客户端文件，但没有一处涉及inputAnnotations、createInputAnnotationsFromComposerTags、内联 tag 区间或annotatedFiles。
两组表现一致（仅供参考，非本 PR 引起）
- ↑ 历史回溯只会把 prompt 恢复成纯文本、没有 chip，重发时也不带注解。
- mid-turn 送达后，气泡里既不显示 chip，也不显示该引用的附件标记。
- stock Web Shell 把
!命令发到POST /session/:id/shell，并丢弃inputAnnotations（App.tsx:17204）。因此 PR 的 shell 模式位移只对读取onSubmit元数据的宿主可见；单测覆盖了它（「位移忽略 shell 的!」这个变异体被杀死）。顶部 tag（placement: 'top'）同样只有宿主 API 能触发，也有单测覆盖（「前缀位移强制为 0」这个变异体被杀死）。
本次未覆盖：Windows 与 Linux 浏览器、触摸/textarea（移动端）后端，以及 composerTag.serialized 与插入文本不一致的自定义宿主 provider。最后这种情况对应陈旧区间回退，只有单测覆盖。
3 participants
What this PR does
When a prompt contained plain text spelled exactly like a later inline reference chip (e.g. typing
literal @foo thenand then inserting an@foochip at the end), the submittedinputAnnotationsattached to the first textual match instead of the chip, so the user-message bubble rendered the typed look-alike as the chip and the real chip as plain text. This PR threads the editor-known inline chip ranges through submit into annotation generation, so annotations are emitted at the chip's real offsets; the previous forwardindexOfscan is kept for tags that have no known placement (composer-level top tags,tagsOverride, textarea/touch backend).Why it's needed
The offset was wrong at generation time:
createInputAnnotationsFromComposerTagsdiscarded the CodeMirror placement ranges the composer already had and re-searched the final prompt text withcontent.indexOf(serialized, cursor), which returns the first textual occurrence regardless of whether it was a chip. The submitted prompt string itself was always correct — only the display annotation was misplaced — but the bubble then misrepresented which occurrence the user selected (#12980).Reviewer Test Plan
How to verify
literal @foo thenas ordinary text (do not select a reference for this occurrence).+file-reference picker to insert an@fooreference at the end.@foostays plain text, the trailing one renders as the chip. Before this PR the two were reversed.Component-level evidence (real CodeMirror editor + real
useComposerCoresubmit pipeline + the samesplitComposerTagContentByAnnotationsthe bubble renders from):packages/web-shell/client/hooks/useComposerCore.issue-12980.dom.test.tsxfails on unfixed code with the annotation at[8, 12)instead of[18, 22)— the exact offsets from the issue — and passes with this change. A full browser click-through of the+picker was not run here; the picker reaches this same insertion path (addTags({ placement: 'inline', position: 'end' })).Test output:
pnpm typecheckinpackages/web-shellpasses; eslint on touched files is clean.Evidence (Before & After)
Before: repro test above fails —
expected 8 to be 18(inputAnnotations[0].startlands on the plain-text look-alike). After: same test passes, annotation at[18, 22), andsplitComposerTagContentByAnnotationskeepsliteral @foo thenas text with only the trailing@foorendered as the reference chip. No browser recording: covered at component level as described above.Tested on
Environment (optional)
Unit/component tests only:
vitest runinpackages/web-shell(jsdom + real CodeMirror view).Risk & Scope
!); a slice-match guard drops any placement whose range does not match the serialized text, degrading to plain text instead of a wrong chip.+picker; daemon persistence/reload of annotations; the touch/textarea backend (never produces inline placements, unchanged). Deferred review findings from PR #12404: fix(web-shell): preserve reference tags across reloads #12426 (post-generation annotation rewrites) is a separate issue and untouched.createInputAnnotationsFromComposerTagsgains an optional third parameter; existing callers and the pinned ordered-matching test for repeated serialized references are unaffected.Linked Issues
Fixes #12980
中文说明
本 PR 做了什么
当 prompt 中同时存在与某个内联引用 chip 序列化文本完全相同的普通文字时（例如先输入
literal @foo then，再在末尾插入@foochip），提交生成的inputAnnotations会挂到文本上的第一个匹配处而不是 chip 本身，导致用户消息气泡把普通文字渲染成 chip、把真正的 chip 渲染成普通文字。本 PR 把编辑器已知的内联 chip 区间随提交路径传入注解生成，直接在 chip 的真实偏移处生成注解；对没有已知区间的 tag（composer 顶部 tag、tagsOverride、textarea/触摸后端）保留原有的前向indexOf扫描。为什么需要
偏移在注解生成时就是错的：
createInputAnnotationsFromComposerTags丢弃了 composer 已有的 CodeMirror 区间，改用content.indexOf(serialized, cursor)在最终 prompt 文本里重新查找，命中的是第一个文本匹配而不论该处是否为 chip。提交的 prompt 字符串本身一直是对的，只有展示注解挂错，但气泡会错误呈现用户实际选中的位置（#12980）。评审验证计划
如何验证
literal @foo then（该处不要选择引用）。+文件引用选择器在末尾插入@foo引用。@foo保持普通文字，末尾的渲染为 chip。在本 PR 之前两者是反的。组件级证据（真实 CodeMirror 编辑器 + 真实
useComposerCore提交管线 + 气泡渲染所用的同一个splitComposerTagContentByAnnotations）：packages/web-shell/client/hooks/useComposerCore.issue-12980.dom.test.tsx在未修复代码上失败，注解落在[8, 12)而非[18, 22)——与 issue 中的偏移完全一致；本改动后通过。未在浏览器中完整点击+选择器路径；该路径汇入同一个插入入口（addTags({ placement: 'inline', position: 'end' })）。测试输出：
packages/web-shell的pnpm typecheck通过；改动文件 eslint 干净。前后对比证据
修复前：上述复现测试失败——
expected 8 to be 18（inputAnnotations[0].start落在普通文字上）。修复后：同一测试通过，注解位于[18, 22)，splitComposerTagContentByAnnotations把literal @foo then保持为文本、仅末尾@foo渲染为引用 chip。无浏览器录屏：如上所述以组件级覆盖。测试平台
环境（可选）
仅单元/组件测试：在
packages/web-shell下vitest run（jsdom + 真实 CodeMirror view）。风险与范围
!的偏移）；对切片校验不匹配的区间直接丢弃，降级为普通文字而不是挂错 chip。+选择器的浏览器完整点击路径；注解经 daemon 持久化与重载；触摸/textarea 后端（不产生内联区间，行为不变）。Deferred review findings from PR #12404: fix(web-shell): preserve reference tags across reloads #12426（注解生成后的改写）是独立问题，未改动。createInputAnnotationsFromComposerTags新增可选第三参数；既有调用方与固化重复序列化文本顺序匹配行为的测试均不受影响。关联 Issue
Fixes #12980