Skip to content

feat(repl): C.0.2 Tab completion for boxen REPL#769

Merged
jsavin merged 2 commits into
developfrom
worktree-boxen-repl-completion
Jun 10, 2026
Merged

feat(repl): C.0.2 Tab completion for boxen REPL#769
jsavin merged 2 commits into
developfrom
worktree-boxen-repl-completion

Conversation

@jsavin

@jsavin jsavin commented Jun 10, 2026

Copy link
Copy Markdown
Owner

Summary

Adds Tab-key completion to the boxen --debug-tui REPL: slash commands (/h<TAB>/help) and ODB paths. Single candidate replaces input inline; multiple candidates open a transient floating popup above the input bar. Linenoise REPL behavior unchanged.

This is milestone C.0.2 from planning/phase_c/REPL_SCHISM_EXECUTION_PLAN.md Section 2 — second of seven boxen-parity steps before the C.6 default flip.

Changes

Adapter refactor (repl_completion.h, repl.c)

  • New repl_completion.h exposes repl_complete_slash_command_path with a generic (add_fn, ctx) callback signature
  • repl.c's existing logic refactored to use the callback; a thin linenoise_add_adapter keeps the linenoise REPL byte-identical
  • repl_slash_commands_list[] exposed as extern const char *const

New completion popup module (boxen_completion_popup.{c,h})

  • Self-contained transient window: open, close, navigate, selected, count, is_open
  • Anchored above the input bar, ~40 cols wide, height clamped to fit screen
  • Selected row uses BOXEN_ATTR_REVERSE; truncates long candidates
  • Uses boxen_window_raise for z-order (NOT set_modal — see "Design notes" below)

State + key handling (boxen_repl.c, boxen_repl_internal.h)

  • New completion_hook function pointer on state (test seam — same pattern as slash_dispatch_hook / repl_eval_hook); production wires it to boxen_repl_real_completion
  • Tab key: 0 candidates → silent no-op; 1 → inline replace; N>1 → open popup
  • Popup-mode key routing in on_input: Tab/Down → navigate +1, Up → navigate -1, Enter → accept, Escape → dismiss, any other → close + fall through

Tests (tests/boxen_repl_tests.c)

  • test_tab_completes_single_slash_command — inline replace for single candidate
  • test_tab_opens_popup_with_multiple_candidates — popup opens, input_buf unchanged
  • test_tab_completes_odb_path_prefix — ODB-path completion behaves same as slash command
  • test_escape_dismisses_popup_without_accept — Escape closes popup, input_buf unchanged

Design notes

  • Adapter callback over fake-linenoiseCompletions struct: cleaner end-state, no fragile compatibility shim. Single line change at each caller in repl.c.
  • boxen_window_raise not set_modal: the initial plan called for set_modal, but boxen_dispatch_event routes all keys to the modal window — and since the popup has no input_fn, Escape was silently dropped. raise gives us z-order without hijacking key routing; the REPL input window keeps focus, and on_input routes keys based on s->completion_popup != NULL.
  • Completion hook seam: test binary doesn't link repl.c. Following the established hook pattern for slash_dispatch_hook / repl_eval_hook, the completion hook is injected at runtime — tests use synthetic candidates, production wires to boxen_repl_real_completion under #ifndef BOXEN_REPL_OMIT_MAIN.

Test plan

  • Unit: 784/784 (+4 new; was 780 after C.0.1)
  • Integration: 2186 pass / 21 baseline (character-for-character match)
  • make -C frontier-cli clean
  • Local /gate (running)
  • Manual (linenoise REPL — regression check): dist/frontier-cli, /h<TAB>/help, /jump sys<TAB> → candidate list
  • Manual (boxen REPL): dist/frontier-cli --debug-tui, /h<TAB> → inline /help; /<TAB> → popup above input; Tab/Up/Down navigate; Enter accepts; Escape dismisses; typing closes popup and inserts the char

Out of scope (deferred per execution plan)

  • Fuzzy matching, documentation preview pane, verb sig caching, Tab-cycling without popup, scroll-within-popup → all deferred (some to later C.0.x, some to follow-ups)

Refs: #691

🤖 Generated with Claude Code

Adds Tab-key completion to the boxen --debug-tui REPL: slash commands
(/h<TAB> -> /help) and ODB paths (system.<TAB> -> popup of candidates).
Single candidate replaces input inline; multiple candidates open a
transient floating popup above the input bar.

- New repl_completion.h header extracts the completion-callback adapter
  pattern from repl.c. Linenoise REPL keeps its current behavior;
  boxen REPL uses the same underlying logic via the new callback.
- New boxen_completion_popup.{c,h} module wraps the popup window
  lifecycle (raise for z-order above input bar, navigation).
  Uses boxen_window_raise instead of set_modal: modal would redirect
  key dispatch away from the input window (D3 nuance resolved).
- completion_hook seam on boxen_repl_state_t mirrors the existing
  slash_dispatch_hook / repl_eval_hook pattern so the test binary
  does not need to link the full repl.c surface.

Keys while popup open: Tab/Down navigate +1, Up navigate -1,
Enter accepts, Esc dismisses, any other key closes and falls through.

Refs: planning/phase_c/REPL_SCHISM_EXECUTION_PLAN.md Section 2
Issue: #691

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

jsavin commented Jun 10, 2026

Copy link
Copy Markdown
Owner Author

Local /gate Round 1 (pre-merge HEAD 50d44d2)

Verdict: PASS WITH NOTES — 0 P0, 0 P1, 6 P2 (all cosmetic doc/comment/contract cleanup).

Reviewer summary

Reviewer Verdict
bar-raiser APPROVED — clean refactor, set_modal→raise pivot correct, 5 P2 cosmetic
security CLEAR TO LAND — heap discipline solid, bounded copies throughout, 1 P2 contract clarification
concurrency PASS — no yield points in completion call chain; GIL held throughout enumeration

P2 items (all <10 min fixes, addressing in-worktree before merge)

  1. Popup height doc drift — header says min(count+2, 12), impl min(count, 10). Update header.
  2. Stale set_modal comment — file-header block contradicts inline pivot rationale at lines 137-142. Update file-header.
  3. Test mock macro — uses BOXEN_REPL_INPUT_MAX (1024) instead of BOXEN_COMPLETION_CANDIDATE_MAX (1024). Silent UB if either changes. Switch to canonical macro.
  4. Redundant set_user_databoxen_window_open(...,p) already sets it. Drop or annotate.
  5. Narrower-than-legacy non-slash completion — by design (C.0.2 scope). Document in function header so next maintainer knows.
  6. bc_add NULL contract — internal callers safe today; document completion MUST NOT be NULL in repl_completion.h.

Fixing all six before merge.

Address six P2 findings from local /gate round 1, all cosmetic:

- P2-1: popup height doc in header now matches impl (min(count, 10))
- P2-2: file-header design notes updated to reflect set_modal -> raise pivot
- P2-3: test mock signatures use BOXEN_COMPLETION_CANDIDATE_MAX
  (was BOXEN_REPL_INPUT_MAX -- both 1024 but semantically wrong)
- P2-4: drop redundant set_user_data call (boxen_window_open already
  wires user_data via its third argument)
- P2-5: document scope-reduction in boxen_repl_real_completion: C.0.2
  does slash + ODB path; richer non-slash deferred per execution plan
- P2-6: document non-NULL contract for completion in repl_completion.h
  (bc_add internal callers are trusted; no defensive guard needed)

No behavior change; tests unchanged at 784/784, integration baseline
character-for-character.

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

jsavin commented Jun 10, 2026

Copy link
Copy Markdown
Owner Author

Local /gate Round 2 (post-fix HEAD 0b56b0e)

Verdict: PASS — all six round-1 P2s addressed; spot-check only (delta is 23/-11 LOC, all docs/contract).

Round-1 P2 verification

Round-1 P2 Fix location Status
1: Popup height doc drift boxen_completion_popup.h:40 (changed to min(count, 10))
2: Stale set_modal file-header boxen_completion_popup.c:9-15 (rewritten for raise)
3: Test mock macro tests/boxen_repl_tests.c:652,657,667,679 (BOXEN_COMPLETION_CANDIDATE_MAX)
4: Redundant set_user_data boxen_completion_popup.c:131 (dropped)
5: Document narrower scope boxen_repl.c:1136-1152 (scope-note added)
6: bc_add NULL contract repl_completion.h:14-15 (contract documented)

Tests at new HEAD

  • Unit: 784/784
  • Integration: empty diff vs baseline /tmp/integ-failures-bf51d18.txt

Reviewer assessment

Round-1 bar-raiser explicitly stated "Ready to merge with two cosmetic doc/comment cleanups" (paraphrasing — actually 5 P2s). All P2s now addressed. Round-1 security ("Clear to land") and concurrency ("PASS") concerns also resolved.

No new code paths introduced; no new behavior to re-review. Going straight to merge once CI is green.

@jsavin jsavin merged commit b01a747 into develop Jun 10, 2026
1 check passed
@jsavin jsavin deleted the worktree-boxen-repl-completion branch June 10, 2026 07:35
@jsavin

jsavin commented Jun 10, 2026

Copy link
Copy Markdown
Owner Author

/auto Summary

What went well:

  • The plan agent's recommendation to use a hook-seam pattern (mirroring slash_dispatch_hook / repl_eval_hook) for completion_hook paid off — the test binary stays decoupled from the full repl.c surface, and the 4 behavioral tests landed cleanly.
  • The implementer caught a runtime issue during 2.B: boxen_window_set_modal(true) hijacks key routing to the modal, and since the popup intentionally has no input_fn, Escape was silently dropped. Pivoted to boxen_window_raise for z-order only — REPL input window keeps focus, all keys flow to on_input which inspects s->completion_popup. The plan called for set_modal; the implementer correctly diagnosed and pivoted, then documented the rationale inline. Good autonomous judgment.
  • Round-1 review surfaced 6 P2s all clustered around doc/comment/contract consistency — exactly the kind of polish that's cheap during the same PR and would otherwise rot. Folded all 6 into one cleanup commit.

What could improve:

  • The plan called out the C.0.2 non-slash completion as narrower than legacy ("slash commands and ODB paths"), but the function-header comment didn't initially say so. Bar-raiser caught it as P2-5. Worth adding to the plan template for future C.0.x: when scope is deliberately narrower than the comparable legacy code, the function header should say so.
  • The boxen_window_raise pivot decision wasn't captured anywhere planning-doc-side, only in inline code comments. If C.0.3 (palette modal) needs similar trade-offs, the lesson lives in this PR's commit history only.

Recommendations:

  • For future C.0.x: each milestone's plan section should include a "modal interactions" sub-bullet — set_modal vs raise vs focus, key-routing implications — to head off the rediscovery loop.

Issues filed: #770 (non-slash completion parity follow-up, P2 for C.0.x late or C.0.6).

🤖 Generated with Claude Code /auto

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