fix(ci): ratchet the raw-handle debt baseline across the PR boundary by proggeramlug · Pull Request #7825 · PerryTS/perry

proggeramlug · 2026-08-11T01:48:58Z

Fixes #7659. Reported by @jdalton in #7389.

The Raw-handle debt ratchet step compared the counted sites against
scripts/raw_handle_debt_baseline.txt from the pull request checkout — a
number the same diff is free to move. Add bare get_raw_{mut,const}_ptr reads,
raise the baseline and the per-module ceilings to match, and the step passes.
--update refuses to raise, but nothing made CI run --update.

What changed

scripts/raw_handle_debt.py --no-raise-vs <ref> reads both recorded files out
of the merge base and fails if the checked-out copies are larger anywhere:

change	verdict
total baseline raised	❌
an existing module's ceiling raised	❌
a module added to the ceilings list	❌ (a raise from zero, not a fresh start)
unchanged	✅
total lowered / module cleaned away	✅
the merge base recorded nothing (gate predates it)	✅

Covering the per-module ceilings as well as the total closes the "clean one
module, dirty another" path, where the total can stay flat while a ceiling
moves.

The way this rule dies is by passing

An unfetched merge base makes git show <ref>:<path> report every file as
absent — indistinguishable from "the gate did not exist on that side", which is
a legal no-op. So the ref is resolved first, and an unresolvable one raises
rather than comparing against nothing:

$ python3 scripts/raw_handle_debt.py --no-raise-vs deadbeef…
::error::cannot resolve deadbeef…. The merge base was not fetched, so the
raw-handle ratchet cannot compare against it -- failing rather than passing on
a comparison that did not happen.
exit=1

The workflow gates the step on github.event_name == 'pull_request' rather
than on an empty variable, so a push build skips it by declaration instead of
by silence.

Acceptance criteria

a PR raising the baseline relative to its merge base fails the step
unchanged baseline passes
lowered baseline passes
--self-test still runs, and grew eight cases (three raises that must
fire, four legal diffs that must not, and the unresolvable ref)
the current-count check is retained and runs first
the checkout state is handled: git cat-file -e first, then
git fetch --no-tags --depth=1 origin "$BASE_SHA" only if missing —
actions/checkout's default depth does not include the base commit

Validation

Run in a real checkout against origin/main:

$ python3 scripts/raw_handle_debt.py --self-test
self-test ok (998 sites across 110 files); all three per-module rules fire,
clean case silent; merge-base rule rejects all three raises and passes four
legal diffs

$ python3 scripts/raw_handle_debt.py --no-raise-vs origin/main
recorded debt vs. origin/main: baseline 998 -> 998, 110 -> 110 module
ceiling(s), none raised            # exit 0

$ echo 999 > scripts/raw_handle_debt_baseline.txt
$ python3 scripts/raw_handle_debt.py --no-raise-vs origin/main
::error::recorded raw-handle debt rose vs. origin/main: 1 violation(s)
  baseline raised 998 -> 999 relative to the merge base. …     # exit 1

test.yml parses; the lint job now has both Raw-handle debt ratchet and
Raw-handle debt ratchet vs. merge base.

Summary by CodeRabbit

New Features
- Pull requests now automatically verify that raw-handle debt does not increase compared with the merge base.
- Checks detect higher overall debt, increased per-module limits, and newly introduced debt areas.
- Clear failures are reported when the comparison baseline cannot be resolved.
Documentation
- Added documentation describing the debt-ratcheting rules and pull-request checks.
Tests
- Added coverage for increases, permitted reductions, new modules, and unavailable baselines.

The `Raw-handle debt ratchet` step compared the counted sites against `scripts/raw_handle_debt_baseline.txt` FROM THE PULL REQUEST CHECKOUT — a number the same diff is free to move. Add bare `get_raw_{mut,const}_ptr` reads, raise the baseline and the per-module ceilings to match, and the step passes: the ratchet measured the diff against a value the diff had already edited. `--update` refuses to raise, but nothing made CI run `--update`. `--no-raise-vs <ref>` reads both recorded files out of the pull request's merge base and fails if the checked-out copies are larger anywhere: the total, an existing module's ceiling, or a module absent from the base's list (which is a raise from zero, not a fresh start). Unchanged and lower both pass, so paying debt down stays a one-step change. THE WAY THIS RULE DIES IS BY PASSING. An unfetched merge base makes `git show <ref>:<path>` report every file as absent, which is indistinguishable from "the gate did not exist on that side" — and that reads as a legal no-op. So the ref is RESOLVED FIRST, and an unresolvable one raises rather than comparing against nothing; the workflow gates the step on `github.event_name == 'pull_request'` rather than on an empty variable, so a push build skips it by declaration instead of by silence. `--self-test` grew eight cases: three raises that must fire, four legal diffs that must not, and the unresolvable ref that must fail rather than pass. Verified end to end against `origin/main` in a real checkout — clean tree passes, `999` in the baseline file fails with the specific diagnostic, and a bogus SHA exits 1 with the fetch hint. Fixes #7659

coderabbitai · 2026-08-11T01:49:30Z

📝 Walkthrough

Walkthrough

The raw-handle debt checker now compares pull-request totals and module ceilings with the merge base. CI resolves or fetches the base commit and runs this check only for pull requests. Self-tests cover increases, reductions, unchanged debt, and unresolved references.

Changes

Raw-handle debt ratchet

Layer / File(s)	Summary
Debt comparison contract `scripts/raw_handle_debt.py`	The checker parses module ceilings and rejects increased totals, increased existing ceilings, and newly listed modules.
Revision comparison execution `scripts/raw_handle_debt.py`	The checker resolves Git references, reads baseline files from revisions, and exposes `--no-raise-vs <ref>`.
Validation and CI wiring `.github/workflows/test.yml`, `scripts/raw_handle_debt.py`, `changelog.d/7825-raw-handle-merge-base-ratchet.md`	Self-tests cover comparison outcomes and unresolved references. Pull-request CI runs the merge-base check and the changelog documents the behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

PerryTS/perry#7389: The change extends this raw-handle debt ratchet and CI check with merge-base comparisons.
PerryTS/perry#7488: Both changes modify the raw-handle debt gate and its ceilings.

Suggested labels: tooling

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the CI raw-handle debt ratchet change across the pull request boundary.
Description check	✅ Passed	The description explains the problem, implementation, acceptance criteria, related issue, and validation steps.
Linked Issues check	✅ Passed	The changes satisfy `#7659` by enforcing merge-base comparisons, retaining existing checks, resolving refs, and handling the workflow checkout state.
Out of Scope Changes check	✅ Passed	The workflow, script, self-tests, baseline updates, and changelog fragment are directly related to the raw-handle debt ratchet objective.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ 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 fix/7659-raw-handle-merge-base

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

coderabbitai

Actionable comments posted: 1

🤖 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 @.github/workflows/test.yml:
- Around line 262-267: Update the workflow step around BASE_SHA and
raw_handle_debt.py to also obtain the pull request head SHA, fetch both commit
histories with sufficient depth, compute their actual merge base using git
merge-base, and pass that resulting SHA to --no-raise-vs instead of BASE_SHA.
Preserve the existing commit-availability handling while ensuring the comparison
targets the PR’s merge base.

🪄 Autofix

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: e60ebfaa-bf6c-41d7-a71a-4034f5bf20ba

📥 Commits

Reviewing files that changed from the base of the PR and between 1804991 and 49a3ed7.

📒 Files selected for processing (3)

.github/workflows/test.yml
changelog.d/7825-raw-handle-merge-base-ratchet.md
scripts/raw_handle_debt.py

coderabbitai · 2026-08-11T01:55:21Z

+        env:
+          BASE_SHA: ${{ github.event.pull_request.base.sha }}
+        run: |
+          git cat-file -e "$BASE_SHA^{commit}" 2>/dev/null \
+            || git fetch --no-tags --depth=1 origin "$BASE_SHA"
+          python3 scripts/raw_handle_debt.py --no-raise-vs "$BASE_SHA"


🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Use the actual Git merge base.

Line 263 passes the base branch tip, not git merge-base of the pull request head and base. If the base branch advances after the pull request branches, this gate compares against a different revision than the PR objective requires.

Fetch both histories with sufficient depth. Compute git merge-base "$BASE_SHA" "$HEAD_SHA". Pass that SHA to --no-raise-vs. GitHub Actions checks out a pull request merge commit by default and exposes the head SHA separately. (docs.github.com)

🤖 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 @.github/workflows/test.yml around lines 262 - 267, Update the workflow step around BASE_SHA and raw_handle_debt.py to also obtain the pull request head SHA, fetch both commit histories with sufficient depth, compute their actual merge base using git merge-base, and pass that resulting SHA to --no-raise-vs instead of BASE_SHA. Preserve the existing commit-availability handling while ensuring the comparison targets the PR’s merge base.

Covers the 32 PRs admin-merged in one pass (audited in principle at the maintainer's direction): PerryTS#7768 PerryTS#7772 PerryTS#7779 PerryTS#7784 PerryTS#7785 PerryTS#7786 PerryTS#7788 PerryTS#7789 PerryTS#7797 PerryTS#7798 PerryTS#7801 PerryTS#7802 PerryTS#7804 PerryTS#7805 PerryTS#7806 PerryTS#7807 PerryTS#7808 PerryTS#7810 PerryTS#7811 PerryTS#7815 PerryTS#7816 PerryTS#7818 PerryTS#7819 PerryTS#7820 PerryTS#7821 PerryTS#7822 PerryTS#7823 PerryTS#7824 PerryTS#7825 PerryTS#7826 PerryTS#7827 PerryTS#7828. (PerryTS#7787 closed as already-landed via the PerryTS#7786 stack.) Per-change history lives in each PR's changelog.d fragment as usual. Claude-Session: https://claude.ai/code/session_01Y1QZ5wUP9gRSwpiweT4Wix

…ss_* (#7838) (#7840) * fix(runtime): convert #7811/#7815's new bare raw-handle reads to across_* (#7838) The raw-handle debt ratchet went red on main: 1,013 bare reads against a baseline of 998, with four per-module violations. The +15 all arrived with the two #6949 rooting fixes in the 2026-08-11 batch, and #7825 -- same batch -- closed the hole that had been letting a PR's own checkout carry the comparison baseline, so the ratchet only started seeing them once it landed. All fifteen convert. None needed a ceiling raise and the baseline is untouched: the total lands back on 998 exactly, and regex/replace_fn.rs returns to its recorded ceiling of 3. #7815 (5 sites, three modules, all previously unlisted -> all now zero) disposable.rs, messaging.rs, builtins/formatting/boxed_primitives.rs. Each is the exact shape across_* exists for: allocate the receiver, run one allocating coercion, then write through a re-read pointer. #7811 (10 sites in regex/replace_fn.rs) NOT the sanctioned "loop across a user-visible trap" shape that raw_handle_debt_files.txt permits joining the list for -- that shape is the three PRE-EXISTING reads (`cur_str` in the two string-replacer loops, plus the subject re-read in call_string_replace_callback), which is what the ceiling of 3 was recorded for. The ten new ones are plain root -> one allocating js_string_coerce -> read-for-the-call, which across_* expresses directly. Two small local combinators carry the four two-receiver call sites; two receivers compose by NESTING across_const, the same way path::value_args::with_two_headers already does it. Two of the conversions are not purely mechanical, because writing the ordering out made a latent stale-address use visible in disposable.rs: - set_nonenum ran crate::object::set_property_attrs(obj as usize, ..) AFTER js_object_set_field_by_name, which allocates when the object grows. That side table is keyed on the address, so a pre-call copy does not fault -- it files the attributes under an address nothing looks up, and `error` / `suppressed` / `message` silently become enumerable on a SuppressedError that grew during the set. - js_nanbox_pointer(obj) was built BEFORE the SuppressedError.prototype lookup and returned afterwards. A NaN-box is a frozen address the collector cannot rewrite, so the returned value named from-space if the lookup collected. The box is now built last, after every allocating call. Both are the #7192 shape (the store is in-frame but after a call that allocates), and neither is reachable without evacuation moving the receiver, so this is ordering hygiene rather than an observed crash. Verified: scripts/raw_handle_debt.py exits 0 at 998/998, "110 module(s) within ceilings; every other runtime module is locked at zero". * docs(changelog): fragment for #7840 --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>

Ralph Küpper added 2 commits August 11, 2026 03:48

docs(changelog): add fragment for the #7659 merge-base ratchet

49a3ed7

coderabbitai Bot reviewed Aug 11, 2026

View reviewed changes

proggeramlug merged commit 3eea8c7 into main Aug 11, 2026
18 of 19 checks passed

proggeramlug deleted the fix/7659-raw-handle-merge-base branch August 11, 2026 05:26

This was referenced Aug 11, 2026

raw-handle ratchet red on main: #7811/#7815 landed 15 bare reads the same batch's #7825 now counts #7838

Closed

fix(runtime): convert #7811/#7815's new bare raw-handle reads to across_* (#7838) #7840

Merged

Navigation Menu

Uh oh!

Uh oh!

fix(ci): ratchet the raw-handle debt baseline across the PR boundary - #7825

fix(ci): ratchet the raw-handle debt baseline across the PR boundary#7825
proggeramlug merged 2 commits into
mainfrom
fix/7659-raw-handle-merge-base

proggeramlug commented Aug 11, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Aug 11, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Aug 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

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

Uh oh!

What changed

The way this rule dies is by passing

Acceptance criteria

Validation

Summary by CodeRabbit

Uh oh!

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

Uh oh!

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Aug 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

proggeramlug commented Aug 11, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Aug 11, 2026 •

edited

Loading