Skip to content

Commit 07df4b7

Browse files
committed
fix(teams): commit accumulated text only after successful _teams_send
Address Codex P2 finding on PR #88. The previous flow updated ``accumulated`` and ``session.sequence`` BEFORE awaiting ``_teams_send``, so a 429/network failure mid-stream would leave the partial RawMessage returned to the caller (and ``session._text`` feeding the final close- activity) carrying text that Teams never displayed to the user. Fix: - Build ``candidate_accumulated = accumulated + text`` and ``next_sequence = session.sequence + 1`` as candidates. - Send the activity with the candidate values. - Commit ``accumulated`` and ``session.sequence`` only after the send succeeds. The soft-cancel path (logged warn + session.cancel + break) now genuinely returns the high-water mark of successfully-sent text. Matches the documented "RawMessage carries pre-failure content" guarantee in ``docs/UPSTREAM_SYNC.md`` non-parity table. Updated test_emit_send_failure_cancels_session: previously asserted ``result.raw["text"] == "helloworld"`` (which contained the rejected "world" chunk); now asserts ``"hello"`` and ``session.sequence == 1``. Skipped Codex P2 #2 (active_streams clobber timing) — matches upstream TS behavior; deferring fix as a Python-only improvement would be a divergence and is design-significant. File as follow-up if desired. 23 native-streaming tests pass. https://claude.ai/code/session_01FyMxQn2BEAzmwKS1GZczKj
1 parent 2e96fbb commit 07df4b7

2 files changed

Lines changed: 43 additions & 12 deletions

File tree

src/chat_sdk/adapters/teams/adapter.py

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1141,16 +1141,22 @@ async def _stream_via_emit(
11411141
if not text:
11421142
continue
11431143

1144-
accumulated += text
1144+
# Build a CANDIDATE accumulated value — only commit to
1145+
# ``accumulated`` after a successful ``_teams_send``. If the
1146+
# send raises (429, network error, etc.) we treat it as a
1147+
# soft cancel below; the partial ``RawMessage`` returned to
1148+
# the caller and ``session._text`` (which feeds the final
1149+
# close-activity) must NOT contain text Teams rejected.
1150+
candidate_accumulated = accumulated + text
11451151
# Emit each chunk as an incremental typing activity. We send
11461152
# the cumulative text (not deltas) — Teams clients render the
11471153
# latest text on every streaming activity. Matches upstream's
11481154
# ``stream.emit(text)`` semantics where the SDK accumulates
11491155
# under the hood and ships the latest snapshot.
1150-
session.sequence += 1
1156+
next_sequence = session.sequence + 1
11511157
channel_data: dict[str, Any] = {
11521158
"streamType": _STREAM_TYPE_STREAMING,
1153-
"streamSequence": session.sequence,
1159+
"streamSequence": next_sequence,
11541160
}
11551161
# Hazard #7 — only include ``streamId`` once the server has
11561162
# assigned one. Sending ``"streamId": None`` on the first
@@ -1160,13 +1166,13 @@ async def _stream_via_emit(
11601166

11611167
activity_payload: dict[str, Any] = {
11621168
"type": "typing",
1163-
"text": accumulated,
1169+
"text": candidate_accumulated,
11641170
"channelData": channel_data,
11651171
"entities": [
11661172
{
11671173
"type": "streaminfo",
11681174
"streamType": _STREAM_TYPE_STREAMING,
1169-
"streamSequence": session.sequence,
1175+
"streamSequence": next_sequence,
11701176
}
11711177
],
11721178
}
@@ -1177,16 +1183,22 @@ async def _stream_via_emit(
11771183
# 429 mid-stream is the most common transient — treat
11781184
# any send failure as a soft cancel: stop emitting,
11791185
# leave the session canceled so close() won't try to
1180-
# finalize, and log. The caller still gets a
1181-
# RawMessage with whatever text was already accepted
1182-
# so SentMessage history is consistent.
1186+
# finalize, and log. ``accumulated`` and
1187+
# ``session.sequence`` are NOT updated, so the partial
1188+
# RawMessage returned to the caller (and the final
1189+
# close activity, if any) only carry text Teams
1190+
# actually accepted.
11831191
self._logger.warn(
11841192
"Teams stream emit failed; canceling stream",
11851193
{"threadId": thread_id, "error": str(exc)},
11861194
)
11871195
session.cancel()
11881196
break
11891197

1198+
# Send succeeded — commit the accumulated state.
1199+
accumulated = candidate_accumulated
1200+
session.sequence = next_sequence
1201+
11901202
if session.stream_id is None:
11911203
chunk_id = result.get("id") or ""
11921204
session.first_chunk_id = chunk_id

tests/test_teams_native_streaming.py

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,17 @@ async def text_gen():
394394

395395
@pytest.mark.asyncio
396396
async def test_emit_send_failure_cancels_session(self):
397-
"""A 429 / network error mid-stream cancels the session, no exception."""
397+
"""A 429 / network error mid-stream cancels the session, no exception.
398+
399+
What to fix if this fails: ``_stream_via_emit`` must update
400+
``accumulated`` ONLY after a successful ``_teams_send``. If the send
401+
raises, ``accumulated`` (and the partial ``RawMessage`` returned to
402+
the caller + ``session._text`` feeding the final close-activity)
403+
must NOT contain the rejected chunk's text — Teams never displayed
404+
it to the user. See ``src/chat_sdk/adapters/teams/adapter.py``
405+
around line 1144 (build ``candidate_accumulated`` first, commit
406+
only on success).
407+
"""
398408
adapter = _make_adapter()
399409
adapter._teams_send = AsyncMock(
400410
side_effect=[
@@ -416,9 +426,18 @@ async def text_gen():
416426
assert session.canceled is True
417427
# Two attempted sends (first ok, second failed); no third.
418428
assert adapter._teams_send.await_count == 2
419-
# RawMessage carries the accumulated text including the failed-send chunk
420-
# (which the user already saw rendered locally).
421-
assert result.raw["text"] == "helloworld"
429+
# The rejected "world" chunk MUST NOT appear in the partial
430+
# RawMessage. Teams never accepted it, so SentMessage history must
431+
# match what the user actually saw.
432+
assert result.raw["text"] == "hello", (
433+
"Partial RawMessage on send failure must contain only "
434+
"successfully-sent text. Found 'world' (rejected by Teams) "
435+
"in the result, indicating accumulated was updated before "
436+
"the send confirmed."
437+
)
438+
# session.sequence stays at 1 (first send incremented it; second
439+
# didn't because it failed before commit).
440+
assert session.sequence == 1
422441

423442
@pytest.mark.asyncio
424443
async def test_cancelled_error_propagates_and_marks_session_canceled(self):

0 commit comments

Comments
 (0)