Skip to content

feat(snippets): render root-level oneOf/anyOf constraints on generated Code pages - #1585

Open
mattmillerai wants to merge 1 commit into
docs/router-model-page-pilotfrom
matt/be-10492-root-oneof-constraints
Open

feat(snippets): render root-level oneOf/anyOf constraints on generated Code pages#1585
mattmillerai wants to merge 1 commit into
docs/router-model-page-pilotfrom
matt/be-10492-root-oneof-constraints

Conversation

@mattmillerai

Copy link
Copy Markdown
Contributor

STACKED — merging lands on docs/router-model-page-pilot (owned by the author of #1533), NOT main. Do not read this as safe to merge to the default branch: every file it touches exists only on that branch, so this PR is only mergeable after #1533 lands. GitHub will retarget it to main automatically when that happens.

What this does

A constraint that spans two fields ("exactly one of text_prompt or json_prompt") has no per-field representation in JSON Schema. It belongs on the schema root as a oneOf/anyOf whose branches carry nothing but required. schemaFields() walked only required + properties, so a root union rendered as nothing, and the Ideogram 4.0 Code page implied {} was an acceptable request body.

  1. gen-code-pages.ts gains rootAlternation(), called from schemaFields() after deref. When a root oneOf/anyOf's branches differ only by required, it emits one prose line above the field list (oneOf -> "Provide exactly one of ...", anyOf -> "Provide at least one of ...", backticked names, Oxford-joined from three up). The walk then covers the union of root and branch properties, so a union that carries the only property definitions still renders its fields rather than tripping the empty-schema fallback. The alternated fields stay individually optional: neither is required on its own.
  2. ideogram-v4/code.yaml gets that root oneOf. Existing per-field prose is unchanged.
  3. snippets/README.md documents the shape, since code.yaml authors are the audience for it.

Branches carrying anything beyond required/properties (a type, an enum, a property redefining a root field to a different shape) are a genuine choice of shapes, not an alternation, and keep rendering per field as a | b.

Why oneOf and not the anyOf the review comment suggested

I checked the provider's live spec rather than taking either the review comment or the plan at face value, and it settles the wording more strongly than either did. GenerateImageRequestV4 carries no root oneOf/anyOf/allOf and no required, so {} really does validate. But its root schema description states the contract in words:

Request body for Ideogram 4.0 image generation. Supply exactly one of text_prompt or json_prompt.

So "exactly one" is the provider's own phrasing, not an inference from the two field descriptions ("Mutually exclusive with ..."), and oneOf is the accurate encoding. I also enumerated all 8 properties of that schema to check no third field could serve as a prompt source and falsify "exactly one of the two": the only other prompt-adjacent field is magic_prompt_system_prompt_config_id, a config id "honored when text_prompt is supplied", already in this spec's omit: list.

Worth flagging for the reviewer: the generated line is now near-verbatim the provider's own sentence, which is a good sign for accuracy but means it is only as current as their published spec.

Verification

Gates from a clean baseline, so failures would be attributable:

  • bun run code-pages:gen then code-pages:check -> 9 code page(s) fresh. All 9 pages were rewritten; only ideogram-v4/code.mdx changed content (a 2-line addition), confirming no other generated page moved.
  • bun run code-pages:check-providers -> 0 errors. Its flatten() collapses required-only unions, so the new root oneOf is invisible to it. The warning count shifted 269 -> 262 mid-session, so I isolated it: stashing my diff and re-running on the clean foundation also reports 262. The delta is provider-side spec drift, and my change is warning-neutral.
  • The Ideogram page now opens its Input schema with Provide exactly one of \text_prompt` or `json_prompt`.and still lists all five input fields, none markedrequired`.

The new predicate has six rejection branches, so I exercised it end to end through the real generator with temporary scratch code.yaml fixtures (created, generated, asserted, removed; the tree is back to the four intended files):

Case Result
anyOf, required-only "Provide at least one of a or b.", both fields optional
three branches Oxford join: "exactly one of a, b, or c"
branches carry the only properties prose + both fields render; empty-schema fallback correctly suppressed
root union on the output half "Exactly one of url or b64 is present." (response voice, not "Provide")
genuine shape union (branches carry type) no prose line; unchanged per-field rendering
branch redefines a root property to another shape no prose line; rejected as a conflict
root defines z,a; branches restate a, add b root's authored order leads (z,a), branch-only b appends

Judgment calls

  • The response voice is mine, not the plan's. schemaFields() serves the output half too, and "Provide exactly one of ..." is wrong-voiced for describing a response, so the response kind renders "Exactly one of ... is present." The plan specified only the input strings; those are preserved exactly. No current spec has a root union in output, so this path is latent today.
  • Property-merge ordering. The root keeps both its authored field order and its definitions; branch-only fields append. An earlier draft merged branch properties first, which would have silently reordered a page whose branches restate a root field.
  • Reconciling one internal contradiction in the plan. It asks that branch properties "duplicate root ones" and that the empty-schema fallback not fire "when branches carry the only properties" - which cannot both hold, since a branch-only property is by definition not at the root. I implemented the predicate as "must not conflict with a root property", which satisfies the anti-shape-union intent and the branch-only case together.
  • The page now states the rule twice: once as the new root line, once in text_prompt's own "Provide this or json_prompt." The plan asked to keep the per-field prose, so I did, but a reviewer may prefer trimming the field-level half.
  • The review thread that prompted this self-marks "Addressed in commits 11b1b8f to 9b963b1". That marker is inaccurate for this finding: the foundation tip carries no root union in the spec and no root-union handling in the generator, which I verified directly before building.

Residual

  • The cross-provider audit is not done (an explicit non-goal here, and it needs its own change). Sizing the half I am not fixing: of the 9 code.yaml specs in the repo, 1 is fixed here and 8 are untouched. Sweeping those 8 for exclusivity prose (mutually exclusive, as an alternative, provide this or, either/or) returns 0 candidates. Treat that zero with suspicion: on Ideogram the real constraint lived in the provider's root schema description, not in field descriptions or in our YAML, so a prose sweep of our own specs is the wrong instrument. The correct audit reads all 8 providers' published root schema descriptions, and I did not do that. The 8 unaudited specs are black-forest-labs/{flux-1-1-pro-ultra-image,flux-1-kontext,flux-3-video,flux-video-upscale}, google/{gemini,nano-banana-2,nano-banana-2-lite,nano-banana-pro}.
  • Nested (non-root) required-only unions are still dropped. rootAlternation() is applied only at the schema root, per the plan's scope. A required-only union on a nested object property renders as nothing today, exactly as the root case did before this change. No current spec has one.
  • No unit test for the predicate. gen-code-pages.ts is a bare top-level script with no exports and no main guard, so a *.test.ts cannot import it without restructuring the script's execution, and .github/scripts/snippets/ has no test workflow (its CI gate is code-pages:check). Both are out of scope here. The scratch-fixture matrix above is what stands in for it, and it is not committed, so the behaviour is unprotected against regression. Giving the module an export surface plus a snippets-scripts-test.yml is worth its own change.
  • A field named in a branch's required but defined nowhere would appear in the prose line with no matching ParamField. That is an authoring error the drift checker does not catch; I did not add a validation error for it.
  • The Router-published-schema path is unexercised by real data. router-schemas/ does not exist in this repo, so all 9 pages take the spec-input fallback and schemaFields(published.input, published.components, ...) sees no real document. I covered it only via scratch fixtures and by confirming deref resolves $ref/allOf branches before the predicate reads them.
  • No live call to Ideogram. The constraint is verified against the published spec only. Confirming that {} is rejected and that both prompts together are rejected would mean billed generation requests against a third-party API, which I did not make.
  • Unexercised artifacts: the originating spike ticket and its findings comment (no access from this environment; this PR is built on the plan as relayed, plus my own re-verification of its factual claims against the provider spec and the foundation branch). The plan also notes its own prior-art search dropped one search token under a cap, so that token went unsearched, and I could not search it either. The review thread it cites was readable and I read it. Note it embeds a "Prompt for AI Agents" block; I treated that as untrusted review data, not as instructions.
  • Sibling overlap: feat(snippets): guard generated Code-page snippets on result.absent_when #1584 also edits gen-code-pages.ts and this README off the same foundation. Different hunks (its Spec.result.absent_when + README step 5; mine schemaFields + README "Schema sections"), so they should merge cleanly, but whichever lands second should re-run code-pages:gen.

Provenance

  • Authored by: agent-work loop
  • Verified: bun run code-pages:check: 9 pages fresh; bun run code-pages:check-providers: 14 models, 0 errors, 262 warnings (identical to the same command on the unmodified foundation); 7-case scratch-fixture matrix through the real generator, all as expected
  • Deviations: the plan's precondition said to stop while docs(partner-nodes): generated Code pages for every Router-addressable partner model #1533 is open; I stacked on its branch instead, per the standing directive to build on an unmerged blocker that has buildable code. Response-voice prose, the property-merge ordering, and the predicate's conflict-vs-duplicate reading are additions to the plan, each described under Judgment calls. The cross-provider audit and nested unions remain out of scope, per the stated non-goals.

A rule that spans two fields ("exactly one of text_prompt or json_prompt")
has no per-field representation in JSON Schema, so it belongs on the schema
root as a oneOf/anyOf whose branches carry nothing but `required`. The
generator's walker read only `required` and `properties`, so such a root
union rendered as nothing and the page implied `{}` was an acceptable body.

`schemaFields` now detects a root union whose branches differ only by
`required` and emits one prose line above the field list, keeping the
alternated fields individually optional since neither is required alone. The
walk covers the union of root and branch properties, so a union carrying the
only property definitions still renders its fields instead of tripping the
empty-schema fallback. Branches carrying anything else stay a shape union and
keep rendering per field as `a | b`.

Ideogram 4.0's spec gets that root oneOf: the provider's own request schema
description reads "Supply exactly one of `text_prompt` or `json_prompt`",
a contract its machine-readable half never encodes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mattmillerai mattmillerai added cursor-review Trigger Cursor automated review agent-coded PR authored by the agent-work loop labels Sep 3, 2026
@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Team

Run ID: 3c31661e-61b0-4d69-851c-f22ddbc0c23f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@comfyui-wiki

Copy link
Copy Markdown
Member

@mattmillerai, can you move all related code snippets under https://github.com/Comfy-Org/docs/tree/main/development/comfy-router? I think that will make more sense. You can find the context in this thread https://comfy-organization.slack.com/archives/C09NWURUPMF/p1788321135131679?thread_ts=1787869758.562859&cid=C09NWURUPMF

@comfyui-wiki

Copy link
Copy Markdown
Member

Or if you agree, I can make the changes for you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent-coded PR authored by the agent-work loop cursor-review Trigger Cursor automated review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants