navicore/seq-lisp

Fork 0

Phase 3 delivered: #67

Merged

navicore merged 4 commits from self-hosting-io-and-load into main

2026-03-28 00:03:03 +00:00

navicore commented

2026-03-27 23:15:17 +00:00

(Migrated from github.com)

read-file, write-file, file-exists? — new Seq-side builtins in eval.seq wrapping file.slurp, file.spit, file.exists?
lib/reader/load.slisp — (load path) reads a file and evaluates all expressions via eval-all-with-errors
7 file I/O tests in the main suite, 5 loader tests in the reader suite
Checkpoint met: (load "examples/factorial.slisp") runs and prints output
Registered in dispatch chain and suggestion list for autocomplete

Note: load evaluates in the host's environment, so definitions from loaded files aren't accessible to the caller. This will be naturally resolved in Phase
4 when we build the self-hosted evaluator.

- read-file, write-file, file-exists? — new Seq-side builtins in eval.seq wrapping file.slurp, file.spit, file.exists? - lib/reader/load.slisp — (load path) reads a file and evaluates all expressions via eval-all-with-errors - 7 file I/O tests in the main suite, 5 loader tests in the reader suite - Checkpoint met: (load "examples/factorial.slisp") runs and prints output - Registered in dispatch chain and suggestion list for autocomplete Note: load evaluates in the host's environment, so definitions from loaded files aren't accessible to the caller. This will be naturally resolved in Phase 4 when we build the self-hosted evaluator.

claude[bot] commented

2026-03-27 23:19:15 +00:00

(Migrated from github.com)

PR Review: Phase 3 - File I/O Builtins and Self-Hosted Loader

Good progress on Phase 3. The implementation is generally solid, but there are a few issues worth addressing before merge.

Critical: Accidental artifact file committed

A file named 'hello from seqlisp' (with spaces) was accidentally committed. Its contents are '/tmp/seqlisp-test.txt' - clearly a test artifact that should not be in the repo. Remove before merge.

Bug: Broken test - load file with syntax error

In tests/lisp/reader/load.slisp, the test writes the file after trying to load it. The load call is in the let-result binding, but the file does not exist at that point - this always fails with file-not-found, not a syntax error. The test also unconditionally returns true, so it always passes regardless of actual behavior. Fix: write the file first, then assert-error on the load.

Minor: write-file evaluates content before path

In eval-write-file-with-env, the stack manipulation evaluates content before path. Lisp convention is left-to-right evaluation, so users expect path evaluated first. No practical impact for string-literal args, but inconsistent with convention.

Minor: No test for write-file permission failure

The suite covers the happy path and read-of-nonexistent-file, but has no test for write-file failing (e.g., writing to a read-only path). Worth adding if file.spit surfaces a meaningful error in that case.

Minor: Temp files not cleaned up

Tests write to /tmp but never remove files. The file-exists-false assertion relies on /tmp/seqlisp-nonexistent-file.txt not existing - a prior run that created that file would cause a spurious failure.

Implementation looks correct

Stack manipulation in eval-read-file-with-env and eval-file-exists-with-env follows existing patterns cleanly. The load function in lib/reader/load.slisp is minimal and clear. The +3 then count in the dispatch chain is correct. The acknowledged environment-isolation limitation of load is reasonable for this phase.

Severity	Issue
Must fix	Remove 'hello from seqlisp' artifact file
Must fix	Fix broken logic in load syntax-error test
Nice to fix	Evaluate path before content in write-file
Optional	Add negative test for write-file permission failure
Optional	Clean up temp files after tests

PR Review: Phase 3 - File I/O Builtins and Self-Hosted Loader Good progress on Phase 3. The implementation is generally solid, but there are a few issues worth addressing before merge. --- **Critical: Accidental artifact file committed** A file named 'hello from seqlisp' (with spaces) was accidentally committed. Its contents are '/tmp/seqlisp-test.txt' - clearly a test artifact that should not be in the repo. Remove before merge. --- **Bug: Broken test - load file with syntax error** In tests/lisp/reader/load.slisp, the test writes the file _after_ trying to load it. The load call is in the let-result binding, but the file does not exist at that point - this always fails with file-not-found, not a syntax error. The test also unconditionally returns true, so it always passes regardless of actual behavior. Fix: write the file first, then assert-error on the load. --- **Minor: write-file evaluates content before path** In eval-write-file-with-env, the stack manipulation evaluates content before path. Lisp convention is left-to-right evaluation, so users expect path evaluated first. No practical impact for string-literal args, but inconsistent with convention. --- **Minor: No test for write-file permission failure** The suite covers the happy path and read-of-nonexistent-file, but has no test for write-file failing (e.g., writing to a read-only path). Worth adding if file.spit surfaces a meaningful error in that case. --- **Minor: Temp files not cleaned up** Tests write to /tmp but never remove files. The file-exists-false assertion relies on /tmp/seqlisp-nonexistent-file.txt not existing - a prior run that created that file would cause a spurious failure. --- **Implementation looks correct** Stack manipulation in eval-read-file-with-env and eval-file-exists-with-env follows existing patterns cleanly. The load function in lib/reader/load.slisp is minimal and clear. The +3 then count in the dispatch chain is correct. The acknowledged environment-isolation limitation of load is reasonable for this phase. | Severity | Issue | |----------|-------| | Must fix | Remove 'hello from seqlisp' artifact file | | Must fix | Fix broken logic in load syntax-error test | | Nice to fix | Evaluate path before content in write-file | | Optional | Add negative test for write-file permission failure | | Optional | Clean up temp files after tests |

claude[bot] commented

2026-03-27 23:52:34 +00:00

(Migrated from github.com)

Code Review: Phase 3 — File I/O Builtins & Self-Hosted Loader

Overall this is a solid, well-tested addition. The implementation is consistent with existing patterns in eval.seq, and the test coverage is thorough. A few issues need attention before merging.

🔴 Must Fix

Accidentally committed file: hello from seqlisp

A file named hello from seqlisp (with spaces) was added to the repo root. Its contents are just the string /tmp/seqlisp-test.txt — this looks like a leftover from manual testing that was accidentally staged. It must be removed before merging.

🟡 Potential Bugs

eval-write-file-with-env: argument evaluation order vs. file.spit argument order

The implementation evaluates content before path, leaving the stack as ContentStr PathStr when file.spit is called (PathStr on top). This only works correctly if file.spit expects ( content path -- bool ) with path on top of stack. If the convention is ( path content -- bool ), the arguments are swapped and every write-file call silently writes to the wrong location.

This is worth verifying against the Seq stdlib definition of file.spit, since the bug would be silent (the call succeeds but writes to the content string interpreted as a path).

Empty if branch in eval-read-file-with-env and eval-file-exists-with-env

dup eval-err? if
else
  ...
then

When eval-err? is true, control falls through the empty if branch and the error result is left on the stack implicitly. While this probably works (the error propagates), the empty branch is fragile and easy to misread. The existing pattern elsewhere uses explicit handling. Consider:

dup eval-err? if
  nip  ; drop the env/arg, keep the error
else
  ...
then

🟡 Design / Consistency

Asymmetric error handling in load

load has two distinct failure modes with different shapes:

read-file fails → returns an EvalResult error (caught by ok? / assert-error)
eval-all-with-errors fails → returns a raw (error msg ...) list (requires (car result) inspection)

This is confirmed by the test using (assert-eq (car result) 'error) rather than (assert-error ...). Callers must know to handle both shapes. A comment in lib/reader/load.slisp documenting this contract would help — or, if eval-all-with-errors can be wrapped into an EvalResult, the API would be more uniform.

🟢 Minor / Suggestions

Test files hardcode /tmp/ paths with no cleanup

The tests write 5+ files to /tmp/ and leave them behind. This is fine for local runs, but in CI or on macOS (where /tmp is a symlink to /private/tmp) it could cause subtle issues. Consider:

Checking that file.spit/file.slurp work correctly through symlinks in the CI environment
Adding a note or a teardown step if the test harness supports it

eval-write-file-with-env stack manipulation is dense

The argument-juggling section (dup rot swap, rot rot, etc.) is harder to follow than the single-argument builtins. A few inline stack-effect comments at key points would make it easier to audit and maintain.

Parenthesis count in tests/lisp/all.slisp

The closing paren count was bumped from )))))))))))))))))))))))) to )))))))))))))))))))))))) — this looks correct to match the added (append file-io-tests, but worth double-checking the nesting depth is balanced.

✅ What's Working Well

Consistent use of check-arity and the existing dispatch pattern
file-exists? correctly returns #t/#f symbols rather than propagating an error for the "not found" case — that's the right behavior
Test coverage is comprehensive: roundtrip, overwrite, multiline, error on missing file, and return value assertions
The self-hosted load is clean and minimal
PR notes clearly document the known Phase 4 limitation around loaded definitions not persisting to the caller scope

## Code Review: Phase 3 — File I/O Builtins & Self-Hosted Loader Overall this is a solid, well-tested addition. The implementation is consistent with existing patterns in `eval.seq`, and the test coverage is thorough. A few issues need attention before merging. --- ### 🔴 Must Fix **Accidentally committed file: `hello from seqlisp`** A file named `hello from seqlisp` (with spaces) was added to the repo root. Its contents are just the string `/tmp/seqlisp-test.txt` — this looks like a leftover from manual testing that was accidentally staged. It must be removed before merging. --- ### 🟡 Potential Bugs **`eval-write-file-with-env`: argument evaluation order vs. `file.spit` argument order** The implementation evaluates `content` before `path`, leaving the stack as `ContentStr PathStr` when `file.spit` is called (PathStr on top). This only works correctly if `file.spit` expects `( content path -- bool )` with path on top of stack. If the convention is `( path content -- bool )`, the arguments are swapped and every `write-file` call silently writes to the wrong location. This is worth verifying against the Seq stdlib definition of `file.spit`, since the bug would be silent (the call succeeds but writes to the content string interpreted as a path). **Empty `if` branch in `eval-read-file-with-env` and `eval-file-exists-with-env`** ```seq dup eval-err? if else ... then ``` When `eval-err?` is true, control falls through the empty `if` branch and the error result is left on the stack implicitly. While this probably works (the error propagates), the empty branch is fragile and easy to misread. The existing pattern elsewhere uses explicit handling. Consider: ```seq dup eval-err? if nip ; drop the env/arg, keep the error else ... then ``` --- ### 🟡 Design / Consistency **Asymmetric error handling in `load`** `load` has two distinct failure modes with different shapes: - `read-file` fails → returns an `EvalResult` error (caught by `ok?` / `assert-error`) - `eval-all-with-errors` fails → returns a raw `(error msg ...)` list (requires `(car result)` inspection) This is confirmed by the test using `(assert-eq (car result) 'error)` rather than `(assert-error ...)`. Callers must know to handle both shapes. A comment in `lib/reader/load.slisp` documenting this contract would help — or, if `eval-all-with-errors` can be wrapped into an `EvalResult`, the API would be more uniform. --- ### 🟢 Minor / Suggestions **Test files hardcode `/tmp/` paths with no cleanup** The tests write 5+ files to `/tmp/` and leave them behind. This is fine for local runs, but in CI or on macOS (where `/tmp` is a symlink to `/private/tmp`) it could cause subtle issues. Consider: - Checking that `file.spit`/`file.slurp` work correctly through symlinks in the CI environment - Adding a note or a teardown step if the test harness supports it **`eval-write-file-with-env` stack manipulation is dense** The argument-juggling section (`dup rot swap`, `rot rot`, etc.) is harder to follow than the single-argument builtins. A few inline stack-effect comments at key points would make it easier to audit and maintain. **Parenthesis count in `tests/lisp/all.slisp`** The closing paren count was bumped from `))))))))))))))))))))))))` to `))))))))))))))))))))))))` — this looks correct to match the added `(append file-io-tests`, but worth double-checking the nesting depth is balanced. --- ### ✅ What's Working Well - Consistent use of `check-arity` and the existing dispatch pattern - `file-exists?` correctly returns `#t`/`#f` symbols rather than propagating an error for the "not found" case — that's the right behavior - Test coverage is comprehensive: roundtrip, overwrite, multiline, error on missing file, and return value assertions - The self-hosted `load` is clean and minimal - PR notes clearly document the known Phase 4 limitation around loaded definitions not persisting to the caller scope

claude[bot] commented

2026-03-28 00:04:05 +00:00

(Migrated from github.com)

Code Review: Phase 3 — File I/O Builtins & Self-hosted Loader

Overall this is a clean, well-scoped addition. The implementation is straightforward and the tests cover the important cases. A few things worth discussing:

Argument Evaluation Order in `write-file`

In eval-write-file-with-env, content is evaluated before path (see the comment "Evaluate content first"). Standard Lisp convention evaluates arguments left-to-right. This reversal won't matter for side-effect-free expressions, but it's a semantic inconsistency worth noting. Consider whether left-to-right order is achievable without adding significant stack complexity, or at least add a comment explicitly calling out the deviation.

Asymmetric Error Handling in `load`

The error contract documented in lib/reader/load.slisp is accurate but creates a real footgun:

(load "missing.slisp")     ; → evaluator error (catchable with try)
(load "bad-syntax.slisp")  ; → returns (error ...) as *data* (NOT catchable with try)

Callers cannot uniformly handle errors from load — they need different strategies depending on whether the failure was at the file-read or eval stage. This is understandable given eval-all-with-errors's contract, and the comment flags that Phase 4 will unify it. But the test "load file with eval error returns error list" implicitly validates this asymmetry as correct behavior. It might be worth naming that test more explicitly (e.g., "load eval error returns data not exception") so future readers understand this is a known asymmetry, not an oversight.

Test Isolation

All tests write to predictable /tmp paths:

"/tmp/seqlisp-io-test.txt"
"/tmp/seqlisp-nonexistent-file.txt"   ; assumed to not exist

The file-exists-false test will fail flakily if a previous run of the test suite was interrupted before cleanup — or if two test suite runs happen in parallel. Minor concern for CI, but worth a thought. Also, none of the test files are cleaned up, which accumulates noise in /tmp over repeated runs.

`file-exists-false` Fragility

(test 'file-exists-false
  (assert-false (file-exists? "/tmp/seqlisp-nonexistent-file.txt")))

This relies on a file not existing, but /tmp/seqlisp-nonexistent-file.txt is a perfectly plausible path that could exist on a shared CI runner. A safer pattern would be to write a file, delete it, then assert it no longer exists — though this requires a delete-file builtin that doesn't exist yet. Alternatively, use a more unique path (UUID-based or process-id-based). Low priority, but worth flagging.

Stack Manipulation Readability in `eval-write-file-with-env`

The write-file implementation has several multi-step stack dances (rot rot, dup rot swap) that are hard to follow without careful annotation. The existing comments help, but intermediate stack-state comments would make future maintenance easier:

rot rot       # -> ContentStr PathExpr Env
eval-with-env # -> ContentStr PathResult

Consider adding ( -- stack-state ) annotations at the tricky spots. The read-file and file-exists? implementations are much simpler and easy to follow.

Minor: `then then then` count

After adding the three new file I/O dispatches, the dispatch chain now has:

  then then then then then then then then then

Nine thens — this is already fragile to count. Not blocking, but a local word or a comment marking the count would help catch off-by-one errors when the next builtin is added.

What's Good

The three builtins have clear, consistent error messages.
The arity check pattern (check-arity + early return on error) matches the existing codebase conventions.
load correctly propagates file-read errors upward and returns eval errors as data per eval-all-with-errors contract.
Test coverage is solid: write-then-read round trip, overwrite, multiline, missing file, returns-true — all the basics are covered.
The known limitation (definitions from loaded files don't persist to caller scope) is clearly documented with a forward pointer to Phase 4.

Good work overall. The main things to address before merge are the arg-evaluation order question and test isolation. The asymmetric error handling in load is an acceptable Phase 3 trade-off as long as the asymmetry stays documented.

## Code Review: Phase 3 — File I/O Builtins & Self-hosted Loader Overall this is a clean, well-scoped addition. The implementation is straightforward and the tests cover the important cases. A few things worth discussing: --- ### Argument Evaluation Order in `write-file` In `eval-write-file-with-env`, content is evaluated **before** path (see the comment "Evaluate content first"). Standard Lisp convention evaluates arguments left-to-right. This reversal won't matter for side-effect-free expressions, but it's a semantic inconsistency worth noting. Consider whether left-to-right order is achievable without adding significant stack complexity, or at least add a comment explicitly calling out the deviation. --- ### Asymmetric Error Handling in `load` The error contract documented in `lib/reader/load.slisp` is accurate but creates a real footgun: ```lisp (load "missing.slisp") ; → evaluator error (catchable with try) (load "bad-syntax.slisp") ; → returns (error ...) as *data* (NOT catchable with try) ``` Callers cannot uniformly handle errors from `load` — they need different strategies depending on whether the failure was at the file-read or eval stage. This is understandable given `eval-all-with-errors`'s contract, and the comment flags that Phase 4 will unify it. But the test `"load file with eval error returns error list"` implicitly validates this asymmetry as correct behavior. It might be worth naming that test more explicitly (e.g., `"load eval error returns data not exception"`) so future readers understand this is a *known* asymmetry, not an oversight. --- ### Test Isolation All tests write to predictable `/tmp` paths: ```lisp "/tmp/seqlisp-io-test.txt" "/tmp/seqlisp-nonexistent-file.txt" ; assumed to not exist ``` The `file-exists-false` test will fail flakily if a previous run of the test suite was interrupted before cleanup — or if two test suite runs happen in parallel. Minor concern for CI, but worth a thought. Also, none of the test files are cleaned up, which accumulates noise in `/tmp` over repeated runs. --- ### `file-exists-false` Fragility ```lisp (test 'file-exists-false (assert-false (file-exists? "/tmp/seqlisp-nonexistent-file.txt"))) ``` This relies on a file not existing, but `/tmp/seqlisp-nonexistent-file.txt` is a perfectly plausible path that could exist on a shared CI runner. A safer pattern would be to write a file, delete it, then assert it no longer exists — though this requires a `delete-file` builtin that doesn't exist yet. Alternatively, use a more unique path (UUID-based or process-id-based). Low priority, but worth flagging. --- ### Stack Manipulation Readability in `eval-write-file-with-env` The write-file implementation has several multi-step stack dances (`rot rot`, `dup rot swap`) that are hard to follow without careful annotation. The existing comments help, but intermediate stack-state comments would make future maintenance easier: ```seq rot rot # -> ContentStr PathExpr Env eval-with-env # -> ContentStr PathResult ``` Consider adding `( -- stack-state )` annotations at the tricky spots. The read-file and file-exists? implementations are much simpler and easy to follow. --- ### Minor: `then then then` count After adding the three new file I/O dispatches, the dispatch chain now has: ```seq then then then then then then then then then ``` Nine `then`s — this is already fragile to count. Not blocking, but a local word or a comment marking the count would help catch off-by-one errors when the next builtin is added. --- ### What's Good - The three builtins have clear, consistent error messages. - The arity check pattern (`check-arity` + early return on error) matches the existing codebase conventions. - `load` correctly propagates file-read errors upward and returns eval errors as data per `eval-all-with-errors` contract. - Test coverage is solid: write-then-read round trip, overwrite, multiline, missing file, returns-true — all the basics are covered. - The known limitation (definitions from loaded files don't persist to caller scope) is clearly documented with a forward pointer to Phase 4. Good work overall. The main things to address before merge are the arg-evaluation order question and test isolation. The asymmetric error handling in `load` is an acceptable Phase 3 trade-off as long as the asymmetry stays documented.

claude[bot] commented

2026-03-28 00:12:46 +00:00

(Migrated from github.com)

Code Review: Phase 3 — File I/O Builtins & Self-hosted Loader

Overall this is a solid, well-structured addition. The stack manipulation is dense but appears correct, and the error documentation in load.slisp is a nice touch. A few items worth noting:

Bug / Behavioral Issue

write-file evaluates arguments right-to-left (eval.seq ~line 2430)

Due to stack management, content is evaluated before path:

rot                       # -> PathExpr ContentExpr Env
dup rot swap              # -> PathExpr Env ContentExpr Env
eval-with-env             # evaluates ContentExpr first

This deviates from the left-to-right evaluation order that most Lisps guarantee. For pure expressions it's invisible, but (write-file (side-effecting-path) (side-effecting-content)) will evaluate content before path. If the evaluator ever documents evaluation order guarantees, this will need to be consistent or explicitly noted.

Error Message Quality

Both read-file and write-file produce opaque errors:

"read-file: could not read file"
"write-file: could not write file"

Including the path in the message (e.g., "read-file: could not read file: /some/path") would make debugging significantly easier, especially when load calls read-file internally and the error propagates up.

Test Isolation Concerns (`tests/lisp/core/file_io.slisp`)

file-exists-false is flaky: It asserts (file-exists? "/tmp/seqlisp-nonexistent-file.txt") returns #f, but there's no guarantee that file doesn't exist (e.g., left over from a crashed run). A more uniquely named or randomly-suffixed path would be safer.
No cleanup: Test files in /tmp/ accumulate across runs. For CI this is fine, but consider whether the test suite should clean up after itself.
Missing edge cases:
- (write-file "/tmp/..." "") — empty string content
- (write-file (string-append "/tmp/" "test.txt") "data") — path via expression (would also exercise the evaluation order issue above)
- (write-file 42 "data") — non-string path at eval time (not just a literal)

Minor: Silent empty branch pattern

In eval-read-file-with-env and eval-file-exists-with-env:

dup eval-err? if
  # Error evaluating path - propagate
else

The "propagate" comment is doing heavy lifting — the intent is that the error stays on the stack and naturally falls through. This is idiomatic Forth, but the comment could be clearer: # leave EvalResult on stack; falls through to ;.

Acknowledged Debt

The asymmetry in load.slisp (file error → evaluator error; eval error → error-list data) is clearly documented with a Phase 4 note. The test "load file with eval error returns error list" correctly covers this behavior. No action needed here, just confirming the documentation is sufficient.

Dispatch Chain Length

The then then then then then then then then then chain is getting long. Not a blocker, but as builtins accumulate this O(n) string comparison dispatch may be worth addressing (e.g., a lookup table or hash dispatch) before it becomes painful. This isn't new to this PR — just worth flagging as accumulating tech debt.

Good coverage on the happy-path tests and error cases. The load tests in reader/load.slisp are particularly thorough. Nice work on Phase 3.

## Code Review: Phase 3 — File I/O Builtins & Self-hosted Loader Overall this is a solid, well-structured addition. The stack manipulation is dense but appears correct, and the error documentation in `load.slisp` is a nice touch. A few items worth noting: --- ### Bug / Behavioral Issue **`write-file` evaluates arguments right-to-left** (`eval.seq` ~line 2430) Due to stack management, content is evaluated before path: ```seq rot # -> PathExpr ContentExpr Env dup rot swap # -> PathExpr Env ContentExpr Env eval-with-env # evaluates ContentExpr first ``` This deviates from the left-to-right evaluation order that most Lisps guarantee. For pure expressions it's invisible, but `(write-file (side-effecting-path) (side-effecting-content))` will evaluate content before path. If the evaluator ever documents evaluation order guarantees, this will need to be consistent or explicitly noted. --- ### Error Message Quality Both `read-file` and `write-file` produce opaque errors: ``` "read-file: could not read file" "write-file: could not write file" ``` Including the path in the message (e.g., `"read-file: could not read file: /some/path"`) would make debugging significantly easier, especially when `load` calls `read-file` internally and the error propagates up. --- ### Test Isolation Concerns (`tests/lisp/core/file_io.slisp`) 1. **`file-exists-false` is flaky**: It asserts `(file-exists? "/tmp/seqlisp-nonexistent-file.txt")` returns `#f`, but there's no guarantee that file doesn't exist (e.g., left over from a crashed run). A more uniquely named or randomly-suffixed path would be safer. 2. **No cleanup**: Test files in `/tmp/` accumulate across runs. For CI this is fine, but consider whether the test suite should clean up after itself. 3. **Missing edge cases**: - `(write-file "/tmp/..." "")` — empty string content - `(write-file (string-append "/tmp/" "test.txt") "data")` — path via expression (would also exercise the evaluation order issue above) - `(write-file 42 "data")` — non-string path at eval time (not just a literal) --- ### Minor: Silent empty branch pattern In `eval-read-file-with-env` and `eval-file-exists-with-env`: ```seq dup eval-err? if # Error evaluating path - propagate else ``` The "propagate" comment is doing heavy lifting — the intent is that the error stays on the stack and naturally falls through. This is idiomatic Forth, but the comment could be clearer: `# leave EvalResult on stack; falls through to ;`. --- ### Acknowledged Debt The asymmetry in `load.slisp` (file error → evaluator error; eval error → error-list data) is clearly documented with a Phase 4 note. The test `"load file with eval error returns error list"` correctly covers this behavior. No action needed here, just confirming the documentation is sufficient. --- ### Dispatch Chain Length The `then then then then then then then then then` chain is getting long. Not a blocker, but as builtins accumulate this O(n) string comparison dispatch may be worth addressing (e.g., a lookup table or hash dispatch) before it becomes painful. This isn't new to this PR — just worth flagging as accumulating tech debt. --- Good coverage on the happy-path tests and error cases. The `load` tests in `reader/load.slisp` are particularly thorough. Nice work on Phase 3.