-
Notifications
You must be signed in to change notification settings - Fork 11
add a type for input schema properties #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
'properties': { | ||
'operation': { | ||
'type': 'string', | ||
'enum': ['add', 'subtract', 'multiply', 'divide'], | ||
}, | ||
'a': {'type': 'number'}, | ||
'b': {'type': 'number'}, | ||
}, | ||
'a': {'type': 'number'}, | ||
'b': {'type': 'number'}, | ||
'required': ['operation', 'a', 'b'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should make it aligned with this https://github.com/modelcontextprotocol/modelcontextprotocol/blob/e7999bf7c268f786df89b0ffb694bdd62cc84827/schema/2025-03-26/schema.ts#L800
It means that we may want to define InputSchemaProperty
type that has type
, properties
, and required
And the properties
is also the same type.(recursion)
Probably we could also leverage sealed class
since it can be string
or object
type.
Yeah, it's a good starting. Let's improve it together. (I left a comment) |
Thanks for the feedback @leehack. I will make those changes. To be clear, dart is a very new language for me so I apologize for the any silly mistakes and I am wide open to hearing I'm doing it wrong :) |
No worries. Take your time and let me know if you need any support. |
I think I have the recursion wrangled. (Famous Last Words...) At the top level we accept a I renamed For the value of type there appear to be 6 possible values. . You mentioned sealed class. I am not sure if you meant to use it for the type field. We could use an enum for the type to restrict it further than just any arbitrary string. |
I was thinking something like this. This way we can make it typesafe but it'll lose the flexibility. What do you think about this? import 'package:meta/meta.dart';
/// Base class for InputSchema of a JSON Schema.
@immutable
sealed class InputSchema {
/// The JSON Schema type (e.g., "object", "string", "number").
final String type;
const InputSchema({required this.type});
/// Converts the schema to a JSON-like map using pattern matching.
Map<String, dynamic> toJson() {
// Use pattern matching on the instance itself
return switch (this) {
// Match ObjectInputSchema and destructure its properties
ObjectInputSchema(:var properties, :var required) => {
'type': type,
if (properties != null)
'properties': properties.map((key, value) => MapEntry(key, value.toJson())),
if (required != null && required.isNotEmpty)
'required': required,
},
// Match other simple types
StringInputSchema() => {'type': type},
NumberInputSchema() => {'type': type},
BooleanInputSchema() => {'type': type},
};
}
// --- Factory Constructors ---
/// Factory constructor for an Object schema.
factory InputSchema.object({
Map<String, InputSchema>? properties,
List<String>? required,
}) {
return ObjectInputSchema(
properties: properties,
required: required,
);
}
/// Factory constructor for a String schema.
factory InputSchema.string() {
return StringInputSchema();
}
/// Factory constructor for a Number schema.
factory InputSchema.number() {
return NumberInputSchema();
}
/// Factory constructor for a Boolean schema.
factory InputSchema.boolean() {
return BooleanInputSchema();
}
}
/// Represents a simplified JSON Schema "object" type.
@immutable
final class ObjectInputSchema extends InputSchema {
/// Defines the nested properties (fields) of the object.
/// Keys are property names, values are their corresponding schemas.
/// This allows for recursive definitions.
final Map<String, InputSchema>? properties;
/// Lists the names of required properties for this object.
final List<String>? required;
/// Creates a simplified Object schema.
/// Use InputSchema.object factory constructor for external creation.
const ObjectInputSchema({
this.properties,
this.required,
}) : super(type: 'object');
}
/// Placeholder for a basic String schema.
@immutable
final class StringInputSchema extends InputSchema {
/// Use InputSchema.string factory constructor for creation.
const StringInputSchema() : super(type: 'string');
}
/// Placeholder for a basic Number schema.
@immutable
final class NumberInputSchema extends InputSchema {
/// Use InputSchema.number factory constructor for creation.
const NumberInputSchema() : super(type: 'number');
}
/// Placeholder for a basic Boolean schema.
@immutable
final class BooleanInputSchema extends InputSchema {
/// Use InputSchema.boolean factory constructor for creation.
const BooleanInputSchema() : super(type: 'boolean');
}
// Example Usage
void main() {
final simpleSchema = InputSchema.object(
required: ["name"],
properties: {
"name": InputSchema.string(),
"age": InputSchema.number(),
"isActive": InputSchema.boolean(),
"address": InputSchema.object(
properties: {
"street": InputSchema.string(),
"city": InputSchema.string(),
},
),
},
);
final jsonMap = simpleSchema.toJson();
print(jsonMap);
} |
Interesting. Thanks for the code. I definitely wasn't on that brain wave :) My gut instinct is I like it. I like the discovery provided by the explicit typing and I think the code reads better. It does lose some flexibility but I think with regards to the json schema spec we can expect some stability. I don't think being rigid will bite us here once we've covered all of our bases. |
Cool. The sample code might be missing all the optional fields of the json schema spec. Probably we would need to add them otherwise, I suspect it would start to fail a lot of json validation. lol |
I think this is closer to what you were saying in #11 but it feels weird to have inputSchemaProperties.properties. I don't know. Let me know what you think and I can try to put together an actual PR