Skip to content

refactor: consolidate the parse+schema+build query idiom#74

Merged
bhekanik merged 1 commit into
mainfrom
refactor/shared-query-builder
Jun 11, 2026
Merged

refactor: consolidate the parse+schema+build query idiom#74
bhekanik merged 1 commit into
mainfrom
refactor/shared-query-builder

Conversation

@bhekanik

@bhekanik bhekanik commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

What

Consolidates the repeated parse_queryMxrSchema::buildQueryBuilder::new(&schema).build(&ast) idiom into mxr_search::CompiledQuery + a build_query(&str) convenience. Audit backlog item P2 #20. Off main; behavior-preserving.

Why

Several daemon search sites (batch-mutation address loop, archive_ask) repeated the same three-step query compilation. Any change to the path meant touching every caller.

How

  • CompiledQuery::parse(query) parses once and captures the schema; build() yields a fresh Box<dyn Query> per call (paging loops consume the query each page, so they need a fresh one without rebuilding the schema).
  • build_query(&str) is the single-use convenience.
  • mutations.rs batch loop → CompiledQuery (preserves its parse-once/schema-once behaviour); archive_ask.rsbuild_query.
  • search_execute.rs is intentionally left alone — it works from a pre-parsed ast with optional account scoping (build_in_account), a different shape.

Verification

  • New golden test asserts build_query/CompiledQuery produce byte-identical ({:?}) query output to the manual path across several queries, plus parse-error propagation.
  • mxr-search 40 + daemon consumer tests pass; clippy + fmt clean.

Generated with Claude Code


Summary by cubic

Consolidates the repeated query parse+schema+build path into mxr_search::CompiledQuery and a build_query(&str) helper. This removes duplication and keeps paging loops efficient while preserving behavior.

  • Refactors
    • Add CompiledQuery::parse(query) (parses once, holds MxrSchema); build() returns a fresh Box<dyn Query> per call.
    • Add build_query(&str) for single-use callers; re-export both from mxr_search.
    • Update daemon: mutations.rs uses CompiledQuery; archive_ask.rs uses build_query. Leave search_execute.rs as-is (pre-parsed AST/account-scoped path).
    • Tests assert helper output matches the manual path and that parse errors propagate.

Written for commit 9ba92e1. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Refactor
    • Improved search query processing efficiency by optimizing how queries are compiled and applied across paginated results.
    • Streamlined internal search infrastructure for better performance.

Several daemon search sites repeated the same three-step dance:
parse_query(str) -> MxrSchema::build() -> QueryBuilder::new(&schema).build(&ast).
If the query-compilation path ever changed, every caller had to change
with it.

Add `mxr_search::CompiledQuery` (parses once, holds the schema, builds a
fresh Tantivy query per call — paging loops consume the query each page)
plus a `build_query(&str)` convenience for single-use sites. Switch the
batch-mutation address loop to CompiledQuery (preserving its parse-once /
schema-once behaviour) and archive_ask to build_query. A golden test
asserts the helper produces byte-identical query output to the manual
path, so this is a pure consolidation.
@vercel

vercel Bot commented Jun 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
mxr-mail Ready Ready Preview, Comment Jun 10, 2026 7:15pm

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bf58e38c-9b49-43db-bfa8-4a276f47208c

📥 Commits

Reviewing files that changed from the base of the PR and between 7bb7a09 and 9ba92e1.

📒 Files selected for processing (4)
  • crates/daemon/src/handler/archive_ask.rs
  • crates/daemon/src/handler/mutations.rs
  • crates/search/src/lib.rs
  • crates/search/src/query_builder.rs

📝 Walkthrough

Walkthrough

This PR introduces a query compilation API that separates parsing from query building, enabling pagination without repeated schema construction. CompiledQuery is added to query_builder.rs, exported from lib.rs, and integrated into two handler code paths that perform paginated and filtered searches.

Changes

Query Compilation API and Handler Refactoring

Layer / File(s) Summary
New query compilation API with CompiledQuery and build_query
crates/search/src/query_builder.rs
CompiledQuery struct holds a built MxrSchema and parsed QueryNode. CompiledQuery::parse parses once; CompiledQuery::build produces a fresh Tantivy query on demand. build_query function performs parse+build in one call. Import statement updated to include parse_query. Tests verify equivalence with manual schema+builder path and error handling.
Public re-exports from search module
crates/search/src/lib.rs
CompiledQuery and build_query are re-exported from query_builder alongside the existing QueryBuilder.
Handler refactoring to use new query API
crates/daemon/src/handler/archive_ask.rs, crates/daemon/src/handler/mutations.rs
archive_ask.rs refactors structured-filter query construction to use build_query directly. mutations.rs refactors pagination query construction to parse once with CompiledQuery::parse and rebuild the Tantivy query on each page via build.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A query once parsed, then built fresh each time,
No schema rebuilt—optimization sublime!
Two handlers now leverage this API,
Pagination flows swift, pagination flows spry!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title directly and clearly describes the main refactoring change: consolidating the repeated parse+schema+build query compilation pattern into a single helper API.
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 refactor/shared-query-builder

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

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

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 4 files

Re-trigger cubic

@bhekanik bhekanik merged commit ac5f006 into main Jun 11, 2026
25 of 27 checks passed
@bhekanik bhekanik deleted the refactor/shared-query-builder branch June 11, 2026 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant