Feature: API verification via doc/asset#939
Merged
Scriptwonder merged 18 commits intoCoplayDev:betafrom Mar 16, 2026
Merged
Conversation
Implements get_type, get_member, and search actions via live C# reflection. Assembly cache with domain reload invalidation, extension method discovery, ambiguity detection, generic type parsing.
- Register 'docs' group in tool_registry (opt-in, not in defaults) - unity_reflect wraps get_type/get_member/search actions to Unity C# - 17 tests covering param validation, routing, scope handling
13 tests covering get_type, get_member, search, generic types, ambiguity detection, namespace resolution, and error handling.
Fetches + parses Unity ScriptReference HTML from docs.unity3d.com. Version-aware URL construction, method/property URL fallback, stdlib html.parser, 28 tests covering extraction and error handling.
reflect CLI routes to Unity (standard pattern). docs CLI calls Python functions directly (no C# handler).
- CLI docs.py: call _get_doc() directly instead of duplicating fallback logic - UnityReflect.cs: use UnityTypeResolver.ResolveAny() for primary type resolution - UnityReflect.cs: cache GetAssemblyTypeCache() before loops (avoid repeated lock) - UnityReflect.cs: cache ObsoleteAttribute lookup (was called twice per method) - UnityReflect.cs: use HashSet instead of List for obsolete member dedup - UnityReflect.cs: reuse assembly cache in FindExtensionMethods
Unity ScriptReference now uses h3 (not h2) for section titles, "signature-CS sig-block" divs (not <pre> in signature), "name lbl"/"desc" td classes (not "name-collumn"/"desc-collumn"). Parser now supports both old and new formats. 5 new tests added.
Instructs AI to use unity_reflect/unity_docs and asset search to verify Unity APIs before answering, rather than relying on training data. Highlights common hallucination areas.
- get_manual: fetch Unity Manual pages by slug with version fallback - get_package_doc: fetch package docs with redirect tracking (URP etc.) - _ManualPageParser for article-style HTML (h1 title, h2/h3 sections) - _fetch_url_full returns final URL after redirects - 12 new tests, 1002 total passing
Searches ScriptReference, Manual, and package docs concurrently via asyncio.gather. Returns all hits with source labels. 5 new tests, 1007 total passing.
MCP clients serialize list params as strings. Changed queries parameter from list[str] to comma-separated string for compatibility.
lookup now detects asset-related keywords (shader, material, texture, sprite, lit, urp, etc.) and automatically includes manage_asset search alongside doc queries. Runs in parallel, fails silently if no Unity connection.
Previous: used full query as search pattern (e.g., *MeshRenderer 2D lights URP*) — matched nothing. Now extracts individual terms, strips stopwords, infers filter_type from keywords (shader→Shader, etc.), runs multiple focused searches in parallel. Deduplicates results.
- manifest.json: add both tools in alphabetical order - README.md: add to Available Tools, update Recent Updates - README-zh.md: mirror changes in Chinese - SKILL.md: add Docs group to tool categories with trust hierarchy - tools-reference.md: full reference for both tools with examples - workflows.md: add 5 API verification workflow patterns
Contributor
Reviewer's GuideAdds a new opt-in Sequence diagram for full Unity API verification workflowsequenceDiagram
actor Dev as Developer
participant Client as MCPClient/CLI
participant PyRef as unity_reflect(Python)
participant CSRef as UnityReflect(C#)
participant PyDocs as unity_docs(Python)
participant DocsSite as docs.unity3d.com
participant Assets as manage_asset/ProjectAssets
Dev->>Client: Ask Unity API question / run reflect/docs command
rect rgb(230,230,255)
Note over Client,PyRef: Step 1 — search type
Client->>PyRef: action=search, query="NavMesh"
PyRef->>CSRef: unity_reflect {action: search, query}
CSRef->>CSRef: Scan assemblies by scope
CSRef-->>PyRef: Matching types (NavMeshAgent, NavMeshPath,...)
PyRef-->>Client: search results
end
rect rgb(230,255,230)
Note over Client,PyRef: Step 2 — get_type summary
Client->>PyRef: action=get_type, class_name="UnityEngine.AI.NavMeshAgent"
PyRef->>CSRef: unity_reflect {action: get_type, class_name}
CSRef->>CSRef: Reflect methods/properties/fields/events
CSRef-->>PyRef: Member name lists
PyRef-->>Client: Type info + member summary
end
rect rgb(255,240,230)
Note over Client,PyRef: Step 3 — get_member signatures
Client->>PyRef: action=get_member, class_name="NavMeshAgent", member_name="SetDestination"
PyRef->>CSRef: unity_reflect {action: get_member,...}
CSRef->>CSRef: Collect overloads, params, return type
CSRef-->>PyRef: Overload details
PyRef-->>Client: Full signatures
end
rect rgb(230,255,255)
Note over Client,PyDocs: Step 4 — get documentation
Client->>PyDocs: action=get_doc, class_name="NavMeshAgent", member_name="SetDestination"
PyDocs->>DocsSite: HTTP GET ScriptReference URL
DocsSite-->>PyDocs: HTML page
PyDocs->>PyDocs: Parse description, signatures, params, examples
PyDocs-->>Client: Structured doc data
end
opt Shader/Material question
Client->>PyDocs: action=lookup, query includes shader/material terms
PyDocs->>Assets: manage_asset search (shader/material)
Assets-->>PyDocs: Matching project assets
PyDocs-->>Client: Docs hits + project asset matches
end
Client-->>Dev: Verified APIs + examples (reflection > assets > docs)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
📝 WalkthroughWalkthroughAdds two new Unity verification tools—unity_reflect (Unity-side C# reflection handler) and unity_docs (HTTP doc fetchers/parsers)—with server-side tool implementations, CLI commands, Unity tests, Python tests, manifest and documentation updates. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI / User
participant Server as Server (unity_reflect)
participant Unity as Unity Editor
CLI->>Server: reflect get_type { class_name }
Server->>Unity: RPC "unity_reflect" with params
Unity-->>Server: JSON result (type info / members)
Server-->>CLI: Formatted output (success/data)
sequenceDiagram
participant CLI as CLI / User
participant Server as Server (unity_docs)
participant Web as docs.unity3d.com
participant Unity as Unity Editor
CLI->>Server: docs get_doc { class, member, version? }
Server->>Web: HTTP GET (version-aware URLs, fallbacks)
alt Unity context present & asset lookup requested
Server->>Unity: RPC asset search
Unity-->>Server: asset results
end
Web-->>Server: HTML response
Server->>Server: parse HTML -> structured dict
Server-->>CLI: formatted doc result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
unity_docs._search_assets, the broadexcept Exception: passswallows all errors silently; consider at least logging viactxor narrowing the exception types so asset-search failures are visible during debugging without crashing the tool. - The CLI
docscommand calls the internal_get_dochelper directly instead of the publicunity_docstool wrapper, which bypasses validation and future behavior changes; consider routing throughunity_docsfor consistency with other tools and a single execution path.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `unity_docs._search_assets`, the broad `except Exception: pass` swallows all errors silently; consider at least logging via `ctx` or narrowing the exception types so asset-search failures are visible during debugging without crashing the tool.
- The CLI `docs` command calls the internal `_get_doc` helper directly instead of the public `unity_docs` tool wrapper, which bypasses validation and future behavior changes; consider routing through `unity_docs` for consistency with other tools and a single execution path.
## Individual Comments
### Comment 1
<location path="TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/UnityReflectTests.cs" line_range="197-207" />
<code_context>
+ // ── generic types ───────────────────────────────────────────
+
+ [Test]
+ public void GetType_GenericList_Resolves()
+ {
+ var jo = Invoke("get_type", new JObject { ["class_name"] = "List<T>" });
+
+ Assert.IsTrue((bool)jo["success"]);
+ var data = jo["data"];
+ Assert.IsTrue((bool)data["found"], "List<T> should resolve via generic normalization");
+ }
+ }
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a test for multi-parameter generic types (e.g., `Dictionary<TKey, TValue>`) to further validate `NormalizeGenericName`
Right now we only cover the single-parameter path (`List<T> → List�`). Since the normalization logic also handles multiple generic parameters by counting commas with nesting, it would be helpful to add a test that exercises a 2+ parameter case (e.g., `Dictionary<TKey, TValue>`) to ensure that arity detection remains correct.
```suggestion
// ── generic types ───────────────────────────────────────────
[Test]
public void GetType_GenericList_Resolves()
{
var jo = Invoke("get_type", new JObject { ["class_name"] = "List<T>" });
Assert.IsTrue((bool)jo["success"]);
var data = jo["data"];
Assert.IsTrue((bool)data["found"], "List<T> should resolve via generic normalization");
}
[Test]
public void GetType_GenericDictionary_Resolves()
{
var jo = Invoke("get_type", new JObject { ["class_name"] = "Dictionary<TKey, TValue>" });
Assert.IsTrue((bool)jo["success"]);
var data = jo["data"];
Assert.IsTrue(
(bool)data["found"],
"Dictionary<TKey, TValue> should resolve via generic normalization with multiple type parameters"
);
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+197
to
+207
| // ── generic types ─────────────────────────────────────────── | ||
|
|
||
| [Test] | ||
| public void GetType_GenericList_Resolves() | ||
| { | ||
| var jo = Invoke("get_type", new JObject { ["class_name"] = "List<T>" }); | ||
|
|
||
| Assert.IsTrue((bool)jo["success"]); | ||
| var data = jo["data"]; | ||
| Assert.IsTrue((bool)data["found"], "List<T> should resolve via generic normalization"); | ||
| } |
Contributor
There was a problem hiding this comment.
suggestion (testing): Consider adding a test for multi-parameter generic types (e.g., Dictionary<TKey, TValue>) to further validate NormalizeGenericName
Right now we only cover the single-parameter path (List<T> → List�). Since the normalization logic also handles multiple generic parameters by counting commas with nesting, it would be helpful to add a test that exercises a 2+ parameter case (e.g., Dictionary<TKey, TValue>) to ensure that arity detection remains correct.
Suggested change
| // ── generic types ─────────────────────────────────────────── | |
| [Test] | |
| public void GetType_GenericList_Resolves() | |
| { | |
| var jo = Invoke("get_type", new JObject { ["class_name"] = "List<T>" }); | |
| Assert.IsTrue((bool)jo["success"]); | |
| var data = jo["data"]; | |
| Assert.IsTrue((bool)data["found"], "List<T> should resolve via generic normalization"); | |
| } | |
| // ── generic types ─────────────────────────────────────────── | |
| [Test] | |
| public void GetType_GenericList_Resolves() | |
| { | |
| var jo = Invoke("get_type", new JObject { ["class_name"] = "List<T>" }); | |
| Assert.IsTrue((bool)jo["success"]); | |
| var data = jo["data"]; | |
| Assert.IsTrue((bool)data["found"], "List<T> should resolve via generic normalization"); | |
| } | |
| [Test] | |
| public void GetType_GenericDictionary_Resolves() | |
| { | |
| var jo = Invoke("get_type", new JObject { ["class_name"] = "Dictionary<TKey, TValue>" }); | |
| Assert.IsTrue((bool)jo["success"]); | |
| var data = jo["data"]; | |
| Assert.IsTrue( | |
| (bool)data["found"], | |
| "Dictionary<TKey, TValue> should resolve via generic normalization with multiple type parameters" | |
| ); | |
| } |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces an opt-in “docs” tool group that helps assistants verify Unity APIs via live Editor reflection (unity_reflect) and fetch official Unity documentation (unity_docs), reducing incorrect/outdated API guidance.
Changes:
- Added
unity_reflect(Unity-side C# reflection tool + Python server wrapper + CLI command) with tests. - Added
unity_docs(Python-only docs fetch/parse + asset-assisted lookup + CLI command) with tests. - Updated skill/docs/manifest/instructions to document and surface the new “docs” tool group.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| unity-mcp-skill/references/workflows.md | Adds API verification workflow guidance for unity_reflect / unity_docs. |
| unity-mcp-skill/references/tools-reference.md | Documents the new docs tools and parameters. |
| unity-mcp-skill/SKILL.md | Adds “Docs” tool category entry to the skill overview table. |
| manifest.json | Registers unity_docs and unity_reflect in the tool manifest list. |
| docs/i18n/README-zh.md | Updates Chinese README tool list and recent updates to include the new tools. |
| TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/UnityReflectTests.cs.meta | Adds Unity meta for the new EditMode test. |
| TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/UnityReflectTests.cs | Adds Unity EditMode tests for the C# UnityReflect tool. |
| Server/tests/test_unity_reflect.py | Adds Python unit tests for server-side unity_reflect tool behavior + group registration. |
| Server/tests/test_unity_docs.py | Adds Python unit tests for docs URL building, parsing, and tool actions. |
| Server/src/services/tools/unity_reflect.py | Implements server-side unity_reflect MCP tool routing to Unity. |
| Server/src/services/tools/unity_docs.py | Implements Python-only unity_docs MCP tool (fetch/parse/lookup + optional asset search). |
| Server/src/services/registry/tool_registry.py | Registers the new docs tool group. |
| Server/src/main.py | Updates server instructions to nudge API verification when docs group is enabled. |
| Server/src/cli/main.py | Registers new CLI subcommands: reflect and docs. |
| Server/src/cli/commands/reflect.py | Adds CLI commands for reflect type/member/search. |
| Server/src/cli/commands/docs.py | Adds CLI command to fetch ScriptReference docs (docs get). |
| README.md | Updates English README tool list and recent updates to include new tools. |
| MCPForUnity/Editor/Tools/UnityReflect.cs.meta | Adds Unity meta for the new C# tool. |
| MCPForUnity/Editor/Tools/UnityReflect.cs | Implements Unity-side reflection tool (type/member/search + extension/obsolete detection). |
| .claude/skills/unity-mcp-skill/references/workflows.md | Mirrors workflow doc updates for Claude skill copy. |
| .claude/skills/unity-mcp-skill/references/tools-reference.md | Mirrors tool reference updates for Claude skill copy. |
| .claude/skills/unity-mcp-skill/SKILL.md | Mirrors skill overview updates for Claude skill copy. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+225
to
+229
| self._in_param_table = False | ||
|
|
||
| if tag == "div" and self._in_subsection: | ||
| self._in_subsection = False | ||
|
|
| seen_paths: set[str] = set() | ||
|
|
||
| async def _do_search(params: dict) -> list[dict]: | ||
| search_params: dict[str, Any] = {"action": "search", "path": ".", "pageSize": 10} |
Comment on lines
+550
to
+552
| except Exception: | ||
| pass # No Unity connection or import failure — skip silently | ||
| return None |
Comment on lines
+85
to
+89
| if query is not None: | ||
| params_dict["query"] = query | ||
| if action_lower == "search" and scope is not None: | ||
| params_dict["scope"] = scope | ||
|
|
| Use `unity_docs` `lookup` action to search multiple APIs in a single call: | ||
|
|
||
| ```python | ||
| # Search ScriptReference + Manual + package docs in parallel |
| - **`get_doc`**: Fetch ScriptReference docs for a class or member. Parses HTML to extract description, signatures, parameters, return type, and code examples. | ||
| - **`get_manual`**: Fetch a Unity Manual page by slug. Returns title, sections, and code examples. | ||
| - **`get_package_doc`**: Fetch package documentation. Requires package name, page slug, and package version. | ||
| - **`lookup`**: Search all doc sources in parallel (ScriptReference + Manual + package docs). Supports batch queries. For asset-related queries (shader, material, texture, etc.), also searches project assets via `manage_asset`. |
Comment on lines
+626
to
+636
| private static string[] FindExtensionMethods(Type targetType) | ||
| { | ||
| lock (ExtensionMethodCache) | ||
| { | ||
| if (ExtensionMethodCache.TryGetValue(targetType, out var cached)) | ||
| return cached; | ||
| } | ||
|
|
||
| var extensionNames = new HashSet<string>(); | ||
| var cache = GetAssemblyTypeCache(); | ||
|
|
| Use `unity_docs` `lookup` action to search multiple APIs in a single call: | ||
|
|
||
| ```python | ||
| # Search ScriptReference + Manual + package docs in parallel |
| - **`get_doc`**: Fetch ScriptReference docs for a class or member. Parses HTML to extract description, signatures, parameters, return type, and code examples. | ||
| - **`get_manual`**: Fetch a Unity Manual page by slug. Returns title, sections, and code examples. | ||
| - **`get_package_doc`**: Fetch package documentation. Requires package name, page slug, and package version. | ||
| - **`lookup`**: Search all doc sources in parallel (ScriptReference + Manual + package docs). Supports batch queries. For asset-related queries (shader, material, texture, etc.), also searches project assets via `manage_asset`. |
Comment on lines
+190
to
+200
| self.signatures.append("".join(self._current_text).strip()) | ||
|
|
||
| if tag == "div" and self._in_signature: | ||
| # Capture inline signature text (modern Unity docs don't use <pre>) | ||
| text = " ".join("".join(self._current_text).split()).strip() | ||
| # Remove "Declaration" prefix that appears inside the sig block | ||
| if text.startswith("Declaration"): | ||
| text = text[len("Declaration"):].strip() | ||
| if text: | ||
| self.signatures.append(text) | ||
| self._in_signature = False |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/skills/unity-mcp-skill/references/workflows.md:
- Around line 1906-1913: Add a clear opt-in prerequisite note above the "Full
API Verification Before Writing Code" section stating that the tools
`unity_reflect` and `unity_docs` must be enabled/installed (or the user must
join the opt-in group) before following the workflow; update the paragraph
before the header to mention users on fresh setups will need to opt in or enable
these tools to avoid "tool not found" errors, and reference the tool names
`unity_reflect` and `unity_docs` explicitly so readers know what to enable.
In `@manifest.json`:
- Around line 176-183: The manifest's published version was not updated after
adding new public tools (unity_docs, unity_reflect); update the "version" field
in manifest.json to a new appropriate semantic version (e.g., increment patch or
minor from 9.5.3) so published metadata matches the shipped capabilities,
ensuring the change is committed alongside the additions of unity_docs and
unity_reflect.
In `@MCPForUnity/Editor/Tools/UnityReflect.cs`:
- Around line 704-718: The current scope checks for "packages" and "project"
rely solely on assemblyName and miss asmdef/custom assemblies; update the logic
that contains the switch (the case "packages" and case "project" branches that
use assemblyName) to inspect the assembly's path/location in addition to its
name: determine the assembly file path (e.g., use Assembly.Location or the
caller-provided assemblyPath) and make "packages" return true when the path
contains "/Packages/" or the name follows package-style naming (contains a dot
like "com."), while excluding Unity and BCL prefixes
(assemblyName.StartsWith("Unity") / "System"/"mscorlib"/"netstandard"); make
"project" return true when the path contains "/Assets/" or
"/Library/ScriptAssemblies/" or when the assembly name matches project asmdef
patterns (non-Unity, non-BCL names including custom asmdef names and
Assembly-CSharp legacy names). Update the checks in the UnityReflect.cs switch
(the assemblyName variable and the "packages"/"project" cases) accordingly so
custom asmdef assemblies are classified correctly.
- Around line 247-252: get_member currently calls ResolveType() directly and
therefore accepts ambiguous short type names (e.g., "Button", "Image"); modify
get_member to mirror get_type behavior by rejecting unqualified short type names
before calling ResolveType: inspect classResult.Value (after
NormalizeGenericName), and if it does not contain a namespace qualifier (no '.'
or assembly-qualified marker) return an explicit error/ambiguous response rather
than falling back to namespace-prefix resolution; keep ResolveType usage for
fully-qualified names only and ensure the same ambiguity check is applied to any
code paths in get_member that use memberResult.Value.
In `@Server/src/cli/commands/docs.py`:
- Around line 6-8: The CLI command function get_doc is missing the required
`@handle_unity_errors` decorator; add `@handle_unity_errors` immediately above the
get_doc function definition to ensure Unity connection errors are handled
consistently, and add an import for handle_unity_errors (e.g., from
cli.utils.errors import handle_unity_errors) at the top of the file if it’s not
already imported; ensure the decorator is applied to the same function
referenced as get_doc so the docs get command follows the project CLI
error-handling guideline.
In `@Server/src/cli/commands/reflect.py`:
- Around line 27-30: The CLI currently prints whatever run_command returns (in
functions using get_config, run_command, and format_output) even when
run_command returns {"success": False,...}, causing failures to be treated as
successful exits; update each command handler (the blocks around run_command
calls at the shown ranges) to detect result.get("success") is False, output a
clear error (using click.echo or raising click.ClickException) with the error
details from the result, and ensure the process exits non‑zero (raise or
sys.exit(1)) instead of printing and returning 0 so handle_unity_errors behavior
and proper failure signaling are preserved.
In `@Server/src/services/tools/unity_docs.py`:
- Around line 184-200: The signature text is being appended twice for legacy
pages because a <pre> inside the signature block appends on </pre> and the
enclosing </div> appends the same content again; update the logic so the
div-closure handler (the block that checks self._in_signature and appends to
self.signatures) skips appending when the signature was already captured by the
inner </pre>. Concretely: when you handle the </pre> branch inside a signature
(where you currently set self._in_pre = False and append to self.signatures),
set a small sentinel (e.g., self._signature_from_pre = True), and in the
div-closure branch for self._in_signature only append if that sentinel is not
set; after handling the div, reset the sentinel and self._in_signature.
Reference symbols: _in_code_example, _in_pre, _current_text, _in_signature,
signatures, and add/clear the new _signature_from_pre flag.
- Around line 317-323: _flush_section currently only appends a section when
self._current_heading is set, so any intro text collected in self._content_parts
before the first heading is dropped; change _flush_section to also emit a
section when self._content_parts is non-empty even if self._current_heading is
None (e.g., append {"heading": self._current_heading or "", "content":
"\n".join(self._content_parts)}) and then clear self._current_heading and
self._content_parts so the page summary before the first heading is preserved;
see _flush_section, _current_heading, _content_parts, and sections for where to
apply this logic.
In `@Server/src/services/tools/unity_reflect.py`:
- Around line 27-29: The mcp_for_unity_tool decorator on the Unity Reflect tool
is using group="docs", which is not part of the allowed MCP groups; change the
group parameter on the mcp_for_unity_tool decorator (located above the Unity
Reflect tool definition) to one of the permitted values ('core', 'vfx',
'animation', 'ui', 'scripting_ext', 'probuilder', or 'testing') or, if the
intent is to introduce a new group, update the group policy/docs accordingly to
include "docs" and ensure manage_tools behavior is adjusted; update only the
decorator's group value or the group policy reference to resolve the conflict.
- Around line 87-88: Validate the incoming scope before adding it to
params_dict: check that when action_lower == "search" and scope is not None the
value of scope is one of the allowed constants in VALID_SCOPES (raise or return
an error / skip adding if invalid), and only then set params_dict["scope"] =
scope; update the code path in the function containing
action_lower/scope/params_dict (the block that currently uses VALID_SCOPES) to
enforce this server-side whitelist and handle invalid values explicitly (e.g.,
log and return a validation error or omit the parameter).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7da177a4-b0cb-4a6b-a3d0-501e91da48a5
📒 Files selected for processing (22)
.claude/skills/unity-mcp-skill/SKILL.md.claude/skills/unity-mcp-skill/references/tools-reference.md.claude/skills/unity-mcp-skill/references/workflows.mdMCPForUnity/Editor/Tools/UnityReflect.csMCPForUnity/Editor/Tools/UnityReflect.cs.metaREADME.mdServer/src/cli/commands/docs.pyServer/src/cli/commands/reflect.pyServer/src/cli/main.pyServer/src/main.pyServer/src/services/registry/tool_registry.pyServer/src/services/tools/unity_docs.pyServer/src/services/tools/unity_reflect.pyServer/tests/test_unity_docs.pyServer/tests/test_unity_reflect.pyTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/UnityReflectTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/UnityReflectTests.cs.metadocs/i18n/README-zh.mdmanifest.jsonunity-mcp-skill/SKILL.mdunity-mcp-skill/references/tools-reference.mdunity-mcp-skill/references/workflows.md
Comment on lines
+176
to
+183
| { | ||
| "name": "unity_docs", | ||
| "description": "Fetch Unity documentation (ScriptReference, Manual, package docs)" | ||
| }, | ||
| { | ||
| "name": "unity_reflect", | ||
| "description": "Inspect Unity C# APIs via live reflection" | ||
| }, |
Contributor
There was a problem hiding this comment.
Bump manifest version when adding new public tools.
This PR exposes new tools (unity_docs, unity_reflect), but manifest.json still reports 9.5.3 (Line 4). That can desync published metadata from shipped capability.
Suggested version bump
- "version": "9.5.3",
+ "version": "9.5.4",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@manifest.json` around lines 176 - 183, The manifest's published version was
not updated after adding new public tools (unity_docs, unity_reflect); update
the "version" field in manifest.json to a new appropriate semantic version
(e.g., increment patch or minor from 9.5.3) so published metadata matches the
shipped capabilities, ensuring the change is committed alongside the additions
of unity_docs and unity_reflect.
Comment on lines
+704
to
+718
| case "unity": | ||
| return assemblyName.StartsWith("UnityEngine") | ||
| || assemblyName.StartsWith("UnityEditor") | ||
| || assemblyName.StartsWith("Unity."); | ||
|
|
||
| case "packages": | ||
| return !assemblyName.StartsWith("System") | ||
| && !assemblyName.StartsWith("mscorlib") | ||
| && !assemblyName.StartsWith("netstandard"); | ||
|
|
||
| case "project": | ||
| return assemblyName == "Assembly-CSharp" | ||
| || assemblyName == "Assembly-CSharp-Editor" | ||
| || assemblyName.StartsWith("Assembly-CSharp-firstpass") | ||
| || assemblyName.StartsWith("Assembly-CSharp-Editor-firstpass"); |
Contributor
There was a problem hiding this comment.
packages and project scope matching is not reliable in modern Unity projects.
packages currently admits every non-BCL assembly, so it includes UnityEngine*, UnityEditor*, and Assembly-CSharp*. project only matches the legacy Assembly-CSharp* names, so any custom asmdef disappears. That makes scope="packages" and scope="project" return misleading results.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@MCPForUnity/Editor/Tools/UnityReflect.cs` around lines 704 - 718, The current
scope checks for "packages" and "project" rely solely on assemblyName and miss
asmdef/custom assemblies; update the logic that contains the switch (the case
"packages" and case "project" branches that use assemblyName) to inspect the
assembly's path/location in addition to its name: determine the assembly file
path (e.g., use Assembly.Location or the caller-provided assemblyPath) and make
"packages" return true when the path contains "/Packages/" or the name follows
package-style naming (contains a dot like "com."), while excluding Unity and BCL
prefixes (assemblyName.StartsWith("Unity") / "System"/"mscorlib"/"netstandard");
make "project" return true when the path contains "/Assets/" or
"/Library/ScriptAssemblies/" or when the assembly name matches project asmdef
patterns (non-Unity, non-BCL names including custom asmdef names and
Assembly-CSharp legacy names). Update the checks in the UnityReflect.cs switch
(the assemblyName variable and the "packages"/"project" cases) accordingly so
custom asmdef assemblies are classified correctly.
Comment on lines
+6
to
+8
| from cli.utils.config import get_config | ||
| from cli.utils.output import format_output | ||
|
|
Contributor
There was a problem hiding this comment.
Add @handle_unity_errors to the docs get command.
get_doc is a CLI command but currently skips the required decorator, so Unity connection failures won’t be handled consistently.
Suggested fix
import asyncio
import click
from cli.utils.config import get_config
+from cli.utils.connection import handle_unity_errors
from cli.utils.output import format_output
@@
`@docs.command`("get")
`@click.argument`("class_name")
`@click.argument`("member_name", required=False)
`@click.option`("--version", "-v", default=None, help="Unity version (e.g., 6000.0).")
+@handle_unity_errors
def get_doc(class_name: str, member_name: str | None, version: str | None):As per coding guidelines Server/src/cli/commands/*.py: “Python CLI commands must use the @handle_unity_errors decorator for error handling”.
Also applies to: 16-33
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Server/src/cli/commands/docs.py` around lines 6 - 8, The CLI command function
get_doc is missing the required `@handle_unity_errors` decorator; add
`@handle_unity_errors` immediately above the get_doc function definition to ensure
Unity connection errors are handled consistently, and add an import for
handle_unity_errors (e.g., from cli.utils.errors import handle_unity_errors) at
the top of the file if it’s not already imported; ensure the decorator is
applied to the same function referenced as get_doc so the docs get command
follows the project CLI error-handling guideline.
Comment on lines
+27
to
+30
| config = get_config() | ||
| result = run_command("unity_reflect", {"action": "get_type", "class_name": class_name}, config) | ||
| click.echo(format_output(result, config.format)) | ||
|
|
Contributor
There was a problem hiding this comment.
Structured tool failures are treated as successful CLI execution.
These handlers print the response directly even when run_command returns {"success": False, ...}. Because this is not an exception, @handle_unity_errors won’t trigger, and the command can exit 0 on failures.
Suggested fix
import click
from cli.utils.config import get_config
-from cli.utils.output import format_output
+from cli.utils.output import format_output, print_error
from cli.utils.connection import run_command, handle_unity_errors
+def _echo_or_exit(result: dict, output_format: str) -> None:
+ if isinstance(result, dict) and result.get("success") is False:
+ print_error(result.get("message") or result.get("error") or "Unity command failed.")
+ raise SystemExit(1)
+ click.echo(format_output(result, output_format))
+
@@
config = get_config()
result = run_command("unity_reflect", {"action": "get_type", "class_name": class_name}, config)
- click.echo(format_output(result, config.format))
+ _echo_or_exit(result, config.format)
@@
result = run_command("unity_reflect", {
"action": "get_member",
"class_name": class_name,
"member_name": member_name,
}, config)
- click.echo(format_output(result, config.format))
+ _echo_or_exit(result, config.format)
@@
result = run_command("unity_reflect", {
"action": "search",
"query": query,
"scope": scope,
}, config)
- click.echo(format_output(result, config.format))
+ _echo_or_exit(result, config.format)Also applies to: 44-50, 66-72
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Server/src/cli/commands/reflect.py` around lines 27 - 30, The CLI currently
prints whatever run_command returns (in functions using get_config, run_command,
and format_output) even when run_command returns {"success": False,...}, causing
failures to be treated as successful exits; update each command handler (the
blocks around run_command calls at the shown ranges) to detect
result.get("success") is False, output a clear error (using click.echo or
raising click.ClickException) with the error details from the result, and ensure
the process exits non‑zero (raise or sys.exit(1)) instead of printing and
returning 0 so handle_unity_errors behavior and proper failure signaling are
preserved.
Comment on lines
+27
to
+29
| @mcp_for_unity_tool( | ||
| group="docs", | ||
| description=( |
Contributor
There was a problem hiding this comment.
group="docs" conflicts with the current allowed MCP group set.
This tool registers with a group outside the documented allowed list. Please align to an allowed group or update the group policy/docs in tandem.
As per coding guidelines: “MCP tool group parameter must be set to one of: 'core' (default, enabled), 'vfx', 'animation', 'ui', 'scripting_ext', 'probuilder', or 'testing' (disabled by default, toggled via manage_tools)”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Server/src/services/tools/unity_reflect.py` around lines 27 - 29, The
mcp_for_unity_tool decorator on the Unity Reflect tool is using group="docs",
which is not part of the allowed MCP groups; change the group parameter on the
mcp_for_unity_tool decorator (located above the Unity Reflect tool definition)
to one of the permitted values ('core', 'vfx', 'animation', 'ui',
'scripting_ext', 'probuilder', or 'testing') or, if the intent is to introduce a
new group, update the group policy/docs accordingly to include "docs" and ensure
manage_tools behavior is adjusted; update only the decorator's group value or
the group policy reference to resolve the conflict.
- Fix lock ordering deadlock: switch ExtensionMethodCache to ConcurrentDictionary - Add ambiguity check to get_member (was missing, get_type already had it) - Add Dictionary<TKey, TValue> generic resolution test
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
MCPForUnity/Editor/Tools/UnityReflect.cs (1)
707-725:⚠️ Potential issue | 🟠 Major
packages/projectscope still misclassifies modern asmdef assemblies.
packagescurrently accepts every non-BCL assembly, so it also pulls inUnity*and project assemblies, whileprojectonly recognizes the legacyAssembly-CSharp*names. In asmdef-based projects that makes both scopes misleading. Please classify from assembly location (/Packages/vs/Assets/or/Library/ScriptAssemblies/) instead of assembly name alone.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Editor/Tools/UnityReflect.cs` around lines 707 - 725, MatchesScope currently classifies assemblies by name; update it to inspect the assembly file path (assembly.Location or assembly.CodeBase) instead when handling "packages" and "project". In the MatchesScope method (and any callers using an assemblyName string), accept or obtain the assembly's Location and for scope "packages" return true when the path contains "/Packages/" (and still exclude BCL prefixes like "System", "mscorlib", "netstandard"); for scope "project" return true when the path contains "/Assets/" or "/Library/ScriptAssemblies/" (or when the assemblyName matches legacy Assembly-CSharp* names as a fallback). Also handle null/empty Location safely by falling back to the original name-based checks so existing behavior is preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@MCPForUnity/Editor/Tools/UnityReflect.cs`:
- Around line 278-357: The get_member path fails to resolve extension methods
because it only inspects instance/static members on type; add a fallback before
the final "not found" return that searches for static extension methods whose
first parameter matches the target type (use flags and look for methods marked
with System.Runtime.CompilerServices.ExtensionAttribute or
MethodInfo.IsDefined(typeof(ExtensionAttribute), false)), e.g. iterate candidate
assemblies/types (or reuse the same source used to populate extension_methods in
get_type) to find methods named memberName where the first parameter's type
matches or is assignable to the inspected type, format overloads with
FormatMethodDetail, and return a SuccessResponse with member_type =
"extension_method", overload_count and overloads and declare the extension
method's declaring_type via FormatTypeName so extension names returned by
get_type are resolved here.
In `@Server/src/services/tools/unity_docs.py`:
- Around line 613-621: The current loop in lookup that zips labels and results
discards responses where result is an Exception or result.get("success") is
False, turning real fetch errors into silent "not found" hits; update the logic
in the loop that builds hits to also collect per-source errors (e.g., build an
errors list with entries like {"source": label, "error": result} for Exception
or {"source": label, "error": result} when success is False) and after the loop,
if hits is empty but errors is non-empty, return or raise a failure indicating
all sources errored (or include the errors in the lookup response payload) so
callers can distinguish real errors from an empty-not-found result; keep use of
labels and results and preserve existing success/data handling for actual found
hits.
- Around line 600-607: The code currently lowercases query into manual_slug and
only tries that form which can miss mixed-case slugs; change it to first try the
original-spelling slug (replace spaces/underscores with hyphens but keep case)
when calling _get_manual and _get_package_doc, then add a fallback task that
tries the lowercased variant only if it differs. Specifically, keep the
normalized hyphenized slug (e.g., original_slug = query.replace(" ",
"-").replace("_","-")), append tasks.append(("manual",
_get_manual(original_slug, version))), and if original_slug.lower() !=
original_slug append the lowercase fallback tasks.append(("manual_fallback",
_get_manual(original_slug.lower(), version))) and mirror this for the package
page variable used with _get_package_doc when package and pkg_version are
provided.
---
Duplicate comments:
In `@MCPForUnity/Editor/Tools/UnityReflect.cs`:
- Around line 707-725: MatchesScope currently classifies assemblies by name;
update it to inspect the assembly file path (assembly.Location or
assembly.CodeBase) instead when handling "packages" and "project". In the
MatchesScope method (and any callers using an assemblyName string), accept or
obtain the assembly's Location and for scope "packages" return true when the
path contains "/Packages/" (and still exclude BCL prefixes like "System",
"mscorlib", "netstandard"); for scope "project" return true when the path
contains "/Assets/" or "/Library/ScriptAssemblies/" (or when the assemblyName
matches legacy Assembly-CSharp* names as a fallback). Also handle null/empty
Location safely by falling back to the original name-based checks so existing
behavior is preserved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 519a804c-d7a4-4653-8cf9-8a75d73fd077
📒 Files selected for processing (6)
MCPForUnity/Editor/Tools/UnityReflect.csServer/src/services/tools/unity_docs.pyServer/src/services/tools/unity_reflect.pyTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/UnityReflectTests.csunity-mcp-skill/references/tools-reference.mdunity-mcp-skill/references/workflows.md
✅ Files skipped from review due to trivial changes (1)
- unity-mcp-skill/references/workflows.md
🚧 Files skipped from review as they are similar to previous changes (1)
- unity-mcp-skill/references/tools-reference.md
- get_member now resolves extension methods as fallback before "not found" - lookup preserves original-case slugs for Manual pages (UIE-USS-Properties-Reference) - lookup surfaces per-source errors instead of silently dropping them
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
MCPForUnity/Editor/Tools/UnityReflect.cs (2)
685-690: Minor: Redundant type matching conditions.
IsAssignableFromalready returns true for exact type matches and subclass relationships, making theIsSubclassOfand==checks redundant. This doesn't affect correctness but adds slight overhead.♻️ Simplify type matching
var paramType = firstParam.ParameterType; - if (paramType.IsAssignableFrom(targetType) || targetType.IsSubclassOf(paramType) - || paramType == targetType - || (paramType.IsGenericType && IsGenericMatch(paramType, targetType))) + if (paramType.IsAssignableFrom(targetType) + || (paramType.IsGenericType && IsGenericMatch(paramType, targetType))) { extensionNames.Add(method.Name); }The same simplification applies to
FindExtensionMethodInfosat lines 728-730.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Editor/Tools/UnityReflect.cs` around lines 685 - 690, The type-matching condition is redundant: remove the explicit paramType == targetType and targetType.IsSubclassOf(paramType) checks and rely on paramType.IsAssignableFrom(targetType) plus the existing generic fallback IsGenericMatch(paramType, targetType); update the block that currently adds method.Name to extensionNames (using paramType, targetType, IsGenericMatch) to only test IsAssignableFrom || (paramType.IsGenericType && IsGenericMatch(...)); make the same simplification inside FindExtensionMethodInfos so both places use the streamlined logic.
86-93: Consider logging swallowed exceptions in debug builds.The empty catch block is understandable for handling dynamic assemblies that throw on
GetExportedTypes(), but silently dropping all exceptions can mask unexpected issues during development.♻️ Optional: Add debug logging
try { _assemblyTypeCache[asm.FullName] = asm.GetExportedTypes(); } - catch + catch (Exception ex) { - // Some assemblies throw on GetExportedTypes + // Some assemblies (dynamic, reflection-only) throw on GetExportedTypes +#if UNITY_EDITOR && DEBUG + UnityEngine.Debug.LogWarning($"[UnityReflect] Skipping assembly {asm.GetName().Name}: {ex.GetType().Name}"); +#endif }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Editor/Tools/UnityReflect.cs` around lines 86 - 93, The empty catch around asm.GetExportedTypes() swallows all exceptions; change it to catch Exception ex and, in debug/editor builds, log the exception (e.g., via UnityEngine.Debug.LogException or Debug.LogWarning) along with contextual info such as asm.FullName and the _assemblyTypeCache key so developers can see unexpected failures during development while still ignoring known problematic assemblies in release builds.Server/src/services/tools/unity_docs.py (3)
91-100: Consider validating URL scheme for defense in depth.While all current callers construct URLs with hardcoded
https://docs.unity3d.combases,urlopenaccepts arbitrary schemes includingfile://. Adding a scheme check would provide defense against future misuse.🛡️ Optional hardening
def _do_fetch() -> tuple[int, str, str]: + if not url.startswith(("https://", "http://")): + raise ValueError(f"Invalid URL scheme: {url}") req = Request(url, headers={"User-Agent": "MCPForUnity/1.0"})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Server/src/services/tools/unity_docs.py` around lines 91 - 100, Add a scheme check at the top of _do_fetch: parse url with urllib.parse.urlparse and ensure parsed.scheme == "https" (or a whitelist if you want to allow "http"); if the scheme is not allowed, raise a suitable exception (e.g., ValueError or URLError) before calling Request/urlopen so file:// or other schemes cannot be used. Update the imports if needed and keep existing HTTPError/URLError handling intact.
618-632: Past issues addressed; minor style note onzip().Error surfacing (lines 619-631) and original-case slug handling (lines 600-606) are now correctly implemented per past review feedback.
Line 621: While
labelsandresultslengths are inherently equal (both derived fromtasks), addingstrict=Truetozip()is a Python best practice that documents this invariant.♻️ Optional: Add strict=True to zip
- for label, result in zip(labels, results): + for label, result in zip(labels, results, strict=True):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Server/src/services/tools/unity_docs.py` around lines 618 - 632, The loop that pairs labels and results should use zip(labels, results, strict=True) to assert the lists are the same length; update the for loop that currently reads for label, result in zip(labels, results): to include strict=True so mismatched lengths raise an error, leaving the handling of hits, errors, and the subsequent isinstance/result checks unchanged (references: labels, results, hits, errors).
562-570: Broad exception handling is acceptable but consider logging.The defensive exception handling is appropriate since asset search is an optional enhancement. However, silently swallowing the exception when
ctx.warning()fails (lines 568-569) loses diagnostic information. Consider logging at debug level if a logger is available.🔍 Optional: Add debug logging for swallowed exceptions
except Exception as e: try: if hasattr(ctx, 'warning'): await ctx.warning(f"Asset search failed: {e}") except Exception: - pass # ctx might not be usable + pass # ctx might not be usable; asset search is optionalGiven the defensive nature of this code (asset search is optional), the current approach is acceptable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Server/src/services/tools/unity_docs.py` around lines 562 - 570, In the except block that handles failures calling ctx.warning (inside the asset search handling in unity_docs.py), preserve diagnostic info by logging the swallowed exception at debug level: if a module or passed-in logger is available (e.g., logger), call logger.debug("Asset search ctx.warning failed", exc_info=True) or similar; otherwise fallback to printing to stderr. Ensure you reference the caught exception variable e and the ctx.warning call when adding the debug log so the diagnostic is recorded without changing the existing control flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Server/src/services/tools/unity_docs.py`:
- Around line 660-668: The code silently drops exceptions from asyncio.gather
results so callers never see batch-level failures; update the post-gather
handling (the block that iterates over results and builds query_results) to
collect exceptions into an errors list (e.g., gather_errors) and include them in
the returned response alongside query_results and found count; specifically,
when iterating over results from asyncio.gather in the caller of _lookup_single,
push Exception instances into gather_errors (preserving the exception
message/trace) instead of just continuing, and ensure the final return payload
includes that errors list so callers can see why queries failed.
---
Nitpick comments:
In `@MCPForUnity/Editor/Tools/UnityReflect.cs`:
- Around line 685-690: The type-matching condition is redundant: remove the
explicit paramType == targetType and targetType.IsSubclassOf(paramType) checks
and rely on paramType.IsAssignableFrom(targetType) plus the existing generic
fallback IsGenericMatch(paramType, targetType); update the block that currently
adds method.Name to extensionNames (using paramType, targetType, IsGenericMatch)
to only test IsAssignableFrom || (paramType.IsGenericType &&
IsGenericMatch(...)); make the same simplification inside
FindExtensionMethodInfos so both places use the streamlined logic.
- Around line 86-93: The empty catch around asm.GetExportedTypes() swallows all
exceptions; change it to catch Exception ex and, in debug/editor builds, log the
exception (e.g., via UnityEngine.Debug.LogException or Debug.LogWarning) along
with contextual info such as asm.FullName and the _assemblyTypeCache key so
developers can see unexpected failures during development while still ignoring
known problematic assemblies in release builds.
In `@Server/src/services/tools/unity_docs.py`:
- Around line 91-100: Add a scheme check at the top of _do_fetch: parse url with
urllib.parse.urlparse and ensure parsed.scheme == "https" (or a whitelist if you
want to allow "http"); if the scheme is not allowed, raise a suitable exception
(e.g., ValueError or URLError) before calling Request/urlopen so file:// or
other schemes cannot be used. Update the imports if needed and keep existing
HTTPError/URLError handling intact.
- Around line 618-632: The loop that pairs labels and results should use
zip(labels, results, strict=True) to assert the lists are the same length;
update the for loop that currently reads for label, result in zip(labels,
results): to include strict=True so mismatched lengths raise an error, leaving
the handling of hits, errors, and the subsequent isinstance/result checks
unchanged (references: labels, results, hits, errors).
- Around line 562-570: In the except block that handles failures calling
ctx.warning (inside the asset search handling in unity_docs.py), preserve
diagnostic info by logging the swallowed exception at debug level: if a module
or passed-in logger is available (e.g., logger), call logger.debug("Asset search
ctx.warning failed", exc_info=True) or similar; otherwise fallback to printing
to stderr. Ensure you reference the caught exception variable e and the
ctx.warning call when adding the debug log so the diagnostic is recorded without
changing the existing control flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ff4359e6-c7e0-4f96-8109-6471fda1dd23
📒 Files selected for processing (2)
MCPForUnity/Editor/Tools/UnityReflect.csServer/src/services/tools/unity_docs.py
Comment on lines
+660
to
+668
| results = await asyncio.gather(*tasks, return_exceptions=True) | ||
|
|
||
| query_results = [] | ||
| for result in results: | ||
| if isinstance(result, Exception): | ||
| continue | ||
| if isinstance(result, dict): | ||
| query_results.append(result) | ||
|
|
Contributor
There was a problem hiding this comment.
Exception results from batch queries are silently discarded.
When _lookup_single throws an exception, it's silently skipped (lines 664-665). Unlike the error handling in _lookup_single (which collects errors per-source), batch-level exceptions don't surface to the caller. If all queries fail with exceptions, the response shows found: 0 with no indication of why.
🐛 Suggested fix to track failed queries
query_results = []
+ failed_queries = []
for result in results:
if isinstance(result, Exception):
+ failed_queries.append({"error": str(result)})
continue
if isinstance(result, dict):
query_results.append(result)
+
+ # Include failed queries in summary if any
+ summary = {
+ "total": len(queries),
+ "found": len(all_found),
+ "missed": len(all_missed),
+ }
+ if failed_queries:
+ summary["failed"] = len(failed_queries)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Server/src/services/tools/unity_docs.py` around lines 660 - 668, The code
silently drops exceptions from asyncio.gather results so callers never see
batch-level failures; update the post-gather handling (the block that iterates
over results and builds query_results) to collect exceptions into an errors list
(e.g., gather_errors) and include them in the returned response alongside
query_results and found count; specifically, when iterating over results from
asyncio.gather in the caller of _lookup_single, push Exception instances into
gather_errors (preserving the exception message/trace) instead of just
continuing, and ensure the final return payload includes that errors list so
callers can see why queries failed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add two new MCP tools (unity_reflect and unity_docs) that let AI assistants verify Unity API signatures via live C# reflection and fetch official documentation on demand, reducing hallucinated/outdated API answers. Inspired by https://github.com/Razpines/UnityRAG
Type of Change
Changes Made
Testing/Screenshots/Recordings
Documentation Updates
Additional Notes
Summary by Sourcery
Add new Unity API verification and documentation tools, expose them via MCP, CLI, and tool groups, and document workflows for using them to reduce hallucinated or outdated Unity API usage.
New Features:
Enhancements:
Tests:
Summary by CodeRabbit
New Features
Documentation
Tests