plgr repl impl

Solid scaffold. The design split (program-edits-recompile vs queries-rerun) is implemented faithfully, the wire-contract handling in run.rs is correct, history persistence via the new vim-line Store is clean, and the rule-3 invariant (no solve/clause-walk symbol) holds at the source level. The simple gates work. Issues below, ordered by impact.

Real bugs

1. Multi-line queries silently truncate at the first Enter. app.rs:130-141 submit() dispatches as soon as the line starts_with("?-"), regardless of whether it ends in .. So:

?- foo(X),
   bar(X).

gets dispatched after the first line with goal foo(X),, which the query parser rejects. The clause-accumulation logic exists right there — it just isn't reached for queries. Same bug affects :- directives entered across lines (a leading : short-circuits to dispatch). Fix: only dispatch immediately when the line ends in . (or is a meta-command, which is single-line by spec). Let queries/directives flow through the pending accumulator like clauses already do.

2. save_history can wipe valid history. app.rs:104-114 writes the store unconditionally on exit. If load_history failed silently (permission flake, transient I/O error, etc.), the store is empty, and we then overwrite a real history file with nothing. Guard the save: skip if the store is empty AND no entries were pushed this session, or only overwrite if load succeeded.

Design checkpoints not met

3. Completion ignores session predicates (docs/design/REPL.md checkpoint 7). app.rs:122 calls completion::candidates(&prefix, &[]) with an empty extras slice. The function already accepts session names — just thread self.session predicate heads through. Today, Tab on a session-defined parent only completes if it happens to match a builtin/stdlib name.

4. Rule-3 CI guard is missing (docs/design/REPL.md checkpoint 6). The design explicitly calls for a CI assertion that plg-repl contains no solve/clause-walk symbol. Today it's true by inspection but nothing pins it. justfile installs plgr but adds no gate.

5. :load collapses a multi-clause file into one buffer entry. app.rs:240-253 load_file() calls add_clause(text.trim()), so session.clauses ends up with one giant string. :list then renders one blob and predicate ordering blurs across :load calls. The TODO comment acknowledges this; the design's "ordered session source buffer (clauses + directives)" is what should be implemented. Split the file into clauses before pushing.

Minor

6. engine.rs shells to plgc instead of linking plg-compiler (the design target). Labelled scaffold and $PLGC is supported — fine for now, but the error path only catches spawn-fail; missing-from-PATH surfaces a system-y errno string. A one-line "install with just install" hint would smooth the first-run experience.

7. UI prompt differs from design. docs/design/REPL.md says "?- prompt" for the terminal-native transcript; app.rs:24 uses "plg> ". The doc comment justifies the change (neutral so it accepts both clauses and queries) — fine, but design doc and impl should agree.

8. :edit is stubbed. app.rs:236 logs "not wired yet". Either remove from the design surface until implemented or stub out the $EDITOR path.

What's good

run.rs correctly handles the wire contract — exit-1-with-stdout → Ok, exit-2/3 → Failed with stderr. Stdin nulled, timeout via PLG_REPL_TIMEOUT, 50 ms poll.
Compiled holds its TempDir so the binary lives exactly as long as it's the current one — drop semantics are right.
capitalization_hint is a nice touch for beginners.
input.rs adapter is small and clearly scoped to what vim-line's contract leaves to the host; the reserved HistorySearch/Accept/Cancel actions are correctly forward-compatible no-ops.
Panic hook restores the terminal — important and easy to forget.

Suggested order

Fix 1 (multi-line queries) and 2 (history save) before merge — both are silent-data-loss class. 3, 4, 5 can land as follow-ups; they're scope completion against the design doc, not regressions.

## Review — plgr REPL impl Solid scaffold. The design split (program-edits-recompile vs queries-rerun) is implemented faithfully, the wire-contract handling in `run.rs` is correct, history persistence via the new `vim-line` `Store` is clean, and the rule-3 invariant (no `solve`/clause-walk symbol) holds at the source level. The simple gates work. Issues below, ordered by impact. ### Real bugs **1. Multi-line queries silently truncate at the first Enter.** `app.rs:130-141` `submit()` dispatches as soon as the line `starts_with("?-")`, regardless of whether it ends in `.`. So: ``` ?- foo(X), bar(X). ``` gets dispatched after the first line with goal `foo(X),`, which the query parser rejects. The clause-accumulation logic exists right there — it just isn't reached for queries. Same bug affects `:-` directives entered across lines (a leading `:` short-circuits to dispatch). Fix: only dispatch immediately when the line ends in `.` (or is a meta-command, which is single-line by spec). Let queries/directives flow through the `pending` accumulator like clauses already do. **2. `save_history` can wipe valid history.** `app.rs:104-114` writes the store unconditionally on exit. If `load_history` failed silently (permission flake, transient I/O error, etc.), the store is empty, and we then overwrite a real history file with nothing. Guard the save: skip if the store is empty AND no entries were pushed this session, or only overwrite if load succeeded. ### Design checkpoints not met **3. Completion ignores session predicates** (`docs/design/REPL.md` checkpoint 7). `app.rs:122` calls `completion::candidates(&prefix, &[])` with an empty extras slice. The function already accepts session names — just thread `self.session` predicate heads through. Today, Tab on a session-defined `parent` only completes if it happens to match a builtin/stdlib name. **4. Rule-3 CI guard is missing** (`docs/design/REPL.md` checkpoint 6). The design explicitly calls for a CI assertion that `plg-repl` contains no `solve`/clause-walk symbol. Today it's true by inspection but nothing pins it. `justfile` installs `plgr` but adds no gate. **5. `:load` collapses a multi-clause file into one buffer entry.** `app.rs:240-253` `load_file()` calls `add_clause(text.trim())`, so `session.clauses` ends up with one giant string. `:list` then renders one blob and predicate ordering blurs across `:load` calls. The TODO comment acknowledges this; the design's "ordered session source buffer (clauses + directives)" is what should be implemented. Split the file into clauses before pushing. ### Minor **6. `engine.rs` shells to `plgc`** instead of linking `plg-compiler` (the design target). Labelled scaffold and `$PLGC` is supported — fine for now, but the error path only catches spawn-fail; missing-from-PATH surfaces a system-y errno string. A one-line "install with `just install`" hint would smooth the first-run experience. **7. UI prompt differs from design.** `docs/design/REPL.md` says "`?- ` prompt" for the terminal-native transcript; `app.rs:24` uses `"plg> "`. The doc comment justifies the change (neutral so it accepts both clauses and queries) — fine, but design doc and impl should agree. **8. `:edit` is stubbed.** `app.rs:236` logs "not wired yet". Either remove from the design surface until implemented or stub out the `$EDITOR` path. ### What's good - `run.rs` correctly handles the wire contract — exit-1-with-stdout → `Ok`, exit-2/3 → `Failed` with stderr. Stdin nulled, timeout via `PLG_REPL_TIMEOUT`, 50 ms poll. - `Compiled` holds its `TempDir` so the binary lives exactly as long as it's the current one — drop semantics are right. - `capitalization_hint` is a nice touch for beginners. - `input.rs` adapter is small and clearly scoped to what `vim-line`'s contract leaves to the host; the reserved `HistorySearch`/`Accept`/`Cancel` actions are correctly forward-compatible no-ops. - Panic hook restores the terminal — important and easy to forget. ### Suggested order Fix 1 (multi-line queries) and 2 (history save) before merge — both are silent-data-loss class. 3, 4, 5 can land as follow-ups; they're scope completion against the design doc, not regressions.

navicore referenced this pull request from a commit

2026-06-16 05:33:35 +00:00

⏺ All four landed, green across clippy/fmt/tests/workspace:

navicore added 1 commit

2026-06-16 05:33:35 +00:00

⏺ All four landed, green across clippy/fmt/tests/workspace:

CI - Linux / CI - Linux x86_64 (pull_request) Successful in 1m40s

Details

8a64f1d9e1

1. Multi-line queries/directives no longer truncate (app.rs submit). The
  immediate-dispatch condition is now "is a meta-command (: but not :-) or ends
  in ."; ?- queries and :- directives flow through the pending accumulator until
  their terminating ., just like clauses. Single-line ?- fact(X). and :list still
  dispatch instantly.
  2. save_history won't clobber a real file (app.rs). Guarded with if
  self.store.is_empty() { return; } — a failed load_history (transient read
  error) leaves the store empty, and we now refuse to overwrite the on-disk
  history with nothing.
  3. Rule-3 CI guard (main.rs, two #[cfg(test)] tests): one asserts plg-repl
  doesn't depend on plg-runtime (no solver to interpret with), the other scans
  src/ for any fn solve definition. The invariant is now pinned by cargo test,
  not just inspection. (Had to make the source-scan match line-start fn
  declarations so it didn't trip over its own assertion string — caught and
  fixed.)
  4. Doc reconciled (REPL.md). The presentation-model section now states the
  landed terminal-style transcript and documents the deliberate neutral plg>
  prompt with its rationale, instead of the stale ?- /bordered-panes description.

  Left as tracked follow-ups, per my recommendation: #3 session-predicate
  completion and #5 :load clause-splitting — both real design-checkpoint
  completion, larger than a quick fix. The review's #6 (engine PATH hint) was
  already covered, and #8 (:edit stub) stays parked and unadvertised.