fix(tooling): require the plumbing-skip's sole ctor be reachable as the helper's result - #6
Open
chethanuk wants to merge 19 commits into
Open
fix(tooling): require the plumbing-skip's sole ctor be reachable as the helper's result#6chethanuk wants to merge 19 commits into
chethanuk wants to merge 19 commits into
Conversation
Bumps the `wasmtime` runtime floor from `>=45.0.0` to `>=46.0.1`, off a release affected by GHSA-4ch3-9j33-3pmj (medium) — a `wasmtime-wasi` `FilePerms` bypass on hard-link and rename destinations, whose affected range includes the `45.0.0` that `uv.lock` had pinned. The upstream core backport is `45.0.3`, but the `wasmtime-py` binding never tagged a `45.0.x` patch, so `46.0.1` is the only patched binding release. CI already resolved the unbounded floor to `46.0.1` (verified against main's own install log), so this is behaviour-neutral for the test matrix; the value is pinning the lockfile and the declared floor off the affected release for minimum-version resolvers (`uv sync`, distro packagers, pre-existing venvs). Vera's own WASI host (`vera/runtime/wasi_host.py`) is not exploitable: it preopens with the default `READ_WRITE` `FilePerms`, so there is no restricted permission for a link/rename to bypass. The maintainer commit additionally reconciles the pin's rationale comment (Dependabot's bump left it describing `>=45.0.0`) to record both the load-bearing `>=45` floor requirement (the wasmtime-py#337 `BaseException` host-import trampoline that `execute()`'s exit-130 mapping depends on, aallan#595/aallan#599) and the security reason for `46.0.1`, and adds a CHANGELOG `### Security` entry. Co-Authored-By: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-Authored-By: Claude <noreply@anthropic.invalid>
Raises the `setuptools` build-system floor from `>=82.0.1` to `>=83.0.0`.
`83.0.0` is the latest release, unyanked, and requires Python `>=3.10`
(below Vera's `>=3.11` floor). It affects only `[build-system].requires`,
which CI exercises with build isolation on all twelve
`{ubuntu, macos-15, macos-26, windows} × {3.11, 3.12, 3.13}` cells;
`uv.lock` does not record setuptools, so no lockfile refresh is owed and
`uv lock --check` stays meaningful.
Co-Authored-By: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-Authored-By: Claude <noreply@anthropic.invalid>
…docs
Hand-authored, self-contained SVGs in assets/diagrams/ (design system,
conventions, and inventory in its README.md), embedded across
vera/README, README, spec/06/07/11/12/13, DE_BRUIJN, TOOLCHAIN, TESTING,
LSP_SERVER and HISTORY. The headline is the overall compiler
architecture diagram vera/README.md previously lacked.
Every figure that replaced an ASCII original keeps the text in a
collapsed <details> block tagged `text` (skipped by the doc gates'
skip_langs) — the docs are read by agents in terminals and fed to
llms-full.txt, where images are invisible. No diagram carries live
counts; the one data figure (history-growth) plots the historical
release columns from HISTORY.md and says so on its face.
The architecture and pipeline figures draw the check -> {verify |
compile} fork truthfully: vera compile does not consume verify results
and contract guards are always emitted. The replaced ASCII's "runtime
contract insertion for Tier 3" caption echoed the spec drift tracked in
aallan#958; its corrected text version rides in the details block. README's
project-structure tree also gets its stale module counts fixed
(codegen 11 -> 13, wasm 9 -> 19, verified against the tree). All embeds
use repo-relative paths so they render from any ref; the latent
switch-to-absolute-if-ever-published-to-PyPI constraint is recorded in
assets/diagrams/README.md.
Co-Authored-By: Claude <noreply@anthropic.invalid>
Extends the set on maintainer go, all candidates including the marginal tier: the subtyping and effect-row lattices (spec §2.8, §7.8), module resolution with the transitive-but-not-visible rule (§8.6), the async eager/concurrent model and the vera serve request lifecycle (§9.5), closure heap layout + call_indirect dispatch (§11.10), browser dynamic import introspection (§12.9.1), the WASI GC-exempt arena and the server-world adapter sequence (§13.3, §13.7), the FAQ's three verification layers, the contract-driven-testing loop, and the Dafny/Lean/Koka/F* comparison matrix (grounded strictly in the FAQ's own claims), the CONTRIBUTING gate pipeline (count-free — hook counts are policed doc counts), the DE_BRUIJN binding-stack timeline, and the host-binding-families map with the IO-stays-inline rationale (vera/README.md). Same design system and conventions as wave 1; every embed verified to sit under its intended section heading. The ci-gates and comparison figures carry no live counts; each cell of the comparison traces to a sentence in FAQ.md. Co-Authored-By: Claude <noreply@anthropic.invalid>
… drift The behavioural fix: vera/README.md §Runtime contracts still described the pre-aallan#957 world (codegen "classifies contracts using the verifier's tier results", Tier-1 guards "omitted"), contradicting the architecture diagram embedded above it. It now states the truth: compile never consults the verifier, guards are always emitted, and the §11.8 aspiration stays tracked in aallan#958. Drift, each verified against the tree, the registry, or the tracker before fixing: SKILL's conformance count (103 -> 143) + spec table chapter-13 row + both multi-file conformance companions named; CLAUDE/AGENTS pipeline gains the resolve stage (and resolve_imports in the stage-function list); FAQ gains IO.read_char and the Exn<T> effect; README's "three-tier verification" delivered-claim scoped to the two implemented tiers; DESIGN gains HttpServer in the effects row (a list claiming to mirror `vera effects --json`), HttpServer/Random/Diverge rows in the built-in effects table, a Tier-1 coverage sentence aligned to spec §6.8, and the tiers + effect-row-lattice diagrams; KNOWN_ISSUES gains rows for the open aallan#439 and aallan#770 limitations and clarifies the shipped-aallan#813 row header; HISTORY's stage index gains Stages 16-17 and the intro catches up (Stage 17, 94 days); vera/README's module-map line counts regenerated from disk (worst: verifier.py 1,005 -> 6,582; two same-named rows disambiguated by section) + eq_ability.py row + JS totals; TESTING drops a reference to the split-away test_codegen.py; spec §12 stops citing the closed aallan#465 as a tracker and unifies the heap-growth wording; TOOLCHAIN's text version moves fmt to the parse column (vera fmt formats a type-broken file - verified empirically); DE_BRUIJN frames its early VeraBench figure as a snapshot. build_site.py now rewrites IMAGE embeds to raw.githubusercontent URLs - a blob/ page is HTML, not image bytes - fixing the four diagrams inlined into llms-full.txt. Also fixes six rendering defects in the wave-2 SVGs (two z-order occlusions: host-families' browser/IO cards under the adapter band, lsp-session's gate card under the invalidation card; four text overflows: language-comparison NOTES column, effect-handlers notes, browser-bindings, wasi-arena), found by maintainer review + a systematic overflow/overlap scan of all 33. Examples and conformance fixtures came back clean: no workaround shapes for fixed bugs. Test-code staleness (the aallan#869 table-forcing fold, the aallan#570 wide-graph avoidance, the aallan#516 docstring, the ch09_decimal "not supported" note - reproduced working) lands as a separate PR. Co-Authored-By: Claude <noreply@anthropic.invalid>
One figure, in the site's own design language (paper-transparent, ink pills, single flame accent, JetBrains Mono): the model writes -> the compiler proves -> it ships, with the dotted-flame diagnostics return. Embedded in docs/index.html §Why — the sentence it draws — and mirrored into the generated index.md via build_index_md. Arrowheads use markerUnits=userSpaceOnUse so head stroke weight equals line weight (the default strokeWidth scaling rendered ~18px heads with ~3.6px strokes against 2px dotted lines), the return line is 2.5px true round dots (dotted reads lighter than solid at equal weight), and the path stops short of the caption so the head sits in clear air — maintainer review findings. Co-Authored-By: Claude <noreply@anthropic.invalid>
…n#959 review) The wave-1 integration mis-targeted this block, and the revert reconstructed it from the details copy without its original language tag — an MD040 regression against main. Byte-identical to main again. Co-Authored-By: Claude <noreply@anthropic.invalid>
The consistency sweep's test auditors found five places where the test code still described (or worked around) bugs that are long fixed; each change was verified by running the affected tests before and after. - tests/test_codegen_monomorphize.py: TWO_PARAM_SRC carried an array_fold whose only purpose was forcing a dispatch table around the closed aallan#869; the fold is gone and main calls unwrap_second directly (sum unchanged at 107) — the fixture itself is now the regression pin. - tests/test_codegen_gc_alloc.py: the aallan#570-era "1,000 elements to stay under the bug threshold" graph is promoted to the true 5,000-element Array<Box> wide graph the docstring always intended (test_wide_graph_with_gc_pressure, sum 12,497,500). - tests/test_runtime_traps.py: the module docstring deferred "Stage 2 (source mapping) and Stage 3 (Fix: paragraphs)" as future aallan#516 work; both shipped (v0.0.124/v0.0.125) and are exercised throughout the file — the docstring now says so. - tests/conformance/ch09_decimal.vera: the "decimal_compare is not yet supported in codegen" note was disproven by running it; the program now exercises decimal_compare through a three-arm Ordering match (Less/Equal/Greater → 1+2+4, ensures ==7; main sum 51 → 58), with a matching manifest feature tag. - tests/test_codegen_data_types.py: the aallan#773 parenthetical now speaks of the scalar-representation Eq limitation in the past tense. TESTING.md's per-file table follows the four shifted line counts. Skip-changelog: test-code annotation hygiene only; no compiler, spec, or behaviour change (the CHANGELOG sweep bullet covering this lands with the v0.1.1 release commit). Co-Authored-By: Claude <noreply@anthropic.invalid>
Cuts v0.1.1 and opens HISTORY Stage 18 (the visual documentation pass): the 34-figure SVG set, the six-auditor consistency sweep, the first external contributions + the aallan#957 verifier-soundness fix, the wasmtime CVE floor bump, and the stale-test retirement. - CHANGELOG: [Unreleased] rolls to [0.1.1] - 2026-07-08 (+ compare links); the stale-test retirement gains its bullet under Fixed. - Version bumps: vera/__init__.py, pyproject.toml, uv.lock, docs/index.html tag link. - HISTORY: intro + By-the-numbers total advance to Stage 18 with the git-computed active-day count; the stage index closes row 17 and adds row 18; the Stage 15/16/17 section headings close their date ranges to match the index (the sweep closed the index side only); the Stage 18 section lands with the v0.1.1 row. - docs/: site assets regenerated (build_site.py). Co-Authored-By: Claude <noreply@anthropic.invalid>
docs: v0.1.1 — the 34-figure visual documentation release
The loop-web.svg embed rendered top-heavy and visually left-shifted: the drawing occupied a 1000x252 canvas with 70 units of dead space above and 166 to the right (vs 30 left, 18 below). The viewBox is recropped symmetric around the browser-measured content bbox (12 units each side) and the embed's CSS margins equalised — 48px of air above and below the drawing, and it now spans the column. The audit that rode along, every claim checked against the registry or a live run: - The effect enumeration gains Diverge and "eight algebraic effects" corrects to nine (vera effects --json lists 9 effects); README.md's identical list gets the same one-word fix. - The Inference provider list gains Mistral in both places it appears (vera/runtime/inference.py detects four provider keys). - The three-tier feature card is scoped to what shipped, mirroring README: Z3 static + runtime fallback delivered, the Z3-guided middle tier specified but not implemented. - The WASI showcase shows the compile command's actual output line and a consistent examples/http_server.wasm artifact path (the old text was a paraphrase with a mismatched path); the wasmtime serve banner was verified against a real run. - safe_divide and classify_sentiment link their examples/ counterparts the way the fizzbuzz sample does; the generated index.md gets the same links plus the fizzbuzz one it never had. scripts/build_site.py carries the parallel copies for the generated index.md; docs/ assets regenerated. hello_world / factorial outputs, the browser-bundle listing, and the E001 block (pinned by TestErrorDisplaySync, 28 tests green) all verified unchanged-correct. Co-Authored-By: Claude <noreply@anthropic.invalid>
docs(site): rebalance the loop figure + audit the landing page's claims
|
CodeAnt AI is reviewing your PR. |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Warning Review limit reached
Next review available in: 42 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 Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThe PR tightens the diagnostic plumbing-skip rule so a helper’s ChangesDiagnostic plumbing, versioning, and release notes
Documentation, diagrams, and planning updates
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 6 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsLinked repositories: Public OSS repositories can only analyze public repositories installed in this organization. No linked repositories were analyzed; skipped 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 |
codeant-ai
Bot
added
the
size:L
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request narrows the diagnostic-fields gate's plumbing-skip logic by ensuring that a helper's sole own-scope Diagnostic constructor is actually reachable as the helper's result (either returned, appended, or bound to a local that is later returned/appended). It also raises the wasmtime dependency floor to >=46.0.1 to address a security advisory (GHSA-4ch3-9j33-3pmj) and updates the test suite and documentation accordingly. The review feedback suggests supporting annotated assignments (ast.AnnAssign) in the reachability check to prevent annotated plumbing constructors from being incorrectly flagged as non-reachable.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+290
to
+292
| if (isinstance(node, ast.Assign) and node.value is ctor | ||
| and len(node.targets) == 1 and isinstance(node.targets[0], ast.Name)): | ||
| local_name = node.targets[0].id |
There was a problem hiding this comment.
In Python, variable assignments can also be written as annotated assignments (e.g., d: Diagnostic = Diagnostic(...)), which are represented in the AST as ast.AnnAssign rather than ast.Assign. Since this codebase uses type annotations and mypy, it is highly recommended to support ast.AnnAssign so that annotated plumbing constructors are not incorrectly flagged as non-reachable.
Suggested change
| if (isinstance(node, ast.Assign) and node.value is ctor | |
| and len(node.targets) == 1 and isinstance(node.targets[0], ast.Name)): | |
| local_name = node.targets[0].id | |
| if (isinstance(node, ast.Assign) and node.value is ctor | |
| and len(node.targets) == 1 and isinstance(node.targets[0], ast.Name)): | |
| local_name = node.targets[0].id | |
| elif isinstance(node, ast.AnnAssign) and node.value is ctor and isinstance(node.target, ast.Name): | |
| local_name = node.target.id |
|
CodeAnt AI finished reviewing your PR. |
7 tasks
… into v0.1.1 (aallan#963) Six themed stages (19–24) continuing HISTORY's numbering replace the tier/milestone mix, all 100 open issues placed exactly once; the fold-in moves the landing-page-audit and roadmap bullets into [0.1.1], corrects the external-contribution history (chethanuk is the third outside contributor, after @rlseaman PR aallan#173 and @rzyns PR aallan#685), and trims the Stage 17/18 intros to the established scale. Co-Authored-By: Claude <noreply@anthropic.invalid>
chethanuk
force-pushed
the
fix/issue-956-plumbing-skip-reachability
branch
2 times, most recently
from
July 9, 2026 01:44
b54f2be to
0f08954
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@scripts/check_diagnostic_fields.py`:
- Around line 276-284: _bump_target currently only walks ast.Name, ast.Tuple,
and ast.List, so star-unpacking targets like a, *b or for/with targets with
starred bindings are missed and under-counted by _rebound_names. Update
_bump_target in check_diagnostic_fields.py to recurse into ast.Starred nodes as
well, using its nested value so rebounded names inside star-unpacking are
counted. This will keep _ctor_is_reachable_as_result from treating star-unpacked
rebinding as still reachable.
🪄 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: a2818350-9420-453a-932f-723cd67bb554
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock,!uv.lock
📒 Files selected for processing (6)
CHANGELOG.mdROADMAP.mdTESTING.mdpyproject.tomlscripts/check_diagnostic_fields.pytests/test_diagnostic_fields.py
chethanuk
force-pushed
the
fix/issue-956-plumbing-skip-reachability
branch
from
July 9, 2026 03:43
0f08954 to
f1267f4
Compare
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 `@DESIGN.md`:
- Around line 25-33: The “Inference” row in DESIGN.md overstates current support
by claiming LLM calls are mockable via handle[Inference], but user-defined
Inference handlers are not yet supported. Update that row’s description in the
comparison table to remove the handle[Inference] claim and rephrase it to
describe only the currently supported typed, contract-verifiable, explicit
Inference.complete effect behavior.
In `@README.md`:
- Line 226: The documented test counts are inconsistent between README and
TESTING.md, so update the stale count to match the authoritative source used by
check_doc_counts.py. Review the test total in README alongside the Overview
table in TESTING.md, then make both files report the same current number of
tests and files so the docs stay aligned with the codebase.
🪄 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: 51652b4d-5520-4ea0-9ec0-b9b5429851d6
⛔ Files ignored due to path filters (41)
assets/diagrams/architecture.svgis excluded by!**/*.svgassets/diagrams/async-model.svgis excluded by!**/*.svgassets/diagrams/browser-bindings.svgis excluded by!**/*.svgassets/diagrams/checker-passes.svgis excluded by!**/*.svgassets/diagrams/ci-gates.svgis excluded by!**/*.svgassets/diagrams/closure-layout.svgis excluded by!**/*.svgassets/diagrams/contract-testing.svgis excluded by!**/*.svgassets/diagrams/diagnostic-card.svgis excluded by!**/*.svgassets/diagrams/effect-handlers.svgis excluded by!**/*.svgassets/diagrams/effect-row-lattice.svgis excluded by!**/*.svgassets/diagrams/faq-layers.svgis excluded by!**/*.svgassets/diagrams/gc-cycle.svgis excluded by!**/*.svgassets/diagrams/history-growth.svgis excluded by!**/*.svgassets/diagrams/host-families.svgis excluded by!**/*.svgassets/diagrams/httpserver-lifecycle.svgis excluded by!**/*.svgassets/diagrams/language-comparison.svgis excluded by!**/*.svgassets/diagrams/lsp-session.svgis excluded by!**/*.svgassets/diagrams/memory-layout.svgis excluded by!**/*.svgassets/diagrams/module-resolution.svgis excluded by!**/*.svgassets/diagrams/pipeline.svgis excluded by!**/*.svgassets/diagrams/server-world.svgis excluded by!**/*.svgassets/diagrams/slot-evolution.svgis excluded by!**/*.svgassets/diagrams/slot-numbering.svgis excluded by!**/*.svgassets/diagrams/slot-scopes.svgis excluded by!**/*.svgassets/diagrams/subtyping-lattice.svgis excluded by!**/*.svgassets/diagrams/testing-layers.svgis excluded by!**/*.svgassets/diagrams/tiers.svgis excluded by!**/*.svgassets/diagrams/toolchain.svgis excluded by!**/*.svgassets/diagrams/wasi-arena.svgis excluded by!**/*.svgassets/diagrams/wasi-component.svgis excluded by!**/*.svgassets/diagrams/wasmtime-embedding.svgis excluded by!**/*.svgassets/diagrams/workflow.svgis excluded by!**/*.svgassets/diagrams/z3-refutation.svgis excluded by!**/*.svgdocs/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/**docs/loop-web.svgis excluded by!**/*.svg,!docs/**tests/conformance/ch09_decimal.verais excluded by!**/*.verauv.lockis excluded by!**/*.lock,!uv.lock
📒 Files selected for processing (35)
AGENTS.mdCHANGELOG.mdCLAUDE.mdCONTRIBUTING.mdDESIGN.mdDE_BRUIJN.mdFAQ.mdHISTORY.mdKNOWN_ISSUES.mdLSP_SERVER.mdREADME.mdROADMAP.mdSKILL.mdTESTING.mdTOOLCHAIN.mdassets/diagrams/README.mdpyproject.tomlscripts/build_site.pyscripts/check_diagnostic_fields.pyspec/02-types.mdspec/06-contracts.mdspec/07-effects.mdspec/08-modules.mdspec/09-standard-library.mdspec/11-compilation.mdspec/12-runtime.mdspec/13-wasi.mdtests/conformance/manifest.jsontests/test_codegen_data_types.pytests/test_codegen_gc_alloc.pytests/test_codegen_monomorphize.pytests/test_diagnostic_fields.pytests/test_runtime_traps.pyvera/README.mdvera/__init__.py
Comment on lines
+25
to
+33
| | Effects | Algebraic, row-polymorphic (`IO`, `Http`, `HttpServer`, `State`, `Async`, `Inference`, `Random`, `Diverge`, plus the parameterised exception effect `Exn<T>` — all reported by `vera effects --json`) | All state and side effects explicit; effects are typed, trackable, and handleable | | ||
| | Error handling | `Result<T,E>` ADTs for expected errors; `Exn<T>` algebraic effect for exceptions | Errors are values; `match` enforces handling every case; `Exn<T>` is handleable like any other effect | | ||
| | Inference | `Inference.complete` as an algebraic effect | LLM calls are typed, contract-verifiable, mockable via `handle[Inference]`, and explicit in signatures | | ||
| | Data types | Algebraic data types + exhaustive `match` | No classes, no inheritance; compiler enforces every case is handled | | ||
| | Polymorphism | Monomorphized generics (`forall<T where Eq<T>>`) | No runtime dispatch; four built-in abilities (`Eq`, `Ord`, `Hash`, `Show`); types fully specialised at compile time | | ||
| | Refinement types | `{ @T \| predicate }` checked by Z3 | Encode value-level constraints in the type system; rejected statically or at runtime | | ||
| | Collections | `Array<T>`, `Map<K,V>`, `Set<T>` | Functional, immutable; no mutation, no loops; `array_map`/`filter`/`fold`/`slice` as built-ins | | ||
| | Standard library | 164 built-in functions | Strings, arrays, maps, sets, decimals, math (log/trig/constants/utilities), JSON, HTML, Markdown, regex, base64, URL — no external deps | | ||
| | Modules | `module`/`import` with explicit re-exports | Programs split across files; `vera check` resolves the module graph | | ||
| | Modules | `module`/`import` with explicit `public`/`private` visibility | Programs split across files; `vera check` resolves the module graph; re-exports are tracked in [#127](https://github.com/aallan/vera/issues/127) | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Remove the handle[Inference] claim.
As per KNOWN_ISSUES.md, user-defined Inference handlers are still unsupported, so this row overstates current behaviour.
Proposed fix
-| Inference | `Inference.complete` as an algebraic effect | LLM calls are typed, contract-verifiable, mockable via `handle[Inference]`, and explicit in signatures |
+| Inference | `Inference.complete` as an algebraic effect | LLM calls are typed, contract-verifiable, and explicit in signatures; user-defined handlers are not yet supported |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| | Effects | Algebraic, row-polymorphic (`IO`, `Http`, `HttpServer`, `State`, `Async`, `Inference`, `Random`, `Diverge`, plus the parameterised exception effect `Exn<T>` — all reported by `vera effects --json`) | All state and side effects explicit; effects are typed, trackable, and handleable | | |
| | Error handling | `Result<T,E>` ADTs for expected errors; `Exn<T>` algebraic effect for exceptions | Errors are values; `match` enforces handling every case; `Exn<T>` is handleable like any other effect | | |
| | Inference | `Inference.complete` as an algebraic effect | LLM calls are typed, contract-verifiable, mockable via `handle[Inference]`, and explicit in signatures | | |
| | Data types | Algebraic data types + exhaustive `match` | No classes, no inheritance; compiler enforces every case is handled | | |
| | Polymorphism | Monomorphized generics (`forall<T where Eq<T>>`) | No runtime dispatch; four built-in abilities (`Eq`, `Ord`, `Hash`, `Show`); types fully specialised at compile time | | |
| | Refinement types | `{ @T \| predicate }` checked by Z3 | Encode value-level constraints in the type system; rejected statically or at runtime | | |
| | Collections | `Array<T>`, `Map<K,V>`, `Set<T>` | Functional, immutable; no mutation, no loops; `array_map`/`filter`/`fold`/`slice` as built-ins | | |
| | Standard library | 164 built-in functions | Strings, arrays, maps, sets, decimals, math (log/trig/constants/utilities), JSON, HTML, Markdown, regex, base64, URL — no external deps | | |
| | Modules | `module`/`import` with explicit re-exports | Programs split across files; `vera check` resolves the module graph | | |
| | Modules | `module`/`import` with explicit `public`/`private` visibility | Programs split across files; `vera check` resolves the module graph; re-exports are tracked in [#127](https://github.com/aallan/vera/issues/127) | | |
| | Effects | Algebraic, row-polymorphic (`IO`, `Http`, `HttpServer`, `State`, `Async`, `Inference`, `Random`, `Diverge`, plus the parameterised exception effect `Exn<T>` — all reported by `vera effects --json`) | All state and side effects explicit; effects are typed, trackable, and handleable | | |
| | Error handling | `Result<T,E>` ADTs for expected errors; `Exn<T>` algebraic effect for exceptions | Errors are values; `match` enforces handling every case; `Exn<T>` is handleable like any other effect | | |
| | Inference | `Inference.complete` as an algebraic effect | LLM calls are typed, contract-verifiable, and explicit in signatures; user-defined handlers are not yet supported | | |
| | Data types | Algebraic data types + exhaustive `match` | No classes, no inheritance; compiler enforces every case is handled | | |
| | Polymorphism | Monomorphized generics (`forall<T where Eq<T>>`) | No runtime dispatch; four built-in abilities (`Eq`, `Ord`, `Hash`, `Show`); types fully specialised at compile time | | |
| | Refinement types | `{ `@T` \| predicate }` checked by Z3 | Encode value-level constraints in the type system; rejected statically or at runtime | | |
| | Collections | `Array<T>`, `Map<K,V>`, `Set<T>` | Functional, immutable; no mutation, no loops; `array_map`/`filter`/`fold`/`slice` as built-ins | | |
| | Standard library | 164 built-in functions | Strings, arrays, maps, sets, decimals, math (log/trig/constants/utilities), JSON, HTML, Markdown, regex, base64, URL — no external deps | | |
| | Modules | `module`/`import` with explicit `public`/`private` visibility | Programs split across files; `vera check` resolves the module graph; re-exports are tracked in [`#127`](https://github.com/aallan/vera/issues/127) | |
🤖 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 `@DESIGN.md` around lines 25 - 33, The “Inference” row in DESIGN.md overstates
current support by claiming LLM calls are mockable via handle[Inference], but
user-defined Inference handlers are not yet supported. Update that row’s
description in the comparison table to remove the handle[Inference] claim and
rephrase it to describe only the currently supported typed, contract-verifiable,
explicit Inference.complete effect behavior.
…he helper's result _plumbing_ctors already required a genuine helper method (aallan#827's fix) whose sole own-scope Diagnostic() construction is unambiguous, but never confirmed that construction was actually the helper's output. A helper that builds its one ctor and hands it to something other than return/append — e.g. self.dispatch(d) instead of self.errors.append(d) — still had it elected as plumbing and skipped by all three passes, so a bogus spec_ref or unregistered error_code on it would ship silently. _ctor_is_reachable_as_result now requires the sole ctor be return-ed, appended, or bound to a local that is later return-ed/appended, gated after the existing single-ctor check so aallan#827's ambiguity handling is untouched. On vera/ the exempt set is unchanged: all five real helpers return or append their sole ctor directly, so the gap was latent, closed defensively. The name-based return/append match in the second half of the check has no real data-flow: a local rebound after the ctor-binding assign would still match on the name alone and be wrongly treated as reachable. Fixed conservatively via _rebound_names, which counts every name-binding form in own scope — plain and augmented assignment, for/with targets, walrus, and except-as names — not just plain Assign. A name rebound more than once by any of these forms is unreliable, so the ctor is inspected rather than exempted. Separately, _is_append_call treated any .append(...) call as evidence of reachability, including an append to an unrelated throwaway local the helper never reads again. Every real diagnostic-list sink in vera/ has the self.<attr>.append(...) shape, so the check is now scoped to that. Fixes aallan#956 Co-Authored-By: Claude <noreply@anthropic.invalid>
chethanuk
force-pushed
the
fix/issue-956-plumbing-skip-reachability
branch
from
July 9, 2026 05:25
f1267f4 to
078b1d8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
scripts/check_diagnostic_fields.py (1)
284-292: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
_bump_targetstill missesast.Starred, under-counting star-unpacking rebinds.
a, *b = [...](or the same shape in afor/withtarget) nests the rebound name in anast.Starrednode inside theTuple/List._bump_targetonly recurses intoast.Tuple/ast.List, so a starred-unpacked rebind of the ctor-bound local is never counted by_rebound_names. That keeps the rebind count at 1, so_ctor_is_reachable_as_resultwould wrongly treat the name as reachable — exactly the escape#956closes, via a different binding shape than the ones currently tested.This mirrors a prior review comment on this same function; flagging again since the fix isn't present in the code under review.
🐛 Proposed fix
def _bump_target(target: ast.expr, counts: dict[str, int]) -> None: """Record every plain name a (possibly-tuple/list) assignment target binds — recursing into nested unpacking so ``a, (b, c) = ...`` counts all three.""" if isinstance(target, ast.Name): counts[target.id] = counts.get(target.id, 0) + 1 elif isinstance(target, (ast.Tuple, ast.List)): for elt in target.elts: _bump_target(elt, counts) + elif isinstance(target, ast.Starred): + _bump_target(target.value, counts)🤖 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 `@scripts/check_diagnostic_fields.py` around lines 284 - 292, The `_bump_target` helper still under-counts rebinding when a target contains `ast.Starred`, so star-unpacking like `a, *b = ...` is missed and `_rebound_names` can report the ctor-bound local as still reachable. Update `_bump_target` to recurse into `ast.Starred` targets as well as `ast.Tuple` and `ast.List`, so nested unpacking in assignments, `for`, and `with` targets is counted correctly. Verify the fix through the `_rebound_names` and `_ctor_is_reachable_as_result` path so starred rebinds no longer slip through.
🤖 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 `@scripts/check_diagnostic_fields.py`:
- Around line 295-319: _rebound_names is missing some valid Python 3.11+
rebinding forms, so its count can underreport later reassignment. Update
_rebound_names to also recognize ast.AnnAssign and capture bindings from
match/case patterns while walking the function scope, and make sure the existing
helpers (_walk_own_scope and _bump_target) handle those targets consistently so
_ctor_is_reachable_as_result stays sound.
---
Duplicate comments:
In `@scripts/check_diagnostic_fields.py`:
- Around line 284-292: The `_bump_target` helper still under-counts rebinding
when a target contains `ast.Starred`, so star-unpacking like `a, *b = ...` is
missed and `_rebound_names` can report the ctor-bound local as still reachable.
Update `_bump_target` to recurse into `ast.Starred` targets as well as
`ast.Tuple` and `ast.List`, so nested unpacking in assignments, `for`, and
`with` targets is counted correctly. Verify the fix through the `_rebound_names`
and `_ctor_is_reachable_as_result` path so starred rebinds no longer slip
through.
🪄 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: 0ab7bbe9-40e0-4279-8a08-5ba1fb902439
📒 Files selected for processing (5)
CHANGELOG.mdROADMAP.mdTESTING.mdscripts/check_diagnostic_fields.pytests/test_diagnostic_fields.py
…(PR aallan#964 review) A value-carrying annotated assignment (d: object = self.template) rebinds the local without being an ast.Assign, so a bogus ctor followed by an AnnAssign rebind was still elected as plumbing and skipped by all three passes (probe-confirmed EXEMPT). Dually, d: Diagnostic = Diagnostic(...) was never recognised as the initial binding, permanently inspecting genuine plumbing. Both arms fixed: _rebound_names counts value-carrying AnnAssign targets (a bare annotation does not assign and is not counted), and the binding recognition accepts an AnnAssign whose value is the ctor. Six new fixtures: the two AnnAssign directions (RED-confirmed pre-fix, each arm mutation-killed), the bare-annotation distinction, and one pin each for the with/walrus/except rebind arms so a dropped arm ships RED (PR aallan#965 review ask, pinned here where the machinery lives). Exempt set on vera/ unchanged (the same five plumbing ctors). TESTING.md/ROADMAP counts follow (6,827 -> 6,833).
…s, match captures, params, nonlocal (PR aallan#964 panel review) The enumeration approach was fail-OPEN for every binding form it did not list: starred unpacks (a, *d = ...), import ... as d, match-case captures, a parameter shadowed by the ctor's name, a nonlocal declaration rebinding the helper's local from a nested function, and a return that precedes the binding (order-insensitive name match). Each shape kept a swapped-out bogus ctor exempt (six RED fixtures confirmed pre-fix). _rebound_names now counts bindings generically: any Store/Del-context Name in own scope (subsuming the old enumeration), the string-carrying forms (import-as, except-as, match captures), the helper's parameters, and nonlocal declarations across nested scopes; a bare annotation (no value) is still not a rebind. The return/append name-match gains an order guard (at-or-after the binding line). Params/nonlocal/order/ Store arms each mutation-killed; exempt set on vera/ unchanged. TESTING.md/ROADMAP counts follow (6,833 -> 6,839).
…es, cron failures file a tracking issue (aallan#960) Weekly (Mon 07:00 UTC + on-demand) run of check_limitations_sync.py --check-states (aallan#852). Hardened in review: UNKNOWN issue states (gh missing/auth/rate-limit/timeout) now fail the run instead of passing vacuously; scheduled failures file/update a labelled tracking issue mirroring nightly-stress.yml; Windows cp1252 pipe-decode fixed via ASCII-only message + explicit child-stdio UTF-8. CHANGELOG/ROADMAP/TESTING lockstep included. External contribution by @chethanuk (PR aallan#960); review hardening by the maintainer loop. Co-Authored-By: Claude <noreply@anthropic.invalid>
…an#962) One include cell (Python 3.12 — platform coverage) giving Linux aarch64 real end-to-end CI coverage (aallan#702). Review lockstep: README §Supported platforms promotes the tested cell (3.11/3.13 stay wheel-checked only), the wheel-availability gate gains manylinux_2_38_aarch64 rows for all three Pythons (validated live, 36/36), matrix prose 12→13 in four places, the stale Debian 12 glibc example dropped (bookworm ships 2.36 < the 2.38 floor), CHANGELOG + ROADMAP entries. Advisory by decision: runs on every commit, not yet a required merge check. External contribution by @chethanuk (PR aallan#962); review lockstep by the maintainer loop. Co-Authored-By: Claude <noreply@anthropic.invalid>
# Conflicts: # CHANGELOG.md # ROADMAP.md # TESTING.md
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
2 participants
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.
User description
Pre-flight validation ahead of aallan#956.
Summary
Test plan
CodeAnt-AI Description
Keep diagnostic-field checks from skipping helpers that do not return their diagnostic
What Changed
Impact
✅ Fewer silent diagnostic field escapes✅ Clearer missing spec_ref and error_code failures✅ Safer helper refactors💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit
Security
Bug Fixes
Tests
Documentation