adityamparikh opened a new pull request, #132: URL: https://github.com/apache/solr-mcp/pull/132
## Summary Three MCP tool response records in \`Dtos.java\` carried fields that conveyed no real information to the LLM client. Removing them simplifies the JSON wire format and eliminates legacy \`java.util.Date\` usage entirely. ## Fields removed | Record | Field | Why noise | |---|---|---| | \`SolrMetrics\` | \`timestamp\` (Date) | Always \"now\" — metrics collected synchronously per call. MCP host records call timing already. | | \`SolrHealthStatus\` | \`lastChecked\` (Date) | Same: always \"now\". | | \`SolrHealthStatus\` | \`solrVersion\` (String) | Always null in every code path. \`@JsonInclude(NON_NULL)\` was hiding it on the wire anyway. | | \`SolrHealthStatus\` | \`status\` (String) | Same: always null. | | \`CollectionCreationResult\` | \`success\` (boolean) | Always \`true\` on return — failures throw before reaching the result. | | \`CollectionCreationResult\` | \`message\` (String) | Always the literal \"Collection created successfully\". | | \`CollectionCreationResult\` | \`createdAt\` (Date) | Always \"now\". | After this change: - \`SolrMetrics\` → \`(indexStats, queryStats, cacheStats, handlerStats)\` - \`SolrHealthStatus\` → \`(isHealthy, errorMessage, responseTime, totalDocuments, collection)\` - \`CollectionCreationResult\` → \`(name)\` ## Date → Instant migration: not needed The original motivation was \"why are we using \`java.util.Date\` instead of \`java.time.Instant\`?\" — once every \`Date\` field turned out to be cargo-culted noise rather than load-bearing state, the migration is moot. Both \`java.util.Date\` and \`com.fasterxml.jackson.annotation.JsonFormat\` imports drop from \`Dtos.java\` entirely. ## Wire format impact For LLM clients consuming the existing JSON, the change is purely subtractive — three keys disappear from each affected response. Any code that depended on those values (none in this repo) would break, but since \`@JsonInclude(NON_NULL)\` was already dropping \`solrVersion\` and \`status\` from the wire, only \`timestamp\` / \`lastChecked\` / \`success\` / \`message\` / \`createdAt\` are visible behavioral changes. ## Test plan - [x] \`./gradlew clean build\` — BUILD SUCCESSFUL, all tests pass - [x] \`./gradlew nativeTest -Pnative\` — 157/157 tests pass, 0 failures - [x] Test assertions updated where they referenced removed fields (\`CollectionServiceIntegrationTest\`, \`CollectionServiceTest\`, \`ConferenceEndToEndIntegrationTest\`, \`McpClientIntegrationTestBase\`) - [ ] CI native + docker matrix on the PR ## Related This addresses a sub-thread of review on #131 (schema modification PR), where the new \`SchemaUpdateResult\` followed the same convention and the reviewer asked why we used \`Date\`. After this PR merges, #131 will pick up the same treatment for \`SchemaUpdateResult\` via rebase. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
