Skip to content

fix(lint): re-record thread-local ratchet; rename test-local class id (follow-up to #11674) - #11706

Merged
proggeramlug merged 1 commit into
mainfrom
fix/11674-dead-class-keys-ensure
Sep 30, 2026
Merged

proggeramlug merged 1 commit into
mainfrom
fix/11674-dead-class-keys-ensure

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up to #11674. I rebuilt this PR on current main (43f1c80, after the shapes P4 series) and redid every count from scratch with the scripts. The P4 series had already fixed most of the original PR; only two reds remain on main:

windows-build GC structural audits (and tls-budget's self-test job): check_thread_locals.py fails, on Linux too. I re-recorded it with --update. Every count only goes down:

  • async_hooks.rs: 7→6
  • gc/layout_tables.rs: 3→2
  • node_stream_constructors.rs: 3→2
  • the stale gc/layout.rs entry is deleted
  • hot declarations: 528→522

lint / class-id audit: #11674's test-local ANON_CLASS_ID = 0x0075_5eed in static_shapes_tests.rs reads as a drifted mirror of put_value.rs's unrelated test-local ANON_CLASS_ID = 0x8783_1001. I renamed the new one to REP_SEED_ANON_CLASS_ID.

No longer needed, so dropped: the dead shape_id_for_class_keys_ensure (already gone on main), the gc/layout.rs file-size split (under the cap now), and the shape-descriptor census baseline edit (passes on main).

Validation (Linux, perrymaster)

  • RUSTFLAGS="-D warnings" cargo check -p perry-runtime --lib --tests: clean, on both pristine main and this branch.
  • cargo test -p perry-codegen --lib region_loop_tests: 6/6 pass, on both pristine main and this branch. The earlier CI failure of a_bare_store_of_a_value_not_proven_a_double_names_its_key_to_the_prime came from the old base and is not reproducible at 43f1c80.
  • check_thread_locals.py, class_id_collisions.py, check_file_size.sh, shape_descriptor_census.py, cargo fmt --check: all pass.

Not run

  • The full run_lint_gates.sh on the rebuilt branch. It passed on the previous version: 105 of 107, the 2 known failures being public-baseline and cargo xwin.
  • No Windows build.
  • No full cargo test.

proggeramlug pushed a commit that referenced this pull request Sep 30, 2026
@coderabbitai

coderabbitai Bot commented Sep 30, 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: c5c9ab19-df7c-477f-9c75-24acdbac8fed

📥 Commits

Reviewing files that changed from the base of the PR and between 62cef20 and a7f5bb0.

📒 Files selected for processing (2)
  • changelog.d/11706-main-reds-after-11674.md
  • scripts/thread_local_cold_allowlist.json
🚧 Files skipped from review as they are similar to previous changes (2)
  • changelog.d/11706-main-reds-after-11674.md
  • scripts/thread_local_cold_allowlist.json

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


📝 Walkthrough

Walkthrough

The static-shape test now uses a renamed class ID constant. The thread-local cold allowlist counts and entries were updated, and a changelog entry records these changes and the reported audit results.

Changes

Ratchet audit updates

Layer / File(s) Summary
Static-shape test identifier
crates/perry-runtime/src/object/static_shapes_tests.rs
The test-local constant and its uses were renamed to REP_SEED_ANON_CLASS_ID. The shape-mint arguments and expected result are unchanged.
Allowlist counts and changelog
scripts/thread_local_cold_allowlist.json, changelog.d/11706-main-reds-after-11674.md
The allowlist total and listed counts decreased, and the gc/layout.rs entry was removed. The changelog records the count updates, reported audit failures, and test identifier rename.

Priority: ⬇️ Low

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

Change: Other

Merge Risk: ⚪ Minimal · up to a7f5b

This change repairs audit and lint failures on main by updating ratchet counts and adding a changelog entry. It does not appear to alter production behavior. Merge risk is minimal, and the author reports that the relevant Linux checks pass.

🚥 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 3 files. (2 skipped: 2… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Title check ✅ Passed The title clearly identifies both primary changes: re-recording the thread-local ratchet and renaming the test-local class ID. It also identifies the follow-up context.
Description check ✅ Passed The description provides a clear summary, concrete change list, related issue reference, validation results, and unrun checks. It does not use the template section headings or checklist, but the requi…
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 3 files. (2 skipped: 2 unsupported.)

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

… (follow-up to #11674)

check_thread_locals.py: async_hooks.rs 7->6, gc/layout_tables.rs 3->2,
node_stream_constructors.rs 3->2, stale gc/layout.rs entry removed. All
counts only go down. Fails windows-build's GC structural audits and
tls-budget's self-test job.

class_id_collisions.py: #11674's test-local ANON_CLASS_ID (0x0075_5eed)
in static_shapes_tests.rs read as a drifted mirror of put_value.rs's
unrelated test-local ANON_CLASS_ID; renamed to REP_SEED_ANON_CLASS_ID.
@proggeramlug
proggeramlug force-pushed the fix/11674-dead-class-keys-ensure branch from 62cef20 to a7f5bb0 Compare September 30, 2026 12:02
@proggeramlug proggeramlug changed the title fix: repair main's warnings/windows-build/lint reds (follow-up to #11674) fix(lint): re-record thread-local ratchet; rename test-local class id (follow-up to #11674) Sep 30, 2026
@proggeramlug
proggeramlug merged commit 5fbc2c3 into main Sep 30, 2026
34 of 36 checks passed
@proggeramlug
proggeramlug deleted the fix/11674-dead-class-keys-ensure branch September 30, 2026 12:04
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