Skip to content

test(codegen): cover NBC order and numeric field intersection - #10964

Closed
proggeramlug wants to merge 2 commits into
PerryTS:mainfrom
proggeramlug:fix/10954-nbc-order-tests
Closed

proggeramlug wants to merge 2 commits into
PerryTS:mainfrom
proggeramlug:fix/10954-nbc-order-tests

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #10954.

Pass the PERRY_L14_NBC_ORDER gate result into the shape-input helper so unit tests can exercise both modes without mutating process-wide environment variables. The new test uses two proven receivers and checks that only fields numeric on both are admitted to the number-by-construction fixpoint. It also rejects an unproven receiver and verifies the disabled mode contributes no shape inputs.

Verification: cargo test -p perry-codegen nbc_order_intersects_numeric_fields_before_proving_property_locals --lib -- --nocapture (1 passed), cargo fmt --all -- --check, and git diff --check.

Summary by CodeRabbit

  • Tests
    • Expanded coverage for numeric-field intersections involving two receivers.
    • Added verification for both enabled and disabled numeric ordering paths.
    • Confirmed numeric-field intersections are evaluated before property-local validation.
    • Improved test isolation so parallel checks can reliably exercise each configuration independently.

@coderabbitai

coderabbitai Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 219881cf-951f-445a-a1c7-ed65993254a9

📥 Commits

Reviewing files that changed from the base of the PR and between a022cf2 and 2a7d713.

📒 Files selected for processing (3)
  • changelog.d/10964-nbc-order-test-coverage.md
  • crates/perry-codegen/src/collectors/hir_facts.rs
  • crates/perry-codegen/src/collectors/number_by_construction.rs

Included review availability: Your plan provides up to 8 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

The change passes the NBC order gate into numeric input shaping and adds tests for enabled and disabled modes. The tests verify numeric-field intersection across proven receivers, exclusion of receiver-specific fields, and retention of dynamic fields for unproven receivers.

Changes

NBC shape input collection

Layer / File(s) Summary
Pass the NBC order gate
crates/perry-codegen/src/collectors/number_by_construction.rs, crates/perry-codegen/src/collectors/hir_facts.rs
shape_numeric_inputs now accepts an enabled argument. The HIR facts collector passes the configured NBC order state.
Validate shape input intersection
crates/perry-codegen/src/collectors/number_by_construction.rs, changelog.d/10964-nbc-order-test-coverage.md
Tests verify shared numeric fields, exclusion of fields present on only one proven receiver, dynamic handling for unproven receivers, and the disabled path. The changelog records the coverage.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to 2a7d7

The change makes NBC shape-input gating explicit and adds coverage for enabled intersection behavior and disabled no-op behavior. It is mergeable with normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the test coverage added for NBC order and numeric-field intersection, which matches the main changes.
Description check ✅ Passed The description explains the purpose, implementation, linked issue, test behavior, and verification commands. It does not use the template headings or checklist, but it includes the critical informati…
Linked Issues check ✅ Passed The changes satisfy the coding requirements in #10954. The new shape_input_tests module covers the enabled PERRY_L14_NBC_ORDER path with two proven receivers. The test admits fields numeric on bot…
Out of Scope Changes check ✅ Passed The changes remain within #10954. The gate-parameter plumbing enables isolated tests for both modes. The added changelog entry documents the requested coverage. No unrelated production behavior, publi…
Full details: Docstring Coverage

Explanation

Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🧪 Generate unit tests (beta)
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR

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.

❤️ Share

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

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Landed on main in merge train 256 (#11018, v0.5.1638), main f5cfbff882.

Carried at head 2a7d7134bb. The landed tree is byte-identical to the validated train tree (9b108dd3c9), and CI on the train head passed every job except the known public-baseline lint step: all 6 gap shards, cargo-test, e2e-scoped, gc-stress, check, warnings and security-audit green.

Trains rebase-merge, so commits get new SHAs and GitHub cannot mark this PR merged. Closed as landed.

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.

PERRY_L14_NBC_ORDER's ON path is exercised by no test — including the intersection rule

1 participant