test(v3): property tests for the empty-bloom needle guard (CIP-3665) - #431
Conversation
Follow-up from the PR #421 review: generalise the example-based empty-bloom guard tests into a property test over generated plaintexts, freshly encrypted through ZeroKMS each run. New e2e suite member tests/sqlx/tests/encrypted_domain/property/empty_bloom_guard.rs checks, over every ordered (value, needle) pair in each generated batch: 1. cross-emptiness never matches (a non-empty needle never matches an empty-bloom value, and vice-versa); 2. an empty needle matches a value iff that value's bloom is also empty (the LIKE '' shape — never the pre-guard match-everything); 3. `@@` agrees with the by-name eql_v3.matches (and its (domain, jsonb) overload) on every pair — operator/function equivalence under the guard; 4. the premise: sub-trigram plaintexts (< 3 chars) extract an empty bloom and all-letter plaintexts at/above the floor extract a non-empty one — the generated-input generalisation of empty_bloom_needle_is_actually_empty. Plus the deterministic bloom hits (equal plaintexts across independently encrypted ciphertexts; substring needles), since a bloom admits false positives but never false negatives. Pairs of two non-empty, unrelated blooms assert only property 3 — a hit there may be a legitimate false positive. Sub-trigram inputs exist in no generated fixture (TEXT_FIXTURES has none) and proptest can only generate plaintexts, so the suite is e2e/proptest-e2e-gated and named in the test:sqlx:e2e task per that task's comment. Classification is by measured bloom cardinality, not plaintext length. The empty string is generated on purpose: issue #262 is an _ord_ore CHECK concern, and this suite encrypts with the match index only.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR adds a feature-gated end-to-end property-test suite for empty-bloom matching. It validates bloom premises, matching equivalence, deterministic matches, permitted false positives, and integrates the suite into the SQLx e2e task. ChangesEmpty-bloom guard validation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Proptest
participant ZeroKMS
participant SQLx
Proptest->>ZeroKMS: Batch-encrypt generated plaintexts
ZeroKMS-->>Proptest: Return encrypted payloads
Proptest->>SQLx: Measure bloom cardinalities
SQLx-->>Proptest: Return bloom classifications
Proptest->>SQLx: Execute @@ and eql_v3.matches assertions
SQLx-->>Proptest: Return matching results
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/sqlx/tests/encrypted_domain/property/empty_bloom_guard.rs`:
- Around line 269-285: Update seeded so the fixed full-length "aardvark" seed is
appended before computing prefix, ensuring prefix always selects a value when
the original batch contains only sub-trigram strings. Preserve the existing
duplicate and deterministic seed additions while keeping the substring-needle
pair guaranteed for every generated batch.
🪄 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: 0ae8e8bc-b7a5-4cff-b611-3a7f66c0dcab
📒 Files selected for processing (4)
mise.tomltests/sqlx/tests/encrypted_domain/property/README.mdtests/sqlx/tests/encrypted_domain/property/empty_bloom_guard.rstests/sqlx/tests/encrypted_domain/property/mod.rs
freshtonic
left a comment
There was a problem hiding this comment.
Comment-only review.\n\nFinding: the new tracked files contain the private identifier , which makes fail and in turn fails . CI identifies three occurrences:\n\n- \n- \n- \n\nPlease describe the public behavior directly (or link a public GitHub issue) in those locations. The property/e2e jobs themselves are otherwise green.
freshtonic
left a comment
There was a problem hiding this comment.
Correction to my preceding review (shell formatting stripped identifiers): CIP-3665 causes test:public_identifiers and ci-required to fail. Occurrences are property/README.md line 86, property/empty_bloom_guard.rs line 2, and property/mod.rs line 39 under tests/sqlx/tests/encrypted_domain.
… (CIP-3665) The SQL doc validation CI job (mise run test:public_identifiers) rejects private issue-tracker identifiers in tracked files. Keep only the public PR #421 reference in the property-test docs and comments.
seeded() computed `prefix` before appending the fixed seeds, so a batch of only sub-trigram generated values left `prefix = None` and the substring-needle deterministic pair never fired — despite the suite doc claiming it always does. Append the fixed seeds (incl. the full-length "aardvark") first so a value at or above the trigram floor always exists to prefix. Addresses CodeRabbit review on PR #431. CIP-3665
Adds a property test for the empty-bloom needle guard in
eql_v3.matches, closing the loop on the review follow-up from #421 (#421 (comment)). Linear: CIP-3665.What
PR #421 fixed the empty-needle bug — an empty needle bloom (
{}, from a sub-trigram search string) was@>-contained by every value, so a bare containment matched EVERY row — with a generated guard (empty_bloom_guardincrates/eql-codegen/templates/functions/wrapper.sql.j2) and pinned it with example-based tests over thev3_text_empty_bloomfixture. This PR generalises those examples into a property test over generated plaintexts, freshly encrypted through ZeroKMS each run.New suite member:
tests/sqlx/tests/encrypted_domain/property/empty_bloom_guard.rs(e2e suite,proptest-e2e-gated, listed in thetest:sqlx:e2etask as the task's comment requires).Properties
Each proptest case generates a batch of strings — sub-trigram (
[a-z]{0,2}, including"") and full-length ([a-z]{3,8}) — plus deterministic seeds ("","a","pq","aardvark", a duplicate of the first value, and a 3-char prefix needle), batch-encrypts them with thematchindex (one ZeroKMS round trip), and checks over every ordered pair(value, needle):LIKE ''shape — an empty needle matches a value iff that value's bloom is also empty (never the pre-guard match-everything).col @@ needleagrees with the by-nameeql_v3.matches(col, needle)and with the(domain, jsonb)convenience overload on every generated pair.empty_bloom_needle_is_actually_empty, which pins this for two hand-picked values.On top of the guard's truth table it also asserts the deterministic bloom hits (a bloom admits false positives, never false negatives): equal plaintexts match across independently encrypted ciphertexts, and a substring needle matches its superstring value. Pairs with two non-empty blooms and no substring relationship assert only property 3 (a hit there may be a legitimate bloom false positive — no plaintext oracle exists).
Why e2e, not fixture
Sub-trigram plaintexts exist in no generated fixture (
eql-domains::TEXT_FIXTUREShas none — its shortest value is 3 chars), and proptest can only generate plaintexts, not ciphertext, so the suite must pay for fresh encryption. Classification is by measured bloom cardinality (cardinality(eql_v3.match_term(...))), not by plaintext length, so properties 1–3 stay honest for any tokenizer behaviour; property 4 separately pins the length ⇒ cardinality premise for the all-letter generator.The fixture suite's deliberate exclusion of the empty string (issue #262) is untouched — that is an
_ord_oreCHECK concern (ob: []); this suite encrypts with thematchindex only, so""is generated on purpose as the canonicalLIKE ''needle.Notes
"", single-char needles, duplicate plaintexts across independent ciphertexts, and prefix needles) behave exactly per the guard's truth table.Testing
Ran locally against a freshly migrated database (this branch's built EQL) with real ZeroKMS credentials:
cargo fmtandcargo clippy --features proptest-e2e --testsare clean.Summary by CodeRabbit
Tests
Documentation