You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Copy file name to clipboardExpand all lines: .agents/skills/code-review/SKILL.md
+25-8Lines changed: 25 additions & 8 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -92,6 +92,16 @@ You are a senior engineer conducting a thorough code review. Review **only the l
92
92
-**AnimatePresence**: Is it used properly with unique keys for dialog/modal transitions?
93
93
-**Reduced Motion**: Is `useReducedMotion()` respected for accessibility?
94
94
95
+
### Async State, Defaults & Persistence
96
+
-**Async Source of Truth**: During async provider/model/session mutations, does UI/session/localStorage state update only after the backend accepts the change? If the UI updates optimistically, is there an explicit rollback path?
97
+
-**UI/Backend Drift**: Could the UI show provider/model/project/persona X while the backend is still on Y after a failed mutation, delayed prepare, or pending-to-real session handoff?
98
+
-**Requested vs Fallback Authority**: Do explicit user or caller selections stay authoritative over sticky defaults, saved preferences, aliases, or fallback resolution?
99
+
-**Dependent State Invalidation**: When a parent selection changes (provider/project/persona/workspace/etc.), are dependent values like `modelId`, `modelName`, defaults, or cached labels cleared or recomputed so stale state does not linger?
100
+
-**Persisted Preference Validation**: Are stored selections validated against current inventory/capabilities before reuse, and do stale values fail soft instead of breaking creation flows?
101
+
-**Compatibility of Fallbacks**: Are default or sticky selections guaranteed to remain compatible with the active concrete provider/backend, instead of leaking across providers?
102
+
-**Best-Effort Lookups**: Do inventory/config/default-resolution lookups degrade gracefully on transient failure, or can they incorrectly block a primary flow that should still work with a safe fallback?
103
+
-**Draft/Home/Handoff Paths**: If the product has draft, Home, pending, or pre-created sessions, did you review those handoff paths separately from the already-active session path?
104
+
95
105
### General Code Quality
96
106
-**Error Handling**: Are errors handled gracefully with user-friendly messages?
97
107
-**Loading States**: Are loading states shown during async operations?
@@ -104,13 +114,18 @@ You are a senior engineer conducting a thorough code review. Review **only the l
104
114
105
115
### Step 0: Run Quality Checks
106
116
107
-
Before reading any code, run the project's CI gate to establish a baseline:
117
+
Before reading any code, run the project's CI gate to establish a baseline. Use **check-only** commands so the baseline never mutates the working tree — otherwise auto-formatters can introduce unstaged diffs and you'll end up reviewing formatter output instead of the author's actual changes.
118
+
119
+
Avoid `just check-everything` as the baseline in this repo: that recipe runs `cargo fmt --all` in write mode and will modify the working tree. Run the non-mutating equivalents instead:
108
120
109
121
```bash
110
-
just ci
122
+
cargo fmt --all -- --check
123
+
cargo clippy --all-targets -- -D warnings
124
+
(cd ui/desktop && pnpm run lint:check)
125
+
./scripts/check-openapi-schema.sh
111
126
```
112
127
113
-
This runs: `pnpm check` (Biome lint/format + file sizes), `pnpm typecheck`, `just clippy` (Rust linting), `pnpm test`, `pnpm build`, and `just tauri-check` (Rust type checking).
128
+
If the project has a stronger pre-push or CI gate than this helper set, run that fuller gate when the review is meant to be PR-ready, but only after confirming it is also non-mutating (or run it from a clean stash). In this repo, targeted tests for the changed area plus the pre-push checks are often the practical follow-up.
114
129
115
130
Report the results as pass/fail. Any failures are automatically **P0** issues and should appear at the top of the findings list. Do not skip this step even if the user only wants a quick review.
116
131
@@ -120,7 +135,8 @@ For each file in the list:
120
135
121
136
1. Run `git diff main...HEAD -- <file>` to get the exact lines that changed
122
137
2. Review **only those changed lines** against the Review Checklist — do not flag issues in unchanged code
123
-
3. Note the file path and line numbers from the diff output for each issue found
138
+
3. For stateful UI or async flow changes, trace the full path end to end: user selection -> local/session state update -> persistence -> backend prepare/set/update call -> failure/rollback path
139
+
4. Note the file path and line numbers from the diff output for each issue found
124
140
125
141
### Step 2: Categorize Issues
126
142
@@ -152,16 +168,17 @@ After reviewing all files, provide:
152
168
153
169
### Step 3b: Self-Check
154
170
155
-
Before presenting findings to the user, silently review the issue list two more times:
171
+
Before presenting findings to the user, silently review the issue list three times:
156
172
157
173
1.**Pass 1**: For each issue, ask — is this genuinely a problem, or could it be intentional/acceptable? Remove false positives.
158
174
2.**Pass 2**: For each remaining issue, ask — does the recommended fix actually improve the code, or is it a matter of preference?
175
+
3.**Pass 3**: For async state/default-resolution issues, ask — can the UI, persisted state, and backend ever disagree after a failure, fallback, or session handoff?
159
176
160
-
After both passes, tag each surviving issue as one of:
177
+
After these passes, tag each surviving issue as one of:
161
178
-**[Must Fix]** — clear violation, will likely get flagged in PR review
162
179
-**[Your Call]** — valid concern but may be intentional or a reasonable tradeoff (e.g. stepping outside the design system for a specific reason). Present it but let the user decide.
163
180
164
-
Only present issues that survived both passes.
181
+
Only present issues that survived these passes.
165
182
166
183
### Step 4: Fix Issues
167
184
@@ -189,7 +206,7 @@ Once all issues are fixed, display:
189
206
190
207
**✅ Code review complete! All issues have been addressed.**
191
208
192
-
Your code is ready to commit and push. Lefthook will run the full CI gate (`just ci`) automatically when you push.
209
+
Your code is ready to commit and push. Lefthook and CI will run the repo's configured gates when you push.
193
210
194
211
Next steps: generate a PR summary that explains the intent of this change, what files were modified and why, and how to verify the changes work.
0 commit comments