docs: fix errors and gaps surfaced by a full documentation audit#419
Merged
Conversation
Every claim in the reference/tutorial/upgrade docs was verified against the src/v3 SQL source and the shipped @cipherstash/eql 3.0.2 bundle (byte-equivalent surfaces — every fix below applies to both). Highlights: Copy-pasteable SQL that failed when run: - GIN containment recipe was a syntax error (cast over a function call needs its own parens in an index_elem): ((eql_v3.to_ste_vec_query(col)::jsonb) jsonb_path_ops) — fixed in json-support, database-indexes, and the v2.3 guide (v2 form). Verified empirically on Postgres 17. - Query operands were cast to *storage* domains (public.eql_v3_*) whose CHECK requires the ciphertext key 'c' that query payloads omit — all examples now cast to the eql_v3.query_<T>_<variant> operand domains, and sql-support now documents that family (it was entirely absent). - Text match spelled @> in the proxy tutorial and README — it raises since U-008; fixed to @@. - README's GRANT ALTER ON ALL TABLES is not valid PostgreSQL (no grantable ALTER privilege); the Database Permissions section now defers to docs/reference/permissions.md. - v3.0 U-002's must-succeed verification needle carried hm, which the query_json validator rejects. - sql-documentation.md worked examples predated the eql_v3_internal split and the v=3 pin. Injectivity residue (the class PR #418 fixed, in the places it missed): - eql-functions' equality section no longer claims the ordering domains carry hm; coverage lists gain _ord_ope / text_search_ore throughout. - database-indexes: no more field-level eq_term equality-index advice (that overload is a deprecated ordering-bytes alias); GROUP BY guidance covers the numeric _ord domains (no eq_term — group on ord_term); text_ord_ope joins every hm-domain enumeration; op joins the term probe. - adding-a-scalar-encrypted-domain-type §2 now states the criterion: ordering-term-only domains are admissible iff ordering-term equality is lossless for the kind; string-like kinds must lead with Hm. The old unqualified rule would have generated silently-wrong equality for a future string-like scalar. Permissions, precisely (with sources): - permissions.md gains an Install privileges section: CREATE OPERATOR CLASS/FAMILY are superuser-gated in stock PostgreSQL (docs quoted); Aurora delegates to rds_superuser (release-noted); RDS-proper has no documented delegation (probe provided, verified both ways on PG 17); Supabase/Cloud SQL/Azure refuse (cited). Extension allow-lists are a different mechanism and never the reason. - Fixed the json_search validator claim: the CHECK calls the public eql_v3_is_valid_ste_vec_* validators, so the cast needs no eql_v3_internal grant (the doc's own privilege test proves it). Operational honesty: - README no longer calls re-install "idempotent": the installer's DROP SCHEMA eql_v3 CASCADE drops every functional index built on the extractors; both install notes now say to re-create indexes + ANALYZE after upgrades. - U-009: the shipped uninstaller has no BEGIN/COMMIT — the atomicity and lock-count claims now say psql --single-transaction. - "Silently skipped" opclass → skipped with a NOTICE, everywhere. Stale surfaces: phantom jsonb_array_elements_text removed from two docs; bare eql_v3_json no longer presented as searchable JSON; SchemaVersion 2→3; JSONB→JSON consts; 44→47 operator counts; 14→25 generated files (query-operand + json_entry surfaces now documented); text has eight domains; bigint→uuid as the worked hypothetical; eql-bindings README term table gains the op/OpeCllw row (ob is _ord_ore-only in v3); eql-functions gains the undocumented eql_v3.version() and eql_v3.lints(); json-support documents the function-form containment entrypoints (Supabase/PostgREST) and the storage-only eql_v3_json sibling. mise run docs:validate passes (267 files, 0 errors). Claude-Session: https://claude.ai/code/session_01BkEpKJC3975NHsKgMrCT8R
Contributor
|
Warning Review limit reached
Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis documentation-focused change aligns EQL v3 references, tutorials, permissions, indexing guidance, generated-output descriptions, JSON/query-domain semantics, and upgrade instructions with current v3 behavior. ChangesEQL v3 query and domain surfaces
Permissions and operational guidance
Release and migration documentation
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Docs-only PR that incorporates a repo-wide documentation audit against the src/v3 SQL surface and the shipped @cipherstash/eql 3.0.2 bundle, fixing broken copy/paste SQL, updating terminology/surfaces (query-operand domains, JSON/SteVec semantics), and tightening operational + permissions guidance to match current v3 behavior.
Changes:
- Fixes multiple SQL snippets and operator/cast examples so they run correctly (GIN cast parens, query-operand domain casts,
@@text match). - Updates reference docs to reflect the current v3 surface (injectivity split, OPE/ORE terms, JSON storage vs searchable domains, diagnostics functions).
- Reworks permissions/install/operational guidance to accurately describe superuser-gated ORE opclass behavior and upgrade/reinstall implications.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates TOC, components summary, permissions section, and upgrade notes to match current v3 behavior. |
| docs/upgrading/v3.0.md | Clarifies v3.0 breaking changes and corrects/modernizes verification and operator examples. |
| docs/upgrading/v2.3.md | Fixes a GIN index expression syntax issue and adds context about v2 source paths being removed. |
| docs/tutorials/proxy-configuration.md | Updates operator/cast examples (@@, query-operand casts) and term naming (op vs ob). |
| docs/reference/sql-support.md | Expands/clarifies supported surfaces, operand typing rules, and term/capability nuances. |
| docs/reference/query-performance.md | Updates GROUP BY guidance to reflect injectivity split across domains. |
| docs/reference/permissions.md | Adds install-privilege matrix + probe and corrects JSON cast/grant requirements. |
| docs/reference/json-support.md | Fixes GIN containment recipe and documents function-form entrypoints + domain distinctions. |
| docs/reference/eql-functions.md | Updates operator/function examples to use query-operand domains; adds diagnostics section. |
| docs/reference/database-indexes.md | Corrects index recipes/requirements around injectivity and ORE opclass behavior. |
| docs/reference/catalog-driven-architecture.md | Updates catalog naming and documents query-operand + json_entry generated surfaces. |
| docs/reference/adding-a-scalar-encrypted-domain-type.md | Tightens the injectivity/losslessness criterion for domain shapes and updates generator/output details. |
| docs/README.md | Refreshes reference index descriptions and term naming (op). |
| docs/development/sql-documentation.md | Fixes examples/templates to match v3 schema split and aggregate definitions. |
| docs/concepts/WHY.md | Updates ordering description to reflect CLLW-OPE default and ORE opt-in. |
| crates/eql-bindings/README.md | Updates term/wire key table to include op and correct hm/ob scoping. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Upgrade the RDS for PostgreSQL row in the install-privileges table from 'unverified' to supported: multiple CipherStash customers run ORE with the custom operator class on RDS in production. AWS release-notes the rds_superuser delegation only for Aurora, so the row keeps the honest sourcing (production-verified, probe available for the doubtful) rather than a docs citation. database-indexes and the U-003 TL;DR now say RDS/Aurora rather than singling out Aurora. Claude-Session: https://claude.ai/code/session_01BkEpKJC3975NHsKgMrCT8R
coderdan
commented
Jul 24, 2026
freshtonic
approved these changes
Jul 25, 2026
freshtonic
left a comment
freshtonic
left a comment
Contributor
There was a problem hiding this comment.
Reviewed the full diff against the current main surface — this is an accurate, high-value docs audit. Every technical claim I spot-checked holds:
- Verified against code:
crates/eql-domains/src/term.rsexists and carriesbinding_newtype;remove_generated_orphanslives in codegen; therender_query_*/json_entrycross renderers exist;eql_v3.version()/eql_v3.lints()exist. - Verified the precise counts this PR updates:
src/v3/scalars/integer/emits exactly 25 files (14→25) and each domain operators file has exactly 47CREATE OPERATOR(44→47). - The stale-fact corrections all match the shipped 3.0 surface:
@>→@@for text match,v:'2'→v:'3'/ SchemaVersion 3,eql_v3.hmac_256→eql_v3_internal.hmac_256,min_sfunc→eql_v3_internal.min_sfunc(+COMBINEFUNC/PARALLEL),jsonb→jsonfamily rename, and thequery_<T>_<variant>operand-domain casts (term-only, noc).
Genuine correctness fixes worth calling out: the GIN index expression now parenthesises the operand (USING gin ((eql_v3.to_ste_vec_query(col)::jsonb) jsonb_path_ops)) — the unparenthesised form is a real syntax problem — and the text-injectivity / _ord_ope equality notes are now consistent across database-indexes/eql-functions. The per-platform ORE-opclass table (RDS/Aurora ✅ vs Supabase/Cloud SQL/Azure ❌, with sources) is a strong addition over the previous blanket "superuser" framing.
No inaccuracies found. Approving.
- README: the domain-variant sentence used `public.eql_v3_json` as its example while describing the searchable surface; that domain is storage-only. Lead with `public.eql_v3_json_search` and state the storage-only variants explicitly. - json-support: the typed-operand examples were not boolean predicates — `doc -> $1` returns a `public.eql_v3_json_entry`, so both the ✅ and the ⚠ snippet failed if copy-pasted. Both are now full predicates against a `query_integer_ord` operand; verified on PG 17 that the typed forms plan through the encrypted operator and that the bare selector falls through to native `jsonb -> text` / `jsonb > jsonb`. - sql-support: link the Proxy repo from the parameter-binding note.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@docs/reference/adding-a-scalar-encrypted-domain-type.md`:
- Around line 41-42: Complete the UUID example in the introductory scalar-type
guidance by either replacing it with an existing integer- or string-backed type
or explicitly directing readers to the fourth-shape non-integer wiring steps. At
the later `uuid => uuid::Uuid` mapping explanation, clarify that the mapping
alone does not generate `ScalarType` and that non-integer types also require
updates to `scalar_domains.rs` and the fixture router; apply these changes at
both referenced documentation sites.
In `@docs/reference/catalog-driven-architecture.md`:
- Around line 78-79: Update the catalog-driven architecture description around
the JSON row to identify JSON as a mixed family: retain the three hand-written
Shape::SteVec domains while noting its generated scalar storage domain under
src/v3/scalars/json/. Keep the scalar_families() behavior description accurate
by explaining that it filters out JSON as a mixed family.
In `@docs/reference/database-indexes.md`:
- Around line 44-49: Remove the blank line between the blockquoted ORE
explanation and the fenced SQL example, or prefix that line with “>” so the
blockquote remains contiguous and satisfies Markdownlint MD028.
In `@docs/reference/sql-support.md`:
- Around line 85-86: Update the ordered-domain variant inventories consistently:
in docs/reference/sql-support.md lines 85-86, add _ord_ope and _ord_ore to the
MIN/MAX required variants; in docs/reference/database-indexes.md lines 84-86,
add text_ord, text_ord_ope, and text_ord_ore to the ordering-term list; and in
crates/eql-bindings/README.md lines 36-38, add the text ordering variants to the
OpeCllw and OreBlock256 backing lists.
🪄 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: fa8c1ed3-7ac0-4627-a3fb-af5a8615721e
📒 Files selected for processing (16)
README.mdcrates/eql-bindings/README.mddocs/README.mddocs/concepts/WHY.mddocs/development/sql-documentation.mddocs/reference/adding-a-scalar-encrypted-domain-type.mddocs/reference/catalog-driven-architecture.mddocs/reference/database-indexes.mddocs/reference/eql-functions.mddocs/reference/json-support.mddocs/reference/permissions.mddocs/reference/query-performance.mddocs/reference/sql-support.mddocs/tutorials/proxy-configuration.mddocs/upgrading/v2.3.mddocs/upgrading/v3.0.md
- catalog-driven-architecture §2 called `JSON` "the hand-written SteVec family", contradicting §2.3 and the catalog: the family carries three `Shape::SteVec` domains *plus* a generated `Shape::Scalar` storage domain (`public.eql_v3_json`, rendered into `src/v3/scalars/json/`). Also names the two iterators that differ on it — `scalar_families()` filters it out wholesale, `families_with_scalar_domains()` does not. - sql-support: the constructs matrix writes `_ord` where the domain-variant table above spells out `_ord` / `_ord_ope` / `_ord_ore`. Aggregates and operators are generated for all three, so state the shorthand once rather than expanding every row.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #420.
Follow-up to #418. After that fix, we audited every doc in this repo against two ground truths — the
src/v3SQL source and the shipped@cipherstash/eql3.0.2 bundle (verified byte-equivalent surfaces, so each fix applies to both). This PR resolves everything the audit found: ~70 findings across 16 files.mise run docs:validatepasses (267 files, 0 errors).Copy-pasteable SQL that failed when run
USING gin ((eql_v3.to_ste_vec_query(col)::jsonb) jsonb_path_ops). Fixed in json-support, database-indexes, and the v2.3 guide (v2 form). Verified empirically on PG 17 (the repo's own sqlx tests already used the correct form).$1::public.eql_v3_text_eqfails the domain CHECK (storage domains require the ciphertext keyc; query payloads are term-only). All examples now use theeql_v3.query_<T>_<variant>operand domains — andsql-support.mdnow documents that 39-domain family, which was previously absent from the page that tells users how to bind parameters.@>in the proxy tutorial and README troubleshooting — raises since U-008; now@@.GRANT ALTER ON ALL TABLES— PostgreSQL has no grantableALTERtable privilege; the section now defers todocs/reference/permissions.mdwith a corrected summary.hm, which thequery_jsonvalidator rejects (per the guide's own U-004/U-010). Corrected needle verified against the shipped validator.eql_v3_internalsplit and thev=3pin (eql_v3.hmac_256,eql_v3.min_sfunc,'v' = '2').Injectivity residue (the #418 class, in the places it missed)
eql-functions.mdno longer claims the ordering domains "carry anhmterm"; coverage lists gain_ord_ope/text_search_orethroughout (they have real operators and aggregates).database-indexes.md: the field-leveleq_termequality-index recommendation is gone (that overload is a deprecated alias returning ordering bytes; entry=is blocked); GROUP BY guidance now covers numeric_ordcolumns (noeq_term— group onord_term);text_ord_opejoins every hm-domain enumeration; the term probe gainsop.adding-a-scalar-encrypted-domain-type.md§2 now states the criterion, not just the numeric rule: ordering-term-only domains are admissible iff ordering-term equality is lossless for the kind; string-like kinds must lead withHm. As written, the old rule would have generated silently-wrong=for a future string-like scalar.Permissions, precisely (with sources)
permissions.md:CREATE OPERATOR CLASS/FAMILYare superuser-gated in stock PostgreSQL 14–18 (official docs quoted); Aurora PostgreSQL delegates them tords_superuser(Aurora 12.12.0 release note); RDS-proper has no documented delegation — a live-instance probe is provided (verified both ways on PG 17); Supabase refuses (Allowpostgresrole tocreate operator classandcreate operator familysupabase/supautils#72), Cloud SQL refuses (documented category rule), Azure Flexible Server refuses (NOSUPERUSER admin, no carve-out). Also states why this is a different mechanism from extension allow-lists — the opclass ships no files.json_searchgrant over-claim: the domain CHECK calls the publiceql_v3_is_valid_ste_vec_*validators (deliberately, for uninstall survival), so the cast needs noeql_v3_internalgrant — the repo's ownv3_privilege_tests.rspins this.Operational honesty
DROP SCHEMA eql_v3 CASCADE, which cascade-drops every functional index built on the extractors. Both install notes now say to re-create indexes +ANALYZEafter upgrades.BEGIN/COMMIT— atomicity and the 6,433-lock figure hold underpsql --single-transaction, which the note now says (and the lock-table formula now includesmax_prepared_transactions).Stale surfaces
Phantom
jsonb_array_elements_text(removed in 3.0) deleted from two docs; bareeql_v3_jsonno longer presented as searchable (it's storage-only;eql_v3_json_searchis the queryable domain — README components table and Getting Started fixed);SchemaVersion2→3;JSONB→JSONconsts; operator counts 44→47 (and 220→235); the generated surface is 25 files per scalar, not 14 — the query-operand andjson_entrysurfaces are now documented in both contributor docs; text has eight domains; the worked "add a type" hypothetical is nowuuid(bigint shipped long ago); the eql-bindings README term table gains theop/OpeCllwrow and scopesobto_ord_ore(it claimedobbacks_ord, wrong for v3);eql-functions.mdgains the undocumentedeql_v3.version()andeql_v3.lints();json-support.mdgains the function-form containment entrypoints (jsonb_contains/jsonb_contained_by/jsonb_array— the only path for Supabase PostgREST) and theeql_v3_jsonvs_json_searchdistinction; docs/README descriptions and TOCs updated; WHY.md's ordering description now says CLLW-OPE default / block-ORE opt-in; "prereleases ship three artifacts" → releases; the 2.3-era performance table is labelled as predating the 3.0 JSON redesign.Flagged, not changed (need owner confirmation)
upgrading/v3.0.md"cipherstash-client 0.38.1+ already emits it" — not verifiable from this repo.proxy-configuration.md/ README external linkcipherstash.com/docs/stack/cipherstash/encryption/schema— path looks off vs the docs-site tree; please confirm."v":3(was"v":2) — the pt envelope is Proxy-side; please confirm the Proxy accepts/expects 3 there.RDS-proper opclass supportResolved (ce132f4): confirmed from production — customers run ORE with the custom operator class on RDS; the permissions table row is now a ✅ with that sourcing.https://claude.ai/code/session_01BkEpKJC3975NHsKgMrCT8R
Summary by CodeRabbit
@@, and encrypted-domain variants.