feat(stack-drizzle)!: remove EQL v2 root and collapse ./v3 to root#770
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
🦋 Changeset detectedLatest commit: f5548a1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
81ed000 to
a1bfe1a
Compare
Delete the EQL v2 authoring surface (`src/index.ts` `encryptedType` + config extraction, `src/operators.ts`, `src/schema-extraction.ts`) and promote the EQL v3 implementation from `./v3` to the package root as a hard break with no alias. - Drop the `./v3` subpath from the `exports` map and remove `typesVersions`; the v3 impl now lives at `src/*` and is the sole `.` export. - De-suffix the public API: `createEncryptionOperatorsV3` -> `createEncryptionOperators`, `extractEncryptionSchemaV3` -> `extractEncryptionSchema`. No `*V3` aliases (they would type-check v2 call sites against v3 semantics). - Delete the two v2 operator test files; move the v3 test suite to `__tests__/*` and repoint imports. ESM+CJS both preserved. - Fix the `@cipherstash/bench` importer: rewrite the Drizzle table to v3 `types.*` domains + `EncryptionV3`, switch operator usages off the removed v2-only ops (`like`/`ilike`, jsonb-path) to `matches`/`contains`/`selector`, and update the bench fixture schema.sql to the eql_v3 domains/index terms. - Update the README and the bundled `stash-drizzle` skill to the collapsed root imports; remove v2 authoring guidance. Existing v2 ciphertext still decrypts via `@cipherstash/stack`; only the Drizzle-side v2 authoring/query-building is removed (Decision 6). BREAKING CHANGE: `@cipherstash/stack-drizzle` no longer exports an EQL v2 surface and the `./v3` subpath is removed. Import the v3 API from the package root with the de-suffixed names.
a1bfe1a to
36b099b
Compare
…ollapse PR 3 made EncryptionV3 an overloaded alias; ReturnType<typeof EncryptionV3> now resolves to the nominal overload (EncryptionClient) not the typed client. Infer through a single-signature helper so the bench handle keeps the typed client type.
2a1dca6 to
30e7b55
Compare
…the image The bench fixture schema moved to concrete EQL v3 domains (`public.eql_v3_text_search`, `eql_v3.eq_term(...)`), but tests-bench.yml started `postgres-eql:17-2.3.1` — which ships EQL v2 — and ran the suite immediately. `applySchema` failed with `type "public.eql_v3_text_search" does not exist`. Nothing tied the image tag to the EQL version the fixtures need, so the coupling broke silently. Install v3 the way every integration suite already does: a vitest `globalSetup` calling test-kit's `installEqlV3`, which shells out to the real `stash eql install --eql-version 3`. That needs only a database URL — no CipherStash credentials — so `db-only.test.ts` stays the credential-free smoke test it is meant to be, and CI and `pnpm test:local` become the same path rather than CI depending on a step the local flow never ran. The globalSetup imports `@cipherstash/test-kit/install` (a new narrow subpath export) rather than the barrel: the barrel reaches `needle-for.ts`, which consumes stack source through the `@/` alias, and bench has no `stackSourceAlias` in its vitest config. `install.ts` depends on node builtins only. Workflow changes: - Use the `integration-setup` composite, which already builds the `stash` CLI the install needs (pinned to this job's existing Node 22). - Start only the `postgres` service; docker-compose.yml also defines a PostgREST that bench never talks to. - Widen the trigger paths to follow the package graph. bench depends on `@cipherstash/stack` and `@cipherstash/stack-drizzle`, and now on the CLI's EQL installer, but the filter covered only `packages/bench/**` and `local/**` — a break upstream could never trigger this job. No changeset: `@cipherstash/bench` and `@cipherstash/test-kit` are both private, and workflow files are repo tooling.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/stack-drizzle/src/column.ts (1)
148-149: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid the type-erasing carrier cast.
Line 149 bypasses type checking with
as unknown as. Preserve the concrete builder type while adding the carrier shape instead.Proposed fix
- writeBuilder(column as unknown as EqlV3ColumnCarrier, builder) + writeBuilder(column as typeof column & EqlV3ColumnCarrier, builder)As per coding guidelines, avoid
as unknownin source; use a specific assertion or a reasoned Biome ignore.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/stack-drizzle/src/column.ts` around lines 148 - 149, Replace the type-erasing cast in the second writeBuilder call with a specific assertion that preserves the concrete builder type while satisfying the EqlV3ColumnCarrier shape. Update the surrounding builder typing or overloads as needed so writeBuilder accepts the carrier without using as unknown; only use a narrowly scoped Biome ignore if no type-safe assertion is possible.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/tests-bench.yml:
- Line 15: Update both path-filter lists in the bench workflow to include
packages/stack/**, ensuring changes such as packages/stack/src/encryption/v3.ts
and related Stack exports trigger bench smoke tests. Preserve the existing
filters while adding coverage for all direct Stack source changes.
In `@packages/stack-drizzle/src/codec.ts`:
- Around line 38-44: Update the envelope validation around hasSteVec and missing
to require a ciphertext field on the SteVec root, specifically ensuring
envelope.sv[0].c is defined when a non-empty sv array is present. Reject values
such as { v: 3, sv: [{}] } with EqlV3CodecError before decryption, while
preserving acceptance of valid ciphertext envelopes.
---
Nitpick comments:
In `@packages/stack-drizzle/src/column.ts`:
- Around line 148-149: Replace the type-erasing cast in the second writeBuilder
call with a specific assertion that preserves the concrete builder type while
satisfying the EqlV3ColumnCarrier shape. Update the surrounding builder typing
or overloads as needed so writeBuilder accepts the carrier without using as
unknown; only use a narrowly scoped Biome ignore if no type-safe assertion is
possible.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9e066948-7c0e-4222-95a3-462ae4467e5f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (43)
.changeset/remove-eql-v2-drizzle-root.md.github/workflows/tests-bench.ymlpackages/bench/__benches__/drizzle/operators.bench.tspackages/bench/__tests__/drizzle/operators.explain.test.tspackages/bench/package.jsonpackages/bench/sql/schema.sqlpackages/bench/src/drizzle/setup.tspackages/bench/src/harness/global-setup.tspackages/bench/src/harness/seed.tspackages/bench/vitest.config.tspackages/stack-drizzle/README.mdpackages/stack-drizzle/__tests__/bigint.test.tspackages/stack-drizzle/__tests__/codec.test.tspackages/stack-drizzle/__tests__/column.test.tspackages/stack-drizzle/__tests__/drizzle-operators-bigint.test.tspackages/stack-drizzle/__tests__/drizzle-operators-jsonb.test.tspackages/stack-drizzle/__tests__/exports.test.tspackages/stack-drizzle/__tests__/operators.test-d.tspackages/stack-drizzle/__tests__/operators.test.tspackages/stack-drizzle/__tests__/schema-extraction.test.tspackages/stack-drizzle/__tests__/selector.test.tspackages/stack-drizzle/__tests__/sql-dialect.test.tspackages/stack-drizzle/__tests__/types.test-d.tspackages/stack-drizzle/__tests__/types.test.tspackages/stack-drizzle/integration/adapter.tspackages/stack-drizzle/integration/json-adapter.tspackages/stack-drizzle/integration/lock-context.integration.test.tspackages/stack-drizzle/integration/null-persistence.integration.test.tspackages/stack-drizzle/integration/relational.integration.test.tspackages/stack-drizzle/package.jsonpackages/stack-drizzle/src/codec.tspackages/stack-drizzle/src/column.tspackages/stack-drizzle/src/index.tspackages/stack-drizzle/src/operators.tspackages/stack-drizzle/src/schema-extraction.tspackages/stack-drizzle/src/sql-dialect.tspackages/stack-drizzle/src/types.tspackages/stack-drizzle/src/v3/index.tspackages/stack-drizzle/src/v3/operators.tspackages/stack-drizzle/src/v3/schema-extraction.tspackages/stack-drizzle/tsup.config.tspackages/test-kit/package.jsonskills/stash-drizzle/SKILL.md
💤 Files with no reviewable changes (6)
- packages/stack-drizzle/tests/drizzle-operators-bigint.test.ts
- packages/stack-drizzle/tests/drizzle-operators-jsonb.test.ts
- packages/stack-drizzle/src/v3/index.ts
- packages/stack-drizzle/src/v3/schema-extraction.ts
- packages/stack-drizzle/package.json
- packages/stack-drizzle/src/v3/operators.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/stack-drizzle/src/column.ts (1)
148-149: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid the type-erasing carrier cast.
Line 149 bypasses type checking with
as unknown as. Preserve the concrete builder type while adding the carrier shape instead.Proposed fix
- writeBuilder(column as unknown as EqlV3ColumnCarrier, builder) + writeBuilder(column as typeof column & EqlV3ColumnCarrier, builder)As per coding guidelines, avoid
as unknownin source; use a specific assertion or a reasoned Biome ignore.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/stack-drizzle/src/column.ts` around lines 148 - 149, Replace the type-erasing cast in the second writeBuilder call with a specific assertion that preserves the concrete builder type while satisfying the EqlV3ColumnCarrier shape. Update the surrounding builder typing or overloads as needed so writeBuilder accepts the carrier without using as unknown; only use a narrowly scoped Biome ignore if no type-safe assertion is possible.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/tests-bench.yml:
- Line 15: Update both path-filter lists in the bench workflow to include
packages/stack/**, ensuring changes such as packages/stack/src/encryption/v3.ts
and related Stack exports trigger bench smoke tests. Preserve the existing
filters while adding coverage for all direct Stack source changes.
In `@packages/stack-drizzle/src/codec.ts`:
- Around line 38-44: Update the envelope validation around hasSteVec and missing
to require a ciphertext field on the SteVec root, specifically ensuring
envelope.sv[0].c is defined when a non-empty sv array is present. Reject values
such as { v: 3, sv: [{}] } with EqlV3CodecError before decryption, while
preserving acceptance of valid ciphertext envelopes.
---
Nitpick comments:
In `@packages/stack-drizzle/src/column.ts`:
- Around line 148-149: Replace the type-erasing cast in the second writeBuilder
call with a specific assertion that preserves the concrete builder type while
satisfying the EqlV3ColumnCarrier shape. Update the surrounding builder typing
or overloads as needed so writeBuilder accepts the carrier without using as
unknown; only use a narrowly scoped Biome ignore if no type-safe assertion is
possible.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9e066948-7c0e-4222-95a3-462ae4467e5f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (43)
.changeset/remove-eql-v2-drizzle-root.md.github/workflows/tests-bench.ymlpackages/bench/__benches__/drizzle/operators.bench.tspackages/bench/__tests__/drizzle/operators.explain.test.tspackages/bench/package.jsonpackages/bench/sql/schema.sqlpackages/bench/src/drizzle/setup.tspackages/bench/src/harness/global-setup.tspackages/bench/src/harness/seed.tspackages/bench/vitest.config.tspackages/stack-drizzle/README.mdpackages/stack-drizzle/__tests__/bigint.test.tspackages/stack-drizzle/__tests__/codec.test.tspackages/stack-drizzle/__tests__/column.test.tspackages/stack-drizzle/__tests__/drizzle-operators-bigint.test.tspackages/stack-drizzle/__tests__/drizzle-operators-jsonb.test.tspackages/stack-drizzle/__tests__/exports.test.tspackages/stack-drizzle/__tests__/operators.test-d.tspackages/stack-drizzle/__tests__/operators.test.tspackages/stack-drizzle/__tests__/schema-extraction.test.tspackages/stack-drizzle/__tests__/selector.test.tspackages/stack-drizzle/__tests__/sql-dialect.test.tspackages/stack-drizzle/__tests__/types.test-d.tspackages/stack-drizzle/__tests__/types.test.tspackages/stack-drizzle/integration/adapter.tspackages/stack-drizzle/integration/json-adapter.tspackages/stack-drizzle/integration/lock-context.integration.test.tspackages/stack-drizzle/integration/null-persistence.integration.test.tspackages/stack-drizzle/integration/relational.integration.test.tspackages/stack-drizzle/package.jsonpackages/stack-drizzle/src/codec.tspackages/stack-drizzle/src/column.tspackages/stack-drizzle/src/index.tspackages/stack-drizzle/src/operators.tspackages/stack-drizzle/src/schema-extraction.tspackages/stack-drizzle/src/sql-dialect.tspackages/stack-drizzle/src/types.tspackages/stack-drizzle/src/v3/index.tspackages/stack-drizzle/src/v3/operators.tspackages/stack-drizzle/src/v3/schema-extraction.tspackages/stack-drizzle/tsup.config.tspackages/test-kit/package.jsonskills/stash-drizzle/SKILL.md
💤 Files with no reviewable changes (6)
- packages/stack-drizzle/tests/drizzle-operators-bigint.test.ts
- packages/stack-drizzle/tests/drizzle-operators-jsonb.test.ts
- packages/stack-drizzle/src/v3/index.ts
- packages/stack-drizzle/src/v3/schema-extraction.ts
- packages/stack-drizzle/package.json
- packages/stack-drizzle/src/v3/operators.ts
🛑 Comments failed to post (2)
.github/workflows/tests-bench.yml (1)
15-15: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Trigger bench smoke tests for all Stack changes.
@cipherstash/stack/v3resolves throughpackages/stack/src/encryption/v3.ts, which these filters exclude. A broken v3 client/export can therefore merge without running this smoke test. Includepackages/stack/**(or every direct Stack source/export dependency) in both path lists.Proposed fix
- - 'packages/stack/src/eql/v3/**' + - 'packages/stack/**' ... - - 'packages/stack/src/eql/v3/**' + - 'packages/stack/**'Also applies to: 28-28
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/tests-bench.yml at line 15, Update both path-filter lists in the bench workflow to include packages/stack/**, ensuring changes such as packages/stack/src/encryption/v3.ts and related Stack exports trigger bench smoke tests. Preserve the existing filters while adding coverage for all direct Stack source changes.packages/stack-drizzle/src/codec.ts (1)
38-44: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Validate the SteVec root ciphertext.
Line 38 accepts
{ v: 3, sv: [{}] }as an envelope, althoughsv[0].cis required. That invalid value reaches decryption instead of producingEqlV3CodecError.Proposed fix
+function hasCiphertextCarrier(value: unknown): boolean { + return ( + typeof value === 'object' && + value !== null && + !Array.isArray(value) && + 'c' in value && + value.c !== undefined + ) +} + - const hasSteVec = Array.isArray(envelope.sv) && envelope.sv.length > 0 + const hasSteVec = + Array.isArray(envelope.sv) && + envelope.sv.length > 0 && + hasCiphertextCarrier(envelope.sv[0])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.function hasCiphertextCarrier(value: unknown): boolean { return ( typeof value === 'object' && value !== null && !Array.isArray(value) && 'c' in value && value.c !== undefined ) } const hasSteVec = Array.isArray(envelope.sv) && envelope.sv.length > 0 && hasCiphertextCarrier(envelope.sv[0]) const missing = envelope.v === undefined ? '"v"' : envelope.c === undefined && !hasSteVec ? 'a ciphertext ("c", or a non-empty "sv" for a SteVec document)' : undefined🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/stack-drizzle/src/codec.ts` around lines 38 - 44, Update the envelope validation around hasSteVec and missing to require a ciphertext field on the SteVec root, specifically ensuring envelope.sv[0].c is defined when a non-empty sv array is present. Reject values such as { v: 3, sv: [{}] } with EqlV3CodecError before decryption, while preserving acceptance of valid ciphertext envelopes.
… scaffold, stale shipped docs Six review findings, each with the regression test that would have caught it. - `packages/bench/sql/schema.sql` indexed `eql_v3.ste_vec(enc_jsonb)`, a mechanical rename of the v2 expression. It builds, and `db-only.test.ts`'s `pg_indexes` check passes, but nothing the adapter emits ever mentions it: `@>` on `eql_v3_json_search` inlines to `eql_v3.to_ste_vec_query(a)::jsonb` (the bundle's own comment says so). A permanently-dead index in the one fixture whose job is to prove index engagement. Now indexed on the inlined expression, with `scripts/__tests__/bench-index-expressions.test.mjs` pinning each bench index against the operator body in the vendored EQL bundle — no database, no credentials, which matters because the bench's own EXPLAIN assertions need credentials and never run in CI. - `stash init --drizzle` scaffolded `extractEncryptionSchemaV3` from `@cipherstash/stack-drizzle/v3` into the user's repo — both removed here, so a freshly-initialised project would not resolve. Flipped the drizzle half of the generated strings (the `@cipherstash/stack/v3` half stays for PR 7) and pinned it in `utils-codegen-drizzle.test.ts`. - `skills/stash-encryption`, `packages/stack/README.md` and `AGENTS.md` still documented the `./v3` subpath and the `*V3` names. Both skills and the stack README are shipped artifacts. `no-removed-drizzle-surface.test.mjs` scans the shipped file set (deliberately not CHANGELOGs or specs, which should still name the old surface). - `vitest.shared.ts` aliased `@cipherstash/stack-drizzle/v3` to the deleted `src/v3/index.ts`; `vitest-shared-alias.test.mjs` asserts every alias target exists on disk. - The bench `matches` operand was the shared `value` prefix, which every seeded row contains — the bloom index had nothing to narrow, so the number measured a full scan. Uses a full seeded value now. - The bench importer's only CI gate was the Bun job's `turbo build`, which swallows its own test failures. Added an explicit typecheck step to the main test job. The eq/matches index assertions are sound and now say why: the operator wrappers are `LANGUAGE sql IMMUTABLE STRICT` single-SELECT bodies, so the planner inlines them and applies the same inlining to the stored index expression. The old comment claimed the adapter emitted `eql_v3.eq_term(col) = eql_v3.hmac_256(value)` — it emits `eql_v3.eq(col, term)`, and `eql_v3.hmac_256` does not exist in the bundle at all.
CI runs against the PR MERGE commit, so it saw a file this branch never had: `remove-v2` gained `utils-codegen.test.ts` (from the v3 domain-picker work) after this branch was cut, and it pins the generated `stash init` client to `extractEncryptionSchemaV3` / `@cipherstash/stack-drizzle/v3` — exactly the two names this PR removes. Textually the merge is clean; the conflict is semantic, which is why it only surfaced in CI. Points those assertions at the collapsed root and adds the matching negatives, so a regression to the `./v3` specifier fails here rather than in a scaffolded project. Narrowed `utils-codegen-drizzle.test.ts` to what the merged-in suite does not cover — `generatePlaceholderClient`, still untested — and updated it to the new `ColumnDef` shape (`domain`, not `dataType`/`searchOps`).
Part of the EQL v2 removal effort (#707). PR 5 of the linear stack — stacked on #769 (PR 4, stack-supabase).
What this does
Removes the EQL v2 root from
@cipherstash/stack-drizzleand collapses./v3→., per the design §"PR 5".src/index.ts,src/operators.ts,src/schema-extraction.ts) — confirmed v2-only (eql_v2_encryptedtype,@cipherstash/stack/schemaimports) — plus two v2-only query-builder test files../v3→.hard break, no alias (Decision 5): movedsrc/v3/*→src/*, de-suffixedcreateEncryptionOperatorsV3/extractEncryptionSchemaV3to unsuffixed. Removed the./v3export block andtypesVersionsfrom package.json; root keepsimport+require.packages/benchimporter fixed and given a realbuild(tsc --noEmit) script it previously lacked, so the importer is guarded in CI.stash-drizzle), README, changeset (stack-drizzle: major,stash: patch) updated.Green gate
build ✓ · test:types 7 pass · test 364 pass (all mock-driven, 0 skipped) · bench
tsc --noEmitexit 0 ·code:check0 errors.Decision 6
The deleted v2 root was authoring/query-building only — no decrypt path lived there (decrypt is via
@cipherstash/stack). The v3 codec + its 26-test round-trip suite are retained; read coverage is not dropped.Notes for reviewer
sql/schema.sqlv3 index SQL is best-effort, not live-validated — theeql_v3.eq_term/match_term/ste_vecindex-term function names weren't run against a live EQL v3 DB (no creds). Verify against the installed bundle before relying on bench numbers. Outside the build/typecheck gate.examples/basic(also a drizzle importer) intentionally left for PR 7.🤖 Generated with Claude Code
Summary by CodeRabbit
@cipherstash/stack-drizzleAPI./v3entry point and version-suffixed helper names.