Skip to content

Add new features to kagi-cli - enhanced output format, batch-threading and rate-limiting, autocomplete, pretty colors#7

Merged
Microck merged 8 commits intoMicrock:mainfrom
neuralnexus:main
Mar 17, 2026
Merged

Add new features to kagi-cli - enhanced output format, batch-threading and rate-limiting, autocomplete, pretty colors#7
Microck merged 8 commits intoMicrock:mainfrom
neuralnexus:main

Conversation

@neuralnexus
Copy link
Contributor

Summary

last commit added several new features to the Kagi CLI:

  1. Multiple output formats: Added support for JSON, Pretty, Compact, Markdown, and CSV formats
  2. Batch search capability: New batch command for parallel searches with rate limiting
  3. Enhanced CLI: Improved help text, autocomplete support, and colorized output
  4. Updated documentation: README updates

Verification

  • [ X ] cargo fmt --check
  • [ X ] cargo clippy --all-targets --all-features -- -D warnings
  • [ X ] cargo test -q

Docs

  • [ X ] README updated if user-facing behavior changed
  • [ X ] CHANGELOG updated for notable user-facing changes

Auth / Secrets

  • [ X ] No tokens, cookies, or local config secrets were committed
  • [ X ] Any live verification steps are documented without exposing credentials

@neuralnexus neuralnexus requested a review from Microck as a code owner March 17, 2026 05:17
@Microck
Copy link
Owner

Microck commented Mar 17, 2026

thanks for putting this together. there are some good additions here, but i don’t think this is ready to merge yet.

the main issue is that a few of the new features are either incomplete or break the cli contract in ways that will surprise users and scripts:

  • shell completion is advertised in the help text, changelog, and readme, but it is not actually implemented. kagi --generate-completion bash currently fails with unexpected argument '--generate-completion'. if completion support is meant to be part of this pr, it needs to be wired up for real. otherwise the docs/help/changelog need to stop claiming it exists.
  • batch breaks the json-first output contract. even when using --format json or --format compact, it prints === Results for: ... === headers and spacing around each result, so the output is no longer valid machine-readable json. if batch is going to support structured formats, it needs to emit a real structured envelope instead of mixing formatting into stdout.
  • batch errors are swallowed and the command still exits successfully. for example, with an invalid session token it prints an error per query but still exits 0. that makes it unsafe for scripting and automation. failed queries should either produce a non-zero exit code or a structured failure result that callers can reliably detect.
  • there is no validation for --concurrency or --rate-limit. --concurrency 0 hangs, and --rate-limit 0 panics in the rate limiter and still ends up exiting 0. these need explicit argument validation at the clap layer or before the batch run starts.
  • the csv formatter is lossy. it replaces " with ', which mutates the original data and still does not fully solve csv escaping. this should use proper csv escaping instead of rewriting content.
  • the docs still reference the old --pretty flag in places, but the cli now only supports --format pretty. if the flag change is intentional, the docs need to be updated consistently in this pr.

one smaller cleanup item: futures was added as a dependency, but it doesn’t seem to be used anymore after the test changes, so that should probably be removed.

i’d like to see these fixed before merge, especially the completion mismatch, the batch output/exit-code behavior, and the missing validation on batch flags.

@neuralnexus
Copy link
Contributor Author

AI is making me lazy anymore... will fix.

@neuralnexus
Copy link
Contributor Author

Ok, I think that's better now. I'm turning in for the night, let me know if you see any other issues and I'll work on them tomorrow.

@Microck
Copy link
Owner

Microck commented Mar 17, 2026

most of the earlier issues look fixed now, but the rate limiter still is not enforcing the configured rpm correctly.

i live-tested kagi batch rust go python --rate-limit 1 --concurrency 3, and it completed in about 61 seconds. at 1 rpm, three requests should take roughly two minutes, so the current implementation is still allowing an extra request through after the first wait.

the bug appears to be in RateLimiter::acquire(): it sleeps while holding the mutexes and then returns without consuming a refilled token or updating last_refill. i’d fix that loop before merge and add a behavior test that proves the limiter actually throttles correctly under contention.

@neuralnexus
Copy link
Contributor Author

rate limit is a bit more robust now.

@Microck
Copy link
Owner

Microck commented Mar 17, 2026

thanks for fixing everything and for the contribution.

everything looks in order on my side now. i’ll just do a final polish pass on the docs/readme and then merge.

@Microck Microck merged commit 3a77bb6 into Microck:main Mar 17, 2026
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.

2 participants