fix: verification criteria should refer to the checklist #115

Merged

jercik merged 2 commits from docs/remove-verification-count into main 2026-10-04 19:36:40 +00:00

Conversation 3 Commits 2 Files changed 1 +1 -3

jercik commented 2026-10-04 19:24:05 +00:00

Owner

Copy link

Removes repeated verification-checklist counts and points completion to the Verify checks, following #104, while preserving every verification obligation.

Removes repeated verification-checklist counts and points completion to the Verify checks, following [#104](https://code.j4k.dev/j4k-oss/agent-skills/pulls/104), while preserving every verification obligation.

jercik added 1 commit 2026-10-04 19:24:06 +00:00

docs: remove duplicated verification checklist count 566874923f

jercik referenced this pull request 2026-10-04 19:24:31 +00:00

fix(skills): avoid repeating reengineering counts #104

forgejo-actions commented 2026-10-04 19:27:10 +00:00

Copy link

Review 01M44678YXKT9H92FKQ98EYJ0S — head 55a1a2ba7e7eea885c9a3cf8fd20ce2f50875820

Review — j4k-oss/agent-skills @ e828679c2c

Scope: diff against base tree 8d139933be25
Status: dispatched — coverage complete (5/5 slots terminal)
Facts: current review-wide projection

Computed under:

{
  "abandonment": "abandonment-v1",
  "anchor_recipe": 1,
  "batch_policy": "batch-v1",
  "coverage": "coverage-v3",
  "dispatch_policy": "dispatch-v2",
  "grounder_version": 1,
  "grounding_read_rule": "grounding-read-v1",
  "promotion_policy": "promotion-v1",
  "report": "report-v4",
  "tally": "tally-v1",
  "triage_settle": "triage-settle-v2"
}

Findings (2)

low — Code-comment rule for the implementer is stated as a passive fact, not an instruction to put it in the brief

• claim: 01M446BS1TJ50EBSRPK86H4DNC
• anchor: skills/reengineer-program/references/implementation.md (snippet)

Require it to report each such read and the question it answered. Rationale stays in ADRs, without ADR citations or restatements in code comments.

• lens: writing-quality · arm: default
• verdicts: 1 valid / 0 invalid / 0 uncertain
  • pass 01M446CQE51A61MYZPYYFYFCP8 · valid: The grounded excerpt (exact match) shows the sentence 'Rationale stays in ADRs, without ADR citations or restatements in code comments.' directly after 'Require it to report each such read...', in a paragraph that otherwise lists what to give or require of the implementer. The reviewer reports that the same file elsewhere explicitly marks one rule for the brief ('Copy the dependency-handling rule ... into the brief'), and that SKILL.md already gives the orchestrator this rule. Given that context, a literal reader can plausibly take the passive sentence as a note to itself, not an item for the implementer's brief, and leave it out. The implementer would then write ADR citations or rationale into comments. The writing skill asks prose to lead with the action and its object. The proposed fix, 'Require it to keep rationale in ADRs, without ADR citations or restatements in code comments.', is a minimal rewording that keeps the rule and its scope, matches the surrounding directive voice, and removes the ambiguity. Low severity is appropriate.
• disposition: none

The orchestrating agent can leave this rule out of the implementer's brief, and the implementer then writes ADR citations or rationale into code comments. The implementer is a fresh sub-agent that never reads SKILL.md, so the rule reaches it only if the orchestrator passes it on.

The Rebuild section's paragraph is a list of things to give or require of the implementer: "Give the implementer the confirmed design, CONTEXT.md, …" and "Require it to report each such read …". It ends with a sentence in a different voice: "Rationale stays in ADRs, without ADR citations or restatements in code comments." Two paragraphs later, the file says explicitly what goes into the brief: "Copy the dependency-handling rule from item 4 of references/compare-designs.md into the brief". A literal reader who sees one rule explicitly marked for the brief can reasonably take the passive sentence as background it already follows itself. SKILL.md already gives the orchestrator the same rule ("Keep rationale in ADRs, without citations or restatements in code comments."), so this sentence adds something only if it makes the rule part of the brief.

Correction: replace the sentence with "Require it to keep rationale in ADRs, without ADR citations or restatements in code comments." This keeps the rule and its scope, and it matches the "Require it to …" directive just before it. The writing skill asks for content that leads with the action and its object.

Examined: skills/reengineer-program/references/implementation.md (Rebuild section) and skills/reengineer-program/SKILL.md ("Record decisions as they settle"). This claim rests on reading the text; I did not run an agent through the procedure. Evidence that orchestrators reliably include this rule in implementer briefs under the current wording would refute it.

low — Subtract section repeats the ADR length limit from SKILL.md instead of naming the drop's ADR

• claim: 01M446C038QAMWH8P6JVN0HX67
• anchor: skills/reengineer-program/references/implementation.md (snippet)

Fold accepted fallout into the drop's 1–3 sentences, or record a separate ADR when it is a separate promise.

• lens: writing-quality · arm: default
• verdicts: 1 valid / 0 invalid / 0 uncertain
  • pass 01M446CQE51A61MYZPYYFYFCP8 · valid: The grounded excerpt (exact match) says 'Fold accepted fallout into the drop's 1–3 sentences'. The reviewer quotes SKILL.md's ADR format rule ('Each ADR has a title, status frontmatter, and 1–3 sentences') and reports that SKILL.md is the only place that tells agents to load this reference, so that rule is always in context when this sentence is read. Here the number only identifies the drop's ADR text; it adds no constraint beyond the format rule, which applies to every ADR. If the limit changes, this copy goes stale and gives conflicting guidance, which the skill's 'One Idea, One Place' guidance targets. The proposed fix, 'Fold accepted fallout into the drop's ADR, or record a separate ADR when it is a separate promise.', keeps both the instruction and its alternative. The length limit stays enforced by the format rule, so nothing a reader needs is lost. The copy still matches today, so low severity is appropriate. This is a duplicated numeric constraint, not a restated set of members.
• disposition: none

"The drop's 1–3 sentences" restates the ADR format limit that SKILL.md already defines: "Each ADR has a title, status frontmatter, and 1–3 sentences, without extra sections." If that limit changes, this reference keeps the old number. An agent folding fallout into a drop would then follow whichever number it reads last.

Here the number works only as a name for the drop's ADR text. The rule that bounds that text is already loaded, because SKILL.md requires the agent to read references/implementation.md before any subtraction. Under the writing skill's "One Idea, One Place" guidance, the constraint should live in one place.

Correction: "Fold accepted fallout into the drop's ADR, or record a separate ADR when it is a separate promise." This keeps the instruction and its alternative. The length limit is still enforced by the ADR format rule in SKILL.md.

Examined: skills/reengineer-program/references/implementation.md (Subtract section) and skills/reengineer-program/SKILL.md ("Record decisions as they settle" and "Optional design and code changes"). At this revision both say 1–3 sentences, so the copy is not yet wrong. The claim would fail if the reference were meant to be loaded without SKILL.md, but SKILL.md is the only place that tells the agent to read it.

Other claims

• grounding-pending (0)
• ungrounded (0)
• rejected (0)
• duplicate-of (0)
• unadjudicated (0)

Coverage

Coverage pass: 01M446791WHMFKDPB5F7BEWEXW
Accounting: complete
Slot health: healthy

lens	part	arm	unit status	runs	loss
general-bug	whole	default	no-claims	1	no
writing-quality	whole	default	claims-emitted	1	no
test-trimming	whole	default	no-claims	1	no
restated-sets	whole	default	no-claims	1	no
project-docs	whole	default	no-claims	1	no

<!-- review:summary --> **Review** `01M44678YXKT9H92FKQ98EYJ0S` — head `55a1a2ba7e7eea885c9a3cf8fd20ce2f50875820` # Review — j4k-oss/agent-skills @ e828679c2ccd Scope: diff against base tree `8d139933be25` Status: dispatched — coverage complete (5/5 slots terminal) Facts: current review-wide projection Computed under: ```json { "abandonment": "abandonment-v1", "anchor_recipe": 1, "batch_policy": "batch-v1", "coverage": "coverage-v3", "dispatch_policy": "dispatch-v2", "grounder_version": 1, "grounding_read_rule": "grounding-read-v1", "promotion_policy": "promotion-v1", "report": "report-v4", "tally": "tally-v1", "triage_settle": "triage-settle-v2" } ``` ## Findings (2) ### low — Code-comment rule for the implementer is stated as a passive fact, not an instruction to put it in the brief - claim: `01M446BS1TJ50EBSRPK86H4DNC` - anchor: `skills/reengineer-program/references/implementation.md` (snippet) ``` Require it to report each such read and the question it answered. Rationale stays in ADRs, without ADR citations or restatements in code comments. ``` - lens: writing-quality · arm: default - verdicts: 1 valid / 0 invalid / 0 uncertain - pass `01M446CQE51A61MYZPYYFYFCP8` · valid: The grounded excerpt (exact match) shows the sentence 'Rationale stays in ADRs, without ADR citations or restatements in code comments.' directly after 'Require it to report each such read...', in a paragraph that otherwise lists what to give or require of the implementer. The reviewer reports that the same file elsewhere explicitly marks one rule for the brief ('Copy the dependency-handling rule ... into the brief'), and that SKILL.md already gives the orchestrator this rule. Given that context, a literal reader can plausibly take the passive sentence as a note to itself, not an item for the implementer's brief, and leave it out. The implementer would then write ADR citations or rationale into comments. The writing skill asks prose to lead with the action and its object. The proposed fix, 'Require it to keep rationale in ADRs, without ADR citations or restatements in code comments.', is a minimal rewording that keeps the rule and its scope, matches the surrounding directive voice, and removes the ambiguity. Low severity is appropriate. - disposition: none > The orchestrating agent can leave this rule out of the implementer's brief, and the implementer then writes ADR citations or rationale into code comments. The implementer is a fresh sub-agent that never reads `SKILL.md`, so the rule reaches it only if the orchestrator passes it on. > > The Rebuild section's paragraph is a list of things to give or require of the implementer: "Give the implementer the confirmed design, `CONTEXT.md`, …" and "Require it to report each such read …". It ends with a sentence in a different voice: "Rationale stays in ADRs, without ADR citations or restatements in code comments." Two paragraphs later, the file says explicitly what goes into the brief: "Copy the dependency-handling rule from item 4 of `references/compare-designs.md` into the brief". A literal reader who sees one rule explicitly marked for the brief can reasonably take the passive sentence as background it already follows itself. `SKILL.md` already gives the orchestrator the same rule ("Keep rationale in ADRs, without citations or restatements in code comments."), so this sentence adds something only if it makes the rule part of the brief. > > Correction: replace the sentence with "Require it to keep rationale in ADRs, without ADR citations or restatements in code comments." This keeps the rule and its scope, and it matches the "Require it to …" directive just before it. The writing skill asks for content that leads with the action and its object. > > Examined: `skills/reengineer-program/references/implementation.md` (Rebuild section) and `skills/reengineer-program/SKILL.md` ("Record decisions as they settle"). This claim rests on reading the text; I did not run an agent through the procedure. Evidence that orchestrators reliably include this rule in implementer briefs under the current wording would refute it. ### low — Subtract section repeats the ADR length limit from SKILL.md instead of naming the drop's ADR - claim: `01M446C038QAMWH8P6JVN0HX67` - anchor: `skills/reengineer-program/references/implementation.md` (snippet) ``` Fold accepted fallout into the drop's 1–3 sentences, or record a separate ADR when it is a separate promise. ``` - lens: writing-quality · arm: default - verdicts: 1 valid / 0 invalid / 0 uncertain - pass `01M446CQE51A61MYZPYYFYFCP8` · valid: The grounded excerpt (exact match) says 'Fold accepted fallout into the drop's 1–3 sentences'. The reviewer quotes SKILL.md's ADR format rule ('Each ADR has a title, status frontmatter, and 1–3 sentences') and reports that SKILL.md is the only place that tells agents to load this reference, so that rule is always in context when this sentence is read. Here the number only identifies the drop's ADR text; it adds no constraint beyond the format rule, which applies to every ADR. If the limit changes, this copy goes stale and gives conflicting guidance, which the skill's 'One Idea, One Place' guidance targets. The proposed fix, 'Fold accepted fallout into the drop's ADR, or record a separate ADR when it is a separate promise.', keeps both the instruction and its alternative. The length limit stays enforced by the format rule, so nothing a reader needs is lost. The copy still matches today, so low severity is appropriate. This is a duplicated numeric constraint, not a restated set of members. - disposition: none > "The drop's 1–3 sentences" restates the ADR format limit that `SKILL.md` already defines: "Each ADR has a title, `status` frontmatter, and 1–3 sentences, without extra sections." If that limit changes, this reference keeps the old number. An agent folding fallout into a drop would then follow whichever number it reads last. > > Here the number works only as a name for the drop's ADR text. The rule that bounds that text is already loaded, because `SKILL.md` requires the agent to read `references/implementation.md` before any subtraction. Under the writing skill's "One Idea, One Place" guidance, the constraint should live in one place. > > Correction: "Fold accepted fallout into the drop's ADR, or record a separate ADR when it is a separate promise." This keeps the instruction and its alternative. The length limit is still enforced by the ADR format rule in `SKILL.md`. > > Examined: `skills/reengineer-program/references/implementation.md` (Subtract section) and `skills/reengineer-program/SKILL.md` ("Record decisions as they settle" and "Optional design and code changes"). At this revision both say 1–3 sentences, so the copy is not yet wrong. The claim would fail if the reference were meant to be loaded without `SKILL.md`, but `SKILL.md` is the only place that tells the agent to read it. ## Other claims - grounding-pending (0) - ungrounded (0) - rejected (0) - duplicate-of (0) - unadjudicated (0) ## Coverage Coverage pass: 01M446791WHMFKDPB5F7BEWEXW Accounting: complete Slot health: healthy | lens | part | arm | unit status | runs | loss | | --- | --- | --- | --- | --- | --- | | general-bug | whole | default | no-claims | 1 | no | | writing-quality | whole | default | claims-emitted | 1 | no | | test-trimming | whole | default | no-claims | 1 | no | | restated-sets | whole | default | no-claims | 1 | no | | project-docs | whole | default | no-claims | 1 | no |

🚀 1

jercik added 1 commit 2026-10-04 19:28:26 +00:00

fix: preserve verification reference after count removal 55a1a2ba7e

jercik changed title from docs: remove duplicated verification checklist count to fix: verification criteria should refer to the checklist 2026-10-04 19:28:46 +00:00

jercik commented 2026-10-04 19:28:46 +00:00

Author

Owner

Copy link

Replying to review comment #119750

Fixed summary-only claim 01M4460YJRVEEE8HY451F57TM3 and duplicate 01M44614A8W0Q6NVD8YFYVSZNG in 55a1a2b: completion now requires the verification checks defined above to pass, preserving all obligations without a copied count or an ambiguous antecedent.

> Replying to review comment #119750 Fixed summary-only claim `01M4460YJRVEEE8HY451F57TM3` and duplicate `01M44614A8W0Q6NVD8YFYVSZNG` in 55a1a2b: completion now requires the verification checks defined above to pass, preserving all obligations without a copied count or an ambiguous antecedent.

jercik referenced this pull request 2026-10-04 19:35:13 +00:00

docs: clarify existing implementation ADR guidance #116

jercik commented 2026-10-04 19:35:50 +00:00

Author

Owner

Copy link

Replying to review comment #119750

Tracked in #116: the summary-only implementer-brief clarity claim 01M446BS1TJ50EBSRPK86H4DNC and repeated ADR-format-limit claim 01M446C038QAMWH8P6JVN0HX67 are owned by an independent follow-up preserving the existing ADR requirements. Neither is claimed fixed in #115 by this unmerged follow-up.

> Replying to review comment #119750 Tracked in https://code.j4k.dev/j4k-oss/agent-skills/pulls/116: the summary-only implementer-brief clarity claim `01M446BS1TJ50EBSRPK86H4DNC` and repeated ADR-format-limit claim `01M446C038QAMWH8P6JVN0HX67` are owned by an independent follow-up preserving the existing ADR requirements. Neither is claimed fixed in #115 by this unmerged follow-up.

jercik merged commit f8473dd033 into main 2026-10-04 19:36:40 +00:00

jercik referenced this pull request from a commit 2026-10-04 19:36:42 +00:00

fix: verification criteria should refer to the checklist (#115)

jercik referenced this pull request 2026-10-04 19:45:37 +00:00

docs: clarify existing implementation ADR guidance #116

jercik referenced this pull request 2026-10-04 19:58:10 +00:00

fix(skills): avoid repeating reengineering counts #104

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

j4k-oss/agent-skills!115

No description provided.