feat(bin): opt-in daemon fast-path for search and vsearch#5
feat(bin): opt-in daemon fast-path for search and vsearch#5lukeboyett wants to merge 10 commits intofix/db-transaction-typefrom
Conversation
`qmd search foo` on a large index pays ~500–800 ms of Node + better-
sqlite3 + sqlite-vec bootstrap every call — most of the wall clock is
module initialization, not the query itself. `qmd mcp --http --daemon`
already ships as an always-warm server and already exposes a
`POST /search` REST endpoint (used today by MCP-adjacent tooling), so
the cold-start cost is avoidable for callers willing to keep a daemon
running.
This PR wires `bin/qmd` to that existing endpoint. When `QMD_DAEMON_URL`
is set, the shell wrapper:
- Health-checks `GET /health` with a 1 s timeout. On failure it falls
through to the normal cold-start CLI.
- Translates `qmd search foo -c alma --limit 5` into
`POST /search {"searches":[{"type":"lex","query":"foo"}],
"collections":["alma"],"limit":5}`.
- Does the same for `vsearch` with `type:"vec"`.
- Prints whatever JSON the daemon returns (users that want formatted
text leave `QMD_DAEMON_URL` unset).
- Silently falls through when `--index <name>` is used (one daemon =
one index) or when any curl error occurs.
No new daemon or new endpoints are introduced; this is purely a CLI
routing change over existing server surface. Stays POSIX sh so it
works under the current #!/bin/sh shebang.
Tests:
- `test/mcp.test.ts` "MCP HTTP Transport" suite gains contract tests
for the REST path `bin/qmd` depends on: happy path with
`type="lex"` + collections, happy path with `null` collections
(unset `-c`), happy path with `type="vec"` (`vsearch`), and a 400
for missing `searches`. These pin the payload/response shape so a
future upstream change to that endpoint can't silently break the
fast-path.
Docs:
- README gains a "Fast CLI via daemon (scripts / agents)" section with
the opt-in env var, example payload, and fall-through rules.
- CHANGELOG entry under Changes.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a45a7cc1b4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _qmd_limit=10 | ||
| _qmd_collection="" | ||
| _qmd_query="" | ||
| shift # drop the subcommand |
There was a problem hiding this comment.
This shift starts destructive parsing of $@, and the loop below consumes all remaining args before the daemon request. If /health passes but POST /search fails (the path this change says should “silently fall through”), execution reaches the final exec ... "$@" with mutated/empty argv, so the cold-start fallback no longer receives the original search/vsearch command and query.
Useful? React with 👍 / 👎.
| -l|--limit) _qmd_limit="$2"; shift 2 ;; | ||
| --limit=*) _qmd_limit="${1#*=}"; shift ;; |
There was a problem hiding this comment.
-n limit flag in daemon fast-path parser
The fast-path parses -l|--limit, but the CLI’s search limit option is -n (see src/cli/qmd.ts option parsing/help). Under daemon mode, qmd search -n 1 foo hits the generic -* branch, so -n is discarded, 1 is treated as part of the query text, and the request still uses the default limit (10), producing incorrect query and result count.
Useful? React with 👍 / 👎.
| -c|--collection) _qmd_collection="$2"; shift 2 ;; | ||
| --collection=*) _qmd_collection="${1#*=}"; shift ;; |
There was a problem hiding this comment.
This parser stores collections in a single scalar, so repeated -c/--collection flags overwrite each other; the main CLI supports multiple collections (collection is declared multiple: true in src/cli/qmd.ts). With daemon routing enabled, a call like qmd search foo -c alpha -c beta will silently search only beta, changing result scope.
Useful? React with 👍 / 👎.
Three linked fixes for the daemon fast-path, addressing Codex round 1 on #5: - P1 "Preserve CLI args when daemon POST falls through": the previous implementation did `shift` in the outer shell and then iterated `$@` destructively. A healthy /health followed by a failed POST would reach the fallback `exec` with mutated/empty argv, so the cold-start CLI would see the wrong command. Moved the whole fast-path into a shell function (`_qmd_try_daemon`) so its parsing operates on a local copy of the args. The outer shell's `$@` is never touched, and on any `return 1` (health fail, POST fail, empty query) control falls through to the lockfile-gated `exec` with the exact argv the user typed. - P1 "Support -n limit flag in daemon fast-path parser": upstream's search limit flag is `-n <num>` (`src/cli/qmd.ts:2486`, documented at `--help` line 2773). The fast-path was parsing `-l|--limit` which does not exist on `search` (`-l` is a `get` option). `qmd search foo -n 3` would discard `-n`, treat `3` as part of the query, and use the default limit of 10. Now parses `-n`, `--n`, `--n=...`. - P2 "Preserve all collection filters": `collection` is declared `multiple: true` in `src/cli/qmd.ts:2496` — `qmd search foo -c a -c b` should hit both collections. The parser stored a single scalar so only the last `-c` survived. Collections are now accumulated into a JSON array (`["a","b"]`) or rendered as `null` when no `-c` is passed. Tests: new `test/bin-qmd-daemon-fastpath.test.ts` spins up a tiny HTTP mock on a random port, exports QMD_DAEMON_URL, and spawns the shell wrapper via async spawn (spawnSync would deadlock the mock's event loop). It covers: - payload shape for `search -c -n`, - `vsearch` → `type:"vec"`, - multiple `-c` flags accumulate, - `--collection=` long form mixes with `-c`, - unset `-c` yields `collections: null`, - `--index` bypasses (zero captures), - non-`search` subcommands don't touch the daemon, - health-check 500 falls through without attempting POST.
|
All three addressed in 4a9eb76. P1 "Preserve CLI args when daemon POST falls through" — fast-path logic moved into a shell function ( P1 "Support -n limit flag" — upstream's search limit flag is P2 "Preserve all collection filters" — New integration suite @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a9eb760c0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| -*) | ||
| shift ;; | ||
| *) | ||
| if [ -z "$_qmd_query" ]; then _qmd_query="$1" | ||
| else _qmd_query="$_qmd_query $1"; fi |
There was a problem hiding this comment.
With QMD_DAEMON_URL set, options that are valid for search/vsearch but not explicitly handled in this parser (for example --min-score 0.8 or --intent auth) pass through the -* branch and only the flag token is shifted. On the next iteration, the option value is treated as a positional term and appended to _qmd_query, so the daemon receives a mutated query (e.g. foo 0.8) while the option semantics are silently lost. This produces incorrect search behavior in fast-path mode instead of preserving CLI behavior or falling back.
Useful? React with 👍 / 👎.
Codex P1 on #5 round 2: the fast-path's catch-all for unrecognised dashed flags was `shift` (drop the flag token only). Several upstream `search` / `vsearch` options take a value — `--min-score <N>`, `-C/--candidate-limit`, `--intent <text>`, etc. — while others are booleans. A call like `qmd search foo --min-score 0.8` would drop `--min-score`, then on the next loop the `*)` arm would treat `0.8` as part of the query text. The daemon would receive `foo 0.8` and the min-score semantics would vanish silently. Rather than enumerate every upstream flag (brittle — breaks each time upstream adds one), the unknown-flag branch now `return 1`s out of the fast-path and lets the cold-start CLI handle the invocation. The opt-in fast-path stays a strict subset of interactive CLI semantics: if we can't decode the argument safely, the user gets correct results via the cold-start path instead of wrong results fast. Tests cover both a value-taking unknown flag (`--min-score 0.8`) and a boolean one (`--json`). Both now fall through; the mock server records no `/search` request.
|
Addressed in 661d203: unknown flags now @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 661d203f58
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| -n) _qmd_limit="$2"; shift 2 ;; | ||
| --n) _qmd_limit="$2"; shift 2 ;; |
There was a problem hiding this comment.
The fast-path consumes -n/--n with an unconditional shift 2; when users pass a missing-value form like qmd search foo -n (or similarly -c/--collection without a value), /bin/sh aborts with shift: can't shift that many and exits 2 before this function can return 1 to fall back to the cold-start CLI. This breaks the intended “safe fall-through” behavior whenever the daemon is reachable, so these branches should verify argument arity before shifting and return non-zero on malformed input.
Useful? React with 👍 / 👎.
Codex P2 on #5 round 3: `-n`, `--n`, `-c`, `--collection` unconditionally called `shift 2`. If the user omitted the value (e.g. `qmd search foo -n` or the flag happens to be the last token), `/bin/sh` aborted the whole script with "shift: can't shift that many" (exit 2) instead of letting the fast-path `return 1` and fall back to the cold-start CLI. Added `[ $# -ge 2 ] || return 1` guards before every `shift 2` in the fast-path. Malformed input is now indistinguishable from any other reason to fall through — the cold-start CLI handles the error reporting, which keeps the fast-path a strict subset of CLI semantics. Tests: - `dangling value flag (-n with no argument) falls through without aborting shell` - `dangling -c with no argument also falls through cleanly` Both assert stderr does NOT contain "can't shift" and that no `/search` request was made.
|
Addressed in ff67a6b. P2 "Guard value flags before shifting argv" — valid catch. P1 "Avoid leaking unknown option values into query text" — I believe this is the same concern as round 2 and is already resolved in 661d203 (the prior commit). Line 117 of the current Any unhandled dashed token (including @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff67a6b827
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| curl -sf --max-time 1 "${QMD_DAEMON_URL}/health" >/dev/null 2>&1 || return 1 | ||
|
|
||
| _qmd_limit=10 |
There was a problem hiding this comment.
When QMD_DAEMON_URL is set and the caller does not pass -n/--n, the fast-path always sends "limit": 10, so qmd search <query> returns a different number of results than the normal CLI path (which defaults to 5, or 20 only for explicit JSON/files output). This makes result cardinality depend on transport mode rather than user flags, which is a behavior regression for scripts that rely on the command’s default result count.
Useful? React with 👍 / 👎.
Codex P2 on #5 round 4: the fast-path defaulted to `limit: 10` when the user didn't pass `-n`, but the interactive CLI defaults to 5 (src/cli/qmd.ts --help line 2773: "Max results (default 5, or 20 for --files/--json)"). Scripts written against the CLI's default would see different result counts depending on whether QMD_DAEMON_URL was set — a behavior regression. Fix: `_qmd_limit=5`. The `--files` / `--json` special case (default 20) isn't reachable here because those flags are unknown to the fast-path parser and already route through the cold-start branch. Test: new `unset -n uses 5 to match the interactive CLI default`.
|
Addressed in fdd7ceb. P2 "Align daemon default limit with CLI default" — legit. Interactive CLI default is 5 ( P1 "Avoid leaking unknown option values into query text" — re-flagged for the third consecutive round; I believe the comment is stale. The -*)
# Unknown flag. ... fall through to the cold-start CLI ...
return 1 ;;Existing tests that specifically exercise the scenario described ( @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fdd7cebaeb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _qmd_limit="$2" | ||
| shift 2 | ||
| ;; | ||
| --n=*) _qmd_limit="${1#*=}"; shift ;; |
There was a problem hiding this comment.
The fast-path forwards -n/--n as raw text into JSON, but the interactive CLI normalizes with parseInt(... ) || defaultLimit (src/cli/qmd.ts, parseCLI), so some inputs produce transport-dependent results. For example, qmd search foo -n 0 yields the default limit in cold-start mode but sends "limit":0 through the daemon (typically returning zero results), and --n=1e2 can become 100 instead of 1. Coercing and defaulting the limit in bin/qmd the same way as parseCLI would keep behavior consistent regardless of transport.
Useful? React with 👍 / 👎.
Codex P2 on #5 round 5: the fast-path forwarded the raw -n string into the JSON payload, but the interactive CLI resolves the limit with `parseInt(values.n, 10) || defaultLimit` (src/cli/qmd.ts:2550). As a result some inputs produced transport-dependent results: - `qmd search foo -n 0` → CLI resolves to 5 (default), fast-path was sending `"limit":0` (typically zero results). - `-n 1e2` → CLI `parseInt("1e2",10)` = 1 (stops at 'e'), fast-path was forwarding "1e2" which the daemon would fail/misinterpret. - Any non-numeric (`-n abc`, `-n -1`, empty) → CLI → default, fast-path → garbage payload. Fix: after parsing, normalise `_qmd_limit`. Empty, zero, or any value containing a non-digit collapses to the default of 5 — matching the `parseInt || default` semantics as closely as POSIX shell can. Tests: - `-n 0 normalises to the default 5` - `-n <non-numeric> normalises to the default 5`
|
Addressed in 79d29d2: fast-path now collapses empty/zero/non-numeric P1 "unknown option values" has now been re-flagged on 4 consecutive commits without a concrete reproducer, while the @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79d29d2988
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # alphanumeric, etc.) collapses to the default of 5, exactly as the | ||
| # interactive CLI would resolve it. | ||
| case "$_qmd_limit" in | ||
| ''|0|*[!0-9]*) _qmd_limit=5 ;; |
There was a problem hiding this comment.
-n with CLI semantics before building JSON payload
The fast-path -n normalization does not match src/cli/qmd.ts (parseInt(values.n, 10) || defaultLimit) and can emit invalid JSON numbers. With the current check, values like -n 00 remain 00, so the payload contains "limit":00 (invalid JSON for /search), while values like -n 1e2 are forced to 5 even though the CLI resolves them to 1. In both cases, daemon mode behavior diverges from normal qmd search and can silently fall back to cold-start output instead of returning daemon JSON.
Useful? React with 👍 / 👎.
Codex P1 on #5 round 6: the previous normalisation collapsed any value containing a non-digit to the default, and left zero-padded numbers like "00" intact. Both diverge from the interactive CLI: - `parseInt("1e2", 10)` === 1, not the default. The fast-path was collapsing "1e2" to 5, so a user passing `-n 1e2` would get different result counts depending on QMD_DAEMON_URL. - `"00"` with my all-digits regex stayed "00", producing `"limit":00` — invalid JSON — and causing the daemon request to fail (silently falling through to cold-start, but with user-observable confusion and no daemon speedup). Fix mirrors parseInt more precisely: 1. sed extracts the leading run of digits (`1e2` -> `1`, `5abc` -> `5`, `abc` -> empty). 2. Empty leading digits -> default 5. 3. Otherwise coerce via `$((expr))`, which normalises padded values (`007` -> 7) and emits plain integers. 4. If the coerced value is <= 0, fall back to default — matching `parseInt(...) || defaultLimit`. New tests exercise the specific divergence cases: - `-n 1e2` -> 1 (matches parseInt) - `-n 5abc` -> 5 (matches parseInt) - `-n abc` -> 5 (no leading digits -> default) - `-n 00` -> 5 (zero -> default, emits valid JSON) - `-n 007` -> 7 (strips leading zeros, emits valid JSON) Each JSON payload is round-tripped through JSON.parse to prove it's structurally valid — a payload with leading zeros would have thrown.
|
Addressed in 4da193c. P1 "Parse -n with CLI semantics before building JSON payload" — good catch on both edges. Now mirrors
New tests cover P1/P2 repeat flags from prior rounds already addressed in 661d203 and 79d29d2; tests cover both. No new code change there. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4da193c33a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _qmd_leading=$(printf '%s' "$_qmd_limit" | sed -n 's/^\([0-9][0-9]*\).*$/\1/p') | ||
| if [ -z "$_qmd_leading" ]; then | ||
| _qmd_limit=5 |
There was a problem hiding this comment.
-n values in daemon limit parsing
The fast-path limit normalizer only captures unsigned leading digits (sed ... '^([0-9][0-9]*)'), so signed inputs are silently rewritten to the default limit in daemon mode. With QMD_DAEMON_URL set, qmd search foo -n -1 or qmd search foo --n=+7 will send "limit":5, while the cold-start CLI path uses parseInt(values.n, 10) || defaultLimit and therefore interprets those as -1 and 7. This creates transport-dependent behavior for the same command and can change result counts for scripts that pass signed -n values.
Useful? React with 👍 / 👎.
Codex rounds 5-7 on #5 kept surfacing new edge cases while trying to match `parseInt(values.n, 10) || defaultLimit` in POSIX shell (`1e2` → 1, `00` as invalid JSON, `-1`/`+7` signed values, zero-padding). Each fix exposed another subtle divergence from JS number parsing. Shifting approach: the fast-path now only accepts a plain positive integer with no sign, no leading zeros, no exponent, no trailing characters. Everything else (empty, `0`, `00`, `007`, `-1`, `+7`, `1e2`, `5abc`, `abc`, …) returns 1 from the fast-path function so the cold-start CLI handles the input with its real `parseInt` logic. This matches the PR's stated philosophy — "fast-path stays a strict subset of CLI semantics" — and eliminates the sh-vs-JS number-parsing minefield entirely. Users passing weird `-n` values get the same results they'd get without `QMD_DAEMON_URL` set, just slower. Tests: - `strict -n validation: non-plain-positive-integer values fall through` covers 0, 00, 007, -1, +7, 1e2, 5abc, abc. - `plain positive integers in -n are forwarded as-is` covers 1, 3, 20, 100.
|
Pivoting strategy in 20a0ff7. Rounds 5-7 chased New approach: strict validation. Fast-path accepts ONLY plain positive integers (no sign, no leading zeros, no exponent, no trailing chars). Everything else falls through to the cold-start CLI, which owns the real parseInt semantics. This matches the PR's stated philosophy that the fast-path is a strict subset of CLI behavior. Users passing weird -n values get the cold-start's (authoritative) interpretation — just slower. One test walks the full list of rejected shapes: The repeat P1/P2 flags from prior rounds no longer apply to the new parser. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20a0ff7a58
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| curl -sf --max-time 30 -X POST \ | ||
| -H "Content-Type: application/json" \ | ||
| -d "$_qmd_payload" \ | ||
| "${QMD_DAEMON_URL}/search" || return 1 |
There was a problem hiding this comment.
When /search fails after sending some bytes (for example, connection reset or --max-time 30 expiring mid-response), curl can write partial JSON to stdout before exiting non-zero. This function then returns 1, so the wrapper falls through to the cold-start CLI and emits a second output format, producing mixed/truncated output on a single invocation. That breaks script consumers expecting one valid payload shape; buffer the daemon response and print it only after curl succeeds.
Useful? React with 👍 / 👎.
…fallback Codex P2 on #5 round 8: `curl` was streaming the daemon response straight to stdout. If the daemon started writing then disconnected (connection reset, `--max-time` expiring mid-response), partial JSON would hit stdout before the fast-path's `return 1` triggered the cold-start fallback. Script consumers would then see `{"partial":` + cold-start text concatenated on one invocation, breaking expectations. Fix: capture the response in a shell variable, only `printf` it on success. A curl error (any exit code) short-circuits via `|| return 1` and stdout stays clean. The cold-start CLI then runs with a virgin output stream. Test: `daemon that writes partial bytes then drops the connection leaks no output` stands up a mock that responds 200 to /health, opens /search with a partial `{"partial":` then destroys the socket. Assertion: `stdout` contains no `partial`.
|
Addressed in 9b81ab8. P2 "Suppress partial daemon output before cold-start fallback" — good catch. curl was streaming straight to stdout, so a mid-response failure (socket reset, max-time mid-body) would emit partial JSON before fall-through. Now the response is captured in a shell variable and only printed on curl success. New test stands up a server that opens with @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b81ab8dcf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _qmd_collections_json="null" | ||
| fi | ||
|
|
||
| _qmd_payload="{\"searches\":[{\"type\":\"${_qmd_qtype}\",\"query\":\"${_qmd_q_escaped}\"}],\"collections\":${_qmd_collections_json},\"limit\":${_qmd_limit}}" |
There was a problem hiding this comment.
When the fast-path handles vsearch, the payload only includes type, query, collections, and limit, so the daemon uses /search defaults (minScore falls back to 0 and reranking stays enabled). In the normal CLI path, vsearch sets a default minScore of 0.3 and runs vectorSearch() (vector-only, no rerank), so enabling QMD_DAEMON_URL changes which results are returned and their ordering for the same command. This affects any script relying on qmd vsearch output consistency across daemon vs cold-start execution.
Useful? React with 👍 / 👎.
…ough /search Codex P2 on #5 round 9: `qmd vsearch` cold-start sets `minScore: 0.3` (vs 0 for search), runs a vector-only pipeline, and does not rerank. The daemon's `/search` REST endpoint exposes none of those knobs — it always calls `store.search(...)` (hybrid + rerank) with the caller-supplied minScore (default 0). Routing vsearch through the fast-path would silently change result ordering and filtering for scripts that rely on vsearch's cold-start behaviour. Rather than add pseudo-plumbing, vsearch is now *always* handled by the cold-start CLI. The fast-path is search-only. This matches the same strict-subset principle the PR has been applying to unknown flags and weird -n values: if the semantics aren't reliably expressible on the daemon side, cold-start owns it. Changes: - `_qmd_try_daemon` no longer accepts `vsearch`. - Top-level dispatch is `case "${1:-}" in search)` only. - Test `vsearch is intentionally NOT handled by the fast-path` asserts zero captures on the mock when vsearch is invoked with QMD_DAEMON_URL set. - README and CHANGELOG updated: fast-path is search-only, vsearch semantics rationale documented.
|
Addressed in d5b01e9. P2 "Preserve vsearch semantics in daemon request" — valid, and the fix is structural: vsearch is now always handled by cold-start, even when QMD_DAEMON_URL is set. Its vector-only pipeline + 0.3 minScore default + no-rerank behavior can't be expressed through the daemon's The remaining two flags are the same stale P1 and P2 from rounds 2-6, both already covered by the strict parser and tests since earlier commits. @codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Closing — this is tracked upstream at tobi#582. Fork branch retained. |
1 participant
Summary
Wires
bin/qmdto the existingPOST /searchREST endpoint onqmd mcp --http --daemonsoqmd search/qmd vsearchcan skip the ~500–800 ms per-call Node + better-sqlite3 + sqlite-vec bootstrap on large indexes.Opt-in via
QMD_DAEMON_URL. Default interactive behavior is unchanged.Why
For scripted / agent workflows on a large index, the cold-start cost dominates wall clock: module loading, SQLite open, extension init.
qmd mcp --http --daemonalready ships as an always-warm process and already exposes the structured-search REST endpoint. This PR is purely a CLI routing change — no new daemon, no new endpoints, no architectural duplication with MCP.Behavior
When
QMD_DAEMON_URLis set,bin/qmd:GET /healthwith a 1 s timeout.qmd search foo -c alma --limit 5→POST /search {"searches":[{"type":"lex","query":"foo"}],"collections":["alma"],"limit":5}.vsearchusestype:"vec".--index <name>(one daemon = one index).Why opt-in, not auto-detect
The daemon returns structured JSON; the cold-start CLI prints formatted text. Auto-routing would surprise interactive users by changing stdout shape. Opt-in via env var means scripts explicitly choose JSON and interactive users never notice.
Tests
New contract tests in
test/mcp.test.tsunder"MCP HTTP Transport":POST /searchhappy path withtype="lex"+ collections — asserts the{docid, file, title, score}fields the fast-path relies on.POST /searchwithnullcollections (unset-ccase).POST /searchwith missingsearchesreturns 400.POST /searchwithtype="vec"(vsearchrouting).These pin the payload/response shape so a future change to that endpoint can't silently break the CLI fast-path.
Docs
Notes
fix/db-transaction-typebecause a cleanmaincurrently fails to build; that baseline build fix is the parent PR.