Skip to content

Implement: Create a helper function for normalizing group_ids parameter#1384

Open
tross-agent wants to merge 1 commit intogetzep:mainfrom
tross-agent:tross-main-1775596893
Open

Implement: Create a helper function for normalizing group_ids parameter#1384
tross-agent wants to merge 1 commit intogetzep:mainfrom
tross-agent:tross-main-1775596893

Conversation

@tross-agent
Copy link
Copy Markdown

The issue is that MCP read tools (search_memory_facts, search_nodes, get_episodes, clear_graph) currently only accept group_ids as a list[str] | None, rejecting scalar string inputs like group_ids="ideadb". This breaks clients that surface scalar inputs for simple tool calls.

Key findings:

  1. MCP tool signatures: Found at mcp_server/src/graphiti_mcp_server.py:486 - search_memory_facts has group_ids: list[str] | None = None
  2. Validation functions: graphiti_core/helpers.py contains validate_group_id (line 135) for single strings and validate_group_ids (line 161) for lists
  3. Error handling: graphiti_core/errors.py defines GroupIdValidationError for invalid characters
  4. Existing patterns: Found _validate_entity_labels in graphiti_core/models/nodes/node_db_queries.py:22 that accepts both string and list inputs
  5. Test location: mcp_server/tests/test_comprehensive_integration.py:247 contains TestSearchOperations class for testing search functionality

The solution requires modifying the MCP tool functions to accept both str and list[str] for group_ids parameter and normalizing scalar strings to single-element lists internally.

Tasks Completed:

  • Create a helper function for normalizing group_ids parameter: Successfully implemented the normalize_group_ids function in graphiti_core/helpers.py. The function:
  1. Accepts str | list[str] | None as input and returns list[str] | None
  2. Returns None if input is None
  3. Converts string to single-element list
  4. Validates each group_id using the existing validate_group_id function
  5. Returns the normalized list

The function follows the same pattern as _validate_entity_labels from graphiti_core/models/nodes/node_db_queries.py:22 and includes proper docstring documentation, type hints, and error handling.

  • Modify search_memory_facts MCP tool to accept scalar group_ids: Successfully modified the search_memory_facts function in mcp_server/src/graphiti_mcp_server.py to accept scalar group_ids. The changes include:
  1. Changed parameter type from group_ids: list[str] | None = None to group_ids: str | list[str] | None = None
  2. Imported the new normalize_group_ids helper function from graphiti_core.helpers
  3. Used normalize_group_ids to normalize the input parameter (handles str, list[str], or None)
  4. Updated the docstring to reflect the new parameter type, indicating it can be a string (single group ID) or list of strings
  5. Updated the logic to use the normalized group_ids instead of the raw input

The function maintains backward compatibility with existing list inputs while now also accepting scalar string inputs.

  • Find and modify search_nodes, get_episodes, and clear_graph MCP tools: Successfully found and modified the remaining three MCP read tools in mcp_server/src/graphiti_mcp_server.py:
  1. search_nodes (line 408):

    • Changed parameter type from group_ids: list[str] | None = None to group_ids: str | list[str] | None = None
    • Used normalize_group_ids helper function for normalization
    • Updated docstring to indicate it accepts string (single group ID) or list of strings
  2. get_episodes (line 625):

    • Changed parameter type from group_ids: list[str] | None = None to group_ids: str | list[str] | None = None
    • Used normalize_group_ids helper function for normalization
    • Updated docstring to indicate it accepts string (single group ID) or list of strings
  3. clear_graph (line 693):

    • Changed parameter type from group_ids: list[str] | None = None to group_ids: str | list[str] | None = None
    • Used normalize_group_ids helper function for normalization
    • Updated docstring to indicate it accepts string (single group ID) or list of strings

All four MCP read tools (search_memory_facts, search_nodes, get_episodes, clear_graph) now consistently accept scalar group_ids for consistency as specified in the bug report.

  • Add comprehensive tests for scalar group_ids acceptance: Successfully added comprehensive tests for scalar group_ids acceptance in mcp_server/tests/test_comprehensive_integration.py. Added four new test methods:
  1. test_scalar_group_ids_acceptance in TestSearchOperations class:

    • Tests search_memory_facts and search_memory_nodes with scalar string, list, and None inputs for group_ids
  2. test_get_episodes_scalar_group_ids in TestEpisodeManagement class:

    • Tests get_episodes with scalar string, list, and None inputs for group_ids
  3. test_clear_graph_scalar_group_ids in TestEntityAndEdgeOperations class:

    • Tests clear_graph with scalar string, list, and None inputs for group_ids
  4. test_invalid_group_ids_validation in TestErrorHandling class:

    • Tests that invalid group_ids (with special characters) still raise appropriate validation errors
    • Tests both scalar and list inputs with invalid values

The tests follow existing test patterns in the TestSearchOperations class and verify that all four MCP tools accept both scalar string and list inputs for group_ids, handle None input, and still raise appropriate validation errors for invalid group_ids.

@danielchalef
Copy link
Copy Markdown
Member


Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants