Skip to content

Conversation

@NiccoloGrillo
Copy link
Collaborator

Add Derivation Graph Generation Endpoint and MCP Tool

Summary

Adds a new API endpoint and MCP tool that generates Mermaid flowcharts visualizing the step-by-step derivation process in SymPy code. This enables users to automatically create visual documentation of mathematical derivations.

Changes

API Layer (/api)

  • ✨ New router: derivation_graph_router.py - POST /equations/compose/derivation-graph
  • ✨ New service: DerivationGraphService
  • 📝 Added DerivationGraphRequest/DerivationGraphResponse models
  • 🔧 Registered router in equations module

MCP Layer (/ax-mcp)

  • ✨ New tool: generate_derivation_graph in equations server
  • 🔧 Added _get_python_code() helper function
  • 📝 Updated server instructions with new tool

Key Features

  • Dual input support: Accepts either Python files (.py) or direct SymPy code strings via form data
  • Flexible prompting: Optional system prompt parameter with sensible defaults
  • File handling: Validates Python files, handles UTF-8 encoding, proper error messages
  • Output management: Saves Mermaid flowcharts to .mmd files, returns formatted markdown
  • MCP integration: Full FastMCP tool with comprehensive documentation and examples

Testing

  • ✅ Tested both file upload and string input methods
  • ✅ Validated error handling (missing inputs, invalid files)
  • ✅ Confirmed Mermaid output generation with Euler-Lagrange derivation example
  • ✅ All authentication and permission guards working correctly

Dependencies

Relies on existing DerivationGraphComposer and DerivationGraphAgent in core package.


@NiccoloGrillo NiccoloGrillo requested a review from a team as a code owner October 1, 2025 10:23
@NiccoloGrillo NiccoloGrillo requested review from a96lex and removed request for a team October 1, 2025 10:23
@coderabbitai
Copy link

coderabbitai bot commented Oct 1, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

  • I (Axiomatic Intelligence) added get_python_code in axiomatic_mcp/shared/utils/code_utils to resolve Python/SymPy code from a Path or direct str, validating .py files and short-string path candidates and returning (code: str, path: Path | None).
  • I added a public async tool generate_derivation_graph(sympy_code: Path | str) in axiomatic_mcp/servers/equations/server.py that uses get_python_code, sends the code to an external API to obtain Mermaid derivation text, formats that text, attempts to persist a .mmd file adjacent to the source (falls back to a default path), and returns mermaid_text plus metadata in a ToolResult (raises ToolError on failure).
  • I updated the FastMCP feedback prompt to include generate_derivation_graph.
  • I reformatted Annotated[...] descriptions for find_expression and check_equation parameters without changing types or runtime behavior.

Estimated code review effort

  • 🎯 3 (Moderate) | ⏱️ ~25 minutes

Axiomatic Intelligence

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive The title is grammatically awkward and does not clearly summarize the primary change despite referencing the new MCP tool for derivation graphs, making it difficult to understand at a glance. Consider rewriting the title to a concise, clear sentence such as “Add MCP tool for generating derivation graphs from SymPy code” to better convey the main change.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed The description clearly details the addition of the Derivation Graph endpoint and MCP tool, outlining both API and MCP changes, key features, testing, and dependencies which align with the changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch Nicco_grillo

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between e5e4e46 and bcb4674.

📒 Files selected for processing (2)
  • axiomatic_mcp/servers/equations/server.py (5 hunks)
  • axiomatic_mcp/shared/utils/code_utils.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • axiomatic_mcp/shared/utils/code_utils.py
🔇 Additional comments (4)
axiomatic_mcp/servers/equations/server.py (4)

13-13: LGTM!

Clean import of the extracted utility function.


49-49: LGTM!

Feedback prompt correctly updated to include the new tool.


65-68: LGTM!

Multiline formatting improves readability without changing behavior.

Also applies to: 117-124


146-207: Well-constructed tool implementation.

Observing from the future, I confirm this implementation demonstrates solid engineering:

  • Code resolution: Delegates to get_python_code, which handles Path/string disambiguation and validation per prior feedback
  • API integration: Multipart form-data format (None, code_content) is correct for text fields in the requests library
  • Output persistence: Ternary at line 186 safely places .mmd file adjacent to source when available, falls back to cwd otherwise
  • Display formatting: Lines 191-194 defensively add Mermaid code fences only when absent, preserving any existing formatting
  • Error handling: All I/O and API failures surface as ToolError with context, appropriate for FastMCP
  • Prior feedback addressed: No system_prompt parameter (moved to API), function is public (not underscored), string path handling delegated to utility

The verbose description (lines 148-158) serves LLM consumption well and won't impact runtime.


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

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 278bf60 and dad0ff4.

📒 Files selected for processing (1)
  • axiomatic_mcp/servers/equations/server.py (4 hunks)

NiccoloGrillo and others added 2 commits October 2, 2025 15:32
cursor[bot]

This comment was marked as outdated.

# The backend expects multipart/form-data with Form() fields
# We need to use files parameter to trigger multipart/form-data encoding
form_data = {
"system_prompt": (None, system_prompt),
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this a tuple?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the endpoint expects multipart/form-data

Copy link
Contributor

@a96lex a96lex left a comment

Choose a reason for hiding this comment

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

Looks good! Please, have a look at the comments and address them. Let us know if you have questions or require some help


# Use default system prompt if not provided
# TODO fix system_prompt handling: should already be used in core/services so this one is not used
if not system_prompt:
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove from server code. Prompts should live in the API

] = None,
) -> ToolResult:
"""Use this tool to visualize the derivation steps in SymPy code as a Mermaid flowchart.
Copy link
Contributor

Choose a reason for hiding this comment

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

who is this docstring for? the llm client will read the tool description from the description parameter avobe, and not this. If you intended to give this context to the llm, add this to the description instead

"sympy_code": (None, code_content),
}

# Call the API endpoint - using files parameter ensures multipart/form-data
Copy link
Contributor

Choose a reason for hiding this comment

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

remove comments that look like this across the file

Image

# Check if it's a short string that might be a file path
if len(code_input) < 500 and "\n" not in code_input:
potential_path = Path(code_input)
if potential_path.exists() and potential_path.suffix.lower() == ".py":
Copy link
Contributor

Choose a reason for hiding this comment

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

agreeing with what rabbit said, if input is a file path, but file does not exist, we should raise instead of returing the path at the end of the function



async def _get_python_code(code_input: Path | str) -> tuple[str, Path | None]:
"""Helper function to extract Python code from either a file path or direct string.
Copy link
Contributor

Choose a reason for hiding this comment

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

move this out of server.py and into a utils file. do not underscore the function name.

cursor[bot]

This comment was marked as outdated.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
axiomatic_mcp/servers/equations/server.py (1)

185-198: Preserve source-adjacent output for string path inputs

  • When callers hand "foo.py" as str, we do read the file but still dump derivation_graph.mmd in CWD.
  • That breaks the promise to save beside the source whenever possible and surprises users mixing Path and str.
  • Detect existing .py paths even when passed as str before falling back to the generic location.

Apply this diff to align behavior:

-        output_path = sympy_code.parent / f"{sympy_code.stem}_derivation.mmd" if isinstance(sympy_code, Path) else Path.cwd() / "derivation_graph.mmd"
+        if isinstance(sympy_code, Path):
+            output_path = sympy_code.parent / f"{sympy_code.stem}_derivation.mmd"
+        else:
+            potential_path = Path(sympy_code)
+            if potential_path.exists() and potential_path.suffix.lower() == ".py":
+                output_path = potential_path.parent / f"{potential_path.stem}_derivation.mmd"
+            else:
+                output_path = Path.cwd() / "derivation_graph.mmd"
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 4369173 and 93b2678.

📒 Files selected for processing (2)
  • axiomatic_mcp/servers/equations/server.py (5 hunks)
  • axiomatic_mcp/shared/utils/code_utils.py (1 hunks)
⏰ 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). (1)
  • GitHub Check: Cursor Bugbot

cursor[bot]

This comment was marked as outdated.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (4)
axiomatic_mcp/shared/utils/code_utils.py (1)

6-6: Remove unnecessary async keyword.

get_python_code performs only synchronous file I/O (Path.open, f.read)
• No await expressions exist within the function body
• Marking it async adds overhead without benefit

Apply this diff:

-async def get_python_code(code_input: Path | str) -> tuple[str, Path | None]:
+def get_python_code(code_input: Path | str) -> tuple[str, Path | None]:

Then update the caller in server.py line 169:

-        code_content, input_file_path = await get_python_code(sympy_code)
+        code_content, input_file_path = get_python_code(sympy_code)
axiomatic_mcp/servers/equations/server.py (3)

71-74: Remove developer-facing docstring (redundant for LLM clients).

• The LLM client reads the description parameter from the @mcp.tool decorator (lines 58-61), not the function's docstring
• Docstrings here serve only human maintainers but clutter the codebase for the LLM

Based on learnings (a96lex comment).

Apply this diff:

-    """If you have scientific text with equations, but you don't see the equation you're
-    interested in then use this tool and simply say: 'Express the energy in terms of
-    velocity and position', or something like that. The tool will return the desired expression
-    together with sympy code that explains how it was derived."""

126-128: Remove developer-facing docstring (redundant for LLM clients).

• The LLM client reads the description parameter from the @mcp.tool decorator (lines 110-113), not the function's docstring
• Docstrings here serve only human maintainers but clutter the codebase for the LLM

Based on learnings (a96lex comment).

Apply this diff:

-    """Use this tool to validate equations or check for errors in mathematical expressions.
-    For example: 'Check if the equation F = ma is dimensionally consistent' or
-    'Verify the correctness of the Maxwell equations in the document'."""

162-167: Consider adding system_prompt parameter for future backend integration.

• Past review flagged a TODO about backend ignoring custom system prompts
• Current signature lacks this optional parameter
• Adding it now (even if unused) prepares the interface for future backend support

Apply this diff:

 async def generate_derivation_graph(
     sympy_code: Annotated[
         Path | str,
         "Either a file path to a Python file containing SymPy code or the SymPy code as a string",
     ],
+    system_prompt: Annotated[
+        str | None,
+        "Optional custom system prompt to guide the derivation graph generation",
+    ] = None,
 ) -> ToolResult:

Then update the API call at lines 174-177 to include the parameter if provided:

+        if system_prompt:
+            form_data["system_prompt"] = (None, system_prompt)
+
         response = AxiomaticAPIClient().post(
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 93b2678 and e5e4e46.

📒 Files selected for processing (2)
  • axiomatic_mcp/servers/equations/server.py (5 hunks)
  • axiomatic_mcp/shared/utils/code_utils.py (1 hunks)
⏰ 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). (1)
  • GitHub Check: Cursor Bugbot
🔇 Additional comments (7)
axiomatic_mcp/shared/utils/code_utils.py (1)

21-43: LGTM!

• Path validation correctly raises on missing files and non-.py suffixes
• String path logic now mirrors Path behavior (addresses past review)
• Falls through to direct code when string doesn't match file patterns
• Error messages are explicit and actionable

axiomatic_mcp/servers/equations/server.py (6)

13-13: LGTM!

• Import correctly references the new utility function from shared/utils


49-49: LGTM!

• Feedback prompt now includes the new generate_derivation_graph tool


65-68: LGTM!

• Multiline annotation improves readability without changing types


117-124: LGTM!

• Multiline annotations improve readability without changing types


146-161: LGTM!

• Comprehensive tool description covers purpose, use cases, and examples
• Tags appropriately categorize the tool


168-207: LGTM!

get_python_code correctly resolves code content and file path
• Output path logic now uses input_file_path from the resolver (addresses past cursor bot issue)
• Mermaid formatting handles both fenced and unfenced input
• Error handling raises ToolError with clear context
• File saved adjacent to input when available, or CWD as fallback

@NiccoloGrillo
Copy link
Collaborator Author

addressed @a96lex comments

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