v2 coverage audit: false positive on first(callable): Option, and no detector for a subclass that extends Registry but bypasses the base store

[jessegall]

Coverage audit of the v2 registry / invariant prophets against a real 885-file codebase

I ran the new root-cause/registry/invariant family (v2, feat/root-cause-precedence @ adfbd09, all 7
phases) over a snapshot of a real app (the jessegall/workflows package, 885 PHP files) plus minimal
isolated repros, to check the thesis "the prophets flag what opposes the invariant/registry architecture."

Overall: the family is in great shape — precedence/deferral works (the invariant root cause leads its
deferred symptom), and the exemptions are genuinely well-considered (finder prefixes, <thing>For<Other>
directional lookups, the own-?T-contract tail). Most of what looked like misses on first glance turned
out to be correct, intentional exemptions. Credit where due; this is a tight ruleset.

Two real issues remain — one false positive and one coverage gap — plus two minor notes.

---

1. False positive — RegistryReturnContractProphet flags first(callable): Option (a predicate scan) — SIN / Structural / commit-blocking

A registry base with a predicate finder:

abstract class Registry  // marked: extends/named Registry
{
    /** @return Option<TType> */
    public function first(callable $predicate): Option
    {
        return Option::first($this->all(), $predicate);
    }
}

is flagged as a registry-contract sin ("return T or throw, with a has() companion"). But first(callable $predicate) is not a key lookup — it's a predicate scan, inherently value-or-nothing, exactly like
search*. Returning Option is the correct design; throwing would be wrong. The allowlist
(FINDER_PREFIXES = ['find','search','try','lookup'] + *OrNull/*OrDefault/<thing>For<Other>) simply
doesn't anticipate first / a callable-predicate getter.

• Real evidence: Support/Registry.php:87 (public function first(callable $predicate): Option) — a
  blocking SIN on the canonical, correct base registry.
• Note: the package's own scaffolded Registry.stub happens to avoid this only because it ships
  without a first(); any registry base that adds first/firstWhere/a callable-predicate finder hits it.

Suggested fix: treat a method that takes a callable/predicate and returns Option as a scan (exempt),
and/or add first to the finder allowlist.

---

2. Coverage gap — a subclass that extends Registry but defeats the base store is flagged by nothing — architecture opposition, no detector

This is the case that started the audit. ResourceRegistry extends Registry<AssistantResource> but:

class ResourceRegistry extends Registry
{
    private array|null $resources = null;

    // overrides all() to read a PRIVATE store and never touches the base $items
    public function all(): array { return $this->resources ??= $this->build(); }
    // ...
}

Because all() is overridden to read $this->resources and the base $items is bypassed, the inherited
register() / registerMany() are dead — calling them writes to $items, which all()/find()/get()
never read. It "is a Registry" by inheritance but cannot be used as one via registration; it's really a
discovered catalog wearing the base. That directly opposes the registry architecture (the base's whole
mechanism is silently neutered), yet 0 prophets flag it (verified across RegistryReturnContract,
RegistryNamingHonesty, RegistryPattern, NoOptionToNull, PreferOptionOverNull).

Minimal repro (judge it — only nameFor gets flagged, for an unrelated reason; the override/bypass is invisible):

final class ThingRegistry extends Registry           // App\Support\Registry (scaffolded)
{
    private array $things = [];
    public function all(): array { return $this->things; }   // ← bypasses base $items; inherited register() is now dead
    public function keyForName(string $name): Option { /* ... */ }
}

Suggested detector: flag a subclass of a Registry (or any base) that overrides a method the base
implements in terms of a protected store and stops using that store, leaving inherited mutators
(register*) unreachable — i.e. "inherited base mechanism defeated / dead inherited mutator." (Generalizes
beyond registries.)

---

3. Discussion (not asserting a bug) — the exemption seam lets a built Option be collapsed to null

ResourceRegistry::findModelClass():

public function findModelClass(string $type): string | null
{
    return $this->find($type)->transform(fn ($r) => $r->modelClass())->getOr(null);
}

builds a real Option (via find()) and collapses it with getOr(null). This is exempt by every
relevant prophet, each defensibly:

• NoOptionToNullProphet — exempts the return …->getOr(null) tail as "the method's own ?T contract"
  (its own WHAT DOES NOT list, ~line 109);
• RegistryReturnContract / PreferOptionOverNull — exempt find* as a finder.

So a registry method that constructs an Option and immediately throws it away slips through. Per the strict
architecture ("don't unwrap a real Option back to null — map it or throw"), this opposes the model; per the
current exemptions it's intentional. Worth a decision: should the own-contract exemption still apply when
the collapsed value came from a constructed Option (a find()/transform() chain), vs. a bare nullable
passthrough? Flagging here as a judgment call, not a defect.

---

4. Minor UX — single-file judging under-reports (index-dependent prophets go silent)

judge --file=ResourceRegistry.php → "Righteous: No sins found." The same file inside a full-tree
judge surfaces findings, because the NeedsCodebaseIndex prophets (e.g. NoOptionToNull resolving that
find() returns Option) have no call graph to consult on a lone file. Devs (and the pre-commit
--staged/--git paths) judging a small file set can get false comfort. Worth either building a minimal
index for the file's referenced types, or a one-line "N index-dependent prophets limited in single-file
mode" notice.

---

Repro

# point a scroll at the target tree with the registry/invariant prophets enabled, then:
vendor/bin/commandments judge --no-cache
# (1) -> SIN: RegistryReturnContractProphet on a base with first(callable): Option
# (2) -> ThingRegistry / ResourceRegistry override+bypass: no finding

Happy to provide the full minimal repro project (scaffolded Registry/Option + the two repro classes) or
a PR for the first/predicate-scan allowlist fix.

[jessegall]

Thanks for the thorough audit — fixes up in #120.

1. False positive — fixed (v2.0.1). A getter taking a callable/Closure is now treated as a predicate scan (value-or-nothing, like search*) and exempted from the registry contract, in the marked path and the shared RegistryLeak (so markerless agrees); first also added to the finder allowlist. Your Support/Registry.php:87 first(callable): Option no longer fires. Side note you nailed: the test that "covered" this was asserting the bug (first(callable): Option should fire) — it's been corrected to a real key-lookup example, and a new test pins the scan exemption.

2. Coverage gap — closed (v2.1.0, new RegistryBaseBypassProphet). Flags exactly the ResourceRegistry/ThingRegistry shape: extends Registry + own array store + overrides all() without parent::all() + doesn't override register()/registerMany() → inherited mutators are dead. Stays quiet when the override delegates to parent::all() or the subclass feeds its own store via its own register(). Kept it registry-scoped for now (low FP); the fully-general "dead inherited mutator" version is a bigger inter-class analysis — happy to track that separately if you want it.

3. getOr(null) on a constructed Option in a finder — deliberate keep (for now). Agreed it's the judgment call you framed, not a defect. Distinguishing a built-Option chain (find()->transform()->getOr(null)) from a bare nullable passthrough would mean threading provenance through the exemption, with real FP risk, and the find* name already announces "absence is expected." Leaving it exempt; reasonable to revisit if it bites in practice.

4. Single-file under-reporting — clarification, not a silent index. judge --file does build the full-scroll codebase index (ScrollManager::judgeFile → codebaseIndexFor($scroll)), so NeedsCodebaseIndex prophets run with the full call graph on the targeted file. The difference you saw is reporting scope: --file/--staged/--git only surface findings located on the targeted files, so a finding in a consumer file isn't shown. If you have a case where the file's own finding is missed under --file but appears in a full-tree run, that's a real bug — drop the scroll config here and I'll chase it.

[jessegall]

Closing — the two actionable items are fixed in #120 (① false positive, ② new RegistryBaseBypassProphet), tagged v2.0.1 + v2.1.0 on the branch; ③ and ④ are answered above (deliberate exemption / scope clarification). Reopen if ④ turns out to drop a file's own finding under --file — I'd want the scroll config to repro. #120 is green and ready to merge whenever you want the fixes on main + Packagist.