fix(api): Modality zero-value and type-preserving observability attributes#25
Merged
Merged
Conversation
…utes Follow-ups from the extensibility review: - Add tensor.proto with a DataType enum and a single row-major Tensor type (dense or sparse, any dtype). Replace DenseFloatTensor and SparseFloatTensor with Tensor across embedding, classification, and scoring outputs; collapse the EmbeddingOutput oneof to one field. - Modality: drop the "0 = image" default so UNSPECIFIED just means unset. - LoadInfo/RuntimeEvent attributes: map<string,string> -> google.protobuf.Struct so numeric, boolean, and list telemetry keeps its type on the wire. - docs: document the enum-growth client rule and the explicit-integer-units choice for durations/timestamps; mirror all of the above in api.md. Pre-adoption at schema revision 1, so the tensor-type and map->Struct changes are intentionally wire-breaking. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
tanmayv25
requested review from
alec-flowers,
connorcarpenter15,
dagil-nvidia,
nv-anants,
nvda-mesharma and
saturley-hall
as code owners
July 17, 2026 01:19
connorcarpenter15
left a comment
connorcarpenter15
left a comment
Member
There was a problem hiding this comment.
LGTM overall, but not sure we need a separate tensor.proto file yet. Can it be merged with inputs.proto?
Contributor
Author
|
I don't think inputs.proto is the right place as it is used by EmbeddingOutput. |
Follow-ups from the extensibility review (tensor-type changes dropped; that belongs in a dedicated design when a real consumer exists): - Modality: drop the "0 = image" default so UNSPECIFIED just means unset. - LoadInfo/RuntimeEvent attributes: map<string,string> -> google.protobuf.Struct so numeric, boolean, and list telemetry keeps its type on the wire instead of being flattened to strings. - docs: document the enum-growth client rule and the explicit-integer-units choice for durations/timestamps; mirror the above in api.md. The map->Struct change is intentionally wire-breaking; safe pre-adoption at schema revision 1. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
tanmayv25
changed the title
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
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two follow-ups from the schema extensibility/stability review, plus doc clarifications. Tightly scoped and mirrored in
docs/api.md.The generic tensor-type work that was originally on this branch has been dropped — a dtype-aware tensor (ergonomic-vs-raw, layout, dense/sparse) should be designed as one coherent piece when a real non-fp32 or tensor-inference consumer actually exists, not assembled speculatively now.
Changes
1. Fix the
Modalityzero-value foot-gunMODALITY_UNSPECIFIED = 0previously documented "treated as image," which permanently erased the old-client-vs-forgot-to-set distinction. NowUNSPECIFIEDsimply means the sender left it unset; no engine behavior is prescribed in the enum comment. Comment-only, non-breaking.2. Type-preserving observability attributes
LoadInfo.attributesandRuntimeEvent.attributeschange frommap<string,string>togoogle.protobuf.Struct, so numeric / boolean / list telemetry keeps its JSON type on the wire instead of being flattened to strings. This also unifies them with the existingStructbags (EngineError.details,KvSessionRef.attributes_struct). Implements #24.3. Documentation
_ms/_unix_nanos) by design.Compatibility & verification
input.proto,observability.proto,docs/api.md).protoccompile: OKbuf lint(STANDARD): PASSbuf breakingvsmain: only themap→Structchange on the twoattributesfields — intentionally wire-breaking, safe pre-adoption at schema revision 1.markdownlint docs/api.md: 0 errorsCloses #24
🤖 Generated with Claude Code