qt-cpp-review

Invoke when the user asks to review, check, audit, or look over Qt6 C++ code — or suggest before committing. Runs deterministic linting (60+ rules) then six parallel deep- analysis agents covering model contracts, ownership, threading, API correctness, error handling, and performance. Reports only h

By theqtcompanyrnd · 707 installs

npx skills add theqtcompanyrnd/agent-skills --skill qt-cpp-review

Source repository · Upstream listing

Qt Code Review A structured, read only code review skill for Qt6 C++ code that combines deterministic linting with parallel agent driven deep analysis across six focused domains. When to use this skill When the user mentions review related tasks: "review", "check", "audit", "look over", "code review", "sanity check" Suggest running this skill before committing code When the user asks to validate Qt6 C++ code quality Arguments /qt cpp review — review using universal Qt6 C++ rules only /qt cpp review framework — also apply Qt framework/module development rules (BC, exports, d pointers, qdoc, QML versioning) Framework mode detection If $ARGUMENTS contains "framework", enable framework mode. If the argument is not passed, auto detect by scanning the first few files in scope for framework signals. If two or more of the following are found, suggest to the user: "This looks like Qt framework/module code. Run /qt cpp review framework to also apply framework specific rules (BC, exports, qdoc, QML versioning)?" Framework signals (any two = likely framework code): QT BEGIN NAMESPACE / QT END NAMESPACE Q CORE EXPORT , Q GUI EXPORT , Q WIDGETS EXPORT , or any Q EXPORT macro include <QtModule/private/ p.h (private headers) Q DECLARE PRIVATE , Q D() , Q Q() qt internal add module or qt add module in CMakeLists.txt sync.profile or .qmake.conf in the repository root Do not auto enable framework mode — only suggest it. Let the user confirm. When framework mode is enabled: 1. Pass framework to the linter (if supported) 2. Load references/qt framework checklist.md alongside the universal checklist 3. Include framework rules in each agent's mission context Scope detection Detect the user's intended scope from their language: Diff/commit scope (narrow) Triggered by language like: "this commit", "these changes", "the diff", "what I changed", "my changes", "staged changes", "outstanding changes", "before I commit" Action : Run git diff (unstaged) and git diff cached (staged) to obtain the changeset. If the user says "this commit", use git diff HEAD~1..HEAD . Review only the changed lines plus sufficient surrounding context (±50 lines) for understanding. Only report issues found in the changed lines — do not report issues in unchanged surrounding context. Codebase scope (wide) Triggered by language like: "review the codebase", "audit the project", "check the repository", "review src/", or when a specific file/directory path is given without commit language. Action : Glob for .cpp , .h , .hpp files in the specified scope. Review all matched files. Execution order The review proceeds in three phases. Never skip a phase. Phase 1: Deterministic linting (scripts) Run the unified Python linter against the target files. Requires Python 3.6+ (no external dependencies). If Python is not available, warn the user and skip to Phase 2. This single pass scanner encodes all mechanically checkable rules from the Qt review guidelines. It reads each file once and evaluates all rules per line. Output is deterministic and repeatable. The linter is authoritative — do not second guess its output. Collect all output before proceeding to Phase 2. Rule categories (60+ checks): INC (Includes) — ordering, qglobal.h, qNN duplication DEP (Deprecated) — obsolete Qt/std class usage PAT (Patterns) — anti patterns (min/max, std::optional, NRVO, COW detach, etc.) MDL (Model) — QAbstractItemModel contract (begin/end balance, dataChanged roles, flags, default: in data()) ERR (Error Handling) — QFile::open, QJsonDocument::isNull, QNetworkReply::error, SSL, timeouts, arg() mismatch LCY (Lifecycle) — deleteLater, Q ASSERT side effects, null guards, unbounded containers, qDeleteAll depth API (Naming) — get prefix, enum hygiene, QList<QString HDR/TMO/CND/VAL/TRN — headers, timeouts, conditionals, value classes, ternary operator Phase 2: Agent driven deep analysis (6 parallel agents) Launch six focused review agents in parallel. Name each agent descriptively when launching (e.g. "Agent 1: Model Contracts") to provide progress visibility. Each agent has a tight scope and a specific checklist. Agents are READ ONLY — they must never edit or write files. Tool agnostic agent contract : Each agent described below is a self contained review mission. In Claude Code, launch them as general purpose subagents. In other tools, implement each as whatever subprocess, prompt chain, or analysis pass the tool supports. The key requirement is that each agent: Has read access to all source files in scope Can search/grep the codebase to trace symbols Reports findings in the structured format below Applies confidence thresholds: 80 = confirmed finding, 60–79 = investigation target (max 10 total across all agents), <60 = suppress Does NOT duplicate findings from Phase 1 lint output (pass lint output as context to each agent) See Agent missions below for the six agents. Phase 3: Consolidation and reporting Merge lint script output and all agent findings. Deduplicate (same file+line+issue = one finding). Apply confidence scoring. Format the final report using the output format below. Agent missions Launch all six agents in parallel. Pass each agent: 1. The list of files in scope 2. The Phase 1 lint output (so they skip already flagged issues) 3. Their specific mission below Each agent should read all files in scope, then focus on its assigned categories. Agent 1: Model Contracts Scope : QAbstractItemModel signal protocol, role system, index validity, proxy model correctness. Check for : beginInsertRows / endInsertRows balance — every structural model change (add/remove/move) must use the correct begin/end pairs. layoutChanged is NOT a substitute for insert/remove. roleNames() returning roles that data() does not handle (missing switch cases, fall through to default) dataChanged emitted with empty roles vector (forces full refresh instead of targeted update) beginRemoveRows called with first last (edge case when container is empty — QAIM contract violation) flags() returning inappropriate flags (e.g. ItemIsEditable for non editable items) setData() returning true without emitting dataChanged Proxy models accessing source model internals instead of going through data() / index() API Filter/proxy models using source model indices to index into filtered containers (wrong index space) References : references/qt review checklist.md § Model Contracts Agent 2: Ownership & Lifecycle Scope : Memory ownership, parent child, resource cleanup, Rule of Five, RAII correctness. Check for : Structs/classes with raw pointers where new is visible and no corresponding delete / deleteLater /smart pointer wrapping exists (Rule of Five violation) Missing deleteLater() on QNetworkReply in finished handlers Q ASSERT wrapping side effectful expressions (compiled out in release builds — the side effect disappears) Q ASSERT as the sole null guard (crashes in release) Polymorphic QObject subclasses missing Q DISABLE COPY MOVE Polymorphic classes missing virtual destructor QTimer/QObject created with new but no parent and no other lifecycle management (scope, smart pointer, explicit delete) QObject::connect() called with potentially null sender/receiver outside a null guard (runtime warning) m recentlyAccessed style tracking lists that maintain pointers to objects that may be deleted elsewhere (dangling) Unbounded container growth (append without cap or trim) Destructor not cleaning up owned children recursively Abstract interfaces with no implementations beyond one class (YAGNI violation — codebase scope only) References : references/qt review checklist.md § Ownership & Lifecycle, § Polymorphic Classes, § RAII Classes Agent 3: Thread Safety Scope : Cross thread QObject access, mutex consistency, signal emission from worker threads. Check for : QObject member variables written from QtConcurrent::run() or QThread worker without synchronization (mutex, atomic, queued connection, or other thread safe primitive) Signals emitted from worker threads connected with Qt::DirectConnection (or explicit non queued connections) to main thread receivers Model mutations ( addNote , removeRows , etc.) from background threads Shared containers ( QList , QHash ) modified from multiple threads without consistent synchronization Non atomic increment/decrement of shared counters ( m operationCount++ from multiple threads) QTimer or other QObject operations from non owner thread References : references/qt review checklist.md § Thread Safety Agent 4: API, Naming & C++ Correctness Scope : Qt naming conventions, const correctness, move semantics, enum hygiene, noexcept correctness. Check for : get prefix on mere getters (Qt reserves get for user interaction or out parameter decomposition) Non const getter methods (especially Q PROPERTY READ accessors — UB via meta object system) Missing std::forward<T () on forwarding/universal references return std::move(localVar) preventing NRVO const local variable preventing implicit move on return (e.g. const QJsonDocument doc(...); return doc; forces copy) const method returning mutable pointer through raw pointer indirection ( findById() const returning T lets callers mutate via a const accessor — const doesn't propagate through raw pointers) noexcept on functions containing Q ASSERT (incompatible — Q ASSERT may throw for testing, noexcept terminates) Unscoped enums without explicit underlying type Missing trailing comma on last enumerator switch over enum with default: label (suppresses Wswitch) QList<QString instead of QStringList Missing const on methods that don't modify state Case sensitive string comparison for user facing sort Duplicated validation logic across classes const QMetaObject::Connection preventing handle cleanup References : references/qt review checklist.md § API & Naming, § Enums, § Methods, § Move Semantics, § Operators Agent 5: Error Handling & Validation Scope : Missing error checks, input validation, security. Check for : QFile::open() return value ignored QJsonDocument::fromJson() result not checked for isNull() / isObject() before use QNetworkReply::error() not checked before readAll() XML writer hasError() not checked after writing Hardcoded http:// instead of https:// in URLs No SSL error handling ( QNetworkAccessManager::sslErrors ) No timeout on network requests ( setTransferTimeout ) Negative values accepted where only positive are valid (e.g. timer intervals, font sizes) No schema/version validation on imported data No input length validation on imported/downloaded data (unbounded strings from untrusted sources) QString::arg() with wrong placeholder count saveToFile() returning true regardless of I/O errors Inconsistent error reporting patterns across methods References : references/qt review checklist.md § Error Handling & Validation Agent 6: Performance & Code Quality Scope : Performance anti patterns, dead code, unnecessary copies, code smells. Check for : QRegularExpression constructed inside a loop (expensive compilation on every iteration) roleNames() rebuilding QHash on every call (should cache) Non const range for over COW shared QList/QHash triggering unnecessary detach/deep copy Non const operator[] on shared QHash (triggers detach) — use .value() for reads Expensive operation before cheap early exit check (wasted allocation) Dead/unreachable code (functions never called, branches that are always true/false given preconditions) Magic numb