Commit graph inspector/core/json/rootUnion.ts
Author SHA1 Message Date
cliffhall
775fec0f6a fix: address Copilot review round 10 — structural const match, deep-link constants
- selectBranchIndex compares a discriminator structurally: a `const` may be
  an object or array, and deep-link arguments arrive as freshly parsed
  instances that could never be reference-equal to the schema's own.
- New applySchemaConstants, applied by InspectorView AFTER the deep link's
  `appArgs` are spread over the seeded defaults. The form renders a pinned
  field read-only, so a link disagreeing with one would otherwise auto-open an
  App with a hidden value contradicting the shape on screen.

Signed-off-by: cliffhall <cliff@futurescale.com>
2026-08-26 04:23:14 -04:00
cliffhall
246ee1c262 fix: address Copilot review round 9 — canonical constant comparison, name-based inference, permissive schemas
- rootUnion: schema values are compared through a key-sorted rendering, so two
  discriminator constants written as {a,b} and {b,a} are recognized as the same
  value rather than as mutually exclusive alternatives.
- selectBranchIndex: after the const and required passes, a name only ONE
  alternative declares identifies that alternative; a name several declare is
  ambiguous and says nothing.
- additionalProperties counts as restrictive only when its schema states an
  assertion — an annotation-only `{ title: ... }` is the equivalent of
  `true`, like `{}`.
- Drop the unjustified double casts from the ToolTestModal fixtures; the
  schemas are assignable to Partial<Tool> as written.

Signed-off-by: cliffhall <cliff@futurescale.com>
2026-08-26 04:07:50 -04:00
cliffhall
85b8e4601c fix: address Copilot review rounds 7 and 8
- rootUnion: a `oneOf` demands that EXACTLY one alternative match, which
  flattening cannot preserve, so it is offered only when a discriminator makes
  the alternatives mutually exclusive; `anyOf` makes no such claim and is
  unaffected. An empty `additionalProperties` schema is the equivalent of
  `true` and no longer declines a union.
- selectBranchIndex: falls back to the branch whose own required fields the
  values supply, so an undiscriminated union's values open the picker on the
  shape they describe rather than on the first branch.
- admitsNull: honors a sibling `nullable: true` beside `const: null`.
- SchemaForm: a value is carried only where the incoming branch leaves the
  root's declaration as it found it; anything it declares itself is reset,
  since it may type the name differently from where the value was typed.
  `hasOwn`/`fromEntries` throughout, so `constructor` is not read as a
  supplied value and `__proto__` is not dropped into the prototype setter.
- TUI: the same prototype-safe construction on the way out of the form, and
  the chosen branch's `required` list is checked at submit — rendering a
  branch's fields optional makes the FORM satisfiable, not the call, so the
  missing names are reported rather than a known-invalid call being sent.

Signed-off-by: cliffhall <cliff@futurescale.com>
2026-08-26 03:54:24 -04:00
cliffhall
c15e30414f fix: address Copilot review round 6 — additionalProperties, annotations, prototype-safe records
- rootUnion: decline a union that adds fields under a restrictive root
  `additionalProperties`. The keyword constrains what its SIBLING
  `properties` does not name, so the original schema admits none of the
  branch fields; flattening moves them beside the keyword, where they read as
  allowed and the form would submit what the schema forbids.
- `default` joins the annotation set: it constrains nothing, so two
  declarations suggesting different initial values no longer conflict.
- Property records are read with `Object.hasOwn` and built with
  `fromEntries`/`defineProperty`. `properties` is a JSON record, so
  `constructor` is a legal argument name that `in` reported as a collision
  the root never made, and `__proto__` is one that assignment would drop
  into the legacy prototype setter instead of keeping as a field.
- SchemaForm: a value survives a branch switch unless BOTH branches declare
  the name. A root argument the outgoing branch merely specialized is still a
  root argument in a branch that inherits it, so dropping it erased a valid
  value.

Signed-off-by: cliffhall <cliff@futurescale.com>
2026-08-26 02:56:55 -04:00
cliffhall
f556f8831b fix: address Copilot review round 5 — union member faithfulness, partial discriminators, const-null siblings
- rootUnion: union members get the same flattenability test as allOf members,
  so a branch whose nested `allOf: [false]` makes it unsatisfiable is no
  longer offered as a callable alternative with that constraint erased.
- selectBranchIndex: weigh only the pinned constants the caller SUPPLIED. One
  it did not supply is a value this identification exists to seed, so
  requiring it meant a deep link naming `kind` alone matched no branch
  whenever the branches also pinned, say, a `version`. A supplied constant
  that disagrees is still evidence against.
- admitsNull: `const: null` is conjunctive with its siblings, not an
  override — `{ type: "string", const: null }` and
  `{ const: null, anyOf: [...] }` reject every value, so the claim is made
  only where the type and a sibling union leave null on the table.

Signed-off-by: cliffhall <cliff@futurescale.com>
2026-08-26 02:38:14 -04:00
cliffhall
7e4af5ed34 fix: address Copilot review round 4 — allOf faithfulness, const null, branch-aware seeding
- rootUnion: an `allOf` member is foldable only when it states nothing beyond
  what the merge applies (`type`/`properties`/`required` plus annotations),
  so a nested `anyOf`, a `not`, or an `additionalProperties` can no longer
  be erased along with the keyword; members are checked against the
  ACCUMULATED merge, so two of them contradicting each other is caught even
  when neither contradicts the root.
- admitsNull: `const: null` admits null, so a required field pinned to it is
  no longer seeded with the one value it accepts and then reported missing.
- collectSchemaDefaults takes the values a caller is about to overlay, and
  seeds the branch THEY identify — the App deep link passes its `appArgs`, so
  branch 0's defaults can no longer sit invisibly under branch 1's arguments.
- The branch inference moves into core as `selectBranchIndex`, shared by the
  seeding and the form's picker so the two cannot disagree.

Signed-off-by: cliffhall <cliff@futurescale.com>
2026-08-26 02:24:17 -04:00
cliffhall
d8e9e502be test: cover the non-object property-declaration paths in rootUnion
The previous commit's coverage gate failed on `core/json/rootUnion.ts`
branches (89.81%). A property declared with JSON Schema's boolean form has no
keywords to merge, so the two sides agree only by being the same declaration —
now tested both ways, and the dead `baseProperty !== undefined` guard is gone
(`conflicts` is only reached for a name the base declares).

Signed-off-by: cliffhall <cliff@futurescale.com>
2026-08-26 02:09:44 -04:00
cliffhall
1a271e4931 fix: address Copilot review round 3 — conjunctive keywords, unflattenable allOf, branch inference
- rootUnion: a branch restating a constraint the root states differently is
  declined (keywords at one level are conjunctive and no intersection is
  computed here), while annotations may disagree freely. An `allOf` with a
  member it cannot fold in — JSON Schema's boolean form, or a $ref whose
  constraints are unknown rather than absent — is left intact instead of
  merged-and-stripped, which would turn an unsatisfiable schema into a
  fillable form.
- rootUnion: `declaresAnyFields` counts the raw composition members, so
  `hasInputFields` reports fields for a union the resolver declines and an
  App tool is never auto-invoked with empty arguments.
- SchemaForm: the picker opens on the branch the supplied values identify
  (an App deep link overlays its args, so they can name another branch), and
  a branch switch no longer carries a value the outgoing branch declared —
  the branches may type the same name differently.
- TUI: a base property some branch redeclares is offered in EVERY branch's
  section under that branch's inherited or specialized declaration, so a
  branch that does not redeclare it can still supply a required root
  argument.

Signed-off-by: cliffhall <cliff@futurescale.com>
2026-08-26 02:01:44 -04:00
cliffhall
84cd0c10f0 fix: address Copilot review round 2 — conjunctive merges, generated-name collisions, CLI branch selection
- rootUnion: a name both the root and a branch declare is now MERGED rather
  than replaced, so a root `minimum` survives a branch `maximum`; a branch
  that contradicts the root on a property's `type` describes an unsatisfiable
  value and is declined outright. Branch offerability also requires that
  `properties` really is an object (a malformed `properties: null` threw)
  and that the member's own `type` admits objects.
- rootUnion: `ownFields` becomes `declaredFields` — a branch commonly
  specializes a root property, and consumers must render the specialization.
- TUI: the generated select and branch prefixes step aside from any name the
  schema itself declares (JSON has no reserved namespace), decode filters them
  by exact name, a branch's specialization renders in its own section, and a
  `const` is rendered as a one-option select whose typed value is restored
  from the schema on submit.
- convertToolParameters: identify the branch from the discriminator constants
  the call supplies; when none identifies one, coerce only names every
  declaring branch types the same way rather than picking an arbitrary branch.

Signed-off-by: cliffhall <cliff@futurescale.com>
2026-08-26 01:41:17 -04:00
cliffhall
2782ed75d1 fix: address Copilot review — const precedence, TUI variant model, decline both-keyword unions
- rootUnion: decline a schema carrying BOTH oneOf and anyOf rather than
  reading one and silently dropping the other's constraints.
- SchemaForm: handle `const` before every type dispatch and for every
  constant type, so a numeric/boolean constant, or a schema carrying both
  `const` and `enum`, can no longer offer a forbidden value.
- collectSchemaDefaults / TUI initialValue: `const` outranks `default`,
  which is an annotation a schema may set to something its const rejects.
- convertToolParameters: resolve from the first branch's merged schema, so a
  branch's specialization of a root-declared property is what types the arg.
- TUI: ink-form keys values by field name across the whole form, so two
  branches' `kind` were one field and the later section's initial value
  decided what the earlier submitted. Branch fields now render under prefixed
  names behind a Variant select, and decodeFormValues translates back on
  submit, dropping every branch but the chosen one.

Signed-off-by: cliffhall <cliff@futurescale.com>
2026-08-26 01:01:54 -04:00
cliffhall
5933162dd0 fix: render tool schemas with root-level anyOf/oneOf composition
A tool's `inputSchema` may carry composition keywords at its root — the
2026-07-28 revision makes this explicit — but both form builders enumerate
the root's `properties` and nothing else. A schema keeping its fields on
`allOf`/`oneOf`/`anyOf` branches therefore rendered no controls at all: no
branch picker, no fields, not even the raw-JSON fallback a union-typed
*property* falls back to, so the tool could only be called with empty
arguments.

Adds `core/json/rootUnion.ts`, one flattening shared by all three clients so
they cannot drift on which schemas they can render:

- `allOf` is merged unconditionally (conjunctive — there is no choice to
  present), a root `oneOf`/`anyOf` is returned as branches.
- Web `SchemaForm` renders a Variant picker above the fields and prunes the
  outgoing branch's values on a switch, since they are no longer visible and
  would describe a shape the call is not making.
- TUI `schemaToForm` gives each branch its own section with optional fields —
  ink-form is static, and only one alternative applies to a call.
- `convertToolParameters` looks through the branches, so a CLI `--tool-arg`
  is typed by the schema that declares it rather than sent as a string.

Required-field gating asks whether *any* branch is satisfied, which is
selection-independent and never blocks arguments the schema accepts. A
`const` is seeded like a `default` and rendered read-only: it admits exactly
one value, so anything else the user could type produces a rejected call.

Adds a `root-union-schemas-http.json` showcase server.

Closes #2123

Signed-off-by: cliffhall <cliff@futurescale.com>
2026-08-26 00:37:43 -04:00