-
Notifications
You must be signed in to change notification settings - Fork 1
feat: add validation and update methods to Root CMS Client #851
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
base: main
Are you sure you want to change the base?
Conversation
packages/root-cms/core/validation.ts
Outdated
| @@ -0,0 +1,279 @@ | |||
| import {z} from 'zod'; | |||
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.
i'm not a fan of having a dependency on zod for this, it should be easy enough for us to write our own validators for field data.
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.
I updated this to pull from the zod bundled with genkit - not sure if this follows the "spirit" of your request though. :P
The reason I chose this approach is because when using genkit, you can specify the ai.generate request with a response schema (defined in Zod schema): https://genkit.dev/docs/flows/#defining-and-calling-flows
This essentially guarantees that the AI response follows the structure outlined by the schema. So if we already have Zod in our project we could start using this immediately.
Now, unfortunately, I noticed that the firebase/ai package defines its own Schema and doesn't use Zod: https://github.com/firebase/firebase-js-sdk/blob/5511b4fa7de0e98f3a3b933645584ba758de4405/packages/ai/src/requests/schema-builder.ts#L34
But... this doesn't come with a validate method, so it's not too useful here. :(
Anyway, LMK if after seeing this you'd prefer to remove the dependency entirely.
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.
for validation purposes via the cms client calls we should be able to validate our own data without the conversion to zod (which takes up extra cpu cycles and memory unnecessarily). if we need to convert to zod schemas for passing to genkit then that would make sense but i would consider that a separate call from data validation.
there's a future where we may remove genkit + zod entirely in which case the reason for the zod conversion here would be less clear.
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.
for validation purposes via the cms client calls we should be able to validate our own data without the conversion to zod (which takes up extra cpu cycles and memory unnecessarily). if we need to convert to zod schemas for passing to genkit then that would make sense but i would consider that a separate call from data validation.
there's a future where we may remove genkit + zod entirely in which case the reason for the zod conversion here would be less clear.
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.
done - updated to remove zod and wrote our own validators. the test is actually unchanged, so it should be easy to add a zod converter in the future if we need it with genkit, or a firebase/ai Schema converter too
In order to eventually add MCP and other tools for AI-assisted workflows we need to control the data that comes into the system and reject data that doesn't conform to the schemas (so MCP clients and AI-assist tools can be subject to validation flows and correct their issues).
validateFieldsmethod that works by converting a Root CMS schema to a Zod schema, and then invokes Zod's validate method. Zod was chosen because it's used by genkit (https://genkit.dev/docs/models/#structured-output), and presumably we'll be able to easily convert it to the Firebase AI Schema (https://firebase.google.com/docs/reference/unity/class/firebase/a-i/schema) used by the Firebase AI package.validateoption to theSaveDraftDataOptionsupdateDraftDatamethod on Root CMS client that is similar tosaveDraftDatabut targets a specific field (identified by its path - e.g., 'hero.title' or 'content.0.text'). Validation is also optional for this.With these methods we'll be able to move to the next step which will be to add a simple MCP server with tools like "save doc" and "update doc" to allow MCP clients to update CMS content in a structured way.