feat(cli): --opt-report — surface which values could not be statically typed, why, and whether the developer can fix it by proggeramlug · Pull Request #7037 · PerryTS/perry

proggeramlug · 2026-07-30T03:50:13Z

Closes #6952. Unblocks #7034 P0.

Why now

Perry's speed comes from proving static types and selecting unboxed representations. When a proof fails the value stays NaN-boxed and the fast paths silently do not fire — and nobody, including us, could see which values failed or why without reading LLVM IR.

Yesterday that stopped being theoretical. #7034 measured the flagship representation on the workload that motivates it:


`Ptr<Shape>` promoted locals on the object/property-heavy program	0
`PERRY_PTR_SHAPE_LOCALS=0` vs default on that file	byte-identical output

A shipped representation was doing essentially nothing on realistic code, and it was found by accident. Establishing that one fact took six compiler invocations, an IR dump, a symbol-level size A/B, and a PERRY_REPSEL_DEBUG grep whose output format had to be reverse-engineered.

It is also the third instance of this blindness in a week: one agent discovered its gap test was promoting zero locals only by hand-instrumenting the compiler; another spent hours discovering that adding type annotations made a benchmark slower. Both would have been one line of --opt-report output.

The acceptance test, in one command

benchmarks/app-patterns/kernels/batch.ts is added in this PR — a faithful reconstruction of the program #7034 measured (the original was an ad-hoc file in that session's worktree and was never committed). Every record in it escapes, which is the point: that is what real TypeScript does.

$ perry compile batch.ts -o batch --opt-report

Representation summary
----------------------
  Ptr<Shape>          0 selected /    4 denied   (0% of 4 candidates)
  I32/U32/Str         3 selected /    0 denied   (100% of 3 candidates)
  specialized ABI     0 selected /    3 denied   (0% of 3 candidates)
  TOTAL            3 values unboxed, 7 left Boxed

*** Ptr<Shape> promoted 0 of 4 candidates in this build. ***
    Every candidate was denied; the rules are in collectors/ptr_shape.rs.

Denied values, hottest first
----------------------------
Hotness is a static proxy, not a profile. `per-element callback` means the
enclosing region is an iterating builtin's callback: it has no loop of its
own but runs once per element, so loop depth alone would rank it last.

Perry limitation (not your code) (7 value(s))
  batch.ts :: alloc-site `object literal { ... }` [per-element callback of Array.prototype.map]
      in closure closure#7 -> Boxed
      ptr-shape rule 1 (provenance)
      allocated in expression position and never bound to a `let`/`const`, so
      the shape proof has no local to anchor to. This is the
      `.map(x => ({...}))` / `return { ... }` idiom.
      allocation position: return
      tracking: #7034 §4 (return-shape facts)
  batch.ts :: local `row` [loop depth 1]
      in function buildRows -> Boxed
      ptr-shape rule 2 (containment)
      stored into an array or object (element/property of a container).
      Container slots do not carry a shape fact yet.
      candidate class: Row
      tracking: #7034 §3/§5 P4-P5 (elements and fields)
  batch.ts :: local `acc` [loop depth 0]
      in function totalsRow -> Boxed
      ptr-shape rule 2 (containment)
      returned from this function. Return positions do not carry a shape fact
      yet, so returning a record forfeits its proof.
      candidate class: Row
      tracking: #7034 §4 (return-shape facts)
  ...

Unboxed values (3)
-----------------
  batch.ts :: local `i` -> I32 (in function buildRows)
  batch.ts :: local `b` -> I32 (in function summarize)
  batch.ts :: local `i` -> I32 (in function totalsRow)

What it covers

Mostly surfacing, not new analysis — the collectors already knew which rule rejected each candidate, they just continued.

Ptr<Shape> (collectors/ptr_shape.rs): all five rules plus class admission and the env gate. The escape kind is discriminated rather than lumped into one bucket, because reassignment, closure capture, call argument, return, and container element are five different fixes. Also reports allocation sites that never became candidates — the .map(x => ({...})) idiom that rule 1 structurally cannot see, which on real code is the majority.
Canonical i32/u32/Str locals (expr/slot_rep.rs): wins. Folded into the same call site as PERRY_REPSEL_DEBUG so the two mechanisms cannot diverge.
Specialized ABI (codegen/typed_abi.rs): the only place params and returns get a representation today. Extends the existing 37-variant TypedCloneRejectionReason vocabulary with an opt_report_reason() rendering rather than paralleling it.

Every denial carries a position, the rule in the collector's own numbering, a human expansion, an actionability tier, and a tracking issue when the fix is ours. Wins are reported too — a report that only nags is less trusted than one showing the ratio.

Hotness: two columns, not one

#7034 §8 measured that of 247 PIC/guard blocks in the motivating program, only 39 sit in an explicit loop and 208 are in map/sort/reduce callback bodies that have no loop of their own but run once per element. A loop-nesting proxy ranks those at zero and is wrong.

So loop depth and per-element-callback status are reported as separate columns, and the report says in-line that both are static proxies, not a profile. The ranking places per-element callbacks above plain loop depth; there is a unit test asserting exactly that, because it is the easy thing to get backwards.

Proof it is observational

This needed a correction mid-review, and the correction is the more interesting result.

The obvious check — compile once with the flag off, once with it on, diff the .ll — passed, and was not evidence. Perry's codegen is nondeterministic run-to-run: artifacts.rs iterates hir.closure_source_text, a HashMap, when emitting retained closure-source constants, so the @.str.N numbering is a per-process permutation. Two runs can match by luck (they did, at first) or differ for reasons unrelated to the change (they did, later). Filed separately as #7038; it is pre-existing and untouched by this PR.

The sound version — four runs per arm, plus a normalizer that canonicalises @.str.N by content:

=== RAW md5 (the pre-existing nondeterminism, in BOTH arms) ===
  OFF    1 c8f51e1f7ca5ba71e6cd785827e0e976
  OFF    1 d5f7462479bf33726126117e038f8126
  OFF    1 d67dc9790fa27305ab027a73285e67df
  OFF    1 f34c01e832bac8092790c95e90945c96
  ON     1 c8f51e1f7ca5ba71e6cd785827e0e976
  ON     1 d5f7462479bf33726126117e038f8126
  ON     1 d67dc9790fa27305ab027a73285e67df
  ON     1 f34c01e832bac8092790c95e90945c96

=== NORMALIZED (string-constant numbering canonicalised) ===
   8 1170b66968be9b8c79d2756a1ce46942

Both arms produce the same set of raw hashes — the flag adds no variation — and after canonicalising the single varying construct all eight runs are identical. The only differing lines in any pair are @.str.N definitions and the matching js_register_function_source calls; nothing else in the module moves.

The flag is therefore not in the object-cache key. It does disable build/object cache reuse for its own run — a cache hit skips compile_module entirely and the report would come up empty — exactly as --trace llvm and --explain-lowering already do.

Off by default: the gate is a OnceLock read of PERRY_OPT_REPORT, checked before any reason string is formatted, at every recording entry point. There is a test asserting the sink stays empty when it is off.

Compiled output is also still correct: batch.ts built with the flag on runs byte-for-byte identical to node --experimental-strip-types batch.ts.

Proof it does not lie

For two cases, the report's claim was cross-checked against --trace llvm. A shape-proven local lowers to bare fixed-offset field access with no guard diamond; a denied one keeps the guarded path.

function	report says	`js_typed_feedback_class_field_*_guard` calls in IR
`contained`	`p` → Ptr<Shape> selected	0
`escaping`	`q` → Boxed, rule 2 (return)	2
`totalsRow` (batch.ts)	`acc` → Boxed, rule 2 (return)	2

Both directions, on the same build.

Tests

15 new tests, run by the per-PR cargo-test gate (not an integration suite that only runs nightly).

The five end-to-end collector tests run the real collector over hand-built HIR and assert both halves of the contract: that the report names the right value with the right rule, and that the returned fact map is unchanged. That second assertion is what stops a future edit from "fixing" a report line by changing the proof.

Teeth were verified by mutation, not assumed — deleting the return-position context tracking turns contained_local_wins_and_returned_local_is_denied_with_its_rule red while the other 14 stay green. One test also found a real renderer bug during development (the wins section was skipped when there were no denials).

Two fixture traps were caught and are commented in place: ModuleDispatchFacts::default() is deliberately fail-safe with every barrier on, so using it would have made every test in the module vacuously assert the rule-5 kill; and the report gate is process-global, so tests serialise on a lock rather than racing through the env var.

Scope — what it cannot yet see

Stated plainly so nobody assumes coverage that is not there:

No file:line. HIR keeps binding names through lowering but drops source positions, so values are reported by function + name. The one exception is Expr::New, which already carries byte_offset (fix(compile): render lowering-error span (file:line:col + snippet) like check (#5249) #5253), so unbound allocation sites — which have no name at all — get one. The LocalId -> Span side-table is filed as opt-report v2: LocalId → Span side-table so --opt-report can print file:line and a source snippet #7036.
No field-level reasons. Ptr<Shape>'s numeric-field proof runs per field but is not reported per field; only the local's outcome is.
No cross-module reasons. Attribution is per module. A denial caused by a callee in another module is reported at the use site with the local rule, not traced across the module boundary.
Ptr<NumArray> is not instrumented. Same continue-site treatment would apply; it was left out to keep this reviewable.
Anonymous closures are named closure#<FuncId>, matching the emitted perry_closure_<module>__<id> symbol.
The per-element-callback attribution is syntactic: arr.map(x => …) is marked, const f = …; arr.map(f) is not. Absence of the mark is not evidence a body is cold, and the report does not claim otherwise.

Notes

crates/perry-codegen/src/collectors/ptr_shape.rs grew past the 2000-line gate, so its --opt-report tests were split into a sibling file; the same was done for the typed_abi.rs addition. This PR adds zero new file-size violations — the seven the gate still reports are all at their exact origin/main sizes and pre-date this branch.
chain_admissible now delegates to the function that also names the failing disqualifier, so the gate and the explanation cannot drift apart. Same set of conditions, same boolean.
Two issues were filed from this work: opt-report v2: LocalId → Span side-table so --opt-report can print file:line and a source snippet #7036 (the LocalId -> Span side-table for file:line) and codegen: same input compiles to different LLVM IR run-to-run — HashMap iteration order in closure-source registration breaks reproducible builds #7038 (the pre-existing codegen nondeterminism found while trying to prove observational purity).
Local Node is 26.3.0 against the repo's pinned 26.5.0, so the oracle diff above was run on 26.3.0. This PR adds no gap tests and touches no runtime semantics, but that is a caveat on that one check.

Summary by CodeRabbit

New Features
- Added the perry compile --opt-report flag to explain why values remain boxed instead of using optimized representations.
- Supports human-readable output and stable JSON output for CI comparisons.
- Reports successful optimizations, denied opportunities, actionable reasons, source locations, and hotness indicators.
- Reports are emitted to stderr and do not alter generated code.
Documentation
- Added CLI guidance for --opt-report and the PERRY_OPT_REPORT environment variable.

coderabbitai · 2026-07-30T03:50:40Z

📝 Walkthrough

Walkthrough

Changes

The PR adds perry compile --opt-report with text and JSON output, compiler attribution scopes, structured representation-selection denials, Ptr Shape diagnostics, cache bypassing, documentation, tests, and a batch benchmark fixture.

Optimization report

Layer / File(s)	Summary
Report schema, attribution, and rendering `crates/perry-codegen/src/opt_report/*`, `crates/perry-codegen/src/lib.rs`	Adds report records, scopes, callback hotness attribution, aggregation, ordering, text rendering, JSON rendering, and tests.
Ptr Shape denial reporting `crates/perry-codegen/src/collectors/*`	Adds structured Ptr Shape acceptance and denial reporting, escape-context tracking, admission causes, allocation-site discovery, and candidate metadata.
Ptr Shape report validation `crates/perry-codegen/src/collectors/ptr_shape_opt_report_tests.rs`	Tests accepted locals, specific escape reasons, module barriers, unbound allocations, and disabled reporting.
Codegen attribution and representation events `crates/perry-codegen/src/codegen/*`, `crates/perry-codegen/src/expr/slot_rep.rs`	Scopes module, function, method, static-method, and closure lowering, and records specialized-ABI and canonical-slot decisions.
CLI and compile pipeline integration `crates/perry/src/commands/compile/*`, `crates/perry/src/commands/dev.rs`, `crates/perry/src/commands/run/mod.rs`	Adds the CLI format option, environment handling, cache exclusion, report rendering to stderr, and explicit disabled settings for other compile callers.
Documentation and benchmark coverage `changelog.d/7037-opt-report.md`, `docs/src/cli/flags.md`, `benchmarks/app-patterns/kernels/batch.ts`	Documents opt-report behavior and adds a deterministic batch-processing benchmark with checksum output.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CompileCLI
  participant CompilePipeline
  participant Codegen
  participant OptReport
  participant Renderer
  CompileCLI->>CompilePipeline: parse --opt-report format
  CompilePipeline->>Codegen: disable cache and compile modules
  Codegen->>OptReport: record selections and denials
  CompilePipeline->>OptReport: take_entries()
  OptReport->>Renderer: render ordered entries
  Renderer-->>CompileCLI: write text or JSON to stderr

Possibly related issues

repsel row 1: scoping Ptr<Shape> beyond locals — measured, promotion is 0 on the motivating workload; build --opt-report (#6952) first #7034 — Covers the opt-report measurement instrument, including Ptr Shape diagnostics and the batch reproduction fixture.
opt-report v2: LocalId → Span side-table so --opt-report can print file:line and a source snippet #7036 — Proposes adding source locations and snippets to the opt-report entries introduced here.

Possibly related PRs

PerryTS/perry#6903 — Introduces the canonical-local representation-selection path now instrumented by opt-report.
PerryTS/perry#6905 — Adds the specialized-entry representation plan reported in compile_function.
PerryTS/perry#6911 — Introduces the Ptr Shape proof pipeline now extended with structured reporting.

Suggested labels: tooling

Suggested reviewers: thehypnoo

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR implements opt-report surfacing, hotness ranking, actionability tiers, disabled-by-default behavior, and stable JSON output as requested in `#6952`.
Out of Scope Changes check	✅ Passed	The benchmark, tests, docs, and changelog fragment all support the opt-report feature and appear in scope.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check	✅ Passed	The title clearly names the new --opt-report CLI feature and its purpose.
Description check	✅ Passed	The description covers the feature, motivation, tests, and linked issues, though it doesn't follow the template headings exactly.

✨ 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 feat/6952-opt-report

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.}

…y typed, and why Perry's speed comes from proving static types and selecting unboxed representations. When a proof fails the value stays NaN-boxed and the fast paths silently do not fire — and until now the only way to find out WHICH values failed was to read LLVM IR. That blindness is not theoretical. #7034 measured the flagship representation on the object-heavy workload that motivates it and found Ptr<Shape> promotes ZERO locals there; PERRY_PTR_SHAPE_LOCALS=0 vs default produced byte-identical output. A shipped representation was doing nothing on realistic code, and it was found by accident. Establishing that single fact took six compiler invocations, an IR dump, a symbol-level size A/B and a PERRY_REPSEL_DEBUG grep. perry compile batch.ts -o batch --opt-report now answers it in one command, and names the rule that denied each candidate. This is mostly surfacing, not new analysis: the collectors already know which rule rejected each candidate, they just `continue`. The change records (value, reason) at those sites, plus one aggregation layer and one renderer. Covered: Ptr<Shape> locals and allocation sites (all five rules plus class admission, with the escape KIND discriminated — reassignment vs closure capture vs call argument vs return vs container element), canonical i32/u32/Str locals, and the specialized-ABI entry decision (the only param/return representation today). Reports wins as well as misses. Hotness is reported as two separate columns, loop depth AND whether the enclosing region is an iterating builtin's callback, because #7034 §8 measured that 208 of 247 guard sites in the motivating workload sit in callback bodies with zero loop depth. Collapsing them into one number would rank them last. Observational only: emitted LLVM IR is byte-identical with the flag on and off (verified on two programs). Off by default, gated by a OnceLock read of PERRY_OPT_REPORT before any reason string is formatted. Like --trace llvm it disables build/object cache reuse for its own run, since a cache hit skips codegen and there would be nothing to report; it is deliberately NOT in the object-cache key, because it changes no emitted byte. Values are reported by function + binding name. HIR keeps names through lowering but drops source positions, so there is no file:line yet; the LocalId -> Span side-table that would add it is #7036.

coderabbitai

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/perry-codegen/src/codegen/closure.rs`:
- Around line 734-741: When opt-reporting is disabled, avoid evaluating
report-name arguments: in crates/perry-codegen/src/codegen/closure.rs lines
734-741, guard the func_names clone/closure-name format with
opt_report::enabled(); in crates/perry-codegen/src/codegen/method.rs lines
373-377 and 1417-1421, likewise defer the instance- and static-method format!
calls until reporting is enabled, while preserving the existing enter_closure
and method reporting behavior when enabled.

In `@crates/perry-codegen/src/codegen/function.rs`:
- Around line 616-633: Update the opt-report call in the specialized entry
reporting block to describe only parameter representations, since
SpecFnPlan::reps excludes the return representation. Replace the "(parameters +
return)" label with "(parameters)" and leave the existing parameter
representation collection unchanged.

In `@crates/perry-codegen/src/codegen/mod.rs`:
- Around line 194-202: Extend opt-report callback scanning beyond the methods
and constructors currently visited by scan_module. Update the scanner to
traverse every lowered class-region body, including getters, setters, static
methods, and computed members, so closures in each are marked as per-element
callbacks before region lowering.

In `@crates/perry/src/commands/compile/build_cache.rs`:
- Around line 391-395: Update the cache-bypass condition in the build-cache
logic to disable caching only when opt-report is explicitly requested via
args.opt_report or when PERRY_OPT_REPORT has a recognized value: 1, text, or
json. Ignore other environment values, such as 0, so they do not prevent cached
builds.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0eb13bfe-93c8-47e9-8cc8-5c2f18935646

📥 Commits

Reviewing files that changed from the base of the PR and between 0b707a7 and 66b0632.

📒 Files selected for processing (23)

benchmarks/app-patterns/kernels/batch.ts
changelog.d/7037-opt-report.md
crates/perry-codegen/src/codegen/closure.rs
crates/perry-codegen/src/codegen/entry.rs
crates/perry-codegen/src/codegen/function.rs
crates/perry-codegen/src/codegen/method.rs
crates/perry-codegen/src/codegen/mod.rs
crates/perry-codegen/src/codegen/typed_abi_opt_report.rs
crates/perry-codegen/src/collectors/mod.rs
crates/perry-codegen/src/collectors/ptr_shape.rs
crates/perry-codegen/src/collectors/ptr_shape_opt_report_tests.rs
crates/perry-codegen/src/collectors/ptr_shape_report.rs
crates/perry-codegen/src/expr/slot_rep.rs
crates/perry-codegen/src/lib.rs
crates/perry-codegen/src/opt_report/callbacks.rs
crates/perry-codegen/src/opt_report/mod.rs
crates/perry-codegen/src/opt_report/render.rs
crates/perry/src/commands/compile/build_cache.rs
crates/perry/src/commands/compile/run_pipeline.rs
crates/perry/src/commands/compile/types.rs
crates/perry/src/commands/dev.rs
crates/perry/src/commands/run/mod.rs
docs/src/cli/flags.md

coderabbitai · 2026-07-30T04:11:30Z

+    // `--opt-report` (#6952): closures are the position #7034 §8 found most
+    // of the guard sites in, so they get their own scope with the source
+    // function name when one is known.
+    let opt_report_name = func_names
+        .get(&func_id)
+        .cloned()
+        .unwrap_or_else(|| format!("closure#{func_id}"));
+    let _opt_report_scope = crate::opt_report::enter_closure(&opt_report_name, func_id);


🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win

Avoid report-name allocations while opt-report is disabled.

The report helpers no-op when disabled, but their arguments are evaluated first. This unconditionally clones/formats names during ordinary compilation, violating the requirement for no reporting cost when disabled.

crates/perry-codegen/src/codegen/closure.rs#L734-L741: only clone or format opt_report_name after checking opt_report::enabled().

crates/perry-codegen/src/codegen/method.rs#L373-L377: defer the instance-method format! until reporting is enabled.

crates/perry-codegen/src/codegen/method.rs#L1417-L1421: defer the static-method format! until reporting is enabled.

📍 Affects 2 files

crates/perry-codegen/src/codegen/closure.rs#L734-L741 (this comment)

crates/perry-codegen/src/codegen/method.rs#L373-L377

crates/perry-codegen/src/codegen/method.rs#L1417-L1421

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-codegen/src/codegen/closure.rs` around lines 734 - 741, When opt-reporting is disabled, avoid evaluating report-name arguments: in crates/perry-codegen/src/codegen/closure.rs lines 734-741, guard the func_names clone/closure-name format with opt_report::enabled(); in crates/perry-codegen/src/codegen/method.rs lines 373-377 and 1417-1421, likewise defer the instance- and static-method format! calls until reporting is enabled, while preserving the existing enter_closure and method reporting behavior when enabled.

coderabbitai · 2026-07-30T04:11:30Z

+        // `--opt-report` (#6952): the spec-ABI win, recorded at the same site
+        // as the PERRY_REPSEL_DEBUG line so the two cannot diverge.
+        if crate::opt_report::enabled() {
+            crate::opt_report::select(
+                crate::opt_report::Position::Param,
+                "(parameters + return)",
+                None,
+                crate::opt_report::Analysis::SpecAbi,
+                &plan
+                    .reps
+                    .iter()
+                    .map(|r| r.label().to_string())
+                    .collect::<Vec<_>>()
+                    .join(","),
+                0,
+                Some(format!("specialized entry for {}", f.name)),
+            );
+        }


🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not report an un-specialized return as selected.

SpecFnPlan::reps is parameter-only (reps.len() == f.params.len()), while the specialized function still returns DOUBLE. Labeling this entry as "(parameters + return)" falsely reports a return-representation win. Rename it to "(parameters)" or emit a separate, accurate return decision.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-codegen/src/codegen/function.rs` around lines 616 - 633, Update the opt-report call in the specialized entry reporting block to describe only parameter representations, since SpecFnPlan::reps excludes the return representation. Replace the "(parameters + return)" label with "(parameters)" and leave the existing parameter representation collection unchanged.

coderabbitai · 2026-07-30T04:11:30Z

+    // `--opt-report` (#6952): mark the closures that are iterating-builtin
+    // callbacks before any region is lowered, so their denials carry the
+    // per-element hotness column. No-op when the report is off.
+    crate::opt_report::scan_module(hir);
+    // Module-wide fallback attribution scope. Per-region scopes nest inside
+    // it and restore it on drop, so decisions taken outside any region (the
+    // specialized-ABI entry decision) still know their module.
+    let _opt_report_module_scope = crate::opt_report::enter_module(&hir.name);
+


🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Scan callbacks in every lowered class region.

scan_module only traverses instance methods and constructors. Closures in getters, setters, static methods, and computed members are therefore never marked as per-element callbacks, so their report hotness is incomplete. Extend the scanner to cover those bodies.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-codegen/src/codegen/mod.rs` around lines 194 - 202, Extend opt-report callback scanning beyond the methods and constructors currently visited by scan_module. Update the scanner to traverse every lowered class-region body, including getters, setters, static methods, and computed members, so closures in each are marked as per-element callbacks before region lowering.

coderabbitai · 2026-07-30T04:11:30Z

+    // #6952: a cached build reuses the finished binary and never runs codegen,
+    // so the report would be empty. Same reasoning as explain-lowering above.
+    if args.opt_report.is_some() || std::env::var("PERRY_OPT_REPORT").is_ok() {
+        return Err("opt-report".to_string());
+    }


🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win

Only disable the cache for recognized report values.

Line 393 treats any present PERRY_OPT_REPORT value as enabled, but the pipeline only recognizes 1, text, and json. For example, PERRY_OPT_REPORT=0 silently disables the build cache without producing a report.

Proposed fix

- if args.opt_report.is_some() || std::env::var("PERRY_OPT_REPORT").is_ok() { + if args.opt_report.is_some() + || matches!( + std::env::var("PERRY_OPT_REPORT").as_deref(), + Ok("1") | Ok("text") | Ok("json") + ) + {

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

// #6952: a cached build reuses the finished binary and never runs codegen,

// so the report would be empty. Same reasoning as explain-lowering above.

if args.opt_report.is_some() || std::env::var("PERRY_OPT_REPORT").is_ok() {

return Err("opt-report".to_string());

}

// `#6952`: a cached build reuses the finished binary and never runs codegen,

// so the report would be empty. Same reasoning as explain-lowering above.

if args.opt_report.is_some()

|| matches!(

std::env::var("PERRY_OPT_REPORT").as_deref(),

Ok("1") | Ok("text") | Ok("json")

)

{

return Err("opt-report".to_string());

}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry/src/commands/compile/build_cache.rs` around lines 391 - 395, Update the cache-bypass condition in the build-cache logic to disable caching only when opt-report is explicitly requested via args.opt_report or when PERRY_OPT_REPORT has a recognized value: 1, text, or json. Ignore other environment values, such as 0, so they do not prevent cached builds.

… to the same IR (#7038) (#7039) * fix(codegen): sort closure-source emission so the same input compiles to the same IR emit_artifacts iterated hir.closure_source_text -- a HashMap -- when emitting retained closure-source constants, so the @.str.N numbering of those constants was a per-process permutation. Same input, different .ll on every run. The loop immediately above walks hir.functions (a Vec) and was already deterministic; only the materialized-closure loop keyed off map order. Collect, filter, sort by FuncId, then emit. Emission order is the only thing that changes. Why it matters beyond tidiness: it silently invalidates any A/B that compares raw LLVM IR. That technique underpins the planned codegen-perturbation measurement, and #7037 hit it directly -- its first observational-equivalence proof passed on nondeterministic output and had to be redone with a normalizer and four runs per arm. Evidence -- 6 compiles of one file, md5 of the concatenated per-module .ll: pristine main 5 distinct hashes across 6 runs this fix 1 hash across 6 runs Reported by the #7037 agent while proving --opt-report observational. Closes #7038. * changelog: fragment for #7039 --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>

proggeramlug pushed a commit that referenced this pull request Jul 30, 2026

docs(changelog): fragment for #7037

e4d4c62

proggeramlug mentioned this pull request Jul 30, 2026

codegen: same input compiles to different LLVM IR run-to-run — HashMap iteration order in closure-source registration breaks reproducible builds #7038

Closed

Ralph Küpper added 2 commits July 30, 2026 05:56

docs(changelog): fragment for #7037

66b0632

proggeramlug force-pushed the feat/6952-opt-report branch from e4d4c62 to 66b0632 Compare July 30, 2026 03:57

proggeramlug merged commit b5dbf3f into main Jul 30, 2026
5 of 6 checks passed

proggeramlug deleted the feat/6952-opt-report branch July 30, 2026 04:00

coderabbitai Bot reviewed Jul 30, 2026

View reviewed changes

proggeramlug mentioned this pull request Jul 30, 2026

fix(codegen): sort closure-source emission so the same input compiles to the same IR (#7038) #7039

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

feat(cli): --opt-report — surface which values could not be statically typed, why, and whether the developer can fix it - #7037

feat(cli): --opt-report — surface which values could not be statically typed, why, and whether the developer can fix it#7037
proggeramlug merged 2 commits into
mainfrom
feat/6952-opt-report

proggeramlug commented Jul 30, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 30, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Jul 30, 2026

Uh oh!

coderabbitai Bot Jul 30, 2026

Uh oh!

coderabbitai Bot Jul 30, 2026

Uh oh!

coderabbitai Bot Jul 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

proggeramlug commented Jul 30, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why now

The acceptance test, in one command

What it covers

Hotness: two columns, not one

Proof it is observational

Proof it does not lie

Tests

Scope — what it cannot yet see

Notes

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

proggeramlug commented Jul 30, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 30, 2026 •

edited

Loading