test(nextly): the search-path probe sets up through the connection it reads - #1727
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedNext included review available in 59 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
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 |
|
@codex review |
![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: fb2731bf7f
ℹ️ 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".
| // 🔴 Through Drizzle, not `client.query`. Database access in this repository | ||
| // is Drizzle-only, and the exemption a test might claim — that a fixture is | ||
| // not product code — does not apply to the thing under test here: these |
There was a problem hiding this comment.
This comment explains compliance with repository policy and rebuts a hypothetical test exemption instead of only explaining why the helper uses the client-bound Drizzle instance. Keep the same-session rationale, but remove the policy/review discussion so the comment follows the repository's enforced comment convention.
AGENTS.md reference: AGENTS.md:L277-L279
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Right, and it is a rule I had read an hour earlier while checking the one about
DDL helpers — AGENTS.md, same section: comments describe the code, never
conversations. I argued with a review inside the source file.
Both comments are rewritten to say only why the code is what it is:
- the setup helper now explains that these statements establish the session state
the assertions depend on, so theSET search_pathhas to be sent the way the
reads are sent; - the docblock says the fixture tables are synthetic and written out rather than
derived because deriving them would tie the file to another table's shape,
with no mention of any rule or exemption.
The reasoning about which rule applies belongs in the PR conversation, which is
where I have put it.
@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
@nextlyhq/eslint-plugin
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: |
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! 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". |
1 participant
Follows #1721, from a Codex P1 that landed as it merged. Test-only, so no changeset.
What was wrong
The search-path probe set its fixture up with
pg.Client.querydirectly. Thisrepository's rule is that database access is Drizzle-only, and the exemption a
test might claim — that a fixture is not product code — does not hold here: those
statements establish the session state the assertions depend on, so they have
to travel the same path the reads do.
Setup and teardown now go through
db.execute(sql.raw(...))on the sameclient-bound instance, so the
SET search_pathstill lands on the connectionthat reads. The
Clientis kept and still closed inafterAll— it is what pinsthe session, so nothing else can release it.
Where I did not follow the advice, and why
The review also asked me to "derive table DDL through the production helpers
rather than introducing a second DDL implementation". I have not, and I want to
be explicit rather than quietly partial.
These fixture tables are synthetic: a decoy and a subject that exist only to
be resolved by the search path, with no production counterpart. The hazard that
rule guards — a fixture copying a real table's definition and then drifting from
it — has nothing to drift from here. Deriving them from a real table would couple
this file to that table's shape and change what it measures.
Measured rather than asserted: this repository's own PostgreSQL integration tests
create synthetic probe tables inline for the same reason.
getSchemaEventsDdl, the helper the rule names, builds one specific productiontable. There is no helper for a two-column decoy, and writing one would be the
second DDL implementation the advice warns against.
The reasoning is now in the file's docblock, so the next reader sees why these
are hand-written and does not have to re-litigate it.
Gates
nextly
check-types0 ·lint0 · the probe is still collected byvitest.integration.config.ts(5 tests, skipped locally for the documentedmissing-URL reason).
from this machine, and #1721 merged while its Postgres leg was queued. That leg
is running on
mainnow and is the first real verdict on these assertions.