feat: render into a caller-supplied IO sink (with_terminal/app io kwarg) #39

Merged
s-celles merged 10 commits from feat/injectable-io into main 2026-07-26 15:36:36 -07:00
s-celles commented 2026-07-17 08:09:46 -07:00 (Migrated from github.com)

What

Adds an io keyword to with_terminal and app, letting a Tachikoma app
render into a caller-supplied IO sink — a socket, a pipe, an IOBuffer
instead of only a terminal this process is attached to.

sink = IOBuffer()
app(model; io = sink, tty_size = (rows = 24, cols = 80))

Why

Terminal is already IO-polymorphic — every write goes through t.io — but
with_terminal could only ever build that sink from a path, so the only
reachable sinks were /dev/tty, a tty path, or stdout. There was no way to
hand it a sink you already hold. That makes it awkward to drive a Tachikoma UI
from anything that isn't a local terminal: streaming frames to a WebSocket,
piping to another process, or capturing to a buffer for a test.

This is a small extension of the existing remote_tty machinery rather than a
new path: an injected sink is a remote terminal in every respect that matters —
it isn't the terminal this process owns — so it reuses that mode's handling
(no raw mode on the process's own stdin, none of the kitty/graphics probes that
would write to a terminal the frames aren't going to).

Two things an injected sink can't do that a tty can

  • It can't be probed for its size. So tty_size is required alongside io,
    and check_resize! must not probe: it was calling terminal_size() whenever
    remote_tty_path was nothing — which is the injected case — and under an
    app's own stdout capture that reaches a pipe and returns the 80×24 default,
    silently resizing every frame. A new external_size flag stops that.
  • It has no SIGWINCH. So a new set_size!(::Terminal, sz) lets the caller
    — a socket that just received a resize frame, say — declare the new size.

The caller's io is the caller's to close: a socket outlives any one app, so
with_terminal never closes a sink it didn't open.

Compatibility

Purely additive. The default path (no io) is unchanged. The pre-existing
13-field positional Terminal constructor is preserved for callers that predate
the new external_size field.

Tests

test/test_injectable_io.jl (new, wired into runtests.jl) covers the injected
path end to end, including a real Model driven through app() into an
IOBuffer with no terminal involved, plus the size-required guard, the
resize path, and the don't-close-the-caller's-sink guarantee. Full suite green
locally (5178 passed, 0 failures; +19 over baseline).


Developed with AI assistance.


Maintainer follow-ups (kahliburke)

  • leave_tui! branched on remote_tty_path, which an injected io leaves
    nothing, so teardown called set_raw_mode!(false) on a stdin that
    enter_tui! had deliberately not touched — dropping an embedding REPL out of
    raw mode on every app exit. Both paths now derive "not our terminal" from one
    _is_remote_terminal predicate so they cannot drift again.
  • An explicit input= now takes precedence over an INPUT_IO a host already
    installed, and that previous source is restored on exit. It was silently
    ignored, leaving the app taking no keys with nothing to explain why.
  • Restored demos/TachikomaDemos/Project.toml [sources] to ../..; running
    the test suite rewrites it to an absolute local path, which had been committed.
  • Added tools/io_sink_demo.jl, a manual harness (frames and input over a
    socket, set_size! over a second port, nc as the client). It covers the one
    thing no automated test can: set_raw_mode! is a no-op when stdin isn't a
    tty, so the teardown regression is invisible headless.
  • Merged main.
## What Adds an `io` keyword to `with_terminal` and `app`, letting a Tachikoma app render into a caller-supplied `IO` sink — a socket, a pipe, an `IOBuffer` — instead of only a terminal this process is attached to. ```julia sink = IOBuffer() app(model; io = sink, tty_size = (rows = 24, cols = 80)) ``` ## Why `Terminal` is already IO-polymorphic — every write goes through `t.io` — but `with_terminal` could only ever *build* that sink from a path, so the only reachable sinks were `/dev/tty`, a tty path, or `stdout`. There was no way to hand it a sink you already hold. That makes it awkward to drive a Tachikoma UI from anything that isn't a local terminal: streaming frames to a WebSocket, piping to another process, or capturing to a buffer for a test. This is a small extension of the existing `remote_tty` machinery rather than a new path: an injected sink is a remote terminal in every respect that matters — it isn't the terminal this process owns — so it reuses that mode's handling (no raw mode on the process's own stdin, none of the kitty/graphics probes that would write to a terminal the frames aren't going to). ## Two things an injected sink can't do that a tty can - **It can't be probed for its size.** So `tty_size` is required alongside `io`, and `check_resize!` must not probe: it was calling `terminal_size()` whenever `remote_tty_path` was `nothing` — which is the injected case — and under an app's own stdout capture that reaches a pipe and returns the 80×24 default, silently resizing every frame. A new `external_size` flag stops that. - **It has no SIGWINCH.** So a new `set_size!(::Terminal, sz)` lets the caller — a socket that just received a resize frame, say — declare the new size. The caller's `io` is the caller's to close: a socket outlives any one app, so `with_terminal` never closes a sink it didn't open. ## Compatibility Purely additive. The default path (no `io`) is unchanged. The pre-existing 13-field positional `Terminal` constructor is preserved for callers that predate the new `external_size` field. ## Tests `test/test_injectable_io.jl` (new, wired into `runtests.jl`) covers the injected path end to end, including a real `Model` driven through `app()` into an `IOBuffer` with no terminal involved, plus the size-required guard, the resize path, and the don't-close-the-caller's-sink guarantee. Full suite green locally (5178 passed, 0 failures; +19 over baseline). --- *Developed with AI assistance.* --- ### Maintainer follow-ups (kahliburke) - `leave_tui!` branched on `remote_tty_path`, which an injected `io` leaves `nothing`, so teardown called `set_raw_mode!(false)` on a stdin that `enter_tui!` had deliberately not touched — dropping an embedding REPL out of raw mode on every app exit. Both paths now derive "not our terminal" from one `_is_remote_terminal` predicate so they cannot drift again. - An explicit `input=` now takes precedence over an `INPUT_IO` a host already installed, and that previous source is restored on exit. It was silently ignored, leaving the app taking no keys with nothing to explain why. - Restored `demos/TachikomaDemos/Project.toml` `[sources]` to `../..`; running the test suite rewrites it to an absolute local path, which had been committed. - Added `tools/io_sink_demo.jl`, a manual harness (frames and input over a socket, `set_size!` over a second port, `nc` as the client). It covers the one thing no automated test can: `set_raw_mode!` is a no-op when stdin isn't a tty, so the teardown regression is invisible headless. - Merged `main`.
kahliburke (Migrated from github.com) reviewed 2026-07-17 14:40:50 -07:00
kahliburke (Migrated from github.com) left a comment

Review — render into a caller-supplied IO sink

Well-motivated, well-commented, and genuinely additive. Treating an injected sink as a remote terminal (reusing the remote_tty handling) is the right call, and the check_resize! fix addresses a real latent bug (probing a captured-stdout pipe returns the 80×24 default and silently resizes every frame). Two substantive issues before merge, plus a few smaller notes.

1. app(; io=...) still dups fd 0 — headless use throws (see inline)

app unconditionally runs the fd-0 dup when INPUT_IO[] is unset. Base.TTY(RawFD(saved_fd)) throws EINVAL when fd 0 isn't a tty — exactly the headless/socket case this feature targets. The new test only passes because it manually sets T.INPUT_IO[], papering over the wart the real caller hits. There's no public input kwarg on app. Suggest skipping the dup when io !== nothing and/or adding an input kwarg.

2. resize! doesn't trigger the clear its docstring promises (see inline)

draw! decides to emit CLEAR_SCREEN from check_resize!'s return, which is now hard-wired to false for external_size. resize!'s return value goes to the caller, not into draw!. So the next frame has resized == false: no CLEAR_SCREEN. resize_buf! blanks both buffers so grown regions redraw, but on a shrink stale glyphs outside the new bounds are never cleared from the receiver — unlike the tty path, which does emit the clear. Suggest a pending-clear flag resize! sets and draw! consumes (or have check_resize! return true once after an external resize), and fix the docstring's causal claim.

Smaller points

  • on_terminal is undocumented. It's added to app's signature but not the docstring, yet it's the only way a driver gets the Terminal to call resize! on. Worth a line.
  • Base.resize!(::Terminal, sz) semantics. Overloading resize! where the 2nd arg is a (rows, cols) NamedTuple rather than a length stretches the resize!(collection, n) contract. A named set_size! would read clearer; if kept, the docstring is good (modulo #2).
  • Test coverage. The resize test asserts t.size/frame area but never inspects emitted bytes for a clear, so it wouldn't catch #2.

Done well

  • Not closing the caller's sink (io === nothing && tty_io !== stdout) is correct and tested.
  • Backward-compat 13-field positional constructor preserved and documented.
  • _remote = tty_out !== nothing || io !== nothing cleanly unifies the two off-the-local-terminal modes.
  • Required-tty_size guard fails loudly at the call site rather than visually later.

Solid, mergeable direction — I'd want #1 fixed and #2 reconciled first. Both are contained changes.

Reviewed with Claude Code.

## Review — render into a caller-supplied IO sink Well-motivated, well-commented, and genuinely additive. Treating an injected sink as a remote terminal (reusing the `remote_tty` handling) is the right call, and the `check_resize!` fix addresses a real latent bug (probing a captured-stdout pipe returns the 80×24 default and silently resizes every frame). Two substantive issues before merge, plus a few smaller notes. ### 1. `app(; io=...)` still dups fd 0 — headless use throws (see inline) `app` unconditionally runs the fd-0 dup when `INPUT_IO[]` is unset. `Base.TTY(RawFD(saved_fd))` throws `EINVAL` when fd 0 isn't a tty — exactly the headless/socket case this feature targets. The new test only passes because it manually sets `T.INPUT_IO[]`, papering over the wart the real caller hits. There's no public `input` kwarg on `app`. Suggest skipping the dup when `io !== nothing` and/or adding an `input` kwarg. ### 2. `resize!` doesn't trigger the clear its docstring promises (see inline) `draw!` decides to emit `CLEAR_SCREEN` from `check_resize!`'s return, which is now hard-wired to `false` for `external_size`. `resize!`'s return value goes to the *caller*, not into `draw!`. So the next frame has `resized == false`: no `CLEAR_SCREEN`. `resize_buf!` blanks both buffers so grown regions redraw, but on a **shrink** stale glyphs outside the new bounds are never cleared from the receiver — unlike the tty path, which does emit the clear. Suggest a pending-clear flag `resize!` sets and `draw!` consumes (or have `check_resize!` return `true` once after an external resize), and fix the docstring's causal claim. ### Smaller points - **`on_terminal` is undocumented.** It's added to `app`'s signature but not the docstring, yet it's the only way a driver gets the `Terminal` to call `resize!` on. Worth a line. - **`Base.resize!(::Terminal, sz)` semantics.** Overloading `resize!` where the 2nd arg is a `(rows, cols)` NamedTuple rather than a length stretches the `resize!(collection, n)` contract. A named `set_size!` would read clearer; if kept, the docstring is good (modulo #2). - **Test coverage.** The resize test asserts `t.size`/frame area but never inspects emitted bytes for a clear, so it wouldn't catch #2. ### Done well - Not closing the caller's sink (`io === nothing && tty_io !== stdout`) is correct and tested. - Backward-compat 13-field positional constructor preserved and documented. - `_remote = tty_out !== nothing || io !== nothing` cleanly unifies the two off-the-local-terminal modes. - Required-`tty_size` guard fails loudly at the call site rather than visually later. Solid, mergeable direction — I'd want #1 fixed and #2 reconciled first. Both are contained changes. *Reviewed with Claude Code.*
kahliburke (Migrated from github.com) commented 2026-07-17 14:40:50 -07:00

Headless app(io=...) throws before it gets here. A few lines up (unchanged), when INPUT_IO[] is unset app does saved_fd = ccall(:dup, ...) then Base.TTY(RawFD(saved_fd)), which throws EINVAL when fd 0 isn't a tty — the exact headless/socket-server case io targets. The new test only works because it pre-sets T.INPUT_IO[], so the wart is hidden. Since there's no public input kwarg, a socket-driven caller has to poke a global to avoid the crash. Suggest skipping the fd-0 dup when io !== nothing, and/or adding an input kwarg so the driver supplies its input source cleanly.

**Headless `app(io=...)` throws before it gets here.** A few lines up (unchanged), when `INPUT_IO[]` is unset `app` does `saved_fd = ccall(:dup, ...)` then `Base.TTY(RawFD(saved_fd))`, which throws `EINVAL` when fd 0 isn't a tty — the exact headless/socket-server case `io` targets. The new test only works because it pre-sets `T.INPUT_IO[]`, so the wart is hidden. Since there's no public `input` kwarg, a socket-driven caller has to poke a global to avoid the crash. Suggest skipping the fd-0 dup when `io !== nothing`, and/or adding an `input` kwarg so the driver supplies its input source cleanly.
kahliburke (Migrated from github.com) commented 2026-07-17 14:40:50 -07:00

This causal claim doesn't hold. draw! emits CLEAR_SCREEN based on check_resize!'s return, not resize!'s — and check_resize! now hard-returns false for external_size. So after an out-of-band resize!, the next draw! has resized == false and emits no clear. resize_buf! blanks both buffers so grown regions redraw via the diff, but on a shrink stale glyphs outside the new bounds persist on the receiver — the tty resize path clears, this one doesn't. Suggest a pending-clear flag that resize! sets and draw!/check_resize! consumes (or return true once from check_resize! after an external resize), then this docstring becomes accurate.

**This causal claim doesn't hold.** `draw!` emits `CLEAR_SCREEN` based on `check_resize!`'s return, not `resize!`'s — and `check_resize!` now hard-returns `false` for `external_size`. So after an out-of-band `resize!`, the next `draw!` has `resized == false` and emits no clear. `resize_buf!` blanks both buffers so grown regions redraw via the diff, but on a **shrink** stale glyphs outside the new bounds persist on the receiver — the tty resize path clears, this one doesn't. Suggest a pending-clear flag that `resize!` sets and `draw!`/`check_resize!` consumes (or return `true` once from `check_resize!` after an external resize), then this docstring becomes accurate.
Sign in to join this conversation.
No description provided.