fix(input): restore IME cursor positioning reverted in #4779 by BenGuanRan · Pull Request #4993 · QwenLM/qwen-code

BenGuanRan · 2026-06-11T07:11:43Z

Summary

Restores PR #4652 (IME physical cursor positioning) which was silently reverted as part of PR #4779. See issue #4987 for context — the revert was my mistake while resolving a merge conflict, and a merged feature should not be rolled back inside an unrelated PR without explanation.

What's restored

Two cherry-picks, both with the original authors preserved:

51cafe433 — original PR feat(input): move physical cursor to visual cursor for IME input #4652 (@zzhenyao): adds BaseTextInput physical-cursor positioning via addLayoutListener + CursorContext, the prefixWidth plumbing through InputPrompt / AgentComposer, and the patches/ink+7.0.3.patch that exposes ink/dom and ink/components/CursorContext (plus the patch-package postinstall step).
370e480b8 — cherry-pick of a19eec771 by @LaZzyMan: fixes the dangling showYoloStyling reference in prefixWidth after PR fix(ui): distinguish auto approval mode indicators #4600 renamed it to approvalModePromptStyle. This fix existed on a side branch but never landed on main — it's needed here because feat(stats): add interactive /stats dashboard with cross-session tracking #4779's revert hid the original bug.

Verification

npm run build and npm run typecheck pass cleanly across all workspaces.
14 tests fail under npm run preflight (1 in memoryDiagnostics.test.ts, 13 in AuthDialog.test.tsx with vi.waitFor timeouts). These touch auth dialog rendering and heap-snapshot cleanup — unrelated to anything in this PR. Likely pre-existing on main; please confirm during review.

Test plan

Maintainer reruns CI to confirm the 14 unrelated failures reproduce on main.
Manual check: with a CJK IME (e.g. Pinyin, Japanese), composition text appears at the cursor position rather than at the start of the line. This is the regression originally reported in CJK (Chinese/Japanese/Korean) IME composition text appears at wrong position — input shows on an extra line #3456.

Closes #4987.

BenGuanRan · 2026-06-11T07:38:31Z

CI failure note: all three OS test jobs fail on the same two cases in src/utils/yaml-parser.test.ts:

stringify > known limitations — nested YAML (pin until js-yaml lands) > garbles array-of-records
stringify > known limitations — nested YAML (pin until js-yaml lands) > garbles record-of-records

The test names self-describe as pins for a known upstream js-yaml limitation. The same two failures reproduce on the latest main (run on afd631335) without this PR's changes — they're unrelated to the IME cursor restoration and not caused by anything in #4993.

Lint, CodeQL, and Classify PR all pass.

…nLM#4652) * feat(input): add useCursor hook for IME physical cursor tracking * refactor(input): optimize cursor positioning effect * feat(input): move setCursorPosition to render phase for immediate cursor positioning * fix(input): calculate absolute cursor position by walking yoga tree * fix(input): use addLayoutListener instead of useCursor for zero-jitter cursor positioning * perf(input): stable addLayoutListener subscription and skip redundant cursor updates * fix(input): revert lastPos dedup that broke cursorDirty one-shot flag * feat(input): use patch-package to expose Ink internals for IME cursor positioning * fix(input): address review feedback — prefixWidth, remove useBoxMetrics, pin ink version Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>

QwenLM#4600 unified the approval-mode prompt styling into approvalModePromptStyle and removed the showAutoAcceptStyling / showYoloStyling constants, but left a dangling showYoloStyling reference in the prefixWidth calculation. This broke the tsc build (TS2304: Cannot find name showYoloStyling). npm run bundle (esbuild) skips type-checking, so the break only surfaced under npm run build / tsc --build. Replace it with approvalMode === ApprovalMode.YOLO, matching how the rest of the file branches on approval mode after QwenLM#4600. Both ternary arms stay 2 because every approval-mode prefix is a single char (getApprovalModePromptStyle returns prefix > or *), so the rendered prefix width and cursor positioning are unchanged.

BenGuanRan · 2026-06-11T08:13:59Z

@zzhenyao help review , sorry for that commit-revert

DragonnZhang

No issues found. LGTM! — qwen3-coder via Qwen Code /review

zzhenyao · 2026-06-11T08:55:57Z

@zzhenyao help review , sorry for that commit-revert

LGTM, thanks for restoring!

wenshao · 2026-06-11T20:28:41Z

    "build:sdk:python": "python3 -m build packages/sdk-python",
    "check-i18n": "npm run check-i18n --workspace=packages/cli",
    "preflight": "npm run clean && npm ci && npm run format && npm run lint:ci && npm run build && npm run typecheck && npm run test:ci",
+    "postinstall": "patch-package",


[Critical] "postinstall": "patch-package" will block all npm install runs if the Ink patch fails to apply (ink version bump, patch conflict, file permissions, etc.). This breaks CI pipelines, Docker builds, and fresh checkouts with no fallback.

Suggested change

"postinstall": "patch-package",

"postinstall": "patch-package || true"

Alternatively, run patch-package only in CI with --error-on-fail, or move it to prepare where failures are more expected.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

wenshao · 2026-06-11T20:28:41Z

+      : 2 // "! " = 2 chars
+    : commandSearchActive
+      ? 6 // "(r:) " (inner) + " " (outer) = 6 cols
+      : approvalMode === ApprovalMode.YOLO


[Suggestion] Dead ternary: both branches return 2, making the approvalMode === ApprovalMode.YOLO check meaningless. Reads as if YOLO needs a distinct width, but produces the same value as the default branch. A future change that adds a multi-character prefix for a new approval mode may update only one branch, causing a silent IME cursor misalignment.

Suggested change

: approvalMode === ApprovalMode.YOLO

: 2; // "> " or "* " = 2 chars

— DeepSeek/deepseek-v4-pro via Qwen Code /review

wenshao · 2026-06-11T20:28:41Z

+  if (!node.parentNode)
+    return node['nodeName'] === 'ink-root' ? (node as DOMElement) : undefined;
+  return findRootNode(node.parentNode as Record<string, unknown>);
+}


[Suggestion] findRootNode recurses up Ink's internal parentNode chain with no depth limit. If Ink's DOM structure ever changes unexpectedly (e.g., a virtual wrapper node introduced between the Box and ink-root, or a render bug creating a self-reference), this will crash with RangeError: Maximum call stack size exceeded — a hard CLI process crash with no recovery.

Suggested change

}

function findRootNode(

node: (Record<string, unknown> & { parentNode?: unknown }) | null,

depth = 0,

): DOMElement | undefined {

if (!node || depth > 50) return undefined;

// ... rest unchanged

return findRootNode(node.parentNode as Record<string, unknown>, depth + 1);

}

— DeepSeek/deepseek-v4-pro via Qwen Code /review

wenshao · 2026-06-11T20:28:42Z

    </Text>
  );

+  // Calculate prefix width for physical cursor positioning


[Suggestion] prefixWidth values are hardcoded (2 for default/YOLO, 6 for search modes) rather than derived from the actual prefix string. If shell/approval mode prefixes change (e.g., a custom approval mode with a 3-character prefix), the physical cursor X position will be silently misaligned. The IME popup will appear offset from the actual composition text.

Consider computing prefixWidth from the rendered prefix string using stringWidth() instead of maintaining a parallel hardcoded mapping.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

wenshao · 2026-06-11T20:28:42Z

+  // but BEFORE onRender() — yoga layout is fresh, terminal not yet written.
+  // addLayoutListener requires the root node (ink-root), not the component
+  // node. We find it by walking up the Ink DOM parent chain.
+  const rootRef = useRef(null);


[Suggestion] useRef(null) without a type parameter — TypeScript infers MutableRefObject<null>, so rootRef.current is typed as null even though React will assign a DOMElement at runtime. A future refactor could add incorrect null guards based on this type, silently breaking the feature.

Suggested change

const rootRef = useRef(null);

const rootRef = useRef<DOMElement | null>(null);

— DeepSeek/deepseek-v4-pro via Qwen Code /review

This was referenced Jun 11, 2026

PR #4779 silently reverted #4652 which was already merged to main #4987

Open

feat(stats): add interactive /stats dashboard with cross-session tracking #4779

Merged

BenGuanRan mentioned this pull request Jun 11, 2026

/stats permanently double-counts a session if /stats is opened during the first-ever turn (introduced by #4779) #4994

Closed

zzhenyao and others added 2 commits June 11, 2026 16:08

BenGuanRan force-pushed the fix/restore-4652 branch from 370e480 to 45cd12e Compare June 11, 2026 08:09

DragonnZhang approved these changes Jun 11, 2026

View reviewed changes

wenshao requested changes Jun 11, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(input): restore IME cursor positioning reverted in #4779#4993

fix(input): restore IME cursor positioning reverted in #4779#4993
BenGuanRan wants to merge 2 commits into
QwenLM:mainfrom
BenGuanRan:fix/restore-4652

BenGuanRan commented Jun 11, 2026 •

edited

Loading

Uh oh!

BenGuanRan commented Jun 11, 2026

Uh oh!

BenGuanRan commented Jun 11, 2026

Uh oh!

DragonnZhang left a comment

Uh oh!

zzhenyao commented Jun 11, 2026

Uh oh!

wenshao Jun 11, 2026

Uh oh!

wenshao Jun 11, 2026

Uh oh!

wenshao Jun 11, 2026

Uh oh!

wenshao Jun 11, 2026

Uh oh!

wenshao Jun 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	"postinstall": "patch-package",
	"postinstall": "patch-package \|\| true"

	: approvalMode === ApprovalMode.YOLO
	: 2; // "> " or "* " = 2 chars

-}
+function findRootNode(
+  node: (Record<string, unknown> & { parentNode?: unknown }) | null,
+  depth = 0,
+): DOMElement | undefined {
+  if (!node || depth > 50) return undefined;
+  // ... rest unchanged
+  return findRootNode(node.parentNode as Record<string, unknown>, depth + 1);
+}

	const rootRef = useRef(null);
	const rootRef = useRef<DOMElement \| null>(null);

Conversation

BenGuanRan commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What's restored

Verification

Test plan

Uh oh!

BenGuanRan commented Jun 11, 2026

Uh oh!

BenGuanRan commented Jun 11, 2026

Uh oh!

DragonnZhang left a comment

Choose a reason for hiding this comment

Uh oh!

zzhenyao commented Jun 11, 2026

Uh oh!

wenshao Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

wenshao Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

wenshao Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

wenshao Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

wenshao Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

BenGuanRan commented Jun 11, 2026 •

edited

Loading