-
Notifications
You must be signed in to change notification settings - Fork 1
zod schema #4
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
zod schema #4
Conversation
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.
Pull Request Overview
This PR updates the job and provider constructors to pass the 'model' parameter, while integrating Zod for schema validation of AI jobs. Key changes include:
- Updating calls to the superclass constructor across multiple provider job classes.
- Introducing Zod schemas and updating the job dump() methods to include type, model, and params.
- Refactoring the job loading logic to use a discriminated union based on the new job types.
Reviewed Changes
Copilot reviewed 13 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/providers/* | Update constructors to pass the model to the superclass |
| src/jobs/load.ts | Refactor job loading logic to use 'type' and consistency in parameters |
| src/jobs/job.ts | Introduce Zod schemas and update the base dump method |
| src/jobs/image.ts | Update ImageJob constructor and dump() method |
| src/jobs/embedding.ts | Update EmbeddingJob constructor and dump() method |
| src/jobs/chat.ts | Update ChatJob constructor and dump() method |
| README.md | Update installation instructions to include the zod dependency |
Files not reviewed (4)
- package.json: Language not supported
- test/snapshots/chat.test.ts.snap: Language not supported
- test/snapshots/embedding.test.ts.snap: Language not supported
- test/snapshots/image.test.ts.snap: Language not supported
Comments suppressed due to low confidence (2)
src/jobs/load.ts:39
- [nitpick] Consider including additional context (e.g., job type or provider) in the error message to aid debugging.
throw new Error("Failed to load job");
src/jobs/chat.ts:158
- Ensure that all invocations of ChatJob are updated to supply the 'model' parameter, as the constructor now requires it.
constructor(model: string) {
…, embedding, and image jobs
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.
Pull Request Overview
This PR updates the instantiation of various AI job classes to pass the model parameter into their superclass constructor and integrates Zod for runtime schema validation of AI jobs. It also updates the job dispatch logic in the loader and adjusts the README for Zod installation.
- Constructors in provider classes now call super(model) instead of super().
- The job loader now dispatches jobs based on the "type" field, and job dump functions include type and model information.
- The job schemas are refactored using Zod, and the README reflects the dependency on Zod.
Reviewed Changes
Copilot reviewed 13 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/providers/*.ts | Update of constructors to pass model to super. |
| src/jobs/load.ts | Refactored job dispatch based on jobSchema and type field. |
| src/jobs/job.ts | Introduction of Zod schemas and updates to job dump formatting. |
| src/jobs/image.ts, embedding.ts, chat.ts | Constructors and dump functions updated to include model and type. |
| README.md | Installation instructions updated to include Zod. |
Files not reviewed (4)
- package.json: Language not supported
- test/snapshots/chat.test.ts.snap: Language not supported
- test/snapshots/embedding.test.ts.snap: Language not supported
- test/snapshots/image.test.ts.snap: Language not supported
Comments suppressed due to low confidence (1)
src/jobs/load.ts:24
- The previous branch handling 'listModels' jobs was removed. Confirm that listModels functionality is no longer required or add a condition to support it if needed.
- } else if (obj.models && "listModels" in provider) {
No description provided.