adityamparikh opened a new issue, #282: URL: https://github.com/apache/solr-mcp/issues/282
## Summary Measure whether the Solr Agent Skill ([#279](https://github.com/apache/solr-mcp/issues/279), PR [#281](https://github.com/apache/solr-mcp/pull/281)) makes an agent better at using this server's tools, without hurting tool selection or misleading it. Builds on the tool-selection eval harness from #256 (PRs [#264](https://github.com/apache/solr-mcp/pull/264), [#265](https://github.com/apache/solr-mcp/pull/265)). Opt-in and report-only, like #256: `./gradlew build` never calls a model, there is no CI job, and fork PRs get no secrets. ## Problem `SolrSkillFormatTest` checks that the skill is well formed. Nothing checks that it helps. A skill can be well formed and useless (the baseline already behaves correctly), or harmful (it changes tool selection, or states something the server no longer does). Each fact in the skill also has to stay true as the tools change. ## Layers ### Layer 0: skill claims checked in the build (no LLM) Most claims already have tests. Add only the gaps: - Objects nested in a JSON array are dropped: strengthen `IndexingServiceIntegrationTest.testArraysOfObjects` (`:545`), whose expectations are comments today. - Documents without an `id` get a generated id and duplicate on re-send: no test exists. - Collections created with defaults share one schema: add only after [#271](https://github.com/apache/solr-mcp/pull/271) is decided, or the test blocks or inverts it. - Tool names in `SKILL.md`: parse the backticked names and compare them with runtime `listTools()`, as `McpClientIntegrationTestBase` (`:101`) does. `McpToolRegistrationTest` is reflection over four services and skips `AliasService` in its unique-name check. - Error prefixes quoted in `troubleshooting.md`: check server-originated prefixes against the source; keep an allow-list for text that comes from Solr, such as "already exists". - The Solr 9.4 / 10 matrix runs only on push to `main`, so PR builds check the Solr 10 claims against 9.9 only. Say so in the test names or docs rather than assume. ### Layer 1: A/B behaviour evals (opt-in) #### Harness changes needed on top of #264 / #265 | Need | State today | Change | |---|---|---| | Two arms | One `ChatClient`; system prompt is the server instructions only (`EvalConfiguration`) | A second bean or per-prompt `.system()` / `.tools()`; `-Peval.arms` plumbing like `eval.runs` | | Skill access | None | `read_skill_file(path)`: serves a fixed allow-list of the skill's classpath files, never a filesystem path | | Activation logging | `ToolCallAuditAdvisor` records every tool call | None | | Answer-text checks | `check()` scores tool calls only; the answer is never read | Capture the output text | | Judge | Not implemented: no `TypeSafeClient`, `evalJudge` or Laya wiring | Prefer code checks on tool calls and Solr state; add a judge only where a semantic check is unavoidable, with a labelled set of at least 10 cases per judged criterion (#256 spec) | | Run isolation | One shared Solr and one `devnexus` collection; runs are sequential in one cached context | Fresh collection per run; until #271 merges, a fresh configset or fresh Solr too, because `_default` collections share a schema. Per-run collection names let JUnit run in parallel | | Solr 10 | `-Dsolr.test.image` is forwarded per Gradle invocation | A separate `./gradlew eval` run, not a per-scenario switch | | Solr unreachable | Solr is a singleton container; stopping it breaks later scenarios and a restart remaps the port | A separate profile or context whose `SOLR_URL` points at a closed port | | Tool names | The harness shows the model `get_schema`; the skill says `get-schema` | Scorers use the underscore names; check that the mismatch does not penalise the skill arm | | Report | JUnit pass or fail per run; no `summary.md` | Build the pass-rate summary with an interval per scenario and arm | | Controls | Only `workshops-by-type` exists (28/35 baseline) | The other tool-selection scenarios from #256 must land before "must not regress" can run | #### Arms - **Baseline:** tools plus server instructions. - **Skill:** adds the skill's name and description to the system prompt, plus `read_skill_file`. Document the preamble text verbatim; results are sensitive to it. - **Skill inlined:** `SKILL.md` in the system prompt. This splits the lift into P(activation) x content effect, so a failure to activate is not read as useless content. #### Scenarios Each targets something the tool descriptions, prompts, server instructions and error hints do not already say. Pass criteria are code checks on tool calls and Solr state. | Scenario | Pass if | |---|---| | Id-less documents "imported again tomorrow" | every document in the `index-json-documents` arguments has an `id`; after the harness replays the recorded call, `numFound` is unchanged | | Find documents where a reviewer is X, with reviewers as objects inside an array | the answer says the data is not indexed; not graded on a 0-hit search | | Batch with one bad document, ids supplied | after an index result with N < M, the model calls `get-schema` or `add-fields`; end state has all M indexed after the fix and re-send | | CSV or XML, then "how many were indexed?" (fresh collection) | a `search` or `get-collection-stats` call follows the index call, and the number in the answer equals `numFound` | | Solr 10: cache stats after no activity | the model runs a search and asks again, or reports the figures as unavailable | | Two collections, conflicting field name | both datasets are fully indexed (N == M each). Settle #271 first; the premise disappears if it merges | | Solr unreachable | the answer attributes the failure to Solr, not the arguments. At setup, assert the exact tool-result text the harness returns, using `get-schema` or `index-*` if those are the message-less paths | Not included, because the baseline already passes: nested JSON then query by flattened name (the `index-json-documents` description says nested objects are flattened with underscores and the response lists the indexed names). Scenario hygiene: - Write 3 to 5 paraphrases per scenario, blind to the skill text. Hold a set out for the decision. - Source some scenarios from real transcripts (the #265 failures, the LLM-ergonomics PRs #161 to #166), because every row above is derived from the skill text and iterating the skill against the same prompts overfits it. #### Controls - Solr-concept questions ("what is docValues?"): choose one metric, either "reads a reference" or "reads `SKILL.md`", and report activation only. - The #256 tool-selection scenarios, once they exist: no regression. #### Analysis (fixed before any run) - At N = 10 per arm, 5/10 against 7/10 has a 95% interval of about -0.21 to +0.55, so a 20-point lift cannot be told from zero. A per-scenario rule of "+20 points on half the scenarios" passes by chance about 11% of the time with no effect, and fails about 36% of the time with a true +20 on all eight. - **One primary hypothesis:** the pooled lift across the targeted scenarios, stratified by scenario (Cochran-Mantel-Haenszel, or a logistic mixed model with scenario and paraphrase effects). Per-scenario Wilson or Newcombe intervals are descriptive only. - **Pair by paraphrase:** each paraphrase runs in both arms, analysed with McNemar or the mixed model. Interleave or randomise arm order so model drift and rate limits do not confound the result. - **Sample size for 80% power:** about 90 runs per arm for 50% to 70%, about 60 per arm for 70% to 90%. A 5-point non-inferiority margin needs hundreds of runs per arm; use 15 points (about 90 per arm). - Exclude harness errors from both arms the same way, and report them. - Activation (14 of 15 has an interval of about 0.70 to 0.99): report only, no threshold. - "Cut that content" only when the interval excludes a meaningful lift, or the baseline is at ceiling. - Cost is mostly wall-clock: over 1,400 sequential runs at this design. Measure tokens on the first run. #### Triage for a failing scenario | Finding | Action | |---|---| | Skill not activated | fix the description | | Activated, wrong reference read | move content into `SKILL.md` | | Read but ignored | reword | | Scorer bug | fix the scorer | | Baseline at ceiling | the content is redundant; cut it | | Negative lift | fix or remove | ### Layer 2: calibration - **Trigger test:** about 30 labelled prompts (should and should not trigger) through `claude -p` with the skill installed, using the `skill-creator` tooling. This is Claude Code with a Claude model, not the Layer 1 host, so treat it as the authoritative activation number and do not compare the layers directly. - **Ablation:** `SKILL.md` alone against `SKILL.md` plus references. - **Canary:** a deliberately wrong, code-measurable claim in a throwaway copy (for example, that the id field is `doc_id`). The chosen N must detect it, which makes it a positive control for sensitivity. - **Models:** the #256 default, the gateway model, and one Claude model for a smaller confirmatory run. Pin a dated model snapshot in the report header. ## CI None, per #256: report-only, and fork PRs get no secrets. The decision rule is a one-off judgement about the skill, not a gate. ## Acceptance criteria - [ ] Layer 0 gaps closed; `./gradlew build` passes. - [ ] `./gradlew eval -Peval.arms=baseline,skill,skill-inlined` runs the scenarios above and writes a summary with per-arm pass rates and intervals, activation, and tokens. - [ ] The analysis plan (primary hypothesis, N, pairing, exclusion rule) is written into the repo before the first reported run. - [ ] The canary is detected at the chosen N. - [ ] A written decision on the skill: keep, cut or rewrite each section, with the triage category for every scenario that did not show a lift. ## Order of work 1. Layer 0 gaps (no dependencies). 2. After #264 and #265 merge and #271 is decided: the harness changes, then Layer 1. 3. Layer 2 last. ## Related #256, #264, #265, #271, #279, #280, PR #281 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
