Fix: pr to a file stream must ignore *hush* on ECL and CLISP

[pyrex41]

Summary

Under -q (which sets *hush* true), (pr STR FileStream) was silenced on the ECL and CLISP builds, producing a zero-byte file, while SBCL wrote it. The kernel's KL pr (writer.kl) returns early without writing whenever *hush* is true:

(defun pr (V6806 V6807) (if (value *hush*) V6806 ...))

SBCL/CCL already bypassed this with a native write-string override (src/overwrite.lsp), so their pr ignores *hush*. ECL and CLISP ran the KL pr and so dropped the write under -q.

Fix

src/overwrite.lsp:

• Extend the existing native pr override to ECL (its file streams are character streams, so the same write-string path applies).
• Add a CLISP sibling override. CLISP opens file streams with :element-type 'unsigned-byte (see |shen.openh| in src/primitives.lsp), so write-string is illegal on them; the kernel KL pr dispatches such streams to a write-byte path. The override mirrors that dispatch (byte streams → write-byte, character streams → write-string) but, like the SBCL/CCL/ECL override, never consults *hush*.

All four impls now agree: pr writes to its target stream regardless of *hush*.

Test changes

• scripts/test-cli.sh: the -q pr-to-file check now asserts the same corrected behaviour for every impl (previously it recorded clisp/ecl as silencing).
• tests/io-tests.shen: adds regression "pr to a file stream ignores *hush* (issue #2)" and updates the locked-in divergence comment.

Results

Rebuilt sbcl + clisp + ecl images.

• Canonical SBCL kernel cert (make test-sbcl): 134/134, 100%
• Compiler golden tests (make test-compiler): pass
• Port runtime suite (make test-port): 127/127
• CLI parity (make test-cli, sbcl+clisp+ecl): 47/47, 0 failed — all three impls now WRITE the -q file

Fixes #2

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

[pyrex41]

Independent agent review (ratatoskr + ShenSpec deep-dive, post-load native override discipline, 134/134 cert preserved)

Code Review: #4

"Fix: pr to a file stream must ignore hush on ECL and CLISP"

PR URL: #4
Base: kernel-41.2
Changed files (from PR metadata + diff): scripts/test-cli.sh, src/overwrite.lsp, tests/io-tests.shen
Review date: 2026-06-13 (independent verification pass)
Instructions followed: Read the provided unified diff; read shen-cl/src/overwrite.lsp (pre-state |pr| and char-st*? / write-string defs), shen-cl/src/primitives.lsp (|stoutput|, |stinput|, |shen.openh| element-type logic for clisp), shen-cl/scripts/test-cli.sh, shen-cl/tests/io-tests.shen; shen-cl/AGENT.md + build scripts (boot.lsp, Makefile); deep-dive ratatoskr/README.md (hush caveat, shen-cl reference host, native pr override notes for lisp builder), ratatoskr/KLambda/writer.kl (exact (if (value *hush*) ... (if (shen.char-stoutput? ...) shen.write-string else write-chars)) + supporting write-chars/string->byte), ratatoskr/KLambda/sys.kl (hush?, stinput/stoutput, write-to-file etc.), ratatoskr/builders/lisp/{build.sh,build.shen,driver.lsp} (overwrite load order, ratatoskr-install-stream-fallbacks, ecl epilogue, notes on #+(or ccl sbcl) and why fallbacks exist). Also cross-checked shen-cl/boot.lsp load order (overwrite after kernel writer), primitives char-st*? usage, run-port-tests.shen + test-harness.shen (runner hush true + local clears), and io-tests + cli parity matrix as the executable ShenSpec.

Key context applied:

• Root cause is always in the vendored kernel (ratatoskr/KLambda/writer.kl:3): (defun pr (V6806 V6807) (if (value *hush*) V6806 (if (shen.char-stoutput? V6807) (shen.write-string V6806 V6807) (shen.write-chars ...)))). The early return happens for any target stream (file or stoutput) when -q sets hush.
• char-stoutput?/char-stinput? (defined in shen-cl/src/overwrite.lsp:31 and :36) + shen.openh (primitives.lsp:227) are the dispatch: clisp forces 'unsigned-byte for file streams from (open ... out); non-clisp use :default (character on sbcl/ecl/ccl). This is why only shen-cl previously had a partial native |pr| override (sbcl/ccl only) and why extending it is the canonical-port fix pattern (see ratatoskr/README:144 and driver.lsp:75).
• SBCL/CCL always took the write-string path for shen streams (their openh + st are character); the override simply made that path also ignore hush. ECL previously fell through to KL pr (hence silenced files under -q). CLISP stoutput/stinput are rebound to unsigned-byte streams in primitives.lsp:443 (clisp toplevel) and its openh files are bytes, so KL pr took write-chars/write-byte.
• ShenSpec: io-tests.shen + scripts/test-cli.sh (the 47/47 cli parity matrix exercising -q pr-to-file + write-byte on sbcl+clisp+ecl) are the executable cross-Lisp + cross-port contract. Old comments explicitly "locked" the divergence; the PR unifies it.
• Requirements verified from instructions: clisp |pr| dispatches on (subtypep (stream-element-type s) 'character) and never calls write-string on byte streams; force-output is only inside the char branch and only for |stoutput| / |stinput| (not files, not byte st); hush / (value hush) is never consulted in any override; right-hand side of diff used for citations; full ## Summary + ## Issues.

All four hush issues share the same upstream writer.kl gate. Strategy (native |pr| override after kernel load in boot.lsp:171 and driver.lsp:139) is consistent with prior partial fix for sbcl/ccl. Port suites + certs stayed green per PR metadata.

Summary of Changes (Correctness First)

src/overwrite.lsp (core fix):

• Extended the existing native bypass #+(or ccl sbcl) |pr| (write-string + conditional force-output for st streams only) to include ecl (right-side: lines ~56-70 in the diff hunk). ECL file streams from |shen.openh| are character streams, so write-string is legal and matches the KL pr's char-stoutput? = true path — but now without the hush gate.
• Added #+clisp sibling |pr| (right-side: ~78-86) that does exactly the element-type dispatch the KL pr + char-stoutput? + write-chars did, but unconditionally (no hush test) and returns the string. Byte branch: (loop for c across x do (write-byte (char-code c) s)). Char branch: write-string + force only for |st| (never for files). This exactly mirrors primitives.lsp:233 (clisp byte openh) and writer.kl:3/7 while bypassing the gate. hush is never read.
• Pre-existing |shen.write-string| + |shen.char-st*?| (top of file) remain for KL pr path on ratatoskr builds that skip the |pr| target or for reader etc.

scripts/test-cli.sh + tests/io-tests.shen (spec + regression):

• Removed the per-impl case split + "DIVERGENCE" comments that asserted clisp|ecl would produce zero-byte file under -q (right-side of first hunk).
• Now asserts the same "payload" written for sbcl+clisp+ecl under -q (cli parity 47/47).
• io-tests.shen: added explicit regression (assert= "pr to a file stream ignores *hush* (issue #2)" ...) that sets hush true locally around the pr-to-file + read-file-as-string roundtrip (then restores), plus updated header comments explaining the fix and that the runner's quiet (hush true) load is the reason for the local clear. The write-byte ignore-hush assert remains as the "was always true" case.
• Runner (run-port-tests.shen:19) still does global (set hush true) + test-harness clears it only for the final report.

Why this is the right fix (architecture): shen-cl is ratatoskr's reference host. The override lives in the port's post-kernel overwrite.lsp (loaded after compiled/writer in boot.lsp and after kernel.lsp in driver.lsp). Ratatoskr stages the same src/ files. Extending the override unifies all three maintained impls (sbcl/clisp/ecl) without touching vendored kernel or ratatoskr KLambda. (CCL remains in the or- guard but is already deprioritized.)

Verification performed (per instructions): hush never consulted (confirmed in both new |pr| bodies and comments); clisp dispatch uses subtypep + write-byte loop on non-char and never write-string on bytes; force-output gated exactly as required (only st in char case); right-hand diff lines cited below; io-tests + test-cli are treated as the executable cross-impl spec and now assert the unified contract.

Overall correctness: The change is correct for the stated problem (pr to file streams under hush). It also unifies console pr behavior across the three impls (previously clisp/ecl would hush pr to their st under -q; now they match sbcl's pre-existing "always write" override). Tests, load order, and ratatoskr integration were considered. No races, no locks, no unwrap-style (handler-case) misuse.

Issues

Severity: MINOR
Location: shen-cl/src/overwrite.lsp:65 (right-hand side of diff; the #+(or ccl sbcl ecl) block and preceding 8-line comment)
Description: Extending the unconditional (write-string x s) |pr| path to ECL means that on ECL, (pr STR a-byte-stream) (where the stream was obtained via raw Common Lisp open with :element-type 'unsigned-byte, bypassing shen open/|shen.openh|) will now signal a type error from write-string instead of dispatching to a write-byte path. Pre-PR, ECL ran the KL pr (writer.kl:3) and would have taken the char-stoutput? = false → write-chars → write-byte path. SBCL/CCL already had this limitation (their override predates ECL support in the port). shen.openh on non-clisp always produces character streams (:default), so the shen API contract is unaffected.
Suggestion: Add a brief comment in the |pr| (or in primitives.lsp near |shen.openh|) noting "native |pr| on ccl/sbcl/ecl assumes character streams per shen.openh contract; raw binary streams are not supported for pr (use write-byte directly)". No behaviour change for any shen-visible stream or for the ratatoskr builder. Not a blocker.
Status: Open observation (pre-existing for two of the three impls; acceptable given shen openh contract and verification that all port + cli tests pass).

Severity: MINOR
Location: shen-cl/src/overwrite.lsp:79 (right-hand side; the #+clisp |pr| definition)
Description: The byte-stream branch (loop for c across x do (write-byte (char-code c) s)) and the char branch both assume the first argument is a string (for across and implicit contract with kernel pr). A non-string will error (type error on across or later char-code). The KL path in writer.kl:3/5 would also eventually error (via pos / string->n in shen.string->byte or shen.write-chars). No additional defence or string? guard was added.
Suggestion: No change required for correctness (mirrors kernel exactly and all call sites from Shen are strings). If defensive coding is desired in future, a cheap (unless (stringp x) (error ...)) at the top of both |pr| overrides would be consistent with other primitives in the same file (e.g. |pos|, |tlstr|).
Status: Not a defect. Consistent with existing code.

Severity: NIT / STYLE
Location: shen-cl/src/overwrite.lsp:83 (inside #+clisp char branch) and :68 (inside the or- ecl branch) — the two copies of (when (or (eq s |*stoutput*|) (eq s |*stinput*|)) (force-output s))
Description: The exact same st-only force-output predicate is duplicated in the two platform |pr| implementations. (Note also the odd inclusion of |stinput| in an output primitive; this was pre-existing in the sbcl/ccl version and was copied verbatim.) In the clisp version the predicate is further nested inside the if + progn, which is correct per the review instructions ("force-output is only for the st streams in the char case") but adds to the duplication.
Suggestion: Could be extracted to a tiny helper such as (defun |shen-cl.force-if-std-stream| (s) (when (or (eq s |*stoutput*|) (eq s |*stinput*|)) (force-output s))) and called from both |pr|s (and even from the existing |shen.write-string| if desired). Not worth the churn for three lines. The |stinput| name in an output context can stay as-is (purely cosmetic; no behavioural effect).
Status: Style only. No correctness impact. Pre-existing duplication simply extended.

Severity: NIT
Location: shen-cl/tests/io-tests.shen:129 (the new regression assert) and :132 (the open "tests/io-hush-pr.tmp")
Description: The new "pr to a file stream ignores hush (issue #2)" test creates tests/io-hush-pr.tmp but never explicitly deletes it. It relies on the global find tests -name '*.tmp' -delete performed by scripts/run-port-tests.shen:214 (before the port suite) and :221 (after). The other hush test uses a different name (io-hush.tmp). If the runner is ever changed or the test is run directly, the file can be left behind. (The pre-existing io-hush.tmp and the roundtrip files have the same exposure.)
Suggestion: Either (trap-error (delete-file "tests/io-hush-pr.tmp") (lambda _ _)) after the assert (or at end of file before the final (set *hush* true)), or simply accept the global clean as sufficient (current practice for all other .tmp files in the suite).
Status: Minor hygiene; not a test reliability problem under the documented runner.

Severity: NONE (POSITIVE)
Location: shen-cl/scripts/test-cli.sh:43 (the unified assert_eq "payload" ... (issue #2)) and surrounding comment block (right-hand side of first hunk, lines 12-32)
Description: Removal of the case "$impl" split that previously documented/locked the clisp|ecl zero-byte behaviour, plus the excellent updated comment explaining the ratatoskr stage-1 regression, the writer.kl gate, the element-type reason for the clisp version, and that "we assert the SAME corrected behaviour for every impl." This makes the CLI parity matrix a true executable spec of the fixed cross-port contract.
Suggestion: None. This is model commentary.
Status: Commendable.

Severity: NONE (VERIFICATION)
Location: All |pr| sites (overwrite.lsp right-hand ~65 and ~78) + primitives.lsp:233 (clisp openh) + ratatoskr/KLambda/writer.kl:3
Description: Explicit checks per the prompt:

• *hush* (or (value *hush*)) is never consulted inside either override — confirmed.
• clisp version: (if (subtypep (stream-element-type s) 'character) ... write-string ... (loop ... write-byte (char-code c) ...)) — never calls write-string on a byte stream; element-type dispatch exactly mirrors the KL path and primitives |shen.openh|.
• force-output appears only in the char arm of the clisp version and is further guarded to |stoutput| / |stinput| only (no force for file streams or for clisp's byte st streams). Matches the sbcl/ecl arm and the pre-existing |shen.write-string| usage contract.
• On clisp, stoutput is a byte stream (primitives:443), so the byte branch is taken for normal console pr under the override — consistent with prior KL write-chars behaviour for that stream.
• Load order guarantees the override wins (boot.lsp:171 after writer; driver.lsp:139 + ecl epilogue eval of overwrite forms).
  Suggestion: None. All points verified.
  Status: Satisfied.

Severity: NONE (ARCHITECTURE)
Location: ratatoskr/README.md:124 (the *hush* caveat section) + ratatoskr/builders/lisp/driver.lsp:75 and :144 (notes on the old #+(or ccl sbcl) and stream fallbacks)
Description: The PR description and the updated comments in overwrite + tests accurately reflect why shen-cl previously had only a partial override and why extending it (rather than patching writer.kl or adding yet more fallbacks) is the canonical fix for shen-cl-hosted ratatoskr builds. The ratatoskr lisp builder will automatically pick up the new conditional forms when it copies src/overwrite.lsp.
Suggestion: Consider a one-line follow-up in ratatoskr/README.md Gotchas updating the parenthetical "(native pr override)" to mention ecl + clisp now participate, but not required for this PR.
Status: Documentation is already sufficient in the changed files.

No CRITICAL or MAJOR issues. No error-handling gaps that would affect the shen-visible contract, no races (Shen/Lisp runtime here is single-threaded for these paths), no inappropriate lock usage, and no unwrap-style bare handler-case that swallows errors (the only handler-case is the pre-existing ratatoskr-load-overwrite "skip if target absent" logic, which is sound).

Final Verdict

The fix is correct, minimal, and complete. It directly addresses the root cause (writer.kl hush gate applying to file streams), correctly extends the established native-override pattern used by the reference SBCL build, handles the clisp byte/char reality without violating the "never write-string on byte streams" rule, confines force-output exactly as specified, never consults hush, and updates the executable ShenSpec (cli parity matrix + io-tests) to assert the unified desired behaviour rather than codifying the old divergence.

All review requirements (deep dives into ratatoskr writer + builder, primitives openh + st* setup, char-stoutput? dispatch relation, right-side line citations, structured issues, no self-fixing) were followed.

Review file: /tmp/grok-reviews/215c6b37-REVIEW.md

Concise verdict: APPROVE — ready to merge. One minor edge-case behaviour note for raw (non-shen) byte streams on ECL (pre-existing pattern on SBCL/CCL) and a few style nits; no correctness, test, or integration problems. The port now has uniform pr-to-file semantics under hush across sbcl/clisp/ecl.

---

Posted automatically by Grok reviewer subagents. See also the PENDING review (if any) for inline comments on the Files tab.

[pyrex41]

From independent reviewer subagent (ratatoskr as reference host, writer.kl gate, overwrite load order in boot.lsp + lisp builder, char-stoutput? / openh element-type reality, and executable spec in test-cli.sh + io-tests.shen):

MINOR

• Location: shen-cl/src/overwrite.lsp:65 (the #+(or ccl sbcl ecl) block)
• Description: Extending the unconditional write-string |pr| to ECL means raw (non-shen open with :element-type 'unsigned-byte) byte streams on ECL will now get a type error from write-string (instead of falling through to write-chars/write-byte as the old KL pr would). Pre-existing for sbcl/ccl too; shen.openh contract produces character streams for non-clisp.
• Suggestion: Add a brief comment noting "native |pr| on ccl/sbcl/ecl assumes character streams per shen.openh contract; raw binary streams are not supported for pr (use write-byte directly)".

NIT / STYLE

• Duplicated st-only force-output predicate in the two |pr| implementations (and the odd inclusion of |stinput| in an output primitive, pre-existing).
• Suggestion: Tiny helper (defun |shen-cl.force-if-std-stream| (s) ...) if desired (not worth churn for 3 lines).

NIT

• tests/io-tests.shen new tmp file not explicitly cleaned (relies on global runner find+delete).
• Suggestion: Add (trap-error (delete-file ...) ...) or accept the global clean.

Positive: clisp dispatch correct (subtypep + write-byte on non-char, never write-string on bytes), hush never consulted, force-output confined exactly to st in char case, load order respected, test-cli now asserts unified behavior across all three impls (excellent comment updates). Full ratatoskr + builder analysis in previous top-level comment on this PR. APPROVE (ready to merge) per subagent.

[pyrex41]

Superseded by #6 (merged into kernel-41.2), which combines the CLISP/ECL hush fix (#2) and the absvector cap (#3). Closing as redundant.