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: .github/prompts/codex-pr-review.md
+38-21Lines changed: 38 additions & 21 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -23,16 +23,23 @@ Open CoDesign is an open-source AI design tool — Electron desktop app that tur
23
23
-`packages/templates/` — built-in demo prompts
24
24
-`packages/shared/` — types, utils, zod schemas
25
25
26
-
**Hard constraints and review checks:**
26
+
**Project constraints:**
27
27
- ≤ 30 prod dependencies
28
28
- MIT-compatible permissive licenses only (reject GPL/AGPL/SSPL/proprietary/unclear copied assets)
29
29
- All LLM calls via `@mariozechner/pi-ai` (no direct provider SDK imports in app code)
30
-
- No silent fallbacks — every error must surface in UI or throw with context
31
-
- Every UI value via `packages/ui` tokens (no hardcoded `#fff` / `16px` / fonts)
30
+
- No silent fallbacks for user-visible failure, data loss, auth/security decisions,
31
+
or persisted state. Best-effort cleanup, optional discovery, and non-critical
32
+
listing paths may degrade quietly when the UI still presents a truthful state.
33
+
- App chrome should use `packages/ui` tokens for shared colors, typography,
34
+
spacing, radii, and recurring component dimensions. Do not flag incidental
35
+
one-off Tailwind size utilities (for example icon sizes or compact row
36
+
heights) unless they duplicate an established token, hardcode colors/fonts,
37
+
cause visible inconsistency, or create a pattern that should be promoted to
38
+
a shared token.
32
39
33
-
Public context: `CLAUDE.md`, `AGENTS.md` if present, `.github/PULL_REQUEST_TEMPLATE.md`, package manifests, lockfiles, changed source files, and other files committed to the public repository.
40
+
Public context: `AGENTS.md` if present, `CLAUDE.md`, `.github/PULL_REQUEST_TEMPLATE.md`, package manifests, lockfiles, changed source files, and other files committed to the public repository. If `AGENTS.md` and `CLAUDE.md` conflict, prefer `AGENTS.md`.
34
41
35
-
Internal-only context: `docs/**`, `.claude/**`, and `.Codex/**` may exist in maintainer workspaces but are not guaranteed to exist in public clones. Do not cite those files, ask contributors to read them, or base a public finding solely on them. If an internal file conflicts with public repo files, use the public file as the review source and, at most, ask a maintainer-facing question.
42
+
Internal-only context: `docs/**`, `.claude/**`, and `.Codex/**` may exist in maintainer workspaces but are not guaranteed to exist in public clones. Do not cite those files, ask contributors to read them, or base a public finding solely on them. If a PR adds or modifies `.claude/**`, `.Codex/**`, `.env*`, or other local-agent/private config, that diff is public and should usually be flagged as unrelated/private configuration. If an internal file conflicts with public repo files, use the public file as the review source and, at most, ask a maintainer-facing question.
36
43
37
44
## PR Context (required)
38
45
@@ -84,10 +91,10 @@ fi
84
91
85
92
1.**Load context (progressive)**: PR metadata, diff, `CLAUDE.md`, `AGENTS.md` if present, `.github/PULL_REQUEST_TEMPLATE.md`, relevant package manifests/lockfiles, then only the source files referenced by the diff.
86
93
2.**Determine review mode**: `initial` if no prior bot review exists for an earlier commit, otherwise `follow-up after new commits`.
87
-
3.**Review the latest PR diff in full**: correctness, security (OWASP top 10), regressions, data loss, performance, maintainability, **and adherence to hard constraints**.
88
-
4.**Follow-up context**: when `IS_FOLLOW_UP_REVIEW=true`, use the previous bot review and compare diff for context — do not limit the review to those changes.
94
+
3.**Review the latest PR diff in full**: correctness, security (OWASP top 10), regressions, data loss, performance, maintainability, **and adherence to project constraints**.
95
+
4.**Follow-up context**: when `IS_FOLLOW_UP_REVIEW=true`, use the previous bot review and compare diff for context. Do not limit the review to those changes, but do not repeat an already-known optional/nit finding unless the new diff makes it worse or the previous review explicitly made it a merge blocker.
89
96
5.**Check tests**: note missing or inadequate Vitest/Playwright coverage.
90
-
6.**Constraint checks**: silent fallbacks, hardcoded UI values, direct SDK imports, license of new deps, install-size impact.
97
+
6.**Constraint checks**: material silent fallbacks, material hardcoded UI values, direct SDK imports, license of new deps, install-size impact.
91
98
7.**Freshness checks**: for dependency, runtime, or API-version claims, verify against the repository files first. If the repository files are insufficient and network is available, use public authoritative sources such as npm package metadata, GitHub releases, or official docs. Never report a version-related issue from model memory alone.
92
99
8.**Linked issue validation**: when the PR title/body says it closes, fixes, resolves, implements, or completes an issue, fetch that issue body and recent public comments. Compare the issue's acceptance criteria, claimed scope, and follow-up comments against the actual diff and tests.
93
100
9.**Respond** with an evidence-based review comment (no code changes).
@@ -119,14 +126,27 @@ Example finding:
119
126
```
120
127
````
121
128
122
-
## Severity Calibration
129
+
## Severity And Noise Control
123
130
124
-
Treat severity as a merge decision aid, not a list of every possible improvement:
131
+
Use severity to reflect merge risk, not personal taste:
125
132
126
-
-**Blocker**: security issue, data loss, build/release breakage, direct violation of a live required check, or an issue-closing claim that is clearly false and would mislead maintainers into closing user-visible work.
127
-
-**Major**: realistic user-facing regression, broken core workflow, wrong runtime behavior on a supported path, or missing implementation for an explicit completion claim.
128
-
-**Minor**: localized bug, important but non-blocking test gap, misleading diagnostic, or follow-up needed for a secondary path.
129
-
-**Nit**: wording, small maintainability, or style issues with no behavioral risk.
133
+
-**Blocker**: exploitable security issue, likely data loss, broken required CI/build/release path, incompatible license, or a regression that makes the PR unsafe to merge.
134
+
-**Major**: likely user-visible behavior regression, broken core workflow, incorrect persistence/runtime contract, or a broad architectural constraint violation.
135
+
-**Minor**: real issue that should be fixed before or soon after merge, but the PR can still be evaluated as directionally sound. Missing changesets for user-visible changes usually belong here.
136
+
-**Nit**: style, naming, small cleanup, or consistency polish. Nits must never be described as merge blockers.
137
+
138
+
Do not file a finding when the only concern is:
139
+
140
+
- an incidental one-off Tailwind size utility that is consistent with nearby code,
141
+
- an optional test that would be nice but is not proportional to the risk,
142
+
- a cosmetic ordering/formatting preference that Biome accepts,
143
+
- an existing codebase pattern that the PR merely follows.
144
+
145
+
For follow-up reviews, avoid review churn:
146
+
147
+
- If an earlier optional/nit finding remains but all material issues are fixed, move it to **Residual observations** in the summary or omit it.
148
+
- If a previous finding is resolved, mention it briefly in the summary only when it helps the maintainer understand readiness.
149
+
- If the PR is ready to merge except for non-blocking polish, say that clearly.
130
150
131
151
When a concern only appears under a self-contradictory configuration, a deliberately unsupported path, or a scope that belongs to a follow-up issue, do not label it Major. Put it in Summary as residual risk or suggest a follow-up issue instead. Do not report commit-trailer compliance findings unless a public workflow, branch protection rule, or live required check in this repository currently enforces them.
132
152
@@ -136,7 +156,7 @@ When a concern only appears under a self-contradictory configuration, a delibera
136
156
-**Biome autofixes**: when the only problem is Biome formatting or a safe autofix, do not frame it as a behavioral finding; tell the contributor to run `pnpm lint:fix`, commit the result, and let CI rerun.
137
157
-**Mode line**: summary must start with `Review mode: initial` or `Review mode: follow-up after new commits`.
138
158
-**Evidence**: cite specific public repository files and line numbers using `path:line`.
139
-
-**No private citations**: never cite `docs/**`, `.claude/**`, `.Codex/**`, local absolute paths, workflow runner temp paths, or any file absent from the public checkout.
159
+
-**No private citations**: never cite maintainer-local `docs/**`, untracked `.claude/**`, untracked `.Codex/**`, local absolute paths, workflow runner temp paths, or any file absent from the public checkout. If the PR itself adds/modifies private config files, cite the public diff path and explain why it should be removed.
140
160
-**No speculation**: if uncertain, say so; if not found, say "Not found in the public repo".
141
161
-**No stale version claims**: when judging "latest", "unsupported", "deprecated", or "current stable", include the checked source in the reasoning. If you cannot verify it during the run, do not file a finding.
142
162
-**Linked issue claims**: if a PR claims to close/implement an issue, verify the issue acceptance criteria against the diff and tests before accepting the claim.
@@ -147,18 +167,15 @@ When a concern only appears under a self-contradictory configuration, a delibera
147
167
-**Fresh-head only**: before posting, re-fetch live PR head SHA; if it differs from `CURRENT_HEAD_SHA`, stop without posting a stale review.
148
168
-**Attribution**: report only issues introduced or directly triggered by the diff.
149
169
-**High signal**: if confidence < 80%, do not report; ask a question if needed.
150
-
-**No praise**: report issues and risks only.
151
-
-**Concrete fixes**: every issue must include a specific code suggestion snippet.
170
+
-**No filler praise**: keep positive comments brief and useful. It is okay to say "No issues found" or note that prior findings were resolved.
171
+
-**Concrete fixes**: every issue must include a specific next action. Include a code snippet only when a code snippet is the clearest fix; for changesets, docs, test coverage, or scope issues, a concrete command or prose action is fine.
152
172
153
173
## Response Format
154
174
155
175
**Findings**
156
176
157
177
-[Severity] Title — why it matters, evidence `path:line`
158
-
Suggested fix:
159
-
```language
160
-
// minimal change snippet
161
-
```
178
+
Suggested fix / next action: one concise prose action, command, or code snippet.
0 commit comments