cli: surface the E602 skip warning on the compile error path (#1004)#1021
Conversation
When CodegenSkip drops a called function, the caller's dangling call fails WAT assembly with an opaque `unknown func`; cmd_compile returned on the text error path before printing warnings, so the E602 skip reason was dropped (the --json envelope already carried it). Print accumulated warnings on the error path too, and drop the now-covered pragma. Docs lockstep: KNOWN_ISSUES/SKILL rows removed, ROADMAP PR-C retired, CHANGELOG + counts. Co-Authored-By: Claude <noreply@anthropic.invalid>
📝 WalkthroughWalkthrough
ChangesCompile diagnostics
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1021 +/- ##
=======================================
Coverage 93.51% 93.51%
=======================================
Files 96 96
Lines 32005 32017 +12
Branches 456 456
=======================================
+ Hits 29929 29941 +12
Misses 2063 2063
Partials 13 13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Greptile SummaryThis PR fixes a presentation gap in
Confidence Score: 5/5Safe to merge — a small, targeted text-output addition with no behavioural changes to compilation logic and full regression coverage. The change touches only the stderr-printing loops on two error return paths in cmd_compile; the actual compiler pipeline is untouched. Both new code paths are exercised by tests confirmed RED before the fix, the previously-noted type-error path asymmetry has been addressed in this same PR, and all documentation counts are internally consistent. No files require special attention.
|
| Filename | Overview |
|---|---|
| vera/cli.py | Adds warning printing before errors on both the type-error text path (new) and codegen-error text path (primary fix); removes the pragma: no cover now covered by the new test |
| tests/test_cli.py | Adds three regression tests: text-path E602 warning visible, JSON-path E602 warning (control), and type-error text path warning visibility — each validated RED-first |
| CHANGELOG.md | Adds [Unreleased] Fixed entry covering both the codegen-error and type-error path corrections |
| KNOWN_ISSUES.md | Removes the now-resolved #1004 row; updates the #1006 cross-ref to reflect E602 is now visible even when a caller exists |
| ROADMAP.md | Retires PR-C row, reduces sprint count from 11 to 10, updates test count to 7,403 |
| TESTING.md | Updates test_cli.py count (260 to 263) and line count (4,033 to 4,109) to match the three new tests; updates total tests to 7,403 |
| SKILL.md | Removes the now-obsolete Opaque unknown func after a skip workaround row |
| docs/SKILL.md | Mirror of SKILL.md: same row removed from the docs copy |
| docs/llms-full.txt | Mirror of SKILL.md: same row removed from the LLM-readable asset |
| README.md | Updates test count in the project status line from 7,400 to 7,403 |
Reviews (2): Last reviewed commit: "cli: also print type_warnings on the com..." | Re-trigger Greptile
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 `@ROADMAP.md`:
- Around line 17-29: Add a one-sentence entry for completed issue `#1004` to the
relevant current Stage table in HISTORY.md, matching the table’s existing style
and documenting the CLI-diagnostics work. Do not modify the ROADMAP.md diff or
add unrelated history details.
In `@tests/test_cli.py`:
- Around line 755-774: Update test_compile_skip_warning_shown_on_error_path_1004
to assert the specific E602 diagnostic marker "[E602]" instead of the broad
"skipped" substring, while preserving the existing unknown-func assertion and
error-path expectations.
🪄 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: 9a33dac5-49cc-49af-a22e-ef0968ed1c0b
⛔ Files ignored due to path filters (2)
docs/SKILL.mdis excluded by!docs/**docs/llms-full.txtis excluded by!docs/**
📒 Files selected for processing (8)
CHANGELOG.mdKNOWN_ISSUES.mdREADME.mdROADMAP.mdSKILL.mdTESTING.mdtests/test_cli.pyvera/cli.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
aallan/vera-bench(manual)
💤 Files with no reviewable changes (1)
- SKILL.md
…eview) Greptile review flagged the sibling asymmetry: cmd_compile's text type-error path dropped type_warnings while its JSON branch carried them — the same class as the codegen-error path this PR fixed. Mirror the warning-print there, tested with a type-error + E200-warning program. Also tighten the skip-warning test to assert the [E602] code marker (CodeRabbit). Counts +1 -> 7403. Co-Authored-By: Claude <noreply@anthropic.invalid>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
vera/cli.py (1)
448-455: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEmit accumulated warnings on the
--target wasi-p2error path. Invera/cli.py, theemit_wasi_component(...)ValueErrorbranch still prints onlyError: ...in text mode, while the JSON branch includesall_warnings. Print those warnings there too so compile failures don’t drop the earlier diagnostics.🤖 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 `@vera/cli.py` around lines 448 - 455, Update the text-mode ValueError handling in emit_wasi_component to print every accumulated warning from all_warnings to stderr alongside the existing error message. Preserve the JSON branch and warning formatting already used 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.
Outside diff comments:
In `@vera/cli.py`:
- Around line 448-455: Update the text-mode ValueError handling in
emit_wasi_component to print every accumulated warning from all_warnings to
stderr alongside the existing error message. Preserve the JSON branch and
warning formatting already used elsewhere.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 02f1dbd8-c84f-4180-bb62-8c69ea3e226a
📒 Files selected for processing (6)
CHANGELOG.mdREADME.mdROADMAP.mdTESTING.mdtests/test_cli.pyvera/cli.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
aallan/vera-bench(manual)
|
@coderabbitai The outside-diff finding on |
|
Tip For best results, initiate chat on the files or code changes.
✏️ Learnings added
|
Summary
vera compile(text mode) now prints the E602 "function skipped" warning on the error path, so aCodegenSkipof a called function no longer surfaces as an opaqueunknown funcalone.When
CodegenSkipdrops a function (an unsupported construct) that has a caller, the caller's danglingcall $ffails WAT assembly withunknown func: failed to find name $f.cmd_compilereturned after printing only the errors on the text path, so the E602 warning explaining why the function was skipped was dropped. The warning was already inresult.diagnostics— the--jsonenvelope emitted it — so this was purely a text-presentation gap.Change
vera/cli.py— print accumulated warnings on the text error path too (the JSON error branch already included them); remove the now-coveredpragma: no cover.tests/test_cli.py— a text test (skippednow prints alongsideunknown func), reproduced RED first, plus a JSON control that covers the other leg of the codegen-error branch.Validation
Reproduced live first (
hash(@Decimal)in a contract predicate skipsh;maincalls it): text mode showed onlyunknown func, JSON carriedwarnings: [E602]. RED text test confirmed the drop, GREEN after the fix.test_cli.py(262) + the #922 degradation suite pass; mypy, ruff, doc-counts, and site-assets gates green.Docs
KNOWN_ISSUES + SKILL rows removed, the #1006 cross-ref de-staled, ROADMAP's PR-C retired (Stage 19 sprint count 11 → 10), CHANGELOG
[Unreleased]entry, test counts reconciled (7402). Site assets regenerated.Closes #1004
🤖 Generated with Claude Code
Summary by CodeRabbit
vera compilenow shows the E602 “function skipped” warning alongside the related compilation errors on both text and JSON outputs.