chore(skills): probe distinctions and refusals; tighten reviews by OmarAlJarrah · Pull Request #1422 · dexpace/morphic

OmarAlJarrah · 2026-11-03T15:55:13Z

Summary

Four gaps in the skills, each found while a check or a review passed over a defect.

A distinction probed where nothing depends on it

Some checks tell two things apart by a property, such as the $id base two texts are read against. Such a check matters only where something depends on that property: a base matters only to a reference resolved against it. No rule in morphic-verifying-changes or morphic-reviewing-prs asked a change adding such a check to compile an input where nothing depends on the property. The guard rules (test-guards-fail-closed, build-guards-see-later-writes, split-guard-per-site) ask whether a guard catches what it targets, not whether it also fires where its premise has nothing to act on.

#1406 showed the cost. At its a4dda68, the $id base checks fired on any text under another $id, and this spec writes no $ref:

openapi: 3.1.0
info: {title: t, version: "1"}
paths: {}
components:
  schemas:
    S:
      type: string
      allOf:
        - $id: 'https://ex.test/a/w.json'
          minLength: 1
        - maxLength: 5

On main (268945f), S gets the constraints {minLength: 1, maxLength: 5}. At #1406's a4dda68 it gets {maxLength: 5}, the branch is kept whole under openapi:allOf/0, and an info says the branch "reads its $refs against another base", though it writes none. CI passed on a4dda68, and the mutants #1406 listed reddened tests. Neither could show the defect, because at a4dda68 every spec #1406 had added to test the checks wrote a $ref. The same build also moved to Unmodeled the description of a branch whose only $ref sat in properties, though a description reads against no base. A probe that only deletes the $ref would also pass a check that looks for a reference anywhere in the branch rather than in what it compares, so the rule asks for a second form. A later #1406 round found a third. At 1855a63, where both forms matched main, a branch whose not wrote an absolute $ref still went to Unmodeled with its description, though an absolute $ref names one schema under any base. So the rule also asks for the dependent made inert.

This change:

Adds distinction-where-nothing-depends to .claude/skills/morphic-verifying-changes/references/assertions-and-reach.md, beside test-guards-fail-closed. Compile a fixture in three forms, on origin/main and on the branch: with what depends on the property deleted, moved outside what the check compares, and made inert (an absolute $ref). The documents and diagnostics must match unless the change says why they don't, and test rows pin each form. Its example says text depends on a base only through a relative reference.
Says in the same rule where the probe's control comes from. The probe passes on main by design, so main-control ("keep only those that fail there") would throw it away. Its control is instead a build whose check fires without the dependency, which must differ (planted-control). For fix(compilers/openapi): judge union branches against the body #1406, a4dda68 is that build.
Makes code-before-claims in .claude/skills/morphic-reviewing-prs/references/review-probes.md ask for this probe in a review's first round, citing the rule, and adds fix(compilers/openapi): judge union branches against the body #1406 to its Why.

Three things are deliberately left as they are here:

morphic-verifying-changes/SKILL.md's claim → probe table gets no row, because its body is at the 500-word cap. The skill's References list already routes to assertions-and-reach.md, and code-before-claims, which the review's second pass cites, now names the rule.
text-decides-under-one-base in morphic-openapi-compiler/references/openapi-lessons.md is unchanged here. On main it trusts a textual "equal" only under one base, whether or not the text writes a reference, which is the over-firing this rule probes for. At f5db471, fix(compilers/openapi): judge union branches against the body #1406 rewrites it to trust text writing no relative $ref or $dynamicRef under any base, and to probe the $ref-deleted form against main. That fix belongs with the checks it describes. fix(compilers/openapi): judge union branches against the body #1406 also edits openapi-lessons.md, where this PR adds refusal-findings-in-its-table, but in other rules: at 7ffeebb, whose skills change is f5db471's, the two merge without conflict, and the skills lint is clean on the merge.
near-miss in the review probes is unchanged. It adds a keyword the issue's example does not expect, to test a predicate deciding whether a new path applies. This rule deletes, moves or neutralises what a check depends on, to see whether the check fires where it should not. They perturb the input in opposite directions, so neither covers the other.

Claims about an open PR (#1425)

The curating rule let a Why's evidence count only if an issue, a merged PR or a revision pins it, and the review's claim sweep refused "a count resting on an open PR". Neither covered a universal. This PR's first Why said "every spec testing #1406's $id base checks wrote a $ref". That held at a4dda68 and was false at #1406's 1855a63, whose fix adds a spec writing none (TestAllOf_ABranchWritingNoReferenceFoldsUnderAnyBase), and the claim sweep passed it.

morphic-curating-skills/SKILL.md now pins "any claim about a PR" the same way. The body stays at its 500-word cap through four rewordings that keep their meaning ("needs" for "depends on", "as an agent follows a summary, not the body", "takes a rule" for "takes a new rule", and "The gate does not run it" in place of naming the push checklist, which the shipping skill already does).
sweep-each-claim-class refuses "a claim resting on an open PR's head" in place of "a count resting on an open PR". A claim naming the revision it held at passes, and the sweep's command now asks for headRefOid as well as state.
The sweep found three more sites. The two fix(compilers/openapi): judge union branches against the body #1406 Whys this PR adds now name a4dda68. claims.md said a test's two CONNECTs are what "test(compilers/openapi): keep the external order oracle offline #1235 removes", and test(compilers/openapi): keep the external order oracle offline #1235 is open, so the clause is gone and the Why keeps its evidence, test: the external-corpus order oracle fetches example.com over the network #1146. The other lines citing test(compilers/openapi): keep the external order oracle offline #1235 describe review work that already happened or scope themselves in time, so no push to test(compilers/openapi): keep the external order oracle offline #1235 can make them false.

Neighbours found by act alone (#1426)

edited-rule-against-its-neighbours named the acts an edited rule governs and read the rules on the same acts. That is how this PR's first round missed text-decides-under-one-base: it governs the same construct, text compared under $id bases, through another act.

The rule now has a review name the constructs an edited rule names as well as its acts, with a keyword, a field and a check as examples, and grep for the rules on each. Its heading, Why and Probe say so. To stay within 150 words, its closing sentence now reads "A script finds a pair, not a conflict."
posted-findings had the same by-act test for a committable suggestion ("breaks no rule governing the same act") and now says "the same act or construct".

A refusal's findings and its source table (#1432)

A compile returning no Document has its findings held to SourceTable, which names the source and its overlay and no document a $ref read. Only compilers.Detection.Compile checks that, under the engine, so a test calling openapi.New().Compile never meets it. #1432 records a refusal on #1430's 185c594, made after the load phase, that passed every compile-level test. On the same commit, morphic validate fails with a broken contract on a document whose $ref reads one that draws a load finding.

A new rule, refusal-findings-in-its-table in morphic-openapi-compiler/references/openapi-lessons.md, says every finding of a refusal made after a $ref has read a document must index that table. It is scoped by that read rather than by a phase, since the read is what puts a finding outside the table: a refusal made in the load phase after one would break the same way. On main none does: the load phase refuses only in the steps up to building the model, before resolveRefs reads another document (steps in compilers/openapi/internal/load/phases.go), and a document past a budget fails the $refs naming it instead (docs/compilers/openapi.md#source-budgets). It says to probe it through the engine on a split document and to hold ir.FirstOutsideTable(diags, len(SourceTable(...))) false in a test. It is its own rule because the bound rule could not hold it under 150 words, and because a refusal that is not a bound would never reach a Probe inside the bound rule.
bound-outcome-from-the-rules points to it, and its scope moves from a phase, "load or binding", to a kind: a bound of a kind source-budgets or references holds, in whichever phase. At 185c594, fix(compilers/openapi): budget the copies each $ref lowers #1430's bound, MaxReferenceExpansion, is an openapi.Limits budget enforced in internal/lowering that joins docs/compilers/openapi.md#source-budgets, so the old wording would not have reached the change that showed the gap. A list of phases ("load, binding or lowering") would instead send to those two entries a lowering bound of a kind they do not hold, which docs/compilers/openapi.md keeps in its construct's entry: allof-redeclared-property judges a field's first 64 declarations, and in allof-disjoint-conjuncts a read of null past its budget reports nothing. The narrower wording dates from the skills' first import, and no issue records a decision behind it.
The openapi SKILL.md lists the new rule beside the bound rule.
The issue proposed listing "documents a $ref reads" among the classes code-before-claims says the corpus cannot contain. The corpus already holds such specs (git grep -lE '\$ref: *.?\./[^#]+\.(ya?ml|json)' 268945f4d -- testdata lists seven), so that list would be wrong. What it lacks is a refusal meeting a finding in one of them, and the new rule is where a change adding a refusal will meet that.

Closes #1420
Closes #1425
Closes #1426
Closes #1432

Test plan

A check that tells two things apart by a property, such as the `$id` base two texts are read against, matters only where something depends on that property, here a reference resolved against the base. No rule asked a change adding such a check to compile an input where nothing does, so fixtures that all held the dependency could not show the check firing without it, and mutants reddening tests said nothing about that input. Add `distinction-where-nothing-depends` beside `test-guards-fail-closed` in the verifying-changes assertions reference: delete the dependency from one of the check's fixtures, keep the property, compile it on main and on the branch, and require the same document and diagnostics. The probe passes on main by design, so its control is a build whose check fires without the dependency, which must differ. Point the review's `code-before-claims` probe at the rule.

The Why of `distinction-where-nothing-depends` said every spec testing #1406's `$id` base checks wrote a `$ref`. The tests #1406 added with its fix write none, so the sentence was false of the PR as it merges. Say instead that every spec wrote one while the checks fired on any text under another `$id`.

Deleting what a check depends on shows the check firing where nothing depends on it, but not a check that looks for the dependency anywhere in its input rather than in what it compares. Both forms show a defect at #1406's a4dda68: a type lost the `minLength` of an inline `allOf` branch writing no `$ref`, and the `description` of one whose only `$ref` sat in `properties`, though a description reads against no base. Ask for the second form too, the dependent moved outside what the check compares, and give the rule both symptoms as its evidence.

A Why could pin a count only to an issue, a merged PR or a revision, but nothing held a universal to the same, so "every spec testing #1406's checks wrote a `$ref`" went false as #1406 moved (#1425). The curating rule now pins any claim about a PR that way, and the claim sweep refuses one resting on an open PR's head. The two #1406 Whys this branch added now name a4dda68, and `claims.md` no longer says what the open #1235 removes. The neighbours review found rules by the act they govern, so a rule probing `$id` base checks was never read against `text-decides-under-one-base`, which governs the same construct (#1426). It now greps for the constructs an edited rule names too, and `posted-findings` holds a suggestion to both. A refusal's findings are held to `SourceTable`, which names no document a `$ref` read, and only `compilers.Detection.Compile` checks that, so a refusal made after the load phase can pass every compiler-level test and fail in the engine (#1432). A new rule, `refusal-findings-in-its-table`, says how to probe it. The bound rule points at it and now covers bounds in lowering, which is where the refusal that showed this sits, and the openapi skill lists it. The curating SKILL.md stays at 500 words through four rewordings that keep their meaning.

Deleting or moving what a check depends on misses a check that counts the dependent however it is spelled, though one spelling does not depend on the property: an absolute `$ref` names one schema under any base. At #1406's 1855a63, where both earlier forms matched `main`, an inline `allOf` branch under another `$id` whose `not` wrote an absolute `$ref` still went to `Unmodeled` with its `description`; f5db471 matches `main` there too. Ask for that third form, and say text depends on a base only through a relative reference. The heading said nothing depends on what the check compares, where the rule means that what it compares ignores the property; the heading and the pointer in `code-before-claims` now say so. The claim sweep's command also reads a PR's head, as #1425 proposed.

A refusal's findings fall outside its source table only once a `$ref` has read a document, since that document is the one entry the table lacks. The rule and the openapi skill's bullet said "after the load phase", which is where the refusal that showed this sits, but a refusal made in the load phase after such a read breaks the same way. Scope the rule, its heading and the bullet by the read instead.

The rule gave examples of the acts an edited rule governs but none of the constructs it names, so a reader taking "construct" for a spec keyword alone would not grep for a field or a check such as `SourceTable`. Name a keyword, a field and a check, and shorten the closing sentence to keep the rule within its cap.

Widening the rule from "load or binding" to "load, binding or lowering" reached the expansion budget that sits in lowering, but also sent every other lowering bound to `source-budgets` or `references`. Lowering keeps bounds those entries hold no kind of in the entry of the construct they bound: the redeclared-property fold judges 64 declarations, and a null read's budget in the disjoint-conjuncts check reports nothing. Scope the rule to a bound of a kind either entry holds, in whichever phase, which keeps the fixed scan bounds and the expansion budget and leaves those to their own entries.

OmarAlJarrah added 3 commits November 3, 2026 18:54

This was referenced Nov 3, 2026

skills: the claim sweep refuses a count resting on an open PR but not a universal #1425

Closed

skills: edited-rule-against-its-neighbours finds neighbours by act, not by construct #1426

Closed

OmarAlJarrah changed the title ~~chore(skills): probe a distinction where nothing depends on it~~ chore(skills): probe distinctions and refusals; tighten reviews Nov 3, 2026

OmarAlJarrah added 6 commits November 3, 2026 20:25

Merge branch 'main' into chore/skills-probe-premise-absent

2fdd23c

chore(skills): rewrap the claim sweep's line past 100 columns

ac97e22

OmarAlJarrah mentioned this pull request Nov 3, 2026

skills: the claim sweep reads a citation without the scope it is cited for #1441

Closed

Merge branch 'main' into chore/skills-probe-premise-absent

6002154

OmarAlJarrah merged commit d3ce8e8 into main Nov 3, 2026
4 checks passed

OmarAlJarrah deleted the chore/skills-probe-premise-absent branch November 3, 2026 18:25

Navigation Menu

Repository navigation

chore(skills): probe distinctions and refusals; tighten reviews - #1422

chore(skills): probe distinctions and refusals; tighten reviews#1422
OmarAlJarrah merged 11 commits into
mainfrom
chore/skills-probe-premise-absent

OmarAlJarrah commented Nov 3, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

OmarAlJarrah commented Nov 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

A distinction probed where nothing depends on it

Claims about an open PR (#1425)

Neighbours found by act alone (#1426)

A refusal's findings and its source table (#1432)

Test plan

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

OmarAlJarrah commented Nov 3, 2026 •

edited

Loading