Skip to content

fix: pipe operator into module-qualified calls (#326)#410

Merged
aallan merged 3 commits into
mainfrom
fix/pipe-module-call-e201
Mar 28, 2026
Merged

fix: pipe operator into module-qualified calls (#326)#410
aallan merged 3 commits into
mainfrom
fix/pipe-module-call-e201

Conversation

@aallan

@aallan aallan commented Mar 28, 2026

Copy link
Copy Markdown
Owner

Summary

  • _check_pipe in vera/checker/expressions.py only handled FnCall on the RHS. A ModuleCall fell through to the _synth_expr fallback, which saw a zero-argument call and emitted a spurious E201 (arity mismatch)
  • Fix synthesises a new ast.ModuleCall with the LHS prepended as first arg and delegates to _check_module_call — the same pattern already used in wasm/operators.py and smt.py (which both handled this correctly)
  • Searched the codebase for workarounds (Module::fn(@T.0) where a pipe would have been natural) — none found beyond the intentional qualified-call demo in examples/modules.vera

Changes

  • vera/checker/expressions.py — add ModuleCall branch to _check_pipe
  • tests/test_checker.py — two regression tests in TestModuleCallParsed (single pipe, chained pipes)
  • tests/conformance/ch04_pipe_module_call.vera — new run-level conformance test using vera.math::abs()
  • tests/conformance/vera/math.veravera.math stub for the conformance suite resolver
  • KNOWN_ISSUES.md — remove Pipe operator with module-qualified calls produces spurious E201 #326

Test plan

  • vera check accepts @Int.0 |> mod::fn() without E201
  • Chained pipes x |> mod::f() |> mod::g() type-check correctly
  • Existing arity-error tests still fire for genuine wrong-arity calls
  • All 65 conformance programs pass
  • Full test suite green

Closes #326

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Pipe operator now correctly accepts module-qualified calls (resolves the spurious error).
  • Improvements

    • Added end-to-end tests covering module-qualified calls used as pipe targets.
    • Conformance test suite expanded to 65 programs; testing counts and related documentation updated.

@coderabbitai

coderabbitai Bot commented Mar 28, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c2c87b2d-e4c0-43b6-b089-bee3cb0d673f

📥 Commits

Reviewing files that changed from the base of the PR and between f43cf89 and 6e9d4dc.

📒 Files selected for processing (2)
  • TESTING.md
  • tests/test_checker.py

📝 Walkthrough

Walkthrough

Fixes pipe desugaring in the type checker to accept module-qualified calls as pipe targets, adds unit and conformance tests for that case, updates the conformance manifest, and adjusts documentation and metrics (conformance suite count and known-issues entry).

Changes

Cohort / File(s) Summary
Docs & test metrics
CLAUDE.md, TESTING.md, KNOWN_ISSUES.md
Updated conformance program count from 6465, adjusted test totals and test_checker.py counts, and removed the documented bug entry for issue #326.
Conformance manifest
tests/conformance/manifest.json
Added new manifest entry ch04_pipe_module_call (file ch04_pipe_module_call.vera) for chapter 4 section 4.5 at run level; features include pipe_operator and module_calls.
Unit tests
tests/test_checker.py
Added three parsed regression tests in TestModuleCallParsed covering module-qualified RHS of the pipe operator (single, chained, and argument-order regression). Also extended in-test MATH_MODULE with public fn tag(@int, @string->@string).
Type checker
vera/checker/expressions.py
Extended _check_pipe to accept ast.ModuleCall on RHS by desugaring `left

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Caller as LHS Expression
participant Pipe as _check_pipe
participant ModCall as ast.ModuleCall (constructed)
participant ModuleChk as _check_module_call
Caller->>Pipe: provide LHS value
Pipe->>ModCall: construct ModuleCall with args=(LHS, ...original args)
Pipe->>ModuleChk: delegate ModuleCall for typechecking
ModuleChk-->>Pipe: return result type
Pipe-->>Caller: return final type

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

compiler, tests, docs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely summarizes the main fix: enabling the pipe operator to work with module-qualified calls, directly addressing issue #326.
Linked Issues check ✅ Passed All coding requirements from #326 are met: _check_pipe now handles ModuleCall by prepending LHS as first argument, regression tests validate single and chained pipes, spurious E201 is eliminated, and conformance coverage is added.
Out of Scope Changes check ✅ Passed All changes directly address the pipe-operator desugaring bug: checker logic, regression tests, conformance test, and documentation updates reflecting the fix and conformance suite expansion are all in scope.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/pipe-module-call-e201

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented Mar 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.30%. Comparing base (6ad5006) to head (6e9d4dc).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #410   +/-   ##
=======================================
  Coverage   90.30%   90.30%           
=======================================
  Files          49       49           
  Lines       19104    19107    +3     
  Branches      220      220           
=======================================
+ Hits        17252    17255    +3     
  Misses       1848     1848           
  Partials        4        4           
Flag Coverage Δ
javascript 50.58% <ø> (ø)
python 95.31% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

#326)

_check_pipe in checker/expressions.py only handled FnCall on the RHS;
ModuleCall was passed to the fallback _synth_expr which saw an
arg-less call and emitted E201. The fix synthesises a new ast.ModuleCall
with the LHS prepended and delegates to _check_module_call — mirroring
the pattern already used in wasm/operators.py and smt.py.

Also adds:
- Two regression tests in TestModuleCallParsed (single + chained pipe)
- Conformance test ch04_pipe_module_call.vera (level: run, vera.math::abs)
- Removes #326 from KNOWN_ISSUES.md

Co-Authored-By: Claude <noreply@anthropic.invalid>
@aallan aallan force-pushed the fix/pipe-module-call-e201 branch from 6a202e2 to ae85a53 Compare March 28, 2026 18:43

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/test_checker.py`:
- Around line 3308-3335: Add a test that locks the argument order for piping
into module-qualified calls that already take arguments: create a new test (e.g.
test_pipe_module_call_arg_order_regression) that uses the same mod =
self._resolved(("math",), self.MATH_MODULE) setup and constructs a source where
a value is piped into a module call that already has an asymmetric argument (for
example: `{ `@Int.0` |> math::max(123) }`) then run parse_to_ast and typecheck
with resolved_modules=[mod] and assert no error diagnostics; this ensures the
pipe inserts the receiver as the first argument (math::max(value, 123)) rather
than appending and will catch future prepend/append regressions.
🪄 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: 6edb91f3-1568-4660-879c-a62bd0f17179

📥 Commits

Reviewing files that changed from the base of the PR and between 6ad5006 and 6a202e2.

⛔ Files ignored due to path filters (2)
  • tests/conformance/ch04_pipe_module_call.vera is excluded by !**/*.vera
  • tests/conformance/vera/math.vera is excluded by !**/*.vera
📒 Files selected for processing (6)
  • CLAUDE.md
  • KNOWN_ISSUES.md
  • TESTING.md
  • tests/conformance/manifest.json
  • tests/test_checker.py
  • vera/checker/expressions.py
💤 Files with no reviewable changes (1)
  • KNOWN_ISSUES.md

Comment thread tests/test_checker.py

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

83-120: ⚠️ Potential issue | 🟡 Minor

Documentation counts are now internally inconsistent.

The updated 65-program values are correct in this section, but the file still references older totals at Line 154 (315 tests) and Lines 319 and 361 (64 conformance programs). Please update those lines to keep TESTING.md self-consistent.

🛠️ Suggested doc fix
-# Via pytest (parametrized — 315 tests)
+# Via pytest (parametrized — 325 tests)

-| `check_conformance.py` | All 64 conformance programs pass their declared level (parse/check/verify/run) |
+| `check_conformance.py` | All 65 conformance programs pass their declared level (parse/check/verify/run) |

-| `check_conformance.py` | All 64 conformance programs pass their declared level |
+| `check_conformance.py` | All 65 conformance programs pass their declared level |

As per coding guidelines, "Review Markdown files for factual accuracy against the codebase, broken links, and outdated information."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@TESTING.md` around lines 83 - 120, Update TESTING.md so all conformance
counts match the conformance suite's actual size (65 programs). Replace the
outdated numeric references ("315 tests" and every "64 conformance programs")
with the correct values consistent with the manifest and the section that lists
65 programs; double-check the Directory structure and any "Count" column entries
in the "Test levels" table and the textual summaries to use 65 where appropriate
(refer to tests/conformance/ and manifest.json to verify). Ensure no other
scattered references to 64 or 315 remain so the document is internally
consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@TESTING.md`:
- Around line 83-120: Update TESTING.md so all conformance counts match the
conformance suite's actual size (65 programs). Replace the outdated numeric
references ("315 tests" and every "64 conformance programs") with the correct
values consistent with the manifest and the section that lists 65 programs;
double-check the Directory structure and any "Count" column entries in the "Test
levels" table and the textual summaries to use 65 where appropriate (refer to
tests/conformance/ and manifest.json to verify). Ensure no other scattered
references to 64 or 315 remain so the document is internally consistent.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7d3a128f-c26a-4b02-b41e-0acf0ff9b0c0

📥 Commits

Reviewing files that changed from the base of the PR and between 6ad5006 and ae85a53.

⛔ Files ignored due to path filters (2)
  • tests/conformance/ch04_pipe_module_call.vera is excluded by !**/*.vera
  • tests/conformance/vera/math.vera is excluded by !**/*.vera
📒 Files selected for processing (6)
  • CLAUDE.md
  • KNOWN_ISSUES.md
  • TESTING.md
  • tests/conformance/manifest.json
  • tests/test_checker.py
  • vera/checker/expressions.py
💤 Files with no reviewable changes (1)
  • KNOWN_ISSUES.md

- Regenerate docs/ site assets (conformance count 64→65)
- Fix remaining TESTING.md stale references: 315→325 parametrized tests,
  two more 64→65 conformance counts in script table and pre-commit table
- Add test_pipe_module_call_arg_order_regression to lock down that the
  pipe LHS is prepended as first arg (not appended)
- Update test_checker.py count 496→497, line count 5495→5519
- Update total test count 3103→3104

Co-Authored-By: Claude <noreply@anthropic.invalid>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/test_checker.py`:
- Around line 3322-3334: In test_pipe_chained_module_calls_typechecks, the
source string imports an unused symbol `max`; update the import to only bring in
`abs` by changing the import clause in the test's source from "import math(abs,
max);" to "import math(abs);" so the test only imports what it uses (reference:
the test function name test_pipe_chained_module_calls_typechecks and the source
variable containing the import).
- Around line 3336-3358: The test test_pipe_module_call_arg_order_regression
currently uses math::max which is symmetric (max(`@Int`,`@Int`)) so it cannot catch
prepend-vs-append regressions; change the test to call an asymmetric function
(e.g. math::tag with signature tag(`@Int`,`@String` -> `@String`)) so the LHS being
appended would produce a type error—update the test source to import math(tag)
and use the pipe expression `@Int.0` |> math::tag("ok") (or add tag to MATH_MODULE
and then use that symbol) so the argument ordering is actually validated by the
typechecker.
🪄 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: f3a16427-78a7-48a2-b575-d3dae49fe01d

📥 Commits

Reviewing files that changed from the base of the PR and between ae85a53 and f43cf89.

⛔ Files ignored due to path filters (1)
  • docs/llms.txt is excluded by !docs/**
📒 Files selected for processing (2)
  • TESTING.md
  • tests/test_checker.py

Comment thread tests/test_checker.py
Comment thread tests/test_checker.py
- Add tag(@int, @string -> @string) to MATH_MODULE so prepend-vs-append
  is type-observable: if LHS were appended, checker would see String where
  Int is expected and emit a type error
- Replace symmetric math::max(123) with math::tag("ok") in the arg-order
  regression test
- Fix unused 'max' import in test_pipe_chained_module_calls_typechecks
- Update test_checker.py line count in TESTING.md (5519→5522)

Co-Authored-By: Claude <noreply@anthropic.invalid>
@aallan

aallan commented Mar 28, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai The TESTING.md counts were already fixed in commit f43cf89 (pushed before this review round) — all 6465 references including the 315325 parametrized test count and the two script table entries. python scripts/check_doc_counts.py passes clean.

@coderabbitai

coderabbitai Bot commented Mar 28, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@aallan: Understood — thanks for the heads-up. Confirmed: commit f43cf89 covers all the 6465 transitions (including the 315325 parametrised count and both script table rows), and scripts/check_doc_counts.py passing clean is the authoritative check here. I'll treat those count observations as resolved.


🧠 Learnings used
Learnt from: aallan
Repo: aallan/vera PR: 0
File: :0-0
Timestamp: 2026-03-25T16:24:36.297Z
Learning: In repo aallan/vera, the script `scripts/check_doc_counts.py` automatically validates that documented example and conformance program counts in TESTING.md, CLAUDE.md, and related docs match the actual file counts (`ls examples/*.vera | wc -l`, etc.). Do not flag count mismatches as bugs without first verifying against this script.

Learnt from: CR
Repo: aallan/vera PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2026-03-28T10:59:21.300Z
Learning: Update relevant documentation and tests when making code changes

Learnt from: aallan
Repo: aallan/vera PR: 325
File: tests/test_verifier_coverage.py:595-612
Timestamp: 2026-03-20T14:08:38.587Z
Learning: In the Vera compiler (`vera/`), pipe-operator desugaring (`|> mod::fn()`) has a known arity mismatch issue when the callee is a module-qualified call. Use the direct call form (`mod::fn(arg)`) instead. This applies to both production Vera source files and test programs in `tests/test_verifier_coverage.py` and similar test files.

Learnt from: aallan
Repo: aallan/vera PR: 344
File: tests/conformance/manifest.json:815-831
Timestamp: 2026-03-25T10:03:24.704Z
Learning: In `vera/wasm/inference.py`, `_infer_fncall_wasm_type` must explicitly handle Map and Set builtin functions (e.g. `map_new`, `set_new`, and related ops) to return `"i32"` so that WASM `if (result i32)` blocks containing Map/Set function calls are correctly annotated. Missing these entries was the root cause of Map/Set if-expression result-type failures (fixed in PR `#344`, commit 741f19e).

@aallan aallan merged commit 0a137a2 into main Mar 28, 2026
17 checks passed
@aallan aallan deleted the fix/pipe-module-call-e201 branch March 28, 2026 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pipe operator with module-qualified calls produces spurious E201

1 participant