Fix required fields validation and normalize template titles#4
Conversation
The schema title was using the raw template name (e.g. "Document") instead of snake_case (e.g. "document"), causing errors with OpenAI/Anthropic APIs. Additionally, all fields are now always included in the required array, as strict mode requires every property to be listed as required. https://claude.ai/code/session_01VMGZKwtTLfYK7r3pL2514T
There was a problem hiding this comment.
Pull request overview
This PR updates JSON Schema generation for templates/sections and adjusts feature tests accordingly, including normalizing schema document titles and changing how required is emitted.
Changes:
- Normalize schema document
titleforTemplate/EphemeralTemplateusingStr::snake($name). - Change
Section/EphemeralSectionschema generation to always emitrequiredand currently include all field names in it. - Update feature test expectations for the new schema output.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Feature/TemplateTest.php | Updates expected schema document title casing/format. |
| tests/Feature/SectionTest.php | Updates expectations around section schema required fields. |
| tests/Feature/SchematicServiceTest.php | Updates expected schema document title returned by service. |
| tests/Feature/EphemeralTemplateTest.php | Updates expected schema document title casing/format. |
| tests/Feature/EphemeralSectionTest.php | Updates expectations around ephemeral section schema required fields and presence. |
| src/Models/Template.php | Normalizes schema document title with Str::snake(). |
| src/Models/Section.php | Alters section JSON Schema required generation (now includes all fields, always emits key). |
| src/Ephemeral/EphemeralTemplate.php | Normalizes schema document title with Str::snake(). |
| src/Ephemeral/EphemeralSection.php | Alters ephemeral section JSON Schema required generation (now includes all fields, always emits key). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public function toJsonSchemaDocument(): array | ||
| { | ||
| return [ | ||
| '$schema' => config('schematic.schema.draft'), | ||
| 'title' => $this->name, | ||
| 'title' => Str::snake($this->name), | ||
| ...$this->toJsonSchema(), | ||
| ]; |
There was a problem hiding this comment.
PR description mentions that the schema output now always includes the required key (even when empty), but Template::toJsonSchema() still conditionally omits required when there are no enabled sections (if ($required !== [])). If the goal is parity with the updated Section schemas, consider always emitting 'required' => $required here as well, or adjust the PR description to clarify this only applies to section schemas.
| public function toJsonSchemaDocument(): array | ||
| { | ||
| return [ | ||
| '$schema' => config('schematic.schema.draft'), | ||
| 'title' => $this->name, | ||
| 'title' => Str::snake($this->name), | ||
| ...$this->toJsonSchema(), | ||
| ]; | ||
| } |
There was a problem hiding this comment.
PR description says the required key is now always included in schema output, but EphemeralTemplate::toJsonSchema() still omits required when there are no enabled sections (if ($required !== [])). Either align this with the updated EphemeralSection schema behavior (always include required, possibly empty) or clarify in the PR description that the change is limited to section schemas.
| foreach ($this->field_definitions as $field) { | ||
| $properties[$field->name] = $field->toJsonSchemaProperty(); | ||
|
|
||
| if ($field->required) { | ||
| $required[] = $field->name; | ||
| } | ||
| $required[] = $field->name; | ||
| } | ||
|
|
||
| $schema = [ | ||
| 'type' => 'object', | ||
| 'properties' => $properties, | ||
| 'required' => $required, | ||
| ]; |
There was a problem hiding this comment.
This change makes every field required in the generated JSON Schema ($required[] = $field->name;), which contradicts the documented behavior that required: false makes a field optional (typically paired with nullable: true). It also makes FieldDefinition::$required (and the addField(..., bool $required)) ineffective for schema generation. Consider restoring the conditional so only $field->required fields go into required (while still always emitting the required key, possibly as an empty array), so the schema matches both docs and the FieldDefinition API.
| foreach ($this->fieldDefinitions() as $field) { | ||
| $properties[$field->name] = $field->toJsonSchemaProperty(); | ||
|
|
||
| if ($field->required) { | ||
| $required[] = $field->name; | ||
| } | ||
| $required[] = $field->name; | ||
| } | ||
|
|
||
| $schema = [ | ||
| 'type' => 'object', | ||
| 'properties' => $properties, | ||
| 'required' => $required, | ||
| ]; |
There was a problem hiding this comment.
This change makes every field required in the generated JSON Schema ($required[] = $field->name;), which contradicts the documented behavior that required: false makes a field optional (typically paired with nullable: true). It also makes FieldDefinition::$required (and the addField(..., bool $required)) ineffective for schema generation. Consider restoring the conditional so only $field->required fields are listed in required (while still always emitting the required key, possibly empty), so the schema matches both docs and the FieldDefinition API.
Summary
This PR fixes the JSON schema generation to correctly include all fields in the
requiredarray, regardless of their required status, and normalizes template titles to snake_case format for consistency.Key Changes
Required Fields Validation: Modified
EphemeralSectionandSectionclasses to include all fields in therequiredarray instead of only those explicitly marked as required. This ensures the schema accurately reflects all fields that must be present.Template Title Normalization: Updated
EphemeralTemplateandTemplateclasses to convert template names to snake_case format usingStr::snake()when generating JSON schema documents. This provides consistent naming conventions in schema output.Schema Structure Simplification: Removed the conditional check that omitted the
requiredkey when the array was empty. Therequiredkey is now always included in the schema output.Implementation Details
Illuminate\Support\Strutility is now imported in both Template classes to support the snake_case conversion.