Drive trait schema enums from PreviewTraits canonical lists#164
Merged
Conversation
Step #5 of the architectural roadmap. The previous shape duplicated the 12-string `dynamicTypeSize` enum verbatim across `previewStart` and `previewConfigure` schemas — adding a new dynamic type size meant editing PreviewTraits + both schemas, with no test catching a forgotten edit. Convert `PreviewTraits.validColorSchemes`, `validDynamicTypeSizes`, `validLayoutDirections`, `validLegibilityWeights` from `Set<String>` to ordered `[String]` so they can drive both validation (via `.contains`) and the wire-relevant `enum` arrays in MCP tool schemas (the snapshot test pins the array order in JSON). Add a small `traitProperty(...)` helper in `Sources/PreviewsCLI/Handlers/TraitPropertySchema.swift` that builds the consistent `{type, enum, description}` shape — descriptions stay per-handler since `previewStart` and `previewConfigure` legitimately say different things ("override" vs "Pass empty string to clear"). Behavior preserved: `ListToolsSnapshotTests` confirms wire bytes are identical pre/post. New `TraitSchemaTests` asserts both handlers' enum constraints match the canonical PreviewTraits arrays. Catches the realistic regression: adding a value to PreviewTraits but forgetting to pick it up in a schema. Doesn't catch a contributor copy-pasting the literals inline with matching values — that's a source-level concern beyond runtime testing, and the snapshot test catches any value drift anyway. The Set→Array conversion is a public API shape change in PreviewsCore. Internal usage (`.contains`, `.union`, `.sorted`) all work on arrays or are easily wrapped in `Set(...)` (only `allPresetNames` needed an update). External usage in `BridgeGeneratorTraitsTests` continues to pass — its `==` comparison was already against ordered literals. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two small polish items from the code-reviewer pass: - `traitProperty(...)`: drop the `Dictionary(uniqueKeysWithValues:)` ceremony. Key insertion order is irrelevant because the JSONEncoder used by `ListToolsSnapshotTests` outputs sorted keys — the tuple ordering was inert and invited confusion. - `PreviewTraits.valid*` doc: lead with the wire-shape rationale (why arrays, not Sets) and mention O(n) `.contains` as a consequence rather than a separate sentence. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
Step #5 of the architectural-refactor roadmap. The previous shape duplicated the 12-string `dynamicTypeSize` enum verbatim across `previewStart` and `previewConfigure` schemas — adding a new value meant editing `PreviewTraits` plus both schemas, with no test catching a forgotten edit.
After the refactor, both schemas reference `PreviewTraits.validColorSchemes` / `validDynamicTypeSizes` / `validLayoutDirections` / `validLegibilityWeights` directly. Adding a new value touches one place (`PreviewTraits`) and both schemas pick it up automatically.
Changes
Acceptance gate
What's NOT changed
Test plan
🤖 Generated with Claude Code