fix: render nested/Optional arrays as structured forms & align string field style with ToolsTab#1258
Conversation
- Replace <Input type="text"> with <Textarea> for plain-text string properties in DynamicJsonForm, matching the height and style of direct-parameter string inputs in ToolsTab - Special-format strings (email, uri, date, date-time) keep <Input> with their appropriate type attribute - Boolean fields already used <Switch>; update tests to query role="switch" / aria-checked instead of role="checkbox" - Update test assertions to reflect the textarea element type Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
Overview
The PR fixes two real, user-visible issues in DynamicJsonForm:
- FastMCP/Pydantic v2 schemas (
List[Model],Optional[List[T]],List[Optional[T]]) were falling back to a raw JSON editor instead of getting structured Add/Remove controls. The fix routes those schemas through the existingnormalizeUnionTypehelper before type dispatch. - Plain string fields in
DynamicJsonFormrendered as single-line<Input>whileToolsTabrenders direct-parameter strings as<Textarea>— a visible mismatch. Now both use<Textarea>.
Also swaps the boolean checkbox for shadcn <Switch> and tidies description whitespace.
The core approach is sound: normalizing anyOf:[X,null] at the top of renderFormFields and on propSchema.items cleanly handles all three Optional/nested patterns with a single helper instead of scattering special-cases through the type switch.
What's done well
normalizeUnionTypeis applied at exactly the right entry points — top ofrenderFormFieldsand onpropSchema.items— soOptional[List[T]],List[Optional[T]], and top-levelOptional[Array]all converge on the same code paths.- The new array case correctly distinguishes "items are an object with properties" from
isSimpleObjectitems, and the per-card layout with a bottom-right Remove button is reasonable. - The new tests are targeted and meaningful — the four cases in "Complex Array Rendering" precisely cover the bugs the PR claims to fix, and the union-type test (
anyOf:[array,null]) is the kind of test that would have caught this regression in the first place. - Style alignment with
ToolsTab.tsxis verified — that component does renderprop.type === "string"as<Textarea>, so this PR brings the nested form into line.
Issues to address
1. Regression: pattern validation silently dropped for plain strings (correctness)
Plain-text strings switched from <Input pattern={...}> to <Textarea>, which has no pattern attribute. The PR removes the corresponding test with a comment explaining <textarea> doesn't support it, but schema.pattern constraints from server-side tool schemas will now be ignored client-side — a real loss of validation that isn't called out in the PR description.
Suggested fix: keep <Input type=\"text\"> when propSchema.pattern is present, or implement pattern checking manually (onBlur validation against new RegExp(propSchema.pattern)).
2. Regression: top-level array description disappears
The diff removes these blocks from the structured array case:
- {propSchema.description && (
- <p className=\"text-sm text-gray-600\">{propSchema.description}</p>
- )}
- {propSchema.items?.description && (
- <p className=\"text-sm text-gray-500\">Items: {propSchema.items.description}</p>
- )}Two test assertions for \"Test array field\" and \"Items: Array item\" were correspondingly deleted. The PR description doesn't mention this. For top-level array schemas (where there's no parent object label upstream to surface the description), users will lose this context.
Suggested fix: restore both blocks above the items list, or move into the per-card object header.
3. required attribute dropped on booleans (minor)
Previous <Input type=\"checkbox\" required={isRequired}> is replaced with <Switch>, which doesn't accept required. Usually inconsequential for booleans (false is a valid value), but inconsistent with the rest of the form. Probably acceptable, but worth a deliberate decision rather than a silent drop.
4. Parameter reassignment style (minor)
In renderFormFields, both propSchema = normalizeUnionType(propSchema) and the description-trim block reassign the function parameter. Functionally fine, but a local const normalizedSchema = ... would read more clearly given the function is ~300 lines long.
5. "True"/"False" label (minor UX)
The new <span> next to the Switch reads \"True\" : \"False\". Most toggle UIs either omit a value label entirely (relying on the toggle's on/off visual + a separate field label) or use lowercase/sentence-case. Capitalized "True"/"False" reads more like a Python repr than a UI element.
Smaller observations
- The
specialFormatMaprefactor (typedRecord<SpecialFormat, string>) is a nice tightening — restrictsinputTypeto known good values. - Description-trimming is reasonable defensive handling for Pydantic docstrings (
\"\"\"\n text \n\"\"\"). Allocating a new schema object on every render in a recursive function is wasteful but negligible at typical form sizes; not worth optimizing. canRenderTopLevelFormnow usesnormalizeUnionTypecorrectly. Note this meansOptional[primitive]at the top level becomes form-capable — that's the desired behavior but worth confirming with a test (none added).- Test changes around
role=\"switch\"/aria-checkedare the canonical RTL approach for shadcn Switch.
Risk summary
- Behavioral — Two silent regressions (pattern validation, array description) that aren't reflected in the PR description. The pattern one in particular has correctness implications if tool authors rely on
patternfor input-shape constraints. - Test coverage — Good for the bugs being fixed; insufficient for the regressions introduced (no test that
patternis enforced; assertion for the array description was deleted rather than relocated). - Performance — No concerns.
- Security — Loss of client-side
patternvalidation is not a security boundary (server should always re-validate), but it does weaken UX-level input guardrails.
Recommendation
The core fix is correct and valuable, but the PR should preserve pattern validation and the array description. Items 3–5 are polish and not blocking.




Problem
Two related rendering gaps in
DynamicJsonForm:List[T]→{ type: "array", items: { type: "object", properties: {...} } }Optional[List[str]]→{ anyOf: [{ type: "array", items: {...} }, { type: "null" }] }List[Optional[T]]→ items wrapped inanyOf: [object, null]All three fell back to a raw JSON editor instead of structured Add/Remove controls.
DynamicJsonFormrendered plain-text strings as<Input>(single-line, 36 px), whileToolsTabrenders direct-parameter strings as<Textarea>— visually inconsistent.Solution
DynamicJsonForm.tsxcanRenderTopLevelForm— callsnormalizeUnionType(s)first soanyOf:[array,null]top-level schemas enable form mode.renderFormFields— callsnormalizeUnionType(propSchema)at the top of every recursive call soanyOf:[X,null]fields resolve to their real type before any switch/depth check.case "array"— normalizespropSchema.itemsthroughnormalizeUnionType; gates structured rendering onisSimpleObject(itemSchema) || (itemSchema.type === "object" && !!itemSchema.properties)so arrays of objects get Add/Remove controls instead of falling back to JSON.case "string"— plain text now renders as<Textarea>(matchingToolsTab); special formats (email,uri,date,date-time) keep<Input type="...">.case "boolean"— replaced<input type="checkbox">with the shadcn<Switch>component for a cleaner toggle UI; description renders above the toggle.propSchema.descriptionto strip leading/trailing whitespace from Python triple-quoted docstrings.Tests
DynamicJsonForm.test.tsx— updated string assertions fromtype="text"→type="textarea", boolean assertions fromrole="checkbox"→role="switch"/aria-checked,querySelector("input")→querySelector("textarea"), removed inapplicablepatterntest.DynamicJsonForm.array.test.tsx— replaced legacy "Complex Array Fallback" block with four targeted cases covering: object arrays, untyped fallback,Optional[List[X]], andList[Optional[X]]; updated boolean array assertions torole="switch".Test plan
cd client && npm test— all 63 tests passnpm run lintpasses with no new warningsList[SomeModel]parameter → fields render as structured form with Add/RemoveOptional[List[str]]→ same structured form, not a JSON boxemail,date, etc.) still render as typed<input>elements