From 46a3e6f7be83ce39ae58214d38d9350f0597ea60 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Thu, 23 Jul 2026 18:52:09 -0700 Subject: [PATCH 1/2] Codify comment layout in the Documentation Style Conventions (#419) Add the multi-line comment layout rules (one sentence per line, never wrap, marker-agnostic '-' sub-bullets for parallel sub-topics only) with two worked examples, adopted from ESPHome-Config's local section. Fold the redundant line-length bullet into the sentence-per-line rule. --- AGENTS.md | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index dd8abc93..626d7ccb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -144,10 +144,27 @@ Applies to code and workflow (`#`) comments alike. - Comment only when the code does not explain itself or the logic is genuinely complex. Self-evident code needs no comment. - Write for the human reading *this* project's code now: state only the non-obvious *why*. No cross-project references (do not name other repos), no historic or design narrative, no rule citations - governance lives in this file, not echoed inline. -- **Keep it short. One line is the default; a comment earns a second line only by carrying a constraint the code cannot.** Most comments are one sentence. Don't restate *what* the code does - a well-named symbol already says it. +- **Keep it short.** One line is the default. A comment earns a second line only by carrying a constraint the code cannot. Most comments are one sentence, and never restate *what* the code does - a well-named symbol already says it. +- **Structured, not prose: one sentence per line, and never wrap a sentence across lines.** No block paragraphs and no multi-sentence run-ons. A comment that genuinely needs several sentences is several lines, each a single sentence. A sentence too long for one sensible line is too long - split the thought. +- **A multi-line comment shows whether it is a continuation or a list.** A continuation of the same topic stays unindented, one sentence per line. Mark a sub-topic with a `-` after the comment marker (`# -`, `// -`), and only for genuine sub-topics - parallel items hanging off a lead line, never a continuation of one thought. - **No class-, type-, or file-header summary comment blocks.** A type or file gets a comment only for a specific non-obvious point, kept terse - never a block summarizing what the file contains or what the class is for. A summary restates the declaration below it, goes stale as the file grows, and is the file-scope form of the design narrative and verbosity creep this section already bans. A license or provenance header a tool or policy requires is not a summary and is unaffected. - **Do not grow a comment across edits.** When you touch code near an existing comment, the comment must come out **same length or shorter** - never append "one more clause" of rationale. If a block comment has crept to multiple sentences of prose, cut it back to its single load-bearing point as part of your change. Verbosity creep is the specific regression to prevent: every iteration that adds a clause is a regression, not an improvement. -- Match the surrounding code's line length (typically ~120), not an 80-column wrap. + +A continuation stays unindented, one sentence per line: + +```text +# Change gate for the compile tests. +# An esp-idf build costs minutes, so gate on what each test covers. +# A diff that cannot be computed runs everything. +``` + +Sub-topics take a `-` after the comment marker, each elaborating a distinct item named in the lead: + +```text +# Source lint plus change-gated compile tests. +# - compile-test builds the external component. +# - template-compile-test builds one example device per template. +``` ### Character Set From 970b0d733f203b04c5244ebe97f4a6f13ae6ff4a Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Thu, 23 Jul 2026 21:38:45 -0700 Subject: [PATCH 2/2] Define per-section fidelity for AGENTS.md; make fleet-law verbatim (#420) Classify all 18 AGENTS.md sections (16 verbatim incl one operational-scoped, 2 intent), declare each fidelity in files.json, add spec/section-model.md as the ground-truth classification + review-weight principle, generalize the Branching Model SHA sentence, and update fidelity-model.md's rationale. ~90% of AGENTS.md is now byte-checked fleet-wide. --- AGENTS.md | 2 +- spec/fidelity-model.md | 3 ++- spec/files.json | 4 +-- spec/section-model.md | 61 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 66 insertions(+), 4 deletions(-) create mode 100644 spec/section-model.md diff --git a/AGENTS.md b/AGENTS.md index 626d7ccb..901084cb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -39,7 +39,7 @@ A state-changing GitHub call is the highest-blast-radius thing an agent does her - `develop` is the integration branch. Feature branches -> `develop` is **squash-only**; develop is kept linear. - `develop` -> `main` is **merge-commit only** (no squash, no rebase). Merge commits preserve develop's commit list as a real second-parent reference on main, which lets the release model attribute releases to the develop commits that produced them (see "Release Model" below). Branch protection enforces this: the develop ruleset allows only `squash`, the main ruleset allows only `merge`. - All commits on both branches must be cryptographically signed (SSH or GPG). Squash and merge commits created via the GitHub UI are signed by GitHub's web-flow key. -- **`develop` is forward-only - no `main -> develop` back-merges.** The develop ruleset's squash-only setting physically blocks merge commits on develop. Historical back-merge commits visible in `git log` (`b9b0447`, `410ba56`, `ffb9e64`, `5ce95cf`, etc.) predate this rule and must not be repeated. +- **`develop` is forward-only - no `main -> develop` back-merges.** The develop ruleset's squash-only setting physically blocks merge commits on develop. Any historical back-merge commits in `git log` predate this rule and must not be repeated. - **Executing a `develop -> main` promotion safely - two traps, both learned the hard way:** - **Never delete `develop`.** A promotion PR's head *is* `develop`, so `gh pr merge --delete-branch` (and the repo's "Automatically delete head branches" toggle, which is why that toggle is [kept off](./repo-config/settings.json)) deletes `develop` itself. Merge a promotion with a plain `gh pr merge --merge`, no `--delete-branch`. If `develop` is ever lost this way, restore it to the merged PR's head SHA - the SHA is still reachable as the merge commit's second parent: `gh api -X POST "repos///git/refs" -f ref=refs/heads/develop -f sha="$(gh pr view --json headRefOid --jq .headRefOid)"`. - **Spurious EOL-only conflicts resolve by taking `develop`.** When develop declared workflow YAML as LF while main is still CRLF, `develop -> main` conflicts *whole-file* on those paths. develop's `required_linear_history` + PR rulesets forbid resolving on `develop` (no merge commit, no force-push), so resolve on a throwaway branch off `main`: `git checkout -b promote/develop-to-main origin/main && git merge origin/develop`, take develop's side for the EOL-conflicted files (`git checkout --theirs `) **after confirming each is content-identical modulo EOL or that develop is a strict superset** (`diff <(git show :2:f|tr -d '\r') <(git show :3:f|tr -d '\r')`), then open that branch -> `main`. Verify no genuine main-only content is dropped (build/test where the repo supports it). diff --git a/spec/fidelity-model.md b/spec/fidelity-model.md index 145a3f37..8143f3b8 100644 --- a/spec/fidelity-model.md +++ b/spec/fidelity-model.md @@ -19,7 +19,7 @@ Fidelity is a declared field defaulting to `presence`, never inferred from `whol ## Why Each Unit Sits Where It Does -- **verbatim** - `.markdownlint-cli2.jsonc` (fleet-generic, no governed divergence), the `github-release` job region of the release task (the canonical orchestration a repo must not fork), and the universal rule sections of `AGENTS.md` (`Repository Boundaries and Write Safety`, `Git and Commit Rules`, `Verification Discipline`) - fleet-law with no repo-specific content (no SHAs, no `ptr727/` references), where a paraphrase or a missing rule is a defect, not an adaptation. The rest of `AGENTS.md` stays intent because it carries repo-specific content (the `Branching Model` cites this repo's own historical SHAs, others carry project-type examples). +- **verbatim** - `.markdownlint-cli2.jsonc` (fleet-generic, no governed divergence), the `github-release` job region of the release task (the canonical orchestration a repo must not fork), and the universal rule sections of `AGENTS.md` - fleet-law with no repo-specific content (no SHAs, no `ptr727/` references), where a paraphrase or a missing rule is a defect, not an adaptation. Which `AGENTS.md` sections are verbatim and which stay intent (the few that describe one repo) is defined in [section-model.md][section-model]. - **interface** - the release and PR workflows. Their fixed contract is the job and check names plus the artifact handoff, while the leaf build jobs are owned. See the override seam in [`AGENTS.md`][agents]. - **intent** - `.editorconfig` and `.gitattributes` (the `[*] end_of_line` default and path pins vary by platform), `cspell.json` (the words list and file scope vary), `CODESTYLE.md` / `WORKFLOW.md` / `AUDIT.md` / `.github/copilot-instructions.md` (carried docs judged by meaning), and the ruleset payloads (whose live state is diffed separately). - **presence** - `README.md`, `HISTORY.md`, `.gitignore`, and the per-repo config that only needs to exist. @@ -41,4 +41,5 @@ A verbatim mismatch is one of two things, told apart **by hash, not by a version [audit]: ./audit.py [files]: ./files.json [files-schema]: ./files.schema.json +[section-model]: ./section-model.md [scope-model]: ./scope-model.md diff --git a/spec/files.json b/spec/files.json index c4678bbb..692e4cb1 100644 --- a/spec/files.json +++ b/spec/files.json @@ -1,8 +1,8 @@ { "$schema": "./files.schema.json", - "note": "The standardization baseline: files and sections a fleet repo is expected to carry, and their intent authority. The audit mechanically checks presence (letter). Equivalence (intent) is judged by hand, and a section for an absent language or target is N/A. Each entry, and each section, carries an appliesTo selector - see spec/scope-model.md for the scope model and selector vocabulary. Each entry also has a fidelity (presence by default, or intent, verbatim, interface) governing how faithfully the content is checked - see spec/fidelity-model.md.", + "note": "The standardization baseline: files and sections a fleet repo is expected to carry, and their intent authority. The audit mechanically checks presence (letter). Equivalence (intent) is judged by hand, and a section for an absent language or target is N/A. Each entry, and each section, carries an appliesTo selector - see spec/scope-model.md for the scope model and selector vocabulary. Each entry also has a fidelity (presence by default, or intent, verbatim, interface) governing how faithfully the content is checked - see spec/fidelity-model.md. The per-section fidelity of AGENTS.md, which sections are verbatim fleet-law and which are repo-specific, is defined in spec/section-model.md.", "baseline": [ - { "path": "AGENTS.md", "fidelity": "intent", "sections": [{ "name": "Repository Boundaries and Write Safety", "fidelity": "verbatim" }, { "name": "Git and Commit Rules", "fidelity": "verbatim" }, "Branching Model", "Release Model", { "name": "Operational Repositories", "appliesTo": ["operational"] }, "Pull Request Title and Commit Message Conventions", "Documentation Style Conventions", { "name": "Verification Discipline", "fidelity": "verbatim" }, "PR Review Etiquette", "Workflow YAML Conventions"], "intentRef": "AGENTS.md", "appliesTo": "*" }, + { "path": "AGENTS.md", "fidelity": "intent", "sections": [{ "name": "Foundational Principles", "fidelity": "verbatim" }, { "name": "Repository Boundaries and Write Safety", "fidelity": "verbatim" }, { "name": "Git and Commit Rules", "fidelity": "verbatim" }, { "name": "Branching Model", "fidelity": "verbatim" }, { "name": "Release Model", "fidelity": "verbatim" }, { "name": "Operational Repositories", "fidelity": "verbatim", "appliesTo": ["operational"] }, { "name": "Repository Onboarding and Conformance", "fidelity": "verbatim" }, { "name": "Pull Request Title and Commit Message Conventions", "fidelity": "verbatim" }, { "name": "Documentation Style Conventions", "fidelity": "verbatim" }, { "name": "Verification Discipline", "fidelity": "verbatim" }, { "name": "PR Review Etiquette", "fidelity": "verbatim" }, { "name": "Communicating with the User", "fidelity": "verbatim" }, { "name": "Workflow YAML Conventions", "fidelity": "verbatim" }, { "name": "Supported Development Platforms", "fidelity": "verbatim" }, { "name": "Devcontainer", "fidelity": "intent" }, { "name": "Editor and Tasks", "fidelity": "verbatim" }, { "name": "Repository Details", "fidelity": "verbatim" }, { "name": "Repository Layout", "fidelity": "intent" }], "intentRef": "AGENTS.md", "appliesTo": "*" }, { "path": "CODESTYLE.md", "fidelity": "intent", "whole": true, "placeholders": ["InternalsVisibleTo project names"], "intentRef": "CODESTYLE.md", "appliesTo": "*" }, { "path": "WORKFLOW.md", "fidelity": "intent", "whole": true, "intentRef": "WORKFLOW.md", "appliesTo": "*" }, { "path": "README.md", "appliesTo": "*" }, diff --git a/spec/section-model.md b/spec/section-model.md new file mode 100644 index 00000000..9b1f2eaf --- /dev/null +++ b/spec/section-model.md @@ -0,0 +1,61 @@ +# AGENTS.md Section Model + +Companion to [fidelity-model.md][fidelity-model]. That doc defines how a carried *file* is verified. This one defines how the *sections* of `AGENTS.md` carry, and records the fidelity of each. It is the ground truth an agent or human consults before adding, removing, or re-typing a section, not a judgment re-derived each session. + +## Why sections have fidelity + +`AGENTS.md` is the fleet's cross-cutting rule book, and its sections are not equal. Most state a rule that is identical in every repo. A few describe the repo they live in. Treating them all as carried-by-intent is what lets a rule silently paraphrase, weaken, or vanish, which is the recurring drift this model exists to stop. So each section carries a declared fidelity, checked by the audit against the hub canonical. + +## The categories + +A section is one of the following. Fidelity is declared in [files.json][files], never inferred. + +- **verbatim** - universal fleet-law rule *text*, byte-identical in every repo after EOL and action-pin normalization. The audit content-hashes each downstream copy against the hub's `## ` block. A verbatim section may carry clauses only some repos exercise (for example "a source-only repo carries no build task"). The *text* is still identical everywhere - applicability is per-repo, the wording is not. +- **intent** - the section *describes this particular repo* (its own directory tree, its own devcontainer and toolchain), so its content legitimately varies. The heading must be present, the body is judged by meaning rather than hashed. + +`appliesTo` scope is orthogonal to fidelity. A section may apply to a subset of repos (for example `operational`) and is checked only for repos in that subset. + +**The default is verbatim.** `AGENTS.md` holds rule text, and rule text is universal. The repo-specific *values* live in other files (`.editorconfig`, `.devcontainer/`, the real tree), never in this prose. `intent` is the deliberate exception for a section that is inherently a description of one repo. + +## The classification + +| Section | Fidelity | Reason | +| --- | --- | --- | +| Foundational Principles | verbatim | the governing rationale, universal | +| Repository Boundaries and Write Safety | verbatim | universal write-safety law | +| Git and Commit Rules | verbatim | universal git law | +| Branching Model | verbatim | universal (repo-specific history SHAs removed so it can carry) | +| Release Model | verbatim | universal release contract, all target shapes described inline | +| Operational Repositories | verbatim, `appliesTo: operational` | universal for operational repos, N/A for release repos | +| Repository Onboarding and Conformance | verbatim | universal audit and registry obligations | +| Pull Request Title and Commit Message Conventions | verbatim | universal, with generic examples | +| Documentation Style Conventions | verbatim | all rule text, including the Line Endings *rule* (a repo's `.editorconfig` value is not here) | +| Verification Discipline | verbatim | universal verification law | +| PR Review Etiquette | verbatim | universal review-loop contract | +| Communicating with the User | verbatim | universal | +| Workflow YAML Conventions | verbatim | universal conventions, applied to whatever workflows a repo has | +| Supported Development Platforms | verbatim | universal | +| Editor and Tasks | verbatim | universal editor convention (standard set plus per-language additions) | +| Repository Details | verbatim | universal About-panel convention | +| Devcontainer | intent | describes this repo's toolchain and devcontainer, genuinely per-repo | +| Repository Layout | intent | describes this repo's directory tree, genuinely per-repo | + +## Changing the structure carries review weight + +The set of sections, and each section's fidelity, is itself governed. + +- **Adding a section** is a fleet-wide act - it declares a new rule every repo must carry. Add it to `AGENTS.md`, to `files.json`, and to the table above in the same change, with its fidelity chosen deliberately. +- **Changing a verbatim section** re-vendors it across the whole fleet. The audit drift wave that follows is the mechanism working, not a regression. +- **Changing a section's fidelity** (intent to verbatim or back) is a governance decision, recorded here with its reason. +- **A downstream repo's extra section** the hub does not declare is drift to reconcile, not a local liberty. Either the rule belongs fleet-wide, so promote it here, or it is repo-specific content that does not belong in a carried section. + +## Enforcement + +`files.json` declares each section's fidelity. [validate.py][validate] proves every declared verbatim section resolves in the hub `AGENTS.md`. [audit.py][audit] checks each repo's copy - presence for `intent`, byte-match for `verbatim` - and classifies a mismatch as stale (re-vendor) or modified (review). + + + +[audit]: ./audit.py +[fidelity-model]: ./fidelity-model.md +[files]: ./files.json +[validate]: ./validate.py