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

Some $ref values are not found during the conversion even if they are defined. #104

Closed
numbbbbb opened this issue Dec 4, 2024 · 13 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request good first issue Good for newcomers question Further information is requested

Comments

@numbbbbb
Copy link

numbbbbb commented Dec 4, 2024

The example swagger file.

Check the output of OpenApi.convert(swagger).paths, you can see there are still $ref in it. Not sure if it's a bug or expected behavior.

@samchon samchon self-assigned this Dec 4, 2024
@samchon
Copy link
Owner

samchon commented Dec 4, 2024

I'll test the file.

@numbbbbb
Copy link
Author

numbbbbb commented Dec 4, 2024

Probably related to the logic here. Especially this line, if schema is a string spread it with ...schema seems weird.

@samchon samchon added bug Something isn't working enhancement New feature or request labels Dec 4, 2024
@samchon
Copy link
Owner

samchon commented Dec 4, 2024

Found that v3.1 converted OpenAPI document has the #/definitions/AuthorWithPapers like reference as you've mentioned.

Thanks for bug reporting.

I'll fix it ASAP.

@samchon
Copy link
Owner

samchon commented Dec 4, 2024

I had not checked the allOf case. This is the reason of the bug.

@numbbbbb
Copy link
Author

numbbbbb commented Dec 4, 2024

Thanks a lot!

@samchon
Copy link
Owner

samchon commented Dec 4, 2024

semanticscholar.json

Succeeded to fix the bug, and would be published to v2.0.1 soon.

@samchon samchon closed this as completed in fe4f81e Dec 4, 2024
samchon added a commit that referenced this issue Dec 4, 2024
Fix #104: conversion OpenAPI version when `allOf.length` is 1.
@samchon
Copy link
Owner

samchon commented Dec 4, 2024

Upgrade to 2.0.1 version please.

samchon added a commit to samchon/nestia that referenced this issue Dec 4, 2024
Related PR in `@samchon/openapi`: samchon/openapi#105
samchon added a commit to samchon/nestia that referenced this issue Dec 4, 2024
@numbbbbb
Copy link
Author

numbbbbb commented Dec 4, 2024

I feel like the 2.0.1 introduced more issues related to $ref.

To reproduce:

const swagger =
	await fetch(
		"https://api.semanticscholar.org/graph/v1/swagger.json",
	).then((r) => r.json()).then(a => {
		console.log(a);

		return a;
	});

const document = OpenApi.convert(swagger);
console.log(JSON.stringify(document));

I've attached the result. You can see it sill has many $ref in it.

result.json

samchon added a commit that referenced this issue Dec 4, 2024
@samchon
Copy link
Owner

samchon commented Dec 4, 2024

https://github.com/samchon/openapi/blob/master/test/features/issues/test_issue_104_upgrade_v20_allOf.ts

I enhanced the test function about this issue by composing IHttpLlmApplication by the Swagger v2 document. And no error had reported during the LLM function calling schema composition.

What is the problem point? Can you point a detailed accessor of it?

If what you want is erasing $ref properties during the OpenApi.IDocument conversion, I think it is not proper approach. No reason to escape $ref type in the OpenApi.IDocument type conversion. Instead, if what you want to is to composing an LLM function calling schema erasing from every $ref type like Google Gemini case, and you can accomplish it like below:

const swagger: SwaggerV2.IDocument = {...};
const document: OpenApi.IDocument = OpenApi.convert(swagger);
const app: IHttpLlmApplication<"gemini"> = HttpLlm.application({
  model: "gemini",
  document,
});
TestValidator.equals("errors")(app.errors.length)(0);

@samchon samchon added good first issue Good for newcomers question Further information is requested labels Dec 4, 2024
@samchon samchon reopened this Dec 4, 2024
@numbbbbb
Copy link
Author

numbbbbb commented Dec 4, 2024

Thanks @samchon! That makes sense.

Let me explain the feature I'm working on and hopefully get some insights from you.

What I'm trying to do in my project:

  1. convert any source OpenAPI JSON to the normalized OpenAPI 3.1.0, this step is already supported by your project
  2. extract and store the APIs into a database, for now I'll store each entry in the document.paths. So if there are 10 paths, I'll create 10 rows in the database, each row stores one entry in document.paths.
  3. when I need to use one API, I'll get it from the database and convert it to a function call parameter for a target model

The challenge is that I don't want to store the full document in the database which is too large and hard to modify. But if I only store the document.paths the $ref in it won't work because schemas are not stored.

So the solution I have right now is:

  1. replace all $ref with the real schema so each row in the database has everything it needs and don't rely on the schemas
  2. when I need an API, I'll wrap the individual path object I get from the database to make it looks like a complete OpenAPI 3.1.0 object. Then use HttpLlm.application() to get the function call parameters

Seems like I can't erase $ref from the OpenApi.IDocument for now. So probably I need to store the schemas somewhere and combine it with individual API path before calling HttpLlm.application(). It's doable but if you have a better solution please let me know.

@samchon
Copy link
Owner

samchon commented Dec 4, 2024

Then collect only referenced components.schemas for each operation through OpenApiTypeChecker module's visit and isReference functions.

For reference, due to recursive reference case, erasing $ref type is not always possible.

https://github.com/samchon/openapi/blob/master/src/utils/OpenApiTypeChecker.ts

@samchon
Copy link
Owner

samchon commented Dec 4, 2024

Here is the LLM schema converter module, but may good to reference.

https://github.com/samchon/openapi/blob/master/src/composers/llm/LlmSchemaV3_1Composer.ts

@numbbbbb
Copy link
Author

numbbbbb commented Dec 4, 2024

Thanks! I'll take a look.

@numbbbbb numbbbbb closed this as completed Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request good first issue Good for newcomers question Further information is requested
Projects
Status: Done
Development

No branches or pull requests

2 participants