[UPDATE PRIMITIVE] Auto-infer codeql_query_run format from @kind to enable result caching#275
[UPDATE PRIMITIVE] Auto-infer codeql_query_run format from @kind to enable result caching#275
Conversation
Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>
data-douser
left a comment
There was a problem hiding this comment.
@copilot The current changes look alright, but there must be some improvement client integration tests for client/integration-tests/primitives/tools/codeql_query_run/** in order to ensure that the changes actually address the reported problem with query results caching. Use TDD methodology to detect (red) the reported problem and validate (green) the solution, iterating as necessary to ensure feature reliability.
Co-authored-by: data-douser <70299490+data-douser@users.noreply.github.com>
Added a client integration-test fixture TDD validation (locally, before pushing the fixture):
The other JS/independent |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
There was a problem hiding this comment.
Pull request overview
Fixes codeql_query_run result post-processing so that when format is omitted (and no legacy evaluationFunction is used), the server infers an appropriate default format from the query’s @kind metadata. This enables interpretation (SARIF/graphtext) and therefore query-results caching for problem, path-problem, and graph queries—matching the tool’s documented behavior.
Changes:
- Infer
effectiveFormatfrom@kind(problem/path-problem→sarif-latest,graph→graphtext) whenformatis not provided. - Add unit tests covering the new inference branches and explicit-format override.
- Add a client integration fixture asserting interpretation + caching markers appear when
formatis omitted; update bundle + changelog.
Show a summary per file
| File | Description |
|---|---|
| server/src/lib/result-processor.ts | Infers default output format from query @kind before deciding whether to interpret/cache results. |
| server/test/src/lib/result-processor.test.ts | Adds Vitest mocking for CLI execution and new unit tests validating @kind→format inference behavior. |
| client/integration-tests/primitives/tools/codeql_query_run/auto_cache_problem_kind/test-config.json | New integration test asserting interpreted output + cache-key markers when format is omitted for a @kind problem query. |
| client/integration-tests/primitives/tools/codeql_query_run/auto_cache_problem_kind/before/monitoring-state.json | Adds the “before” monitoring state for the new integration fixture. |
| client/integration-tests/primitives/tools/codeql_query_run/auto_cache_problem_kind/after/monitoring-state.json | Adds the “after” monitoring state for the new integration fixture (successful tool call recorded). |
| server/dist/codeql-development-mcp-server.js | Re-bundled server output reflecting the updated result processor logic. |
| CHANGELOG.md | Documents the bug fix under Unreleased → Fixed with PR link. |
Copilot's findings
- Files reviewed: 6/8 changed files
- Comments generated: 1
| // Resolve effective format: when caller did not explicitly request a format | ||
| // and is not using the legacy evaluationFunction, infer one from the query's | ||
| // @kind metadata so that results from problem/path-problem/graph queries are | ||
| // automatically interpreted and cached. | ||
| let effectiveFormat: string | undefined = format as string | undefined; | ||
| if (!effectiveFormat && !evaluationFunction && queryPath) { | ||
| try { | ||
| const metadata = await extractQueryMetadata(queryPath); | ||
| if (metadata.kind === 'problem' || metadata.kind === 'path-problem') { | ||
| effectiveFormat = 'sarif-latest'; | ||
| } else if (metadata.kind === 'graph') { | ||
| effectiveFormat = 'graphtext'; | ||
| } | ||
| if (effectiveFormat) { | ||
| logger.info(`No format specified; defaulting to '${effectiveFormat}' for @kind ${metadata.kind} query`); | ||
| } | ||
| } catch (metaErr) { | ||
| logger.error('Failed to infer default format from query metadata:', metaErr); | ||
| } |
codeql_query_run's post-processor returned early when noformatwas supplied, so@kind problem/path-problem/graphqueries skipped BQRS interpretation and were never written to the query results cache — despite the tool description already advertising kind-based defaults.The post-processor now reads the query's
@kindmetadata and selects a default format when none is given (and no legacyevaluationFunctionis in play), so SARIF/graphtext output is produced and cached automatically.📝 Update Information
Primitive Details
codeql_query_runThis PR is for updating an existing MCP server primitive and must ONLY include these file types:
✅ ALLOWED FILES:
server/src/**/*.ts)server/src/tools/*.ts)server/test/**/*.ts)README.md, server docs)server/src/types/*.ts)server/src/lib/*.ts)package.json,tsconfig.json)🚫 FORBIDDEN FILES:
Rationale: This PR should contain only the files necessary to update and test the primitive.
🚨 PRs that include forbidden files will be rejected and must be revised.
🛑 MANDATORY PR VALIDATION CHECKLIST
BEFORE SUBMITTING THIS PR, CONFIRM:
CHANGELOG.md, the rebuiltserver/dist/bundle, and a newclient/integration-tests/primitives/tools/codeql_query_run/auto_cache_problem_kind/fixture)result-processor.tsUpdate Metadata
formatcontinues to take precedence🎯 Changes Description
Current Behavior
processQueryRunResultsreturns early wheneverformatisundefinedand no legacyevaluationFunctionis set, skipping BQRS interpretation, SARIF/graphtext rendering, and the auto-cache write that depends on the interpreted output. Callers had to passformatexplicitly to get cached results, contradicting the documented default in the tool'sformatschema description.Updated Behavior
When neither
formatnorevaluationFunctionis provided, the processor inspects the query's@kindmetadata and applies a default:problem/path-problem→sarif-latestgraph→graphtexttable) → unchanged (still returns early; nothing to interpret)Inferred and explicit formats then flow through the existing interpret + cache path identically.
Motivation
The tool description already promised this behavior; auto-caching is a prerequisite for downstream tools (
query_results_cache_retrieve, false-positive triage prompts) to find results without forcing every caller to know the format conventions.🔄 Before vs. After Comparison
Functionality Changes
API Changes
No schema changes. The
formatparameter oncodeql_query_runremains optional with the same enum.Output Format Changes
No structural change to tool output. For affected queries, the response now includes the existing "Query results interpreted successfully…" / "Results cached with key…" lines instead of returning the raw
query runstdout untouched.🧪 Testing & Validation
Test Coverage Updates
@kindbranchtable-kind no-op and explicit-format-overrideValidation Scenarios
format: 'csv'on a@kind problemquery still routes tocsv(override test).@kind problem,@kind path-problem,@kind graphqueries triggerbqrs interpretwith the inferred format. End-to-end client integration test (auto_cache_problem_kind) assertsQuery results interpreted successfully with format: sarif-latestandResults cached with key:appear in the tool response whenformatis omitted.extractQueryMetadatauses an mtime-keyed in-memory LRU, so the extra read is amortized across calls.Test Results
result-processor.test.tsincluding 5 new cases)codeql_query_run/auto_cache_problem_kindclient integration fixture passes viagh-ql-mcp-client integration-tests; TDD-validated red→green by reverting the fix locally (RED: assertion failure on missingQuery results interpreted successfully…line; GREEN: PASS after restoring the fix). Other JS/independentcodeql_query_runfixtures (basic_query_run,custom_log_directory,evaluator_logging_with_tuple_counting,javascript_tools_print_ast) continue to pass.📋 Implementation Details
Files Modified
server/src/lib/result-processor.tsserver/test/src/lib/result-processor.test.ts,client/integration-tests/primitives/tools/codeql_query_run/auto_cache_problem_kind/{test-config.json,before/monitoring-state.json,after/monitoring-state.json}server/dist/codeql-development-mcp-server.js(.map)CHANGELOG.md(Unreleased / Fixed)Code Changes Summary
@kind→ format inferenceDependencies
extractQueryMetadata🔍 Quality Improvements
Bug Fixes (if applicable)
codeql_query_rundoes not auto-cache@kind problem/path-problemresults unlessformatis passed explicitly.processQueryRunResultsreturned before resolving the query path or inspecting@kind, so the interpret + cache branch only ran with an explicit format.queryPathfirst, then inferformatfrom@kindmetadata when the caller did not specify one.bqrs interpretis invoked with the correct inferred format for each kind, and that non-interpretable kinds remain a no-op. A client integration fixture independently asserts the auto-cache markers appear in the tool response whenformatis omitted.Performance Improvements
Not applicable.
Code Quality Enhancements
effectiveFormatvariable makes the explicit-vs-inferred path obviousexecuteCodeQLCommandviavi.hoistedto capture the interpret invocation; client integration fixture exercises the full server-side path🔗 References
Related Issues/PRs
query_results_cache_retrieve#263 (query_results_cache_retrieveschema fix — same caching pipeline)External References
@kindmetadata documented in the in-repocodeql://learning/query-basicsresource.Validation Materials
.qlfixtures with@kind problem,path-problem,graph,table(unit tests);ExampleQuery1.ql(@kind problem) for the client integration fixture🚀 Compatibility & Migration
Backward Compatibility
Breaking Changes (if any)
None.
API Evolution
query runstdout👥 Review Guidelines
For Reviewers
Please verify:
Testing Instructions
Validation Checklist
result-processorandcli-tool-registrytests pass; existing JS-basedcodeql_query_runintegration fixtures continue to pass@kindinference (problem, path-problem, graph, table-skip, explicit-override) plus one client integration fixture asserting the auto-cache markers appear whenformatis omittedextractQueryMetadatafailure path is exercised by an inner try/catch + loggerauto_cache_problem_kindfixture runs the bundled server end-to-end throughgh-ql-mcp-clientand asserts the response containsQuery results interpreted successfully with format: sarif-latestandResults cached with key:📊 Impact Assessment
Performance Impact
extractQueryMetadatacall perquery_runinvocation that omitsformat(cached after first read per file mtime)Server Impact
AI Assistant Impact
query_results_cache_*tools usable without re-running queries🔄 Deployment Strategy
Rollout Considerations
Post-Deployment Validation
Update Methodology: This update follows best practices: