oxidecomputer / oxidecomputer/typify
Confusing interaction between additionalProperties and optional fields.
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 898
- Forks
- 114
- Avg merge
- 4h 18m
- Merged PRs (30d)
- 14
Description
given this schema:
https://github.com/modelcontextprotocol/specification/blob/main/schema/2024-11-05/schema.json#L144
{
"$schema": "http://json-schema.org/draft-07/schema#",
"definitions": {
"ClientCapabilities": {
"description": "Capabilities a client may support. Known capabilities are defined here, in this schema, but this is not a closed set: any client can define its own, additional capabilities.",
"properties": {
"experimental": {
"additionalProperties": {
"additionalProperties": true,
"properties": {},
"type": "object"
},
"description": "Experimental, non-standard capabilities that the client supports.",
"type": "object"
},
"roots": {
"description": "Present if the client supports listing roots.",
"properties": {
"listChanged": {
"description": "Whether the client supports notifications for changes to the roots list.",
"type": "boolean"
}
},
"type": "object"
},
"sampling": {
"additionalProperties": true,
"description": "Present if the client supports sampling from an LLM.",
"properties": {},
"type": "object"
}
},
"type": "object"
}
}
}
My understanding (I could be incorrect) is that all three properties should be optional, as none are specified as required. However, typify produces
pub struct ClientCapabilities {
#[doc = "Experimental, non-standard capabilities that the client supports."]
#[serde(default, skip_serializing_if = "std::collections::HashMap::is_empty")]
pub experimental: std::collections::HashMap<String, serde_json::Map<String, serde_json::Value>>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub roots: Option<ClientCapabilitiesRoots>,
#[doc = "Present if the client supports sampling from an LLM."]
#[serde(default, skip_serializing_if = "serde_json::Map::is_empty")]
pub sampling: serde_json::Map<String, serde_json::Value>,
}
On the serialization side, everything looks good, since these fields will not be present if empty. On the deserialization side, there's some unintended behavior, since the human-language specification seems to intend the presence of the sampling field to have meaning.
If Option<Map> becomes Map is a deliberate design choice, I could probably get away with using a custom typify::TypeSpaceSettings::with_map_type, but I figured I would ask here to be sure.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the ClientCapabilities schema at schema/2024-11-05/schema.json#L144 and the generated Rust type shown in the issue. Trace how additionalProperties and absent required fields are mapped during type generation, then compare that behavior with TypeSpaceSettings::with_map_type. Done means the project has a decided, tested behavior for preserving or intentionally collapsing the presence semantics of sampling and experimental.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- compilers, devtools
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100