Skip to content

emrg: the TUI keeps a message's clock inside the terminal width - #2669

Merged
argszero merged 1 commit into
masterfrom
fix/tui-clock-inside-terminal-width
Nov 2, 2026
Merged

argszero merged 1 commit into
masterfrom
fix/tui-clock-inside-terminal-width

Conversation

@argszero

@argszero argszero commented Nov 2, 2026

Copy link
Copy Markdown
Owner

Closes #2668

The TUI showed no time on any message. The value was never missing — the daemon stamps the record, the frames carry the same string, and every row renderer appended it to the first line. It was appended after the content had already been wrapped to the full width, so it began past the last column and the terminal clipped it. Measured on the checkout before this change, at terminal width 60, a user row rendered 67 cells:

span cells
"> " role prefix 2
markdown content, padded to the render width 58
" 14:04" clock 7

An assistant row measured the same 67, and so did a replayed one. Every width lost exactly the clock's 7 cells, on every path, for every message.

What changed

emrg/client/python_tui/widgets/chat_row.py gains the two helpers the three renderers now share:

  • clock_width(clock) — the columns a clock needs, gap included. Every renderer subtracts it from the width it wraps to, so the clock's space is part of the line's budget instead of an addition to it.
  • place_clock(line, clock) — appends the clock ahead of the padding a markdown renderer left on the line, so the moment follows the text rather than sitting behind the pad.

UserMarkdown.render and StreamingMarkdown.render reserve that width before wrapping; all three renderers place the clock through the helper. A row with no readable moment reserves nothing and renders nothing, as before.

After the change, at widths 60/80/100/120, every rendered line is within the width and the clock is on the first line with the declared two-cell gap — on the live user path, the live assistant path, and the /resume / /rewind replay path.

Why the old guard was green

test_message_time_is_the_daemons.py asserted that "09:25" appeared in the row's joined span text. An overflowing tail satisfies that reading, which is why the suite stayed green while the host saw nothing. The new assertions are test_*_keeps_its_clock_inside_the_terminal, parametrized over the four widths, measuring the rendered line against the terminal it is painted into and driving the replay through the app's own _replay_rows / _paint_replay_rows rather than building rows in the test.

Mutation arms (both measured on this tree, both redden — 12 failures each)

arm result
clock width dropped from the wrap budget 12 failed
trailing padding left ahead of the clock 12 failed
restored (this PR) 39 passed

Verification

emrg/client/app.py imports; python -m emrg --help runs; full suite 6066 passed, 36 skipped with PYTHONPATH pinned to the checkout — without it the imports resolve to the installed tree under the host's install directory, which is not this change.

CI is pending on this head.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20261102-142057

Reviewed on head f2076c90, with emrg.__file__ printed from this checkout, and the guard measured in both directions rather than trusted.

The overflow was real, and the fix closes it. Content wrapped to the full width left the clock with no column to start in, so it began past the last column and the terminal clipped it. clock_width now comes out of the wrap budget before layout, and place_clock puts the moment ahead of the trailing padding the renderer leaves on the line. All three render paths — live user, live assistant, replay — go through those two helpers, and the replay arm drives the app's real _replay_rows / _paint_replay_rows rather than a fixture built in the test.

Arm A, the guard is not vacuous. Dropping clock_width from the user row's budget reddens 8 of the file's 39 — the user-row and replay cases at 60 / 80 / 100 / 120: 8 failed, 31 passed. Restored: 39 passed, and git status empty.

Arm B, one claim in the new test is unguarded — recorded, not a blocker. test_a_row_with_no_moment_still_fits_the_width_it_is_painted_into says such a row "must not reserve columns for one either", but its assertions measure only len(text) <= width and the absence of a clock. Measured: making clock_width return len(_CLOCK_GAP) + len(clock or "00:00") — reserving unconditionally — leaves all six of those parametrisations green (6 passed, 33 deselected). The behaviour is correct on this head, because clock_width is falsy exactly when the clock is; what is missing is a reading that would notice that changing. Issue #2668 asks that the clock be visible inside the width, and that half both holds and reddens, so I am not holding the PR for the other sentence.

CI: test 3m28s and test-windows 4m40s pass on f2076c90 (run 41398683389).

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20261102-143629

Second vote, cast on the landing tree rather than on diff(master, head): master has moved to a14727e9 (the #2667 merge), so this head no longer contains it and the plain diff shows that merge as a reversal this PR does not make.

Landing readings, taken this cycle.

  • check-merge-freshness.py 2669 → STALE (diverged, behind_by 1, base 0607f464). Measured, never refreshed: a push moves the head and voids the standing vote.
  • check-merge-plan-suite.py 2669 → final tree 0fd71c483e1a against a14727e9, 6066 passed, 37 skipped in 263.18s, rc 0.
  • check-merge-landing-diff.py 2669 → merging changes three paths and only this PR's own: emrg/client/python_tui/widgets/chat_row.py (+37 −1), markdown.py (+17 −9), tests/test_message_time_is_the_daemons.py (+109 −0).
  • check-merge-pairs.py → #2669 is the only open PR, so there is no pair to conflict; 0 blocked.

The guard, armed in both directions by me this cycle — not quoted from an earlier review. With PYTHONPATH pinned to this checkout and emrg.__file__ printed from it:

arm edit measured
baseline none 39 passed
A drop clock_width from the user row's wrap budget (avail = max(1, ctx.width - len(prefix)), mutated line printed first) 8 failed, 31 passed — the user-row and replay cases at 60 / 80 / 100 / 120
restored git checkout -- 39 passed, git status empty, HEAD f2076c90

Arm A failing at all four widths, on the replay path as well as the live one, is what makes the guard a reading rather than a shape: the moment stops being visible when its columns leave the budget, and the test says so.

One gap that stands unchanged, as reported on the previous review and not a blocker here. test_a_row_with_no_moment_still_fits_the_width_it_is_painted_into states that a clockless row "must not reserve columns for one either", and no assertion would notice if it did. That is a sentence about behaviour the PR gets right by construction (clock_width is falsy exactly when the clock is), not a defect in the shipped rendering; #2668 asks that the clock be visible inside the width, and that half both holds and reddens. It belongs in a follow-up that reads the reservation rather than the text.

CI: test 3m28s and test-windows 4m40s pass on f2076c90 (run 41398683389). The suite above is the merged result with today's master, which is the tree a merge would produce.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20261102-145718

Measured on the tree this merge would produce, because the head is STALE: check-merge-freshness.py answers #2669 STALE (head f2076c90, base 0607f464) — diverged, behind_by=1. Refreshing was not an option (a push moves the head and voids both standing votes), so this vote rests on the landing tree instead:

  • landing tree 0fd71c483e1a against master a14727e9 — check-merge-plan-suite.py 2669: 6066 passed, 37 skipped in 285.57s
  • check-merge-landing-diff.py 2669: the merge changes 3 paths (chat_row.py +37 −1, markdown.py +17 −9, tests/test_message_time_is_the_daemons.py +109 −0), and the two other paths diff(master, head) shows are the base's own later commits, read there as reversals this PR does not make. The landing change is the one reviewed here.
  • check-merge-pairs.py 2669 2671: 2 clean and healthy, 0 blocked — the two open PRs do not interfere.

The code. One seam, three call sites, and the reason written where it binds: clock_width() returns the gap plus the clock's own columns (0 when there is no clock), and every renderer subtracts it from the width it wraps to; place_clock() trims the renderer's trailing padding before appending, so the moment follows the text instead of sitting at the far edge — the padding was the second half of the defect and is fixed with the first. ChatRow, UserMarkdown and StreamingMarkdown all route through the two helpers, so the three paths the issue names share one formatter and one placement.

Armed in both directions on this PR's own tree (refs/pull/2669/head in a detached worktree, emrg.__file__ printed from it):

Arm Change Result
baseline as pushed 39 passed
A drop clock_width from the user row's wrap budget 8 failed, 31 passed — every width parametrisation of the user path
B make clock_width reserve unconditionally (len(clock or "00:00")) 39 passed — survivor

Arm B is the follow-up this vote does not claim: the new docstring's "0 when there is no clock" is stated and unguarded, so a regression that reserves clock columns on a clockless row would pass every test here. It is a follow-up for its own issue rather than a veto — the half #2668 actually asks for (the clock visible inside the width) both holds and reddens, and a clockless row outside the TUI's message path is not a user-visible state.

CI on this head is green on both legs (41398683389: test 3m28s, test-windows 4m40s), and the link to #2668 reads ok in both directions.

@argszero
argszero merged commit e9e822d into master Nov 2, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

emrg: the TUI renders a message's clock past the last column, so no time is visible

1 participant