Skip to content

Conversation

@ikamman
Copy link

@ikamman ikamman commented May 22, 2025

I think it should be that way. With the Param it doesn't make any sense.

@reneleonhardt
Copy link

@graniet Could you have a look?
There has no AI review helper like CodeRabbit been enabled yet 😉

@graniet graniet requested a review from Copilot June 19, 2025 13:34
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Updates the array item type from ParameterProperty to ParametersSchema, ensuring a unified schema representation.

  • Change ParameterProperty.items field to use ParametersSchema
  • Update ParamBuilder’s items field and method signature accordingly

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/chat/mod.rs Changed items field type to Option<Box<ParametersSchema>>
src/builder.rs Updated items field type and items method signature in ParamBuilder
Comments suppressed due to low confidence (4)

src/chat/mod.rs:93

  • The doc comment above items still refers to parameter property, but the field now uses ParametersSchema. Please update the comment to reflect the new schema type (e.g., "defines the schema for array items").
    pub items: Option<Box<ParametersSchema>>,

src/builder.rs:677

  • [nitpick] The field name items with type ParametersSchema may be ambiguous; consider renaming to item_schema or items_schema to clarify it holds a schema rather than a property.
    items: Option<Box<ParametersSchema>>,

src/builder.rs:706

  • [nitpick] The parameter name schema_property is generic; consider renaming to item_schema to align with the field name and clarify its purpose.
    pub fn items(mut self, schema_property: ParametersSchema) -> Self {

src/builder.rs:706

  • There aren't tests covering the new items method with ParametersSchema; adding unit tests to ensure array parameters serialize correctly would improve coverage.
    pub fn items(mut self, schema_property: ParametersSchema) -> Self {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants