Skip to content

Migrate onnx-genai metadata emitters to the current pipeline/speculative contract - #554

Merged
justinchuby merged 3 commits into
mainfrom
justinchuby-fix-onnx-genai-metadata-schema-drift
Aug 23, 2026
Merged

Migrate onnx-genai metadata emitters to the current pipeline/speculative contract#554
justinchuby merged 3 commits into
mainfrom
justinchuby-fix-onnx-genai-metadata-schema-drift

Conversation

@justinchuby

@justinchuby justinchuby commented Aug 23, 2026

Copy link
Copy Markdown
Member

Fixes six long-standing schema-conformance failures caused by mobius's legacy onnx-genai emitters drifting from the runtime's published JSON schema.

Why the drift was invisible

The conformance tests searched a couple of hard-coded local onnx-genai checkout paths and pytest.skipped when none was found. CI has no such checkout, so six tests never ran there; on the machines that did have one, the result depended on whatever revision that clone happened to sit on. Two upstream contract redesigns accumulated unnoticed.

The two contract redesigns

  1. pipeline is now PipelineSpec — a single property, workflow (a typed SSA graph), with additionalProperties: false. The legacy emitters produced {models, dataflow, strategy, phases}, all of which the schema now rejects.
  2. speculative is now SpeculativeContract — requires {proposer, target, vocabulary, max_proposal_width} and forbids every field of the old flat MTP block. The flat SpeculatorConfig that block was modelled on still exists upstream, but it describes a HuggingFace config.json section, not InferenceMetadata.speculative.

What was migrated

Diffusion — build_diffusion_pipeline_metadata

Now emits the denoise loop explicitly: the solver, sigma/alpha schedule, timestep table, guidance combine and output clamp are real ONNX components built from mobius's policy library and shipped with the document.

It cannot delegate to build_diffusion_workflow_metadata: the ComfyUI conversion path has no component graphs at all (it deliberately does not build or export them), and that builder derives everything from live ir.Models. So the workflow is built directly, reusing workflow_metadata's _invoke / _publish_workflow_v1 and mobius.generation's solver builders. The schedule is derived from the scheduler's own betas through the same diffusers-compatible helpers the package exporter uses, so a ComfyUI conversion and a package export of the same checkpoint describe the same dynamics. img2img's start_step lowers to a sliced schedule; the VAE scaling_factor and the sigma-space initial-state scale are emitted as explicit components.

Three cases now fail closed instead of being silently mis-described:

  • an ancestral sampler (mobius ships no stochastic solver),
  • Karras/exponential sigma spacing (the workflow ships the sigma table as a constant, so a use_karras_sigmas hint is no longer sufficient),
  • a latent-only graph with no VAE decode.

MTP — write_mtp_speculator_metadata

Emits a SpeculativeContract anchored to the backbone's workflow: it registers the head as a workflow component, names the target by its declared logits port role, states the hidden handoff as port_bindings.target_hidden_context plus a hidden_states role on the target output, and completes the rollback capacity its own claim requires.

It is declared block, not chained: a chained proposer must expose a logits_output carrying the next-token distribution, and this sidecar emits only mtp_hidden — the runtime decodes it through the target's shared LM head, which is why that initializer is listed in shared_weights.

VLM

build_native_vlm_package_metadata produces mobius's internal structural descriptor, not a publishable document — build_vlm_workflow_metadata already consumes it and republishes only preprocessing under a real pipeline.workflow. The actual bug was that write_native_vlm_package_metadata wrote that descriptor to inference_metadata.yaml. It now writes the workflow document, and the tests validate the published document instead of the descriptor.

CI visibility

The upstream schema is vendored under src/mobius/integrations/onnx_genai/_schema/ and is the default, so conformance never skips and drift becomes a test failure. A local checkout is no longer consulted implicitly — one that is ahead of or behind main reintroduces exactly the machine-dependent result that hid this bug. Set ONNX_GENAI_SCHEMA to validate against a specific revision.

Three further conformance tests (codec, speech-to-text, duplex workflows) were skipping for the same reason and now run.

Tests

Migrated in the same change rather than loosened:

  • TestMtpSpeculatorMetadata::test_exact_schema_keys_and_values asserts the new contract; test_no_legacy_field_names is repointed at the now-legacy field names.
  • A new MTP test anchors against a workflow write_decoder_workflow_metadata actually emits (which ships ~11 generated policy components), so target selection can't regress to "the only ONNX component".
  • New tests pin that the solver schedule comes from the scheduler's betas rather than a placeholder ramp.
  • Diffusion/ComfyUI tests assert workflow structure instead of the removed strategy block; scheduler facts that were duplicated into the document are asserted on the parsed ComfyUIWorkflow where they actually live.

Verification

Rebased onto current main (includes #552 and #553).

python -m pytest tests/build_graph_test.py tests/cli_test.py src/ -q \
  -k "not phi4mm and not apply_weights_unknown" -n auto
before (main at branch point) after
full suite 8 failed / 4613 passed 0 failed / 4641 passed
src/mobius/integrations/onnx_genai/ with ONNX_GENAI_SCHEMA at upstream main 7 failed / 240 passed 258 passed / 1 skipped

Six of those eight failures are the schema-drift ones fixed here. The other two — qwen_image_test.py::test_deterministic_l4_l5_image_edit_golden and TestNativeVlmPackageMetadata::test_cached_gemma_processor_matches_emitted_patch_budget — were fixed by #553, which is now merged; this branch preserves both of its changes through the rebase.

lintrunner is clean.

Sequencing note

workflow_metadata.py and auto_export.py are not modified (both are read from only), so this does not conflict with #551.

@justinchuby
justinchuby requested review from a team and a lite review from Copilot August 23, 2026 01:36
@CLAassistant

CLAassistant commented Aug 23, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@github-actions

github-actions Bot commented Aug 23, 2026

Copy link
Copy Markdown

🏗️ Architecture Diff

Comparing 77ade40a52e72c

Model Sub-model Changes Status

No architecture changes detected.


Legend: ⚪ No change · 🔵 Minor (attrs/inits) · 🟡 Moderate (nodes added/removed) · 🔴 Major (interface changed)

@github-actions

github-actions Bot commented Aug 23, 2026

Copy link
Copy Markdown

Performance Comparison

Comparing 77ade40a52e72c

Model Metric Baseline Current Delta
bert (feature-extraction) model_size_bytes 359 KB 359 KB +0.0%
bert (feature-extraction) num_nodes 68 68 +0.0%
falcon model_size_bytes 364 KB 364 KB +0.0%
falcon num_nodes 66 66 +0.0%
gemma2 model_size_bytes 428 KB 428 KB +0.0%
gemma2 num_nodes 105 105 +0.0%
gpt2 model_size_bytes 388 KB 388 KB +0.0%
gpt2 num_nodes 54 54 +0.0%
llama model_size_bytes 425 KB 425 KB +0.0%
llama num_nodes 60 60 +0.0%
llama (static-cache) model_size_bytes 425 KB 425 KB +0.0%
llama (static-cache) num_nodes 56 56 +0.0%
mamba (ssm-text-generation) model_size_bytes 296 KB 296 KB +0.0%
mamba (ssm-text-generation) num_nodes 94 94 +0.0%
phi3 model_size_bytes 421 KB 421 KB +0.0%
phi3 num_nodes 58 58 +0.0%
phi3 (static-cache) model_size_bytes 421 KB 421 KB +0.0%
phi3 (static-cache) num_nodes 54 54 +0.0%
qwen2 model_size_bytes 425 KB 425 KB +0.0%
qwen2 num_nodes 60 60 +0.0%
qwen2 (static-cache) model_size_bytes 425 KB 425 KB +0.0%
qwen2 (static-cache) num_nodes 56 56 +0.0%
qwen3_5_moe (hybrid-text-generation) model_size_bytes 506 KB 506 KB +0.0%
qwen3_5_moe (hybrid-text-generation) num_nodes 264 264 +0.0%
qwen3_5_text (hybrid-text-generation) model_size_bytes 458 KB 458 KB +0.0%
qwen3_5_text (hybrid-text-generation) num_nodes 126 126 +0.0%
qwen3_5_vl (hybrid-qwen-vl) model_size_bytes 977 KB 977 KB +0.0%
qwen3_5_vl (hybrid-qwen-vl) num_nodes 428 428 +0.0%
t5 (seq2seq) model_size_bytes 836 KB 836 KB +0.0%
t5 (seq2seq) num_nodes 176 176 +0.0%
whisper (speech-to-text) model_size_bytes 1008 KB 1008 KB +0.0%
whisper (speech-to-text) num_nodes 128 128 +0.0%

No performance regressions.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR migrates Mobius’s onnx-genai metadata emitters and conformance tests to match the runtime’s current published JSON schema, particularly the typed SSA pipeline.workflow (PipelineSpec) and the new SpeculativeContract, and makes schema validation deterministic in CI by vendoring the schema.

Changes:

  • Replace legacy diffusion/VLM/MTP metadata shapes with the current pipeline.workflow contract and updated speculative metadata contract.
  • Vendor the onnx-genai inference_metadata.schema.json under src/mobius/integrations/onnx_genai/_schema/ and update tests to always validate against it (with ONNX_GENAI_SCHEMA override support).
  • Update ComfyUI conversion to emit the new workflow-based diffusion metadata and ship required policy component artifacts.

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/mobius/integrations/onnx_genai/speech_to_text_workflow_metadata_test.py Stops skipping schema validation; uses vendored/override schema path helper.
src/mobius/integrations/onnx_genai/inference_metadata.py Major contract migration: diffusion emits typed SSA workflow + policy components; MTP speculative metadata moved to SpeculativeContract; VLM writer publishes workflow document.
src/mobius/integrations/onnx_genai/inference_metadata_test.py Updates schema lookup to vendored default; rewrites diffusion/MTP/VLM tests for new contracts and adds new regression assertions.
src/mobius/integrations/onnx_genai/duplex_workflow_metadata_test.py Stops skipping schema validation; uses vendored/override schema path helper.
src/mobius/integrations/onnx_genai/decoder_metadata_test.py Stops skipping schema validation; uses vendored/override schema path helper.
src/mobius/integrations/onnx_genai/convert.py Ensures diffusion workflow metadata includes VAE scaling factor and saves generated policy component artifacts alongside metadata.
src/mobius/integrations/onnx_genai/convert_test.py Updates assertions to the workflow-based diffusion schema and component model.
src/mobius/integrations/onnx_genai/comfyui.py Tightens ComfyUI parsing (reject latent-only) and translates to workflow-based diffusion metadata.
src/mobius/integrations/onnx_genai/comfyui_test.py Updates ComfyUI translation tests to the workflow contract and new failure-closed behaviors.
src/mobius/integrations/onnx_genai/codec_workflow_metadata_test.py Stops skipping schema validation; uses vendored/override schema path helper.
src/mobius/integrations/onnx_genai/_schema/README.md Documents why the schema is vendored and how to update/override it.
src/mobius/integrations/onnx_genai/init.py Updates diffusion metadata docstring to reflect workflow-based emission and shipped policy artifacts.
pyproject.toml Packages the vendored schema JSON in wheel/sdist via package-data.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/mobius/integrations/onnx_genai/comfyui.py
Comment thread src/mobius/integrations/onnx_genai/inference_metadata.py Outdated
justinchuby and others added 2 commits August 22, 2026 18:45
…ive contract

mobius's legacy onnx-genai emitters had drifted from two upstream contract
redesigns, and the drift was invisible because every schema conformance test
skipped in CI.

Why it stayed hidden: the conformance tests searched a couple of hard-coded
local onnx-genai checkout paths and `pytest.skip`ped when none existed. CI has
no such checkout, so six tests never ran there, and on the machines that did
have one the result depended on whatever revision that clone sat on.

The two contract redesigns:

1. `pipeline` is now `PipelineSpec`, whose only property is `workflow` (a typed
   SSA graph) with `additionalProperties: false`. The legacy emitters produced
   `{models, dataflow, strategy, phases}`, which the schema now rejects
   outright.
2. `speculative` is now `SpeculativeContract`, requiring
   `{proposer, target, vocabulary, max_proposal_width}` and forbidding every
   field of the old flat MTP block. The flat `SpeculatorConfig` that block was
   modelled on still exists upstream, but it describes a HuggingFace
   `config.json` section, not `InferenceMetadata.speculative`.

What changed:

* Diffusion (`build_diffusion_pipeline_metadata`). Now emits the denoise loop
  as an explicit workflow: the solver, schedule, timestep table, guidance
  combine and output clamp are real ONNX components built from mobius's policy
  library and shipped with the document. It cannot delegate to
  `build_diffusion_workflow_metadata`, because the ComfyUI conversion path has
  no component graphs at all -- it deliberately does not build or export them
  -- so it builds the workflow directly while reusing that module's `_invoke`
  and `_publish_workflow_v1`. The schedule is derived from the scheduler's own
  betas via the same diffusers-compatible helpers the package exporter uses, so
  a ComfyUI conversion and a package export of one checkpoint describe the same
  dynamics. img2img's `start_step` lowers to a sliced schedule.

  Three things now fail closed rather than being silently mis-described: an
  ancestral sampler (no deterministic solver exists), Karras/exponential sigma
  spacing (the workflow ships the sigma table as a constant, so a hint field is
  not enough), and a latent-only graph with no VAE decode.

* MTP (`write_mtp_speculator_metadata`). Now emits a `SpeculativeContract`
  anchored to the backbone's workflow: it registers the head as a workflow
  component, names the target by its declared `logits` port role, states the
  hidden handoff as `port_bindings.target_hidden_context` plus a
  `hidden_states` role on the target output, and completes the rollback
  capacity its own claim requires. It is `block` rather than `chained` because
  a chained proposer must expose a `logits_output` and this sidecar emits only
  `mtp_hidden` -- the runtime decodes it through the shared LM head, which is
  why that initializer is in `shared_weights`.

* VLM. `build_native_vlm_package_metadata` produces mobius's internal
  structural descriptor, not a publishable document -- `build_vlm_workflow_metadata`
  already consumes it and republishes only `preprocessing` under a real
  `pipeline.workflow`. The bug was that `write_native_vlm_package_metadata`
  wrote the descriptor to `inference_metadata.yaml`; it now writes the workflow
  document, and the tests validate that published document instead.

* CI visibility. The schema is vendored under `_schema/` and is the default, so
  conformance never skips and drift is a test failure. A local checkout is no
  longer consulted implicitly, since one that is ahead of or behind `main`
  reintroduces the same machine-dependent result; set `ONNX_GENAI_SCHEMA` to
  validate against a specific revision. Three further conformance tests in the
  codec, speech-to-text and duplex suites were skipping for the same reason and
  now run.

Tests updated in step: the MTP tests assert the new contract (with the
banned-name test repointed at the now-legacy field names) and are anchored to a
workflow mobius actually emits, and the diffusion/ComfyUI tests assert workflow
structure rather than the removed `strategy` block.

Full suite: 8 failed / 4613 passed -> 1 failed / 4640 passed. The remaining
failure (`qwen_image_test.py::test_deterministic_l4_l5_image_edit_golden`) is
pre-existing and handled separately.

Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Drop the before/after narration comparing against the superseded pipeline and
speculative shapes; the docstrings describe the contract the code emits today.
The banned-name test keeps its list of now-legacy fields, since rejecting them
is the point of that test.

Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@justinchuby
justinchuby force-pushed the justinchuby-fix-onnx-genai-metadata-schema-drift branch from cd1f69e to e5a00bf Compare August 23, 2026 01:46
The translated document declares the sampler as executable components under
``policies/*.onnx``, but ``parse_comfyui_workflow`` built those graphs and threw
them away, so a caller holding only the metadata could not produce a loadable
package. ``ComfyUIWorkflow`` now carries them and exposes
``save_policy_components``; the dict-only wrappers point at it.

``convert_comfyui_workflow`` keeps building its own package rather than reusing
the parse-time one, because the checkpoint's scheduler config can reconcile to a
different solver than the ComfyUI sampler implied -- reusing it would leave that
run's components behind for a document that never references them. That is now
stated where someone would otherwise "simplify" it, and covered by a test
asserting the written set equals the referenced set exactly.

Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@justinchuby
justinchuby merged commit b65bbd9 into main Aug 23, 2026
23 checks passed
@justinchuby
justinchuby deleted the justinchuby-fix-onnx-genai-metadata-schema-drift branch August 23, 2026 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants