Not to use typia.llm.application<Class>() directly in test.
#210
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Pull Request Overview
This pull request refactors test files to use a centralized LlmApplicationFactory utility instead of directly calling typia.llm.application(), and upgrades the typia package from a development version to stable version 10.0.1 and @samchon/openapi to version 5.0.0.
- Introduced
LlmApplicationFactoryutility to standardize LLM application creation from JSON schema applications - Updated all test files to use the new factory pattern for consistent application instantiation
- Upgraded dependencies from development versions to stable releases
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/src/utils/LlmApplicationFactory.ts | New factory utility that converts JSON schema applications to LLM applications with validation support |
| test/src/features/llm/function_calling/validate_llm_function_calling_union.ts | Refactored to use LlmApplicationFactory and pass model string instead of full application |
| test/src/features/llm/function_calling/validate_llm_function_calling_tags.ts | Refactored to use LlmApplicationFactory and pass model string instead of full application |
| test/src/features/llm/function_calling/validate_llm_function_calling_sale.ts | Refactored to use LlmApplicationFactory and pass model string instead of full application |
| test/src/features/llm/function_calling/validate_llm_function_calling_recursive.ts | Refactored to use LlmApplicationFactory and pass model string instead of full application |
| test/src/features/llm/function_calling/validate_llm_function_calling_optional.ts | Refactored to use LlmApplicationFactory and pass model string instead of full application |
| test/src/features/llm/function_calling/validate_llm_function_calling_example.ts | Refactored to use LlmApplicationFactory and pass model string instead of full application |
| test/src/features/llm/function_calling/validate_llm_function_calling_default.ts | Refactored to use LlmApplicationFactory and pass model string instead of full application |
| test/src/features/llm/function_calling/validate_llm_function_calling_additionalProperties.ts | Refactored to use LlmApplicationFactory and pass model string instead of full application |
| test/src/executable/sale.ts | Updated to use LlmApplicationFactory and simplified vendor configuration array |
| test/package.json | Updated typia from development version to stable 10.0.1 |
| pnpm-lock.yaml | Updated lock file entries for [email protected] and @samchon/[email protected] |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (parameters.success === false) | ||
| throw new Error("Failed to compose schema."); |
There was a problem hiding this comment.
These error messages are not descriptive enough. When schema composition fails, it's unclear whether it's the parameters or output schema that failed. Consider providing more context:
- For line 53:
throw new Error("Failed to compose parameters schema."); - For line 81:
throw new Error("Failed to compose output schema.");
Additionally, consider including the actual error details from parameters.error or output.error to help with debugging.
| ILlmSchema<Model>, | ||
| IOpenApiSchemaError | ||
| >; | ||
| if (output.success === false) throw new Error("Failed to compose schema."); |
There was a problem hiding this comment.
This error message is identical to the one at line 53, making it difficult to distinguish between parameter schema composition failures and output schema composition failures. Consider using a more specific message like: throw new Error("Failed to compose output schema.");
Additionally, consider including the actual error details from output.error to help with debugging.
| if (output.success === false) throw new Error("Failed to compose schema."); | |
| if (output.success === false) throw new Error(`Failed to compose output schema. Details: ${JSON.stringify(output.error)}`); |
| texts: await ShoppingSalePrompt.texts(), | ||
| handleParameters: async (parameters) => { | ||
| if (process.argv.includes("--file")) | ||
| fs.promises.writeFile( |
There was a problem hiding this comment.
Missing await keyword before fs.promises.writeFile(). This should be await fs.promises.writeFile(...) to ensure the file write completes before continuing, consistent with the pattern used in handleCompletion at line 72 and other test files.
| fs.promises.writeFile( | |
| await fs.promises.writeFile( |
This pull request primarily updates the
typiaand@samchon/openapidependencies from their previous development versions to the latest stable releases, and refactors the usage of LLM application creation in test files to use a new factory utility. The refactoring improves maintainability and consistency by centralizing model and application instantiation logic. Additionally, file naming conventions in test outputs are updated to reflect these changes.Dependency upgrades:
typiafrom10.0.0-dev.20251107-4to10.0.1and@samchon/openapifrom5.0.0-dev.20251107-3to5.0.0across all relevant entries inpnpm-lock.yamlandtest/package.json. This ensures the codebase uses the latest stable versions, improving reliability and compatibility. [1] [2] [3] [4] [5] [6]Test code refactoring:
typia.llm.applicationwith the newLlmApplicationFactory.convertutility in all LLM function calling test files, standardizing how LLM applications are instantiated and improving code maintainability. [1] [2] [3] [4]Test output consistency:
modelstring instead of the previous application model property, ensuring consistency with the new instantiation approach. [1] [2] [3]These changes collectively modernize dependency management and streamline the test codebase for easier future updates and better clarity.