Skip to content

Copilot Chat OTel: emit attachments as typed parts with mime type, size and token estimate - #336985

Open
BorisTkachenko wants to merge 7 commits into
microsoft:mainfrom
BorisTkachenko:boryst/otel-attachment-parts
Open

BorisTkachenko wants to merge 7 commits into
microsoft:mainfrom
BorisTkachenko:boryst/otel-attachment-parts

Conversation

@BorisTkachenko

Copy link
Copy Markdown

Fixes #336984

With captureContent on, gen_ai.input.messages emits text, tool calls, tool results and reasoning as typed parts, but a binary attachment falls into the default: branch of normalizeProviderMessages and is serialised as a text part holding the raw provider block. For an uploaded image that text is {"type":"image","source":{"type":"url","url":"https://github.com/github-copilot/chat/attachments/…"}}: no mime type, no size, no token estimate, and not the shape the GenAI semantic conventions define for attachments.

What changes

  • messageFormatters.ts: normalizeProviderMessages now maps binary blocks to the schema's uri / blob / file parts with modality (image | document) and mime_type, plus size_bytes, width/height (images) and estimated_tokens when they can be derived. It understands the three provider shapes we send: Anthropic image/document (source.type base64 | url | file_id), Chat Completions image_url (string or { url, detail, media_type }), and Responses input_image / input_file. Inline data is measured directly; a detail: 'low' image is estimated at the flat price. Anything with no usable source keeps the previous text fallback, so nothing that was emitted before disappears.
  • IImageService.getUploadedAttachmentMetadata(uri): uploads are the one place the bytes of a URL-referenced attachment are still in the process, so ImageServiceImpl remembers mime type, byte size, dimensions and the token estimate per uploaded URL (bounded map, session-scoped). normalizeProviderMessages takes an optional resolveAttachment hook and ChatMLFetcherImpl passes the lookup through. Tool-result images uploaded via imageDataPartToTSX are covered by the same path.
  • platform/tokenizer/common/attachmentTokenCost.ts: calculateImageTokenCost and estimateDocumentTokenCost move from tokenizer/node/tokenizer.ts to the common layer (re-exported from their old location, so extChatTokenizer.ts is untouched), and a dimensions-based calculateImageTokenCostForDimensions is split out so the estimate can be computed without re-encoding bytes into a data URL.

estimated_tokens is the same prompt-budget estimate the tokenizer already uses; it is documented as an estimate, not a billed figure, since no provider reports per-part usage.

Example

Before:

{"type":"text","content":"{\"type\":\"image\",\"source\":{\"type\":\"url\",\"url\":\"https://github.com/github-copilot/chat/attachments/0f8f…\"}}"}

After:

{"type":"uri","modality":"image","mime_type":"image/png","uri":"https://github.com/github-copilot/chat/attachments/0f8f…","size_bytes":184233,"width":1440,"height":900,"estimated_tokens":1105}

Not in this PR

  • The BYOK Anthropic/Gemini providers build their input messages from LanguageModelChatMessage parts in byokOTelHelpers.ts and still drop data parts as [non-text content]; they can adopt the same part types in a follow-up.
  • After a window reload the upload map is empty, so a historical image re-sent by URL is emitted as a uri part with whatever mime type the wire carries and no size or estimate.

Testing

  • messageFormatters.spec.ts: new cases for every handled shape (inline and URL images, resolver merge and precedence, PDF blob, detail: low, Responses input_file by data and by id, unreadable header, unusable source fallback).
  • New attachmentTokenCost.spec.ts and imageServiceImpl.spec.ts.
  • vitest --run on the three files: 86 passed. eslint --max-warnings=0 clean on all touched files. tsc --noEmit --project tsconfig.json reports no errors under src/.

🤖 Generated with Claude Code

…ze and token estimate

Binary attachment blocks in the request body used to hit the default
branch of normalizeProviderMessages and were serialised into a `text`
part holding the raw provider JSON. For an uploaded chat image that text
carried nothing but the attachment URL.

Map them to the GenAI schema's `uri`, `blob` and `file` parts instead,
with `modality`, `mime_type`, `size_bytes`, `width`/`height` and an
`estimated_tokens` figure derived from the bytes. Uploaded attachments
leave the process before the request is built, so the image service now
remembers what it saw at upload time per URL and the fetcher hands that
lookup to the normaliser. The image and document token estimators move
to the common layer so the OTel formatter can share them.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings September 21, 2026 08:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Service wiring and nested tool-result normalization remain incomplete, and low-detail URL estimates can be inaccurate.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 High severity · 2 Medium severity

Open (3)
What changed in this PR

Adds typed OpenTelemetry attachment metadata for local Copilot Chat.

Changes:

  • Normalize provider attachments into URI, blob, or file parts.
  • Retain uploaded-image metadata for telemetry.
  • Share attachment token estimates and add tests.
File Description
extensions/​copilot/​src/​platform/​tokenizer/​node/​tokenizer.ts Re-exports moved token-cost helpers.
extensions/​copilot/​src/​platform/​tokenizer/​common/​test/​attachmentTokenCost.spec.ts Tests attachment token estimates.
extensions/​copilot/​src/​platform/​tokenizer/​common/​attachmentTokenCost.ts Provides shared token-cost helpers.
extensions/​copilot/​src/​platform/​otel/​common/​test/​messageFormatters.spec.ts Tests typed attachment normalization.
extensions/​copilot/​src/​platform/​otel/​common/​messageFormatters.ts Emits typed OTel attachment parts.
extensions/​copilot/​src/​platform/​otel/​common/​index.ts Exports attachment metadata types.
extensions/​copilot/​src/​platform/​image/​node/​test/​imageServiceImpl.spec.ts Tests uploaded metadata retention.
extensions/​copilot/​src/​platform/​image/​node/​imageServiceImpl.ts Caches uploaded-image metadata.
extensions/​copilot/​src/​platform/​image/​common/​imageService.ts Extends the image-service contract.
extensions/​copilot/​src/​extension/​prompt/​node/​test/​chatMLFetcherRetry.spec.ts Supplies the new test dependency.
extensions/​copilot/​src/​extension/​prompt/​node/​test/​chatMLFetcherResponseApiTelemetry.spec.ts Updates telemetry test construction.
extensions/​copilot/​src/​extension/​prompt/​node/​chatMLFetcher.ts Resolves attachment metadata for telemetry.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread extensions/copilot/src/extension/prompt/node/chatMLFetcher.ts
Comment thread extensions/copilot/src/platform/otel/common/messageFormatters.ts
Comment thread extensions/copilot/src/platform/otel/common/messageFormatters.ts Outdated
@BorisTkachenko

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

…result attachments, price URL images at the block's detail

- chatLibMain registers ImageServiceImpl so ChatMLFetcherImpl resolves there.
- Attachment blocks inside Anthropic tool_result.content, OpenAI tool
  messages and Responses function_call_output.output get the same typed
  parts; other blocks pass through unchanged.
- A URL image whose dimensions the resolver knows is re-estimated with the
  requesting block's detail, so detail: low reports 85 like the inline case.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Malformed sources and extreme image dimensions can still produce incorrect attachment telemetry.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 High severity · 3 Medium severity

Open (4)
Resolved since last review (2)

Comment thread extensions/copilot/src/platform/otel/common/messageFormatters.ts Outdated
Comment thread extensions/copilot/src/platform/otel/common/messageFormatters.ts Outdated
…output shape on a typed part, keep image scaling finite

- Empty url / data / file_id / image_url / file_data (and an empty data-URL
  payload) no longer produce a typed part; an empty file_data falls through
  to a valid file_id.
- function_call_output keeps its joined-text response unless at least one
  block actually became a typed attachment part.
- calculateImageTokenCostForDimensions rejects non-positive or non-finite
  dimensions and keeps the first resize fractional, so a 1x10000 image no
  longer divides by zero into NaN.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Invalid dimensions can throw during telemetry normalization, and unknown MIME types are emitted as schema-invalid null values.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2 High severity

Open (2)
Resolved since last review (3)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Omit unknown MIME types instead of emitting null

extensions/​copilot/​src/​platform/​otel/​common/​messageFormatters.ts:510

When neither the provider block nor resolver supplies a MIME type, this emits mime_type: null; blobPart and filePart use the same fallback. The GenAI message schema allows this optional field only as a string, so historical URL attachments and provider file IDs without metadata become schema-invalid. Make mime_type optional and omit it when unknown, then update the affected constructors and expectations together.

Comment thread extensions/copilot/src/platform/otel/common/messageFormatters.ts Outdated
…quest

An image header can legitimately report a zero dimension. The upload map
stored width/height before the token estimate threw, and the formatter's
recalculation at the block's detail was unguarded, so preparing
gen_ai.input.messages could throw while sending the chat request.

- Upload map and inline blobs price the dimensions first and only record
  them once that succeeds; otherwise only the byte size is kept.
- The formatter's recalculation is guarded and falls back to whatever the
  resolver estimated.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Attachment fallback handling and unnecessary JPEG decoding remain unresolved.

Review effort: Balanced
Findings: None

Resolved since last review (2)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Empty data URLs suppress valid file ID fallbacks

extensions/​copilot/​src/​platform/​otel/​common/​messageFormatters.ts:485

Both Responses attachment branches return immediately when a non-empty data URL is present, even if referencedPart rejects it because its payload is empty. For example, image_url: 'data:image/png;base64,' or file_data: 'data:application/pdf;base64,' prevents a valid file_id on the same block from being emitted, so the whole block falls back to text. Return only when URL normalization succeeds; otherwise continue to the existing file-ID fallback.

Medium severity No-op OTel path unnecessarily decodes large inline JPEGs

extensions/​copilot/​src/​platform/​otel/​common/​messageFormatters.ts:529

When OTel is disabled, NoopOTelService.startSpan() still returns a span object, so the request path still calls normalizeProviderMessages. This new image branch calls getImageDimensions; for JPEG data URLs, getJpegDimensions base64-decodes the entire payload before reading its header. Large inline JPEGs therefore incur an extra full decode and temporary byte allocation on every request even though the no-op service discards the result. Gate content normalization when config.enabled is false, or add a header-only JPEG dimension reader that avoids decoding the full payload.

…PEG headers without decoding the payload

- A Responses `input_image`/`input_file` block whose data URL has no payload
  now falls through to its `file_id` instead of dropping the whole block to
  the text fallback.
- `getJpegDimensions` decodes the base64 payload in growing prefixes and
  stops at the frame header. Inline JPEGs can be megabytes and this runs on
  the request path, including when the OTel service is a no-op.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@BorisTkachenko

Copy link
Copy Markdown
Author

Both of the "previously missed" items from the last review overview are addressed in ed122f5:

  • Empty data URLs suppress valid file ID fallbacks. The input_image and input_file branches now return the URL part only when normalization produced one; a data URL with no payload falls through to the block's file_id. Test: a block with image_url: 'data:image/png;base64,' and file_id yields the file part instead of the text fallback, same for file_data on input_file.
  • Unnecessary JPEG decoding. getJpegDimensions no longer decodes the whole base64 payload. It decodes a 48 KiB prefix, walks the marker segments to the frame header and only decodes a larger prefix when metadata pushes the header past what it has. The byte-based reader shares the same walker. This also removes the full decode from the existing token-counting path, which called the same reader per request. Tests cover a header inside the first chunk (single atob call on a prefix), a header behind 200 KiB of APP1 segments, agreement with the byte reader, and the not-found / not-a-JPEG errors.

I did not gate on config.enabled in the fetcher: the surrounding block intentionally captures content for the debug panel whenever a span handle exists, and changing that felt out of scope here.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Mixed Responses tool outputs can still expose unusable attachment blocks as raw provider objects.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)

Comment thread extensions/copilot/src/platform/otel/common/messageFormatters.ts
…ool outputs

When a tool result or Responses function_call_output mixes a typed attachment
with an attachment that has no usable source, the latter was passed through
as a raw provider block. It now gets the same JSON-as-text fallback that
top-level content uses, so consumers only ever see spec-shaped parts or
the original non-attachment blocks.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

Cross-provider telemetry, image parsing, token accounting, and dependency injection changes warrant final human validation.

Review effort: Balanced
Findings: None

Resolved since last review (1)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Copilot Chat OTel: emit attachments in gen_ai.input.messages as typed parts with mime type, size and token estimate

3 participants