Skip to content

MCP: Fix get_cratedb_documentation_index tool #60

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
amotl merged 2 commits into from
Jul 22, 2025
Merged

MCP: Fix get_cratedb_documentation_index tool #60

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a09c39a
MCP: Fix defunct `get_cratedb_documentation_index` tool
amotl Jul 22, 2025
bf94ed2
Example: Make `examples/mcptools.sh` use the current version
amotl Jul 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# CrateDB MCP changelog

## Unreleased
- MCP: Fixed defunct `get_cratedb_documentation_index` tool

## v0.0.4 - 2025-07-21
- Parameters: Added CLI option `--host` and environment variable `CRATEDB_MCP_HOST`
Expand Down
2 changes: 1 addition & 1 deletion cratedb_mcp/tool.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def get_table_metadata() -> dict:
documentation_index = DocumentationIndex()


def get_cratedb_documentation_index() -> dict:
def get_cratedb_documentation_index() -> list:
Copy link

@coderabbitai coderabbitai bot Jul 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

LGTM! Type annotation fix aligns with actual return type.

The change from -> dict to -> list appears to correctly reflect what documentation_index.items() actually returns. This type mismatch likely caused the tool dysfunction mentioned in the PR objectives.

To confirm the fix is correct, let's verify what documentation_index.items() actually returns:

🏁 Script executed:

#!/bin/bash
# Verify the return type of documentation_index.items()
ast-grep --pattern $'class DocumentationIndex {
  $$$
  def items($_) {
    $$$
  }
  $$$
}'

Length of output: 93

Let’s locate and inspect the actual DocumentationIndex implementation and its items() method:

🏁 Script executed:

#!/bin/bash
set -e

# Find the file(s) defining DocumentationIndex
echo "Finding DocumentationIndex definition…"
DOC_FILE=$(rg -l "class DocumentationIndex")

# Dump the start of that file to see its class signature and context
echo "----- $DOC_FILE (head) -----"
sed -n '1,200p' "$DOC_FILE"

# Show exactly how items() is implemented
echo "----- items() implementation -----"
grep -R "def items" -n "$DOC_FILE"

Length of output: 7191

Inspecting get_cratedb_documentation_index implementation:

🏁 Script executed:

#!/bin/bash
# Show the function definition and surrounding context in tool.py
rg -n "get_cratedb_documentation_index" -A5 -B5 cratedb_mcp/tool.py

Length of output: 409

Fix return type annotation for get_cratedb_documentation_index

The call to documentation_index.items() returns the result of .to_dict(), i.e. a Python dict, not a list. Please update the signature accordingly:

• File: cratedb_mcp/tool.py
• Line 43: change

-def get_cratedb_documentation_index() -> list:
+def get_cratedb_documentation_index() -> dict:

(or an equivalent Dict[str, Any] annotation for clarity)

This will realign the annotation with the actual return value and prevent downstream type-checker or client issues.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def get_cratedb_documentation_index() -> list:
-def get_cratedb_documentation_index() -> list:
+def get_cratedb_documentation_index() -> dict:
🤖 Prompt for AI Agents 
In cratedb_mcp/tool.py at line 43, the return type annotation of
get_cratedb_documentation_index is incorrectly set to list, but the function
actually returns a dictionary from documentation_index.items(). Update the
return type annotation to Dict[str, Any] or an equivalent dictionary type to
accurately reflect the returned value and avoid type-checking errors.

Copy link
Member Author

@amotl amotl Jul 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai: Thanks for finding this. We found the to_dict() method uses a semantically wrong name, so this conveyed a wrong impression about its return type. We've created a ticket to fix it on the other project later. Currently, it doesn't do any other harm.

Copy link

@coderabbitai coderabbitai bot Jul 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amotl Thank you for the clarification! That makes perfect sense - having a method named to_dict() that actually returns a list would definitely create confusion about return types. It's great that you've created a ticket to address the semantic naming issue in the other project. This kind of misleading method naming can easily lead to type annotation errors like we saw here.

✏️ Learnings added 
Learnt from: amotl
PR: crate/cratedb-mcp#60
File: cratedb_mcp/tool.py:43-43
Timestamp: 2025-07-22T12:46:59.324Z
Learning: In the cratedb-mcp repository, the DocumentationIndex class has a method named `to_dict()` that actually returns a list, not a dictionary. This misleading method name can cause confusion about return types in functions that call it, such as `get_cratedb_documentation_index()`. A ticket was created to fix this semantic naming issue: https://github.com/crate/about/issues/55

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

🧠 Learnings used 
Learnt from: amotl
PR: crate/cratedb-mcp#17
File: CHANGES.md:5-5
Timestamp: 2025-05-16T13:35:04.487Z
Learning: For the cratedb-mcp repository, version numbers and release dates in CHANGES.md are updated by a dedicated "Release vX.Y.Z" commit as part of the release process, not during documentation preparation PRs.

Learnt from: amotl
PR: crate/cratedb-mcp#0
File: :0-0
Timestamp: 2025-05-17T14:55:23.400Z
Learning: In the cratedb-mcp repository, the `backlog.md` document is intended as a functional planning document that doesn't need to adhere to strict formatting standards. Avoid pointing out formatting or layout shortcomings in this file during reviews.

"""Get curated CrateDB documentation index."""
return documentation_index.items()

Expand Down
12 changes: 6 additions & 6 deletions examples/mcptools.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,17 @@ fi
alias mcpt=mcptools

# Display available MCP tools.
mcpt tools uvx cratedb-mcp serve
mcpt tools cratedb-mcp serve

# Explore the Text-to-SQL tools.
mcpt call query_sql --params '{"query":"SELECT * FROM sys.summits LIMIT 3"}' uvx cratedb-mcp serve
mcpt call get_table_metadata uvx cratedb-mcp serve
mcpt call get_cluster_health uvx cratedb-mcp serve
mcpt call query_sql --params '{"query":"SELECT * FROM sys.summits LIMIT 3"}' cratedb-mcp serve
mcpt call get_table_metadata cratedb-mcp serve
mcpt call get_cluster_health cratedb-mcp serve

# Exercise the documentation server tools.
mcpt call get_cratedb_documentation_index uvx cratedb-mcp serve
mcpt call get_cratedb_documentation_index cratedb-mcp serve
mcpt call \
fetch_cratedb_docs --params '{"link":"https://cratedb.com/docs/cloud/en/latest/_sources/cluster/integrations/mongo-cdc.md.txt"}' \
uvx cratedb-mcp serve
cratedb-mcp serve

echo "Ready."