Add tool_result pruning utility and split NL/T workflow into two passes #72
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests
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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This PR implements a comprehensive optimization strategy for the Unity MCP (Model Context Protocol) system focused on reducing conversation size and improving workflow efficiency. The changes introduce a tool result pruning system that condenses verbose JSON outputs into concise summaries, making AI conversations more manageable and cost-effective.
The core addition is prune_tool_results.py, a utility that processes MCP conversation JSON to transform detailed tool outputs into one-line summaries. For example, SHA256 hashes are truncated to 8 characters with ellipsis, script validation diagnostics become simple warning/error counts, and file search results show only position coordinates rather than full text excerpts.
Complementing this utility, the PR modifies five Unity MCP tools to provide "lean responses" by default:
find_in_filenow returns only the first match with position coordinates (startLine/Col,endLine/Col) instead of text excerptsvalidate_scriptreturns diagnostic counts ({warnings: 1, errors: 2}) instead of full diagnostic objectsget_shareturns only essential fields (sha256,lengthBytes) without metadata like timestamps or pathsread_consoledefaults to error-only entries with just level and message fields, stripping timestampsread_resourcereturns only hash and byte length unless text content is explicitly requested viainclude_text=True
The GitHub CI workflow is restructured into a two-pass approach: an NL (Natural Language) pass using Claude Sonnet for read-only analysis, followed by a T (Tool) pass using Claude Haiku for editing operations. This separation keeps individual AI sessions focused and manageable. The workflow improvements also include better Unity bridge connectivity through shared status directories and more robust port detection using jq for JSON parsing.
Eight new test files validate the trimmed behavior across all modified tools, ensuring the optimization doesn't break functionality while confirming the expected minimal response formats. The changes are designed to be backward-compatible, maintaining the same API contracts while returning condensed data structures.
Changed Files
| Filename | Score | Overview |
|---|---|---|
| README-DEV.md | 5/5 | Documents new pruning utility and lean tool responses feature |
| prune_tool_results.py | 4/5 | New utility script that condenses MCP tool results into one-line summaries |
| tests/test_get_sha.py | 5/5 | Adds test for trimmed get_sha output validation |
| UnityMcpBridge/UnityMcpServer~/src/tools/resource_tools.py | 4/5 | Implements lean responses for read_resource and find_in_file tools |
| .github/workflows/claude-nl-suite.yml | 4/5 | Splits workflow into NL and T passes with improved Unity bridge connectivity |
| UnityMcpBridge/UnityMcpServer~/src/tools/read_console.py | 4/5 | Trims console output to essential fields and defaults to errors only |
| tests/test_read_resource_minimal.py | 4/5 | New test validating minimal read_resource behavior |
| tests/test_validate_script_summary.py | 4/5 | New test for condensed script validation diagnostic counts |
| tests/test_find_in_file_minimal.py | 4/5 | New test ensuring find_in_file returns position data only |
| tests/test_read_console_trimmed.py | 4/5 | New test validating trimmed console log output |
| UnityMcpBridge/UnityMcpServer~/src/tools/manage_script.py | 4/5 | Implements condensed responses for validate_script and get_sha tools |
Confidence score: 4/5
- This PR implements thoughtful optimizations that should reduce conversation overhead without breaking functionality
- Score reflects well-structured changes with comprehensive test coverage, though the complexity of modifications across multiple tool implementations introduces some risk
- Pay close attention to the workflow changes in
.github/workflows/claude-nl-suite.ymland ensure the resource_tools.py modifications don't break existing integrations
Sequence Diagram
sequenceDiagram
participant User as User/CI Trigger
participant GitHub as GitHub Actions
participant Docker as Unity Container
participant MCP as MCP Server
participant Unity as Unity Editor
participant Tools as Unity Bridge Tools
User->>GitHub: "Trigger workflow dispatch"
GitHub->>GitHub: "Setup Python environment with uv"
GitHub->>GitHub: "Install MCP server dependencies"
GitHub->>GitHub: "Configure Unity licensing (ULF/EBL)"
GitHub->>Docker: "Start Unity container with MCP bridge"
Docker->>Unity: "Launch Unity Editor in batchmode"
Unity->>Tools: "Execute MCPForUnityBridge.StartAutoConnect"
Tools->>Tools: "Start TCP bridge on random port"
Tools->>GitHub: "Write status JSON with unity_port"
GitHub->>GitHub: "Wait for Unity bridge readiness"
GitHub->>GitHub: "Write MCP config (.claude/mcp.json)"
GitHub->>GitHub: "Create report skeletons and directories"
Note over GitHub,MCP: NL Pass (Natural Language)
GitHub->>MCP: "Start Claude NL pass with allowed tools"
MCP->>Unity: "mcp__unity__list_resources"
Unity->>MCP: "Return minimal CS file URIs"
MCP->>Unity: "mcp__unity__read_resource (metadata only)"
Unity->>MCP: "Return SHA256 + lengthBytes only"
MCP->>Unity: "mcp__unity__find_in_file (first match)"
Unity->>MCP: "Return startLine/Col, endLine/Col only"
MCP->>Unity: "mcp__unity__validate_script"
Unity->>MCP: "Return warnings/errors counts only"
MCP->>Unity: "mcp__unity__read_console (trimmed)"
Unity->>MCP: "Return level/message pairs only"
MCP->>GitHub: "Write NL test results to reports/*_results.xml"
Note over GitHub,MCP: T Pass (Transformational)
GitHub->>MCP: "Start Claude T pass with editing tools"
MCP->>Unity: "mcp__unity__apply_text_edits with ranges"
Unity->>MCP: "Apply atomic text edits"
MCP->>Unity: "mcp__unity__script_apply_edits with ops"
Unity->>MCP: "Apply structured method operations"
MCP->>Unity: "mcp__unity__get_sha for preconditions"
Unity->>MCP: "Return SHA256 + lengthBytes only"
MCP->>GitHub: "Write T test results to reports/*_results.xml"
GitHub->>GitHub: "Canonicalize testcase names (NL/T prefixes)"
GitHub->>GitHub: "Backfill missing test placeholders"
GitHub->>GitHub: "Merge fragments into single JUnit XML"
GitHub->>GitHub: "Build markdown summary from JUnit"
GitHub->>GitHub: "Collect execution transcript"
GitHub->>GitHub: "Apply prune_tool_results.py to transcript"
GitHub->>GitHub: "Publish JUnit report and upload artifacts"
GitHub->>Docker: "Stop Unity container"
GitHub->>User: "Return Pro license (if used)"
11 files reviewed, 3 comments
| lines = data["lines"] or [] | ||
| lvls = {"info":0,"warning":0,"error":0} | ||
| for L in lines: | ||
| lvls[L.get("level","" ).lower()] = lvls.get(L.get("level","" ).lower(),0)+1 |
There was a problem hiding this comment.
logic: Potential KeyError if level has unexpected value. Consider using lvls.get(L.get('level', '').lower(), 0) consistently.
| lvls[L.get("level","" ).lower()] = lvls.get(L.get("level","" ).lower(),0)+1 | |
| level_key = L.get("level", "").lower() | |
| lvls[level_key] = lvls.get(level_key, 0) + 1 |
| data = obj.get("data", {}) or {} | ||
| msg = obj.get("message") or obj.get("status") or "" | ||
| # Common tool shapes | ||
| if "sha256" in str(data): |
There was a problem hiding this comment.
logic: Using str(data) for SHA256 detection is fragile - could match unrelated strings containing 'sha256'. Consider checking data.get('sha256') directly.
| if "sha256" in str(data): | |
| if data.get("sha256"): |
| loop = asyncio.new_event_loop() | ||
| try: | ||
| resp = loop.run_until_complete( | ||
| read_resource(uri="unity://path/Assets/A.txt", ctx=None, project_root=str(proj)) | ||
| ) | ||
| finally: | ||
| loop.close() |
There was a problem hiding this comment.
style: Consider using asyncio.run() instead of manually creating and closing event loop for cleaner async test execution
| loop = asyncio.new_event_loop() | |
| try: | |
| resp = loop.run_until_complete( | |
| read_resource(uri="unity://path/Assets/A.txt", ctx=None, project_root=str(proj)) | |
| ) | |
| finally: | |
| loop.close() | |
| resp = asyncio.run( | |
| read_resource(uri="unity://path/Assets/A.txt", ctx=None, project_root=str(proj)) | |
| ) |
Summary
prune_tool_results.pyto condense tool_result JSON outputs into one-line summariesfind_in_filereturns first match positions only,validate_script&get_shaprovide condensed counts and hashes,read_consoletrims entries, andread_resourcereturns only hash and byte length unless text is requestedTesting
pytesthttps://chatgpt.com/codex/tasks/task_e_68bcbbf232cc832791102f8596bd5683