Skip to content

Fix the run_tui name collision that broke luminary map --tui (and CI mypy) - #41

Merged
rossry merged 1 commit into
mainfrom
fix/tui-records-name-collision
Aug 28, 2026
Merged

rossry merged 1 commit into
mainfrom
fix/tui-records-name-collision

Conversation

@rossry

@rossry rossry commented Aug 28, 2026

Copy link
Copy Markdown
Owner

What

luminary/mapping/store.py -> records.py (af9c574) also renamed run_tui's parameter to saved — which was already the local holding termios.tcgetattr(fd):

def run_tui(core: SessionCore, saved: MappingStore, fps: float) -> None:
    ...
    saved = termios.tcgetattr(fd)     # the records are gone from here on
    ...
    saved.save_state(core.state, core.plan)   # AttributeError: 'list' object ...

So luminary map --tui raised before the operator's first keystroke, on the resume refresh. mypy caught it; CI has been red since.

  • parameter is now records (it is a MappingStore, from records.py)
  • the terminal flags are tty_attrs

Why it got through

Every existing test monkeypatches run_tui away, so nothing had ever executed its body — the loop was covered only by parse_keys unit tests either side of it.

This adds one that runs the real loop over a pty and asserts both halves of the collision: the resume refresh reaches the records, and cbreak is undone on exit. Two traps if it ever needs editing, both documented inline — tty.setcbreak flushes pending input (TCSAFLUSH), so a keystroke written before the loop starts is discarded; and in canonical mode an unterminated one is never readable at all. A small typist thread presses q until the loop takes it.

Checks

  • CI mypy target: Success: no issues found in 56 source files (was 2 errors)
  • black --check: clean
  • full suite: 439 passed

Not covered: the fix is verified by test and by types, but I have not run luminary map --tui against the sphere.

The store rename left `run_tui(core, saved: MappingStore, ...)` sharing a
name with `saved = termios.tcgetattr(fd)` four lines below. The tty flags
won, so the resume refresh called `save_state` on a list of terminal
attributes: `luminary map --tui` raised AttributeError before the
operator's first keystroke. mypy was the only thing that saw it, and CI
had been red since.

Parameter is now `records` (it is a MappingStore, from records.py); the
flags are `tty_attrs`.

The reason a rename could break the loop unnoticed is that every existing
test monkeypatches `run_tui` away, so nothing had ever run its body. Added
one that does, over a pty: it asserts the resume refresh reaches the
records and that cbreak is undone on the way out. Two traps worth knowing
if it ever needs editing -- `tty.setcbreak` flushes pending input
(TCSAFLUSH), so a keystroke written before the loop starts is discarded,
and in canonical mode an unterminated one is never readable at all. A
small typist thread presses q until the loop takes it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FbzmvdzTNSUxDUjVraRZMx
@rossry
rossry merged commit 6bc144e into main Aug 28, 2026
1 check 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.

1 participant