feat(server): RFC 0032 green (correctness + descriptions) — .3/.4/.5/.6 - #519
Conversation
The remaining four §5 scenarios of the query-schema / cost-model resource: - RFC0032.3 severity correctness: the document's bands are asserted against `SeverityName::floor`/`ceil` (the DSL compiler's functions, never repeated literals) — a unit test beside the builder in `mcp.rs` (§6) plus a served-surface arm in the §5 map file. - RFC0032.4 cost-tier stability: the unit test writes a real Parquet file with the same `PromotedAttributes` value and harvests the bloom-filtered column set from the footer; the document's `mechanism: "bloom"` kinds, placeholders expanded over its own promoted section, must cover exactly that set. Severity's entry is `statistics`, never `bloom`, and a recursive walk pins the only numeric leaves to `format_version` and the severity bands (structure, never numbers). New dev-deps `parquet`/`bytes` for the footer read — both already in the tree via ourios-parquet. - RFC0032.5 tool descriptions: each RFC 0027 tool gains exactly one advisory sentence naming `ourios://query-schema` (§3.3 / the #465 placement rule); the test pins the URI to one occurrence per description and keeps tier vocabulary, severity-band numerals, and promoted keys out. `query_logs`'s stats phrase rewords to "row-group pruning stats" so the tier-token pin holds. - RFC0032.6 read-only contract: the RFC 0027 suite runs unmodified in the same binary; the new arm pins the rest — a populated and an empty store serve identical resource bytes (configuration-derived, no query, no tenant data), and an unknown URI still answers the JSON-RPC resource-not-found error. RFC text (separate concern, same PR): §5.6 now reads "passes with the §3.1 two-resource amendment applied" and records the one relocated assertion (`rfc0027_6_grammar_resource`'s exactly-one count moved to RFC0032.1's exactly-two); §3.4's "verbatim" aligned to match. Invariants/hazards: read-only MCP surface only — no pillar, ingest, or schema change. Hazard §4.6 (DSL vs engine leakage) is preserved: the resource keeps describing the DSL field surface, and the new tests pin the document to the DSL compiler and the writer rather than literals. All six RFC0032 scenarios green; zero remaining rfc0032 ignores. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR adds schema and parquet invariant tests, replaces ignored RFC0032.3–.6 integration stubs with MCP end-to-end checks, updates MCP tool documentation, adds test dependencies, and clarifies RFC acceptance criteria. ChangesQuery schema validation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Pull request overview
Completes RFC 0032’s final green slice by turning the remaining RFC0032.3–.6 stubs into concrete assertions, tightening the anti-drift guarantees between the served MCP ourios://query-schema resource, the DSL compiler severity mapping, and the Parquet writer’s actual bloom-filter behavior; additionally aligns the RFC text to the already-landed two-resource amendment.
Changes:
- Implements RFC0032.3/.4 served-surface integration assertions and adds unit tests in
mcp.rsthat pin severity bands toSeverityName::floor/ceiland bloom-backed fields to a bloom set harvested from a real Parquet footer. - Enforces RFC0032.5 tool-description placement rules (exactly one
ourios://query-schemapointer; no tier/severity/promoted-key leakage into descriptions) and strengthens RFC0032.6 read-only/tenant-independence properties. - Updates RFC 0032 text to reflect the §3.1 two-resource amendment as the only permitted change to the RFC 0027 §5 suite expectations.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| docs/rfcs/0032-query-schema-cost-model-resource.md | Aligns §3.4 and §5.6 wording with the already-required two-resource amendment and clarifies the relocated count assertion. |
| crates/ourios-server/tests/it/rfc0032_query_schema.rs | Converts RFC0032.3–.6 stubs into end-to-end MCP integration tests, covering severity bands, bloom classification expansion, tool description constraints, and read-only behavior. |
| crates/ourios-server/src/mcp.rs | Updates tool docstrings to include the schema-resource pointer and adds unit tests that anti-drift pin the document to compiler and writer behavior via real Parquet footer inspection. |
| crates/ourios-server/Cargo.toml | Adds dev-dependencies needed to read Parquet footers in tests (parquet, bytes). |
| Cargo.lock | Lockfile update to reflect the new direct dev-dependency edges. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/mcp.rs`:
- Around line 384-387: Update the documentation immediately above template_drift
to describe the returned content as “drift data” rather than “template text,”
while retaining the instruction to treat it strictly as data and never as
instructions.
In `@crates/ourios-server/tests/it/rfc0032_query_schema.rs`:
- Around line 449-452: Update the assertion in the query-schema test to retain
the original contents[0].text strings returned by read_query_schema before JSON
parsing, then compare those raw strings for exact byte identity between
populated_doc and empty_router. Avoid comparing parsed serde_json::Value values
for this check.
- Around line 367-373: Update the test’s router_with_mcp setup to pass an
explicit promoted-key configuration including k8s.namespace.name and http.route
instead of relying on defaults. Extend the related schema assertions to verify
that each configured promoted key is absent, including the assertion block also
covering lines 415–419, while preserving the existing service.name check.
🪄 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: c5b9a944-8242-44f9-8b51-a70a4444001c
⛔ 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/mcp.rscrates/ourios-server/tests/it/rfc0032_query_schema.rsdocs/rfcs/0032-query-schema-cost-model-resource.md
…bytes in .6 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y
faa66bb to
7410520
Compare
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y
…ny gate Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y
.1/.2 resource + config threading (#518), .3/.4 document correctness, .5 tool-description placement, .6 read-only contract (#519). All scenarios are local tests, green on main — no pending measurement gates this flip. Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
What
RFC 0032's final green slice — RFC0032.3/.4/.5/.6, completing all six §5 scenarios.
SeverityName::floor()/ceil()(the DSL functions, never literals), as a unit test beside the builder plus a served-surface mirror arm.PromotedAttributes, the bloom-filtered column set is harvested from the footer, and the document'smechanism: "bloom"classifications must cover exactly that harvested set. Severity carries no bloom and classifiesstatistics. A recursive walk pins the document's only numeric leaves toformat_version+ severity bands — no cost numbers can creep in.ourios://query-schemaURI exactly once, and no tier token, digit, or promoted key appears in any description (the MCP query surface: expose the stored-log query schema as a resource (RFC 0027 amendment) #465 placement rule, machine-checked).query_logs's pre-existing "scanned/pruned" phrase reworded to "row-group pruning stats" so the tier-token pin holds; RFC0027.7 still passes.-32002, and the RFC 0027 suite passes unmodified in the same binary.RFC text alignment riding along (intrinsic to .6): §5.6's "verbatim" wording now states the two-resource amendment and names the one relocated count assertion; §3.4's cross-referencing sentence aligned to match.
With this merged, all six scenarios are discharged — the status flip red→green follows as its own PR.
Checks run
cargo fmt --all --check; workspacecargo clippy --all-targets --all-features -- -D warnings(exit checked unmasked); strict rustdoc;cargo nextest run -p ourios-server— 116 passed, 9 skipped (all pre-existing env-gated); full workspace nextest 1089 passed;mdbook buildclean. Re-verified after rebasing onto the merged #518.🤖 Generated with Claude Code
https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y
Summary by CodeRabbit
Documentation
Tests
ourios://query-schemaJSON surface across scenarios .3–.6.