fix(pipeline): widen lower_where sig to accept any predicate expression by cuzzo · Pull Request #40 · cuzzo/clear

cuzzo · 2026-05-09T16:33:54Z

Summary

PipelineHost#lower_where had a Sorbet sig that constrained expr_node to AST::BinaryOp, but CLEAR's WHERE clause accepts ANY boolean expression. The narrow sig caused a runtime Sorbet TypeError for any non-BinaryOp predicate.

Found while VM-stress-testing the register backend: transpile-tests/80_concurrent_where.cht uses WHERE isPositive(_.value) (a FuncCall predicate) and tripped the sig.

Why master appeared green

The Zig backend's pipeline pipeline routes CONCURRENT operators through the legacy string-based transpile_pipeline path, which doesn't reach lower_where for these expressions. The bc emitter's bc-specific concurrent dispatch (lower_bc_concurrent_where) calls lower_where directly with the user-written predicate — that's where the sig fires. So the bug was latent on master and only surfaced via the register VM.

Fix

One line: match visit_pipeline_expr_mir's T.untyped (which the method's body delegates to). A narrower union (T.any(AST::BinaryOp, AST::FuncCall, AST::Literal)) would still reject other valid shapes (Identifier, UnaryOp, MethodCall, ...). Sibling methods lower_select and lower_take_while have no sig at all — this brings lower_where into line.

Regression test

spec/pipeline_host_lower_where_sig_spec.rb constructs a lower_where call with each of 6 valid predicate AST shapes (BinaryOp / FuncCall / Literal / Identifier / UnaryOp / MethodCall) and asserts no Parameter 'expr_node' TypeError is raised.

Without this fix, 5 of the 6 cases fail. Verified by stashing the fix and re-running the spec.

Test plan

bundle exec rspec spec/pipeline_host_lower_where_sig_spec.rb — 7 examples, 0 failures
bundle exec prspec spec/ — 4691 examples, 0 failures
./clear test transpile-tests/ — 540 passed, 0 leaks
transpile-tests/80_concurrent_where.cht passes on the register VM (will be visible on register-machine PR rebase)

🤖 Generated with Claude Code

PipelineHost#lower_where had `expr_node: AST::BinaryOp` in its Sorbet sig. But CLEAR's `WHERE` clause accepts ANY boolean expression: comparisons (`x > 0`), function calls (`isPositive(x)`), literals (`TRUE`), identifiers (`flag`), unary ops (`!cond`), method calls (`x.isPositive()`), etc. The narrow sig caused a runtime Sorbet TypeError on every non-BinaryOp predicate that flowed through this method: Parameter 'expr_node': Expected type AST::BinaryOp, got type AST::FuncCall ... The Zig backend's `lower_pipeline` didn't typically reach `lower_where` for these expressions (its CONCURRENT path falls back to the legacy string-based `transpile_pipeline`), which is why master tests stayed green -- the bug only surfaced via the register-VM emitter's bc-specific concurrent dispatch (register_bc_emitter.rb routing CONCURRENT WHERE through PipelineHost#lower_where with the user-written predicate). Fix: match `visit_pipeline_expr_mir`'s `T.untyped` (the method `lower_where`'s body delegates to). A union like `T.any(AST::BinaryOp, AST::FuncCall)` would still reject other valid predicate shapes; matching the sibling's precedent is the correct width. Sister methods `lower_select` and `lower_take_while` have no sig at all -- they take any expression too. This brings `lower_where` into line. New regression spec: spec/pipeline_host_lower_where_sig_spec.rb constructs a lower_where call with each of the 6 valid predicate AST shapes and asserts no `Parameter 'expr_node'` TypeError is raised. Without this fix, 5 of the 6 cases fail. Found by VM stress-test: 80_concurrent_where (CONCURRENT WHERE isPositive(_.value)) on the bc target.

codecov-commenter · 2026-05-09T16:36:24Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.08%. Comparing base (08b4c61) to head (e3c1f69).
⚠️ Report is 4 commits behind head on master.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #40      +/-   ##
==========================================
- Coverage   90.09%   90.08%   -0.02%     
==========================================
  Files         185      185              
  Lines       49984    49987       +3     
  Branches    11984    11986       +2     
==========================================
- Hits        45035    45031       -4     
- Misses       4949     4956       +7

Flag	Coverage Δ
ruby	`86.76% <100.00%> (+<0.01%)`	⬆️
zig	`95.89% <ø> (-0.05%)`	⬇️

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.

github-actions · 2026-05-09T16:36:53Z

Bencher Report

Branch	hotfix-pipeline-where
Testbed	ubuntu-latest

⚠️ WARNING: No Threshold found!
Without a Threshold, no Alerts will ever be generated.
leak-run-ms (Measure (units))
leak-build-ms (Measure (units))
leak-count (Measure (units))
Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the --ci-only-thresholds flag.

Click to view all benchmark results

Benchmark	leak-build-ms	Measure (units) x 1e3	leak-count	leak-run-ms	Measure (units)
benchmarks/concurrent/05_backpressure/bench	📈 view plot ⚠️ NO THRESHOLD	5.45 units x 1e3	📈 view plot ⚠️ NO THRESHOLD	📈 view plot ⚠️ NO THRESHOLD	1,648.35 units
benchmarks/concurrent/10_shard_vs_locked/bench	📈 view plot ⚠️ NO THRESHOLD	5.23 units x 1e3	📈 view plot ⚠️ NO THRESHOLD	📈 view plot ⚠️ NO THRESHOLD	60,004.42 units
benchmarks/concurrent/16_observables/bench	📈 view plot ⚠️ NO THRESHOLD	5.37 units x 1e3	📈 view plot ⚠️ NO THRESHOLD	📈 view plot ⚠️ NO THRESHOLD	96.93 units
benchmarks/inter-clear/03_concurrent_mvcc_vs_rwlock/bench	📈 view plot ⚠️ NO THRESHOLD	2.77 units x 1e3	📈 view plot ⚠️ NO THRESHOLD	📈 view plot ⚠️ NO THRESHOLD	371.86 units
benchmarks/sequential/07_pointer_chase/bench	📈 view plot ⚠️ NO THRESHOLD	1.79 units x 1e3	📈 view plot ⚠️ NO THRESHOLD	📈 view plot ⚠️ NO THRESHOLD	553.17 units
benchmarks/sequential/12_weak_ref_graph/bench	📈 view plot ⚠️ NO THRESHOLD	1.82 units x 1e3	📈 view plot ⚠️ NO THRESHOLD	📈 view plot ⚠️ NO THRESHOLD	259.83 units
benchmarks/server/03_pathological/server	📈 view plot ⚠️ NO THRESHOLD	1.94 units x 1e3	📈 view plot ⚠️ NO THRESHOLD	📈 view plot ⚠️ NO THRESHOLD	1,002.75 units

🐰 View full continuous benchmarking report in Bencher

@list

… drift Add dynamic-array (#39) and nested Int64[][]@list (#40) shapes to the move-modality x owning-shape matrix; both fail their GIVE baseline (proven: `[]i64` vs `array_list.Aligned`; `*T` vs `*const T`) so all their give/bare/copy cells are :in_dev gated by #39/#40. list/set/pool/map unchanged (give :pass, bare/copy :in_dev gated by #37). 18 cells, in_dev=14, 4 give run 0 fail/leak. Coverage-integrity drift (#41) -- why the class went undetected: surface_registry access_gate baseline claims escape_sinks takes_arg/give_arg, true only for a struct (Counter); the taxonomy not crossing sink x shape made the reporter mark the whole takes/give surface covered, masking the collection-shape gap. Register takes_move_modality in TEMPLATE_COVERAGE as the truthful owner of takes_arg/give_arg across collection shapes; delete the dead `when :takes_arg,:give_arg` branch in ownership_surface_smoke (no cell ever had that sink -- unreachable; verified the template's cell count/result is unchanged by the removal). Document the new template in README so coverage.rb stays green. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@list

mir_emitter's :list_with_elem_cleanup template emitted { for (xs.items) |*__e| { CheatLib.cleanup(ElemT, rt.cleanupAlloc(), __e); } xs.deinit(rt.frameAlloc()); } The trailing `xs.deinit(...)` requires *self: *T (mutable). When the binding is const-bound -- as it is when a nested Int64[][]@list arrives through a TAKES param via `xs: anytype` (by-value -> const) -- Zig rejects: `expected '*T', found '*const T'`. Proven (#40, give_nested fuzz cell). Surgical fix: wrap the outer deinit in @constcast(&xs).deinit(...). Comptime no-op for mutable bindings (Zig erases the cast); fixes the const case. The dual-allocator semantics of this kind (cleanupAlloc for elements, frameAlloc for container) are NOT compatible with the unified CheatLib.cleanup shim (single alloc), so the inline template stays and only the const issue is addressed. The fuller cleanup-emission unification proposed in the plan is deferred -- the shim would need a second cleanup_alloc parameter to subsume this kind, a larger change beyond the bug-fix scope. Verified: srb green, 4819/0 specs, 564/564 0 leaks, byte-identical diff = 74 (all :list_with_elem_cleanup callsites; the added @constcast is a comptime no-op for mutable bindings). give_nested fuzz cell (currently :in_dev gated by #40) compiles + runs. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@list

…lowering) cf150a2 dropped the CopyNode/MoveNode exclusions at the slice path on the theory that they were redundant. They are not: for GIVE of a MUTABLE Int64[] caller (frame-allocated ArrayList) into a TAKES Int64[] callee (slice param with heapAlloc free), extracting `.items` yields the correct slice TYPE but the callee then frees with heapAlloc what the caller frame-allocated -- runtime "Invalid free" (observed in fuzz bundle 10/14, fuzz_takes_move_modality_321f95466a). The OLD exclusion blocked this path; restore it. #39 (dynamic Int64[] -> TAKES) cannot be safely fixed at the mir_lowering layer alone -- it requires either (a) escape analysis promoting `MUTABLE xs: Int64[]` to heap when xs escapes via TAKES, or (b) a calling-convention change so TAKES Int64[] callees receive the owning ArrayList not a slice. Both are larger; #39 stays open. Also filed #42 for COPY of @set/@pool/@map/dyn-array into TAKES (dupeValue lacks comptime arms for those shapes; the COPY emits a slice-shaped value the callee can't use as an owning container). Final fuzz state (12/18 :pass, 6 :in_dev): - :pass: bare/COPY/GIVE of @list, GIVE of @set/@pool/@Map, bare of @set/@pool/@Map, all nested @list cases - :in_dev: COPY of @set/@pool/@Map (gated by #42) - :in_dev: dynarr give/bare/copy (gated by #39) #37 (bare + COPY for @list) and #40 (nested @list cleanup) remain FIXED. Standalone cells all pass; the fuzz bundle has a residual interleaving issue in 1 of cells 11/12 that does not reproduce standalone or in transpile-tests (564/564 0 leaks, prspec 4819/0, srb green). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…#41) A sink-only TEMPLATE_COVERAGE claim (e.g. access_gate declaring escape_sinks: [:takes_arg, ...] when the only cell uses a struct Counter) made coverage.rb mark the entire takes/give surface covered, masking the collection-shape gap that hid #37/#39/#40/#42. SINK_REQUIRES_SHAPES declares per-sink required shape sets. For each cross-cut sink (takes_arg, give_arg, return_value, struct_field_store, list_append), coverage.rb unions cleanup_value_shapes across every template whose escape_sinks includes that sink, and reports any required shape missing. templates_covering_sink helper enumerates them. Schema additions: - takes_move_modality now declares cleanup_value_shapes alongside collection_shapes (heap_list / set / pool / hash_map / dynamic_array -- the shapes it actually exercises). - access_gate declares cleanup_value_shapes: [:struct_owned_fields] (the Counter cell shape). The check surfaces #43: no template covers union_owned_payload for takes_arg/give_arg (real gap -- a union-with-owned-payload TAKES is the next #37-class blind spot, now visible). The other sinks (return_value/struct_field_store/list_append) report clean because ownership_surface_smoke's full cleanup_value_shapes list combined with its escape_sinks claim already satisfies them. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@list

… drift Add dynamic-array (#39) and nested Int64[][]@list (#40) shapes to the move-modality x owning-shape matrix; both fail their GIVE baseline (proven: `[]i64` vs `array_list.Aligned`; `*T` vs `*const T`) so all their give/bare/copy cells are :in_dev gated by #39/#40. list/set/pool/map unchanged (give :pass, bare/copy :in_dev gated by #37). 18 cells, in_dev=14, 4 give run 0 fail/leak. Coverage-integrity drift (#41) -- why the class went undetected: surface_registry access_gate baseline claims escape_sinks takes_arg/give_arg, true only for a struct (Counter); the taxonomy not crossing sink x shape made the reporter mark the whole takes/give surface covered, masking the collection-shape gap. Register takes_move_modality in TEMPLATE_COVERAGE as the truthful owner of takes_arg/give_arg across collection shapes; delete the dead `when :takes_arg,:give_arg` branch in ownership_surface_smoke (no cell ever had that sink -- unreachable; verified the template's cell count/result is unchanged by the removal). Document the new template in README so coverage.rb stays green. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@list

mir_emitter's :list_with_elem_cleanup template emitted { for (xs.items) |*__e| { CheatLib.cleanup(ElemT, rt.cleanupAlloc(), __e); } xs.deinit(rt.frameAlloc()); } The trailing `xs.deinit(...)` requires *self: *T (mutable). When the binding is const-bound -- as it is when a nested Int64[][]@list arrives through a TAKES param via `xs: anytype` (by-value -> const) -- Zig rejects: `expected '*T', found '*const T'`. Proven (#40, give_nested fuzz cell). Surgical fix: wrap the outer deinit in @constcast(&xs).deinit(...). Comptime no-op for mutable bindings (Zig erases the cast); fixes the const case. The dual-allocator semantics of this kind (cleanupAlloc for elements, frameAlloc for container) are NOT compatible with the unified CheatLib.cleanup shim (single alloc), so the inline template stays and only the const issue is addressed. The fuller cleanup-emission unification proposed in the plan is deferred -- the shim would need a second cleanup_alloc parameter to subsume this kind, a larger change beyond the bug-fix scope. Verified: srb green, 4819/0 specs, 564/564 0 leaks, byte-identical diff = 74 (all :list_with_elem_cleanup callsites; the added @constcast is a comptime no-op for mutable bindings). give_nested fuzz cell (currently :in_dev gated by #40) compiles + runs. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

@list

…lowering) cf150a2 dropped the CopyNode/MoveNode exclusions at the slice path on the theory that they were redundant. They are not: for GIVE of a MUTABLE Int64[] caller (frame-allocated ArrayList) into a TAKES Int64[] callee (slice param with heapAlloc free), extracting `.items` yields the correct slice TYPE but the callee then frees with heapAlloc what the caller frame-allocated -- runtime "Invalid free" (observed in fuzz bundle 10/14, fuzz_takes_move_modality_321f95466a). The OLD exclusion blocked this path; restore it. #39 (dynamic Int64[] -> TAKES) cannot be safely fixed at the mir_lowering layer alone -- it requires either (a) escape analysis promoting `MUTABLE xs: Int64[]` to heap when xs escapes via TAKES, or (b) a calling-convention change so TAKES Int64[] callees receive the owning ArrayList not a slice. Both are larger; #39 stays open. Also filed #42 for COPY of @set/@pool/@map/dyn-array into TAKES (dupeValue lacks comptime arms for those shapes; the COPY emits a slice-shaped value the callee can't use as an owning container). Final fuzz state (12/18 :pass, 6 :in_dev): - :pass: bare/COPY/GIVE of @list, GIVE of @set/@pool/@Map, bare of @set/@pool/@Map, all nested @list cases - :in_dev: COPY of @set/@pool/@Map (gated by #42) - :in_dev: dynarr give/bare/copy (gated by #39) #37 (bare + COPY for @list) and #40 (nested @list cleanup) remain FIXED. Standalone cells all pass; the fuzz bundle has a residual interleaving issue in 1 of cells 11/12 that does not reproduce standalone or in transpile-tests (564/564 0 leaks, prspec 4819/0, srb green). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…#41) A sink-only TEMPLATE_COVERAGE claim (e.g. access_gate declaring escape_sinks: [:takes_arg, ...] when the only cell uses a struct Counter) made coverage.rb mark the entire takes/give surface covered, masking the collection-shape gap that hid #37/#39/#40/#42. SINK_REQUIRES_SHAPES declares per-sink required shape sets. For each cross-cut sink (takes_arg, give_arg, return_value, struct_field_store, list_append), coverage.rb unions cleanup_value_shapes across every template whose escape_sinks includes that sink, and reports any required shape missing. templates_covering_sink helper enumerates them. Schema additions: - takes_move_modality now declares cleanup_value_shapes alongside collection_shapes (heap_list / set / pool / hash_map / dynamic_array -- the shapes it actually exercises). - access_gate declares cleanup_value_shapes: [:struct_owned_fields] (the Counter cell shape). The check surfaces #43: no template covers union_owned_payload for takes_arg/give_arg (real gap -- a union-with-owned-payload TAKES is the next #37-class blind spot, now visible). The other sinks (return_value/struct_field_store/list_append) report clean because ownership_surface_smoke's full cleanup_value_shapes list combined with its escape_sinks claim already satisfies them. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(pipeline): widen lower_where sig to accept any predicate expression#40

fix(pipeline): widen lower_where sig to accept any predicate expression#40
cuzzo wants to merge 1 commit into
masterfrom
hotfix-pipeline-where

cuzzo commented May 9, 2026

Uh oh!

codecov-commenter commented May 9, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 9, 2026 •

edited

Loading

⚠️ WARNING: No Threshold found!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

cuzzo commented May 9, 2026

Summary

Why master appeared green

Fix

Regression test

Test plan

Uh oh!

codecov-commenter commented May 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented May 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bencher Report

⚠️ WARNING: No Threshold found!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov-commenter commented May 9, 2026 •

edited

Loading

github-actions Bot commented May 9, 2026 •

edited

Loading