fix(clp-mcp-server): Return an error when a KQL query has no results.

[20001020ycx]

Description

When no log events match a given KQL query, the MCP server currently returns {"Error":"Page index is out of bounds."}, which is confusing for both end users and LLMs. This PR adds an explicit check for this scenario and includes unit tests.

Checklist

• The PR satisfies the contribution guidelines.
• This is a breaking change and that has been indicated in the PR title, OR this isn't a
  breaking change.
• Necessary docs have been updated, OR no docs need to be updated.

Validation performed

1. Unit Tests: uv run --group dev pytest tests/test_session_manager.py -vv
2. Lint Tests: uv run ruff check clp/components/clp-mcp-server && task lint:check-py.
3. Manual Integration Tests: Built using task docker-images:package with docker compose. Connected Claude Desktop agent to the running MCP server service in docker compose. Manually instructed LLM to perform a kql that returns empty result:

Summary by CodeRabbit

• Bug Fixes

  • Queries that return no results now surface a clear error: "No log events found matching the KQL query." — prompting users to broaden their search when no log events match.

• Tests

  • Added tests validating error propagation before instructions are provided and when queries yield no results; removed an outdated no-instruction test and added related test constants.

[coderabbitai]

Walkthrough

Added an empty-results failure path to SessionState.cache_query_result_and_get_first_page to return a structured error when a query yields no log events; tests updated to cover both uninitialized-instructions and empty-result error cases.

Changes

Cohort / File(s)	Summary
Implementation components/clp-mcp-server/clp_mcp_server/server/session_manager.py	Added early-return logic in SessionState.cache_query_result_and_get_first_page to detect empty query results and return {"Error": "No log events found matching the KQL query."}. Preserved existing _GET_INSTRUCTIONS_NOT_RUN_ERROR behavior and updated the docstring to document the new error return.
Tests components/clp-mcp-server/tests/test_session_manager.py	Added EMPTY_LOG_ENTRIES = 0 and NO_RESULTS_FOUND_IN_KQL_QUERY_ERR constants. Removed test_no_get_instruction and added test_error_handling, which asserts error propagation when get_instructions() has not been called and when a query returns zero results; verifies errors appear in first_page and page_data.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant SessionState
    rect rgb(245,250,255)
    Note right of SessionState: cache_query_result_and_get_first_page flow
    end
    Client->>SessionState: cache_query_result_and_get_first_page(results)
    alt results empty
        SessionState-->>Client: {"Error":"No log events found matching the KQL query."}
    else get_instructions not run
        SessionState-->>Client: _GET_INSTRUCTIONS_NOT_RUN_ERROR
    else results present
        SessionState->>SessionState: cache results (PaginatedQueryResult)
        SessionState-->>Client: return first page and metadata
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title Check	✅ Passed	The pull request title "fix(clp-mcp-server): Return an error when a KQL query has no results." directly and accurately describes the main change in the changeset. The code modifications in session_manager.py implement exactly what the title promises: checking for empty results and returning an error message. The title is concise, specific, and clear enough that a teammate scanning the history would immediately understand the primary change. It avoids vague language and accurately represents the purpose of the fix.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6abc934 and bd66a0c.

📒 Files selected for processing (2)

• components/clp-mcp-server/clp_mcp_server/server/session_manager.py (1 hunks)
• components/clp-mcp-server/tests/test_session_manager.py (3 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

components/clp-mcp-server/tests/test_session_manager.py (2)

components/clp-mcp-server/clp_mcp_server/server/session_manager.py (5)

• SessionState (58-160)
• cache_query_result_and_get_first_page (79-102)
• cache_query_result_and_get_first_page (200-210)
• get_page_data (113-151)
• get_instructions (104-111)

components/clp-mcp-server/clp_mcp_server/server/server.py (1)

• get_instructions (31-40)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: package-image
• GitHub Check: lint-check (ubuntu-24.04)
• GitHub Check: lint-check (macos-15)
• GitHub Check: build (macos-15)

🔇 Additional comments (4)

components/clp-mcp-server/clp_mcp_server/server/session_manager.py (1)

86-87: LGTM!

The documentation update accurately describes the error return format for failures.

components/clp-mcp-server/tests/test_session_manager.py (3)

30-30: LGTM!

The constant clearly represents the empty results scenario for testing.

---

45-45: LGTM!

The constant is used for substring matching in assertions, which is appropriate for identifying the specific error condition.

---

120-141: LGTM!

The test effectively validates both error scenarios:

1. Proper error when get_instructions() hasn't been called (lines 128-134)
2. Proper error when query results are empty after get_instructions() is called (lines 137-140)

The test correctly verifies the order of checks in the implementation.

[LinZhihao-723]

Can we drop "Try broadening your search criteria."? Or AI must need this hint to proceed?

[20001020ycx]

emmm, yeah, agreed, this error should be general enough. But didn't we already violate this generosity when designing this error message:

    _GET_INSTRUCTIONS_NOT_RUN_ERROR: ClassVar[dict[str, str]] = {
        "Error": "Please call `get_instructions()` first to understand how to use this MCP server."
    }

I think the immediate move is to follow what you have suggested. Then we should refactor in a way that these functions should just return some generic error code, then add another layer to add on the instruction/hint for AI to better understand.

[LinZhihao-723]

I think this example message is fine, since AI must first call get_instruction.
But I am not sure if the one I commented is the same. AI could terminate search instead of broading the query, right?

[20001020ycx]

Well, the decision is up to AI, no matter what we do, we are merely giving a hint. Even with get_instructions, simply based on the dostring in the mcp tool call get_instructions(), AI is able to invoke it most of the time as the first API call from our empirical experience. Adding the static check is just for a sanity check.

Similar thing goes with this line, even with our current implementation, that the returned error message is confusing, AI will continue doing the search rather than stop from our empirical experience.

[LinZhihao-723]

Sure. But:

• Without calling get_instructions, we shouldn't proceed any tool calls.
• If a query returns nothing, broading the search might not be the action to take all the time.

[LinZhihao-723]

For the PR title, how about:

fix(clp-mcp-server): Return an error when a KQL query has no results.