docs: add software-engineering working principles + repository agent skills - #17
docs: add software-engineering working principles + repository agent skills#17egparedes wants to merge 11 commits into
Conversation
Add content/knowledge/ as a new top-level area for reference material — distilled practice that proposals are written and reviewed against, as opposed to the proposals themselves under personal/ and shared/. Its first document is a distillation of software design and engineering practice: prime directives on complexity and modularity, deep modules and information hiding, tactical and strategic domain modelling, daily craft, and a red-flag checklist a reviewer can run over a proposal or a diff. To make it actually reach the agents writing proposals, it is wired into the paths they already read: AGENTS.md documents the directory and makes reading the principles step 1 of authoring, and content/index.md gains a Knowledge section. Also resolves the placeholder link in model-state spec v2, which already cited this document by its intended path.
| writing or reviewing code, from the prime directives down to a red-flag checklist you | ||
| can run over a diff. | ||
|
|
||
| ## 1. Prime directives |
There was a problem hiding this comment.
Is the star trek reference from you or from the LLM?
There was a problem hiding this comment.
Who could know that Claude is a trekkie? 😆
54ae9d3 to
c09af0b
Compare
The principles document is reference material for humans deciding how to write and judge a design, not a procedure for an agent to execute. Wiring it into AGENTS.md turned it into the latter: a mandatory step 1 of authoring, with a checklist an agent would dutifully recite over every proposal. Revert those pointers; the document stays discoverable through content/index.md, where readers find it.
Five skills under .agents/skills/ covering the procedures AGENTS.md only summarizes: drafting a proposal, cross-checking it against what exists, reviewing it, keeping one vocabulary, and graduating it to shared/ or to icon4py. Each carries trigger-query evals so over- and under-triggering can be measured rather than guessed. .claude/skills is a symlink to .agents/skills so Claude Code discovers them without a second copy. AGENTS.md gains a Skills table pointing at them for agents that do not discover skills automatically.
There was a problem hiding this comment.
Pull request overview
Adds durable software-engineering guidance and repository-specific agent workflows for authoring and managing icon4py proposals.
Changes:
- Adds and indexes software-engineering working principles.
- Adds five agent skills with documentation, references, and trigger evaluations.
- Replaces a placeholder principles link with a Quartz wikilink.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
AGENTS.md |
Documents repository skills. |
content/index.md |
Adds the Knowledge section. |
content/knowledge/software-engineering/principles.md |
Defines working principles. |
content/personal/jcanton/model-state/model-state_specV2.md |
Fixes the principles link. |
.agents/skills/README.md |
Describes skill discovery and evaluation. |
.agents/skills/drafting-a-proposal/SKILL.md |
Defines proposal drafting workflow. |
.agents/skills/drafting-a-proposal/references/authoring-mechanics.md |
Documents authoring mechanics. |
.agents/skills/drafting-a-proposal/evals/trigger-queries.json |
Tests drafting triggers. |
.agents/skills/cross-checking-proposals/SKILL.md |
Defines proposal cross-checking. |
.agents/skills/cross-checking-proposals/evals/trigger-queries.json |
Tests cross-checking triggers. |
.agents/skills/reviewing-a-proposal/SKILL.md |
Defines proposal review workflow. |
.agents/skills/reviewing-a-proposal/evals/trigger-queries.json |
Tests review triggers. |
.agents/skills/keeping-one-vocabulary/SKILL.md |
Defines vocabulary management. |
.agents/skills/keeping-one-vocabulary/references/glossary-template.md |
Provides a glossary template. |
.agents/skills/keeping-one-vocabulary/evals/trigger-queries.json |
Tests vocabulary triggers. |
.agents/skills/graduating-a-proposal/SKILL.md |
Defines proposal graduation and retirement. |
.agents/skills/graduating-a-proposal/evals/trigger-queries.json |
Tests graduation triggers. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The skills pointed at content/knowledge/software-engineering/principles.md, which is written for humans deciding how to write or judge a design — not a procedure for an agent to execute. Cut every such reference: - drafting no longer opens by reading the principles. Securing the need, users, ranked goals and alternatives is its job; judging the design is reviewing's, later. - reviewing carries its own standard. Its red-flag pass, which delegated to §6 of the principles, becomes a recurring-defect pass naming the failures this repository actually produces — mechanism-as-requirement, phase-ordered decomposition, a word doing two jobs — with the instruction to drop any flag that cannot be made concrete in icon4py terms. - the skills README records the boundary: content/knowledge/ is human-facing by default, glossary.md is the read/write exception, anything else added there is a case-by-case decision. The glossary stays where it is. Agents both read and write it, so it is a registry of terms in use rather than guidance to follow. Also from review: - reviewing triggered on any document under content/ but assumed a proposal and recommended a proposal status; restricted to content/personal/ and content/shared/, with a Scope section for what to do otherwise. - reviewing quoted a promise AGENTS.md does not make; removed with the section that made it. - cross-checking claimed to run before a proposal is reviewed, which its own eval marks should_trigger:false and which reviewing never invokes. The condition is now moved-to-shared or merged, which graduating does invoke. - graduating linked back to a proposal it then deletes; now a commit-pinned permalink, which survives retirement.
The glossary was the one document under content/knowledge/ that agents were allowed to read, which made the "knowledge/ is for humans" rule a rule with an exception — the kind an agent has to reason about before applying, and so the kind it gets wrong. Move it to content/glossary.md. Placement now decides the audience: under knowledge/ means human-only, outside it means shared. With the exception gone the rule can be stated plainly, so AGENTS.md now carries it. The layout block lists every subfolder of content/ — index.md, glossary.md, knowledge/, personal/, shared/, templates/ — each with a line saying what it is for, and a dedicated section states that documents under knowledge/ are written for humans: agents must not read, cite, or apply them, and no skill may depend on them. Addresses the review comment that content/knowledge/ was missing from the canonical layout. Declining it earlier left AGENTS.md an incomplete map of content/; describing the directory and forbidding its use is better than omitting it, since an agent that never hears of a directory cannot be told to leave it alone. Skills follow the move: keeping-one-vocabulary bootstraps and indexes content/glossary.md under a new Glossary section, and the skills README states the rule as absolute rather than as a default with case-by-case exemptions.
content/knowledge/ took a nested layout entry, a bullet, and a fourteen-line section to say one thing an agent acts on: do not read it. The detail — what the directory contains, why guidance differs from procedure — is for a person choosing where to put a document, and it is already in .agents/skills/README.md. Now one layout line and one bullet carrying the rule and the placement principle it implies. AGENTS.md is 16 lines shorter.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
.agents/skills/cross-checking-proposals/SKILL.md:33
- This adds Knowledge entries to the candidate list, and step 3 then instructs the agent to open every shortlisted candidate. That directly conflicts with the repository rule that no skill may read
content/knowledge/(AGENTS.md:54-56) and with this PR's stated audience boundary. Restrict candidate documents to the proposal sections.
`content/index.md` is a keyword map precisely so this step is cheap. Intersect the
document's `tags` and problem terms with the index keywords, and read across **all**
sections — Personal, Shared, and Knowledge.
.agents/skills/graduating-a-proposal/SKILL.md:73
- The lifecycle above normally moves a proposal to
content/shared/before graduation to icon4py, so hard-codingcontent/personal/produces a 404 permalink for shared proposals even when the SHA is pinned. Build the permalink from the proposal's actual path at that commit.
(`.../blob/<full-sha>/content/personal/...`), not a branch link. Retirement deletes
- drafting-a-proposal's frontmatter did not parse. The description contained "authoring mechanics: the ...", and ": " inside an unquoted plain scalar is a YAML mapping delimiter, so the file failed with "mapping values are not allowed here" and the skill would not be discovered at all. Replaced the colon with a dash. Checked all five; it was the only one. - cross-checking told the agent to shortlist across "Personal, Shared, and Knowledge" and then open every candidate, which walks straight into the directory no skill may read. Restricted to the sections that hold proposals, and said why a keyword match under Knowledge is not a candidate. The rule was added after this text was written and this is where it leaked. - graduating hard-coded content/personal/ in the permalink it hands to an ADR, but a proposal has normally moved to content/shared/ by the time it graduates, so the pinned link 404s. Build it from the path at that commit.
The reference file existed to keep SKILL.md short by deferring the file-shaped mechanics until step 5. That trade only pays if the deferred material is absent from context until then, and roughly two thirds of it — frontmatter block, status semantics, index entry format, Python version assumptions, publishing notes, most of the file layout — is in AGENTS.md, which CLAUDE.md loads for every agent working here. So it saved nothing and cost a second authoritative copy: the index-entry rules needed the same edit in both files in each of the last two commits. Four things were genuinely only in the reference and are now in SKILL.md: the TL;DR is load-bearing and states the decision, tags reuse existing index vocabulary, wikilinks always carry a label, and `updated:` is an optional frontmatter field. The supersession header example moved to the restructuring section that uses it. Step 5 now points at AGENTS.md for the rest instead of restating it. SKILL.md goes 119 → 137 lines and the 116-line reference is deleted. The skills README records the rule so the next skill does not reintroduce it.
It was never skill guidance — it is a document copied verbatim into content/glossary.md, which is what content/templates/idea.md is for a proposal. Keeping it under .agents/ put the template for the one artifact humans and agents jointly own in the one directory a human would not look in. Folding it into SKILL.md would have been worse than either: it carries its own YAML frontmatter and pipe-escaped tables, so inlining needs an outer fence a copier then has to strip, and the bootstrap branch it serves runs once in the repository's life. content/templates/ already holds skeletons, is already excluded from the published site by ignorePatterns, and is already in the AGENTS.md layout. The skill now copies it exactly as drafting-a-proposal copies idea.md. No skill has a references/ directory now — each is a SKILL.md plus its evals — so the README states that as the shape rather than explaining when an exception is allowed.
Bootstraps content/glossary.md from the template and fills it with the terms whose meaning a newcomer would guess wrong: a GT4Py `program` is one compiled kernel, `domain` is an index range and not the problem domain, `backend` is a compilation target, `embedded` means interpreted-in-Python, a savepoint is a Fortran state dump and not a restart point, `nudging` is a named zone, a granule is a unit of porting. Everyday words used in their everyday sense are deliberately absent. Grouped by origin — GT4Py, ICON, icon4py, CF — because that decides who owns the meaning: upstream terms may not be silently redefined here, icon4py's own are ours to settle. Each entry cites the icon4py module or the proposal that pins it; none is asserted from general knowledge. Three terms are recorded as contested rather than resolved. `component` has four incompatible signatures open at once, `model state` is the same disagreement seen from the data side, and `step` means a composition unit, a `StepInfo`, or the timestep depending on the document. Each lists every meaning with its document and what decision would settle it, per the rule that a glossary asserting a winner nobody agreed to is worse than no glossary. One deprecated alias: orchestration once meant DaCe whole-program compilation, which was removed from icon4py as unmaintained; in current documents it means a driver sequencing components.
The template existed to serve one branch — "if the glossary does not exist" — which ran once, in 3df71fa, and can never run again. Keeping it would leave a skeleton of a document that is now real, and a bootstrap instruction in the skill that no invocation can ever reach. Removed with the section of keeping-one-vocabulary that used it. Two lines from that section were not bootstrap-specific and are kept: the glossary records only terms already in use, and a glossary asserting a winner nobody agreed to is worse than no glossary — the latter moved to the contested-terms step it actually qualifies. AGENTS.md goes back to describing templates/ as the idea skeleton.
Two related additions: a reference document for humans, and the agent skills that
carry this repo's procedures.
1.
content/knowledge/— reference material for humansA new top-level area for material a person consults while deciding how to write
or judge a design. Its first document,
knowledge/software-engineering/principles.md,covers prime directives on complexity and modularity, deep modules and information
hiding, tactical and strategic domain modelling, daily craft, and a red-flag checklist.
It is not a proposal: nothing here is up for adoption in icon4py, it never graduates
to
shared/, and it is not retired when a proposal is implemented.Agents do not read it.
AGENTS.mddocuments the directory and forbids its use —no skill reads, cites, or depends on it. Guidance a human weighs is not a procedure an
agent executes, and an agent reciting a checklist over every proposal produces the
appearance of review rather than review. An earlier revision of this branch made
reading it step 1 of authoring; that was reverted in 3dc48ad.
The rule takes no exceptions, which is what makes it applicable without judgement.
The shared vocabulary that agents read and write therefore sits outside the
directory, at
content/glossary.md(6c10adf). Placement decides the audience.AGENTS.mdnow describes every subfolder ofcontent/—index.md,glossary.md,knowledge/,personal/,shared/,templates/— rather than only the proposaldirectories.
2.
.agents/skills/— repository agent skillsFive skills covering the procedures
AGENTS.mdonly summarizes: drafting a proposal,cross-checking it against what already exists, reviewing it, keeping one vocabulary,
and graduating it to
shared/or to icon4py. Each is self-contained — the standard askill applies lives in the skill — and each ships trigger-query evals so over- and
under-triggering can be measured rather than guessed.
.claude/skillsis a symlink to.agents/skillsso Claude Code discovers them without a second copy.reviewing-a-proposalcarries its own review gate: a requirements pass, a design pass,and a recurring-defect pass naming the failures this repository actually produces —
mechanism-as-requirement, phase-ordered decomposition, a word doing two jobs — with the
instruction to drop any flag that cannot be made concrete in icon4py terms.
Also
model-state_specV2.mdhad a placeholder[Working Principles](https://github.com/)link that already cited the principles by their intended path; it now resolves as a
wikilink. (@jcanton: one line, link only, no content change.)
For reviewers
principles.mdomitsauthor/created/statuson purpose —those are specified for proposals, and this is not one. Easy to add if the group
would rather the vocabulary be uniform across all of
content/.content/glossary.mddoes not exist yet;keeping-one-vocabularybootstraps it fromits template on first use, and
content/index.mdcarries the placeholder section.