fix(runtime): validate preinstalled shape key counts

[proggeramlug]

Summary

Fix the release-blocking ShapeId/ObjectHeader publication disagreement reached by fast-json-stringify. A preinstalled class ShapeId is now birth-stamped only when its descriptor agrees with every current structural fact from the separately supplied keys global; otherwise the existing exact-descriptor fallback runs.

Changes

• Validate the current logical key count alongside the keys pointer, live inline-slot bound, semantic generation, and object kind before publishing a preinstalled shape.
• Add a focused regression test that models a stale ShapeId paired with a keys array whose logical count has diverged.
• Keep the existing descriptor/ObjectHeader parity assertion unchanged.

Related issue

Fixes #9265

Test plan

Run on root@perrymaster.skelpo.net:

• cargo fmt --all -- --check
• git diff --check
• cargo test -p perry-runtime preinstalled_shape -- --nocapture
• cargo build -p perry -p perry-runtime-static -p perry-stdlib-static
• RUST_TEST_THREADS=1 cargo test -p perry --test issue_6559_real_libs_e2e real_fast_json_stringify_serializer -- --nocapture

Additional repository check:

• ./scripts/check_file_size.sh
• Full release build and workspace suite (not run)

Screenshots / output

The exact issue reproducer passes: 1 passed, 0 failed.

Checklist

• I have NOT bumped the workspace version or edited CLAUDE.md / CHANGELOG.md
• My commits follow the loose prefix convention used in the log
• I have read CONTRIBUTING.md and agree to the Code of Conduct

Summary by CodeRabbit

• Bug Fixes
  • Improved object allocation reliability when preinstalled shape metadata no longer matches the current object keys.
  • Added validation to detect key-count inconsistencies and safely use an exact shape description instead.
  • Prevented stale metadata from being applied during object creation, helping maintain correct object behavior and performance.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 87936d6c-d575-4fcd-a425-d6e9334da9cb

📥 Commits

Reviewing files that changed from the base of the PR and between 6c880be and 78dcfb7.

📒 Files selected for processing (4)

• changelog.d/9283-preinstalled-shape-facts.md
• crates/perry-runtime/src/object/alloc.rs
• crates/perry-runtime/src/object/shapes.rs
• crates/perry-runtime/src/object/shapes_tests.rs

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

---

📝 Walkthrough

Walkthrough

The runtime now validates the logical key count of a preinstalled shape before birth-stamping. A mismatch triggers exact descriptor fallback. A regression test covers stale module keys.

Changes

Shape key-count validation

Layer / File(s)	Summary
Descriptor key-count validation crates/perry-runtime/src/object/shapes.rs, crates/perry-runtime/src/object/alloc.rs, changelog.d/9283-preinstalled-shape-facts.md	try_birth_stamp_preinstalled_shape derives the current logical key count and rejects mismatched descriptors. Comments and the changelog describe the validation.
Exact fallback regression test crates/perry-runtime/src/object/shapes_tests.rs	The test mutates a keys array after minting a ShapeId and verifies that allocation publishes an exact descriptor.

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

Merge Risk: ⚪ Minimal · up to 78dcf

This localized runtime fix rejects stale shape metadata before publication and falls back to an exact descriptor, preserving object-header consistency. No actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes satisfy issue [#9265] by validating the logical key count before publishing a preinstalled ShapeId, preserving the existing parity assertion, adding a regression test, and reporting a pass…
Out of Scope Changes check	✅ Passed	The changes are limited to the requested runtime validation, regression test, explanatory comment, and changelog entry. No unrelated code changes are identified.
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 4 functions across 3 files. (1 skipped: 1 …
Title check	✅ Passed	The title clearly and concisely identifies the main change: validating preinstalled shape key counts in the runtime.
Description check	✅ Passed	The description includes all required sections, explains the ShapeId validation change, references issue #9265, documents targeted tests and builds, and clearly states that the full release build and …

Full details: Linked Issues check

Explanation

The changes satisfy issue [#9265] by validating the logical key count before publishing a preinstalled ShapeId, preserving the existing parity assertion, adding a regression test, and reporting a passing fast-json-stringify test with rebuilt runtime archives.

Full details: Docstring Coverage

Explanation

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 4 functions across 3 files. (1 skipped: 1 unsupported.)

Full details: Description check

Explanation

The description includes all required sections, explains the ShapeId validation change, references issue #9265, documents targeted tests and builds, and clearly states that the full release build and workspace suite were not run.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

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

[proggeramlug]

Audited the diff — this addresses the root cause and I'd merge it. One question at the end that I would not block on.

The fix lands exactly where #9265's assertion pointed. shapes.rs's debug_assert! requires both facts:

d.keys == keys as u64 && d.logical_key_count == key_count

…but the direct-stamp fast path only proved the keys pointer and the live-slot bound. It justified skipping the count with this reasoning, which the diff removes:

Canonical class keys never mutate in place: structural growth forks a new keys array and mints a new ShapeId, so exact (ShapeId, keys pointer) identity also carries the descriptor's logical key count.

The replacement is the important part: the id and the keys pointer arrive through separate module globals, so that identity does not in fact carry the count, and the two can drift. Computing logical_key_count and comparing it before the stamp store makes the fast path prove everything the downstream assert requires, rather than inferring one fact from another. Declining to an exact mint-and-validate descriptor on divergence is the conservative direction.

The regression test is genuine rather than a restatement: it mints an id for a length-1 keys array, mutates (*keys).length = 0 to model the drift, and asserts the newborn does not receive the stale id. That fails without the check.

Question, not a blocker

This makes the allocator tolerant of divergence rather than preventing it. If the two globals drifting apart is a legitimate consequence of independent publication, that is exactly right. If drift instead indicates something upstream corrupting or reusing a keys array, this fix would make the symptom disappear while leaving that cause in place — and the assertion that caught it is a debug_assert!, compiled out of release builds, so there would be no second chance to notice.

The comment says this is the state "fast-json-stringify reached through AJV's resolve module" — if that path is understood well enough to say the drift is expected there, I am satisfied. Worth a sentence in the PR either way, since the next person reading this will ask the same thing.

[proggeramlug]

Merged.

Validating the current logical key count alongside the keys pointer before birth-stamping a preinstalled ShapeId is the right shape: the failure mode is a ShapeId that claims a structure the separately-supplied keys global no longer has, and the only safe answer when those two disagree is to fall back to the exact-descriptor path rather than publish a shape nobody can trust.

I probed the disagreement directly rather than the happy path — 200 class instances plus 150 object literals, then delete and property-add to force key-count divergence from the preinstalled descriptor, then JSON.stringify over the diverged shapes (the route fast-json-stringify takes), then all of it again after a 40k-allocation GC churn. Byte-identical to node 26.5.1 throughout: {"y":2,"z":3} after a delete-and-add, Object.keys reporting a,b / a,b,c / b across three literals that started identical.

Validation: perry-runtime 2886 passed / 0 failed at RUST_TEST_THREADS=1; perry-codegen 31 suites / 0 failures; all 60 lint gates plus the TLS checkers. Six earlier regression probes (tagged and scalar array stores, pointer↔scalar churn, growth-forwarding receivers, BigInt negation, iterator protocols) re-run at zero diff lines. Validated on a shared branch with #9284, #9286 and #9288.