fix(cli): make @ completion category tabs clickable - #8395
Conversation
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. Test Plan (not a blocker): src/ui/components/CompletionCategoryMouseController.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.mouse.test.tsx — no such file or directory; src/ui/components/InputPrompt.suggestionMouse.test.tsx — no such file or directory; src/ui/hooks/useCompletion.test.ts — no such file or directory.
中文说明
已审查——无阻断问题。 建议见行内评论。 Test Plan（非阻断）：src/ui/components/CompletionCategoryMouseController.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.mouse.test.tsx — no such file or directory; src/ui/components/InputPrompt.suggestionMouse.test.tsx — no such file or directory; src/ui/hooks/useCompletion.test.ts — no such file or directory。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| handler(makeEvent({ name: 'left-press', col: 19, row: 5 })); | ||
|
|
||
| expect(onSelectCategory).toHaveBeenCalledWith('session'); |
There was a problem hiding this comment.
[Suggestion] The SGR→0-based column offset (event.col - 1) in CompletionCategoryMouseController is not pinned by any test — the mutation event.col - 1 → event.col survives all three controller tests. — Concrete cost: every test coordinate is strictly interior/exterior under both mappings (col 19 → 18 or 19 both land inside the mocked session rect [18,24); col 9 → 8 or 9 both miss). A click at SGR col 18 (layout col 17, the gap just before the Sessions tab) selects nothing under correct code but selects 'session' under the mutant, so a one-column hit-region shift on the column side would ship green.
Add a boundary case that flips under the mutation:
handler(makeEvent({ name: 'left-press', col: 18, row: 5 }));
expect(onSelectCategory).not.toHaveBeenCalled();中文说明
CompletionCategoryMouseController 里的 SGR→0 基列偏移（event.col - 1）没有被任何测试钉住——把 event.col - 1 突变成 event.col 后，三个控制器测试仍全部通过。具体代价：所有测试坐标在两种映射下都严格落在命中区内部或外部（col 19 → 18 或 19 都落进 mock 的 session 矩形 [18,24)；col 9 → 8 或 9 都错过）。真实输入可以区分二者：在 SGR col 18（布局 col 17，即 Sessions 标签前的空隙）点击时，正确代码什么都不选，而突变体会选中 'session'，因此列方向上一格的命中区偏移会在测试全绿的情况下溜进去。
建议补充一个能在该突变下翻转的边界用例（见上方代码块）。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| if ( | ||
| category === activeCategory || | ||
| !availableCategories.includes(category) | ||
| ) { |
There was a problem hiding this comment.
[Suggestion] The same-category no-op guard in selectCategory (category === activeCategory) is not pinned by any test — removing that clause survives the whole suite. — Concrete cost: the mouse controller emits onSelectCategory for any tab click, including the already-active tab (neither the controller nor handleCategorySelect filters against activeCategory). Under the mutant, selectCategory('all') while activeCategory === 'all' falls through to setActiveSuggestionIndex(0) / setVisibleStartIndex(0), so a no-op click on the active tab jumps the highlight and scroll position to the top of the list.
Add a case in useCompletion.test.ts that re-selects the active category and asserts the indices are preserved:
it('keeps active and scroll indices when re-selecting the active category', () => {
const { result } = renderHook(() => useCompletion());
act(() => {
result.current.setSuggestions(mixed);
result.current.setActiveSuggestionIndex(1);
result.current.setVisibleStartIndex(1);
});
act(() => result.current.selectCategory('all')); // already active
expect(result.current.activeCategory).toBe('all');
expect(result.current.activeSuggestionIndex).toBe(1);
expect(result.current.visibleStartIndex).toBe(1);
});中文说明
selectCategory 中的同类别空操作守卫（category === activeCategory）没有被任何测试钉住——删掉这一子句后整个测试套件仍全部通过。具体代价：鼠标控制器会对任意标签点击发出 onSelectCategory，包括点击当前已激活的标签（控制器和 handleCategorySelect 都没有针对 activeCategory 做过滤）。在突变体下，当 activeCategory === 'all' 时调用 selectCategory('all') 会穿透到 setActiveSuggestionIndex(0) / setVisibleStartIndex(0)，于是点击当前激活标签这个空操作会把高亮和滚动位置跳到列表顶部。
建议在 useCompletion.test.ts 中补充一个用例：重新选中当前激活的类别，并断言高亮/滚动索引保持不变（见上方代码块）。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. 1 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 did not run locally. Test Plan (not a blocker): src/ui/components/CompletionCategoryMouseController.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.mouse.test.tsx — no such file or directory; src/ui/components/InputPrompt.suggestionMouse.test.tsx — no such file or directory; src/ui/hooks/useCompletion.test.ts — no such file or directory.
中文说明
已审查。 建议见行内评论。 1 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。 未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。 Test Plan（非阻断）：src/ui/components/CompletionCategoryMouseController.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.mouse.test.tsx — no such file or directory; src/ui/components/InputPrompt.suggestionMouse.test.tsx — no such file or directory; src/ui/hooks/useCompletion.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.21.4)
| expect(CompletionCategoryMouseController).toHaveBeenCalled(); | ||
| const props = vi.mocked(CompletionCategoryMouseController).mock.calls[0][0]; | ||
| expect(props.categories).toEqual(['all', 'file', 'session']); |
There was a problem hiding this comment.
[Suggestion] The wiring test pins only categories and onSelectCategory on the mocked CompletionCategoryMouseController — the containerRef/categoryRefs plumbing and the tab-bar ref collection (categoryRefs.current[i] = node) are asserted nowhere in the suite. — Failure scenario: the type-valid one-line mutation categoryRefs={itemRefs} at the controller mount site (both refs are MutableRefObject<Array<DOMElement | null>>, so typecheck passes) survives every automated test — verified: with the mutation in place all 51 tests across the 5 relevant files still pass, while the control mutation on the asserted categories prop fails as expected. The controller is mocked here, never mounted in SuggestionsDisplay.test.tsx (no mouseEnabled), and the controller's own unit test injects synthetic refs, so a ref-index swap would ship green and tab clicks in production would resolve against suggestion-row rects, silently selecting the wrong category or nothing.
Pin the tab-bar ref collection in this same wiring test — refs are populated by SuggestionsDisplay's own render even while the controller is mocked, so categoryRefs.current holds one node per tab (3 in this fixture); under the mutation it would hold the 2 suggestion-row nodes instead, which makes this assertion mutation-fatal:
| expect(CompletionCategoryMouseController).toHaveBeenCalled(); | |
| const props = vi.mocked(CompletionCategoryMouseController).mock.calls[0][0]; | |
| expect(props.categories).toEqual(['all', 'file', 'session']); | |
| expect(CompletionCategoryMouseController).toHaveBeenCalled(); | |
| const props = vi.mocked(CompletionCategoryMouseController).mock.calls[0][0]; | |
| expect(props.categories).toEqual(['all', 'file', 'session']); | |
| expect(props.categoryRefs.current.filter(Boolean)).toHaveLength(3); |
中文说明
[建议] 接线测试只钉住了 mock 的 CompletionCategoryMouseController 上的 categories 和 onSelectCategory —— containerRef/categoryRefs 接线以及标签栏 ref 收集（categoryRefs.current[i] = node）在整个测试套件中没有任何断言。— 失败场景：在控制器挂载处做一个类型合法的单行突变 categoryRefs={itemRefs}（两个 ref 都是 MutableRefObject<Array<DOMElement | null>>，类型检查能通过），可以在所有自动化测试下存活——已验证：突变后 5 个相关文件的全部 51 个测试仍然通过，而对已断言的 categories 属性做对照突变则如期失败。由于控制器在这里被 mock、在 SuggestionsDisplay.test.tsx 中从未真正挂载（没有 mouseEnabled）、控制器自己的单测又注入合成 ref，ref 索引错配会在测试全绿的情况下发布，生产中标签点击会按建议行的命中区域解析，静默选中错误分类或什么都不选。建议在同一个接线测试中钉住标签栏 ref 收集：即使控制器被 mock，ref 也会由 SuggestionsDisplay 自己的渲染填充，因此 categoryRefs.current 应包含每个标签一个节点（此夹具中为 3 个）；突变后其中只会有 2 个建议行节点，所以该断言能在突变下翻转（见英文部分代码块）。
— qwen3.8-max via Qwen Code /review (v0.21.4)
| expect(selectCategory).toHaveBeenCalledWith('session'); | ||
| unmount(); |
There was a problem hiding this comment.
[Suggestion] The new routing test asserts only the selectCategory call, not the second half of handleCategorySelect — the setExpandedSuggestionIndex(-1) reset (handleCategorySelect in InputPrompt.tsx). — Failure scenario: mutation-verified — deleting setExpandedSuggestionIndex(-1) from handleCategorySelect survives all 220 tests across the 4 relevant files. A user who expanded a long suggestion label and then clicks a category tab keeps the stale expandedIndex, which after the category filter change points at a different row (or past the end) of the newly filtered list, so the expansion indicator/description renders on the wrong row. captured.props['expandedIndex'] is already observable in this harness, so the assertion is cheap.
Assert the reset after driving onSelectCategory — note the assertion is only mutation-fatal if the test first reaches an expanded state (drive the component's expansion keypress); asserting -1 from a never-expanded state passes trivially:
expect(selectCategory).toHaveBeenCalledWith('session');
expect(captured.props!['expandedIndex']).toBe(-1);
unmount();中文说明
[建议] 新的路由测试只断言了 selectCategory 调用，没有断言 handleCategorySelect 的另一半—— setExpandedSuggestionIndex(-1) 重置（InputPrompt.tsx 中的 handleCategorySelect）。— 失败场景：已通过突变验证——删除 handleCategorySelect 中的 setExpandedSuggestionIndex(-1) 后，4 个相关文件的全部 220 个测试仍然通过。用户展开一条较长的建议标签后点击分类标签，会残留过期的 expandedIndex；分类过滤变化后它指向新过滤列表中的另一行（或越界），展开指示/描述会渲染在错误的行上。captured.props['expandedIndex'] 在此测试夹具中已经可以观测到，补充断言成本很低。请在驱动 onSelectCategory 之后断言该重置——注意：只有测试先进入展开状态（触发组件的展开按键）后再断言，才能在突变下翻转；从未展开时断言 -1 会恒真通过（见英文部分代码块）。
— qwen3.8-max via Qwen Code /review (v0.21.4)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally. Test Plan (not a blocker): src/ui/components/CompletionCategoryMouseController.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.mouse.test.tsx — no such file or directory; src/ui/components/InputPrompt.suggestionMouse.test.tsx — no such file or directory; src/ui/hooks/useCompletion.test.ts — no such file or directory.
中文说明
已审查。 建议见行内评论。 未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。 Test Plan（非阻断）：src/ui/components/CompletionCategoryMouseController.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.mouse.test.tsx — no such file or directory; src/ui/components/InputPrompt.suggestionMouse.test.tsx — no such file or directory; src/ui/hooks/useCompletion.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| col >= rect.x && | ||
| col < rect.x + rect.width && | ||
| row >= rect.y && | ||
| row < rect.y + rect.height |
There was a problem hiding this comment.
[Suggestion] The containment predicate re-implements the existing, unit-tested pointInViewport() from packages/cli/src/ui/selection/selection-coords.ts verbatim — same half-open bounds (left/top inclusive, right/bottom exclusive), and ElementMetrics is structurally identical to its ViewportRect. The rect.width > 0 && rect.height > 0 guards are implied by the strict inequalities too (a zero span can never satisfy v >= p && v < p + 0). — Concrete cost: the "right/bottom edge exclusive" rule now lives in three places: here, pointInViewport (tested in selection-coords.test.ts), and the pre-existing inline copy in HistoryItemDisplay.tsx (isInside). Any future correction to edge handling (e.g. wide-character rounding) applied to one copy silently diverges from the others, so tab clicks and text selection would disagree about the same screen cell with no shared code to fix once.
Reuse the shared helper instead:
import { pointInViewport } from '../selection/selection-coords.js';
// ...
pointInViewport({ x: col, y: row }, rect)or hoist the predicate next to findItemAtLayoutRow in packages/cli/src/ui/utils/list-mouse.ts and have both call sites use it.
中文说明
[建议] 该包含判断谓词逐字重复实现了现有且已有单测的 pointInViewport()（packages/cli/src/ui/selection/selection-coords.ts）——相同的半开边界（左/上包含、右/下不包含），且 ElementMetrics 与其 ViewportRect 结构完全一致。rect.width > 0 && rect.height > 0 守卫同样由严格不等式隐含（零跨度永远无法满足 v >= p && v < p + 0）。— 具体代价："右/下边界不包含"规则现在存在于三处：此处、pointInViewport（selection-coords.test.ts 中有对应测试）以及 HistoryItemDisplay.tsx 中已有的内联副本（isInside）。未来对边界处理的任何修正（例如宽字符取整）若只应用于其中一份副本，会与其他副本悄然分叉，导致标签点击与文本选择对同一屏幕单元格的判断不一致，且没有共享代码可以一次性修复。建议改用共享辅助函数 pointInViewport({ x: col, y: row }, rect)，或将该谓词提升到 packages/cli/src/ui/utils/list-mouse.ts 的 findItemAtLayoutRow 旁边，供两个调用点复用。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| for (let i = 0; i < categories.length; i++) { | ||
| const node = categoryRefs.current[i]; | ||
| if (!node) continue; |
There was a problem hiding this comment.
[Suggestion] The whole handleMouse is a near-copy of RowMouseController.resolveIndex (packages/cli/src/ui/components/shared/RowMouseController.tsx) — same container guard, same event.col - 1 transform, same layoutRowForEvent(container, event.row, terminalHeight) call, same iterate-refs → measureElementPosition → skip-degenerate-rect loop, same useMouseEvents headless-layer shape — making this the third copy of the controller skeleton (with TextInputMouseController.tsx). — Concrete cost: drift across sibling copies already happened in this codebase — the layoutRowForEvent docstring in measure-element-position.ts exists because the anchor correction drifted between RowMouseController and TextInputMouseController and produced an off-by-one bug, forcing extraction of the shared helper. The measure-loop and degenerate-rect skip remain per-copy re-derivations — and already differ in form (rect.height <= 0 skip in RowMouseController vs rect.width > 0 && rect.height > 0 here) — so the next coordinate or measurement fix must be located and re-applied in each controller independently, and SuggestionsDisplay.tsx now mounts two of these side by side sharing one containerRef.
Extract the shared core (event → layout point + measured rects → hit) beside findItemAtLayoutRow in packages/cli/src/ui/utils/list-mouse.ts, or generalize RowMouseController with a pluggable hit predicate, so both layers reuse one implementation.
中文说明
[建议] 整个 handleMouse 与 RowMouseController.resolveIndex（packages/cli/src/ui/components/shared/RowMouseController.tsx）近乎逐行相同——相同的容器守卫、相同的 event.col - 1 转换、相同的 layoutRowForEvent(container, event.row, terminalHeight) 调用、相同的"遍历 refs → measureElementPosition → 跳过退化矩形"循环、相同的 useMouseEvents 无头层结构——使其成为控制器骨架的第三份副本（另有 TextInputMouseController.tsx）。— 具体代价：兄弟副本间的分叉在本代码库已经发生过——measure-element-position.ts 中 layoutRowForEvent 的文档注释正是因为锚点修正在 RowMouseController 与 TextInputMouseController 之间分叉、产生了 off-by-one 缺陷后才提取出共享辅助函数。测量循环与退化矩形跳过目前仍是逐副本重新推导——且形式已经不同（RowMouseController 用 rect.height <= 0 跳过，此处用 rect.width > 0 && rect.height > 0）——下一次坐标或测量修复必须在每个控制器中分别定位并重新应用，而 SuggestionsDisplay.tsx 现在并行挂载其中两个、共享同一个 containerRef。建议把共享核心（事件 → 布局坐标 + 测量矩形 → 命中）提取到 packages/cli/src/ui/utils/list-mouse.ts 的 findItemAtLayoutRow 旁边，或为 RowMouseController 增加可插拔的命中谓词，让两层复用同一实现。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| vi.mocked(layoutRowForEvent).mockImplementation( | ||
| (_node, terminalRow) => terminalRow - 1, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The layoutRowForEvent stand-in ignores its third argument, so the terminalHeight wiring from useTerminalSize into the row mapping is unpinned. — Failure scenario: probe-verified — the one-line mutant layoutRowForEvent(containerRef.current, event.row, 0) survives all 3 controller tests. In real code, terminalHeight feeds the frame-anchor correction (anchor = min(0, terminalHeight - frameHeight)), which is what maps SGR mouse rows onto layout rows when the Ink frame is taller than the terminal; a regression that corrupts or drops that argument ships green and manifests as tab clicks hitting the wrong category (or nothing) precisely in those tall-frame layouts. The killing pattern already exists in RowMouseController.test.tsx (terminalRow1Based - 1 - Math.min(0, terminalHeight - frameHeight)).
| vi.mocked(layoutRowForEvent).mockImplementation( | |
| (_node, terminalRow) => terminalRow - 1, | |
| ); | |
| vi.mocked(layoutRowForEvent).mockImplementation( | |
| (_node, terminalRow, terminalHeight) => | |
| terminalRow - 1 - Math.min(0, terminalHeight - 40), | |
| ); |
Also add one case with a frame taller than the terminal so the anchor branch is actually exercised.
中文说明
[建议] layoutRowForEvent 的替身实现忽略了第三个参数，因此从 useTerminalSize 到行映射的 terminalHeight 接线没有被测试钉住。— 失败场景：已通过探针验证——单行突变 layoutRowForEvent(containerRef.current, event.row, 0) 在全部 3 个控制器测试下存活。真实代码中，terminalHeight 参与帧锚点修正（anchor = min(0, terminalHeight - frameHeight)），当 Ink 帧高于终端时正是它把 SGR 鼠标行映射到布局行；破坏或丢弃该参数的回归会在测试全绿的情况下发布，并恰好在这种高帧布局中表现为标签点击命中错误分类（或无命中）。能杀死该突变的写法在 RowMouseController.test.tsx 中已有先例（terminalRow1Based - 1 - Math.min(0, terminalHeight - frameHeight)）。请按英文部分的 suggestion 修改，并补充一个帧高于终端的用例，让锚点分支真正被执行。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| onSelectCategory={ | ||
| suggestionsFromExport || | ||
| commandSearchActive || | ||
| reverseSearchActive | ||
| ? undefined | ||
| : handleCategorySelect | ||
| } |
There was a problem hiding this comment.
[Suggestion] No test pins that onSelectCategory is undefined while command search, reverse search, or export completion is active — the gate can be deleted wholesale without any test failing. — Failure scenario: probe-verified — replacing the whole gate with onSelectCategory={handleCategorySelect} leaves all 206 tests in the two relevant files passing. The PR's own E2E plan lists "Export completion, command search, and reverse search do not receive a category-selection callback" as a regression check, but the suite only exercises the positive path. Today the impact is masked because availableCategories is gated in lockstep (no tabs render in those modes), but if a future change diverges the two gates, a tab click during search/export would route selectCategory at the default @-completion state and collapse expandedIndex behind the search UI — and nothing in the suite would fire.
In the existing command-search test, assert:
expect(captured.props!['onSelectCategory']).toBeUndefined();— likewise for export-completion mode if a harness exists.
中文说明
[建议] 没有测试钉住"命令搜索、反向搜索或导出补全激活时 onSelectCategory 为 undefined"——整个门控可以被删除而没有任何测试失败。— 失败场景：已通过探针验证——把整个门控替换为 onSelectCategory={handleCategorySelect} 后，两个相关文件的全部 206 个测试仍然通过。PR 自己的 E2E 计划把"导出补全、命令搜索和反向搜索不会收到分类选择回调"列为回归检查项，但自动化测试只覆盖了正向路径。当前影响被掩盖，因为 availableCategories 与之同步门控（这些模式下不渲染标签栏）；但一旦未来改动使两处门控分叉，搜索/导出期间的标签点击会在默认 @ 补全状态上触发 selectCategory 并在搜索界面后折叠 expandedIndex——整套测试不会有任何反应。建议在现有命令搜索测试中补充英文部分的断言（若导出补全有测试夹具，同样处理）。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| />, | ||
| ); | ||
|
|
||
| expect(CompletionCategoryMouseController).not.toHaveBeenCalled(); |
There was a problem hiding this comment.
[Suggestion] Of the four conjuncts gating this mount (mouseEnabled && showTabBar && availableCategories && onSelectCategory), only mouseEnabled has a negative test. — Failure scenario: probe-verified — removing onSelectCategory && (or showTabBar &&) from the mount condition leaves all 5 tests in this file passing. The first mutant is the load-bearing one: if the controller mounts while onSelectCategory is undefined, the first tab click executes onSelectCategory(categories[i]) on undefined — a TypeError in the mouse handler. Today InputPrompt's sibling gating keeps that state unreachable, but this file exists specifically to pin the mount contract of SuggestionsDisplay, and two of its four conditions can be deleted green.
Add a negative case, e.g.:
it('does not mount the category controller without onSelectCategory', () => {
render(
<SuggestionsDisplay
{...samePropsAsEnabledTest}
onSelectCategory={undefined}
/>,
);
expect(CompletionCategoryMouseController).not.toHaveBeenCalled();
});(or availableCategories={['all']} to pin the showTabBar conjunct).
中文说明
[建议] 门控该挂载的四个条件（mouseEnabled && showTabBar && availableCategories && onSelectCategory）中，只有 mouseEnabled 有负向测试。— 失败场景：已通过探针验证——从挂载条件中删除 onSelectCategory &&（或 showTabBar &&）后，该文件全部 5 个测试仍然通过。第一个突变是关键：若控制器在 onSelectCategory 为 undefined 时挂载，首次标签点击会对 undefined 调用 onSelectCategory(categories[i])——鼠标处理器中抛出 TypeError。当前 InputPrompt 的同步门控使该状态不可达，但该测试文件的职责正是钉住 SuggestionsDisplay 的挂载契约，而其中两个条件目前可以在测试全绿的情况下被删除。建议补充一个负向用例（例如英文部分：以与启用用例相同的 props 但不传 onSelectCategory 渲染，断言控制器未被调用；或用 availableCategories={['all']} 钉住 showTabBar 条件）。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| const col = event.col - 1; | ||
| const row = layoutRowForEvent( | ||
| containerRef.current, | ||
| event.row, | ||
| terminalHeight, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] This controller silently depends on alternate-screen (VP) coordinate assumptions that its two sibling consumers (RowMouseController, TextInputMouseController) explicitly document in full paragraphs — the only undocumented copy of the most bug-prone mapping in this feature. — Failure scenario: layoutRowForEvent's frame-anchor correction (min(0, terminalHeight - frameHeight)) is only valid when Ink's frame is bottom-pinned (VP/alternate-screen mode). Today the component is safe purely because its only mount site is gated behind mouseEnabled (= VP mode && ui.mouseTracking). A future maintainer reusing this controller on another surface — or loosening the mount gate / passing bypassVpGate — gets no compile or runtime error: tab clicks just map onto wrong rows (wrong tab or nothing), with no diagnostic. This defect class has history here: measure-element-position.ts records a prior off-by-one in exactly this correction.
Document the constraint mirroring the siblings, e.g. at the top of the component:
/**
* Headless mouse layer for the completion category tab bar.
* Coordinates assume alternate-screen (VP) mode, same as RowMouseController —
* layoutRowForEvent's anchor correction presumes a bottom-pinned frame;
* inline mode is intentionally unsupported. Mount only behind `mouseEnabled`.
*/中文说明
[建议] 该控制器隐式依赖备用屏幕（VP）坐标假设，而它的两个兄弟使用者（RowMouseController、TextInputMouseController）都用完整段落明确注明了这一约束——这是本特性中最容易出错的映射的唯一未注明副本。— 失败场景：layoutRowForEvent 的帧锚点修正（min(0, terminalHeight - frameHeight)）仅在 Ink 帧底部对齐（VP/备用屏幕模式）时有效。当前组件安全仅仅因为其唯一挂载点被 mouseEnabled（= VP 模式 && ui.mouseTracking）门控。未来若有维护者把该控制器复用到其他界面——或放宽挂载门控/传入 bypassVpGate——不会得到任何编译或运行时报错：标签点击只会静默映射到错误的行（选错分类或无响应），且没有任何诊断信息。该缺陷类别在此处有历史先例：measure-element-position.ts 记录过同一修正上的 off-by-one。建议仿照兄弟控制器补充文档注释（见英文部分代码块）。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| act(() => { | ||
| (captured.props!['onSelectIndex'] as (index: number) => void)(0); | ||
| }); | ||
| expect(captured.props!['expandedIndex']).toBe(0); |
There was a problem hiding this comment.
[Suggestion] This test's precondition is load-bearing on a state leak: expandedSuggestionIndex is only ever set to a non--1 value inside the search branch (the EXPAND_SUGGESTION handler in InputPrompt.tsx is the sole non-reset setter), and the search-exit path the test drives (handleSuggestionSelect's search branch) never resets it — so the setup assertion pins leaked state. — Failure scenario: probe-verified — the Escape cancel path resets expandedSuggestionIndex, but the accept paths (mouse handleSuggestionSelect search branch; keyboard ACCEPT_SUGGESTION_REVERSE_SEARCH / SUBMIT_REVERSE_SEARCH) do not. The moment someone fixes that asymmetry, this test fails at the setup assertion (expected -1 to be +0) before the code under test runs, making a legitimate fix look like it broke the collapse behavior — and with the leak fixed, expandedIndex can never be non--1 while the tab bar is visible (tabs are gated off during search), so the collapse call this test exists to prove becomes unreachable.
Fix the leak at the source, mirroring the normal branch and the Escape cancelSearch helper:
// handleSuggestionSelect's search branch (InputPrompt.tsx):
setExpandedSuggestionIndex(-1);Then either drop the now-unreachable collapse call + this test, or keep the call as defense and rewrite this test's setup so it no longer depends on the search round-trip.
中文说明
[建议] 该测试的前置条件建立在一个状态泄漏之上：expandedSuggestionIndex 只会在搜索分支中被置为非 -1 值（InputPrompt.tsx 的 EXPAND_SUGGESTION 处理器是唯一不做重置的设置点），而测试驱动的搜索退出路径（handleSuggestionSelect 的搜索分支）从不重置它——因此准备阶段的断言钉住的是泄漏状态。— 失败场景：已通过探针验证——Escape 取消路径会重置 expandedSuggestionIndex，但接受路径（鼠标 handleSuggestionSelect 搜索分支、键盘 ACCEPT_SUGGESTION_REVERSE_SEARCH / SUBMIT_REVERSE_SEARCH）不会。一旦有人修复该不对称，本测试会在准备阶段断言处失败（expected -1 to be +0），先于被测代码执行，使合理的修复看起来像是破坏了折叠行为——而且泄漏修复后，标签栏可见时 expandedIndex 永远不可能为非 -1（搜索期间标签栏被门控关闭），本测试要证明的折叠调用将变得不可达。建议在源头修复泄漏（镜像其普通分支与 Escape cancelSearch 辅助函数，见英文部分代码块），然后删除已不可达的折叠调用与本测试，或保留防御性调用并重写测试准备阶段、使其不再依赖搜索往返。
— qwen3.8-max via Qwen Code /review (v0.21.6)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here. Test Plan (not a blocker): src/ui/components/CompletionCategoryMouseController.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.mouse.test.tsx — no such file or directory; src/ui/components/InputPrompt.suggestionMouse.test.tsx — no such file or directory; src/ui/hooks/useCompletion.test.ts — no such file or directory.
中文说明
已审查——无阻断问题。 1 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。 Test Plan（非阻断）：src/ui/components/CompletionCategoryMouseController.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.mouse.test.tsx — no such file or directory; src/ui/components/InputPrompt.suggestionMouse.test.tsx — no such file or directory; src/ui/hooks/useCompletion.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.21.7)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): src/ui/components/CompletionCategoryMouseController.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.mouse.test.tsx — no such file or directory; src/ui/components/InputPrompt.suggestionMouse.test.tsx — no such file or directory; src/ui/hooks/useCompletion.test.ts — no such file or directory.
中文说明
已审查。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan（非阻断）：src/ui/components/CompletionCategoryMouseController.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.mouse.test.tsx — no such file or directory; src/ui/components/InputPrompt.suggestionMouse.test.tsx — no such file or directory; src/ui/hooks/useCompletion.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| const visiblePosition = findElementAtMouseEvent( | ||
| containerRef.current, | ||
| itemRefs.current, |
There was a problem hiding this comment.
[Suggestion] This refactor onto findElementAtMouseEvent removed the last production caller of findItemAtLayoutRow and the VisibleItemRect interface in packages/cli/src/ui/utils/list-mouse.ts — after this diff their only references are list-mouse.ts itself and list-mouse.test.ts (verified by repo-wide grep; frameAnchor/terminalRowToLayoutRow in the same file remain used by measure-element-position.ts and selection-coords.ts). The same half-open row-span containment is re-implemented inline in mouse-hit.ts (containsPoint's 'row' mode), so two copies of the geometry now exist — one dead but exported and unit-tested, which reads as alive. — Failure scenario: a future fix to list-row hit-testing (gap/overlap handling) applied to the still-tested findItemAtLayoutRow changes nothing at runtime — the live code is the containsPoint loop in mouse-hit.ts — while its green suite suggests the fix landed; the gap-row case is currently tested only against the dead copy. Fix (spans files outside this diff, so no one-click suggestion): delete findItemAtLayoutRow/VisibleItemRect and their describe block in list-mouse.test.ts, or have the 'row' mode delegate to findItemAtLayoutRow so the predicate stays single-sourced.
中文说明
[建议] 这次重构改用 findElementAtMouseEvent 后，packages/cli/src/ui/utils/list-mouse.ts 中的 findItemAtLayoutRow 与 VisibleItemRect 接口失去了最后一个生产调用方——本次 diff 之后，它们仅被 list-mouse.ts 自身和 list-mouse.test.ts 引用（已全仓 grep 验证；同文件的 frameAnchor/terminalRowToLayoutRow 仍被 measure-element-position.ts 与 selection-coords.ts 使用）。同样的半开行区间包含判断又在 mouse-hit.ts 中以行内方式重新实现（containsPoint 的 'row' 模式），于是同一套几何逻辑现在存在两份副本——其中一份已死但仍被导出且仍有单测，看起来仍然有效。— 失败场景：未来对列表行命中检测的修复（间隙/重叠处理）若落在仍有测试的 findItemAtLayoutRow 上，运行时不会有任何变化——真正生效的代码是 mouse-hit.ts 里的 containsPoint 循环——而其绿色测试会让人觉得修复已生效；目前 gap-row 用例只测了那份死代码。修复方式（涉及 diff 之外的文件，故不提供一键 suggestion）：删除 findItemAtLayoutRow/VisibleItemRect 及 list-mouse.test.ts 中对应的 describe 块；或让 'row' 模式委托给 findItemAtLayoutRow，使该谓词保持单一来源。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| if ( | ||
| category === activeCategory || | ||
| !availableCategories.includes(category) |
There was a problem hiding this comment.
[Suggestion] The availability guard clause !availableCategories.includes(category) is not pinned by any test — probe-verified: deleting just that clause survives the whole suite (baseline 25/25 green; mutant 84/84 across all 7 PR-related test files). The two new tests cover only the success path and the same-category early return, which short-circuits before this clause. Under current wiring the clause is also structurally unreachable — the sole production caller passes categories[index] where categories is the same array reference as availableCategories, with index < categories.length guarded — so it is simultaneously dead today and untested. — Failure scenario: if a future surface passes an arbitrary category through onSelectCategory, the guard's behavior is unproven; deleting the clause today survives every test, so such a regression would ship green. Fix: add a unit test in useCompletion.test.ts (single-category suggestion set, call selectCategory('session'), assert the category stays 'all' with indices preserved), or deliberately delete the unreachable clause.
中文说明
[建议] 可用性守卫子句 !availableCategories.includes(category) 没有被任何测试钉住——已通过突变探针验证：仅删除该子句后整个测试套件仍全部通过（基线 25/25 绿；突变体在本 PR 相关的全部 7 个测试文件中 84/84 通过）。两个新增测试只覆盖了成功路径和同类别提前返回（后者在该子句之前就短路了）。在当前接线下该子句实际上也不可达——唯一的生产调用方传入的是 categories[index]，其中 categories 与 availableCategories 是同一个数组引用，且有 index < categories.length 守卫——因此它目前既是死代码又没有测试。— 失败场景：如果未来有新的入口通过 onSelectCategory 传入任意类别，该守卫的行为未被证明；现阶段删除该子句不会让任何测试失败，因此这类回归会在测试全绿的情况下被发布。修复方式：在 useCompletion.test.ts 中补充单测（单类别建议集下调用 selectCategory('session')，断言类别保持 'all' 且索引不变），或有意删除这条不可达的子句。
— qwen3.8-max via Qwen Code /review (v0.21.10)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): src/ui/components/CompletionCategoryMouseController.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.mouse.test.tsx — no such file or directory; src/ui/components/InputPrompt.suggestionMouse.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.test.tsx — no such file or directory; src/ui/hooks/useCompletion.test.ts — no such file or directory; and 2 more.
中文说明
已审查。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan（非阻断）：src/ui/components/CompletionCategoryMouseController.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.mouse.test.tsx — no such file or directory; src/ui/components/InputPrompt.suggestionMouse.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.test.tsx — no such file or directory; src/ui/hooks/useCompletion.test.ts — no such file or directory; and 2 more。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| const props = vi.mocked(CompletionCategoryMouseController).mock.calls[0][0]; | ||
| expect(props.categories).toEqual(['all', 'file', 'session']); | ||
| expect(props.containerRef.current).not.toBeNull(); | ||
| expect(props.categoryRefs.current.filter(Boolean)).toHaveLength(3); |
There was a problem hiding this comment.
[Suggestion] R6-1: The mount test verifies the count of attached category refs but not their positional correspondence with categories — probe-verified: the type-valid one-line mutation categoryRefs.current[i] = node → categoryRefs.current[i + 1] = node in SuggestionsDisplay.tsx survives the entire src/ui suite (6738 tests pass), because slots 1–3 still hold three truthy nodes. — Failure scenario: under that mutation, clicking the Files tab resolves to ref slot 2 and activates Sessions, and clicking the last tab hits the controller's index < categories.length guard and does nothing — all with every automated test passing. This is the residual hole in the assertion this thread suggested in round 2: the count pins presence, not position.
| expect(props.categoryRefs.current.filter(Boolean)).toHaveLength(3); | |
| expect(props.categoryRefs.current.filter(Boolean)).toHaveLength(3); | |
| [0, 1, 2].forEach((i) => | |
| expect(props.categoryRefs.current[i]).not.toBeNull(), | |
| ); |
中文说明
[建议] R6-1：该挂载测试只验证了已挂载分类 ref 的数量，没有验证它们与 categories 的位置对应关系——已通过探针验证：在 SuggestionsDisplay.tsx 中做类型合法的单行突变 categoryRefs.current[i] = node → categoryRefs.current[i + 1] = node，整个 src/ui 测试套件（6738 个测试通过）依然全绿，因为槽位 1–3 中仍有 3 个非空节点。— 失败场景：在该突变下，点击 Files 标签会命中 ref 槽位 2 而激活 Sessions；点击最后一个标签会命中控制器的 index < categories.length 守卫而毫无反应——所有测试仍然通过。这是第 2 轮线程所建议断言的残留漏洞：数量断言钉住了"存在"，但没有钉住"位置"。建议修复在数量断言之外追加按索引逐一断言非空（见英文部分 suggestion 代码块，已验证在突变下会翻红）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| { col: 11, row: 5 }, | ||
| 40, | ||
| 'rect', | ||
| ), | ||
| ).toBe(1); | ||
| }); |
There was a problem hiding this comment.
[Suggestion] R6-2: The 'rect'-mode tests probe the gap column and tab left edges but never the exclusive right/bottom edges of containsPoint — probe-verified: the mutant point.x < rect.x + rect.width → point.x <= rect.x + rect.width in mouse-hit.ts survives the entire src/ui suite (6738 tests pass). — Failure scenario: with <=, a click one column past the last tab (x=16, still inside the container's [2,18)) resolves to tab index 1, so clicking empty space right of the last tab activates it; the bottom edge (point.y < rect.y + rect.height) is equally unpinned because every test mocks layoutRowForEvent to row 4.
| { col: 11, row: 5 }, | |
| 40, | |
| 'rect', | |
| ), | |
| ).toBe(1); | |
| }); | |
| { col: 11, row: 5 }, | |
| 40, | |
| 'rect', | |
| ), | |
| ).toBe(1); | |
| expect( | |
| findElementAtMouseEvent( | |
| container, | |
| [first, second], | |
| { col: 17, row: 5 }, | |
| 40, | |
| 'rect', | |
| ), | |
| ).toBeNull(); | |
| }); |
中文说明
[建议] R6-2：'rect' 模式的测试探测了标签之间的间隙列与标签左边界，但从未探测 containsPoint 的排他右/下边界——已通过探针验证：将 mouse-hit.ts 中的 point.x < rect.x + rect.width 突变为 point.x <= rect.x + rect.width，整个 src/ui 测试套件（6738 个测试通过）依然全绿。— 失败场景：若使用 <=，最后一个标签右侧一列的点击（x=16，仍在容器 [2,18) 内）会解析为标签索引 1，即点击最后一个标签右侧的空白区域也会激活该标签；下边界（point.y < rect.y + rect.height）同样未被钉住，因为所有测试都把 layoutRowForEvent mock 为第 4 行。建议修复在现有测试末尾追加右排他边界探针（见英文部分 suggestion 代码块，已验证在突变下会翻红）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): PR #8395 makes the @ completion picker's category tabs ...: none — all planned checks completed within budget..
Test Plan (not a blocker): src/ui/components/CompletionCategoryMouseController.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.mouse.test.tsx — no such file or directory; src/ui/components/InputPrompt.suggestionMouse.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.test.tsx — no such file or directory; src/ui/hooks/useCompletion.test.ts — no such file or directory; and 2 more.
中文说明
已审查。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：PR #8395 makes the @ completion picker's category tabs ...：none — all planned checks completed within budget.。
Test Plan（非阻断）：src/ui/components/CompletionCategoryMouseController.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.mouse.test.tsx — no such file or directory; src/ui/components/InputPrompt.suggestionMouse.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.test.tsx — no such file or directory; src/ui/hooks/useCompletion.test.ts — no such file or directory; and 2 more。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| mode: ElementHitMode, | ||
| ): number | null { | ||
| if (!container) return null; |
There was a problem hiding this comment.
[Suggestion] The null-container guard added in the new shared util findElementAtMouseEvent has zero test coverage — probe-verified: deleting if (!container) return null; survives all 24 tests across the four mouse test files. Both production callers pass nullable refs (containerRef.current in RowMouseController and CompletionCategoryMouseController), so the guard is load-bearing, not dead code. — Failure scenario: if a future refactor drops the guard, any mouse event delivered before the container ref attaches (mount race) dereferences null in layoutRowForEvent/measureFrameHeight and throws inside the mouse handler instead of the event being silently ignored; no test can catch that regression today.
Add one assertion in mouse-hit.test.ts:
expect(
findElementAtMouseEvent(null, [first], { col: 3, row: 5 }, 40, 'row'),
).toBeNull();中文说明
[Suggestion] 新共享工具函数 findElementAtMouseEvent 中新增的空 container 守卫没有任何测试覆盖——已通过变异探测验证：删除 if (!container) return null; 后，四个鼠标测试文件中的全部 24 个用例仍然通过。两个生产调用方传入的都是可为空的 ref（RowMouseController 与 CompletionCategoryMouseController 中的 containerRef.current），因此该守卫是真正起作用的，而不是死代码。——失败场景：如果未来某次重构删除了这个守卫，那么在 container ref 挂载之前到达的鼠标事件（挂载竞态）会在 layoutRowForEvent/measureFrameHeight 中对 null 解引用并在鼠标处理器内部抛出异常，而不是被静默忽略；当前没有任何测试能捕获这种回归。修复方式：在 mouse-hit.test.ts 中补充一条断言（见英文部分代码块）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| it('ignores non-press mouse events', () => { | ||
| const handler = mountAndGetHandler(); |
There was a problem hiding this comment.
[Suggestion] The test that pins the left-press-only guard cannot discriminate it — probe-verified: findElementAtMouseEvent is mocked to return null in beforeEach and is never overridden here, so deleting if (event.name !== 'left-press') return; from the controller keeps the whole suite green (4/4 with the mutant). The sibling 'ignores clicks outside category bounds' test shares the same shape. — Failure scenario: a refactor that broadens the guard (e.g. also accepting left-release) ships green; in production SGR button tracking emits a release event at the press coordinates for every click and motion events while held, so a press-on-tab-A / drag / release-on-tab-B gesture would then select the tab under the release point instead of the pressed tab.
Make the mock return a hittable index so only the guard can prevent the callback:
| it('ignores non-press mouse events', () => { | |
| const handler = mountAndGetHandler(); | |
| it('ignores non-press mouse events', () => { | |
| const handler = mountAndGetHandler(); | |
| vi.mocked(findElementAtMouseEvent).mockReturnValue(2); |
中文说明
[Suggestion] 本应锁定「仅响应 left-press」守卫的测试实际上无法区分该守卫——已通过变异探测验证：findElementAtMouseEvent 在 beforeEach 中被 mock 为返回 null，且该测试从未覆盖这个返回值，因此即使从控制器中删除 if (event.name !== 'left-press') return;，整个测试套件仍然全绿（变异体下 4/4 通过）。相邻的 'ignores clicks outside category bounds' 测试也是同样的形态。——失败场景：任何放宽该守卫的重构（例如同时接受 left-release）都会在测试全绿的情况下合入；生产环境中 SGR 按键追踪会为每次点击在按下坐标处发出释放事件、在按住时发出移动事件，因此「在标签 A 按下、拖动、在标签 B 释放」的手势将选中释放位置下的标签而不是按下位置的标签。修复方式：让 mock 返回一个可命中的索引，使只有该守卫能阻止回调触发（见英文部分 suggestion 块；已验证该修复对正确代码 4/4 通过、对变异体生效）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| [first, second], | ||
| { col: 10, row: 5 }, | ||
| 40, | ||
| 'rect', |
There was a problem hiding this comment.
[Suggestion] R6-2: The 'rect'-mode tests still never probe the exclusive right/bottom edges of containsPoint — re-checked at the reviewed commit and independently re-derived this round: the mutant point.x < rect.x + rect.width → point.x <= rect.x + rect.width survives the whole file. The mocked tabs cover x∈[2,8) and x∈[10,16); the tests probe the gap column x=9 (col: 10) and tab interiors, but never the other gap column x=8, and the bottom edge is unprobed because the layoutRowForEvent mock always returns y=4. — Failure scenario: a future off-by-one making the right/bottom edge inclusive ships green — with the real tab bar's marginLeft={1} gaps, a click in the single gap column between two tabs would activate the left tab instead of nothing, and no test in this file can catch it.
Pin the exclusive right edge with a new probe, and pin the bottom edge by overriding layoutRowForEvent to return 5 (one row below the rects):
expect(
findElementAtMouseEvent(
container,
[first, second],
{ col: 9, row: 5 },
40,
'rect',
),
).toBeNull();中文说明
[Suggestion] R6-2：'rect' 模式测试仍然没有探测 containsPoint 的右/下排他边界——已在被审提交上复查，并在本轮独立重新推导：变异体 point.x < rect.x + rect.width → point.x <= rect.x + rect.width 在整个文件中存活。mock 的两个标签分别覆盖 x∈[2,8) 与 x∈[10,16)；测试探测了间隙列 x=9（col: 10）和标签内部，但从未探测另一个间隙列 x=8；下边界同样无法被探测，因为 layoutRowForEvent 的 mock 恒返回 y=4。——失败场景：未来任何把右/下边界变成包含边界的 off-by-one 都会在全绿中合入——真实标签栏使用 marginLeft={1} 间隙，点击两个标签之间唯一的间隙列会激活左侧标签而不是无操作，而本文件中没有任何测试能捕获它。修复方式：新增一条探测以锁定右排他边界（见英文部分代码块）；并把 layoutRowForEvent 的 mock 覆盖为返回 5（矩形下方一行）以锁定下边界。
— qwen3.8-max via Qwen Code /review (v0.21.10)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): PR #8395 makes the @ completion picker's category tabs ...: none — all checks I started were completed within budget..
Test Plan (not a blocker): src/ui/components/CompletionCategoryMouseController.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.mouse.test.tsx — no such file or directory; src/ui/components/InputPrompt.suggestionMouse.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.test.tsx — no such file or directory; src/ui/hooks/useCompletion.test.ts — no such file or directory; and 2 more.
中文说明
已审查。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：PR #8395 makes the @ completion picker's category tabs ...：none — all checks I started were completed within budget.。
Test Plan（非阻断）：src/ui/components/CompletionCategoryMouseController.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.mouse.test.tsx — no such file or directory; src/ui/components/InputPrompt.suggestionMouse.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.test.tsx — no such file or directory; src/ui/hooks/useCompletion.test.ts — no such file or directory; and 2 more。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| 6. Confirm that `Ctrl+Tab`, `Ctrl+Shift+Tab`, and `Ctrl+Left/Right` retain their | ||
| existing behavior in a terminal that forwards those keys. |
There was a problem hiding this comment.
[Suggestion] Step 6 of the added E2E plan verifies keybindings that no longer exist at the reviewed commit: Ctrl+Tab / Ctrl+Shift+Tab / Ctrl+Left/Right completion-tab switching was removed by #8576 (commit e20601d6c4, a verified ancestor of this PR's head) — keyBindings.ts now binds only bare Left/Right (modifiers pinned false) to COMPLETION_TAB_LEFT/RIGHT, and keyMatchers.test.ts asserts the Ctrl variants as negative cases. — Failure scenario: a reviewer following this plan presses Ctrl+Tab or Ctrl+←/→ in a terminal that forwards them, observes no category switch, and either files a false regression against this PR or burns time bisecting behavior that #8576 intentionally removed — while the actual current keyboard path (bare ←/→ while the tab bar is visible, Esc to dismiss) goes unverified.
| 6. Confirm that `Ctrl+Tab`, `Ctrl+Shift+Tab`, and `Ctrl+Left/Right` retain their | |
| existing behavior in a terminal that forwards those keys. | |
| 6. Confirm that bare `Left`/`Right` cycle the category tabs while the tab bar | |
| is visible, and that `Esc` dismisses the completion menu. |
中文说明
新增 E2E 计划的第 6 步验证的按键绑定在被审阅提交上已不存在：Ctrl+Tab / Ctrl+Shift+Tab / Ctrl+Left/Right 的补全分类切换已被 #8576（提交 e20601d6c4，已验证为本 PR head 的祖先）移除——keyBindings.ts 现在只把无修饰键的 Left/Right（修饰键固定为 false）绑定到 COMPLETION_TAB_LEFT/RIGHT，且 keyMatchers.test.ts 已将 Ctrl 组合断言为负例。— 失败场景：按此计划验证的 Reviewer 在会转发这些按键的终端里按下 Ctrl+Tab 或 Ctrl+←/→，发现分类没有切换，可能会对本 PR 误报回归，或者花费时间二分排查 #8576 有意移除的行为——而当前实际的键盘路径（标签栏可见时无修饰 ←/→ 循环切换、Esc 关闭菜单）反而没有被验证。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| setActiveCategory(category); | ||
| setActiveSuggestionIndex(0); | ||
| setVisibleStartIndex(0); |
There was a problem hiding this comment.
[Suggestion] This re-implements the category-change body that switchCategory (lines 143–157) already carries — the "category change ⇒ reset active/scroll index" triple now lives in three places (here, switchCategory, and the fallback effect at lines 105–112), even though both callbacks document the same contract and this PR's own scope requires keyboard and mouse behavior to match. — Failure scenario: a future change to category-change side effects (an additional reset, or a different reset index) applied to one callback but not the other makes keyboard cycling and mouse tab clicks diverge silently — the mouse path was added precisely because both must behave identically.
Consolidation sketch (spans both callbacks, so not a one-click suggestion): have switchCategory compute the cyclic neighbor and delegate:
const switchCategory = useCallback(
(direction: 1 | -1) => {
const index = availableCategories.indexOf(activeCategory);
const next =
availableCategories[
(index + direction + availableCategories.length) %
availableCategories.length
];
selectCategory(next);
},
[activeCategory, availableCategories, selectCategory],
);One deliberate difference to decide when consolidating: today's switchCategory resets indices even when the cycle lands on the same category (only possible with a single-'all' tab list, unreachable through the >2-categories keyboard gate), which selectCategory's same-category guard skips.
中文说明
这里重新实现了 switchCategory（第 143–157 行）已有的分类变更主体——"切换分类 ⇒ 重置高亮/滚动索引"的三连调用现在出现在三处（此处、switchCategory、以及第 105–112 行的回退 effect），尽管两个回调的文档契约相同，且本 PR 的范围本身就要求键盘与鼠标行为一致。— 失败场景：未来对分类变更副作用的修改（新增一个重置项，或使用不同的重置索引）如果只应用到其中一个回调，键盘循环切换与鼠标点击标签页会悄无声息地分叉——而新增鼠标路径的原因恰恰是两者必须表现一致。
合并示意（跨两个回调，无法用一键 suggestion 表达）：让 switchCategory 计算循环相邻分类后委托给 selectCategory（代码见英文部分）。合并时需要决定一个有意的差异：现在的 switchCategory 即使循环回到同一分类也会重置索引（仅在只有单个 'all' 标签时可能发生，且键盘侧 >2 个分类的门控使其不可达），而 selectCategory 的同分类守卫会跳过该情况。
— qwen3.8-max via Qwen Code /review (v0.21.10)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "PR #8395 makes the @ completion picker's category tabs…": none — all checks above completed within budget; no layer left unwalked for chunk 1.; "PR #8395 makes the @ completion picker's category tabs…": none — all planned checks completed within budget.; "PR #8395 makes the @ completion picker's category tabs…": none — finished within budget; no check left unfinished.; "PR #8395 makes the @ completion picker's category tabs…": none — all checks I started were completed within budget..
Test Plan (not a blocker): src/ui/components/CompletionCategoryMouseController.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.mouse.test.tsx — no such file or directory; src/ui/components/InputPrompt.suggestionMouse.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.test.tsx — no such file or directory; src/ui/hooks/useCompletion.test.ts — no such file or directory; and 2 more.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："PR #8395 makes the @ completion picker's category tabs…"：none — all checks above completed within budget; no layer left unwalked for chunk 1.；"PR #8395 makes the @ completion picker's category tabs…"：none — all planned checks completed within budget.；"PR #8395 makes the @ completion picker's category tabs…"：none — finished within budget; no check left unfinished.；"PR #8395 makes the @ completion picker's category tabs…"：none — all checks I started were completed within budget.。
Test Plan（非阻断）：src/ui/components/CompletionCategoryMouseController.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.mouse.test.tsx — no such file or directory; src/ui/components/InputPrompt.suggestionMouse.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.test.tsx — no such file or directory; src/ui/hooks/useCompletion.test.ts — no such file or directory; and 2 more。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| expect(result.current.activeCategory).toBe('session'); | ||
| expect(result.current.activeSuggestionIndex).toBe(0); | ||
| expect(result.current.visibleStartIndex).toBe(0); |
There was a problem hiding this comment.
[Suggestion] The reset-on-category-change assertions are vacuous: the mixed fixture has exactly one item per category, so after selectCategory('session') the filtered list has length 1 and the pre-existing shrink-clamp effect (prev >= suggestions.length → max(0, length - MAX_SUGGESTIONS_TO_SHOW) for visibleStartIndex, prev >= length → length - 1 for the active index) produces the asserted zeros on its own. Probe-verified: deleting setVisibleStartIndex(0) from changeCategory leaves this suite passing 26/26; deleting setActiveSuggestionIndex(0) is caught only incidentally by the pre-existing wrap test, which starts from prev = -1 and never asserts visibleStartIndex. — Failure scenario: a user scrolled deep in a long all list (e.g. visibleStartIndex 10) clicks a tab whose filtered list has more than 10 items → the clamp condition is false → scroll/highlight are not reset → SuggestionsDisplay renders slice(scrollOffset, scrollOffset + 8) from the middle of the new category, with every test green.
Suggested fix — give the reset tests a fixture the clamp cannot mask (a target category with ≥2 items, indices pre-set to 1):
// e.g. session:1 + session:2 in the fixture, then:
result.current.setActiveSuggestionIndex(1);
result.current.setVisibleStartIndex(1);
// without the resets in changeCategory the indices stay at 1
// and the toBe(0) assertions fail中文说明
[Suggestion] 分类切换时的重置断言是空转的：mixed fixture 每个分类恰好只有一条建议，selectCategory('session') 之后过滤结果长度为 1，既有的收缩钳制 effect（visibleStartIndex 的 prev >= suggestions.length → max(0, length - MAX_SUGGESTIONS_TO_SHOW)、活跃索引的 prev >= length → length - 1）本身就会把两个索引钳到 0。探针验证：从 changeCategory 中删除 setVisibleStartIndex(0) 后整个测试文件仍然 26/26 全绿；删除 setActiveSuggestionIndex(0) 只被既有的回绕测试顺带捕获（该测试从 prev = -1 开始，且从不检查 visibleStartIndex）。— 失败场景：用户在很长的 all 列表中向下滚动（如 visibleStartIndex 为 10）后点击一个过滤结果超过 10 条的分类 → 钳制条件不成立 → 滚动位置与高亮不会重置 → SuggestionsDisplay 从新分类的中间开始渲染（slice(scrollOffset, scrollOffset + 8)），而所有测试仍然是绿的。
建议修复——给重置测试换一个钳制无法掩盖的 fixture（目标分类至少 2 条数据，并预先把索引设为 1）：这样一旦 changeCategory 缺少重置，索引会停在 1，toBe(0) 断言就会失败。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| if (rect.height <= 0) continue; | ||
| rects.push({ index, top: rect.y, height: rect.height }); | ||
| } |
There was a problem hiding this comment.
[Suggestion] The 'row' mode builds VisibleItemRect objects whose index is the position within the passed (visible-slice) elements array, contradicting the documented contract of VisibleItemRect.index in list-mouse.ts — "Index into the full items array (not the visible slice)". The pre-PR sole producer (RowMouseController pushing scrollOffset + visiblePos) honored that contract; after this PR the only producer violates it, and RowMouseController silently compensates with scrollOffset + visiblePosition. — Concrete cost: this utility exists to be shared (both completion rows and their category tabs route through it). A next consumer of findElementAtMouseEvent(..., 'row') — or of findItemAtLayoutRow following the documented contract — will reasonably treat the returned index as a full-array index; adding scrollOffset again (or omitting it where the doc says it is already included) yields an off-by-scrollOffset selection in a scrolled list.
Suggested fix: restore the contract inside the utility (accept a scrollOffset parameter and push { index: scrollOffset + index, ... }, removing the call-site compensation in RowMouseController), or amend the VisibleItemRect.index doc to state the index is caller-assigned and that findElementAtMouseEvent assigns visible-slice positions.
中文说明
[Suggestion] 'row' 模式构造的 VisibleItemRect 的 index 是传入的（可见切片）elements 数组内的位置，与 list-mouse.ts 中 VisibleItemRect.index 的文档契约——"完整 items 数组的索引（不是可见切片）"——相矛盾。PR 之前唯一的生产者（RowMouseController push scrollOffset + visiblePos）遵守该契约；此 PR 之后唯一的生产者违反了它，并由 RowMouseController 用 scrollOffset + visiblePosition 隐式补偿。— 具体代价：这个工具函数的存在目的就是为了共享（补全行和分类标签都经过它）。下一个遵循文档契约的 findElementAtMouseEvent(..., 'row') 或 findItemAtLayoutRow 使用者会合理地把返回的索引当作完整数组索引；再次加上 scrollOffset（或按文档以为已包含而省略）会在已滚动的列表中产生偏移 scrollOffset 的错误选中。
建议修复：在工具函数内部恢复契约（接受 scrollOffset 参数并 push { index: scrollOffset + index, ... }，同时移除 RowMouseController 调用处的补偿），或者修改 VisibleItemRect.index 的文档，说明索引由调用方约定、findElementAtMouseEvent 返回的是可见切片位置。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| it('ignores missing and degenerate child elements', () => { | ||
| vi.mocked(measureElementPosition).mockImplementation((node) => | ||
| node === container |
There was a problem hiding this comment.
[Suggestion] The only test claiming to cover missing (null) children cannot detect removal of the if (!element) continue; guards in findElementAtMouseEvent — the mock maps the null entry to the zero-width else-branch rect that the degenerate-rect skip discards anyway. Probe-verified both directions: deleting both guards leaves all 7 tests green; hardening the mock to throw on null fails against the guard-less source and passes against the intact source. — Failure scenario: a future refactor deleting (or moving after the measureElementPosition call) the guard ships green. In production, CompletionCategoryMouseController passes categoryRefs.current typed Array<DOMElement | null> (tab refs are unattached between render and event; React calls ref callbacks with null on unmount), and the real measureElementPosition opens with const { yogaNode } = node; → a left-press on the tab bar throws TypeError: Cannot destructure property 'yogaNode' of 'null', crashing the mouse handler.
| it('ignores missing and degenerate child elements', () => { | |
| vi.mocked(measureElementPosition).mockImplementation((node) => | |
| node === container | |
| it('ignores missing and degenerate child elements', () => { | |
| vi.mocked(measureElementPosition).mockImplementation((node) => { | |
| if (node === null) { | |
| throw new Error('null element must not be measured'); | |
| } | |
| return node === container | |
| ? { x: 0, y: 0, width: 20, height: 2 } | |
| : { x: 0, y: 4, width: 0, height: 1 }; | |
| }); |
中文说明
[Suggestion] 这是唯一一个声称覆盖缺失（null）子元素的测试，但它无法发现 findElementAtMouseEvent 中的 if (!element) continue; 守卫被删除——mock 把 null 条目映射为一个零宽度矩形，反正会被退化矩形跳过逻辑丢弃。探针双向验证：删除两处守卫后 7 个测试仍然全绿；让 mock 在遇到 null 时抛错后，在无守卫的源码上会失败、在完整源码上通过。— 失败场景：未来某次删除该守卫（或把它移到 measureElementPosition 调用之后）的重构会带着绿灯上线。生产中 CompletionCategoryMouseController 传入的是 Array<DOMElement | null> 类型的 categoryRefs.current（渲染与事件之间 tab ref 可能尚未挂载；React 在卸载时会以 null 调用 ref 回调），而真实的 measureElementPosition 开头就是 const { yogaNode } = node; → 在标签栏上左键按下会抛出 TypeError: Cannot destructure property 'yogaNode' of 'null'，导致鼠标处理器崩溃。
建议修复：让 mock 在遇到 null 时大声失败（见上方 suggestion），从而真正钉住该守卫。
— qwen3.8-max via Qwen Code /review (v0.21.11)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "You are review agent reverse-audit — Reverse audit agent…": none — did not run the vitest suite locally (worktree has no installed node_modules; the PR's own e2e record and later verification stages cover execution).; "You are review agent reverse-audit — Reverse audit agent…": did not empirically verify whether ink returns a non-zero covering rect for an unmounted DOMElement (decides whether the stale-ref trigger above can actually …; "You are review agent reverse-audit — Reverse audit agent…": did not re-walk the rect-mode "many category tabs overflow the container width → column guard clips the overflowing tab" edge (deemed unrealistic: availableCat…; "You are review agent reverse-audit — Reverse audit agent…": none — all planned checks completed within budget.; "You are review agent reverse-audit — Reverse audit agent…": none — all planned checks completed within budget., and 2 more.
Test Plan (not a blocker): src/ui/components/CompletionCategoryMouseController.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.mouse.test.tsx — no such file or directory; src/ui/components/InputPrompt.suggestionMouse.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.test.tsx — no such file or directory; src/ui/hooks/useCompletion.test.ts — no such file or directory; and 2 more.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："You are review agent reverse-audit — Reverse audit agent…"：none — did not run the vitest suite locally (worktree has no installed node_modules; the PR's own e2e record and later verification stages cover execution).；"You are review agent reverse-audit — Reverse audit agent…"：did not empirically verify whether ink returns a non-zero covering rect for an unmounted DOMElement (decides whether the stale-ref trigger above can actually …；"You are review agent reverse-audit — Reverse audit agent…"：did not re-walk the rect-mode "many category tabs overflow the container width → column guard clips the overflowing tab" edge (deemed unrealistic: availableCat…；"You are review agent reverse-audit — Reverse audit agent…"：none — all planned checks completed within budget.；"You are review agent reverse-audit — Reverse audit agent…"：none — all planned checks completed within budget.，另有 2 条。
Test Plan（非阻断）：src/ui/components/CompletionCategoryMouseController.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.mouse.test.tsx — no such file or directory; src/ui/components/InputPrompt.suggestionMouse.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.test.tsx — no such file or directory; src/ui/hooks/useCompletion.test.ts — no such file or directory; and 2 more。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| if (rect.height <= 0 || rect.width <= 0) continue; | ||
| if (containsPoint(rect, point)) return index; |
There was a problem hiding this comment.
[Suggestion] 'rect' mode silently ignores indexOffset while 'row' mode adds it (index: indexOffset + index, line 66) — the shared parameter is mode-dependent with nothing in the signature or doc comment saying so; the rect branch returns the bare loop index. — Concrete cost: the only 'rect' caller today (CompletionCategoryMouseController) passes the default 0, so there is no live bug, but a future 'rect' caller passing a non-zero offset (the parameter's position and default imply it applies to both modes) would receive un-offset indices and select the wrong element; until then this is a divergent pair of hit-test branches whose index semantics a future editor must rediscover by reading both.
| if (rect.height <= 0 || rect.width <= 0) continue; | |
| if (containsPoint(rect, point)) return index; | |
| if (rect.height <= 0 || rect.width <= 0) continue; | |
| if (containsPoint(rect, point)) return indexOffset + index; |
中文说明
[建议] 'rect' 模式会静默忽略 indexOffset，而 'row' 模式会加上它（index: indexOffset + index，第 66 行）——这个共享参数的行为依赖模式，但函数签名和文档注释都没有说明；rect 分支直接返回循环里的裸 index。— 具体代价：当前唯一的 'rect' 调用方（CompletionCategoryMouseController）传入默认值 0，因此没有现实 bug；但未来如果有 'rect' 调用方传入非零偏移量（参数的位置和默认值都暗示它对两种模式生效），就会拿到未偏移的索引并选中错误的元素。在此之前，这对命中测试分支的索引语义已经出现分歧，未来的修改者必须通读两个分支才能重新发现这一点。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| if (index !== null && index < categories.length) { | ||
| onSelectCategory(categories[index]); | ||
| } |
There was a problem hiding this comment.
[Suggestion] The index < categories.length stale-ref guard is paired with no test — probe-verified: deleting the clause leaves all 30 tests across the four mouse suites green. — Failure scenario: SuggestionsDisplay fills categoryRefs.current[i] per render but never truncates the array; if the tab count shrinks while the picker is open, a stale detached DOM node at the old index could resolve a click to an index >= categories.length, and without this guard onSelectCategory(undefined) would fire. Downstream selectCategory ignores unavailable categories, so no incorrect behaviour ships today — but the guard exists solely for this scenario and nothing exercises it. Suggested fix — add one case to CompletionCategoryMouseController.test.tsx:
it('ignores a resolved index beyond the category list', () => {
vi.mocked(findElementAtMouseEvent).mockReturnValue(3);
// render with 3 categories, send a left-press, then:
expect(onSelectCategory).not.toHaveBeenCalled();
});中文说明
[建议] index < categories.length 这个防陈旧 ref 的守卫没有任何测试覆盖——已通过探针验证：删除该条件后，四个鼠标测试套件的全部 30 个测试仍然全部通过。— 失败场景：SuggestionsDisplay 每次渲染都会填充 categoryRefs.current[i]，但从不截断该数组；如果选择器打开期间分类数量减少，旧索引位置上残留的已卸载 DOM 节点可能把一次点击解析为 >= categories.length 的索引；若没有这个守卫，onSelectCategory(undefined) 就会被触发。下游的 selectCategory 会忽略不可用的分类，所以今天不会有错误行为上线——但该守卫正是为这个场景而存在的，却没有任何测试覆盖它。建议修复——在 CompletionCategoryMouseController.test.tsx 中补一个用例：
it('ignores a resolved index beyond the category list', () => {
vi.mocked(findElementAtMouseEvent).mockReturnValue(3);
// 用 3 个分类渲染，发送一次鼠标左键按下，然后断言：
expect(onSelectCategory).not.toHaveBeenCalled();
});— qwen3.8-max via Qwen Code /review (v0.21.11)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "You are review agent reverse-audit — Reverse audit agent…": none — did not run the vitest suite locally (worktree has no installed node_modules; the PR's own e2e record and later verification stages cover execution).; "You are review agent reverse-audit — Reverse audit agent…": did not empirically verify whether ink returns a non-zero covering rect for an unmounted DOMElement (decides whether the stale-ref trigger above can actually …; "You are review agent reverse-audit — Reverse audit agent…": did not re-walk the rect-mode "many category tabs overflow the container width → column guard clips the overflowing tab" edge (deemed unrealistic: availableCat…; "You are review agent reverse-audit — Reverse audit agent…": none — all planned checks completed within budget.; "You are review agent reverse-audit — Reverse audit agent…": none — all planned checks completed within budget., and 2 more.
Test Plan (not a blocker): src/ui/components/CompletionCategoryMouseController.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.mouse.test.tsx — no such file or directory; src/ui/components/InputPrompt.suggestionMouse.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.test.tsx — no such file or directory; src/ui/hooks/useCompletion.test.ts — no such file or directory; and 2 more.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："You are review agent reverse-audit — Reverse audit agent…"：none — did not run the vitest suite locally (worktree has no installed node_modules; the PR's own e2e record and later verification stages cover execution).；"You are review agent reverse-audit — Reverse audit agent…"：did not empirically verify whether ink returns a non-zero covering rect for an unmounted DOMElement (decides whether the stale-ref trigger above can actually …；"You are review agent reverse-audit — Reverse audit agent…"：did not re-walk the rect-mode "many category tabs overflow the container width → column guard clips the overflowing tab" edge (deemed unrealistic: availableCat…；"You are review agent reverse-audit — Reverse audit agent…"：none — all planned checks completed within budget.；"You are review agent reverse-audit — Reverse audit agent…"：none — all planned checks completed within budget.，另有 2 条。
Test Plan（非阻断）：src/ui/components/CompletionCategoryMouseController.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.mouse.test.tsx — no such file or directory; src/ui/components/InputPrompt.suggestionMouse.test.tsx — no such file or directory; src/ui/components/SuggestionsDisplay.test.tsx — no such file or directory; src/ui/hooks/useCompletion.test.ts — no such file or directory; and 2 more。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| if (rect.height <= 0 || rect.width <= 0) continue; | ||
| if (containsPoint(rect, point)) return index; |
There was a problem hiding this comment.
[Suggestion] 'rect' mode silently ignores indexOffset while 'row' mode adds it (index: indexOffset + index, line 66) — the shared parameter is mode-dependent with nothing in the signature or doc comment saying so; the rect branch returns the bare loop index. — Concrete cost: the only 'rect' caller today (CompletionCategoryMouseController) passes the default 0, so there is no live bug, but a future 'rect' caller passing a non-zero offset (the parameter's position and default imply it applies to both modes) would receive un-offset indices and select the wrong element; until then this is a divergent pair of hit-test branches whose index semantics a future editor must rediscover by reading both.
| if (rect.height <= 0 || rect.width <= 0) continue; | |
| if (containsPoint(rect, point)) return index; | |
| if (rect.height <= 0 || rect.width <= 0) continue; | |
| if (containsPoint(rect, point)) return indexOffset + index; |
中文说明
[建议] 'rect' 模式会静默忽略 indexOffset，而 'row' 模式会加上它（index: indexOffset + index，第 66 行）——这个共享参数的行为依赖模式，但函数签名和文档注释都没有说明；rect 分支直接返回循环里的裸 index。— 具体代价：当前唯一的 'rect' 调用方（CompletionCategoryMouseController）传入默认值 0，因此没有现实 bug；但未来如果有 'rect' 调用方传入非零偏移量（参数的位置和默认值都暗示它对两种模式生效），就会拿到未偏移的索引并选中错误的元素。在此之前，这对命中测试分支的索引语义已经出现分歧，未来的修改者必须通读两个分支才能重新发现这一点。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| if (index !== null && index < categories.length) { | ||
| onSelectCategory(categories[index]); | ||
| } |
There was a problem hiding this comment.
[Suggestion] The index < categories.length stale-ref guard is paired with no test — probe-verified: deleting the clause leaves all 30 tests across the four mouse suites green. — Failure scenario: SuggestionsDisplay fills categoryRefs.current[i] per render but never truncates the array; if the tab count shrinks while the picker is open, a stale detached DOM node at the old index could resolve a click to an index >= categories.length, and without this guard onSelectCategory(undefined) would fire. Downstream selectCategory ignores unavailable categories, so no incorrect behaviour ships today — but the guard exists solely for this scenario and nothing exercises it. Suggested fix — add one case to CompletionCategoryMouseController.test.tsx:
it('ignores a resolved index beyond the category list', () => {
vi.mocked(findElementAtMouseEvent).mockReturnValue(3);
// render with 3 categories, send a left-press, then:
expect(onSelectCategory).not.toHaveBeenCalled();
});中文说明
[建议] index < categories.length 这个防陈旧 ref 的守卫没有任何测试覆盖——已通过探针验证：删除该条件后，四个鼠标测试套件的全部 30 个测试仍然全部通过。— 失败场景：SuggestionsDisplay 每次渲染都会填充 categoryRefs.current[i]，但从不截断该数组；如果选择器打开期间分类数量减少，旧索引位置上残留的已卸载 DOM 节点可能把一次点击解析为 >= categories.length 的索引；若没有这个守卫，onSelectCategory(undefined) 就会被触发。下游的 selectCategory 会忽略不可用的分类，所以今天不会有错误行为上线——但该守卫正是为这个场景而存在的，却没有任何测试覆盖它。建议修复——在 CompletionCategoryMouseController.test.tsx 中补一个用例：
it('ignores a resolved index beyond the category list', () => {
vi.mocked(findElementAtMouseEvent).mockReturnValue(3);
// 用 3 个分类渲染，发送一次鼠标左键按下，然后断言：
expect(onSelectCategory).not.toHaveBeenCalled();
});— qwen3.8-max via Qwen Code /review (v0.21.11)
|
@qwen-code /triage |
|
Sandboxed verification: Skipped because the pre-execution risk screen refused this sponsored run: the model risk screen failed or returned an unparseable answer. A maintainer who has reviewed the diff can run the verification manually in a disposable environment. 中文 — 判定：
|
|
Thanks for the PR! Template looks good ✓ Linked issue status — escalation resolved: #8330 was closed as completed on 2026-08-19 with no closing PR (eight days after #8576 landed the keyboard path), so the last run flagged the ambiguity and asked for a maintainer call. That call has now been made: @wenshao built this PR on top of today's Problem: observed bug with solid evidence — a real Warp reproduction in #8330 plus a terminal-level SGR mouse capture showing category-label clicks were no-ops. Not theoretical hardening. Direction: aligned, and now explicitly maintainer-endorsed. The change reuses the existing SGR mouse pipeline behind the existing Size: not applicable — all changes are in Approach: minimal and right-shaped. Hit-testing is extracted from Risk: no elevated risk signals (no high-risk paths matched). Housekeeping: the branch is 592 commits behind Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 关联 issue 状态——升级已有结论： #8330 于 2026-08-19 以"已完成"关闭且没有关闭者引用（在 #8576 的键盘方案合入八天后），因此上一轮审查标记了这一歧义并请求维护者决定。现在已有结论：@wenshao 在今天的 问题：已观测到的 bug，证据扎实——#8330 中的真实 Warp 复现，加上终端级 SGR 鼠标抓包证明点击分类标签无效。不是理论性加固。 方向：对齐，且已获维护者明确认可。改动复用现有 SGR 鼠标管线，并置于现有 规模：不适用——全部改动位于 方案：最小且形态正确。命中测试从 风险：无升级风险信号（未命中高风险路径）。 事务性说明：分支落后 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewRe-read against
Non-blocking notes carried from Stage 1: test-plan step 4 mentions Files changed (15)
TestingThe PR's own CI on this commit — the gate never executes PR code, so this run's evidence is the PR's CI via the API plus the maintainer's real E2E below. The full
On the central behavioural claim: the unit suite pins the wiring gates and exact hit bounds, but the click behaviour itself was independently verified by maintainer @wenshao on the real bundled binary atop today's What remains unverified here: the macOS/Windows/integration jobs (skipped for this fork PR). Sandboxed verification would close the machine-generated gap if wanted: a sponsored 中文说明代码审查针对
来自 Stage 1 的非阻塞提示：测试计划第 4 步提到的 测试本 commit 上 PR 自身的 CI——门禁从不执行 PR 代码，因此本轮证据为通过 API 读取的 PR CI 结果与下述维护者真实 E2E。完整的 关于核心行为性结论：单元测试钉住了接线门控与精确命中边界，而点击行为本身由维护者 @wenshao 在今天的 此处仍未验证：macOS/Windows/集成任务（该 fork PR 被跳过）。如需机器生成的 A/B 证明，可用赞助式 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean across every gate; only non-blocking nits remain (a stale keyboard step in the description, a stale branch that still merges cleanly, fork-CI matrix skips). The question that parked this PR at 3/5 last run — "is a mouse affordance still wanted now that #8576 solved the navigation problem and the reporter closed #8330?" — has been answered in the strongest possible form: @wenshao merged the branch onto today's On the code itself, my independent proposal for this problem — refs on the tab labels, a click controller reusing the existing SGR coordinate helpers rather than a parallel mechanism, and a Non-blocking, for the merge: a rebase before landing keeps the squash tidy (592 commits behind, still MERGEABLE), and test-plan step 4 still names the 中文说明Confidence: 4/5 —— 各阶段门禁全部干净；仅剩非阻塞小项（描述中一处过时的键盘步骤、分支较旧但仍可干净合并、fork CI 矩阵跳过）。 上一轮让本 PR 停在 3/5 的问题——"既然 #8576 已解决导航问题、报告者也关闭了 #8330，鼠标点击能力是否仍被需要？"——如今已以最有力的形式得到回答：@wenshao 将分支合并到今天的 就代码本身而言，我对这个问题的独立方案——标签加 ref、复用现有 SGR 坐标助手的点击控制器（而非平行机制）、与键盘路径共享重置语义的 非阻塞、供合并时参考：合并前 rebase 可保持 squash 干净（落后 592 个提交，仍为 MERGEABLE）；测试计划第 4 步仍写着已被 #8576 替换的 — Qwen Code · qwen3.8-max Reviewed at |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
Maintainer verification — real local E2E, recommend mergeI built this PR on top of today's Verdict: works as described — recommend merge. Notes below are non-blocking. Setup
1. A/B — the fix is real, and the bug reproduces on
|
| SGR event | result |
|---|---|
left-release (0;…m) |
ignored |
drag / move (32;…M) |
ignored |
bare motion (35;…M) |
ignored |
right press (2;…M) |
ignored |
middle press (1;…M) |
ignored |
scroll-up (64;…M) |
ignored |
left press (0;…M) |
switches to Sessions |
4. Four tabs, index mapping stays correct
With two MCP servers configured the bar becomes All / Files / Sessions / MCP; all four labels activate their own category (an off-by-one in categories[index] would surface here).
5. Overflowing (bottom-pinned) frame — the negative frameAnchor path
At 14 rows the ink frame is taller than the terminal, so the logo, tips and footer are scrolled off and the frame is bottom-pinned. A click at terminal row 3 still resolves to Sessions, i.e. layoutRowForEvent's negative anchor is applied correctly. Also verified at 18 and 24 rows.
6. Wrapped, two-row labels
At 34 columns the tab bar wraps and each label Box becomes 2 rows tall. Clicking either half hits — containsPoint handles multi-row rectangles.
7. Category-set churn — the index-based categoryRefs risk does not bite
@ (4 tabs: All/Files/Sessions/MCP) → type src so the middle category (session) disappears and MCP shifts from index 3 to index 2 → all three remaining tabs still hit their own category → backspace back to 4 tabs → Sessions hits again. No stale-ref misfire.
8. ui.mouseTracking: false
The identical click is ignored, and — worth confirming — the raw SGR bytes do not leak into the input buffer.
9. Keyboard is untouched
Bare ←/→ still cycle categories on both builds, and after Esc dismisses the picker the arrows go back to moving the caret (@abc + Esc + ←← + X → @aXbc).
Static checks on the merged tree
| check | result |
|---|---|
| Focused suite (9 files incl. all new ones) | 93 passed |
packages/cli src/ui/{components,hooks,utils} |
261 files / 5137 passed, 1 skipped |
npm run lint |
pass |
npm run typecheck |
pass |
| Mutation probes on the new code | 7 of 8 killed |
Killed mutants: dropping the horizontal bound in containsPoint; dropping the
indexOffset in row mode; widening the left-press guard; dropping either half
of selectCategory's early return; mounting the controller without the
mouseEnabled gate; dropping the setExpandedSuggestionIndex(-1) reset. The one
survivor is an equivalent mutant (see note 2).
Non-blocking notes
-
Fixes #8330no longer closes anything, and the stated motivation is stale.
@ completion tab switching is inaccessible in Warp because Ctrl+Tab switches terminal tabs #8330 was closed on 2026-08-19 by fix(cli): switch @ completion category tabs with bare arrow keys #8576 ("switch @ completion category tabs
with bare arrow keys"), which merged on 2026-08-11 and is already in this
PR's own base. My run confirms bare←/→cycle categories on plainmain,
so "could leave the tabbed@picker without any navigation path that reaches
Qwen Code" is no longer accurate. The mouse path is still worth having — it is
what the issue thread explicitly asked for — but the PR body should say it
adds a pointer path alongside fix(cli): switch @ completion category tabs with bare arrow keys #8576's bare arrows, and useRefs #8330
rather thanFixes. -
Dead guard in
mouse-hit.ts. In'rect'mode the
if (rect.height <= 0 || rect.width <= 0) continue;skip is unreachable —
containsPointalready rejects a degenerate rectangle (point.y < rect.y + 0
is never true). This is the mutant that survived; removing the width/height
half of that line changes nothing. Per Simplicity First it could go, or the
comment could say it's a cheap short-circuit. -
onSelectCategory's gate duplicatescategoryTabsVisible.
InputPrompt.tsxalready computes
categoryTabsVisible = !export && !commandSearch && !reverseSearch && !isAttachmentMode && availableCategories.length > 2,
and the new gate re-derives a subset of it (it omitsisAttachmentMode).
Harmless today —availableCategoriesisundefinedin those modes so the bar
cannot render — butonSelectCategory={categoryTabsVisible ? handleCategorySelect : undefined}
would be shorter and would not drift. -
No hover affordance on the tabs. Suggestion rows highlight on hover;
the tabs do not, so nothing signals that they are clickable. Worth a follow-up. -
Platform coverage. On this PR only
Test (ubuntu-latest, Node 22.x)ran —
the macOS and Windows jobs showskipping. My verification is Linux-only too,
so Warp on macOS (the reporter's actual environment) is still unverified by
anyone. The geometry work is terminal-agnostic, so I don't consider this a
blocker, but it is the one gap left. -
Harness gotcha, not a PR defect, but worth knowing. Any stdout written
before ink takes the alternate screen shifts every mouse hit test by that
many rows. In my containerrgwas missing, so the crawler printed 3 lines at
startup and both the tab hit test and the pre-existing row hit test were off
by exactly 3 rows until I putrgonPATH. This affectsRowMouseController
identically onmain, so it is pre-existing — but it is the failure mode a
user would report as "clicks land on the wrong row".
中文说明
维护者验证 —— 本地真实环境 E2E，建议合并
我把这个 PR 合到今天的 main 上，构建出真实的打包 CLI，在终端里用原始 SGR
鼠标字节点击分类标签。PR 声明的行为全部复现，包括它自己的 e2e 记录留给 reviewer
的那部分（"Manual Warp execution remains for reviewer verification"）。
结论：行为与描述一致，建议合并。 下面的说明均不阻断合并。
环境
| 基线 | main @ 37876bcb56 + PR head 749804ff，合并无冲突（PR 基线落后 main 584 个提交） |
| 构建 | npm ci && npm run build && npm run bundle，通过 scripts/cli-entry.js 启动（打包产物，不是 npm run dev） |
| 夹具 | tmux -L pr8395，隔离的 HOME 与 QWEN_RUNTIME_DIR，构造历史会话文件，使裸 @ 同时产生 file 与 session 两类建议（≥2 类才会渲染标签栏） |
| 点击 | 用 tmux send-keys -H 把原始 ESC[<0;<col>;<row>M / m 直接注入 pane 的 stdin —— 与真实终端发出的字节完全相同，且绕过 tmux 自身的鼠标处理 |
| 尺寸 | 120×40、100×24、100×18、100×14、46×30、34×30 |
| 平台 | Linux，Node v22.22.2 |
1. A/B 对照 —— 修复真实有效，且问题在 main 上可复现
同一工作区、同一终端、同一处点击（第 14 行第 22 列，落在 Sessions 内）。
带本 PR： 精确激活对应标签，首条结果重新高亮，建议列表按该分类过滤。
纯 main（同一棵树，把 PR 的生产文件回退到 origin/main 后重新打包）：完全无反应。
截图见英文部分 1（01-pr-click-sessions.png / 02-main-baseline-noop.png）。
2. 命中区域精确 —— 完整列扫描
对标签行的每一列都先复位到已知标签再点击，并做两遍（复位到 All 和复位到
Sessions），以便把"未命中"与"命中我们刚复位的那个标签"区分开：
第 1– 2 列 → 未命中 容器左侧留白
第 3– 7 列 → All 渲染为 " All " （5 列）
第 8 列 → 未命中 marginLeft 间隙
第 9–15 列 → Files 渲染为 " Files " （7 列）
第 16 列 → 未命中 marginLeft 间隙
第 17–26 列 → Sessions 渲染为 " Sessions " （10 列）
第 27–46 列 → 未命中 间隙 + "(←/→ to switch)" 提示
这与实际渲染的标签边界完全一致，因此 PR 所说"点击标签之间或标签之外不改变当前
分类"在单列精度上成立。
纵向上只有标签行是可点的：row−2、row−1 和 row+1（marginBottom 空行）都不
改变分类；row+2 是第一条建议行，正确地交给既有的 RowMouseController。
3. 只有鼠标左键按下会激活标签
在 All 处于激活状态时，向 Sessions 中心发送各类原始事件：
| SGR 事件 | 结果 |
|---|---|
左键释放 (0;…m) |
忽略 |
拖拽 / 移动 (32;…M) |
忽略 |
裸移动 (35;…M) |
忽略 |
右键按下 (2;…M) |
忽略 |
中键按下 (1;…M) |
忽略 |
滚轮上滚 (64;…M) |
忽略 |
左键按下 (0;…M) |
切换到 Sessions |
4. 四个标签时索引映射依然正确
配置两个 MCP server 后标签栏变成 All / Files / Sessions / MCP，四个标签都能各自
激活对应分类（categories[index] 若有 off-by-one，会在这里暴露）。
5. 帧溢出（底部对齐）场景 —— 负 frameAnchor 路径
终端只有 14 行时，ink 帧高于终端，logo、tips 和 footer 都被滚出屏幕，帧底部对齐。
此时点击终端第 3 行仍然正确命中 Sessions，说明 layoutRowForEvent 的负锚点
修正生效。18 行和 24 行同样验证通过。
6. 标签换行成两行时
34 列时标签栏换行，每个标签 Box 变成 2 行高。点击上下任意一半都能命中 ——
containsPoint 正确处理了跨行矩形。
7. 分类集合变化 —— 基于下标的 categoryRefs 风险未发生
@（4 个标签 All/Files/Sessions/MCP）→ 输入 src 使中间的 session 分类消失、
MCP 从下标 3 移到下标 2 → 剩下三个标签仍各自命中 → 退格回到 4 个标签 →
Sessions 再次命中。没有出现 ref 过期导致的错位。
8. ui.mouseTracking: false
同样的点击被忽略；并且值得确认的是，原始 SGR 字节不会泄漏进输入框。
9. 键盘行为未受影响
裸 ←/→ 在两个构建上都仍能循环切换分类；Esc 关闭选择器后方向键恢复为移动
光标（@abc + Esc + ←← + X → @aXbc）。
合并后代码树上的静态检查
| 检查项 | 结果 |
|---|---|
| 聚焦测试（9 个文件，含全部新增文件） | 93 通过 |
packages/cli 的 src/ui/{components,hooks,utils} |
261 文件 / 5137 通过，1 跳过 |
npm run lint |
通过 |
npm run typecheck |
通过 |
| 对新增代码的变异测试 | 8 个中杀死 7 个 |
被杀死的变异体：去掉 containsPoint 的水平边界；去掉行模式的 indexOffset；放宽
left-press 判断；去掉 selectCategory 早返回的任意一半；绕过 mouseEnabled 直接
挂载控制器；去掉 setExpandedSuggestionIndex(-1) 重置。唯一存活的是等价变异体
（见说明 2）。
不阻断合并的说明
-
Fixes #8330已经关不掉任何东西，且动机描述已过时。
@ completion tab switching is inaccessible in Warp because Ctrl+Tab switches terminal tabs #8330 已于 2026-08-19 被 fix(cli): switch @ completion category tabs with bare arrow keys #8576（"switch @ completion category tabs with bare
arrow keys"）关闭；fix(cli): switch @ completion category tabs with bare arrow keys #8576 于 2026-08-11 合入，已经包含在本 PR 自己的基线里。
我的实测确认裸←/→在纯main上就能切换分类，因此"可能让带分类的@选择器
没有任何能真正到达 Qwen Code 的导航方式"这一说法已不准确。鼠标路径本身仍然值得
合入 —— 这正是 issue 讨论中明确要求的 —— 但 PR 描述应改成"在 fix(cli): switch @ completion category tabs with bare arrow keys #8576 的裸方向键
之外新增一条指针路径"，并把Fixes #8330改为Refs #8330。 -
mouse-hit.ts中的无效保护分支。 在'rect'模式下
if (rect.height <= 0 || rect.width <= 0) continue;是不可达的 ——
containsPoint已经排除了退化矩形（point.y < rect.y + 0永远为假）。这正是存活
的那个变异体：把这一行的宽/高判断删掉，行为不变。按 Simplicity First 可以删掉，
或者在注释里说明它只是一个廉价短路。 -
onSelectCategory的门控与categoryTabsVisible重复。
InputPrompt.tsx已经算出
categoryTabsVisible = !export && !commandSearch && !reverseSearch && !isAttachmentMode && availableCategories.length > 2，
而新增的门控只是它的一个子集（漏了isAttachmentMode）。目前无害 ——
这些模式下availableCategories本来就是undefined，标签栏根本不会渲染 ——
但写成onSelectCategory={categoryTabsVisible ? handleCategorySelect : undefined}
更短，也不会日后走样。 -
标签没有 hover 反馈。 建议行有 hover 高亮，标签没有，用户没有任何提示知道它们
可以点击。建议后续补上。 -
平台覆盖。 本 PR 上只有
Test (ubuntu-latest, Node 22.x)实际运行，macOS 和
Windows 的 job 都是skipping。我的验证也只在 Linux 上做，因此
macOS 上的 Warp（报告者的真实环境）至今没有任何人验证过。几何计算与终端无关，
我不认为这是阻断项，但这是唯一剩下的缺口。 -
夹具层面的坑（不是本 PR 的缺陷，但值得知道）。 在 ink 接管备用屏幕之前写到
stdout 的任何输出，都会让所有鼠标命中判定整体偏移相应的行数。我的容器里缺少
rg，导致启动时 crawler 打印了 3 行日志，于是标签命中判定和既有的建议行命中
判定都恰好偏了 3 行，直到把rg加进PATH才正常。这个问题在main上同样影响
RowMouseController，属于既有问题 —— 但它正是用户会以"点击落在错误的行上"报上来
的那种故障。
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Sandboxed verification: Skipped because the PR is not open for verification (state=MERGED, draft=false). 中文 — 判定：
|
3 participants
What this PR does
Makes the category tabs in the
@completion picker clickable when terminal mouse tracking is active. A left-button press is mapped through the existing SGR mouse pipeline and the rendered Ink bounds to select the exact category under the pointer. Exact category changes reset the active suggestion and scroll position, while the existing keyboard cycling behavior remains unchanged.Why it's needed
In Warp,
Ctrl+Tabswitches terminal tabs andCtrl+Left/Rightis commonly consumed for word movement. The completion category bar was display-only: suggestion rows registered mouse hit areas, but category labels had no refs or mouse handler. This could leave the tabbed@picker without any navigation path that reaches Qwen Code.Reviewer Test Plan
How to verify
ui.mouseTrackingand the terminal buffer enabled.@shows at least All / Files / Sessions, click each category label and confirm the exact tab activates, the first result is highlighted, and the visible suggestions are filtered to that category.Ctrl+Tab,Ctrl+Shift+Tab, andCtrl+Left/Rightswitching still works in a terminal that forwards those keys.ui.mouseTracking: falseand confirm Qwen Code does not intercept category clicks.Automated verification:
Evidence (Before & After)
Before: the issue's raw SGR reproduction leaves the active category unchanged, and
upstream/mainrenders category labels without refs or a click controller. After: the new controller test sends a left press inside the rendered Sessions bounds and selectssession; outside-bound clicks, move/release events, and mouse-disabled wiring are covered by regression tests. The current-head focused suite passes 7 files and 77 tests; it also pins unavailable-category no-ops and keeps row hit-testing single-sourced. Manual Warp execution remains for reviewer verification because Warp and a released globalqwenexecutable were not available in this environment.Tested on
Environment (optional)
Node.js 26.5.0 and npm 11.17.0 in the local worktree. No released global
qwenexecutable or Warp session was available for manual terminal capture.Risk & Scope
layoutRowForEventandmeasureElementPositionhelpers and tests exact hit bounds.Linked Issues
Fixes #8330
中文说明
这个 PR 做了什么
当终端鼠标追踪开启时，让
@补全选择器的分类标签可以点击。鼠标左键按下事件会通过现有 SGR 鼠标管线和 Ink 实际渲染边界，精确选择指针下方的分类。切换到具体分类时会重置当前高亮建议与滚动位置，原有键盘循环切换行为保持不变。为什么需要它
在 Warp 中，
Ctrl+Tab会切换终端标签页，Ctrl+Left/Right也常被终端用于按词移动。此前补全分类栏只负责显示：建议行注册了鼠标命中区域，但分类标签没有 ref 或鼠标处理器。因此，带分类的@补全选择器可能没有任何能够真正到达 Qwen Code 的导航方式。Reviewer 测试计划
如何验证
ui.mouseTracking与终端缓冲区。@后至少出现 All / Files / Sessions 的工作区中，依次点击每个分类标签，确认精确激活对应标签、首条结果被高亮，并且可见建议按该分类过滤。Ctrl+Tab、Ctrl+Shift+Tab和Ctrl+Left/Right切换仍然有效。ui.mouseTracking: false后重启，确认 Qwen Code 不会拦截分类标签点击。自动验证命令与英文部分相同：聚焦测试、整仓 lint、整仓 build 和整仓 typecheck 均已运行。
前后证据
修复前：Issue 中的原始 SGR 复现表明点击后当前分类不变，且
upstream/main的分类标签没有 ref 或点击控制器。修复后：新增控制器测试在 Sessions 的实际边界内发送鼠标左键按下事件并选中session；越界点击、移动/释放事件以及关闭鼠标时的接线也有回归测试。当前 head 的聚焦测试共 7 个文件、77 个用例，全部通过；同时覆盖不可用分类的 no-op，并让行命中判断保持单一实现。由于当前环境没有 Warp 和已发布的全局qwen，真实 Warp 手工执行留给 Reviewer 验证。测试平台
环境（可选）
本地工作树使用 Node.js 26.5.0 和 npm 11.17.0。当前环境没有已发布的全局
qwen或 Warp 会话，因此没有生成真实终端录屏。风险与范围
layoutRowForEvent和measureElementPosition，并测试了精确命中边界。关联 Issue
Fixes #8330