Document reviewer conventions in AGENTS.md and the review skills, and make review flags act on every finding

[pfultz2]

Motivation

Guidance that reviewers had given repeatedly in sessions (lint workflow, test
patterns, kernel-header rules, IR gotchas) only existed as per-session memory, so
each new session or contributor rediscovered it. The review skills also had two
gaps: /migraphx-simplify kept flagging things reviewers had explicitly rejected
as false positives, and /migraphx-code-review --fix, --comment, and --select
could be read as acting on correctness findings only, leaving quality,
convention, test-coverage, and precedent findings unapplied and unposted.

Technical Details

AGENTS.md

• Build and format: tools/format.py -i origin/develop formats changed lines only
  (whole-file clang-format adds unrelated hunks); trust make for the
  kernel-header → embed → relink chain; run make -k tests before ctest.
• Linting: run make analyze and judge by exit code (cppcheck reports as style:,
  clang-tidy emits ANSI escapes between location and severity); rename instead of
  NOLINT for readability-identifier-naming; root .clang-tidy AllowedTypes
  for cheap-copy opaque types; never redefine a parent .clang-tidy key in a child
  config; #ifndef CPPCHECK instead of cppcheck-suppress syntaxError; how to
  resolve the cppcheck/clang-tidy cast conflict; #elifdef is C++23.
• Style: structured bindings with init-capture, no locally-called named lambdas,
  non-template helpers declared in headers and defined in .cpp, a helper struct
  instead of 5+ parameter static helpers, static member helpers stay in their
  class, 1–3 line comments, no narrow predicates in core headers, explicit JIT
  source templates, insert + replace + DCE rather than in-place replace.
• Testing: TEST_CASE_REGISTER(name<T>) for per-type tests and TEST_CASE_TEMPLATE
  for kernel tests, the single-thread kernel harness, no property-based pass
  tests, copy the module for no-op pass tests, -p is appendable and --passes
  does not exist, run only the tests that exercise the change, never modify a
  failing test to match output.
• Debugging and perf: MIGRAPHX_DISABLE_PASSES, MIGRAPHX_GPU_DUMP_ASM with
  MIGRAPHX_USE_HIPRTC=0, MIGRAPHX_GPU_DEBUG, env vars are memoized, Release
  builds never fire assert, driver time vs perf, -b N for cold buffers,
  interleaved A/B with median ratio and no concurrent CPU load.
• GPU backend: instantiation-reducing if constexpr branches, kernel toolkit
  helpers are kept without callers, %[name] inline-asm operands, gfx1201 is wave32.
• Common issues: reshape/flatten copy at runtime so inserted view chains need
  reshape_lazy, no contiguous before reshape, squeeze/unsqueeze are not
  inverses on {1}, nested sub-matchers need match::opaque to avoid OOM in CI,
  target knobs go in backend_options not env vars.
• Commit guidelines: diff changesets against origin/develop (HEAD^2 on a merge
  commit).

.claude/skills/migraphx-simplify/SKILL.md

• Do-not-flag caveats for patterns reviewers reverted: boundary asserts
  re-asserted by internal helpers, static member helpers, orphaned kernel toolkit
  helpers, branch-pinning test stubs, instantiation-reducing if constexpr,
  merged JIT templates, static_cast<T&&> moves, ::value in tests.
• New findings: std::for_each must never become a raw loop, contiguous before
  reshape, pair member access over structured bindings, locally-called named
  lambdas, 5+ parameter static helpers, narrow predicates in core headers, inline
  non-template helpers in headers.
• Comment rewrites are capped at 1–3 lines and must not grow the comment.

.claude/skills/migraphx-code-review/SKILL.md

• New What the flags act on section with a category table; --fix, --comment,
  and --select now state explicitly that they act on every reported finding
  (correctness, language-pitfall, api-abi, ir-contract, quality,
  conventions, test-coverage, precedent). The reuse path acts on the full
  earlier list, picker labels carry [category], --fix has per-category apply
  guidance, and --comment posts non-correctness findings as inline comments too.
• Angle J additions: in-place replace_instruction, nested sub-matcher without
  match::opaque, generated view chain inserted as reshape instead of
  reshape_lazy, unsqueeze used to invert a squeeze on {1}, preconditions
  enforced only by assert.

Changelog Category

Add a CHANGELOG.md entry for any option other than Not Applicable

• • Added: New functionality.
• • Changed: Changes to existing functionality.
• • Removed: Functionality or support that has been removed. (Compared to a previous release)
• • Optimized: Component performance that has been optimized or improved.
• • Resolved Issues: Known issues from a previous version that have been resolved.
• • Not Applicable: This PR is not to be included in the changelog.

Follow the LLVM AI Tool Use Policy for contributions using AI.

[Copilot]

Copilot review overview

🟡 Changes recommended

Some new guidance contradicts implementation behavior and could lead to invalid refactors or misleading measurements.

Review effort: Balanced
Findings: 4

Open (4)

• Exclude generated-kernel and constexpr helpers from host-only rule · New
• Clarify HIPRTC requires CMake configuration and rebuilding · New
• Describe driver timing as whole-program wall-clock measurement · New
• Require zero-stride scalar condition for the unsqueeze warning · New

What changed in this PR

This documentation-only PR centralizes MIGraphX contributor conventions and clarifies how review tools handle findings.

Changes:

• Documents build, lint, testing, debugging, and GPU development practices.
• Refines review guidance to avoid known false positives.
• Clarifies that review flags cover every finding category.

File	Description
AGENTS.md	Adds contributor workflows and implementation guidance.
.claude/​skills/​migraphx-simplify/​SKILL.md	Refines cleanup rules and exceptions.
.claude/​skills/​migraphx-code-review/​SKILL.md	Expands review checks and clarifies flag behavior.

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #5356   +/-   ##
========================================
  Coverage    92.71%   92.72%           
========================================
  Files          635      635           
  Lines        36203    36203           
========================================
+ Hits         33565    33566    +1     
+ Misses        2638     2637    -1     

see 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.