qt-cpp-review

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

1. 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.

python3 references/lint-scripts/qt_review_lint.py <files...>
# If python3 is not found, fall back to:
python references/lint-scripts/qt_review_lint.py <files...>

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 numbers without named constants
• God classes violating Single Responsibility
• Copy-pasted validation/logic across classes
• Stale member caches not invalidated on model changes

(e.g. search cache surviving data edits)

• QMap/QHash iteration order nondeterminism when selecting

a "best" or "first" entry (.first() changes if keys are

added; use deterministic tie-breaking)

• QMap for small fixed-size constant data (use array/switch)
• Returning QList/container by value from frequently-called

methods (implicit deep copy on every call — return const ref

or cache)

• Member variables maintained (appended, capped) but never

read by any method (dead state — wasted CPU and memory)

• Missing re-entrancy guard on methods that emit signals

which could trigger re-entry

• Setter silently resetting unrelated state without signal
• Early return skipping status/signal updates

References: references/qt-review-checklist.md §

Performance & Code Quality

---

Confidence scoring guidelines

| Confidence | Meaning | Action |

|------------|---------|--------|

| 90–100 | Certain: direct rule violation with full symbol trace | Report as finding |

| 80–89 | High: rule violation confirmed but edge case possible | Report as finding |

| 60–79 | Medium: likely issue but cannot fully verify | Report as investigation target |

| <60 | Low: suspicion only | Suppress entirely |

Investigation targets are findings the agent believes are real

but cannot fully verify — e.g. noexcept correctness requiring

whole-program analysis, dead code that may have callers outside

scope, or design-intent judgments like virtual access levels.

These are presented in a separate section for human verification.

Maximum 10 investigation targets per report, prioritized by

confidence within the 60–79 band.

Output format

Present the final report as follows. Use exactly this structure.

## Qt Code Review Report

**Scope**: [diff: `git diff HEAD~1..HEAD` | files: <paths>]
**Files reviewed**: N
**Issues found**: N (M from lint, K from deep analysis)

---

### Lint findings

For each lint finding:

#### [L-NNN] <Short title>
- **File**: `path/to/file.cpp:42`
- **Rule**: <rule ID from checklist>
- **Finding**: <what the script detected>
- **Mitigation**: <what to do, in prose — no code patches>

---

### Deep analysis findings

For each agent finding:

#### [D-NNN] <Short title>
- **File**: `path/to/file.cpp:42`
- **Category**: <agent name: Model Contracts | Ownership &
  Lifecycle | Thread Safety | API & C++ Correctness | Error
  Handling | Performance & Quality>
- **Confidence**: NN/100
- **Finding**: <description of the issue>
- **Trace**: <how the issue was confirmed — which symbols were
  followed, what was checked>
- **Mitigation**: <what to do, in prose — no code patches>

---

### Investigation targets (human verification needed)

Findings the agent identified but could not fully verify.
Maximum 10, sorted by confidence. These require human judgment.

For each investigation target:

#### [I-NNN] <Short title>
- **File**: `path/to/file.cpp:42`
- **Category**: <agent name>
- **Confidence**: NN/100
- **Finding**: <what the agent suspects>
- **Unverified because**: <what the agent could not confirm —
  e.g. "cannot trace all callees for throw potential",
  "only one implementation visible in scope">
- **How to verify**: <specific action for the reviewer>

---

### Summary

| Category | Lint | Deep | Investigate | Total |
|----------|------|------|-------------|-------|
| ...      | N    | N    | N           | N     |
| **Total**| **M**| **K**| **I**       | **N** |

Findings below confidence 60 are suppressed entirely.

References

The following reference files contain detailed checklists

extracted from the Qt wiki "Things To Look Out For In Reviews":

• references/qt-review-checklist.md — Universal Qt6 C++ review

rules (always loaded)

• references/qt-framework-checklist.md — Qt framework/module

development rules (loaded only in framework mode)

• references/qt-deprecated-classes.md — Classes and patterns

that should no longer be used in Qt implementation

• references/lint-scripts/qt_review_lint.py — Single-pass

Python linter (runs all 60+ checks in <1s)