Keep doc annotations when a type is inlined into a schema - #189
Merged
Merged
Conversation
`-spectra()` doc annotations (title, description, deprecated, examples, examples_function) only reached the generated schema for the type that schema generation was entered with. Every type resolved while inlining lost them silently, so `deprecated => true` on a type used as a map field produced nothing at all in the output. Only to_schema_for_sp_type/3 merges a type's doc into its schema, and the recursive descents in do_to_schema/3 bypassed it. Route them all through to_schema_for_sp_type/3: user type and remote type resolution, map field values (exact and optional), record fields, union branches and anyOf members, and list and non-empty list elements. Docs merge along the resolution chain, so on a conflicting key the annotation written nearest the use site wins. That matches how an alias of a documented record already behaved (record_ref_with_own_doc_test); keys only one of them sets are kept from both. type_parameters were unaffected by the bug and keep applying alongside a doc on the same type. This changes generated JSON Schema and OpenAPI output for annotated sub-schemas, so it belongs in a minor release. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Nffy1QUKxifokPEBLX3Qc
eqwalizer rejects passing a json:decode_value() straight to json_schema_validator_helper:validate_or_skip/1, which takes a map(). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Nffy1QUKxifokPEBLX3Qc
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Enum-optimized unions and codec-provided nested schemas can still lose declared annotations, with additional advertised paths lacking coverage.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Ensures nested JSON Schema/OpenAPI types retain their -spectra() annotations.
Changes:
- Routes recursive schema generation through annotation merging.
- Adds inline annotation regression tests.
- Documents propagation and precedence behavior.
File summaries
| File | Description |
|---|---|
src/spectra_json_schema.erl |
Preserves annotations during recursive schema generation. |
test/spectra_json_schema_inline_doc_test.erl |
Tests nested annotation propagation. |
test/inline_doc_remote_helper.erl |
Provides an annotated remote type fixture. |
README.md |
Documents annotation propagation and precedence. |
CHANGELOG.md |
Records the behavior fix. |
Review details
Suppressed comments (1)
src/spectra_json_schema.erl:411
- Annotated enum aliases are still dropped when they appear in a union whose members all expand to literals. That path returns the unified schema from
try_generate_enum_schema/3, sogenerate_anyof_schema/3(and thereforeto_schema_for_sp_type/3) is never called. For example, an annotatedstatus() :: active | inactiveused instatus() | pendingloses its annotation, contrary to the stated union-branch behavior. Preserve the optimization only when no member resolves to documented metadata, or emit documented branches viaanyOf.
Schemas = lists:map(fun(T) -> to_schema_for_sp_type(TypeInfo, T, Config) end, Types),
- Files reviewed: 5/5 changed files
- Comments generated: 4
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The record field descent is a separate code path from the map field one and had no coverage, and examples_function was claimed to follow an inlined type without a test evaluating the MFA in that position. Add a record holder whose fields use an annotated type and an examples_function type. Name examples_function in the README list, and state the two positions that cannot carry a nested annotation: a union of literals collapsed into one enum schema, and a type whose schema comes from a custom codec. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Nffy1QUKxifokPEBLX3Qc
Review follow-ups. Tests: assert whole schemas instead of loose patterns for the union branches and list elements, and add a negative case pinning that an unannotated type stays bare and a sibling's annotation does not leak onto it or the parent. Docs: the CHANGELOG entry now separates the fix from the behaviour changes and names two consequences of merging at every position — examples are validated and converted there, so an invalid example raises where it used to pass unnoticed, and examples_function runs once per position. The README adds the parameterized-type gap to the list of positions that do not carry the annotation, and clarifies that an alias of a codec-handled type does keep its own annotation. Also record in the source comment that only the two reference-resolution sites change output today; the descent sites hold the invariant for callers that build an sp_type() tree with a doc on a structural child. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Nffy1QUKxifokPEBLX3Qc
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Nffy1QUKxifokPEBLX3Qc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
-spectra()doc annotations (title,description,deprecated,examples,examples_function) reached the generated schema only for the type that schema generation was entered with. Every type resolved during inlining lost its annotations silently, sodeprecated => trueon a named type used in a map-field position produced nothing in the output, with no warning.type_parameterswere unaffected: they are applied from the type node itself rather than from its doc.The cause
to_schema_for_sp_type/3is the only path that merges a type's doc into its schema. The recursive descents insidedo_to_schema/3calleddo_to_schema/3directly and so bypassed the merge.The change
Every descent and every reference resolution now goes through
to_schema_for_sp_type/3::=) and optional (=>) alikeanyOfmembersWorth being precise about which of those carry the fix. A doc only reaches a type node through
spectra_abstract_code, which attaches it to the top node of a stored named type or record, so today only the two reference-resolution sites change the output. Reverting any one of the other six leaves the whole suite green. They are kept because they hold the invariant for a caller that builds ansp_type()tree by hand with a doc on a structural child, which the Elixir wrappers do, and because a partial rule is the thing that caused this bug. The comment onto_schema_for_sp_type/3records this.Precedence
Docs merge along the resolution chain, and on a conflicting key the annotation written nearest the use site wins. Keys that only one of them sets are kept from both. This preserves the behaviour
record_ref_with_own_doc_testalready pinned for a type alias of a documented record, where the alias's owntitlewins over the record's. For the reported case there is no conflict: the parent's field position carries no annotation, so the inlined type's own annotation lands on the property, which is the fix.Compatibility
This changes generated JSON Schema and OpenAPI output for anyone already using annotated sub-schemas: nested properties now carry the titles, descriptions, deprecation flags and examples their types declare. It is the documented behaviour of the annotation, but it is visible, so it belongs in a minor release.
Two consequences of merging at every position, both in the CHANGELOG and README:
examplesare validated and converted at each position, so an example that does not encode as its own type now raises{invalid_example, ...}where it previously passed unnoticed unless that type was a schema entry point.examples_functionis invoked once per position rather than once per schema.Three positions still do not carry the annotation, all unchanged here: a union of literals that collapses to one
enumschema, a type whose schema comes from a custom codec, and a parameterized type. The README lists them.Tests
New
test/spectra_json_schema_inline_doc_test.erl, withtest/inline_doc_remote_helper.erlas the other-module type source. Twelve tests, all failing onmain.title,description,deprecatedandexamplesobject() | deprecated_string(), where only the string branch is deprecatedoptional(key) => named_type()value, which also asserts the key stays out ofrequiredexamples_functionon an inlined type, asserting the MFA is evaluated and its values convertedtype_parameters(maxLength,pattern) still apply alongside a doc annotation on the same typeEach generated schema is also run through the existing Python JSON Schema validator helper.
Known issues, not addressed here
#{binary() => documented()}) now has its examples converted even though the resulting schema is discarded, becauseadditionalPropertiesis a boolean. An invalid example therefore raises for a fragment that is thrown away. Suppressing that needs a way to mark the traversal as validation-only.spectra_openapi:type_doc/3still replaces a doc wholesale rather than merging along the chain. An alias that sets onlytitlehides the referenced type'sdescriptionanddeprecatedat the parameter level, while the parameter'sschemanow carries both. The two layers of the same document disagree.Verification
Every target CI runs was run locally:
make test(756 tests),make proper(9/9 properties),make type_check,make lint,make dialyzer,make xref,make hank,make check_app_callsandmake format_verify.🤖 Generated with Claude Code
https://claude.ai/code/session_017Nffy1QUKxifokPEBLX3Qc