Skip to content

feat(stack-encrypt): ordering operators Lt, Lte, Gt, Gte and Between, and remove query_index - #1208

Merged
coderdan merged 3 commits into
nextfrom
feat/stack-encrypt-ordering-operators
Oct 17, 2026
Merged

coderdan merged 3 commits into
nextfrom
feat/stack-encrypt-ordering-operators

Conversation

@coderdan

Copy link
Copy Markdown
Contributor

Summary

Stacked on #1206. This PR implements ADR-0009 decisions 1 and 7 for the orderings: the operators Lt, Lte, Gt, Gte and Between, Between as one term carrying both bounds, and the removal of query_index, since every index a plan can declare is now reachable through an operator.

let under = cipher.query(op::Lt, &40u32).on(&age_plan).await?;          // OreTerm<u32>
let range = cipher.query(op::Between, &(18u32, 65)).on(&age_plan).await?; // RangeQuery<OreTerm<u32>>
let ope   = cipher.query(op::Gte, &40u32).via(Ope).on(&age_plan).await?;  // OpeTerm<u32>
  • The operators. Unit types in op, like Eq. The ORE and OPE indexes answer them over the integer types, through a new marker op::Ordered; ORE is the default and OPE is named with .via(Ope). An ordering on text does not compile: text has no order term this crate produces, and the ADR leaves that to the catalog.
  • What a comparison sends. The index's own term for the value. This crate's ORE is CLLW, whose ciphertext has no left half to send alone, so what is sent is what is stored, as for an HMAC. Supports is where block ORE's left-only term goes when stack-encrypt: delete dynamic::Value and Scalar, derive terms through vitaminc, add block ORE, drop Bool order terms #1140 lands, and the ADR says so now.
  • Between is one term. sem::RangeQuery<T> carries both bounds' terms, derived under the one label and settled together, read with lo(), hi() and into_bounds(). A range whose low bound is above its high bound is refused when the query is awaited, before either term is derived and before any keyset is loaded, with the new TermError::InvertedRange; (v, v) is one value.
  • An operator says what it takes. Operator gains type Arity, One or Pair. op::Input<A> relates the value a query is given to the plaintext the plan is checked against, and derives one term per plaintext. Supports, Answers and DefaultIndex are keyed by that input, so Supports<Eq, S> reads as before and Between is Supports<Between, (S, S)>. Supports::query takes the input's terms. A query given the wrong shape (op::Between with one value, op::Lt with a pair) does not compile.
  • query_index is gone, with QueryIndexBuilder and QueryIndex. query_index(Ore, &v) is query(op::Lt, &v) (any ordering gives the same term) and query_index(Match, text) is query(op::Matches, text). The chain holds its own input, index and label; Resolves::resolve takes the index and the call's context and returns the index and the label.

ADR-0009's decision 2 now shows the shape that ships, decision 7 names RangeQuery and InvertedRange, and the query_index sentences are past tense.

Tests

  • tests/op.rs: every ordering sends the stored ORE term and orders as the values; OPE through via; a typed target answers through its index; Between equals the Gte and Lte terms of its bounds, brackets a stored value, and (v, v) is one value; an inverted range is refused through on and in an all batch with no keyset loaded; a fields plan answers the orderings under the field's label, including an index-only i64 field with negative values, refuses the default Ore where only Ope is declared, and refuses a value of the wrong type.
  • Unit test in op: the range check accepts (1, 2) and (2, 2) and refuses (3, 2).
  • Two new compile-fail cases: an ordering on a text plan, and Between given one value or Lt given a pair.
  • tests/plan.rs and tests/plan_grammar.rs migrated from query_index to the operators, and codes.rs carries the new error.

Test plan

  • cargo nextest run -p stack-encrypt --all-features: 583 passed.
  • Doctests, clippy with -D warnings with and without --all-features, rustdoc with warnings as errors, cargo fmt --check.
  • cargo check --workspace --all-features --all-targets, clippy on the Go guest with eql,deterministic-kms, and cargo check -p eql-bindings --features stack-encrypt: nothing outside the crate used query_index.
  • CI, including mutants on the diff.

#1140 is listed as blocking this issue. What it blocks is the left-ciphertext query term, not the operators: the operators ship here over CLLW, and block ORE changes Supports<Lt, S>::Query for its own index when it arrives.

Closes #1186

https://claude.ai/code/session_01VYKAWbd361xDLmqBdLoUcB

@coderdan
coderdan requested a review from a team as a code owner October 14, 2026 07:06
@changeset-bot

changeset-bot Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 8daa5ae

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 81946b7f-b629-48a6-9cb7-c4c0cbe67d68

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-14T07:10:41.353869Z 69ef7b3 PR opened
🔒 Security Review ✅ Completed 2026-10-14T07:12:16.755376Z 69ef7b3 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Mutation testing (cargo-mutants, --in-diff, stack-auth + stack-encrypt)

caught missed unviable timeout
134 0 45 0

Every mutant in the changed lines was caught by a test.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 69ef7b3c35

ℹ️ 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".

Comment thread packages/stack-encrypt/src/plan/chain.rs

@freshtonic freshtonic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of this PR's diff against #1206.

Ordering on folded text. My earlier ADR-0009 concern was that a strict < on folded or collated text can drop real matches, and verify cannot bring back rows the server did not return. This PR does not implement that rule. op::Ordered covers only the integer types, and an ordering on a text plan does not compile (tests/ui/op_order_on_text.rs). On integers, CLLW ORE and OPE order is exact. So Lt, Lte, Gt, Gte and an inclusive Between give no false negatives and no false positives, and no verify is needed.

Other checks. Between derives both bounds under one label and returns one term. InvertedRange is refused before any keyset load, also in an all batch, and (v, v) is accepted. The signed i64 test covers negative values. Mutants: 2 caught, 0 missed. Codex already raised the loss of a query path for Match<O> without MatchCase and for ORE/OPE over text; I do not repeat it.

No blocking defects from me.

Suggested follow-up PR

  • Ordered is public and not sealed. See the inline comment at src/op/mod.rs:256.
  • ADR-0009 line 265 still lists "ORE and OPE on every kind, text included (Lt, Between)" as exact, and line 839 says the same for AtPath<Lt> on a text leaf. This PR excludes text. Reword both so the text-order decision stays open for the catalog (my earlier ADR point, still open).
  • CI: the Test EQL PG17 shards fail on SteVec selector drift, the same as the base. This PR does not cause it.

Comment thread packages/stack-encrypt/src/op/mod.rs
@coderdan
coderdan force-pushed the feat/stack-encrypt-short-match-empty-filter branch from b05caf6 to 9dbb6b5 Compare October 16, 2026 11:16
@coderdan
coderdan force-pushed the feat/stack-encrypt-ordering-operators branch from 69ef7b3 to 9895211 Compare October 16, 2026 11:16
@coderdan

Copy link
Copy Markdown
Contributor Author

Rebased with #1190 onto the current #1177. ADR-0009 was merged by hand in three places: decision 2 now sketches Supports<Op, In: Input<Op::Arity>> with check(&self, input), carry and query(.., token: QueryToken), as the code has it, keeping #1177's prose on QueryToken; decision 7 and the consequences take this PR's Between and query_index text; Between examples keep #1177's by-value (lo, hi). Code unchanged.

@coderdan

Copy link
Copy Markdown
Contributor Author

Correction to the note above: decision 7 keeps this PR's cipher.query(op::Between, &(lo, hi)).on(&plan), since query takes its input by reference (value: &Q). The by-value (18, 65) examples elsewhere in ADR-0009 are #1177's text and are left to #1177. Amended into this PR's commit; #1209 to #1216 rebased onto it.

@coderdan
coderdan force-pushed the feat/stack-encrypt-ordering-operators branch from 9895211 to 909eb50 Compare October 16, 2026 11:17
@coderdan
coderdan added this pull request to stack #1230 October 16, 2026 11:24
@coderdan
coderdan force-pushed the feat/stack-encrypt-short-match-empty-filter branch from 9dbb6b5 to a9f42c1 Compare October 16, 2026 12:06
@coderdan
coderdan force-pushed the feat/stack-encrypt-ordering-operators branch from 909eb50 to 20513db Compare October 16, 2026 12:06
@coderdan

Copy link
Copy Markdown
Contributor Author

Ordered is sealed in 20513db (inline reply). The ADR-0009 wording on text ordering is fixed in #1177 (266f436), where that text lives. Everything above was rebased.

@blacksmith-sh

This comment has been minimized.

@coderdan
coderdan force-pushed the feat/stack-encrypt-short-match-empty-filter branch from a9f42c1 to c8a3934 Compare October 16, 2026 13:11
@coderdan
coderdan force-pushed the feat/stack-encrypt-ordering-operators branch from 20513db to 63331c7 Compare October 16, 2026 13:11

Copy link
Copy Markdown
Contributor
  • packages/stack-encrypt/CHANGELOG.md:136: the migration note says query_index(Match, text) becomes query(op::Matches, text), "whose MatchQuery carries the same filter". They are not the same. The old QueryIndex returned the stored MatchTerms, ran no EmptyTermText check and needed no MatchCase. The new path refuses text that produces no tokens, does not compile for a config without MatchCase, and returns a different type. Also, src/op/mod.rs:411 still says a config without MatchCase "can still store match terms and query them by index", but this PR deletes that path.
  • src/op/mod.rs:283: Ordered has no #[diagnostic::on_unimplemented], unlike Supports/TermFor/DefaultIndex. So query(op::Lt, "x") reports only "String: Ordered is not satisfied" (tests/ui/op_order_on_text.stderr) and never says orderings are integer-only. In the same way, src/plan/chain.rs:611 puts F: Input<Op::Arity> on the whole impl block. As a result query(op::Between, &40u32) fails with E0599 ("method on exists but its trait bounds were not satisfied: u32: Input<Pair>") and never says to pass &(lo, hi).

Generated by Claude Code

… no query_index

ADR-0009 decisions 1 and 7. `op::Lt`, `Lte`, `Gt` and `Gte` take one
value and `op::Between` a pair of bounds, inclusive at both ends. The ORE
and OPE indexes answer them over the integer types (`op::Ordered`), ORE by
default and OPE through `.via(Ope)`; an ordering on text does not compile,
since text has no order term this crate produces. A comparison's query
term is the index's own term for the value: this crate's ORE is CLLW,
whose ciphertext has no left half to send alone, so what is sent is what
is stored, as for an HMAC, and `Supports` is where the left-only term of
block ORE (#1140) goes when it lands. A range's query term is one
`sem::RangeQuery` carrying both bounds' terms, derived under the one
label, read with `lo()` and `hi()`. A range whose low bound is above its
high bound is refused when the query is awaited, before either term is
derived and before any keyset is loaded (`TermError::InvertedRange`).

An operator now says what a query takes through `op::Arity`: `One`
plaintext or a `Pair` of them. `op::Input<A>` relates the value a query is
given to the plaintext the plan is checked against, and derives one term
per plaintext. `Supports`, `Answers` and `DefaultIndex` are keyed by that
input, so `Supports<Eq, S>` reads as before and `Between` is
`Supports<Between, (S, S)>`; `Supports::query` takes the input's terms.

`StackCipher::query_index` goes, with `QueryIndexBuilder` and
`QueryIndex`: every index a plan can declare is reachable through an
operator now, and the chain holds its own input, index and label.
`Resolves::resolve` takes the index and the call's context and returns
the index and the label. ADR-0009's decision 2 shows the shape that
ships, and its mentions of `query_index` are past tense.

Closes #1186

Claude-Session: https://claude.ai/code/session_01VYKAWbd361xDLmqBdLoUcB
Ordered was public and not sealed, so a downstream crate could implement
it for its own type, such as a newtype over folded text that also
implements the CLLW traits. Between refuses an inverted range with the
type's PartialOrd while the server compares ORE terms, so such a type
could refuse a valid range or accept an empty one, and it would open
ordering on text, which ADR-0009 leaves to the catalog. Ordered now
requires op::sealed::Sealed, which the macro implements for the integer
types.

A compile_fail doctest pins it with a type that meets every other bound,
so the seal is the only reason it fails; without the seal the same
snippet compiles. A trybuild test would pin rustc's list of sample
implementors, which changes whenever a later layer seals another type.
- The CHANGELOG's `query_index` migration note no longer says
  `query(op::Matches, text)` is the same call. It returns a `MatchQuery`,
  refuses text that cuts no token, and compiles only for a configuration
  with `op::MatchCase`. `query_index(Ore, text)` and `query_index(Ope,
  text)` have no replacement until the order of text is settled, and the
  note says so.
- `op::MatchCase`'s doc no longer says a configuration without it can be
  queried by index: that path is gone, and the doc names the one-line impl
  that makes such a configuration queryable.
- `op::Ordered` has a curated diagnostic: an ordering over text, or over
  any type outside the integers, now says the orderings are integer-only.
- `query` carries the `Input<Op::Arity>` bound and `op::Input` has a
  curated diagnostic, so `query(op::Between, &40u32)` fails at the value
  and says to pass `&(lo, hi)`, not only with an E0599 on `on`.
@coderdan
coderdan force-pushed the feat/stack-encrypt-short-match-empty-filter branch from c8a3934 to c816046 Compare October 17, 2026 02:29
@coderdan
coderdan force-pushed the feat/stack-encrypt-ordering-operators branch from 63331c7 to 8daa5ae Compare October 17, 2026 02:29
@coderdan

Copy link
Copy Markdown
Contributor Author

Thanks @tobyhede. All four are addressed in 8daa5ae:

  • CHANGELOG:136. The migration note no longer says query(op::Matches, text) carries the same filter. It now says the new call returns a MatchQuery rather than the stored MatchTerms, refuses text that cuts no token (EmptyTermText), and compiles only for a configuration with MatchCase. It also says query_index(Ore|Ope, text) has no replacement until the order of text is settled.
  • op/mod.rs:411. The MatchCase doc no longer offers the deleted query-by-index path. It now says a configuration without the impl can store terms, but nothing can query them, and names the one-line impl.
  • Ordered diagnostic. It now has #[diagnostic::on_unimplemented]: "String is not ordered: the ordering operators are answered over the integer types only", with a note listing the integer types and pointing to ADR-0009 for text. tests/ui/op_order_on_text.stderr is re-recorded.
  • Between with one value. query itself now carries F<Q>: Input<Op::Arity>, and op::Input has a curated diagnostic. query(op::Between, &40u32) now fails at the value with "u32 is not what a query of arity Pair is given", plus a note to pass &(lo, hi). The E0599 on .on still follows it. tests/ui/op_between_takes_a_pair.stderr is re-recorded.

@coderdan
coderdan removed this pull request from stack #1230 October 17, 2026 05:57
Base automatically changed from feat/stack-encrypt-short-match-empty-filter to next October 17, 2026 06:00
@coderdan
coderdan merged commit ab2f6d0 into next Oct 17, 2026
68 of 94 checks passed
@coderdan
coderdan deleted the feat/stack-encrypt-ordering-operators branch October 17, 2026 06:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

stack-encrypt: ordering operators Lt, Lte, Gt, Gte and Between, and remove QueryUsing::index

3 participants