docs: jsonb domains + per-domain terms/extractors in the manifest (closes #365, #366) - #368
Merged
Merged
Conversation
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 |
coderdan
force-pushed
the
docs/eql-manifest-generator
branch
from
July 6, 2026 04:18
ec03a7b to
d09852a
Compare
…ifest Resolves #365 and #366 — both via the catalog dump, no SQL autogen. eql_domains::CATALOG (Shape::SteVec), but their SQL is deliberately hand-written (hand-tuned CHECKs, #354) and `scalar_families()` filters them out of the dump. Add them as a new **additive** `stevec` field on `CatalogDump` — scalar-only consumers (the fixture-coverage task) read only `types[]`, so they're unaffected. The manifest now carries all 51 domains (48 scalar + 3 jsonb). from eql_domains::Term) to each `DomainEntry`, linking a domain to its extractor functions (e.g. integer_ord -> eql_v3.ord_term). Authoritative — resolves the docs drift-lint false-flags on eq_term / ord_term / match_term. Both fields are additive; all eql-codegen + eql-domains tests pass (89 + 91). The manifest generator maps them through (termFunctions on scalars; the jsonb family as `json`-capability domains). Full pipeline verified end-to-end (doxygen -> XML + dump-catalog -> 984 functions + 51 domains). Stacked on #364. Claude-Session: https://claude.ai/code/session_01CqDNqLSEEkCi7xAJFq7HJA
coderdan
force-pushed
the
docs/eql-manifest-jsonb-terms
branch
from
July 6, 2026 04:21
145a5ba to
85ed74d
Compare
tobyhede
reviewed
Jul 6, 2026
tobyhede
left a comment
tobyhede
left a comment
Contributor
There was a problem hiding this comment.
Two low-severity issues from review — both confirmed against the code, neither a correctness bug.
Per @tobyhede's review on #368: - dump.rs doc comments referenced `eql_v3.json` / "eql_v3-relative" — the jsonb domains resolve under `public` (only the extractor *functions* are eql_v3). Corrected the CatalogDump.stevec and SteVecEntry.full_name docs. - The catalog models no per-SteVec-entry terms (JSONB_DOMAINS carry `terms: &[]`, enforced by `shape_and_terms_are_consistent`), so `term_infos(d.terms)` was provably empty and the searchable SteVec family rendered inert (no termFunctions). Hardcode the real hand-written ste_vec extractors for now: `hm` -> eq_term, `oc` -> ore_cllw (src/v3/jsonb/operators.sql). The manifest now links public.json / jsonb_entry / jsonb_query to eql_v3.eq_term / eql_v3.ore_cllw. Operators for the SteVec family remain a follow-up (DB introspection of pg_operator is the reliable source). Claude-Session: https://claude.ai/code/session_01CqDNqLSEEkCi7xAJFq7HJA
The inline TermInfo { .. } literals added in the review-response commit
tripped cargo fmt --check (Rust workspace crates CI). Expand them to the
one-field-per-line form rustfmt wants.
Claude-Session: https://claude.ai/code/session_01CqDNqLSEEkCi7xAJFq7HJA
coderdan
commented
Jul 6, 2026
The hardcoded hm/oc terms were stamped onto all three SteVec domains, but
per src/v3/jsonb/{functions,operators}.sql the eq_term/ore_cllw extractors
take public.jsonb_entry (the sv *element* type) exclusively:
- eq_term(jsonb_entry) -> = / <> (coalesce(hm, oc))
- ore_cllw(jsonb_entry) -> < <= > >= (oc)
The public.json container and public.jsonb_query domains carry no term
extractors — their surface is containment (@>, <@) and path navigation.
stevec_terms() now keys on the catalog domain name and returns terms only
for `entry`; json/query resolve to []. Tests assert the per-domain split.
Claude-Session: https://claude.ai/code/session_01CqDNqLSEEkCi7xAJFq7HJA
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
2 participants
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.
Stacked on #364. Closes #365 and #366 — both achieved through the catalog dump, no SQL autogen.
#365 — jsonb domains in the manifest
Turns out there's nothing to "move":
json/jsonb_entry/jsonb_queryare already ineql_domains::CATALOGasShape::SteVecentries. Their SQL is deliberately hand-written (src/v3/jsonb/) with performance-tuned CHECKs (validator functions, inline expressions — #354), and the catalog comment is explicit that "the SQL generator SKIPS these; the catalog drives only their inventory." So full SQL autogen isn't the design (and would fight the hand-tuning) — the gap was simply thatscalar_families()filters them out ofdump-catalog.Fix:
dump_catalog()now emits them as a new additivestevecfield onCatalogDump. Existing scalar-only consumers (the fixture-coverage task inmise.toml, the dump tests) read onlytypes[], so they're untouched. The manifest now carries all 51 domains (48 scalar + 3 jsonb).#366 — per-domain terms + extractors
Each
DomainEntrygains atermsfield (key+extractor+ctor, straight fromeql_domains::Term), so the manifest links a domain to its extractor functions:{ "segment": "ord", "supported_ops": ["=","<>","<","<=",">",">="], "terms": [{ "key": "ob", "extractor": "ord_term", "ctor": "ore_block_256" }] }The docs manifest maps this to
termFunctions(e.g.integer_ord→eql_v3.ord_term) — authoritative, and it resolves the docs drift-lint false-flags oneq_term/ord_term/match_term(real generated extractors that doxygen didn't capture cleanly).Safety
cargo test -p eql-codegen -p eql-domains— 89 + 91 pass, incl. new tests (stevec_jsonb_family_is_dumped,ordered_domain_exposes_its_extractor_and_ctor).doxygen → XML+dump-catalog→ 984 functions + 51 domains.Docs-side consumer updated in cipherstash/docs#46.
https://claude.ai/code/session_01CqDNqLSEEkCi7xAJFq7HJA