Add IO.read_char effect operation (#618)#689
Conversation
New `IO.read_char` op for single-character input — unblocks real-time CLI programs (paced REPLs, terminal games) that can't be written with line-buffered `IO.read_line`. Signature: `op read_char(Unit -> @Result<String, String>)`. Returns one Unicode character from stdin, or `Err("EOF")` when the stream closes. Terminal target host_read_char dispatches: stdin_buf (test fixture) > Windows msvcrt.getwch() > Unix non-TTY sys.stdin.read(1) > Unix TTY termios raw mode. The non-TTY guard matters: `tty.setraw()` on a pipe raises EINVAL, so the os.isatty() check keeps `echo "X" | vera run` portable. KeyboardInterrupt deliberately NOT caught here so Ctrl-C exits interactive prompts cleanly — host_sleep takes the same stance for the same reason. Browser target is a stub that returns Result.Err pointing at #609 — the actual implementation needs JSPI for the suspend/resume primitive (keypress listener pushes chars into a queue, read_char suspends WASM and resumes on next event). Same primitive #609 needs for IO.sleep. Wired into IO_BINDINGS so #609's landing can replace just the function body. Closes #618 (terminal half). Browser half stays open under the JSPI umbrella tracked by #609. Co-Authored-By: Claude <noreply@anthropic.invalid>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #689 +/- ##
==========================================
- Coverage 91.07% 90.88% -0.19%
==========================================
Files 60 60
Lines 23458 23529 +71
Branches 259 259
==========================================
+ Hits 21364 21384 +20
- Misses 2087 2138 +51
Partials 7 7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds IO.read_char: effect/type registration, WASM import and inference, Python host implementation and browser stub, conformance/CLI/codegen/verifier tests, and documentation/scripts updates (counts and line-number offsets). ChangesIO.read_char Implementation
Sequence Diagram(s)sequenceDiagram
participant VeraModule as Vera Program
participant HostImport as vera.read_char
participant InputSource as stdin_buf / TTY / msvcrt / pipe
participant WASMHeap as WASM Heap
VeraModule->>HostImport: call read_char()
HostImport->>InputSource: select input method (stdin_buf / TTY / piped)
InputSource-->>HostImport: char / EOF / exception
HostImport->>WASMHeap: allocate Result<String,String> on heap
WASMHeap-->>HostImport: return i32 pointer
HostImport-->>VeraModule: return i32 pointer
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a Comment |
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
TESTING.md (1)
246-246:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale example count in verification coverage section.
This line states "Across all 34 example programs", but the overview at line 12 correctly shows 35. This should be updated to match.
🤖 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 `@TESTING.md` at line 246, Update the stale count in the verification coverage section by changing the phrase "Across all 34 example programs" to "Across all 35 example programs" so it matches the overview count; locate the string "Across all 34 example programs" in TESTING.md and replace the numeric literal to 35.
🤖 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 `@SKILL.md`:
- Line 1320: Update the prose that states IO provides "ten" operations to
reflect the new total "eleven" now that IO.read_char has been added; locate the
sentence immediately preceding the IO operations table (mentions IO operations
count) and change "ten" to "eleven" and ensure any adjacent
grammar/pluralization still reads correctly after the change (the added table
row is the `IO.read_char` entry).
In `@spec/07-effects.md`:
- Line 295: Update the heading/intro sentence that states the IO effect provides
"ten" operations to reflect the new total of "eleven" operations now that
`read_char` was added; locate the IO effect description paragraph near the
`read_char` table entry and change the numeric count from ten to eleven so the
spec remains consistent with the table listing `read_char`.
In `@TESTING.md`:
- Line 492: The table entry for `check_examples.py` still reads "All 34 `.vera`
examples pass `vera check` + `vera verify`" but the documentation now has 35
examples; update the table cell text to "All 35 `.vera` examples pass `vera
check` + `vera verify`" so the count matches the overview. Locate the table row
referencing `check_examples.py` and change only the numeric count from 34 to 35.
- Line 584: The table row for `check_examples.py` currently states "All 34
examples" but should match the overview's count of 35; update the cell text in
TESTING.md's pre-commit hooks table (the `check_examples.py` row) from "All 34
examples pass `vera check` + `vera verify`" to "All 35 examples pass `vera
check` + `vera verify`".
- Line 43: The inline comment after the command "python
scripts/check_examples.py # 34 example programs" is stale;
update that trailing comment to reflect the correct count (change "34" to "35")
so it matches the overview at line 12 and the actual number of example programs
referenced by the repo.
- Around line 9-12: Update the outdated example-count references in TESTING.md
to the authoritative value 37 (the count comes from check_doc_counts.py which
uses len(list((root / "examples").glob("*.vera")))); specifically replace the
header occurrence showing "35" (the line with "**Example programs**") and the
four other places that currently say "34" (around the locations noted) so all
five occurrences read "37". Ensure the wording and formatting remain unchanged
aside from the numeric value.
In `@tests/test_cli.py`:
- Around line 1834-1838: The subprocess.run calls in tests/test_cli.py that
invoke [sys.executable, "-m", "vera.cli", ...] use text=True but not an explicit
encoding, so on non-UTF-8 locales the multi-byte UTF-8 input path test can fail;
update each relevant subprocess.run invocation (the one passing input="é" and
the sibling read_char subprocess tests) to pass encoding="utf-8" in addition to
text=True so the child process I/O is forced to UTF-8 and the UTF-8 path/input
assertions are reliable.
In `@vera/codegen/api.py`:
- Around line 1400-1421: Wrap calls that may raise at
runtime—sys.stdin.fileno(), termios.tcgetattr(fd), tty.setraw(fd), and
termios.tcsetattr(fd, termios.TCSADRAIN, old)—in try/except blocks that catch
all exceptions except KeyboardInterrupt and convert them into the Result.Err
return via _alloc_result_err_string(caller, "<error>") (include the exception
text). Ensure any early-read path also wraps sys.stdin.read(1) similarly. Update
the TTY branch around the old variable so that failures to get/set attributes or
to enter raw mode always return _alloc_result_err_string(caller, f"...{exc}...")
instead of letting exceptions escape, while preserving KeyboardInterrupt
propagation.
---
Outside diff comments:
In `@TESTING.md`:
- Line 246: Update the stale count in the verification coverage section by
changing the phrase "Across all 34 example programs" to "Across all 35 example
programs" so it matches the overview count; locate the string "Across all 34
example programs" in TESTING.md and replace the numeric literal to 35.
🪄 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: 69a261da-4ca5-4713-98df-6733360fad72
⛔ Files ignored due to path filters (7)
docs/SKILL.mdis excluded by!docs/**docs/index.htmlis excluded by!docs/**docs/index.mdis excluded by!docs/**docs/llms-full.txtis excluded by!docs/**docs/llms.txtis excluded by!docs/**examples/read_char.verais excluded by!**/*.veratests/conformance/ch07_io_read_char.verais excluded by!**/*.vera
📒 Files selected for processing (19)
AGENTS.mdCHANGELOG.mdCLAUDE.mdFAQ.mdROADMAP.mdSKILL.mdTESTING.mdscripts/check_skill_examples.pyscripts/check_spec_examples.pyspec/07-effects.mdspec/12-runtime.mdtests/conformance/manifest.jsontests/test_cli.pytests/test_verifier.pyvera/browser/runtime.mjsvera/codegen/api.pyvera/codegen/assembly.pyvera/environment.pyvera/wasm/inference.py
PR #689 review surfaced three classes of issue. All fixed, minimal changes, no behaviour change for the happy path. 1. Doc-count drift — finished the 34→35 example sweep that the earlier commit only partially completed. Six remaining call sites in TESTING.md (validation-script comment line 43, verification-coverage section line 246, slot-feature row line 281, round-trip section line 334, validation-scripts table line 492, pre-commit-hooks table line 584). CodeRabbit pointed at four of the six; I caught the other two by grepping for `All 34|all 34|34 example` after applying its suggestions. Also bumped "ten → eleven" in spec/07-effects.md (line 289) and SKILL.md (two places, lines 1314 + 1332) since read_char makes it eleven IO operations. 2. test_cli.py — added explicit `encoding="utf-8"` to the three new `subprocess.run` calls for read_char tests. Matches the CLAUDE.md cross-platform pitfalls convention: CI sets `PYTHONUTF8=1` as a backstop but the explicit form is portable to local Windows shells without that variable. Particularly important for the `é` UTF-8 round-trip test which would silently break on non-UTF-8 locales otherwise. 3. host_read_char — wrapped `sys.stdin.fileno()`, `sys.stdin.read(1)` (in both branches), and the `termios.tcgetattr` / `tty.setraw` / `termios.tcsetattr` sequence in `except Exception` blocks. System errors (closed stdin, monkey-patched stream without a fileno, `termios.error` on weird devices) now become Result.Err rather than propagating as wasmtime traps. `Exception` excludes `KeyboardInterrupt` and `SystemExit` since those are direct `BaseException` subclasses, so Ctrl-C still terminates interactive prompts — same stance as host_sleep. The inner try/finally is preserved so terminal state is restored even when setraw or read raises. Validation: 3,800 pytest pass + 99 browser pytest pass, all conformance/example/spec/skill/HTML/README/FAQ doc validators green, doc counts consistent, mypy clean, site assets rebuilt. Co-Authored-By: Claude <noreply@anthropic.invalid>
|
Tip For best results, initiate chat on the files or code changes.
A few notes on the applied items worth confirming:
All CI green with 3,800 + 99 browser tests passing is a solid validation gate. Nothing outstanding from my side — this looks ready. |
There was a problem hiding this comment.
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 (2)
TESTING.md (2)
92-92:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale conformance count.
Line 92 still references "86 small, focused programs" but the conformance suite now contains 87 programs (per line 11 and the addition of ch07_io_read_char documented at line 119). This should be updated to maintain consistency with the rest of the document.
📝 Proposed fix
-The conformance suite is a collection of 86 small, focused programs in `tests/conformance/` that systematically validate every language feature against the spec. Each program is self-contained and imports nothing, with the single exception of `ch07_cross_module_contracts.vera` which depends on `ch07_cross_module_contracts_lib.vera`. Each program tests one feature or a small group of related features. +The conformance suite is a collection of 87 small, focused programs in `tests/conformance/` that systematically validate every language feature against the spec. Each program is self-contained and imports nothing, with the single exception of `ch07_cross_module_contracts.vera` which depends on `ch07_cross_module_contracts_lib.vera`. Each program tests one feature or a small group of related features.🤖 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 `@TESTING.md` at line 92, Update the stale conformance count in TESTING.md by changing the sentence that currently reads "86 small, focused programs" to "87 small, focused programs" (this is the paragraph describing the conformance suite that also mentions ch07_cross_module_contracts.vera and the new ch07_io_read_char addition); ensure the rest of the sentence and references (ch07_cross_module_contracts.vera and ch07_cross_module_contracts_lib.vera) remain unchanged so the doc stays consistent.
193-193:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale parametrised test count.
Line 193 states "parametrized — 430 tests" but with 87 conformance programs (per line 11) and 5 checks per program (parse/check/verify/run/format idempotency per line 200), the correct count is 435 tests. This matches line 82 which correctly shows "435" for
test_conformance.py. The value 430 reflects the old count (86 × 5).📝 Proposed fix
-# Via pytest (parametrized — 430 tests) +# Via pytest (parametrized — 435 tests) pytest tests/test_conformance.py -v🤖 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 `@TESTING.md` at line 193, Update the stale test count in TESTING.md: change the header text "Via pytest (parametrized — 430 tests)" to "Via pytest (parametrized — 435 tests)". Ensure this reflects the 87 conformance programs and 5 checks per program (parse/check/verify/run/format idempotency) so it matches the `test_conformance.py` count shown elsewhere.
🤖 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 `@vera/codegen/api.py`:
- Around line 1384-1424: Move the stdin redirection/TTY check before calling
msvcrt.getwch(): call sys.stdin.fileno() and os.isatty(fd) (with the same
exception handling that returns _alloc_result_err_string(caller, ... ) on
failure) and if not os.isatty(fd) on Windows use sys.stdin.read(1) wrapped in
try/except and return _alloc_result_ok_string(caller, ch) or
_alloc_result_err_string(caller, ...) / EOF exactly like the Unix non-TTY path;
only call msvcrt.getwch() when os.isatty(fd) is True, preserving the existing
error/EOF handling and the same return helpers (_alloc_result_ok_string /
_alloc_result_err_string) and `caller` context.
---
Outside diff comments:
In `@TESTING.md`:
- Line 92: Update the stale conformance count in TESTING.md by changing the
sentence that currently reads "86 small, focused programs" to "87 small, focused
programs" (this is the paragraph describing the conformance suite that also
mentions ch07_cross_module_contracts.vera and the new ch07_io_read_char
addition); ensure the rest of the sentence and references
(ch07_cross_module_contracts.vera and ch07_cross_module_contracts_lib.vera)
remain unchanged so the doc stays consistent.
- Line 193: Update the stale test count in TESTING.md: change the header text
"Via pytest (parametrized — 430 tests)" to "Via pytest (parametrized — 435
tests)". Ensure this reflects the 87 conformance programs and 5 checks per
program (parse/check/verify/run/format idempotency) so it matches the
`test_conformance.py` count shown elsewhere.
🪄 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: f49af71d-260f-4666-8fe4-d29bcd51e427
⛔ Files ignored due to path filters (2)
docs/SKILL.mdis excluded by!docs/**docs/llms-full.txtis excluded by!docs/**
📒 Files selected for processing (6)
CHANGELOG.mdSKILL.mdTESTING.mdspec/07-effects.mdtests/test_cli.pyvera/codegen/api.py
Three findings, all valid: 1. host_read_char Windows path — the previous structure went straight to `msvcrt.getwch()` regardless of whether stdin was a TTY or a redirected pipe. Lifted the `os.isatty(fd)` check above the platform fork so the non-TTY (pipe / redirect) branch is now shared across platforms — both use `sys.stdin.read(1)` for identical encoding behaviour. Only TTY stdin goes through `msvcrt.getwch()` (Windows) or termios raw mode (Unix). Why this matters: `msvcrt.getwch()` technically works on redirected stdin via Win32's `_getch` fallback but decodes raw bytes rather than honouring Python's stdin encoding. A piped UTF-8 sequence like "é" would round-trip differently from the Unix path — exactly the kind of silent cross-platform divergence that breaks CI on Windows-3.13 weeks after merge. Also wrapped `msvcrt.getwch()` in `except Exception` for symmetry with the Unix path's defensive handling. 2. TESTING.md L92 — "86 small, focused programs" → "87". 3. TESTING.md L193 — "Via pytest (parametrized — 430 tests)" → "435 tests" (matches `test_conformance.py`'s live count of 87 programs × 5 stages). Plus a grep sweep caught one more stale 86 in ROADMAP.md L190 (#679 conformance-gap description). Validation: 3,800 pytest pass + 99 browser pytest pass, mypy clean, doc counts consistent, all doc validators green, piped smoke test still works. Co-Authored-By: Claude <noreply@anthropic.invalid>
|
@coderabbitai also applied the two outside-diff TESTING.md findings in 9983b2c: L92 "86 small, focused programs" → "87", and L193 "Via pytest (parametrized — 430 tests)" → "435". A grep sweep also caught a stale 86 in ROADMAP.md L190 (#679 row description), fixed in the same commit. |
Internal multi-agent review (code-reviewer, pr-test-analyzer, comment-analyzer, silent-failure-hunter) caught two correctness bugs and three quality issues that CodeRabbit's two rounds missed. Critical: 1. host_read_char now catches KeyboardInterrupt at each blocking call (sys.stdin.read(1) in both branches, msvcrt.getwch() on Windows) and raises _VeraExit(130). Same fix host_sleep applies for the same reason (#589-class WasmTrapError contract violation when a raw Python KeyboardInterrupt unwinds through the host import). The prior comment claimed parity with host_sleep's stance, but host_sleep actually catches KeyboardInterrupt — the comment was factually wrong and the missing catch was a real bug. User-facing behaviour is unchanged: Ctrl-C still terminates the program with exit code 130. 2. Fixed stale cross-reference in tests/conformance/ch07_io_read_char.vera (pointed at tests/test_codegen.py instead of tests/test_cli.py). Important: 3. Termios restore failure can no longer mask the original read error. Inner tcsetattr in finally is now wrapped in its own try/except — if restore fails the terminal stays in raw mode (rare; same fd just worked) but the original read error still surfaces. 4. tcgetattr failure now reports "tcgetattr failed: ..." rather than the misleading "raw-mode read failed: ..." — raw mode never started in that case. 5. Removed redundant local `import os` inside host_read_char (`os` already imported at module level). Test coverage: 6. Five new test_codegen.py::TestIOOperations tests covering the stdin_buf fixture path that subprocess tests can't reach: single read, empty-buf EOF, sequential cursor advance, read-then-EOF, 2-byte UTF-8 round-trip. All platform-independent (StringIO, no reliance on host stdin encoding). 7. Tightened the 3 existing test_cli.py read_char tests: result.stdout.rstrip() == "<exact>" (not substring `in`), plus assert result.stderr == "" — catches regressions where a future host prints to stderr on the way to a clean exit. Findings skipped with reasons (logged in pr-review reports): - Windows-monkeypatch test for the platform-fork bug: infra complexity exceeds the benefit; existing tests exercise the platform-shared path on Windows CI anyway. - Example program Unix-centric phrasing: minor cosmetic; examples are illustrative not exhaustive. - 4-byte emoji UTF-8 test: 2-byte é already pins the multi-byte path adequately. - PYTHONIOENCODING guidance in error message: overengineering for a rare failure mode. - Comment about dispatch-order being load-bearing: adequately implied by the existing comment block. - Test docstring paraphrasing: matter of taste. Validation: 3,805 pytest pass + 99 browser pytest pass, mypy clean, all doc validators green, doc counts consistent at the new 3,935 / 87 / 35. Co-Authored-By: Claude <noreply@anthropic.invalid>
pr-review-toolkit resultsRan a comprehensive internal review with four specialised agents in parallel: Critical (applied)
Important (applied)
Test coverage (applied)
Skipped with reason
Strengths the agents noted
Validation3,805 pytest pass + 99 browser pytest pass, mypy clean, all doc validators green, doc counts consistent (3,935 tests, 87 conformance, 35 examples). |
pr-review-toolkit:
|
pr-review-toolkit:
|
pr-review-toolkit:
|
pr-review-toolkit:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@vera/codegen/api.py`:
- Around line 1469-1487: The current read loop uses tty.setraw(fd) which clears
ISIG so Ctrl-C becomes '\x03' and KeyboardInterrupt won't be raised; replace the
raw mode call with cbreak (e.g. use tty.setcbreak(fd)) or explicitly preserve
the ISIG flag on the termios flags before applying the mode so the signal
generation remains enabled, keeping the existing restore logic around
termios.tcsetattr(fd, termios.TCSADRAIN, old) and the outer except
KeyboardInterrupt that raises _VeraExit; update the call site where
tty.setraw(fd) is invoked (and any related comment) so single-character reads
still allow KeyboardInterrupt.
- Around line 1481-1491: The finally block currently swallows errors from
termios.tcsetattr so a failed restore can be hidden; modify the surrounding read
routine to capture any exception raised by termios.tcsetattr(fd,
termios.TCSADRAIN, old) (e.g., store it as restore_exc) instead of silently
passing, then after the try/except that handles the read decide the returned
result: if the read raised an exception prefer returning that error via
_alloc_result_err_string(caller, ...), if the read succeeded but restore_exc is
set return an Err with a message like "raw-mode restore failed: {restore_exc}"
via _alloc_result_err_string, and if both failed prefer the read error; ensure
KeyboardInterrupt still raises _VeraExit(130) as before.
🪄 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: 5a039fb0-587c-4e70-a7f8-e9b71c4af11c
⛔ Files ignored due to path filters (1)
tests/conformance/ch07_io_read_char.verais excluded by!**/*.vera
📒 Files selected for processing (6)
CHANGELOG.mdROADMAP.mdTESTING.mdtests/test_cli.pytests/test_codegen.pyvera/codegen/api.py
Two findings, both valid and material.
1. tty.setraw → tty.setcbreak (CRITICAL). setraw() clears the
termios ISIG flag, which suppresses SIGINT generation — so
the `except KeyboardInterrupt` clause added in the previous
pr-review-toolkit round was actually UNREACHABLE in TTY
mode. Ctrl-C would arrive in the read buffer as the literal
byte `\x03` instead of raising KeyboardInterrupt, meaning a
Tetris-style game would receive `\x03` as input and the user
could never exit.
tty.setcbreak() is the right primitive: disables ICANON +
ECHO (so we get one character without waiting for Enter and
without echoing) but PRESERVES ISIG. Ctrl-C → SIGINT →
KeyboardInterrupt → _VeraExit(130) works as intended.
Verified empirically: `cfmakecbreak` keeps the ISIG bit set
(mode[3] & termios.ISIG != 0), `cfmakeraw` clears it.
2. Capture and surface restore_exc instead of swallowing it.
The previous round wrapped tcsetattr in an inner finally
with bare `except Exception: pass`, which kept the original
read error visible but lost the restore error entirely.
Now captures the restore exception into a local
`restore_exc` variable. Resolution logic:
- If read failed: return the read error (more actionable).
- If read succeeded but restore failed: return a distinct
"raw-mode restore failed: ..." error.
- If both failed: read error wins.
- KeyboardInterrupt still raises _VeraExit(130) as before
(and now this branch is actually reachable).
No silent failures from this path now.
Also updated:
- The top-of-function comment block to describe cbreak (not
raw) mode and the ISIG-preservation rationale.
- examples/read_char.vera's prose to mention cbreak vs raw.
Validation: 3,805 pytest pass + 99 browser pytest pass, mypy
clean, all 35 examples + 87 conformance programs pass, all
doc validators green.
Co-Authored-By: Claude <noreply@anthropic.invalid>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@CHANGELOG.md`:
- Line 11: The changelog incorrectly states the Unix TTY uses "termios
raw-mode"; update the Added summary for IO.read_char to reflect the actual
implementation: state that Unix uses termios cbreak mode via tty.setcbreak()
(with raw mode explicitly avoided to preserve ISIG), not raw-mode, so replace
the phrase "termios raw-mode (Unix TTY)" with wording that mentions
cbreak/tty.setcbreak and preservation of ISIG; ensure the rest of the sentence
about Windows/msvcrt.getwch() and piped input remains unchanged.
In `@vera/codegen/api.py`:
- Around line 1493-1519: The raw-mode read path using tty.setcbreak +
sys.stdin.read(1) currently returns the literal Ctrl-D byte as Ok("\x04");
modify the logic in the function handling the read (the block that returns
_alloc_result_ok_string(caller, ch)) to treat the ASCII EOT character '\x04' as
EOF: if ch == '\x04' then return _alloc_result_err_string(caller, "EOF")
(matching the existing EOF handling), otherwise return
_alloc_result_ok_string(caller, ch); keep existing restore_exc checks and
KeyboardInterrupt/_VeraExit behavior unchanged.
🪄 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: 33fa6386-ef6c-4080-9ca9-5ab687d81a39
⛔ Files ignored due to path filters (1)
examples/read_char.verais excluded by!**/*.vera
📒 Files selected for processing (2)
CHANGELOG.mdvera/codegen/api.py
Two final findings, both valid.
1. Unix TTY cbreak now maps `\x04` (Ctrl-D) to EOF (IMPORTANT).
Without this mapping, a user pressing Ctrl-D in a real-time
CLI program would get `Ok("\x04")` and have to know to
interpret the literal byte as end-of-input — surprising and
platform-specific. cbreak disables ICANON, which is the
line discipline layer that normally turns Ctrl-D-at-start-
of-line into an empty read in canonical mode. Without
ICANON, Ctrl-D arrives as a literal `\x04` byte.
The mapping is restricted to the Unix TTY cbreak branch:
- Piped `\x04` on the non-TTY shared path stays literal
(a pipe is a byte stream, the producer chose to include
`\x04`).
- Windows `msvcrt.getwch()` branch has its own end-of-input
convention (Ctrl-Z `\x1A`) — left untouched for now;
similar mapping could be added if a regression surfaces.
Added a regression test (test_io_read_char_stdin_buf_passes
_eot_literally) that pins the non-TTY asymmetry: piped
`\x04` returns `Ok("\x04")`, not `Err("EOF")`.
2. CHANGELOG wording fix (MINOR). The Added entry for #618
still said "termios raw-mode (Unix TTY)" — stale from
before the round-3 setraw → setcbreak fix. Updated to
"termios cbreak mode via `tty.setcbreak()` (cbreak
preserves ISIG so Ctrl-C still raises SIGINT, unlike raw
mode which would suppress it)".
Validation: 3,806 pytest pass + 99 browser pass, mypy clean,
ruff S rules clean, all 35 examples + 87 conformance programs
pass, doc counts consistent at 3,936 / 87 / 35.
Co-Authored-By: Claude <noreply@anthropic.invalid>
Summary
Closes #618 (terminal half). The browser half stays open under the JSPI umbrella tracked by #609 —
IO.read_charneeds the same suspend/resume primitive thatIO.sleepneeds in the browser, and the wiring is in place so #609's landing can replace just the stub body.IO.read_charop:Unit -> @Result<String, String>. Returns one Unicode character from stdin, orErr("EOF")when the stream closes. Unblocks real-time CLI programs (paced REPLs, terminal games) that can't be written with line-bufferedIO.read_line.tty.setraw()on a pipe raises EINVAL, so theos.isatty()check keepsecho "X" | vera runportable across Linux CI runners. KeyboardInterrupt is not caught — same stance as host_sleep — so Ctrl-C exits interactive prompts cleanly.Result.Errwith a message pointing at Browser runtime: implement IO.sleep via JSPI (or Asyncify fallback) so animations don't freeze the tab #609 + IO.read_char — single-character input portable across terminal and browser targets #618. Wired intoIO_BINDINGSso Browser runtime: implement IO.sleep via JSPI (or Asyncify fallback) so animations don't freeze the tab #609's JSPI landing can drop in the real implementation without touching anything else.ch07_io_read_char.vera(verify-level — pins the signature + effect-row wiring; no run assertion because a successful run depends on host stdin behaviour).examples/read_char.verademonstrating the read-one-char pattern with full failure-mode handling.Test plan
pytest tests/ -q— 3,800 passed + 15 skipped (browser tests excluded above the line; see next)pytest tests/test_browser.py -q— 99 passed (browser stub doesn't trap any existing fixture)python scripts/check_conformance.py— all 87 conformance programs pass their declared levelpython scripts/check_examples.py— all 35 examples passvera check+vera verifypython scripts/check_spec_examples.py/check_skill_examples.py/check_html_examples.py/check_examples_readme.py/check_readme_examples.py/check_faq_examples.py/check_examples_doc.py— all greenpython scripts/check_doc_counts.py— consistent (3,930 tests, 87 conformance, 35 examples)python scripts/check_limitations_sync.py/check_version_sync.py/check_site_assets.py/check_licenses.py— all greenmypy vera/— cleanecho "X" | vera run examples/read_char.vera→You pressed: X🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests
Bug Fixes