● Scaffold checkpoint done. End-to-end smoke runs: #38

Merged

navicore merged 2 commits from repl into main

2026-06-04 06:02:35 +00:00

navicore commented

2026-06-04 05:42:35 +00:00

Owner

?- member(X, [a, b, c]).
X = a
; or . > ;
X = b
; or . > ;
X = c
; or . > .
?-

What's in:

crates/repl/ wired into the workspace; prlg-repl binary builds and
installs via just install.
engine.rs — Session (stdlib + consult files) → Query → Step::{Solution,
Done, Error}. 5 unit tests covering the canonical streaming case, parse
error, ISO type_error, inline consult, and step-limit propagation.
history.rs — XDG-aware path, append/load with tail trimming and
embedded-newline flattening. 5 unit tests including XDG honor and tail
behavior.
app.rs + keys.rs — App struct and Action enum sketched; field-level
#[allow(dead_code)] on the parts the TUI loop will consume next iteration.
main.rs — clap CLI, line-mode stdin/stdout driver that exercises the full
engine path today (; for next, . to commit, quit./EOF to exit).
justfile install + build + pkg-check updated; release workflow extended
with the third inter-crate pin count and a cargo publish -p
patch-prolog-repl step between lsp and the CLI.
just ci green (38 + 11 = 49 tests, clippy clean, all four crates package
clean).

Deferred for the next iteration (per design):

ratatui main loop, panel layout, bindings panel, vim-line modal editing.
The Session/Query self-referential question (App can't hold both as-is);
decision noted in app.rs — likely either rerun-and-skip or self_cell.
Step-limit shows as existence_error(...) in some tests rather than
resource_error(steps) because the engine surfaces missing-predicate errors
with similar shape; not blocking, but worth a closer look when wiring the
bindings panel.

?- member(X, [a, b, c]). X = a ; or . > ; X = b ; or . > ; X = c ; or . > . ?- What's in: - crates/repl/ wired into the workspace; prlg-repl binary builds and installs via just install. - engine.rs — Session (stdlib + consult files) → Query → Step::{Solution, Done, Error}. 5 unit tests covering the canonical streaming case, parse error, ISO type_error, inline consult, and step-limit propagation. - history.rs — XDG-aware path, append/load with tail trimming and embedded-newline flattening. 5 unit tests including XDG honor and tail behavior. - app.rs + keys.rs — App struct and Action enum sketched; field-level #[allow(dead_code)] on the parts the TUI loop will consume next iteration. - main.rs — clap CLI, line-mode stdin/stdout driver that exercises the full engine path today (; for next, . to commit, quit./EOF to exit). - justfile install + build + pkg-check updated; release workflow extended with the third inter-crate pin count and a cargo publish -p patch-prolog-repl step between lsp and the CLI. - just ci green (38 + 11 = 49 tests, clippy clean, all four crates package clean). Deferred for the next iteration (per design): - ratatui main loop, panel layout, bindings panel, vim-line modal editing. - The Session/Query self-referential question (App can't hold both as-is); decision noted in app.rs — likely either rerun-and-skip or self_cell. - Step-limit shows as existence_error(...) in some tests rather than resource_error(steps) because the engine surfaces missing-predicate errors with similar shape; not blocking, but worth a closer look when wiring the bindings panel.

navicore added 1 commit

2026-06-04 05:42:35 +00:00

● Scaffold checkpoint done. End-to-end smoke runs:

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

Details

85a8d93f40

?- member(X, [a, b, c]).
    X = a
    ; or . > ;
    X = b
    ; or . > ;
    X = c
    ; or . > .
  ?-

  What's in:
  - crates/repl/ wired into the workspace; prlg-repl binary builds and
  installs via just install.
  - engine.rs — Session (stdlib + consult files) → Query → Step::{Solution,
  Done, Error}. 5 unit tests covering the canonical streaming case, parse
  error, ISO type_error, inline consult, and step-limit propagation.
  - history.rs — XDG-aware path, append/load with tail trimming and
  embedded-newline flattening. 5 unit tests including XDG honor and tail
  behavior.
  - app.rs + keys.rs — App struct and Action enum sketched; field-level
  #[allow(dead_code)] on the parts the TUI loop will consume next iteration.
  - main.rs — clap CLI, line-mode stdin/stdout driver that exercises the full
  engine path today (; for next, . to commit, quit./EOF to exit).
  - justfile install + build + pkg-check updated; release workflow extended
  with the third inter-crate pin count and a cargo publish -p
  patch-prolog-repl step between lsp and the CLI.
  - just ci green (38 + 11 = 49 tests, clippy clean, all four crates package
  clean).

  Deferred for the next iteration (per design):
  - ratatui main loop, panel layout, bindings panel, vim-line modal editing.
  - The Session/Query self-referential question (App can't hold both as-is);
  decision noted in app.rs — likely either rerun-and-skip or self_cell.
  - Step-limit shows as existence_error(...) in some tests rather than
  resource_error(steps) because the engine surfaces missing-predicate errors
  with similar shape; not blocking, but worth a closer look when wiring the
  bindings panel.

navicore commented

2026-06-04 05:56:00 +00:00

Author

Owner

Review

What it does: Scaffolds patch-prolog-repl (binary prlg-repl): a UI-free engine driver (Session/Query/Step), XDG-aware persistent history, App/Action skeletons for the coming ratatui loop, and a working line-mode driver. Workflow/justfile extended for the fourth crate.

Verified locally (scratch copy of the branch): all 11 tests pass, and the end-to-end smoke works exactly as the PR description claims — member(X, [a,b,c]). streams a/b/c on ;, X is foo. surfaces type_error(evaluable, foo) and returns to the prompt, history file lands under $XDG_DATA_HOME/prlg/history, quit. is not recorded. Checkpoints 2, 3, 5, 6 from docs/design/REPL.md are demonstrably working in line mode; 1 and 4 are the pending TUI work, as declared.

Design conformance is good: vim-line pinned =7.5.7 per the doc's clone-or-depend decision, history-as-only-persistent-state per Constraints, fresh solver per goal per Domain Events, STDLIB_PL consumed from core (the #36 lesson applied), and pkg-check packages the new crate. The Session/Query self-referential problem is honestly flagged in app.rs rather than hacked around.

🔴 Blocking

Nothing. Incomplete is the declared state, and nothing here breaks a release if one fired today — with one caveat in 🟡 1.

🟡 Worth addressing (small, ideally before merge)

README Status section is stale and self-contradictory. It says "Running the binary today produces a placeholder line and exits — the engine is exercised via cargo test," but main.rs ships a working line-mode REPL (the PR description even shows its transcript). If a release tag fires before the TUI lands, this is the crates.io-facing description. One-paragraph fix.
Test race on XDG_DATA_HOME. history::tests::xdg_data_home_is_honored_when_set mutates the process env (unsafe set_var) while app::tests::app_constructs_from_session_with_empty_transcript concurrently calls App::new → default_path() → history::load(). Today it's benign (recalled isn't asserted), but it's a latent flake — and the app test also reads the developer's real ~/.local/share/prlg/history. Cleanest fix: give App a with_history_path(Option<PathBuf>) constructor for tests, which the TUI loop will likely want anyway.
ingest() silently drops future directives. It manually extends directives.dynamic. Destructure instead — let ProgramDirectives { dynamic } = d; — so adding a field to ProgramDirectives in core becomes a compile error here rather than a silent drop.
Three declared-but-unused deps (vim-line, ratatui, crossterm). Fine if the TUI lands before the next release; if not, installers compile the full TUI stack for a line-mode binary. Note unused_crate_dependencies is allow-by-default, so clippy -D warnings won't flag this.
record_history records goals that failed to parse. Plausibly intentional (recall-to-fix-the-typo is good UX) — if so, a one-line comment; if not, move the call inside the Ok arm.

🟢 Minor / for the next iteration

On the deferred Session/Query question: rerun-and-skip is correct here (goals are side-effect-free) but O(n²) across ; presses. self_cell works; a third option worth weighing is changing core's Solver to hold Arc<CompiledDatabase> instead of &db, which dissolves the lifetime entirely and simplifies every embedder.
stream_solutions prompts ; or . > even for zero-binding true. solutions; SWI auto-commits deterministic goals. Needs one-solution lookahead — fine to defer.
history::append flattens \n but not \r — a \r\n goal leaves a stray CR on the line.
Query isn't fused — next() after Done/Error is documented as "don't," but unguarded. Cheap to fuse when the TUI loop makes the call pattern less linear.
step_limit_propagates_to_solver asserts any Error — already acknowledged in the PR description; tighten once the resource_error vs existence_error shape question is resolved.
Observed in the smoke run, core not this PR: errors render as error(type_error(evaluable, foo), Cannot evaluate as arithmetic) — unquoted prose as the second arg of the error term reads oddly.

Verdict

Good checkpoint — honest about what's stubbed, the engine/UI seam is in the right place, and everything claimed is reproducible. Items 1–3 are each a few minutes and worth folding in before merge; the rest can ride with the TUI iteration.

Review by Claude Code

# Review **What it does:** Scaffolds `patch-prolog-repl` (binary `prlg-repl`): a UI-free engine driver (`Session`/`Query`/`Step`), XDG-aware persistent history, `App`/`Action` skeletons for the coming ratatui loop, and a working line-mode driver. Workflow/justfile extended for the fourth crate. **Verified locally** (scratch copy of the branch): all 11 tests pass, and the end-to-end smoke works exactly as the PR description claims — `member(X, [a,b,c]).` streams a/b/c on `;`, `X is foo.` surfaces `type_error(evaluable, foo)` and returns to the prompt, history file lands under `$XDG_DATA_HOME/prlg/history`, `quit.` is not recorded. Checkpoints 2, 3, 5, 6 from `docs/design/REPL.md` are demonstrably working in line mode; 1 and 4 are the pending TUI work, as declared. **Design conformance is good:** vim-line pinned `=7.5.7` per the doc's clone-or-depend decision, history-as-only-persistent-state per Constraints, fresh solver per goal per Domain Events, `STDLIB_PL` consumed from core (the #36 lesson applied), and `pkg-check` packages the new crate. The Session/Query self-referential problem is honestly flagged in `app.rs` rather than hacked around. ## 🔴 Blocking Nothing. Incomplete is the declared state, and nothing here breaks a release if one fired today — with one caveat in 🟡 1. ## 🟡 Worth addressing (small, ideally before merge) 1. **README Status section is stale and self-contradictory.** It says "Running the binary today produces a placeholder line and exits — the engine is exercised via `cargo test`," but `main.rs` ships a working line-mode REPL (the PR description even shows its transcript). If a release tag fires before the TUI lands, this is the crates.io-facing description. One-paragraph fix. 2. **Test race on `XDG_DATA_HOME`.** `history::tests::xdg_data_home_is_honored_when_set` mutates the process env (`unsafe set_var`) while `app::tests::app_constructs_from_session_with_empty_transcript` concurrently calls `App::new` → `default_path()` → `history::load()`. Today it's benign (`recalled` isn't asserted), but it's a latent flake — and the app test also reads the developer's *real* `~/.local/share/prlg/history`. Cleanest fix: give `App` a `with_history_path(Option<PathBuf>)` constructor for tests, which the TUI loop will likely want anyway. 3. **`ingest()` silently drops future directives.** It manually extends `directives.dynamic`. Destructure instead — `let ProgramDirectives { dynamic } = d;` — so adding a field to `ProgramDirectives` in core becomes a compile error here rather than a silent drop. 4. **Three declared-but-unused deps** (`vim-line`, `ratatui`, `crossterm`). Fine if the TUI lands before the next release; if not, installers compile the full TUI stack for a line-mode binary. Note `unused_crate_dependencies` is allow-by-default, so `clippy -D warnings` won't flag this. 5. **`record_history` records goals that failed to parse.** Plausibly intentional (recall-to-fix-the-typo is good UX) — if so, a one-line comment; if not, move the call inside the `Ok` arm. ## 🟢 Minor / for the next iteration - On the deferred Session/Query question: rerun-and-skip is *correct* here (goals are side-effect-free) but O(n²) across `;` presses. `self_cell` works; a third option worth weighing is changing core's `Solver` to hold `Arc<CompiledDatabase>` instead of `&db`, which dissolves the lifetime entirely and simplifies every embedder. - `stream_solutions` prompts `; or . >` even for zero-binding `true.` solutions; SWI auto-commits deterministic goals. Needs one-solution lookahead — fine to defer. - `history::append` flattens `\n` but not `\r` — a `\r\n` goal leaves a stray CR on the line. - `Query` isn't fused — `next()` after `Done`/`Error` is documented as "don't," but unguarded. Cheap to fuse when the TUI loop makes the call pattern less linear. - `step_limit_propagates_to_solver` asserts any `Error` — already acknowledged in the PR description; tighten once the `resource_error` vs `existence_error` shape question is resolved. - Observed in the smoke run, core not this PR: errors render as `error(type_error(evaluable, foo), Cannot evaluate as arithmetic)` — unquoted prose as the second arg of the error term reads oddly. ## Verdict Good checkpoint — honest about what's stubbed, the engine/UI seam is in the right place, and everything claimed is reproducible. Items 1–3 are each a few minutes and worth folding in before merge; the rest can ride with the TUI iteration. --- *Review by Claude Code*

navicore added 1 commit

2026-06-04 06:01:14 +00:00

● All four addressed; just ci green.

CI - Linux / CI - Linux x86_64 (pull_request) Successful in 54s

Details

816449f852

- 🟡 1 README Status now describes the working line-mode REPL (goal/;/RET
  semantics, history path) instead of "placeholder and exits."
  - 🟡 2 App::with_history_path(session, Option<PathBuf>) added; App::new
  delegates. The app test now passes None, so it no longer reads the
  developer's real history file and cannot race the XDG_DATA_HOME
  env-mutation test.
  - 🟡 3 engine::ingest now destructures ProgramDirectives { dynamic }. A
  future field upstream becomes a compile error here.
  - 🟡 5 Inline comment at the record_history(goal) call site noting that
  recording failed-parse goals is intentional (SWI-style up-arrow
  typo-recall).