● just ci green, line-mode fallback still works under pipe input. #40

Merged

navicore merged 3 commits from repl-slice-B-phase-1 into main 2026-06-04 13:47:58 +00:00

Conversation 1 Commits 3 Files changed 6 +954 -74

navicore commented 2026-06-04 13:05:51 +00:00

Owner

Copy link

Phase 1 of slice B is in:

• app.rs — App extended with prompt: String, cursor: usize, history_index, saved_prompt, mode: PromptMode {
  Editing | Streaming }. All state transitions
  (insert/backspace/move/recall/take_prompt/submit/advance/stop_streaming) are pure — no terminal, no ratatui. 8
  new unit tests cover edge cases: multibyte chars at cursor, backspace at column 0, history walk +
  in-progress-edit snapshot, submit-then-advance to Done, parse error stays in Editing, stop_streaming is silent.
• keys.rs — Action enum filled in; map_key(KeyEvent, PromptMode) -> Action. Mode-aware: in Streaming, ; advances
  and ./Esc/Enter commits; in Editing, ; is just a literal char (you need it for (a; b) disjunction goals).
  Ctrl-C quits in either mode. Esc in Editing is Ignore so it doesn't clash with Streaming's Esc=StopStreaming
  muscle memory.
• ui.rs — 3-region vertical layout: transcript (Min, scroll-tail), bindings (Length 5, border + 3 content
  lines), prompt (Length 1, real terminal cursor via Frame::set_cursor_position). Bindings panel shows "(no
  current solution)" when idle. Per-mode prompt prefix: ?- editing, ; or . > streaming.
• tui.rs — terminal lifecycle (raw mode + alternate screen + panic hook that restores terminal before delegating
  to the original hook), event loop (100ms poll, dispatch Action through App). commit_goal handles SWI-parity
  quit/halt, records history before submit (typo-recall), and pushes to the in-session recall buffer.
• main.rs — --no-tui flag added; TTY auto-detect via IsTerminal::is_terminal() picks tui::run vs the existing
  run_line_mode. Headless smoke tests that pipe stdin still hit line-mode unchanged.

Tests: 28 + 7 keys + 0 tui = 35 (12 → 35). Workspace just ci clean.

Phase 2 is vim-line — swap the editing-mode keymap and the prompt rendering for modal vim semantics.

Phase 1 of slice B is in: - app.rs — App extended with prompt: String, cursor: usize, history_index, saved_prompt, mode: PromptMode { Editing | Streaming }. All state transitions (insert/backspace/move/recall/take_prompt/submit/advance/stop_streaming) are pure — no terminal, no ratatui. 8 new unit tests cover edge cases: multibyte chars at cursor, backspace at column 0, history walk + in-progress-edit snapshot, submit-then-advance to Done, parse error stays in Editing, stop_streaming is silent. - keys.rs — Action enum filled in; map_key(KeyEvent, PromptMode) -> Action. Mode-aware: in Streaming, ; advances and ./Esc/Enter commits; in Editing, ; is just a literal char (you need it for (a; b) disjunction goals). Ctrl-C quits in either mode. Esc in Editing is Ignore so it doesn't clash with Streaming's Esc=StopStreaming muscle memory. - ui.rs — 3-region vertical layout: transcript (Min, scroll-tail), bindings (Length 5, border + 3 content lines), prompt (Length 1, real terminal cursor via Frame::set_cursor_position). Bindings panel shows "(no current solution)" when idle. Per-mode prompt prefix: ?- editing, ; or . > streaming. - tui.rs — terminal lifecycle (raw mode + alternate screen + panic hook that restores terminal before delegating to the original hook), event loop (100ms poll, dispatch Action through App). commit_goal handles SWI-parity quit/halt, records history before submit (typo-recall), and pushes to the in-session recall buffer. - main.rs — --no-tui flag added; TTY auto-detect via IsTerminal::is_terminal() picks tui::run vs the existing run_line_mode. Headless smoke tests that pipe stdin still hit line-mode unchanged. Tests: 28 + 7 keys + 0 tui = 35 (12 → 35). Workspace just ci clean. Phase 2 is vim-line — swap the editing-mode keymap and the prompt rendering for modal vim semantics.

navicore added 1 commit 2026-06-04 13:05:51 +00:00

● just ci green, line-mode fallback still works under pipe input. ... dbc0ceef1f

Phase 1 of slice B is in:
  - app.rs — App extended with prompt: String, cursor: usize, history_index, saved_prompt, mode: PromptMode {
  Editing | Streaming }. All state transitions
  (insert/backspace/move/recall/take_prompt/submit/advance/stop_streaming) are pure — no terminal, no ratatui. 8
  new unit tests cover edge cases: multibyte chars at cursor, backspace at column 0, history walk +
  in-progress-edit snapshot, submit-then-advance to Done, parse error stays in Editing, stop_streaming is silent.
  - keys.rs — Action enum filled in; map_key(KeyEvent, PromptMode) -> Action. Mode-aware: in Streaming, ; advances
  and ./Esc/Enter commits; in Editing, ; is just a literal char (you need it for (a; b) disjunction goals).
  Ctrl-C quits in either mode. Esc in Editing is Ignore so it doesn't clash with Streaming's Esc=StopStreaming
  muscle memory.
  - ui.rs — 3-region vertical layout: transcript (Min, scroll-tail), bindings (Length 5, border + 3 content
  lines), prompt (Length 1, real terminal cursor via Frame::set_cursor_position). Bindings panel shows "(no
  current solution)" when idle. Per-mode prompt prefix: ?-  editing, ; or . >  streaming.
  - tui.rs — terminal lifecycle (raw mode + alternate screen + panic hook that restores terminal before delegating
  to the original hook), event loop (100ms poll, dispatch Action through App). commit_goal handles SWI-parity
  quit/halt, records history before submit (typo-recall), and pushes to the in-session recall buffer.
  - main.rs — --no-tui flag added; TTY auto-detect via IsTerminal::is_terminal() picks tui::run vs the existing
  run_line_mode. Headless smoke tests that pipe stdin still hit line-mode unchanged.

  Tests: 28 + 7 keys + 0 tui = 35 (12 → 35). Workspace just ci clean.

  Phase 2 is vim-line — swap the editing-mode keymap and the prompt rendering for modal vim semantics.

navicore added 1 commit 2026-06-04 13:11:31 +00:00

no claude dc430298e9

navicore commented 2026-06-04 13:22:40 +00:00

Author

Owner

Copy link

Review

What it does: Phase 1 of slice B — the real TUI. Pure state transitions on App (prompt buffer/cursor/history-walk/mode), mode-aware key mapping, 3-panel ratatui layout, terminal lifecycle with a panic hook, and TTY auto-detect with --no-tui + line-mode fallback.

Verified locally (scratch copy of the branch): 28 + 7 = 35 tests pass, clippy clean, piped line-mode unchanged, and — driving the real binary under a pty — the full TUI path works: goal commit → X = a in transcript + bindings panel with the ; for next hint → ; advance → . stop → quit. exits cleanly, terminal restored. The architecture is right: App mutators are genuinely pure (the 8 new tests prove the multibyte cursor handling — the #36 lesson applied at the editing layer), the panic hook is the correct shape, ;-as-literal-while-Editing is the right call for disjunction goals, and record-before-submit carries the #38 typo-recall rationale forward with a comment.

🟡 Worth addressing

1. TTY detection only checks stdout (main.rs): echo "p(1)." | prlg-repl in a terminal launches the TUI and silently discards the piped input — demonstrated under a pty (alt-screen entered, goal never consulted; crossterm falls back to /dev/tty for keys so the keyboard still works, but the pipe is lost). io::stdin().is_terminal() && io::stdout().is_terminal() routes piped-stdin runs to line mode, which consumes them as intended.
2. KeyEventKind not filtered (tui.rs): Event::Key(key) dispatches without checking key.kind == KeyEventKind::Press. On Windows — and any kitty-protocol terminal — crossterm also emits Release/Repeat, so every keystroke inserts twice. One-line guard.
3. Stale bindings panel (ui.rs::latest_solution): the doc comment says it returns None "if the last outcome was Done/Error," but find_map skips over Done/Error/Goal entries and returns an older goal's solution. Repro: run a goal, commit after a solution, then submit a goal that parse-errors — the bindings panel still shows the previous goal's bindings as if current. Cleanest fix isn't transcript archaeology at render time: hold current_solution: Option<RenderedSolution> on App, set in advance, cleared in stop_streaming/Done/Error.
4. keys.rs comment/code drift, and two missing keys. The comments promise "Ctrl-D maps to Quit (empty prompt — caller checks)" and ":q typed and committed" — neither exists in map_key or commit_goal. Terminal users will hit Ctrl-D. Relatedly, crossterm delivers ^J as Ctrl-J rather than Enter (crossterm #371) — the pty harness proved input queued before raw-mode enables arrives as \n → Ctrl-J → silently swallowed. Mapping Ctrl-J → CommitGoal and Ctrl-D-on-empty → Quit closes both.
5. Transcript tail-scroll breaks once lines wrap: scroll is computed from logical line count, but the Paragraph has Wrap on — each wrapped line adds a visual row the calculation doesn't see, so the newest entries scroll out of view exactly when output gets long. Either drop wrap, or render only the last N entries that fit.
6. submit() doc comment overpromises: it claims "single-solution-zero-binding goals never need the ; prompt," but true. yields Solution first → enters Streaming and demands an extra keypress; only goals whose first step is Done/Error skip it. That's the #38 deferred-lookahead item — fine to keep deferring, but the comment should say what the code does.

🟢 Minor

• Cursor column = byte offset (ui.rs) misplaces the caret on multibyte input — acknowledged in the comment as phase-2/vim-line territory, fine. Same for long prompts walking off the right edge (no horizontal scroll).
• In-session recalled.push doesn't dedup consecutive repeats — ten p(1).s = ten up-arrows.
• enter_terminal: if EnterAlternateScreen fails, raw mode is left enabled (no rollback on the error path).
• Idle loop redraws at 10fps; harmless (ratatui diffs), but a longer poll when nothing is in flight would be kinder to laptops.

Verdict

Solid phase-1 TUI — clean state/render/lifecycle separation, honest tests, and it demonstrably works end-to-end under a pty. Items 1–4 are each a few lines and worth landing now (1 and 3 are user-visible wrong behavior, 2 bites every Windows user, 4 is comments promising keys that don't exist); 5–6 can ride with phase 2 if preferred.

---

Review by Claude Code

# Review **What it does:** Phase 1 of slice B — the real TUI. Pure state transitions on `App` (prompt buffer/cursor/history-walk/mode), mode-aware key mapping, 3-panel ratatui layout, terminal lifecycle with a panic hook, and TTY auto-detect with `--no-tui` + line-mode fallback. **Verified locally** (scratch copy of the branch): 28 + 7 = 35 tests pass, clippy clean, piped line-mode unchanged, and — driving the real binary under a pty — the full TUI path works: goal commit → `X = a` in transcript + bindings panel with the `; for next` hint → `;` advance → `.` stop → `quit.` exits cleanly, terminal restored. The architecture is right: `App` mutators are genuinely pure (the 8 new tests prove the multibyte cursor handling — the #36 lesson applied at the editing layer), the panic hook is the correct shape, `;`-as-literal-while-Editing is the right call for disjunction goals, and record-before-submit carries the #38 typo-recall rationale forward with a comment. ## 🟡 Worth addressing 1. **TTY detection only checks stdout** (`main.rs`): `echo "p(1)." | prlg-repl` in a terminal launches the TUI and **silently discards the piped input** — demonstrated under a pty (alt-screen entered, goal never consulted; crossterm falls back to `/dev/tty` for keys so the keyboard still works, but the pipe is lost). `io::stdin().is_terminal() && io::stdout().is_terminal()` routes piped-stdin runs to line mode, which consumes them as intended. 2. **`KeyEventKind` not filtered** (`tui.rs`): `Event::Key(key)` dispatches without checking `key.kind == KeyEventKind::Press`. On Windows — and any kitty-protocol terminal — crossterm also emits Release/Repeat, so every keystroke inserts twice. One-line guard. 3. **Stale bindings panel** (`ui.rs::latest_solution`): the doc comment says it returns `None` "if the last outcome was Done/Error," but `find_map` skips over `Done`/`Error`/`Goal` entries and returns an *older goal's* solution. Repro: run a goal, commit after a solution, then submit a goal that parse-errors — the bindings panel still shows the previous goal's bindings as if current. Cleanest fix isn't transcript archaeology at render time: hold `current_solution: Option<RenderedSolution>` on `App`, set in `advance`, cleared in `stop_streaming`/`Done`/`Error`. 4. **keys.rs comment/code drift, and two missing keys.** The comments promise "Ctrl-D maps to Quit (empty prompt — caller checks)" and "`:q` typed and committed" — neither exists in `map_key` or `commit_goal`. Terminal users *will* hit Ctrl-D. Relatedly, crossterm delivers `^J` as Ctrl-J rather than Enter (crossterm #371) — the pty harness proved input queued before raw-mode enables arrives as `\n` → Ctrl-J → silently swallowed. Mapping Ctrl-J → CommitGoal and Ctrl-D-on-empty → Quit closes both. 5. **Transcript tail-scroll breaks once lines wrap**: `scroll` is computed from logical line count, but the `Paragraph` has `Wrap` on — each wrapped line adds a visual row the calculation doesn't see, so the newest entries scroll out of view exactly when output gets long. Either drop wrap, or render only the last N entries that fit. 6. **`submit()` doc comment overpromises**: it claims "single-solution-zero-binding goals never need the `;` prompt," but `true.` yields `Solution` first → enters Streaming and demands an extra keypress; only goals whose *first* step is Done/Error skip it. That's the #38 deferred-lookahead item — fine to keep deferring, but the comment should say what the code does. ## 🟢 Minor - Cursor column = byte offset (`ui.rs`) misplaces the caret on multibyte input — acknowledged in the comment as phase-2/vim-line territory, fine. Same for long prompts walking off the right edge (no horizontal scroll). - In-session `recalled.push` doesn't dedup consecutive repeats — ten `p(1).`s = ten up-arrows. - `enter_terminal`: if `EnterAlternateScreen` fails, raw mode is left enabled (no rollback on the error path). - Idle loop redraws at 10fps; harmless (ratatui diffs), but a longer poll when nothing is in flight would be kinder to laptops. ## Verdict Solid phase-1 TUI — clean state/render/lifecycle separation, honest tests, and it demonstrably works end-to-end under a pty. Items 1–4 are each a few lines and worth landing now (1 and 3 are user-visible wrong behavior, 2 bites every Windows user, 4 is comments promising keys that don't exist); 5–6 can ride with phase 2 if preferred. --- *Review by Claude Code*

navicore referenced this pull request from a commit 2026-06-04 13:46:28 +00:00

● All seven PR #40 items addressed; just ci green.

navicore added 1 commit 2026-06-04 13:46:28 +00:00

● All seven PR #40 items addressed; just ci green. ... f7263d007e

- 🟡 1 main.rs now requires stdin().is_terminal() && stdout().is_terminal() — piped goals in a TTY no longer
  silently vanish into the alt-screen. Doc comment updated.
  - 🟡 2 tui.rs filters on key.kind == KeyEventKind::Press — no more double-insert on Windows/kitty terminals.
  - 🟡 3 App.current_solution: Option<RenderedSolution> added; advance sets it on Solution,
  stop_streaming/Done/Error/parse-error all clear it. ui::render_bindings reads app.current_solution directly.
  Stale-bindings-after-parse-error regression test (parse_error_after_success_clears_current_solution) catches it,
  plus two more for the Done/stop_streaming clears.
  - 🟡 4 map_key(event, mode, prompt_empty) — Ctrl-J → CommitGoal in Editing (crossterm-#371: queued \n arrives as
  Ctrl-J), Ctrl-D on empty prompt → Quit (SWI parity), non-empty Ctrl-D → Ignore. Two new tests. Action::Quit doc
  comment now describes what's actually wired.
  - 🟡 5 ui::render_transcript drops Wrap — scroll math now matches visual rows again. Long lines clip
  horizontally; phase-2 can revisit with a real line-window.
  - 🟡 6 App::submit doc rewritten to match what the code actually does — Solution always enters Streaming
  (including true.); only Done/Error skip the streaming detour.
  - 🟢→🟡 enter_terminal rolls back disable_raw_mode() if EnterAlternateScreen fails — no more wedged shell on
  partial init.

  Also caught by item 3's regression test: submit was reusing the prior App.active query, tripping the PR #39
  debug_assert on Arc strong-count. Fixed by clearing active and current_solution at the top of submit — the
  assert was doing its job.

  Tests: 35 (added 5 — three App, two keys).

navicore merged commit 34d837979f into main 2026-06-04 13:47:58 +00:00

navicore deleted branch repl-slice-B-phase-1 2026-06-04 13:47:58 +00:00

navicore referenced this pull request from a commit 2026-06-04 13:48:00 +00:00

Merge pull request '● just ci green, line-mode fallback still works under pipe input.' (#40) from repl-slice-B-phase-1 into main

navicore referenced this pull request 2026-06-04 14:22:23 +00:00

● just ci green; line-mode fallback unchanged (it never touches vim-line); 29 unit tests. #41

Commenting is not possible because the repository is archived.

Reviewers

No reviewers

Labels

Clear labels   

bug

Something isn't working

  

documentation

Improvements or additions to documentation

  

duplicate

This issue or pull request already exists

  

enhancement

New feature or request

  

good first issue

Good for newcomers

  

help wanted

Extra attention is needed

  

invalid

This doesn't seem right

  

question

Further information is requested

  

wontfix

This will not be worked on

No labels

bug

documentation

duplicate

enhancement

good first issue

help wanted

invalid

question

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

navicore/patch-prolog-v1!40

No description provided.