docs/test(gc): re-derive the current collector page's claims instead of asserting them (#7877) - #7936
Conversation
…of asserting them (#7877) The GC reference pages drifted again in the landing train that created them. Rather than correct the prose a second time, bind the checkable half of it to the code: `scripts/check_gc_doc_claims.py` verifies that every repo path the current pages cite exists, that every documented number matches the constant it came from (`gc-fact` markers), and that every source-map row still names a defined symbol (`gc-symbol` markers). The operations page now names no issue numbers at all, because an issue reference is a claim about a tracker that this repository cannot see. Corrected, each verified against code rather than against another document: weak processing is registry-scoped and holder-sliced on every path; the block reuse pool's cap is process-wide and budget-scaled, not per-thread; old-page defrag has its rewrite contract and is opt-in pending a fragmentation corpus; tenuring is an adaptive 1-4 threshold, not `PROMOTION_AGE = 2`; whole-block in-place promotion and the type-dependent born-old thresholds were undocumented; `js_arena_stats` reports a live census, so the ratchet README's whole-block triage advice no longer holds; and the memory-model source map pointed 8 of 9 rows at files deleted in the `gc.rs` split. Two inline literals become named constants so the page can cite them: `BLOCK_POOL_CAP_DEFAULT_BYTES` and `SCAVENGE_NURSERY_CAP_DEFAULT_MB`. Neither changes a value.
📝 WalkthroughWalkthroughThe change adds a GC documentation claim checker, names two runtime defaults, updates GC and accounting documentation, and runs validation in Linux lint and Windows structural-audit workflows. ChangesGC documentation validation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CI
participant Checker
participant Documentation
participant RustSources
CI->>Checker: Run self-test and repository audit
Checker->>Documentation: Read paths and claim markers
Checker->>RustSources: Verify constants and symbols
Checker-->>CI: Return validation status
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
…dd the changeset The four claims that went false in the last landing train now carry a gc-symbol marker naming their test, so a rename fails lint and a behaviour change fails cargo-test. A doc sentence and a green suite can no longer disagree quietly.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@scripts/check_gc_doc_claims.py`:
- Around line 241-261: Update check_repo() to count gc-symbol markers separately
from gc-fact markers and append a validation problem when the symbol count is
below its minimum floor. Extend the --self-test coverage so removing all
gc-symbol markers causes the audit to fail, ensuring the symbol rule cannot pass
vacuously.
- Around line 181-205: Update the Rust lookup logic in the gc-fact and gc-symbol
validation loops to inspect parsed Rust items or lexer-filtered source, not raw
text, so comments and string literals cannot satisfy markers. Ensure removed
declarations hidden only in comments or literals are reported as missing, and
add sabotage cases covering both scenarios.
🪄 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: 6b9d9689-01c2-4c96-aa56-c0e3981f56bd
📒 Files selected for processing (9)
.github/workflows/test.ymlCLAUDE.mdbenchmarks/gc_ratchet/README.mdchangelog.d/7936-gc-doc-claim-gate.mdcrates/perry-runtime/src/gc/heap_budget.rscrates/perry-runtime/src/gc/policy.rsdocs/src/internals/garbage-collector.mddocs/src/internals/memory-model.mdscripts/check_gc_doc_claims.py
| source = source_path.read_text(encoding="utf-8") | ||
| item = re.search(RUST_ITEM_RE.format(name=re.escape(name)), source) | ||
| if item is None: | ||
| problems.append( | ||
| f"{rel}: gc-fact {name} is not defined in {path} " | ||
| "(renamed or deleted -- update the page, not the marker)" | ||
| ) | ||
| continue | ||
| if not values_agree(documented, item.group("rhs")): | ||
| problems.append( | ||
| f"{rel}: gc-fact {name} documents {documented.strip()!r} " | ||
| f"but {path} defines {item.group('rhs').strip()!r}" | ||
| ) | ||
| for match in SYMBOL_RE.finditer(text): | ||
| name, path = match.group("name"), match.group("path") | ||
| source_path = root / path | ||
| if not source_path.is_file(): | ||
| problems.append(f"{rel}: gc-symbol {name} names {path}, which is not a file") | ||
| continue | ||
| source = source_path.read_text(encoding="utf-8") | ||
| if not re.search(SYMBOL_DEF_RE.format(name=re.escape(name)), source): | ||
| problems.append( | ||
| f"{rel}: gc-symbol {name} is not defined in {path} " | ||
| "(moved or renamed -- the source map is pointing at nothing)" | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Parse Rust items before accepting a marker.
Lines 182 and 201 search raw Rust text. Both patterns also match Rust-like text in comments and string literals. If a refactor removes a documented item but retains its old declaration in a comment, the marker stays green.
Strip comments and literals with a Rust-aware lexer, or parse item declarations. Add sabotage cases where the only matching text is in a comment or string literal.
🧰 Tools
🪛 ast-grep (0.45.1)
[warning] 181-181: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.search(RUST_ITEM_RE.format(name=re.escape(name)), source)
Note: [CWE-1333] Inefficient Regular Expression Complexity.
(redos-non-literal-regex-python)
[warning] 200-200: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.search(SYMBOL_DEF_RE.format(name=re.escape(name)), source)
Note: [CWE-1333] Inefficient Regular Expression Complexity.
(redos-non-literal-regex-python)
🤖 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 `@scripts/check_gc_doc_claims.py` around lines 181 - 205, Update the Rust
lookup logic in the gc-fact and gc-symbol validation loops to inspect parsed
Rust items or lexer-filtered source, not raw text, so comments and string
literals cannot satisfy markers. Ensure removed declarations hidden only in
comments or literals are reported as missing, and add sabotage cases covering
both scenarios.
| def check_repo(root: Path) -> list[str]: | ||
| problems = [] | ||
| facts = 0 | ||
| for rel in CURRENT_DOCS: | ||
| path = root / rel | ||
| if not path.is_file(): | ||
| problems.append(f"{rel}: checked document is missing") | ||
| continue | ||
| text = path.read_text(encoding="utf-8") | ||
| facts += len(FACT_RE.findall(text)) | ||
| problems.extend(check_document(rel, text, root)) | ||
| # A rule that inspects nothing passes vacuously. The fact rule is the one | ||
| # with a population that can silently go to zero (delete every marker and it | ||
| # is green), so it asserts its own subject was live. | ||
| if facts < MIN_FACTS: | ||
| problems.append( | ||
| f"only {facts} gc-fact markers found, expected at least {MIN_FACTS}. " | ||
| "Rule 2 is the only defence against a documented number drifting; " | ||
| "removing markers instead of fixing them disarms it." | ||
| ) | ||
| return problems |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep the gc-symbol rule non-vacuous.
check_repo() counts only gc-fact markers. If all gc-symbol markers are removed, the symbol rule receives no input and the audit can still pass. Count gc-symbol markers, enforce a minimum floor, and make --self-test prove that removing all symbol markers fails.
As per coding guidelines: “A gate must assert its subject was live, not merely that nothing threw.”
🧰 Tools
🪛 ast-grep (0.45.1)
[warning] 249-249: XPath query is request-/variable-derived; use parameterized XPath to prevent injection.
Context: FACT_RE.findall(text)
Note: [CWE-643] Improper Neutralization of Data within XPath Expressions ('XPath Injection').
(xpath-injection-python)
🤖 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 `@scripts/check_gc_doc_claims.py` around lines 241 - 261, Update check_repo()
to count gc-symbol markers separately from gc-fact markers and append a
validation problem when the symbol count is below its minimum floor. Extend the
--self-test coverage so removing all gc-symbol markers causes the audit to fail,
ensuring the symbol rule cannot pass vacuously.
Source: Coding guidelines
1 participant
Closes #7877.
Why this is a gate and not a correction
#7877 was reopened because the "current source of truth" page drifted inside the
same landing train that created it. Correcting the prose a second time buys
another few days. The half of a reference page that a machine can re-derive is
its paths and its numbers, so this binds those to the code and lets
lintadjudicate.
scripts/check_gc_doc_claims.py, three rules, each with a--self-testarm thatproves it can fail:
path:LINEcitation is rejected outright —
memory-model.md's source map pointed 8 ofits 9 rows at a
crates/perry-runtime/src/gc.rsdeleted in the module split,with line numbers, and nothing noticed.
<!-- gc-fact: NAME = VALUE in PATH -->marker and is compared against theconstin that file; asource-map row carries
<!-- gc-symbol: NAME in PATH -->and must still namea definition. 17 facts are re-derived today. The rule asserts its own subject
was live: deleting the markers instead of fixing them trips a floor.
phase is atomic and unsliced (perf(gc): full/fallback weak processing is an unsliced O(total heap) atomic pause #7874)" was false the moment perf(gc): full/fallback weak processing is an unsliced O(total heap) atomic pause #7874 closed, and
an issue's state is not a fact this repository holds. Off that page the rule
is narrower — it flags "#N tracks", "tracked by #N", "blocked on #N" — so the
historical, causal references in
memory-model.mdare untouched.The script says plainly what it cannot catch: prose that is simply wrong about
behaviour. That is why rule 2 exists — a number is the part of a behavioural
claim a machine can hold onto.
What was actually wrong
Every row verified against code, not against another document.
FullWeakProcessingStatesnapshots the holder registry and consumes a bounded number per step — O(registered holders), resumablePROMOTION_AGE = 2js_arena_stats"sums block offsets … UNCHANGED" (ratchet README)CLAUDE.md: registry is ingc/mod.rs, "~55 entries"gc/roots.rs, and the population is 123 — the count appeared twice in CLAUDE.md and only one copy got corrected by hand, which is the argument against the number being in prose at allCLAUDE.mdalso citedcrates/perry-codegen/src/codegen.rsandcrates/perry-hir/src/lower.rsfor the add-a-widget recipe; both are moduletrees now. Rule 1 found those, not a human.
Code changes
Two inline literals become named constants so the page can cite them:
BLOCK_POOL_CAP_DEFAULT_BYTES(64 MiB) andSCAVENGE_NURSERY_CAP_DEFAULT_MB(16). Neither changes a value or a code path.
Validation
--self-test: green, and each rule's failing arm is asserted, including aplanted drifted value, a renamed constant, a renamed symbol, and a marker
naming a file that does not exist.
documented number and changing the constant in the source both fail
(exit 1, checked unpiped); renaming
tenuring_survivalsfails; adding#7876 tracks itto the operations page fails; deleting every marker failsthe vacuity floor. Tree clean afterwards.
python3 scripts/gc_gate_wiring_check.py→ 7 gates main-line-reachable;scripts/check_file_size.sh→ OK;cargo fmt --all -- --check→ clean.test.yml→lint(a required context) and into the WindowsGC structural audits step, alongside the existing knob audit. Per CLAUDE.md's
hazard 1, it is a plain step with no
continue-on-error.Corpus (runtime touched, so this is not optional)
Rebuilt
-p perry -p perry-runtime-static -p perry-stdlib-staticafter the twoconstant extractions, archive mtime confirmed to move past the edit, and the
19-program GC corpus compiled and run against node's recorded output:
Including the
iso_misscanary atchecksum 437840 misses 0, which is gated onthe miss counter rather than on the aggregate.
Summary by CodeRabbit
Documentation
Quality Improvements