[raliclo]

Summary

cargo-test fails on every PR at program_facts::element_tests::product_element_identity_preserves_codegen_facts. The test (c0d6cb8) asserts that the elision facts are identical with and without element source identity. #14354 (59925ca) added number_element_reads and #14537 added primitive_elements; both are keyed by element_offset, where 0 means "no identity", so they can exist only when identity does. git bisect from c0d6cb8 (good) names 59925ca as the first bad commit.

This PR keeps the test's intent and makes it precise: identity may add those two facts and nothing else.

Changes

• crates/perry/src/commands/compile/program_facts/element_tests.rs: assert number_element_reads and primitive_elements are empty without identity, then compare every other elision fact after clearing those two. sites, solver stats and digest_for are still compared whole (they were already equal).

Related issue

Introduced by #14354 / 59925ca (bisect), extended by #14537.

If the intended invariant is the stricter one — identity must not add any elision — then the fix belongs in those two facts instead, and this PR is the wrong one; happy to close it in that case.

Test plan

Base 6dfb4af3aa (current main).

	main	this branch
cargo test --release -p perry --bin perry product_element_identity_preserves_codegen_facts	FAIL at the elision assert_eq! (only number_element_reads: {75: false} and primitive_elements: {75} differ)	ok
cargo test --release -p perry --bin perry program_facts	not run as a whole (only the one test above)	18 passed
same test with a probe adding one entry to another elision field (literal_string_locals)	—	FAIL (the narrowed assertion still catches other drift)

Bisect: git bisect run with that single test between c0d6cb8 (good) and 91a26fd~1 (bad), 9 steps, no skips.

Built on macOS (Apple M6), nightly from rust-toolchain.toml, LLVM 23.1.5.

Test-only change; a changelog.d fragment is included because the path is under crates/ and I cannot set the skip-changelog label — drop it if you label the PR instead.

Checklist

• I have NOT bumped the workspace version or edited CHANGELOG.md
• My commits follow the loose feat: / fix: / docs: / chore: prefix convention used in the log
• I've read CONTRIBUTING.md and agree to the Code of Conduct

🤖 Generated with Claude Code

https://claude.ai/code/session_01CUQanVsnitcrmYLAWWPc3p

Summary by CodeRabbit

• Tests
  • Refined compilation test coverage for element identity, allowing the associated facts to vary while verifying that other compilation results and the overall digest remain consistent. This also checks that those facts are absent when element identity is not enabled.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The element identity test now permits two identity-specific elision entries while checking that other elision facts, solver outputs, and the facts digest remain unchanged. A changelog entry records the test behavior and its reported failure on main since #14354.

Changes: Element identity elision test

Layer / File(s)	Summary
Compare elision facts crates/perry/src/commands/compile/program_facts/element_tests.rs, changelog.d/14598-element-identity-test.md	The test checks that number_element_reads and primitive_elements are empty without identity. It removes these entries from the identity-enabled result before comparing other elision facts. The remaining fact, solver output, and digest checks remain. The changelog describes the test and its reported failure since #14354.

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

Change: Other

Possibly related PRs

• PerryTS/perry#14382: It changes how for-of reads receive element_offset values and affects element-read facts that this test compares.

Merge Risk: Minimal · up to 64d21

This change only relaxes a test assertion to allow two element-identity-specific facts and adds a changelog note, with no production behavior change. The failing CI check concerns an unrelated runtime file outside this PR's diff and should be resolved separately from this change.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title names the main change: the element-identity test now lets identity add only the facts keyed by element_offset. The test fix matches the raw summary and objectives. It is specific and accurat…
Description check		The description covers the summary, the changes, the related issue (#14354 and #14537 bisect), and a detailed test plan. It omits the template's required checkbox items for cargo build and the workspa…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title names the main change: the element-identity test now lets identity add only the facts keyed by element_offset. The test fix matches the raw summary and objectives. It is specific and accurate, though long at 99 characters.

Full details: Description check

Explanation

The description covers the summary, the changes, the related issue (#14354 and #14537 bisect), and a detailed test plan. It omits the template's required checkbox items for cargo build and the workspace test command, and it does not mention the docs or platform checkboxes. These gaps are non-critical, so the description is mostly complete.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 47faf483-6a0c-4957-a698-4b1a7af28ef9

Commits

Reviewing files that changed from the base of the PR and between c85ac1e and 64d21b7.

Files selected for processing: 1

• crates/perry/src/commands/compile/program_facts/element_tests.rs

Files skipped from review as they are similar to previous changes: 1

• crates/perry/src/commands/compile/program_facts/element_tests.rs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 4 remain after this review.

[raliclo]

Superseded by 398e5b4 on main, which takes the same approach (facts keyed by element_offset may appear only with identity; everything else equal). Verified: cargo test --release -p perry --bin perry product_element_identity_preserves_codegen_facts passes on c9b18ad22a.