docs(runtime): narrow the diagnostics feature comment to what it actually gates

[proggeramlug]

crates/perry-runtime/Cargo.toml's comment above diagnostics = [] says "None are on a hot path." That is true of the four things it enumerates — PERRY_GC_DIAG, the typed-feedback trace dump, the v8 heap-snapshot builder, process.report — and false as a blanket statement, because the feature does not gate hot_diag at all:

• lib.rs declares pub mod hot_diag; with no #[cfg].
• hot_diag.rs contains zero cfg(feature occurrences.
• enum_on() is called per string concat (string/concat.rs:688 and :1052) and per property enumeration, in every build including what ships.

So stripping the diagnostics feature does not remove that call, and a reader who takes the comment at face value concludes it does.

This is worth narrowing because comments in this tree are load-bearing in people's reasoning. A peer session quoted a neighbouring hot_diag comment ("an unarmed build pays one relaxed load") as evidence about generated code and proposed a fix on that basis; reading the actual gate bodies showed both gates carry the same OnceLock probe and the difference is a short-circuit at the call site. Same failure shape as the tautological debug_assert the GC-header gate replaced, and as the denial pointing at closed #7109 that #10804 fixes: a reference that reads as evidence and isn't.

No performance claim attached — deliberately

The obvious follow-up was implemented, measured, and reverted: collapse enum_on()'s OnceLock-probe-plus-AtomicBool-load into one three-state AtomicU8 with a #[cold] #[inline(never)] resolve arm.

Measured against its exact parent commit (the branch was rebased onto the base arm's commit first, so the comparison isolates one variable rather than mixing in five commits of unrelated churn):

arm	instr per short concat	bare-loop control
base	399.76	3.01
three-state gate	401.19	2.99

+1.43 on ~400 — indistinguishable from zero. The control is valid in both arms, so the measurement is sound and the hypothesis is simply wrong.

Why it was wrong is the useful part. A peer measured a real win — zod −5.2%, an S40 fixture −12.4% — from deleting an early-returning diagnostic gate. That came from removing the call and its inlining barrier entirely, not from making the gate's body cheaper. Saving one load out of four hundred instructions is below what any probe here resolves. Recorded in the changelog so the next person doesn't re-derive it.

Summary by CodeRabbit

• Documentation
  • Clarified the scope of diagnostic serializers and related feature settings.
  • Documented that certain diagnostic checks remain active in all builds.
  • Added performance context for diagnostic checks and previously evaluated optimizations.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 9c059989-42f3-40fd-89bc-41869e9b128c

📥 Commits

Reviewing files that changed from the base of the PR and between c1d9f73 and d9c20bf.

📒 Files selected for processing (2)

• changelog.d/10820-diag-feature-scope-comment.md
• crates/perry-runtime/Cargo.toml

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

---

📝 Walkthrough

Walkthrough

The PR corrects the diagnostics feature comment to distinguish gated serializers from unconditional hot_diag behavior. It adds a changelog entry that records this scope correction and related performance measurements.

Changes

Diagnostics feature scope

Layer / File(s)	Summary
Document diagnostics feature scope crates/perry-runtime/Cargo.toml, changelog.d/10820-diag-feature-scope-comment.md	The feature comment identifies the gated serializers and states that hot_diag is unconditional. The changelog records the same correction and the measured optimization result.

Priority: ⬇️ Low

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

Change: Other

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (3 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description provides a detailed summary, rationale, concrete technical changes, and benchmark results. However, it does not follow the required template structure and omits the required Related is…	Rewrite the description using the repository template. Add the Summary, Changes, Related issue, Test plan, Screenshots / output, and Checklist sections. State the validation commands that were run and mark the applicable checklist items.
Linked Issues check	⚠️ Warning	Issue #7109 requires the module-init and program-entry canonical representation-selection exclusion to appear as a counted module_init_context denial in optimization reports. The whole-PR diff chang…	Add the module_init_context denial to the relevant representation-selection report path, and add automated coverage that verifies the denial appears for module-init or program-entry locals without changing selection behavior.
Out of Scope Changes check	⚠️ Warning	The changed files concern runtime diagnostic-feature documentation and a changelog entry for issue #10820. They do not support issue #7109's module-init representation-selection reporting objective. T…	Remove these diagnostic-documentation changes from this pull request or link them to the appropriate diagnostics issue and submit the issue #7109 implementation separately.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: narrowing the diagnostics feature comment to the behavior it actually controls.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…

Full details: Description check

Explanation

The description provides a detailed summary, rationale, concrete technical changes, and benchmark results. However, it does not follow the required template structure and omits the required Related issue, standard Test plan checklist, Screenshots / output section, and Checklist.

Full details: Linked Issues check

Explanation

Issue #7109 requires the module-init and program-entry canonical representation-selection exclusion to appear as a counted module_init_context denial in optimization reports. The whole-PR diff changes only the diagnostics comment in crates/perry-runtime/Cargo.toml and adds changelog.d/10820-diag-feature-scope-comment.md. It does not change representation-selection logic, optimization-report generation, or tests. The required denial is therefore not implemented or verified.

Full details: Out of Scope Changes check

Explanation

The changed files concern runtime diagnostic-feature documentation and a changelog entry for issue #10820. They do not support issue #7109's module-init representation-selection reporting objective. The diff also documents a reverted AtomicU8 performance experiment, which is unrelated to representation-selection reporting.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

🧪 Generate unit tests (beta)

• Commit to this branch
• 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[proggeramlug]

Landed via merge train 242 (#10830) as v0.5.1621 — e2a0839074.

Eight PRs travelled together because their file sets are disjoint — 30 files, +1,514/−101, zero overlap. Validated as one tree: ten cheap gates, -D warnings across all targets, five pinned artifacts byte-identical before and after, seven unit suites with an empty failing set, both compiler-output suites at failed_workloads=[], repsel_census rc=0, and a 250-fixture sweep with one area per PR (class 84, string 50, object 40, map 21, stream 18, bind 14, url 12, regex 11) — zero unexplained regressions.

Two of the eight needed a fix before they could land, both made in the train rather than bounced back.

#10816 bound sep_jv unconditionally in string/split.rs while reading it only inside #[cfg(feature = "regex-engine")], so RUSTFLAGS="-D warnings" cargo check -p perry --bins failed. Worth knowing why this is invisible in normal review: a one-invocation whole-workspace build unifies cargo features, so the regex engine is always on and the binding always read — only the per-package command, one of six run_lint_gates.sh derives, sees it. Same family as cargo check --lib not compiling cfg(test) code. Gated behind the feature that reads it; lim_jv on the next line was checked separately and is genuinely used outside the block.

#10817 added 15 dispatch entries without regenerating the docs, so the API-docs-drift check failed. Regenerated from a built binary: 2855 → 2870, exactly your 15, with perry.d.ts correctly unchanged at 2026 since those rows are dispatch-table rather than public surface. it_manifest_consistency passes on the assembled tree, which is the stronger signal — a green drift check only proves the files match the binary; that suite proves the manifest is internally consistent.

For future PRs in this area: scripts/regen_api_docs.sh hardcodes <worktree>/target/release/perry and, with that binary absent, regenerates from nothing and leaves both files truncated. A real regeneration moves the header counts and leaves the tail intact — worth checking the tail, not just the count.

One more thing, aimed at whoever cuts the next PR here: verify() flagged an exponential-backoff manifest entry in #10817 as missing from the train. That was correct — train 240 removed the binding, and restoring the entry would have failed manifest sync. main is moving several times an hour at the moment, so a PR cut against a base more than a few hours old is worth rebasing before review rather than after.