adityamparikh commented on PR #196: URL: https://github.com/apache/solr-mcp/pull/196#issuecomment-5684446901
Rebased onto #205 and pushed. **This PR now depends on #205 and should merge after it.** The first four commits shown are #205's; GitHub cannot retarget the base to a branch on the fork, so they appear here until #205 lands. The rebase was necessary rather than cosmetic. #205 introduces `forward()`, the CSV/XML path. Before the rebase this PR could not reach it, so `a caller that splits a large input across several calls does not pay for a hard commit per call` was only true for JSON and markdown; CSV and XML still issued `setAction(COMMIT, waitFlush=true, waitSearcher=true)`. Neither PR is wrong alone — the gap only exists once both are merged, which is exactly why it survived review twice. Three fixes on top: **Soft commit for CSV and XML** (`a4fb3d3`). Measured against Solr directly, same endpoint and document, median of 10: hard commit 22.79 ms vs soft 8.26 ms. Across the MCP tools, 61 single-document CSV calls go from 1853 ms to 585 ms (per extra call 30.4 ms to 9.5 ms). JSON and markdown, which this did not touch, stayed flat — that control is what makes the number trustworthy rather than a warm-cache artifact. **The size threshold was wrong, and unreachable** (`205a32d`). Observed live: Sonnet 5 in Claude Desktop was asked to index an 86 KB markdown file. After three minutes nothing had reached the server — the model was still emitting the payload as tool arguments, about 76,000 output tokens, past the output budget, so the call could only ever truncate and fail. The instructions said to split inputs larger than `a few hundred KB`; for record data one byte costs roughly one output token, so that threshold is multiples past any output budget. Now ~20 KB per call, with the byte-per-token rule stated so a caller can size its own input, and an instruction to count and batch *before* writing the first call. The guidance also moved into the tool descriptions: `instructions` is returned once at initialize and not every client puts it in the model's context, whereas tool descriptions always are. That reverses `6f15281` deliberately — happy to drop it if you would rather keep the single sour ce. **Input format vs conversion target** (`4b0815f`). `Send data in the format you already have; when you must convert, emit CSV, and never convert to XML` put those two clauses side by side, so a model holding XML could read `never XML` as applying to its input — and converting XML is the worst possible move, since #205 forwards it to Solr unparsed. Now stated separately, with conversion scoped to formats that have no tool of their own. Only `index-markdown-documents` previously carried an as-is rule; all four do now. Build green: 392 tests, 0 failures, 7 skipped (the pre-existing `@Disabled` OTLP class). -- 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]
