Evals for the CAP MCP server - #129
Open
larsPlessing wants to merge 47 commits into
Open
Conversation
Code-only, fully deterministic evals for search_docs (issue cap/cdsnode#2786): pure-arithmetic IR metrics over a frozen, human-authored golden set. No LLM. - metrics: Recall@K, Precision@K, MRR, Hit-Rate@K, nDCG@K (pass@k = Hit-Rate@K) - stable content-derived doc ids; pre-flight aborts on stale relevant_doc_ids - gate thresholds (recall/mrr/hit-rate) with non-zero exit on regression - results appended to runs/result.jsonl (capped to keepRuns); no per-run folders - config.json + EVAL_* env overrides (k, offline, capire_version, gates, keepRuns, resultsName, compareFormat) - npm scripts: evals, evals:show (latest or by run_id), evals:compare (html charts default | md tables via compareFormat), evals:baseline, evals:test - compare dashboard: per-metric trends, per-question sparklines, per-run drill-down - 67 unit tests (metrics, runner, config, ids, cli, compare) Adds evals/ module inside the cloned repo; no new runtime deps, production lib/* untouched.
Remove the separately-pinned data/baseline.json and the evals:baseline promote step. The baseline is now the OLDEST run in runs/result.jsonl: - first run has no baseline (it is the reference); later runs diff against it - store.js: baselineRun() replaces promoteBaseline() - cli.js: reads baseline from existing runs before appending - show.js: finds baseline among the runs already loaded from result.jsonl - config.js: drop paths.baseline / EVAL_BASELINE - remove evals:baseline npm script Caveat documented: result.jsonl is capped to keepRuns, so the oldest (baseline) run slides forward as old runs are pruned. Tests updated (67 pass).
Evaluate the real search_docs tool through its public handler and assert on
what it returns, instead of reusing its internal retrieval functions — so a
change inside search_docs is reflected in (and can fail) the eval.
- retriever: call tools.search_docs.handler({query, maxResults:K}); parse each
returned chunk's id from its first line; keep the raw K slots (no dedup)
- ids: doc identity is a deterministic "<source-url>#<breadcrumb-slug>" parsed
from the chunk first line (parseId/buildIdMap) — no content hashing, no
embeddings involved in identity; survives re-index at page level
- metrics: score the raw K slots (duplicates kept) — Precision counts relevant
slots/K, Recall counts distinct relevant docs found, nDCG clamped to 1
- golden set regenerated (cap-golden-v2) with URL#breadcrumb relevant_doc_ids
- compare: per-run drill-down now lists each question's retrieved top-K results
(ranked, hits marked) alongside its relevant_doc_ids
- docs (METRICS/README) and tests updated; 67 unit tests pass
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![hyperspace-pr-bot[bot]](https://avatars.githubusercontent.com/u/2531208?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
The PR introduces a well-structured, deterministic RAG evaluation harness with solid metric math, good test coverage, and a clean dependency-injection seam for testing. However, there are several real bugs that need attention: (1) A zero baseline value is treated as "no baseline" due to a falsy check (baseAgg[key] instead of baseAgg[key] != null) in runner.js, silently suppressing deltas when a metric was 0.00 in the prior run. (2) process.env.CDS_MCP_OFFLINE is set permanently and never restored, leaking the side-effect across all subsequent run() calls in the same process. (3) parseGateOverrides silently produces NaN for invalid numeric values (e.g. EVAL_GATES='mrr=abc'), and gate validation in validateConfig only covers GATED_KEYS rather than all METRIC_KEYS, so user-set gates on precision_at_k/ndcg_at_k are never range-checked. (4) The evals:show npm script reads process.argv[1] (the Node script path) instead of process.argv[2] (the first user argument), meaning npm run evals:show -- <run_id> always passes the wrong value and the target lookup always fails. Please address these before merging.
PR Bot Information
Version: 1.29.14
- Correlation ID:
3d08eab0-8ce5-11f1-8ee2-d05dba928c8e - Experiment Variant:
control - LLM:
anthropic--claude-4.6-sonnet - Event Trigger:
pull_request.opened - File Content Strategy: Full file content
…ls:show
- add evals/bin/{eval,compare}.js thin CLI wrappers; npm scripts now point at files
- remove evals:show (lib/show.js + bin + script); compare's per-run drill-down covers it
- update README folder tree, run commands, and outputs prose
larsPlessing
changed the title
- config: reject non-numeric gate values; validate gates on all metrics - cli: restore CDS_MCP_OFFLINE after run() (no env leak) - runner: use != null so a 0.00 baseline still yields a real delta - compare: escape backslashes before pipes in markdown cells - tests for each
correctness: - round() symmetric on negatives (no -0; fixes masked boundary regressions) - validate keepRuns (reject 0/fractional); validate golden questions - readRuns skips wrong-shape lines; parseGateOverrides rejects empty value methodology: - nDCG credits distinct relevant docs once (dupes can't clamp-mask ordering) - diagnose(): 0.02 dead-band + reports all causes, not first-match-wins - pin baseline via baselineRunId instead of only sliding-oldest - gate (absolute floor) vs diagnosis (delta) now clearly labeled as separate - makeRunId keeps ms precision so same-second batches sort by time - retriever warns when unparseable chunks are dropped golden set → cap-golden-v3 (42 canonically-labeled questions); real retriever scores below the aspirational gates by design (documented). 92 tests pass.
Drop cap-011..cap-042; keep the canonical-label fixes on the original 10. Update README gate note to the 10-question scores (Recall 0.65 fails).
larsPlessing
changed the title
- bin/ entry points disable no-console explicitly (same as lib/run.js, index.js) - drop unused imports (METRIC_KEYS in cli, key param in lineChartSvg) - eslint evals/ now clean; lib code already routes logs via injected logger
search_docs reads CDS_MCP_OFFLINE at module-load time, so mutating it inside run() was ineffective on the 2nd+ run of a batch (module already cached) and leaked process state. Set it once in bin/eval.js before importing the chain; drop the fragile per-run set/restore + try/finally from run().
PDT42
reviewed
Aug 4, 2026
- parseId returns the Source: URL (already has the #section anchor); drops the breadcrumb-slug/nourl scheme and throws when a chunk has no Source: URL - retriever no longer filters null ids; update tests + docs; trim comments
…chunk text, +coverage - config.label / EVAL_LABEL shown in console + compare reports - parseId + pre-flight now warn instead of aborting; run proceeds - URL-less chunks get synthesized #generated-anker-N ids (inherit page URL); retriever resolves them corpus-consistently by text so full K is kept - compare.html: expand each retrieved id to its chunk text - unit coverage to ~99% line / 100% funcs (new retriever + store tests)
- run() prints a one-line PASS/FAIL confirmation instead of the full summary; remove renderConsole*/renderMetricRow/diagnosisProse/worstQuestions + tests - extract buildPerQuestionModel shared by html+md (was duplicated; also swaps nested .find() for a per-run id→metrics Map) - hoist escHtml/statusIcon/gateStr to module scope; trim comments
resolveIds returns aligned {ids, texts}; retriever exposes lastTexts and the
runner stores retrieved_texts, so compare shows the exact retrieved text even
when a different corpus is live — and two slots sharing an id keep distinct text.
MODEL_NAME in calculateEmbeddings.js reads CDS_MCP_MODEL (default: Xenova/all-MiniLM-L6-v2); MODEL_DIR is now per-model (models/<org>--<name>/) to avoid stale-file conflicts on switch. EVAL_MODEL / config.json model field records and propagates the model to the eval run.
Add EXTRA_FILES for onnx_data sidecars; download them if present (skip 404). Use path-based InferenceSession.create when external data files exist so the runtime can resolve them (buffer-load ignores sidecar files).
…mbed) onnxruntime-web cannot resolve .onnx_data sidecar files in Node.js (Emscripten MountedFiles limitation). When model.onnx_data* files are present, dynamically import onnxruntime-node (already a transitive dep) and load by path instead.
…s labels, 2-col grid
…Source:) Chunks without a Source: URL now get a stable capire://generated/<slug> id derived from the breadcrumb text, so corpora like llm_chunks_no_chunksize are fully retrievable. #generated-anker is now only for truly empty first lines.
PDT42
requested changes
Aug 10, 2026
Comment on lines
+8
to
+11
| // Examples: | ||
| // npm run evals:compare | ||
| // node evals/bin/compare.js --runs runs-xenova/result.jsonl | ||
| // node evals/bin/compare.js --runs runs-pplx/ --out runs-pplx/compare.html |
|
|
||
| const { runAll } = await import('../lib/cli.js') | ||
|
|
||
| runAll() |
|
|
||
| **Example:** at least one relevant doc is in the top 5, so `Hit-Rate@5 = 1`. | ||
|
|
||
| ### nDCG@K — *is the whole ranking well-ordered, not just the first hit?* |
There was a problem hiding this comment.
I guess this only makes sense if we always provide R relevant links in well sorted order.
|
|
||
| `EVAL_GATES` is a comma list: `EVAL_GATES='recall_at_k=0.9,mrr=0.7,ndcg_at_k=null'`. | ||
|
|
||
| ### Useful commands |
There was a problem hiding this comment.
The available configuration options seem excessive.
| @@ -0,0 +1,111 @@ | |||
| import path from 'path' | |||
Comment on lines
+54
to
+66
| export function resolveIds(chunks, corpusChunks) { | ||
| const idByText = new Map() | ||
| for (const [id, text] of chunkEntries(corpusChunks)) { | ||
| if (!idByText.has(text)) idByText.set(text, id) | ||
| } | ||
| const ids = [] | ||
| const texts = [] | ||
| for (const [localId, text] of chunkEntries(chunks)) { | ||
| ids.push(idByText.get(text) || localId) | ||
| texts.push(text) | ||
| } | ||
| return { ids, texts } | ||
| } |
There was a problem hiding this comment.
I don't understand why this would be necessary: What does this actually achieve?
Comment on lines
+38
to
+45
| test('parseId finds Source: URL in the body when not on the first line', () => { | ||
| // LLM-generated or longer chunks may have the Source: URL anywhere | ||
| const chunk = 'CAP Security > Data Privacy\nsome long description\n> Source: https://cap.cloud.sap/docs/guides/security/#data-privacy\nmore body' | ||
| assert.equal(parseId(chunk), 'https://cap.cloud.sap/docs/guides/security/#data-privacy') | ||
| // Markdown heading style | ||
| const chunk2 = 'Heading\nbody\n# Source: /docs/guides/deploy#section\nmore' | ||
| assert.equal(parseId(chunk2), '/docs/guides/deploy#section') | ||
| }) |
| @@ -0,0 +1,127 @@ | |||
| import { metricsFor, relevantHitsAtRank, mean, round } from './metrics.js' | |||
Comment on lines
+127
to
+133
| if (hasExternalData) { | ||
| const ortNode = await import('onnxruntime-node') | ||
| session = await ortNode.InferenceSession.create(modelPath) | ||
| } else { | ||
| const modelBuffer = await fs.readFile(modelPath) | ||
| session = await ort.InferenceSession.create(modelBuffer) | ||
| } |
|
|
||
| const MODEL_NAME = 'Xenova/all-MiniLM-L6-v2' | ||
| const MODEL_DIR = path.resolve(__dirname, '..', 'models') | ||
| const MODEL_NAME = process.env.CDS_MCP_MODEL || 'Xenova/all-MiniLM-L6-v2' |
There was a problem hiding this comment.
We can not "just" switch the model here ... The tokenization and normalization are model specific and may not work with another model. I don't think the capability to use another embeddings model belongs in this PR.
- Revert embedding-model switchability from calculateEmbeddings.js/index.js (model-specific tokenization doesn't belong in this PR; back to hardcoded Xenova/all-MiniLM-L6-v2, onnxruntime-web only) — per @PDT42. - Rename for intent: cli.js→evaluate.js (run→evaluate, runAll→evaluateAndCompare), runner.js→report.js, retriever.js→search-docs.js (makeDefaultRetriever→makeSearchDocsRunner); update tests + docs. - Trim config surface: config.json is the single source; only EVAL_LABEL and EVAL_RUNS_DIR honoured from env. Drop EVAL_K/GATES/CONFIG/COMPARE_FORMAT/ RESULTS_NAME/CAPIRE_VERSION/BASELINE_RUN_ID/GOLDEN_SET/KEEP_RUNS and gate-override string parsing; rewrite README Configuration. - golden-set: capire-2 drop trailing slash (/docs/guides/domain#associations). - bin/eval.js: explain why the wrapper must exist (sets CDS_MCP_OFFLINE before module-load). Tests: 109 eval + 159 server, all pass.
- Add build-source-tree.js → source-tree.json (page→anchors, page-scoped heading→url, breadcrumb→url, leaf→url) from llms-full.txt. - ids.js: replace golden-URL body-substring credit with structural resolution — a chunk is credited for every section it covers; a heading whose Source line was split off at a chunk boundary is recovered from the tree; a breadcrumb-keyed LLM-summary corpus (no Source: lines) resolves via breadcrumb→url. - Thread sourceIndex through search-docs.js + evaluate.js; add evals:build-source-tree script. Lets the live (breadcrumb-keyed) corpus be scored against URL golden ids, so current vs latest embeddings are comparable. 112 tests pass.
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
3 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.
Adds a deterministic evaluation for the MCP server's
search_docs.search_docsoutput (black-box) against a frozen golden set with human-authored relevance labels.result.jsonl, gates against the oldest run, and builds an HTML/Markdown comparison dashboard.config.json/EVAL_*env;npm run evals(N runs + compare),npm run evals:compare,npm run evals:test.