Adding Qodo skills for code reviews

[markpete]

Added skills files to be used by Qodo to direct code reviews and to give feedback in a supportive, encouraging, and educational voice.

[qodo-code-review]

PR Summary by Qodo

Add Qodo review skills for FRC code safety, architecture, and mentoring tone

📝 Documentation ⚙️ Configuration changes 🕐 10-20 Minutes

AI Description

• Add Qodo skill definitions to guide consistent, educational code review feedback.
• Encode FRC-specific safety checks (current limits, soft limits, CAN usage) for hardware
 protection.
• Standardize review guidance for WPILib architecture and telemetry/units best practices.

Diagram

graph TD
  Q["Qodo Review Engine"] --> S["skills/ (skill library)"] --> P["frc-student-mentor-persona"] --> O["Review comments"]
  S --> A["frc-wpilib-architecture"] --> O
  S --> T["frc-telemetry-units"] --> O
  S --> H["frc-robot-safety-can"] --> O

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Separate “persona” vs “checklists” strictly (composition-first)

• ➕ Keeps tone/voice guidance in one place and reduces duplicated phrasing across skills
• ➕ Easier to evolve the communication style without touching domain checklists
• ➖ Requires Qodo configuration/support for composing multiple skills reliably
• ➖ May make single-skill usage less convenient for smaller repos/teams

2. Provide a top-level skills index + naming conventions

• ➕ Improves discoverability and prevents near-duplicate skills over time
• ➕ Enables lightweight validation (required frontmatter fields, consistent naming)
• ➖ Adds a small amount of process/maintenance overhead
• ➖ Not strictly necessary for the initial set of four skills

Recommendation: The current approach (one SKILL.md per concern with clear frontmatter and focused checklists) is a good baseline for reuse and incremental expansion. If this library is expected to grow, consider adding an index/README and ensuring the mentor persona skill is always composed with the technical check skills to keep tone consistent.

Files changed (4) +86 / -0

Other (4) +86 / -0

SKILL.mdAdd FRC robot safety and CAN/hardware protection review skill +23/-0

Add FRC robot safety and CAN/hardware protection review skill

• Introduces a Qodo skill focused on hardware-protective software checks: current limiting, soft limits/limit switches, neutral mode configuration, and CAN bus bandwidth/status frame hygiene. Includes guidance on delivering safety feedback in a non-accusatory, competition-readiness framing.

skills/frc-robot-safety-can/SKILL.md

SKILL.mdAdd student-mentor persona skill for supportive review tone +17/-0

Add student-mentor persona skill for supportive review tone

• Defines an encouraging, pedagogical communication style for reviewing high-school student code, emphasizing explaining the “why,” praising good patterns, and using collaborative phrasing. Links learners to WPILib documentation where appropriate.

skills/frc-student-mentor-persona/SKILL.md

SKILL.mdAdd telemetry/units and logging conventions review skill +22/-0

Add telemetry/units and logging conventions review skill

• Adds a skill to promote WPILib Units usage for dimensional safety, structured telemetry/logging practices, and avoidance of periodic-loop overhead. Reinforces constants hygiene to reduce magic numbers and improve maintainability.

skills/frc-telemetry-units/SKILL.md

SKILL.mdAdd WPILib command-based architecture review skill +24/-0

Add WPILib command-based architecture review skill

• Adds guidance for reviewing modern WPILib command-based composition, subsystem requirement correctness, and periodic loop performance pitfalls. Encourages feedback that explains runtime impact and shows concise before/after examples.

skills/frc-wpilib-architecture/SKILL.md

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Overbroad string-allocation rule ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The telemetry skill tells reviewers to flag string concatenation in every periodic() and
SmartDashboard.putData() use without requiring repeated publication, measurable allocation
pressure, or a string-oriented dashboard API. This can turn harmless status formatting into
false-positive review comments while failing to distinguish genuinely costly high-frequency
telemetry.

Code

skills/frc-telemetry-units/SKILL.md[15]

+   - Avoid raw string concatenation inside `SmartDashboard.putData()` or `periodic()`.

Relevance

●●● Strong

Repo has accepted guidance to avoid repeated SmartDashboard.putData/string work in hot loops;
narrowing rule matches precedent.

PR-#167

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The added instruction provides no frequency, allocation, or API context, so it treats ordinary
formatting as defective; the feedback review confirmed the rule should be narrowed while preserving
checks for repeated hot-loop allocations.

skills/frc-telemetry-units/SKILL.md[14-16]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The blanket prohibition on raw string concatenation near `periodic()` and `SmartDashboard.putData()` is too broad and can generate noisy or incorrect findings.

## Issue Context
String construction can matter when repeatedly allocated and published in a high-frequency loop, but not every concatenation is a defect; `putData()` should also be distinguished from string-specific dashboard calls.

## Fix Focus Areas
- skills/frc-telemetry-units/SKILL.md[14-16]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Conflate vendor current limits ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The safety skill requires TalonFX, SparkMAX, and SparkFlex controllers to use a single stator/supply
current-limit concept, although CTRE and REV expose different current-limit configuration models.
Reviewers may therefore look for the wrong API or report valid REV current-limit settings as
missing.

Code

skills/frc-robot-safety-can/SKILL.md[13]

+   - Every motor controller configuration (TalonFX / SparkMAX / SparkFlex) **must** explicitly configure a stator/supply current limit (e.g., 30A–40A for drivetrains, 20A–30A for intakes/indexers).

Relevance

●●● Strong

Team frequently reviews stator/supply limits; clarifying vendor-specific APIs avoids confusion and
aligns with prior current-limit focus.

PR-#193
PR-#11

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The added rule names three controller families while requiring the TalonFX-specific stator/supply
terminology uniformly; the feedback review confirmed this is a vendor-specific API ambiguity rather
than a lack of current-limit support.

skills/frc-robot-safety-can/SKILL.md[11-13]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The rule conflates CTRE TalonFX stator/supply limits with REV Spark MAX/Spark Flex current-limit settings, which can produce false positives and invalid fix recommendations.

## Issue Context
All named controller families support current limiting, but their APIs and terminology differ. The skill should require the appropriate supported limit for each vendor rather than one shared configuration concept.

## Fix Focus Areas
- skills/frc-robot-safety-can/SKILL.md[12-13]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context

Review mode: 🚀 Fast: These are four small, self-contained Markdown skill files that add review guidance and tone without changing runtime behavior or critical configuration.

Tip of the day

💡 Did you know, you can turn on the rule miner and Qodo learns your standards from review history

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗