review-pr-local: make report_error! form audit mandatory (APP-5522)

.agents/skills/review-pr-local/SKILL.md

@@ -22,13 +22,25 @@ skill marks as overridable.
 - When a PR is clearly a V0 or initial implementation, frame robustness suggestions such as timeouts, retries, and lifecycle management as optional future work rather than blocking concerns, unless they risk correctness, security, data loss, or a persistent UI hang.
 - For Rust changes, apply the repository conventions from `AGENTS.md`: avoid unnecessary type annotations, prefer imports over long path qualifiers, name context parameters `ctx` and place them last, remove unused parameters instead of prefixing them with `_`, and prefer inline format arguments in macros.
 - Audit the comments a PR adds or changes against the "Comments" guidance under "Development Guidelines" in `AGENTS.md` — comments carry a maintenance cost, so a new comment should earn its place. Check each added/changed comment individually against every named sub-rule (Minimalist Comments, Strictly "Why" Only, No Line-by-Line Narrations, Clean Docstrings, Single-source of documentation, Don't enumerate function call sites, No "transformation comments") rather than forming one overall impression of the comment's quality. Common issues to flag: comments that restate what the code already says instead of explaining non-obvious *why*; "transformation" comments that describe the edit rather than the current state (e.g. "this used to ..."); doc comments that narrate a function's internal steps or enumerate its callers; explanations duplicated at a call site or reference that the declaration's doc comment already covers; and existing comments removed as collateral of an otherwise unrelated change. Read the full list in `AGENTS.md` rather than relying on these examples alone. A comment that is technically accurate, well-written, or explains a subtle/important issue is not exempt from these rules — do not let those qualities substitute for the rule-by-rule check. Treat a confirmed violation as `⚠️ [IMPORTANT]`, not a nit.
-- When a PR adds or changes calls to log macros (`log::*` / `safe_*`) or error reporting (`report_error!` / `report_if_error!`), review it against `.agents/skills/logging-and-error-reporting/SKILL.md`. Common issues to flag: using `log::error!` for a failure that should be a Sentry issue (only `report_error!` and panics create issues — `log::*` at Error/Warn/Info are just breadcrumbs); interpolating per-instance data into a `report_error!` grouping message instead of `.context()`/`extra:`; demoting a real typed error into `extra:` or stringifying it with `anyhow!("{e}")` instead of reporting it as the payload; an inappropriate log level for hot paths; and secrets/PII in reports or Info-and-above logs (use the `safe_*` macros for sensitive detail).
+- When a PR adds or changes calls to log macros (`log::*` / `safe_*`), review the level choice against `.agents/skills/logging-and-error-reporting/SKILL.md`: using `log::error!` for a failure that should be a Sentry issue (only `report_error!` and panics create issues — `log::*` at Error/Warn/Info are just breadcrumbs), an inappropriate level for hot paths, and secrets/PII in Info-and-above logs (use the `safe_*` macros for sensitive detail). For `report_error!` / `report_if_error!` calls, run the mandatory audit below instead of relying on a narrative pass.
 - Avoid wildcard `_` match arms when an enum can reasonably be matched exhaustively; exhaustive matches are preferred so future variants are surfaced during review.
 - For new or changed feature flags, prefer high-level runtime checks with `FeatureFlag::YourFlag.is_enabled()` over `#[cfg(...)]` unless the code cannot compile without a compile-time gate.
 - Flag nested or redundant `TerminalModel` locking when the call stack may already hold the model lock. Prefer passing locked references down the stack and keeping lock scopes short.
 - In WarpUI code, flag inline `MouseStateHandle::default()` usage during render or event handling. Mouse state handles should be created during construction and then cloned/referenced where needed.
 - For user-facing UI changes, mention missing validation only when it is tied to a concrete risk or when the PR changes behavior that should be verified visually.
 
+## Pre-Verdict Audit: error-reporting form
+
+This specializes the core skill's Pre-Verdict Audit (error-reporting category). Whenever the diff adds or changes a `report_error!` or `report_if_error!` call, this audit is mandatory, no matter how large the diff is — a holistic read-through is not sufficient, and skimming past most of a large migration is exactly how the mass `log::error!` → `report_error!` migration merged this form of bug undetected.
+
+Before drafting the body or choosing a verdict: list every `report_error!` / `report_if_error!` call the diff adds or changes, one by one with its file:line. For each one, check it against `.agents/skills/logging-and-error-reporting/SKILL.md` (rules 1–5 and the Anti-patterns block define the exact forms; this list is a lookup index, not a restatement) for:
+- a real, typed error demoted into `extra:` instead of reported as the payload
+- a typed error stringified into the grouping message (`anyhow!("{e}")` / `"{e:?}"`) instead of preserved via `.context()` / `anyhow::Error::new`
+- per-instance/variable data interpolated into the grouping message instead of carried via `.context()` / `extra:`
+- the same failure reported more than once instead of once at the sink ("Report once, at the sink")
+
+Also confirm hot/per-frame or per-message paths use `ReportErrorLogMode::OncePerRun` where the skill calls for it. Treat a confirmed violation as `⚠️ [IMPORTANT]`. The enumerated list is the evidence this audit ran — do not substitute a summary like "spot-checked the report_error! sites."
+
 ## Behavioral or UI-impacting changes require visual evidence
 
 - If the PR changes anything user-visible (UI components, layout, styling, copy in surfaces users see, terminal/Warp app visuals, or other behavior a user can perceive), analyze both `pr_description.txt` and any PR comments available in the workflow context for attached screenshots, GIFs, or videos demonstrating the change end to end.