docs(rfc-0002): specify the query DSL — Branch B, surface β - #143
Merged
Conversation
Resolves the §3 prior decision (distance from OTTL) on the β (pipe-composable) surface, per the audience analysis (primary: Perses dashboard authors; future: MCP agents) — §3.5/§3.6. Advances RFC 0002 drafted → specified. Adds the design (§6): Branch-B predicate grammar over the OTel data model (bare top-level fields, resource./attr. prefixes, bare-identifier severity), first-class OTel-canonical fields (service/trace_id/span_id/ scope) + template primitives, the β pipe surface, and the two-front-ends- one-core model (string DSL for humans + a structured JSON surface for MCP agents, both compiling to one IR → DataFusion LogicalPlan, no SQL leakage). Adds the owned EBNF grammar (§7), 11 greppable RFC0002.x acceptance criteria (§5), the testing strategy (§8), and narrows the open questions (§9) — surface, severity casing, agent-friendliness, and first-class fields now resolved. The RFC 0007 execution layer this DSL targets is already green; this is the user-facing language in front of it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRFC 0002 moves from a draft decision tree to specified status, committing to Branch B predicates and a β pipe-composable surface, adding concrete acceptance criteria, a full design with EBNF and DataFusion LogicalPlan mapping (Ourios extensions), updated tests, and narrowed open questions. ChangesQuery DSL RFC Specification (Branch B + β)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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 |
![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 `@docs/rfcs/0002-query-dsl.md`:
- Around line 303-305: The markdown uses unlabeled fenced code blocks for query
examples (e.g., the blocks containing `service == "api" and severity >= error
and attr.http.status_code == 500` and the multi-line query starting `service ==
"api" and severity >= error | range(-1h, now) | count by template_id ...`);
update each unlabeled triple-backtick fence to include a language identifier
(use "text" or a suitable label) for the blocks at the three occurrences so they
satisfy MD040 and render with correct syntax highlighting.
🪄 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: 4309b6d8-9839-411b-a76d-1ba8d165456e
📒 Files selected for processing (1)
docs/rfcs/0002-query-dsl.md
Contributor
There was a problem hiding this comment.
Pull request overview
This PR advances RFC 0002 (Query DSL) from drafted → specified by turning the prior decision tree into a concrete specification for Branch B (Ourios-native predicates, distanced from OTTL) on the β (pipe-composable) query surface, including acceptance criteria, design details, and an owned EBNF grammar.
Changes:
- Updates RFC 0002 frontmatter/status and rewrites the document to a specified, testable spec (decision record + design).
- Adds §5 acceptance criteria (RFC0002.1–.11) and a concrete compilation model (“two front-ends, one core”).
- Introduces an owned EBNF grammar (§7) and an updated testing strategy aligned with the repo’s red→green process.
Comments suppressed due to low confidence (1)
docs/rfcs/0002-query-dsl.md:521
- There is an extra stray closing code fence (```), which leaves the markdown in an unterminated code block and will break rendering in mdBook/GitHub. It looks like a leftover from the diff snippet and should be removed.
</details>
---
💡 <a href="/jensholdgaard/ourios/new/main?filename=.github/instructions/*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.
Review fixes: RFC0002.1 no longer claims service.name pushdown (RFC 0005 §3.6 indexes only template_id/time/severity_text) — non-indexed predicates compile to a correct Filter with no pruning claim. The §7 EBNF now defines field_list/sort_key/value/time/integer and a severity_name production, so it derives the documented examples (severity >= error, sort count desc). Label two fenced blocks (MD040) and drop a stray trailing code fence. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review pass 2: RFC0002.1 no longer ties severity pushdown to severity_text or to QueryRequest-equivalence (the DSL resolves severity to a severity_number range per §6.2/RFC0002.5 that the current request cannot express) — it now scopes equivalence to template+time and defers severity to RFC 0007 §4.3. §6.2 now maps ts -> time_unix_nano and observed_ts -> observed_time_unix_nano. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…predicate Review pass 3: specify trace_id/span_id literals as lowercase-hex strings (hex-decoded to the byte columns, OTLP/JSON convention); map lossy -> the lossy_flag column explicitly; and defer the bare drift predicate (RFC 0001 6.7 drift is an audit-stream property, not an RFC 0005 data column, so it needs a future audit-stream query path) — the alias query is already served by resolves_to. drift removed from the field list + EBNF. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review pass 4 (internal consistency): drop drift from the §3.3 shared list and principle #4 (it is deferred per §6.3; drift-alias is resolves_to); fix the §6.2 hex wording (hex strings parsed case-insensitively, canonical lowercase — was self-contradictory); scope severity_name to a severity RHS in the §7 EBNF (split comparison into severity_cmp | scalar_cmp so `service == error` is not grammatical); cite CLAUDE.md §4.6 in principle #2. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…alify citations Review pass 5: scalar_cmp now uses scalar_path (= nonsev_field | resource/ attr), so severity is reachable only via severity_cmp — severity == \"error\" / severity == null are ungrammatical in §7, not just a semantic error (field = nonsev_field | severity is retained for field_list/sort_key). Qualify the two remaining bare hazard citations as CLAUDE.md §4.6 (§10) and CLAUDE.md §3.7 (references). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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 `@docs/rfcs/0002-query-dsl.md`:
- Line 190: Replace ambiguous subsection citations "[§4.6]", "[§4.5]", and
"[§4.7]" with resolvable references in the RFC text: find occurrences near the
header "RFC0002.1 — A Branch-B predicate parses and compiles to a filter" (and
the other two occurrences) and change them to either explicit anchors (e.g.,
"see §4 — Principle 6" or an HTML/Markdown anchor link to the exact principle)
or a phrase like "Principle 6 in §4" so the references resolve even though
Section 4 has no sub-sections; update all three tokens ("[§4.6]", "[§4.5]",
"[§4.7]") consistently.
🪄 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: a6ec07ca-2115-4d60-8604-5283aebb4480
📒 Files selected for processing (1)
docs/rfcs/0002-query-dsl.md
Review pass 6: §4 is a flat principle list (no subsections), so the acceptance-criteria tags now read [CLAUDE.md §4.6] (the no-leakage hazard, RFC0002.1/.3) and [§4 P5]/[§4 P7] (this RFC design principles 5/7, RFC0002.4/.10). The §6.4 sort synopsis now says sort <field-or-aggregate> to match the §7 sort_key grammar + the sort count desc example. The time production parenthesises ([ - ] , duration) so the alternation precedence is unambiguous across EBNF dialects (matters for constrained decoding). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…perand Review pass 17: removed an orphaned closing paren in the §6.4 call-node sentence; §6.1 now states the string functions (matches/contains/ starts_with/ends_with) require a string operand — a non-string path (severity, numeric/bool attr, lossy, ts) is a compile-time type error (RFC0002.8), not a silent coercion. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… syntax
Review pass 18: §6.4 range synopsis lists the now keyword (the §7 time
form), not just durations/RFC3339. The structured surface field is now
structured — a bare top-level name string or { resource: key } / { attr:
key } — so agents never build or escape DSL path syntax (resource[...]),
realising the agents-emit-JSON-not-syntax goal; call-node args use the
same structured field.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e; single-line β Review pass 19 (10 comments): cite the no-leakage hazard as CLAUDE.md §4 hazard 6 (the hazards are a numbered list under §4, no §4.6 heading); severity_cmp uses ord_op (no regex =~/!~ on numeric severity); add a bool_lit predicate atom + structured const node so an unfiltered (match-all true) query is expressible on both surfaces; show the β example in canonical single-line form. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…citation Review pass 20: status note now names validated then accepted (per the README ladder) after §9 validation, not green→accepted directly; §7 lexical note defines digit + letter (self-contained grammar); the §9 resolution cites the OTTL README + OTel logs spec "linked in §11 References" (not implying the OTel spec has a §11). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…at to verification.md Review pass 21: resource[...]/attr[...] now described as needed for any non-bare-identifier key (chars outside the set, digit-leading segment, reserved-word collision), not just out-of-set characters; the §5 preface now cites the docs/verification.md §2.2 doc-comment form (/// Scenario RFC0002.<n> — <title>. + /// See ...). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment on lines
+431
to
+434
| `{ "resource": "<key>" }` / `{ "attr": "<key>" }` (`<key>` the raw OTel | ||
| attribute key, e.g. `"k8s.pod.name"`). An **`op`** is a §7 `cmp_op` | ||
| string (`"=="`, `">="`, `"=~"`, …); a **`value`** is a JSON | ||
| primitive (string / number / bool / null), with durations and timestamps |
Comment on lines
+525
to
+526
| field_list = field , { "," , field } ; | ||
| sort_key = field | ident ; (* ident = an aggregate output, e.g. count *) |
jensholdgaard
added a commit
that referenced
this pull request
Jun 7, 2026
…e blocker Review: RFC 0002 status note now credits #143 (the spec PR) alongside #144-#154; RFC 0007 §8 + the alternatives no longer say parser integration is blocked on the RFC 0002 §3 branch decision (resolved, Branch B) — the contradiction with the green status note is removed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jensholdgaard
added a commit
that referenced
this pull request
Jun 7, 2026
RFC 0002 (query DSL) is 11/11 green (#143-#154) and RFC 0007 (querier execution frontend) RFC0007.1-.5 are all live + passing; the prove-thesis gate is cleared. RFC 0001 deliberately stays specified (its miner criteria .5/.6/.8/.9/.10/.11 + hazards/invariants remain red-gate stubs). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jun 9, 2026
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
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.
What
Advances RFC 0002
drafted → specifiedfor the decision you made: Branch B (distance from OTTL) on the β (pipe-composable) surface. Doc-only.It rewrites the RFC from a decision-tree into a spec:
resource./attr.prefixes,severity >= error), first-class OTel-canonical fields (service/trace_id/span_id/scope) + template primitives, the β pipe surface, and the two-front-ends-one-core model (string DSL for humans + a structured JSON surface for MCP agents → one IR → DataFusionLogicalPlan, no SQL leakage). Includes a Mermaid compile-flow diagram.RFC0002.xacceptance criteria (Given/When/Then), including string≡structured equivalence (.2), no-leakage (.3), default time window (.4), YAML-embeddability (.10), structured-schema validation (.11).The RFC 0007 execution layer this targets is already
green; this specifies the language in front of it.Status / process
This is a spec advancement to your RFC — like the §9 amendment (#139), I'm holding the merge for your review rather than self-merging. I'll address any Copilot/CodeRabbit prose nits to keep it clean. Once you approve (and optionally run the §9 OTel-AI questions), merge it and I'll drive RFC 0002
specified → red → green(parser + compiler against the proven execution layer).🤖 Generated with Claude Code
Summary by CodeRabbit