20 lines
1.8 KiB
Markdown
20 lines
1.8 KiB
Markdown
|
---
|
||
|
|
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]].
|