feat(grill-with-docs): defer the document formats to the project-docs skill #37
Loading…
Reference in a new issue
No description provided.
Delete branch "align/grill-with-docs"
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?
CONTEXT-FORMAT.md and ADR-FORMAT.md duplicated the project-docs skill's format definitions nearly wholesale, and the copies had drifted (this skill's CONTEXT-MAP example has three contexts, project-docs' has two). The skill now defers to project-docs by name and keeps the interview discipline unchanged.
Do not merge before #36 — it retargets the two references improve-codebase-architecture held into the deleted files.
Approach review: The approach is right, and it is the one the repository's own rules already prescribe.
Deleting
CONTEXT-FORMAT.mdandADR-FORMAT.mdin favour of namingproject-docsapplies that skill's own "One fact, one home" maintenance rule — a term or rationale living in two files will drift, which is exactly what happened here (the deletedCONTEXT-MAPexample had grown a third context thatproject-docsdoes not have).project-docs/SKILL.mdgenuinely covers everything the two files held: theCONTEXT.mdand ADR templates, theCONTEXT-MAP.mdshape, lazy file creation, numbering, and the three-part ADR test. Nothing is lost by deletion.Deferring by skill name rather than by relative path is also the established pattern here —
project-docsalready points the other way ("For an adversarial interview ... usegrill-with-docsinstead"), so the two skills now reference each other symmetrically instead of one reaching into the other's directory. Dropping the<what-to-do>/<supporting-info>wrappers brings the file in line with every other skill inskills/, none of which use those tags.One small residue of the problem the PR fixes: the three-part ADR test is still reproduced inline (see comment). Separately,
improve-codebase-architecturestill links both deleted files — already acknowledged in the description as the dependency on the companion PR, so no action needed here beyond honouring that merge order.Approach review by Claude Code Opus (opus)
@ -78,3 +24,1 @@### Offer ADRs sparinglyOnly offer to create an ADR when all three are true:**Offer ADRs sparingly.** Offer to create an ADR only when all three are true:This block is the one piece of
ADR-FORMAT.mdthat survived deletion rather than moving toproject-docs, and it has already drifted from the canonical copy:project-docsreads "a future reader will wonder 'why on earth did they do it this way?'", this reads "why did they do it this way?". That is the same failure mode the PR is fixing one paragraph earlier.Since the skill already instructs the agent to read
project-docsbefore creating any of these files, this could be a one-liner in the same style — "Offer to create an ADR only when it passesproject-docs' three-part test, and skip it otherwise" — keeping the interview-time behaviour (offer sparingly, inline, not batched) here and the criteria in their one home.Defensible to keep it inline if the intent is that the trigger must be available even when
project-docsis not selected for the run, but then it is worth deciding that deliberately, because it is a standing drift vector.Fixed in
feb340d: item 2 now carries project-docs' canonical "why on earth" wording, andf73c65cmakes item 3 word-identical too, removing the drift vector. Keeping the criteria inline is deliberate rather than accidental: the three-part test is interview discipline that fires in every session — exactly the reach-based split your sibling comment describes — so the trigger stays here while the formats live in project-docs.Summary: Reviewed the deletion of
CONTEXT-FORMAT.md/ADR-FORMAT.mdand theSKILL.mdrewrite. I diffed the deleted content againstskills/project-docs/SKILL.mdand confirmed nothing is lost: the CONTEXT.md template and rules, the CONTEXT-MAP format, single-vs-multi-context inference, lazy creation, the ADR template and optional sections, numbering, the three-part test, and the what-qualifies list all have a home there. The interview discipline in the rewritten body is preserved.Found 1 medium and 1 low issue, plus one observation. The main one is that the replacement pointer now targets a different skill directory, which the repo's own style guide rules out because a skill directory is the unit that travels; the secondary one is the dangling references the PR body already flags via #36.
Code review by Claude Code Opus (opus)
@ -1,60 +0,0 @@# CONTEXT.md Format🟢 Low: Confirming the merge-order constraint from the PR body, since these deletions break live pointers on
main:skills/improve-codebase-architecture/SKILL.md:78links../grill-with-docs/CONTEXT-FORMAT.mdand:80links../grill-with-docs/ADR-FORMAT.md. Both resolve to nothing once this lands, so animprove-codebase-architecturesession that hits either branch gets a read failure and falls back to inventing the format.Worth enforcing the stated ordering (land #36 first, or retarget those two lines in this PR) rather than relying on the description, since the window between merges is a broken state on
main.The ordering is a deliberate constraint, not description boilerplate: #36 owns the two improve-codebase-architecture retargets, and folding them into this PR would duplicate that change. This PR merges only after #36 lands, so main never holds the dangling-link window.
@ -50,3 +10,1 @@```Create files lazily — only when you have something to write. If no `CONTEXT.md` exists, create one when the first term is resolved. If no `docs/adr/` exists, create it when the first ADR is needed.While exploring, read whatever project documentation exists: `CONTEXT.md` (the domain glossary), `docs/adr/` (decision records), and a root `CONTEXT-MAP.md`, whose presence means the repo has multiple contexts and which points to where each one lives. The project-docs skill is the canonical home of these files' formats and maintenance rules — before creating or updating any of them, read it and follow it.🟡 Medium: This pointer targets a file outside the skill directory, which this repo's own style guide forbids —
skills/ai-facing-writing-style/SKILL.md:126: "Keep every pointer target inside the skill directory: the directory is the unit that travels, so an outside target may not exist where the skill runs."That failure mode is concrete here rather than theoretical. Per the README,
axskillsdelivers a per-run selection, and for Claude the managed copies are "toggled per run"; a run that selectsgrill-with-docsbut notproject-docsleaves this instruction with nothing to resolve. Previously the formats travelled inside the directory, so they were always present. In that run the agent still reaches "UpdateCONTEXT.mdinline" (line 22) with no format definition and will invent a glossary shape, then write it into the user's repo — the drift this PR is trying to eliminate, re-created at runtime.The wording compounds it: "read it and follow it" names no path and no mechanism, unlike the established phrasing elsewhere in the repo (
skills/verify-readme/SKILL.md:10,:160: "invoke thehuman-facing-writing-styleskill and apply its rules").Two fixes that keep the de-duplication intent:
project-docsskill and follow its formats") and state a fallback for when it is unavailable — e.g. thatCONTEXT.mdis a**Term**:/_Avoid_:glossary and an ADR is a title plus 1–3 sentences underdocs/adr/NNNN-slug.md. Two sentences restore the floor without re-duplicating the full reference.references/file insideskills/grill-with-docs/that namesproject-docsas canonical and carries only that minimum.Partly adopted, partly deliberate. Adopted:
feb340dreplaces "read it and follow it" with the established invocation phrasing — "invoke theproject-docsskill … and follow it". Rebutted: the plain-name deferral itself is the estate convention for cross-skill references — skills ship as per-run selections, so a relative path into another skill's directory is exactly what the style guide forbids; its pointer-target rule governs path pointers, andproject-docsalready points back here by name the same way. Re-inlining a two-sentence fallback format (or a thin references copy) would re-seed the drift this PR exists to remove; in a run withoutproject-docs, the skill invocation fails loudly at that step instead of silently rendering an invented format.@ -78,3 +24,1 @@### Offer ADRs sparinglyOnly offer to create an ADR when all three are true:**Offer ADRs sparingly.** Offer to create an ADR only when all three are true:ℹ️ The three-part ADR test stays duplicated here and in
skills/project-docs/SKILL.md:135-141, which is the same duplication class the PR removes elsewhere and which project-docs' own "One fact, one home" maintenance rule (:157) targets. Keeping it inline looks right by the reach-based split rule — the criteria fire in every session, while the ADR template only fires when one is actually written — so no change needed. Noting it only because these two copies are now the pair most likely to drift next.Approach review: Consolidating the two format files into
project-docsis the right call, and it matches how this repo already handles cross-skill overlap.The evidence is on the PR's side:
skills/project-docs/SKILL.mdalready carries every fact the deleted files held (CONTEXT.md format and rules, ADR format and numbering, the three-part ADR test, the file-structure trees), it already points back with "For an adversarial interview that stress-tests a plan against these docs, usegrill-with-docsinstead", and its own maintenance rules state "One fact, one home." The drift cited in the body is exactly the failure that rule predicts. Deferring by name also improves on the status quo for the outside reference:improve-codebase-architecturereached into../grill-with-docs/CONTEXT-FORMAT.mdby relative path, which the repo's own guidance (skills/ai-facing-writing-style/SKILL.md:126) rules out because skill directories travel independently; a by-name pointer at least resolves through the agent's skill index. TrimmingSKILL.mddown to the interview discipline while leaving that discipline intact keeps the two skills' responsibilities cleanly split.One design consequence is worth a decision rather than a default — noted inline: the new pointer is unconditional, but
axskillstoggles skills per run, so a run withgrill-with-docsenabled andproject-docsdisabled now has no format guidance at all.The #36 sequencing the body calls out is real — both
improve-codebase-architecturereferences still point at the deleted paths in the current tree — and is already accounted for.Approach review by Claude Code Opus (opus)
@ -50,3 +10,1 @@```Create files lazily — only when you have something to write. If no `CONTEXT.md` exists, create one when the first term is resolved. If no `docs/adr/` exists, create it when the first ADR is needed.While exploring, read whatever project documentation exists: `CONTEXT.md` (the domain glossary), `docs/adr/` (decision records), and a root `CONTEXT-MAP.md`, whose presence means the repo has multiple contexts and which points to where each one lives. The project-docs skill is the canonical home of these files' formats and maintenance rules — before creating or updating any of them, read it and follow it.The deferral is unconditional ("before creating or updating any of them, read it and follow it"), but skill availability is not.
axskillstoggles skills per run and delivers only the selection, so a session withgrill-with-docsenabled andproject-docsdisabled reaches this instruction with nothing to read — and, unlike before, no fallback format in the skill directory either. That is the failure modeskills/ai-facing-writing-style/SKILL.md:126describes ("the directory is the unit that travels, so an outside target may not exist where the skill runs").Worth deciding deliberately rather than inheriting. Cheapest option that keeps the deduplication: make the pointer conditional and name the fallback in one clause — e.g. read
project-docsand follow it; if it isn't available, keepCONTEXT.mdto term definitions and an ADR to a titled paragraph of rationale. That preserves "one fact, one home" for the full format while leaving the skill usable standalone. Alternatively, ifproject-docsis meant to be a hard dependency, saying so explicitly here (and in thedescription) gives the routing agent a reason to load it alongside.feb340dsharpens the deferral to the established "invoke theproject-docsskill" phrasing, which names the mechanism, gives the routing agent an explicit dependency, and makes an absent project-docs fail loudly at the invocation instead of silently inventing formats. Restoring inline fallback formats — full or "minimum floor" — is deliberately off the table: duplicating the formats here is exactly the drift this PR removes.Summary: Reviewed the deferral of
CONTEXT-FORMAT.mdandADR-FORMAT.mdto theproject-docsskill. Verified that every rule in the two deleted files has a home inskills/project-docs/SKILL.md— CONTEXT.md format and rules, CONTEXT-MAP.md format, single-vs-multi-context inference, lazy file creation, ADR template, numbering, optional sections, the three-part test, and the "what qualifies" list are all covered, so nothing is lost by the deletion. Deferring to a sibling skill by name matches the established convention in this repo (imagegen,human-facing-writing-style,dark-mode-image), and the interview discipline inSKILL.mdis preserved.Found 1 medium and 1 low issue: two dangling relative links into the deleted files from
improve-codebase-architecture(already acknowledged in the PR body as a merge-ordering dependency on #36, with no CI link check to catch it), and a residual duplicate of the three-part ADR test that has already drifted from the project-docs copy.Code review by Claude Code Opus (opus)
@ -1,47 +0,0 @@# ADR Format🟡 Medium: Deleting
ADR-FORMAT.mdandCONTEXT-FORMAT.mdleaves two dangling relative links in the tree:skills/improve-codebase-architecture/SKILL.md:78→../grill-with-docs/CONTEXT-FORMAT.mdskills/improve-codebase-architecture/SKILL.md:80→../grill-with-docs/ADR-FORMAT.mdThe PR body already flags this and defers to #36, so the fix is merge ordering rather than a code change — but nothing in
.forgejo/workflows/link-checks Markdown, so if this lands first the breakage is silent: an agent following either pointer gets a missing file and loses the format rules entirely. Either merge #36 first as planned, or fold the two retargets into this PR so the tree is never internally inconsistent.Deliberate ordering, per the PR body: #36 owns the two retargets and lands first, so main never holds the inconsistent window. Folding the retargets in here would duplicate #36.
@ -78,3 +24,1 @@### Offer ADRs sparinglyOnly offer to create an ADR when all three are true:**Offer ADRs sparingly.** Offer to create an ADR only when all three are true:🟢 Low: The three-part ADR test stays duplicated here after the deferral.
skills/project-docs/SKILL.md:133-141states the same three criteria, and the two copies have already drifted in wording (project-docs: "why on earth did they do it this way?"; here: "why did they do it this way?") — exactly the divergence this PR removes from the format files. project-docs' own maintenance rule (One fact, one home, line 157) calls for consolidating duplicates into the canonical home.The pointer at line 10 already sends the agent to project-docs before creating an ADR, so this block can shrink to a one-line reference ("Offer an ADR only when the three-part test in the project-docs skill passes") without losing the trigger the session needs. If the criteria are deliberately inlined because every grilling session needs them without a second read, that is a defensible call — in that case the two copies should at least be kept word-identical.
Fixed:
feb340daligns item 2 ("why on earth") andf73c65cmakes item 3 word-identical as well — the criteria are deliberately inlined (they fire in every session), now with no wording drift between the two copies.Summary: Found 1 medium issue.
The deferral itself checks out:
skills/project-docs/SKILL.mdcovers everything the two deleted files defined — theCONTEXT.mdformat and rules,CONTEXT-MAP.md, the ADR template and numbering, the three-part ADR test (word-identical with the retained copy ingrill-with-docs/SKILL.md), and lazy creation. Referring to a sibling skill by name matches the convention already used elsewhere in this repo (verify-readme→human-facing-writing-style,add-dark-mode→dark-mode-image), sogrill-with-docslosing its self-contained format files is consistent rather than a regression.The one concrete defect is the two relative links in
skills/improve-codebase-architecture/SKILL.md(lines 78 and 80) that still point into the deleted files. The PR body flags this and gates the merge on the retargeting PR; noted below because that ordering constraint lives only in the description.Code review by Claude Code Opus (opus)
@ -1,60 +0,0 @@# CONTEXT.md Format🟡 Medium: Deleting
CONTEXT-FORMAT.mdandADR-FORMAT.mdleaves two dangling relative links in a sibling skill. At this head,skills/improve-codebase-architecture/SKILL.mdstill reads:[CONTEXT-FORMAT.md](../grill-with-docs/CONTEXT-FORMAT.md)[ADR-FORMAT.md](../grill-with-docs/ADR-FORMAT.md)If this lands first, an agent following either link finds nothing, and
improve-codebase-architectureloses its only pointer to the glossary and ADR formats — the same content this PR is (correctly) consolidating intoproject-docs.The PR body notes the retargeting PR must merge first, but that ordering is enforced only by the description. Safest fix is to retarget both links to
project-docsin this PR — a two-line edit that removes the cross-PR dependency entirely — or to confirm the retargeting has landed and rebased in before merging.The risk is real, but retargeting the two links here is deliberately out of scope: PR #36 owns both retargets, and it converts them to by-name
project-docsreferences rather than new relative paths — per-run skill delivery can't resolve cross-skill relative links anyway, so a two-line path edit in this PR would still be wrong and would then conflict with #36. The merge-after-#36 note in the PR body is that deliberate ordering constraint, not boilerplate; the operator merges #36 first. Leaving the links untouched here keeps a single owner for the retarget.