Repository navigation
Conversation
|
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Comment |
Mutation testing (cargo-mutants,
|
| caught | missed | unviable | timeout |
|---|---|---|---|
| 28 | 0 | 39 | 0 |
Every mutant in the changed lines was caught by a test.
467b4a5 to
2d33d6c
Compare
Review (high effort): findings and how each was addressedAn independent review of the
The reviewer judged the planted false-positive test deterministic enough: missing 1000 times in a row is effectively impossible at that fill. 561 tests pass locally, with clippy and rustdoc clean. |
| type Query: QueryTerm; // what this index sends for `Op` | ||
| fn query(term: Self::Term) -> Self::Query; // made from the term it derived | ||
| type Query: TermFor<Op>; // what this index sends for `Op` | ||
| type Carry; // what the query keeps of the value |
There was a problem hiding this comment.
The name of this type doesn't make it clear what it's doing. Consider a rename and add some better docs to explain.
There was a problem hiding this comment.
An example of a query type that has a carry would be good.
| /// never from a target. | ||
| /// | ||
| /// A Bloom filter can hold every bit of the query without the value holding | ||
| /// the needle, so a match query is |
There was a problem hiding this comment.
Don't use coded words like "needle". Its a query term or search query.
2d33d6c to
4187894
Compare
|
Rebased onto #1190's chain rework (operator first, |
4187894 to
4c17100
Compare
A Bloom filter can hold every bit of a query without the value holding
the needle, so a `Matches` query returns false positives. ADR-0009
decision 4 makes such a term `Probabilistic`, with `verify` as the
operator's own meaning run on a decrypted value.
- `op::Probabilistic { type Plaintext; fn verify(&self, &Plaintext) -> bool }`.
Infallible: every outcome other than a match is `false`.
- `MatchQuery` keeps the needle's tokens as the index cut them, through a
new `Supports::carry` taken before the value is consumed. `verify` cuts
the decrypted value the same way and checks every needle token is among
its tokens. The tokens are zeroized on drop and never shown by `Debug`.
- `Supports::Query` is bounded by `op::TermFor<Op>`: any query term answers
`Eq`, only a probabilistic one answers `Matches`, so an index cannot
claim a text match without saying how its answer is checked
(tests/ui/op_inexact_operator_needs_probabilistic.rs).
- `verify` on an exact term does not compile (tests/ui/op_verify_exact_term.rs).
The tests plant a real Bloom false positive under a 32-bit filter and
check `verify` drops it.
Closes #1183
Claude-Session: https://claude.ai/code/session_01VYKAWbd361xDLmqBdLoUcB
- `Supports::query` takes the sealed `QueryToken` (from the #1190 fix), so no caller can pair a needle with another value's filter and have `verify` drop the real rows. `Needle`'s doc says how one is joined. - `TermFor`'s doc and ADR-0009 say the "only a probabilistic term answers `Matches`" rule binds this crate's terms; a term in another crate can opt in by hand. - The tokenizer zeroizes its own copies of the text: the folded string, its characters, and each n-gram, built at its final size so it does not reallocate. `verify` runs it on every decrypted row. - `MatchQuery`'s `PartialEq` says two needles can share a filter and still verify differently. - ADR-0009 decision 4's code shows the trait as this PR ships it; `target_limit` joins it with #1184. - Tests: `verify` is a token test, not a substring test ("abcd" verifies "abc bcd"), and the standard tokenizer verifies whole words in any order. Claude-Session: https://claude.ai/code/session_01VYKAWbd361xDLmqBdLoUcB
4c17100 to
36ecd73
Compare
Summary
Stacked on #1190. This PR implements ADR-0009 decision 4 for
Matches: a match query can say, after decrypt, whether a returned row really matches.op::Probabilistichastype Plaintextandfn verify(&self, &Plaintext) -> bool. It is infallible, because every outcome other than a match isfalse.MatchQueryverifies. It keeps the query's tokens (sem::QueryTokens), cut from the query's value by the index's own tokenizer and case folding. They are taken through a newSupports::carry, before the query's value is consumed.verifycuts the decrypted value the same way and checks that every query token is among its tokens. That is the meaning a Bloom match approximates, soverifykeeps every row the filter would keep if it had no false positives.MatchQuery'sDebugshows only the filter's positions.Supports::Queryis nowop::TermFor<Op>. Any query term answersEq; only a probabilistic one answersMatches. So none of this crate's indexes can claim a text match without saying how its answer is checked. A term in another crate can opt in by hand, so there the bound is a guide.verifyruns on every decrypted row.verify. Callingverifyon anEqualityTermdoes not compile.Tests
verifykeeps real matches, folds case only when the index does, and rejects other text.verifydrops that row.MatchQuery'sDebugshows no query token.tests/ui/op_verify_exact_term.rsandtests/ui/op_inexact_operator_needs_probabilistic.rspin the two compile errors.Test plan
cargo nextest run -p stack-encrypt --all-features: 561 passed. The review fixes are in a second commit; see the review comment.-D warningswith and without--all-features, and rustdoc with warnings as errors.Closes #1183
https://claude.ai/code/session_01VYKAWbd361xDLmqBdLoUcB