qt-cpp-review — Skillopedia

Qt Code Review Read-only review of Qt6 C++ code: a deterministic Python linter first, then agent-driven deep analysis across six rule domains, then one consolidated report. When to use this skill - Review-related requests: "review", "check", "audit", "look over", "code review", "sanity check" - Suggest it before committing code Arguments - — universal Qt6 C++ rules - — also apply Qt framework/module rules (binary compatibility, exports, d-pointers, qdoc, QML versioning) Framework mode Enable when contains "framework". Otherwise, if the lint run or a quick Grep shows two or more of these signa…

, '', ln) for ln in lines]\n code_text = \"\\n\".join(code_lines)\n\n file_flags: dict[str, bool] = {\n \"has_haserror\": \"hasError\" in code_text,\n \"has_sslerrors\": \"sslErrors\" in code_text,\n \"has_transfer_timeout\": bool(re.search(\n r'(setTransferTimeout|setTimeout|transferTimeout)', code_text)),\n \"has_deletelater\": \"deleteLater\" in code_text,\n \"has_destructor\": bool(RE_DESTRUCTOR.search(code_text)),\n \"is_header\": is_header,\n }\n\n # Model signal balance (for post-scan MDL-1/MDL-6)\n has_rolenames = bool(RE_ROLE_NAMES.search(code_text))\n has_case_role = bool(RE_CASE_ROLE.search(code_text))\n has_default_case = bool(RE_DEFAULT_CASE.search(code_text))\n has_structural_mut = bool(RE_STRUCTURAL_MUT.search(code_text))\n has_begin_insert = bool(RE_BEGIN_INSERT.search(code_text))\n has_end_insert = bool(RE_END_INSERT.search(code_text))\n has_begin_remove = bool(RE_BEGIN_REMOVE.search(code_text))\n has_end_remove = bool(RE_END_REMOVE.search(code_text))\n has_begin_move = bool(RE_BEGIN_MOVE.search(code_text))\n has_end_move = bool(RE_END_MOVE.search(code_text))\n has_begin_reset = bool(RE_BEGIN_RESET.search(code_text))\n has_end_reset = bool(RE_END_RESET.search(code_text))\n\n # Containers with size caps (for procedural LCY-5)\n capped_containers: set[str] = set()\n for match in re.finditer(RE_TRIM_CALL, code_text):\n capped_containers.add(match.group(1))\n for match in re.finditer(RE_SIZE_GUARD, code_text):\n capped_containers.add(match.group(1))\n\n # State tracking for post-scan and procedural rules\n last_qt_include_line = 0\n first_std_include_line = 0\n layout_changed_line = 0\n\n # Pre-compute active rule list (avoid rebuilding per line)\n active_simple = RULES_SIMPLE + RULES_FRAMEWORK if framework else RULES_SIMPLE\n\n # --- Line-by-line scan ---\n for i, line in enumerate(lines):\n lineno = i + 1\n\n if RE_COMMENT_LINE.match(line):\n continue\n\n # --- Tier A: simple rules (+ framework rules if enabled) ---\n for rule in active_simple:\n if rule.header_only and not is_header:\n continue\n if not rule.pattern.search(line):\n continue\n if rule.exclude and rule.exclude.search(line):\n continue\n emit(lineno, rule.id, rule.message)\n\n # --- Tier A special: multi-exclude rules ---\n\n # ENM-2: unscoped enum without class or explicit type\n if (RE_ENM2_UNSCOPED.search(line)\n and not RE_ENM2_CLASS.search(line)\n and not RE_ENM2_TYPED.search(line)):\n emit(lineno, \"ENM-2\",\n \"Unscoped enum without explicit underlying type \\u2014 add type to prevent BiC\")\n\n # VAR-3: direct brace init (needs 2 excludes + keyword check)\n if (RE_VAR3_DIRECT_INIT.search(line)\n and not RE_VAR3_COPY_INIT.search(line)\n and not RE_VAR3_KEYWORDS.search(line)):\n emit(lineno, \"VAR-3\",\n \"Direct brace initialization \\u2014 prefer copy-init (var = {...}) for safety\")\n\n # PAT-1: std::optional::value() — only when line has optional context\n if RE_PAT1_OPT_VALUE.search(line) and RE_PAT1_OPT_CTX.search(line):\n emit(lineno, \"PAT-1\",\n \"std::optional::value() \\u2014 use *opt or opt->foo (value() throws on empty)\")\n\n # PAT-2: std::optional default-constructed without nullopt\n if RE_PAT2_OPT_DEFAULT.search(line) and \"nullopt\" not in line:\n emit(lineno, \"PAT-2\",\n \"std::optional default-constructed \\u2014 use std::nullopt explicitly (GCC warning bug)\")\n\n # LCY-3: Q_ASSERT with side-effectful expression\n if RE_LCY3_ASSERT_SIDE.search(line) and RE_LCY3_EFFECTS.search(line):\n emit(lineno, \"LCY-3\",\n \"Q_ASSERT wraps side-effectful call \\u2014 compiled out in release\")\n\n # ERR-1: QFile::open without guard (needs 2 positive + 1 negative)\n if (RE_ERR1_OPEN.search(line)\n and RE_ERR1_IODEV.search(line)\n and not RE_ERR1_GUARDED.search(line)):\n emit(lineno, \"ERR-1\",\n \"QFile::open() return not checked \\u2014 silent failure if file cannot open\")\n\n # --- Tier B: context-aware rules ---\n for rule in RULES_CONTEXT:\n if not rule.pattern.search(line):\n continue\n if rule.exclude and rule.exclude.search(line):\n continue\n # Build context window (use comment-stripped lines)\n if rule.scope_aware:\n ctx = _scope_truncated_window(\n code_lines, i, rule.context_before, rule.context_after)\n else:\n ctx_parts = []\n if rule.context_before:\n start = max(i - rule.context_before, 0)\n ctx_parts.append(\"\\n\".join(code_lines[start:i]))\n if rule.context_after:\n end = min(i + 1 + rule.context_after, num_lines)\n ctx_parts.append(\"\\n\".join(code_lines[i + 1:end]))\n ctx = \"\\n\".join(ctx_parts)\n has_ctx = bool(rule.context_pattern.search(ctx)) if rule.context_pattern else True\n if has_ctx == rule.context_match_required:\n emit(lineno, rule.id, rule.message)\n\n # --- Tier C: file-flag-gated rules ---\n for rule in RULES_FLAG:\n if rule.requires_flag and not file_flags.get(rule.requires_flag):\n continue\n if rule.requires_no_flag and file_flags.get(rule.requires_no_flag):\n continue\n if not rule.pattern.search(line):\n continue\n if rule.exclude and rule.exclude.search(line):\n continue\n emit(lineno, rule.id, rule.message)\n\n # --- Tier D: Procedural rules (cannot be table-driven) ---\n\n # INC-3: dynamic regex from capture group, cross-file search\n # (framework only — qNN headers are a Qt module convention)\n m = RE_INC3_QNN.search(line) if framework else None\n if m:\n std_name = m.group(2)\n if re.search(rf'^\\s*#\\s*include\\s+\u003c{re.escape(std_name)}>', full_text, re.M):\n emit(lineno, \"INC-3\",\n f\"Both q{m.group(1)}{std_name} and \u003c{std_name}> included \\u2014 remove one\")\n\n # ERR-6: count placeholders vs .arg() calls\n # Use max placeholder index (e.g. %2 → need 2 args) vs .arg() count\n if RE_ERR6_MULTI_PH.search(line) and RE_ERR6_ARG.search(line):\n max_ph = max(int(p.lstrip(\"%\")) for p in RE_ERR6_PH.findall(line))\n args = len(RE_ERR6_ARG.findall(line))\n if max_ph > args:\n emit(lineno, \"ERR-6\",\n f\"QString::arg() has %{max_ph} but only {args} .arg() calls\")\n\n # LCY-4: Q_ASSERT(var) as sole null guard — extract var, check dereference\n m = RE_LCY4_ASSERT_PTR.search(line)\n if m:\n varname = m.group(1)\n after = \"\\n\".join(lines[i + 1:min(i + 6, num_lines)])\n if f\"{varname}->\" in after:\n if not re.search(\n rf'if\\s*\\(\\s*!?{re.escape(varname)}|'\n rf'{re.escape(varname)}\\s*[!=]=',\n after\n ):\n emit(lineno, \"LCY-4\",\n f\"Q_ASSERT({varname}) is sole null guard \\u2014 crashes in release\")\n\n # LCY-5: unbounded container growth — extract name, check capped set\n container = None\n m = RE_LCY5_APPEND.search(line)\n if m:\n container = m.group(1)\n else:\n m2 = RE_LCY5_SHIFT.search(line)\n if m2:\n container = f\"m_{m2.group(1)}\"\n if container and container not in capped_containers:\n emit(lineno, \"LCY-5\",\n f\"{container} grows without size cap \\u2014 unbounded memory growth\")\n\n # LCY-1: reply read without deleteLater — 2 branches\n if RE_ERR3_READALL.search(line):\n if not file_flags[\"has_deletelater\"]:\n emit(lineno, \"LCY-1\",\n \"QNetworkReply read without deleteLater() \\u2014 reply leaked\")\n else:\n window = \"\\n\".join(lines[max(i - 10, 0):min(i + 11, num_lines)])\n if \"deleteLater\" not in window:\n emit(lineno, \"LCY-1\",\n \"QNetworkReply read without deleteLater() in this handler\")\n\n # --- State tracking for post-scan rules ---\n if RE_QT_INCLUDE.search(line):\n last_qt_include_line = lineno\n if RE_STD_INCLUDE.search(line) and first_std_include_line == 0:\n first_std_include_line = lineno\n if RE_LAYOUT_CHANGED.search(line):\n layout_changed_line = lineno\n\n # --- Post-scan file-level checks (Tier D) ---\n\n # INC-2: std header before Qt header\n if (first_std_include_line > 0\n and last_qt_include_line > 0\n and first_std_include_line \u003c last_qt_include_line):\n emit(first_std_include_line, \"INC-2\",\n \"C++ standard header before Qt header \\u2014 reorder by descending specificity\")\n\n # MDL-1: layoutChanged for structural mutations without begin/end\n if layout_changed_line and has_structural_mut:\n if not (has_begin_insert or has_begin_remove or has_begin_move):\n emit(layout_changed_line, \"MDL-1\",\n \"layoutChanged for structural changes \\u2014 use beginInsertRows/beginRemoveRows\")\n\n # MDL-6: unbalanced begin/end pairs\n for begin, end, name in [\n (has_begin_insert, has_end_insert, \"InsertRows\"),\n (has_begin_remove, has_end_remove, \"RemoveRows\"),\n (has_begin_move, has_end_move, \"MoveRows\"),\n (has_begin_reset, has_end_reset, \"ResetModel\"),\n ]:\n if begin and not end:\n emit(1, \"MDL-6\", f\"begin{name} without matching end{name}\")\n if not begin and end:\n emit(1, \"MDL-6\", f\"end{name} without matching begin{name}\")\n\n # MDL-7: roleNames + data() with default: swallowing roles\n if has_rolenames and has_case_role and has_default_case:\n for j, ln in enumerate(lines):\n if RE_DEFAULT_CASE.search(ln):\n emit(j + 1, \"MDL-7\",\n \"data() switch has default: \\u2014 may hide unhandled roles; list all cases\")\n break\n\n return findings\n\n\n# ---------------------------------------------------------------------------\n# Main\n# ---------------------------------------------------------------------------\n\nCPP_SUFFIXES = (\".cpp\", \".cc\", \".cxx\", \".h\", \".hh\", \".hpp\", \".hxx\")\nRE_HUNK = re.compile(r\"^@@ -\\S+ \\+(\\d+)(?:,(\\d+))? @@\")\n\n\ndef changed_lines(diff_args: list[str]) -> tuple[str, dict[str, set[int]]]:\n \"\"\"Return the repository root and, per changed C++ file (relative to\n the root), the line numbers added or modified by `git diff \u003cdiff_args>`.\n Files with deletions only map to an empty set.\"\"\"\n def git(*args: str) -> str:\n return subprocess.run([\"git\", *args], capture_output=True, text=True,\n check=True).stdout\n\n root = git(\"rev-parse\", \"--show-toplevel\").strip()\n diff = git(\"-C\", root, \"-c\", \"core.quotePath=false\", \"diff\", \"-U0\",\n \"--no-color\", \"--no-ext-diff\", \"--src-prefix=a/\",\n \"--dst-prefix=b/\", *diff_args)\n changed: dict[str, set[int]] = {}\n current: set[int] | None = None\n for line in diff.splitlines():\n if line.startswith(\"+++ \"):\n path = line[4:].rstrip(\"\\t\")\n current = None\n if path.startswith(\"b/\") and path.endswith(CPP_SUFFIXES):\n current = changed.setdefault(path[2:], set())\n elif current is not None and line.startswith(\"@@\"):\n match = RE_HUNK.match(line)\n if match:\n start = int(match.group(1))\n count = int(match.group(2)) if match.group(2) is not None else 1\n current.update(range(start, start + count))\n return root, changed\n\n\ndef main() -> int:\n if len(sys.argv) \u003c 2:\n print(\"Usage: qt_review_lint.py [--framework] [--summary] \u003cfile> [file ...]\",\n file=sys.stderr)\n print(\" qt_review_lint.py --files-from=- \u003c list.txt\",\n file=sys.stderr)\n print(\" qt_review_lint.py [--framework] [--summary] --diff=\u003cgit diff args>\",\n file=sys.stderr)\n return 2\n\n framework = False\n summary = False\n diff_args: list[str] | None = None\n files: list[str] = []\n for arg in sys.argv[1:]:\n if arg == \"--framework\":\n framework = True\n elif arg == \"--summary\":\n summary = True\n elif arg.startswith(\"--diff=\"):\n diff_args = shlex.split(arg[len(\"--diff=\"):])\n elif arg == \"--files-from=-\":\n files.extend(line.strip() for line in sys.stdin if line.strip())\n else:\n files.append(arg)\n\n root: str | None = None\n changed: dict[str, set[int]] = {}\n if diff_args is not None:\n if files:\n print(\"Error: --diff takes no file arguments\", file=sys.stderr)\n return 2\n try:\n root, changed = changed_lines(diff_args)\n except (OSError, subprocess.CalledProcessError) as e:\n detail = getattr(e, \"stderr\", None) or str(e)\n print(f\"Error: git diff failed: {detail.strip()}\", file=sys.stderr)\n return 2\n files = [os.path.join(root, rel) for rel in sorted(changed)\n if os.path.isfile(os.path.join(root, rel))]\n if not files:\n print(f\"No changed C++ files in git diff {' '.join(diff_args)}\")\n return 0\n elif not files:\n print(\"Error: no input files specified\", file=sys.stderr)\n return 2\n\n all_findings: list[Finding] = []\n for filepath in files:\n all_findings.extend(scan_file(filepath, framework=framework))\n\n if root is not None:\n all_findings = [f for f in all_findings\n if f.line in changed.get(os.path.relpath(f.file, root), ())]\n\n # Sort by file, then line\n all_findings.sort(key=lambda f: (f.file, f.line))\n\n if summary:\n if root is None:\n dirs = [os.path.dirname(os.path.abspath(f)) for f in files]\n root = os.path.commonpath(dirs)\n print(f\"Paths relative to {root}/\")\n if diff_args is not None:\n total = sum(len(lines) for lines in changed.values())\n print(f\"Diff `git diff {' '.join(diff_args)}`: {len(changed)} C++ file(s), \"\n f\"{total} added or modified line(s)\")\n print(\" \" + \" \".join(f\"{rel} +{len(lines)}\"\n for rel, lines in sorted(changed.items())))\n print_summary(all_findings, root)\n else:\n for finding in all_findings:\n print(finding)\n\n return 1 if all_findings else 0\n\n\ndef print_summary(findings: list[Finding], root: str) -> None:\n \"\"\"Print findings grouped by (rule, message), locations per file,\n with paths relative to root.\n\n Example:\n LCY-2 QObject created with new but no parent (3)\n main.cpp:12,40 widget.cpp:88\n \"\"\"\n groups: dict[tuple[str, str], dict[str, list[int]]] = {}\n for f in findings:\n path = os.path.relpath(os.path.abspath(f.file), root)\n groups.setdefault((f.rule, f.message), {}).setdefault(path, []).append(f.line)\n print(f\"{len(findings)} finding(s), {len(groups)} distinct rule/message pair(s)\")\n for (rule, message), by_file in sorted(groups.items()):\n count = sum(len(v) for v in by_file.values())\n print(f\"{rule} {message} ({count})\")\n locs = [f\"{path}:{','.join(str(n) for n in sorted(lines))}\"\n for path, lines in sorted(by_file.items())]\n print(\" \" + \" \".join(locs))\n\n\nif __name__ == \"__main__\":\n sys.exit(main())\n","content_type":"text/x-python; charset=utf-8","language":"python","size":37704,"content_sha256":"b776510c73df9045671d3fa767db2121ba2086631c17b50dbf7427d82776c3cc"},{"filename":"references/qt-deprecated-classes.md","content":"# Qt/std Classes That Should Not Be Used in Qt Implementation\n\nReference for the `lint-deprecated.sh` script and deep analysis.\n\n## Qt Classes → Replacements\n\n| Deprecated | Replacement | Rationale |\n|------------|-------------|-----------|\n| Java-style iterators | STL iterators | `QT_NO_JAVA_STYLE_ITERATORS` |\n| `Q_FOREACH` | Range-based for | `QT_NO_FOREACH` |\n| `QScopedPointer` | `std::unique_ptr` | Can't be moved; use `const unique_ptr` for scoped semantics |\n| `QSharedPointer` / `QWeakPointer` | `std::shared_ptr` / `std::weak_ptr` | QSP needs 2× atomic ops on copy; removal planned for Qt 7 |\n| `QAtomic*` | `std::atomic` | Exception: static `QBasicAtomic*` (no runtime init) |\n| `QPair` | `std::pair` | QPair is a type alias since Qt 6.0 |\n| `QSharedDataPointer` | `QExplicitlySharedDataPointer` | QSDP detaches prematurely (atomic check on each access) |\n| `q(v)nprintf()` | `std::(v)snprintf()` | Platform-dependent fallbacks; must `#include \u003ccstdio>` |\n| `qMin`/`qMax`/`qBound` | `(std::min)()` / `(std::max)()` / `std::clamp()` | Mixed-type args in Qt 6 are harder to understand; note arg order difference |\n| `QChar` (as object) | `char16_t` | Language support; QChar as namespace (e.g. `QChar::isLower()`) is OK |\n| `count()` / `length()` | `size()` | Consistency with std library |\n\n## std Classes → Replacements\n\n| Deprecated | Replacement | Rationale |\n|------------|-------------|-----------|\n| `std::mutex` | `QMutex` | QMutex uses futexes (faster). Exception: std::mutex + std::condition_variable combo is more efficient than QMutex + QWaitCondition |\n\n## Anti-Patterns (not class-specific)\n\n| Pattern | Fix | Rule |\n|---------|-----|------|\n| `std::optional::value()` | Use `*opt`, `opt->foo`, `if (opt)` | Throws on empty; use pointer-compatible subset |\n| `std::optional{}` default ctor | Use `std::nullopt` explicitly | GCC maybe-unused warning bug |\n| `std::has_alternative\u003cT>` + `get\u003cT>` | Use `get_if\u003cT>` or `std::visit` | DRY; Coverity false positives |\n| `p = realloc(p, ...)` | `tmp = realloc(p, ...); check; p = tmp;` | Leaks on failure |\n| `std::make_unique\u003cT[]>(n)` for scalar T | `q20::make_unique_for_overwrite\u003cT[]>(n)` | Value-init zeros memory unnecessarily |\n| `value_or()` with non-trivial arg | Ternary or if/else | Arg always evaluated |\n| `QDateTime::currentDateTime()` | `currentDateTimeUtc()` | 100× faster, stable across DST |\n| `QThreadPool::globalInstance()` + blocking | Dedicated pool or `releaseThread()` | Deadlock risk |\n","content_type":"text/markdown; charset=utf-8","language":"markdown","size":2483,"content_sha256":"67e079f0bd869bd6238dd784786260d2058500915804911e94829d32e265534a"},{"filename":"references/qt-framework-checklist.md","content":"# Qt Framework Development Checklist\n\nRules specific to developing Qt library modules and framework code.\nThese rules apply when contributing to qtbase, qtdeclarative, or\nother Qt modules — NOT to application code using Qt.\n\nActivated when:\n- User passes `framework` argument: `/qt-cpp-review framework`\n- Auto-detected via framework signals in the codebase\n\nEach rule has a short ID prefixed with `FW-` for cross-referencing.\n\n## API Design\n\n- **FW-API-1**: Static factory members → `create()`. Non-static\n factory functions → `createFoo()`.\n- **FW-API-2**: Don't default arguments of non-Trivial Type. Use\n out-of-line overloading instead (binary compatibility).\n- **FW-API-4**: Don't define symbols in a Qt library that don't\n reference at least one type from that library (ODR violation\n risk). Includes Q_DECLARE_METATYPE_EXTERN, qHash, relational\n operators, math functions.\n- **FW-API-6**: \"Iteratable\" does not exist → \"Iterable\".\n- **FW-API-7**: \"Status\" is acceptable (Oxford dict. Meaning 5);\n \"State\" also correct for system condition.\n- **FW-API-8**: \"Mutable\" ≠ opposite of `const`. Correct terms:\n \"Mutating\", \"modifiable\", \"non-const\", \"variable\".\n\n## Public Headers\n\n- **FW-HDR-1**: Don't move code around in public headers when\n changing them (makes API-change-reviews hard).\n- **FW-HDR-2**: New overrides of virtual functions must be designed\n for skipping (existing subclasses won't call them).\n\n## Includes (Framework)\n\n- **FW-INC-1**: Include as `\u003cQtModule/qheader.h>` (public) or\n `\u003cQtModule/private/qheader_p.h>` (private), not\n `\u003cQtModule/QHeader>` (CamelCase form).\n- **FW-INC-2**: Group includes: module → dep Qt modules → QtCore →\n C++ → C → platform/3rd-party. Alphabetical within groups.\n- **FW-INC-3**: qNN headers sort by eventual name (in C++ group).\n Don't include both qNNfoo.h and \u003cfoo>.\n- **FW-INC-5**: Prefer forward-declaring in headers. Use\n qcontainerfwd.h / qstringfwd.h.\n- **FW-INC-6**: Don't include qglobal.h. Use fine-grained headers.\n\n## Variables (Framework)\n\n- **FW-VAR-1**: Static constexpr in exported classes: define in\n both .h and .cpp (MinGW DLL-import issue).\n- **FW-VAR-2**: Static/thread_local variables: use `constexpr`\n if possible; otherwise `Q_CONSTINIT const` if possible;\n otherwise `Q_CONSTINIT` if possible; otherwise add a comment\n that the variable is known to cause runtime initialization.\n Don't reorder keywords (Q_CONSTINIT first — may be attribute).\n Rationale: avoids Static Initialization Order Fiasco and\n improves startup performance for libraries linking the module.\n\n## Methods (Framework)\n\n- **FW-MTH-2**: If inline must be out-of-class: `inline` on\n declaration, never on definition (MinGW DLL export issue).\n- **FW-MTH-3**: Const-ref getter → add lvalue-this overload\n (`const &`) and rvalue-this overload (`&&` returning by value).\n- **FW-MTH-4**: Pass geometric types by value regardless of ABI.\n Ditto views and built-in types.\n\n## Properties (Framework)\n\n- **FW-PRP-2**: Existing QML-exposed classes: do NOT add FINAL to\n new or existing properties (source compat breakage for\n subclasses outside the module).\n\n## Documentation\n\n- **FW-DOC-1**: New public classes: complete docs with `\\since`\n tag and overview section; check `\\ingroup` for discoverability.\n- **FW-DOC-2**: Mention in \"What's New in Qt 6\" if appropriate.\n\n## Value Classes\n\n- **FW-VAL-1**: Follow draft QUIP-22 value-class mechanics.\n- **FW-VAL-2**: Never QSharedPointer for d-pointers (2× size).\n Use QExplicitlySharedDataPointer, not QSharedDataPointer.\n- **FW-VAL-3**: Don't forget Q_DECLARE_SHARED (provides\n Q_DECLARE_TYPEINFO + ADL swap).\n- **FW-VAL-4**: Member-swap: swap in declaration order, use\n member's member-swap > qt_ptr_swap > std::swap. Never qSwap.\n- **FW-VAL-5**: Don't add Q_DECLARE_METATYPE (automatic since\n Qt 6).\n- **FW-VAL-6**: Move SMFs: inline and noexcept.\n- **FW-VAL-7**: Never export non-polymorphic class wholesale.\n Export only public/protected out-of-line members.\n\n## Polymorphic Classes (Framework)\n\n- **FW-PLY-1**: Dtor out-of-line (=default in .cpp) — required\n for stable ABI. Subclass dtors: `override`.\n\n## QObject Subclasses (Framework)\n\n- **FW-QOB-2**: Always override QObject::event(), even if just\n `return Base::event()`. Out-of-line, protected.\n- **FW-QOB-3**: Include all moc files in main .cpp.\n- **FW-QOB-5**: Reuse QObject::d_ptr (or comment why not).\n\n## RAII Classes (Framework)\n\n- **FW-RAI-1**: QUIP-19: `[[nodiscard]]` on ctors.\n\n## Special Member Functions (Framework)\n\n- **FW-SMF-2**: SMF/swap argument name: always `other`.\n- **FW-SMF-3**: Copy SMFs of implicitly-shared classes: usually\n NOT noexcept (allocation on detach).\n- **FW-SMF-4**: Every ctor: `Q_IMPLICIT` or `explicit`.\n- **FW-SMF-5**: Default ctors: implicit (not explicit).\n- **FW-SMF-7**: Move-assignment: use QT_MOVE_ASSIGNMENT_OPERATOR_\n IMPL_VIA_{MOVE_AND_SWAP|PURE_SWAP}. PURE_SWAP only for\n memory-only resources.\n\n## Enums (Framework)\n\n- **FW-ENM-1**: qnamespace.h is over-populated — a new general\n enum there needs strong justification. Prefer the owning class,\n or a Q_NAMESPACE domain namespace when the vocabulary is shared\n across classes (class scope limits use cases and drives\n duplication — weigh both; see universal ENM-10).\n- **FW-ENM-3**: New enumerators: `\\value [since VERSION]` in docs.\n- **FW-ENM-6**: Scoped enums in QML-exposed classes:\n `Q_CLASSINFO(\"RegisterEnumClassesUnscoped\", \"false\")`.\n\n## Namespaces\n\n- **FW-NSP-1**: Namespaces: `QtFoo`, not `QFoo`.\n\n## Templates (Framework)\n\n- **FW-TPL-2**: Prefer `std::disjunction_v` over `||`.\n Ditto conjunction/negation.\n- **FW-TPL-3**: Never chain is_same in a disjunction. Use\n specialized helper.\n- **FW-TPL-4**: Canonical constraint form:\n `template \u003ctypename T, if_condition\u003cT> = true>`.\n\n## Relational Operators (Framework)\n\n- **FW-REL-1**: Avoid signed/unsigned comparison. Use\n `q20::cmp_*` (Qt's C++20 backport shim).\n\n## Conditional Compilation (Framework)\n\n- **FW-CND-2**: Use `__cpp_lib_*` macros, not `__has_include()`\n for standard library feature detection.\n- **FW-CND-3**: Don't check `defined()` if initial version is in\n required C++ standard.\n\n## QML Module Versioning\n\n- **FW-QML-1**: New properties/methods/signals must be revisioned.\n- **FW-QML-2**: Use two-argument forms for REVISION/Q_REVISION.\n- **FW-QML-3**: Don't add new props/signals to QObject class\n itself (affects all QML consumers).\n\n## Commit Message\n\n- **FW-CMT-1**: Demand rationale (not just Jira/task link).\n- **FW-CMT-2**: Reject unrelated drive-by changes.\n- **FW-CMT-3**: Drive-by changes spelled out in commit message.\n- **FW-CMT-4**: Amends: full sha1.\n- **FW-CMT-5**: ChangeLog entry: correct tense (past for fixes,\n present for new features).\n- **FW-CMT-6**: Imperative mood, no passive voice.\n- **FW-CMT-7**: Correct capitalization.\n- **FW-CMT-8**: Change-Id last; Pick-to/Task-number/Fixes before.\n","content_type":"text/markdown; charset=utf-8","language":"markdown","size":6926,"content_sha256":"39d3cef13506fe67cd2cdf570794c54624c95073fcdfba14866359be50d037ee"},{"filename":"references/qt-review-checklist.md","content":"# Qt6 Code Review Checklist\n\nDistilled from the Qt Wiki \"Things To Look Out For In Reviews\".\nEach rule has a short ID for cross-referencing in review reports.\n\nRules specific to Qt framework/module development (binary\ncompatibility, export macros, d-pointers, qdoc, QML versioning)\nare in `qt-framework-checklist.md` — loaded only when reviewing\nQt module code.\n\n## API & Naming\n\n- **API-3**: Check naming consistency with similar Qt classes\n (e.g. `timeout` not `timeOut`, `size()` not `count()`).\n- **API-5**: `get`-prefix means user interaction or decomposition\n (out-params), NOT mere getters.\n\n## Public Headers\n\n- **HDR-3**: Protect min/max calls: `(std::min)(a,b)`,\n `(std::numeric_limits\u003cT>::min)()`. Also in .cpp files\n (unity builds).\n\n## Includes\n\n- **INC-4**: Include everything needed in-size (Lakos). Don't\n rely on transitive includes.\n\n## Variables\n\n- **VAR-3**: Braced initializers: opening `{` on same line.\n Prefer `var = {` over `var{`.\n- **VAR-4**: Never use dynamically-sized containers for\n statically-sized data. Use `std::array` or C arrays.\n\n## Methods\n\n- **MTH-1**: Inline methods: prefer defining in class body\n (skip `inline` keyword).\n\n## Macros\n\n- **MAC-1**: Function-scope macros → `do {} while(false)`.\n\n## Properties\n\n- **PRP-1**: New classes: Q_PROPERTY FINAL unless intended for\n override.\n- **PRP-3**: Avoid properties with same name as meta-methods\n (shadowing in QML).\n\n## Timeouts\n\n- **TMO-1**: No ints/qint64 for timeouts or intervals. Use\n QDeadlineTimer or std::chrono types.\n\n## Polymorphic Classes\n\n- **PLY-2**: Q_DISABLE_COPY_MOVE on polymorphic classes.\n- **PLY-3**: Overridden virtuals: same default args and access\n specifier as base. Comment if intentional deviation.\n- **PLY-4**: Virtual functions marked by exactly ONE of\n `virtual`, `override`, `final`.\n- **PLY-5**: If class is `final`, use `override` on methods\n (not `final`).\n- **PLY-6**: Virtual access: public if callable, private if\n reimpl shouldn't call base, protected if reimpl should call\n base.\n\n## QObject Subclasses\n\n- **QOB-1**: Always include Q_OBJECT macro.\n- **QOB-4**: Idiomatic element order: Q_OBJECT, Q_PROPERTY,\n Q_CLASSINFO, public (enums, ctors, all non-mutating methods),\n public slots, signals, event handlers, protected, private.\n\n## RAII Classes\n\n- **RAI-2**: Q_DISABLE_COPY. Make movable (or comment why not).\n- **RAI-3**: Move-assignment: use move-and-swap.\n\n## Tests\n\n- **TST-1**: QCOMPARE_EQ for QStringList comparisons.\n- **TST-2**: QCOMPARE: tested value first, expected second.\n- **TST-3**: QSKIP over `#if` for non-pertinent tests.\n\n## Special Member Functions\n\n- **SMF-1**: Order: default ctor, non-SMF ctors, copy ctor,\n copy-assign, move ctor, move-assign, dtor, swap.\n- **SMF-6**: Never implement copy/move ctor via assignment.\n\n## Enums\n\n- **ENM-1**: Trailing comma on last enumerator — reduces diff\n noise when adding new values.\n- **ENM-2**: Scoped or explicit underlying type — prevents the\n underlying type from changing (binary compatibility break).\n- **ENM-4**: Purpose clarity: enumeration (no =), QFlags\n (= 0x), strong typedef (arithmetic ops).\n- **ENM-5**: `{}` (value 0) should mean \"default\".\n- **ENM-7**: Switch over enum: no `default:` label, list all\n enumerators explicitly.\n- **ENM-8**: A fixed, project-owned vocabulary must be an enum,\n never a string. A parameter, member, or DTO/JSON field holding\n a closed set of values (state, mode, kind, phase, status, …)\n typed as QString gladly accepts new — and wrong — values;\n adding a value must break every switch that forgot it, which\n a string silently doesn't. Tells: `QString state`, comparisons\n against value literals (`mode == \"dark\"`), a comment\n enumerating the legal values. Strings are only for identifiers\n the project does NOT own (external system names, protocol or\n vendor ids).\n- **ENM-9**: The boolean parameter trap: non-intuitive `bool`\n parameters — or a call site like `f(text, false, false, true)`\n — must be named enums:\n `f(text, LogRule::DontLog, ChangeRule::TestOnly,\n FailureRule::AssertOnFailure)`. Even a single bool deserves an\n enum when the call site doesn't read (`sort(true)`). Boolean\n parameters are allowed when their function is clear at the\n call site, like `setVisible(true)`.\n- **ENM-10**: Placement: class scope when the vocabulary belongs\n to one class's API; a `Q_NAMESPACE`-registered namespace when\n shared across classes. Never duplicate an enum because the\n first copy was scoped too narrowly — widen the original.\n\n## Exceptions / noexcept\n\n- **NXC-1**: If a function is marked `noexcept`, verify it\n cannot fail — check for allocation, Q_ASSERT (precondition\n style), and calls to functions that may throw. Flag only if\n a clear throwing path is found (Lakos Rule).\n- **NXC-2**: Smart pointer `operator*()` may be noexcept but\n then must not contain Q_ASSERT.\n- **NXC-3**: Q_ASSERTs checking caller obligations\n (preconditions) are incompatible with noexcept. Q_ASSERTs\n verifying internal invariants are acceptable in noexcept\n functions. If the distinction is unclear, report as an\n investigation target for human verification.\n\n## Functions — Returning Data\n\n- **RET-1**: Prefer returning by value (compilers dislike\n out-params).\n- **RET-2**: Write functions to enable RVO/NRVO. Don't mix\n named and unnamed returns in the same function. Flag only\n when mixed return paths are visible in the source — do not\n guess whether the compiler will apply the optimization.\n\n## Move Semantics\n\n- **MOV-1**: Distinguish rvalue refs (std::move) from universal\n refs (std::forward).\n- **MOV-2**: Document moved-from state: default-constructed,\n valid-but-unspecified, or partially-formed.\n\n## Operators\n\n- **OPR-1**: Operators as hidden friends of least-general class.\n- **OPR-2**: Never break equality/qHash relation.\n- **OPR-3**: No fuzzy FP comparisons in regular relational\n operators.\n\n## Lambdas\n\n- **LAM-1**: Always name lambdas (except IILE, private slots).\n- **LAM-2**: Use domain-specific names, not prose of\n implementation.\n- **LAM-3**: Stateful lambdas: lambda-returning-lambda pattern.\n- **LAM-4**: Omit `()` when empty (unless needed for ->, mutable,\n noexcept).\n\n## Templates\n\n- **TPL-1**: Know mandates (static_assert) vs constraints\n (SFINAE). Use constraints when overloaded.\n- **TPL-5**: Don't explicitly specify deducible template args.\n\n## Ternary Operator\n\n- **TRN-1**: Nested ternaries: one condition per line.\n- **TRN-2**: Long condition: break with `?`/`:` at start of\n continuation line.\n- **TRN-3**: Never use ternary to invert/convert to bool.\n\n## Relational Operators\n\n- **REL-2**: Never convert unsigned to signed for comparison.\n\n## Conditional Compilation\n\n- **CND-1**: Don't extra-indent inside temporary #ifdefs.\n\n## Model Contracts (QAbstractItemModel)\n\n- **MDL-1**: Structural changes (add/remove/move rows) must use\n proper begin/end signals, not `layoutChanged`.\n- **MDL-2**: `dataChanged` should pass specific changed roles,\n not an empty vector (empty = \"all roles changed\").\n- **MDL-3**: `setData()` must emit `dataChanged` before\n returning true (or not return true without emitting).\n- **MDL-4**: `beginRemoveRows(parent, 0, count-1)` where\n count==0 violates QAIM contract (first > last).\n- **MDL-5**: `flags()` should return appropriate flags per\n item type (e.g. no `ItemIsEditable` on category nodes).\n- **MDL-6**: begin/end signal pairs must be balanced within\n each code path.\n- **MDL-7**: `data()` switch should list all role cases\n explicitly; avoid `default:` (suppresses -Wswitch).\n- **MDL-8**: `roleNames()` return must match `data()` switch\n cases. Missing cases return `QVariant()` silently.\n- **MDL-9**: Proxy/filter models must use source model's\n `data()`/`index()` API, not raw struct pointers.\n- **MDL-10**: `roleNames()` should cache the QHash (static\n local or member), not rebuild on every call.\n\n## Error Handling & Validation\n\n- **ERR-1**: Check `QFile::open()` return value before\n reading/writing.\n- **ERR-2**: Check `QJsonDocument::fromJson()` result with\n `isNull()`/`isObject()` before accessing data.\n- **ERR-3**: Check `QNetworkReply::error()` before\n `readAll()`.\n- **ERR-4**: Use `https://` not `http://` for network URLs.\n- **ERR-5**: Set `QNetworkRequest::setTransferTimeout()` on\n all network requests.\n- **ERR-6**: Match `QString::arg()` placeholder count to\n `.arg()` call count.\n- **ERR-7**: Check `QXmlStreamWriter::hasError()` after\n writing.\n- **ERR-8**: Validate negative values in integer setters\n (not just zero).\n- **ERR-9**: Handle `QNetworkAccessManager::sslErrors` signal.\n- **ERR-10**: Validate schema version on imported data.\n- **ERR-11**: Validate input lengths from untrusted sources\n (imported JSON, network downloads).\n- **ERR-12**: Consistent error reporting pattern across\n methods (don't mix return-bool, set-error, emit-signal).\n\n## Resource Lifecycle\n\n- **LCY-1**: Call `deleteLater()` on QNetworkReply in every\n finished handler.\n- **LCY-2**: QObject-derived objects created with `new` should\n have a parent (or explicit lifecycle management).\n- **LCY-3**: Never put side-effectful expressions inside\n `Q_ASSERT` (compiled out in release).\n- **LCY-4**: Don't use `Q_ASSERT(ptr)` as the sole null guard\n before dereference (crashes in release).\n- **LCY-5**: Cap unbounded container growth (append-only lists\n with no trim/clear).\n- **LCY-6**: Destructor must clean up owned children\n recursively (qDeleteAll on direct children leaks\n grandchildren).\n\n## Thread Safety\n\n- **THR-1**: Never write QObject member variables from\n `QtConcurrent::run()` without synchronization (mutex,\n atomic, or queued invocation).\n- **THR-2**: Never emit signals from worker threads with\n `Qt::DirectConnection` to main-thread receivers.\n- **THR-3**: Never mutate QAbstractItemModel from background\n threads.\n- **THR-4**: Protect shared containers consistently with mutex\n across all code paths (not just some).\n- **THR-5**: Use `std::atomic` or mutex for shared counters\n accessed from multiple threads.\n\n## Performance & Code Quality\n\n- **PRF-1**: Don't construct `QRegularExpression` inside loops\n (expensive compilation).\n- **PRF-2**: Cache `roleNames()` QHash (static local or member).\n- **PRF-3**: Use `const auto&` in range-for over shared\n containers to avoid COW detach.\n- **PRF-4**: Use `.value()` not `operator[]` for reads on\n shared QHash/QMap (avoids detach).\n- **PRF-5**: Put cheap early-exit checks before expensive\n operations.\n- **PRF-6**: Flag likely dead code (unreachable branches, unused\n methods, unused members). If callers may exist outside the\n reviewed scope (templates, plugins, reflection), report as\n investigation target instead of confirmed finding.\n- **PRF-7**: Extract magic numbers to named constants.\n- **PRF-8**: Don't use `QMap` for small fixed-size constant\n data (use array, switch, or if-chain).\n- **PRF-9**: Invalidate member caches when underlying data\n changes.\n- **PRF-10**: Add re-entrancy guards on methods that emit\n signals which could trigger recursive calls.\n","content_type":"text/markdown; charset=utf-8","language":"markdown","size":11077,"content_sha256":"3c38a74d1836963ab6f7a69187fc9152086ef65d6e4517dae196352795951a18"}],"content_json":{"type":"doc","content":[{"type":"heading","attrs":{"level":1},"content":[{"text":"Qt Code Review","type":"text"}]},{"type":"paragraph","content":[{"text":"Read-only review of Qt6 C++ code: a deterministic Python linter first, then agent-driven deep analysis across six rule domains, then one consolidated report.","type":"text"}]},{"type":"heading","attrs":{"level":2},"content":[{"text":"When to use this skill","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Review-related requests: \"review\", \"check\", \"audit\", \"look over\", \"code review\", \"sanity check\"","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Suggest it ","type":"text"},{"text":"before committing","type":"text","marks":[{"type":"strong"}]},{"text":" code","type":"text"}]}]}]},{"type":"heading","attrs":{"level":2},"content":[{"text":"Arguments","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"/qt-cpp-review","type":"text","marks":[{"type":"code_inline"}]},{"text":" — universal Qt6 C++ rules","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"/qt-cpp-review framework","type":"text","marks":[{"type":"code_inline"}]},{"text":" — also apply Qt framework/module rules (binary compatibility, exports, d-pointers, qdoc, QML versioning)","type":"text"}]}]}]},{"type":"heading","attrs":{"level":2},"content":[{"text":"Framework mode","type":"text"}]},{"type":"paragraph","content":[{"text":"Enable when ","type":"text"},{"text":"$ARGUMENTS","type":"text","marks":[{"type":"code_inline"}]},{"text":" contains \"framework\". Otherwise, if the lint run or a quick Grep shows ","type":"text"},{"text":"two or more","type":"text","marks":[{"type":"strong"}]},{"text":" of these signals, suggest framework mode to the user but do not enable it yourself: ","type":"text"},{"text":"QT_BEGIN_NAMESPACE","type":"text","marks":[{"type":"code_inline"}]},{"text":", any ","type":"text"},{"text":"Q_*_EXPORT","type":"text","marks":[{"type":"code_inline"}]},{"text":" macro, ","type":"text"},{"text":"\u003cQtModule/private/*_p.h>","type":"text","marks":[{"type":"code_inline"}]},{"text":" includes, ","type":"text"},{"text":"Q_DECLARE_PRIVATE","type":"text","marks":[{"type":"code_inline"}]},{"text":"/","type":"text"},{"text":"Q_D()","type":"text","marks":[{"type":"code_inline"}]},{"text":"/","type":"text"},{"text":"Q_Q()","type":"text","marks":[{"type":"code_inline"}]},{"text":", ","type":"text"},{"text":"qt_internal_add_module","type":"text","marks":[{"type":"code_inline"}]},{"text":" in CMakeLists.txt, ","type":"text"},{"text":"sync.profile","type":"text","marks":[{"type":"code_inline"}]},{"text":" or ","type":"text"},{"text":".qmake.conf","type":"text","marks":[{"type":"code_inline"}]},{"text":" at the repo root.","type":"text"}]},{"type":"paragraph","content":[{"text":"When enabled: pass ","type":"text"},{"text":"--framework","type":"text","marks":[{"type":"code_inline"}]},{"text":" to the linter, and name ","type":"text"},{"text":"references/qt-framework-checklist.md","type":"text","marks":[{"type":"code_inline"}]},{"text":" in the prompts of the agents covering A2 or A4 (they apply it as mapped at the end of ","type":"text"},{"text":"agent-rules.md","type":"text","marks":[{"type":"code_inline"}]},{"text":"). In diff scope, also check the commit message against the FW-CMT rules yourself; load only that section with ","type":"text"},{"text":"sed -n '/^## Commit Message/,$p' references/qt-framework-checklist.md","type":"text","marks":[{"type":"code_inline"}]},{"text":".","type":"text"}]},{"type":"heading","attrs":{"level":2},"content":[{"text":"Scope detection","type":"text"}]},{"type":"paragraph","content":[{"text":"Diff scope","type":"text","marks":[{"type":"strong"}]},{"text":" — \"this commit\", \"these changes\", \"the diff\", \"what I changed\", \"staged changes\", \"before I commit\". The range is ","type":"text"},{"text":"HEAD","type":"text","marks":[{"type":"code_inline"}]},{"text":" (working tree plus index), ","type":"text"},{"text":"--cached","type":"text","marks":[{"type":"code_inline"}]},{"text":" for staged changes only, or ","type":"text"},{"text":"HEAD~1..HEAD","type":"text","marks":[{"type":"code_inline"}]},{"text":" for \"this commit\". Do not load the diff itself: the linter's ","type":"text"},{"text":"--diff","type":"text","marks":[{"type":"code_inline"}]},{"text":" mode (Phase 1) lists the changed files with their changed-line counts and reports only findings on changed lines.","type":"text"}]},{"type":"paragraph","content":[{"text":"Codebase scope","type":"text","marks":[{"type":"strong"}]},{"text":" — \"review the codebase\", \"audit src/\", or an explicit path with no commit language. Glob ","type":"text"},{"text":"*.cpp","type":"text","marks":[{"type":"code_inline"}]},{"text":", ","type":"text"},{"text":"*.h","type":"text","marks":[{"type":"code_inline"}]},{"text":", ","type":"text"},{"text":"*.hpp","type":"text","marks":[{"type":"code_inline"}]},{"text":" under the path.","type":"text"}]},{"type":"heading","attrs":{"level":2},"content":[{"text":"Token budget rules","type":"text"}]},{"type":"paragraph","content":[{"text":"These rules exist to keep the review cheap. Follow them.","type":"text"}]},{"type":"ordered_list","attrs":{"order":1,"listStyle":"number"},"content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Do not load the reference files into the main context.","type":"text","marks":[{"type":"strong"}]},{"text":" Agents read ","type":"text"},{"text":"references/agent-rules.md","type":"text","marks":[{"type":"code_inline"}]},{"text":" themselves; you pass its absolute path and the section IDs. Only an inline run (Phase 2) reads it in the main context. Lint messages are self-describing; the checklists are for humans.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Pass lint results in ","type":"text","marks":[{"type":"strong"}]},{"text":"--summary","type":"text","marks":[{"type":"code_inline"},{"type":"strong"}]},{"text":" form","type":"text","marks":[{"type":"strong"}]},{"text":" (grouped by rule, paths relative to a root printed once), never the raw per-line listing.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Point agents at content, never paste it.","type":"text","marks":[{"type":"strong"}]},{"text":" Every token you write into a prompt is output on the orchestrating model, the most expensive tokens in the review. Prompts carry paths, commands and section IDs; the agents load the rules, the files and the diff. In diff scope an agent gets the ","type":"text"},{"text":"git diff -U50 \u003crange> -- \u003cfiles>","type":"text","marks":[{"type":"code_inline"}]},{"text":" command instead of the hunks, and reads other files only to trace a specific symbol.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Scale the agent count to the scope","type":"text","marks":[{"type":"strong"}]},{"text":" (see Phase 2). Launch no agents for a small scope and split a large one into shards of about equal line count.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Use the dedicated review agent where the harness has one.","type":"text","marks":[{"type":"strong"}]},{"text":" If the ","type":"text"},{"text":"cpp-review-agent","type":"text","marks":[{"type":"code_inline"}]},{"text":" definition (listed as ","type":"text"},{"text":"qt-development-skills:cpp-review-agent","type":"text","marks":[{"type":"code_inline"}]},{"text":" when installed with the plugin, plain ","type":"text"},{"text":"cpp-review-agent","type":"text","marks":[{"type":"code_inline"}]},{"text":" when copied by hand) is available, launch the agents with it (read-only, tools limited to Read, Grep, Glob and read-only Bash); otherwise use the harness's general-purpose agent. The definition lives in the plugin's ","type":"text"},{"text":"agents/","type":"text","marks":[{"type":"code_inline"}]},{"text":" directory, outside this skill, so it is missing when the skill was copied on its own or the harness does not install ","type":"text"},{"text":"agents/","type":"text","marks":[{"type":"code_inline"}]},{"text":". Then add one line to each prompt: read-only (Bash only for ","type":"text"},{"text":"cat","type":"text","marks":[{"type":"code_inline"}]},{"text":", ","type":"text"},{"text":"grep","type":"text","marks":[{"type":"code_inline"}]},{"text":", ","type":"text"},{"text":"awk","type":"text","marks":[{"type":"code_inline"}]},{"text":", ","type":"text"},{"text":"git diff","type":"text","marks":[{"type":"code_inline"}]},{"text":", ","type":"text"},{"text":"git show","type":"text","marks":[{"type":"code_inline"}]},{"text":"); take ","type":"text"},{"text":"file:line","type":"text","marks":[{"type":"code_inline"}]},{"text":" from the printed numbers; in diff scope do not re-read the files; Grep or Read other files only to trace a symbol; prefer few confirmed findings over many guesses.","type":"text"}]}]}]},{"type":"heading","attrs":{"level":2},"content":[{"text":"Execution order","type":"text"}]},{"type":"paragraph","content":[{"text":"Never skip a phase.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Phase 1: Deterministic linting","type":"text"}]},{"type":"code_block","attrs":{"wrap":false,"language":"bash"},"content":[{"text":"# codebase scope\npython3 \u003cskill>/references/lint-scripts/qt_review_lint.py --summary [--framework] \u003cfiles...>\n# diff scope, run inside the repository; no file arguments\npython3 \u003cskill>/references/lint-scripts/qt_review_lint.py --summary [--framework] --diff=\u003crange>\n# fall back to `python` if python3 is not found","type":"text"}]},{"type":"paragraph","content":[{"text":"Python 3.7+, no dependencies. If Python is unavailable, warn and continue (in diff scope, take the file list from ","type":"text"},{"text":"git diff --stat \u003crange>","type":"text","marks":[{"type":"code_inline"}]},{"text":"). The linter is authoritative; do not second-guess its output. Rule families: INC, DEP, PAT, MDL, ERR, LCY, API, ENM, TMO, CND, VAL, HDR, TRN, VAR (60+ checks). Exit code 1 simply means findings exist; 2 means a usage or git error.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Phase 2: Deep analysis (inline, or one agent per shard)","type":"text"}]},{"type":"paragraph","content":[{"text":"The six sections of ","type":"text"},{"text":"references/agent-rules.md","type":"text","marks":[{"type":"code_inline"}]},{"text":" are the analysis. Agents read it themselves; read it yourself only when running inline:","type":"text"}]},{"type":"table","attrs":{"layout":null},"content":[{"type":"tr","content":[{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Section","type":"text"}]}]},{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Mission","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"A1","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Model Contracts","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"A2","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Ownership & Lifecycle","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"A3","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Thread Safety","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"A4","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"API, Naming & C++ Correctness","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"A5","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Error Handling & Validation","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"A6","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Performance & Code Quality","type":"text"}]}]}]}]},{"type":"paragraph","content":[{"text":"Choose the fan-out from the scope size. Count the lines of the files in scope (the changed lines in diff scope), taken from the lint summary or ","type":"text"},{"text":"wc -l","type":"text","marks":[{"type":"code_inline"}]},{"text":". A block is about 2,500 lines; the line count alone decides the split, never the number of files.","type":"text"}]},{"type":"table","attrs":{"layout":null},"content":[{"type":"tr","content":[{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Scope","type":"text"}]}]},{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Agents","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Up to 2,500 lines","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"0 (inline)","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"More than 2,500 lines","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"one per shard, ","type":"text"},{"text":"ceil(lines / 2500)","type":"text","marks":[{"type":"code_inline"}]},{"text":" shards","type":"text"}]}]}]}]},{"type":"paragraph","content":[{"text":"Inline.","type":"text","marks":[{"type":"strong"}]},{"text":" A small scope does not repay the cost of agents, which each rebuild the context and roughly double the review's token cost for the same findings. Do the analysis yourself: read ","type":"text"},{"text":"references/agent-rules.md","type":"text","marks":[{"type":"code_inline"}]},{"text":" and the files (or the diff) with the one Bash call shown under the agent prompt below, and apply A1-A6 in a single pass under the same rules, confidence thresholds and output format. Do not re-report lint rows, and do not run inline above the limit because the code looks simple. Report ","type":"text"},{"text":"Agents: 0 (inline)","type":"text","marks":[{"type":"code_inline"}]},{"text":".","type":"text"}]},{"type":"paragraph","content":[{"text":"Shards.","type":"text","marks":[{"type":"strong"}]},{"text":" Cut the files, in directory order, into n = ","type":"text"},{"text":"ceil(lines / 2500)","type":"text","marks":[{"type":"code_inline"}]},{"text":" shards of about ","type":"text"},{"text":"lines / n","type":"text","marks":[{"type":"code_inline"}]},{"text":" lines each; how many files a shard holds does not matter. Keep a directory in one shard where it fits, so that a class's header and source, and the models and views that reference each other, land together; split a larger directory by file. A file larger than a shard gets a shard to itself. Each shard agent applies all six sections to its shard only and may Grep the whole scope to trace a symbol. Run up to six shard agents at a time and the rest in a second wave. Cross-shard checks (ownership, model contracts, duplicated logic) are then best effort; say so in the report header.","type":"text"}]},{"type":"paragraph","content":[{"text":"Launch the agents in parallel, named after their shard (e.g. \"Review shard 2/6: src/models\"). Keep each prompt short; it contains only:","type":"text"}]},{"type":"ordered_list","attrs":{"order":1,"listStyle":"number"},"content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"The absolute path of ","type":"text"},{"text":"references/agent-rules.md","type":"text","marks":[{"type":"code_inline"}]},{"text":" and the section IDs to apply (A1-A6), plus the absolute path of ","type":"text"},{"text":"references/qt-framework-checklist.md","type":"text","marks":[{"type":"code_inline"}]},{"text":" for A2 and A4 agents in framework mode","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"The root directory from the lint summary, the files in scope relative to it and, in diff scope, the ","type":"text"},{"text":"git diff -U50 \u003crange> -- \u003cfiles>","type":"text","marks":[{"type":"code_inline"}]},{"text":" command for them","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"The lint summary rows for that agent's own files only (a shard agent never needs the other shards' rows), with the instruction \"do not re-report these lines\"","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"The instruction to load everything with one Bash call before analysing, and to return findings in the format at the top of the rules file. Each tool call re-sends the agent's whole context, so one batched read instead of one Read per file is the single largest saving on large scopes. Both forms print per-file line numbers for the ","type":"text"},{"text":"file:line","type":"text","marks":[{"type":"code_inline"}]},{"text":" locations (","type":"text"},{"text":"cat -n","type":"text","marks":[{"type":"code_inline"}]},{"text":" on several files numbers them as one run, and ","type":"text"},{"text":"git diff","type":"text","marks":[{"type":"code_inline"}]},{"text":" has no line numbers):","type":"text"}]},{"type":"code_block","attrs":{"wrap":false,"language":"bash"},"content":[{"text":"# codebase scope\ncd \u003croot> && cat \u003crules> && for f in \u003cfiles...>; do echo \"== $f\"; cat -n \"$f\"; done\n# diff scope; numbers context and added lines with the new-file line\ncd \u003croot> && cat \u003crules> && git diff -U50 \u003crange> -- \u003cfiles> | awk '/^@@/{split($3,a,/[+,]/);n=a[2];h=1;print;next} /^diff /{h=0} h&&/^[ +]/{printf \"%5d %s\\n\",n++,$0;next} {print}'","type":"text"}]}]}]},{"type":"paragraph","content":[{"text":"Agents are read-only and must never edit files. In tools without agents, run each mission as whatever prompt chain or subprocess the tool supports; the contract is the same.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Phase 3: Consolidation","type":"text"}]},{"type":"paragraph","content":[{"text":"Merge lint and agent findings. Deduplicate on file+line+issue. When duplicates disagree on confidence, keep the block with the highest score and its trace; never average scores. Write the report in the format below.","type":"text"}]},{"type":"heading","attrs":{"level":2},"content":[{"text":"Confidence scoring","type":"text"}]},{"type":"table","attrs":{"layout":null},"content":[{"type":"tr","content":[{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Confidence","type":"text"}]}]},{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Meaning","type":"text"}]}]},{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Action","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"80-100","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Rule violation confirmed with a symbol trace","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Finding","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"60-79","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Likely, not fully verifiable (e.g. noexcept needing whole-program analysis, dead code with possible external callers)","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Investigation target (max 10)","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"\u003c60","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Suspicion only","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Suppress","type":"text"}]}]}]}]},{"type":"heading","attrs":{"level":2},"content":[{"text":"Output format","type":"text"}]},{"type":"paragraph","content":[{"text":"Use exactly this structure. Lint findings are grouped by rule with one mitigation each; only deep-analysis findings get a full block.","type":"text"}]},{"type":"code_block","attrs":{"wrap":false,"language":""},"content":[{"text":"## Qt Code Review Report\n\n**Scope**: [diff: `git diff HEAD~1..HEAD` | files: \u003cpaths>]\n**Files reviewed**: N\n**Issues found**: N (M from lint, K from deep analysis)\n**Agents**: N (shards ... | inline)\n**Framework mode**: [on / off]\n\n---\n\n### Lint findings\n\n| Rule | Count | Locations | Mitigation |\n|---|---|---|---|\n| LCY-2 QObject created with new but no parent | 3 | `main.cpp:12,40`, `widget.cpp:88` | Pass a parent or hold in a smart pointer. |\n| ... | | | |\n\n---\n\n### Deep analysis findings\n\n#### [D-NNN] \u003cShort title>\n- **File**: `path/to/file.cpp:42`\n- **Category**: \u003cA1-A6 mission name>\n- **Confidence**: NN/100\n- **Finding**: \u003cwhat is wrong>\n- **Trace**: \u003csymbols followed, what was checked>\n- **Mitigation**: \u003cwhat to do, in prose — no code patches>\n\n---\n\n### Investigation targets (human verification needed)\n\n#### [I-NNN] \u003cShort title>\n- **File**: `path/to/file.cpp:42`\n- **Category**: \u003cA1-A6 mission name>\n- **Confidence**: NN/100\n- **Finding**: \u003cwhat is suspected>\n- **Unverified because**: \u003cwhat could not be confirmed>\n- **How to verify**: \u003cspecific action for the reviewer>\n\n---\n\n### Summary\n\n| Category | Lint | Deep | Investigate | Total |\n|---|---|---|---|---|\n| ... | N | N | N | N |\n| **Total** | **M** | **K** | **I** | **N** |\n\nFindings below confidence 60 are suppressed.","type":"text"}]},{"type":"heading","attrs":{"level":2},"content":[{"text":"References","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"references/agent-rules.md","type":"text","marks":[{"type":"code_inline"}]},{"text":" — agent missions A1-A6 and the agent output format (read by the agents, not the orchestrator)","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"references/qt-review-checklist.md","type":"text","marks":[{"type":"code_inline"}]},{"text":" — universal rule IDs and rationale (human reference, not loaded during a review)","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"references/qt-framework-checklist.md","type":"text","marks":[{"type":"code_inline"}]},{"text":" — framework rules (read by A2 and A4 agents in framework mode)","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"references/qt-deprecated-classes.md","type":"text","marks":[{"type":"code_inline"}]},{"text":" — background for the DEP lint rules (human reference)","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"references/lint-scripts/qt_review_lint.py","type":"text","marks":[{"type":"code_inline"}]},{"text":" — single-pass linter, ","type":"text"},{"text":"--summary","type":"text","marks":[{"type":"code_inline"}]},{"text":" for grouped output, ","type":"text"},{"text":"--diff=\u003crange>","type":"text","marks":[{"type":"code_inline"}]},{"text":" for changed lines only, ","type":"text"},{"text":"--framework","type":"text","marks":[{"type":"code_inline"}]},{"text":" for module rules","type":"text"}]}]}]}]},"metadata":{"date":"2026-11-02","name":"qt-cpp-review","author":"@skillopedia","source":{"stars":522,"repo_name":"agent-skills","origin_url":"https://github.com/TheQtCompanyRnD/agent-skills/blob/HEAD/skills/qt-cpp-review/SKILL.md","repo_owner":"TheQtCompanyRnD","body_sha256":"8021e34b9288cfe251c3d628482545b30bb98164ab3d0bd3c9d8a5d288a742a7","cluster_key":"ddb57ee56307deba51760170ba5a6141ef1853377621a1fa237ce5ae44fbd7fc","clean_bundle":{"format":"clean-skill-bundle-v1","source":"TheQtCompanyRnD/agent-skills/skills/qt-cpp-review/SKILL.md","text_bytes":69986,"source_commit":"6845f02d41171a6df8b6e6fe225b7f5d1a6f4adf","skills_sh_rank":12638,"attachment_count":6,"text_attachments":6,"attachment_storage":"skill_attachments","binary_attachments":0},"cluster_size":1,"skill_md_path":"skills/qt-cpp-review/SKILL.md","import_metadata":{"date":"2026-11-02","author":"@skillopedia","version":"v1","category":"software-engineering","category_label":"Engineering"},"exact_dupes_collapsed_into_this":0},"license":"LicenseRef-Qt-Commercial OR BSD-3-Clause","version":"v1","category":"software-engineering","metadata":{"author":"qt-ai-skills","version":"3.0","category":"review","qt-version":"6.x"},"import_tag":"skills-sh-top20k-2026-11","description":"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 deep-analysis agents (scaled to the size of the change) covering model contracts, ownership, threading, API correctness, error handling, and performance. Reports only high-confidence issues (>80/100) with structured mitigations. Read-only — never modifies code.","argument-hint":"[framework]","compatibility":"Designed for Claude Code, GitHub Copilot, and similar agents.","disable-model-invocation":false}},"renderedAt":1793676260502}

Important: agents should read /llm.txt, /llms.txt, or /.well-known/skills.json to discover the public Skillopedia API.