-
Couldn't load subscription status.
- Fork 4
mcp tool of the new derive graph (flow chart) from sympy code. #38
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.
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
base: main
Are you sure you want to change the base?
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit 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
Estimated code review effort
Axiomatic Intelligence Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Knowledge base: Disabled due to 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (4)
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. Comment |
There was a problem hiding this 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
📒 Files selected for processing (1)
axiomatic_mcp/servers/equations/server.py(4 hunks)
… for API requests and improve mermaid text formatting
| # 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), |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this 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: |
There was a problem hiding this comment.
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. | ||
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| # 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": |
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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.
There was a problem hiding this 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"asstr, we do read the file but still dumpderivation_graph.mmdin CWD.- That breaks the promise to save beside the source whenever possible and surprises users mixing
Pathandstr.- Detect existing
.pypaths even when passed asstrbefore 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
📒 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
There was a problem hiding this 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 unnecessaryasynckeyword.•
get_python_codeperforms only synchronous file I/O (Path.open, f.read)
• Noawaitexpressions exist within the function body
• Marking itasyncadds overhead without benefitApply 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.pyline 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
descriptionparameter from the@mcp.tooldecorator (lines 58-61), not the function's docstring
• Docstrings here serve only human maintainers but clutter the codebase for the LLMBased 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
descriptionparameter from the@mcp.tooldecorator (lines 110-113), not the function's docstring
• Docstrings here serve only human maintainers but clutter the codebase for the LLMBased 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 addingsystem_promptparameter 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 supportApply 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
📒 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-
.pysuffixes
• 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 actionableaxiomatic_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_graphtool
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_codecorrectly resolves code content and file path
• Output path logic now usesinput_file_pathfrom 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
|
addressed @a96lex comments |

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)derivation_graph_router.py- POST/equations/compose/derivation-graphDerivationGraphServiceDerivationGraphRequest/DerivationGraphResponsemodelsMCP Layer (
/ax-mcp)generate_derivation_graphin equations server_get_python_code()helper functionKey Features
.py) or direct SymPy code strings via form data.mmdfiles, returns formatted markdownTesting
Dependencies
Relies on existing
DerivationGraphComposerandDerivationGraphAgentin core package.