Skip to content

feat(debugger-tui): B.7 — scratch-eval pane (LAST Phase B milestone of #691)#749

Merged
jsavin merged 2 commits into
developfrom
worktree-phase-b7-scratch-eval
Jun 7, 2026
Merged

feat(debugger-tui): B.7 — scratch-eval pane (LAST Phase B milestone of #691)#749
jsavin merged 2 commits into
developfrom
worktree-phase-b7-scratch-eval

Conversation

@jsavin

@jsavin jsavin commented Jun 7, 2026

Copy link
Copy Markdown
Owner

Summary

Implements Milestone B.7 of planning/phase_b/EXECUTION_PLAN.md — the scratch-eval pane. This is the last Phase B milestone. Builds on B.6 (#747, merged at 0d59f774c).

What this PR proves: the user can evaluate UserTalk expressions during a debug suspension without leaving the TUI. The expected workflow: hit a breakpoint, press : to activate the eval pane, type an expression (local(x = 5); return x + 10), press Enter, see the result in the history. Closes the "I want to poke at variables while suspended" UX gap that motivated B.7 in Phase B planning (option B of the earlier scoping decision).

With B.7 merged, Phase B is complete. Issue #691 closes.

What ships

  • frontier-cli/debugger_tui.c: activation handler, eval input mode, Backspace/Escape/Enter handling, script/eval outbound dispatch via tui_json_escape, response parser (result/error fields), eval history ring buffer rendering in footer, auto-hide on RUNNING transition
  • frontier-cli/debugger_tui_internal.hEVAL_HISTORY_MAX = 16, EVAL_INPUT_BUF_SIZE = 256, new state fields (eval_pane_active, eval_input_buf, eval_input_cursor, eval_history ring + indices, eval_next_id)
  • tests/debugger_tui_tests.c — 15 new behavioral tests covering: : activation only when SUSPENDED, character input, Backspace, Escape preserves buffer, Enter dispatches script/eval with JSON-escaped expression, response parsing for result vs error, history ring buffer wrap, RUNNING-transition auto-hide

Diff: 3 files, +1006/-2.

script/eval wire format (verified)

Sub-agent quoted directly from op_handler.c:931:

if (strcmp(op, "script/eval") == 0) {

Dispatched as {"op":"script/eval","id":N,"params":{"expression":"<tui_json_escape(input)>"}} — JSON-escape applied to the user's expression before interpolation, following the B.4 pattern.

Plan deviation: A.7 cursor "borrow-restore" pattern for footer_win

The A.7 boxen_window_set_cursor API gates on cursor_owner_allowed(win) which checks win == g_focused_window. The footer window has no on_input callback — it can't hold focus for key routing.

Resolution: borrow-restore pattern. boxen_window_focus(footer_win) immediately before cursor calls, boxen_window_focus(script_win) immediately after. Key routing stays on script_win throughout; on_input checks eval_pane_active to route keys to the eval handlers. This is not a workaround — it's the correct design for a display-only window that needs temporary cursor ownership. The A.7 contract is honored (focus gate respected) while key routing remains in the script pane.

Alternative considered: give footer_win its own on_input callback. Rejected because it would split key dispatch across two windows and complicate the SUSPENDED/RUNNING state machine. Borrow-restore keeps the single-callback model.

Plan minor: single-row footer constraint

The B.0/B.6 layout reserves only one footer row. With the eval pane active, the most-recent history entry renders inline alongside the input: "[N] last_result | : <input>". The full history ring buffer is maintained in state; older entries are accessible via the eval_history array (would need a multi-row footer to display, which is Phase C scope).

Test plan

  • make build — clean
  • ./tools/run_headless_tests.sh748/748 pass (was 733; +15 new B.7 tests)
  • cd tests && make test-integration — 2186 pass / 21 baseline failures (failure names character-for-character identical; verified independently against /tmp/integ-failures-bf51d18.txt)
  • ./tests/debugger_tui_test.sh9/9 pass (no regression on B.6's shell test framework)

TDD red/green

RED: test_colon_key_activates_eval_pane failed at assert(g_state.eval_pane_active == true) before implementation.
GREEN: 80/80 in debugger_tui_tests after implementation.

Lessons-applied proactively (from B.0-B.6 review history)

  • tui_json_escape applied to user expression before JSON interpolation (per B.4 P1 security)
  • Eval history strings heap-allocated with calloc, capped at EVAL_HISTORY_MAX, freed on ring-buffer wrap (per B.1 P1-C)
  • : activation gated on debug_state == TUI_DEBUG_SUSPENDED (per B.3 / B.4 lesson)
  • Auto-hide on RUNNING transition mirrors B.4 condition modal lifecycle
  • A.7 cursor API directly (no reverse-video workaround)
  • New state fields cleared in state_init, freed in state_teardown
  • No new file-static globals (per PR #741 follow-up: P2 polish from B.3 /gate (3 items) #742); per-session counter via eval_next_id field
  • No log_warn in cap paths (already mitigated in B.6 stub; pattern continues)

What's NOT in this PR (and why)

  • Multi-row footer for full history display — Phase C scope (planning/phase_c/OVERVIEW.md covers REPL-in-boxen which subsumes this)
  • Full readline-like editing (history nav, completion) — Phase C scope
  • WS-attached eval — permanently deferred (UAF risk per handoff doc)

With B.7 merged, Phase B is COMPLETE

Phase B closes the original #691 scope:

File>Open and all other menu-handler debug scenarios are now usable end-to-end via frontier-cli --debug-tui. Issue #691 closes when this lands.

Phase C (planning/phase_c/OVERVIEW.md) tracks the boxen-native REPL — the natural next step where B.7's scratch-eval pane becomes the seed of the full REPL input line.

🤖 Generated with Claude Code

jsavin and others added 2 commits June 7, 2026 06:08
Implements the last Phase B milestone: a vi-style scratch-eval pane
that lets the user evaluate UserTalk expressions while the debugger
is suspended.

What ships:
- ':' activates single-line expression input in the footer (SUSPENDED only)
- Escape dismisses without clearing the buffer (preserved for re-edit)
- Enter submits via script/eval; response appended to history ring buffer
- Output buffer (EVAL_HISTORY_MAX=16) rendered in footer above input line
- Auto-hide on RUNNING transition (tui_do_continue + tui_do_step)
- A.7 cursor API for text-input cursor position/visibility:
  boxen_window_focus borrow-restore pattern satisfies cursor_owner_allowed
  gate without redirecting key routing away from script_win's on_input
- tui_json_escape applied to expression before JSON interpolation (B.4 P1)
- Heap-allocated history strings; free on ring-buffer wrap (no leak)

Wire format (op_handler.c:931):
  {"op":"script/eval","id":N,"params":{"expression":"<escaped>"}}

Tests: 15 new unit tests (748 total, +15 from B.6 baseline of 733).
All four verification gates green:
  build:       clean
  unit:        748/748
  integration: 2186 pass / 21 fail (diff against baseline: 0 new failures)
  shell:       9/9 (debugger_tui_test.sh)

Closes #691 (bare-linenoise REPL debugging deferred to Phase C per
planning/phase_c/OVERVIEW.md).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…espace + cursor borrow

Round-1 /gate fix loop on PR #749. One P0 + 2 P1s from bar-raiser.
Security APPROVED, concurrency APPROVED.

1. P0-1 (bar-raiser): script/eval response was parsed against a
   fabricated wire shape, not the actual send_eval_success output.
   send_eval_success in op_handler.c emits result as an OBJECT with
   value+type keys, not a bare string. Production responses silently
   dropped at Case 2. The masking regression test was rewritten to
   use the actual wire shape; Case 2 now discriminates eval responses
   by id match + value/type keys before falling through to
   lines/frames/locals.

2. P1-1 (bar-raiser): ID namespace was split between tui_req_id and
   eval_next_id (both starting at 1). Merged into single tui_req_id
   counter; eval_next_id field removed. Response matcher uses new
   s->eval_last_dispatched_id field set at submit time. Single
   namespace eliminates future op-collision risk.

3. P1-2 (bar-raiser): Per-keystroke cursor updates were silently
   dropped because they happened with script_win focused (A.7 gate
   requires the window owning the cursor to be focused). Applied the
   borrow-restore pattern (boxen_window_focus footer, set cursor,
   boxen_window_focus script_win) at character/Backspace handlers.
   New regression test asserts cursor column tracks input length
   across multiple keystrokes and a Backspace.

P2 items deferred to follow-up issue per /auto Phase 7.

Verified:
- make build: clean
- ./tools/run_headless_tests.sh: 749/749 pass
- cd tests && make test-integration: 2186 pass / 21 baseline failures
  (failure names character-for-character identical to baseline)
- ./tests/debugger_tui_test.sh: 9/9 pass

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jsavin

jsavin commented Jun 7, 2026

Copy link
Copy Markdown
Owner Author

/gate Round 2 — Convergence (Phase B closes here)

Verdict: PASS — all reviewers APPROVE, 0 P0/P1 remaining.

Reviewer Round 1 Round 2
bar-raiser 1 P0 + 2 P1 APPROVE
security APPROVE APPROVE
concurrency APPROVE APPROVE

Round-1 P0 + P1s addressed in commit e54468c3a

  1. P0-1: script/eval response shape mismatch — production responses use object {value, type}, not bare string. Code expected wrong shape. Test masked the bug. Fixed Case 2 discriminator to match against actual send_eval_success wire shape from op_handler.c:251-268 (verified by sub-agent direct quote). Rewrote regression test to use the real shape: {"id":N,"result":{"value":"2","type":"number"},"success":true}.

  2. P1-1: ID namespace split — removed eval_next_id; merged into single tui_req_id counter; added eval_last_dispatched_id state field for response matching. Single namespace eliminates future op-collision risk.

  3. P1-2: cursor column desync — per-keystroke cursor updates were silently dropped because A.7 gate requires the cursor-owning window to be focused. Applied borrow-restore pattern at character/Backspace/Enter/Escape handlers. New regression test test_eval_cursor_column_tracks_input asserts cursor column tracks input length across keystrokes (would catch a dropped set_cursor call).

Round-2 P3 nit (deferred)

  • eval_last_dispatched_id is never cleared after a response is consumed. Stale id persists until next eval. With 0x7FFF tui_req_id wrap and next eval immediately overwriting, collision risk is negligible. Worth a one-line clear for hygiene; non-blocking.

Tests at HEAD e54468c3a

Phase B series — final P0/P1 counts

Milestone PR Round 1 P0/P1 Total /gate rounds
B.0 #734 0 P0, 4 P1 2
B.1 #737 0 P0, 5 P1 2
B.2 #739 0 P0, 1 P1 2
B.3 #741 0 P0, 0 P1 1
B.4 #743 0 P0, 2 P1 2
B.5 #745 2 P0, 3 P1 2
B.6 #747 0 P0, 0 P1 1
B.7 #749 1 P0, 2 P1 2

The class of bug that surfaced in B.5 and B.7 round 1 was the same shape: unit tests fitted to assumed wire formats rather than verified against actual handler source. B.6's shell test (#736) closed half the gap; the other half is the PR-author-side discipline of quoting the actual op_handler.c emit lines (which the B.7 sub-agent did the second time around). Both lessons codified in #748 follow-up.

Phase B is complete

Issue #691 closes when this lands. The TUI debugger is feature-complete for the original scope:

  • B.0: skeleton + boxen integration
  • B.1: script pane + debug/getSource
  • B.2: stack + locals (innermost frame)
  • B.3: F5/F10/F11/Shift-F11 step/continue
  • B.4: F9 breakpoint toggle + Shift-F9 condition modal
  • B.5: cmd-double-click identifier resolution + watchpoint modal
  • B.6: lazy-attach wiring + drain-before-free
  • B.7: : scratch-eval pane with ring history

Phase C (planning/phase_c/OVERVIEW.md) tracks the boxen-native REPL where B.7's scratch pane becomes the seed of the full REPL input line.

🤖 Generated with Claude Code /gate

@jsavin jsavin merged commit 0e25cc7 into develop Jun 7, 2026
1 check passed
@jsavin jsavin deleted the worktree-phase-b7-scratch-eval branch June 7, 2026 16:32
@jsavin

jsavin commented Jun 7, 2026

Copy link
Copy Markdown
Owner Author

/auto Summary (Phase B closing PR)

What went well: B.7 ships the last user-facing feature of Phase B: a working scratch-eval pane the user can use during debug suspension. Two-round convergence; round 2 found 0 P0/P1. The borrow-restore cursor pattern (A.7 focus gate workaround for footer text input) is a clean architectural addition that future B.x-style display-only-window-with-cursor patterns can reuse. Sub-agent quoted op_handler.c:251-268 directly for the wire format this time after the round-1 review fix — finally the response-side verification habit kicked in.

What could improve: P0-1 (response shape mismatch) was the SAME class of bug as B.5's two P0s — unit test fitted to the assumed wire format rather than verified against actual handler source. Three repeats across the run (B.5 watchpoint dispatch, B.5 cmd-click scroll math, B.7 eval response). The B.6 shell test (#736) closes about half the gap — boundary-crossing real-binary execution. The other half is PR-author-side discipline: quote the actual op_handler.c emit lines AND parse lines, not just the dispatch entry. The B.7 sub-agent did this correctly the second time around; should become standard practice for any wire-format-touching work going forward.

Recommendations: For Phase C (boxen-native REPL per planning/phase_c/OVERVIEW.md), the same lessons that compounded across B.0-B.7 apply: caps before allocation, calloc not malloc, tui_json_escape on user input, A.7 cursor with borrow-restore pattern, no log_warn in hot paths until proven NULL-safe, heap state with drain-before-free teardown. B.7's scratch-eval pane is structurally the seed of the Phase C REPL input line — the borrow-restore pattern transfers directly, the eval history ring buffer becomes the REPL output buffer, and the : activation gate goes away because the REPL pane is always active. Phase C is now well-founded.

Filed #750 with the 5 remaining B.7 polish items. The full Phase B-run summary lives in the ~/Desktop/phase-b-running-notes.md doc for the autonomous-run wrap-up.

🤖 Generated with Claude Code /auto

jsavin added a commit that referenced this pull request Jun 7, 2026
Closes #750. Five small polish items from PR #749's deferred /gate
findings.

1. Clear eval_last_dispatched_id after consuming response (hygiene)
2. History entry tags show the dispatched expression instead of
   placeholder "[eval]" -- added eval_last_expr field, stashed at
   submit, rendered in history line
3. [N] label uses monotonic eval_display_counter; no longer clamps
   at EVAL_HISTORY_MAX after ring wrap
4. strdup OOM accounting drift: decrement eval_history_count + counter
   on failure so visible count reflects reality
5. New behavioral test: modal open + press : => eval pane NOT
   activated (locks in the safe-by-construction behavior)

Verified:
- make build: clean
- ./tools/run_headless_tests.sh: 753/753 pass (was 749; +4 polish tests)
- cd tests && make test-integration: 2186 pass / 21 baseline failures
- ./tests/debugger_tui_test.sh: 9/9 pass
- python3 tests/debugger_tui_pty_test.py: 4/4 pass

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
jsavin added a commit that referenced this pull request Jun 9, 2026
…plit

After shipping Phase B (B.0-B.8 via PRs #722, #748, #749, #751, #752, #756)
and interactive testing, the persistent-input-line shape from the original
OVERVIEW was found inadequate: --debug-tui as a separate mode has no way
for a typist to cause code to run from inside it, and the empty-attach UX
is unusable.

Replacement design (planning/phase_c/PROPOSAL_REVISED.md):

- Frontier REPL window (port of repl.c to boxen-native input + scrollback)
  with /edit and /debug slash commands taking optional @-prefixed paths
  resolved against current REPL navigation context
- Script editor windows opened on demand, one per script, multiple OK
- Edit mode: full source pane, Enter compiles+runs, Option-Enter compiles+
  debugs starting suspended, Cmd-S saves (semantics deferred -- see below)
- Debug mode: source pane goes read-only, follows currently-selected stack
  frame (step-into swaps source), stack/locals split appears on the right
- Esc ends debug + stays on currently-displayed script (preserves the
  spot-the-bug-here workflow); Q ends debug + returns to original script
- The 'o' keybind in the stack pane opens any frame's source in a separate
  independent editor window for side-by-side inspection
- Concurrent-edit detection via per-script timeModified comparison
- Multiple debug sessions coexist (PR #722 lazy-attach + drain-before-free)
  -- GIL still serializes UserTalk execution, only suspension state is
  truly parallel
- Phase B artifacts (tui_dispatch_debug_run, debug_get_script_source,
  scratch-eval pane, F-key state machine) carry forward; nothing wasted
- --debug-tui flag deprecated in C.0, removed in C.6

OVERVIEW.md retained for historical context with a header pointing to the
revised proposal.

Sequential milestones C.0 through C.6 with C.0 (REPL skeleton) as the
largest LOC and C.2-C.3 (debug split + step-into swap) as the design-
iteration-heaviest.
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