Repository navigation
docs(metadata): design of record for generic component batching - #2010
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2010 +/- ##
==========================================
- Coverage 80.79% 80.45% -0.35%
==========================================
Files 425 425
Lines 208150 208790 +640
Branches 208150 208790 +640
==========================================
- Hits 168174 167977 -197
- Misses 34336 35153 +817
- Partials 5640 5660 +20
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
justinchuby
force-pushed
the
justinchuby/vision-batching-design
branch
5 times, most recently
from
August 25, 2026 04:19
9d7f4b8 to
f2b3adb
Compare
justinchuby
force-pushed
the
justinchuby/vision-batching-design
branch
2 times, most recently
from
August 25, 2026 07:17
766bff0 to
d92ac67
Compare
Vision encoders are the motivating workload — a request may carry zero, one, or many images, images from different requests arrive independently, and one session call per image leaves an encoder-shaped GPU idle — but nothing in the design is modality-specific. Audio encoders, rerankers, classifiers, and diffusion conditioners pose the identical question: may the runtime put several independent items into one invocation of this component, and under what structural conditions? That is one declarative fact on a component, not a model feature. The gap this documents, with evidence rather than assertion: the preprocessing tensor vocabulary and the packed *shape* already exist (`BatchLayout:: TokenPacked`, `preprocessing.image`/`audio` programs), while the two things a runtime would need do not. `WorkflowComponent` declares no batching capacity, bound, or co-batching precondition; `token_packed` has no consumer outside the metadata crate (`request_axis()` returns `None` for it, so the interpreter's three request-axis call sites never see a packed value); the image adapter passes exactly one encoded item per invocation even though the preprocessor beneath accepts many; no declared program can produce the `offsets`/`owner` pair `token_packed` names; and validation checks nothing about that pair. The cost of the last gap is already visible — the schema doc comment calls `offsets` request-aligned, the canonical fixture declares it `shared`, and §10.1 describes a `cu_seqlens`-style `rows + 1` vector. Nothing rejects any of the three. What is proposed, all absent by default and therefore backward compatible: `WorkflowComponent.batch_capacity` (axis, max_rows, uniform_axes; absent means one request row per invocation), `TensorContract.pad_mask` linking a padded value to the bool value that says which entries are real, and the `item_offsets` / `item_owner` image-output content roles so a declared program can produce what a packed layout references. Grouping faces two independent kinds of raggedness — item count and item shape — so the two are declared on different axes and never compete for one. Strict `token_packed` validation is specified with its rationale: `offsets` is `shared`, not `request_aligned`, because an exclusive prefix sum is not permutation-followable — permuting rows invalidates it rather than permuting it, so a runtime that regroups recomputes it instead of gathering it. Ownership stays where it already is: metadata describes the bound, the preprocessor produces per-item tensors and packing metadata, the scheduler decides which items co-batch, the interpreter builds and splits the grouped invocation, and ORT and native execute it through the one component-execution seam. Fail-closed is scoped precisely: declining to group is always safe, but grouping without honoring a declared mask or packing is a wrong-answer bug and is forbidden. No new capability identifier is introduced, because no package's correctness requires that its encoder be batched — which is the narrow statement of the broader direction that a fact the workflow structure already determines should not also be serialized as a flag. Re-litigating existing catalogue entries is explicitly out of scope. Also documented: no model-specific booleans anywhere, no `supports_batching` flag (a boolean cannot answer "how many" or "along which axis"), the seven-phase PR DAG from schema through validation, preprocessor, interpreter, backend parity, scheduler grouping, and E2E, and a nine-row acceptance matrix covering non-batchable components, padded/masked groups, variable item size and count, packed ownership including deliberately corrupted offsets, multi-request grouping, ORT/native parity, concurrency, and grouped-versus-sequential performance. Docs only. No schema, validator, or runtime change lands here. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com>
The encoder batching design of record was written from the image case: one
request owns some number of images, images differ in patch count, and the
runtime needs a declared bound before it may put several into one invocation.
Video makes the same problem strictly harder in a way that changes the shape of
the contracts, so the design is generalized now rather than retrofitted later.
A video clip is itself a variable-length group of frames. That adds a third,
independent kind of raggedness on top of the two the design already had:
- item count - how many items a request owns (offsets/owner on the item axis)
- item extent - how far two items differ on a shape axis (mask, or pack it)
- item nesting - an item that is itself a group whose parts must stay
attributable after the group is split
A single video encoder input needs all three at once - clips from several
requests, differing frame counts, differing patch counts per frame - so each is
declared on its own axis and the three never compete for one.
Contract changes in the proposal:
- TensorContract.pad_mask becomes a list of { value, axes } entries instead of
a single { value, axis }. A clip padded to the group's frame count and to
the group's patch count needs a temporal truth and a spatial truth, and one
mask cannot be both. Entries must be disjoint and must cover every free axis
exactly once; an entry may span several axes, which is how a joint
spatiotemporal mask is spelled.
- Ownership content roles are named for the nesting level, not the modality:
item_offsets / item_owner for items in rows, subitem_offsets / subitem_owner
for parts in items. Clips-in-requests and frames-in-clips are then the same
two rules that images-in-requests and audio-windows-in-utterances are.
- token_packed validation gains a nesting rule: a value packed on an inner
axis must declare its own offsets/owner pair whose extents agree with the
outer packed extent, and the execution-time invariants are checked at every
level, not only the outermost.
Also documented:
- 2.7 states the video gap with citations - video is a first-class workflow
output role with a fixture and a conformance test, while video encoding has
no frame-sequence producer at all (temporal_patch_size replicates one still
image, and ImagePreprocessor::preprocess takes &[DynamicImage]), no temporal
validity vocabulary on the image side, and no nesting.
- 3.4 works the same declaration through image, video, audio, and text
encoders, and separates variable clips, variable frames, and variable
resolutions into the three distinct questions they are. Batch compatibility
falls out as a derived predicate over declared uniform_axes extents rather
than a scheduler policy.
- Fail-closed gains an explicit prohibition on making two items compatible by
changing them: no trimming, no resampling to a common frame count, no
downscaling to a common resolution. Padding with a recorded mask and packing
are the only sanctioned reconciliations, so the correct response to an
incompatible pair is two groups.
- The phase DAG splits the preprocessor phase into items (P3a) and nesting
(P3b, the missing frame-sequence producer), and the acceptance matrix grows
video rows - variable frames per clip, variable clips per request, mixed
spatial and temporal raggedness, nested ownership corruption, mixed-modality
serving, a third modality reusing the path unchanged - with performance
reported separately for image and video in frames/s and clips/s.
The contracts stay modality-agnostic throughout. An axis is an axis to the
interpreter and the scheduler; only a preprocessing program's content roles say
whether it counts frames, patches, mel bins, or tokens, and nothing in the
batching path reads them. A "video batching" code path would be the same defect
as a "video attention" kernel, so 7 now rejects is_video and
supports_video_batching by name alongside the image equivalents.
Docs only. No schema, validator, or runtime change.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com>
Six generic constraints surfaced by working the video case through the encoder batching design. Each one is model-agnostic and each one closes a hole where a runtime could otherwise be correct by accident. Nesting depth is capped at two levels. Parts in items and items in rows covers frame -> clip -> request, frame -> window -> request, and token -> segment -> request, and every further level multiplies the validation surface, the split implementation, and the corruption cases that must be tested. The cap is stated so a third level is a deliberate schema change with its own design rather than something a package can assert into existence; a value packed on three axes is rejected at load. uniform_axes covers temporal and spatial agreement alike. A frame-count-pinned encoder pins its temporal axis, a resolution-pinned encoder pins its geometry axes, and an encoder pinned both ways pins both. To the scheduler these are the same test on declared extents, so pinning a temporal axis needs no new concept - it is just another way for two items to be incompatible, answered with two groups rather than with resampling. Padding is right-padding. Real entries along a padded axis must form a prefix, so a mask is a leading run of true. A mask alone would permit interior holes, which forces every consumer that slices, reduces, or pools to handle gaps rather than a length; left padding would shift position indices - temporal position embeddings above all - between a solo item and the same item in a group, which is a wrong answer rather than a slow one. Prefix validity is also what makes a padded level and a packed level describe the same geometry, since an offsets vector is a prefix sum. Item budgets are distinct from request rows, and both are distinct from parts. One request can contribute eight clips and seven requests can contribute one image between them, so a decode-side row bound is not an item bound in either direction. max_rows is renamed max_items to say so, and axis_budgets is added to bound the summed extent of a free axis across the group: four one-frame clips and four sixty-frame clips satisfy the same item bound and differ by an order of magnitude in work. Device memory remains a fourth quantity that stays out of metadata entirely, because a byte number measured on one machine is a cost model and wrong on the next. Packed payloads must alias rather than copy. Grouping exists to save time, and a grouping layer that assembles its input through host memory can consume the whole win before the kernel starts. A device-resident item stays device-resident when it joins a group; splitting a packed output is an aliasing operation wherever the span is contiguous, which is the practical reason the ownership rules demand contiguity at every level; and any copy that cannot be avoided is counted and reported next to the throughput numbers. This also settles that padding always materializes while packing can alias - relevant for video, where padding sixteen clips to the longest clip's frame count is several times the input buffer for no additional answer. Backend readiness is asked before a group is formed, not discovered by attempting one. The native path is reported to crash above batch one for vision models - batch_vision.rs limits native to batch 1 in two places and its probe deliberately never attempts batch 2 - and the regression test that would catch it returns early when its model file is missing, so it is not a standing guard. Until parity is proven, native reports grouped execution as unsupported and therefore never receives a group; ORT grouping is unaffected, since the two backends answer independently. Declining is safe and costs only the optimization; attempting is a segfault that takes every unrelated in-flight request with it. A decline must still be counted, because "grouping silently never happened" and "grouping happened and did not help" are different bugs with the same throughput number. Fixtures are synthetic, tiny, and checked in. A new P3c phase adds a generated video-encoder workflow fixture under tests/fixtures/onnx_genai_workflows/ built by a scripts/build_tiny_*.py generator like the ~40 that already exist, with deterministic outputs so solo-versus-group equality is exact, variable frame and clip counts, and both resolution-pinned and resolution-agnostic variants. It gates the video rows and is what makes the existing batch-invariance test unskippable in P5. A fixture that cannot be regenerated from a script in this repository is not a fixture, it is a dependency. The acceptance matrix grows five rows for these: depth bound rejection, right-padding enforcement including a left-padded item and an interior hole, budgets binding independently, no hidden host round-trip with transfer counters, and an unproven backend declining observably while ORT groups the same workload. Performance reporting adds padding overhead alongside frames/s and clips/s. Docs only. No schema, validator, or runtime change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com>
The generic component batching design of record carried eight contract-level defects found in review. This rework replaces the affected contracts rather than patching prose around them. Nested packing is now one physically flattened packed axis plus an ordered, bounded chain of ownership levels (frames -> clips -> rows), one offsets/owner pair per level, with a complete YAML example and concrete numbers showing frame offsets composed through clip offsets. Every claim of two physical packed axes is withdrawn: a second packed axis would make one geometry expressible two ways and put every per-row split on a strided gather, which value.rs states outright cannot alias. Compatibility is stated as a mechanism, not a claim. InferenceMetadata and every struct this design extends are deny_unknown_fields, so an old runtime rejects the whole document rather than ignoring a new field; the forward-compatibility promise in the schema_version doc comment does not describe this codebase, and nothing validates schema_version at all. The design now requires a pre-deserialization version gate on the onnx-model-package precedent, a minor bump, and conditional emission, and it no longer uses ignore-unknown as the rationale for adding no capability identifier. Component-global integer axes are gone. batch_capacity declares uniform_dimensions and budgets keyed by shape symbol, so ports of differing rank stay coherent, and budgets bind materialized footprint: a packed dimension is charged the sum of valid extents, a padded dimension the enclosing count times the padded extent, and composed dimensions multiply. max_items is folded into a budget on the item level's own symbol. Output-side raggedness gets a declared producer: input companions may be reused only for extent-preserving levels (packed_extent: preserved), otherwise offsets and owners are explicit component outputs (packed_extent: produced), with a validation rule and negative fixture. Serving emit carves referenced offsets/owner companions out of the shared rank>0 rejection, admitted only when int64, rank 1, and referenced by another emitted value's layout, and delivers rebased per-request offsets with no invocation-global owner values. Request rows and packed items are separated. row_scope.axis is the request row axis and never the packed item axis; compact(selection) is lifted through the owner maps into a stable item permutation, with companions recomputed rather than gathered, and repeated or dropped source rows are well defined. Native readiness is per (component, operator class, provider) rather than one global flip, and retains the real CNN crash guard alongside the new synthetic fixture. Also resolves the review notes: padding declares concise int64 valid_lengths with an exact companion shape instead of a boolean mask, offsets and owner extents are stated per level, load-time and invocation-time checks are separated, the packed axis is pinned to 0 for zero-copy aliasing, companion validation never triggers a hidden device-to-host copy, and request_expanded semantics and zero-item behavior are specified. The acceptance matrix grows to 27 rows covering the version gate, conditional emission, produced companions, the serving carve-out, transfer accounting, request_expanded, and zero-item groups. Docs only. No code, schema, or fixture changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com>
…orms Second review of the encoder-batching design returned three blockers and a set of correctness notes. This reworks the design of record accordingly; still docs only, no schema or code. BL1 — packed_extent moves onto each ownership level. A value-wide flag is a category error: a token-merging encoder produces its token->clip level while preserving the clip->row level it never touched, and one flag can state only one of those. `levels` entries now carry `extent: preserved | produced`; input levels carry none. The canonical media_tokens example marks the inner token level produced and the outer clip->row level preserved, and §4 rule 5 validates per level, with the three negative cases spelled out. BL2 — the version gate now has a stated grammar. `[v]major[.minor]`, minor defaulting to 0, so the three spellings already in the tree (absent in 14 of the 39 inference_metadata.yaml files, `v1` in 19, `1.0` in 6) all normalize to v1.0 with no byte changed. Documents carrying the new fields are v1.1, and the gate rejects pre-deserialization on major mismatch *and* on a minor above the runtime's. This is deliberately stricter than onnx-model-package's validate_version, which parses the minor and ignores it; the divergence is justified from deny_unknown_fields and recorded so a later reader does not "fix" it. The acceptance row now requires existing fixture bytes and version strings to be unchanged, not rewritten to v1.0. BL3 — uniform_dimensions is scoped to true per-item ordinary dimensions: never a port's flattened packed symbol, never an ownership level's unit count, since both count the group the scheduler assembled. A fixed-frame video encoder therefore drops the frames->clips level entirely and pins `frames` as an ordinary per-clip dimension. The "pinned or budgeted, never both" rule is withdrawn and replaced: budgets are nesting paths read outermost-first and must be group-rooted, so a pinned symbol carries no singleton entry but does participate in composed footprint entries, because a pinned symbol is equal within a group and still varies between groups. Also from the review notes: valid_lengths is corrected to a new generic role rather than a generalization of an existing audio role, with the coexistence mapping to frame_lengths / sample_lengths / valid_frames stated; cross-port level consistency is keyed on companion-pair identity rather than level index, which the clip pair at level 1 of an input and level 0 of a pooled output requires; a declared invocation-global owner output is internal and per-request owners are derived from the rebased offsets; the fixture generator count is corrected to 26 generators and 19 workflow fixtures; and the vocabulary citations are re-pinned to the exact blocks. The acceptance matrix grows to 28 rows with a fixed-frame video row, a pinned-resolution budget case, and expanded version-gate and output-producer rows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com>
…base PR #2011 (d60e207) added a `## Session concurrency` section to DESIGN.md immediately after the runtime-services paragraph this branch extends. Both paragraphs are retained — the encoder-grouping paragraph stays attached to the runtime-services list it qualifies, and the session-concurrency section follows it — but "the one significant gap" now reads as a claim about the whole document rather than about that list, and session concurrency has its own pending migration phases. Scoped to "the gap in that list" and rewrapped. No change to ENCODER_BATCHING.md or INFERENCE_METADATA_DECISIONS.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com>
Upstream churn since this branch's original base moved two cited files: `workflow.rs` gained six lines above the invoke path and five above the image preprocess adapter, and `value.rs` gained five lines above `alias_with_offset`. Every citation into those two files is re-pinned to the line range that still holds the cited code; no prose, rule, example, or matrix row changes. workflow.rs 2152,4181,4846 -> 2158,4186,4851 (request_axis call sites) workflow.rs 3020-3116 -> 3025-3121 (run_image_preprocess_adapter) workflow.rs 3084 -> 3089 (preprocess_encoded, x2) workflow.rs 1713-1927 -> 1719-1933 (WorkflowNode::Invoke) workflow.rs 2535 -> 2541 (invoke_onnx_component) workflow.rs 1115-1145 -> 1121-1151 (per-invocation symbol binding) value.rs 1521-1537 -> 1526-1542 (alias residency/strided, x2) value.rs 1538-1546 -> 1543-1551 (alias_with_offset signature) value.rs 1567-1573 -> 1572-1578 (zero-element alias) All 68 fully-qualified citations in both documents resolve in range against d60e207; the remaining short-form references were re-verified by hand. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com>
Two amendments adopted from the schema implementation (#2009), both places where the implementation was stricter than this document and right to be. **The packed axis rule now covers every packed value.** §4 rule 2 scoped `axis == 0` to values a `batch_capacity` component consumes or produces. That scope was too narrow: the runtime splits a packed value per request whenever it hands one back — rule 8's emit path rebases offsets and derives per-request owners with no capacity declaration in sight — and that split wants the same contiguous span for the same aliasing reason. Under the old scope a strided packed emit was legal precisely where nothing had declared it could pay for one. No fixture in the tree declares a non-zero packed axis, so the wider rule rejects nothing that exists. **A preprocessing-program companion must carry the companion role.** §3.3 introduced `pack_offsets` and `pack_owner` but stated their use only descriptively. §4 rule 4 now requires them of any level companion that is a declared program output. The asymmetry with §3.2, where a length value is resolved by name and its role is descriptive, is deliberate and now explained: length vectors have established modality spellings that predate this design and cannot be narrowed to one role, while the companion roles are new and have nothing to accommodate — so requiring them costs nothing and catches a program that wires, say, `grid_dimensions` into a `levels` entry at load rather than shipping a plausible int64 rank-1 vector that means something else. Decisions §10.5 and §10.6 restate both, matrix row 17 gains the two rejections, and the P2 phase box's stale "§3.1 rules 1-7" is corrected to 1-9 to match the body. No other rule, example, or acceptance row changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com>
…hat a document must say Two contract-visible consequences of the schema implementation's code review (#2009), recorded here so the design of record does not lag the contract. **A ragged emit publishes the account of its raggedness.** §4 rule 8 required a packed emit to declare every level's `offsets` and `owner`, but said nothing about a padded emit, which is the same hole. §3.2 rejects a payload-shaped validity mask precisely so the length vector is the only account of the padding that exists anywhere; a caller that receives the payload without it holds trailing entries it cannot distinguish from real ones. Rule 8 is generalized to cover both, the serving carve-out now admits an `int64` rank-1 `shared` output named as another emitted value's `valid_lengths` as well as one named as an offsets/owner, and §3.2, decisions §10.5/§10.6, matrix row 25, and P2's negative fixtures follow. A `valid_lengths` companion needs no rebasing on delivery — a length is already relative to the item it measures — so a request receives the slice indexing its own items. **A version may gate what a document must say; never what it may say wrongly.** §6.1 gains the distinction, prompted by a rule that demanded an explicit append axis and thereby rejected two shipped diffusion packages that meant the default they were written against. When a new field makes an existing default ambiguous, the demand to state intent explicitly starts at the version that introduced the ambiguity. Range checks and every rule about a value a document did state stay unconditional at every version — relaxing those lets an older spelling assert something false rather than merely leave something unsaid. The two look alike in a validator and are opposites in effect: one preserves packages that predate a question, the other would preserve their bugs. Packages under the exemption should still be migrated, so it stays a guarantee rather than a live path nothing exercises. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com>
…not compatibility
Delta review found the design promising more compatibility than it delivers.
`levels` **replaces** the flat `TokenPacked { offsets, owner, axis }` pair rather
than joining it, so a v1.0 document that spells the flat form is refused by a
v1.1 runtime. §6.1 previously read as though every v1.0 document loaded
unchanged, and matrix row 22 asserted it outright.
§6.1 gains the exception explicitly: the promise is narrowed to documents that
do not use `token_packed`, the refusal is called a `deny_unknown_fields`-class
parse failure that P1 owes an actionable message, and the migration is stated —
`{ offsets: X, owner: Y, axis: 0 }` becomes
`{ axis: 0, levels: [ { offsets: X, owner: Y } ] }`, which means exactly what the
flat form meant, so no document loses expressiveness. A dual-accepting
deserializer is rejected on the record: two spellings of one fact (Rule 10)
carried indefinitely, with every §4 rule written twice, once per shape. The
break is affordable because the blast radius is measured rather than assumed —
no package under `tests/fixtures/onnx_genai_workflows/` or
`examples/inference_metadata/` declares `token_packed` at all, the only in-repo
document spelling the flat form is one metadata test, and the layout has no
runtime consumer, so there is no deployed behavior to preserve. This project
accepts a development-time break over a permanent second spelling; saying so
stops a later reader restoring the flat form for compatibility it never had.
Row 22 no longer promises unqualified v1.0 loading, P1 owns the migration of
`redesign_invariants.rs:158` plus a test for the refusal and the rewrite, and
§2.1 names the flat shape as the one being replaced.
Also corrected: the stale flat examples in decisions §8.2 and §10.1, which were
normative and showed a spelling this design removes; the fixture count (17
workflow fixture directories, 19 entries counting `README.md` and
`SOURCE_COMMIT`); and the acceptance-matrix intro's row range, 16-27 rather
than 16-26.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com>
…te runs first The flat `token_packed` migration raised a question the previous commit stated the outcome of but not the mechanism: what kind of change a retired spelling is, and where its refusal belongs relative to the version gate. Both are §6.1's to answer, so both are now written there. **A retired spelling is a third kind of change.** An additive field is a version question — a document that uses it declares a higher minor and an older runtime refuses on the version. A rule demanding a document state something newly ambiguous is the second, and is version-scoped. A retired spelling is neither: both the flat pair and `levels` belong to the same version line, so the stale document is well-formed, declares a supported version, and says something the reader no longer has a meaning for. No comparison of version numbers detects that, so the refusal must be shape recognition and cannot be folded into the gate. The "packages keep loading" claim above is correspondingly scoped to additive fields, which is the only class it was ever true of. **Shape recognition runs after the gate.** A retired-shape message asserts what a spelling means, and that assertion is scoped to one vocabulary. A document declaring v2.0 has not established that it belongs to the line in which the flat pair was retired, so telling its author to rewrite into v1's `levels` claims knowledge of v2 this build does not have; it should be refused on its version and told nothing further. The same reasoning applies to any recognizer whose message asserts what a spelling means — where an existing one runs before the gate, aligning it is a separate change with its own evidence. **The message is the deliverable.** An unknown-field error naming `offsets`, with the path stopping at the contract rather than the layout, reads like a typo and sends a reader hunting for a misspelling that does not exist. The refusal must name the containing path down to the layout, give the exact replacement, and say that no document is converted. The published schema is generated from the same types and closes its objects, so producer-side validation reaches the same verdict as the loader — worth a test rather than a coincidence. Matrix row 22 gains the ordering as an acceptance: a `2.0` document carrying the flat pair is refused on its version only and never receives the migration message. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ed is not published Two corrections to §4 rule 8 and its mirrors in decisions §10.5/§10.6, both raised by the schema PR (#2009) after a review found the second of them as a live contradiction in its validator. **The carve-out's "rank 1" was wrong for a length.** An `offsets` and an `owner` are rank 1 by construction, so the flat phrasing was accurate for them and read as though it were the general rule. It is not: §3.2 fixes a `valid_lengths` at the number of axes outer to the padded one, so a value padded on axis 2 publishes a rank-2 length vector. Under the flat reading, one rule of this design requires that companion and another refuses it — and the refusal necessarily advises `request_aligned`, which is exactly what a companion must not declare. Following the narrower wording would make a correct package wrong. The condition is therefore per *reference*: collect what each declaration naming the value requires of it — role, and the rank that role implies at that site — and admit the value if it satisfies any one. Three consequences are written down because each is a place an implementer must choose. Expectations accumulate rather than overwrite, since one length vector may legitimately bound the same dimension of two outputs. Two declarations demanding different ranks of one value is a genuine contradiction, but it belongs to the padding declaration that is wrong; raising it at the serving rule names an innocent line. And where a padded dimension does not resolve to an axis, the reference carries no rank expectation at all — that declaration is already being rejected, and a second complaint about a rank nothing could compute sends the reader somewhere there is nothing to fix. A value named as a companion but shaped like something else is refused as a malformed companion, naming role and expected rank, rather than falling through to the generic message that advises the one layout a companion may not have. **Declared is not published.** Both halves of rule 8 now require the companion to be emitted — written by some declared step — not merely to appear in the output list. An output nothing writes hands the caller an empty vector beside a ragged payload, which is the precise failure the rule exists to prevent wearing the appearance of compliance. The check is whole-workflow rather than path-sensitive: "written by some declared step" is what is soundly decidable without evaluating branch predicates, and a path-sensitive rule would reject a correct package whose companion is written in a sibling branch of the emit. Acceptance row 25 grows the cases that distinguish the two readings: doubly padded values publishing rank-1 and rank-2 lengths together, a packed value padded on the axes it does not pack publishing three companions at three ranks, a mis-ranked length, and a companion declared but written by no step. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com>
…d say why A follow-on to the previous commit, closing the asymmetry the schema PR (#2009) found on its own side of the same rule. The obligation requires a companion to be emitted; the carve-out already required the value that *names* it to be emitted, but the neighbouring decidability sentence still said "decidable from the declared outputs alone", which invites the weaker reading. Asking one side for an emit while accepting a bare declaration on the other is two answers to one question (Rule 10), and the weaker half is exploitable: declare a padded output beside any `shared` vector, write neither, and the vector walks past the serving rule on the strength of a payload that does not exist. Both halves now say emitted, the exploit is written down as the reason, and an expectation is stated to carry both the declaration that claims the value and whether anything writes that declaration. The decidability claim is corrected rather than dropped: the condition is decided from the workflow's own declarations — its outputs and its steps — with no runtime information. That is what made the check affordable, and it survives the stronger reading; only "outputs alone" was wrong. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com>
…xis` The rank clause was corrected two commits ago, but only in the form "the number of axes outer to the padded one". That is the definition; it is not the check, and an implementer has to derive one from the other. The schema PR (#2009) states the same fact as `rank == axis` because that is what its validator evaluates, and the derivation is exactly where the earlier flat "rank 1" survived review on both sides. Both spellings are now given together, in §3.2 where the shape rule lives, in §4 rule 8's carve-out, and in decisions §10.6. No rule changes; a reader of any of the three gets the check without translating. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com>
…citations Closes the last ambiguity a reviewer found before merge: what happens when an `Emit` carries `valid_length` and its output contract also declares `padding`. `WorkflowNode::Emit.valid_length` limits the emitted prefix on the value's growth axis (`ir.rs:900-916`), and the interpreter honors it by *slicing the payload* — `slice_workflow_prefix` for a whole tensor and `slice_workflow_row` per request row. `padding` links a dimension to a `valid_lengths` companion that §3.2 makes the sole account of the padding and §4 rule 8 requires an emit to publish. Both describe one raggedness, so both cannot be right: the trim publishes a shorter tensor and the companion then measures a tensor that was never delivered. The caller either holds lengths that overrun its payload, or receives a value the runtime rewrote behind the metadata that describes it. The rule is therefore exclusion rather than reconciliation: an `Emit` whose `output` declares `padding` MUST NOT carry `valid_length`, and the refusal names the emit, the output and the padding entry rather than picking one. They are not equivalent and the choice is not arbitrary — a trim destroys the padded extent while `padding` keeps the tensor and states what is real in it, and only the second survives a group, where rows share an extent that one of them fills. `token_packed` needs no separate rule. Per-row trimming routes through `request_axis()`, which is `None` for `TokenPacked`, so a packed output has no per-row prefix to express; a whole-tensor trim of the packed axis is already refused by §4 rule 7. The exclusion binds exactly where padding lives. P2 owns the validator and four fixtures: the per-row and whole-tensor trims of a padded output, plus the two positives that must keep working — a `valid_length` emit into an output with no `padding`, and a padded output emitted with no `valid_length`. Acceptance row 25 gains the negative case. Also re-pins six engine citations that drifted when the workflow runtime's state gained named owners upstream: the three `request_axis()` call sites, the image adapter and its one-element `preprocess_encoded` call, the node runner and `invoke_onnx_component`, and the per-invocation symbol binding. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com>
…refuses it The exclusion added in b469916 justified the `token_packed` case by routing through `request_axis()` and appealing to §4 rule 7. That reasoning was weaker than the truth: an emit carrying `valid_length` into an output whose layout declares no request axis is *already* refused at load, as ragged emission with no row axis to be ragged along (`validation.rs:3288-3302`). Since `request_axis()` is `None` for `TokenPacked`, a packed output refuses the combination whatever its padding says — no appeal to rule 7, which needs a declared `padding` entry on the packed dimension to bite at all. A packed *and* padded output now hears from both rules, and the doc says why that is correct rather than redundant: they answer different questions, and suppressing either would leave a document whose only remaining message explains the wrong defect. Three further points the rule needs stated, each of which a reader would otherwise have to guess at: - The refusal MUST name which account is authoritative. "These two disagree" is not actionable until the reader knows which one to delete. `padding.valid_lengths` wins, and the asymmetry is structural: a trim is invisible in the contract, while a `padding` entry is part of what the caller reads. - The exclusion is flat and does not ask which axis the trim is about. An emit's growth axis is stated only by default (`ir.rs:909-915`), so a narrowed rule would have to decide which axis an emit-level length is *about* in order to buy an admission nobody needs, guessing in the direction of admitting the ambiguous case. - It MUST NOT be version-gated. It judges a pair of things a document *did* state, which §6.1 keeps unconditional; and a gate would be inert anyway, since `padding` is itself a v1.1 field, so any document able to express the contradiction has already been forced to declare v1.1. P2 gains a fifth fixture pinning the interaction — a packed and padded output emitted with a `valid_length` must still be refused by the pre-existing rule, so that adding this exclusion is shown not to have shadowed it — and both positive controls must be guarded against becoming vacuous. Acceptance row 25 gains the packed variant of the trimmed-padded pair. Also narrows the `Emit.axis` citation from `ir.rs:905-915` to `909-915`, the doc comment that actually states the default. Docs only; no code, schema, or fixture changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com>
§9's intro partitions the matrix as rows 1-9 modality-neutral, 10-15 modality-specific geometry, 16-27 structural bounds and backend readiness — and then stops, leaving row 28 outside every band it names. The row was added after the sentence was written and the sentence was never widened, which is how a self-referential count goes stale: nothing checks prose against the table it describes. Row 28 is the performance comparison against sequential direct execution, and it does not belong in the 16-27 band. Grouping exists to be faster than executing items one at a time; every other row asks whether grouping is *correct*, and this one asks whether it was worth doing. It gets its own clause saying so. Audited the rest of both documents for the same failure rather than fixing only the instance found. The fixture counts (17 directories, 19 entries) still hold. The PR-DAG box's "§3.1 rules 1-9, §4 rules 1-8" is not stale despite §4 having nine rules: rule 9 states invocation-time preconditions, which load-time validation cannot check, and it is attributed to P4 where it belongs. Docs only; no code, schema, or fixture changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com>
`tests/adapter_artifact_compat.rs` resolves from no directory in the tree: the file is `crates/onnx-genai-metadata/tests/adapter_artifact_compat.rs`, and the row's siblings give either a full path from the repository root or a bare file name. This one is neither, so a reader following it finds nothing and cannot tell whether the test was moved, renamed, or never existed. Pre-existing, from the redesign in #828, and outside this PR's subject. Fixing it anyway because the citation checker this branch runs flags it, and claiming "every citation in both documents resolves" while knowingly leaving one that does not would make the claim worth nothing. The audit that found it was prompted by the same class of defect found in a PR body upstream of this branch: published prose that nothing type-checks. Docs only; no code, schema, or fixture changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com>
PR #2009 merged as 0448f2b, which lands phases P1 and P2 of this design: `WorkflowComponent.batch_capacity`, `TensorContract.padding`, the `levels` ownership chain, the `pack_offsets` / `pack_owner` / `valid_lengths` content roles, the `schema_version` gate, and the load-time validators for all of them. The design of record was still written as if none of it existed, which made its status line, its evidence section, and 57 of its source citations false. What changed: - Status is `partly implemented` rather than `proposed`, naming which phases landed and which did not. The interpreter, preprocessor, scheduler, and backend phases remain unimplemented, so a package can now *declare* a batched encoder that no runtime will group; §6 already requires that asymmetry to be safe, and the note says so. - §2 keeps its observations in the past tense and adds a `Since #2009` note per subsection. The observations are the record of why each field exists and are not recoverable from the field, so they are marked rather than deleted. §2.3, §2.4, and §2.5 are untouched because nothing closed them. - §3.3 and the `valid_lengths` argument no longer claim the roles are absent from the vocabulary; they are declarable and nothing produces them, which is a different and still-open gap. - §6.1's `schema_version` doc-comment defect is recorded as corrected, in the narrow form this design argued for: compatibility comes from a field being absent by default, which buys nothing for a field that was reshaped. - §8 marks P1 and P2 as shipped while keeping the full obligation text, so the phase list stays reviewable against the design instead of decaying into a changelog. - INFERENCE_METADATA_DECISIONS.md §10.4 becomes `What §10.1 did not have`, and §10.5 / §10.6 drop `proposed` for a status note that separates the load-time half now in force from the runtime half that is not. DESIGN.md and docs/README.md follow. - 57 citations re-pinned across the two documents, 39 mechanically and 16 by hand where the merge moved the claim rather than the line. Several were in range but semantically wrong after the `Image*` to `Vision*` rename, which line-number repair alone would not have caught. Two pre-existing defects found while checking: a link to a renamed §6 anchor, and a short-form `redesign_invariants.rs:158` citation appearing before its first full path. Both fixed. Docs only. Verified: every intra-doc link and anchor resolves, every citation is in range and reads as claimed, 28 acceptance rows with uniform columns, `git diff --check` clean, lines within 88 columns. `cargo test -p onnx-genai-metadata --test encoder_batching` passes 104 tests at this main, which is the evidence for the P2 marker. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com>
§3.2 closed with "the new exclusion is therefore load-bearing only where a request axis exists — `request_aligned` and `request_expanded` outputs", which contradicted the sentence three lines above it and would have licensed a narrower validator than this design requires. The premise is false: a packed value may legally pad. §4 rule 7 forbids a `padding` entry only on the dimension the layout *packs* and calls an entry on any other dimension the normal composition of the three kinds of raggedness. Row 25 already requires such a value — `token_packed` on axis 0, padded on two further dimensions — and it is the case with the most extents an emit-level trim can misdescribe. Scoping the exclusion to layouts with a request axis would exempt exactly that value: still refused, because the pre-existing no-row-axis rule fires, but refused with a message about a missing row axis when the defect is a trim contradicting two published length vectors. §3.2's own reasoning three lines earlier says neither refusal may suppress the other, "since suppressing either would leave a document whose only remaining message explains the wrong defect". The closing clause said the opposite. Withdrawn in the past tense rather than deleted, because the clause is why the correction was needed and a silent replacement teaches nothing. The scoping was also unfalsifiable by the fixtures this document demanded. §8's P2 obligation asked only that the packed variant "still be refused by the pre-existing no-row-axis rule", and row 25 said it was rejected "but by" that rule — both of which pass against a validator that scoped the exclusion away. Both now require the fixture to assert **both** messages, which is what detects the narrowing. P2 also drops a redundancy it could not justify: the two negatives differ only in the `valid_length`'s rank, which the interpreter dispatches on and the load-time predicate does not, so one fixture MAY cover both. Found by the implementation agent, which mutation-verified it by inserting the scoping this document implied and watching its doubly-padded packed test fail. The design moved; the code did not have to. Docs only. Links, anchors, and citations verified; 28 acceptance rows with uniform columns; `git diff --check` clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com>
Two citations added in f2b3adb were in range, resolved cleanly, and pointed at the wrong line. `mod.rs:452` was given as audio's `valid_lengths`. Line 452 is `"validity_mask"`; `valid_lengths` is 451. And `mod.rs:452-454` was given as the three roles this design adds — `valid_lengths`, `pack_offsets`, `pack_owner` — but that range starts one line *after* `valid_lengths`, so it excluded the role the sentence was about while still containing two plausible neighbours. Both are hand-counting errors from tightening a correct wider range into a wrong narrow one. Now `mod.rs:451` for the role, and `mod.rs:440-456` — the whole `AudioOutputContent` block — for the vocabulary, parallel to the `349-363` already used for vision. A block citation cannot be off by one. The verification that missed this is the more useful finding. Every check run on this branch asserts that a citation is *in range*, which is a property of the file's length and not of its content, so a citation can point at unrelated code and pass. Both defects here did. The check that finds them asserts the cited range *contains* a distinctive string from the claim around it; running that over all 97 ranged citations in these four documents flags 29, of which 27 are the heuristic failing to see an anchor on an adjacent line — a struct name one line above the `deny_unknown_fields` attribute the sentence cites — and 2 were these. Each of the 27 was read and confirmed, including the scheduler's `schedule()` body, the multi-row prompt refusal in `ContinuousBatchManager::submit`, the per-row trim path gated on `request_axis().is_some()`, and the `onnx-model-package` minor-ignoring precedent this design deliberately departs from. Docs only. Links, anchors, in-range and content checks all pass; 28 acceptance rows with uniform columns; `git diff --check` clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com>
…fy by content #2009 merged as `0448f2bc6`, moving the files this document cites most. The citations were re-pinned in an earlier commit on this branch, but the check that guarded them asked only whether a line number fell inside the file. That is a property of the file's *length*, not of its *content*: a citation can point at unrelated code that reads plausibly and still pass, and the failure is silent. Verification is now content-aware — for each of the 97 ranged citations, the range's **first** line must contain the symbol or comment the surrounding sentence claims, and the range must contain a second distinctive substring. Anchoring on the head line makes the check position-sensitive, so every citation fails under a one-line drift rather than passing. Applying it to the merged tree found three references that were in range and wrong at the head, all of them narrowings of a previously correct range: - `schema/package.rs:158-174` -> `162-174`. The claim is about `batch_invariance`; lines 158-161 are the unrelated `decoding` field, so the citation opened on someone else's declaration. - `ort/value.rs:1543-1551` -> `1544-1551`. The claim names `alias_with_offset`; 1543 is the closing line of the preceding doc comment. - `ort/value.rs:1526-1542` -> `1524-1543`, in both documents. The claim quotes the no-copy-view doc comment; 1526 is a blank `///`. Re-pinned to the whole comment block, since a block citation cannot be off by one. The remaining 54 ranges verify unchanged against the merged tree, including all 80 citations into the 24 files #2009 touched. Records the convention in the header so the next editor re-pins by locating the named symbol rather than by adjusting the number until it lands inside the file. Docs only: no code, schema, or fixture changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com>
…itation The header already tells an editor to re-pin a drifted citation "by locating the named symbol". For citations into `validation.rs` that instruction could not be followed: seven of them carried a bare `path:line` and named no symbol at all, so a reader who found the number stale had nothing to search for. The document was asking for a repair it did not equip anyone to make. This is not hypothetical drift. #2074 rewrites the stale call-site comment at `validation.rs:4851` and is a net +5 lines above three of the ranges this document cites (`4858-4862`, `5059-5091`, `5102-5130`). A comment-only change moves coordinates just as surely as a behavioural one, so "no behaviour changed" is not a reason to expect citations to hold. When it lands, those three ranges will be five lines off; naming `validate_compaction_derivability` and `validate_packed_emit_companions` beside them makes the drift recoverable with one `grep` instead of silent. The name is the durable half of a citation: a line number is a coordinate into a tree the reader may not have checked out, while a symbol name survives every edit that does not rename it. The convention is recorded in the header so the next citation is written this way rather than repaired later. No citation range is added, removed, or altered by this commit — the ranges before and after are byte-identical, and none of the 31 cited files changed between `46478fc0f` (where the content-aware check last passed on all 97 citations) and `f66e84251`, so that verification transfers rather than needing a re-run. Docs only: no code, schema, or fixture changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com>
…jection The scoping error this branch corrected in `7a158756d` was not merely missed by the acceptance criteria — it was *certified* by them. Row 25 and the P2 fifth fixture demanded that the packed-and-padded trimmed emit be refused by the pre-existing no-row-axis rule, and a validator that narrows the exclusion to layouts declaring a request axis delivers exactly that. The criterion a defective implementation passes as cleanly as the correct one is worse than an absent criterion, because it converts review into evidence. Auditing the whole matrix for that shape found five more clauses that asserted only that something was rejected, never which rule did the rejecting: - Row 11: a package declaring a frames→clips level *and* pinning `frames` — now asserts §3.1 rule 2's own message. - Row 17: "the axis rejection does not depend on the component declaring a capacity" was self-defeating as written. Asserting only refusal on the capacity-less fixture also passes against an implementation that refused it *for having no capacity* — the opposite of the claim. Now asserts the axis message. - Row 22: `2.0` refused on major — now names the major it read, so a minor-version refusal cannot satisfy it. - Row 25: the withheld lengths and the declared-but-unwritten companion now each assert the reason they were not admitted, and assert that neither receives the generic advice to declare a row axis, which is precisely what a companion must not do. - Row 27: request-expanded *and* packed on one axis now asserts the message naming that pair. Such a declaration trips other rules incidentally, so an outcome-only case stays green after this one is narrowed away. Generalized in the §9 preamble rather than left as five repairs: a fixture built to violate one rule usually violates others too, so an outcome-only case is precise by accident and nothing holds that. The failure is one-directional and therefore invisible in review — the outcome survives the narrowing and only the message is lost. Zero acceptance rows now carry a bare rejection clause; all 28 keep uniform columns. No citation range is added, removed, or altered, and no cited file changed between `f66e84251` and `bc715c329`. Docs only: no code, schema, or fixture changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby
force-pushed
the
justinchuby/vision-batching-design
branch
from
August 25, 2026 07:54
d92ac67 to
e7af183
Compare
justinchuby
added a commit
that referenced
this pull request
Aug 26, 2026
…ibe (#2074) ## What this fixes A comment in shipped code states a claim about the validator beneath it that is false, and false in the direction that invites a weaker reimplementation. `validation.rs`, above the serving companion carve-out: > All of that is decidable from the declared outputs alone. The code immediately below does not do that, and must not. `output_companions` opens with `let emitted = emitted_outputs(workflow);` and walks the step tree, then sets `claimant_emitted` on every `CompanionExpectation`. That field exists precisely so a value claimed only by a declaration that no step writes hears about the declaration, rather than being advised to declare a row axis -- which is the one layout a companion may not have. The function's own doc comment says this correctly; only the call-site comment is stale. ## Why a comment is worth a PR The claim is not merely imprecise. An implementer working from it produces the outputs-only check, and that check has a hole: declare a padded output beside any `shared` int64 vector, write neither, and the vector walks past the serving rule on the strength of a payload that does not exist. One emit and one bare declaration are two answers to one question. The design document of record carried the identical sentence and was corrected in review. The code's copy was not -- because the code was already right, so nothing forced the comment to move with it. That is the failure mode worth naming: a wrong comment beside right code has nothing to make it fail, and the test suite cannot see it. ## What is deliberately not changed Eleven lines above sits a similar-sounding claim about the ragged-emission rule -- "checked from the declared output alone". That one is **accurate**: the check reads `declared.contract.batch_layout.request_axis()` and consults no steps. The two comments should not be aligned to each other, because the distinction between them is real and is exactly the thing that was lost. Only the false one moved. ## A second instance of the same class `a_video_program_needs_its_adapter` asserted `!errors(&document).is_empty()` -- that *something* rejected, not that the adapter rule did. Exactly one rule fires today, so the test was precise by accident, and nothing held that. Narrow the ABI-pinning rule away and any other refusal the fixture happened to trigger would keep it green while it had stopped testing adapters. It is the same defect as the comment above, in a different medium: an assertion that cannot fail on the thing it names. A rejection test that does not pin *which* rule rejected cannot detect that rule being narrowed out of existence, because the outcome survives and only the message is lost. It now asserts the message. ## Three more, and what makes a negative assertion load-bearing The design's acceptance row 25 was tightened to require that the withheld -companion cases assert not only the reason they were refused but also that neither receives the generic advice to declare a row axis. Three fixtures asserted only the positive: - `a_companion_of_a_result_nobody_produces_describes_nothing` - `a_declared_companion_that_no_step_writes_is_not_published` - `a_declared_ownership_companion_that_no_step_writes_is_not_published_either` Each is refused for the reason it names, but a narrowed carve-out refuses it too, through the generic message that advises `request_aligned` or `token_packed` -- the one layout a companion may not declare. The outcome survives the narrowing; only the reason is lost. So each now asserts the absence of that advice, and each negative was mutation-verified against a *distinct* narrowing that leaves the positive assertion standing: deleting the unwritten-claimant branch, dropping `padding` from `output_companions`, and dropping the layout companions. The mutation earned its place twice. The first narrowing I hypothesised for the withheld-lengths case -- admitting a companion only when its claimant published every companion -- does refuse the document, but through the wrong-shape message, not the generic one. Had I shipped the comment I first wrote, the assertion would have been correct and its stated justification false: one degree weaker than the code it sits on, which is exactly the defect in the first commit here, arriving in the medium of a test comment. One near-miss worth recording: `grep` for the generic message in `validation.rs` returns nothing, because the phrase is split across a line continuation inside the `format!`. Taking that at face value would have condemned a pre-existing, correct negative assertion as vacuous. A coordinate is not the only thing that can resolve cleanly and mislead; so can a search that finds nothing. ## Provenance Found by the design agent (#2010) reading the merged #2009 surface against the design documents, after #2009 merged as `0448f2bc6`. They could not fix it themselves without their PR losing its docs-only property. ## Verification - `cargo test -p onnx-genai-metadata` -- 344 (`encoder_batching` 104), run on the branch rebased onto current `main`, not on the base it was cut from. Main had moved 14 commits underneath it; none touch `crates/onnx-genai-metadata/`, which is what makes the local result evidence about the merge result. "Is main different" was the wrong question -- the one that decides whether a green run still means anything is whether main moved *under the thing that was verified*. - `cargo fmt --all -- --check`, `cargo clippy -p onnx-genai-metadata --all-targets -- -D warnings`. - No behaviour, no schema, and no fixture document is touched: one comment and four test assertions. `validation.rs` is unchanged since the first commit, so the +5 line shift it causes for `docs/` citations is unchanged too. --------- Signed-off-by: Justin Chu <justinchu@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Generic encoder batching — metadata design of record
Docs only. No code, no schema, no fixtures. Adds
docs/genai/ENCODER_BATCHING.mdas the designof record and mirrors the normative parts into
docs/genai/INFERENCE_METADATA_DECISIONS.md(§10.4–§10.6, §15),
docs/architecture/DESIGN.md, and the docs index.Motivated by image and video encoders; nothing in the contracts is
modality-specific. Contracts carry shape symbols, bounds, lengths, and
ownership levels; a modality vocabulary only produces the semantic values those
contracts point at, so audio windows and text segments reach the same path with
no new concept and no model-family boolean.
Status: the metadata half of this design has shipped
#2009 merged as
0448f2bc6while this PR was open, landing phases P1 and P2:batch_capacity,padding, thelevelsownership chain, the three contentroles, the
schema_versiongate, and the load-time validators for all of them(
crates/onnx-genai-metadata/tests/encoder_batching.rs, 104 tests). The latestcommit here rewrites the document's status framing so it stops describing shipped
fields as missing, and re-pins the citations the merge moved. The runtime half
is untouched — nothing groups, nothing packs, no program produces the
companions — so a package can now declare a batched encoder that no runtime
will honour. That asymmetry is what §6 already requires to be safe: declaring a
capacity is inert until something reads it, and a runtime that declines to group
is always correct.
Requirement keywords are therefore in force at load and describe runtime
behaviour that is not yet implemented. Still do not merge until the design is
accepted — this is the design of record, not a report on the implementation.
The gap this design was written against
The preprocessing tensor vocabulary existed, but component-level batching
capability and a usable
TokenPackedruntime did not.BatchLayout::TokenPackednamed
offsets/ownervalues that no declared program could produce;WorkflowComponenthad no capacity surface; validation of packed values was oneserving-emit rule; the interpreter honors only
SharedandRequestAligned;native batch > 1 vision is a reported crash. Video is expressible on the output
side and absent on the encoder input side. §2 preserves these observations in the
past tense with a
Since #2009note per subsection, because the reason a fieldexists is not recoverable from the field.
The surface
WorkflowComponent.batch_capacityuniform_dimensions(symbols items must agree on) +budgets(materialized-footprint bounds, keyed by shape symbol)TensorContract.padding{ dimension, valid_lengths }linking a padded dimension to its per-item length companionTokenPacked.levels{ offsets, owner }chain over one physically packed axis (axis: 0), innermost first, plusextent: preserved | producedper level on outputspack_offsets,pack_owner,valid_lengthscontent rolesOwnership split is unchanged in shape: metadata describes, the preprocessor
produces, the scheduler groups, the interpreter executes and splits,
and ORT and native answer capability independently through one seam.
Fail-closed throughout: declining to group is always safe; grouping without
honoring a declared length or level is forbidden; a runtime never fabricates
padding, invents companions, or reconciles two items by changing them.
Phasing P0–P7 and a 28-row E2E acceptance matrix (non-batchable, padded,
variable size/count, packed ownership, nested video ownership, multi-request,
ORT/native parity, concurrency, and grouped-versus-sequential performance
reported for images and video separately) are in §8 and §9.
Second review (
d8aac60c) — 3 blockers + notes, all resolved in2daeb83fpacked_extentwas value-widelevels: [{ offsets, owner, extent }]). Canonicalmedia_tokensmarks the inner token levelproducedand the outer clip→row levelpreserved; input levels carry noextent. Grammar, complete YAML, the worked-outputs table, §4 rule 5, decisions §10.6, and matrix row 24 all rewritten per level. A value-wide flag is shown to be unvalidatable for the mixed chain.[v]major[.minor], minor defaults to 0, so absent /v1/1.0all normalize to v1.0 (14 / 19 / 6 of the 39 in-treeinference_metadata.yamlfiles). New batching documents emit v1.1. The gate rejects pre-deserialization on major mismatch and on a minor above the runtime's — deliberately stricter thanonnx-model-package, which parses<major>.<minor>and ignores the minor (lib.rs:563-579); the divergence is justified fromdeny_unknown_fieldsand recorded. Matrix row 23 now requires existing fixture bytes and version strings unchanged, and conditional emission requiresv1.1when new fields are present.uniform_dimensions/ budget rule incoherentuniform_dimensionsnames true per-item ordinary dimensions only — never the flattened packed symbol, never a level's unit-count symbol (new rule 2, since both count the group the scheduler assembled). Fixed-frame video drops the frames→clips level and pinsframesas an ordinary per-clip dimension (§3.4 variants table). "Pinned or budgeted, never both" is withdrawn: budgets are nesting paths read outermost-first and must be group-rooted, so a pinned symbol carries no singleton entry but does participate in composed footprint entries ([frames, patches]), because a pinned symbol is equal within a group and still varies between groups.Review notes:
valid_lengthsis a new generic role, not an existing audio one. The audiovocabulary is
waveform,features,audio_features,valid_frames,valid_samples,sample_lengths,frame_lengths,validity_mask(
schema/mod.rs:350-365) — there is novalid_lengths. The earlier claim iswithdrawn, and a coexistence table states how the modality-specific spellings
keep working (a
paddingentry references its length value by name).index —
{ clip_offsets, clip_owner }is level 1 ofpixel_valuesand level0 of
clip_embeddings, and an index-keyed rule would reject that correctpackage (§4 rule 6).
owneroutput is internal: it must be declared so the workflowvalidates, and is never delivered; per-request owners are derived from the
rebased offsets (§4 rule 8, decisions §10.6).
variable-vs-fixed frame count explicitly, with a matching matrix row 11.
scripts/build_tiny_*.pygenerators feeding19 workflow fixture directories (was "~40").
mod.rs:296-309image,
350-365audio,47-52/53-54schema_version).First review — 8 blockers + N1–N6, resolved in
d8aac60cOne physically flattened packed axis with a bounded ownership chain (no two
physical packed axes); the real schema-version migration mechanism instead of an
ignore-unknown claim; symbol-keyed
uniform_dimensions/budgetsinstead ofcomponent-global integers; a declared output-side ragged producer; a serving
carve-out for referenced companions; request row axis separated from packed item
axis with
compact(selection)lifted through owner maps; budgets bound tomaterialized footprint; a native readiness gate that is per
(component, operator class, provider) and retains the real CNN crash guard.
N1–N6: lengths instead of a mask, exact companion shapes, level extents,
load-time versus invocation checks,
axis: 0for zero-copy aliasing, and nohidden device→host validation transfer — plus
request_expandedsemantics andzero-item behavior.
Verification
git diff --stat origin/main...HEADtouches four.mdfiles.git diff --checkclean; markdown tables column-checked.broken links in the changed files).
path:linecitation re-verified against the tree at this SHA.Rebase onto
d60e2072(#2011)#2011 landed
docs/architecture/SESSION_CONCURRENCY.mdand touched both sharedfiles this PR also edits. The branch is rebased linearly onto latest
main;docs/README.mdauto-merged (the two index entries sit in different rows) andthe one conflict, in
docs/architecture/DESIGN.md, was resolved semantically:the encoder-grouping paragraph stays attached to the runtime-services list it
qualifies, upstream's
## Session concurrencysection follows it, and its claimis scoped from "the one significant gap" to "the gap in that list" now that a
second in-progress area is documented alongside it.
ENCODER_BATCHING.mdandINFERENCE_METADATA_DECISIONS.mdare otherwiseunchanged from reviewed head
2daeb83f6. The only edits are citation drift:workflow.rsgained six lines above the invoke path and five above the imagepreprocess adapter, and
value.rsgained five abovealias_with_offset, so theten citations into those two files are re-pinned. No prose, rule, example, or
matrix row changed — the interdiff over
docs/genai/is exactly ten line-numbersubstitutions.
Amendments from the schema implementation (#2009)
Two places where the schema PR was stricter than this document and right to be;
both adopted rather than narrowed, so the two do not diverge.
axis == 0applies to every packed value, not only one abatch_capacitycomponent consumes or produces. The runtime splits a packed value per request
whenever it hands one back — §4 rule 8's emit path rebases offsets and derives
per-request owners with no capacity in sight — and that split wants the same
contiguous span for the same aliasing reason. The old scope made a strided
packed emit legal exactly where nothing had declared it could pay for one. No
fixture declares a non-zero packed axis, so the wider rule rejects nothing.
pack_offsets/pack_owner. §3.3 introduced the roles but used them onlydescriptively. §4 rule 4 now requires them, scoped to values whose declaration
carries a
contentrole at all — a component's graph outputs on anextent: producedlevel carry none and are explicitly out of reach. Theasymmetry with §3.2, where a length value is resolved by name and its role is
descriptive, is now explained rather than left as a wart: length vectors have
established modality spellings that predate this design, while the companion
roles are new and have nothing to accommodate.
Matrix row 17 gains both rejections; P2's negative-fixture list names both; the
phase box's stale "§3.1 rules 1-7" is corrected to 1-9 to match the body.
Follow-on contracts from the schema code review (#2009)
raggedness. The rule covered packed emits and said nothing about padded
ones, which was an omission. §3.2 rejects a payload-shaped validity mask
precisely so the length vector is the only account of the padding in
existence, so an emitted padded value must declare each entry's
valid_lengthsas a workflow output, and the serving carve-out admits anint64sharedoutput named as another emitted value'svalid_lengthsonthe same terms as an offsets/owner. Unlike
offsets, a length needs norebasing on delivery — a request receives the slice indexing its own items —
and unlike
owner, it is delivered rather than internal.Correction, recorded rather than quietly patched: this paragraph said
"
int64rank-1" when it was written, which was the flat reading later foundto be wrong for a length and fixed in
57b5f088f— the rank is the one thereference demands, rank 1 for an
offsetsorownerby construction and thenumber of axes outer to the padded one (
rank == axis) for avalid_lengths.Row 25 grew to four cases at this round and has grown twice since; the current
count lives in the matrix, which is the only place worth reading it.
The stale summary is the point: the flat reading survived in four places —
§4, decisions §10.6, the schema PR's validator, and both PR bodies — removed
from each at a different time by whoever owned it, each of us then asserting
the fix was complete. Nothing type-checks prose, so a claim restated in a
summary is a copy that goes stale silently. Grep the artifacts you publish,
not only the ones you compile.
must say, never what it may say wrongly. When a new field makes an
existing default ambiguous, the demand to state intent explicitly starts at
the version that introduced the ambiguity; range checks and every rule about
a value a document did state stay unconditional at every version. The two
look alike in a validator and are opposites in effect — one preserves
packages that predate a question, the other would preserve their bugs.
Packages under the exemption should still be migrated, so it stays a
compatibility guarantee rather than a live path nothing exercises.
Delta-review cleanup: the flat
token_packedspelling is migrated, not shimmedlevelsreplaces the flatTokenPacked { offsets, owner, axis }pair ratherthan joining it, so a v1.0 document spelling the flat form is refused by a v1.1
runtime. §6.1 read as though every v1.0 document loaded unchanged and matrix
row 22 asserted it outright; both are corrected.
token_packed. The flat form is the one deliberate break in the v1.1 surface,and P1 owes its refusal an actionable message rather than a
deny_unknown_fieldsserde error.{ offsets: X, owner: Y, axis: 0 }→{ axis: 0, levels: [ { offsets: X, owner: Y } ] }.indefinitely, and every §4 rule written twice — once against each shape.
tests/fixtures/onnx_genai_workflows/orexamples/inference_metadata/declares
token_packedat all, the only in-repo document spelling the flatform is one metadata test, and the layout has no runtime consumer, so no
deployed behavior is preserved by keeping it.
Also corrected: the stale normative flat examples in decisions §8.2 and
§10.1; §2.1 now names the flat shape as the one being replaced; the fixture
count (17 workflow fixture directories, 19 entries counting
README.mdandSOURCE_COMMIT); and the acceptance-matrix intro row range, 16–27.Follow-up: the taxonomy behind the migration, and recognizer ordering (
e9727f04b)The previous revision recorded that the flat spelling is replaced; it did not
record what kind of change that is, or where its refusal belongs. Both are §6.1's
to answer, and the schema PR (#2009) asked directly. §6.1 now states:
none of it. An additive field is a version question: the document declares a
higher minor and an older runtime refuses on the version. A rule demanding that
a document state something newly ambiguous is the second, and is version-
scoped. A retired spelling is neither — both spellings belong to the same
version line, so the stale document is well-formed, declares a supported
version, and says something the reader no longer has a meaning for. No
comparison of version numbers can detect that, so the refusal MUST be shape
recognition and cannot be folded into the gate. The "packages that use nothing
new keep loading" claim is correspondingly scoped to additive fields, which is
the only class it was ever true of.
message asserts what a spelling means, and that assertion is scoped to one
vocabulary. A
v2.0document has not established that it belongs to the line inwhich the flat pair was retired, so telling its author to rewrite it into v1's
levelsclaims knowledge of v2 this build does not have. It is refused on itsversion and told nothing further. Where an existing recognizer runs before the
gate, aligning it is a separate change with its own evidence and is not this
design's to make.
offsets,with the path stopping at the contract rather than the layout, reads like a typo
and sends a reader hunting for a misspelling that does not exist. The refusal
MUST name the containing path down to the layout, give the exact
replacement, and say that no document is converted. The published schema is
generated from the same types and closes its objects, so producer-side
validation reaches the same verdict as the loader — an agreement worth a test
rather than a coincidence.
Acceptance row 22 gains the ordering: a
2.0document carrying the flat pair isrefused on its version only and never receives the migration message.
Follow-up: companion rank is per reference, and declared is not published (
57b5f088f)Two corrections to §4 rule 8 and its mirrors in decisions §10.5/§10.6, raised by
#2009 after a review found the first as a live contradiction in its validator.
offsetsandownerare rank 1 by construction, so the phrasing was accurate for them and read as
though it were general. §3.2 fixes a
valid_lengthsat the number of axesouter to the padded one, so a value padded on axis 2 publishes a rank-2
length. Under the flat reading one rule requires that companion and another
refuses it — and the refusal necessarily advises
request_aligned, which isexactly what a companion MUST NOT declare. The condition is now per
reference: collect what each declaration naming the value requires of it —
role, and the rank that role implies at that site — and admit it if it satisfies
any one. Expectations accumulate (one length may bound the same dimension of
two outputs); conflicting ranks are a defect of the
paddingdeclaration andare reported there, not at the serving rule, which would name an innocent line;
an unresolvable padded
dimensioncarries no rank expectation, since thatdeclaration is already being rejected. A value named as a companion but shaped
like something else is refused as a malformed companion, naming role and
expected rank.
to be emitted — written by some declared step — not merely to appear in the
output list. An output nothing writes hands the caller an empty vector beside a
ragged payload: the rule's own failure case wearing the appearance of
compliance. The check is whole-workflow, not path-sensitive, because
"written by some declared step" is what is soundly decidable without evaluating
branch predicates, and a path-sensitive rule would reject a correct package
whose companion is written in a sibling branch of the emit.
Acceptance row 25 grows the cases that distinguish the two readings: doubly padded
values publishing rank-1 and rank-2 lengths together, a packed value padded on the
axes it does not pack publishing three companions at three ranks, a mis-ranked
length, and a companion declared but written by no step.
Follow-on (
29ae4cbdf): both sides read "emitted". The obligation asks thecompanion to be emitted and the carve-out already asked the value that names it
to be emitted, but the neighbouring decidability sentence still read "decidable
from the declared outputs alone", inviting the weaker reading. One emit and one
bare declaration is two answers to one question (Rule 10), and the weaker half is
exploitable: declare a padded output beside any
sharedvector, write neither, andthe vector walks past the serving rule on the strength of a payload that does not
exist. The decidability claim is corrected rather than dropped — the condition is
decided from the workflow's own declarations, its outputs and its steps, with
no runtime information.
Follow-on (
2b70a8a9c): the length-rank rule is stated in its check form. Therank clause was corrected above, but only definitionally — "the number of axes
outer to the padded one". That is not the check, and the reader has to derive one
from the other; that derivation is exactly where the flat "rank 1" survived review
on both sides. §3.2, §4 rule 8 and decisions §10.6 now give
rank == axis— thepadded dimension's axis index — alongside the definition. No rule changed.
Pre-merge: an emit may not trim a padded output (
b46991675, corrected in9e5bdc996)The last ambiguity a reviewer found: what an
Emitcarryingvalid_lengthmeanswhen its output contract also declares
padding.WorkflowNode::Emit.valid_lengthlimits the emitted prefix on the value's growthaxis (
crates/onnx-genai-metadata/src/schema/ir.rs:900-916), and the interpreterhonours it by slicing the payload —
slice_workflow_prefixfor a whole tensor(
workflow.rs:2215-2226, defined at4696-4747) andslice_workflow_rowperrequest row (
4852-4882).paddinglinks a dimension to avalid_lengthscompanion that §3.2 makes the sole account of the padding and §4 rule 8
requires an emit to publish. Both describe one raggedness, so both cannot be
right: the trim publishes a shorter tensor and the companion then measures a
tensor that was never delivered — the caller either holds lengths that overrun its
payload, or receives a value the runtime rewrote behind the metadata describing
it.
EmitwhoseoutputdeclarespaddingMUST NOT carry
valid_length.not actionable until the reader knows which one to delete. It is
padding.valid_lengths, and the asymmetry is structural rather than apreference:
valid_lengthtruncates what the step writes and is invisible inthe contract, while a
paddingentry is part of what the caller reads andnames the vector rule 8 requires the workflow to publish. A trim also destroys
the padded extent; only
paddingsurvives a group, where rows share an extentthat just one of them fills.
emit's growth axis is stated only by default —
Emit.axis"defaults to thefinal axis" (
ir.rs:909-915) — while a padded value's lengths are indexed bythe axes outer to the padded one. A trim at or outside the padded axis
invalidates the vector; a trim strictly inside it leaves a declared shape the
tensor no longer has. Narrowing to the growth axis would have to decide which
axis an emit-level length is about, to buy an admission nobody needs, and it
would guess in the direction of admitting the ambiguous case.
token_packedneeds no separate rule, and the reason is stronger than the oneb46991675gave. That commit routed the packed case throughrequest_axis()and appealed to §4 rule 7.
9e5bdc996corrects it: an emit carryingvalid_lengthinto an output whose layout declares no request axis isalready refused at load, by a rule that predates this design — ragged
emission needs a declared row axis so the runtime can associate result rows with
requests (
crates/onnx-genai-metadata/src/validation.rs:3288-3302). Sincerequest_axis()isNoneforTokenPacked(ir.rs:67-72), a packed outputrefuses the combination whatever its padding says, with no appeal to rule 7,
which needs a declared
paddingentry on the packed dimension to bite at all. Apacked and padded output hears from both rules, which is correct rather than
redundant: they answer different questions, and suppressing either would leave a
document whose only remaining message explains the wrong defect.
did state, which §6.1 keeps unconditional at every version. A gate would also
be inert:
paddingis itself av1.1field, so a document able to express thecontradiction has already been forced to declare
v1.1— the gate would be asecond, weaker copy of one already in force, and a weaker copy is the kind that
later disagrees with the original.
of a padded output; the two positives that must keep working — a
valid_lengthemit into an output with no
padding, and a padded output emitted withno
valid_length, both guarded against becoming vacuous — and one pinningthe interaction: a packed and padded output emitted with a
valid_lengthmust still be refused by the pre-existing no-row-axis rule, so that adding this
exclusion is shown not to have shadowed it. Acceptance row 25 gains the negative
case and its packed variant.
Audit prompted by a stale summary in the schema PR (
d069fd9da,9d7f4b834)The schema PR found the retired flat "rank 1" reading still live in its own PR
body's summary, after it had been fixed in its code. The same audit run against
this branch's published artifacts found two more instances of the class:
int64rank-1, omitting
valid_lengths— the flat reading, still being publishedin the artifact a reviewer reads first, after four places in the documents had
been corrected. Fixed above, in the past tense with the correction recorded
rather than quietly patched, because the stale copy is the evidence.
stopped, leaving row 28 — performance versus sequential direct execution —
outside every band.
d069fd9dagives it its own clause: every other row askswhether grouping is correct, row 28 asks whether it was worth doing.
Audited rather than spot-fixed. The fixture counts (17 directories, 19 entries)
hold. The PR-DAG box's "§3.1 rules 1-9, §4 rules 1-8" is not stale despite §4
having nine rules — rule 9 states invocation-time preconditions, which load-time
validation cannot check, and it is attributed to P4 where it belongs. One
unrelated pre-existing citation surfaced and is fixed in
9d7f4b834: theadapter-compat row pointed at
tests/adapter_artifact_compat.rs, which resolvesfrom no directory in the tree.
The generalizable finding, which cost two agents four rounds: the flat reading
survived in four places — §4, decisions §10.6, the schema PR's validator, and
both PR bodies — and was removed from each at a different time by whoever owned
it, each then asserting the fix complete. Nothing type-checks prose. Grep the
artifacts you publish, not only the ones you compile.
Rebased onto latest
main, which also required re-pinning six engine citationsthat drifted when the workflow runtime's state gained named owners: the three
request_axis()call sites, the image adapter and its one-elementpreprocess_encodedcall, the node runner andinvoke_onnx_component, and theper-invocation symbol binding.
Citation verification was checking the wrong property (
8512195dc)Every check on this branch asserted that a citation is in range — a property
of the file's length, not of its content. A citation can therefore point at
unrelated code and pass. Two did, both added by me in
f2b3adb6e:mod.rs:452valid_lengths"validity_mask";valid_lengthsis 451mod.rs:452-454valid_lengths, excluding the role the sentence is aboutBoth are hand-counting errors from tightening a correct wider range into a wrong
narrow one, and both stayed plausible because the neighbouring lines are other
roles from the same vocabulary. Now
mod.rs:451andmod.rs:440-456— the wholeAudioOutputContentblock, parallel to the349-363already used for vision. Ablock citation cannot be off by one.
The check that finds this asserts the cited range contains a distinctive string
from the surrounding claim. Run over all 97 ranged citations in these four
documents it flags 29: 27 are the heuristic failing to see an anchor one line
outside the range — a struct name above the
deny_unknown_fieldsattribute thesentence actually cites — and 2 were these. All 27 were read and confirmed,
including
Scheduler::schedule(), the multi-row prompt refusal inContinuousBatchManager::submit, the per-row trim path gated onrequest_axis().is_some(), and theonnx-model-packageminor-ignoring precedentthis design deliberately departs from.
Raised by the implementation agent (#2009), whose framing is the right one: a
check that cannot fail on the defect it is meant to catch is the kind that later
disagrees with the thing it was meant to verify.
Correction: the emit-trim exclusion binds on packed outputs too (
7a158756d)§3.2 closed with "the new exclusion is therefore load-bearing only where a
request axis exists —
request_alignedandrequest_expandedoutputs". Thatcontradicted the sentence three lines above it and would have licensed a
narrower validator than this design requires.
The premise is false: a packed value may legally pad. §4 rule 7 forbids a
paddingentry only on the dimension the layout packs, and calls an entry onany other dimension the normal composition of the three kinds of raggedness. Row
25 already requires such a value —
token_packedon axis 0, padded on twofurther dimensions — and it is the case with the most extents an emit-level
trim can misdescribe. Scoping the exclusion to layouts with a request axis would
exempt exactly that value: still refused, because the no-row-axis rule fires, but
refused with a message about a missing row axis when the defect is a trim
contradicting two published length vectors. §3.2's own reasoning says neither
refusal may suppress the other "since suppressing either would leave a document
whose only remaining message explains the wrong defect". The closing clause said
the opposite.
The scoping was unfalsifiable by the fixtures this document demanded, which
is the part worth generalizing. §8's P2 obligation asked only that the packed
variant "still be refused by the pre-existing no-row-axis rule", and row 25 said
it was rejected "but by" that rule. Both pass against a validator that scoped the
exclusion away, because the outcome is preserved and only the message is
lost. Both now require the fixture to assert both messages. An acceptance row
that checks a rejection without checking which rule rejected cannot detect a rule
being narrowed out of existence underneath it.
P2 also drops a redundancy it could not justify: its two negatives differ only in
the
valid_length's rank, which the interpreter dispatches on and the load-timepredicate does not, so one fixture MAY cover both.
Found by the implementation agent (#2009), which mutation-verified it by
inserting the scoping this document implied and watching its doubly-padded packed
test fail. The clause is withdrawn in the past tense rather than deleted, because
it is the reason the correction was needed.
Post-merge status rewrite (
f2b3adb6e)The design was still written as if none of it existed, which made its status
line, its evidence section, and 57 of its citations false. Corrected:
partly implemented, naming which phases landed. §2gains a two-layer note and per-subsection
Since #2009markers; §2.3(
token_packedhas no runtime), §2.4 (single-item adapter), and §2.5 (noscheduler work item) are untouched because nothing closed them.
valid_lengthsargument said the companion roles were absent from the content-role
vocabulary. They are now in it, and nothing produces them — a different and
still-open gap, so the claim is rewritten rather than re-pinned. §2.8 and
§6.1's
schema_versiondoc-comment defect are recorded as corrected, in thenarrow form this design argued for: compatibility comes from a field being
absent by default, which buys nothing for a field that was reshaped.
list that mutates into a changelog stops being reviewable against the design,
so the checklist stays and the marker goes on top of it.
drop
proposedfor a status note separating the load-time half now in forcefrom the runtime half that is not.
DESIGN.mdand the docs index follow.16 by hand where the merge moved the claim rather than the line. Several
were in range and semantically wrong after the
Image*→Vision*rename —
mod.rs:330-363still resolved, but to a vocabulary that nowcontains the very roles the sentence said were missing. Line-number repair
alone would have preserved a false statement behind a valid citation, which is
the failure mode a mechanical re-pin invites.
Two pre-existing defects surfaced while checking and are fixed: a link to a §6
anchor renamed in an earlier round, and a short-form
redesign_invariants.rs:158citation appearing before its first full path, which resolves from no
directory.
Verified at this head: every intra-doc link and anchor resolves, every citation
is in range and reads as claimed, 28 acceptance rows with uniform columns,
git diff --checkclean, lines within 88 columns, diff touchesdocs/only.Post-merge re-pin, verified by content (
766bff06b)Rebased onto
origin/main46478fc0f, which contains #2009 as0448f2bc6.The point of this round is not the three fixes below — it is that the check
that was guarding the citations could not have found them.
The defect in the method. The previous gate asked whether a cited line
number fell inside the file. That is a property of the file's length, not of
its content. A citation can survive a refactor, land on unrelated code that
reads plausibly, and report green. #2009 added ~2000 lines to
validation.rsand ~270 to
ir.rs; a checker that cannot fail on drift is not evidence thatdrift did not happen.
The replacement. For each of the 97 ranged citations (57 distinct
ranges, 80 of them into the 24 files #2009 touched), a hand-reviewed
manifest records two anchors read from the document's claim, not derived from
the file:
comment the surrounding sentence names;
Head-anchoring is what makes the check position-sensitive. A body-only anchor
survives a shift; a head anchor cannot.
Three references were in range and wrong at the head. All three are
narrowings of a previously correct wider range — the same failure as the audio
off-by-one in
8512195dc, which is now twice enough to be a pattern rather thana slip:
schema/package.rs:158-174batch_invariancedecodingfield162-174ort/value.rs:1543-1551alias_with_offset1544-1551ort/value.rs:1526-1542(×2 docs)///1524-1543— the whole block, since a block citation cannot be off by oneThe other 54 ranges verify unchanged against the merged tree.
Evidence the checker can fail. Two mutations, both run against the shipped
docs:
at every offset. Per-range, shifting one citation in isolation:
57/57 detected at −1, 57/57 at +1. No range survives a one-line move.
check reports PASS on all three (
package.rshas 563 lines,value.rshas 3092); the content-aware check reports FAIL on all three, naming the
line-1 text it found instead. This is the exact class the old gate admitted.
Recorded in the document, not only in this PR. The header now states the
convention — cite so that line 1 carries the claim, cite whole blocks for
block-level claims, and re-pin by locating the named symbol rather than by
adjusting the number until it lands inside the file. A verification habit that
lives only in a PR description is lost at merge.
Docs only. No code, schema, or fixture changes; design contracts are unchanged
by this round.
Citations now name their symbol (
d92ac677b)Rebased onto
origin/mainf66e84251. None of the 31 cited.rsfileschanged between
46478fc0f— where the content-aware check last passed on all97 citations — and
f66e84251, and this round adds, removes, and alters zerocitation ranges (the before/after range multisets are byte-identical). So the
previous verification transfers by argument rather than by re-running it.
The gap this closes is in the header's own instruction. It tells an editor to
re-pin a drifted citation "by locating the named symbol". Seven citations into
validation.rsnamed no symbol — justpath:line— so that instruction wasunfollowable exactly where it was most needed. The document asked for a repair it
did not equip anyone to make.
And the drift is scheduled, not hypothetical. #2074 rewrites the stale
call-site comment at
validation.rs:4851— the one this branch reported, wherethe comment claimed the companion carve-out was "decidable from the declared
outputs alone" while
output_companionswalks the step tree. Its hunk is@@ -4848,7 +4848,12 @@, a net +5 lines, which moves three ranges thisdocument cites:
validation.rs:4858-4862(3 doc sites)4863-4867validation.rs:5059-50915064-5096validation.rs:5102-51305107-51354811-4825sits above the hunk and is unaffected. A comment-only change movescoordinates exactly as far as a behavioural one, so "no behaviour changed" is
not a reason to expect citations to hold — which is the argument for not chasing
numbers at all.
All seven now read
(invalidate_compaction_derivability,…/validation.rs:4858-4862)or name
validate_packed_emit_companions/validate_emit_length_authority.The name is the durable half: a line number is a coordinate into a tree the
reader may not have checked out, while a symbol survives every edit that does
not rename it, and one
greprecovers the range after any drift. A stale barenumber leaves a reader nothing to search for.
On #2074 itself: nothing in this document changes. The corrected comment now
reads "decided from the workflow's own declarations — its outputs and its
steps", which is the phrase
ENCODER_BATCHING.md§4 andINFERENCE_METADATA_DECISIONS.md§10.6 already used verbatim. The document was the correct copy and the comment was the staleone; the fix converges on the doc rather than requiring it to move.
Docs only. No code, schema, or fixture changes; design contracts unchanged.
The acceptance criteria certified the bug they were written against (
e7af18384)Rebased onto
bc715c329. Prompted by #2074's second hunk — a shipped test thatasserted
!errors(&document).is_empty(), that something rejected, never whichrule — I audited this matrix for the same shape and found six clauses with it.
The sharp form of the problem. Row 25 and the P2 fifth fixture did not merely
fail to catch the scoping error corrected in
7a158756d; they were satisfiedby it. Both demanded the packed-and-padded trimmed emit be refused by the
pre-existing no-row-axis rule — which is exactly what a validator narrowing the
exclusion to layouts with a request axis delivers. A criterion a defective
implementation passes as cleanly as the correct one is worse than an absent
criterion, because it converts review into evidence.
Six repairs:
frames"is rejected by rule 2"2.0is refused on major"Generalized rather than left as five repairs. §9's preamble now makes it
normative: every row requiring a rejection is satisfied only by asserting the
message that names the rule. A fixture built to violate one rule usually violates
others too, so an outcome-only case is precise by accident and nothing holds
that. And the failure is one-directional, which is why it is invisible in review:
the outcome survives the narrowing and only the message is lost.
Zero bare rejection clauses remain across all 28 rows (mechanically checked);
columns stay uniform. No citation range is added, removed, or altered and no
cited file changed between
f66e84251andbc715c329, so the content-awarecitation verification continues to transfer.
Docs only. No code, schema, or fixture changes.