Skip to content

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

Merged
acarl005 merged 1 commit into
masterfrom
factory/app-5522-error-reporting-form-audit
Aug 19, 2026
Merged

review-pr-local: make report_error! form audit mandatory (APP-5522)#15296
acarl005 merged 1 commit into
masterfrom
factory/app-5522-error-reporting-form-audit

Conversation

@warp-agent-staging

@warp-agent-staging warp-agent-staging Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Description

Review-prompting fix for APP-5522 (the code-side report_error! form fixes are a separate PR). .agents/skills/review-pr-local/SKILL.md already named the "typed error demoted into extra:" anti-pattern (added in #13483 itself), but only as one prose bullet inside a long list — so it was easy to under-apply on that PR's 345-file, ~974-report_error!-line diff, which is exactly what happened: the bot's review found 2 unrelated compile bugs and zero form findings, including on the very lines (crates/warpui_core/src/runtime/mod.rs:813 and :709) that used the forbidden form.

Companion PR (adds the matching mandatory-audit category to the core skill): warpdotdev/common-skills#76

This is a review-prompting-only change (skill markdown). No Rust code is touched; a separate PR fixes the flagged report_error! call sites themselves.

  • The linked issue is labeled ready-to-spec or ready-to-implement.
  • N/A — no screenshots/video; this is a skill-markdown change with no UI impact.

Linked Issue

APP-5522

What a reviewer would now do differently on PR #13483's diff

Before: the reviewer read "review it against .agents/skills/logging-and-error-reporting/SKILL.md" as one bullet among a dozen others, with no requirement to look at every call site individually — easy to satisfy with a spot-check on a mega-diff. After: a dedicated ## Pre-Verdict Audit: error-reporting form section (mirroring the core skill's Comments/Tests audit, added in the companion PR) makes it mandatory, before drafting the body or verdict, to list every report_error!/report_if_error! call the diff adds or changes, one by one with file:line, and check each against the four named forbidden forms. That per-line enumeration — not a narrative pass — is what would have surfaced the extra: { "error" => %error } demotion at runtime/mod.rs:813/:709 before merge.

Testing

Docs-only change (.agents/skills/review-pr-local/SKILL.md). No Rust code changed, so cargo clippy doesn't apply; ./script/format was run and made no additional changes.

  • I have manually tested my changes locally with ./script/run — N/A, no runtime behavior; this only changes a skill markdown file read by the review agent.

Screenshots / Videos

N/A — not a UI change.

Agent Mode

  • Warp Agent Mode - This PR was created via Warp's AI Agent Mode

Add a dedicated Pre-Verdict Audit section that requires enumerating
one with file:line, and checking each against
logging-and-error-reporting/SKILL.md for the forbidden forms (real
error demoted into extra:, typed error stringified into the grouping
message, variable data interpolated into the grouping message,
double-reporting away from the sink).

Replaces the single narrative bullet (buried in a long list) that
already named this anti-pattern but was easy to under-apply on a
mega-diff, per APP-5522.
@warp-agent-staging

Copy link
Copy Markdown
Contributor Author

This PR was generated with Warp.

Comment @warp-factory on this PR to send it follow-up work.

View run View conversation

@acarl005
acarl005 marked this pull request as ready for review August 19, 2026 00:39
@warp-agent-staging
warp-agent-staging Bot requested a review from acarl005 August 19, 2026 01:16
@acarl005 acarl005 self-assigned this Aug 19, 2026
@acarl005
acarl005 merged commit 0209de5 into master Aug 19, 2026
69 of 71 checks passed
@acarl005
acarl005 deleted the factory/app-5522-error-reporting-form-audit branch August 19, 2026 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant