refactor(fetch): share query building between daemon and direct paths

[IlyaGusev]

Summary

Cleanup follow-up to #52, from a 4-angle review (reuse / simplification / efficiency / altitude) of that diff.

• The /v1/fetch query (url params + optional live=true) was built identically in two places — the daemon's fetch handler and the direct-HTTP fallback in execute() — so any future fetch param would need matched edits and could silently drift between the paths.
• Extracted DaemonRequest::fetch_query() (in daemon.rs, next to the struct both modules already share) as the single source; both call sites now use it. No behavior change.

Considered and skipped: generalizing fetch params into an opaque passthrough (like search's body) — YAGNI with exactly one fetch param today.

Testing

• cargo fmt / cargo clippy --all-targets -- -D warnings / cargo test clean.
• pytest tests/e2e/test_fetch.py — 8/8 pass against the rebuilt binary.
• Re-verified fetch --live through a running daemon (isolated KEENABLE_HOME).

🤖 Generated with Claude Code

[qodo-code-review]

PR Summary by Qodo

Refactor fetch: centralize /v1/fetch query building in DaemonRequest

✨ Enhancement 🕐 10-20 Minutes

AI Description

• Centralize /v1/fetch query parameter construction to prevent drift between code paths.
• Reuse the same helper for daemon-handled and direct-HTTP fetch execution.
• Preserve existing behavior for url repetition and optional live=true.

Diagram

graph TD
A["Direct HTTP execute()"] --> C["DaemonRequest.fetch_query()"] --> D["GET /v1/fetch"] --> E{{"Keenable API"}}
B["Daemon fetch handler"] --> C

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Typed command enum with per-command builders

• ➕ Eliminates stringly-typed command matching and missing-field checks at runtime
• ➕ Makes per-command parameter construction explicit and easier to extend safely
• ➖ Larger refactor footprint across request parsing/dispatch
• ➖ Not justified if the command surface is intentionally flexible/lightweight

2. Return a configured RequestBuilder instead of query Vec

• ➕ Encapsulates both query building and request construction in one place
• ➕ Avoids exposing Vec lifetimes/shape to callers
• ➖ Ties the helper to reqwest and HTTP details (less reusable)
• ➖ Slightly harder to unit-test in isolation without mocking reqwest

Recommendation: Keep the PR’s current approach: a small, shared DaemonRequest::fetch_query() is the lowest-risk way to prevent parameter drift while preserving existing layering (request model stays shared; HTTP sending remains local to each path). The larger typed-command or RequestBuilder encapsulation options are reasonable only if the command set or fetch parameters are expected to grow significantly.

Files changed (2) +19 / -12

Refactor (2) +19 / -12

search.rsUse DaemonRequest::fetch_query() for direct /v1/fetch requests +1/-5

Use DaemonRequest::fetch_query() for direct /v1/fetch requests

• Replaces inline construction of repeated 'url' query params and optional 'live=true' with a call to 'req.fetch_query()'. Keeps the same missing-urls error behavior while ensuring query shape matches the daemon path.

src/commands/search.rs

daemon.rsAdd DaemonRequest::fetch_query() and reuse it in daemon fetch handler +18/-7

Add DaemonRequest::fetch_query() and reuse it in daemon fetch handler

• Introduces 'DaemonRequest::fetch_query()' to build '/v1/fetch' query parameters from 'urls' plus the 'live' flag. Updates the daemon-side fetch handler to call this helper, removing duplicated query-building logic.

src/daemon.rs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review