feat(querier): RFC 0002 green — L4 validation error contract (.14)

[jensholdgaard]

What

The grammar/compile-validation green slice for RFC 0002's L4 amendment — discharges RFC0002.14, the last open scenario in the amendment (execution landed in #533).

• Cases (i)–(iii) (unpinned param(n), a pin buried under or/not, resolves_to as a non-pin) were already enforced by validate_group_terms/pinned_template_id — now pinned end-to-end via run_query: error class QueryError::InvalidQuery, message names param(0)/"pin"/"template", asserted leak-free against the RFC0002.8 engine-token set. Each case is also asserted to parse successfully first, proving the rejection is semantic, not grammatical.
• Case (iv) (param(n)/bucket(w) outside a by-list) needed real enforcement — it previously fell through to a generic "unknown field" error that didn't cite the grammar, which the scenario's Then requires. Fixed in the parser: group terms are now rejected with a message citing "grammar §7 v1.1" across six contexts (predicate paths, project fields, inside matches(...), as a sum(...) path).

Checks run

cargo fmt --all --check; workspace cargo clippy --all-targets --all-features -- -D warnings; cargo nextest run (full workspace) — 1116 passed, 36 skipped, rebased onto the merged #533.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

Summary by CodeRabbit

• Bug Fixes

  • Improved validation for param(...) and bucket(...) expressions used outside permitted by lists.
  • Added clearer, grammar-specific error messages identifying invalid placement and relevant syntax.
  • Ensured invalid grouping expressions produce safe, product-owned errors without exposing underlying engine or SQL details.

• Tests

  • Expanded coverage for invalid param(...) and bucket(...) usage, including compile-time and parse-time validation scenarios.

[coderabbitai]

📝 Walkthrough

Walkthrough

The DSL parser now rejects param(...) and bucket(...) outside by lists with dedicated grammar errors. RFC0002.14 integration coverage validates compile-time and parse-time misuse, error fragments, and leak-free diagnostics.

Changes

Group-term validation

Layer / File(s)	Summary
Parser group-term diagnostics crates/ourios-querier/src/dsl/parse.rs	param(...) and bucket(...) are rejected outside aggregation by lists through a shared grammar-specific diagnostic.
RFC0002 misuse validation crates/ourios-querier/tests/it/rfc0002_dsl.rs	RFC0002.14 now tests semantic and parse-time misuse, required error fragments, leak-free messages, and updated RFC status documentation.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• jensholdgaard/ourios#145: Introduced the DSL string parser extended by these group-term validation changes.
• jensholdgaard/ourios#533: Added related RFC0002 count by grammar constraints and param/bucket misuse tests.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately reflects the RFC 0002 L4 validation green slice and error-contract work.
Description check	✅ Passed	The description explains the change and includes checks run, though it does not follow the exact template sections.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch rfc0002-l4-green-validation

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Pull request overview

Implements the RFC 0002 amendment “green slice” for L4 validation/grammar error contracts (RFC0002.14), ensuring param(n)/bucket(w) misuse produces specific, leak-free errors and that semantic vs grammatical failures are distinguished.

Changes:

• Replaces the RFC0002.14 ignored stub with an end-to-end integration test asserting correct error class/message and token leak-freedom.
• Updates the DSL parser to reject param(…)/bucket(…) when used outside by-lists with a grammar-citing error message.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
crates/ourios-querier/tests/it/rfc0002_dsl.rs	Turns RFC0002.14 into a real async test covering compile-time vs parse-time misuse cases and leak-free error messages.
crates/ourios-querier/src/dsl/parse.rs	Adds targeted parse-time rejection for param(…)/bucket(…) outside by-lists with a specific grammar-based error.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

🧹 Nitpick comments (1)

crates/ourios-querier/src/dsl/parse.rs (1)

1133-1143: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a colocated unit test asserting the diagnostic's message content.

The existing rejects_malformed_group_terms test only checks is_err() for the by-list-outside cases; nothing in this file asserts that the message actually cites the grammar rule (that's currently only verified by the integration test in rfc0002_dsl.rs). A local assertion on err.message()/.to_string() containing e.g. "group_term" would guard the wording contract closer to the source.

As per coding guidelines: "Unit tests must be next to the code and are mandatory for anything non-trivial."

🤖 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-querier/src/dsl/parse.rs` around lines 1133 - 1143, Add a
colocated unit test near group_term_outside_by and rejects_malformed_group_terms
that exercises a malformed group term outside a by-list, asserts parsing returns
an error, and verifies err.message() or to_string() contains “group_term”. Keep
the assertion focused on the diagnostic wording contract.

Source: Coding guidelines

🤖 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.

Nitpick comments:
In `@crates/ourios-querier/src/dsl/parse.rs`:
- Around line 1133-1143: Add a colocated unit test near group_term_outside_by
and rejects_malformed_group_terms that exercises a malformed group term outside
a by-list, asserts parsing returns an error, and verifies err.message() or
to_string() contains “group_term”. Keep the assertion focused on the diagnostic
wording contract.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 04d4f0c9-ead2-477d-bd4e-723d5203858e

📥 Commits

Reviewing files that changed from the base of the PR and between b089313 and 4ac699e.

📒 Files selected for processing (2)

• crates/ourios-querier/src/dsl/parse.rs
• crates/ourios-querier/tests/it/rfc0002_dsl.rs

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.