[PresentJay]

What & why

Fixes #9624. When someone presses Enter to finish composing a character in a chat question, the desktop can submit the answer or advance to the next question before they finish typing.

Use the existing IME composition guard in the question text input. In the Electron reproduction, confirming the Korean character 각 sent one unintended response before the fix and zero afterward. Pressing Enter again after composition submitted the completed answer exactly once. The test uses the actual question component with a local callback.

Verification: The focused IME regression tests, lint and typecheck passed. The repository checks include the full Linux test and terminal integration suites on the PR commit.

How I tested it

Bun 1.4.3, frozen lockfile install with scripts disabled, source branched from current main at b5da7a52bcdac3f547a08ab148cb13a078612dc3.

Regression and renderer behavior

• Before the fix, both new regression cases fail: composing Enter invokes onRespond once instead of zero.
• After the fix, the new cases and the existing question-page tests pass: 5 pass, 0 fail, 14 assertions.
• Rendered the actual repository QuestionForm in a separate Electron 44.10.0 window, using locked Vite 8.3.6, React plugin 6.2.1 and React Compiler 1.0.0.
• Chromium CDP Input.imeSetComposition produces ㄱ → 가 → 각. The composing Enter reports isComposing: true: baseline responds 1 time, fixed code responds 0 times. After composition ends, ordinary Enter responds 1 time and preserves 각.

Before: composing Enter submits	After: composing Enter stays in the input

Evidence manifest. Images are on a separate evidence branch and add no files to this PR.

Boundary: this is actual-component / Chromium IME verification with a local response callback. No running agent or database was involved. It is not a physical macOS keyboard test or a full installed-app question/answer flow. Repeated Korean character insertion was not reproduced, and this PR does not claim to fix it.

Upstream checks

CLA Assistant passed. The upstream CI run and Deploy Preview still require maintainer approval. The Linux result below is from a local run.

Repository checks

Linux, PR head 7f0f088: all 31 Turbo tasks passed, including terminal integration, with forced execution. I ran the tracked source of this commit in an isolated non-root ARM64 Linux container with Bun 1.4.3, Node 22, a frozen dependency install, and a login-shell PATH modeled on Ubuntu. The focused question suites passed 5/5 (14 assertions). No source changes from other PRs were included in the passing run.

The runner uses Docker --init to reap orphaned subprocesses and --tmpfs /tmp:rw,exec,mode=1777,size=2g for executable temporary fixtures with local filesystem notifications. Run bun x turbo test test:integration:terminal --force --concurrency=4 --log-order=stream.

• With overlay-backed temporary files, 30/31 tasks passed. Two GNU tail session-start tests missed the event. Both passed in isolation on tmpfs; independent fix fix(agent-setup): detect short Codex sessions sooner with GNU tail #9620 also made both pass on overlay.
• With tmpfs but no init reaper, 29/31 tasks passed. Two subprocess-cleanup tests still observed dead children, and the container showed zombie processes parented to PID 1. With init, both passed.

Combined macOS verification

On macOS arm64 (Bun 1.4.3, Node 22.23.4, /bin/zsh), I applied the following seven companion patches locally to IME commit 7f0f088. I verified that applying the published diffs to that commit reproduces the tested source tree exactly.

Companion PR	Change included in the run
#9658	Compare overnight recurrence slots by their UTC instants.
#9659	Account for the directories macOS adds to the shell PATH.
#9662	Wait for in-flight filesystem watcher release during shutdown.
#9664	Set terminal capabilities explicitly in hyperlink tests and cover plain URLs on a dumb terminal.
#9665	Exercise the session scan limits with small real directory trees.
#9666	Reduce Git subprocesses when setting up test repositories.
#9667	Check direct and aliased error-message use in one source scan.

TERM=dumb TURBO_FORCE=true bun run test passed 30/30 Turbo tasks in one invocation (zero cache hits, 2m 40.674s), using the repository’s default parallelism and timeouts. All 2,861 host-service tests passed. Repo-wide lint and typecheck also passed on this combination.

The result above applies to the combined checkout. On the IME commit alone, earlier macOS runs encountered locale and PATH assertions, CLI terminal assumptions, intermittent timeouts, and one native FSEvents SIGTRAP. The SIGTRAP did not recur in the passing run; its native cause remains unconfirmed.

No user-facing strings changed; no catalogs were regenerated for submission.

Checklist

• PR title follows conventional commits (type(scope): subject)
• bun run lint and bun run typecheck pass (CI fails on lint warnings too)
• "Allow edits from maintainers" is checked on fork PRs
• I have signed the CLA (existing signature verified)

Summary by CodeRabbit

• Bug Fixes
  • Pressing Enter while composing text with an input method editor no longer submits the question prematurely.
  • The entered text is preserved during composition, and pressing Enter after composition ends submits the question with the selected custom-field value.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

QuestionPage now skips Enter handling during IME composition. A QuestionForm regression test checks that composition Enter does not submit or clear the entered text, and that a subsequent Enter submits the custom-field value.

Changes: IME-safe question submission

Layer / File(s)	Summary
Guard question submission during composition .../QuestionForm/components/QuestionPage/QuestionPage.tsx, .../QuestionForm/QuestionForm.test.tsx	QuestionPage uses isImeComposing to skip onEnter during composition. The test checks composing Enter events and verifies that a later Enter submits the custom-field value.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: harshithmullapudi

Merge Risk: Minimal · up to 7f0f0

This change stops Enter during IME composition from submitting a chat answer, and ordinary Enter still submits as before. No merge-blocking risk was found.

Pre-merge checks: 3 · 1

Failed checks: 1 inconclusive

Check name	Status	Explanation	Resolution
Linked Issues check		#9624 (direct): The change at head replaces the unguarded Enter check in QuestionPage.tsx with if (event.key === "Enter" && !isImeComposing(event)) onEnter();, reusing the existing isImeComposing …	To decide #9620: confirm the tail invocation in the merge-base agent-setup source (the file that builds the GNU tail -F command used by the session-log watcher) and the Start-event assertions in `packages/agent-setup/src/agent-wrappers.te…

Passed checks: 3 passed

Check name	Status	Explanation
Title check		The title uses conventional commit format and names the main change: IME composition Enter no longer submits a chat question answer. It matches the QuestionPage change (Enter handled only when IME com…
Out of Scope Changes check		The whole-PR diff shows two changed files relevant to this PR: QuestionPage.tsx (one import added and one Enter guard changed) and QuestionForm.test.tsx (+58 lines of IME regression tests). Both conne…
Description check		The description follows the repository template closely. It has the 'What & why' section, which states the problem (Enter during IME composition submits a chat answer, issue #9624), the change (reusin…

Full details: Title check

Explanation

The title uses conventional commit format and names the main change: IME composition Enter no longer submits a chat question answer. It matches the QuestionPage change (Enter handled only when IME composition is inactive) and the added IME regression tests in QuestionForm.test.tsx.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff shows two changed files relevant to this PR: QuestionPage.tsx (one import added and one Enter guard changed) and QuestionForm.test.tsx (+58 lines of IME regression tests). Both connect directly to #9624. The diff does not show changes under packages/agent-setup, so no unrelated #9620 code was introduced. The repository checks in the PR description (lint, typecheck, full test runs) are verification evidence, not code changes, and the PR description states no user-facing strings changed. No out-of-scope change was found.

Full details: Description check

Explanation

The description follows the repository template closely. It has the 'What & why' section, which states the problem (Enter during IME composition submits a chat answer, issue #9624), the change (reusing the existing IME composition guard in the question text input), and a link to the issue. It has the 'How I tested it' section with detailed verification: a before/after Electron and Chromium IME reproduction (1 response before, 0 after, and ordinary Enter then submits once), focused regression tests (5 pass, 14 assertions), lint and typecheck, and full repository runs on Linux and macOS. It also states the verification boundaries and what is not claimed. The checklist is present and checked. The description is long and includes companion-PR and macOS material that goes beyond the template. This does not make it incomplete. The template's title guidance is not applied here: the title is outside this description, so it is not assessed. Reviewers can determine what changed, why, and how it was verified.

Full details: Linked Issues check

Explanation

#9624 (direct): The change at head replaces the unguarded Enter check in QuestionPage.tsx with if (event.key === "Enter" && !isImeComposing(event)) onEnter();, reusing the existing isImeComposing helper from @superset/ui/lib/keyboard. The added QuestionForm.test.tsx cases cover composing Enter (no submit) and a later ordinary Enter (one submit preserving the value). This meets the #9624 objective, as stated in the diff and the regression test summary. The diff does not change modifier-key handling. #9620 (direct): The objective is GNU tail -F polling every 50 ms, non-GNU arguments unchanged, and a test asserting exactly one Start event. The whole-PR diff (merge base b5da7a5 to head 7f0f088) shows no changes under packages/agent-setup, so this PR does not implement the #9620 change. The head does not contain the test change named in the objectives. I did not establish whether the merge base already has the 50 ms GNU tail behavior. The #9620 issue is open, which suggests it is not present, but this is unverified. The PR description states the #9620 change was split to a separate branch, so the reported 31/31 Linux run does not establish this PR's head behavior for #9620.

Resolution

To decide #9620: confirm the tail invocation in the merge-base agent-setup source (the file that builds the GNU tail -F command used by the session-log watcher) and the Start-event assertions in packages/agent-setup/src/agent-wrappers.test.ts at the merge base. If the merge base lacks the 50 ms GNU tail polling and the exactly-one-Start test, the PR must either include that change or the #9620 link must be removed from this PR. If the merge base already has it, record that evidence and the #9620 objective is satisfied.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: aea719d4-0575-41b2-8257-e3d7261be7b4

Commits

Reviewing files that changed from the base of the PR and between b5da7a5 and 7f0f088.

Files selected for processing: 2

• apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatSession/components/Transcript/components/ApprovalRow/components/QuestionForm/QuestionForm.test.tsx
• apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatSession/components/Transcript/components/ApprovalRow/components/QuestionForm/components/QuestionPage/QuestionPage.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

[PresentJay]

Could a maintainer please approve the pending GitHub Actions CI runs for this PR and its seven companion PRs (#9658, #9659, #9662, #9664, #9665, #9666, and #9667)?

All eight CI runs are awaiting approval. The combined macOS verification passed all 30 test tasks locally. Thank you for helping us verify each change in upstream CI.