fix(spec): LanguageModelV3ToolResult["result"] change from unknown to NonNullable<JSONValue>
#9931
Conversation
gr2m
changed the title
.result is unknown, but not nullableLanguageModelV3ToolResult["result"] is unknown, but non-nullable
|
We also need to remove the |
|
Removed the |
| * Result of the tool call. This is a JSON-serializable object. | ||
| */ | ||
| result: unknown; | ||
| result: NonNullable<unknown>; |
There was a problem hiding this comment.
we may want to also consider NonNullable<JSONValue> now that JSONValue supports undefined but the implications are unclear
There was a problem hiding this comment.
yeah I. thought so too. I'll do that and see how it goes
There was a problem hiding this comment.
shouldn't it work with JSONObject | JSONArray?
There was a problem hiding this comment.
Hmm it's tricky because it means we cannot use unknown for any (deep) value of the result object
like in line 50 here
ai/packages/openai/src/tool/file-search.ts
Lines 43 to 60 in f0b2157
There was a problem hiding this comment.
the z.unknown() should be z.union([z.string(), z.number(), z.boolean()]) as per OpenAPI spec, will update
OpenAPI spec is
```json
{
"anyOf": [
{
"additionalProperties": {
"anyOf": [
{
"maxLength": 512,
"type": "string"
},
{
"type": "number"
},
{
"type": "boolean"
}
]
},
"description": "Set of 16 key-value pairs that can be attached to an object. This can be\nuseful for storing additional information about the object in a structured\nformat, and querying for objects via API or the dashboard. Keys are strings\nwith a maximum length of 64 characters. Values are strings with a maximum\nlength of 512 characters, booleans, or numbers.\n",
"maxProperties": 16,
"propertyNames": {
"maxLength": 64,
"type": "string"
},
"type": "object"
},
{
"type": "null"
}
]
}
```
https://github.com/gr2m/ai-provider-monitor/blob/fe2659312a6344501c90942e2bf2baca06f5f9d9/cache/openai/routes/responses/post.json#L2571
gr2m
changed the title
LanguageModelV3ToolResult["result"] is unknown, but non-nullableLanguageModelV3ToolResult["result"] change from unknown to JSONObject | JSONArray
| attributes: z.record( | ||
| z.string(), | ||
| z.union([z.string(), z.number(), z.boolean()]), | ||
| ), |
There was a problem hiding this comment.
as a side note: JSON API / JSON Schema specs do not have an "unknown" type. I haven't checked all providers we support yet, but all the important ones do have a JSON API spec for their APIs.
We still use .unknown() in other places related to provider requests/responses that we could probably all get rid off with some research.
| * Result of the tool call. This is a JSON-serializable object. | ||
| */ | ||
| result: unknown; | ||
| result: JSONObject | JSONArray; |
There was a problem hiding this comment.
will change to NonNullable<JSONValue> after discussing with @lgrammel
gr2m
changed the title
LanguageModelV3ToolResult["result"] change from unknown to JSONObject | JSONArrayLanguageModelV3ToolResult["result"] change from unknown to NonNullable<JSONValue>
Background
Discovered in #9896 (review)
Summary
As far as I can tell,
resulthas to be set to something that is not null or undefined. This makes the spec more precise and would have surfaced the problem of #9896 (review) immediatelyManual Verification
When the fix from this PR is applied, a type error is thrown in #9896
Checklist
pnpm changesetin the project root)Future Work
Related Issues