feat(querier): yaml-embeddable queries + structured-surface JSON schema (RFC0002.10/.11) - #149
Conversation
…ma (RFC0002.10/.11) Flip the last two RFC 0002 acceptance stubs green, completing all 11 RFC0002.x scenarios. RFC0002.10 — the canonical β serialisation of any well-formed query is a single-line YAML-safe scalar (the §4 P7 Perses-embedding guarantee). The test embeds each serialised query as a `query:` YAML scalar, round-trips it through serde_yaml, and asserts the recovered scalar re-parses to the same IR — over both a 21-row hand-written table (predicates, severity, calls, escaped/special-char strings, range durations/timestamps, the full stage set) and the bounded RFC0002.7 `wellformed::query` generator (run on a large-stack thread; the generator keeps `prop_recursive` depth small so it never overflows CI's ~2 MiB test-thread stack). Asserts no literal newline in the serialised form. RFC0002.11 — publish a hand-written draft 2020-12 JSON Schema for the structured surface (§6.4) beside the parser, served by `dsl::structured_query_schema()` via `include_str!`. Snapshot-tested (the served schema equals the committed file and compiles as a JSON Schema, so drift is PR-visible like the §7 grammar snapshot). The schema is the gate: well-formed structured requests pass validation and parse; malformed ones (missing predicate, unknown key/field/op, non-primitive value, empty combinator, negative limit, unknown stage) are rejected by the schema before the planner. Validated with the `jsonschema` crate. Hand-written schema chosen over schemars derive: the Raw* types use untagged enums, `serde_json::Value` stage bodies, and a transparent single-tag stage map, so a derived schema would not faithfully capture the deny_unknown_fields / single-stage-tag constraints. The schema names no datafusion/arrow/SQL token (hazard CLAUDE.md §4.6); it is the DSL contract. Dev-deps: serde_yaml (YAML round-trip), jsonschema (no-default-features — drops the reqwest/TLS remote-$ref resolver; the schema is self-contained). RFC0002.9 left ignored (sibling slice owns it). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@coderabbitai review |
|
Warning Review limit reached
More reviews will be available in 31 minutes and 39 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds a JSON Schema definition for the Ourios structured query surface (RFC 0002), exposes it via public API, and implements two validation test suites: RFC0002.10 for YAML round-trip safety of canonical β serialization, and RFC0002.11 for schema-level acceptance of valid and rejection of invalid structured requests. ChangesRFC0002 Structured Query Schema and Validation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@crates/ourios-querier/src/dsl/structured_query.schema.json`:
- Around line 89-92: call_node.args is currently untyped ("items": true)
allowing invalid calls; change the schema so args are validated against the
called function's signature: replace "items": true with a constrained schema
that branches on the predicate.call value (use JSON Schema "if"/"then" or
"oneOf" with conditionals) and for each known call name (e.g., "contains",
"starts_with", etc.) specify the exact args array shape and item types (or $ref
to existing arg-type schemas). Ensure the rule that argument counts and types
match the function signatures (and include a fallback "then": false to reject
unknown signatures) so malformed inputs like
{"predicate":{"call":"contains","args":[1]}} will fail validation.
🪄 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: eef3cb64-45dd-45ee-b1ec-a8bf4becd1e1
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
crates/ourios-querier/Cargo.tomlcrates/ourios-querier/src/dsl/mod.rscrates/ourios-querier/src/dsl/structured_query.schema.jsoncrates/ourios-querier/tests/rfc0002_dsl.rs
There was a problem hiding this comment.
Pull request overview
Completes the final two acceptance scenarios for RFC 0002’s query DSL by (1) proving the canonical string serialization is YAML-embeddable as a single-line scalar, and (2) publishing + snapshot-testing a draft 2020-12 JSON Schema for the structured JSON query surface.
Changes:
- Implements RFC0002.10 acceptance test: YAML round-trip property for canonical query serialization (table-driven + proptest-generated).
- Adds a committed structured-surface JSON Schema and RFC0002.11 acceptance test that snapshots/compiles the schema and validates representative well-formed/malformed requests.
- Adds dev-dependencies needed for YAML round-tripping and JSON Schema validation.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/ourios-querier/tests/rfc0002_dsl.rs | Turns RFC0002.10/.11 red-gate stubs into passing acceptance tests (YAML round-trip + schema snapshot/validation). |
| crates/ourios-querier/src/dsl/structured_query.schema.json | Adds the published JSON Schema contract for the structured query surface. |
| crates/ourios-querier/src/dsl/mod.rs | Exposes the committed schema via structured_query_schema() using include_str!. |
| crates/ourios-querier/Cargo.toml | Adds test-only deps serde_yaml and jsonschema to support the new acceptance tests. |
| Cargo.lock | Locks new transitive dependencies introduced by the added dev-dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Close the gap where the published JSON Schema accepted requests
parse_structured rejects, weakening the RFC0002.11 schema-gate:
- call_node: split into a per-signature oneOf — the string predicates
take [<field>, <string>], resolves_to takes [<non-negative integer>].
- comparison_node: when field is severity, restrict op to the ordering
set and value to string|integer (no regex / non-string-or-int RHS).
- sort.key: require the bare-identifier pattern the parser enforces.
- render: accept null as well as {} for the argument-less body.
Extend the RFC0002.11 validation test with schema-level rejection cases
(severity-with-regex, wrong-arity resolves_to, non-string call arg,
non-identifier sort key) and a {"render":null} acceptance case; the
committed schema file is the regenerated snapshot.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| "then": { | ||
| "properties": { | ||
| "op": { "$ref": "#/$defs/ord_op" }, | ||
| "value": { "type": ["string", "integer"] } | ||
| } | ||
| } |
| ( | ||
| r#"{"predicate":{"field":"severity","op":"==","value":1.5}}"#, | ||
| "non-integer severity value", | ||
| ), | ||
| ( |
What
SLICE 4 (the last) of the RFC 0002 query DSL: flips the final two acceptance stubs green — RFC0002.10 (YAML-embeddability) and RFC0002.11 (structured-surface JSON Schema). After this merges, all 11 RFC0002.x scenarios are green (RFC0002.9 is owned by a sibling slice and left ignored here).
RFC0002.10 — YAML-embeddable single-line scalar (§4 P7)
The Perses-embedding guarantee: the canonical β serialisation of any well-formed query is a single-line scalar that survives a YAML round-trip and re-parses to the same query.
The test embeds each serialised query as a
query:YAML scalar value, round-trips it throughserde_yaml, extracts the scalar, and assertsparse(extracted) == original, over two sources:",\,\t,: , | # &),rangedurations/timestamps, and the full stage set;wellformed::querygenerator (the RFC0002.7 strategy) so the property holds for arbitrary §7-admissible shapes, not just the table.It also asserts the serialised form contains no literal newline. The property runs on a 32 MiB-stack thread (mirroring RFC0002.7) and the generator keeps
prop_recursivedepth small, so it never overflows CI's ~2 MiB default test-thread stack.Dev-dep:
serde_yaml.RFC0002.11 — published + snapshotted JSON Schema for the structured surface (§6.4)
A hand-written draft 2020-12 JSON Schema is committed beside the parser at
crates/ourios-querier/src/dsl/structured_query.schema.jsonand served by the newdsl::structured_query_schema() -> &'static str(viainclude_str!).The test does three things:
parse_structured.resource+attr, empty combinator, unknown function, negative limit, unknown stage kind — all rejected by the schema before the planner.Real validation uses the
jsonschemacrate so .11 tests the schema itself, not just the parser.Schema approach: hand-written, not schemars
The
structured.rsRaw*types useuntaggedenums,serde_json::Valuestage bodies, and atransparentsingle-tag stage map. Aschemars-derived schema would not faithfully capture thedeny_unknown_fields/ single-stage-tag /{resource|attr}one-of constraints, and would couple the published contract to serde-internal shapes. A hand-written schema is the cleaner, more faithful contract here.Hazard CLAUDE.md §4.6 (no DataFusion/arrow/SQL leakage)
The schema is the DSL contract, not a DataFusion artefact: it names no
datafusion/arrow/recordbatch/logicalplan/SQL token (verified). No engine type crosses any public signature —structured_query_schema()returns&'static str.jsonschemais added withdefault-features = falseto drop the reqwest/TLS remote-$refresolver; the schema is self-contained (local$refs only).Verification (reproduces CI)
cargo test --all-features— all green;rfc0002_dslshows 14 passed, 1 ignored (RFC0002.9, sibling slice).cargo fmt --all --check— clean.cargo clippy --all-targets --all-features -- -D warnings— clean.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests