ci(claude): add review-pr skill for domain-aware PR reviews (#26814)

* ci(claude): add review-pr skill for domain-aware PR reviews

Add a Claude Code skill that reviews pull requests with checks
tailored to the domains touched (estimation, control, drivers,
simulation, system, CI/build, messages, board additions).

Built from analysis of 800+ PR reviews across 8 PX4 maintainers.
Includes merge strategy recommendation, interactive dialog for
submitting reviews, and human-sounding PR comment formatting.

Signed-off-by: Ramon Roche <mrpollo@gmail.com>

* ci(copilot): add domain-scoped review instructions for GitHub Copilot

Add .github/instructions/ files that give GitHub Copilot PR reviews
the same domain-aware context as the Claude Code review-pr skill.

Each file is scoped via applyTo to the relevant source paths:
core review, estimation, control, drivers/CAN, simulation, system,
CI/build, messages/protocol, and board additions.

Signed-off-by: Ramon Roche <mrpollo@gmail.com>

* fix(claude): address Copilot review feedback

- Fix step reference in review-pr skill (step 8 -> step 9)
- Capitalize CMake consistently in skill and Copilot instructions

Signed-off-by: Ramon Roche <mrpollo@gmail.com>

---------

Signed-off-by: Ramon Roche <mrpollo@gmail.com>

.github/instructions/board-addition.instructions.md

@@ -0,0 +1,21 @@
+---
+applyTo: "boards/**"
+---
+
+# Board Addition Review Guidelines
+
+In addition to the core code review guidelines, when reviewing new board additions:
+
+- **Flight logs**: require a link to https://logs.px4.io demonstrating basic operation for the vehicle type (hover for multicopters, stable flight for fixed-wing, driving for rovers, etc.); short bench-only logs are insufficient
+- **Documentation**: require a docs page in `docs/en/flight_controller/` with pinout, where-to-buy, connector types, version badge, and manufacturer-supported notice block
+- **USB VID/PID**: must not reuse another manufacturer's Vendor ID; manufacturer must use their own
+- **Board naming**: directory is `boards/{manufacturer}/{board}/`, both lowercase, hyphens for board name
+- **Unique board_id**: registered in `boards/boards.json`, no collisions
+- **Copied code cleanup**: check for leftover files, configs, or comments from the template board. Ask "Is this real or leftover?"
+- **RC configuration**: prefer `CONFIG_DRIVERS_COMMON_RC` over legacy `CONFIG_DRIVERS_RC_INPUT`
+- **No board-specific custom modules**: reject copy-pasted drivers (e.g., custom heater) when existing infrastructure works
+- **Bootloader**: expect a bootloader defconfig (`nuttx-config/bootloader/defconfig`) or explanation of shared bootloader
+- **CI integration**: board must be added to CI compile workflows so it builds on every PR
+- **Flash constraints**: verify enabled modules fit in flash; we are running low across all board targets
+- **Port labels**: serial port labels must match what is physically printed on the board
+- **Hardware availability**: for unknown manufacturers, verify the product exists and is purchasable (no vaporware)

.github/instructions/ci-build.instructions.md

@@ -0,0 +1,13 @@
+---
+applyTo: ".github/**,cmake/**,Makefile,CMakeLists.txt,Tools/**,**/Kconfig"
+---
+
+# CI/Build Review Guidelines
+
+In addition to the core code review guidelines:
+
+- Check for pipeline race conditions (tag + branch push double-trigger, git describe correctness)
+- Container image size: check for layer bloat
+- Ubuntu LTS support policy: only latest + one prior LTS version
+- Consider build time impact of changes
+- Prefer CMake over Makefiles

.github/instructions/code-review.instructions.md

@@ -0,0 +1,32 @@
+---
+applyTo: "src/**,boards/**,platforms/**,msg/**,cmake/**,Makefile,CMakeLists.txt,Tools/**,.github/**"
+---
+
+# PX4 Code Review Guidelines
+
+## Conventions
+
+- PR titles must follow conventional commits: `type(scope): description` (see CONTRIBUTING.md)
+- Types: `feat`, `fix`, `docs`, `style`, `refactor`, `perf`, `test`, `build`, `ci`, `chore`, `revert`
+- Scope should match the primary area of changed files
+- Append `!` before the colon for breaking changes
+- Both squash merge and rebase merge are enabled; merge commits are disabled
+- Commits should be atomic and independently revertable
+- WIP or review-response commits should be squashed before merge
+
+## Core Checks (always apply)
+
+- **Correctness**: logic errors, off-by-ones, unhandled edge cases
+- **Type safety**: int16 overflow, float/double promotion, unsigned subtraction, use `uint64_t` for absolute time
+- **Initialization**: uninitialized variables, missing default construction
+- **Buffer safety**: unchecked array access, stack allocation of large buffers, snprintf bounds
+- **Magic numbers**: every numeric literal needs a named constant or justification
+- **Framework reuse**: use PX4_ERR/WARN/INFO, existing libraries (AlphaFilter, SlewRate, RateControl), MAVLink constants from the library
+- **Naming**: accurate, no unjustified abbreviations, current terminology (GPS -> GNSS for new code)
+- **Unnecessary complexity**: can code be removed instead of added? Is there a simpler pattern?
+- **Test coverage**: new features should include unit or integration tests; bug fixes should include regression tests where practical
+- **Formatting**: `make format` / `make check_format` (astyle) for C/C++ files; `clang-tidy` clean
+- **Coding style**: C/C++ must follow the PX4 coding style (https://docs.px4.io/main/en/contribute/code.html)
+- **Necessity**: challenge every addition. Is this actually needed or just copied?
+- **Architecture fit**: does the code live in the module that naturally owns the data? No unnecessary cross-module dependencies
+- **Ecosystem impact**: consider QGC users, log analysis tools, and third-party integrations

.github/instructions/control.instructions.md

@@ -0,0 +1,15 @@
+---
+applyTo: "src/modules/mc_*control*/**,src/modules/fw_*control*/**,src/modules/flight_mode_manager/**,src/lib/rate_control/**,src/lib/npfg/**,src/modules/vtol_att_control/**"
+---
+
+# Control Review Guidelines
+
+In addition to the core code review guidelines:
+
+- Phase margin: output filters consume margin for no benefit; prefer adjusting gyro/d-gyro cutoffs
+- Check for circular dependencies: sensor data feeding back into its own control loop (e.g., throttle-based airspeed in TECS)
+- NaN propagation in flight-critical math; check `PX4_ISFINITE` before magnitude checks
+- Prefer proper setpoint smoothing over controller output filtering (setpoint generation vs output-stage hacks)
+- Check yaw control edge cases: heading lock, drift, setpoint propagation
+- Verify flight task inheritance chain uses the correct base class for desired behavior
+- Control allocation: verify actuator function ordering and motor index mapping

.github/instructions/drivers.instructions.md

@@ -0,0 +1,13 @@
+---
+applyTo: "src/drivers/**,src/modules/cyphal/**"
+---
+
+# Drivers/CAN Review Guidelines
+
+In addition to the core code review guidelines:
+
+- CAN bus devices behave differently from serial/SPI; check driver assumptions
+- ESC index mapping: telemetry index != channel when motors are disabled
+- ESC hardware quirks: 4-in-1 ESCs may report current on only one channel
+- Verify device_id correctness and I2CSPIDriver patterns
+- Time representation: prefer `hrt_abstime` over iteration counts

.github/instructions/estimation.instructions.md

@@ -0,0 +1,13 @@
+---
+applyTo: "src/modules/ekf2/**,src/lib/wind_estimator/**,src/lib/world_magnetic_model/**"
+---
+
+# Estimation Review Guidelines
+
+In addition to the core code review guidelines:
+
+- Check for singularities in aerospace math (euler angles near gimbal lock, sideslip at low airspeed)
+- Flag aliasing from downsampling sensor data without proper filtering
+- Verify Kalman filter correctness (Joseph form, innovation variance, covariance symmetry)
+- Consider CPU cost on embedded targets (avoid unnecessary sqrt, limit fusion rate)
+- Verify frame/coordinate system correctness (FRD vs NED, body vs earth frame)

.github/instructions/messages.instructions.md

@@ -0,0 +1,13 @@
+---
+applyTo: "msg/**,src/modules/mavlink/**,src/modules/uxrce_dds_client/**"
+---
+
+# Messages/Protocol Review Guidelines
+
+In addition to the core code review guidelines:
+
+- Backwards compatibility: will this break QGC, post-flight tools, or uLog parsers?
+- uORB: `timestamp` for publication metadata, `timestamp_sample` close to physical sample, include `device_id`
+- Don't version messages unless strictly needed
+- Parameter UX: will this confuse users in a GCS? Every new param is a configuration burden
+- MAVLink: use library constants, don't implement custom stream rates

.github/instructions/simulation.instructions.md

@@ -0,0 +1,13 @@
+---
+applyTo: "src/modules/simulation/**,Tools/simulation/**"
+---
+
+# Simulation Review Guidelines
+
+In addition to the core code review guidelines:
+
+- Physics fidelity: noise models should match reality (GPS noise is not Gaussian)
+- Keep gz_bridge generic; vehicle-specific logic belongs in plugins
+- Prefer gz-transport over ROS2 dependencies when possible
+- Use wrench commands for physics correctness vs kinematic constraints
+- Library generic/specific boundary: only base classes in common libs

.github/instructions/system.instructions.md

@@ -0,0 +1,15 @@
+---
+applyTo: "src/modules/commander/**,src/modules/logger/**,src/systemcmds/**,platforms/**,src/modules/dataman/**"
+---
+
+# System Review Guidelines
+
+In addition to the core code review guidelines:
+
+- Race conditions and concurrency: no partial fixes, demand complete solutions
+- Semaphore/scheduling edge cases; understand RTOS guarantees
+- State machine sequential-logic bugs (consecutive RTL, armed/disarmed alternation)
+- Use uORB-driven scheduling (`SubscriptionCallback`), not extra threads
+- `param_set` triggers auto-save; no redundant `param_save_default`
+- Flash/memory efficiency: avoid `std::string` on embedded, minimize SubscriptionData usage
+- Constructor initialization order matters