chore: codify HOW-only comment policy for agents#6394
Conversation
There was a problem hiding this comment.
Pull request overview
This PR codifies a repository-wide policy for agents to avoid “why”/reasoning/narration comments in code and instead route rationale into human-authored ADRs, with explicit carve-outs for required structured comments.
Changes:
- Adds an explicit “HOW-only comments” rule to root
AGENTS.mdCritical Rules, including allowed structured-comment carve-outs. - Introduces repo-wide Copilot guidance via new
.github/copilot-instructions.md. - Updates
docs/AGENTS.mdto state agents must not proactively author/amend ADRs and should surface ADR-candidate rationale to humans.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
docs/AGENTS.md |
Adds guidance about when agents may (and may not) draft/amend ADRs while working under docs/. |
AGENTS.md |
Adds a new Critical Rule forbidding reasoning/narration comments and directing rationale to human-authored ADRs. |
.github/copilot-instructions.md |
New repo-wide Copilot instructions mirroring the HOW-only comment policy and ADR routing. |
|
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
hisImminence
left a comment
There was a problem hiding this comment.
crev review
Specialists run: correctness, devils-advocate. Devil's-advocate hypotheses: 0 raised, 0 promoted.
Findings on lines outside this PR's diff:
- P1
.github/instructions/helm-templates.instructions.md:221— Existing path-scoped instruction requires 'why' in NOTE comments — direct conflict with new HOW-only policy
helm-templates.instructions.mdsays: "always add a{{- /* NOTE: ... */ -}}comment explaining why." The new rule in AGENTS.md and copilot-instructions.md explicitly prohibits 'why' comments. The NOTE convention appears in the carve-out list (so it survives the ban in principle), but the path-scoped instruction still directs agents that NOTE comments should explain why — contradicting the HOW-only framing. An agent with both files in scope receives opposite directives on the same construct. This PR should either (a) updatehelm-templates.instructions.mdto say "explaining the non-obvious constraint or backward-compat reason" instead of "explaining why", or (b) explicitly state in the carve-out that NOTE comments may carry rationale as a named exception.
| - NEVER edit golden files by hand — use `make go.update-golden-only chartPath=...`. | ||
| - NEVER implement CI logic (>20 lines) in bash — use Go scripts in `scripts/` with unit tests. | ||
| - NEVER write reasoning/"why"/narration comments — comments explain only non-obvious HOW. Rationale and decisions belong in an ADR under `docs/adr/`, human-authored; agents NEVER create or edit ADRs automatically or unprompted. Keep only required structured comments: Apache license headers, `## @param`/`## @extra`, the `{{- /* NOTE */ -}}` helper convention, and lint/build pragmas (`//nolint`, `//go:build`, `# yamllint disable`). | ||
| - ALWAYS run `make helm.dependency-update chartPath=...` before testing/linting a chart. |
There was a problem hiding this comment.
P1 · Pragma carve-out omits # yamllint disable-line — agents may remove valid single-line suppressions (via correctness)
The permitted-comment carve-out lists # yamllint disable (block form) but not # yamllint disable-line (single-line suppression). An agent enforcing this rule literally would treat # yamllint disable-line rule:... as a prohibited narration comment and strip it. Both forms are standard yamllint pragma patterns in this repo. The same gap exists in .github/copilot-instructions.md line 12. Additionally, # shellcheck disable used in bash scripts is absent from the exemption list. Fix: change # yamllint disable to # yamllint disable[-line] (or list both), and add # shellcheck disable to the pragmas carve-out.
lint/build pragmas (`//nolint`, `//go:build`, `# yamllint disable`).
| the maintainer-guide table), ask before drafting. | ||
|
|
||
| Never author or amend an ADR proactively — only when a human explicitly asks. | ||
| If you notice rationale that would otherwise become a "why" code comment, surface it to the human as a candidate ADR; only draft/amend the ADR if explicitly asked. |
There was a problem hiding this comment.
P1 · docs/AGENTS.md permits ADR drafting on request; root AGENTS.md says NEVER create/edit ADRs unprompted — ambiguous authority (via devils-advocate)
Root AGENTS.md new rule reads: "agents NEVER create or edit ADRs automatically or unprompted." docs/AGENTS.md new addition reads: "only draft/amend the ADR if explicitly asked." The qualifier mismatch matters: 'unprompted' in the root rule can be read as an absolute ban (since even an explicit human ask arrives via a prompt), while 'if explicitly asked' in docs/AGENTS.md clearly permits drafting on request. An agent with both files in scope will have contradictory authority on the question "can I draft an ADR when the human asks?" The root rule's intent appears to be no-proactive-creation, but the word choice creates the ambiguity. Fix: change root AGENTS.md to "NEVER create or edit ADRs proactively" to align with docs/AGENTS.md.
Never author or amend an ADR proactively — only when a human explicitly asks.
If you notice rationale that would otherwise become a "why" code comment, surface it to the human as a candidate ADR; only draft/amend the ADR if explicitly asked.
| - NEVER edit golden files by hand — use `make go.update-golden-only chartPath=...`. | ||
| - NEVER implement CI logic (>20 lines) in bash — use Go scripts in `scripts/` with unit tests. | ||
| - NEVER write reasoning/"why"/narration comments — comments explain only non-obvious HOW. Rationale and decisions belong in an ADR under `docs/adr/`, human-authored; agents NEVER create or edit ADRs automatically or unprompted. Keep only required structured comments: Apache license headers, `## @param`/`## @extra`, the `{{- /* NOTE */ -}}` helper convention, and lint/build pragmas (`//nolint`, `//go:build`, `# yamllint disable`). | ||
| - ALWAYS run `make helm.dependency-update chartPath=...` before testing/linting a chart. |
There was a problem hiding this comment.
P2 · Policy routes all rationale to ADRs, but ADRs are scoped to architectural decisions — creates a documentation black hole for tactical rationale (via devils-advocate)
The new rule says: "Rationale and decisions belong in an ADR under docs/adr/." However, the repo's maintainer guide restricts ADRs to architectural decisions — bug-fix defaults, timeout values, label exclusions, and similar tactical choices explicitly do not warrant ADRs. For that large class of non-architectural rationale, the policy simultaneously forbids 'why' comments AND makes an ADR disproportionate, leaving no sanctioned place for the rationale to live. The PR should either (a) qualify the statement to "architectural rationale belongs in an ADR," or (b) identify where tactical/non-architectural rationale should go (commit message, PR description, etc.).
NEVER write reasoning/"why"/narration comments — comments explain only non-obvious HOW. Rationale and decisions belong in an ADR under `docs/adr/`
hisImminence
left a comment
There was a problem hiding this comment.
See comments, please fix, then 👍
Which problem does the PR fix?
Closes #6393
Agents add "why"/reasoning/narration comments to code; the repo's instruction files didn't forbid it (the
github-actions.instructions.mdguide was itself introduced with such boilerplate). Rationale belongs in ADRs, not code comments.What's in this PR?
AGENTS.mdCritical Rules:NEVERwrite reasoning/"why"/narration comments — HOW-only; rationale → ADR (docs/adr/), human-authored; agents never auto-create/edit ADRs. Carve-outs for required structured comments (license headers,## @param,{{- /* NOTE */ -}}, lint/build pragmas)..github/copilot-instructions.md(new): same rule, repo-wide for Copilot.docs/AGENTS.md: agents never proactively author ADRs.Checklist
Please make sure to follow our Contributing Guide.
Before opening the PR:
make go.update-golden-only.After opening the PR: