Skip to content

Commit 88b6a52

Browse files
alex-spataruclaude
alex-spataru
and
claude
committed
chore: sync local skills, MCP config and AI search index
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent a1bd3be commit 88b6a52

8 files changed

Lines changed: 1157 additions & 1 deletion

File tree

‎.claude/skills/qt-cpp-review/SKILL.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,11 @@ deprecated classes.
190190
- `noexcept` on a function whose `Q_ASSERT` checks a *precondition* (incompatible).
191191
- Unscoped enum without an explicit underlying type; missing trailing comma on the last
192192
enumerator; `switch` over an enum with a `default:` label (suppresses `-Wswitch`).
193+
- A closed, project-owned vocabulary held in a `QString` (`mode == "dark"`-style
194+
comparisons) instead of an enum — persisted JSON keys and external/vendor ids are exempt.
195+
- Boolean parameter traps: consecutive bool literals at a call site (`f(x, false, true)`),
196+
or a lone `bool` argument the call site can't explain; suggest a named enum. An enum
197+
duplicated across classes instead of centralized (e.g. in `SerialStudio::`).
193198
- `QList<QString>` where `QStringList` is meant.
194199
- A deprecated class from `qt-deprecated-classes.md`. Note repo reality: this codebase uses
195200
`std::shared_ptr` (e.g. `TimestampedFramePtr`), not `QSharedPointer` — do not flag the

‎.claude/skills/qt-cpp-review/references/qt-review-checklist.md‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,22 @@ Each rule has a short ID for cross-referencing in review reports. Repo-specific
6262
- **ENM-2**: Scoped, or explicit underlying type.
6363
- **ENM-5**: `{}` (value 0) should mean "default".
6464
- **ENM-7**: `switch` over an enum: no `default:` label; list all enumerators explicitly.
65+
- **ENM-8**: A fixed, project-owned vocabulary must be an enum, never a string. A parameter,
66+
member, or DTO/JSON field holding a closed set of values (state, mode, kind, phase,
67+
status, ...) typed as `QString` gladly accepts new — and wrong — values; adding a value
68+
must break every switch that forgot it, which a string silently doesn't. Tells:
69+
`QString state`, comparisons against value literals (`mode == "dark"`), a comment
70+
enumerating the legal values. Strings are only for identifiers the project does NOT own
71+
(external system names, protocol/vendor ids — and here, persisted `.ssproj` JSON keys,
72+
which are a wire format).
73+
- **ENM-9**: The boolean parameter trap: non-intuitive `bool` parameters — or a call site
74+
like `f(text, false, false, true)` — should be named enums. Even a single bool qualifies
75+
when the call site doesn't read (`sort(true)`); clear ones like `setVisible(true)` are
76+
fine.
77+
- **ENM-10**: Placement: class scope when the vocabulary belongs to one class's API; a
78+
`Q_NAMESPACE`-registered namespace (like `SerialStudio::`) when shared across classes.
79+
Never duplicate an enum because the first copy was scoped too narrowly — widen the
80+
original.
6581

6682
## Exceptions / noexcept
6783

‎.claude/skills/qt-qml-review/SKILL.md‎

Lines changed: 330 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,330 @@
1+
---
2+
name: qt-qml-review
3+
description: >-
4+
Qt6/QML deep code review for Serial Studio. Use when asked to "review", "audit", "check",
5+
"look over", or "sanity check" QML — or before committing QML changes. Runs the repo linter
6+
(scripts/code-verify.py) as Phase 1 plus optional system qmllint, then six parallel
7+
read-only analysis agents covering bindings, layout, loading/lifecycle, delegates,
8+
states/motion, and performance. Reports only high-confidence findings (>=80/100) with prose
9+
mitigations. Never modifies code.
10+
allowed-tools: Bash(python scripts/code-verify.py:*), Bash(qmllint:*), Bash(which qmllint), Bash(git diff:*), Bash(git show:*), Bash(git log:*), Read, Grep, Glob, Agent
11+
---
12+
13+
# Serial Studio — QML code review
14+
15+
A read-only QML review that pairs the repo's deterministic linter with parallel agent-driven
16+
deep analysis. It finds the QML-semantic bugs `code-verify.py` cannot see (binding loops,
17+
layout sizing breaks, delegate reuse hazards, motion-contract violations). Adapted for this
18+
repo from The Qt Company's `qt-qml-review` skill (v1.0).
19+
20+
This skill is **read-only**. It reports; it never edits. To auto-fix style, use [[ss-verify]].
21+
When authoring QML rather than reviewing it, use [[qt-qml]].
22+
23+
## When to use
24+
25+
- "review", "check", "audit", "look over", "code review", "sanity check" on QML code
26+
- Before committing QML changes (suggest it)
27+
- To validate QML correctness beyond the style linter
28+
29+
## Scope detection
30+
31+
Pick the scope from the user's language.
32+
33+
**Diff scope (narrow)** — "this commit", "these changes", "the diff", "what I changed",
34+
"staged", "before I commit". Get the changeset with `git diff` (unstaged) and
35+
`git diff --cached` (staged); for "this commit" use `git diff HEAD~1..HEAD`. Review only
36+
changed lines plus context (read the surrounding +/-50 lines, but report only issues in the
37+
changed lines). This is the default and the common case here.
38+
39+
**Codebase scope (wide)** — "review the QML", "audit app/qml/Widgets", or a bare path given
40+
without commit language. Glob `*.qml` under the named scope and review all matches.
41+
42+
## Execution order
43+
44+
Three phases. Never skip Phase 1.
45+
46+
### Phase 1 — Deterministic lint (the repo's contract)
47+
48+
The repo's authoritative linter is `scripts/code-verify.py`, not a bundled Qt linter. Run it
49+
read-only on the in-scope files and collect every finding before Phase 2:
50+
51+
```
52+
python scripts/code-verify.py --check <files...>
53+
```
54+
55+
**Errors block CI; advisories are baseline debt — new code must still clear them.** The
56+
linter is authoritative for comment/property/structure/banner rules — do not re-derive or
57+
second-guess them (see [[ss-verify]]). Pass its full output to every Phase 2 agent so they
58+
don't re-report what it already caught.
59+
60+
This skill ships **no second linter**: the mechanically-checkable Qt rules from the upstream
61+
checklist (marked `(lint)`) are cross-checked by the Phase 2 agents instead.
62+
63+
### Phase 1b — System qmllint (optional)
64+
65+
If `qmllint` is on PATH (`which qmllint`), run it with JSON output on the in-scope files and
66+
merge its findings (deduplicate by file+line+issue). qmllint is authoritative for type-level
67+
checks (unresolved types, incompatible assignments, alias cycles). If absent, note that and
68+
continue — never install anything.
69+
70+
### Phase 2 — Deep analysis (6 parallel agents)
71+
72+
Launch the six agents below **in parallel** as read-only general-purpose subagents (one
73+
`Agent` call per mission, all in one message). Name each so progress is visible
74+
("Agent 5: States, Transitions & Motion"). The missions are deliberately **named and
75+
disjoint** — a named lens loads the analysis it names, where a generic "review thoroughly"
76+
pass skims (`doc/claude/j-space.md`, named lenses). Keep them that way: don't merge missions
77+
to save agents, and pass each agent its mission verbatim, not a paraphrased blend. Pass each
78+
agent: (1) the file list in scope, (2) the Phase 1/1b lint output, (3) its mission. Each
79+
agent reads the in-scope files, greps to trace ids/symbols across `app/qml` and the C++
80+
exposure points, and reports in the structured format below. Agents **never** edit or write
81+
files, and never duplicate a Phase 1 finding.
82+
83+
Confidence: `>=80` = confirmed finding; `60-79` = investigation target (max 10 total across
84+
all agents); `<60` = suppress.
85+
86+
## Agent missions
87+
88+
Every agent loads `references/qt-qml-review-checklist.md` (universal rules) and
89+
`references/serial-studio-qml-rules.md` (repo rules — **supersede the checklist on
90+
conflict**; notably: `Cpp_*` context objects are the established C++ surface, and the
91+
Controls style is pinned centrally, so checklist § 14 and IMP-3 never fire here).
92+
93+
---
94+
95+
### Agent 1: Bindings & Properties
96+
97+
**Scope**: binding correctness, property types, alias chains, qualified lookup, binding
98+
loops.
99+
100+
**Check for**:
101+
- Multi-cycle binding loops (A changes B via handler, B's binding updates A) — the runtime
102+
only detects single-cycle.
103+
- Property alias chains (alias to alias) where the intermediate component may not be
104+
initialized.
105+
- Unqualified property access (a bare name where a `root.`-qualified reference is meant).
106+
Unqualified `Cpp_*` access is the repo norm — never flag it.
107+
- `Qt.binding()` closures capturing loop variables declared with `var` (capture by
108+
reference — use `let`).
109+
- Missing `pragma ComponentBehavior: Bound` on files whose delegates access outer-scope ids.
110+
- Missing `readonly` on properties that are bound but never imperatively assigned.
111+
- `property var` where a concrete type exists; imperative `=` silently destroying a
112+
declarative binding.
113+
114+
**Ref**: `qt-qml-review-checklist.md` § 3 (Bindings & Properties).
115+
116+
---
117+
118+
### Agent 2: Layout & Anchoring
119+
120+
**Scope**: anchoring correctness, layout sizing, visual-tree structure.
121+
122+
**Check for**:
123+
- `anchors` and `Layout.*` mixed on the same item.
124+
- Bare `width`/`height`/`x`/`y` on a direct child of a `RowLayout`/`ColumnLayout`/
125+
`GridLayout` (silently breaks size negotiation — must be `Layout.*`).
126+
- Anchoring to an item with `visible: false`, or across unrelated visual-tree branches.
127+
- `implicitWidth`/`implicitHeight` bindings inside a Layout that can feed back into the
128+
layout's own size negotiation.
129+
- Missing `Layout.fillWidth`/`Layout.fillHeight` on items that should stretch; nested
130+
Layouts with no clear sizing policy.
131+
- A reusable component fixing `width`/`height` instead of `implicitWidth`/`implicitHeight`
132+
(prevents consumer resizing).
133+
134+
**Ref**: `qt-qml-review-checklist.md` § 4 (Layout & Anchoring).
135+
136+
---
137+
138+
### Agent 3: Component Loading & Lifecycle
139+
140+
**Scope**: Loader patterns, dynamic object creation, Connections lifecycle, the QML/C++
141+
boundary.
142+
143+
**Check for**:
144+
- `Component.createObject()` return values not tracked or destroyed (leak).
145+
- Loader switching between `source` and `sourceComponent` at runtime (unsupported), or
146+
`Loader.item` accessed without a `status === Loader.Ready` guard.
147+
- Heavy conditional UI built inline instead of behind `Loader { active: ... }`; a Loader
148+
that should be `asynchronous: true`.
149+
- `Image` with a dynamic source and no `Image.status` error handling; large images without
150+
`sourceSize`.
151+
- `Connections` with a dynamically-changing `target` not handling the null-target state.
152+
- Parentless objects returned from invokable C++ functions (JavaScript-ownership surprises).
153+
Do NOT flag `Cpp_*` context objects — that is the repo's established exposure mechanism
154+
(`serial-studio-qml-rules.md`).
155+
156+
**Ref**: `qt-qml-review-checklist.md` § 5 (Loader), § 8 (Images), § 13 (C++ Integration) —
157+
§ 14's "no context properties" rule is superseded.
158+
159+
---
160+
161+
### Agent 4: ListView & Delegate Correctness
162+
163+
**Scope**: model-view patterns, delegate lifecycle, reuse safety, required properties. In
164+
this repo: the Project Editor tree/table delegates, dashboard widget grids, console views.
165+
166+
**Check for**:
167+
- Missing `required property int index` when `index` is used in a delegate that declares
168+
other required properties; roles accessed that the model's `roleNames()` does not define.
169+
- Mutable per-delegate state (JS vars, non-reset properties) combined with
170+
`reuseItems: true` — reset state in the pooled handler, restore in the reused handler;
171+
pooled delegates left visible.
172+
- Complex delegates (nested Repeaters, multiple Loaders, heavy bindings) that will degrade
173+
scroll performance; `Repeater` + `Column` preferred for small static lists.
174+
- `currentIndex` reliance without guards (QTBUG-48633, QTBUG-93293).
175+
- **Any animation inside a recycled `TreeView`/`ListView` delegate** — banned by the repo's
176+
motion contract.
177+
- `parent` used in a delegate as if it were the view (`ListView.view` or an explicit id is
178+
required), or without a null-check during creation/destruction.
179+
180+
**Ref**: `qt-qml-review-checklist.md` § 6 (ListView & Delegates);
181+
`serial-studio-qml-rules.md` § Motion.
182+
183+
---
184+
185+
### Agent 5: States, Transitions & Motion
186+
187+
**Scope**: state-machine correctness plus this repo's chrome-motion contract — the rules
188+
most likely to be violated silently.
189+
190+
**Check for** (general Qt):
191+
- `PropertyChanges.restoreEntryValues` surprises (properties reverting on state exit);
192+
Qt 5-style `PropertyChanges { target: ... }` syntax.
193+
- Deprecated `Connections` handler syntax (`onFoo:` instead of `function onFoo()`).
194+
- Transitions without `from`/`to` that will fire unexpectedly when new states are added.
195+
- Top-level `states` on a reusable component that should use `StateGroup`.
196+
197+
**Check for** (Serial Studio motion contract — blockers, from `serial-studio-qml-rules.md`):
198+
- A popup/menu/dialog with an ad-hoc `Transition` instead of the shared
199+
`PopupEnter`/`PopupExit` pair plus `transformOrigin`.
200+
- A new scale/slide/bounce animation not gated on `Cpp_Misc_GraphicsBackend.reduceMotion`
201+
(pure fades are exempt).
202+
- Animating anything other than `opacity`/`scale`/`Translate`; animating `width`/`height`
203+
of a laid-out item.
204+
- Any animation inside `Widgets/Dashboard/` (existing value-smoothing `Behavior`s are
205+
established — flag new chrome animation only).
206+
207+
**Ref**: `qt-qml-review-checklist.md` § 7 (States), § 15 (Migration);
208+
`serial-studio-qml-rules.md` § Motion.
209+
210+
---
211+
212+
### Agent 6: Performance, Rendering & Repo Conventions
213+
214+
**Scope**: render cost, JavaScript quality, and the repo's registry/render-cadence rules.
215+
Weight findings in per-tick or per-frame code higher.
216+
217+
**Check for** (general Qt):
218+
- Expensive expressions in hot bindings (cache as `readonly property`);
219+
`QRegularExpression`/heavy computation in loops.
220+
- Transparent `Rectangle` where `Item` suffices; `clip: true` without visual need;
221+
`opacity` on complex subtrees (composites the whole branch); `layer.enabled` left on.
222+
- Default `textFormat` where `Text.PlainText` suffices; missing `Animator` types for
223+
render-thread-friendly `opacity`/`scale`/`rotation`/`x`/`y` animation.
224+
- `var` instead of `let`/`const`; loose equality; signals that communicate down (should be
225+
functions) or functions that communicate up (should be signals).
226+
227+
**Check for** (Serial Studio — from `serial-studio-qml-rules.md`):
228+
- A hardcoded `qrc:/icons/...` path (icons resolve via the icon registry only), or a
229+
hand-written `Menu` in the Project Editor (registry-driven menus only).
230+
- An unconditional per-tick `grab()` on an embedded surface (the 2026-08-17 editor
231+
incident: 13% of the GUI thread).
232+
- A **new** Canvas repainting at interactive rates outside the established dashboard-widget
233+
pattern; never propose rewriting the existing dashboard Canvas widgets.
234+
- Hardcoded colors/fonts bypassing `Cpp_ThemeManager`/`Cpp_Misc_CommonFonts` (check
235+
siblings before flagging — some signature colors are deliberate).
236+
- QML wiring that would receive per-frame C++ signals (GUI traffic is chunk/command/tick
237+
rate only).
238+
239+
**Ref**: `qt-qml-review-checklist.md` § 9-12; `serial-studio-qml-rules.md` § Icons,
240+
§ Render cadence, § Dashboard widgets.
241+
242+
---
243+
244+
### Phase 3 — Consolidate and report
245+
246+
Merge lint output, qmllint output (if it ran), and all agent findings. Deduplicate (same
247+
file+line+issue = one finding). Apply confidence scoring. Emit the report in the **Output
248+
format** below. State plainly when nothing was found — do not invent findings to fill the
249+
report.
250+
251+
## Confidence scoring
252+
253+
| Confidence | Meaning | Action |
254+
|------------|---------|--------|
255+
| 90-100 | Certain: direct rule violation with full trace | Report as finding |
256+
| 80-89 | High: confirmed but an edge case is possible | Report as finding |
257+
| 60-79 | Medium: likely but not fully verifiable | Investigation target |
258+
| <60 | Low: suspicion only | Suppress |
259+
260+
**Investigation targets** are real-but-unverifiable findings (a binding loop needing runtime
261+
confirmation, delegate reuse behavior depending on a C++ model's guarantees). Max 10, sorted
262+
by confidence within the 60-79 band.
263+
264+
## Output format
265+
266+
```
267+
## QML Code Review Report
268+
269+
**Scope**: [diff: <git range> | files: <paths>]
270+
**Files reviewed**: N
271+
**Issues found**: N (M from lint, K from deep analysis)
272+
**qmllint**: [ran / not available]
273+
274+
---
275+
276+
### Lint findings (code-verify.py / qmllint)
277+
278+
#### [L-NNN] <short title>
279+
- **File**: `path/to/file.qml:42`
280+
- **Rule**: <rule id / category>
281+
- **Finding**: <what the linter reported>
282+
- **Mitigation**: <what to do, in prose — no code patches>
283+
284+
---
285+
286+
### Deep analysis findings
287+
288+
#### [D-NNN] <short title>
289+
- **File**: `path/to/file.qml:42`
290+
- **Category**: <Bindings & Properties | Layout & Anchoring | Loading & Lifecycle |
291+
ListView & Delegates | States & Motion | Performance & Conventions>
292+
- **Confidence**: NN/100
293+
- **Finding**: <description>
294+
- **Trace**: <ids/symbols followed / what was checked to confirm it>
295+
- **Mitigation**: <what to do, in prose — no code patches>
296+
297+
---
298+
299+
### Investigation targets (human verification needed)
300+
301+
#### [I-NNN] <short title>
302+
- **File**: `path/to/file.qml:42`
303+
- **Category**: <agent name>
304+
- **Confidence**: NN/100
305+
- **Finding**: <what is suspected>
306+
- **Unverified because**: <what could not be confirmed>
307+
- **How to verify**: <specific action for the reviewer>
308+
309+
---
310+
311+
### Summary
312+
313+
| Category | Lint | Deep | Investigate | Total |
314+
|----------|------|------|-------------|-------|
315+
| ... | N | N | N | N |
316+
| **Total**| **M**| **K**| **I** | **N** |
317+
318+
Findings below confidence 60 are suppressed.
319+
```
320+
321+
## References
322+
323+
- `references/qt-qml-review-checklist.md` — universal Qt6 QML review rules (always loaded;
324+
The Qt Company, BSD-3-Clause).
325+
- `references/serial-studio-qml-rules.md` — this repo's QML invariants (motion contract,
326+
registry-driven chrome, `Cpp_*` surface, render cadence) expressed as review rules
327+
(always loaded; supersedes the checklist on conflict).
328+
329+
Phase 1 uses the repo's `scripts/code-verify.py` (see [[ss-verify]]); this skill does
330+
**not** ship a second linter.

0 commit comments

Comments
 (0)