check: the wasm ratio ceiling's stale allowance claim, the fbw-census ring cutoff, and two inherited reds - #1492
Merged
Conversation
`WASM_MAX_DYNASM_RATIO`'s census paragraph excepted builtin_folds_hot and math_folds_hot from its worst-fixture claim as the two that "still carry an allowance". Neither carries a `# pyre-check: max-wasm-ratio` header, and no other fixture in `pyre/bench` does either -- the only in-tree occurrences are the reader, its error message, and check.py's own test fixture. Record what the paragraph's numbers are relative to: they come from a base that has moved, and a later single ubuntu run of 249 gated fixtures read pickle_terminal_raise_resume worst at 3.26x with foriter_make_function_body absent from the table. Under the union-of-runs rule the same comment states, one run is a reason to census again rather than a refutation of the 3.74x the following paragraph's arithmetic rests on, so that arithmetic is left alone. Assisted-by: Claude
`fbw_diag::record` writes the first `RING_ENTRIES` walks into the ring and lets every later one bump the totals only, while the native reader prints a line per walk it takes. The runner printed the ring's contents with no mention of the cutoff, so a run past it handed back a tally that reads like the whole population: `pickle_terminal_raise_resume` takes 79 walks and printed 24 lines. Print `(first 24 walks of 79)` ahead of the records when the count exceeds the ring. A run inside the ring prints nothing new. Assisted-by: Claude
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review. WalkthroughThe changes add hot-mode selfcheck directives, document ChangesWasm diagnostics and validation
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This PR corrects stale census documentation and clarifies truncated diagnostic output without changing gates or emitted code, so no actionable merge-blocking risk remains after normal checks and review. Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 4 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit 2aebf47). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patchNone. 2. Other mismatches introduced by this patchNone. 3. Pre-existing mismatches (already present before this patch)None. 4. Structural adaptations
|
Both fixtures carry the bare `# pyre-check: selfcheck` marker and no `selfcheck-compiles=`/`selfcheck-interpreted`, which `synth_selfcheck_compiles` rejects, so `pyre/check.py` errors out before it runs the synthetic suite. `PYRE_LOOP_CENSUS=1` prints one line for each: `[loop-census] loop hot`, on dynasm and on wasm alike. Both fixtures warm `hot` and then arm the hook, and their invariant is about what the compiled loop does once profiled, so the `loop` arm's `hot` is the shape to declare rather than the interpreted opt-out. Assisted-by: Claude
`cef6ccd9334` added the size audit's `std::env::var_os("PYRE_GC_SIZE_AUDIT")`
read in `majit-gc`'s collector without a triage row, so
`every_live_gate_has_a_triage_entry` failed on every host.
It is inert unless set, which puts it in §6c. The section's stated count was
72 against 74 names already listed; it now reads 75.
Assisted-by: Claude
youknowone
changed the title
This was referenced Aug 26, 2026
youknowone
added a commit
that referenced
this pull request
Aug 26, 2026
`6ad2595c699` added a §7 row for it and #1492 added a §6c listing plus a paragraph, so the merge of the two left one gate filed under two sections whose criteria differ: §6c is the list of gates that are inert unless set, §7 the table that states polarity and a retirement condition per gate. The §7 row is kept and the §6c listing dropped; its paragraph said what the row says. §6c names 74 gates again, which its heading now states. Assisted-by: Claude
This was referenced Aug 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two corrections found while attributing
pickle_terminal_raise_resume'swasm/dynasm ratio, plus the two reds this branch's base was carrying.
Nothing here changes a gate or any emitted code.
WASM_MAX_DYNASM_RATIO's census paragraph named allowance-holders that do not existThe comment excepted
builtin_folds_hotandmath_folds_hotfrom itsworst-fixture claim as the two that "still carry an allowance". Neither carries
a
# pyre-check: max-wasm-ratioheader, and no other fixture inpyre/benchdoes either — the only in-tree occurrences are the reader, its error message,
and check.py's own test fixture.
The paragraph's ratios are left as they are, with a note that they come from a
base that has since moved. One later ubuntu run of 249 gated fixtures read
pickle_terminal_raise_resumeworst at 3.26x withforiter_make_function_bodyabsent from the table, but under the union-of-runs rule the same comment states,
a single run is a reason to census again rather than a refutation of the 3.74x
the following paragraph's arithmetic rests on.
The wasm fbw-census printed a truncated tally with no mention of the cutoff
fbw_diag::recordwrites the firstRING_ENTRIESwalks into the ring and letsevery later one bump the totals only. The native reader prints a line per walk
it takes, so the two sides are directly diffable only while a run stays inside
the ring.
pickle_terminal_raise_resumetakes 79 walks and printed 24lines, which reads like the whole population.
It now prints
[fbw-census] (first 24 walks of 79)ahead of the records whenthe count exceeds the ring, and nothing new for a run inside it. Verified both
directions against a freshly built guest.
Two selfcheck fixtures did not say what they need compiled
4630ebc397a(#1479) addeda_profiler_installed_from_a_call_event_keeps_c_eventsand
a_raising_trace_hook_still_owes_the_leave_eventcarrying a bare# pyre-check: selfcheckmarker and no directive, whichsynth_selfcheck_compilesrejects outright — it kills the whole synthetic suite rather than the one fixture.
They were the only two in the tree in that shape.
PYRE_LOOP_CENSUS=1prints[loop-census] loop hotfor both on dynasm and onwasm, so both now declare
selfcheck-compiles=hot. Validated through check.py'sown reader and by running the pattern.
PYRE_GC_SIZE_AUDITwas read from the environment with no triage entrycef6ccd9334(#1489) added the size audit'sstd::env::var_osread inmajit-gc's collector without a row, soevery_live_gate_has_a_triage_entryfailed on all three hosts. It is inert unless set, which puts it in §6c; the
section's stated count was 72 against 74 names already listed, so it now reads
75. The prose notes that it is the one entry there that aborts rather than
reports.
Summary by CodeRabbit
Diagnostics
Testing
Documentation