Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ prompts/
codex-first-pass.md # Stage 1 prompt (template)
claude-synthesize.md # Stage 2 control arm (template)
claude-synthesize-thesis-first.md # Stage 2 thesis-first arm (template)
_shared/{completeness,privacy,migration-data,perf,parsing,navigation}-rules.md # shared rule blocks
_shared/{completeness,privacy,migration-data,perf,parsing,navigation,rename-compatibility,nullability-boundary}-rules.md # shared rule blocks
```

- **Templates + shared blocks.** Each prompt references the shared rule blocks via
Expand Down
31 changes: 31 additions & 0 deletions prompts/_shared/nullability-boundary-rules.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
When a value crosses the boundary from a DB column into a serializer, into a TypeScript
API type, and into a component, verify its actual nullability at the source rather than
trusting a type further down the chain:
- Check `db/schema.rb` for the column backing any field a serializer passes through
(`status: parent.status`, `foo: record.foo`, etc.). A column without `null: false` can
hold NULL in production even if every factory in the test suite sets a value, and the
serializer must handle that case (a default, an explicit null branch, or a documented
reason the column can never actually be null despite the schema).
- Treat an unchecked TypeScript `as` assertion narrowing a nullable value to a non-null
type (e.g. `status: raw.status as string` where the API can return `null`) as a finding.
The assertion must be justified against the actual schema/serializer, not just against
the shape the author expects the API to return. Flag it and ask for either a real type
(`string | null`) with the component handling `null`, or a comment citing why the source
column is guaranteed non-null.
- This applies most sharply to a new frontend page/component reading a field for the first
time — a nullable column that has been silently null in production for years only
becomes a crash once something finally calls a method on it (`.charAt`, `.toUpperCase`,
etc.) without a null check.

Separately, weigh blast radius when a diff adds a call that can throw inside a React
component subtree sitting under an error boundary (React Router's route error boundary,
or any explicit `ErrorBoundary`): an uncaught throw there takes down everything the
boundary covers, not just the one card or row that failed. Flag a new, unguarded method
call on a value that can be null/undefined (per the point above) more strongly when it
sits in a shared list/page component rather than in an isolated leaf that already has its
own boundary or fallback.

Don't flag routine null-safe code (optional chaining, a default, a type that already
includes `null`/`undefined` and is handled), and don't demand `string | null` for a value
backed by a `null: false` column — the point is to check the real nullability, not to add
a null case to everything reflexively.
7 changes: 5 additions & 2 deletions prompts/claude-synthesize-thesis-first.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,14 @@ Synthesize a single review decision for pull request #{{PR}}.
10. Check whether the diff renames, moves, or deletes a name that is persisted outside
the codebase and read back after deploy, per these rules:
{{@prompts/_shared/rename-compatibility-rules.md}}
11. Validate which of Codex's findings are real (discard false positives), add any genuine
11. Check how the diff carries a value across a DB column / serializer / TypeScript API
type / component boundary, per these rules:
{{@prompts/_shared/nullability-boundary-rules.md}}
12. Validate which of Codex's findings are real (discard false positives), add any genuine
issues Codex missed, and (when a ticket is available) judge genuine misses of the ticket's
intent or clear scope creep — but give credit when the author went beyond the literal
acceptance criteria in a sound way rather than flagging it as non-compliant.
12. Decide ONE verdict. Be pragmatic: use "request_changes" only when there is at least one
13. Decide ONE verdict. Be pragmatic: use "request_changes" only when there is at least one
genuine, blocking issue (such as a bug, regression, privacy violation, half-finished task,
placeholder, or deferred work); otherwise "approve". A change that exceeds the AC without
breaking the ticket's intent is a reason to approve, not to block.
Expand Down
7 changes: 5 additions & 2 deletions prompts/claude-synthesize.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,14 @@ Synthesize a single review decision for pull request #{{PR}}.
10. Check whether the diff renames, moves, or deletes a name that is persisted outside
the codebase and read back after deploy, per these rules:
{{@prompts/_shared/rename-compatibility-rules.md}}
11. Validate which of Codex's findings are real (discard false positives), add any genuine
11. Check how the diff carries a value across a DB column / serializer / TypeScript API
type / component boundary, per these rules:
{{@prompts/_shared/nullability-boundary-rules.md}}
12. Validate which of Codex's findings are real (discard false positives), add any genuine
issues Codex missed, and (when a ticket is available) judge genuine misses of the ticket's
intent or clear scope creep — but give credit when the author went beyond the literal
acceptance criteria in a sound way rather than flagging it as non-compliant.
12. Decide ONE verdict. Be pragmatic: use "request_changes" only when there is at least one
13. Decide ONE verdict. Be pragmatic: use "request_changes" only when there is at least one
genuine, blocking issue (such as a bug, regression, privacy violation, half-finished task,
placeholder, or deferred work); otherwise "approve". A change that exceeds the AC without
breaking the ticket's intent is a reason to approve, not to block.
Expand Down
5 changes: 4 additions & 1 deletion prompts/codex-first-pass.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ verbatim and handed to a second reviewer, so do not add conversational preamble.
9. Check whether the diff renames, moves, or deletes a name that is persisted outside
the codebase and read back after deploy, per these rules:
{{@prompts/_shared/rename-compatibility-rules.md}}
10. Report concrete issues — bugs, regressions, security problems, member-privacy
10. Check how the diff carries a value across a DB column / serializer / TypeScript API
type / component boundary, per these rules:
{{@prompts/_shared/nullability-boundary-rules.md}}
11. Report concrete issues — bugs, regressions, security problems, member-privacy
violations, incomplete tasks/half-measures/placeholders/deferred work, and genuine misses
of the ticket's intent or clear scope creep — each with a file/line reference and a brief
rationale. Do not list "doesn't match acceptance criteria" as an issue by itself; only
Expand Down
Loading