Skip to content

Conversation

@dsarno
Copy link
Owner

@dsarno dsarno commented Sep 7, 2025

Summary

  • add prune_tool_results.py to condense tool_result JSON outputs into one-line summaries
  • trim default Unity MCP tool payloads: find_in_file returns first match positions only, validate_script & get_sha provide condensed counts and hashes, read_console trims entries, and read_resource returns only hash and byte length unless text is requested
  • document lean tool responses and add tests for trimmed outputs
  • split NL/T GitHub workflow into separate NL and T passes to keep conversations lean
  • expose Unity bridge port to the MCP client by sharing the status directory and wiring the port into the workflow
  • guard Unity bridge wait loop against missing status file to prevent premature failures
  • tolerate missing status files during port verification and only probe TCP when a port is found
  • fix NL/T workflow allowed_tools so each pass can access the required Unity edit and console tools
  • remove explicit allowed_tool restrictions so all MCP tools are permitted during the NL and T passes
  • pin tool permissions in CI via .claude/settings.json and explicit allow/deny lists to grant only Unity MCP tools and edits under reports/
  • clarify late-test editing rules in prompt and check structured edits for unbalanced braces

Testing

  • pytest

https://chatgpt.com/codex/tasks/task_e_68bcbbf232cc832791102f8596bd5683

@coderabbitai
Copy link

coderabbitai bot commented Sep 7, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex/add-tool_result-json-pruner-8zy44n

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

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Summary

This pull request implements a comprehensive optimization strategy for the Unity MCP (Multi-Client Protocol) system focused on reducing payload sizes and improving conversation management. The primary innovation is the introduction of "lean tool responses" - a systematic approach where Unity MCP tools now return minimal, essential data by default instead of verbose outputs.

The changes include a new prune_tool_results.py utility that condenses tool_result JSON outputs into concise one-line summaries, making conversation logs more manageable and reducing token usage for AI interactions. The script processes common Unity MCP tool responses and generates targeted summaries while preserving essential information.

Multiple Unity MCP tools have been modified to emit trimmed responses: find_in_file now returns only match positions instead of text excerpts, validate_script provides diagnostic counts rather than full details, get_sha returns only hash and byte length, read_console strips timestamps and metadata, and read_resource returns metadata-only unless explicitly requested otherwise. These changes maintain API compatibility while dramatically reducing JSON payload sizes.

The CI workflow has been restructured to split NL/T testing into two separate passes using different Claude AI models (Sonnet for NL, Haiku for T), which keeps each session focused and lightweight. The workflow improvements include better Unity bridge port sharing through status directory mounting, more robust port detection using host-based JSON parsing, and refined tool permissions via .claude/settings.json.

Additional enhancements include structural validation improvements in the script editing system with unbalanced delimiter checking, updated documentation for the new lean response patterns, comprehensive test coverage for all trimmed output functionality, and clearer guidance for late-test editing operations in the automated testing prompts.

These changes integrate seamlessly with the existing Unity MCP bridge architecture while providing significant performance improvements for token-sensitive AI workflows and CI systems.

Important Files Changed

Changed Files
Filename Score Overview
README-DEV.md 5/5 Added comprehensive documentation for new tool result pruning utility and lean tool response features
UnityMcpBridge/Editor/Tools/ManageScript.cs 4/5 Added structural balance checking for braces/parentheses before script validation to provide better error messages
.claude/prompts/nl-unity-suite-full-additive.md 5/5 Added late-test editing rules for method body modifications during automated testing
.github/workflows/claude-nl-suite.yml 4/5 Split workflow into two passes (NL/T) with different models and improved Unity bridge port sharing
tests/test_read_console_trimmed.py 4/5 Added test coverage for trimmed console output functionality that removes timestamp fields
tests/test_get_sha.py 5/5 Added assertion to validate pruned response format returning only essential hash and length data
UnityMcpBridge/UnityMcpServer~/src/tools/resource_tools.py 4/5 Modified read_resource and find_in_file tools to return minimal output by default with opt-in for full content
UnityMcpBridge/UnityMcpServer~/src/tools/manage_script.py 4/5 Added output trimming to validate_script and get_sha tools to return condensed counts and metadata only
UnityMcpBridge/UnityMcpServer~/src/tools/read_console.py 4/5 Modified to return trimmed console output with only level and message fields, excluding timestamps
tests/test_find_in_file_minimal.py 4/5 Added test coverage for find_in_file tool's new minimal position-only output format
tests/test_validate_script_summary.py 5/5 Added test coverage for validate_script tool's condensed diagnostic count format
tests/test_read_resource_minimal.py 4/5 Added test coverage for read_resource tool's metadata-only default behavior
prune_tool_results.py 4/5 New utility script for condensing verbose JSON tool outputs into one-line summaries for conversation management

Confidence score: 4/5

  • This PR implements well-designed optimizations with comprehensive testing and documentation, making it relatively safe to merge
  • Score reflects the complexity of workflow changes and potential for inter-pass communication issues in the new two-pass CI approach
  • Pay close attention to .github/workflows/claude-nl-suite.yml for workflow isolation and UnityMcpBridge/UnityMcpServer~/src/tools/resource_tools.py for API compatibility

Sequence Diagram

sequenceDiagram
    participant User
    participant GitHub as "GitHub Actions"
    participant Docker as "Unity Container"
    participant Unity as "Unity Editor"
    participant Claude as "Claude Agent"
    participant MCP as "MCP Server"
    participant Tools as "Unity MCP Tools"

    User->>GitHub: Trigger NL/T workflow
    GitHub->>GitHub: Setup Python/UV environment
    GitHub->>GitHub: Install MCP server dependencies
    GitHub->>GitHub: Configure Unity licensing
    GitHub->>Docker: Start Unity container with MCP bridge
    Docker->>Unity: Launch headless Unity Editor
    Unity->>Unity: Start MCP bridge on TCP port
    Unity->>GitHub: Write status JSON with port info
    GitHub->>GitHub: Wait for Unity bridge ready
    GitHub->>GitHub: Write MCP config and permissions
    
    Note over GitHub: NL Pass
    GitHub->>Claude: Run NL pass with Sonnet model
    Claude->>MCP: Connect via stdio transport
    MCP->>Unity: Establish TCP connection to bridge
    
    loop NL Tests (NL-0 to NL-4)
        Claude->>Tools: Call manage_script/apply_text_edits
        Tools->>Unity: Execute script operations
        Unity->>Tools: Return results
        Tools->>Claude: Return lean response
        Claude->>Tools: Call validate_script
        Tools->>Unity: Validate syntax
        Unity->>Tools: Return diagnostics summary
        Tools->>Claude: Return warnings/errors counts
        Claude->>Tools: Call read_console
        Tools->>Unity: Read console messages
        Unity->>Tools: Return trimmed entries
        Tools->>Claude: Return level/message only
        Claude->>GitHub: Write test result XML fragment
    end
    
    Note over GitHub: T Pass  
    GitHub->>Claude: Run T pass with Haiku model
    Claude->>MCP: Connect via stdio transport
    MCP->>Unity: Reuse TCP connection to bridge
    
    loop T Tests (T-A to T-J)
        Claude->>Tools: Call script_apply_edits (structured)
        Tools->>Unity: Execute method operations
        Unity->>Tools: Return results with SHA
        Tools->>Claude: Return minimal payload
        Claude->>Tools: Call get_sha
        Tools->>Unity: Get file hash
        Unity->>Tools: Return hash/length only
        Tools->>Claude: Return SHA without content
        Claude->>GitHub: Write test result XML fragment
    end
    
    GitHub->>GitHub: Merge XML fragments into JUnit
    GitHub->>GitHub: Generate markdown summary
    GitHub->>GitHub: Upload artifacts
    GitHub->>Docker: Stop Unity container
    GitHub->>User: Publish test results
Loading

13 files reviewed, 3 comments

Edit Code Review Bot Settings | Greptile

ignore_case: bool | None = True,
project_root: str | None = None,
max_results: int | None = 200,
max_results: int | None = 1,
Copy link

Choose a reason for hiding this comment

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

style: Default max_results=1 is very restrictive and may limit usefulness for legitimate search scenarios where multiple matches are expected

data = obj.get("data", {}) or {}
msg = obj.get("message") or obj.get("status") or ""
# Common tool shapes
if "sha256" in str(data):
Copy link

Choose a reason for hiding this comment

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

style: Using str(data) to check for 'sha256' presence is inefficient and could match unrelated strings containing 'sha256'

Suggested change
if "sha256" in str(data):
if "sha256" in data:

elif isinstance(convo, list):
convo=[prune_message(m) for m in convo]
json.dump(convo, sys.stdout, ensure_ascii=False)
main()
Copy link

Choose a reason for hiding this comment

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

style: Script calls main() at module level without if __name__ == '__main__': guard, preventing safe imports

Suggested change
main()
if __name__ == '__main__':
main()

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

This PR is being reviewed by Cursor Bugbot

Details

You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.

To receive Bugbot reviews on all of your PRs, please upgrade to Bugbot Pro by visiting the Cursor dashboard. Your first 14 days will be free!

if m:
first = m[0]
return f"find_in_file: {len(m)} match(es) first@{first.get('line',0)}:{first.get('col',0)}"
return "find_in_file: 0 matches"
Copy link

Choose a reason for hiding this comment

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

Bug: Field Name Mismatch Causes Incorrect Summarization

The find_in_file summarization in prune_tool_results.py looks for line and col fields, but the tool now returns startLine and startCol. This causes the summary to always show first@0:0 instead of the actual match position.

Fix in Cursor Fix in Web

@dsarno dsarno deleted the branch nl-CI-workflow-fixes September 7, 2025 23:03
@dsarno dsarno closed this Sep 7, 2025
@dsarno dsarno deleted the codex/add-tool_result-json-pruner-8zy44n branch September 7, 2025 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants