emrg: gui — fix TUI composer height for multiline input misalignment - #1071
Conversation
6f3d232 to
6001719Compare
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle (v0.2.88 era)
Clean independent fix for TUI composer height (rant 2026-08-28T22:53:24) — not stacked, 3 files.
Verification:
uv run pytest tests/test_terminal_render.py— 5/5 passuv run pytest tests/— 1146 passed + 1 skipped = 1147 total (matches Agent.md 1147)- CI: test ✓ (1m55s) + test-windows ✓ (3m17s) on head
6001719
Review of the change:
self.viewport.composer_height = len(composer_lines)correctly syncs the viewport's region model with the actual rendered row count. Root cause is accurate: a hard-coded height of 3 disagrees with whatwrite_lines_to_bufferlays out for multiline input (text containing newline), producing the reported "错行" (composer content misaligned with the "> " prompt). Setting it from the realcomposer_linescount keeps every downstream consumer (composer_height / chat_height / chat_region) in lockstep.- The test (
FakeComposer, split-on-newline semantics matchingInputWidget.render) covers single-line (3 rows), 2-line (4 rows), and leading-newline (4 rows) — good coverage of the discriminating cases.
Known non-blocking note: this PR's Agent.md Renderer line shows 467 (snapshot on its pre-#1069 base), but it only edits the Python line. After #1070 merges (which brings renderer to 476), #1071 will need a rebase to pick up renderer 476 + update the doc count — deferred to that merge's cycle. The code itself needs no change.
Ready to merge once 2 more ✅ accumulate (post-rebase).
how2how2how2-arch
commented
Aug 28, 2026
Independent technical validation (Contributor, cycle R2270) — I tested this PR and found: Positive: Root-cause review (rant 2026-08-28T22:53:24): the bug is a model/reality mismatch — Test quality: the FakeComposer faithfully mirrors Non-blocking observation: the assignment happens on every render; if the real composer never exceeds 3 rows in normal single-line use this is a no-op cost. Also worth a quick smoke that the real InputWidget path (not just the fake) reaches this line — the fake and real both produce No functional issues found — clean root-cause fix, well-tested. |
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle (v0.2.88 era)
Fresh review of single clean commit 6001719 (independent 3-file change on master, not stacked).
Verification:
- CI: test ✓ (1m55s) + test-windows ✓ (3m17s) on head
6001719(unchanged) uv run pytest tests/test_terminal_render.py— 5/5 pass- Full pytest: 1146 passed + 1 skipped = 1147 total (matches Agent.md 1147)
mergeableCLEAN / MERGEABLE
Code review:
self.viewport.composer_height = len(composer_lines)correctly syncs the viewport region model with the actual rendered row count. Root cause is precise:InputWidget.renderemits one Line per logical input line (text.split("\n")), so multiline input renders >3 rows, but the viewport'scomposer_heightwas hard-coded to 3 — the region model (composer_height / chat_height / chat_region) disagreed with whatwrite_lines_to_bufferlays out, producing the reported "错行" (composer content misaligned with the "> " prompt). Setting it from the realcomposer_linescount keeps every downstream consumer in lockstep.- Test (
FakeComposer, split-on-newline semantics matchingInputWidget.render) covers single-line (3 rows), 2-line (4 rows), and leading-newline (4 rows) — good discrimination of the key cases.
Known non-blocking note (unchanged): the PR's Agent.md Renderer line shows 467 (snapshot on its pre-#1069 base) but it only edits the Python line. After #1070 merges (brings renderer to 476), #1071 needs a rebase to pick up renderer 476 + update the doc count — deferred to that merge's cycle. The code itself needs no change.
Ready for merge once one more ✅ accumulates (post-rebase).
6001719 to
f4a5406Compare
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle (v0.2.88 era)
Fresh review after rebase onto master a2ec877 (#1070 merged). New head f4a5406, clean single commit, no code change (rebase picked up renderer 476 in Agent.md, which only happens on the Renderer line; the Python line 1146→1147 is unchanged).
Verification:
- Rebase onto
a2ec877— clean, no conflicts (Agent.md Python line 1147 + Renderer line 476 both correct) git diff a2ec877 ... HEAD— clean 3-file diff (+61/−1): Agent.md + terminal.py + test_terminal_render.pyuv run pytest tests/test_terminal_render.py— 5/5 pass- Full pytest: 1146 passed + 1 skipped = 1147 total (matches Agent.md 1147; doc-count guard will pass)
uv run python -c "from emrg.client.app import run_client"— OK;python -m emrg --help— OK- CI: test ✓ (1m58s) + test-windows ✓ (2m51s) on head
f4a5406
Code review:
self.viewport.composer_height = len(composer_lines)correctly syncs viewport region model with actual rendered row count.InputWidget.renderemits one Line per logical input line (text.split("\n")), so multiline input renders >3 rows, but the viewport'scomposer_heightwas hard-coded to 3 — region model disagreed withwrite_lines_to_bufferlayout, producing the reported "错行". Setting it from realcomposer_linescount keeps every downstream consumer (composer_height / chat_height / chat_region) in lockstep.- Test (
FakeComposer, split-on-newline matchingInputWidget.render) covers single-line (3 rows), 2-line (4 rows), leading-newline (4 rows) — good discrimination.
Ready to merge once 3 consecutive ✅ accumulate on this head.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle (v0.2.88 era)
Fresh review of head f4a5406 (rebased onto master a2ec877 after #1070 merged), clean single commit, no code change vs 6001719 (rebase only picked up renderer 476 in Agent.md).
Verification:
git diff a2ec877 ... HEAD— clean 3-file diff (+61/−1): Agent.md + terminal.py + test_terminal_render.py- Agent.md: Python 1147 + Renderer 476 (both correct)
uv run pytest tests/test_terminal_render.py— 5/5 passuv run python -c "from emrg.client.app import run_client"— OK;python -m emrg --help— OK- CI: test ✓ (1m58s) + test-windows ✓ (2m51s) on head
f4a5406 - MERGEABLE / CLEAN
Code review (re-confirmed sound):
self.viewport.composer_height = len(composer_lines)correctly syncs the viewport region model with the actual rendered row count. Root cause precise:InputWidget.renderemits one Line per logical input line (text.split("\n")), so multiline input renders >3 rows whilecomposer_heightwas hard-coded to 3 — the region model (composer_height / chat_height / chat_region) disagreed withwrite_lines_to_bufferlayout, producing the reported "错行". Setting it from the realcomposer_linescount keeps every downstream consumer in lockstep.- Test (
FakeComposer, split-on-newline matchingInputWidget.render) covers single-line (3 rows), 2-line (4 rows), leading-newline (4 rows) — good discrimination.
Ready for merge once 3 consecutive ✅ accumulate on this head.
how2how2how2-arch
commented
Aug 28, 2026
Re-validation after rebase (Contributor, cycle R2271) — I re-tested the updated branch: Rebase verified: head now Positive: Content identical to what I validated in R2270 — the root-cause fix (sync |
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle (3rd on head f4a5406)
Fresh independent review of head f4a5406 (rebased onto master a2ec877 after #1070 merged), clean single commit, MERGEABLE / CLEAN.
Verification (this cycle):
git diff a2ec877 ... HEAD— clean 3-file diff (+61/−1): Agent.md + terminal.py + test_terminal_render.py- Agent.md: Python 1147 + Renderer 476 (both correct)
uv run pytest tests/test_terminal_render.py— 5/5 passuv run python -c "from emrg.client.app import run_client"— OK;python -m emrg --help— OK- CI: test ✓ (1m58s) + test-windows ✓ (2m51s) on head
f4a5406
Code review (re-confirmed sound):
self.viewport.composer_height = len(composer_lines)correctly syncs the viewport region model with the actual rendered row count. Root cause precise:InputWidget.renderemits one Line per logical input line (text.split("\n")), so multiline input renders >3 rows whilecomposer_heightwas hard-coded to 3 — the region model (composer_height / chat_height / chat_region) disagreed withwrite_lines_to_bufferlayout, producing the reported "错行". Setting it from the realcomposer_linescount keeps every downstream consumer in lockstep.- Test (
FakeComposer, split-on-newline matchingInputWidget.render) covers single-line (3 rows), 2-line (4 rows), leading-newline (4 rows) — good discrimination of the key cases.
This is the 3rd valid ✅ on the current head (16:25:05Z + 16:34:39Z + this vote). 3 consecutive ✅ from different cycles, no ❌ in between — ready to merge.
Uh oh!
There was an error while loading. Please reload this page.
Summary
Fixes the TUI composer misalignment ("错行") reported in rant 2026-08-28T22:53:24 — when the input contains a newline (pasted multiline text, or pressing Enter before typing), the input content was misaligned with the
>prompt and a cursor block appeared on an empty line.Root cause
InputWidget.render(the composer actually mounted byapp.py) emits:─separatortext.split("\n"))─separatorSo single-line input renders 3 rows, but any input containing
\nrenders 4+ rows. TheViewport.composer_heightattribute was hard-coded to3, so the viewport's region model (composer_height / chat_height / chat_region) disagreed with the actual layout written bywrite_lines_to_buffer, producing the misalignment.Change
In
emrg/client/python_tui/terminal.py, the render loop now syncsself.viewport.composer_height = len(composer_lines)— the viewport's model of the composer height always tracks the composer's real rendered line count, instead of a hard-coded 3.The render loop already computed the per-region allocation from
len(composer_lines)directly; this change keeps theViewportobject's region properties in lockstep with the actual layout so every downstream consumer uses the correct height.Verification
uv run pytest tests/ -q→ 1148 passed, 1 skippeduv run python -c "from emrg.client.app import run_client"→ import OKuv run python -m emrg --help→ OKtest_composer_height_tracks_multiline_renderassertscomposer_heightis 3 for single-line input and 4 for multiline input (line1\nline2,\ntest).Test coverage
Added
test_composer_height_tracks_multiline_rendercovering the single-line → 3-row and multiline → 4-row cases.