Skip to content

test(codegen): isolate the default feedback lane - #9999

Closed
proggeramlug wants to merge 2 commits into
mainfrom
fix/typed-feedback-test-isolation-20260908
Closed

proggeramlug wants to merge 2 commits into
mainfrom
fix/typed-feedback-test-isolation-20260908

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Test-only fix

The closure-specialization test asserts that pure feedback recording calls are absent, but did not acquire the suite's environment lock or disable the profiling settings. Other tests in this same parallel suite temporarily enable those settings, causing scoped CI on unrelated codegen PRs to fail at typed_feedback.rs:1171.

Join the existing poison-tolerant lock and scoped environment guards. No production compiler/runtime behavior changes. Based directly on main; independent of the compatibility changes.

Validation

  • Deterministic before: PERRY_TYPED_FEEDBACK=1 makes the existing exact test fail at the same assertion as CI.
  • After: the exact test passes with both PERRY_TYPED_FEEDBACK=1 and PERRY_TYPED_FEEDBACK_TRACE=1 inherited.
  • The full 20-test suite passes with 8 test threads after the fix.
  • Synthetic HIR only; no application artifacts required.

Baseline CI issues, independently confirmed in main run 34232917409: stale public benchmark evidence and native_stack::tests::stack_top_respects_custom_thread_stack_sizes. This PR does not alter or suppress either gate.

Summary by CodeRabbit

  • Bug Fixes

    • Improved test reliability by isolating typed-feedback behavior from inherited profiling settings.
    • Prevented concurrent profiling configuration from affecting closure-specialization verification.
  • Documentation

    • Added a changelog entry documenting the fix.

@coderabbitai

coderabbitai Bot commented Sep 8, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 1d4eec58-d518-4945-aa89-4bce2a25dca3

📥 Commits

Reviewing files that changed from the base of the PR and between 8d88e48 and 14863a9.

📒 Files selected for processing (2)
  • changelog.d/9999-typed-feedback-test-isolation.md
  • crates/perry-codegen/tests/typed_feedback.rs

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


📝 Walkthrough

Walkthrough

The closure-specialization test now isolates profiling environment variables before asserting the default typed feedback behavior. A changelog entry documents the fix.

Changes

Typed feedback test isolation

Layer / File(s) Summary
Isolate the default typed feedback lane
crates/perry-codegen/tests/typed_feedback.rs, changelog.d/9999-typed-feedback-test-isolation.md
The test acquires ENV_LOCK and unsets PERRY_TYPED_FEEDBACK and PERRY_TYPED_FEEDBACK_TRACE. The changelog records the correction.

Priority: ⚪ Not assessed

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 14863

The closure-specialization test now reliably exercises the default feedback behavior even when profiling environment settings are inherited or changed by parallel tests. No production behavior changes, and the change is ready to merge.

Suggested reviewers: jdalton

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the test-only change: isolating the default feedback lane in code generation tests.
Description check ✅ Passed The description clearly explains the test failure, the environment-lock and guard fix, validation results, and scope. It does not use the template headings or include the requested checklist, related-…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ 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/typed-feedback-test-isolation-20260908

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 via #10001 (rebase train; author commits retained). Audited head: 14863a94bfc3eabaf4035fba183ee6d6b5d0772d. Post-merge main 6b92bb2ef5ffe277e5f2773bea1a2e244a58347e is byte-identical to the validated tree. Full audit, validation, and conflict/test-harness repair details are recorded in the train PR.

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.

1 participant