docs(verify-tests): name the rules corpus in the T1-T3 source lines #41
Loading…
Reference in a new issue
No description provided.
Delete branch "align/verify-tests"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The T1-T3 Source lines cited bare relative paths (
rules/core/no-logic-in-tests.md) that only resolve inside the j4k/align repository without saying so; they now name the corpus and its checkout path. T4 stays corpus-less on purpose — it is a candidate for a future align-corpus rule, tracked separately.Approach review: The intent is right — the bare
rules/core/...paths were unresolvable for a reader outside the align checkout, and naming the corpus fixes that. The one approach question is which coordinate to name: the PR encodes a machine-local checkout path (~/Developer/j4k/align/rules/) into a repository whose README describes it as "the shareable half of the skill estate", delivered byaxskillsto other people's agents. A canonical repo coordinate (forge URL, the way the README citeshttps://code.j4k.dev/j4k/axkit/.../axskills) identifies the corpus without asserting where any given machine put it. Secondary point: the same location sentence is now duplicated in three rule files plus SKILL.md, so a corpus move is a four-file edit.Everything else reads as tightening rather than redesign. The
rules/table, the pause-and-report contract, and the audit/fix split are unchanged; the guidance dropped from T2's and T3's Fix sections (empty-describecleanup, don't-auto-edit) still lives in SKILL.md's Apply-fixes step, so nothing is lost. Worth noting for the reader that the diff goes well beyond the Source lines the title scopes it to — step renumbering, the folded-in "Validate prerequisites" section, a rewrittendescription— which is fine as edits but makes the change harder to review against its stated purpose.Approach review by Claude Code Opus (opus)
@ -28,3 +26,3 @@| [`rules/t4-test-what-matters.md`](rules/t4-test-what-matters.md) | T4 | Cover expensive, stable surfaces; drop pass-throughs, choreography, and snapshot spam |Each rule file links back to its source under the repository's `rules/` directory — those are authoritative for intent. Rule files here are the enforcement counterpart.Rules T1–T3 cite their source rule in the rules corpus of the j4k/align repository, checked out at `~/Developer/j4k/align/rules/`; the corpus files are authoritative for intent, and the files here are the enforcement counterpart.Consider naming the corpus by its canonical repo coordinate rather than a local checkout path.
~/Developer/j4k/align/rules/is one machine's layout, but this repo is the shared skill source thataxskillsclones and delivers into other people's agents — for any consumer who cloned align elsewhere (or cannot see it at all) the path is wrong rather than merely unresolvable, which is a worse failure than the bare relative paths it replaces. The README's own convention for pointing at a sibling repo is a forge URL (https://code.j4k.dev/j4k/axkit/src/branch/main/packages/axskills#readme); the same shape here —j4k/alignplus the corpus-relative path, optionally linked — survives a move of anyone's checkout.Keeping the checkout-path form — this is a deliberate ruling for this PR, not an oversight. These Source lines are provenance citations, not must-read pointers: nothing in the skill directs an agent to open the corpus file, and the rule files here remain the self-contained enforcement counterpart. The known tension — the path does not resolve for a consumer without an align checkout at that location — was weighed and accepted; a reader who cannot reach the path loses nothing but the provenance trail, and the corpus plus its relative path (
j4k/align,core/...) is named in the same sentence, so the citation still identifies the source without the checkout.@ -3,3 +3,3 @@Test assertions use literal expected values. Flag any assertion whose expected side is computed — operators, string concatenation, template literals referencing variables, conditionals, loops, or helpers that recompute what production code computes.Source: `rules/core/no-logic-in-tests.md`.Source: `core/no-logic-in-tests.md` in the rules corpus of the j4k/align repository, checked out at `~/Developer/j4k/align/rules/`.The full corpus location is now repeated verbatim in t1, t2, t3 and SKILL.md, so relocating the corpus means editing four files and the copies can drift apart. An alternative that keeps the fix's benefit: state the corpus and its location once in SKILL.md (which the diff already does at line 28) and have each rule file cite only its corpus-relative path —
Source: \core/no-logic-in-tests.md` in the align rules corpus.` The rule files are always read via SKILL.md's table, so the context is present either way.Keeping the repetition — deliberate. SKILL.md directs the agent to read a finding's rule file before acting, so each rule file is read on its own and its Source line stays self-contained rather than leaning on SKILL.md context. The four-file edit cost of a future corpus move was weighed and accepted as the price of standalone provenance citations.
Summary: Docs-only rewrite of the
verify-testsskill. The renumbering of the Task steps is internally consistent (no surviving cross-references to the removed "Validate prerequisites" step), and the guidance dropped from the T1/T2 Fix sections is still carried by SKILL.md §3, so nothing is lost there. Two low-severity findings on the new source lines and on the T3 guard.Code review by Claude Code Opus (opus)
@ -28,3 +26,3 @@| [`rules/t4-test-what-matters.md`](rules/t4-test-what-matters.md) | T4 | Cover expensive, stable surfaces; drop pass-throughs, choreography, and snapshot spam |Each rule file links back to its source under the repository's `rules/` directory — those are authoritative for intent. Rule files here are the enforcement counterpart.Rules T1–T3 cite their source rule in the rules corpus of the j4k/align repository, checked out at `~/Developer/j4k/align/rules/`; the corpus files are authoritative for intent, and the files here are the enforcement counterpart.🟢 Low:
~/Developer/j4k/align/rules/is a machine-specific checkout path, but this repository is the shareable source thataxskillsclones and delivers into any consumer's agent (perREADME.md). On any machine without that checkout, an agent told these files are "authoritative for intent" will try to read a path that does not exist, with no stated fallback. The same line appears inrules/t1-no-logic-in-tests.md:5,rules/t2-no-tests-for-type-guarantees.md:5, andrules/t3-focus-on-functional-core.md:5.A forge URL (e.g. the
j4k/alignrepository oncode.j4k.dev) resolves for every consumer, or keep the local path and add one clause stating that the rule files here are self-contained when the corpus is not checked out. Noteskills/usage-limit-estimator/SKILL.mdalready hardcodes/Users/jercik/...paths, so if that is the accepted convention for this estate this is a deliberate call — worth confirming rather than diverging in style between the two forms.Keeping the checkout-path form — this is a deliberate ruling for this PR, not an oversight. These Source lines are provenance citations, not must-read pointers: nothing in the skill directs an agent to open the corpus file, and the rule files here remain the self-contained enforcement counterpart. The known tension — the path does not resolve for a consumer without an align checkout at that location — was weighed and accepted; a reader who cannot reach the path loses nothing but the provenance trail, and the corpus plus its relative path (
j4k/align,core/...) is named in the same sentence, so the citation still identifies the source without the checkout.@ -18,3 +18,3 @@## Fix**Pause and report.** This rule removes whole tests and can drop coverage the author intended — the user should choose per case. Surface each finding with three options:**Pause and report.** This rule removes whole tests and can drop coverage the author intended — the user chooses per case. Surface each finding with three options:🟢 Low: This rewrite also deletes the trailing
Do not edit T3 findings automatically.line, while the siblingrules/t4-test-what-matters.md:27keeps its identical guard. SKILL.md §3 still routes T3 through pause-and-report, so behavior is specified, but T3 and T4 are the two rules whose fix action deletes whole tests, and dropping the explicit prohibition from only one of them leaves the weaker instruction on the destructive path when the rule file is read on its own (SKILL.md line 19 directs the agent to read the rule file before acting). Either restore the line in T3 or drop it from T4 so both destructive rules read the same.The asymmetry is deliberate, per two rulings encoded in this PR. Fix-action mechanics were single-homed in SKILL.md's Apply fixes step, which owns "leave the test in place and surface the finding" for the pause-and-report rules — that is why T3's trailing line was dropped. T4 keeps its
Do not edit T4 findings automatically.line on purpose: T4 is the deliberately corpus-less rule, tracked separately as a candidate for a future align-corpus rule, and this PR leaves its file untouched. So neither restoring T3's line nor trimming T4's is planned here.Approach review: The intent is right — the bare
rules/core/...citations were ambiguous, and naming the corpus that owns them removes a real guessing game. The rest of the rewrite (tightened prose, merged preflight step, T4 folded into the frontmatter description) is consistent with what the skill already does.One approach-level alternative: the new pointer is a machine-local checkout path (
~/Developer/j4k/align/rules/) repeated verbatim in four files, inside a repository the README describes as "the shareable half of the skill estate" — delivered byaxskillsinto any agent, on any machine, for anyone who selects it. A stable identifier (the forge URL, the form the README already uses forj4k/axkit) resolves regardless of where the corpus is checked out; the local path, if useful, reads better as a hint after it. Stating it once inSKILL.mdand leaving the rule files with corpus-relative paths alone would also keep a future relocation to a one-line edit. Detail is on the inline comment.Smaller consistency note, not blocking: T3 loses its closing
Do not edit T3 findings automatically.line while the untouched T4 file keeps its equivalent, so the two pause-and-report rules no longer follow the same template.SKILL.mdstill covers both, so nothing is lost — worth deciding which shape the rule files should share.Approach review by Claude Code Opus (opus)
@ -28,3 +26,3 @@| [`rules/t4-test-what-matters.md`](rules/t4-test-what-matters.md) | T4 | Cover expensive, stable surfaces; drop pass-throughs, choreography, and snapshot spam |Each rule file links back to its source under the repository's `rules/` directory — those are authoritative for intent. Rule files here are the enforcement counterpart.Rules T1–T3 cite their source rule in the rules corpus of the j4k/align repository, checked out at `~/Developer/j4k/align/rules/`; the corpus files are authoritative for intent, and the files here are the enforcement counterpart.Consider citing the corpus by a location-independent identifier rather than a local checkout path. As written,
~/Developer/j4k/align/rules/only resolves on a machine that happens to have that checkout in that spot; this repo ships to any agentaxskillsdelivers to, so for most readers the path names something that isn't there — the same ambiguity the change set out to fix, one level up.The README already links sibling repos by forge URL (
https://code.j4k.dev/j4k/axkit/...); the same form forj4k/alignidentifies the corpus unambiguously whether or not the reader can fetch it, and the local path can follow as a convenience hint.Related: this sentence names the corpus and the path, and then t1/t2/t3 each repeat both in full. Since this line already establishes the corpus, the rule files could cite just their corpus-relative path (
core/no-logic-in-tests.md) — one place to edit if the corpus ever moves or is renamed, instead of four.Keeping the checkout-path form — this is a deliberate ruling for this PR, not an oversight. These Source lines are provenance citations, not must-read pointers: nothing in the skill directs an agent to open the corpus file, and the rule files here remain the self-contained enforcement counterpart. The known tension — the path does not resolve for a consumer without an align checkout at that location — was weighed and accepted; a reader who cannot reach the path loses nothing but the provenance trail, and the corpus plus its relative path (
j4k/align,core/...) is named in the same sentence, so the citation still identifies the source without the checkout.Summary: Docs-only change to the
verify-testsskill; no correctness, security, or error-handling defects found. The renumbered Task sections (2–5 → 1–4) have no dangling cross-references, all fourrules/*.mdtable links resolve, the frontmatterdescriptionis 557 chars (well under the 1024 limit), and the claim onSKILL.md:28that only T1–T3 carry a Source line matches the files —t4-test-what-matters.mdhas none.Two minor findings, both low/info: the newly cited corpus path is machine-specific, and T3 lost the "do not edit automatically" line that its T4 counterpart still carries.
Detail worth noting but not flagged: the guidance trimmed from the T1 and T2
Fixsections (preserve inputs/title/setup; delete an emptieddescribe; never delete a whole file silently) is all still present inSKILL.md:51-55, so nothing was lost — the trimming is consistent deduplication.Code review by Claude Code Opus (opus)
@ -28,3 +26,3 @@| [`rules/t4-test-what-matters.md`](rules/t4-test-what-matters.md) | T4 | Cover expensive, stable surfaces; drop pass-throughs, choreography, and snapshot spam |Each rule file links back to its source under the repository's `rules/` directory — those are authoritative for intent. Rule files here are the enforcement counterpart.Rules T1–T3 cite their source rule in the rules corpus of the j4k/align repository, checked out at `~/Developer/j4k/align/rules/`; the corpus files are authoritative for intent, and the files here are the enforcement counterpart.🟢 Low:
~/Developer/j4k/align/rules/is a machine-specific checkout location, not a resolvable coordinate. This repo is the shareable source thataxskillsclones and delivers into arbitrary users' agents (README, top section), so for anyone whose align checkout lives elsewhere — or who has none — an agent told the corpus is "authoritative for intent" will try that path and find nothing, with no way to recover the real location from the text.Naming the repo by its forge coordinate keeps the pointer resolvable while the local path stays a convenience, e.g. "…in the rules corpus of
code.j4k.dev/j4k/align(typically checked out at~/Developer/j4k/align/rules/)". Same for the threeSource:lines inrules/t1-*.md:5,rules/t2-*.md:5, andrules/t3-*.md:5.Noting for fairness:
skills/usage-limit-estimator/SKILL.md:14-24already hardcodes absolute/Users/jercik/Developer/...paths, so this may be deliberate house style — in which case ignore.Keeping the checkout-path form — this is a deliberate ruling for this PR, not an oversight. These Source lines are provenance citations, not must-read pointers: nothing in the skill directs an agent to open the corpus file, and the rule files here remain the self-contained enforcement counterpart. The known tension — the path does not resolve for a consumer without an align checkout at that location — was weighed and accepted; a reader who cannot reach the path loses nothing but the provenance trail, and the corpus plus its relative path (
j4k/align,core/...) is named in the same sentence, so the citation still identifies the source without the checkout.@ -18,3 +18,3 @@## Fix**Pause and report.** This rule removes whole tests and can drop coverage the author intended — the user should choose per case. Surface each finding with three options:**Pause and report.** This rule removes whole tests and can drop coverage the author intended — the user chooses per case. Surface each finding with three options:ℹ️ Info: This hunk drops
Do not edit T3 findings automatically., but the parallel line survives atrules/t4-test-what-matters.md:27(Do not edit T4 findings automatically.), leaving the two pause-and-report rules asymmetric. The guardrail itself is not lost —SKILL.md:53still says pause-and-report for T3 and T4 — so this is cosmetic; worth either restoring the line here or dropping T4's for parity.The asymmetry is deliberate, per two rulings encoded in this PR. Fix-action mechanics were single-homed in SKILL.md's Apply fixes step, which owns "leave the test in place and surface the finding" for the pause-and-report rules — that is why T3's trailing line was dropped. T4 keeps its
Do not edit T4 findings automatically.line on purpose: T4 is the deliberately corpus-less rule, tracked separately as a candidate for a future align-corpus rule, and this PR leaves its file untouched. So neither restoring T3's line nor trimming T4's is planned here.Managed review feedback processed for head
541d5fd.Disposition — all seven inline findings rebutted per this PR's deliberate rulings (replies on each thread):
On the approach note that the diff reaches beyond the title's scope: the prose tightening and step-merge rode along intentionally in the same editing pass; the PR body names the substantive edit.
Review-cycle status: slots
approach-smart-1andcode-smart-1published receipt-bound reviews for this head (twice — original run 666 and retry 687). Slotsapproach-smart-2,approach-3, andcode-smart-2failed on both runs withForgejo Actions capability exchange failed: HTTP 400: "Validation failed"from the Axrecipe exchange — deterministic, service-side, and not fixable from this PR. The cycle is incomplete for those three slots until the Axrecipe slot validation is repaired.