feat(snippets): render root-level oneOf/anyOf constraints on generated Code pages - #1585
Conversation
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>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
@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 |
|
Or if you agree, I can make the changes for you |
STACKED — merging lands on
docs/router-model-page-pilot(owned by the author of #1533), NOTmain. 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 tomainautomatically when that happens.What this does
A constraint that spans two fields ("exactly one of
text_promptorjson_prompt") has no per-field representation in JSON Schema. It belongs on the schema root as aoneOf/anyOfwhose branches carry nothing butrequired.schemaFields()walked onlyrequired+properties, so a root union rendered as nothing, and the Ideogram 4.0 Code page implied{}was an acceptable request body.gen-code-pages.tsgainsrootAlternation(), called fromschemaFields()afterderef. When a rootoneOf/anyOf's branches differ only byrequired, 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.ideogram-v4/code.yamlgets that rootoneOf. Existing per-field prose is unchanged.snippets/README.mddocuments the shape, sincecode.yamlauthors are the audience for it.Branches carrying anything beyond
required/properties(atype, anenum, a property redefining a root field to a different shape) are a genuine choice of shapes, not an alternation, and keep rendering per field asa | b.Why
oneOfand not theanyOfthe review comment suggestedI 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.
GenerateImageRequestV4carries no rootoneOf/anyOf/allOfand norequired, so{}really does validate. But its root schema description states the contract in words:So "exactly one" is the provider's own phrasing, not an inference from the two field descriptions ("Mutually exclusive with ..."), and
oneOfis 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 ismagic_prompt_system_prompt_config_id, a config id "honored whentext_promptis supplied", already in this spec'somit: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:genthencode-pages:check-> 9 code page(s) fresh. All 9 pages were rewritten; onlyideogram-v4/code.mdxchanged content (a 2-line addition), confirming no other generated page moved.bun run code-pages:check-providers-> 0 errors. Itsflatten()collapses required-only unions, so the new rootoneOfis 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.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.yamlfixtures (created, generated, asserted, removed; the tree is back to the four intended files):anyOf, required-onlyaorb.", both fields optionala,b, orc"urlorb64is present." (response voice, not "Provide")type)z,a; branches restatea, addbz,a), branch-onlybappendsJudgment calls
schemaFields()serves the output half too, and "Provide exactly one of ..." is wrong-voiced for describing a response, so theresponsekind renders "Exactly one of ... is present." The plan specified only the input strings; those are preserved exactly. No current spec has a root union inoutput, so this path is latent today.text_prompt's own "Provide this orjson_prompt." The plan asked to keep the per-field prose, so I did, but a reviewer may prefer trimming the field-level half.Residual
code.yamlspecs 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 areblack-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}.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.gen-code-pages.tsis a bare top-level script with no exports and no main guard, so a*.test.tscannot import it without restructuring the script's execution, and.github/scripts/snippets/has no test workflow (its CI gate iscode-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 asnippets-scripts-test.ymlis worth its own change.requiredbut defined nowhere would appear in the prose line with no matchingParamField. That is an authoring error the drift checker does not catch; I did not add a validation error for it.router-schemas/does not exist in this repo, so all 9 pages take the spec-input fallback andschemaFields(published.input, published.components, ...)sees no real document. I covered it only via scratch fixtures and by confirmingderefresolves$ref/allOfbranches before the predicate reads them.{}is rejected and that both prompts together are rejected would mean billed generation requests against a third-party API, which I did not make.gen-code-pages.tsand this README off the same foundation. Different hunks (itsSpec.result.absent_when+ README step 5; mineschemaFields+ README "Schema sections"), so they should merge cleanly, but whichever lands second should re-runcode-pages:gen.Provenance
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