feat(codegen): catalog-driven Rust payload bindings (PR 4) - #327
Merged
Conversation
Migrate the two manual JsonSchema impls (SchemaVersion const:2, BloomFilter i16 bounds) to the 1.x json_schema! macro API, the DomainType::schema return type RootSchema -> schemars::Schema, and the schema-inspecting catalog_parity assertions to the 2020-12 shape ($defs, $ref carries description as a sibling). Regenerate schema/v3/*.json as 2020-12. Wire contract (required, strictness, const:2, i16 bounds) unchanged; ts-rs .ts bindings unchanged.
quote/proc-macro2/syn(v2)/prettyplease(=0.2.37) + RUST_GENERATED_MARKER + format_rs (prettyplease then stable rustfmt, with the @generated marker as line 1). rustfmt is the final formatter so committed generated files are clean under cargo fmt --check.
The four writer fns take a GeneratedKind selecting the ownership marker and the cleanup extension (.sql vs .rs). SQL call sites pass Sql; generated SQL unchanged (codegen:parity green).
Move the trait/consts/PhantomData impl into hand-written domain_type.rs and the all() inventory into inventory.rs (re-exported at crate::v3, public paths unchanged). Relocate the non-catalog-derivable float NaN and bool storage-only caveats into the hand-written mod.rs doc so the generated per-family modules don't drop them at cutover. mod.rs stays hand-written. types:check green.
The term->binding-newtype mapping (Hm->Hmac256, Ore->OreBlock256, Bloom->BloomFilter) is wire-contract data, so it lives on Term beside json_key/ctor/extractor (unit-tested). payload_terms returns the distinct field-bearing terms in wire order. PR 4's bindings emitter matches on these instead of a &str round-trip, keeping it exhaustive at compile time.
render_family_bindings renders each DomainFamily to its struct set + three- method DomainType impls (schema -> schemars::Schema, 1.x), with pinned envelope-then-term field order (Term::payload_terms, matching on the enum), the canonical derive/ts/serde attributes, a precise term-newtype import set, and one catalog-derived struct doc line (no field docs).
render_inventory_rs generates all() in CATALOG order (entries via super::), replacing the hand-maintained inventory. mod.rs stays hand-written (module doc + pub mod decls + re-exports).
generate_bindings writes one <family>.rs per catalog family + inventory.rs under crates/eql-bindings/src/v3 (GeneratedKind::Rust ownership). Exposed as `eql-codegen bindings`; the default no-arg run stays SQL-only so build / codegen:parity are unaffected.
Contributor
|
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 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 |
tobyhede
marked this pull request as draft
June 29, 2026 08:05
tobyhede
force-pushed
the
pr4-catalog-driven-bindings-codegen
branch
from
June 29, 2026 09:10
01843c4 to
c60c032
Compare
![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
🧹 Nitpick comments (2)
crates/eql-codegen/src/bindings.rs (1)
28-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep the render helpers non-public.
Only
generate_bindingsis used outside this module in the provided diff. Leavingformat_rs,render_family_bindings, andrender_inventory_rsaspubmakes formatting/output-shape details part ofeql-codegen's public contract for no clear caller. Prefer private visibility here and keepgenerate_bindingsas the single public entrypoint.♻️ Proposed change
-pub fn format_rs(tokens: TokenStream) -> String { +fn format_rs(tokens: TokenStream) -> String { @@ -pub fn render_family_bindings(family: &DomainFamily) -> String { +fn render_family_bindings(family: &DomainFamily) -> String { @@ -pub fn render_inventory_rs() -> String { +fn render_inventory_rs() -> String {Also applies to: 175-212, 220-255
🤖 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 `@crates/eql-codegen/src/bindings.rs` around lines 28 - 32, The render helpers are exposing implementation details unnecessarily; keep only generate_bindings public and make format_rs, render_family_bindings, and render_inventory_rs private within bindings.rs. Update their visibility declarations and verify any internal callers in the same module still compile, preserving generate_bindings as the sole external entrypoint.crates/eql-bindings/src/v3/inventory.rs (1)
6-48: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider caching or lazily initializing the inventory vector.
all()reallocates 46 boxedPhantomDatavalues on every call. For a function used in tests and drift checks, considerLazyLockoronce_cellto return a static slice.// Alternative: static slice avoids per-call allocation static ALL: &[Box<dyn DomainType>] = &[ Box::new(PhantomData::<super::int4::Int4>), // ... ];🤖 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 `@crates/eql-bindings/src/v3/inventory.rs` around lines 6 - 48, The all() function in inventory.rs rebuilds the full DomainType inventory on every call, causing repeated allocations for the PhantomData boxes. Update all() to use a cached, lazily initialized static collection (for example via LazyLock or once_cell) and return a shared slice/reference instead of allocating a new Vec each time; keep the existing inventory entries for symbols like Int4, Text, and Float8 in the cached initializer.
🤖 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 `@crates/eql-bindings/src/v3/mod.rs`:
- Around line 29-34: Update the module-level docs in mod.rs to broaden the
ordered-domain comparison beyond just integers, since the generated API also
includes ob-only ordered payloads for non-text types like date, timestamptz,
float8, and numeric. Adjust the wording around the text ordered domains and the
“integer ordered domains” sentence so it refers to non-text ordered domains
instead, while keeping the explanation that text routes equality through hm
because ORE is not equality-lossless.
In `@crates/eql-bindings/tests/catalog_parity.rs`:
- Around line 1-11: Update the module header doc in catalog_parity to match the
current test coverage: it should no longer say the inventory set/order is “no
longer policed here” since inventory_exactly_covers_catalog_in_order() now
enforces that check. Revise the top comment to clearly describe that the file
validates both the JSON Schema parity and the catalog inventory coverage/order,
using the existing symbols CATLOG, eql_bindings::SchemaVersion, and
inventory_exactly_covers_catalog_in_order() as the anchor points.
In `@crates/eql-codegen/src/writer.rs`:
- Around line 77-80: The directory scan in writer::write_generated_files is
swallowing read_dir entry errors via filter_map(|e| e.ok()), which can hide
cleanup failures. Update the PathBuf collection logic to propagate any
directory-entry I/O error from fs::read_dir(directory) instead of skipping it,
keeping the cleanup path in writer.rs deterministic and surfacing unexpected
generation issues.
In `@mise.toml`:
- Around line 214-222: The rollback trap in the v3 snapshot flow only restores
the src/v3 tree, so interrupted or failed replacement leaves bindings and schema
half-swapped. Update the existing trap and surrounding restore logic in this
script block to also back up and restore crates/eql-bindings/bindings and
crates/eql-bindings/schema when ok is not set. Make sure the mv/removal sequence
remains atomic-on-failure by preserving the old trees until both generated
outputs have been safely moved into place.
---
Nitpick comments:
In `@crates/eql-bindings/src/v3/inventory.rs`:
- Around line 6-48: The all() function in inventory.rs rebuilds the full
DomainType inventory on every call, causing repeated allocations for the
PhantomData boxes. Update all() to use a cached, lazily initialized static
collection (for example via LazyLock or once_cell) and return a shared
slice/reference instead of allocating a new Vec each time; keep the existing
inventory entries for symbols like Int4, Text, and Float8 in the cached
initializer.
In `@crates/eql-codegen/src/bindings.rs`:
- Around line 28-32: The render helpers are exposing implementation details
unnecessarily; keep only generate_bindings public and make format_rs,
render_family_bindings, and render_inventory_rs private within bindings.rs.
Update their visibility declarations and verify any internal callers in the same
module still compile, preserving generate_bindings as the sole external
entrypoint.
🪄 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
Run ID: e987a943-c7b9-4e9a-890a-25220317934d
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (111)
CLAUDE.mdcrates/eql-bindings/Cargo.tomlcrates/eql-bindings/bindings/v3/Bool.tscrates/eql-bindings/bindings/v3/Date.tscrates/eql-bindings/bindings/v3/DateEq.tscrates/eql-bindings/bindings/v3/DateOrd.tscrates/eql-bindings/bindings/v3/DateOrdOre.tscrates/eql-bindings/bindings/v3/Float4.tscrates/eql-bindings/bindings/v3/Float4Eq.tscrates/eql-bindings/bindings/v3/Float4Ord.tscrates/eql-bindings/bindings/v3/Float4OrdOre.tscrates/eql-bindings/bindings/v3/Float8.tscrates/eql-bindings/bindings/v3/Float8Eq.tscrates/eql-bindings/bindings/v3/Float8Ord.tscrates/eql-bindings/bindings/v3/Float8OrdOre.tscrates/eql-bindings/bindings/v3/Int2.tscrates/eql-bindings/bindings/v3/Int2Eq.tscrates/eql-bindings/bindings/v3/Int2Ord.tscrates/eql-bindings/bindings/v3/Int2OrdOre.tscrates/eql-bindings/bindings/v3/Int4.tscrates/eql-bindings/bindings/v3/Int4Eq.tscrates/eql-bindings/bindings/v3/Int4Ord.tscrates/eql-bindings/bindings/v3/Int4OrdOre.tscrates/eql-bindings/bindings/v3/Int8.tscrates/eql-bindings/bindings/v3/Int8Eq.tscrates/eql-bindings/bindings/v3/Int8Ord.tscrates/eql-bindings/bindings/v3/Int8OrdOre.tscrates/eql-bindings/bindings/v3/Numeric.tscrates/eql-bindings/bindings/v3/NumericEq.tscrates/eql-bindings/bindings/v3/NumericOrd.tscrates/eql-bindings/bindings/v3/NumericOrdOre.tscrates/eql-bindings/bindings/v3/Text.tscrates/eql-bindings/bindings/v3/TextEq.tscrates/eql-bindings/bindings/v3/TextMatch.tscrates/eql-bindings/bindings/v3/TextOrd.tscrates/eql-bindings/bindings/v3/TextOrdOre.tscrates/eql-bindings/bindings/v3/TextSearch.tscrates/eql-bindings/bindings/v3/Timestamptz.tscrates/eql-bindings/bindings/v3/TimestamptzEq.tscrates/eql-bindings/bindings/v3/TimestamptzOrd.tscrates/eql-bindings/bindings/v3/TimestamptzOrdOre.tscrates/eql-bindings/schema/v3/bool.jsoncrates/eql-bindings/schema/v3/date.jsoncrates/eql-bindings/schema/v3/date_eq.jsoncrates/eql-bindings/schema/v3/date_ord.jsoncrates/eql-bindings/schema/v3/date_ord_ore.jsoncrates/eql-bindings/schema/v3/float4.jsoncrates/eql-bindings/schema/v3/float4_eq.jsoncrates/eql-bindings/schema/v3/float4_ord.jsoncrates/eql-bindings/schema/v3/float4_ord_ore.jsoncrates/eql-bindings/schema/v3/float8.jsoncrates/eql-bindings/schema/v3/float8_eq.jsoncrates/eql-bindings/schema/v3/float8_ord.jsoncrates/eql-bindings/schema/v3/float8_ord_ore.jsoncrates/eql-bindings/schema/v3/int2.jsoncrates/eql-bindings/schema/v3/int2_eq.jsoncrates/eql-bindings/schema/v3/int2_ord.jsoncrates/eql-bindings/schema/v3/int2_ord_ore.jsoncrates/eql-bindings/schema/v3/int4.jsoncrates/eql-bindings/schema/v3/int4_eq.jsoncrates/eql-bindings/schema/v3/int4_ord.jsoncrates/eql-bindings/schema/v3/int4_ord_ore.jsoncrates/eql-bindings/schema/v3/int8.jsoncrates/eql-bindings/schema/v3/int8_eq.jsoncrates/eql-bindings/schema/v3/int8_ord.jsoncrates/eql-bindings/schema/v3/int8_ord_ore.jsoncrates/eql-bindings/schema/v3/numeric.jsoncrates/eql-bindings/schema/v3/numeric_eq.jsoncrates/eql-bindings/schema/v3/numeric_ord.jsoncrates/eql-bindings/schema/v3/numeric_ord_ore.jsoncrates/eql-bindings/schema/v3/text.jsoncrates/eql-bindings/schema/v3/text_eq.jsoncrates/eql-bindings/schema/v3/text_match.jsoncrates/eql-bindings/schema/v3/text_ord.jsoncrates/eql-bindings/schema/v3/text_ord_ore.jsoncrates/eql-bindings/schema/v3/text_search.jsoncrates/eql-bindings/schema/v3/timestamptz.jsoncrates/eql-bindings/schema/v3/timestamptz_eq.jsoncrates/eql-bindings/schema/v3/timestamptz_ord.jsoncrates/eql-bindings/schema/v3/timestamptz_ord_ore.jsoncrates/eql-bindings/src/lib.rscrates/eql-bindings/src/v3/bool.rscrates/eql-bindings/src/v3/date.rscrates/eql-bindings/src/v3/domain_type.rscrates/eql-bindings/src/v3/float4.rscrates/eql-bindings/src/v3/float8.rscrates/eql-bindings/src/v3/int2.rscrates/eql-bindings/src/v3/int4.rscrates/eql-bindings/src/v3/int8.rscrates/eql-bindings/src/v3/inventory.rscrates/eql-bindings/src/v3/mod.rscrates/eql-bindings/src/v3/numeric.rscrates/eql-bindings/src/v3/terms.rscrates/eql-bindings/src/v3/text.rscrates/eql-bindings/src/v3/timestamptz.rscrates/eql-bindings/tests/catalog_parity.rscrates/eql-bindings/tests/mod_pins_catalog.rscrates/eql-bindings/tests/ts_property_order.rscrates/eql-codegen/Cargo.tomlcrates/eql-codegen/src/bindings.rscrates/eql-codegen/src/consts.rscrates/eql-codegen/src/generate.rscrates/eql-codegen/src/lib.rscrates/eql-codegen/src/main.rscrates/eql-codegen/src/writer.rscrates/eql-codegen/tests/bindings_parity.rscrates/eql-codegen/tests/cli.rscrates/eql-domains/src/spec.rscrates/eql-domains/src/term.rsdocs/reference/adding-a-scalar-encrypted-domain-type.mdmise.toml
One Rust payload struct per eql_v3 SQL domain, generated by eql-codegen from eql-domains::CATALOG, with ts-rs/schemars deriving the committed TypeScript and JSON Schema. Each struct carries a catalog-derived doc — a summary line plus the supported operators and required payload keys (capability label, Term::operators_for_terms, ENVELOPE_KEYS ++ Term::term_json_keys); the keys list surfaces structural distinctions such as text's dual-term ordered domains (hm + ob) versus the integer ordered domains (ob). No per-field docs: per-term semantics live on the shared term newtypes and non-derivable per-family caveats in mod.rs. mise run types:generate regenerates Rust then TS/JSON; types:check is the committed-reference drift gate.
…ndings Byte-parity and determinism gates for the committed Rust bindings, TS property- order pin, catalog/inventory parity, mod.rs pub-mod pin, and a bindings CLI smoke test — so a stale or hand-edited generated file, a dropped domain, or a reordered payload fails CI.
Document in CLAUDE.md and the adding-a-scalar reference that the Rust payload bindings (and downstream TS/JSON via ts-rs/schemars) are catalog-generated, that generated structs carry a catalog-derived struct doc (summary + operators + required keys, no per-field docs), schemars emits JSON Schema 2020-12, non-derivable caveats live in mod.rs, and types:check is the drift gate.
tobyhede
force-pushed
the
pr4-catalog-driven-bindings-codegen
branch
from
June 29, 2026 22:13
c60c032 to
af24b0b
Compare
tobyhede
marked this pull request as ready for review
June 29, 2026 22:24
This was referenced Jun 30, 2026
This was referenced Jul 13, 2026
Closed
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
1 participant
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.
Summary
PR 4 (the final, central PR) of the unified catalog-codegen design.
eql-codegennow emits the Rust payload bindings (crates/eql-bindings/src/v3/<family>.rsstructs +DomainTypeimpls + a generatedinventory.rscarryingall()) from theeql-domainscatalog — the same generate-to-committed-source mechanism it already uses for SQL. The existingts-rs/schemarsderives then produce the committed TypeScript/JSON bindings downstream. Drift is caught by the established committed-reference pattern:mise run types:checkregenerates andgit diffs the committed output, now broadened to cover the generated.rs.The hand-written
DomainTypetrait, the shared newtypes (SchemaVersion/Identifier/Ciphertext/Hmac256/OreBlock256/BloomFilter), thePhantomDataplumbing, and the architectural module doc (incl. the non-catalog-derivable float-NaN and bool storage-only caveats) stay hand-written inmod.rs/domain_type.rs/terms.rs.What changed
schema/v3/*.jsonreshaped to JSON Schema 2020-12 ($defs,$refcarriesdescriptionas a sibling). Wire contract (required, strictness,const: 2, i16 bounds) unchanged;ts-rs.tsunaffected.eql-codegenRust emitter —quote!+prettyplease+ stablerustfmt, with a// @generatedownership marker as line 1;GeneratedKind {Sql, Rust}generalizes the writer;Term::binding_newtype/Term::payload_termskeep the term→newtype map exhaustive at compile time; a dedicatedeql-codegen bindingssubcommand (default no-arg run stays SQL-only).inventory.rsreplaced by generated output. Only diff in the committed.ts/.jsonis doc prose (one catalog-derived struct line, no field docs); fields/types/order/required/strictness unchanged — verified bycatalog_parity+v3_conformance+ the newts_property_orderguard.types:generaterunseql-codegen bindingsfirst;types:checkbroadened tocrates/eql-bindings/src/v3. Added a cargo-level byte-parity + determinism test for the bindings (socargo testalone catches a stale/hand-edited.rs), a CLI smoke test, anENVELOPE_KEYStie-in test, and restored the ordered inventory guard.Notes
capability_labelnowpanic!s on an unmapped bare-domain name instead of silently emitting generic doc text (preserves the compile-checked-catalog intent).CHANGELOG.mdentry: PR 4 is internal —protect-ffi/cipherstash-clientdo not consumeeql-bindings, and the JSON payload shapes are unchanged; only the provenance of the Rust bindings (hand-written → generated) changed. (If a reviewer judges the JSON Schema 2020-12 reshape user-visible, a one-lineChangedentry can be added.)Verification (no DB / no creds)
All green:
cargo test(eql-domains/codegen/bindings),cargo fmt --check,cargo clippy -D warnings,mise run test:crates,codegen:parity,types:check,test:matrix:inventory,test:self_contained_v3.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation