feat(rfc0016): query endpoint handler — POST /v1/query (.1-.4) - #283
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 49 minutes and 56 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. 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 (1)
📝 WalkthroughWalkthroughAdds the ChangesRFC0016 /v1/query querier endpoint
Sequence Diagram(s)sequenceDiagram
participant Client
participant query_handler as POST /v1/query
participant parse_body
participant apply_limit
participant Querier
Client->>query_handler: POST /v1/query<br/>(X-Ourios-Tenant, Content-Type, body)
query_handler->>query_handler: tenant_from_headers()
alt missing / empty tenant
query_handler-->>Client: 400 {error:{kind:"missing_tenant",...}}
end
query_handler->>parse_body: bytes + Content-Type
alt parse error
parse_body-->>query_handler: Err(dsl_message)
query_handler-->>Client: 400 {error:{kind:"dsl_error",...}}
end
query_handler->>apply_limit: clamp Limit stage to MAX_LIMIT
alt Logs statement
query_handler->>Querier: run_query(tenant, statement, now_ns)
else Drift statement
query_handler->>Querier: run_drift(tenant, statement, now_ns)
end
alt success
Querier-->>query_handler: Ok(QueryResult / DriftResult)
query_handler-->>Client: 200 {rows, records, stats}
else QueryError::Dsl / Validation
Querier-->>query_handler: Err(dsl/validation error)
query_handler-->>Client: 400 {error:{kind, message}}
else QueryError::Execution / Storage
Querier-->>query_handler: Err(engine error)
query_handler-->>Client: 500 {error:{kind:"internal",...}}
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Implements the RFC 0016 querier role’s HTTP transport layer: an in-process-testable Axum router() and a serve() wrapper that binds a listener and supports graceful shutdown, exposing POST /v1/query with tenant enforcement, content-type dispatch, and Ourios-owned JSON response/error DTOs.
Changes:
- Add
ourios_server::querier::{router, serve, QuerierConfig, QuerierHandle}withPOST /v1/queryhandler (tenant header,text/plainvsapplication/jsonparsing, limit clamping, error mapping). - Convert RFC0016
.1–.4integration tests from ignored stubs intotower::oneshotrouter-driven green tests using a real RFC 0005 store. - Introduce a library target (
src/lib.rs) and add dependencies (ourios-querier,serde(_json),tower).
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/ourios-server/tests/rfc0016_query_endpoint.rs | Turns RFC0016 .1–.4 into end-to-end in-process handler tests (store fixtures + oneshot requests). |
| crates/ourios-server/src/querier.rs | New querier role implementation: router, server lifecycle, request parsing, query dispatch, and JSON DTOs/errors. |
| crates/ourios-server/src/lib.rs | Adds ourios-server library surface exporting pub mod querier for in-process tests/reuse. |
| crates/ourios-server/Cargo.toml | Adds querier/serde/tower dependencies needed for the new HTTP query endpoint + tests. |
| Cargo.lock | Locks new transitive dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
crates/ourios-server/tests/rfc0016_query_endpoint.rs (1)
127-130: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winCap response body size in the in-process
posthelper.Using
usize::MAXhere makes test runs vulnerable to unbounded buffering if a regression returns an unexpectedly large payload. A fixed upper bound fails fast and keeps CI stable.Proposed patch
@@ const TS0: u64 = 1_775_127_480_000_000_000; /// A default window so wide that fixed-past fixtures always fall inside the /// no-`range` look-back (≈100 years of nanos, well under `u64::MAX`). const HUGE_WINDOW: u64 = 100 * 365 * 24 * 60 * 60 * 1_000_000_000; +const MAX_TEST_RESPONSE_BYTES: usize = 2 * 1024 * 1024; @@ - let bytes = to_bytes(response.into_body(), usize::MAX) + let bytes = to_bytes(response.into_body(), MAX_TEST_RESPONSE_BYTES) .await .expect("read body");🤖 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/ourios-server/tests/rfc0016_query_endpoint.rs` around lines 127 - 130, The to_bytes call in the post helper is using usize::MAX as the buffer limit, which can cause unbounded memory buffering if a regression returns an unexpectedly large response payload. Replace the usize::MAX argument in the to_bytes invocation with a reasonable fixed upper bound value that will fail fast and keep tests stable. Choose a limit appropriate for the expected response size in your tests, such as a specific megabyte value or a defined constant.
🤖 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-server/src/querier.rs`:
- Around line 195-255: Add a `#[cfg(test)]` module at the end of the file to
include unit tests for the three core API contract helpers:
`tenant_from_headers`, `parse_body`, and `apply_limit`. For
`tenant_from_headers`, test valid tenant IDs, missing headers, non-UTF-8 values,
and empty strings. For `parse_body`, test the different Content-Type scenarios
(text/plain, application/json with query wrapper, and structured-IR JSON), UTF-8
validation, and error cases. For `apply_limit`, test scenarios with existing
limits, missing limits using defaults, clamping to cap, and ensure exactly one
Limit stage remains in the output.
- Around line 103-193: The query request path (router, serve, and handle_query
functions) lacks observability integration required by coding guidelines. Add
Prometheus metrics to track query operations in the router function, instrument
the serve function with structured Ourios logs for startup and shutdown events,
and add comprehensive observability to the handle_query function including
metric counters for successful and failed queries, structured Ourios logs for
query execution with tenant and statement details, and distributed tracing spans
that wrap the entire request handling to trace both the Logs and Drift statement
execution paths.
In `@crates/ourios-server/tests/rfc0016_query_endpoint.rs`:
- Around line 169-181: The assertion in the post request block only validates
that one row is returned when scoped to the acme tenant, but does not verify
that the returned row actually belongs to the acme tenant rather than the other
tenant. Add an additional assertion that checks a tenant-identifying field in
the returned response data (accessed through json["rows"] or the response
structure) to confirm the row belongs to acme and not other, ensuring the
tenant-scoping filter is working correctly and not just truncating results.
---
Nitpick comments:
In `@crates/ourios-server/tests/rfc0016_query_endpoint.rs`:
- Around line 127-130: The to_bytes call in the post helper is using usize::MAX
as the buffer limit, which can cause unbounded memory buffering if a regression
returns an unexpectedly large response payload. Replace the usize::MAX argument
in the to_bytes invocation with a reasonable fixed upper bound value that will
fail fast and keep tests stable. Choose a limit appropriate for the expected
response size in your tests, such as a specific megabyte value or a defined
constant.
🪄 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: e9660fdf-c269-471b-9873-40a6c9d1d58f
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
crates/ourios-server/Cargo.tomlcrates/ourios-server/src/lib.rscrates/ourios-server/src/querier.rscrates/ourios-server/tests/rfc0016_query_endpoint.rs
The querier role's transport + dispatch (RFC 0016 §3.2-§3.5), green for the
handler scenarios. Adds a [lib] target so serve/router are testable
in-process; main.rs wiring (env-gating, compose) + OTel metrics are the
next slices.
- querier::serve(QuerierConfig) -> QuerierHandle on the receiver's
bind + axum::serve(...).with_graceful_shutdown(watch) topology; router()
split out for in-process (tower oneshot) tests.
- POST /v1/query: required X-Ourios-Tenant header (missing/empty → 400
before the engine); Content-Type dispatch (text/plain → parse_statement;
application/json → {"query":…} wrapper or structured-IR JSON);
Statement::Logs → run_query, Drift → run_drift; now = wall clock,
server-supplied default window.
- 200 JSON: Ourios-owned response DTOs (no engine type crosses — H6).
Attributes/structured bodies encoded as proto3-JSON via ourios-core's
canonical codec (no opentelemetry-proto serde feature); rendered line as
UTF-8 text; trace/span ids hex. The query's limit caps records only and
is clamped to MAX_LIMIT; a query with none gets DEFAULT_LIMIT so the
endpoint returns rows (RFC 0016 §7).
- Error model (H6): DSL/compile failure → 400 {error:{kind,message}};
Storage → 500 with the engine's already-scrubbed Display.
Fills RFC0016.1/.2/.3/.4 (router via tower oneshot over a real store);
.5/.6/.7 stay #[ignore]d for the main-wiring + observability slices.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review feedback on the query endpoint handler:
- QueryWrapper is now #[serde(deny_unknown_fields)] so a structured-IR
object (predicate/stages) can't be mis-dispatched as the {"query":…}
wrapper (Copilot).
- Correct the reconstruction-marker comment to match the wire value
"retained_verbatim" (Copilot).
- Add colocated #[cfg(test)] unit tests for tenant_from_headers /
apply_limit / parse_body (text-plain + JSON wrapper + structured-IR +
malformed), and a handler test for both application/json request modes
(Copilot + CodeRabbit).
- Strengthen the tenant-isolation test to be tenant-identifying: acme has
template 1, other has template 2; querying template_id==2 as acme
returns 0 rows (proves isolation, not truncation) (CodeRabbit).
Read-path metrics / structured logs / tracing (CodeRabbit) are deferred to
the RFC0016.6 observability slice (the querier has no metric surface yet).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jensholdgaard
force-pushed
the
rfc0016-green-handler
branch
from
June 22, 2026 12:11
ce6c286 to
46b5492
Compare
- Apply DefaultBodyLimit (MAX_BODY_BYTES = 1 MiB) to the query router so an oversized POST body is rejected 413 rather than read whole into memory via the Bytes extractor (Copilot — DoS). + an oversize-body test. - apply_limit now clamps the effective (last) Limit stage in place instead of removing all Limit stages and re-appending, which reordered the pipeline; stage order is DSL semantics (RFC 0002) and reordering is a footgun once more stages execute (Copilot). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Tests no longer hard-code a 2026 timestamp: fixtures are stamped an hour before the request's now (recent_ns), so the no-range look-back window always covers them regardless of the machine's wall clock (Copilot). - apply_limit now normalizes to exactly one Limit (the clamped last, in its position relative to non-limit stages; earlier shadowed limits dropped) — matching its doc and removing the multi-limit footgun. Added a multi-limit unit case (Copilot). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot: apply_limit's no-existing-limit branch appended `default` without clamping, so a misconfigured `default > cap` would violate the 'no larger than cap' contract. Append `default.min(cap)`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Match `&stages[last]` and copy the `u64` out of `Stage::Limit` rather than matching `stages[last]` by value. The prior form compiled (binding a `Copy` field is a partial copy, not a move of the non-`Copy` `Stage`), but the borrow makes the no-move intent explicit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
RFC 0016 green — query endpoint handler (
.1–.4)The querier role's transport + dispatch (RFC 0016 §3.2–§3.5). Its RFC 0017 dependency (the typed-row payload) is green in main, so this returns actual rendered rows.
Changes
[lib]target (src/lib.rs) exposingpub mod queriersoserve/routerare testable in-process; the binary (main.rs) is unchanged this slice.querier::serve(QuerierConfig) → QuerierHandleon the receiver'sbind → axum::serve(...).with_graceful_shutdown(watch)topology (RFC 0003 mirror);router()split out fortower::oneshottests.POST /v1/query: requiredX-Ourios-Tenant(missing/empty →400before the engine);Content-Typedispatch (text/plain→parse_statement;application/json→{"query":…}wrapper or structured-IR JSON);Statement::Logs → run_query,Drift → run_drift;now= wall clock + server default window.200JSON Ourios-owned response DTOs — no engine type crosses (H6). Attributes / structured bodies are proto3-JSON viaourios-core's canonical codec (noopentelemetry-protoserde feature); rendered line as UTF-8 text; trace/span ids hex.limitcapsrecordsand is clamped toMAX_LIMIT; a query with none getsDEFAULT_LIMITso the endpoint returns rows by design (RFC 0017 only populatesrecordswhen a limit is present).400 {error:{kind,message}};Storage→500with the engine's already-scrubbedDisplay.Tests
router+tower::oneshotover a real RFC 0005 store (logs end-to-end, tenant scoping + no-header-400, drift routing, malformed→400 with an H6 denylist guard).#[ignore]d — next slices:main.rsenv-gating + compose (.5/.7), OTel query metrics (.6, will consult the OpenTelemetry MCP for semconv naming).fmt + clippy
-D warningsclean across the server crate.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
POST /v1/query) for Logs and Drift queries with JSON responses.X-Ourios-Tenantheader.{"query": ...}wrapper, and structured JSON).Tests