feat(fetch): add --max-chars flag - #60
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PR Summary by Qodofetch: add --max-chars flag to override fetch content cap
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
Code Review by Qodo
1. --max-chars error exits 2
|
| /// Truncate content at this many characters (default: 50000) | ||
| #[arg(long = "max-chars", value_parser = clap::value_parser!(u64).range(1..))] | ||
| max_chars: Option<u64>, |
There was a problem hiding this comment.
1. --max-chars error exits 2 📘 Rule violation ☼ Reliability
--max-chars uses clap value validation (range(1..)) which causes invalid input (e.g., 0) to exit with code 2, conflicting with the documented requirement that command errors exit with code 1. The new e2e test codifies this non-1 exit behavior, making the error-handling contract harder to standardize later.
Agent Prompt
## Issue description
The new `--max-chars` flag is validated by clap (`range(1..)`) which produces a clap parse error exit code (2) on invalid values like `0`. Compliance requires errors to exit with code 1 and to emit actionable/structured errors.
## Issue Context
A new e2e test (`test_fetch_max_chars_rejects_zero`) asserts exit code 2 for `--max-chars 0`, reinforcing the non-compliant error code behavior.
## Fix Focus Areas
- src/main.rs[196-198]
- tests/e2e/test_fetch.py[51-54]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| /// Truncate content at this many characters (default: 50000) | ||
| #[arg(long = "max-chars", value_parser = clap::value_parser!(u64).range(1..))] | ||
| max_chars: Option<u64>, |
There was a problem hiding this comment.
2. Unbounded max_chars request 🐞 Bug ☼ Reliability
--max-chars accepts any positive u64, so the CLI/daemon can request extremely large fetch responses. Since fetch responses are fully buffered and parsed as JSON before printing, very large responses can drive high memory usage or OOM when the API honors large max_chars values.
Agent Prompt
## Issue description
`--max-chars` currently accepts any positive `u64` and is forwarded to the API. The fetch response is buffered and parsed into `serde_json::Value` before output, so very large responses can cause high memory usage or OOM.
## Issue Context
- The feature intent is to override the backend’s default cap (50,000 chars), so the fix should preserve legitimate increases (e.g. 200,000) while adding a safety guard.
## Fix Focus Areas
- Add a reasonable upper bound in clap parsing (or add a warning/confirmation above a threshold), consistent with expected product limits.
- Optionally document the bound in help/README.
- If large payloads are expected, consider a longer-term refactor to avoid fully buffering/parsing the response (streaming), but the immediate mitigation can be validation/guardrails.
### Code pointers
- src/main.rs[179-199]
- src/daemon.rs[30-49]
- src/commands/search.rs[96-149]
- src/api.rs[143-153]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| # The truncation disclaimer is the load-bearing assert: a stale daemon | ||
| # that drops `max_chars` returns the full page with exit code 0. | ||
| assert "truncated to stay below 50 characters" in data["content"] | ||
|
|
There was a problem hiding this comment.
3. Brittle truncation assertion 🐞 Bug ☼ Reliability
The new e2e test hard-depends on a specific backend-generated truncation disclaimer phrase. If the API changes wording while preserving behavior, the test will fail even though --max-chars plumbing still works.
Agent Prompt
## Issue description
`test_fetch_max_chars` asserts a specific disclaimer substring from live API output. This couples the test to backend prose and can cause false failures on harmless wording changes.
## Issue Context
The comment explains the assert is intended to detect a stale daemon that drops `max_chars`. That intent can still be met with more stable assertions.
## Fix Focus Areas
- Replace the exact-phrase match with a more robust invariant (e.g., content length is <= a small bound; or content contains a generic "truncated" marker; or (best) assert a structured field if the API provides one).
### Code pointers
- tests/e2e/test_fetch.py[42-55]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Summary
--max-charstokeenable fetch. It maps to the API'smax_charsquery param and overrides the backend's default 50000-char content cap.fetch_query()builder.0client-side; the backend accepts any positive integer.Tests
cargo checkandcargo clippypass.--max-chars 50returns truncated content with the disclaimer;--max-chars 0exits 2.test_fetch_max_chars(disclaimer assert catches a stale daemon that drops the param) andtest_fetch_max_chars_rejects_zero.🤖 Generated with Claude Code