My fix was one line. Code review found two holes in it.

The bug I fixed in Microsoft Agent Framework did not crash anything. That is what made it worth fixing.

When the framework called Gemini without streaming, the response knew which model had answered. When it made the same call with streaming, the model name could disappear on the way. The framework records its metrics through OpenTelemetry, and the model name is one of the dimensions those metrics are split by. On streamed calls, the token and duration histograms simply lost that dimension. Every request still succeeded. The per-model view of cost and latency just had a hole in it.

Why it happened

Each streamed chunk from Gemini may or may not carry a model_version. The streaming parser copied that field straight into the update, so a chunk without it produced an update without a model. The aggregator keeps the last non-empty value it sees, and there was nothing to catch up afterwards, so a stream whose chunks carried no version ended with no model at all.

The non-streaming parser, thirty-odd lines above, already handled this: it fell back to the configured model when the response did not name one. Two paths, same input type, different answers.

The one-line fix

So I proposed the obvious thing: apply the same fallback on the streaming path. One line, two tests, parity with the sibling.

It was wrong twice.

Hole one

I had raised this one myself, as an open question in the PR description: was the client's default model really the right fallback? The automated reviewer answered it. The fallback used the client's default model, not the model actually requested. A client configured for gemini-2.5-flash and called with gemini-2.5-pro for one request would label that request as Flash, metrics included. The model resolved for the request was already in scope where the stream was consumed, so the parser now takes it as an argument and falls back to that.

Hole two

Then the maintainer found the subtler one. Applying the fallback chunk by chunk could overwrite a version the server had already reported. If an early chunk said gemini-2.5-pro-002 and the final usage chunk carried no version, the aggregator, which keeps the last non-empty value, would replace the precise version with the requested alias gemini-2.5-pro.

The fix had to remember what the server said for the whole stream, not chunk by chunk:

fallback_model = model
async for chunk in response_stream:
    if chunk.model_version:
        fallback_model = chunk.model_version
    yield self._process_chunk(chunk, fallback_model)

and in the parser:

model=chunk.model_version or model,

What merged

Three iterations, four tests. One checks the version from a chunk is kept, one checks the fallback to the configured model, one the fallback to the requested model, and one replays the maintainer's exact case: an observed server version must survive a later chunk that omits it. The last one fails on the previous commit, which is the point of it.

What I take from it

My first version was not careless. It matched the sibling path exactly, it had tests, and it fixed the reported symptom. It was still wrong, twice. The first hole I had seen and left as a question. The second came from the maintainer.

That is the case for review in one PR. Not catching typos, but catching the second-order behaviour of a change: which value is in scope, what the aggregator does with it next, what a later chunk can undo. Observability bugs make this harder, because nothing fails. The only signal is a number that is quietly attached to the wrong thing, or to nothing.

If you run LLM calls behind OpenTelemetry, it is worth one test that compares a streamed call with a non-streamed one and checks every attribute, not just the status code.

The PR, with the full review thread: microsoft/agent-framework#9139

I am Ayoub Hammoudi, a backend and AI platforms engineer in Paris. More of my work is at github.com/Ayoubhm07.

Comments

[pm25coder]

I read the merged diff at d697e155 before writing this, because your best claim is one a reader can check — that the last test fails on the previous commit. It does, and the ladder between your three versions is sharper than the post says. The same reading turns up one place where the fix stops one path short.

Hole one survives on the sibling path. The non-streaming branch resolves the request model at _chat_client.py:515, then never passes it into the builder at :527. Inside, :1071 is still:

model=response.model_version or self.model,

self.model is the client's configured default — the value hole one was about. So with a gemini-2.5-flash client, options={"model": "gemini-2.5-pro"}, and a response carrying no model_version: the streaming call now reports gemini-2.5-pro (:1106 takes the resolved model), and the non-streaming call still reports gemini-2.5-flash. One request, two labels — the asymmetry the streaming fix removed.

Your suite already contains that scenario. test_get_response_uses_model_from_options (test_gemini_client.py:618) is a flash client, a pro option, and a response built with no model_version, so it does reach the fallback — but it asserts only call_args.kwargs["model"] == "gemini-2.5-pro", i.e. what was sent, not what ChatResponse.model then reads. Adding assert response.model == "gemini-2.5-pro" fails on the current head.

The ladder, for what it's worth. I modelled the four added tests against the four implementations (pre-PR, your first fallback, your second, and the merged one): failures per version 3 / 2 / 1 / 0 — each iteration is pinned by exactly one more test. Which one matters: test_get_response_streaming_model_from_chunk passes against all four, including the pre-PR base, because a chunk that carries a version is labelled correctly by every version. It pins the field; it cannot fail. The tests that separate the versions are the ones whose chunk omits a version.

That table is a transcription of the two expressions plus from_updates' last-non-None rule (_types.py:2283) — not a run of your package, so each row is checkable by reading those lines.

Writing all three versions out is what made the second-order case findable at all; most PR write-ups stop at the symptom.