Skip to content
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

convertOpenAPISchemaToJSONSchema doesnt take into account required params #6699

Closed
5 tasks done
ikalachy opened this issue Sep 5, 2024 · 1 comment · Fixed by #6700
Closed
5 tasks done

convertOpenAPISchemaToJSONSchema doesnt take into account required params #6699

ikalachy opened this issue Sep 5, 2024 · 1 comment · Fixed by #6700
Labels
auto:bug Related to a bug, vulnerability, unexpected error with an existing feature

Comments

@ikalachy
Copy link
Contributor

ikalachy commented Sep 5, 2024

Checked other resources

  • I added a very descriptive title to this issue.
  • I searched the LangChain.js documentation with the integrated search.
  • I used the GitHub search to find a similar question and didn't find it.
  • I am sure that this is a bug in LangChain.js rather than my code.
  • The bug is not resolved by updating to the latest stable version of LangChain (or the specific integration package).

Example Code

    const originalChain = await createOpenAPIChain(spec, {
      ...options,
      llm: llmModel,
      verbose: options.verbose ?? true,
    });

with openapi spec use object param which defines required parameters at the object level using a separate required array.

Error Message and Stack Trace (if applicable)

No response

Description

Currently, the function convertOpenAPISchemaToJSONSchema identifies required parameters only if they are marked with "required": true directly within their property definition.

However, JSON Schema also defines required parameters at the object level using a separate required array, listing the names of properties that are required. Because of this, convertOpenAPISchemaToJSONSchema function might overlook parameters listed in the required array at the object level and therefore not include them as required when constructing API calls.

Examples:
It does take into account only params with defined required=true attribute

 "$schema": "http://json-schema.org/draft-4/schema#",
  "type": "object",
  "properties": {
    "pageNum": {
      "type": "integer",
      "title": "Page number",
      "description": "...",
      "default": 1
    },
  "required": ["pageNum"]
    },

With login we wont get pageNum as a required field and later llm wont provide values for required params as function doen't know that it's required param.

Current implementation will consider only fields with required in the definition of each field, as in this example:

{
  "$schema": "http://json-schema.org/draft-03/schema#",
  "type": "object",
  "properties": {
    "pageNum": {
      "type": "integer",
      "title": "Page number",
      "description": "...",
      "default": 1,
      "required": true
    }
}

System Info

"@langchain/community": "^0.2.31",
"@langchain/core": "^0.2.28",

mac
% node --version
v18.18.2
% yarn --version
1.22.22

Copy link

dosubot bot commented Sep 5, 2024

Hey there, @ikalachy! I'm here to assist you with any bugs, questions, or contributions you might have. Let's tackle this issue together.

The function convertOpenAPISchemaToJSONSchema in LangChain.js currently does not account for required parameters defined at the object level using a separate required array. It only checks for the required property within each individual property schema. This means that parameters listed in the required array at the object level are overlooked and not included as required when constructing API calls.

Here is the relevant part of the code:

export function convertOpenAPISchemaToJSONSchema(
  schema: OpenAPIV3_1.SchemaObject,
  spec: OpenAPISpec
): JsonSchema7Type {
  if (schema.type === "object") {
    return Object.keys(schema.properties ?? {}).reduce(
      (jsonSchema: JsonSchema7ObjectType, propertyName) => {
        if (!schema.properties) {
          return jsonSchema;
        }
        const openAPIProperty = spec.getSchema(schema.properties[propertyName]);
        if (openAPIProperty.type === undefined) {
          return jsonSchema;
        }
        // eslint-disable-next-line no-param-reassign
        jsonSchema.properties[propertyName] = convertOpenAPISchemaToJSONSchema(
          openAPIProperty,
          spec
        );
        if (openAPIProperty.required && jsonSchema.required !== undefined) {
          jsonSchema.required.push(propertyName);
        }
        return jsonSchema;
      },
      {
        type: "object",
        properties: {},
        required: [],
        additionalProperties: {},
      }
    );
  }
  if (schema.type === "array") {
    return {
      type: "array",
      items: convertOpenAPISchemaToJSONSchema(schema.items ?? {}, spec),
      minItems: schema.minItems,
      maxItems: schema.maxItems,
    } as JsonSchema7ArrayType;
  }
  return {
    type: schema.type ?? "string",
  } as JsonSchema7Type;
}

The function iterates over the properties of the object and converts each property schema individually. It only adds properties to the required array if the required property is set within the individual property schema, not if it is set at the object level [1].

To continue talking to Dosu, mention @dosu.

@dosubot dosubot bot added the auto:bug Related to a bug, vulnerability, unexpected error with an existing feature label Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto:bug Related to a bug, vulnerability, unexpected error with an existing feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant