fix(kosong/kimi): move parent type into anyOf/oneOf/allOf items#2182
fix(kosong/kimi): move parent type into anyOf/oneOf/allOf items#2182josephkehan-prog wants to merge 1 commit into
Conversation
Moonshot's tool schema validator rejects nodes that declare `type` on the
same level as a combinator:
400 tools.function.parameters is not a valid moonshot flavored json
schema, details: <At path 'root': when using anyOf, type should be
defined in anyOf items instead of the parent schema>
normalizeKimiToolSchema had two gaps:
1. The root schema object was never normalized — ensureKimiPropertyTypes
only recursed into child schemas.
2. No node (root or nested) handled the type+combinator rule, so tools
whose parameters root is e.g. { type: 'object', anyOf: [...] } (or
that nest that shape under properties / not / if-then-else) were sent
as-is and rejected with HTTP 400.
Add fixCombinatorParentType(), which copies the parent `type` into each
combinator item that lacks one and removes it from the parent (the parent
type constrained every variant anyway, so semantics are preserved). It
runs on the root and on every visited node. Items that already declare
their own `type` are left untouched.
Refs MoonshotAI#792
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5803d73476
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| item['type'] = node['type']; | ||
| } | ||
| } | ||
| delete node['type']; |
There was a problem hiding this comment.
When a schema contains more than one combinator, the first iteration deletes node.type, so subsequent combinators receive undefined; JSON serialization then omits those item types. For example, {type:'object', anyOf:[{required:['a']}], oneOf:[{required:['b']}]} leaves the oneOf item typeless, so Moonshot can still reject the normalized schema. Capture the parent type before the loop and delete it only after all combinators have been processed.
Useful? React with 👍 / 👎.
| if (isRecord(item) && !hasOwn(item, 'type')) { | ||
| item['type'] = node['type']; |
There was a problem hiding this comment.
When a combinator item already has a conflicting type, leaving it untouched while deleting the parent type changes the schema's accepted values. For example, {type:'object', anyOf:[{type:'string'}, {required:['a']}]} originally rejects strings because the parent requires an object, but the normalized schema accepts them through the first branch; {type:'object', allOf:[{type:'string'}]} similarly changes from unsatisfiable to a string schema. The rewrite must preserve the parent constraint for explicitly typed items as well, or reject conflicting schemas rather than broadening them.
Useful? React with 👍 / 👎.
1 participant
Problem
Moonshot's server-side tool schema validator rejects JSON Schemas that declare
typeon the same node as a combinator:Any tool whose parameter schema has
typealongsideanyOf/oneOf/allOf— whether at the root (e.g.{ "type": "object", "properties": {...}, "anyOf": [...] }) or nested underproperties/not/if-then-else— is sent to the API unmodified and the request fails with HTTP 400.Root cause
normalizeKimiToolSchemainpackages/kosong/src/providers/kimi-schema.tsfills in missingtypefields, but:ensureKimiPropertyTypesonly callsrecurseSchema, which visits child schemas but not the root node itself.Refs #792 (same bug class, different validator detail).
Fix
Add
fixCombinatorParentType(node): when a node declares bothtypeand a combinator (anyOf/oneOf/allOf), copy the parenttypeinto each combinator item that lacks one and delete it from the parent. Since the parenttypeconstrained every variant anyway, distributing it into the items preserves the schema's semantics. Items that already declare their owntypeare left untouched.It is called on the root (in
ensureKimiPropertyTypes) and on every visited node (innormalizeProperty), so all combinator positions covered byCHILD_SCHEMA_SLOTSare handled.TYPE_COMPLETION_SKIP_KEYSalready includes the combinator keywords, so the existing type-inference logic does not re-add a parenttypeafterwards.Verification
Ran the patched normalizer against the failing shapes:
{type:"object", properties:{...}, anyOf:[{required:["input"]},{required:["runInput"]}]}→
anyOf:[{required:["input"],type:"object"},{required:["runInput"],type:"object"}], parenttyperemoved ✓{type:"object", anyOf:[...]}underproperties.<name>→ fixed recursively ✓type→ preserved; only the parenttypeis dropped ✓Note: I validated against the built dist output of this package; a maintainer may want to add a unit test case to the kosong test suite covering the type+combinator rule.