Skip to content

fix(tui): split IME backspace text keypresses - #53993

Closed
0disoft wants to merge 2 commits into
NousResearch:mainfrom
0disoft:task/tui-vietnamese-ime-keypress-53982
Closed

fix(tui): split IME backspace text keypresses#53993
0disoft wants to merge 2 commits into
NousResearch:mainfrom
0disoft:task/tui-vietnamese-ime-keypress-53982

Conversation

@0disoft

@0disoft 0disoft commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a TUI keypress parser bug where an IME recomposition chunk such as \x7fô was parsed as one unnamed text key and then discarded by the composer. The parser now splits text tokens on embedded Backspace/DEL control bytes outside bracketed paste mode, preserving the control key and the following composed text.

Related Issue

Fixes #53982

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • Tests (adding or improving test coverage)

Changes Made

  • ui-tui/packages/hermes-ink/src/ink/parse-keypress.ts: split plain text tokens around \b and \x7f before passing each segment to parseKeypress().
  • ui-tui/packages/hermes-ink/src/ink/parse-keypress.test.ts: added regressions for \x7fô, \x7fôi, and embedded backspace text chunks.

How to Test

  1. Run npm run test --workspace ui-tui -- packages/hermes-ink/src/ink/parse-keypress.test.ts.
  2. Run npm run test --workspace ui-tui -- packages/hermes-ink/src/ink/parse-keypress.test.ts src/__tests__/textInputBurstInput.test.ts src/__tests__/textInputFastEcho.test.ts.
  3. Run npm run typecheck --workspace ui-tui.
  4. Run npm run lint --workspace ui-tui -- packages/hermes-ink/src/ink/parse-keypress.ts packages/hermes-ink/src/ink/parse-keypress.test.ts.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix
  • I've run pytest tests/ -q and all tests pass; N/A, this is a TUI TypeScript parser change, so targeted Vitest/typecheck/lint were run instead
  • I've added tests for my changes
  • I've tested on my platform: Windows 11, targeted parser/composer tests

Documentation & Housekeeping

  • I've updated relevant documentation — N/A, no docs change needed
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact — parser-only TUI change, covered by unit tests
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

Validation run locally:

npm run test --workspace ui-tui -- packages/hermes-ink/src/ink/parse-keypress.test.ts
Test Files  1 passed (1)
Tests  17 passed (17)

npm run test --workspace ui-tui -- packages/hermes-ink/src/ink/parse-keypress.test.ts src/__tests__/textInputBurstInput.test.ts src/__tests__/textInputFastEcho.test.ts
Test Files  3 passed (3)
Tests  55 passed (55)

npm run typecheck --workspace ui-tui
passed

npm run lint --workspace ui-tui -- packages/hermes-ink/src/ink/parse-keypress.ts packages/hermes-ink/src/ink/parse-keypress.test.ts
0 errors; 5 existing warnings in unrelated useSessionLifecycle files

@alt-glitch alt-glitch added type/bug Something isn't working comp/tui Terminal UI (ui-tui/ + tui_gateway/) platform/windows Native Windows-specific behavior or breakage P3 Low — cosmetic, nice to have labels Jun 28, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Approved

Focused fix for IME (Input Method Editor) backspace handling in the TUI keypress parser. When an IME composes text with an embedded backspace control byte, the previous code would merge them into a single keystroke. This PR correctly splits text tokens on \x7f and \b boundaries while keeping surrounding composed characters intact.

  • 2 files, 58 additions, 1 deletion
  • New parseTextKeypresses function with clear separation of concerns
  • 3 dedicated test cases covering: IME backspace + composed char, trailing IME text after backspace, embedded backspace splitting
  • Tests verify correct key name and raw content for each split segment
  • No security concerns; changes confined to the keypress parser

Reviewed by Hermes Agent

@harjothkhara

Copy link
Copy Markdown
Contributor

Recommendation: please add one regression that drives the split keypress through the TextInput/useInput boundary, not only parseMultipleKeypresses.

The parser change looks right for the specific �ô token: after building @hermes/ink locally, npm run test --workspace ui-tui -- packages/hermes-ink/src/ink/parse-keypress.test.ts src/__tests__/textInputBurstInput.test.ts src/__tests__/textInputFastEcho.test.ts passes (55 tests), and git diff --check origin/main...HEAD is clean. The remaining risk is the one called out in #53982: the reporter says a parser-only patch did not fully resolve real TUI typing. In production, App.processInput() batches the parsed keys through processKeysInBatch, then TextInput applies backspace/printable insertion with its own PRINTABLE, burst batching, selection, and fast-backspace guards. A test that starts from an input like parseMultipleKeypresses(INITIAL_STATE, "�ô") and verifies the composer value after the resulting backspace + ô events would prove the fix survives that boundary instead of only proving the tokenizer emits two objects.

@0disoft

0disoft commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

Added the requested boundary regression in ui-tui/src/__tests__/textInputImeBoundary.test.tsx.

It mounts the real TextInput, sends the recomposition chunk \x7fô through the Ink stdin/useInput path, and asserts the composer value becomes ô. Re-ran the relevant checks:

npm run test --workspace ui-tui -- packages/hermes-ink/src/ink/parse-keypress.test.ts src/__tests__/textInputImeBoundary.test.tsx src/__tests__/textInputBurstInput.test.ts src/__tests__/textInputFastEcho.test.ts
# 4 files passed, 56 tests passed

npm run typecheck --workspace ui-tui
# passed

npm run lint --workspace ui-tui -- packages/hermes-ink/src/ink/parse-keypress.ts packages/hermes-ink/src/ink/parse-keypress.test.ts src/__tests__/textInputImeBoundary.test.tsx
# 0 errors; existing unrelated useSessionLifecycle warnings only

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tui Terminal UI (ui-tui/ + tui_gateway/) P3 Low — cosmetic, nice to have platform/windows Native Windows-specific behavior or breakage type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: TUI drops Vietnamese tone marks — IME backspace+char batched in one stdin read is silently discarded

4 participants