test(querier): cover no-DataFusion/arrow/SQL-leakage boundary (RFC0007.3)

[jensholdgaard]

What

Flips RFC0007.3 — no DataFusion/arrow/SQL leakage (§4.6) from its #[ignore] red-gate stub to a live, two-level boundary test. Follows B1 (#88) and B2 (#89).

How it's tested

Per RFC 0007 §5/§6 ("compile-/string-level boundary test"):

• String level (colocated unit test rfc0007_3_storage_display_leaks_no_engine_tokens): a Storage error wrapping synthetic engine/arrow/SQL text is scrubbed to the generic Display "failed to read the log store", while Debug preserves the detail for logs. A denylist scan (datafusion, arrow, parquet, sql, select, schema, logical plan, physical, recordbatch, listingtable, during planning) rather than an exact-string match, so a future reword can't let a new engine token slip through.
• Real engine error (tests/boundary.rs::rfc0007_3_real_engine_error_does_not_leak): a corrupt committed *.parquet trips DataFusion schema inference deep in the engine; the surfaced QueryError::Storage Display leaks none of the denylist tokens — proving the scrub holds for errors flowing through storage_err, not just synthetic ones.

The "no engine type in a public signature" half is enforced structurally: QueryRequest/QueryResult/QueryStats/QueryError are all Ourios-owned, so no datafusion/arrow type crosses the public API.

Invariants / hazards

• §4.6 (no DataFusion/arrow/SQL leakage): this PR is the test for that hazard — no production code changed.

Verification (local)

• cargo fmt --all --check ✅
• cargo clippy -p ourios-querier --all-targets --all-features -- -D warnings ✅
• cargo test -p ourios-querier --all-features ✅ (4 lib unit + 1 boundary + 7 execution; only RFC0007.4 remains #[ignore])

RFC 0007 stays specified (RFC0007.4 still needs a test + RFC 0002). Epic: #82.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests
  • Added unit, boundary, and acceptance tests (plus a test helper) that verify operator-facing error messages scrub engine/SQL technical tokens while preserving detailed diagnostics in debug output; acceptance notes updated to mark the real-engine leakage checks as live tests.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 727f00f3-6049-4a88-814c-c28a94095040

📥 Commits

Reviewing files that changed from the base of the PR and between 6c99bf8 and f3d1280.

📒 Files selected for processing (3)

• crates/ourios-querier/src/lib.rs
• crates/ourios-querier/tests/acceptance.rs
• crates/ourios-querier/tests/boundary.rs

🚧 Files skipped from review as they are similar to previous changes (3)

• crates/ourios-querier/tests/boundary.rs
• crates/ourios-querier/src/lib.rs
• crates/ourios-querier/tests/acceptance.rs

---

📝 Walkthrough

Walkthrough

This PR implements RFC0007.3 compliance tests that validate QueryError::Storage's Display output does not leak engine/SQL details while preserving underlying information in Debug. A denylist constant and two test checkpoints (unit test with synthetic errors, integration test with real DataFusion failure) verify the boundary. Test documentation reflects the LIVE status.

Changes

RFC0007.3 no-leakage boundary validation

Layer / File(s)	Summary
Denylist constant and unit test for synthetic errors crates/ourios-querier/src/lib.rs	ENGINE_LEAK_TOKENS denylist and unit test rfc0007_3_storage_display_leaks_no_engine_tokens verify that QueryError::Storage Display output is scrubbed of engine tokens while Debug retains detail.
Integration test with real engine error crates/ourios-querier/tests/boundary.rs	Integration test rfc0007_3_real_engine_error_does_not_leak writes a corrupt committed parquet, triggers a real DataFusion schema-inference failure via Querier::run, and asserts Display output matches "failed to read the log store" (no engine tokens) while Debug preserves detail.
Acceptance test documentation update crates/ourios-querier/tests/acceptance.rs	RFC0007.3 test notes updated from ignored stub to LIVE, enumerating the two concrete test locations and restating enforcement that public signatures leak no engine types.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• jensholdgaard/ourios#86: Introduces the QueryError/Display red-gate setup and RFC0007 test scaffolding that this PR extends with concrete denylist-based boundary tests.
• jensholdgaard/ourios#87: Modifies Querier error handling to surface DataFusion/storage failures as QueryError::Storage with scrubbed Display output, which this PR validates through integration testing.

Poem

🐰 A boundary so clear, a secret kept tight,
Engine tokens hidden from the operator's sight,
Debug whispers truth when developers need to see,
Display keeps it clean—RFC0007.3!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title accurately describes the main change: adding a test for RFC0007.3's no-leakage boundary coverage, matching the test-focused modifications across three files.
Description check	✅ Passed	The PR description covers all required template sections including summary, relation to RFC0007.3, testing approach, local verification, and checklist completion.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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 feat/querier-no-leakage-test

---

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

[Copilot]

Pull request overview

Flips the RFC0007.3 red-gate stub into a live two-level boundary test, verifying that DataFusion/arrow/SQL specifics never leak into operator-facing QueryError::Display. No production code changes.

Changes:

• Adds a unit test in lib.rs that asserts a synthetic engine-text Storage error is scrubbed from Display (denylist scan) while preserved in Debug.
• Adds an integration test (tests/boundary.rs) that drives a real DataFusion failure via a corrupt committed *.parquet and verifies the surfaced QueryError::Storage leaks none of the denylist tokens.
• Replaces the #[ignore] placeholder in tests/acceptance.rs with a comment pointer to the now-live tests and notes the structural enforcement for the public signature half.

Reviewed changes

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

File	Description
crates/ourios-querier/src/lib.rs	Adds string-level unit test + shared ENGINE_LEAK_TOKENS denylist verifying Storage Display scrubbing.
crates/ourios-querier/tests/boundary.rs	New integration test triggering a real engine failure and asserting no engine-token leakage in Display.
crates/ourios-querier/tests/acceptance.rs	Removes the RFC0007.3 ignored stub and documents where the live coverage now lives.

---

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

[jensholdgaard]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.