Repository navigation
chore: measure premises and fix what reviews found - #1169
Conversation
A run of proposed rules shows they are total, never that their premise holds. Sharpen the step that told a design change to weigh two options, add rules for counting what a design assumes and for weighing outside the issue's framing, and give a normative document a review pass that can disagree with it.
The priority order sat in the Go style section, so a design or a document was outside it, and the design skill opened on scale and simplicity alone. State it where every session reads it, with what it asks: the right result for a consumer on real input, shown before anything else is weighed.
A lint that finds no skill printed "clean (0 skills)" and exited 0, so a pass over nothing read as a pass. Reviewing an old revision with today's script reaches that. It now exits 2, as it does for anything else that leaves it unable to check, whether the directory is missing, empty or holds no skill. A one-word span was read as written, quotes included, so a quoted path that exists was reported missing, while the same word in a command passed. Both sites now read a word through path_word, so the two readings cannot drift, and the optional "$" in GOROOT_SRC goes, as no word reaches it with one.
is_spec looked for a version key first on a line, so a spec written as
a one-line flow mapping was left out of the corpus. A sweep run to check
a change to format detection then shared that change's blind spot: a
sniff of a bounded prefix once refused such a spec past the prefix, and
the sweep could not have seen it.
YAML can also write the key after an anchor, a tag or a "? ", so the
reader now takes one anywhere in the file rather than special-casing
"{". A file naming one in a comment is compiled too and fails alike on
both sides, which costs less than a spec left out: over testdata the set
of specs is unchanged, and over a corpus of about 1,950 it gains two
such files and loses none.
JSON too deep to parse raised RecursionError, which ended the whole
sweep over one file. It is now read as text, as malformed JSON is.
Nothing said which YAML version's scalar resolution the compiler follows, and it follows neither: 0644 is 420 and 1_000 is 1000 as in 1.1, while 0o17 is 15 and yes, on and 1:30 stay strings as in 1.2. That is a behaviour of the parser, so it is an entry here, with a canary that goes red when a library change moves any of those readings, and the two functions that rest on it cite it. The values rule points at it rather than restating it. No output changes.
Twelve reviews of past PRs each noted where a skill was wrong, unclear or silent. This places each note where the curating skill says a lesson goes: a sharper existing rule before a new one. New rules: an argv differential for a change under cmd/, what the order oracle compares, a stage after the compile priced from a saved document, CPU time where no instruction counter exists, the toolchain a tree names, what barrier.Catch reaches, JSON null into a non-pointer, a heuristic default run over the golden spec, the tracker searched for what a rule settles, and an ignore rule probed at base and head. resolve-to-true-target returns, from the catalog the skills were built from. Sharpened: check-stays-silent-on-valid-input for a shortcut reader, character-predicate-witness-per-class for a redefined predicate, split-guard-per-site for an effect turned into a return value. Corrected: the push checklist's artifact grep leaves go.sum out, since every h1: hash matched it, and the curating skill no longer says a push chains on the lint. The scope rule's Why drops a go.mod clause no cited PR shows, and cites #324 for the restored ignore entry. Two design rules said a caller found what a reviewer's question led to. CLAUDE.md says the order oracle reads only what diffOrderInvariants names, so a clean run says nothing of a fact held on an operation.
every-return-path-delivers-diagnostics already owned the case of a diagnostic turned into a return value, so the memo lesson goes there and split-guard-per-site points at it, rather than holding a second copy. The order oracle's rule names the other way a clean run says nothing: a spec using YAML anchors is not permuted (#403). "No stand-in" is scoped to a claim about a spec, which measure-before-quantifying's benchmark is not, and resolve-to-true-target says what #108 hoisted, a schema.
A note in annotation.go said Site.Referent is exactly one hop and warned against passing the wrong resolution, while the Site comment says it is the end of the chain. The note was the false one: both callers take it from resolve.Scope.TargetSchema, so there was no choice to get wrong. A chain with a nearer hop writing a description shows it: the property takes the end's. The note now says so and names #1029, which is about that reading. The design record said the site model is built on one-hop resolution. That held when it was written, and the annotation package has resolved no reference since #1016, so the sentence moves to the past tense. Closes #1171
Each part of the sweep's version-key pattern now has a case: a word character or a dot before the key, a bare key at the end of the file, a space before a quoted key's colon and a quoted swagger key. So do the comma and semicolon path_word drops. The multiline flag goes, as the pattern has no anchor left for it to change. A key spelled through an escape or a folded line is still missed, though the compiler reads it, and the pattern's comment now says so. A test named a compiler's sniff of a prefix, which no longer exists, so it names what it holds. The note in annotation.go said what a hop "before" the referent writes lends nothing, which could be read to take in the use site, whose own keywords win. It now says a schema between them. The rest is wrapping, and a rule that cited one PR twice.
The lint held a path to the tree and a test to its declaration, but a line writing `Name` in `path/file.go` was held only as far as the path: the name could be renamed or moved out of the file with nothing saying so. A pointer at code to copy is worth nothing once it has gone stale, so such a line now has to name something the file declares, a mention in a comment not counting.
The skills mostly record what went wrong. The org's styleguide also says how to think before anything does, and most of that reached the skills only as failures to avoid, so this puts four habits where a decision is made, rather than copying its rules. CLAUDE.md now gives the order style concerns resolve in, which was written nowhere here, and says consistency is the last tiebreaker and never overrides the others, since matching the surrounding code is the easiest defence of a bad pattern. morphic-designing-code opens on the styleguide's stance, and two rows bring questions to design time: what would make a function wrong, and what a path every input takes costs. morphic-measuring-performance fires only once a change is tuned or a cost claimed, which is late. Each row of its table names code to copy, as the styleguide opens each chapter on an exemplar and nearly every rule here is drawn from a failure. Review pass 3 questions any added line nobody can explain. These are lenses and not rules: they have no Probe, and no check can tell whether one was applied. morphic-designing-code's reference line also gains the barrier.Catch rule it left out.
tracker-on-what-a-rule-settles said #601 and #630 never appeared in #1167. That held at edf4cf9, which #1170 quotes, and not at the merge, whose body and emitter design name both. The Why now says the PR named neither until a later revision. The lint's docstring gains the form the name-in-file check reads, as it fires on that spelling alone, and loses a broken wrap. A stray blank line inside MainTest goes too.
A merge can leave a sentence true of the PR it cites only as that PR once stood. A rule's Why here went stale that way: it said #1167 left two issues unnamed, and #1167 named both before it landed. Nothing in the merging rules asked for such a sentence to be read again, so this adds a rule whose Probe finds each one.
Two rules can each hold against the tree and still tell a reader opposite things, and no review pass read them side by side. This adds that reading as a class of claim. Run over every rule this branch adds or edits, it found one such pair: the maintained-count rule's Probe said a commit body stays out because a merge sets its own, beside a rule asking that every commit body could stand on main. The Probe now leaves a commit body to that rule.
CLAUDE.md makes the styleguide binding, and most of its rules name review as how they are enforced, while the review's written-rules pass read only the excerpt in CLAUDE.md. morphic-checking-style reads the guide's chapters against the lines a change adds, and lists where this repository departs from the guide with what holds each difference. The org's plugin stays uninstalled. A clone cannot declare it, since .claude holds only the skills, its rule against recursion forbids what the lowering pins, and its last step passes a tree below the coverage gate. A sentence in the pass alone would reach nobody writing Go, and the departures would still need a home. The tree also breaks rules nothing records a decision on, which a separate issue asks to have ruled on.
fuad-daoud
left a comment
There was a problem hiding this comment.
Thanks, Omar. This closes most of #1162 well, and putting a check before a rule where one fits (items 1–3, 10) is better than what the issue proposed. Two issues and three nits below; the issues block Closes #1162 and the lint check's purpose respectively.
Scope. I reviewed through 69f37dbb and re-checked every finding below on 64b18771, where each still applies. ddf7cc23 and 64b18771 themselves (the edited-rule rule and morphic-checking-style) are not reviewed yet; I'll follow up on them. No gate rerun: the head contains main and CI passed on it.
#1162, item by item. Where the issue gave a probe, I reran it on the head.
| # | Verdict | What showed it |
|---|---|---|
| 1 | Fixed | fresh git init: base clean (0 skills) exit 0, head exit 2 |
| 2 | Fixed | 15c2ca10^..15c2ca10: 55 hits, 0 with ':!go.sum' |
| 3 | Fixed | issue's probe [False, True] → [True, True]; sweep.py -b af1aae93: 213 specs, none differ |
| 4, 18 | Fixed | #324's branch: a89252c4 removes .claude/, 16bcd3e2 restores it |
| 5 | Different, reason holds | go test -bench reports wall time over its own fixtures |
| 6 | Fixed | at 325d69c8, 4 tests fail under go1.27.3 and pass under the rule's command |
| 7 | Fixed | morphic -- compile at 15c2ca10 prints help and exits 0, where its parent compiles |
| 8, 13, 14, 15 | Fixed | read against the cited PRs; 15 by reading #212's code only |
| 9 | Fixed | 70 KB one-line flow spec: exit 2 at 40f12412, 0 at its parent |
| 10 | Different, reason holds | compiled values match the entry; each of the 8 rows, changed, reddens the canary |
| 11 | Fixed | json/v2 decodes null into a string as "" with no error |
| 12 | Fixed | the rule's jq on the golden spec: all 5 x-sunset values are dates |
| 16 | Not fixed | see the comment on derive-never-maintain.md |
| 17 | Declined, reason holds | the three tests exist; dropping eol=lf reddens both TestGitAttributes tests |
The other closing keywords hold: #1161's own probe no longer reports the quoted paths, a three-hop chain compiles as #1171's new comment says, and #1165, #1166, #1168 and #1170 get what they ask for.
| over comments and docs; derive or delete each hit. A commit body is left to | ||
| `.claude/skills/morphic-shipping-prs/references/commits-and-prs.md#commit-and-pr-message-form`, as | ||
| a merge sets its own (#1162). |
There was a problem hiding this comment.
issue: item 16 is declined because a stale count in a commit body "cannot land", but it can: gh api repos/dexpace/morphic --jq .squash_merge_commit_message → COMMIT_MESSAGES, so a default squash pastes every branch commit's body. The recent merges' one-line PR: <url> bodies come from the merger's habit, not a setting. commit-and-pr-message-form says the same ("Every commit body could stand on main, since the default squash pastes them all"), as does CLAUDE.md, so this sentence now tells a reviewer to skip what those rules require to be right, and commit-and-pr-message-form's own Probe checks subjects, length and artifacts, not counts.
On this branch's own bodies, git log --format=%B af1aae93..HEAD | grep -cE '<this Probe's pattern>' gives 34 hits, so the probe has work here too. The fix is the issue's direction, and it lints clean:
| over comments and docs; derive or delete each hit. A commit body is left to | |
| `.claude/skills/morphic-shipping-prs/references/commits-and-prs.md#commit-and-pr-message-form`, as | |
| a merge sets its own (#1162). | |
| over comments and docs, and the same grep over `git log --format=%B origin/main..HEAD`, since | |
| the default squash pastes every branch commit's body; derive or delete each hit (#1162). |
Closes #1162 is right once this lands.
| def declares(source: str, name: str) -> bool: | ||
| """Report whether Go source declares name: a function or method, a type, a constant or a | ||
| variable, or a member of a group or struct, which a tab opens.""" | ||
| pattern = rf"^(?:func (?:\([^)]*\) )?|type |const |var |\t){re.escape(name)}\b" | ||
| return re.search(pattern, source, re.M) is not None |
There was a problem hiding this comment.
issue: the \t alternative accepts any statement in a function body, not just a member of a group or struct, so a name that a file only calls passes as declared there. That is the staleness this check exists for: a function moved to a sibling file with a call left behind.
$ python3.14 -I -c 'import sys; sys.path.insert(0, ".claude/skills/morphic-curating-skills/scripts"); import skills_lint as s
src = "package ir\n\nfunc f() {\n\tGone()\n\tLocal := 1\n}\n"
print([s.declares(src, n) for n in ("Gone", "Local")])'
[True, True]The test has a row for a comment mention but none for a statement; a row with func f() {\n\tCalled()\n} and `Called` in `ir/b.go` fails on this code. Holding the tab alternative to the inside of a block fixes the class:
# A const, var or type group, or a struct or interface type, to the parenthesis or brace closing it.
GO_BLOCK = re.compile(r"^(?:(?:const|var|type) \(|type \w+(?:\[[^\]\n]*\])? (?:struct|interface) \{)\n"
r"(.*?)^[)}]$", re.M | re.S)
def declares(source: str, name: str) -> bool:
"""Report whether Go source declares name: a function or method, a type, a constant or a
variable, or a member of a group, struct or interface."""
n = re.escape(name)
if re.search(rf"^(?:func (?:\([^)]*\) )?|type |const |var ){n}\b", source, re.M):
return True
return any(re.search(rf"^\t{n}\b", body, re.M) for body in GO_BLOCK.findall(source))On 64b18771 with this applied: a call and a := are refused; fields, const-group members, interface methods and a generic struct's fields are still accepted; test_skills_lint passes and the lint stays clean (9 skills).
| response, needs its own two-order diff, however many specs reach the sweep, and a spec using YAML | ||
| anchors is not permuted at all (#403). |
There was a problem hiding this comment.
nit: "not permuted at all" holds only where reversing would lift an alias above its anchor. reverseMappings declines only when the reversed tree no longer parses, so an anchor and alias that share a sequence are permuted: tags: [&t {name: a}, *t] and allOf: [&x {...}, *x] both come back permuted, while schemas: {A: &a {...}, B: *a} does not.
| response, needs its own two-order diff, however many specs reach the sweep, and a spec using YAML | |
| anchors is not permuted at all (#403). | |
| response, needs its own two-order diff, however many specs reach the sweep, and a spec is not | |
| permuted at all where reversing its mappings would lift an alias above its anchor (#403). |
| document, and takes a key and its value from one target. #1029 is open on what each hop lends a | ||
| use site. |
There was a problem hiding this comment.
nit: "#1029 is open" is an issue's state in prose; it goes stale the day #1029 closes and nothing reads it. This is the only such statement the diff adds (is open|stays open|still open|until #).
| document, and takes a key and its value from one target. #1029 is open on what each hop lends a | |
| use site. | |
| document, and takes a key and its value from one target. What each hop lends a use site is | |
| #1029. |
| self.report(f, i, f"docs/{doc}.md has no entry {key}") | ||
|
|
||
| def check_span(self, d: Path, f: Path, line: int, tok: str) -> None: | ||
| tok = path_word(tok) |
There was a problem hiding this comment.
nit: rebinding tok here makes the "does not exist" message name the normalized path rather than what the author wrote: `$GOROOT/src/nope/x.go` is reported as GOROOT/src/nope/x.go does not exist, and `$WT/ir/walk.go` as WT/ir/walk.go. Both are paths nobody wrote; the test's expectations at lines 189, 197 and 200 were edited to drop the $ to match. absent's docstring already says it names the path "as shown".
Fix: keep the written form beside the normalized one, e.g. shown, tok = tok, path_word(tok) here and shown, word = word, path_word(word) in the command loop, give missing a shown: str = "" parameter, and call self.absent(shown or p, rel). Then only those two expectations change back.
The lint's name-in-file check took any tab-indented line for a declaration, so a name a file only called passed as declared there, which is the staleness the check exists for. A member is now looked for inside its const, var or type group, or its struct or interface, and the struct header takes any generic, a constraint such as ~[]byte included. The maintained-count Probe set commit bodies aside because a merge sets its own body. A merge only usually does: the squash default is COMMIT_MESSAGES. It now reads them too, and a false count in a pushed body goes right in the squash body at merge, since a pushed branch is never rewritten. A span's missing path is reported as it was written again, which reading spans through path_word had changed. A command's word is reported as before. The order-oracle rule said a spec using anchors is not permuted, where only one whose reversal lifts an alias above its anchor is not, and a rule stated an issue's state in prose.
The departures list called these rules undecided, which stopped being true once each was ruled on. Assertion helpers, type aliases at a boundary and Example functions before a stable API stand as departures, with their reasons, and gosec stays off: it gave 174 findings, 16 in production, and none of those a defect. The rest are breaches, each with an issue of its own, so a new line is held to the rule and not reported as a question. The ruling on assertion helpers rests on t.Helper(), which about half of them lack; that is a breach of a rule of its own, and is filed apart.
Every import of the parser's oas3 package aliased it to its own name, which the styleguide's rule 2.23 rules out: an alias needs a conflict or a mismatch. revive's redundant-import-alias now refuses the shape, with a plant to show it reaches.
Fifteen packages kept their comment in another file, against CLAUDE.md and the styleguide's rule 7.9, and revive's package-comments says only that a comment exists. Each moves unchanged, and openapitest's TB leaves its doc.go for a file of its own. TestPackages_KeepTheirCommentInDocGo now holds every package to a doc.go of its comment and clause alone.
The styleguide's rule 5.26 asks for compliance to be asserted at compile time, and the tree's only assertions sat in a test file. An exported type implementing an exported interface now carries one beside its imports. An unexported implementation stays out: the call passing it fails the build, which is why the rule was taken this narrowly.
CONTRIBUTING.md asks a function that asserts on a test's behalf to call t.Helper, except one a table runs as a case's body, which is the case and reports its own line, and it says nothing checks that. thelper cannot: it reads a signature, so it reported 298 functions, every one of them a table's case, a callback or a function that asserts nothing. The check is a direct call instead, and the tree breaks it nowhere. An earlier commit here calls about half the assertion helpers a breach for lacking t.Helper. That read thelper's count as a fact and was wrong.
The styleguide's rule 11.9 puts a license header on every source file, enforced in CI, and five of the six sibling repositories sampled carry one. goheader now holds it, with a plant to show it reaches. The wording is the org's Go SDK's. The holder is the one this repository's LICENSE and README name, dexpace, where that SDK's header adds a person. Testdata stays as it was, bar the plants goheader reads.
The rules the tree broke no longer stand as breaches, so they leave the list: import aliases, doc.go and license headers are held by a check, and compile-time assertions stay narrow by a rule of their own. Only http.DefaultClient remains, which is a network read's time bound and not a matter of style. The list also said an assertion helper with no t.Helper breaks rule 6.3. It does not, where a table runs it as a case's body: CONTRIBUTING.md records that exception, and thelper, which cannot see it, is why the claim was made.
A published code-review plugin was read against the reviewing skill for what it holds that the skill lacks. Three things are new. A finding is attacked before it is reported, its rule read for scope and exceptions and a tool's hits sorted, which would have stopped this branch filing 298 functions as a breach that none of them was. A review reads what already stands on a PR. A posted finding links by full SHA, and carries a suggestion only where applying it alone is safe. Where a principle opposed a rule here, each side had a point, so the rules keep the better part of both rather than pick a winner. A blind first read stays, and the PR's stated intent then tests each finding. The bar of nothing left to find stays, and a rank says what was shown, so that a defect shows on its input and what is only likely is a question. A defect a change exposes stays in scope, and one outside its mechanism is filed. Reading standing reviews waits for the passes, as a list read first sets the agenda, which the edited-rule check caught. The same check now says to settle any such pair by keeping the best of both. No text was copied. morphic-checking-style also reads CONTRIBUTING.md now, which is where the exception it missed sits.
|
Thanks. Took all five; here is what differs from your suggestions.
The other two went in as you suggested. Since you read it, the branch has grown, so please read it by commit. |
The rule ranking a finding let it give a command or quote a rule, where the skill's own Findings section asks for both, and left a question with no command at all. It now asks for both, and a question says what was tried. The narrow rule on interface assertions said every exported interface. Taken at its word that is false, as thirteen exported methods satisfy error or fmt.Stringer with no assertion. It means an interface this module exports, as the issue it came from did. The rest is wording that overclaimed: every Go file, where testdata holds 42 without a header; a recipe naming make lint for what archtest now catches; "the one rule counted", beside 5.28; and two comments in the helper check.
Summary
This gives a design something that can disagree with it, fixes what reviews of past PRs found wrong
or missing in the skills, and brings the tree to the styleguide rules it broke.
Read it by commit.
315b0fa4adds the license header to every Go file outsidetestdata, with the lintthat holds it, and the sweeps before it are each one mechanism.
A design's premise
A design change has no output to compare, so the checks that catch a defect in code can pass while
the design's premise is wrong. #1167 showed it.
morphic-changing-the-irfired and its step said"weigh two designs", which was followed: the issue's option was weighed against one alternative, and
a program ran the proposed rules over the golden spec and the corpus. Every failure was placed, and
the design was still built on a fact nobody had counted. It kept one error type per body, but most
operations with several error bodies use one type for all of them, so the status is what tells
failures apart. A reviewer's question led to that count, not a check.
That program could not have disagreed: it assumed the premise it was run to support. So this adds
what can:
measure-the-premise(morphic-changing-the-ir): write the job a consumer does and the factabout real input the shape rests on, and count that fact before choosing. Running the rules shows
only that they are total.
weigh-the-framing(morphic-changing-the-ir): weigh a design outside the issue's framing, andcall a choice forced only after naming what the comparison held fixed and changing it.
review-design-rules(morphic-reviewing-prs): a normative document has no output for thebehaviour pass, so read its rules against each other and against another protocol, run them with
a planted defect, check each premise, and run its samples as printed.
tracker-on-what-a-rule-settles(morphic-reviewing-prs, in the claims pass): search thetracker for each construct a rule decides, and require every open issue on it to be closed by the
change, cited as left open, or answered.
The failed step is sharpened rather than given a copy: "weigh two designs" becomes two steps pointing
at the new rules, and
morphic-designing-codegains a row, so a shape outside the IR reaches themtoo.
It also states that correctness comes first, in a new
CLAUDE.mdsection. The priority order was inthe Go style section alone, so a design or a document was outside it, and
morphic-designing-codeopened on scale and simplicity without naming correctness.
CLAUDE.mdis the home rather than a newskill because a skill loads on a trigger and this has to hold always.
No test or lint can tell whether a premise was examined, or an option shown correct, so these are
rules and not checks. They raise the odds; they do not guarantee.
What reviews of past PRs found
Each item of #1162 went where
morphic-curating-skillssends it, a check before a rule and a sharperrule before a copy, which the issue's "each with a Rule, Why and Probe" skipped.
skills_lint.pyexits 2 on a tree with no skill, rather than printingclean (0 skills)go.sumoutsweep.pyreads a version key anywhere in a YAML filediff-contains-only-in-scope-filesintoignore-rule-both-ways, which cites #324 for the restored entry and probes an added pattern toocpu-time-without-a-counter: CPU time of alternated runs, not the benchmarks the issue proposed, which report wall clock and read their own fixturestoolchain-the-tree-namesargv-differentialcatch-reaches-one-goroutine, citingbarrier/doc.goandgoroutine-per-noderather than restating themcheck-stays-silent-on-valid-inputsharpened: a shortcut reader that cannot decide falls back to the full readdocs/parser-library.mdentry,scalar-resolution, with a canary, since it is a library behaviour and not a skill's to statenull-decodes-to-zeroheuristic-default-on-goldencharacter-predicate-witness-per-classsharpened: a redefined predicate is rechecked at each call site, since #189 shows no upper and lower pairresolve-to-true-targetrestored from the catalog the skills were built from, with its probe extended to annotationsevery-return-path-delivers-diagnosticssharpened, as it already owned the class;split-guard-per-sitepoints at itno-maintained-counts-or-state-in-prose's Probe also reads commit bodies, as the default squash pastes them; a false count in a pushed body is fixed in the squash body at merge, since a pushed branch is never rewrittenItem 3 is wider than the issue asked. YAML can also write a version key after an anchor, a tag or a
?, so the reader takes one anywhere rather than special-casing{. A file naming one in a commentis compiled too and fails alike on both builds, which costs less than a spec left out. A key spelled
through an escape or a folded line is still missed, though the compiler reads it; only a YAML reader
would see it, the standard library has none, and the pattern's comment says so.
The other issues
is no longer reported missing.
cost-of-a-later-stage, for a stage a compile's own spread would hide.edited-rule-against-its-neighbours, a class of claim for a change to the skills orCLAUDE.md: name the acts each edited rule governs, find the other rules governing them, and readeach pair. A script can find the pair but not tell a conflict from a restatement.
order-oracle-reads-what-it-compares, and a sentence inCLAUDE.md. Both point atdiffOrderInvariantsrather than list what it compares, which has already moved since the issuewas written, when servers joined.
The styleguide's way of thinking
The skills mostly record what went wrong. The org's styleguide also says how to think before anything
does, and most of that reached the skills only as failures to avoid. This puts four habits where a
decision is made, rather than copying its rules, which
CLAUDE.mdalready carries:CLAUDE.mdnow says clarity, simplicity, concision,maintainability, consistency, which was written nowhere here, and that consistency never overrides
the others, since matching the surrounding code is the easiest defence of a bad pattern.
morphic-designing-codeopens on the stance beside it: data and functions, immutable, explicit,errors handled.
every input takes costs, as
morphic-measuring-performancefires only once a change is tuned or acost claimed.
drawn from a failure.
skills_lint.pynow holds a Go name placed in a file to what the filedeclares, so an example cannot go stale unseen.
These are lenses and not rules: they have no Probe, and no check can tell whether one was applied.
Checking a change against the styleguide
CLAUDE.mdmakes the guide binding, and the review's written-rules pass read only the excerptCLAUDE.mdcarries. Each of the guide's 215 rules ends in an Enforcement line: 187 name reviewand 104 name nothing else, so no tool holds about half of it (styleguide @591e5243,
grep -h '^\*\*Enforcement:\*\*' go/[0-9]*.md | grep -ci review). #1172 asked how to check a changeagainst it:
.gitignorekeeps everything under.claudebut the skills out, so a clone cannot declare it. Installed, its rule 9 forbids the recursionTestLoweringRecursion_IsOnlyTheKnownCyclespins, and its last step passes a tree below the coverage gate.morphic-checking-stylelists the rule headings of the chapters a change calls for, and reads arule's body only where a line looks like a breach: the chapters run to about 32,000 words, and an
exception sits in a rule's body, never its heading. The review's pass takes it, and
CLAUDE.mdnamesit.
references/departures.mdholds each departure with what holds it. The issue named four, and readingthe chapters against the tree added five: chapter 01's linter set, a source's dates kept as text, a
stage that neither logs nor returns a spec problem as an error, the IR's embedded shapes, and
make gateas the finish.Counting the tree also found rules it breaks with nothing recorded either way, and #1173 put each to
a ruling. Assertion helpers (6.4), type aliases at a boundary (5.9) and Example functions before a
stable API (11.5) are departures, and
departures.mdgives each reason.gosecstays off: it gave 174findings, 16 in production and none of those a defect. The rest were breaches, and this brings the tree
to each, one commit an issue:
oas3to its own name lose the alias, and revive'sredundant-import-aliasrefuses the shape.doc.go, test(archtest): nothing holds a package comment to doc.go, and 15 packages keep theirs elsewhere #1174. Fifteen packages move their comment unchanged,openapitest'sTBleaves itsdoc.go, andTestPackages_KeepTheirCommentInDocGoholds every package to the rule.carries one beside its imports. An unexported one stays out, as the call passing it fails the build, and
interface-assertions-stay-narrowsays so.testdataopens with one, as do the plants bar theone that shows the rule reaches, and
goheaderholds it. Five of six sibling repositories sampled carry a header. The wording isgo-sdk's, and the holder is the oneLICENSEand the README name,dexpace, where that header addsa person.
t.Helper(), test: nothing holds an asserting test helper to t.Helper, and thelper cannot #1178, was no breach.CONTRIBUTING.mdalready exempts a function a table runs as acase's body, and
thelper, which reads a signature, cannot see that: of the 298 functions it reports,294 are such cases, 2 are callbacks and 2 assert nothing. So
thelperstays off, andTestTestHelpers_StartWithHelperchecks the rule by use, whichCONTRIBUTING.mdsaid nothing did. Apushed commit body here,
1773af4a, calls these a breach and is wrong; it should not reachmain.Only
http.DefaultClientremains inrules-the-tree-still-breaks, as its fix is #1036's time bound on anetwork read. Rule 5.28, starting enums at one, is none of these: it excuses a zero that is the desired
default, and telling the tree's 22 apart means reading each.
The org skill's recursion line disagrees with the guide's own deviations table. That fix belongs in
the styleguide repository and is not made here.
What a published review procedure added
A published code-review plugin was read against
morphic-reviewing-prs, principle by principle. Whatthe skills already held stays as it was, and no text is copied. Three things are new:
disprove-each-finding. A finding is attacked before it is reported: its rule read for scope andexceptions, a decision looked for on the line, a tool's hits sorted. That would have stopped this
branch filing 298 functions as a breach that none of them was.
read-standing-reviews. A review reads what already stands on a PR, after the passes, as a listread first sets the agenda just as a PR body does.
posted-findings. When asked to post: one comment a defect, code linked by full SHA, and acommittable suggestion only where applying it alone is safe.
Where a principle opposed a rule here, each side had a point, so the rules keep the better part of both
rather than pick a winner:
code-before-claims)rank-by-what-was-shown: the bar stays, a defect shows on its input, and what is only likely is aquestion:question:is the home of low confidence, with no score to argue overfan-out-a-pass-to-a-reader: only when asked, one pass a reader, the code and no conclusionedited-rule-against-its-neighboursnow says the same of any pair of rules that conflict.Found on the way
RecursionError. It is now read as text.checklist and pushes separately.
diff-contains-only-in-scope-filessaid design-only PRs carriedgo.modchanges, which neither PRit cited shows, and a search of every PR numbered up to 340 found none that does. The clause goes.
tracker-on-what-a-rule-settleswent stale that way when docs(emitter-design): settle how an SDK reads a response #1167 landed naming the two issues it saiddocs(emitter-design): settle how an SDK reads a response #1167 left out. The Why is corrected, and
reread-what-cites-a-merged-prasks for the reread.annotation.gosaid a site's referent is one hop, where both callers pass the chain'send, and
docs/micro-compiler-design.mdsaid the same in the present tense, true until feat(compilers/openapi)!: resolve schema $refs in the compiler #1016. Thenote now says what holds, and the design record moves to the past tense.
Left out
0644stays 420, as openapi: leading-zero YAML integers are stored with the wrong value, silently #104asked, where OpenAPI's recommended YAML 1.2 reads 644. The entry says so.
TestGitAttributes_PinsLineEndings,TestGitAttributes_ResolveToLFandTestTrackedFiles_HoldNoCarriageReturnalready hold the one instance there is, so it takes a checkand not a rule.
$refchain's end, skipping an alias's own.resolve-to-true-targetsays so, and its probe gives each hop a distinct value.order-oracle-reads-what-it-comparessays so. Whether the oracle should compare operations is skills: say what the order oracle compares, since a fact on an operation passes it unread #1166's own out-of-scope question.
Closes #1160
Closes #1161
Closes #1162
Closes #1165
Closes #1166
Closes #1168
Closes #1170
Closes #1171
Closes #1172
Closes #1173
Closes #1174
Closes #1175
Closes #1176
Closes #1177
Closes #1178
Test plan
make gatepassed on6f59ffde, which holdsmainthrough #1167:fmt,vet, the vet rules, lint, plantedlint,
build, 100% coverage under the race detector, fuzz and bench-smoke. The two commits since touch.claude,CLAUDE.md,CONTRIBUTING.mdand two comments in one test, and the lint, both scripts' tests,internal/archtestand lint over that test pass on the head.skills_lint.py: clean, each rule within 150 words and eachSKILL.mdwithin its cap. The lint'sand sweep's own tests pass, and each new test failed on the old code first.
off, narrowed or returning another code, each character
path_worddrops, each site not callingit, each part of the new
SPEC,RecursionErroruncaught among them, and each part ofdeclaresasfirst written.
declaresand in the path a span reports, each redden atest, among them a struct header that misses a generic such as
type G[K ~[]byte] struct.scan.MaxNestingsays it bounds a shape and not the spent quantity, and
navigationmirrors the library where therule says to call it. Renaming each named example makes the lint report it.
TestLibraryBehaviour/scalar-resolutionreddens when any of its eight rows is changed to anotherreading.
morphic compileon the entry's sample gives the values it states.testdata, baseorigin/main: 213 specs, 165 exiting 0 and 48 exiting 1 on bothsides, none differing. Its reader over
testdatakeeps the same 213 specs, and over a local corpusof 1,952 it loses none and gains two files that only name a key. It now finds a 70 KB one-line
flow spec, the shape that exits 2 at
40f12412and 0 at its parent.15c2ca10: 55 hits before, 0 withgo.sumleftout.
325d69c8, the four tests fail under barego testand pass under thecommand
toolchain-the-tree-namesquotes.#Na new or changed Why cites was checked clause by clause against that PR's branch, not itssquash, and
git grepfound no remaining mention of what the changed rules replaced.these rules settle, which is where openapi: a use site takes its $ref chain's last annotations, skipping an alias's own #1029, harness: the order-invariance oracle silently declines on any document using YAML anchors #403 and openapi: leading-zero YAML integers are stored with the wrong value, silently #104 came from; openapi: leading-zero YAML integers are stored with the wrong value, silently #104 agrees with the new entry.
morphic compileon a chain whose nearer hop writes a description gives the property thedescription at the chain's end, as the rewritten note says.
edited-rule-against-its-neighboursrun over each rule this branch adds or edits, by the act eachgoverns. What it found is fixed: the maintained-count Probe set a commit body aside beside a rule
asking that it could stand on
main; the new skill softened rule 7.9 to a question whereCLAUDE.mdstates it outright; its first pass dropped
CLAUDE.md's own Go rules; standing reviews were readfirst, where they would set the agenda; and a finding could give a command or a rule, where the
skill's Findings section asks for both.
morphic-checking-stylequotes was run, and each guide rule numberdepartures.mdcites was matched to its heading.gosecat the pinned golangci-lint: each of the 16 production findings was read. The other 158,all in tests, were read by category alone.
holding a count was read and is true on the head.
assertKepttesthelper reached rule 6.4 and, through the lookup,
assertion-helpers-stand, which it meets bycalling
t.Helper(). As a second control,MarshalJSONToinir/json.go, a breach of rule 9.3 byits heading, is resolved by
json-through-v2.internal/archtesttests each redden one. On the realtree, deleting
ir/irtest/doc.go, declaring a variable inpass/doc.goand takingt.Helper()outof
assertKepteach fail their test, and*ir.Anylosing its marker fails the build at itsassertion.
goheaderover the module and the plants draws the one planted finding. A header naming anotherholder is refused as well, checked by hand, as its two-line message cannot be a
// wantmarker.result compared byte for byte with the tree tested.
315b0fa4changes gains exactly theheader's three lines,
8107d5dachanges the 67 alias lines alone,5a420c96moves its 15 commentsbyte for byte, and
b28cf2b6adds assertions alone. The 42 Go files under othertestdatastay asthey were.
eol=lfrule reddens bothTestGitAttributestests, and a CRLF in a tracked golden reddensTestTrackedFiles_HoldNoCarriageReturn.