Simplify composite ONNX GenAI metadata - #478
Merged
justinchuby merged 152 commits intoAug 21, 2026
Merged
Conversation
🏗️ Architecture Diff
bert (feature-extraction) / model — 23 change(s)Op summary: 60 → 68 nodes --- base
+++ head
@@ -10,6 +10,14 @@
Add
Add
LayerNormalization
+Cast
+Unsqueeze
+Shape
+Shape
+Shape
+Concat
+Expand
+Unsqueeze
Transpose
MatMul
AddAdded nodes:
Connectivity changes:
Initializer changes:
Legend: ⚪ No change · 🔵 Minor (attrs/inits) · 🟡 Moderate (nodes added/removed) · 🔴 Major (interface changed) |
Performance Comparison
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the ONNX GenAI inference metadata emitter to produce a normalized composite schema, reducing duplicated/ambiguous scheduling and I/O contract representation across model, pipeline.strategy, and pipeline.phases.
Changes:
- Removes duplicated
run_onscheduling fields frompipeline.strategy.stages, makingpipeline.phasesthe single scheduling/presence source of truth. - Removes top-level
model.ioemission for composite packages (e.g., native VLM), keeping I/O contracts scoped topipeline.models.<component>.io(while preservingmodel.iofor bare single-model packages via the non-pipeline path). - Explicitly classifies diffusion denoisers as
run_on: every_stepinpipeline.phases.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/mobius/integrations/onnx_genai/inference_metadata.py | Normalizes composite metadata by deduplicating stage scheduling fields and scoping I/O contracts to per-component locations; sets diffusion denoiser phase scheduling explicitly. |
| src/mobius/integrations/onnx_genai/inference_metadata_test.py | Updates tests to assert the new normalized schema (phases-based scheduling; decoder I/O read from pipeline.models.decoder.io; no composite model.io). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| assert not any(transform["op"] == "normalize" for transform in transforms) | ||
| assert metadata["model"]["io"]["token_input"] == "input_ids" | ||
| assert metadata["model"]["io"]["kv_inputs"] == [ | ||
| assert "model" not in metadata or "io" not in metadata["model"] |
justinchuby
marked this pull request as draft
August 12, 2026 18:03
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
|
||
|
|
||
| class _PolicyPackage(Protocol): | ||
| def add_policy_component(self, name: str, component: PolicyComponent) -> None: ... |
Member
Author
|
Cross-repo contract audit against onnx-genai
Please continue making sampler RNG batched/counter-based and canonicalizing dtypes. I will push the contract corrections and runtime E2E fixtures to #828, then post the new SHA here for exact producer alignment. |
Member
Author
|
Runtime E2E review of
Also, emitted |
Member
Author
|
ONNX GenAI branch phi/effect-merge blocker is fixed at Emit branches with: outputs:
selected.tokens:
cases: { "true": accepted.tokens, "false": corrected.tokens }
effects:
rng:
incoming: rng.0
cases: { "true": rng.accepted, "false": rng.corrected }
produces: rng.joinedInclude Exact contract/docs: |
Member
Author
|
ONNX GenAI preprocessing-to-workflow SSA blocker is fixed at Producer target:
Canonical documentation: |
Member
Author
|
Follow-up audit of current Mobius head
TTS does not need a new host induction primitive: per the component-centric contract, initialize an integer state cell and invoke/carry a generic ONNX counter update inside the nested loop; use that SSA value for Please generate one concrete package/YAML at the fixed head so I can run ONNX GenAI load+execution cross-repo. |
justinchuby
force-pushed
the
justinchuby/simplify-composite-metadata-producer
branch
from
August 12, 2026 20:18
bde3452 to
774448b
Compare
Member
Author
|
ONNX GenAI loop induction blocker is fixed in Producers may now declare: iteration:
value: loop.i
contract: { dtype: int64, rank: 1, shape: [batch] }and bind Please migrate decoder/diffusion/masked/TTS loop iteration inputs to this field. The separate decoder setup/body output-count and initializer issues from the prior comment still need producer fixes before cross-repo execution. |
Member
Author
|
Cross-repo execution against ONNX GenAI #828 found one remaining producer-side contract mismatch (still present at current head
Other exact
ONNX GenAI fixes discovered by this run: scalar defaults now materialize unbound symbolic axes as singleton tensors, and component shape symbols are invocation-local (with adapter allocation retaining package-scope symbols). Permanent conformance/regression tests are being pushed to #828. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Member
Author
|
Fixed the decoder/VLM logits recurrence in |
Member
Author
|
Remaining cross-repo blockers after |
Member
Author
|
Performance acceptance instrumentation is pushed in The new paired-run gate rejects non-identical model/runtime/EP/device/precision/batch/shape/sampling/RNG/KV/capture/warmup conditions, then checks throughput, TTFT, peak memory, H2D/D2H counts+bytes, device syncs, session/kernel boundaries, device residency, and required island capture/replay. Required plans cover decoder+min-p+termination, speculative accept/reject, and grammar-delimited islands. Current measured upstream baseline at ONNX GenAI
Real Mobius package/KV/per-row serving measurements are still absent. Additionally, |
Member
Author
|
Architecture cleanup progress:
Muse concise metadata currently measures 600 lines / 546 mapping fields / 16,339 bytes, down from the reviewed 1,183-line source (583 lines, 49.3% reduction). A pre-lowering flattened-field baseline was not retained, so only the current 546-field count is evidence-backed. Remaining producer work is blocked on exact current #828 contracts, reported at issuecomment-5274711168: per-row |
Member
Author
|
ONNX GenAI producer blockers are resolved at Use Key migration: Full exact checklist and YAML are in |
Member
Author
|
Cross-repo execution against onnx-genai #828 imported
All seven metadata files pass the callable validator, demonstrating why runtime E2E is now mandatory. ONNX CI/test wiring is ready and will pin the corrected producer commit once these graph defects are regenerated. |
Member
Author
|
Confirmed corrected producer head |
justinchuby
marked this pull request as ready for review
August 13, 2026 05:28
Member
Author
|
Final validation at |
Member
Author
|
CI follow-up: the Integration (fast) rerun completed with the identical two pre-existing DeepSeek numerical-parity failures ( |
Member
Author
|
Real Muse H200 follow-up is pushed at Paired native result on ORT 1.28/H200, exact 68-token prompt, 128 new tokens, 1 warmup + 3 runs, CUDA Graph/shared KV: 63.3653 tok/s median, 49.0217 ms TTFT (2.60% above the historical 61.76 tok/s baseline). The real metadata workflow currently cannot complete in frozen ONNX GenAI
Mobius also fixed BF16 decoder logits → float32 sampler ABI and added the exact runnable native/workflow harness. I returned the PR to draft and have not replaced the HF metadata, because the currently frozen runtime cannot execute the published workflow without runtime changes. |
The formatter disagrees with the hand-wrapping in the new test module. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: justinchuby <justinchuby@users.noreply.github.com>
The static-cache package declares both a `model.io.static_cache` port ABI and a workflow that binds it, because the two answer different questions and two different consumers read them. The pinned runtime rejected that pair outright and told authors to move the ABI to `pipeline.models.<component>.io`, a key the workflow IR had already removed, while its own decode backend refused to load a static-cache graph that did not declare one. No package could satisfy both. The new pin permits the overlap and cross-checks it instead: the ABI's write-destination port must be bound by a state group whose update is an indexed scatter, exactly one component may claim it, and every per-layer key and value pair the ABI names must be a pair that group actually advances. All eleven fixture packages validate against it, which is what this pin buys. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: justinchuby <justinchuby@users.noreply.github.com>
A package could state its decode-step ABI twice: once as `model.io`, and
once as the component ports, invoke bindings and state groups a workflow
engine executes. Two writable answers to one question is a defect whatever
their contents, because nothing forces them to agree and a reader of one
never learns the other said something else. The previous commit made that
worse by adding `model.io` to static-cache exports specifically, which also
meant a bare single-file decoder and a composite package were suddenly
different *kinds* of document rather than one shape with a different number
of components.
`pipeline.workflow` is now the only description, for every export. `model`
keeps package-wide geometry and capabilities and never a port ABI. A runtime
that wants an optimized single-graph path derives it by lowering the
one-component workflow, and a derivation cannot disagree with its source.
Three facts had to move into the workflow for that to be true rather than
merely asserted.
Every ONNX component now declares `ports.inputs`/`ports.outputs`: a full
contract for exactly the graph's inputs and outputs, no more and no fewer. A
subset would let a consumer quietly fall back to opening the artifact; a
superset would be a promise the graph does not keep.
Every ONNX component declares `ports.roles` — what it *does* with a value
bound to a port. An invocation records which SSA value reaches a port, not
whether that port is tokens, a mask or logits, and recovering the difference
from a port's spelling is the name-guessing this format refuses everywhere
else. Mobius mints these names in its own task builders, so it states the
mapping between its vocabulary and the runtime's rather than inferring one; a
port outside that vocabulary gets no role, because a workflow that guesses is
worse than one that stays silent.
State port aliases declare `role` and `layer`. A layer's key buffer and its
value buffer are the same dtype and the same shape, and a cell's label sorts
lexicographically so `cache_10` precedes `cache_2` — a consumer pairing
per-layer buffers positionally would silently transpose two layers' caches
with nothing failing. Both fields are emitted together or not at all, so a
recurrent or convolution cache is never handed a fabricated index that would
corrupt the very ordering the index exists to fix. `IndexedScatter` gains
`kv_length_ports` beside `write_indices_ports`, because a valid length and a
write cursor are both rank-1 integer vectors and are indistinguishable from
each other by shape.
`_add_explicit_io_to_file` is deleted. It was the only route by which an
export could gain a `model.io`, it had no caller, and dead code that can mint
a forbidden key is exactly the kind of thing that gets resurrected. The
`adapter` fixture's hand-written decoder declared `ports: {}`; it now declares
its real ports, since a component that describes nothing is not a description.
`tests/canonical_workflow_contract_test.py` is what keeps this from lapsing.
It asks one set of shape-agnostic questions — workflow present, no `model.io`,
declared ports exactly equal to the graph's, every invoke binding and state
alias resolving to a declared port, scatter control ports declared, decode
step reconstructible from the workflow alone — of dynamic, static-cache, FP8,
heterogeneous and composite packages, and of all 11 checked-in fixtures. None
of the assertions names a feature, so a future feature cannot grow a private
top-level block while every feature-specific test keeps passing.
Verified against ONNX GenAI 02e22dd6, which lands the matching lowering: 11/11
packages validate and 11/11 execute under the runtime conformance suite,
including the fixed-capacity decode path, with no `model.io` in any package.
That last result is what makes removing the second copy safe rather than
merely tidy. The CI pin moves to that commit.
Tests: 4654 passed, 58 skipped in the fast suite; lintrunner clean; fixtures
regenerate byte-identically.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: justinchuby <justinchuby@users.noreply.github.com>
Rebasing onto main brought in inspect_components, which reports a package's components from the task's model_roles. Qwen3-TTS builds four loop-wiring graphs — code_predictor_prefill, code_predictor_step_embedder, code_predictor_indices and talker_text_step — that were never declared there. Two things followed from that omission. build_from_module looks each component up in model_roles and falls back to the "decoder" role when it is absent, so these parameter-free graphs were offered the GQA and QKV-packing passes meant for attention stacks. And inspect_components reported six components for a package that contains ten, so a caller planning per-component work would silently skip four artifacts that ModelPackage.save writes to disk. Declare them with a "glue" role: a graph that reads every tensor it uses from its own graph inputs, carries no weights, and has no fusion to gate. Give them empty HF_COMPONENT_SOURCES entries, because mobius authors them and no HuggingFace sub-module backs them. Guard both directions. arch_validation_test now fails any task that builds a component it does not declare, and exempts glue components from the "must have initializers" check — a wiring graph holding only hoisted constants is what correctness looks like, not a lost-weights bug. A network-free unit test pins the same invariant for Qwen3-TTS in the fast suite. The rebase also resolved two conflicts semantically: __all__ keeps main's inspect_components alongside the branch's generation and fingerprint_model_weights, and unet_parity_test keeps main's _run_onnx helper while taking the branch's FLOAT timestep, which is the dtype the denoiser port now declares. Repin the onnx-genai validation checkout to f19f2e71, the current head of the schema branch after its rebase; the previous pin is no longer on that lineage. 11/11 fixtures validate and 11/11 conformance tests execute against it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: justinchuby <justinchuby@users.noreply.github.com>
The validation checkout pinned f19f2e71, which is a reachable ancestor of justinchuby/onnx-genai@justinchuby/simplify-composite-metadata but no longer its head. An ancestor pin is only as durable as the branch's history: that branch has been force-pushed repeatedly, and each rewrite risks orphaning the commit CI resolves, which would fail the checkout rather than the assertion. Pinning the head keeps the reference on the branch for as long as possible and makes the SHA verifiable with a single git ls-remote. c344c2c7 adds only documentation and a test doc-comment on top of f19f2e71 -- no schema, validator or runtime change -- so it cannot move the contract Mobius emits against. Re-verified against the new pin rather than assumed: all 11 emitted packages validate, and all 11 workflow conformance tests pass, including mobius_static_cache_workflow_executes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: justinchuby <justinchuby@users.noreply.github.com>
Every package these tests build has two layers, and two layers cannot distinguish a correct layer annotation from a broken one. Below ten cells a label sorts the same way under every rule -- lexicographic, numeric, insertion order -- so a `layer` taken from a cell's position instead of parsed from its port name satisfies each existing assertion exactly as the correct value does. Real decoders have twenty to eighty layers, which puts production permanently in the region the suite could not reach. The failure that region hides is silent. A layer's key and value buffers share a shape and a dtype, so two transposed caches raise nothing: no validation error, no shape mismatch, only subtly wrong tokens. So build twelve-layer dynamic, static-cache and hybrid decoders and assert what only holds there: that the labels really do sort out of layer order at this depth (otherwise the rest proves nothing), that each declared layer restates the index its port name carries, that ordering by layer and half recovers the buffer lists the runtime collects, and that a hybrid's two alternating groups own layers a cell's position within its group never equals. Verified by mutation rather than assumed: dropping the parse from _cache_layer_index leaves all 101 pre-existing metadata assertions green and fails seven of these, across all three shapes. Also pin GenAI validation to 0d4738e7, which makes both facts these tests rely on enforceable rather than merely documented -- a group binding key/value roles must now declare kv_length_ports, and every such alias must carry a layer. A real twelve-layer static-cache export validates against it, declaring layers 0 through 11; all 11 fixtures validate and all 11 conformance tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: justinchuby <justinchuby@users.noreply.github.com>
1d8cfefe is additive over the previously pinned 0d4738e7: it adds an end-to-end test that a package carrying no `model:` block still resolves the static-cache ABI and executes through the scatter driver, plus the canonical fixture that test runs against. No validator or schema behaviour changes. That test is worth pinning because it closes the last gap between what this producer emits and what is proven to run. Until now the evidence that a workflow-only package was sufficient came from validation and from Mobius's own conformance run; the ABI resolution itself was argued from call sites. It is now asserted against a real graph on the consumer side. Re-verified against the new pin rather than inherited from the previous run: all 11 emitted packages validate and all 11 conformance tests pass. Compared the emitted static-cache package against the new canonical fixture directly -- `update.write_indices_ports`, `update.kv_length_ports`, per-alias `role` and `layer`, `components.<c>.ports.roles` and the absence of a top-level `model:` block all agree, so the two describe the same contract. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: justinchuby <justinchuby@users.noreply.github.com>
The previously pinned 1d8cfefe is no longer reachable from justinchuby/onnx-genai@justinchuby/simplify-composite-metadata: that branch was rebased onto current main, so every SHA on it changed and the old head became an orphan. `git merge-base --is-ancestor 1d8cfefe 60f41354` is false. CI still resolved the old SHA because GitHub serves unreferenced objects, which is the failure mode worth avoiding -- the pin kept working while pointing at a commit that is on no branch and whose content nobody is maintaining. 60f41354 is the current head of that branch and of PR #828. The rebase carried no content change, so the two facts this pin exists to check are unchanged: the workflow-derived scatter proof and the kv_length_ports / role / layer enforcement. Re-verified against the new head rather than carried over: all 11 emitted packages validate and all 11 workflow conformance tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: justinchuby <justinchuby@users.noreply.github.com>
The branch was rebased onto current main again, so 60f41354 is now unreachable from it (`git merge-base --is-ancestor 60f41354 2498e0bc` is false), the same way 1d8cfefe was before it. As noted last time, an orphaned pin keeps reporting green because GitHub serves unreferenced objects, so this has to be caught by checking lineage rather than by waiting for a red job. 2498e0bc also changes how a component's port roles resolve. A declared role now names its port on its own; the port contracts are consulted only to break a tie between two ports claiming the same role. Previously a role was honoured only if the port also appeared in `ports.inputs`, so a producer that declared roles without transcribing its graph had the declaration silently dropped and was matched by port spelling instead. This producer declares both, so it was never affected and nothing here has to change. Keeping the contracts is deliberate: they are optional now, not forbidden, and they live inside `pipeline.workflow`, which is the one canonical representation -- they are not a competing statement of the ABI the way a second top-level block would be. They also carry the dtype, rank and shape a consumer needs to allocate without opening the ONNX file, and the tests that guard against contract drift assert on them. Dropping them would remove information and weaken those guards to satisfy a preference, not a rule. Re-verified against the new head: 11 of 11 packages validate, 11 of 11 conformance tests pass, 111 metadata tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: justinchuby <justinchuby@users.noreply.github.com>
A component backed by a shipped `.onnx` now declares only `ports.roles`. The artifact travels inside the package and is authoritative for which ports exist and what dtype, rank and shape each one has, so restating that in YAML created a second writable statement of one fact with nothing keeping the two in agreement — the same defect as `model.io`, one level further down. The runtime resolves ports against the live session, which catches a name the graph does not expose rather than agreeing with a stale echo of it. What no graph can state is what a port *means*: `input_ids` and `position_ids` are both rank-2 int64, and nothing in the file says which is the autoregressive sequence. That is the whole remaining declaration, and it is what lets the decode ABI resolve without recognizing a spelling. The static-cache ABI is unaffected: it was already derived from the state-service group's aliases and the scatter's `write_indices_ports` / `kv_length_ports`. Policy graphs keep their contracts, and that boundary was measured rather than assumed. A workflow SSA value inherits its dtype, rank and request axis from the port that produced it, so those contracts are the type annotations of the workflow's own dataflow, not a description of an external interface. Dropping them made 4 of 11 packages invalid — `<node>.when is row-wise but <node>.value declares no request_aligned batch_layout` — because a validator reads metadata without the artifacts and has no other source for the axis. The contract tests move with the truth they check. Roles, invocation bindings, state pairs and the two scatter control ports now resolve against the graph itself instead of against the metadata's agreement with its own copy, which is strictly stronger: a role naming a port the artifact does not expose now fails where before it only had to match a line the same producer wrote. `TestRolesAloneCarryTheDecodeAbi` is the regression guard for the omission. Every port of every roled component is renamed to an opaque label and the ABI must resolve identically, so nothing recognizable is left to have matched on; deleting the role table must then break exactly the sequence and logits binding and leave the cache half untouched, which is what proves the roles are carrying the fact rather than decorating it. Verified against ONNX GenAI 2498e0bc (confirmed as the remote branch and PR #828 head): 11/11 packages valid, 11/11 runtime conformance including `mobius_static_cache_workflow_executes`, 4532 fast-suite tests green, lintrunner clean. 1376 lines of transcription removed from the fixtures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: justinchuby <justinchuby@users.noreply.github.com>
`2498e0bc` was the branch head when it was pinned and is still reachable — `git merge-base --is-ancestor 2498e0bc 0d1f1702` succeeds — so this is not the orphaned-pin failure. It is the weaker version of it: the pin names a commit that is no longer what the branch says, so CI was answering a question about a state that had moved on, and any new check added upstream would not have run here until something happened to notice. Nothing about the pinned commit was wrong. The rule is simply that a pin should name the ref's head, because an ancestor is a state nobody is maintaining and the gap only ever widens. `0d1f1702` adds the artifact to the canonical `tiny-llm-scatter-workflow` fixture and a test that validates that package as a directory rather than as a YAML string, which is the same entry point our fixtures go through. Re-ran both gates against it: 11/11 packages valid and 11/11 runtime conformance, with TensorScatter firing per layer per step on the fixed-capacity path. Unchanged from the previous head, which is the expected result — that head added a test and a fixture and no validation logic — but it is checked rather than assumed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: justinchuby <justinchuby@users.noreply.github.com>
`7324351a` moves the document-level invariants onto `load_metadata_package`. Until now they ran only for callers who already held a parsed document, which excluded everyone loading a package from disk — including the `validate_metadata` binary our CI job invokes. So the entry point our fixtures actually go through was enforcing a strictly weaker rule set than the one we believed we were being checked against. All 11 packages were re-run against it rather than assumed to be unaffected, because a strictness increase on the exact path we validate through is the case most likely to surface something: 11/11 valid, 11/11 runtime conformance, with TensorScatter still firing per layer per step on the fixed-capacity path. They pass for a structural reason rather than a lucky one. The rule that gained teeth forbids a package from carrying both `model.io` and a workflow, and ours carry no `model:` block at all — there is nothing for a coexistence check to find. This is the first external confirmation that emitting one serialized ABI was worth doing for a reason beyond tidiness: the packages that were relying on the loader's silence are the ones this commit would have broken. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: justinchuby <justinchuby@users.noreply.github.com>
A component named in an `indexed_scatter` group's `write_indices_ports` or `kv_length_ports` is being handed the write cursor and valid length of a preallocated cache. Exactly one consumer reads those: the driver that scatters into the buffer at an index, and it binds its ports from the resolved decode ABI, every field of which is found by role. So a component handed the cursor while declaring no sequence role cannot be resolved as a decoder at all, and the package silently degrades to inferring ports from shapes — the behaviour the canonical form exists to remove. Nothing upstream catches it, and the reason is structural rather than an oversight. Identifying the decoder requires a sequence role, so a component that omits one is invisible to the check that would have caught it. The upstream sole-decoder guard works around that by firing only on workflows with a single ONNX component — but policy graphs are ONNX components, and every package we emit ships ten, so the guard is disabled by construction on precisely the packages that matter. Measured against `6e2ddc78`: dropping the role from the shipped `static_cache` fixture still reports `valid`. That leaves the producer as the only place the contradiction is visible, so it is asserted here, on both the built packages and the shipped fixtures. The second copy is not redundant — a fixture is hand-edited and regenerated far more often than the producer is changed, and the mutation above is exactly the edit that would slip through. The rule is scoped by what the ports are for rather than by counting anything. `speculative` owns attention state and declares no sequence role, which is correct: it is driven through explicit invoke bindings and never asks for a single-decoder ABI. It binds no scatter cursor, so the obligation does not reach it, and no exemption list is needed to say so. Pin moves to `6e2ddc78`, which adds that upstream guard. Re-ran both gates against it: 11/11 valid, 11/11 conformance. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: justinchuby <justinchuby@users.noreply.github.com>
…ucer Two changes that both remove a second copy of something the tree already states once. `add_explicit_package_io` wrote `metadata["model"]["io"]`. It had no production callers, and ONNX GenAI is removing the serialized field with no compatibility shim, so the only thing it could still do was tempt a future caller into emitting an ABI the runtime discards at load. Removed with its six tests; the surviving cross-attention test keeps its coverage under a name that says what it checks. The static-cache error message no longer names `model.io.static_cache`, a key nothing emits. The eleven conformance packages committed 146 binary files and about 14 MB of graphs and weights, including a 5 MB blob. They are a deterministic function of the generator, CI already regenerated and compared the whole tree, and no reviewer can read a diff of them. Only the metadata is committed now. Textproto is the right answer for ONNX GenAI's 26 KB synthetic fixtures and the wrong one here: these carry real weight blobs that a text encoding would grow, and this repository does not use the protobuf APIs a textproto writer needs. Tests that need a graph build them once per session through the new `materialized_workflow_packages` fixture; generation takes about four seconds. This decides where CI must point, and the failure would otherwise have been silent in the wrong direction. A checkout of the committed tree alone does not validate — `component 'cache_length_update' artifact ... cannot be opened` — which is correct, because a workflow claims to describe something executable. Verified by archiving the staged tree and running the validator against it. Validation and conformance now run against the regenerated tree, and the conformance harness no longer guesses a default path that resolved inside the ONNX GenAI checkout it is copied into. Two guards keep the removal from hollowing out the assertions that remain. The comparison step checks that every artifact the committed metadata names was really produced, so a generator that stopped emitting one is caught rather than leaving a test with nothing to compare. `_ports` asserts the same per package, because two of its three callers skip components they cannot resolve and would have passed vacuously against an empty directory; a mutation pointing the fixture at a missing package fails all eleven. `test_describes_itself_only_through_the_workflow` also tightened from "no `model.io`" to "no second ABI at all", including the legacy `pipeline.models`, which is the rule the runtime actually enforces. Verified: 11/11 validate_metadata and 11/11 runtime conformance against the regenerated tree at ONNX GenAI 6e2ddc78, canonical contract suite 145 passed, fast suite 4541 passed, lintrunner clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: justinchuby <justinchuby@users.noreply.github.com>
Rebasing 141 commits onto `79b48bc0` conflicted in the five files where this branch's workflow migration overlaps the two new model integrations. The conflicts were resolved in favour of this branch, which is right for the metadata architecture and wrong for everything else main had just added, so the endpoint was then checked against main hunk by hunk rather than trusted. Every `def`/`class` main added was confirmed present, which surfaced what the resolution had silently dropped. `--revision` was threaded through the asset writers by #509 and did not survive: `_write_clip_tokenizer`, `_write_hf_tokenizer` and `_copy_runtime_assets` lost the keyword from their signatures while their bodies still referenced it, and `write_onnx_genai_config` lost the parameter entirely. The last one is the dangerous shape — it has `**kwargs`, so a pinned revision would have been absorbed and ignored rather than raising, and the package would still build and still validate while its tokenizer came from whatever the branch tip happened to be. That is precisely the failure a pin exists to prevent. Restored at all nine sites, plus `_write_text_runtime_assets`, which is this branch's own helper and needed the same parameter to pass it on. Also restored: audio-processor emission for speech-language packages, which #509 added to the multimodal dispatch and this branch's rewrite of that branch did not carry. Expressed as `_has_audio_encoder` rather than main's inline `"audio_encoder" in pkg` so a package object without `keys()` is a False rather than a TypeError. Two of main's tests could not be taken verbatim. `test_revision_is_forwarded_to_detection_and_build` was overwritten wholesale by a test of this branch's and is restored byte-identically alongside it. `test_dispatch_audio_only_multimodal_pipeline` asserts on `pipeline.models`, the legacy composite ABI this branch replaces, so its still-relevant half — that the revision reaches the feature extractor — is covered by a new test against the workflow instead. `test_runtime_onnx_genai_routes_vlm_through_workflow_emitter` pins the writer call exactly and now expects the threaded `revision`. Verified: 4580 passed (up from 4541; the increase is main's new tests now running), 11/11 `validate_metadata` and 11/11 runtime conformance against ONNX GenAI `6e2ddc78`, lintrunner clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: justinchuby <justinchuby@users.noreply.github.com>
justinchuby
force-pushed
the
justinchuby/simplify-composite-metadata-producer
branch
from
August 21, 2026 05:59
f559607 to
43ef64a
Compare
`6e2ddc78` was the branch head when it was pinned, and it is now reachable from no ref on that repository — `git ls-remote` returns zero matches for it and no remote ref has it as an ancestor. The branch was rebased onto a newer base, which rewrote every commit; the same change now exists as `fb358d42` with a different hash. The failure this avoids is a slow one. `actions/checkout` keeps succeeding against an unreferenced object until GitHub collects it, and then fails with `reference is not a tree` — at a time nobody controls, on a commit nobody can inspect, in a job unrelated to whatever change is in flight. Repinned to the live head `52339e10`, verified with `ls-remote` rather than an API lookup, which answers just as happily for an orphan. Not a fast-forward, so both suites were re-run rather than assumed: 11/11 `validate_metadata` and 11/11 runtime conformance against `52339e10`, including `mobius_static_cache_workflow_executes`. The rebase carried the sole-decoder sequence-role rule through unchanged and adds a CUDA device-memory sampler fix that our packages do not exercise. The document had the same fragility as the workflow file and less reason for it, so its two commit citations are replaced by descriptions of what changed. `.github/workflows/main.yml` is now the only place naming a hash, because it is the only place that has to fetch an exact tree. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: justinchuby <justinchuby@users.noreply.github.com>
…ttable The pinned commit 52339e10 is no longer reachable from any ref: the GenAI branch was rebased a second time and now heads at 0497c6f4. An unreferenced commit is GC-eligible, and `actions/checkout` fails with "reference is not a tree" whenever the collection happens to run -- a red build at a time nobody controls, on a commit nobody can inspect. Verified with `git ls-remote` (zero refs match) and `git merge-base --is-ancestor` (not a fast-forward, so the move could not be assumed benign). `gh api .../commits/<sha>` still answers 200 for such objects, which is why it must not be the check used. Repinned to the live branch head, re-verified rather than assumed: all 11 generated packages validate against it, and all 11 execute under the runtime conformance suite, with TensorScatter firing per layer per step on the static-cache package. Also ignore validation/. CI and the materialized_workflow_packages fixture regenerate the full 14 MB of packages there, and the directory was untracked but not ignored -- one `git add -A` would have restored exactly the megabytes of unreviewable graphs that were just removed from the index. Signed-off-by: Justin Chu <justinchu@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: justinchuby <justinchuby@users.noreply.github.com>
The pinned commit has now been rewritten out from under CI twice in a day, because the branch it names is rebased on a schedule this repository does not control. Each time, the pin kept resolving until the unreferenced object was collected, at which point `actions/checkout` would fail with "reference is not a tree" on a commit nobody can inspect. Bumping the pin each time treats the symptom; the cause is that reachability of the pinned object is someone else's to revoke. Anchor it: the tag mobius-pr478-pin in justinchuby/onnx-genai now points at the exact commit this workflow pins, so the object stays reachable across rebases of the branch it came from. The workflow still pins the SHA, not the tag -- the SHA is what makes a run reproducible, and the tag exists only to keep it alive. The pin itself is unchanged and remains the commit both suites were last verified against. Also correct the check the docs recommend. `git ls-remote | grep <sha>` asks whether a commit is a ref *tip*, which a healthy pin stops being as soon as one more commit lands on the branch; on its own it reports a false orphan. The question that decides whether checkout resolves the commit, and whether it can be collected, is `git merge-base --is-ancestor <sha> <branch-tip>`. Both beat `gh api .../commits/<sha>`, which answers 200 for unreferenced objects and so fails open. Signed-off-by: Justin Chu <justinchu@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: justinchuby <justinchuby@users.noreply.github.com>
The GenAI branch was rebased again -- twice within about two hours, this time because origin/main advanced underneath it -- so the previously pinned commit is no longer reachable from PR #828's head. CI never became at risk: the tag mobius-pr478-pin added in the previous commit kept the pinned object referenced, which is exactly the failure this anchor exists to absorb. The tag has been moved to the new pin, per the rule recorded alongside it. Verified rather than taken on assurance, because a pin move is gated on both suites: all 11 generated packages validate at the new head, and all 11 execute under the runtime conformance suite, with TensorScatter firing per layer per step on the static-cache package. The upstream summary described this push as touching neither the schema, the validator, decoder_abi.rs, nor the workflow recognizer. That is right in substance: diffing the metadata crate between the two pins leaves validation.rs and decoder_abi.rs untouched, and the only change is in parser.rs, where MtpProposerSpec::mtp_state_output becomes Option<String> so a proposal-local head can decline to thread recurrent state. That is the speculator sidecar descriptor, not the package load path, and no package this producer emits reaches it. Signed-off-by: Justin Chu <justinchu@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: justinchuby <justinchuby@users.noreply.github.com>
Protein language models were the first real users of the encoder path, and exporting them exposed four defects that all shared one cause: encoders were being treated as decoders that happened to stop early. Metadata. An encoder package fell through to the decoder producer and emitted a greedy autoregressive loop -- max_output_tokens, eos ids, a sampler, a KV cache -- for a bidirectional model that cannot generate anything. The new encoder-embedding producer emits what actually happens: one invoke, one emit, an `embedding` profile carrying mask-aware mean pooling, and `batch_invariance: row_independent`. The declared inputs are read from the artifact rather than the task signature, because optimization prunes unused graph inputs -- ESM-2 has no token-type embedding, so its metadata must not promise one. Batching. The rank-2 int64 attention mask was passed straight to op.Attention, which cannot broadcast it, so every encoder was broken for batch > 1 and merely added a 0/1 bias at batch 1. BERT now builds a 4D bool padding mask once and shares it across layers. DistilBert. It discarded `attention_mask` outright: padded rows changed 99.4% of their values. The mask is now threaded through the encoder layers. This was found by the new padded-batch integration test, not by inspection, which is the argument for the test. ESM-2. It was registered to the generic BERT module, which cannot load the checkpoint: ESM-2 uses rotary embeddings, pre-norm blocks, a final embedding LayerNorm, token dropout, and no token-type embeddings. `models/esm.py` implements it, with submodule names arranged so initializer names match HuggingFace directly and renaming reduces to stripping the `esm.` prefix. ProtBert. Its config declares no `model_type`, so `AutoConfig` refuses it and the builder misrouted to the diffusers path. `model_type` is now inferred from `architectures`, and `--config` accepts a local directory. Verified against real weights on H200. ESM-2 (facebook/esm2_t6_8M_UR50D, MIT) and ProtBert (Rostlab/prot_bert) reproduce HuggingFace to a relative L2 error of 1.2e-4 and 1.7e-5 on human haemoglobin, insulin, and lysozyme; the larger CUDA gap is TF32 and disappears with use_tf32=0. Padding invariance, batch-order invariance, and pad-token isolation are bit-exact. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: justinchuby <justinchuby@users.noreply.github.com> (cherry picked from commit d6c90f199f5b58a253a18414573d7b2999f2d6f5)
Pin validation to the final ONNX GenAI metadata commit, update encoder embedding profiles and auxiliary inputs to the canonical schema, and assign stable symbols to anonymous dynamic dimensions so generated packages validate without null shapes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: justinchuby <justinchuby@users.noreply.github.com>
Refresh PR #478 on the current Mobius main branch without rewriting its history. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: justinchuby <justinchuby@users.noreply.github.com>
Let schema_version govern workflow syntax and let each ONNX artifact remain authoritative for its opset imports. Regenerate every producer fixture and pin cross-repository validation to the corresponding ONNX GenAI schema commit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: justinchuby <justinchuby@users.noreply.github.com>
Correct the producer documentation to keep ONNX port contracts and opsets artifact-owned, retain semantic roles and state aliases, and support independently shaped K/V tensors across layers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: justinchuby <justinchuby@users.noreply.github.com>
Advance the cross-repository conformance pin to the ONNX GenAI revision containing the finalized heterogeneous KV design documentation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: justinchuby <justinchuby@users.noreply.github.com>
justinchuby
marked this pull request as ready for review
August 21, 2026 21:01
justinchuby
deleted the
justinchuby/simplify-composite-metadata-producer
branch
August 21, 2026 21:01
justinchuby
commented
Aug 21, 2026
| cancel-in-progress: true | ||
|
|
||
| jobs: | ||
| onnx-genai-metadata: |
Member
Author
There was a problem hiding this comment.
Remove or made separate
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Produce the canonical ONNX GenAI
pipeline.workflowABI across Mobius exports and validate it against onnx-genai#828.model.io, legacy phase/strategy duplication, and runtime-private scheduling/allocation policy;null;ir_versionandonnx_opsets, leaving schema and ONNX artifacts authoritative;509cd4e9c4471f4cbc59fe44b47168f0ae128fe3;maininto the branch without rebasing.Validation
validate_metadataparser/semantic validator;Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com