Repository navigation
chore(skills): weigh designs, find every reader, gate subjects - #1403
Merged
Merged
Conversation
reuse-library-readings sent a comparison of two schemas to the model, whose Schema.IsEqual calls a schema writing dependentRequired, or any key the model has no field for, equal to one without it. A new rule, compare-raw-nodes, compares raw nodes instead, says a package calls a yml function nodeview lacks directly under its own archtest entry rather than through a forwarding wrapper, and says yml.EqualNodes reads `<<` as a plain key, so its "unequal" is trusted only where that errs safely or the comparison goes key by key through nodeview. Both library behaviours now sit in docs/parser-library.md, each pinned by a canary assertion: IsEqual under unmodeled-keywords and EqualNodes under merge-keys. every-reader searched for GetAllOf() alone and probed only a multipart body, so a change to how another keyword is read had nothing to run. Its search is now the changed keyword's getter or key outside the changed package, which for oneOf also finds the raw reads in bind, naming and oas, and its probe compiles the old and new spelling at every position. The skill's compile step now names validate, or 2>"$file", for reading diagnostics apart from the IR: under zsh, 2>&1 >/dev/null also pipes the IR, which repeats every diagnostic, so a count doubles.
A design review could pass a document while missing what decided its rules, and pass a mechanism without asking what it cost. design-rule-over-its-document listed only the inputs a rule names; it now lists them from their definitions, each field of every IR type the rule reads and each sibling section, with a minimal spec for a field the golden spec never sets. review-design-rules now writes its own program from the document and diffs it per item against the author's, since a rerun of the author's reproduces its numbers. A new design-review-matrix rule crosses each rule's inputs with its consumers and claims nothing is left only on a full matrix. Two new IR lessons: a mechanism a design specifies is prototyped against every requirement its layer's documents state, not only the one it is for, and a plan fact shaping a declaration reads the type's own definition, naming any distant fact it reads and the break that fact causes. review-own-fixes sends a fix that changes a mechanism through both before it is pushed. A new weigh-against-alternatives probe asks a review, before it judges a mechanism's details, what the change must deliver, which alternatives keep that, and at what measured cost, the planned work a mechanism constrains included.
The push checklist's subject check printed an over-length subject and exited 0, so a push chained to it went through, and it appended the PR number to every commit subject, flagging subjects a multi-commit squash never lands. subjects.py replaces the awk. It measures the title, and a branch's one commit, with the " (#N)" the squash appends, and every other commit subject without it; holds each to the Conventional Commits form, no final period and 72 characters; and exits 1 on a finding. The checklist now pushes behind it and the skills lint, the two checks whose every hit is fixed rather than justified, and CLAUDE.md's budgeting rule says which subjects count the number.
On a branch stacked on another PR, origin/main..HEAD also holds that PR's commits, so the subject check measures them as this branch's own, as the awk it replaced did. Say so where the range is chosen, naming #1341, which tracks it.
review-without-origin had a reviewer with no origin remote fetch main into a ref of their own and read origin/main as that ref in the push checklist. That worked while the checklist spelled origin/main inline, but subjects.py reads origin/main by name and exits 2 when main sits under any other ref. Fetching main as the remote-tracking branch origin/main lets the setup, the checklist and its scripts run unedited.
reuse-library-readings tells a reader to call the library for every reading it implements, which sends a schema comparison to IsEqual, the reading compare-raw-nodes rules out. Name the exception where the instruction stands.
The merge-keys canary held IsEqual reading a merged schema as its written twin with a bare require, so a library change there would fail without the message naming docs/parser-library.md#merge-keys that every other assertion in the canary carries. Assert it with that message.
This was referenced Nov 3, 2026
every-reader's probe now compiles a construct at every position that same-construct-at-every-position enumerates, but that list named neither items nor a request or response body, three of the positions a changed oneOf reading had to be compiled at. Name them, so each rule citing the list reaches them.
design-rule-over-its-document asks for each sibling section of a rule's document, but its probe listed only IR fields, so a skipped section passed it; it now greps the document's headings too. The IR skill's step for weighing designs now cites prototype-against-every-requirement, where an author choosing a mechanism reads, rather than only the reference summary.
gate-by-exit-status said the push runs behind every check but those with hits to justify, while the checklist reads git status and the merge-tree trial, which exit 0 whatever they find. Say the push runs behind the checks that exit non-zero on a hit.
The curating skill kept "a fact about one machine, shell or person" out of the skills without saying whether a shell's documented default is one. Read literally it refused the reason the OpenAPI compiler skill gives for avoiding 2>&1 >/dev/null, zsh's MULTIOS, which every zsh user meets. Keep out a fact about one machine or person, and let a shell's documented default stand where a command depends on it. The words come from the fresh-clone limit, whose "names no machine" the narrowed rule and the lint's machine check already hold, and from a wordier upkeep line.
The rule searches a keyword's getter and its key, but its Why gave evidence only for the probe, so nothing said why the key half matters: `oneOf` is read by key in three packages where `GetOneOf()` appears in none (#1397). The pathspec now names the excluded package from `compilers/openapi/internal`. #1406 carries the same rule for the same issue. The wording is now the same in both, so they merge cleanly in either order.
The probe's merged schema wrote only `type`, which a comparison reading the type keywords one at a time through `nodeview` reads alike whether or not its whole-node comparison expands `<<`. So the probe could not tell a comparison that misreads merges from one that does not. Writing `description` beside `type` does: on main, a `oneOf` branch restating a property through `<<` lowers `degraded_lowering`, where the same branch written out lowers `validation_only`.
A later round of the review that led to design-rule-over-its-document, run after listing every field the rules read, still found rules that missed an enumeration's values (the `any` primitive), a kind's own IR document (a tuple's `MinItems`), a section restated instead of cited (`ModelShape`), and a clause the count program never ran (#1402). The rule now lists each enumeration value, each kind's `docs/ir` page and each sibling section, cited rather than restated, and review-design-rules has the reviewer's program implement every clause. `go doc -all` replaces `go doc`, which elides an enumeration's values.
weigh-against-alternatives counted #1233's archtest rule at 479 lines, which that PR has since replaced, and design-review-matrix said #1329's review took eight rounds, which its later rounds outgrew. Each now cites the issue whose body records the count (#1401, #1402), as the curating rule asks of a count in a Why. The line the tracker search overran is reflowed.
gate-by-exit-status said the push "runs behind those exiting non-zero", where "those" reads as the checks whose hits are justified, such as the artifact grep, which exits 0 on a hit. It now says the checks. Step 4 of the IR skill sent every IR design through a rule that prototypes an emitter mechanism on its target's library; it now says emitter mechanism. The review skill's ranking paragraph, which ran to 149 characters on one line, is reflowed.
…eview-and-readers
This was referenced Nov 3, 2026
design-rule-over-its-document listed the enumeration values a rule's fields take. A rule reading a variant reads it through a TypeRef, whose fields (Target, Nullable, Inferred) take no enumeration, so the list never reached TypeKind, whose External value one of #1329's rules missed (#1402). The rule now names every TypeKind and enumeration value the fields reach.
same-construct-at-every-position kept the positions a schema is written at by hand. The list named allOf branches but not oneOf or anyOf ones, nor additionalProperties, prefixItems, patternProperties, not, if/then/ else or the media type's itemSchema, and every-reader's probe compiles a changed spelling at each position it names. The fields table in compilers/openapi/internal/bind/grammar.go already maps every key that holds a schema, or a map or list of them. The rule now reads its positions there, each written inline, as a $ref to a declaration and beside a $ref, and its probe greps them out.
Dropping feat, ci or perf from subjects.py's Conventional Commits form, or adding a type, passed every test, since no case wrote those types. The new test reads the list from CLAUDE.md and asserts the script accepts each type it names and no other. Removing ci from CLAUDE.md reddens it too.
review-own-fixes sent every review fix that changes a mechanism through two design-document rules, which a compiler fix has nothing to bring to; it now names a mechanism a design document specifies. And it said to push behind the push checklist, whose git push origin HEAD exits 1 from the detached worktree the review setup makes, so it names git push origin HEAD:<branch>.
The merge-keys entry said yml.EqualNodes "compares nodes as they are written". It follows aliases, so an alias and the node it names compare equal. Otherwise it compares each node's kind, tag, value and children in order, so besides a `<<`, two mappings writing the same keys in another order compare unequal, and so does one value spelled two ways, `~` and `null` among them. The entry now says so, and the merge-keys canary pins all three on parsed nodes: inverting any of the new assertions reddens it.
compare-raw-nodes sent a comparison to yml.EqualNodes, blaming its false
"unequal" on `<<` alone, or to a key-by-key read through nodeview.Pairs
and nodeview.Child. EqualNodes also tells apart reordered keys and two
spellings of one value, and a key-by-key read fixes only the top level:
{x: {<<: {a: 1}}} still compares unequal to {x: {a: 1}}. On main,
#1278's restated-branch check degrades a union whose branch writes the
body's property with its keys reordered or with 10.0 for 10, as it does
through `<<`.
annotation.RawFromNode already renders a node as the parser reads it,
merges at every depth expanded and keys sorted, and admission.sameText
compares schemas by it. The rule now points there, says a number
spelled two ways still differs, and says text alike names the same
schema only under one base URI (#1408). Calling a yml function nodeview
lacks directly is a reading rule, so it moves to reuse-library-readings
with its Why.
CLAUDE.md, commit-and-pr-message-form and subjects.py said a subject the squash replaces never lands. The repository's default squash body (COMMIT_MESSAGES) pastes every branch commit's message, subject included, though none of the last 40 merges kept it. So the subject never lands as the squash's subject, and where the body carries it, nothing is appended: the 72-character cap without the number still holds.
OmarAlJarrah
commented
Nov 3, 2026
| bound and dropped an `encoding` silently (#998). A changed `oneOf` reading was shown to hold by | ||
| compiling both spellings at each position (#1278), and that key is read where its getter is not | ||
| (#1397). | ||
| - **Probe.** Compile the old and new spelling at every position |
Member
Author
There was a problem hiding this comment.
issue: this Probe passes a planted defect in a changed $ref reading, because every spelling it compiles is read against one base URI.
Rule: CLAUDE.md, "Break what a test claims to catch". A reading of $refs or raw text also depends on the base an $id names, and no position changes that base.
Run: on #1406 at 6a00723, plant a base-blind comparison in admission.sameText (base == other || base != other). Then compile a restating oneOf branch at a component, a property, an items, a parameter, a request body and a response body, with morphic compile --verify, reading each openapi:oneOf reason:
A: {$id: 'https://ex.test/a/node.json', type: string}
B: {$id: 'https://ex.test/b/node.json', type: integer}
Other: {$id: 'https://ex.test/a/other.json', properties: {a: {type: object, properties: {x: {$ref: 'node.json'}}}}}
S:
$id: 'https://ex.test/b/s1.json'
allOf: [{$ref: 'https://ex.test/a/other.json'}]
oneOf: [{required: [a], properties: {a: {type: object, properties: {x: {$ref: 'node.json'}}}}}]| spelling, at all six positions | #1406 | planted |
|---|---|---|
no $id, both $refs '#/components/schemas/A' |
validation_only |
validation_only |
as above: relative $refs under two bases |
degraded_lowering |
validation_only |
the branch's $ref written https://ex.test/b/node.json |
degraded_lowering |
degraded_lowering |
In both builds each spelling gets one verdict at all six positions, so the Probe passes the defect. Only the last two rows show it: one schema, its $ref written relative under an $id and written absolute, gets two verdicts. This is #1408's mechanism. compare-raw-nodes covers it for comparing two schemas, but this Probe does not test for it.
Fix (a description, not a committable suggestion): this rule is at 127 words, so the axis may fit better in same-construct-at-every-position, whose positions this Probe compiles. Next to "inline, as a $ref to a declaration and beside a $ref", it would add: inside a schema whose $id names another base, with its $refs written relative and absolute, the two spellings getting one verdict.
Member
Author
There was a problem hiding this comment.
Fixed in 7351fcc, as suggested, in same-construct-at-every-position: next to writing the schema inline, as a $ref and beside one, the rule adds a schema under an $id naming another base, its $refs written relative and absolute, and its Probe diffs the IR and diagnostics per spelling. every-reader's Probe compiles at those positions, so it now meets the axis.
On main, #1408's spec shows the axis bites: S, its $ref spelled relative under two bases, lowers to one validation-only keyword, while T, the same schema spelled absolute, reports conjuncts that disagree.
prototype-against-every-requirement's probe grepped refine and contract alone, so a mechanism running in plan, emit or an operations document was checked against none of its own stage's requirements. It now greps the stage or domain the mechanism runs in and the README's rules for every stage; with refine it still finds the output-unit and mapped-type requirements #1329's decode mechanisms failed. same-construct-at-every-position's grep printed the parameter's schema key without the state that owns it, so it read like the header's. Three lines of context print every owner.
…eview-and-readers
The merge-keys entry and its canary had come to record how yml.EqualNodes compares nodes: following aliases, reading a `<<` as a key, and telling key orders and spellings such as `~` and `null` apart. Only the first of those concerns merges, and none is a behaviour the compiler depends on: docs/parser-library.md records what the compiler refuses input over or works around, and each entry names the code that exists because of it. No code names EqualNodes there. Its one caller, schema.restatesProperty, cites unmodeled-keywords, and the change that replaces its comparison removes it, so an entry naming it would fail the canary's code check once both land. The entry and the canary return to what merges do. compare-raw-nodes says in its Rule that EqualNodes reads nodes as written, with #1278's degraded unions as the evidence in its Why, rather than citing an entry for it.
every-reader excluded the changed package with ':!compilers/openapi/internal/<package>/*'. Without glob magic a pathspec's `*` also matches `/`, so the exclusion dropped every package nested under the changed one: a change to bind never searched bind/typed. Spelled ':(exclude,glob)...', the `*` stays within the directory, keeping bind/typed while still leaving out bind's own files. Closes #1410.
same-construct-at-every-position wrote the schema inline, as a $ref and beside one, all read against one base URI. A reading of $refs or raw text also depends on the base an $id names, and no position changes it, so a comparison blind to the base passed the Probe that every-reader sends a changed reading through: each spelling took one verdict at every position, in the build with the defect too. The rule now adds a schema under an $id naming another base, its $refs written relative and absolute, and the Probe diffs per spelling. On #1408's spec, S, its $ref spelled relative under two bases, folds into one validation-only keyword, while T, spelled absolute, reports conjuncts that disagree: one schema, two verdicts.
test_cannot_run_exits_two deleted origin/main before running the script with 0, so 0 exited 2 because git log failed, never reaching the guard that refuses a number below one: replacing the guard with `if False` left every test passing. The case now runs with origin/main in place and asserts the guard's message; dropping the guard, or loosening it to `< 0`, fails it. The script's docstring is rewrapped where an edit left one line at 111 characters.
An edit on this branch left "number." alone on a line before the sentence that follows it in the same paragraph.
prototype-against-every-requirement's Probe named the layers a mechanism may run in by hand: plan, refine, emit, operations and write-integrate.md. docs/emitters also holds docs-and-mock.md, surface-verification.md and packages.md, the first two listed in its README beside write-integrate.md, and testing/, whose mandatory tests bind every target's output whatever layer produced it: T-4 writes wire names, never rendered identifiers, and T-7 a diagnostic, never a silent `any` or a panic. The grep read none of them. `<layer>` is now each entry of `ls docs/emitters` the mechanism runs in, and the grep always reads docs/emitters/testing, beside the README and the contract.
The Why said each of #1329's review rounds two to eight found an input or a consumer's requirement no earlier round had listed. #1402, which it cites, says each round after the first found defects a rule should have caught in the first, in five groups, of which only the first two are inputs and requirements; it also records a round after the eighth. The Why now says what #1402 does, and pins no round count.
"Rule format" allowed a Why a count no command derives, "which its citation pins", without saying what pins one. An issue's body, a merged PR or a named revision does; an open PR does not, as what it holds keeps changing after a rule cites it. Two Whys on this branch quoted counts from #1233 and #1329 while both were open, and both moved on before either merged. The lint runs offline and cannot tell the two apart. "Rule format" now says what pins a count, and sweep-each-claim-class checks each cited PR's state with `gh pr view <N> --json state`. To fit their caps, the curating skill's fresh-clone and upkeep lines and the sweep's Why are worded more tightly, the Why keeping its claim that each sampled round left a misattribution unread. A sweep of every Why citing a PR open today found one more count resting on one: no-network-through-a-logging-proxy's two CONNECTs, from #1235's head. #1146's body records them, from a run on main, and the Why now cites it for the count. Closes #1409.
review-own-fixes gave a fix the review makes the passes "before it is pushed", and its Probe read the diff before the push. A review that posts a patch for the author to apply pushes nothing, so the rule read as not applying, and the patch went out with no reread of what it adds. On #1233 a posted redesign was applied verbatim, and a later round found five nits in the lines it added. The rule now covers each fix a review hands on, pushed, posted as a patch or offered as a suggestion, and gives its diff every pass, written rules included, before it leaves. Its Probe applies the fixes in a throwaway worktree before pushing or posting, and posted-findings' Probe sends each suggestion through it after the checks it touches. To fit the cap, the #1143 evidence keeps one of its three examples. Closes #1411.
sweep-each-claim-class's Why, tightened to fit its cap, said #1143's sampled rounds left a misattribution unread, though a later full read found it. It now says the misattribution sat unsampled, as the original evidence did: citations were read for 58 of 203 rules before all 383.
The curating skill sends a lesson a check can catch to a check, written or filed, and keeps a rule naming the filed issue until it lands. The subject rule had a script and no check: subjects.py runs only for whoever runs the push checklist, on a title typed into its command, so a title set or edited after the last push lands unmeasured, and a Dependabot pull request is never measured at all. The gate's pull_request runs could hold both, and #1417 now tracks that. commit-and-pr-message-form names #1417, and its Probe reads the title the PR holds with `gh pr view N --json title` rather than a typed copy. The Why's #1345 clause is worded more tightly to fit the cap.
CLAUDE.md measured a title with `printf ... | wc -c` and audited the history with `awk 'length($0) > 72'`. Both count bytes, wc always and macOS's awk at version 20200816, while the cap is in characters and subjects.py counts characters. At 72 characters, a title holding two arrows measures 76 under wc -c, and the audit counts 23 subjects over the cap on main where a count of characters gives 22: #5's 71-character subject, with two arrows, reads as 75. The measure is now subjects.py, and the audit pipes through `perl -CS -nle 'print if length > 72'`, which decodes UTF-8 and counts characters: on main it gives 22.
review-own-fixes gave each fix a review hands on every pass, and posted-findings sends each suggestion through it. For a fix that is posted rather than pushed, no CI run stands in for the gate pass, so a one-line suggestion called for a local `make gate` and a trial merge of every open PR. The rule now gives each fix the passes its own diff can affect, written rules included, as the review skill's seventh pass already scopes a rerun, so a one-line suggestion meets its written rules and the checks it touches, and a redesign meets every pass it reaches. posted-findings keeps pointing each suggestion there. The #1143 clause of the Why is worded more tightly to fit the cap.
The OpenAPI skill's summary and compare-raw-nodes' heading said two schemas are compared "by their raw nodes". yml.EqualNodes compares raw nodes too, and reuse-library-readings, on the line before, says to call the library's own reading, so the two read together point at the function the rule rejects, the one #1278's degraded unions came from. Both now say the schemas are compared by rendering their raw nodes.
compare-raw-nodes described how to compare two schemas, rendering both with annotation.RawFromNode, comparing the bytes and counting a failed rendering unequal, without naming the code that already does it. admission.sameText in the schema package renders each schema once a run, remembered on the lowering context, and guards a failed rendering, so a second comparison written from the recipe would repeat both and pay for a rendering per comparison. The rule now sends a comparison in the schema package through sameText, and one elsewhere renders each node once. The lint holds the name to what admission.go declares.
design-review-matrix asked a result or a reason for every cell of a rule's inputs crossed with its consumers, and allowed "nothing left" only on a full matrix. A rule reading Union, Model, Property and TypeRef has about 100 inputs: 48 fields, 11 TypeKinds, 27 PrimKinds, the kind pages and its document's sibling sections. Crossed with the 19 or so consumers the rule lists, that is near 1,900 cells for one rule, which invites the sampling the rule exists to stop. One reason may now settle a whole row or column, such as an input no consumer reads, and the Probe counts the cells settled either way.
The subject rule's Probe read the title with `gh pr view N`, but the rule governs the check before the first push too, when no PR exists: the command substitution then comes back empty, and subjects.py reports the empty title as off the form. The Probe now sends a branch with no PR to the push checklist, which takes the title as typed, and reads the PR's own title once it is open. The Rule and Why are worded more tightly to fit the cap.
every-reader searched with `git grep -nE 'Get<Keyword>\(\)|"<keyword>"'`. Filled with a keyword starting with `$`, the regex reads the `$` as an end-of-line anchor, so `"$ref"` matched no line: a change to how the schema package reads `$ref` found no reader outside it, where bind, load, oas, operation and auth read the key. `$id`, `$defs` and `$dynamicRef` matched nothing the same way. Capitalizing the keyword did not name the getter either: `$id`'s is GetID. The search now takes both patterns as fixed strings, with `-F` and an `-e` each, so nothing needs escaping, and names the getter as oas3.Schema declares it. Filled for `oneOf` and `allOf` it finds what the regex did, and for `$ref` and `$id` what the regex missed. The Rule and Why are worded more tightly to fit the cap.
subjects.py took the commits origin/main..HEAD holds as the branch's. Where origin/main lags a main the branch has merged, as after merging a fetched main without updating the ref, or after GitHub updates the branch, main's commits fall in that range. A one-commit branch then read as several, and its subject, which lands with the number, was measured without it: one landing at 74 characters passed. The script now reads HEAD's first-parent line back to origin/main, merges left out, so a commit a merge brought in is never counted, whichever ref it arrived through. A stacked branch that merged its base also stops counting the base's commits; one started on the base's tip still counts them (#1341). The only new difference from what GitHub counts goes the safe way: a feature branch merged into a branch on main is left out, so its one commit's subject may be measured with a number it will not get. A test merges main into a one-commit branch with origin/main left behind, and fails without --first-parent.
The subject rule's Probe passes an open PR's title as "$(gh pr view N --json title --jq .title)". When gh fails, as on the API's 504s, the substitution comes back empty, and subjects.py reported the empty title as off the Conventional Commits form, exiting 1, the code for a finding. A pull request cannot have an empty title, so the script now refuses an empty or blank one in its argument checks and exits 2, the code for a run that gives no verdict. A test holds both spellings; dropping the check, or testing the title without stripping it, fails it.
same-construct-at-every-position's Probe listed positions with `grep -nE -B3 '(: |= )sSchema'`, three lines of context showing the state that owns each key. sParameter's "schema" key sits exactly three lines below its owner, so one more field written before it would hide the owner again, as it hid it at -B2 before. The Rule now names the two declarations that hold the positions, `fields` and `schemaFields` in compilers/openapi/internal/bind/grammar.go, which the skills lint holds to what that file declares, and the Probe compiles at every position and spelling the Rule lists, with no grep standing in for reading them.
design-review-matrix listed the artifacts the plan feeds as "SDK, docs, mock". docs/emitters/docs-and-mock.md also plans a validation emitter, consuming the same document and plan, and the hand list left it out of every matrix. The rule now takes the SDK and the artifacts that document names. To fit the cap, a cell's result is said to come with "its run", and one reason settles "a row or column".
Three defects in one rule. Its Rule said how yml.EqualNodes reads nodes, a library behaviour with no docs/parser-library.md entry or canary, where CLAUDE.md asks a skill to cite an entry rather than restate one. The Rule now names EqualNodes only as a comparison not to use, and #1278's degraded unions in the Why carry why. It sent comparisons in the schema package through admission.sameText, which on main checks that two schemas share a document but not a base: #1408, the defect text-decides-under-one-base describes, sits in that comparator until its fix lands. The pointer now says #1408 tracks the base check. Its Probe compared `{description: d, type: string}` "with its plain spelling" without writing the spelling it is not: the merge the commit that added it described. The Probe now writes it through `<<`. Rendered with annotation.RawFromNode, both merged pairs equal their plain spellings, a key written twice fails to render, and a dependentRequired keeps two schemas apart, as the Probe says.
The push measured a title typed into the command, while the subject rule's Probe reads an open PR's own. A typed copy can differ from the title a squash will land. The block now takes the PR's title from `gh pr view N` when the PR is open, and the typed title when it is not yet, or when gh fails, so an outage leaves the push where it was rather than blocking it. The same section said the lint exits "on a name a skill uses that this branch leaves naming nothing". It exits 1 on any of its findings, word caps, citations and frontmatter among them, so the sentence now says so, keeping a dangling name as its example. To fit the body's cap, the section no longer restates where the squash appends ` (#N)`, which the rule it links states.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Skill fixes for nine issues, and for the two that #1402 folds in (#1356, #1345).
OpenAPI compiler skill
compare-raw-nodes(new, skills: reuse-library-readings sends a schema comparison to a reading blind to some keywords #1396).Schema.IsEqualreads the model, which has no field for some keywords, so two schemas are compared by rendering their raw nodes, asannotation.RawFromNoderenders them: merges expanded at every depth and keys sorted, asadmission.sameTextalready compares onmain. A rendering that fails, as on a key written twice, counts unequal to every other, since two such renderings are bothnilandbytes.Equalwould call them equal. It namesIsEqualandyml.EqualNodesonly as comparisons not to use:EqualNodeshas nodocs/parser-library.mdentry, since no compiler code depends on it once fix(compilers/openapi): judge union branches against the body #1406 lands, so the rule restates none of its behaviour and leaves fix(compilers/openapi): treat a restated union branch as bare #1278's degraded unions in its Why as the reason. In the schema package it sends a comparison throughadmission.sameText, which renders each schema once a run, rather than a second copy of the recipe, and names openapi: inline allOf branches under different $id bases fold a keyword they spell alike #1408 as tracking the base checksameTextlacks onmain; elsewhere it renders each node once. Its Probe writes the<<spelling of both merged schemas it compares. The heading and the skill's summary say "by rendering their raw nodes", sinceEqualNodescompares raw nodes too.text-decides-under-one-base(new). Any comparison of text decides only so far: alike text names one schema only under one base URI, since a relative$refresolves against it (openapi: inline allOf branches under different $id bases fold a keyword they spell alike #1408), and text apart can name one schema, as a$refand its target do, so a textual "equal" is trusted only under one base and its "unequal" only where a false one errs safely. A first draft blamedEqualNodeson<<alone and offered a key-by-key read throughnodeview.Pairsandnodeview.Child, which fixes only the top level.reuse-library-readings(skills: reuse-library-readings sends a schema comparison to a reading blind to some keywords #1396) says to call aymlfunctionnodeviewlacks directly, under the package's owninternal/archtestentry, rather than through a wrapper that only forwards, and that two schemas are compared ascompare-raw-nodessays.docs/parser-library.mdnow records underunmodeled-keywordsthatIsEqualcompares the model alone, so a schema writing such a keyword compares equal to one that does not, pinned by a canary assertion, and the skill cites the entry.EqualNodesgets no entry: the page records what the compiler refuses input over or works around, each entry naming the code that exists because of it, and its one caller,schema.restatesProperty, is deleted by fix(compilers/openapi): judge union branches against the body #1406, so an entry naming it would fail the canary's code check once both land.every-reader(skills: every-reader's search and probe reach only an allOf in a multipart body #1397). The search is now the changed keyword's getter or key, outside the package that changed, and the probe compiles the old and the new spelling at every position, with the multipart body kept as theallOfinstance fix(compilers/openapi): flatten a nested inline allOf #998 found.same-construct-at-every-position, whose positions that probe compiles at, now names the declarations that hold them,fieldsandschemaFieldsincompilers/openapi/internal/bind/grammar.go, every key mapping to a schema or a map or list of them, which the skills lint holds to what that file declares. Its hand list had namedallOfbranches but notoneOforanyOfones,additionalProperties,prefixItemsoritemSchema. Beside writing a schema inline, as a$refand beside one, it now writes it under an$idnaming another base, its$refs relative and absolute, and diffs per spelling: under one base, a comparison blind to the base passed the Probe (openapi: inline allOf branches under different $id bases fold a keyword they spell alike #1408). Searching keys as well as getters matters: foroneOf,GetOneOf()appears only in the schema package, while"oneOf"is also read inbind/grammar.go,naming/steps.goandoas/table_keywords.go. The rule's Why now says so, citing skills: every-reader's search and probe reach only an allOf in a multipart body #1397. The search takes both patterns as fixed strings,git grep -nF -e '<Getter>()' -e '"<keyword>"', so a keyword starting with$matches: as a regex,"$ref"reads$as an anchor and matched none of its readers. It excludes the changed package as':(exclude,glob)compilers/openapi/internal/<package>/*', so a package nested under it, such asbind/typedunderbind, is still searched (skills: every-reader's search skips a package nested under the changed one #1410).morphic validate --verify, which writes no IR, or kept beside the IR with2>"$file". The skill now warns off2>&1 >/dev/null | grep, which zsh'sMULTIOSalso feeds the IR into.Design reviews (
morphic-changing-the-ir,morphic-reviewing-prs)design-rule-over-its-document(skills: a design review samples a rule's inputs and consumers instead of listing them #1402). A rule's inputs are listed from their declarations: each field of every IR type it reads, everyTypeKindand enumeration value those fields reach (go doc -all ./ir <Type>, since plaingo docelides the values; a rule reading a variant through aTypeRefmeets no enumeration among its fields, and docs(emitters): hold every variant an anyOf value admits #1329's missedKindExternal), each kind'sdocs/irpage, and every sibling section (grep -n '^#' <document>), cited rather than restated, each with its effect on the rule. An input the golden spec never sets gets a minimal compiled spec, not a zero count. The enumerations, kind pages and restated sections are what a later review round on docs(emitters): hold every variant an anyOf value admits #1329 found after every field had been listed, recorded in a comment on skills: a design review samples a rule's inputs and consumers instead of listing them #1402.review-design-rules(skills: a design review samples a rule's inputs and consumers instead of listing them #1402) points at the rule below and still runs each sample as printed.program-from-the-document(new, skills: a design-rules review can rerun the author's program and find nothing #1356, skills: a design review samples a rule's inputs and consumers instead of listing them #1402). The reviewer implements every clause of the rules from the document before reading the author's program, runs both, and reads each item they disagree on against the document. "Every clause" answers skills: a design review samples a rule's inputs and consumers instead of listing them #1402's comment, where the author's count program had skipped a clause an earlier round added. Each factirorir/irfactsderives is read through its exported function, from a scratch module whosego.modreplaces this repository's module with the tree under review, as skills: a design-rules review can rerun the author's program and find nothing #1356's later comment asks: a flattening written by hand miscounted there.measure-the-premisecounts through the same derivations, since that miscount was a premise measurement.design-review-matrix(new, skills: a design review samples a rule's inputs and consumers instead of listing them #1402). Each rule's inputs are crossed with its consumers, and "nothing left" is claimed only on a full matrix. Its artifacts are the SDK and thosedocs/emitters/docs-and-mock.mdnames, a validation emitter among them. One reason may settle a row or column, such as an input no consumer reads: a rule readingUnion,Model,PropertyandTypeRefhas about 100 inputs, which against the 19 or so consumers the rule lists is near 1,900 cells.prototype-against-every-requirementanddeclaration-reads-its-definition(new IR lessons, skills: a design review samples a rule's inputs and consumers instead of listing them #1402). The first's Probe greps each entry ofls docs/emittersthe mechanism runs in, with the README, the contract anddocs/emitters/testing, whose mandatory tests bind every target's output whatever layer produced it. The IR skill's step for weighing designs cites the first for an emitter mechanism.review-own-fixessends a fix that changes a mechanism a design document specifies through the first and its matrix row, and pushes withgit push origin HEAD:<branch>, since the review setup's worktree is detached and the checklist'sgit push origin HEADexits 1 there.review-own-fixes(skills: review-own-fixes reads as covering only fixes a review pushes #1411) covers each fix a review hands on, pushed, posted as a patch or offered as a suggestion, and gives its diff each pass it can affect, written rules included, in a throwaway worktree before it leaves, so a one-line suggestion meets its written rules and the checks it touches rather than a localmake gate.posted-findings' Probe sends each suggestion through it.weigh-against-alternatives(new, skills: a review never weighs a design against the alternatives it gave up #1401). Before judging a mechanism's details, a review names what the change must deliver, probes its alternatives onmainand weighs their cost, the planned work the mechanism constrains included. Pass 2 of the review skill now runs it. Its Why cites skills: a review never weighs a design against the alternatives it gave up #1401 for the size of test: give every covered child its own coverage directory #1233's archtest rule, which test: give every covered child its own coverage directory #1233 has since replaced, anddesign-review-matrixcites skills: a design review samples a rule's inputs and consumers instead of listing them #1402 for each round after the first finding defects a rule should have caught, as skills: a design review samples a rule's inputs and consumers instead of listing them #1402 words it, with no round count, so neither rests on an open PR's current state.Push checklist (
morphic-shipping-prs, #1402, #1345)scripts/subjects.pyreplaces the awk subject check. It measures the title, and a branch's one commit, with(#N), measures every other commit subject without it, and exits 1 on a finding. The checklist now pushes behind it and the skills lint, the two checks whose every hit is fixed rather than justified. The artifact greps are still read line by line. A script rather than a longer awk: the gating awk took 65 words of aSKILL.mdbody already at its 500-word cap, and the which-subject-lands logic gets tests. A test holds the types it accepts to the listCLAUDE.mdgives, one the refusal of a number below one, and one an empty or blank title, refused with exit 2, since a failedgh pr viewpasses one. The push block takes the open PR's own title fromgh pr view N, and the typed one before the PR opens or whenghfails. The push section says the lint stops it on any finding, not only on a name left naming nothing.review-without-originnow fetchesmainas the remote-tracking branchorigin/main, the name the setup, the checklist andsubjects.pyread. Under its old wording,mainin a ref of the reviewer's own, the script exits 2.commit-and-pr-message-formandCLAUDE.md's paragraph on budgeting the appended number now say which subjects count it, andgate-by-exit-statussays the push runs behind the checks that exit non-zero on a hit. A replaced subject is said to land, if at all, in the body, where nothing is appended: the repository's default squash body (COMMIT_MESSAGES) pastes every branch commit's message, though none of the last 40 merges kept it.commit-and-pr-message-formnames ci: hold the PR title and commit subjects to the message rules #1417, and its Probe reads an open PR's own title withgh pr view N --json title; a branch with no PR yet takes the push checklist, since the command substitution comes back empty there.CLAUDE.mdmeasures subjects withsubjects.py, which counts characters, where itsprintf ... | wc -ccounted bytes: a 72-character title holding two→measured 76. Its history audit pipes throughperl -CS -nle 'print if length > 72'rather thanawk, which on macOS also counts bytes: onmainit counted 23 subjects over the cap where there are 22, reading refactor(compilers)!: rename frontend→compiler and backend→emitter #5's 71-character subject as 75.Curating skill (#1404, #1409)
sweep-each-claim-classchecks each cited PR withgh pr view <N> --json state. A sweep of every Why citing a PR open today found a third count resting on one,no-network-through-a-logging-proxy's twoCONNECTs from test(compilers/openapi): keep the external order oracle offline #1235's head; test: the external-corpus order oracle fetches example.com over the network #1146's body records them, and the Why now cites it. To fit the 500-word cap, the fresh-clone and upkeep lines are worded more tightly.Limits
origin/main, merges left out, so a commit a merge brought in, frommainor a merged stacked base, is not counted even whereorigin/mainlags it. A branch started on another PR's tip still counts that PR's commits as its own, as the awk did; the docstring says so, naming skills: the push checklist reads a stacked branch's base commits as its own #1341. The one new difference from GitHub's count goes the safe way: a feature branch merged into a branch onmainis left out, so its one commit may be measured with a number it will not get.prototype-against-every-requirement's Probe lists candidate requirements, not a complete set, and the rule says so.review-without-origin, stays open: this PR changes only what that rule fetches.review-without-originnames the remote-tracking branch rather than spelling the fetch command. That is noted on skills: the lint reads a grep pattern holding a slash as a missing path #1317, which tracks the same reading.Closes #1402
Closes #1401
Closes #1398
Closes #1397
Closes #1396
Closes #1404
Closes #1356
Closes #1345
Closes #1409
Closes #1410
Closes #1411
Test plan
python3.14 .claude/skills/morphic-curating-skills/scripts/skills_lint.pyis clean at each commit of the branch, each checked out on its own.python3.14 -m unittest discover -s .claude/skills/morphic-shipping-prs/scriptspasses. Each of nine defects planted insubjects.pyreddens a test: the number on every commit, on none, exit 0 on a finding, merges counted, the title measured bare, no period check,>=at the cap,searchformatch, and exit 1 when it cannot run. Thesearchplant survived the first draft of the tests, which now hold a case for it, and so did dropping the guard refusing a number below one, or loosening it to< 0, since the test of0ran withorigin/maindeleted and exited 2 on git's failure; it now runs withorigin/mainin place and asserts the guard's message. Droppingfeat,ciorperffrom the form, or adding a type, survived too until a test held the types toCLAUDE.md's list; it now reddens on each, and onciremoved fromCLAUDE.md. Changing the one-commit test== 1to<= 1is equivalent: with no commit, the comprehension never reads it.go test ./compilers/openapi/internal/canary -run 'TestLibraryBehaviour/(merge-keys|unmodeled-keywords)$'passes, and inverting the newIsEqualassertion reddensunmodeled-keywordswith the entry's message.merge-keysand its canary matchmain.compare-raw-nodes: on parsed nodes,yml.EqualNodessays "unequal" for{x: {<<: {a: 1}}}against{x: {a: 1}}, for keys in another order, for10.0against10,1e1against10,~againstnullandTrueagainsttrue, and "equal" for an alias against what it names.annotation.RawFromNoderenders each pair alike except the two number spellings. Onmain, fix(compilers/openapi): treat a restated union branch as bare #1278's restated-branch check, which compares withEqualNodes, lowers aoneOfbranch restating the body's propertydegraded_loweringwhen it writes the property with its keys reordered, with10.0for10, or through<<on either side, where the branch written alike lowersvalidation_only. fix(compilers/openapi): judge union branches against the body #1406 at f13367f, which replaces that comparison, lowers eachvalidation_only. A branch addingdependentRequiredlowersdegraded_loweringonmain.every-reader: bare and restatedoneOfbranches at a component, a property,items, a parameter, a request body and a response body. Before fix(compilers/openapi): treat a restated union branch as bare #1278 (438bebe20^) every bare branch lowersvalidation_onlyand every restated onedegraded_lowering; onmainall twelve lowervalidation_only. The search filled forallOffindsoperation/multipart.go, the reader fix(compilers/openapi): flatten a nested inline allOf #998 fixed. Filled forbind, the:(exclude,glob)exclusion keepsbind/typed's 2 files, where:!kept 0, and still leaves outbind's own.testdata/conformance/openapi/unhomed-keywords.yaml,grep -c "kept verbatim"counts 42 behind2>&1 >/dev/nullunder zsh, 21 under bash, 21 fromvalidateand 21 from2>"$file".compile --verifyandvalidate --verifywrite byte-identical stderr with equal exit codes for every filegit ls-files 'testdata/*.yaml' 'testdata/*.json' | grep -v golden.jsonlists.design-rule-over-its-document:grep -n '^#'over docs(emitters): hold every variant an anyOf value admits #1329'sdocs/emitters/plan/types.mdat6ad8bb65lists its visibility sections, an input a later round found;go doc ./ir Propertyandgo doc ./ir Modellist the fields, and docs(emitters): hold every variant an anyOf value admits #1329's member rule at6ad8bb65names none of them: a grep of it for the fields that later decided it finds only "Flatten", inside "Flattened".go doc -all ./ir PrimKindlists all 26 primitive kinds, theanyprimitive among them, wherego doc ./ir PrimKindprints one and an ellipsis.compare-raw-nodes, a failed rendering:{type: string, type: integer}and{minimum: 1, minimum: 2}each rendernilwith an error, andbytes.Equalcalls the two equal; a$refand the schema it names render apart.text-decides-under-one-base: onmain, openapi: inline allOf branches under different $id bases fold a keyword they spell alike #1408's spec foldsS's twonot: {$ref: n.json}branches, spelled alike under two$idbases, into onevalidation_onlyentry, and keepsT's, one spelled absolutely, apart.program-from-the-document: a scratch module requiringgithub.com/dexpace/morphicand replacing it with this tree builds with a plaingo build, decodesmorphic compile's document fortestdata/golden/openapi/hint_positions.yamlintoir.Document, and callsir.FlattenedPropertiesandirfacts.Newacross its 102 types.prototype-against-every-requirement: with<layer>each entry ofls docs/emittersbar the README,contract,targets,testing,milestones.mdandopen-questions.md, and<target>go, the grep prints candidate lines for each, the mandatory tests' T-4 and T-7 among them, and withrefinethey include the import rule indocs/emitters/refine/output-units.mdanddocs/emitters/targets/go/types.md#what-a-mapped-type-must-do.weigh-against-alternatives: the tracker search ontoolchainreturns docs/emitters: a verify stage that compiles and lints generated output #704.declaration-reads-its-definition, applied to docs(emitters): hold every variant an anyOf value admits #1329's member rule: at3e41f8d8eit reads "every shape the union travels in", a fact of the operations, and the document names no break, so the Probe reports a finding; at81ab49bb7the document names a PATCH body as the one distant operation it reads and says a change breaks the union's callers, so it passes.compare-raw-nodes, the comparator: namingsameTextXbesideadmission.gomakes the skills lint report that the file declares no such name.commit-and-pr-message-form's Probe, on this PR: clean on the title it holds. Run for a number with no PR, its first draft reported the empty title as off the form.design-review-matrix,review-design-rulesandprogram-from-the-document's comparison prescribe a report, not a command, and this PR reviews no design document to build one for. skills: a design-rules review can rerun the author's program and find nothing #1356's runs, whichprogram-from-the-document's Why cites, are the case it describes.... && echo pushed, and this branch was pushed behind it; it also stopped two of this branch's commit subjects, at 74 and 73 characters, before they were pushed. It runs from a subdirectory too.gh pr list --state merged --limit 60, each PR's non-merge commits from the API, againstgit log -1 --format=%s <merge commit>). Among the last 300, only two can tell a one-commit PR's subject from its title: test(ir/irverify): make the sort test reach its Path tiebreak #1232 and fix(engine)!: keep compiler diagnostics and exit on spec problems #335, each one commit plus a merge ofmainunder a title that differs, and both landed under the commit's subject. The other 73 one-commit PRs had a title equal to the subject.same-construct-at-every-position, the$idaxis: onmain, openapi: inline allOf branches under different $id bases fold a keyword they spell alike #1408's spec lowersS, its$refspelled relative under two bases, to one validation-only keyword, and reportsT, spelled absolute, as conjuncts that disagree: one schema, two verdicts, which the Probe's per-spelling diff shows.same-construct-at-every-position:fieldsandschemaFieldsingrammar.gohold the schema-bearing keys, among themoneOf,anyOf,additionalProperties,prefixItemsand the media type'sitemSchema, none of which the hand list it replaces named. NamingschemaFieldzthere makes the skills lint report that the file declares no such name.every-reader, filled with each keyword's getter and key outside the schema package:$ref23 lines and$id6 as fixed strings, where the regex form matched 0 of each;oneOfandallOffind what the regex did,operation/multipart.goamong them.subjects.py: a one-commit branch that merged amainnewer than itsorigin/mainref passed a subject landing at 74 characters; with--first-parentit is reported, and a test holds it, failing without the flag. An empty or blank title exits 2; dropping the check, or testing the title unstripped, fails its test.gh pr view; run for a number with no PR, it takes the typed title.compare-raw-nodes' Probe, throughannotation.RawFromNode:{x: {<<: {a: 1}}}and{<<: {description: d, type: string}}each equal their plain spellings, keys written in another order compare equal, a key written twice fails to render, and addingdependentRequiredkeeps two schemas apart.design-rule-over-its-document, forTypeKind:go doc ./ir TypeRefshowsTarget,NullableandInferredand no enumeration, andgo doc -all ./ir TypeKindlists its 11 kinds,KindExternalamong them.review-own-fixes: in a repository with a bare remote,git push origin HEADfrom a detached HEAD exits 1, andgit push origin HEAD:<branch>updates an existing branch.review-without-origin: in a freshgit initwith no remote,subjects.pyexits 2 withmainfetched intorefs/review/mainand 0 with it fetched intorefs/remotes/origin/main, andgit for-each-reflists both fetched refs.admission.sameText, whichcompare-raw-nodesnames. At 0a1215f, each of the other 26 open PRs merged with this branch conflicts in no file it does not already conflict in withmain. Trial merges at 07be505: feat(ir)!: mark inbound operations and give a path its own docs #1366 and feat(irfacts): derive a correlation key for every entity #1354 conflict withmainalone, and this branch adds no conflicting file to either. ci: run coverage in set mode and -race on concurrent packages #1311 merges cleanly andgo test ./compilers/openapi/internal/canarypasses on the merge. ci: move the fuzz search from pull requests to a nightly job #1382, test: move fuzz targets to packages that link no test tooling #1381, ci: reuse test results across checkouts, keep main's cache warm #1373 and test: give every covered child its own coverage directory #1233 merge cleanly.make gateis left to CI.