Hardening Phase 4: enforced VM↔interpreter parity gate#91
Conversation
…orpus `epl run` defaults to the bytecode VM while docs/tests validate against the tree-walking interpreter, so a program that behaves differently across the two backends is a divergence bug nothing caught: tests/parity_check.py diffed them but always exited 0, never gating CI. Add tests/test_parity_corpus.py — walks the real corpus (examples/ + benchmarks/, recursively) and runs every eligible program through BOTH backends via the actual CLI, asserting each exits 0 with byte-identical stdout (54 programs, 0 divergences). Ineligible programs are excluded by a documented, directory-agnostic content filter (servers, interactive, socket/GUI loops, the Node-dependent JS bridge, the Test..End Test DSL, and nondeterministic random/uuid output). Everything else is included by default (fail-closed): a new compute example is auto-covered, and a program that hangs FAILS on a per-program timeout instead of being silently skipped — the failure mode the advisory harness swallowed. parity_check.py keeps its rich manual diffs and now points to the enforced gate.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
epl | 62fb25b | Commit Preview URL Branch Preview URL |
Jul 11 2026, 04:09 PM |
|
Warning Review limit reached
Next review available in: 46 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughThe PR adds a CI parity gate that recursively runs eligible example and benchmark programs through the VM and interpreter using the CLI, requiring successful exits and byte-identical stdout. It also documents the gate and clarifies the manual parity harness. ChangesVM/interpreter parity enforcement
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Pytest
participant EPLCLI
participant Backend
Pytest->>EPLCLI: Run eligible program
EPLCLI->>Backend: Execute with VM or interpreter
Backend-->>EPLCLI: Return exit code and stdout
EPLCLI-->>Pytest: Return captured results
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/test_parity_corpus.py`:
- Around line 48-77: Tighten matching in the _SKIP_TOKENS filtering logic so
collision-prone entries such as “random” and “uuid” are recognized as standalone
words rather than arbitrary substrings, while preserving intentional exclusions.
Alternatively, document in the relevant test/docstring that substring matching
is deliberately over-inclusive; prefer the boundary-aware approach where the
matcher is visible.
🪄 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 Plus
Run ID: 302155b8-1a13-47c0-bf42-d7fa9d767f53
📒 Files selected for processing (3)
CHANGELOG.mdtests/parity_check.pytests/test_parity_corpus.py
| _SKIP_TOKENS = ( | ||
| # Servers and never-exit event loops: they don't run to completion, so there is no | ||
| # terminal stdout/exit status to compare. | ||
| 'WebApp', | ||
| 'Route ', | ||
| 'Listen', | ||
| 'Serve', | ||
| 'serve(', | ||
| 'run_server', | ||
| 'web_api_create', | ||
| # Interactive: block on stdin, so they can't run unattended. | ||
| 'Ask ', | ||
| 'Prompt ', | ||
| 'input(', | ||
| 'Read line', | ||
| # Sockets / desktop GUI event loops: block like a server. | ||
| 'websocket', | ||
| 'WebSocket', | ||
| 'gui_window', | ||
| # Environment-dependent: the JS/TS bridge needs a Node.js runtime that may be absent. | ||
| 'Use javascript', | ||
| # The `Test "…" … End Test` assertion DSL has its own runner; it is not `epl run` code. | ||
| 'End Test', | ||
| # Nondeterministic output: random/uuid legitimately differ between any two runs, so | ||
| # the two backends can't be expected to emit byte-identical stdout. | ||
| 'random', | ||
| 'uuid', | ||
| 'generate_uuid', | ||
| 'random_string', | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Consider tightening skip-token matching to reduce silent over-exclusion.
Several _SKIP_TOKENS entries are bare substrings ('random', 'uuid', 'Listen', 'Serve') that will match anywhere in the file — including comments, variable names, or unrelated strings. A program that merely mentions "random" in a comment would be silently excluded from parity coverage. While this is fail-safe (over-exclusion, not under-exclusion), it can quietly erode coverage without anyone noticing. The >= 30 threshold in test_corpus_present is a coarse backstop but won't catch gradual drift.
Consider word-boundary checks (e.g., regex \b or token-aware scanning) for the shorter, more collision-prone tokens like 'uuid' and 'random', or at minimum adding a brief note in the docstring that the filter intentionally over-matches.
🤖 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 `@tests/test_parity_corpus.py` around lines 48 - 77, Tighten matching in the
_SKIP_TOKENS filtering logic so collision-prone entries such as “random” and
“uuid” are recognized as standalone words rather than arbitrary substrings,
while preserving intentional exclusions. Alternatively, document in the relevant
test/docstring that substring matching is deliberately over-inclusive; prefer
the boundary-aware approach where the matcher is visible.
|
Thanks — evaluated the skip-token nitpick as a senior dev, verifying against the code. Accepted (both shipped in 62fb25b):
Declined — word-boundary matching: it would risk the worse failure mode. |
Phase 4 — VM↔interpreter parity gate
epl rundefaults to the bytecode VM, while the docs and most of the suite validate against the tree-walking interpreter. A program that behaves differently across the two backends is a divergence bug — and nothing caught it:tests/parity_check.pydiffed the backends but always exited 0, so it never gated CI.What this adds
tests/test_parity_corpus.py— walks the real corpus (examples/+benchmarks/, recursively) and runs every eligible program through both backends via the actual CLI, asserting each exits0with byte-identical stdout.test_examples_run.py's top-level-only glob, this coversexamples/apps/,examples/discord_agent/,benchmarks/, …): servers/never-exit loops, interactive stdin-blocking, socket/GUI event loops, the Node-dependentUse javascriptbridge, theTest … End TestDSL, and nondeterministicrandom/uuidoutput.parity_check.pykeeps its rich manual per-divergence diffs and now points to the enforced gate.Verification
ruff format --check+ruff checkclean.Summary by CodeRabbit