Merge pull request 'docs(claude/memory): capture PR-review comment-style feedback' (#72) from docs/claude-memory-pr-comment-style into main

Reviewed-on: #72

home/claude/memory/MEMORY.md

@@ -9,6 +9,8 @@
 - [Jira tooling](jira_tooling.md) — comments are Markdown not wiki; transitions may need assignee; link direction; WSP transition IDs
 - [Jira WSP fields](jira_wsp_fields.md) — WSP field map: issue-type IDs, required Bug fields with allowed values/IDs, Task shortcut, relevant components
 - [Review and comments workflow](workflow_review_and_comments.md) — show PR body and non-trivial Jira comments before posting; terse IaC code comments; PR body content rules
+- [Code comment style](code_comment_style.md) — reviewer feedback: no ticket IDs in comments by default, concise, explain non-obvious why; Helm needs `#` not `{{/* */}}` to render
+- [Copilot review false positives](copilot_review_false_positives.md) — verify Copilot "this breaks X" claims against spec/live config before acting; two recorded Terraform false positives
 - [Sandbox prompts](feedback_sandbox_prompts.md) — don't prompt for sandbox-disable or routine read-only shell ops; broaden permissions instead
 - [Dev clusters disposable](dev_clusters_disposable.md) — Lyra's dev clusters are recreatable; mutate/break freely, no confirmation needed
 - [Nix shell tooling](nix_shell_tooling.md) — any nixpkgs tool runs ad hoc via `nix run`/`nix shell nixpkgs#<pkg>`; a missing command is never a dead end

home/claude/memory/code_comment_style.md

@@ -0,0 +1,19 @@
+---
+name: code_comment_style
+description: "Code/comment style from PR review feedback: no ticket IDs in comments by default, concise, explain the non-obvious why"
+metadata:
+  node_type: memory
+  type: feedback
+  originSessionId: 59e09a3f-1429-4f68-a5fb-9af4390e9b0d
+---
+
+Recurring PR-review feedback from human reviewers (Tom Wilkins, Andrew Hyde, Gilberto Pestanarosa) on the `multicluster` and `unified-helm` repos, on how to write comments in code and IaC:
+
+- **No Jira/WSP ticket IDs in code comments or WAF `msg:` strings by default.** Add a ticket ref only when there is a specific reason to. Never duplicate the id, and never put a ticket URL in a comment. Tracking/rationale belongs in the PR description and the Jira ticket, not in `.tf`, `.tftpl`, or `.yaml`. (Flagged repeatedly — PRs #1735, #1762.)
+- **Comment the non-obvious "why", not the obvious "what".** Drop comments that restate what the code or file plainly does (e.g. a header on `namespace.yaml` re-announcing that it defines a namespace). If a reviewer can't tell why a comment exists, it shouldn't.
+- **Keep it short and readable.** No multi-line block where one line does; if a comment isn't clear after a couple of reads, rewrite it plainer. Prefer trimming to the single load-bearing sentence over hedged prose. (PRs #1745, #216.)
+- **In Helm charts, use `#` YAML comments — not `{{/* */}}` — for anything that must appear in the rendered manifest.** Helm template comments are stripped before render, so port/label explanations meant for the live chart have to be `#`. (PR #216.)
+
+**Why:** Multiple human reviewers, across multiple PRs, consistently push back on verbose comments and gratuitous ticket references. Terse, purpose-driven comments clear review faster.
+
+**How to apply:** When writing or editing comments in code/IaC, default to: no ticket id, one line, non-obvious "why" only. This supersedes the "one-liner + WSP ticket reference" phrasing that used to live in [[workflow-review-and-comments]]. Relates to [[docs_keep_updated]].

home/claude/memory/copilot_review_false_positives.md

@@ -0,0 +1,19 @@
+---
+name: copilot_review_false_positives
+description: "Verify Copilot PR-review 'this breaks X' claims against spec/live config before acting; two recorded false positives"
+metadata:
+  node_type: memory
+  type: feedback
+  originSessionId: 59e09a3f-1429-4f68-a5fb-9af4390e9b0d
+---
+
+The Copilot reviewer on the `multicluster` / `unified-helm` repos raises blocking-sounding "this will fail" claims that are sometimes wrong. Verify against the language spec and the live/`master` config before treating one as real or applying its fix.
+
+Recorded false positives (both Terraform, both Emma-flagged "for future reference"):
+
+- **PR #1742** — claimed `var.map.hyphenated-key` dot access is parsed as subtraction and breaks `terraform plan`. False: HCL2 identifiers may contain hyphens (`ID_Start (ID_Continue | "-")*`), and the same pattern is already live on `master` in prod. Bracket indexing was adopted anyway as marginally clearer, not as a fix.
+- **PR #1745** — claimed the `aks_pools` per-pool `max_surge` lookup was off-by-one and should use `count.index + 1`. False: every config attribute on that resource indexes with `count.index`; only the cosmetic `name`/`az_nodepool` label uses `+1`. Applying `+1` would have introduced a real bug (wrong pool, and out-of-bounds on the last pool).
+
+**Why:** Blindly applying a plausible-but-wrong Copilot suggestion can introduce a real defect or waste review cycles.
+
+**How to apply:** For any Copilot claim that code is broken or unsafe, confirm it against the relevant spec and the existing working config first; if it's wrong, say so plainly on the PR and leave the code. Genuine Copilot catches (over-broad WAF `@beginsWith`, missing input validation, doc/behaviour drift) still get fixed. Relates to [[workflow-review-and-comments]] and [[code_comment_style]].

home/claude/memory/workflow_review_and_comments.md

@@ -11,7 +11,7 @@ metadata:
 
 **Show non-trivial Jira comments before posting:** Same rule for any non-trivial public Jira comment — paste the proposed body in chat first when there is any doubt about content.
 
-**Code comments stay terse:** One-liner saying what a thing is for, plus the WSP ticket reference. Full rationale lives in the Jira ticket or commit/PR description — not in `.tf`, `.tftpl`, or `.yaml` files. See [[git-conventions]].
+**Code comments stay terse:** One line on the non-obvious _why_, and **no Jira/WSP ticket id by default** — add one only when specifically warranted. Full rationale lives in the Jira ticket or commit/PR description, not in `.tf`, `.tftpl`, or `.yaml` files. Reviewers repeatedly strip gratuitous ticket refs and verbose comments; see [[code_comment_style]] for the full rule set and [[git-conventions]].
 
 **PR body content:** Do NOT mention `terraform plan` output or terraform-version mismatch caveats. Stick to: what changed, why, and validation results.
 
@@ -19,4 +19,4 @@ metadata:
 
 **Why:** Lyra reviews everything Claude publishes externally before it goes out; terraform-version noise in PR descriptions is unhelpful clutter.
 
-**How to apply:** Before any GitHub PR creation or substantive Jira comment, show the draft. When writing code comments in IaC files, keep to one-liner + ticket ref.
+**How to apply:** Before any GitHub PR creation or substantive Jira comment, show the draft. When writing code comments in IaC files, keep to a one-line non-obvious _why_ with no ticket id by default ([[code_comment_style]]).