feat(ui): word-wise drag after double-click, line-wise extension after triple-click - #8739
Conversation
…lick Double/triple click already selected a single word/line, but holding and dragging did not extend the selection by word/line. Enter a drag-capable word/line selection on multi-click and extend the range to the word/line boundary under the cursor on move/release, so double-click+drag grows by words and triple-click+drag by lines. Fixes #8738
|
Re-run after the head moved through seven fix rounds plus main merges (now Template looks good ✓ Problem: real and demonstrated, not theoretical. Issue #8738 documents the exact gap — the multi-click branch selects a word/line and returns without ever entering drag mode — and a maintainer's local BASE-vs-HEAD comparison in a real pty confirms every drag-extension gesture is a no-op on the merge base and functional on this head. Direction: aligned. This completes the standard editor gesture set on top of the VP-mode selection shipped in #6937, and the issue carries Size: not applicable — no core paths. Everything sits in Approach: scope feels right, and the diff is minimal — it follows the shape the issue suggested (record a Risk: no elevated risk signals — none of the changed files match the revert-correlated high-risk paths. Moving on to code review. 🔍 中文说明Re-run:head 经过 7 轮修复 + 多次合并 main(现为 模板完整 ✓ 问题:真实且已被证实,不是理论问题。Issue #8738 指出了确切缺口——多点点击分支选中词/行后直接返回、从未进入拖动模式;维护者在真实 pty 中做的 BASE 与 HEAD 对照也确认:所有拖动扩展手势在 merge base 上无效、在本 head 上生效。 方向:对齐。在 #6937 已上线的 VP 模式选择之上补全标准编辑器手势,关联 issue 带有 规模:不适用——未触及核心路径。改动全部位于 方案:范围合理,diff 也是最小集——实现遵循 issue 建议的形态(多点点击时记录 风险:无升级风险信号——改动文件均未命中与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Code reviewFull read of the three production files plus tests at the reviewed head. The approach matches my independent proposal: stay inside the existing anchor/focus model, record a The Critical from my last pass is fixed and regression-tested:
Both minor notes from last pass are addressed too: the orphaned One non-blocking nit, echoed from the maintainer's verification: copying at press and release doubles clipboard writes for plain multi-clicks (2 writes for a double-click, 4 for a triple-click), which shows up as duplicate entries in clipboard-history tools. Final content is always correct. Skipping the release-time copy when it is byte-identical to the press copy would tidy it without touching the streaming fix — fine as a follow-up, not a merge blocker. Also purely cosmetic: the PR body still says "11 tests" where this file now has 22. sequenceDiagram
participant P1 as User
participant P2 as Mouse handler
participant P3 as SelectionState
P1->>P2: double or triple click, hold
P2->>P3: start plus extend the span, record mode and anchor, copy at press
P1->>P2: move or release while held
P2->>P3: extendSpanDrag snaps focus to the word or line under the cursor
P2->>P3: on release, copy again (survives a streaming clear before release)
Testing evidenceThis is an unattended CI-path run, so I did not build or drive the product; the evidence below is the PR's own CI (fetched via API for the reviewed commit) plus the maintainer's independently performed local verification, attributed as such. All checks on the reviewed commit are green — 27 successes, zero failures (the 115 skipped entries and the one pending
The CI suite pins the change — the 12 new tests target behaviors that are all no-ops on the merge base (drag-after-multi-click extension, release-cell extension, collapsed-span copies), so a green suite here is meaningful rather than diff-blind. Beyond that, maintainer @wenshao performed two rounds of local real-environment verification, the latest against exactly this head ( 中文说明代码审查在受审 head 上完整通读了三个生产文件与测试。方案与我独立设想一致:不引入平行状态机,沿用现有 anchor/focus 模型——多点点击时在 上一轮的 Critical 已修复并有回归测试:
上轮两条 minor 也已处理:孤儿 一条非阻塞小问题,与维护者验证结论一致:press 与 release 各复制一次使纯多点点击的剪贴板写入翻倍(双击 2 次、三击 4 次),剪贴板历史类工具会出现连续重复条目;最终内容始终正确。release 时若与 press 已复制内容逐字节相同则跳过即可收敛,且不触及流式修复——适合作为后续项,不阻塞合并。另纯观感:PR 描述仍写 "11 tests",该文件现为 22 个。 (时序图见英文正文,流程为:按住多击 → 记录模式与锚点并 press 复制 → move/release 吸附到光标下的词/行 → release 再复制一次。) 测试证据本次为无人值守 CI 路径运行,不构建、不驱动产品本体;以下证据为 PR 自身 CI(通过 API 按受审 commit 拉取)+ 维护者独立完成的本地真实环境验证(如实注明归属)。 受审 commit 上所有检查全绿——27 项成功、0 失败(115 项 skipped 与唯一 pending 的 (CI 明细表见英文正文标记区域。) CI 套件确实钉住了本次改动——12 个新测试针对的行为在 merge base 上全部无效(多点点击后拖动扩展、释放格扩展、折叠 span 复制),因此这里的绿灯有意义而非对 diff 视而不见。在此之上,维护者 @wenshao 做了两轮本地真实环境验证,最新一轮正是针对本 head( — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean re-review; both prior blockers are fixed and regression-tested, CI is green on the reviewed commit, and the only remaining nit is the cosmetic duplicate-clipboard-write that the verifying maintainer explicitly declined to block on. Stepping back: this one earned its landing. The gesture gap was real — the base code plainly returns from the multi-click branch without entering drag mode — and it was verified the hard way, not just asserted: a maintainer drove a real pty with raw SGR mouse sequences and showed every drag-extension gesture is a no-op on the merge base. The implementation matches what I would have proposed independently (mode + anchor-span ref on top of the existing anchor/focus model, no parallel state machine), and the review process demonstrably worked — my last pass flagged that a single-cell no-drag double-click silently stopped copying, and the fix came back as exactly the mode-aware guard that was missing, with regression tests for both it and the streaming-repaint regression that only surfaced in a real terminal later. The diff is minimal (132 production lines, dead code removed rather than orphaned), the comments explain the non-obvious why (press-time copy, release-cell extension, click-chain preservation), and every new behavior has a test that would fail on the merge base. The 4 rather than 5 is the remaining polish: plain multi-clicks now write to the clipboard twice (press and release), which leaves duplicate entries in clipboard-history tools. Content is always correct, the verifying maintainer flagged it as non-blocking with a concrete optional fix (skip the release copy when byte-identical), and I agree it belongs in a follow-up rather than holding this merge. Approving, pinned to the reviewed commit. ✅ @qwen-code-dev-bot 中文说明置信度:4/5 —— 复审干净:上一轮两个阻塞项均已修复并有回归测试,受审 commit 的 CI 全绿,唯一剩余的小问题是重复剪贴板写入(纯观感),且做验证的维护者已明确表示不以此阻塞。 整体来看:这个 PR 配得上合并。手势缺口真实存在——base 代码在多点点击分支直接返回、未进入拖动模式——而且是硬验证出来的,不是口头断言:维护者在真实 pty 里用原始 SGR 鼠标序列驱动,证明所有拖动扩展手势在 merge base 上无效。实现与我独立设想一致(在现有 anchor/focus 模型上加 mode + 锚点 span 引用,不建平行状态机);评审流程也确实起了作用——我上一轮指出单格无拖动双击会悄悄不再复制,回来的修复正是缺失的那个模式感知守卫,且连同后来只在真实终端暴露的流式重绘回归一起补了回归测试。diff 最小(132 行生产代码,死代码被删除而非遗留),注释解释的是不明显的「为什么」(press 时复制、释放格扩展、连击链保留),每个新行为都有在 merge base 上必失败的测试。 给 4 而非 5 是剩下的一点打磨:纯多点点击现在会写两次剪贴板(press 与 release),剪贴板历史工具会留下重复条目。内容始终正确,验证维护者已标记为非阻塞并给出具体可选修复(release 时与 press 内容逐字节相同则跳过),我同意放后续项而不是卡本次合并。 批准,锚定到受审 commit。✅ — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Needs one focused fix — see my Stage 2 notes above. 🙏
Plain double-click on a single-character word (and triple-click on a one-cell line) no longer copies: the now-drag-capable word/line select flows through left-release, where a 1-cell span is collapsed and cleared before copySelection() runs. Suggested fix: only treat a collapsed release as a bare click in char mode (selection.isCollapsed && selection.mode === 'char'), plus a no-drag double-click regression test. Optional cleanup: selectSpan is now dead code, and the release-branch comment is stale.
|
🔀 Base updated: red check(s) [label] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base:红色检查 [label] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。 |
Drag-capable word/line selections moved the copy from press to release, but the release path cleared any collapsed range before copying, so a plain double-click on a single-character word (or triple-click on a one-cell line) stopped copying. Only treat a collapsed release as a bare click in char mode; in word/line mode a collapsed range is a real single-cell span. Also drop the now-unused selectSpan helper and a stale release-branch comment.
|
🤖 Addressed the latest review feedback (round 1/10). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/10 轮)。改动内容与我反驳保留之处如下: Autofix review round — PR #8739Feedback points and dispositions
Changed files
Commit: Conflict notesNone ( Verification
中文说明Autofix 审查轮次 — PR #8739反馈点及处理结果
变更文件
提交: 冲突说明无( 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 10 this bot stops and leaves the PR for a human. · 有空请复审;第 10 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
wenshao
left a comment
There was a problem hiding this comment.
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.7)
| const spanDrag = spanDragRef.current; | ||
| spanDragRef.current = null; |
There was a problem hiding this comment.
[Critical] Release-time word/line drag extension is dead code. In the left-release handler, spanDragRef.current is set to null before extendSpanDrag(...) is called, and extendSpanDrag's first statement re-reads the ref (const spanDrag = spanDragRef.current; if (!spanDrag) return;) — so the release cell's coordinates are never applied to a word/line drag.
Failure scenario: double/triple-click a word/line and drag, then release at a cell where the terminal emitted no final move event (SGR button tracking samples motion — fast flicks and coalesced events land here). The selection and copied text stop at the last processed move instead of the release point; with no move at all between the multi-click and the release, the range never extends past the clicked word/line. The char-drag path handles exactly this case and has a dedicated test ('includes the release cell when no move event is emitted'); the word/line path silently lacks the same protection, and the three new tests pass because each emits move at the release position first. Probe-verified at HEAD: release at grid 9 with no move yields ex: 2 / 'foo'; with a move at 6 then release at 9, ex: 6 / 'foo bar'; reordering the null after the call fixes both.
| const spanDrag = spanDragRef.current; | |
| spanDragRef.current = null; | |
| const spanDrag = spanDragRef.current; | |
| const mapped = mapEvent(event); | |
| if (mapped) { | |
| if (spanDrag) { | |
| extendSpanDrag(clampToViewport(mapped.point, mapped.rect)); | |
| } else { | |
| selection.extend(clampToViewport(mapped.point, mapped.rect)); | |
| } | |
| } | |
| spanDragRef.current = null; |
中文说明
(严重) release 时的词/行拖动扩展是死代码:left-release 处理器先把 spanDragRef.current 置为 null,再调用 extendSpanDrag(...),而 extendSpanDrag 第一行又去读这个 ref(const spanDrag = spanDragRef.current; if (!spanDrag) return;)——因此释放点的坐标从未被应用到词/行拖动的选区上。
触发场景:双击/三击选中词/行后拖动,若终端在释放位置没有发出最后一个 move 事件(SGR 按钮追踪为采样式上报,快速甩动、事件合并都会命中此情形),选区与复制文本会停在最后一次 move 的位置而不是释放点;若多点点击与释放之间完全没有 move,选区根本不会扩展。字符拖动路径专门处理了该场景并有对应测试('includes the release cell when no move event is emitted'),词/行路径缺失同等保护;三个新测试都在释放前先发了 move,因此全部通过。已在 HEAD 用探针验证:无 move 直接释放到 grid 9 时选区止于 ex: 2(复制 'foo');先 move 到 6 再释放到 9 时止于 ex: 6(复制 'foo bar');把置空语句移到调用之后两处均修复。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.7)
| selection.mode = mode; | ||
| selection.anchor = { x: span.sx, y: span.sy }; | ||
| selection.focus = { x: span.ex, y: span.ey }; | ||
| selection.dragging = true; |
There was a problem hiding this comment.
[Suggestion] The multi-click press now sets selection.dragging = true (the deleted selectSpan set it false), which exposes the multi-click chain to the move handler's lastClickRef.current = null. Any held-button drift now breaks the triple-click chain or extends a plain double-click's copy range.
Failure scenario: under SGR button tracking a move fires whenever the pointer crosses a cell while the button is held. Probe-verified: press1→release1→press2→move(3,1)→release2→press3→release3 yields a null final selection — a 1-cell drift move between press2 and release2 kills the triple-click (press3 computes count=1, char mode). The same mechanism extends a jittery plain double-click's copy range (e.g. 'foo' → 'foo bar'). The old code ignored moves after a multi-click select, so both were impossible.
中文说明
(建议) 多点点击按下时现在设置 selection.dragging = true(被删除的 selectSpan 设置的是 false),这使多点点击链条暴露在 move 处理器的 lastClickRef.current = null 之下:按住期间任何指针漂移都会破坏三击链条,或扩大普通双击的复制范围。
触发场景:SGR 按钮追踪下,按住按键期间指针每跨过一个单元格就会触发 move。探针已验证:press1→release1→press2→move(3,1)→release2→press3→release3 最终得到空选区——press2 与 release2 之间 1 格的漂移就破坏了三击(press3 计算 count=1,进入字符模式)。同样的机制会让抖动的普通双击复制范围扩大(如 'foo' 变成 'foo bar')。旧代码会忽略多点点击之后的 move,这两种情况都不会发生。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.7)
| ? (wordSpanAt(frame, point.x, point.y) ?? { | ||
| sx: point.x, | ||
| sy: point.y, | ||
| ex: point.x, | ||
| ey: point.y, | ||
| }) |
There was a problem hiding this comment.
[Suggestion] The whitespace fallback collapses focus to the single whitespace cell, so stopping a word/line drag over a gap puts whitespace inside the copied range.
Failure scenario: probe-verified — double-click 'foo' in 'foo bar baz' and drag to the space at grid x=3: selection becomes 0-3 and the clipboard gets 'foo ' with a trailing space; dragging left onto the space before the anchor word copies ' bar'. A natural gesture (stopping a word-drag on a gap) produces clipboard text the user must strip. The single-cell fallback is documented as deliberate, but the whitespace-in-clipboard harm is concrete.
中文说明
(建议) 空白回退会把焦点坍缩到单个空白单元格,导致词/行拖动停在空隙处时复制文本带上空白。
触发场景(探针已验证):在 'foo bar baz' 中双击 'foo' 并拖到 grid x=3 的空格处——选区变为 0-3,剪贴板得到带尾随空格的 'foo ';向左拖到锚点词前的空格则会复制出带前导空格的 ' bar'。在空隙处停住词拖动是自然手势,却产生需要用户手工清理的剪贴板内容。单格回退在注释中是有意为之,但空白进入复制范围的实际损害是具体的。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.7)
There was a problem hiding this comment.
Declined. The single-cell fallback when the cursor is over whitespace is documented intentional behavior of this feature (see the comment above extendSpanDrag), and it matches how mainstream editors handle word-drag onto a gap — the gap is included in the selection and thus in the copied text. Changing that would be a behavior/scope decision beyond issue #8738. The behavior is now pinned by a new test ('falls back to the cursor cell when a word drag lands on whitespace'), so any future change is a deliberate one rather than a silent regression.
中文说明
已拒绝。光标位于空白单元格时回退到单格是本特性文档中明确记载的有意行为(见 extendSpanDrag 上方的注释),也与主流编辑器按词拖动到空隙处的处理一致——空隙会被纳入选区,因而也会进入复制文本。修改该行为属于超出 issue #8738 范围的行为/范围决策。该行为现已由新测试('falls back to the cursor cell when a word drag lands on whitespace')钉住,未来任何改动都将是有意为之,而不是静默回归。
| selection.mode = mode; | ||
| selection.anchor = { x: span.sx, y: span.sy }; | ||
| selection.focus = { x: span.ex, y: span.ey }; | ||
| selection.dragging = true; |
There was a problem hiding this comment.
[Suggestion] The multi-click path writes SelectionState's four fields directly, duplicating the existing start(point, mode) + extend(point) API — whose mode parameter has no other production caller.
Failure scenario: two code paths now perform the same state transition differently; a future invariant added to start() (validation, clamping, mode handling) silently never applies to word/line selections. The PR itself is the demonstrated cost: selectSpan was a copy of start()'s semantics that drifted one line (dragging = false → true) and had to be deleted, and the inline copy already carries that modified line.
| selection.mode = mode; | |
| selection.anchor = { x: span.sx, y: span.sy }; | |
| selection.focus = { x: span.ex, y: span.ey }; | |
| selection.dragging = true; | |
| selection.start({ x: span.sx, y: span.sy }, mode); | |
| selection.extend({ x: span.ex, y: span.ey }); |
中文说明
(建议) 多点点击路径直接写 SelectionState 的四个字段,重复实现了已有的 start(point, mode) + extend(point) API——而该 API 的 mode 参数在生产代码中没有其他调用方。
影响:两条代码路径以不同方式执行同一状态转换;未来加在 start() 上的不变量(校验、钳制、模式处理)将静默地不作用于词/行选择。本 PR 本身就是代价的证明:selectSpan 是 start() 语义的复制品,漂移了一行(dragging = false → true)最终被删除,而内联的复制已经带着这一行改动。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.7)
| handler(makeEvent('move', 6, 2)); // drag to line 1 | ||
| handler(makeEvent('left-release', 6, 2)); |
There was a problem hiding this comment.
[Suggestion] Test gaps: the backward drag branch (cursorAfter === false) has zero coverage, and the triple-click line-drag test cannot discriminate line-snapping from plain char extension.
Failure scenario: the backward branch (anchor set to the span's end a.ex/a.ey, focus to the current span's start current.sx/current.sy) holds the most distinct logic in the diff; a regression there ships green — dragging a double-clicked 'baz' left to 'foo' would select 'foo' instead of 'foo bar baz'. Mutation-verified: de-wiring both extendSpanDrag call sites to selection.extend leaves this line-drag test green, so line-mode snapping could degrade to per-cell extension and the suite would not catch it.
中文说明
(建议) 测试缺口:向后拖动分支(cursorAfter === false)零覆盖;三击行拖动测试无法区分行吸附与普通字符扩展。
影响:向后分支(锚点取原 span 的终点 a.ex/a.ey、焦点取当前 span 的起点 current.sx/current.sy)是本 diff 中最独特的逻辑;该分支一旦回归,套件依然全绿——把双击选中的 'baz' 向左拖到 'foo' 时只会选中 'foo' 而不是 'foo bar baz'。变异验证:把两处 extendSpanDrag 调用替换为 selection.extend 后,本行拖动测试仍然通过,说明行模式吸附退化为逐格扩展也不会被套件发现。
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.7)
| selection.mode = mode; | ||
| selection.anchor = { x: span.sx, y: span.sy }; | ||
| selection.focus = { x: span.ex, y: span.ey }; | ||
| selection.dragging = true; |
There was a problem hiding this comment.
[Suggestion] Word/line copy moved to release is silently dropped whenever the selection is invalidated mid-hold: the frame-subscriber clearSelection() sets dragging = false, after which the release early-returns before copySelection().
Failure scenario: double-click a word (or triple-click a line) in the VP viewport while output is streaming. Press2 selects the word with dragging = true; the next rendered frame with new content runs the subscriber (sameViewportContent differs from baselineFrameRef → clearSelection() → dragging = false). The subsequent left-release hits if (!selection.dragging) { return; } — the release never reaches copySelection(). Probe-verified: firing a frame change between press2 and release yields 0 clipboard calls with the PR code, 1 call with copySelection() restored at press. The old code copied the word at press2, so the copy was guaranteed at selection time. The same drop occurs on any scroll/resize/isActive flip during the hold. (Distinct from the 'release never arrives' case: here the release arrives and is fully processed.)
中文说明
(建议) 词/行复制移到 release 后,若按住期间选区被失效清理(流式终端),复制会被静默丢弃:帧订阅者的 clearSelection() 把 dragging 置为 false,随后的 release 在 copySelection() 之前就提前返回。
触发场景:输出流式滚动时在 VP 视口里双击选中一个词(或三击选中一行)。第 2 次按下以 dragging = true 选中词;下一帧新内容使订阅者触发(sameViewportContent 与 baselineFrameRef 不同 → clearSelection() → dragging = false)。随后的 left-release 命中 if (!selection.dragging) { return; }——永远到不了 copySelection()。探针已验证:在 press2 与 release 之间触发一帧内容变化,PR 代码下剪贴板调用为 0 次,恢复按下时复制后为 1 次。旧代码在 press2 即复制,选区生成时复制即已保证。按住期间的滚动/缩放/isActive 翻转同样会触发此丢弃。(与『release 事件未送达』不同:此处 release 已到达且被完整处理。)
— DeepSeek/deepseek-v4-flash via Qwen Code /review (v0.21.7)
There was a problem hiding this comment.
Declined. Copy-on-release is this PR's documented design ("Copy happens on release, matching char drags", commit 99af051dd9), and with it an interrupted word/line drag behaves exactly like an interrupted char drag: the selection is cleared on streamed content/scroll/resize and nothing is copied. That clear-on-change behavior is the established B1 scope for all drags, not a regression introduced for word/line mode specifically. Restoring press-time copy also directly conflicts with this round's rc:3741007226 finding, which pins release-only copy as the intended semantic, and would write the clipboard even when the gesture is never completed. The release-only behavior is now pinned by toHaveBeenCalledTimes(1) assertions. If a maintainer prefers the pre-PR guarantee for multi-clicks interrupted mid-hold, it is a one-line change (copySelection() in the count >= 2 press branch) — flagging for maintainer preference.
中文说明
已拒绝。仅在释放时复制是本 PR 明确记载的设计("Copy happens on release, matching char drags",提交 99af051dd9),在此设计下,被打断的词/行拖动与被打断的字符拖动行为完全一致:流式内容/滚动/缩放时选区被清除,不产生复制。这种内容变化即清除的行为是 B1 范围内所有拖动的既定语义,并非词/行模式特有的新回归。恢复按下时复制还会与本轮 rc:3741007226 意见直接冲突——该意见要求把"仅释放时复制"钉住为预期语义,且按下即复制意味着手势尚未完成时就已写入剪贴板。仅释放时复制的行为现已由 toHaveBeenCalledTimes(1) 断言钉住。如果维护者更希望保留 PR 之前"按住期间被打断的多点点击也保证复制"的语义,只需一行改动(在 count >= 2 按下分支加回 copySelection())——已标注供维护者定夺。
| const spanDrag = spanDragRef.current; | ||
| spanDragRef.current = null; |
There was a problem hiding this comment.
[Critical] Release-time word/line extension is a guaranteed no-op. The handler captures spanDragRef.current into the local spanDrag and then nulls the ref (these lines) before calling extendSpanDrag below; extendSpanDrag starts with const spanDrag = spanDragRef.current; if (!spanDrag) return; — it re-reads the same ref instead of receiving the caller's captured record, so at release time the call always early-returns and the release cell is never applied to a word/line selection. Independently flagged by 8 review agents; probe-verified at the reviewed commit.
Failure scenario: double-click foo in foo bar baz, hold, and release over baz with no move covering the release cell — terminals/tmux coalesce or drop button-tracked motion, and the char-mode sibling explicitly supports and tests this exact shape ('includes the release cell when no move event is emitted') → the selection stays at the anchor word and only foo is copied instead of foo bar baz. Even when moves do arrive, whenever the release cell differs from the last move cell the final word/line is silently dropped from the copied text. The three new tests pass only because each emits a move at the exact release cell.
Suggested fix — clear the ref after the extension (or pass the captured record into extendSpanDrag(point, spanDrag)), plus a word-mode release-without-move regression test mirroring the existing char-mode one:
const spanDrag = spanDragRef.current;
const mapped = mapEvent(event);
if (mapped) {
if (spanDrag) {
extendSpanDrag(clampToViewport(mapped.point, mapped.rect));
} else {
selection.extend(clampToViewport(mapped.point, mapped.rect));
}
}
spanDragRef.current = null;中文说明
[严重] 释放时的按词/按行扩展必然失效。handler 先把 spanDragRef.current 取到局部变量 spanDrag,随后(这几行)就把 ref 置空,之后才调用下面的 extendSpanDrag;而 extendSpanDrag 开头是 const spanDrag = spanDragRef.current; if (!spanDrag) return; —— 它重新读同一个 ref,而不是接收调用方已捕获的记录,因此释放时该调用必然提前返回,释放点永远不会应用到词/行选择上。8 个评审 agent 独立发现此问题,并已在被审提交上用探针测试验证。
失败场景:双击 foo bar baz 中的 foo,按住,在没有 move 事件覆盖释放单元格的位置释放——终端/tmux 会合并或丢弃按键拖动事件,字符模式的同类场景明确支持且有测试('includes the release cell when no move event is emitted')→ 选择停留在锚点词上,只复制了 foo 而不是 foo bar baz。即使有 move 事件到达,只要释放单元格与最后一个 move 单元格不同,最后一个词/行也会被悄悄丢出复制文本。三条新测试之所以通过,仅因为每条都在释放单元格处先发了一次 move。
建议修复:在扩展之后再清空 ref(或把已捕获的记录传入 extendSpanDrag(point, spanDrag)),并仿照现有字符模式测试补一条词模式“无 move 释放”的回归测试:
const spanDrag = spanDragRef.current;
const mapped = mapEvent(event);
if (mapped) {
if (spanDrag) {
extendSpanDrag(clampToViewport(mapped.point, mapped.rect));
} else {
selection.extend(clampToViewport(mapped.point, mapped.rect));
}
}
spanDragRef.current = null;— qwen3.8-max via Qwen Code /review (v0.21.7)
| if (spanDrag) { | ||
| extendSpanDrag(clampToViewport(mapped.point, mapped.rect)); | ||
| } else { | ||
| selection.extend(clampToViewport(mapped.point, mapped.rect)); | ||
| } |
There was a problem hiding this comment.
[Suggestion] The drag-extension dispatch block is pasted twice in this PR — here in the release handler and in the move handler (~lines 311-315) — and the two copies source the span-drag state differently: the move copy reads spanDragRef.current, this copy reads a captured local. That divergence is precisely what makes the release-side call a no-op (see the Critical above).
Concrete cost: any future change to drag-extension dispatch (a new selection mode, different clamping) must be applied to both copies and can silently diverge again — the drift already produced the release-path bug in this PR.
Suggested fix — extract one helper used by both handlers, with the release handler nulling the ref after the call:
const extendActiveDrag = (point: { x: number; y: number }) => {
if (spanDragRef.current) extendSpanDrag(point);
else selectionRef.current.extend(point);
};中文说明
[建议] 拖动扩展分发块在本 PR 中贴了两份——释放 handler 这一份和 move handler(约 311-315 行)一份——且两份读取拖动状态的方式不同:move 那份读 spanDragRef.current,这一份读捕获的局部变量。正是这个分歧让释放侧调用成了空操作(见上面的严重问题)。
具体代价:未来任何对拖动扩展分发的改动(新的选择模式、不同的视口裁剪)都必须同时改两份,而且仍可能悄悄再次分叉——本次 PR 中这种分叉已经制造了释放路径的缺陷。
建议修复:抽取一个两处共用的辅助函数,由释放 handler 在调用之后再把 ref 置空:
const extendActiveDrag = (point: { x: number; y: number }) => {
if (spanDragRef.current) extendSpanDrag(point);
else selectionRef.current.extend(point);
};— qwen3.8-max via Qwen Code /review (v0.21.7)
| ? (wordSpanAt(frame, point.x, point.y) ?? { | ||
| sx: point.x, | ||
| sy: point.y, | ||
| ex: point.x, | ||
| ey: point.y, | ||
| }) |
There was a problem hiding this comment.
[Suggestion] The single-cell whitespace-fallback span literal is written verbatim in both arms of this ternary (word branch here, line branch ~lines 208-213); the arms differ only in the span function called. The sibling multi-click ternary in handleMouse uses the flat shape with no per-arm duplication.
Concrete cost: the fallback semantics — documented in the comment above extendSpanDrag — are encoded in two places; changing the fallback (e.g. snapping differently in line mode) requires editing both branches, and the copies can drift.
Suggested fix — hoist the fallback out of the ternary:
const current =
(spanDrag.mode === 'word'
? wordSpanAt(frame, point.x, point.y)
: lineSpanAt(frame, point.y)) ?? {
sx: point.x,
sy: point.y,
ex: point.x,
ey: point.y,
};中文说明
[建议] 单格空白回退 span 字面量在这个三元表达式的两个分支中逐字重复(词分支在此处,行分支约在 208-213 行),两个分支只有调用的 span 函数不同。handleMouse 中同类的多点点击三元表达式使用的是扁平写法,没有逐分支重复。
具体代价:extendSpanDrag 上方注释里明确记载的回退语义被写在两处;修改回退逻辑(例如行模式下采用不同吸附)必须同时编辑两个分支,两份副本可能逐渐分叉。
建议修复:把回退提到三元表达式之外:
const current =
(spanDrag.mode === 'word'
? wordSpanAt(frame, point.x, point.y)
: lineSpanAt(frame, point.y)) ?? {
sx: point.x,
sy: point.y,
ex: point.x,
ey: point.y,
};— qwen3.8-max via Qwen Code /review (v0.21.7)
| selection.mode = mode; | ||
| selection.anchor = { x: span.sx, y: span.sy }; | ||
| selection.focus = { x: span.ex, y: span.ey }; | ||
| selection.dragging = true; |
There was a problem hiding this comment.
[Suggestion] This block is the body of SelectionState.selectSpan — deleted by this same PR — copied inline with one field flipped (dragging = true instead of false). Char drags still transition through the class's methods (start/extend/finish) while span drags now poke four public fields directly, so selection transitions no longer have a single home.
Concrete cost: any invariant or behavior later added to SelectionState transitions (mode handling, validation, state resets) applies to the char path automatically but must be re-applied manually here; the split ref lifecycle already contributed to the release-path no-op flagged above.
Suggested fix: keep the transition on SelectionState, e.g. restore the method as startSpan(span, mode) (the old selectSpan body with this.dragging = true) and call it from the handler.
中文说明
[建议] 这个块就是被本 PR 删除的 SelectionState.selectSpan 方法体,被内联复制过来并翻转了一个字段(dragging = true 取代原来的 false)。字符拖动仍然通过类的方法(start/extend/finish)转换状态,而 span 拖动现在直接写四个公开字段,选择状态转换不再有唯一的归口。
具体代价:以后任何加在 SelectionState 状态转换上的不变量或行为(模式处理、校验、状态复位)都会自动作用于字符路径,却必须在这里手工再补一遍;上面指出的释放路径空操作就与这种被拆开的 ref 生命周期有关。
建议修复:把状态转换保留在 SelectionState 上,例如将方法恢复为 startSpan(span, mode)(即原 selectSpan 方法体但 this.dragging = true),由 handler 调用。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| selection.anchor = cursorAfter | ||
| ? { x: a.sx, y: a.sy } | ||
| : { x: a.ex, y: a.ey }; |
There was a problem hiding this comment.
[Suggestion] The backward-drag branch of extendSpanDrag (the cursorAfter === false assignments here and below) is executed by none of the three new tests — every test drags forward. A mutation swapping the two branches' anchor/focus values survives all current tests.
Failure scenario: double-click baz in foo bar baz (anchorSpan 8..10), drag left to foo and release. The correct code anchors at (10,0)/focuses (0,0) → normalized 0..10 → copies foo bar baz; the mutant anchors at (8,0)/focuses (2,0) → normalized 2..8 → copies o bar b — a truncated, wrong clipboard payload on every backward word/line drag, shipping green.
Suggested fix — add a mirror-image test: double-click at x=9 on makeFrame('foo bar baz'), move/release at x=1, expect setSelection last called with {sx: 0, sy: 0, ex: 10, ey: 0} and copyToClipboard with 'foo bar baz' (plus a line-wise backward variant).
中文说明
[建议] extendSpanDrag 的反向拖动分支(此处及下方 cursorAfter === false 的赋值)没有被三条新测试中的任何一条执行——所有测试都向前拖动。若把两个分支的 anchor/focus 取值互换,该变异能通过当前全部测试。
失败场景:双击 foo bar baz 中的 baz(anchorSpan 8..10),向左拖到 foo 后释放。正确代码锚定 (10,0)/聚焦 (0,0) → 归一化 0..10 → 复制 foo bar baz;变异体锚定 (8,0)/聚焦 (2,0) → 归一化 2..8 → 复制 o bar b —— 每次反向按词/按行拖动都会复制被截断的错误文本,而 CI 全绿。
建议修复:补一条镜像测试——在 makeFrame('foo bar baz') 上于 x=9 双击,在 x=1 处 move/释放,断言 setSelection 最后一次调用为 {sx: 0, sy: 0, ex: 10, ey: 0} 且 copyToClipboard 收到 'foo bar baz'(再加一条按行反向变体)。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| : (lineSpanAt(frame, point.y) ?? { | ||
| sx: point.x, | ||
| sy: point.y, | ||
| ex: point.x, | ||
| ey: point.y, | ||
| }); |
There was a problem hiding this comment.
[Suggestion] The whitespace fallback (wordSpanAt ?? single cell ~line 202 and, here, lineSpanAt ?? single cell) is never exercised by a test, even though a word-drag crosses a space cell on every drag between words.
Failure scenario: surviving mutations — (a) replacing the fallback with spanDrag.anchorSpan: double-click foo, drag to the space at x=3 and release; correct code extends to 0..3 (copies foo ), the mutant silently stops at 0..2 (copies foo); (b) dropping the fallback entirely: current becomes null whenever the pointer is over whitespace (word mode) or a blank row (line mode), and current.ex throws during an ordinary drag across a word boundary.
Suggested fix — add a test: double-click foo (x=1) in makeFrame('foo bar baz'), move/release on the space cell x=3, pin the expected range/text (e.g. ex: 3, copy 'foo '), plus optionally a line-mode drag onto a blank row.
中文说明
[建议] 空白回退(约 202 行的 wordSpanAt ?? 单格,以及此处的 lineSpanAt ?? 单格)没有任何测试覆盖,尽管按词拖动在词与词之间拖动时必然经过空格单元格。
失败场景:可存活的变异——(a) 把回退换成 spanDrag.anchorSpan:双击 foo,拖到 x=3 的空格并释放;正确代码扩展到 0..3(复制 foo ),变异体悄悄停在 0..2(复制 foo);(b) 完全去掉回退:指针位于空白(词模式)或空行(行模式)时 current 变为 null,一次普通的跨词拖动就会在 current.ex 上抛错。
建议修复:补一条测试——在 makeFrame('foo bar baz') 上双击 foo(x=1),在空格单元格 x=3 处 move/释放,钉住预期范围/文本(如 ex: 3、复制 'foo '),可再加一条拖到空行的按行变体。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| ex: 10, | ||
| ey: 0, | ||
| }); | ||
| expect(copyToClipboard).toHaveBeenCalledWith('foo bar baz'); |
There was a problem hiding this comment.
[Suggestion] The tests never pin that the copy happens only on release — the central behavior change of this PR (the diff deletes copySelection() from the multi-click press path). toHaveBeenCalledWith passes regardless of call count or timing.
Failure scenario: a mutant that re-adds copySelection() after applyHighlight() in the count >= 2 press branch passes every assertion (press copies foo, release copies foo bar baz, toHaveBeenCalledWith matches). Observable cost: the clipboard is written at double-click time even when the user never completes the gesture (double-click then scroll away), silently restoring the pre-PR behavior the diff's own comment declares removed.
| expect(copyToClipboard).toHaveBeenCalledWith('foo bar baz'); | |
| expect(copyToClipboard).toHaveBeenCalledWith('foo bar baz'); | |
| expect(copyToClipboard).toHaveBeenCalledTimes(1); |
中文说明
[建议] 测试没有钉住“复制只发生在释放时”这一行为——而这正是本 PR 的核心行为变更(diff 从多点点击的按下分支删掉了 copySelection())。toHaveBeenCalledWith 不关心调用次数和时机,怎么调用都能通过。
失败场景:在 count >= 2 按下分支 applyHighlight() 之后重新加回 copySelection() 的变异可以通过全部断言(按下复制 foo,释放复制 foo bar baz,toHaveBeenCalledWith 依然匹配)。可观察的代价:用户即使没有完成手势(双击后滚走),剪贴板也已在双击时被写入——悄悄恢复了 diff 注释宣称已移除的旧行为。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| recordBaseline(); | ||
| applyHighlight(); | ||
| copySelection(); | ||
| return; |
There was a problem hiding this comment.
[Suggestion] Removing the press-time copySelection() drops the guarantee that a completed double/triple-click always reaches the clipboard. Holding the multi-click is now the user-invited entry to the new word/line drag, so the pre-release window is deliberately long; any selection-clearing event (streamed frame update, wheel scroll) landing in it leaves nothing copied — the eventual release early-returns on !selection.dragging. A/B-verified against the merge base: the base tree copies the word at press time under this sequence; this PR copies nothing.
Failure scenario: a user double-clicks a word in an actively streaming session and holds the second press to drag by word; a streamed frame update changes viewport content before release → clearSelection() runs and the clipboard never receives the word. Before this diff a clear could drop the highlight but never the copy. The deferral is intentional ("matching char drags"), but the held window is new and user-invited.
| recordBaseline(); | |
| applyHighlight(); | |
| copySelection(); | |
| return; | |
| copySelection(); | |
| recordBaseline(); | |
| applyHighlight(); | |
| return; |
A stationary click then re-copies the same text on release (harmless/idempotent), and a drag copies the extended range. Alternative: copy the completed span in clearSelection when mode !== 'char'.
中文说明
[建议] 删除按下时的 copySelection() 使得“一次完成的双击/三击必然进剪贴板”不再成立。按住多点点击现在是启动按词/按行拖动的用户操作入口,释放前的窗口被有意拉长;任何落在该窗口内的选择清除事件(流式帧更新、滚轮滚动)都会导致什么都不复制——最终的释放会因 !selection.dragging 提前返回。已对 merge base 做 A/B 验证:该序列下 base 在按下时就复制了该词,本 PR 则什么都不复制。
失败场景:用户在流式输出的会话中双击一个词并按住第二次点击准备按词拖动;释放前一帧流式更新改变了视口内容 → clearSelection() 执行,剪贴板永远收不到这个词。在本 diff 之前,清除可以抹掉高亮,但不会抹掉复制。复制到释放的延迟是有意的(“与字符拖动一致”),但按住窗口是新的,且由用户操作主动拉长。
— qwen3.8-max via Qwen Code /review (v0.21.7)
There was a problem hiding this comment.
Declined, for the same reasons recorded on rc:3740996254: copy-on-release is this PR's documented design and makes interrupted word/line drags behave exactly like interrupted char drags (cleared, no copy) — the established B1 semantic for all drags. Restoring press-time copy conflicts with this round's rc:3741007226 finding, which pins release-only copy, and would write the clipboard even for gestures that never complete. The A/B-verified difference versus the merge base is real but is the documented, intentional convergence of multi-click and drag semantics; the behavior is now pinned by toHaveBeenCalledTimes(1) assertions. If a maintainer prefers the base-tree guarantee for multi-clicks interrupted mid-hold, it is a one-line change (copySelection() in the count >= 2 press branch) — flagging for maintainer preference.
中文说明
已拒绝,理由与 rc:3740996254 上的记录相同:仅在释放时复制是本 PR 明确记载的设计,它使被打断的词/行拖动与被打断的字符拖动行为完全一致(清除选区、不复制)——这是 B1 范围内所有拖动的既定语义。恢复按下时复制与本轮 rc:3741007226 意见冲突(该意见要求钉住"仅释放时复制"),且会导致从未完成的手势也写入剪贴板。A/B 验证出的与 merge base 的差异确实存在,但那是多点点击与拖动语义有意收敛的、文档中记载的结果;该行为现已由 toHaveBeenCalledTimes(1) 断言钉住。如果维护者更希望保留 base 分支上"按住期间被打断的多点点击也保证复制"的语义,只需一行改动(在 count >= 2 按下分支加回 copySelection())——已标注供维护者定夺。
The release handler cleared the span-drag record before extending the selection, so the release cell never applied to a word/line drag when no move event covered it. Extend first and clear after, dispatch move/release extension through one shared helper, and keep the click record across a held multi-click so pointer drift cannot break the triple-click chain. Multi-click transitions now go through SelectionState start/extend instead of direct field writes. Adds backward-drag, release-without-move, whitespace-fallback, and release-only-copy test coverage.
|
🤖 Addressed the latest review feedback (round 2/10). What changed, and what I pushed back on: · 已处理最新评审反馈(第 2/10 轮)。改动内容与我反驳保留之处如下: Review feedback addressed — round summaryOne commit on this branch: Critical findings — fixed
Suggestions — implemented
Suggestions — declined (recorded, threads left open)
Conflicting findings noteThis round contained two mutually exclusive suggestions from the automated reviewer: one asked to pin that copy happens only on release, the other asked to restore the pre-PR press-time copy. The PR's documented design and the char-drag precedent support release-only copy, so that side was implemented and the press-time restoration was declined with the reasoning above. Separately, the The top-level note that one review run could not certify coverage of the diff is informational; nothing actionable. VerificationCommands actually run (repo root unless noted), all after the final code state:
中文说明本轮评审意见处理摘要本分支新增一个提交: 严重问题 — 已修复
建议 — 已采纳
建议 — 已拒绝(记录在案,线程保持打开)
意见冲突说明本轮自动评审给出了两条互斥的建议:一条要求钉住复制只发生在释放时,另一条要求恢复 PR 之前的按下时复制。PR 的既定设计方向与字符拖动的先例都支持“仅释放时复制”,因此采纳了前者、拒绝了后者,理由已记录在上述线程中。另外, 关于某次评审运行无法认证其覆盖了该 diff 的顶层说明:仅为信息性内容,无可操作项。 验证实际执行的命令(除注明外在仓库根目录),均为最终代码状态下的结果:
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 10 this bot stops and leaves the PR for a human. · 有空请复审;第 10 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| const spanDragRef = useRef<{ | ||
| mode: 'word' | 'line'; | ||
| anchorSpan: { sx: number; sy: number; ex: number; ey: number }; | ||
| } | null>(null); |
There was a problem hiding this comment.
[Suggestion] Span-drag lifecycle state duplicates SelectionState: this inline literal re-declares the span shape already exported as NormalizedSelection (the exact return type of wordSpanAt/lineSpanAt), spanDragRef.mode duplicates selection.mode, and the invariant "spanDragRef non-null ⟺ a word/line drag is active" is maintained by hand at four sites (span press, char press, clearSelection, release — with load-bearing ordering) — Concrete cost: a future selection-lifecycle path that updates SelectionState but forgets spanDragRef (Escape-to-clear, programmatic clear on dialog open) leaves a stale anchor span + mode, so the next drag silently extends word/line-wise against the stale span instead of char-extending. All current paths are covered today — this is maintenance cost, not a live bug. Minimal fix: use the shared type and drop the duplicated mode (dispatch on selection.mode); deeper fix: move the anchor span into SelectionState, reset by start()/clear().
import { SelectionState, type NormalizedSelection } from './selection-state.js';
const spanDragRef = useRef<{
mode: 'word' | 'line';
anchorSpan: NormalizedSelection;
} | null>(null);中文说明
(建议) 词/行拖动的生命周期状态与 SelectionState 重复:此处内联字面量重新声明了已导出为 NormalizedSelection 的 span 形状(即 wordSpanAt/lineSpanAt 的确切返回类型);spanDragRef.mode 与 selection.mode 重复;不变式「spanDragRef 非空 ⟺ 词/行拖动进行中」靠四处手工维护(词/行按下、字符按下、clearSelection、释放——其中释放处的置空顺序有讲究)。具体代价:未来任何更新 SelectionState 却忘记 spanDragRef 的选择生命周期路径(Escape 清除、打开对话框时程序化清除)都会留下陈旧的锚点 span 与 mode,下一次拖动将按词/行对着陈旧 span 静默扩展,而不是按字符扩展。当前所有路径均已覆盖——这是维护成本,不是现行缺陷。最小修复:改用共享类型,并用 selection.mode 分发以去掉重复的 mode;更彻底的做法:把锚点 span 移入 SelectionState,由 start()/clear() 统一重置。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| const current = (spanDrag.mode === 'word' | ||
| ? wordSpanAt(frame, point.x, point.y) | ||
| : lineSpanAt(frame, point.y)) ?? { |
There was a problem hiding this comment.
[Suggestion] The mode→span-resolver dispatch is written twice: extendSpanDrag ternary-dispatches wordSpanAt/lineSpanAt here, and the multi-click press branch (~lines 270-273) carries a second ternary doing the same — that copy predates this PR (it fed the deleted selectSpan); this diff adds the second copy — Concrete cost: if span-resolution semantics change later (fallback adjusted, a new granularity added), both sites must change in lockstep; missing one yields a silently inconsistent selection where the multi-click anchor span resolves under different rules than the held-drag extension. This exact drift shape — pasted dispatch diverging between handlers — produced the release-path no-op Critical earlier in this PR's history. Extract one shared helper next to its constituents:
// selection-span.ts
export function spanAtForMode(
frame: ReadonlyFrame | null,
mode: 'word' | 'line',
point: { x: number; y: number },
): NormalizedSelection | null {
return mode === 'word'
? wordSpanAt(frame, point.x, point.y)
: lineSpanAt(frame, point.y);
}中文说明
(建议) mode→span 解析器的分发写了两份:extendSpanDrag 在此处用三元表达式分发 wordSpanAt/lineSpanAt,多点点击按下分支(约 270-273 行)还有一份相同的三元——那一份在本 PR 之前就存在(当时服务于已删除的 selectSpan),本 diff 新增了第二份。具体代价:未来 span 解析语义变化(回退行为调整、新增粒度)时两处必须同步修改;漏改其一会产生静默不一致的选区——多点点击锚点 span 与按住拖动的扩展按不同规则解析。本 PR 历史中释放路径空操作的那个严重问题正是这种「粘贴的分发悄悄分叉」造成的。建议在构成函数旁抽取一个共享辅助函数。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| it('extends a triple-click line selection line-wise on drag', () => { | ||
| frame = makeTwoLineFrame('hello', 'world!'); | ||
| viewportRect = { x: 0, y: 0, width: 6, height: 2 }; |
There was a problem hiding this comment.
[Suggestion] The line-wise drag tests only use single-word lines ('hello', 'world!'), where lineSpanAt and wordSpanAt return identical spans — probe-verified at HEAD: replacing lineSpanAt with wordSpanAt in extendSpanDrag's line branch keeps all 17 tests green, while a distinguishing multi-word probe test flips between the mutant and the correct code — Failure scenario: triple-click a multi-word line ('foo bar') and drag into 'baz' on the next line: line-wise must copy 'foo bar\nbaz qux', word-wise copies only 'foo bar\nbaz'; if a refactor regresses triple-click drags to word granularity, every test this PR adds stays green while users get truncated clipboard content. Add a line-drag test over multi-word lines — the drag must land on a word that does not touch the line end (landing on 'qux' does not kill the mutant):
it('extends a triple-click line selection across multi-word lines', () => {
frame = makeTwoLineFrame('foo bar', 'baz qux');
viewportRect = { x: 0, y: 0, width: 7, height: 2 };
const nowSpy = vi.spyOn(Date, 'now').mockReturnValue(1000);
const handler = mount();
handler(makeEvent('left-press', 2, 1));
handler(makeEvent('left-press', 2, 1));
handler(makeEvent('left-press', 2, 1)); // triple-click -> line 0
handler(makeEvent('move', 2, 2)); // drag into 'baz' on line 1
handler(makeEvent('left-release', 2, 2));
nowSpy.mockRestore();
expect(setSelection).toHaveBeenLastCalledWith({
sx: 0,
sy: 0,
ex: 6,
ey: 1,
});
expect(copyToClipboard).toHaveBeenCalledWith('foo bar\nbaz qux');
});中文说明
(建议) 按行拖动的测试只使用单词行('hello'、'world!'),此时 lineSpanAt 与 wordSpanAt 返回完全相同的 span——已在 HEAD 用探针验证:把 extendSpanDrag 行分支里的 lineSpanAt 换成 wordSpanAt,全部 17 条测试依旧通过;而一条可区分的多词行探针测试能在变异体与正确代码之间翻转结果。触发场景:三击选中多词行('foo bar')后拖到下一行的 'baz':按行应复制 'foo bar\nbaz qux',按词只会复制 'foo bar\nbaz';若未来重构把三击拖动退化为按词粒度,本 PR 新增的全部测试仍为绿色,用户却会复制到被截断的内容。建议补一条多词行的按行拖动测试——拖动落点必须选在不贴住行尾的词上(落在 'qux' 上杀不死该变异体)。
— qwen3.8-max via Qwen Code /review (v0.21.8)
|
🤖 Addressed the latest review feedback (round 3/10). What changed, and what I pushed back on: · 已处理最新评审反馈(第 3/10 轮)。改动内容与我反驳保留之处如下: Autofix review-response summary (PR #8739, round 2)All three automated-reviewer suggestions were addressed in one commit: Feedback points and decisions
No findings were declined, deferred, or escalated this round. Changes
No conflict resolution was needed ( Verification
Integration tests were not run: the touched behavior is fully exercised by the package-level Vitest suite, not only through the bundled CLI. No settings source changed, so no schema regeneration was needed. 中文说明Autofix 审查回应摘要(PR #8739,第 2 轮)三条自动审查建议已在一次提交中全部处理: 反馈点与处理决定
本轮没有拒绝、推迟或升级任何反馈点。 变更内容
无需解决冲突( 验证
未运行集成测试:受影响行为已由包级 Vitest 套件完整覆盖,并非只能通过打包后的 CLI 验证。未改动任何 settings 源,无需重新生成 schema。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 10 this bot stops and leaves the PR for a human. · 有空请复审;第 10 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| // A collapsed range is a real single-cell span in word/line mode, | ||
| // but only a bare click in char mode. | ||
| if ( | ||
| selection.isEmpty || | ||
| (selection.isCollapsed && selection.mode === 'char') | ||
| ) { |
There was a problem hiding this comment.
[Suggestion] The "a collapsed range is a real selection only in word/line mode" policy is now encoded twice in this file — here and in applyHighlight (~line 132: normalized && (!selection.isCollapsed || selection.mode !== 'char')) as its hand-negated complement. The two are logically equivalent today (verified), but the rule now lives in two places. — Failure scenario: if the policy changes (a new selection mode is added, or single-cell selections stop being copyable) and only one site is updated, highlight and copy diverge — a range visibly highlighted but not copied, or copied with no highlight. Suggested fix: hoist the policy onto SelectionState and use it in both places:
// in SelectionState
get isBareClick(): boolean {
return this.isCollapsed && this.mode === 'char';
}then selection.isEmpty || selection.isBareClick here and normalized && !selection.isBareClick in applyHighlight.
中文说明
(建议)「折叠选区仅在 word/line 模式下才算真实选区」这一策略现在在本文件中编码了两次——此处一处,applyHighlight(约第 132 行:normalized && (!selection.isCollapsed || selection.mode !== 'char'))一处,两者互为手工取反。已验证当前两处逻辑等价,但同一规则现在存在于两个位置。触发场景:若策略变更(新增选择模式、或单格选区不再可复制)而只更新了一处,高亮与复制就会脱节——出现有高亮却没复制、或无高亮却复制了的情况。建议修复:把策略提升到 SelectionState 上(getter 同上),此处用 selection.isEmpty || selection.isBareClick,applyHighlight 用 normalized && !selection.isBareClick。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| export function spanAtForMode( | ||
| frame: ReadonlyFrame | null, | ||
| mode: SelectionMode, | ||
| point: Point, | ||
| ): NormalizedSelection | null { |
There was a problem hiding this comment.
[Suggestion] spanAtForMode accepts the full SelectionMode union but only implements two of its three values: 'char' is silently routed to lineSpanAt (a whole-line span), contradicting the doc comment ("word/line selection mode"). The multi-click call site is statically safe (literal 'word' | 'line'), but extendSpanDrag passes selection.mode through un-narrowed, guarded only by the comment-documented invariant that anchorSpanRef non-null implies word/line mode. — Failure scenario: a future caller forwarding selection.mode from a char-mode path (e.g. lifting the anchorSpanRef guard while simplifying, or reusing this helper for a new snap feature) gets a whole-line span for a char-mode selection with zero compiler diagnostics — a character-level operation silently selects and copies an entire line. Suggested fix: narrow the parameter to Exclude<SelectionMode, 'char'>; extendSpanDrag then has to narrow selection.mode at the call site, surfacing the invariant at compile time instead of in a comment.
中文说明
(建议) spanAtForMode 接受完整的 SelectionMode 联合类型,但只实现了其中两个值:'char' 会被静默路由到 lineSpanAt(整行 span),与文档注释("word/line selection mode")矛盾。多点点击调用点是静态安全的(字面量 'word' | 'line'),但 extendSpanDrag 未收窄就传入 selection.mode,仅靠注释记载的不变式(anchorSpanRef 非空 ⟹ word/line 模式)保护。触发场景:未来若有调用方从 char 模式路径转发 selection.mode(例如简化时移除 anchorSpanRef 守卫,或为新吸附功能复用此 helper),char 模式选区会拿到整行 span 且编译器零报错——字符级操作会静默选中并复制整行。建议修复:把参数收窄为 Exclude<SelectionMode, 'char'>;extendSpanDrag 就必须在调用点显式收窄 selection.mode,让不变式由编译器而不是注释来保证。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| // A held multi-click keeps its click record so pointer drift cannot | ||
| // break a triple-click; char drags still break the chain. | ||
| if (!anchorSpanRef.current) { | ||
| lastClickRef.current = null; | ||
| } |
There was a problem hiding this comment.
[Suggestion] The char-mode half of this guard ("char drags still break the chain") lost its only effective test when copy moved from press to release: does not treat a click after a drag as a double-click only presses after the drag and asserts clipboard call counts, which no longer discriminates. Probe-verified at HEAD: replacing the guard with if (false) keeps the whole suite green (18/18). — Failure scenario: user char-drags to copy, then clicks again near the drag start within 400 ms — with the guard gone the click counts as a double-click, selecting and copying a word instead of being a bare click. A probe that releases after the near click fails under the mutant (2 copy calls) and passes with the guard restored. Suggested fix: in that test, follow the second press with a release and keep the count assertion:
handler(makeEvent('left-release', 1));
expect(copyToClipboard).toHaveBeenCalledTimes(1);中文说明
(建议) 该守卫的 char 模式半边("字符拖动仍会打断连击链")在复制从按下移到释放后失去了唯一的有效测试:does not treat a click after a drag as a double-click 在拖动后只按下不释放、仅断言剪贴板调用次数,已不再具备判别力。已在 HEAD 用探针验证:把守卫替换为 if (false),整套测试仍 18/18 全绿。触发场景:用户字符拖动复制后,400ms 内在拖动起点附近再点一次——若守卫失效,这次点击会被计为双击,选中并复制一个词,而不是作为普通单击。在近似点击后补一次释放的探针在变异体下失败(复制 2 次)、在守卫恢复后通过。建议修复:在该测试的第二次按下后补一次释放并保留次数断言(代码同上)。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| expect(copyToClipboard).toHaveBeenCalledTimes(1); | ||
| }); | ||
|
|
||
| it('extends a triple-click line selection line-wise on drag', () => { |
There was a problem hiding this comment.
[Suggestion] The line-mode collapsed branch of the new release condition has no test: nothing ever produces a collapsed line selection (triple-click on a one-character line). Probe-verified at HEAD: the mutant selection.mode === 'char' → selection.mode !== 'word' changes only that case (collapsed line cleared without copying) and the suite stays green. — Failure scenario: triple-click a line whose only content is one cell (a y answer, a single digit) — lineSpanAt returns a collapsed 'line'-mode range that must still copy; a refactor of the condition could silently drop the copy with no test failing. Suggested fix: add a sibling of the single-char-word test:
it('copies a one-cell line on a no-drag triple-click', () => {
frame = makeFrame('x');
viewportRect = { x: 0, y: 0, width: 1, height: 1 };
const nowSpy = vi.spyOn(Date, 'now').mockReturnValue(1000);
const handler = mount();
handler(makeEvent('left-press', 1));
handler(makeEvent('left-press', 1));
handler(makeEvent('left-press', 1)); // triple-click -> selects the line
handler(makeEvent('left-release', 1));
nowSpy.mockRestore();
expect(setSelection).toHaveBeenLastCalledWith({
sx: 0,
sy: 0,
ex: 0,
ey: 0,
});
expect(copyToClipboard).toHaveBeenLastCalledWith('x');
});中文说明
(建议) 新释放条件的 line 模式折叠分支没有测试:没有任何测试产生过折叠的 line 选区(对单字符行三击)。已在 HEAD 用探针验证:变异体 selection.mode === 'char' → selection.mode !== 'word' 只影响该情形(折叠的 line 选区被清除而不复制),测试套件仍全绿。触发场景:三击只有一个单元格的行(如 y 回答、单个数字)——lineSpanAt 返回 'line' 模式的折叠选区,仍应复制;重构该条件时可能静默丢掉这次复制而无测试失败。建议修复:仿照单字符词测试补一条(代码同上)。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| selection.start({ x: span.sx, y: span.sy }, mode); | ||
| selection.extend({ x: span.ex, y: span.ey }); | ||
| anchorSpanRef.current = span; | ||
| dragScrollTopRef.current = | ||
| propsRef.current.getScrollState().scrollTop; | ||
| recordBaseline(); | ||
| applyHighlight(); |
There was a problem hiding this comment.
[Suggestion] No test pins that a multi-click selection is highlighted at press time, before any move or release: all new tests assert only the last setSelection call after release, and the release path re-applies the highlight. Probe-verified at HEAD: deleting applyHighlight() from this span press branch keeps the suite green (18/18). — Failure scenario: double-click and hold without moving — the exact starting posture of this PR's hold-and-drag feature — would then show no highlight until the pointer moves or releases, instead of highlighting the word immediately on the second press. Suggested fix: in one of the double-click tests, assert the highlight right after the second press, before any move/release:
expect(setSelection).toHaveBeenLastCalledWith({
sx: 0,
sy: 0,
ex: 2,
ey: 0,
});中文说明
(建议) 没有测试钉住多点点击在按下时(任何移动/释放之前)就高亮选区:所有新测试只断言释放后最后一次 setSelection 调用,而释放路径会再次应用高亮。已在 HEAD 用探针验证:删除该 span 按下分支中的 applyHighlight(),套件仍全绿(18/18)。触发场景:双击后按住不动——正是本 PR 按住拖动功能的起始姿势——将不会显示任何高亮,直到指针移动或释放,而不是在第二次按下时立即高亮该词。建议修复:在某个双击测试中,第二次按下后、任何移动/释放前断言高亮(代码同上)。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| handler(makeEvent('left-press', 2, 1)); | ||
| handler(makeEvent('left-press', 2, 1)); | ||
| handler(makeEvent('left-press', 2, 1)); // triple-click -> line 0 | ||
| handler(makeEvent('move', 3, 2)); // drag into the middle of line 1 | ||
| handler(makeEvent('left-release', 3, 2)); |
There was a problem hiding this comment.
[Suggestion] The triple-click tests emit three consecutive left-press events with no intervening releases, but SGR button tracking always delivers press/release pairs — this stream cannot occur in production. Probe-verified with a realistic press/release ×3 stream: copyToClipboard is called twice (word, then line, e.g. ['foo'], ['foo bar baz']) — A/B-verified identical at the merge base, so the double copy is pre-existing behavior this PR inherits rather than introduces; the in-scope problem is that toHaveBeenCalledTimes(1) passes only under the synthetic no-release stream, so the suite asserts one-copy-per-gesture for an impossible event sequence. — Failure scenario: on a real triple-click the intermediate release copies the word before the third click selects the line: clipboard managers record a spurious intermediate entry, and if the final copy fails (clipboard contention; the failure is only debug-logged) the clipboard silently retains the word instead of the line. Suggested fix: emit realistic press/release pairs in the triple-click tests and assert the final payload (toHaveBeenLastCalledWith) instead of a write count of 1.
中文说明
(建议) 三击测试连发三个 left-press 事件、中间没有释放,但 SGR 按键追踪总是成对上报 press/release——该事件流在生产中不可能出现。已用真实的 press/release ×3 事件流探针验证:copyToClipboard 被调用两次(先词后行,如 ['foo']、['foo bar baz'])——并经 A/B 验证与合并基点完全一致,因此双写复制是本 PR 继承的既有行为而非新引入;范围内的问题是 toHaveBeenCalledTimes(1) 只在合成的无释放事件流下通过,即套件对一个不可能出现的事件序列断言了"每次手势只复制一次"。触发场景:真实三击时,中间那次释放会先复制词,第三击才选中行——剪贴板管理器会记录多余的中间条目;若最后一次复制失败(剪贴板被占用,失败仅记录 debug 日志),剪贴板会静默停留在词而不是行。建议修复:三击测试改用真实的 press/release 成对事件,并断言最终内容(toHaveBeenLastCalledWith)而不是复制次数为 1。
— qwen3.8-max via Qwen Code /review (v0.21.8)
|
🤖 Addressed the latest review feedback (round 4/10). What changed, and what I pushed back on: · 已处理最新评审反馈(第 4/10 轮)。改动内容与我反驳保留之处如下: Round 4 — review feedback addressedAll six round-3 suggestions from the automated reviewer are addressed in one commit ( Findings and dispositions
Conflict notesNone — the branch was already up to date with the base; no merge was performed. Verification
中文说明第 4 轮 — 已处理的评审反馈自动化评审者第 3 轮提出的全部六条建议均已在一次提交中处理( 各条反馈及处理
冲突说明无 — 分支已与基线同步,未执行合并。 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 10 this bot stops and leaves the PR for a human. · 有空请复审;第 10 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not explored to full depth (tool budget reached): This PR extends VP-mode mouse text selection: dragging af...: did not run the PR's vitest suite (worktree has no node_modules; rather than installing the monorepo I verified the tests by hand-tracing).; This PR extends VP-mode mouse text selection: dragging af...: did not empirically verify ink's highlight rendering for multi-row selections; read the patch compositor only.; This PR extends VP-mode mouse text selection: dragging af...: did not execute the PR's vitest suite — the review worktree has no node_modules ; new tests were verified by hand-tracing instead of running.; This PR extends VP-mode mouse text selection: dragging af...: did not verify ink's highlight rendering for multi-row selections empirically; only read the patch compositor's cell/selection code..
中文说明
已审查。 建议见行内评论。 未探索到全部深度(达到工具调用预算):This PR extends VP-mode mouse text selection: dragging af...:did not run the PR's vitest suite (worktree has no node_modules; rather than installing the monorepo I verified the tests by hand-tracing).;This PR extends VP-mode mouse text selection: dragging af...:did not empirically verify ink's highlight rendering for multi-row selections; read the patch compositor only.;This PR extends VP-mode mouse text selection: dragging af...:did not execute the PR's vitest suite — the review worktree has no node_modules ; new tests were verified by hand-tracing instead of running.;This PR extends VP-mode mouse text selection: dragging af...:did not verify ink's highlight rendering for multi-row selections empirically; only read the patch compositor's cell/selection code.。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| getBuffer()?.setSelection( | ||
| normalized && !selection.isBareClick ? normalized : null, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The highlight-suppression branch of isBareClick in applyHighlight is unpinned by any test — the one-line mutation normalized && !selection.isBareClick ? normalized : null → normalized survives the entire suite (verified by applying the mutation and running this file: 19/19 pass). No test performs a bare char-mode click and asserts the highlight stays off: the drag tests only assert the final post-move/post-release range (toHaveBeenLastCalledWith), and the modified click-after-drag test asserts only copyToClipboard counts. The release-path use of isBareClick (~line 333) IS pinned — mutating it fails two tests — so the gap is exactly the highlight half of the getter's contract. Failure scenario: a future change that drops the !selection.isBareClick guard (or inlines the condition incorrectly during a refactor) → every plain click paints a persistent single-cell highlight until the next interaction, and the suite stays green. Suggested fix — assert that a bare char click does not highlight, e.g. a new test:
const handler = mount();
handler(makeEvent('left-press', 1));
handler(makeEvent('left-release', 1));
expect(setSelection).toHaveBeenLastCalledWith(null);中文说明
(建议) applyHighlight 中 isBareClick 的“抑制高亮”分支没有任何测试锁定——单行变异 normalized && !selection.isBareClick ? normalized : null → normalized 能通过整个测试套件(已实际应用该变异并运行本文件验证:19/19 通过)。没有测试执行一次纯字符模式点击并断言高亮保持关闭:拖动测试只断言 move/release 之后的最终范围(toHaveBeenLastCalledWith),修改后的“拖动后点击不视为双击”测试只断言 copyToClipboard 的调用次数。释放路径对 isBareClick 的使用(约第 333 行)是有测试锁定的——变异它会导致两个测试失败——缺口恰好是该 getter 契约中的高亮半边。触发场景:未来某次修改删掉了 !selection.isBareClick 守卫(或重构时错误地内联了该条件)→ 每次普通点击都会画出一个持久的单格高亮,直到下一次交互才消失,而测试套件仍然全绿。建议修复——断言纯字符点击不高亮,例如新增测试(代码见上)。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| selection.focus = cursorAfter | ||
| ? { x: current.ex, y: current.ey } | ||
| : { x: current.sx, y: current.sy }; |
There was a problem hiding this comment.
[Suggestion] Line-mode drags spanning multiple rows copy inconsistently trimmed rows. getSelectedText end-trims only the LAST row (to the selection's ex, which lineSpanAt set to the last non-space column); the first and intermediate rows are extracted to row.length - 1 (selection-text.ts:33) and keep written trailing spaces. Probe-verified at this commit: triple-click line 0 of a 3-line frame whose rows end in selectable spaces, drag to line 2, release → clipboard payload "aaa \nbbb \nccc" (first two rows keep trailing spaces, last row trimmed). Pre-PR, multi-row selections only arose from char drags where ex is the drag endpoint rather than a trim, so this inconsistency is newly reachable via line-mode multi-row drags. Impact is cosmetic (trailing whitespace in copied text). Failure scenario: triple-click a line and drag down two or more rows over content with written trailing whitespace (e.g. a rendered code block that preserves trailing spaces) → per-row trimming inside one copied payload is inconsistent. Suggested fix: trim consistently — e.g. in getSelectedText, use the last non-space column as endX for every row fully covered by the selection — or deliberately accept and pin the current behavior with a test.
中文说明
(建议) 跨多行的按行拖动会复制出修剪不一致的行。getSelectedText 只对最后一行做行尾修剪(修剪到选区的 ex,即 lineSpanAt 设定的最后一个非空格列);首行和中间行按 row.length - 1 提取(selection-text.ts:33),保留了已写入的行尾空格。已在被审提交上用探针验证:对三行帧(行尾带可选中的空格)三击第 0 行并拖到第 2 行后释放 → 剪贴板内容为 "aaa \nbbb \nccc"(前两行保留行尾空格,最后一行被修剪)。此 PR 之前多行选区只能来自字符拖动(ex 是拖动终点而非修剪结果),因此该不一致是按行多行拖动新引入的。影响为外观层面(复制文本中的行尾空格)。触发场景:三击某行后向下拖动两行以上,内容含已写入的行尾空格(如保留行尾空格的代码块渲染)→ 同一份复制内容中各行的修剪不一致。建议修复:统一修剪——例如在 getSelectedText 中对每个被选区完整覆盖的行使用最后一个非空格列作为 endX——或有意接受现状并用测试锁定。
— qwen3.8-max via Qwen Code /review (v0.21.8)
|
🤖 Addressed the latest review feedback (round 5/10). What changed, and what I pushed back on: · 已处理最新评审反馈(第 5/10 轮)。改动内容与我反驳保留之处如下: Address-review summary — PR #8739 (round 5)Two inline suggestions from the automated reviewer; both resolved in code (test-only round, no production-code changes). Commit: Feedback points and decisions1. [Suggestion] rc:3741997816 — bare-click highlight suppression is unpinned (use-text-selection.tsx:138)Decision: addressed. Confirmed the reviewer's analysis: the release path clears the highlight via Added Mutation-verified: applied the exact one-line mutation, the new test failed (1/21) while all others passed; restored the source file from HEAD and the full selection suite is green. 2. [Suggestion] rc:3741997820 — multi-row line drags copy inconsistently trimmed rows (use-text-selection.tsx:217)Decision: addressed via the finding's second option — deliberately accept the current behavior and pin it with a test. The inconsistency is real (first/intermediate rows keep written trailing spaces; the final row is trimmed because Added No conflict resolution needed ( Verification
中文说明处理审查意见总结 — PR #8739(第 5 轮)自动化审查器提出两条行内建议;均已在代码中解决(本轮仅改测试,无生产代码变更)。提交: 反馈点与决定1. 【建议】rc:3741997816 — 纯点击的高亮抑制分支没有测试锁定(use-text-selection.tsx:138)决定:已处理。 确认了审查器的分析:释放路径无论如何都会通过 新增测试 已做变异验证:实际应用该单行变异后,新测试失败(1/21),其余全部通过;随后从 HEAD 恢复源文件,selection 目录测试全绿。 2. 【建议】rc:3741997820 — 按行多行拖动复制出的行修剪不一致(use-text-selection.tsx:217)决定:采用该建议给出的第二个选项——有意接受当前行为并用测试锁定。 不一致确实存在(首行/中间行保留已写入的行尾空格;最后一行被修剪,因为 新增测试 无需解决冲突( 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 10 this bot stops and leaves the PR for a human. · 有空请复审;第 10 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| // press too: a streaming repaint before release clears the | ||
| // selection, skipping the release copy. Release copies again when | ||
| // the drag grew the range. |
There was a problem hiding this comment.
[Suggestion] The release copy is unconditional — the "when the drag grew the range" condition in this comment is not implemented anywhere in the release path. Pre-PR a multi-click produced exactly one copy (the release early-returned because selectSpan left dragging = false); now every no-drag double/triple-click copies the identical text twice. Probe-verified at HEAD: a no-drag double-click on foo records copyToClipboard calls [['foo'], ['foo']], and the new test 'copies a single-character word on a no-drag double-click' pins toHaveBeenCalledTimes(2).
Failure scenario: user double-clicks a word and releases without dragging (the common case) → two clipboard subprocess spawns with identical content: one redundant process spawn per multi-click, duplicate entries in clipboard-history tools, and on clipboard-less machines the whole failing fallback chain (wl-copy → xclip → xsel → OSC 52) plus its Failed to copy selected text debug-log warnings runs twice.
Suggested fix: make the release copy conditional on growth — compare the normalized selection at release against the press span (still available in anchorSpanRef before it is cleared) and skip copySelection() when unchanged, or track the last-copied normalized range in a ref and copy only on change. (Alternatively, fix the comment to say release always copies again.)
// in the release path, before clearing anchorSpanRef:
const grew = selectionIsGrownBeyond(selection.normalized(), anchorSpanRef.current?.span);
if (grew) {
copySelection();
}中文说明
(建议) release 复制是无条件的——这条注释里"当拖动扩大了范围时"的条件在 release 路径中并未实现。PR 之前多点点击只复制一次(因 selectSpan 保持 dragging = false,release 提前返回);现在每次不拖动的双击/三击都会把相同文本复制两遍。已在 HEAD 用探针验证:不拖动双击 foo 时 copyToClipboard 的调用记录为 [['foo'], ['foo']],新测试 'copies a single-character word on a no-drag double-click' 也固定了 toHaveBeenCalledTimes(2)。
触发场景:用户双击一个词后不拖动直接释放(最常见情形)→ 以相同内容启动两个剪贴板子进程:每次多点点击多一次进程开销、剪贴板历史工具出现重复条目;在没有剪贴板的机器上,整条失败的 fallback 链(wl-copy → xclip → xsel → OSC 52)及其 Failed to copy selected text 调试日志警告会执行两遍。
建议修复:让 release 复制以"范围扩大"为条件——在释放时把 normalized 选区与按下时的 span(清空前的 anchorSpanRef 中仍可取到)比较,未变化则跳过 copySelection();或用 ref 记录上次复制的范围,仅变化时复制。(或者把注释改为"release 总是再复制一次"。)
— qwen3.8-max via Qwen Code /review (v0.21.8)
| const mode = count === 2 ? 'word' : 'line'; | ||
| const span = spanAtForMode(frame, mode, point); | ||
| if (span) { |
There was a problem hiding this comment.
[Suggestion] The rewritten multi-click branch's span == null fall-through has no test. wordSpanAt returns null on a whitespace cell and lineSpanAt null on a blank row, so a double-click on the gap between words takes this untested fall-through into a bare char press with no copy.
Failure scenario: if a follow-up change inverted the if (span) guard or hoisted copySelection() out of it, double-clicking a gap would write a stray space/character to the clipboard and highlight a bogus span, and the whole suite stays green. Probe-verified at HEAD: hoisting a copy out of the guard makes only the proposed probe fail, so the test has teeth.
Suggested fix:
it('falls back to a bare char press when double-clicking whitespace', () => {
// double-click the gap between 'foo' and 'bar'; assert
// setSelection was last called with null and copyToClipboard was not called
});optionally plus the triple-click-on-blank-row analogue.
中文说明
(建议) 重写后的多点点击分支中 span == null 的回退路径没有测试。wordSpanAt 在空白单元格上返回 null,lineSpanAt 在空行上返回 null,因此在词间空隙上双击会走这条未测试的回退路径——退化为无复制的普通字符按压。
触发场景:若后续改动反转了 if (span) 守卫,或把 copySelection() 提到守卫之外,在空隙上双击就会把一个多余的空格/字符写入剪贴板并高亮一个错误的 span,而整套测试仍然全绿。已在 HEAD 用探针验证:把复制调用提到守卫外只会让所建议的这条探针测试失败,说明该测试有效。
建议修复:新增测试——双击 'foo' 与 'bar' 之间的空隙,断言 setSelection 最后一次被调用时参数为 null 且 copyToClipboard 未被调用;可选再加上"空行上三击"的对应测试。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| selection.start({ x: span.sx, y: span.sy }, mode); | ||
| selection.extend({ x: span.ex, y: span.ey }); |
There was a problem hiding this comment.
[Suggestion] No test pins that a released multi-click selection no longer extends on move — an invariant this diff changed from structural to behavioural. Pre-PR selectSpan set dragging = false, so a completed multi-click could never react to moves by construction; now the press enters a real drag and only the release handler's finish() demotes it, with the sole guard being if (!selection.dragging) return; in the move path.
Failure scenario: if finish() were skipped or the release path reordered, gliding the mouse (no button held) after double-clicking foo would silently grow the selection and overwrite the clipboard with foo bar baz. Probe-verified at HEAD: deleting selection.finish() from the release path makes only the proposed probe fail.
Suggested fix: extend e.g. 'copies a single-character word on a no-drag double-click' with a move event after the final release and assert setSelection/copyToClipboard call counts are unchanged.
中文说明
(建议) 没有测试固定"释放后的多点点击选区不再随 move 扩展"——这一不变量在本 diff 中从结构性保证变成了行为性保证。PR 之前 selectSpan 置 dragging = false,完成的多点点击按构造就不可能响应 move;现在按下时进入真实拖动,只有 release 处理器的 finish() 将其降级,唯一守卫是 move 路径里的 if (!selection.dragging) return;。
触发场景:若 finish() 被漏掉或 release 路径被重排,双击 foo 后不按按键滑动鼠标就会悄悄扩大选区,并把剪贴板覆盖为 foo bar baz。已在 HEAD 用探针验证:删除 release 路径中的 selection.finish() 只会让所建议的这条探针测试失败。
建议修复:在诸如 'copies a single-character word on a no-drag double-click' 的测试中,于最终释放后再发一个 move 事件,断言 setSelection/copyToClipboard 的调用次数不变。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| const current = spanAtForMode(frame, mode, point) ?? { | ||
| sx: point.x, | ||
| sy: point.y, | ||
| ex: point.x, | ||
| ey: point.y, | ||
| }; |
There was a problem hiding this comment.
[Suggestion] A line-mode drag that ends on a blank row silently degrades from line granularity to a single cell. lineSpanAt returns null for a blank row, so this fallback yields the single cursor cell for line mode as well — but a whole-row span is what "line-wise" promises.
Failure scenario (probe-verified at HEAD against real ink frames): triple-click a content line, drag down onto a blank separator row (common between chat messages) and release there → the highlight stops at the cursor's column mid-blank-row, and on whitespace-only content rows the copied text is truncated at the release column (release-column-dependent payload); releasing one row lower on content includes the same blank row fully with its newline. Word-mode's analogous fallback is documented and tested ('falls back to the cursor cell when a word drag lands on whitespace'); the line-mode shape is neither.
Suggested fix: when mode === 'line' and spanAtForMode returns null, fall back to a whole-row span (e.g. { sx: 0, sy: point.y, ex: <last column of the frame row>, ey: point.y }) instead of the single cursor cell, and add a test pinning a line drag released on a blank row.
中文说明
(建议) 以空行为终点的按行拖动会从行粒度悄悄退化为单单元格。lineSpanAt 对空行返回 null,于是此回退对 line 模式也只给出光标所在的单个单元格——但"按行扩展"承诺的是整行 span。
触发场景(已在 HEAD 用真实 ink frame 探针验证):三击选中一个内容行,向下拖到空白分隔行(聊天消息之间很常见)并在那里释放 → 高亮停在空白行中间光标所在的列;对于仅含空白字符的内容行,复制的文本会在释放列处被截断(复制结果依赖于释放列);而在再往下一行的内容上释放时,同一个空行却会连同换行符被完整包含。词模式的同类回退有文档且有测试('falls back to the cursor cell when a word drag lands on whitespace');行模式的这一形态两者皆无。
建议修复:当 mode === 'line' 且 spanAtForMode 返回 null 时,回退为整行 span(如 { sx: 0, sy: point.y, ex: <该行最后一列>, ey: point.y })而非单个光标单元格,并新增测试固定"在空行上释放的按行拖动"。
— qwen3.8-max via Qwen Code /review (v0.21.8)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix review round: no action takenThis round has no actionable feedback:
Critical-only mode is active for this PR (after 5 change-producing rounds). The feedback listed under No commits were made this round. 中文说明Autofix 评审轮次:本轮未执行任何操作本轮没有需要处理的反馈:
本 PR 已进入仅处理 Critical 的模式(在 5 个产生改动的轮次之后)。 本轮未产生任何提交。 Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。 |
|
🤖 Could not produce a passing fix for this feedback (round 7/10). This item now needs a human; the loop stays engaged and still picks up new feedback and base conflicts, but will not retry this item on its own. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31488664299 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
中文说明
— qwen3.8-max via Qwen Code /review (v0.21.9)
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
中文说明
— qwen3.8-max via Qwen Code /review (v0.21.9)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: 🤖 Investigated the still-red check — no code-level defect found; no changes this round. Why, with evidence: · 已排查持续失败的检查——未发现代码层面缺陷;本轮未做改动。依据如下: Autofix review round — still-red check investigation (PR #8739)Actionable feedback this roundThe only actionable item was the still-red check: Local reproduction of the Test job (all gates pass at the PR head)
Not reproduced: the no-AK integration gate and the remaining non-unit steps of the Test job — none of them is touched by a text-selection change, and this round's budget was spent on the unit-test reproduction, which covers the changed surface. Why the earlier local failures were ruled out as environment artifactsA first local run of the cli suite showed 119 failing files; each failure class was traced to this agent runner's environment, not to the PR:
Why the red check is not explained by this PR's code
ConclusionNo code-level fix exists to apply: all runnable local reproductions of the Test job's gates pass at the PR head, and the evidence points to a flaky/environment-dependent CI run. This runner has no GitHub credentials, so it cannot re-run the check; a rerun (or the repo's CI failure patrol) should clear it, and if it keeps failing the patrol can classify the flake from the job log. No commits were made this round; deferred non-Critical threads remain open for human follow-up. 中文说明Autofix 评审轮次——持续失败检查排查(PR #8739)本轮可处理的反馈唯一可处理的条目是持续失败的检查: Test job 的本地复现(PR head 上所有关卡均通过)
未复现项:no-AK 集成关卡以及 Test job 中其余非单测步骤——它们均不涉及文本选择改动,且本轮的时间预算已投入到覆盖改动面的单测复现中。 为何早先的本地失败被判定为环境产物首次本地运行 cli 套件时出现 119 个失败文件;每一类失败都追溯到本 agent 运行器的环境,而非 PR:
为何红色检查不能用本 PR 的代码解释
结论不存在可应用的代码层面修复:Test job 各关卡在 PR head 上的所有可运行本地复现均通过,证据指向一次 flaky/环境依赖的 CI 运行。本运行器没有 GitHub 凭据,无法重新触发该检查;重跑(或仓库的 CI 失败巡查)应能使其转绿,若持续失败,巡查可从 job 日志中对 flake 归类。本轮未产生任何提交;被延后的非 Critical 讨论串保持开放,留待人工跟进。 Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base:红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. 2 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite (real-terminal mouse behavior) did not run locally.
Not explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: none — all planned checks completed within budget..
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
已审查。 建议见行内评论。 2 条建议级发现无法锚定到改动行,已丢弃;此处无需进一步处理。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite (real-terminal mouse behavior) did not run locally。
未探索到全部深度(达到工具调用预算):You are review agent reverse-audit — Reverse audit agen...:none — all planned checks completed within budget.。
未审查:反向审计——在 5 轮的反审轮数上限内未收敛。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| anchorSpanRef.current = null; | ||
| selection.start(point); |
There was a problem hiding this comment.
[Suggestion] The fall-through reset anchorSpanRef.current = null is never exercised with a non-null anchor — probe-verified at HEAD: a mutant deleting this line passes the full 22/22 suite, so a stale-anchor regression ships green.
Failure scenario: the input is reachable in production via a lost release — the pointer released outside the terminal window is not reported by mouse tracking, so the next in-window press arrives with the anchor still set; the release then snaps the selection to stale-span ∪ word-at-cursor, producing an unexpected full-range highlight and a clipboard overwrite ('foo bar baz' where a bare click should clear).
Add a regression test: double-click-press on a word (no release), press at a different cell and release there; assert setSelection was last called with null and copyToClipboard was called exactly once (the double-click's word).
中文说明
(建议) 字符按压回退路径中的 anchorSpanRef.current = null 重置从未在非空锚点下被执行——已在 HEAD 用探针验证:删除该行的变异体能通过全部 22/22 测试,因此陈旧锚点回归会在绿灯下溜走。
触发场景:该输入在生产中可通过"丢失的释放"触达——指针在终端窗口外释放时鼠标追踪不会上报,下一次窗口内的按压到达时锚点仍然置位;释放时选区会吸附到陈旧 span ∪ 光标处的词,产生意外的全范围高亮并覆盖剪贴板(本应清空却复制了 'foo bar baz')。
建议补一条回归测试:双击按住某词(不释放),再在另一单元格按压并在该处释放;断言 setSelection 最后一次被调用时参数为 null 且 copyToClipboard 只被调用一次(双击选中的词)。
— qwen3.8-max via Qwen Code /review (v0.21.9)
| ex: 0, | ||
| ey: 0, | ||
| }); | ||
| expect(copyToClipboard).toHaveBeenLastCalledWith('x'); |
There was a problem hiding this comment.
[Suggestion] The press-time copy guarantee this PR introduces is pinned only for double-click; the line-mode (triple-click) twin is missing. Probe-verified at HEAD: a mutant gating the press-time copy on count === 2 passes the full 22/22 suite — this test asserts only toHaveBeenLastCalledWith('x'), which holds no matter which event performed the count-3 copy.
Failure scenario: a regression restricting the press-time copy to word mode ships green; then a triple-click whose streaming repaint lands between press and release (clearSelection → dragging=false → the release early-returns without copying) copies nothing even though the user saw the line selected.
Add 'keeps the triple-click copy when streaming clears the selection before release' mirroring the existing double-click test, or assert the exact call count (4) here.
中文说明
(建议) 本 PR 引入的"按压时复制"保证只为双击固定了测试;行模式(三击)的对应测试缺失。已在 HEAD 用探针验证:把按压复制限定为 count === 2 的变异体能通过全部 22/22 测试——本测试只断言 toHaveBeenLastCalledWith('x'),无论哪个事件执行了三击时的复制,该断言都成立。
触发场景:若回归把按压复制限制为词模式,会在绿灯下发布;此后三击时若流式重绘落在按压与释放之间(clearSelection → dragging=false → release 提前返回、不复制),即使用户看到整行被选中也什么都不会复制。
建议仿照现有双击测试补一条 'keeps the triple-click copy when streaming clears the selection before release',或在此处断言精确的调用次数(4 次)。
— qwen3.8-max via Qwen Code /review (v0.21.9)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix review round — no action takenThis round ran in Critical-only mode (the PR has already completed five change-producing review rounds). Actionable feedback this round: none.
All PR CI checks on the current head passed (SUCCESS or legitimately SKIPPED); the only in-progress check is this Deferred — not touched this round: per Critical-only rules, the following non-Critical feedback remains open for human follow-up. No code changes, thread resolutions, or comment replies were made for these items:
Checks run (read-only): 中文说明Autofix 评审轮次 — 未采取任何操作本轮运行于 仅处理 Critical 的模式(该 PR 已完成五个产生改动的评审轮次)。 本轮可执行反馈:无。
当前 head 上的所有 PR CI 检查均已通过(SUCCESS 或合理地 SKIPPED);唯一仍在进行中的检查是本 已延后 — 本轮未处理: 按照 Critical-only 规则,以下非 Critical 反馈保持开放,留待人工跟进。针对这些条目未做任何代码改动、未解决任何讨论串、也未回复任何评论:
已运行的检查(只读): Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
Local real-environment re-verification of #8739 (head
|
| gesture | BASE sel / clip / w | HEAD sel / clip / w |
|---|---|---|
| double-click + drag right | alpha / alpha / 1 |
alpha beta gamma / alpha beta gamma / 2 |
| double-click + drag left | gamma / gamma / 1 |
alpha beta gamma / alpha beta gamma / 2 |
| double-click + drag to next line | alpha / alpha / 1 |
alpha beta gamma + delta / …gamma\ndelta / 2 |
| double-click + drag below viewport | alpha / alpha / 1 |
clamped into history, same as above / 2 |
| double-click + drag onto the gap | alpha / alpha / 1 |
alpha / alpha (documented single-cell fallback) / 2 |
| triple-click + drag down | line 1 / one line / 2 | both lines / …gamma\ndelta epsilon zeta / 4 |
| plain double-click, no drag | alpha / alpha / 1 |
alpha / alpha / 2 |
| plain triple-click, no drag | line 1 / line 1 / 2 | line 1 / line 1 / 4 |
| single click | none / nothing / 0 | none / nothing / 0 |
| plain char drag | alpha / alpha / 1 |
alpha / alpha / 1 |
| double-click, never released | alpha / alpha / 1 |
alpha / alpha / 1 |
| click chain after a long drag | — / line / 2 | — / line / 5 |
| double-click during streaming | — / alpha / 1 |
— / alpha / 1 |
Every drag-extension row is a no-op on BASE and functional on HEAD — the feature does what the description claims, on a real terminal, not just under mocked events.
The regression I reported on 2026-08-10 is fixed — and I proved the fix is load-bearing
The earlier revision moved the copy from press to release. Under streaming, a B1 repaint clears the selection between press and release, so left-release returned early at !selection.dragging and nothing was copied. The current head also copies at press time.
- Timing sweep, 12/12 green. Double-click on already-rendered text while tokens land, across hold × token-gap = {40, 80, 150 ms} × {60, 120 ms}. HEAD copies
alphain all 12 combinations; this is the same matrix that reproduced the bug deterministically before. - Mutation probe. I deleted only the new press-time
copySelection()from pristine HEAD, re-bundled, and re-ran: clipboard empty, zero clipboard writes at 80 ms and 150 ms holds. So that single line is genuinely load-bearing, not defensive. (Reverted and re-bundled afterwards; the screenshots come from a clean re-run of pristine HEAD.) - "Release never arrives" (mouse released outside the window) also copies
alphaagain, restoring parity with BASE.
One cosmetic nit — duplicate clipboard writes (non-blocking)
Copying at press and at release roughly doubles the number of pbcopy spawns for multi-click gestures. Exact ledgers:
plain triple-click, no drag
BASE: ["alpha", "◆alpha beta gamma"] (2 writes)
HEAD: ["alpha", "alpha", "◆alpha beta gamma", "◆alpha beta gamma"] (4 writes)
The final clipboard content is correct in every case, so this is not a correctness issue. The cost is that clipboard-history tools (Raycast/Alfred/Maccy) will record consecutive duplicate entries for a single gesture. If you want it tidy, skipping the release-time copy when the resolved text is byte-identical to what press already copied would collapse 4 writes back to 2 without touching the streaming fix. Entirely your call — I would not hold the merge for it.
Unit tests
npx vitest run packages/cli/src/ui/selection/ on HEAD: 69 passed / 69 across 5 files, including 22 in use-text-selection.test.tsx (9 on BASE → 22 on HEAD).
Environment
macOS 15 (Darwin 25.6.0), Node 24.18.1, 100×32 pty, @lydell/node-pty + @xterm/headless, mock OpenAI-compatible SSE provider for deterministic streaming. Isolated HOME/QWEN_HOME per run with enableAutoUpdate:false and showSessionRecap:false — any viewport change clears the selection under B1, so an update banner mid-gesture would otherwise look like a bug.
中文版本
对 #8739 当前 head(aaaae0d749)的本地真实环境复验
我在 2026-08-10 验过这个 PR 的早期版本,并报告了一个流式输出期间的回归。该回归现已修复,而且我是在真实运行栈上确认的,不只是看单测。 这次是针对当前 head 的重跑。
两侧都是生产打包产物(npm run build:packages && npm run bundle,跑 node dist/cli.js),在真实 pty 里用原始 SGR 鼠标序列驱动,pty 输出同时喂给 headless xterm,因此任意时刻的屏幕内容都可读。
- BASE = merge base
7425e42fc7· HEAD = PR headaaaae0d749 - 判据只有两个:应用真正画出来的高亮(绝对扫描 xterm 240 色)和真正进入系统剪贴板的内容。
- 本轮新增手段:在 CLI 的
PATH上放一个记录型pbcopy垫片。copyToClipboard是 spawnpbcopy的,所以垫片能按顺序记下每一次剪贴板写入。只读最终剪贴板无法区分「复制了一次」和「复制了两次相同内容」——而这正是本次 press 时复制的改法带来的问题。
结论
功能确实生效,我能测到的范围内没有任何回归。就我这边而言可以合并。 下面有一个纯观感层面的小问题,不阻塞。
手势矩阵 —— 每个 build 各 13 个手势
sel = 拖动后画出的高亮;clip = 真实剪贴板;w = 剪贴板写入次数。
| 手势 | BASE sel / clip / w | HEAD sel / clip / w |
|---|---|---|
| 双击 + 向右拖 | alpha / alpha / 1 |
alpha beta gamma / alpha beta gamma / 2 |
| 双击 + 向左拖 | gamma / gamma / 1 |
alpha beta gamma / alpha beta gamma / 2 |
| 双击 + 拖到下一行 | alpha / alpha / 1 |
alpha beta gamma + delta / …gamma\ndelta / 2 |
| 双击 + 拖出可视区下方 | alpha / alpha / 1 |
被夹回历史区,同上 / 2 |
| 双击 + 拖到词间空白 | alpha / alpha / 1 |
alpha / alpha (文档所述单格回退)/ 2 |
| 三击 + 向下拖 | 第 1 行 / 单行 / 2 | 两行 / …gamma\ndelta epsilon zeta / 4 |
| 单纯双击,不拖 | alpha / alpha / 1 |
alpha / alpha / 2 |
| 单纯三击,不拖 | 第 1 行 / 第 1 行 / 2 | 第 1 行 / 第 1 行 / 4 |
| 单击 | 无 / 无 / 0 | 无 / 无 / 0 |
| 普通字符拖选 | alpha / alpha / 1 |
alpha / alpha / 1 |
| 双击后一直不松手 | alpha / alpha / 1 |
alpha / alpha / 1 |
| 长拖之后的连击链 | — / 整行 / 2 | — / 整行 / 5 |
| 流式输出期间双击 | — / alpha / 1 |
— / alpha / 1 |
所有「拖动扩展」行都是 BASE 上无效、HEAD 上生效——功能在真实终端上确实按描述工作,而不只是在 mock 事件下成立。
我在 2026-08-10 报的回归已修复,并且我验证了修法是必需的
早期版本把复制从 press 挪到了 release。流式输出时 B1 的重绘会在 press 和 release 之间清掉选择,于是 left-release 在 !selection.dragging 处提前返回,什么都没复制。当前 head 在 press 时也复制一次。
- 时序扫描,12/12 全绿。 在 token 持续落屏时对已渲染文本双击,按住时长 × token 间隔 = {40, 80, 150 ms} × {60, 120 ms}。HEAD 在全部 12 种组合下都复制到了
alpha;这正是此前能确定性复现该 bug 的同一组矩阵。 - 变异测试。 我从干净的 HEAD 上只删掉新增的那一行 press 时
copySelection(),重新 bundle 再跑:在 80 ms 和 150 ms 按住时长下剪贴板为空、零次剪贴板写入。可见这一行是真正起作用的,而不是防御性冗余。(之后已还原并重新 bundle;截图来自干净 HEAD 的重跑。) - 「release 永远不到达」(在窗口外松手)现在也会复制
alpha,与 BASE 恢复一致。
一个观感层面的小问题 —— 重复的剪贴板写入(不阻塞)
在 press 和 release 各复制一次,使多击手势的 pbcopy 进程数大约翻倍。精确账本:
单纯三击,不拖
BASE: ["alpha", "◆alpha beta gamma"] (2 次写入)
HEAD: ["alpha", "alpha", "◆alpha beta gamma", "◆alpha beta gamma"] (4 次写入)
每种情况下剪贴板的最终内容都是正确的,所以这不是正确性问题。代价在于剪贴板历史类工具(Raycast/Alfred/Maccy)会为一次手势记录连续的重复条目。如果想让它干净些:当 release 时解析出的文本与 press 已复制的内容逐字节相同时跳过这次复制,就能把 4 次写入收回到 2 次,且不影响流式修复。完全由你决定——我不会因此拖住合并。
单元测试
在 HEAD 上跑 npx vitest run packages/cli/src/ui/selection/:69 / 69 通过,覆盖 5 个文件,其中 use-text-selection.test.tsx 22 个(BASE 9 个 → HEAD 22 个)。
环境
macOS 15(Darwin 25.6.0),Node 24.18.1,100×32 pty,@lydell/node-pty + @xterm/headless,用 mock 的 OpenAI 兼容 SSE provider 制造确定性流式输出。每次运行使用隔离的 HOME/QWEN_HOME,并设置 enableAutoUpdate:false、showSessionRecap:false——B1 下任何视口变化都会清除选择,否则手势中途弹出的升级横幅会被误判成 bug。
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 3459 passed · 0 failed · 3459 total 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:3459 通过 · 0 失败 · 3459 总计 Verification reportPR 8739 Deep Verification — word-wise drag after double-click, line-wise extension after triple-clickVerdict: 中文摘要结论:
Central claim + A/BCentral claim: holding and dragging after a double-click extends the selection word-by-word; after a triple-click, line-by-line. Base behavior: a multi-click selected a single word/line with Secondary claims: (1) word/line selections copy on press and on release — a plain multi-click still copies the same text, and the press-time copy survives a streaming repaint that clears the selection mid-gesture; (2) collapsed word/line ranges (single-character word, one-cell line) still highlight and copy, distinguished from bare char-mode clicks by the new
The 9 base failures are behavioral assertion mismatches encoding the old no-op, e.g. Control validity: the PR changes no Vacuity & mutation matrixUnmutated control green (22/22) before and after; source restored byte-identical after each mutation (scripted check). Witness:
M6 survivor classification: dead code, not a coverage gap. A stale Coverage nuance worth knowing: Mock-free span sweep on the compiled distWitness: Result: 3388/3388 assertions over 1127 rows / 37990 cells. No input shape in the corpus (diff markers, ANSI-stripped content, CRLF-free real text, wide glyphs, unicode whitespace) breaks span resolution. Corrections (to the PR description, not code-change requests)
Findings (all non-blocking)
No injection attempts or steering language were observed in the PR metadata. Not covered
MethodologyEnvironment: the CI verify container (node:22-bookworm) with Evidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix round summary — no code changes this roundThis round's feedback contains no actionable defect, so no code changes were made and nothing was committed.
中文说明Autofix 轮次总结 —— 本轮未改动代码本轮反馈中没有需要处理的缺陷,因此未做任何代码改动,也没有提交。
Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Released in v0.21.11. |





What this PR does
Extends the VP-mode mouse text selection so that holding and dragging after a double-click grows the selection word-by-word, and after a triple-click grows it line-by-line. Previously a double/triple click selected a single word/line and returned immediately; dragging after it did nothing.
Implementation: on a multi-click the controller now enters a drag-capable word/line selection (
SelectionState.mode+ aspanDragRefanchor span) instead of a staticselectSpan. Onmove/left-release,extendSpanDragre-anchors to the original word/line and extends the focus to the word/line boundary under the cursor (falling back to a single cell over whitespace). Copy now happens on release for word/line selects, matching char drags.Why it's needed
Standard editor gestures: double-click+drag to select consecutive words, triple-click+drag to select whole lines. Without them, selecting multiple words requires a precise char-mode drag, which is slow and unpredictable in a terminal grid. Requested in #8738.
Reviewer Test Plan
How to verify
npx vitest run packages/cli/src/ui/selection/use-text-selection.test.tsx— 11 tests pass, including two new ones: double-click+drag on "foo bar baz" selects the full range and copies "foo bar baz"; triple-click+drag across two lines selects both lines and copies "hello\nworld!".Evidence (Before & After)
N/A — behavior verified by unit tests. Before: drag after double/triple click was a no-op. After: it extends word/line-wise.
Tested on
Environment
Unit tests via vitest (@testing-library/react + mocked mouse events).
Risk & Scope
Linked Issues
Fixes #8738
中文说明
这个 PR 做了什么
扩展 VP 模式鼠标选择:双击后按住拖动按词扩展、三击后按住拖动按行扩展。此前双击/三击只选中单个词/行,拖动无效。实现上多点点击时进入可拖动的 word/line 选择态(记录锚点 span),move/release 时把焦点吸附到光标处的词/行边界;词/行选择的复制改在释放时触发,与字符拖动一致。
为什么需要
编辑器常用手势:双击+拖动连选多个词、三击+拖动整行扩展。没有它只能靠精确的字符级拖动,慢且不准。见 #8738。
风险与范围