ci(test): fail on dark tests — a test file no registry runs

[jdalton]

Fourth time is a missing gate

#7192, #7216, #7252 and #7270/#7271 each shipped a test file that ran nowhere. All four added a test_gap_gc_* witness to test-files/ and no line to test-parity/gc_repsel_corpus.txt, so the file was compiled by nothing and executed by nothing. The last one was caught by hand at merge.

An unregistered file is not a failing test, it is no test at all. The PR is green. The reviewer sees a witness in the diff next to a passing CI run and reads the two together as "covered". Nothing says otherwise, because nothing ran. That is CLAUDE.md's fourth hazard in its purest form: the gate runs, its subject never does.

Two registration checks already existed and neither could catch the pull request that needed it:

• scripts/gc_repsel_matrix.sh auto-detects unregistered test_gap_repsel_* / test_gap_specabi_*
• gc-moving-witnesses.yml adds the test_gap_gc_* prefix

Both sit behind a 90-minute release build of the compiler, behind a changed-paths relevance filter, and in workflows that are not in branch protection's required contexts. The logic was right; the placement made it unable to fire in time.

What this adds

scripts/check_test_registration.py — the cheap half of those checks, pulled out to where it can block, and generalised past that one corpus. Pure filesystem and text: no compiler, no Node, ~0.2 s, so it runs on every PR.

$ python3 scripts/check_test_registration.py
test registration OK: checked 157 files against 4 registries
  gc-repsel-corpus             49 candidates    49 registered   0 excluded
  feature-matrix-probes        25 candidates    22 registered   3 excluded
  compiler-output-workloads    22 candidates    20 registered   2 excluded
  rust-test-modules            61 candidates    61 registered   0 excluded

Every registration mechanism in the repo

I surveyed the tree for anything that enumerates test files rather than globbing them. Four qualify:

Mechanism	Candidates	Registry	Runner
gc-repsel-corpus	test-files/test_gap_{gc,repsel,specabi}_*.ts	test-parity/gc_repsel_corpus.txt	gc_repsel_matrix.sh (gc-stress, gc-moving-witnesses)
feature-matrix-probes	test-features/probes/**/*.ts	test-features/feature_matrix.toml	gen_feature_matrix.py (feature-matrix)
compiler-output-workloads	benchmarks/compiler_output/fixtures/**/*.ts	benchmarks/compiler_output/workloads.toml	compiler_output_regression.py
rust-test-modules	crates/*/**/tests/**/*.rs below a suite root	the mod declaration in the parent module	cargo test

The last one is the Rust analogue of the same failure and is worth naming: cargo auto-discovers crates/<c>/tests/<suite>.rs, but a file one level deeper — a suite's module directory, or a #[cfg(test)] submodule under src/ — compiles only if a mod declaration names it. Without one, rustc never parses the file. It is not dead code; it is not code. No warning fires.

Everything else is glob-driven and cannot go dark (run_parity_tests.sh, gc_root_dominance_corpus.sh, public_baseline.py, cargo's suite roots). --list names those too, so "considered and safe" is distinguishable from "never looked at".

Today's dark set, and what happened to each

Zero currently-dark files across all four mechanisms — #7263 and #7271 registered the last of the known ones, so this is green on main from the first run. Five candidates are excluded, each named with a reason (never a count):

File	Why it is legitimately not registered
test-features/probes/type_only_imports/model.ts	helper module — imported by probes/type_only_imports/basic.ts, which IS registered
test-features/probes/modules/support/type-only-values.ts	helper module — imported by probes/modules/type-only-imports.ts, which IS registered
test-features/probes/dynamic_import/mod.ts	helper module — it is the target of the import("./mod.ts") under test in probes/dynamic_import/basic.ts
benchmarks/compiler_output/fixtures/raw_numeric_layout_smoke.ts	registered in a different registry: the raw_numeric_layouts workload spec in scripts/run_memory_stability_tests.sh's run_target_collector_architecture_gates. Not dark, just driven elsewhere.
.../native_memory_fixture_project/node_modules/@perry-fixtures/native-memory-fixture/index.ts	vendored package source inside a fixture project, resolved through node_modules by the fixture that imports it

I did not bulk-register anything.

One real finding the gate produced while I was writing it

The first draft's Rust mechanism reported crates/perry/src/commands/compile/resolve/tests/declaration_sidecar_tests/compile_package.rs as dark. It is not — its mod compile_package; lives inside an inline mod declaration_sidecar_tests { … } block two levels up in resolve/tests.rs, so the declaration is nowhere near the immediate parent. The checker now walks up to the crate root, and the self-test pins that exact shape as a false-positive guard. A gate that cries wolf gets disabled, which is its own way of being unable to fail.

How it is proven able to fail

Against all four hazards in CLAUDE.md:

1. No continue-on-error, no || true — the lint step's exit status is the gate.
2. Branch protection — see the section below. Short version: no change needed.
3. Concurrency — lint already scopes cancel-in-progress to pull_request.
4. The subject is asserted live. Each mechanism declares a floor on its candidate set and fails if the glob stops matching. "0 dark files over 0 candidates" and "0 dark files over 157 candidates" print the same verdict and mean opposite things, so every run states checked N files against M registries.

--self-test (32 cases, run in lint alongside the check) plants an unregistered file into each of the four mechanisms — over the real registries, through an in-memory overlay so the checkout is never mutated — asserts the gate names it, then removes it and asserts green. It also asserts that a collapsed candidate set fails, a stale exclusion fails, and a registry entry whose file is gone fails.

Exclusions are justified, not counted. A numeric threshold cannot tell a new dark file from an old one: fix one, add one, tally unchanged. And a stale exclusion — one matching no file on disk — is itself a failure, so an excuse cannot outlive the file it excuses (the rule gc_root_dominance_allowlist.json already uses).

Verified against live behaviour, on disk

Not just through the overlay. For each of the four mechanisms I created a real unregistered file in the working tree and ran the gate:

$ printf 'console.log("planted");\n' > test-files/test_gap_gc_PLANTED_dark_witness.ts
$ python3 scripts/check_test_registration.py
TEST REGISTRATION: a test file exists that nothing runs.

  - DARK TEST test-files/test_gap_gc_PLANTED_dark_witness.ts
      exists on disk but is not registered in test-parity/gc_repsel_corpus.txt, so
      scripts/gc_repsel_matrix.sh (gc-stress, gc-moving-witnesses) never runs it.
      Register it there, or add it to this script's `gc-repsel-corpus` exclusions with a reason.

checked 158 files against 4 registries
  gc-repsel-corpus             50 candidates    49 registered   0 excluded
  ...
EXIT=1

Then the full cycle, to prove the red was the plant and not ambient breakage:

• add the corpus line → exit 0 (50 candidates, 50 registered)
• delete the file, keep the line → exit 1, ROTTED ENTRY … lists 'test_gap_gc_PLANTED_dark_witness' but test-files/test_gap_gc_PLANTED_dark_witness.ts does not exist
• restore → exit 0

Same red-then-green cycle confirmed for a planted probe, a planted compiler-output fixture and a planted .rs module. Working tree left clean.

⚠️ Branch protection: no change needed, and that is deliberate

A repo admin does not have to do anything for this to block merges. Worth reading the reason rather than taking my word for it, because the opposite mistake is why we are here.

The gate is a step in the existing lint job, which is already in branch protection's required contexts (lint, cargo-test, api-docs-drift, security-audit). It is not a new workflow and not a new job, so there is no new context to promote.

That placement is the whole point. CLAUDE.md's hazard 2 is "not in branch protection's required contexts", and its corollary — a new gate must be run once and then promoted — names the second step people forget. gc-root-dominance sat red on main for days blocking nothing because that step was left undone. This gate is put where the step does not exist, so it cannot be skipped.

The corollary's other half is also satisfied: a new gate that has never been green blocks every open PR the moment it becomes required. This one is green on the current main for all four mechanisms today (output above), so it costs open PRs nothing.

If a reviewer would rather see it as its own promotable context, say so and I will split it into a standalone workflow — but that reintroduces exactly the follow-up step this placement removes, so I would argue against it.

⚠️ Separate finding: lint has been silently skipping seven gates since 2026-07-29

I hit this while verifying the gate on CI, and it is worth its own attention because it is a sixth way a gate can be unable to fail.

lint is a sequence of unrelated gates, and a failing step takes every later step in the job to skipped. Public benchmark evidence freshness has failed on main on every run from 2026-07-29 onward — five consecutive nightlies, confirmed on runs 30428100984, 30519299737, 30610234977, 30687607155, 30735945650. Everything after it never executed:

success  Audit workspace architecture
failure  Public benchmark evidence freshness   <-- stale benchmarks/results/public-node-bun-v1.json
skipped  File size limit
skipped  Binding upstream pins (lock-step)
skipped  GC store-site inventory
skipped  Address-classification audit
skipped  Gap snapshot checker self-test
skipped  Platform-aware parity allowlist self-test
skipped  Moving-GC gate wiring
skipped  GC matrix liveness gate

The job reports red, so it does block — but it blocks for one reason while seven other gates say nothing at all. The failure is not from this PR: I restored benchmarks/ and Cargo.toml to pristine origin/main in my tree and benchmarks/ci_public_baseline_check.py still reports public artifact benchmark inputs changed, and none of this PR's nine files is in that check's fingerprint set. It needs ./benchmarks/run_public_baseline.sh re-run, which is a maintainer job (pinned node 22.23.1 + bun 1.3.14).

Two consequences for this PR. First, this PR's own lint run is red at that same pre-existing step, not at mine — see the step list on the run. Second, I gave this gate if: ${{ !cancelled() }} so it executes regardless of what failed above it. It costs 0.2 s and shares no state with any earlier step, so there is no reason for it to be hostage to the benchmark artifact.

I did not add !cancelled() to the other steps — that is a second logical change and touches six gates I did not otherwise modify. I would suggest doing it, and it should probably be asserted by gc_gate_wiring_check.py, which today checks job-level wiring but not "is this step reachable given the steps before it".

Confirmed on this PR's own CI run (job 91545136411) — the guard does what it says:

success  Audit workspace architecture
failure  Public benchmark evidence freshness
skipped  File size limit
skipped  Binding upstream pins (lock-step)
skipped  GC store-site inventory
skipped  Address-classification audit
skipped  Gap snapshot checker self-test
skipped  Platform-aware parity allowlist self-test
skipped  Moving-GC gate wiring
skipped  GC matrix liveness gate
success  Test registration (dark tests)      <-- ran anyway, self-test + check both green

So the lint red on this PR is the pre-existing benchmark-artifact staleness, and the new gate is the one step after it that still executed and reported.

Documentation

The rule is written where a contributor will actually hit it, in four places:

• docs/src/testing/test-registration.md (new, linked from SUMMARY.md) — the full page: why it exists, the four mechanisms, the two legitimate ways out of a failure, and what is deliberately not covered.
• CONTRIBUTING.md, in "what goes in a PR", right under the existing "new behavior needs a test" bullet.
• The three registry files' own headers — gc_repsel_corpus.txt, feature_matrix.toml, workloads.toml each now open with "A NEW TEST FILE MUST BE REGISTERED HERE OR IT WILL NOT RUN", the enforcement chain cheapest-first, and a pointer to the exclusion list. gc_repsel_corpus.txt's header previously described only the two prefixes the matrix auto-detects, which is precisely the gap all four dark witnesses fell through.

Deliberately out of scope

tests/*.sh, tests/*.py, tests/*.ts: 143 of the 171 files there are referenced by nothing in the tree. I found that while surveying and it is a genuine finding, but it is not this bug. There is no registry to diff against, so "unregistered" is not even well defined; each file needs triage (wire it up, or delete it). Inventing a registry retroactively would make this gate red on day one for reasons unrelated to the four dark witnesses. --list prints the count and the reasoning rather than leaving the silence.

Scope

Script + CI + docs. No Rust touched, so cargo test is unaffected — nothing was rebuilt or needed to be. No version lines touched (per CONTRIBUTING, the maintainer bumps at merge). Largest new file is 656 lines, well under the 2000-line cap that #7256 just finished clearing.

Refs #7192, #7216, #7252, #7270, #7271.

Summary by CodeRabbit

• New Features

  • Added automated checks to detect test files missing from their required registries.
  • Added validation for stale registry entries, missing files, and justified exclusions.
  • Added self-tests and diagnostic reporting for registration checks.

• Documentation

  • Added contributor guidance and testing documentation covering registration requirements.
  • Documented registry expectations for feature probes, compiler workloads, and test corpora.

• Chores

  • Integrated test-registration validation into the required lint workflow.

[coderabbitai]

📝 Walkthrough

Walkthrough

This change adds a checker for four registry-driven test suites, runs it in required lint, and documents registration rules, exclusions, commands, and diagnostics.

Changes

Dark Test Registration

Layer / File(s)	Summary
Registration checker scripts/check_test_registration.py	Adds virtual filesystem support, registry parsers, Rust module resolution, and four mechanism definitions.
Registration evaluation and self-tests scripts/check_test_registration.py	Checks candidate floors, missing and stale registrations, exclusions, reverse mappings, CLI modes, and self-test cases.
Lint integration and registry contracts .github/workflows/test.yml, test-parity/gc_repsel_corpus.txt, test-features/feature_matrix.toml, benchmarks/compiler_output/workloads.toml	Runs the checker in lint and documents registration requirements for covered registries.
Contributor guidance and documentation CONTRIBUTING.md, docs/src/testing/test-registration.md, docs/src/SUMMARY.md, changelog.d/7278-dark-test-registration-gate.md	Documents the gate, commands, exclusions, diagnostics, remediation, and covered suites.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Lint
  participant RegistrationChecker
  participant RepositoryTree
  participant TestRegistries
  Lint->>RegistrationChecker: run self-test and validation
  RegistrationChecker->>RepositoryTree: discover candidate files
  RegistrationChecker->>TestRegistries: read registrations and exclusions
  TestRegistries-->>RegistrationChecker: return registry paths
  RegistrationChecker-->>Lint: return diagnostics and status

Loading

Possibly related PRs

• PerryTS/perry#7011: Enhances validation of the GC/repsel registry checked by this gate.
• PerryTS/perry#7206: Adds regression tests covered by the GC/repsel registration mechanism.
• PerryTS/perry#7263: Strengthens enforcement for the GC/repsel registry used by this gate.

Suggested labels: tooling

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the CI change that blocks unregistered test files, despite minor grammatical awkwardness.
Description check	✅ Passed	The description thoroughly covers the motivation, implementation, testing, scope, related issues, and CI behavior, although it does not follow every template heading.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[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 `@CONTRIBUTING.md`:
- Line 80: Update the test-registration paragraph in CONTRIBUTING.md to include
benchmarks/compiler_output/workloads.toml as the fourth explicit registry
example, preserving the existing examples and guidance so the stated count and
listed mechanisms match the documentation.

🪄 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: 1017c0cb-55e0-441f-9c6d-6b88bc741642

📥 Commits

Reviewing files that changed from the base of the PR and between 4e9b1b7 and d2f712f.

📒 Files selected for processing (9)

• .github/workflows/test.yml
• CONTRIBUTING.md
• benchmarks/compiler_output/workloads.toml
• changelog.d/7278-dark-test-registration-gate.md
• docs/src/SUMMARY.md
• docs/src/testing/test-registration.md
• scripts/check_test_registration.py
• test-features/feature_matrix.toml
• test-parity/gc_repsel_corpus.txt

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (5)

scripts/check_test_registration.py (5)

606-610: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Optional: keep the docstring layout in --help.

argparse reflows description by default, so the module docstring loses its structure. Pass formatter_class=argparse.RawDescriptionHelpFormatter to preserve it.

🤖 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_test_registration.py` around lines 606 - 610, Update the
ArgumentParser construction in main to use argparse.RawDescriptionHelpFormatter
via formatter_class, preserving the module docstring’s layout in --help while
leaving the existing arguments unchanged.

---

536-544: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Case 5 replaces the real exclusions instead of adding to them.

evaluate receives only the bogus entry, so the mechanism's real exclusions are dropped for this case. The assertion still passes because it only looks for STALE EXCLUSION. Merging the bogus entry into m.exclusions keeps the case closer to the real configuration.

♻️ Proposed change

         stale = evaluate(
-            Tree(root), m, exclusions={"no/such/file.ts": "deliberately bogus"}
+            Tree(root),
+            m,
+            exclusions={**m.exclusions, "no/such/file.ts": "deliberately bogus"},
         )

🤖 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_test_registration.py` around lines 536 - 544, Update Case 5 in
the test registration checks to merge the bogus “no/such/file.ts” entry into
m.exclusions before passing exclusions to evaluate, preserving the mechanism’s
real exclusions while still exercising stale-exclusion detection.

---

161-169: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Consider scoping the TOML key match to the entry tables.

The regex matches path/source at any position in the file, including unrelated tables. If a future table adds a path key, a dark probe can be masked as registered. The docstring justifies the line-oriented reader, so this is optional. One cheap hardening is to require the key inside the expected table by tracking the current [table] header while scanning lines.

🤖 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_test_registration.py` around lines 161 - 169, Update
_read_toml_paths to track the current TOML table while scanning the manifest and
only collect matching key values from the expected entry table, ignoring
identical keys in unrelated tables. Preserve the existing set[str] return type
and line-oriented parsing behavior.

---

429-436: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Reuse the registry read in the reverse check.

m.registered(tree) runs a second time here after Line 414. Store the first result and reuse it. The rust branch does not set it, so compute it once above the branch or keep a local variable.

🤖 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_test_registration.py` around lines 429 - 436, Store the result
of m.registered(tree) from the earlier registry-check flow in a local variable
and reuse it in the reverse-check block guarded by m.check_reverse and
m.entry_to_path. Ensure the value is initialized or computed once for both
branches, including the rust branch, while preserving the existing sorted
iteration and missing-path validation.

---

199-212: 🎯 Functional Correctness | 🔵 Trivial | ⚖️ Poor tradeoff

The ancestor walk can accept an unrelated mod declaration.

The loop searches for mod <stem> in every ancestor mod.rs or <dir>.rs up to the crate root. A declaration with the same stem in an unrelated ancestor module marks the file as registered. The docstring states that the wide walk is deliberate to avoid false positives, so this is a trade-off note, not a defect. If you want to tighten it later, verify that the matched declaration resolves to this file path, either through the inline module chain or through #[path].

🤖 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_test_registration.py` around lines 199 - 212, The ancestor walk
in the registration check intentionally allows matching mod declarations from
unrelated ancestor modules; preserve this behavior and update the surrounding
documentation or comment to explicitly describe it as a deliberate
false-positive trade-off. Keep the existing inline-module and #[path] handling
unchanged.

🤖 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 `@docs/src/testing/test-registration.md`:
- Around line 82-86: Update the sentence following the statement that the check
runs as a step in lint so it says no separate job or required context is added,
rather than claiming the step does not exist. Keep the surrounding explanation
about branch protection and the existing lint context unchanged.

In `@scripts/check_test_registration.py`:
- Around line 410-417: Update the `rust-test-modules` branch so `dark` excludes
entries present in `exclusions`, matching the non-Rust registration path. Ensure
`registered_keys` and the subsequent `n_registered`/`n_excluded` counts remain
consistent with the adjusted dark set, while preserving
`_rust_module_is_declared` behavior for non-excluded files.
- Around line 117-122: Define a MissingRegistry exception and update the
registry-reading flow used by evaluate to translate FileNotFoundError from read
into MissingRegistry containing the missing relative path. Catch MissingRegistry
around m.registered(tree), append the specified missing-registry problem using
m.registry, m.id, and the missing path, then continue with an empty
registered_keys set.

---

Nitpick comments:
In `@scripts/check_test_registration.py`:
- Around line 606-610: Update the ArgumentParser construction in main to use
argparse.RawDescriptionHelpFormatter via formatter_class, preserving the module
docstring’s layout in --help while leaving the existing arguments unchanged.
- Around line 536-544: Update Case 5 in the test registration checks to merge
the bogus “no/such/file.ts” entry into m.exclusions before passing exclusions to
evaluate, preserving the mechanism’s real exclusions while still exercising
stale-exclusion detection.
- Around line 161-169: Update _read_toml_paths to track the current TOML table
while scanning the manifest and only collect matching key values from the
expected entry table, ignoring identical keys in unrelated tables. Preserve the
existing set[str] return type and line-oriented parsing behavior.
- Around line 429-436: Store the result of m.registered(tree) from the earlier
registry-check flow in a local variable and reuse it in the reverse-check block
guarded by m.check_reverse and m.entry_to_path. Ensure the value is initialized
or computed once for both branches, including the rust branch, while preserving
the existing sorted iteration and missing-path validation.
- Around line 199-212: The ancestor walk in the registration check intentionally
allows matching mod declarations from unrelated ancestor modules; preserve this
behavior and update the surrounding documentation or comment to explicitly
describe it as a deliberate false-positive trade-off. Keep the existing
inline-module and #[path] handling unchanged.

🪄 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: 6b6f408a-5a51-47fd-a7dc-ac496e754d7b

📥 Commits

Reviewing files that changed from the base of the PR and between d2f712f and 95f58e2.

📒 Files selected for processing (9)

• .github/workflows/test.yml
• CONTRIBUTING.md
• benchmarks/compiler_output/workloads.toml
• changelog.d/7278-dark-test-registration-gate.md
• docs/src/SUMMARY.md
• docs/src/testing/test-registration.md
• scripts/check_test_registration.py
• test-features/feature_matrix.toml
• test-parity/gc_repsel_corpus.txt

🚧 Files skipped from review as they are similar to previous changes (7)

• test-features/feature_matrix.toml
• CONTRIBUTING.md
• benchmarks/compiler_output/workloads.toml
• test-parity/gc_repsel_corpus.txt
• changelog.d/7278-dark-test-registration-gate.md
• .github/workflows/test.yml
• docs/src/SUMMARY.md

[coderabbitai]

♻️ Duplicate comments (3)

scripts/check_test_registration.py (2)

117-122: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Handle a missing registry file with a clear failure.

read() calls Path.read_text directly: return (self.root / rel).read_text(encoding="utf-8", errors="ignore"). If a registry file is deleted or renamed, m.registered(tree) raises an unhandled FileNotFoundError, and the script exits with a traceback instead of naming the missing registry. The PR objectives list missing registry files as a validated condition, so this should be an explicit, named problem, not a crash.

🛠️ Proposed fix

     def read(self, rel: str) -> str:
         if rel in self.overrides:
             return self.overrides[rel]
         if rel in self.added:
             return ""
-        return (self.root / rel).read_text(encoding="utf-8", errors="ignore")
+        path = self.root / rel
+        if not path.is_file():
+            raise MissingRegistry(rel)
+        return path.read_text(encoding="utf-8", errors="ignore")

Define MissingRegistry and translate it into a problem inside evaluate() around the m.registered(tree) call.

🤖 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_test_registration.py` around lines 117 - 122, Define a
MissingRegistry exception and update evaluate() to catch FileNotFoundError
raised by m.registered(tree), translate it into a named validation problem that
identifies the missing registry file, and preserve the existing handling for
other errors.

---

410-417: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Exclusions are ignored for the rust-test-modules mechanism.

In this branch, dark is computed only from _rust_module_is_declared, and exclusions is never subtracted: dark = [k for k, p in sorted(keys.items()) if not _rust_module_is_declared(tree, p)]. If a contributor adds a Rust exclusion, the file stays reported as dark, so the documented second way out ("exclude it, with a reason") does not work for this mechanism.

🛠️ Proposed fix

     if m.id == "rust-test-modules":
-        dark = [k for k, p in sorted(keys.items()) if not _rust_module_is_declared(tree, p)]
-        registered_keys: set[str] = set(keys) - set(dark)
+        undeclared = [
+            k for k, p in sorted(keys.items()) if not _rust_module_is_declared(tree, p)
+        ]
+        registered_keys: set[str] = set(keys) - set(undeclared)
+        dark = [k for k in undeclared if k not in exclusions]
     else:
         registered_keys = m.registered(tree)
         dark = sorted(set(keys) - registered_keys - set(exclusions))

🤖 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_test_registration.py` around lines 410 - 417, Update the
rust-test-modules branch in the registration logic to remove keys listed in
exclusions when computing dark, while preserving the existing
_rust_module_is_declared check. Ensure excluded Rust test modules are not
reported as dark and continue contributing to the existing exclusion count.

docs/src/testing/test-registration.md (1)

82-86: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Fix the contradictory sentence.

This bullet states the check runs where it blocks. It is a step in lint, which is already a required context. The final sentence then states "The step does not exist here, so it cannot be forgotten," which reads as contradicting the first sentence. The intended meaning is that no separate branch-protection step is needed, not that the lint step itself is absent. Reword so the two statements agree.

📝 Proposed wording

-  branch protection is hazard 2, and `gc-root-dominance` sat red and blocking
-  nothing for days because of it. The step does not exist here, so it cannot be
-  forgotten.
+  branch protection is hazard 2, and `gc-root-dominance` sat red and blocking
+  nothing for days because of it. This gate adds no new job, so there is no
+  branch-protection step left to forget.

🤖 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 `@docs/src/testing/test-registration.md` around lines 82 - 86, Reword the final
sentence in the “It runs where it blocks” bullet so it clarifies that no
separate branch-protection job or step is needed, while preserving that the
check runs as a required step within `lint`.

🧹 Nitpick comments (1)

scripts/check_test_registration.py (1)

429-436: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Avoid re-reading the registry for the reverse check.

registered_keys is already computed via m.registered(tree) at Line 414 for non-Rust mechanisms. This loop calls m.registered(tree) again: for entry in sorted(m.registered(tree)):, re-parsing the same registry file. Reuse registered_keys instead of recomputing it.

     if m.check_reverse and m.entry_to_path is not None:
-        for entry in sorted(m.registered(tree)):
+        for entry in sorted(registered_keys):
             rel = m.entry_to_path(entry)

🤖 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_test_registration.py` around lines 429 - 436, Update the
reverse-check loop guarded by m.check_reverse and m.entry_to_path in the
registration validation flow to iterate over the existing registered_keys value
rather than calling m.registered(tree) again. Preserve the current sorting and
path-existence validation behavior.

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

Duplicate comments:
In `@docs/src/testing/test-registration.md`:
- Around line 82-86: Reword the final sentence in the “It runs where it blocks”
bullet so it clarifies that no separate branch-protection job or step is needed,
while preserving that the check runs as a required step within `lint`.

In `@scripts/check_test_registration.py`:
- Around line 117-122: Define a MissingRegistry exception and update evaluate()
to catch FileNotFoundError raised by m.registered(tree), translate it into a
named validation problem that identifies the missing registry file, and preserve
the existing handling for other errors.
- Around line 410-417: Update the rust-test-modules branch in the registration
logic to remove keys listed in exclusions when computing dark, while preserving
the existing _rust_module_is_declared check. Ensure excluded Rust test modules
are not reported as dark and continue contributing to the existing exclusion
count.

---

Nitpick comments:
In `@scripts/check_test_registration.py`:
- Around line 429-436: Update the reverse-check loop guarded by m.check_reverse
and m.entry_to_path in the registration validation flow to iterate over the
existing registered_keys value rather than calling m.registered(tree) again.
Preserve the current sorting and path-existence validation behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 83d30782-44ab-4a3a-a353-ce029a14b02a

📥 Commits

Reviewing files that changed from the base of the PR and between 95f58e2 and abce569.

📒 Files selected for processing (9)

• .github/workflows/test.yml
• CONTRIBUTING.md
• benchmarks/compiler_output/workloads.toml
• changelog.d/7278-dark-test-registration-gate.md
• docs/src/SUMMARY.md
• docs/src/testing/test-registration.md
• scripts/check_test_registration.py
• test-features/feature_matrix.toml
• test-parity/gc_repsel_corpus.txt

🚧 Files skipped from review as they are similar to previous changes (7)

• test-features/feature_matrix.toml
• benchmarks/compiler_output/workloads.toml
• docs/src/SUMMARY.md
• CONTRIBUTING.md
• test-parity/gc_repsel_corpus.txt
• .github/workflows/test.yml
• changelog.d/7278-dark-test-registration-gate.md

[coderabbitai]

🧹 Nitpick comments (3)

docs/src/testing/test-registration.md (2)

49-49: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use the same runner label as the script.

The script sets runner="scripts/gen_feature_matrix.py (feature-matrix.yml)" for feature-matrix-probes. This table says (feature-matrix). --list is described as authoritative, so match its text.

📝 Proposed wording

-| `test-features/feature_matrix.toml` | `test-features/probes/**/*.ts` | `scripts/gen_feature_matrix.py` (`feature-matrix`) |
+| `test-features/feature_matrix.toml` | `test-features/probes/**/*.ts` | `scripts/gen_feature_matrix.py` (`feature-matrix.yml`) |

🤖 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 `@docs/src/testing/test-registration.md` at line 49, Update the runner label in
the test-registration table for feature-matrix probes to exactly match the
authoritative value emitted by the script’s --list output:
scripts/gen_feature_matrix.py (feature-matrix.yml), replacing the current
feature-matrix text.

---

8-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Align the runtime claim with the script docstring.

This page states "about a fifth of a second". scripts/check_test_registration.py states "~1 second" in its module docstring. Pick one figure so the two sources agree.

🤖 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 `@docs/src/testing/test-registration.md` around lines 8 - 10, Align the runtime
estimate in docs/src/testing/test-registration.md with the “~1 second” figure
documented in the check_test_registration.py module docstring, updating only the
conflicting “about a fifth of a second” wording.

scripts/check_test_registration.py (1)

431-440: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Suppress the DARK TEST flood when the registry file is missing.

If m.registered(tree) raises MissingRegistry, registered_keys becomes empty. Every candidate then also appears as a DARK TEST line. For gc-repsel-corpus that is 45 or more extra lines on top of the one real problem. The exit status is still correct, so this is report quality only. Skip the dark scan when the registry is absent.

♻️ Proposed change

     else:
+        registry_missing = False
         try:
             registered_keys = m.registered(tree)
         except MissingRegistry as exc:
             res.problems.append(
                 "MISSING REGISTRY %s: mechanism %s reads it, but %s does not "
                 "exist. Restore the file, or point the mechanism elsewhere."
                 % (m.registry, m.id, exc.args[0])
             )
             registered_keys = set()
-        dark = sorted(set(keys) - registered_keys - set(exclusions))
+            registry_missing = True
+        dark = (
+            []
+            if registry_missing
+            else sorted(set(keys) - registered_keys - set(exclusions))
+        )

🤖 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_test_registration.py` around lines 431 - 440, Update the
MissingRegistry handling in the registration-check flow around
m.registered(tree) to record that the registry is unavailable, and skip the
subsequent dark-test calculation/reporting for that mechanism. Preserve the
existing missing-registry problem and exit-status behavior while preventing
candidates from being emitted as DARK TEST entries.

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

Nitpick comments:
In `@docs/src/testing/test-registration.md`:
- Line 49: Update the runner label in the test-registration table for
feature-matrix probes to exactly match the authoritative value emitted by the
script’s --list output: scripts/gen_feature_matrix.py (feature-matrix.yml),
replacing the current feature-matrix text.
- Around line 8-10: Align the runtime estimate in
docs/src/testing/test-registration.md with the “~1 second” figure documented in
the check_test_registration.py module docstring, updating only the conflicting
“about a fifth of a second” wording.

In `@scripts/check_test_registration.py`:
- Around line 431-440: Update the MissingRegistry handling in the
registration-check flow around m.registered(tree) to record that the registry is
unavailable, and skip the subsequent dark-test calculation/reporting for that
mechanism. Preserve the existing missing-registry problem and exit-status
behavior while preventing candidates from being emitted as DARK TEST entries.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 59958286-32d0-4449-bb69-d4097cea039a

📥 Commits

Reviewing files that changed from the base of the PR and between abce569 and f5f2a5a.

📒 Files selected for processing (9)

• .github/workflows/test.yml
• CONTRIBUTING.md
• benchmarks/compiler_output/workloads.toml
• changelog.d/7278-dark-test-registration-gate.md
• docs/src/SUMMARY.md
• docs/src/testing/test-registration.md
• scripts/check_test_registration.py
• test-features/feature_matrix.toml
• test-parity/gc_repsel_corpus.txt

🚧 Files skipped from review as they are similar to previous changes (7)

• docs/src/SUMMARY.md
• test-parity/gc_repsel_corpus.txt
• test-features/feature_matrix.toml
• .github/workflows/test.yml
• benchmarks/compiler_output/workloads.toml
• CONTRIBUTING.md
• changelog.d/7278-dark-test-registration-gate.md