DOC-6809 Docs-only MCP server: spec + v0 prototype#3585
Open
andy-stark-redis wants to merge 25 commits into
Open
DOC-6809 Docs-only MCP server: spec + v0 prototype#3585andy-stark-redis wants to merge 25 commits into
andy-stark-redis wants to merge 25 commits into
Conversation
Investigation into a read-only "docs MCP server" — distinct from the existing data-plane redis/mcp-redis — that exposes the documentation corpus as agent-queryable tools over the docs.ndjson feed we already publish. Spec plus a working v0 (search_docs + get_page, self-contained BM25, no datastore). Two findings from running it against the live feed shaped the design: indexing 2,531 pages takes ~0.3s and queries ~100ms, so search compute is a non-issue and Rust/WASM would be premature optimisation; but untuned lexical ranking mis-ranks natural-language queries — with no stemmer, "append" misses XADD's "appends", and canonical command pages lose to release-notes, operator custom-resources, and client-library pages that repeat the same terms. That ranking gap, not speed, is what justifies the vector-search upgrade path in the spec. Learned: measured perf rules out WASM; lexical ranking (not speed) is the real limit Constraint: docs MCP server stays read-only, no DB connection, no creds (unlike mcp-redis data plane) Rejected: Rust/WASM for search speed | ~0.3s index + ~100ms queries on 2,531 pages, compute isn't the bottleneck Directive: don't over-tune lexical BM25 weights (overfits to sample queries); fix ranking via stemming/analyzer or vector search Reversibility: clean Ticket: DOC-6809 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
|
Contributor
🛡️ Jit Security Scan Results
✅ No security findings were detected in this PR
Security scan by Jit
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
… Codex) Address Cursor Bugbot findings on PR #3585 plus a Codex follow-up review. The feed's `id` is the last URL path segment and is NOT unique (~213 ids map to several pages), so `byId` is now a multimap and `get_page` resolves by the unique `url` first; an ambiguous `id` or partial `url` returns candidate urls instead of silently returning the wrong page. The MCP handler now sets `isError` when a tool result carries an `error`. Smoke test rewritten as assertions (incl. a colliding id="install" fixture pair) so this can't regress. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
test/mcp-client.mjs drives the built stdio server through the real MCP client (spawn -> initialize -> tools/list -> tools/call) against the fixture, so the transport path and isError signalling are exercised end-to-end, not just the tool logic. Live-feed testing surfaced that the spec's assumed section roles (syntax/examples) don't match the feed (it uses content/parameters/example/ returns, no syntax), so a roles filter can silently return nothing — recorded in SPEC open questions to resolve before building get_examples/get_command. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Round-2 Bugbot findings on the get_page/search resolution logic were churn: the round-1 duplicate-id fix introduced two of them. Rather than three point- patches, consolidate resolution into one convergence model (per Codex design review): collect candidate pages across all supplied handles (exact url, id, boundary-anchored suffix url) and resolve only when they converge on exactly one page — so an ambiguous url still tries id, and url/id pointing at different pages reports conflicting handles instead of silently applying precedence. - Partial-url suffix matching is now anchored to "/" path segments, so "get" matches .../commands/get but not .../config-get or .../arget (was 19 false matches on the live feed, now 2 legitimate ones). [Bugbot High] - Ambiguous url no longer short-circuits the id fallback. [Bugbot Medium] - Removed the `version` param: it advertised a `latest` default it never enforced. Deferred until a committed URL/version model exists. [Bugbot Medium] - Smoke test extended with boundary and conflicting-handles assertions (17 total). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…icient npm run eval scores search_docs retrieval against a curated question set (test/eval/cases.json — command lookups phrased without the command name), reporting recall@k / MRR with a data-integrity check that flags expected urls missing from the feed. Feed read from DOCS_NDJSON or a gitignored local cache. Baseline on the live feed (22 cases): recall@1 32%, @3 45%, @5 59%, @10 73%, MRR 0.42. Confirms with data what was hypothesised: lexical retrieval alone is not good enough — canonical command pages lose to sibling commands (zadd -> zincrby), operator pages (del -> remove-node), and concept pages (ft.create -> a full-text concept page), and there is no stemming (append !-> appends). This is the measured case for the stemming/boost/vector work in SPEC §6/§10 and lets future ranking changes be compared against a baseline instead of tuned blind. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two principled ranking changes, measured against the eval baseline: - Porter stemmer (src/stem.ts) applied to both index and query tokens so word forms conflate (append/appends, prepend/prepending, expires/expire). Stopwords are filtered on raw tokens before stemming. - Page-type weighting on the final score: demote release-notes/REST-API (x0.5) and operator (x0.7) pages, modestly boost /commands/* (x1.5) — targeting the observed failures where operator/concept pages outranked command pages. Eval (22 command-lookup cases): recall@1 32->50, @3 45->68, @5 59->86, @10 73->95, MRR 0.42->0.65; misses 6->1. Caveat recorded in README/SPEC: the eval is command-heavy so the /commands/* boost partly flatters it — concept/ how-to cases still needed before calling lexical sufficient generally, and the residual miss (del/unlink beaten by flushdb for "remove a key") is a semantic gap that motivates vector search. Smoke still green (17 assertions). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bugbot High: collectCandidates always merged id matches even when url already resolved to an exact page, so get_page(url=<exact>, id=<its non-unique id>) — the natural flow when an agent echoes both fields from a search hit — returned an ambiguous error instead of the page. The consolidation's "always converge" rule over-corrected: an exact url is unique and should win. Fix: an exact url short-circuits to that page, EXCEPT when a supplied id points somewhere else entirely (idPages don't include the exact page) — still a genuine conflict, so still reported. This reconciles the Bugbot finding with Codex's earlier conflict-detection ask: exact-url + its-own-id resolves; exact-url + a different id conflicts. Smoke covers both (17 assertions). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
De-bias the retrieval eval (was 22 command-only cases): tag each case command/concept and add 13 concept/how-to cases with feed-verified ground truth (incl. pages under /operate/, to test the demotion). Runner now reports recall per kind. Result under the shipped weighting: command recall@5 86% / MRR 0.65 but **concept recall@5 46% / MRR 0.29** — concept retrieval is ~half as good. The /commands/* x1.5 boost is the cause: it ranks command pages above the canonical concept page when both compete (persistence -> bgrewriteaof, replication -> cluster-replicate, keyspace-notifications -> expire), and the blanket /operate/ demotion drags down legitimate concept pages. Ablation (neutralise command boost, demote only rest-api/release-notes/references): concept @5 46->62, MRR .29->.45; command @5 86->73. Command-optimised vs balanced is a workload call — left as an open decision in SPEC, weighting unchanged for now. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per the concept-case eval finding, switch page weighting from command-optimised to balanced: demote only release-notes/REST-API/references (x0.5); drop the /commands/* x1.5 boost and the blanket /operate/ x0.7 demotion. This trades command recall@5 86->73 for concept 46->62 and the best overall MRR (0.53), and stops burying legitimate /operate/ concept pages (persistence, replication). Command ranking should be lifted by better signal (vectors), not the boost. Eval + smoke green; SPEC decision marked resolved. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ical A no-Redis experiment (vector-eval/) to decide whether vector search is worth the hosted infra before building it: embed the corpus with bge-small (fastembed/ONNX), rank in numpy, and score vector + hybrid (RRF fused with the production lexical ranker, dumped via dump-lexical.mjs) against the same 35-case eval. Verdict: hybrid clearly wins — overall recall@5 69->86, MRR .53->.66; command @5 73->91 — so the RediSearch/RedisVL build is justified, and it should be a HYBRID ranker (vector alone only modestly beats lexical). Concept queries stay weak (vector ties lexical; hybrid lifts @5 62->77 but not top-1/3), most likely an artefact of the coarse page-level chunking used here. A first section-level attempt (21.5k chunks) stalled on this machine: embedding runs only ~6 chunks/s (unoptimised CPU ONNX), so it ran >1h and the CPU collapsed with no checkpoint written. The committed version is the slim, instrumented rebuild: one page-level chunk per page (~2.5k), batched embedding with progress logging, end-of-run cache. Section-level chunking over the feed's sections[] is the next experiment and needs proper batching/caching to be runnable here. Learned: hybrid (lexical+vector RRF) clearly beats either alone; vector alone only modest; concept stays weak on coarse page-level chunks Directive: build the hosted path as HYBRID not pure vector; use the feed sections[] for section-level chunking to attack the concept gap; don't read the ~6 chunks/s rate as a production latency signal (unoptimised local ONNX) Gaps: section-level chunking and real production embedding latency not yet measured; concept n=13 so those numbers are directional Reversibility: clean Ticket: DOC-6809 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
🧠 Redis MemoryFound 5 related items from repository history:
Memory updated at 25c1d25 |
…erdict Add section-level chunk mode (one chunk per feed section + a page anchor, ~15.3k chunks) with checkpoint/resume, alongside the page-level mode. Re-run on the same 35-case eval sharpens the vector picture: - Section-level chunking is the big lift, and it fixes concept: vector MRR .47->.64 (concept), .60->.72 (overall). The coarse page-level chunks were the concept bottleneck, as hypothesised. - Surprise: with strong section-level embeddings, equal-weight RRF hybrid now *dilutes* the top ranks. Pure VECTOR is best by MRR/@1-@3 (overall .72 vs hybrid .67; concept .64 vs .53); HYBRID is best by recall@5/@10 (concept @5 92% / @10 100%). Fusing a strong retriever with a weaker one pulls its confident #1 hits down. Refined decision (SPEC §6/§10): build section-level embeddings + a WEIGHTED fusion favouring vector (not equal-weight RRF), or ship pure vector; tune against this eval before hosting. Checkpoint/resume proved out — this run resumed cleanly from 5120/15300 after the prior session's process exited. Learned: section-level chunks fix vector's concept gap; equal-weight RRF dilutes a strong vector retriever's top ranks (precision vs recall trade-off) Directive: use section-level chunking + weighted fusion (favour vector) for the hosted build; don't equal-weight-RRF a strong vector with weak lexical Gaps: weighted/score fusion not yet measured; small eval (35 cases); single small model (bge-small); sections truncated ~1200 chars Reversibility: clean Ticket: DOC-6809 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
🧠 Redis MemoryFound 5 related items from repository history:
Memory updated at a276698 |
…wins With section embeddings cached, sweep fusion methods on the eval (no re-embedding): pure vector, equal RRF, and weighted RRF favouring vector at 2x/3x/5x. Result: weighted RRF (~2-3x vector) recovers the top-1 precision that equal-weight RRF lost AND keeps hybrid's top-k recall — overall MRR .73 (vs .72 pure vector, .69 equal RRF), command MRR .80, concept @5 92% / @10 100%. So the earlier "RRF dilutes the top ranks" was specifically *equal* weighting; weight vector above lexical and it's the best of both. n=35 so 2x vs 3x is noise. Resolves the open "which fusion" question (SPEC §6/§10): the hosted build is section-level embeddings + vector-weighted RRF. Learned: equal-weight RRF dilutes a strong retriever; weighting it ~2-3x above the weak one recovers top-1 precision while keeping top-k recall Directive: hosted build = section-level embeddings + vector-weighted RRF (not equal-weight); don't read 2x-vs-3x as significant at n=35 Reversibility: clean Ticket: DOC-6809 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
🧠 Redis MemoryFound 5 related items from repository history:
Memory updated at 0c3e7df |
Add a Paice/Lancaster stemmer (src/stem-paice.ts, standard Lancaster rule table) switchable via STEMMER=paice (default stays Porter, zero runtime impact when unused), and bake it off on the eval. Verdict: Porter wins or ties — Porter overall @5 69% / MRR .53 vs Paice 63% / .52. Paice edges @1 (aggressive conflation occasionally nabs the exact top hit; command @1 45% vs 41%) but is worse at @3-@10 and on concept, and over-stems (organization->org, maximum->maxim), which pulls in false matches and costs mid-rank recall. Kept Porter as the default analyzer. Small margins at n=35, so "Paice isn't better here", not "Paice is bad". And it matters least where we're headed: in the vector-weighted-RRF hybrid recipe lexical is the minority signal, so the stemmer mainly affects the pure-lexical (stdio/no-datastore) deployment — where Porter is the better default. Learned: Paice/Lancaster over-stems this technical corpus (organization->org); marginally better @1, worse mid-rank/concept than Porter; net keep Porter Directive: keep Porter as default; STEMMER=paice env flag left in for re-runs; stemmer choice is second-order given hybrid weights lexical low Reversibility: clean Ticket: DOC-6809 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
🧠 Redis MemoryFound 5 related items from repository history:
Memory updated at f54a8bb |
…ge-small Embed the section-level corpus with bge-base-en-v1.5 (per-model cache) and compare at the chosen weighted-RRF 3:1 recipe. bge-base overall MRR .74 vs bge-small .73, command .82 vs .80, concept .61 vs .62 — recall@5 identical (91/91/92). The only real gain is command (already strong); concept (the weak spot) is flat within n=13 noise. bge-base costs ~2x the vector size + compute and embedded at half the speed (~3 vs ~6-11 chunks/s), so it isn't justified. Keep bge-small for the hosted build. Learned: a bigger embedding model (bge-base) gives only a rounding-error overall gain here, concentrated on already-strong command queries, nothing on concept — bge-small is good enough Directive: use bge-small for the hosted build; revisit a bigger model only if the eval grows and concept is still short Reversibility: clean Ticket: DOC-6809 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
🧠 Redis MemoryFound 5 related items from repository history:
Memory updated at 0001afe |
…ake-off The bge-base bake-off (0001afe) committed only the README documenting the result; the eval_vector.py change that actually enabled running a second model was left uncommitted in the working tree. Restore it so the tree matches the documented state. Makes the embedding model selectable via EMBED_MODEL (default stays bge-small-en-v1.5), gives non-default models their own per-model cache file (default model keeps embeddings-{mode}.npz for back-compat), and detects the embedding dimension from the first batch instead of hard-coding DIM=384 (bge-base is 768-dim, so the old constant would have mis-shaped the array). Learned: a documented experiment can leave its enabling code uncommitted if only the write-up is staged; check the tree matches the claim Directive: default path is byte-for-byte unchanged; re-run other models with EMBED_MODEL=BAAI/bge-base-en-v1.5 Reversibility: clean Ticket: DOC-6809 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…line eval First step of the hosted-phase prototype (redis-eval/, sibling to vector-eval/). Loads the cached bge-small section vectors into a Redis 8 FLAT/COSINE index and checks that KNN + app-layer weighted RRF reproduces the offline numpy ranking, holding embedding constant (same cached corpus + Python query vectors on both sides) so the only variable is Redis vs numpy. It does: command metrics are bit-identical (MRR .795), KNN p50 4.1ms at K=200. The lone concept-MRR delta (.638->.599) is a single query where redis-py/connect and ioredis/connect carry an exactly-equal cosine score (gap 0.00e+00) that numpy's stable argsort and Redis's internal order tie-break differently — parity to tie-breaking, not a ranking defect (diag_divergence.py proves it). That tie also exposed a corpus quirk, not a Redis one: client "connect" pages embed identically because their section anchor text is templated per-client, so the embedding can't separate redis-py from ioredis for a "Python" query — a chunking lever for later, not now. Learned: RediSearch FLAT reproduces numpy cosine ranking to within tie-breaking; equal-score chunks are the only divergence and don't move recall@k/MRR except at an exact tie Constraint: keep app-layer weighted RRF favouring vector ~2-3x — command parity depended on fusing the Redis vector ranking with the dumped lexical exactly as the offline recipe did Directive: store owner (page URL) as an unindexed hash field returned via RETURN, NOT in the index SCHEMA — redis-py rejects a non-indexed non-sortable field, and owner never needs indexing Gaps: parity used Python-embedded query vectors; Node/ONNX embedding parity is Step 2 and still unverified Ticket: DOC-6809 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
🧠 Redis MemoryFound 5 related items from repository history:
Memory updated at 27084d0 |
…n fastembed Confirms that embedding queries in-process in the Node MCP server is safe. Node `fastembed` (v2.1.0, the Node port of the same Qdrant fastembed the Python eval uses) embeds bge-small identically to Python: cosine 1.00000 across all 545 compared texts (35 queries + a 510-chunk corpus sample), 0 below 0.999. So the cached Python corpus vectors and every offline number stay valid regardless of which language does the embedding. Input drift is eliminated by having Python export the exact strings both sides embed. The eval pure-vector metrics are identical; the only movement is wrrf-v3 command MRR .795->.789 on one query — the same RRF tie-break sensitivity noted in Step 1, where cosine-1.0 is not bit-identical so a couple of near-tied chunks reorder in the tail and fusion flips one fused rank while the vector-only metric doesn't move. Not an embedding discrepancy. Learned: fastembed-js reproduces Python fastembed bge-small vectors to cosine 1.0, so embed-in-process-in-Node is safe and cross-language corpus/query mixing is valid Directive: embed with plain embed() and prepend the BGE query prefix manually — do NOT use queryEmbed/passageEmbed, whose built-in prefix wording can differ from the eval's and silently break parity Constraint: fastembed-js downloads a ~128M model cache to node/local_cache/ (gitignored) — keep it out of git and out of the npm package Reversibility: clean Ticket: DOC-6809 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
🧠 Redis MemoryFound 5 related items from repository history:
Memory updated at ee3e586 |
… weighted RRF Resolves the fusion fork: keep BM25 lexical + weighted-RRF fusion in the app layer, use Redis only for vector KNN. Redis 8.8 has native FT.HYBRID (8.4.4+), but it does not reproduce the validated recipe (vector ~3x lexical, overall MRR .73) for two independent reasons, and native_hybrid.py decomposes both on the 35-case eval. First, its COMBINE RRF exposes only CONSTANT/WINDOW — no per-retriever weights, so it is equal-weight RRF (the variant the fusion sweep already found dilutes the top ranks); COMBINE LINEAR can weight but fuses raw scores, a different algorithm, and lands at .501. Second and more decisive, its lexical side is Redis's own BM25 over a body-text index (MRR .117, 0% command recall@5) versus our Porter-stemmed, field-boosted Node ranker (.530): a command query like "append an entry to a stream" carries no xadd token, so plain body-text BM25 ranks streams tutorials above the XADD page, and only the Node ranker's title/slug boosts + page-type weighting fix that. app-wrrf over our lexical + Redis vector reproduces Step 1 at .731; the same fusion over Redis's own BM25 collapses to .431. A corollary worth remembering: RRF gives every input list fixed rank-reciprocal mass regardless of that list's quality, so it injects a bad retriever's distractors — which is why app-wrrf-over-Redis-BM25 (.431) scored below native LINEAR (.501) on the same signals; RRF only wins when both retrievers are good. This matches SPEC section 6 — lexical has to live app-side for the stdio no-datastore mode anyway. Learned: native FT.HYBRID can't express our recipe — RRF is equal-weight-only and its raw BM25 lexical is far weaker than our tuned Node ranker (0% command recall@5); the all-in-Redis showcase costs ~.23 MRR Constraint: lexical BM25 + weighted-RRF fusion stay app-side; Redis is the vector-KNN backend only — do not route search through FT.HYBRID Directive: RRF is only safe when every fused list is a good retriever (fixed rank-reciprocal mass injects a bad list's distractors); if a weak signal must be fused, prefer LINEAR score fusion with a low weight Directive: FT.HYBRID call gotchas are in redis-eval/README — VSIM @field $param, KNN/COMBINE counts are k/v-pair counts, no DIALECT, project with LOAD not RETURN Gaps: native BM25 was queried naively (OR of terms, body-only index); a Redis index mirroring the Node analyzer would narrow the lexical gap but not enable weighted fusion Ticket: DOC-6809 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
🧠 Redis MemoryFound 5 related items from repository history:
Memory updated at 72b91c8 |
…N + BM25) First change to the shipped server: a hosted hybrid search mode behind a REDIS_URL feature flag, implementing the recipe the measure-first work (Steps 1-3) validated. When REDIS_URL is set, search_docs fuses the existing in-memory BM25 lexical ranker with vector KNN from Redis via weighted reciprocal-rank fusion (vector ~3x); the query is embedded in-process with fastembed-js. Without REDIS_URL the server stays lexical-only and datastore-free (the stdio mode), and that default path is deliberately untouched. Redis is the vector backend only — lexical and fusion stay app-side, per the Step 3 finding that native FT.HYBRID can't express the weighted recipe. New modules: embed.ts, chunk.ts (a faithful port of the Python build_chunks so Node corpus vectors match the offline ones), vector-store.ts, hybrid.ts; a build-time loader (scripts/load-index.mjs) and a through-the-tool eval (test/eval/run-hybrid.mjs). Verified live against local Redis 8.8: hybrid overall MRR .704 / command .783 / concept .570 (concept @10 100%), within tie-break noise of the offline .731 — the gap stacks the Step 1 KNN and Step 2 embedding tie-breaks, not a ranking difference. Lexical-only default unchanged (.525) and smoke passes. Net payoff: overall MRR .53 -> .70, concept @10 77% -> 100%. Two things bit and are worth flagging. Making search_docs async (the hybrid path must await embedding + Redis) silently broke every synchronous caller — run.mjs, dump-lexical.mjs, smoke.ts — which failed only at runtime, not at compile time, because they discarded the now-Promise return. And node-redis v6 renamed its schema enums (SCHEMA_FIELD_TYPE / SCHEMA_VECTOR_FIELD_ALGORITHM, not SchemaFieldTypes / VectorAlgorithms) — that one does fail at compile time. For iteration speed the loader seeded vectors from the cached Python dump rather than re-embedding 15k chunks in Node at ~4/s; Step 2 proved the two are cosine-1.0 identical, so it is equivalent, and the production --embed path is still there. Learned: hybrid lifts overall MRR .53->.70 and concept @10 77%->100% live through the real tool; the ~.03 gap under offline .73 is stacked FLOAT32 KNN + embedding tie-break noise, not a ranking regression Constraint: the no-REDIS_URL lexical-only path must stay datastore-free and behaviourally unchanged — it is the stdio deployment (SPEC §6); the feature flag is the only thing that turns on Redis Directive: making a tool handler async ripples to every caller — after such a change grep for sync callers (eval/dump/smoke scripts), they fail at runtime not compile time Directive: the loader --vectors seed path is a dev shortcut (valid because Step 2 proved Node==Python vectors); production must run load-index without it so corpus embedding goes through fastembed-js Gaps: no HTTP/SSE endpoint, rate-limiting or deploy yet (stopped at the vertical slice for review); FLAT index only (HNSW deferred); package-lock is gitignored so deps float on ^ranges Ticket: DOC-6809 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
🧠 Redis MemoryFound 5 related items from repository history:
Memory updated at 0ccf6c3 |
…ast top-k Two safe fixes from the Bugbot review of the hybrid work. (1) main() now throws if loadFeed returns zero pages, instead of advertising readiness over an empty index and then serving only empty results / not-found errors (Medium 3542934577). (2) search() scores into an interim array and defers the per-section matchingSections() re-analysis until after the top-k slice, so it runs for the ~10 returned pages rather than every positive-score page in the corpus (Low 3638621205). Verified the search refactor preserves ranking exactly: lexical eval still overall MRR .525 / command .571, hybrid still .704 / .783, smoke all green. Those eval numbers are the regression oracle for any change to the scoring loop. Learned: the 35-case eval (npm run eval / eval:hybrid) is the regression oracle for search() changes — lexical .525 / hybrid .704 must still hold after touching the scoring loop Directive: matchingSections is display-only metadata and must not influence ranking — compute it after sort+slice, never let it move results Ticket: DOC-6809 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
🧠 Redis MemoryFound 5 related items from repository history:
Memory updated at a8b2731 |
Tidies the Step-4 hybrid module boundary in response to two Bugbot findings, both about the feature-flag boundary not being clean. (1) index.ts no longer statically imports HybridSearcher/VectorStore — it `await import()`s them inside the `if (REDIS_URL)` block, so the default lexical-only stdio mode no longer loads fastembed (native onnxruntime) or the redis client at startup. This restores the invariant the Step-4 note claimed but the implementation broke: the no-REDIS_URL path is genuinely dependency-free, and can't crash on a platform lacking the native ONNX binary when it isn't even doing vector search (Medium 3644667453). (2) The normalizeUrl helper, previously copy-pasted byte-identically into search.ts, chunk.ts, hybrid.ts and vector-store.ts, is now a single export in url.ts — the hybrid path matches pages across those modules by normalized url, so a divergent copy would silently break cross-module findability (Low 3644535220). Lexical .525 / hybrid .704 and smoke unchanged after the refactor. Learned: my own Step-4 static imports violated the dep-free-default-path constraint I'd documented — a feature flag gates *use* but a static import still loads the module graph; only dynamic import actually defers the cost Constraint: the no-REDIS_URL lexical path must not statically import hybrid.ts/vector-store.ts/embed.ts (they pull in native onnxruntime + redis) — load them via dynamic import inside the REDIS_URL branch only Constraint: normalizeUrl has ONE definition (url.ts); the chunker, vector store, hybrid fusion and lexical index all match pages by its output, so a second copy that drifts breaks findability silently Ticket: DOC-6809 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
🧠 Redis MemoryFound 5 related items from repository history:
Memory updated at defc966 |
…d path The --vectors seed path built a Float32Array view directly over the readFile Buffer (`new Float32Array(buf.buffer, buf.byteOffset, ...)`). A pooled Node Buffer's byteOffset isn't guaranteed 4-byte aligned, and an unaligned offset makes the typed-array view throw RangeError. It worked in every test only because our ~23MB vectors.f32 gets a dedicated allocation at offset 0 — not a contract. Now copies into a fresh 0-offset ArrayBuffer first (Low 3644788302). Verified: loader still seeds 15300 vectors and hybrid eval holds at MRR .704. Learned: a Float32Array view over a Node Buffer can throw on a non-4-aligned pooled byteOffset — large files hide it (offset 0); copy into a fresh ArrayBuffer before viewing, don't trust the direct view Ticket: DOC-6809 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
🧠 Redis MemoryFound 5 related items from repository history:
Memory updated at dbd7987 |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit dbd7987. Configure here.
Two Low findings from Bugbot's round-4 re-scan, both in Step-4 code. (1) embed.ts memoized the FlagEmbedding.init() promise with `??=`, which permanently caches a *rejected* init — a transient first-run failure (model download timeout, fs perms) would poison the singleton and break hybrid mode until process restart. Now clears the cache on rejection so the next embedQuery retries (3645222048). (2) load-index.mjs accessed chunks[0].vec.length with no empty guard, throwing an opaque "Cannot read properties of undefined" on an empty/invalid feed; now it fails with a clear message, and fromEmbed() bails before loading native ONNX to embed nothing — which also removes an ugly libc++abi teardown crash on the empty path (3645222055). Verified: smoke green, seed path still loads 15300 vectors, hybrid MRR .704 unchanged, empty-feed now errors cleanly on both embed and seed paths. Learned: a promise memoized with `??=` caches rejections too — for lazy init of a fallible singleton (model load, DB connect) always clear the cache in .catch() so a transient failure doesn't permanently wedge the feature Ticket: DOC-6809 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
🧠 Redis MemoryFound 5 related items from repository history:
Memory updated at 2bdf854 |
… validation Independent Codex review of the Step-4 hybrid code surfaced a deeper class of issue than Bugbot's line-level drip — resource lifecycle, error suppression, resilience, input validation. Fixes the four highest-value findings; two production-grade items (atomic index swap on reload; FT.INFO schema/population verification at startup) are left as in-code TODOs, deferred as beyond the offline single-instance prototype. - Graceful degradation (hybrid.ts): a transient embedding or Redis KNN failure no longer fails search_docs — it logs and returns the already-computed lexical results. Hybrid is an enhancement over a working lexical base, so it should fall back to it, not collapse. - Redis lifecycle (index.ts): the store is now closed on SIGINT/SIGTERM and on connection close, and on startup failure — previously the open socket could keep the stdio process alive after the client disconnected. - dropIndex error scoping (vector-store.ts): only the "index doesn't exist" error is swallowed; auth/network/permission errors propagate instead of being silently ignored and letting the loader reuse a stale index. - Seed validation (load-index.mjs): the --vectors seed is checked for dim == EMBED_DIM, owners length == n, and byteLength == n*dim*4 before loading, so a truncated/mismatched dump fails loudly instead of "loading" and being silently misindexed by Redis. - EMBED_DIM moved to a dep-free constants.ts so vector-store and the loader's seed path can validate the dimension without importing embed.ts (which pulls in fastembed's native ONNX runtime) — same dep-free-default-path principle as the dynamic-import fix. Verified: smoke green, lexical MRR .525 and hybrid .704 both unchanged, validated seed still loads 15300 vectors. Learned: the independent Codex sweep caught a whole class Bugbot's per-line scan missed (lifecycle/resilience/validation) in one pass — worth running once on a large new subsystem rather than waiting out the bot's round-by-round drip Constraint: hybrid search MUST degrade to lexical-only when the vector path (embedding or Redis KNN) fails — never fail search_docs when lexical results exist Constraint: the Redis backend must be closed on shutdown (signals + connection close) so a live socket can't keep the stdio server alive after disconnect Directive: EMBED_DIM lives in constants.ts (dep-free) — import it there, not from embed.js, in any module that must stay off the fastembed/native-ONNX graph (vector-store, loader seed path) Ticket: DOC-6809 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
🧠 Redis MemoryFound 5 related items from repository history:
Memory updated at 0df2793 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note: not finished - just using the PR for Bugbot feedback so far.
What
Investigation (DOC-6809) into a read-only docs MCP server — a knowledge-plane server that exposes the Redis documentation corpus as agent-queryable tools, distinct from the existing data-plane
redis/mcp-redis(which reads/writes a live Redis instance). It's read-only, needs no database or credentials, and returns citations to our docs.It reuses the feed we already publish (Hugo per-page
index.json→generate_ndjson.py→docs.ndjson), so there's no new content pipeline.Contents
build/docs-mcp-server/SPEC.md— design: 5-tool surface (search_docs,get_page,get_section,get_examples,get_command), deployment/search-backend matrix, phased plan.build/docs-mcp-server/node/— a working v0 prototype (TypeScript,@modelcontextprotocol/sdk, stdio) with a self-contained BM25 index and two tools:search_docs+get_page. Includes an offline smoke test (npm run smoke).Findings from running v0 against the live feed (2,531 pages)
Status
Investigation / prototype. Not wired into any published site.
search_docs/get_pageonly;get_examples,get_section,get_commandare v1.🤖 Generated with Claude Code
Note
Low Risk
New build-only prototype and docs; read-only public corpus, no auth or production wiring in this PR.
Overview
Adds
build/docs-mcp-server/as a new investigation package: a knowledge-plane MCP server over existingdocs.ndjson, separate from data-planemcp-redis.Design (
SPEC.md) defines a five-tool surface, stdio vs hosted deployment, lexical v1 vs vector-on-Redis v2, and defersversionfiltering until the feed has a real multi-version model.Shipped prototype (
node/) is a TypeScript stdio MCP server withsearch_docsandget_page: in-memory BM25 (Porter stemming, field/page-type boosts), NDJSON feed load, ambiguousid/ partial-urlhandling, MCPisErroron failed lookups, and refusal to start on an empty index. WithREDIS_URL,search_docsswitches to hybrid mode (lexical + Redis section-vector KNN + weighted RRF, query embed via fastembed-js);scripts/load-index.mjsbuilds the vector index.Measurement harnesses add
vector-eval/(offline numpy + bge-small, fusion sweep) andredis-eval/(Redis KNN parity, Node embed parity, nativeFT.HYBRIDcomparison, vector dump for loader seeding), plusnpm run eval/eval:hybridon a 35-case suite..claude/state/assess-comments.coverage.mdis updated with evidence from PR #3585 (capability counts, bugbot drip-feed / fix-quality notes)—ledger maintenance only, not product code.Reviewed by Cursor Bugbot for commit 753fd94. Bugbot is set up for automated code reviews on this repo. Configure here.