Fix MCP tools query results to include units and improve dimensions

[jfrench9]

Summary

This PR addresses issues in the MCP tools query functionality by enhancing the data structure and field naming conventions used in query results. The changes focus on improving data completeness and clarity in the elements and facts tools.

Key Changes

• Enhanced data completeness: Modified query results to include unit values alongside dimensional data, ensuring consumers have access to complete measurement information
• Improved field naming: Updated dimension field names to follow more consistent and descriptive naming conventions
• Better data accuracy: Refined the query result structure to provide more accurate and comprehensive information to downstream consumers

Files Modified

• elements_tool.py: Updated query result structure to include unit information
• facts_tool.py: Refactored dimension field naming and enhanced query result completeness

Breaking Changes

⚠️ Potential breaking changes: The dimension field name updates may impact existing integrations that rely on the previous field naming structure. Consumers of these tools should verify their field mappings are still valid.

Testing Notes

• Verify that query results now include unit values where expected
• Confirm that dimension fields use the updated naming conventions
• Test downstream consumers to ensure compatibility with the new field structure
• Validate that the enhanced query results maintain backward compatibility where possible

Infrastructure Considerations

• Monitor MCP tool consumers for any issues related to the field name changes
• Consider updating any documentation that references the old dimension field names
• Review logging and monitoring to ensure the enhanced query results are captured appropriately

---

🤖 Generated with Claude Code

Branch Info:

• Source: bugfix/mcp-tools-query-fix
• Target: main
• Type: bugfix

Co-Authored-By: Claude noreply@anthropic.com

[jfrench9]

@claude please review this PR

[claude]

Claude finished @jfrench9's task —— View job

---

Code Review for PR #2

Tasks

• Read modified files to understand changes
• Analyze code quality and correctness
• Check for breaking changes and compatibility issues
• Provide comprehensive review feedback

---

Review Summary

This PR makes important corrections to align the MCP tools with the actual graph database schema. The changes fix incorrect field references and improve data completeness by properly querying Unit nodes.

✅ What's Good

1. Correct Schema Alignment: The changes properly align with the actual schema definitions:

   • FactDimension nodes have axis_uri and member_uri properties (not dimension_type/dimension_value)
   • Unit nodes have a value property for currency/unit information
   • Facts don't have a currency_code property

2. Improved Data Completeness: Adding OPTIONAL MATCH (f)-[:FACT_HAS_UNIT]->(u:Unit) ensures consumers get unit information, which is essential for financial data interpretation

3. Consistent Field Naming: The dimension fields now use the accurate schema names throughout all queries and sample code

🔍 Issues Found

1. Breaking Changes Not Clearly Documented (Medium Priority)

The PR description mentions potential breaking changes, but the impact is actually more significant:

In facts_tool.py:

• dimension_type → axis_uri
• dimension_value → member_uri
• currency_code (from Fact) → currency (from Unit.value)

In elements_tool.py:

• currency_code (from Fact) → currency (from Unit.value)

Recommendation: Update the PR description with a clear migration guide showing the exact field name mappings.

2. Inconsistent Field Naming in Results (Low Priority)

In facts_tool.py:166-171, the result uses inconsistent key names:

pattern = {
    "element": row["element"],
    "dimension_type": row["dim_type"],      # ⚠️ Uses old naming
    "dimension_value": row["dim_value"],    # ⚠️ Uses old naming
    "fact_count": row["fact_count"],
}

The query correctly uses axis_uri and member_uri, but the pattern dictionary uses the old naming convention. This creates confusion about what the actual field names are.

Recommendation: Either:

• Use axis_uri and member_uri consistently in the pattern dict, OR
• Document that the result intentionally uses simplified names for better readability

3. Whitespace-Only Change (Trivial)

facts_tool.py:137 has a trailing whitespace removal (cosmetic only).

4. Missing Error Handling (Low Priority)

When querying for Unit values, there's no explicit handling for cases where:

• A fact has no associated Unit node (the OPTIONAL MATCH handles this, but results show null)
• Multiple units exist for a fact (edge case in financial data)

Recommendation: Consider documenting the expected behavior when u.value is null in the sample results.

📝 Suggestions

1. Add Migration Notes: Include a clear before/after comparison in the PR description:

   BREAKING CHANGES:
   - FactDimension fields: dimension_type → axis_uri, dimension_value → member_uri
   - Currency field: Moved from Fact.currency_code to Unit.value (via FACT_HAS_UNIT relationship)
   

2. Consistency Check: Verify that the pattern dictionary keys (line 168-171) align with your API contract. If consumers expect dimension_type/dimension_value, keep them; otherwise update to axis_uri/member_uri.

3. Documentation Update: Consider adding inline comments explaining why OPTIONAL MATCH is used for Units (some facts may not have units).

🎯 Testing Recommendations

1. ✅ Verify Unit.value returns expected currency codes (USD, EUR, etc.)
2. ✅ Test facts without associated units (ensure graceful null handling)
3. ✅ Confirm dimensional queries work with axis_uri/member_uri filters
4. ⚠️ CRITICAL: Test all downstream consumers of these tools to ensure they handle the new field names

Verdict

Conditional Approval - The code changes are correct and improve data accuracy, but:

• Need clearer documentation of breaking changes
• Should resolve the field naming inconsistency in the pattern dictionary (line 168-171)
• Recommend updating PR description with migration guide before merging

---

Job Run: View Logs
Branch: bugfix/mcp-tools-query-fix