fix(moonshot): normalise union type arrays in _fill_missing_type to prevent TypeError#30753
Open
SimoKiihamaki wants to merge 1 commit into
Open
Conversation
…revent TypeError (Fixes NousResearch#30095) When a JSON Schema property uses a union type array like "type": ["number", "string"], the set membership test node["type"] not in {None, ""} raises TypeError: unhashable type 'list' because Python lists are not hashable. Moonshot does not accept union type arrays anyway — it requires a single scalar type. Normalise to the first concrete (non-null) type before the set membership check. 7 new regression tests covering: number|string, string|integer, null|string, all-null, key preservation, nested properties, and array items.
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR fixes a crash in Moonshot schema sanitization when encountering JSON Schema union types expressed as type: [...] arrays, and adds regression tests to ensure union types are normalized deterministically.
Changes:
- Normalize list-valued
typefields (e.g.,["number", "string"]) to the first concrete non-null type. - Add regression tests covering union types in top-level properties, nested objects, and array items.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
agent/moonshot_schema.py |
Normalizes JSON Schema union-type arrays to a single Moonshot-compatible scalar type. |
tests/agent/test_moonshot_schema.py |
Adds regression tests ensuring union-type arrays don’t crash and normalize correctly across nesting forms. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+174
to
+183
| node_type = node.get("type") | ||
| # Guard: list types (e.g. ["number", "string"]) are not hashable and | ||
| # cannot be tested with `not in {None, ""}`. Normalise to the first | ||
| # non-null element so the rest of the pipeline sees a plain string type. | ||
| if isinstance(node_type, list): | ||
| concrete = next( | ||
| (t for t in node_type if t not in (None, "null", "")), | ||
| "string", | ||
| ) | ||
| return {**node, "type": concrete} |
Collaborator
|
Echo from edge-channels-email:
I received your email titled: "Re: [NousResearch/hermes-agent] fix(moonshot): normalise union type arrays in _fill_missing_type to prevent TypeError (PR #30753)"
You wrote (preview):
"alt-glitch left a comment (NousResearch/hermes-agent#30753)
Duplicate of #28422 — only fixes the _fill_missing_type() crash site (L169), while #28422 fixes both crash sites (L147 _repair_schema + L169 _fill_missing_type). Also duplicates #30226 and #28"
This is an automated test reply from Super AXL.
|
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.
Fix #30095
Problem
When using
kimi-k2.6(or any Moonshot model), any tool whose JSON Schema uses a union type array ("type": ["number", "string"]) causes a crash insidesanitize_moonshot_tools():This is a hard crash — the agent cannot call any tool that has a union-typed parameter.
Root Cause
_fill_missing_type()inagent/moonshot_schema.pyline 169:When
node["type"]is a Pythonlist(e.g.["number", "string"]), thenot in {None, ""}set membership test raisesTypeErrorbecause Python lists are not hashable.Fix
Added a guard at the top of
_fill_missing_type()that detects list types and normalises to the first concrete (non-null) type. Moonshot does not accept union type arrays anyway — it requires a single scalar type — so this is both correct and safe.Testing
TestUnionTypeList:["number", "string"]→"number"["string", "integer"]→"string"["null", "string"]→"string"(null skipped)["null"]→"string"(all-null fallback)