Skip to content

repl: inline ghost-text suggestions for symbol autocomplete - #30413

Merged
alii merged 8 commits into
mainfrom
farm/a2da00c4/repl-inline-suggestions
Aug 15, 2026
Merged

repl: inline ghost-text suggestions for symbol autocomplete#30413
alii merged 8 commits into
mainfrom
farm/a2da00c4/repl-inline-suggestions

Conversation

@robobun

@robobun robobun commented May 8, 2026

Copy link
Copy Markdown
Collaborator

Problem

  • bun repl only offers completions on Tab, and only for globals. There is no inline hint while typing, and obj.<Tab> cannot complete properties.

Fix

Inline, fish/IntelliSense-style hints in bun repl: as you type, the remainder of the best matching symbol is drawn dimmed after the cursor; Right, End, Ctrl+E, Ctrl+F or Tab accept it.

bun repl showing dimmed inline completion hints being accepted with the right arrow, End and Tab

❯ cons·ole            "ole" is dimmed
❯ console.l·og        properties of a resolved `a.b.c` chain
❯ JSON.·parse         first property after a bare dot
❯ x instan·ceof       keyword fallback when no global matches

src/runtime/cli/repl.rs

  • parse_completion_context splits the text before the cursor into an optional ident(.ident)* chain and the word being typed. When the word follows a . that is not preceded by such a chain (p.then(x).th|) it returns None and nothing is suggested, since globals and keywords are never valid there (an earlier revision suggested tls for foo().t and this for foo().th). The .. of a spread ([...args, [...a.b) is not treated as member access, and a chain may start with this, which the REPL evaluates as globalThis. Nothing is suggested inside string or template text (Tab just indents there, also when the template was opened on an earlier line of the same input); the ${…} holes of a template count as code.
  • resolve_object_expr walks the chain from globalThis one property at a time through the new Bun__REPL__getProperty binding: the segment is decoded as UTF-8 and looked up with ordinary semantics (the base is boxed with toObject, then JSObject::getIfPropertyExists, so inherited properties such as o.constructor and chains through primitives such as s.length.toF resolve). This deliberately runs getters, as node's completion preview does; a throwing getter or a missing property just ends the chain. The REPL evaluator is not involved, so _ / _error are untouched.
  • update_suggestion calls the existing Bun__REPL__getCompletions, keeps candidates that are IdentifierNames (bun_core::identifier::is_identifier, the check the printer uses for a.b vs a["b"], so café qualifies and "foo-bar" / "0" do not), takes the shortest (first, for an empty prefix), and stores only the remainder. A remainder that would wrap past the terminal width is dropped at this point, so what can be accepted is always exactly what was drawn.
  • refresh_line draws the remainder as ESC[2m…ESC[0m when the cursor is at end of line; Enter and Ctrl+C clear to end of line first so the ghost never ends up in the submitted line. Every editing key recomputes or clears the suggestion.
  • Tab: accepts a visible ghost; otherwise the existing completion UI now also works on obj.prefix chains, does nothing mid-identifier (con|sole used to become consolesole), and inserts spaces through one insert_tab_spaces helper on every "nothing to complete" path.
  • Only computed in interactive TTY sessions with colors enabled, and only on the first line of an input (continuation lines and .editor mode get no hints); piped and NO_COLOR sessions are unchanged apart from the Tab improvements. Per keystroke this costs one property enumeration of the target (enumerating the globalThis chain measures at roughly 20 µs in a release build).

src/jsc/bindings/bindings.cpp

  • Bun__REPL__getProperty (new, see above): a top exception scope, UTF-8 name, base boxed like getCompletions already did for its target, undefined on miss or throw.
  • Bun__REPL__getCompletions uses DECLARE_TOP_EXCEPTION_SCOPE and clears exceptions itself: the Rust caller has no exception scope, and the old throw scope tripped validateExceptionChecks on the ASAN lane once this was called per keystroke.
  • Dropped the manual prototype walk. JSObject::getPropertyNames already covers the chain (deduplicated, depth-capped), so the walk only listed inherited names once per level (on main is + Tab shows isPrototypeOf twice) and spun forever on a Proxy whose getPrototypeOf trap changes its answer, which now matters because user objects reach this function on every keystroke.

Verification

  • test/js/bun/repl/repl.test.ts, inline suggestions: 21 PTY tests (global / property / keyword / empty-prefix hints, acceptance via Right, End and Tab, no hint after a non-chain dot or inside a string, Tab inside a string and on continuation lines, template holes, spread, this., chains through primitives, non-ASCII names, prefixes and chain segments, segments inherited from Object.prototype, the misbehaving Proxy, chain Tab completion without colors, ghost text not evaluated on Enter). All of them time out on the released bun; each test added in the last two rounds was also run against the revision it fixes and fails there.
  • Whole file: 169 pass. withTerminalRepl now sends Ctrl+U before .exit, so tests that leave text on the prompt exit immediately instead of waiting for the 2 s kill.
  • Source lints, cargo fmt, clang-format and prettier clean. Throwing getters, missing segments and non-ASCII segments were also exercised under BUN_JSC_validateExceptionChecks=1.

Background

  • Ghost text: the suggestion is not part of the line buffer. Repl.suggestion holds just the remainder; it is redrawn on every refresh and only becomes input when an accept key copies it into the LineEditor.
  • Bun__REPL__getCompletions predates this PR: given an object (or undefined for globalThis) and a prefix it returns a JS array of matching property names. Both the new hint path and Tab go through it.
  • JSC 8-bit strings are Latin-1, and the general-purpose JSValue::get builds its key that way (it also uses Bun's prototype-pollution-mitigating lookup, which stops at Object.prototype). User-typed chain segments are UTF-8 and need real property semantics, hence the dedicated binding.
  • IdentifierName is the grammar for what may follow a .; it allows Unicode ID_Start / ID_Continue characters and reserved words, which is why the filter is the identifier check rather than an ASCII scan.
  • The line editor runs in raw mode while typing (SIGINT is only enabled while awaiting a promise), so a completer that does not return blocks the REPL; hence the Proxy test.
History

Originally written against the Zig REPL; after #30412 moved the REPL to src/runtime/cli/repl.rs the feature was re-ported there and the .zig file is untouched. Later rounds switched the C++ side to a top exception scope (ASAN lane), made the acceptance tests assert on evaluated results rather than echoed input, then stopped suggesting globals/keywords after a non-chain dot, moved the candidate filter to identifier::is_identifier, and removed the prototype walk; the latest round replaced JSValue::get in the chain walk with Bun__REPL__getProperty, fixed the spread cases, made this. chains and chains through primitives work, and stopped completing inside strings (later refined so template holes and continuation lines are handled).


no test proof · iteration 23 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/repl/repl.test.ts

@github-actions github-actions Bot added the claude label May 8, 2026
@robobun

robobun commented May 8, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 4:40 PM PT - Aug 14th, 2026

@robobun, your commit 6e412e6 is still building in Build #96842, but has 1 failures so far (All Failures):

@coderabbitai

coderabbitai Bot commented May 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

The Zig REPL gains inline "ghost text" suggestions for property and keyword completions. A new suggestion buffer stores the completion remainder, displayed dimmed after typed input when the cursor is at end-of-line. Completion logic parses dotted identifier chains, resolves JSC properties, and computes suggestions; Tab, End, and Right keys accept suggestions, while Enter and Ctrl+C clear them to prevent suggestion text from being submitted.

Changes

REPL Inline Ghost Text Suggestions

Layer / File(s) Summary
Suggestion Buffer Lifecycle
src/cli/repl.zig
Repl struct gains suggestion: ArrayList(u8) field; initialized in Repl.init and deinitialized in Repl.deinit; cleared on break/cancel and when lines are replaced.
Completion Context & Suggestion Computation
src/cli/repl.zig
New infrastructure parses dotted identifier chains (ident(.ident)*) and trailing prefix from cursor, resolves object expressions via JSC property walking when present, computes suggestion remainders (stores only remainder), supports keyword fallback, and accepts suggestions by inserting into line editor.
Ghost Text Rendering & Documentation
src/cli/repl.zig
refreshLine() appends dimmed ghost text after typed input when cursor is at end-of-line and colors are enabled; help text updated to document Tab, Right, and End keybindings for suggestion acceptance.
Key Handling & Suggestion Lifecycle
src/cli/repl.zig
Editing, movement, deletion, history, and special keys updated: many edits recompute suggestions; Home clears; End/Right at EOL accept; Tab accepts visible suggestion before the completion UI; Enter, Ctrl+C, and cmdBreak clear suggestions before submit/cancel.
Test Infrastructure & Inline Suggestion Tests
test/js/bun/repl/repl.test.ts
withTerminalRepl helper accepts optional environment overrides merged into spawned process environment. New describe("inline suggestions") block validates global completion, property completion after dot, Tab/Right/End acceptance, keyword fallback when no global match, ghost-text-disabled behavior, and regression ensuring ghost text is not evaluated on Enter.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the main change: inline ghost-text suggestions for REPL symbol autocomplete.
Description check ✅ Passed The description explains the problem, implementation, behavior, scope, and verification results in substantial detail.

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/js/bun/repl/repl.test.ts`:
- Around line 1083-1095: Remove the explicit per-test timeout on the "tab
accepts the visible suggestion" test: delete the trailing ", 15000" from the
test(...) call that wraps the withTerminalRepl block (so the test declaration
uses the default Jest timeout), relying on the existing waitFor call in
withTerminalRepl (await waitFor(`${DIM}ingify`, 10000)) for timing; if you truly
need more time for the ghost suggestion, increase the waitFor timeout value
instead of adding a per-test timeout.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ebf94720-91d9-49d2-bd03-486447b0fd1a

📥 Commits

Reviewing files that changed from the base of the PR and between 6d0d86b and d035ac9.

📒 Files selected for processing (2)
  • src/cli/repl.zig
  • test/js/bun/repl/repl.test.ts

Comment thread test/js/bun/repl/repl.test.ts
@github-actions

github-actions Bot commented May 8, 2026

Copy link
Copy Markdown
Contributor

This PR may be a duplicate of:

  1. fix(repl): tab completion now resolves the target object from dot expressions #27519 - Both PRs implement dot-chain resolution for REPL tab completion (e.g. console.log), modifying handleTab() in repl.zig to resolve the target object before calling getCompletions

🤖 Generated with Claude Code

@Jarred-Sumner

Copy link
Copy Markdown
Collaborator
  • does it work on windows
  • does it work in Apple Terminal

@Jarred-Sumner

Copy link
Copy Markdown
Collaborator

does it work in tmux

@alii

alii commented May 8, 2026

Copy link
Copy Markdown
Member

testing all

@robobun

robobun commented May 8, 2026

Copy link
Copy Markdown
Collaborator Author

does it work on windows

Yes. Ghost text is rendered with ESC[2m…ESC[0m — the same SGR-dim sequence the REPL already uses for its prompt — and bun_core/output.zig enables ENABLE_VIRTUAL_TERMINAL_PROCESSING on stdout/stderr at startup, so Windows Terminal and conhost (Win10 1511+) process it. Clearing the ghost on Enter uses ESC[0K (erase-to-EOL), and accepting via →/End maps through the existing readKey() handlers for ESC[C / ESC[F / ESC[4~, which Windows Terminal emits with ENABLE_VIRTUAL_TERMINAL_INPUT (already set in setupTerminal()). zig:check-all passes for the Windows targets.

The new tests sit inside the existing describe.todoIf(isWindows) Terminal block — same PTY-harness limitation as every other interactive REPL test, not a feature gap.

does it work in Apple Terminal

Yes. Only stock VT100/xterm sequences are used: SGR dim/reset (ESC[2m / ESC[0m), erase-line (ESC[2K), erase-to-EOL (ESC[0K), and cursor-forward (ESC[{n}C) — all of which Terminal.app has supported forever and the REPL already relies on for its prompt/redraw. No iTerm2/Kitty/OSC extensions. The feature also auto-disables when use_colors is false, so dumb/NO_COLOR sessions never see raw escapes.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/cli/repl.zig (1)

985-1004: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Suppress ghost text when it would wrap onto the next terminal row.

If the dimmed remainder crosses the right edge, the later \r + cursor-move restore runs on the wrapped row, so the caret lands on the wrong line; Enter/Ctrl+C then only clear the first row of that suggestion. Guard this render path with the remaining columns, or truncate the suggestion to fit.

💡 Proposed fix
-    // Write the inline ghost suggestion after the typed text. Only shown when
-    // the cursor is at end-of-line so it visually continues the input.
-    if (self.suggestion.items.len > 0 and
-        self.use_colors and
-        self.line_editor.cursor == self.line_editor.buffer.items.len)
+    const terminal_width = `@as`(usize, self.terminal_width);
+    const cursor_pos = prompt_len + self.line_editor.cursor;
+    const suggestion_fits = cursor_pos + self.suggestion.items.len < terminal_width;
+
+    // Write the inline ghost suggestion after the typed text. Only shown when
+    // the cursor is at end-of-line so it visually continues the input.
+    if (self.suggestion.items.len > 0 and
+        self.use_colors and
+        self.line_editor.cursor == self.line_editor.buffer.items.len and
+        suggestion_fits)
     {
         self.write(Color.dim);
         self.write(self.suggestion.items);
         self.write(Color.reset);
     }
 
     // Position cursor
-    const cursor_pos = prompt_len + self.line_editor.cursor;
-    if (cursor_pos < self.terminal_width) {
+    if (cursor_pos < terminal_width) {
         self.write("\r");
         if (cursor_pos > 0) {
             var buf: [16]u8 = undefined;
             const seq = std.fmt.bufPrint(&buf, CSI ++ "{d}C", .{cursor_pos}) catch return;
             self.write(seq);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/repl.zig` around lines 985 - 1004, The ghost suggestion must be
suppressed or truncated when it would wrap the line: compute cursor_pos
(prompt_len + self.line_editor.cursor) and remaining_cols = self.terminal_width
- cursor_pos, and only render the dimmed suggestion (self.write(Color.dim);
self.write(self.suggestion.items); self.write(Color.reset)) when remaining_cols
> 0 and self.suggestion.items.len <= remaining_cols — otherwise either skip
rendering the suggestion entirely or truncate self.suggestion.items to
remaining_cols before writing; keep the subsequent cursor positioning logic
(cursor_pos, self.write("\r"), CSI "{d}C") unchanged so the caret lands on the
correct row.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/cli/repl.zig`:
- Around line 1088-1094: The current isIdentStart and isIdentPart functions use
ASCII checks and fail for non-ASCII ECMAScript identifiers; replace their logic
to call bun.js_lexer.isIdentifierStart(c) and
bun.js_lexer.isIdentifierContinue(c) respectively (and update any other copies
noted around the file, e.g., the occurrences near the functions at lines
referenced in the review) so the REPL uses the full JS identifier rules for
inline suggestions and Tab completion; ensure the signatures and return types
remain the same and import or reference bun.js_lexer as needed.

---

Outside diff comments:
In `@src/cli/repl.zig`:
- Around line 985-1004: The ghost suggestion must be suppressed or truncated
when it would wrap the line: compute cursor_pos (prompt_len +
self.line_editor.cursor) and remaining_cols = self.terminal_width - cursor_pos,
and only render the dimmed suggestion (self.write(Color.dim);
self.write(self.suggestion.items); self.write(Color.reset)) when remaining_cols
> 0 and self.suggestion.items.len <= remaining_cols — otherwise either skip
rendering the suggestion entirely or truncate self.suggestion.items to
remaining_cols before writing; keep the subsequent cursor positioning logic
(cursor_pos, self.write("\r"), CSI "{d}C") unchanged so the caret lands on the
correct row.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0e0da2bf-4edf-4c67-98cb-2518ccad0304

📥 Commits

Reviewing files that changed from the base of the PR and between d035ac9 and df8f489.

📒 Files selected for processing (1)
  • src/cli/repl.zig

Comment thread src/cli/repl.zig Outdated
@robobun

robobun commented May 8, 2026

Copy link
Copy Markdown
Collaborator Author

does it work in tmux

Yes — tmux passes all of these through natively. The ghost text adds nothing to the escape-sequence vocabulary the REPL already uses for its prompt/redraw: SGR dim/reset (ESC[2m/ESC[0m), erase-line (ESC[2K), erase-to-EOL (ESC[0K), cursor-forward (ESC[{n}C). No OSC/DCS passthrough needed (unlike .copy's OSC 52).

Comment thread test/js/bun/repl/repl.test.ts
Comment thread test/js/bun/repl/repl.test.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/cli/repl.zig (1)

528-541: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update .help to mention Ctrl+E/Ctrl+F accepting ghost text too.

The handlers now accept visible suggestions on both control keys, but the help still describes them only as cursor movement/forward-char. That leaves the documented keybindings out of sync with the implementation.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/repl.zig` around lines 528 - 541, The help text is out of sync:
update the repl.print lines that document Ctrl+E and Ctrl+F (the lines currently
printing "Move to end of line" and "Move backward/forward one character") to
indicate these keys also accept visible ghost text/suggestions; edit the
corresponding repl.print calls so the captions read something like "Move to end
of line / Accept suggestion" and "Move backward/forward one character / Accept
suggestion" to match the implemented handlers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/cli/repl.zig`:
- Around line 991-999: The current visibility check in the rendering branch
(uses self.suggestion.items, self.use_colors, self.line_editor.cursor,
self.line_editor.buffer.items, cursor_pos, and self.terminal_width) only
suppresses drawing but leaves self.suggestion populated, allowing accept-keys
(Tab/Right/End and handlers for Ctrl+E/Ctrl+F) to apply an unseen ghost; update
the code so that when that visibility predicate fails you either clear
self.suggestion (e.g., set it to empty) right there, or make every acceptance
path (the functions/methods that handle Tab/Right/End and Ctrl+E/Ctrl+F)
re-check the identical visibility predicate before mutating the buffer; pick one
approach and apply it consistently so acceptance cannot apply a suggestion that
was not rendered.
- Around line 1112-1120: CompletionContext currently only records the prefix and
prefix_start, so tab completion replaces only the left-hand slice and leaves the
existing suffix (e.g., con|sole → consolesole); add a field to CompletionContext
(e.g., prefix_end or suffix_start) that marks the end of the identifier under
the cursor, and update the tab-completion logic that currently deletes only the
left slice to instead delete the entire token range from prefix_start to that
end before inserting the completion; alternatively, if you prefer not to track
an end, change the completion handler (the code around where CompletionContext
is used for Tab completion) to scan forward from prefix_start to the token end
and replace that whole span.

---

Outside diff comments:
In `@src/cli/repl.zig`:
- Around line 528-541: The help text is out of sync: update the repl.print lines
that document Ctrl+E and Ctrl+F (the lines currently printing "Move to end of
line" and "Move backward/forward one character") to indicate these keys also
accept visible ghost text/suggestions; edit the corresponding repl.print calls
so the captions read something like "Move to end of line / Accept suggestion"
and "Move backward/forward one character / Accept suggestion" to match the
implemented handlers.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: af583f4b-9e61-4795-9031-83184c4168ba

📥 Commits

Reviewing files that changed from the base of the PR and between df8f489 and a5a309c.

📒 Files selected for processing (1)
  • src/cli/repl.zig

Comment thread src/cli/repl.zig Outdated
Comment thread src/cli/repl.zig Outdated
Comment thread src/cli/repl.zig Outdated
Comment thread src/cli/repl.zig Outdated
Comment thread src/cli/repl.zig Outdated
@robobun

robobun commented May 9, 2026

Copy link
Copy Markdown
Collaborator Author

Status

Head b6e1cc2. The last two rounds addressed the review comments on the previous pushes:

  • no global/keyword hint after a dot that does not follow an identifier chain (p.then(x).th|), and none inside a string literal; a spread's .. is not member access (before a bare word or a chain), this.x completes against the global object, chains through primitives (s.length.toF) resolve, Tab inside a string indents instead of completing (including a template opened on an earlier line of the input), and template ${…} holes count as code
  • candidate filter is identifier::is_identifier, the typed word may contain non-ASCII, and chain segments are looked up as UTF-8 with ordinary property semantics through the new Bun__REPL__getProperty binding (café. and o.constructor. both resolve)
  • Bun__REPL__getCompletions no longer walks the prototype chain itself (duplicate Tab listings, possible spin on a Proxy)
  • demo recording added to the description

Verified locally with the debug build: test/js/bun/repl/repl.test.ts 169 pass / 0 fail; every test added in these rounds was also run against the revision it fixes and fails there. Throwing getters and non-ASCII segments exercised under BUN_JSC_validateExceptionChecks=1. Source lints, rustfmt, clang-format, prettier clean.

CI: build #96957 for this head has 177 of 179 jobs green with two darwin jobs still queued and nothing failed. The previous build's single red job (test/bake/deinitialization.test.ts segfaulting on Windows 2019 x64 in dev-server teardown) does not involve the REPL and was handed to main-break triage.

Files touched: src/runtime/cli/repl.rs, src/jsc/bindings/bindings.cpp and headers.h (REPL functions only), test/js/bun/repl/repl.test.ts. All review threads are replied to; the automated review's final pass found nothing further and asked for a human look, so this is ready for review.

@robobun
robobun force-pushed the farm/a2da00c4/repl-inline-suggestions branch from 3c1e367 to c55e83a Compare May 15, 2026 02:06
Comment thread src/runtime/cli/repl.rs Outdated
Comment thread src/runtime/cli/repl.rs Outdated
@robobun
robobun force-pushed the farm/a2da00c4/repl-inline-suggestions branch from 3e3d7fd to d4d8572 Compare June 5, 2026 19:37
Comment thread src/jsc/bindings/bindings.cpp Outdated
Comment thread src/runtime/cli/repl.rs Outdated

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No new issues found — all prior inline threads are resolved. Deferring to a human: this is a ~450-line user-facing REPL feature with UX/keybinding design choices, per-keystroke JS getter invocation via resolve_object_expr, and a JSC exception-scope change in bindings.cpp. Also, the maintainer's Windows/Apple Terminal/tmux question got a spec-based answer plus "testing all", but no manual-test confirmation was ever posted.

What was reviewed:

  • parse_completion_context / resolve_object_expr edge cases (empty segments, non-object mid-chain, throwing getters) — all bail to UNDEFINED and clear the exception.
  • Bun__REPL__getCompletions scope change — DECLARE_TOP_EXCEPTION_SCOPE + local clearException matches the sibling Bun__REPL__evaluate pattern; no exception escapes the FFI boundary.
  • Suggestion lifecycle across all key handlers — every buffer-mutating arm calls update_suggestion() or suggestion.clear(); Enter/Ctrl+C wipe rendered ghost via ESC[0K.
  • Width-wrap guard keeps render/accept in sync; the stale-terminal_width limitation is pre-existing and tracked in #31897.
Extended reasoning...

Overview

Adds fish-style inline ghost-text completion to bun repl. In src/runtime/cli/repl.rs (~450 lines added): a new suggestion: Vec<u8> field, parse_completion_context (backward scan for ident(.ident)* chains), resolve_object_expr (walks the chain from globalThis via JSValue::get()), update_suggestion (queries Bun__REPL__getCompletions, picks the shortest dot-accessible match, falls back to a JS-keyword table, drops if it would wrap), accept_suggestion, ghost rendering in refresh_line, and ~15 key-handler arms wired to recompute/clear/accept. In src/jsc/bindings/bindings.cpp: Bun__REPL__getCompletions switches from DECLARE_THROW_SCOPE to DECLARE_TOP_EXCEPTION_SCOPE and clears exceptions locally at every checkpoint. In test/js/bun/repl/repl.test.ts: 9 new PTY-driven tests plus an env override on withTerminalRepl.

Security risks

None material. The completion path runs only in interactive TTY sessions. resolve_object_expr invokes property getters on user-defined objects on each keystroke — that's a design choice (matches Node's REPL) rather than a security issue, since the user is already executing arbitrary JS in the same VM. Exceptions from getters are cleared and suggestion computation returns early. No filesystem, network, or privilege boundaries are crossed.

Level of scrutiny

Medium-high. This is a net-new user-facing feature, not a bug fix or mechanical change. It carries UX/design decisions a maintainer should sign off on: which keys accept the suggestion (→/End/Ctrl+E/Ctrl+F/Tab), the hardcoded JS_KEYWORDS fallback table, running getters per keystroke, and terminal escape-sequence behavior across platforms. It also touches JSC exception-scope handling in C++ — a category CLAUDE.md flags as review-sensitive. The PR has been through several rounds of automated review (7 of my own inline findings + 4 CodeRabbit findings, all fixed and resolved), so code-correctness confidence is reasonably high, but that doesn't substitute for human sign-off on the feature design.

Other factors

  • Jarred asked whether it works on Windows / Apple Terminal / tmux; robobun gave a correct spec-level answer (only stock VT100/xterm SGR/erase/CUF sequences, VT processing already enabled on Windows), and alii replied "testing all" — but no follow-up confirming manual test results was posted. That's the kind of thing a human reviewer should confirm before merge.
  • A duplicate-PR bot flagged overlap with #27519 (dot-chain Tab completion) — worth a human check that these don't conflict.
  • All 9 new tests are inside the existing describe.todoIf(isWindows) PTY block, so Windows CI coverage is absent (pre-existing harness limitation, not a regression).
  • The one known remaining limitation (stale terminal_width) is pre-existing, documented, and tracked in #31897.
  • No prior top-level review body from me on this PR, so this defer is not redundant.

While typing in the interactive REPL, show a dimmed inline completion
hint after the cursor (fish/IntelliSense-style). Suggestions draw from
globalThis properties, simple `obj.prop` chains resolved via property
lookup, and a small JS keyword fallback. Right/End/Ctrl+E/Ctrl+F/Tab
accept the visible suggestion; Enter/Ctrl+C wipe it from the submitted
line before moving on.

Implementation in src/runtime/cli/repl.rs:
- parse_completion_context() scans backward from the cursor to split
  the input into an optional ident.ident… object expression and a
  trailing prefix; anything more complex than a dotted chain safely
  falls back to global completion.
- resolve_object_expr() walks that chain from globalThis via
  JSValue::get() so _/_error are untouched.
- update_suggestion() asks Bun__REPL__getCompletions for matches,
  picks the shortest dot-accessible one (or first for an empty
  prefix), and drops it if it would wrap past the terminal width
  (display width, not byte length) — keeping render and accept in
  sync so Tab/Right/End can never apply text the user didn't see.
- handle_tab() now resolves property chains, bails mid-identifier
  (no-op) to avoid splitting the token, and gates single-completion
  inserts on is_dot_accessible.

Bun__REPL__getCompletions (bindings.cpp) switched from
DECLARE_THROW_SCOPE to DECLARE_TOP_EXCEPTION_SCOPE and now clears
exceptions locally: it's an FFI boundary with no JSError propagation
on the caller side, and the previous scope tripped
verifyExceptionCheckNeedIsSatisfied under
BUN_JSC_validateExceptionChecks (x64-asan CI).
@robobun
robobun force-pushed the farm/a2da00c4/repl-inline-suggestions branch from dae5126 to 0cab329 Compare August 14, 2026 19:01
Comment thread src/jsc/bindings/bindings.cpp Outdated
Comment thread src/runtime/cli/repl.rs Outdated
Comment thread src/runtime/cli/repl.rs
Comment thread src/runtime/cli/repl.rs Outdated
Comment thread src/runtime/cli/repl.rs Outdated
Comment thread src/runtime/cli/repl.rs Outdated
Comment thread src/runtime/cli/repl.rs Outdated
Comment thread src/runtime/cli/repl.rs Outdated
Fold the duplicated clear-ghost-then-clear-buffer sequence in
handle_enter/handle_ctrl_c into one helper, simplify the ArrowRight
arm so it no longer needs explaining, and cut the remaining comments
down to what isn't already obvious from the code.
Comment thread src/runtime/cli/repl.rs Outdated
Comment thread src/runtime/cli/repl.rs Outdated
@robobun

robobun commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator Author

Rebased onto current main (0cab3292), then a comment-trim pass (9e306f4d).

Rebase conflicts resolved:

Comment-cop pass: the remaining comments are cut down to what the code doesn't already say. Two of them pointed at code worth simplifying instead: the Enter/Ctrl+C clear-ghost sequence is now one erase_suggestion() helper, and the ArrowRight arm is restructured so it no longer needs a comment. Section headers and the one-line SAFETY notes match the file's existing style and are left as-is.

Verified after both: repl.test.ts 157/157, all 146 source lints pass.

Comment thread src/runtime/cli/repl.rs Outdated
Comment thread src/runtime/cli/repl.rs Outdated
Comment thread src/runtime/cli/repl.rs Outdated
Comment thread src/jsc/bindings/bindings.cpp Outdated
…n walk

parse_completion_context now returns None when the word follows a `.`
that is not preceded by an identifier chain (`p.then(x).th|`), so the
global/keyword fallback no longer suggests e.g. `this` or `tls` for a
property name. Tab inserts spaces there like the other no-completion
paths (insert_tab_spaces).

Completion candidates are filtered with identifier::is_identifier
instead of an ASCII-only check, and the word scan takes non-ASCII bytes
as part of the word, so `cafe\u0301s`-style names are both offered and
matchable; the remainder is taken via strip_prefix and the width guard
measures display columns.

Bun__REPL__getCompletions drops its manual prototype walk:
JSObject::getPropertyNames already covers the chain (deduplicated and
depth-capped), so the walk only produced duplicate entries and could
spin on a Proxy whose getPrototypeOf trap changes its answer.

Test helper now clears the line before `.exit` so tests that leave text
on the prompt exit immediately instead of waiting for the kill timeout.
@alii

alii commented Aug 14, 2026

Copy link
Copy Markdown
Member

@robobun add a gif to the pr body demoing this feature

Comment thread src/runtime/cli/repl.rs Outdated
Comment thread src/runtime/cli/repl.rs
Comment thread src/runtime/cli/repl.rs
@robobun

robobun commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator Author

Added a demo recording to the description (global hint accepted with Right, property hint accepted with Tab, a resolved process.release.na chain accepted with End, and the bare-dot / narrowing case on JSON.). It was captured from this branch's debug build in a PTY; the image is hosted on a SHA-pinned branch of the robobun/bun fork so it is not part of the diff.

…strings

resolve_object_expr went through JSValue::get, which builds the key as
Latin-1 (so a non-ASCII segment such as `café` never matched) and stops
at Object.prototype (so `o.constructor.` did not resolve). It now calls
Bun__REPL__getProperty, which decodes the segment as UTF-8 and uses
JSObject::getIfPropertyExists under a top exception scope, returning
undefined for misses and throwing getters.

parse_completion_context no longer treats the last dot of a spread
(`[...args`) as member access, and ghost text is not computed while the
line is inside a string literal, where identifiers are noise.
Comment thread src/jsc/bindings/bindings.cpp Outdated
Comment thread src/runtime/cli/repl.rs
Comment thread src/runtime/cli/repl.rs Outdated
A leading `this` segment now stays on globalThis, which is what the REPL
evaluates top-level `this` to, instead of being looked up as a property.
The spread check in parse_completion_context is now a single helper used
both before the word and while walking the chain, so `[...console.l`
resolves `console` rather than bailing on the spread's last dot.
Comment thread src/runtime/cli/repl.rs
Comment thread src/jsc/bindings/bindings.cpp
Bun__REPL__getProperty now calls toObject on its base like
getCompletions does, so `x.length.toF` reaches Number.prototype, and
handle_tab applies the same string-literal check as the hint path
instead of splicing a completion into the literal.
Comment thread src/runtime/cli/repl.rs
Comment thread src/runtime/cli/repl.rs Outdated
…r lines

ends_inside_string now tracks `${...}` holes (including nested templates),
so completion works inside an interpolation and stays off in the
surrounding template text, and it scans a list of slices so handle_tab
can prepend the multiline or editor buffer: a template opened on a
previous line of the same input still suppresses completion on the
continuation line, while ordinary continuation lines keep completing.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

b6e1cc2 addresses both open nits from the last pass — ends_inside_string now tracks ${…} holes with a depth stack (so `${cons completes again while template text stays inert), and handle_tab feeds multiline_buffer/editor_buffer into the same check so a backtick opened on an earlier line is seen. Both have targeted tests ("suggests inside a template hole…", "tab on a continuation line of a template literal indents", plus the negative "…outside a string still completes"). This run found nothing new.

Given the size — ~500 lines of new REPL logic, a hand-rolled string/chain scanner, and a new C++ binding that runs user getters per keystroke — a human look is still worthwhile before merge.

What was reviewed:

  • ends_inside_string: nested `${ `${x}` }`, object-literal braces inside a hole, and part-boundary escapes (multiline buffers always end in \n, so a trailing \ cannot straddle parts).
  • Bun__REPL__getProperty: top exception scope clears on both toObject and getIfPropertyExists, undefined on miss; matches the sibling getCompletions boxing.
  • resolve_object_expr: chain values are consumed synchronously between FFI calls with no intervening allocation, so no GC rooting needed; leading-this special case is first-segment-only.
  • parse_completion_context: spread guard applied at both dot-consumption sites via one member_dot_ends_at closure.
Extended reasoning...

Overview

This PR adds fish-style inline ghost-text completion to bun repl. It touches four files: repl.rs (~350 net new lines: update_suggestion/accept_suggestion/erase_suggestion, a parse_completion_context backward scanner, an ends_inside_string heuristic lexer, resolve_object_expr chain walker, and per-key hooks in the input loop), bindings.cpp (~60 lines: switch Bun__REPL__getCompletions to a top exception scope, drop its redundant prototype walk, and add Bun__REPL__getProperty), headers.h (one decl), and repl.test.ts (~320 lines of PTY tests). The head commit b6e1cc2 is the response to my two prior inline nits from 2026-08-15T00:17.

Prior findings addressed

Both remaining unresolved threads from the previous run are fixed:

  1. Continuation-line string context. ends_inside_string now takes &[&[u8]] and handle_tab supplies [multiline_buffer|editor_buffer, line[..cursor]], so a template opened on an earlier line is visible. The chosen approach (feed prior lines rather than blanket-disable Tab in Multiline mode) preserves property completion on continuation lines that are not inside a string, which the new "tab on a continuation line outside a string still completes" test locks in. I traced the part-boundary escape case (\ at end of part 0): both buffers always append \n before the next part begins, so the escape consumes the newline within part 0 and state carries correctly.

  2. Template interpolation holes. ends_inside_string gained a holes: Vec<u32> stack: on $ followed by { under quote == b''it pushes depth 1 and dropsquote; unquoted {/}adjust the innermost depth; hitting 0 pops and restoresquote = b''. I hand-traced nested templates, an object literal inside a hole, and a bare } outside any hole — all resolve correctly. The known regex-literal-with-quote over-block I noted last time is intentionally left (matches is_incomplete_code's existing blind spot; disproportionate to fix).

Security risks

None material. The chain resolver deliberately runs getters/Proxy traps (as Node's completion preview does), scoped to what the user typed at their own prompt; a throwing getter is caught by the top exception scope and yields undefined. The Proxy-with-cycling-getPrototypeOf hang the PR fixes on the getCompletions side has a dedicated test.

Level of scrutiny

Medium. The REPL is an interactive dev-tool surface, not a hot runtime path, and every failure mode here degrades to "no suggestion" or "insert two spaces". But the change is not mechanical: it introduces a bespoke lexer heuristic and a new C++ binding that is invoked on every keystroke, and it has taken 23 iterations with six earlier findings from this reviewer alone. That history argues for one human pass over the final shape rather than an automated approval.

Other factors

Test coverage is thorough (18 new PTY tests, each asserting on evaluated results rather than echoed input, plus the negative-contract tests). The C++ exception-scope change was verified under BUN_JSC_validateExceptionChecks=1 per the PR body. No CODEOWNERS path is touched. Deferring rather than approving on size and novelty, not on any open concern.

@alii
alii merged commit 2c2ef7c into main Aug 15, 2026
8 of 9 checks passed
@alii
alii deleted the farm/a2da00c4/repl-inline-suggestions branch August 15, 2026 01:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants