docs(root): promote seven confirmed review patterns into the agent guide - #655
Conversation
These are not new advice. Each has at least one dated instance in merged code, and several have three or four across packages that share no source. AGENTS.md takes the five that fit as short enforced rules -- three on testing, where the recurring failure is a green that carries no information, and two on deriving rather than duplicating. The two that need a paragraph go to .claude/rules: how a derived check diverges from what it checks, and why a squash merge makes every ancestry check unsound. Nothing here is mechanically enforced, so no open PR starts failing. What changes is what a reviewer applies.
|
@codex please review this PR |
|
Warning Review limit reached
Next review available in: 4 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe6a88368d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
@nextlyhq/adapter-drizzle
@nextlyhq/adapter-mysql
@nextlyhq/adapter-postgres
@nextlyhq/adapter-sqlite
@nextlyhq/admin
@nextlyhq/admin-css
@nextlyhq/blocks-engine
@nextlyhq/blocks-react
@nextlyhq/builder
create-nextly-app
nextly
@nextlyhq/plugin-form-builder
@nextlyhq/plugin-page-builder
@nextlyhq/plugin-sdk
@nextlyhq/plugin-seo
@nextlyhq/storage-s3
@nextlyhq/storage-uploadthing
@nextlyhq/storage-vercel-blob
@nextlyhq/ui
commit: |
…y hold The tail-marker check assumed the final commit ADDS something. When it only removes content, grepping for added text finds nothing whether or not the commit landed, and a mode or binary change is invisible to text search at all. Each case now has its own direction. The LIMIT 1 rule was stated as a property of the clause, which made it read as self-contradicting: hunting a witness is sound when the claim is existential. It is now stated as a property of the claim -- one row settles it, or it does not. The lint warning claim named the root script; the hook runs turbo over packages/* only, and admin-css lints without --max-warnings 0, so a warning there exits clean. And an unreachable guard is only free when it does no work. One that queries or recomputes pays on every call whether or not it can reject. Also widens the derived-check paths to the release inputs the rule uses as its own example, and adds the stronger form: a boundary the system cannot cross beats a scan looking for crossings.
|
@codex please review this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9121dac5ae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The merge-verification marker needed a uniqueness and path scope to prove anything, tree comparison replaces a diffstat that only reports a path was touched, and the module-resolution remedy now separates a missing build from a stale install. The derived-checks rule loads for package JavaScript, where admin-css keeps its product code and tests, and no longer calls every conservative catch sound regardless of what it swallows. Compile-time contract tests are exempted from the runtime break rule, and deliberate removal of an ineffective test from the count rule.
|
@codex please review this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f492d5882
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A compile-time contract test is verified by the diagnostic it predicted. A typo or an unrelated type error in the same file stops compilation too, and `@ts-expect-error` suppresses whatever error follows it, so such a test stays green once the code fails for a different reason and after the rejection it asserts stops happening.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3dfc66a56f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A measurement that confirms a true fact about an adjacent property closes the question while leaving the defect in place, and it does so carrying the authority of having been run.
Invoking a known defect for one purpose while an adjacent claim assumes its absence leaves both on the page with neither wrong on its own. The check is mechanical: re-read every statement about the population the defect affects.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79b3ea15ad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…for it A name is a claim made by something outside your control, so the cases where it diverges from expectation are the cases the check exists for. Five instances in one area: SQLSTATE class over a code list, column set over index name, capability probe over version string, the shared indexability rule over a restatement, and object comparison over a grep marker.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c47a92f245
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Measurement and assertion were the same failure: a necessary-but-insufficient property goes green from both the correct implementation and the broken one, carrying the authority of having been checked. One rule with two worked examples rather than two that each look narrow.
"Ask what else would make it pass" is the operational form of naming the separating property, not a second rule, and the count-drop caveat is a rider on test-is-evidence rather than a peer of it. Propagating a cited defect through a document is situational, so it moves to the scoped rules where its worked example has room. AGENTS.md is loaded on every task and attention there is rivalrous.
|
@codex please review this pr |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
A probe, a derived check, a test, a post-apply verifier and the suite baseline diff all carried the same defect in one week, and each existed to catch the layer above it. Confirming an instrument on a case where nothing moved cannot distinguish it from one that never reports anything.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 530e32aff4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The compartmentalisation failure is easier to see with no document involved: a disposable copy used deliberately for one control, then an in-place mutation with a hand-rolled backup twenty minutes later, with the boundary existing only as a category rather than in the work.
|
@codex please review this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5daa24f3df
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Merge verification compares the PR's delta rather than whole objects, since a base change to the same file makes blobs differ legitimately; probes the merge commit rather than origin/main; matches markers as fixed strings; and treats a rewritten branch as a case the tail heuristic does not cover. A cited resolution failure is no longer assumed environmental without checking what main changed, and stale dist is separated from missing dist. Structural identification is qualified: SQLSTATE class is too coarse when the caller must tell integrity failures apart, and a column set does not identify a database object when several cover the same columns. A comment naming an expected TypeScript diagnostic does not make tsc validate it. An ineffective test is repaired before it is deleted when it is the only coverage. A guard that is a precondition never moves behind the work it protects. A manifest assertion is only a boundary where the resolver agrees with it. Scoped rules now load by extension across the repo rather than by directory.
|
@codex please review this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac6216a059
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
gh reports the merge commit but does not fetch it, so probing the SHA exits unable-to-resolve rather than verifying. After a rebase the computed merge base IS origin/main, so the manifest diff is empty regardless of what the rebase brought in, and empty reads as nothing-relevant-changed. Config extensions now load the derived-checks rule, where pnpm-workspace.yaml and its mirrored ALL_PACKAGES list are one of its own examples.
|
@codex please review this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 612bd0c8c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…tigation
HEAD@{1} after a multi-step rebase is the last pick, not the pre-rebase tip, so
the merge base came out as origin/main again and the empty diff returned. The
path filter omitted tsup configs and the lockfile, which are the two inputs most
likely to explain a stale-output or resolution failure. And a positive control
on the accepted form does not distinguish an unrelated error confined to the
rejected line, so only an assertion the checker evaluates counts.
|
@codex please review this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1665ceec19
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…scope NOT P in SQL is not the complement of P: for a NULL input both are UNKNOWN and WHERE keeps only TRUE, so a counterexample hunt over nullable data certifies the universal claim it was meant to refute. A marker beginning with - is parsed as an option without -e. A squash commit does not have the PR head as an ancestor, so both refs need fetching before step 3 dereferences headRefOid. Shell verifiers are derived checks in every sense but the language.
|
@codex please review this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31d3ca6b05
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ed artefacts git grep exits 1 for no-match and for a pathspec matching no files alike, so a mistyped path certifies a removal without reading the file. Require a hit against the preimage first. Scope now covers css and sql, where the repo has explicit source/derived pairs.
|
@codex please review this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53a7a6d9af
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… no successor If text was added and removed within one PR, the pre-merge state never had it, so its absence afterwards is guaranteed whether or not the removal landed. Neither preimage separates that from a merged removal; the delta does. And a test deleted because the behaviour is gone has no remaining file to point at, so requiring one forces a false coverage comment.
|
@codex please review this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d9948ac35
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…iscoverable mergeBase..mergeCommit sweeps in every commit main gained after the branch point; the squash commit's parent IS main at merge time, so that range is the squash patch. Snapshots and env examples are tracked derived artefacts with no source extension of their own. And .claude/rules is a Claude-only discovery location, so the review prompt now enumerates it — a reviewer running elsewhere was never loading the rules it is asked to enforce.
|
@codex please review this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04c2f06083
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…hing The documented start command only tests the connection and exits 1 on failure, and docker:up brings up the dev stack rather than the test containers, failing on a name conflict where one already exists. Names the containers directly.
…y differ docker start handles the usual case of existing-but-stopped containers; compose up is needed only on a fresh clone. The services set a fixed container_name, so one compose project owns them and it is whichever directory first brought them up — rarely the worktree you are in, which is why compose up conflicts.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87b3179bd8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… start ORIG_HEAD is rewritten by any later command that sets it, git reset included, so it points at the rebased tip and the diff comes out empty again. The branch reflog moves once per rebase. Also records that the repo's index key sorts columns, so the signature does not separate (a,b) from (b,a) today.
|
@codex please review this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8cd64100d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| `git diff <mergeBase>..<headRefOid> -- <path>` against | ||
| `git diff <mergeCommit>^..<mergeCommit> -- <path>`, expecting the PR's hunks |
There was a problem hiding this comment.
When the base independently gains one of the PR's changes after the branch point, the first range contains that hunk but the second cannot: <mergeCommit>^ already contains it, so the squash patch correctly omits it. I reproduced this with a PR changing both two→TWO and five→FIVE while the base independently changed two→TWO; the clean squash delta contained only five→FIVE, yet this procedure reports the merge as incomplete. The git diff <commit> <commit> form compares the two endpoint trees, so accept PR hunks already present in the squash parent/final tree instead of requiring every PR hunk in the squash patch.
AGENTS.md reference: AGENTS.md:L115-L132
Useful? React with 👍 / 👎.
| # their structure mirrors `UserExtSchemaService.generateMigrationSQL()`. | ||
| - "**/*.css" | ||
| - "**/*.sql" | ||
| - ".changeset/**" |
There was a problem hiding this comment.
When a change edits only packages/nextly/src/dispatcher/handlers/__tests__/eslint-rule-fires.fixture.ts.disabled, none of these globs loads the rule, even though that tracked file is explicitly a regression instrument that is renamed to .ts to verify an ESLint failure. The detailed test/instrument guidance is therefore absent for a concrete artifact it governs, and extending the suffix allowlist again only moves the same hole to the next file type; use an unconditional path boundary such as **/* instead of enumerating extensions.
AGENTS.md reference: AGENTS.md:L205-L214
Useful? React with 👍 / 👎.
| `docker start nextly-postgres17-test nextly-mysql-test` (add | ||
| `nextly-postgres15-test` for the 15 leg). | ||
| - Never created, on a fresh clone: | ||
| `docker compose -f docker-compose.test.yml up -d postgres17-test postgres15-test mysql-test`. |
There was a problem hiding this comment.
When these containers are newly created or restarted with their tmpfs data gone, both startup commands return before the database health check succeeds; this is especially visible for mysql-test, whose health check allows a 30-second startup period. Docker Compose provides --wait specifically to wait for services to become running or healthy, while the integration suites begin issuing connections and setup queries as soon as they start, so following this setup can produce connection failures that look like test regressions. Add --wait to the fresh-clone command and an explicit health poll after docker start.
Useful? React with 👍 / 👎.
1 participant
Implements
tasks/left-tasks/181-promote-eight-confirmed-review-patterns-into-agents-md.md, approved by the founder.Why move them
tasks/review-lessons.mdis past 1,350 lines. It is a ledger nobody reads before writing code.AGENTS.mdand.claude/rules/are read by every session and every review bot, on every task — so a pattern only changes behaviour once it lives there.These are not new advice. Each has at least one dated instance in merged code, and several have three or four across packages that share no source.
What goes where
AGENTS.mdtakes the five that fit as short enforced rules.Three on testing, grouped with the existing baseline rule because they share its subject — the recurring failure in this repo is a green that carries no information:
Two in Conventions:
Plus the rule that this PR is itself an instance of: a documented rule with nothing enforcing it is not a control, and filing a task is not installing one.
.claude/rules/takes the two that need a paragraph:derived-checks.md— how a derived check diverges from what it checks. Three axes, not one: same computation, same DOMAIN, same failure semantics. Includes the distinction that cost the most to learn — an existential search may short-circuit, a universal claim may not, andLIMIT 1looks identical in both — and why a barecatchis only a defect when its fallback makes a claim.verifying-merged-work.md— squash merges make every ancestry check unsound, and the lost commits are always at the tail, so the marker must come from the FINAL commit. Also: before calling a red run flake, name the mechanism; and the environment states that wear the costume of code defects.Blast radius: checked, not assumed
The task flagged this as guard-lands-last shaped, so I checked before writing.
Nothing here is mechanically enforced, so no open PR starts failing. What changes is what a reviewer applies going forward.
I also verified the one rule that COULD have bitten — every open PR's changeset against the 22-package fixed group:
None are short. #646 is a useful case for whoever builds the CI check in task 176: derive the expected set from the PR's own
config.json, or a PR that adds a package fails for being right.Scope
Docs only —
AGENTS.mdand two new rule files. No changeset, per the convention for docs-only PRs.pnpm lintclean.Deliberately not included
The eighth item from the task, whether a fallback is conservative is a joint property of the value and the caller's use, is folded into
derived-checks.mdrather than given its own bullet — it is a refinement of the bare-catchrule, not a separate instruction, andAGENTS.mdearns its authority by staying short.