v0.0.123: tee IO.print writes live to sys.stdout (#543)#544
Merged
Conversation
Closes #543. IO.print writes were accumulated in an in-memory output_buf (the v0.0.120 implementation of #522 trap preservation) and only flushed to sys.stdout after execute() returned. That was correct for trap preservation and for --json output (where the transcript packs into the envelope), but it had an unintended side effect: any program using ANSI escape sequences for animation, progress bars, REPLs, or any other interactive pattern was invisible until exit. The 470-line Conway implementation that surfaced #515 made it visible: the user saw nothing for ~16 seconds (200 generations x 80ms IO.sleep) and then exit fired and only the final frame was visible because the 199 preceding cursor-home + clear-screen sequences processed within microseconds. Fix is a tee: host_print always writes to output_buf (preserving the #522 trap-preservation contract) AND writes to sys.stdout with an explicit per-write flush() when execute(tee_stdout=True). New tee_stdout: bool = False parameter on execute() defaults off so test helpers (_run_io, _run in test_codegen.py) still get silent runs. cmd_run text mode opts in (tee_stdout=not as_json); JSON mode stays off (live writes would split the envelope for downstream consumers parsing our stdout). The cmd_run text-mode and WasmTrapError-handler paths now skip re-writing exec_result.stdout / exc.stdout (those bytes already streamed live), only emitting a closing newline if the last write did not include one -- without that change every transcript would double-print. Tests: new TestStdoutTee543 class in tests/test_runtime_traps.py (6 tests): live streaming in text mode, write count and order preservation, JSON-mode tee suppression, trap-preservation invariant still holds (#522 regression guard), per-call flush count matches per-call IO.print count, default execute() stays silent. Docs: SKILL.md IO.print row notes per-call flush; new paragraph explains the buffering / live-write semantics for both text and JSON modes. Note: this builds on top of #542 (v0.0.122 GC fix); merge order should be #542 then this PR, with a rebase if needed. Co-Authored-By: Claude <noreply@anthropic.invalid>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughImplements live stdout streaming for Changes
Sequence DiagramsequenceDiagram
participant CLI as cmd_run
participant Exec as execute()
participant Host as host_print
participant Buf as output_buf
participant Out as sys.stdout
participant JSON as JSON_Mode
rect rgba(100,200,150,0.5)
Note over CLI,JSON: Text mode (tee_stdout = True)
CLI->>Exec: execute(tee_stdout=True)
Exec->>Host: IO.print("msg")
activate Host
Host->>Buf: write(text)
Host->>Out: write(text)
Host->>Out: flush()
deactivate Host
Exec-->>CLI: ExecuteResult(stdout=buf)
end
rect rgba(150,150,200,0.5)
Note over CLI,JSON: JSON mode (tee_stdout = False)
CLI->>Exec: execute(tee_stdout=False)
Exec->>Host: IO.print("msg")
activate Host
Host->>Buf: write(text)
Host--xJSON: (no live write)
deactivate Host
Exec-->>CLI: ExecuteResult(stdout=buf) wrapped in JSON envelope
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Suggested Labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #544 +/- ##
=======================================
Coverage 91.02% 91.02%
=======================================
Files 58 58
Lines 22033 22034 +1
Branches 259 259
=======================================
+ Hits 20055 20056 +1
Misses 1971 1971
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:
|
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@HISTORY.md`:
- Around line 259-260: Collapse the two multi-line HISTORY entries for v0.0.122
and v0.0.123 into single-line summary rows (e.g., "v0.0.122 | 27 Apr |
Conservative GC bounds-checked against $heap_ptr" and "v0.0.123 | 27 Apr |
IO.print writes mirror live to sys.stdout"), and update the tagged-release
roll-up string that currently reads "121 tagged releases" to "123 tagged
releases" in both HISTORY.md and the paired ROADMAP.md so the totals stay in
sync.
In `@ROADMAP.md`:
- Line 11: The ROADMAP.md line currently states "33 examples" but the
repository's checked count and the docs invariant expect "32"; update the phrase
"33 examples" to "32 examples" in ROADMAP.md and scan other documentation
summaries (e.g., README.md, TESTING.md, CLAUDE.md and any other prose that
mentions the example count) to change any "33" occurrences to "32" so they stay
in sync with scripts/check_doc_counts.py and the repo-wide checked count.
In `@tests/test_codegen.py`:
- Around line 1925-1927: Replace the brittle string assertions that only check
comment markers ("Layer 2 (issue `#515`)" / "Layer 1 (issue `#515`)") with
assertions that inspect the WebAssembly text in the variable wat for the actual
guard instruction patterns around the i32.load in the scan loop; specifically,
locate the i32.load occurrence for the scan path and assert the surrounding
instruction sequence includes the expected heap-pointer guard pattern (e.g.,
explicit i32.const/i32.add bounds computation and a conditional/br_if or
i32.lt_u check) for both the per-iteration (Layer 1) and outer (Layer 2) cases,
using regex or index-based substring checks rather than comment text so the test
fails if the guard instructions regress.
In `@tests/test_runtime_traps.py`:
- Around line 479-508: The test test_json_mode_does_not_tee_to_stdout currently
only checks captured.out but not captured.err; after calling captured =
capsys.readouterr() add an assertion that captured.err == "" to ensure no
human-readable stderr leaked in JSON mode (this applies to any other tests that
call cmd_run(..., as_json=True) — update those similarly). Locate the assertion
block in test_json_mode_does_not_tee_to_stdout and insert the captured.err check
immediately after reading outputs from capsys to fail tests on any stderr
leakage.
In `@vera/cli.py`:
- Around line 624-636: The code that appends a trailing newline when
exec_result.stdout doesn't end with one should only do so for interactive
terminals; update the branches that currently call sys.stdout.write("\n")/flush
(the normal branch handling exec_result.stdout and the trap/alternative branch
around the same logic) to first check sys.stdout.isatty() and only write/flush
the "\n" when isatty() is True; locate the logic around the exec_result.stdout
checks in cli.py (and the similar block at the other occurrence noted) and gate
the extra newline on sys.stdout.isatty() in both places.
In `@vera/codegen/assembly.py`:
- Around line 680-702: The code currently only bounds-checks a decoded obj_size
but can still treat non-header payload as a header and flip the mark bit; add an
explicit header-validity check immediately after computing local $header and
local $obj_size (before any mark-store or clearing/setting of the mark bit) that
verifies the word at local $header satisfies the header tag/format (e.g.,
mask/compare against the known HEADER_TAG or other header invariants used
elsewhere in the runtime) and that obj_ptr and obj_size are in-range; if the
header test fails, branch to $m_loop (same path used for the size-overflow
check). Alternatively (or additionally), move the mark bit out of object payload
state so mark writes never touch decoded payload words — but the minimum fix is
to add the header-format test using the same header-mask/constant used by the
allocator/GC before any mark-store.
🪄 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: 588d693b-4911-4eb8-8e63-2dd3bf721141
⛔ Files ignored due to path filters (5)
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/**
📒 Files selected for processing (16)
CHANGELOG.mdHISTORY.mdKNOWN_ISSUES.mdREADME.mdROADMAP.mdSKILL.mdTESTING.mdpyproject.tomlscripts/check_skill_examples.pytests/test_codegen.pytests/test_runtime_traps.pyvera/README.mdvera/__init__.pyvera/cli.pyvera/codegen/api.pyvera/codegen/assembly.py
💤 Files with no reviewable changes (1)
- KNOWN_ISSUES.md
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 (1)
HISTORY.md (1)
297-297:⚠️ Potential issue | 🟠 MajorFix tagged-release total mismatch in HISTORY.md footer.
Line 297 still reports “122 tagged releases”, but the file now includes v0.0.123 (Line 260~) and ROADMAP.md shows 123 tagged releases (Line 274~). This violates the repo rule that those totals are incremented together.
[/raise_major_issue]🔧 Proposed fix
-Total: **810+ commits, 122 tagged releases, 40 active development days.** +Total: **810+ commits, 123 tagged releases, 40 active development days.**🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@HISTORY.md` at line 297, Update the tagged-releases total in the HISTORY.md footer: change the string "Total: **810+ commits, 122 tagged releases, 40 active development days.**" to reflect 123 tagged releases (matching the presence of v0.0.123 in this file and ROADMAP.md’s count), ensuring the numeric total for "tagged releases" is incremented from 122 to 123 while keeping the rest of the footer text unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@HISTORY.md`:
- Line 260: Edit the v0.0.123 row in HISTORY.md so it contains only the
user-visible outcome (the concise release summary) and remove the internal
“Discovered while …” troubleshooting/process note; specifically trim the text
for the entry referencing v0.0.123 (the row that mentions IO.print writing
mirror live to sys.stdout and the linked issue `#543`) to a single-sentence
outcome-only line and move any explanatory discovery details to CHANGELOG if you
want to preserve them.
---
Outside diff comments:
In `@HISTORY.md`:
- Line 297: Update the tagged-releases total in the HISTORY.md footer: change
the string "Total: **810+ commits, 122 tagged releases, 40 active development
days.**" to reflect 123 tagged releases (matching the presence of v0.0.123 in
this file and ROADMAP.md’s count), ensuring the numeric total for "tagged
releases" is incremented from 122 to 123 while keeping the rest of the footer
text 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: f6dd192e-e614-4ab8-be8b-24422756f556
📒 Files selected for processing (3)
HISTORY.mdROADMAP.mdTESTING.md
Three fixes from the post-rebase CodeRabbit pass on #544: * HISTORY.md footer: "122 tagged releases" -> "123 tagged releases". CodeRabbit reported it as "121 -> 123" but the prior fix in #542 had already taken it to 122; bumping it the rest of the way for this release. * tests/test_runtime_traps.py: add captured.err == "" assertion in test_json_mode_does_not_tee_to_stdout. Mirrors the JSON-mode invariant that TestStdoutOnTrap522 already pins on its four JSON tests -- in JSON mode the entire output is the envelope on stdout and stderr must stay empty so downstream consumers parsing our output see exactly one machine-readable document. * uv.lock: regenerated for the 0.0.122 -> 0.0.123 version bump. Lint job was failing on uv lock --check. Three other CodeRabbit findings verified as either incorrect or out of scope (replying inline on the PR): * HISTORY.md "collapse multi-line entries" -- the v0.0.122 / v0.0.123 rows are already single rows; CodeRabbit was reading wrapped display as multi-line content. * ROADMAP.md "33 examples -> 32 examples" -- ls examples/*.vera reports 33; check_doc_counts.py and the live count agree. CodeRabbit's expected count is wrong. * tests/test_codegen.py opcode assertions -- already strengthened in #542 (commit 75682a1 "PR #542 review fixes"). Markers stay as discoverability anchors; opcode patterns are the contract. * vera/cli.py isatty() gating on the trailing newline -- pre-existing behaviour from before #543, unchanged by this PR. Out of scope: changing the trailing-newline policy is a separate concern with its own backwards-compat surface. * vera/codegen/assembly.py header-validity check -- same finding as the one already discussed and resolved on #542. No separate HEADER_TAG invariant exists in this codebase (header is (size << 1) | mark with no marker bit beyond mark itself). The obj_ptr + obj_size <= heap_ptr bound check is the strongest validation possible without introducing a new tag scheme. Co-Authored-By: Claude <noreply@anthropic.invalid>
Owner
Author
|
@coderabbitai thanks. Two of six findings actioned in 510135a; four verified as either incorrect or already-addressed (details below). Plus the Actioned:
Not actioned (verified):
|
|
Tip For best results, initiate chat on the files or code changes.
Actioned — acknowledged:
Not-actioned — all accepted:
✏️ Learnings added
🧠 Learnings used |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
HISTORY.md (1)
260-260:⚠️ Potential issue | 🟡 MinorTrim the
v0.0.123HISTORY row to an outcome-only summary.Line 260 is carrying implementation/process narrative (
Discovered while ...) and exceeds the repository’s HISTORY row convention.✂️ Proposed tightening
-| v0.0.123 | 27 Apr | **`IO.print` writes mirror live to `sys.stdout`** ([`#543`](https://github.com/aallan/vera/issues/543)) — `vera run` text mode now flushes per call, so animations, progress bars, REPL-style output, and any program using ANSI cursor / clear-screen escapes render in real time instead of dumping the whole transcript at exit. Tee preserves the in-memory capture, so trap preservation (`#522`) and JSON-envelope packaging still work. Discovered while watching the v0.0.122 GC fix run Conway end-to-end and seeing only the final frame. | +| v0.0.123 | 27 Apr | **`IO.print` writes mirror live to `sys.stdout`** ([`#543`](https://github.com/aallan/vera/issues/543)) — `vera run` text mode now flushes per call, so animations, progress bars, REPL-style output, and ANSI cursor / clear-screen escapes render in real time instead of dumping output at exit; tee preserves in-memory capture so trap preservation (`#522`) and JSON-envelope packaging still work. |Based on learnings: In
HISTORY.mdforaallan/vera, “HISTORY file is single-line summaries. Detail is for the CHANGELOG.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@HISTORY.md` at line 260, The HISTORY.md entry for "v0.0.123" is too verbose and includes implementation/process narrative; update the single-line row that starts with "| v0.0.123 | 27 Apr | **`IO.print` writes mirror live to `sys.stdout`** ..." to a concise outcome-only summary (e.g. "`IO.print` now flushes live to stdout so animations/progress render in real time; tee still preserves capture") removing the "Discovered while..." sentence and any extra context so the row conforms to the single-line HISTORY convention.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@TESTING.md`:
- Line 82: The table row describing test_runtime_traps.py is outdated: update
the phrase "all four JSON-emitting tests" to reflect the new count of five
JSON-mode tests; locate the row referencing `test_runtime_traps.py` (the long
description mentioning `trap_kind`, captured `stdout`, captured `stderr`, and
JSON-mode invariants) in TESTING.md and change "four" to "five" so the Markdown
accurately matches the updated `tests/test_runtime_traps.py`.
---
Duplicate comments:
In `@HISTORY.md`:
- Line 260: The HISTORY.md entry for "v0.0.123" is too verbose and includes
implementation/process narrative; update the single-line row that starts with "|
v0.0.123 | 27 Apr | **`IO.print` writes mirror live to `sys.stdout`** ..." to a
concise outcome-only summary (e.g. "`IO.print` now flushes live to stdout so
animations/progress render in real time; tee still preserves capture") removing
the "Discovered while..." sentence and any extra context so the row conforms to
the single-line HISTORY convention.
🪄 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: 55bbf33a-681a-4974-94b2-31fec06e6224
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock,!uv.lock
📒 Files selected for processing (3)
HISTORY.mdTESTING.mdtests/test_runtime_traps.py
Two small follow-ups from the second CodeRabbit pass: * TESTING.md test_runtime_traps.py row: "all four JSON-emitting tests" -> "all five JSON-emitting tests". Stale after round 1 added test_json_mode_does_not_tee_to_stdout (which made it five JSON-mode tests across the file: four in TestStdoutOnTrap522 + one in TestStdoutTee543). * HISTORY.md v0.0.123 row: removed the "Discovered while watching..." trailing sentence per CodeRabbit. The first two sentences are outcome-focused (what changed, what's preserved); the discovery narrative was process not outcome. Other rows in the table follow the same outcome-only convention. Co-Authored-By: Claude <noreply@anthropic.invalid>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #543.
Summary
IO.printwrites were accumulated in an in-memoryoutput_buf(the v0.0.120 implementation of #522 trap preservation) and only flushed tosys.stdoutafterexecute()returned. That was correct for trap preservation and for--jsonoutput, but it had an unintended side effect: programs using ANSI escape sequences (cursor home, clear screen) for animation, progress bars, REPLs, or any other interactive pattern were invisible until exit.The 470-line Conway implementation that surfaced #515 made it visible: ~16 seconds (200 generations × 80ms
IO.sleep) of nothing on screen, then exit fired and only the final frame was visible because the 199 preceding cursor-home + clear-screen escape sequences processed within microseconds and the eye couldn't resolve them.Fix
Tee
host_print(vera/codegen/api.py):output_bufstill receives every byte (soWasmTrapError.stdout,ExecuteResult.stdout, and the--jsonenvelope'sstdoutfield are unchanged — the IO.print output lost when program traps (stdout fully buffered) #522 trap-preservation contract still holds).sys.stdoutwith an explicit per-writeflush()whenexecute(tee_stdout=True).New
tee_stdout: bool = Falseparameter onexecute(). Default off so test helpers (_run_io,_runintests/test_codegen.py) get silent runs.cmd_runtext mode opts in (tee_stdout=not as_json); JSON mode stays off (live writes would corrupt the envelope for downstream consumers parsing our stdout).The
cmd_runtext-mode andWasmTrapError-handler paths now skip re-writingexec_result.stdout/exc.stdout(those bytes already streamed live), only emitting a closing\nif the program's last write didn't include one — without that change every transcript would have double-printed.Verification
Pre-fix: nothing visible for 1 second, then all three lines appear at once. Post-fix: each tick appears live as
IO.sleepreturns.Tests
New
TestStdoutTee543class intests/test_runtime_traps.py(6 tests):IO.printappears exactly once)IO.printcountexecute()behaviour stays silent for the test suiteDocumentation
IO.printrow in the IO operation table notes "no implicit newline; flushes per call"; new paragraph after the table explains the buffering / live-write semantics for text mode and JSON mode and the trap-preservation guarantee.Note on merge order
This PR builds on top of #542 (v0.0.122 GC fix). Recommended merge order: #542 first, then this. No code conflict either way; if this lands first, #542 needs a small CHANGELOG/HISTORY/version-link rebase.
Test plan
pytest tests/ -q— 3,561 passed, 14 skippedmypy vera/— cleanpython scripts/check_doc_counts.py— consistent (3,575 tests)python scripts/check_version_sync.py— version 0.0.123 consistentIO.print+IO.sleepprogram renders liveCo-Authored-By: Claude noreply@anthropic.invalid
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests