Skip to content

Commit 5040547

Browse files
authored
Merge pull request #92 from microsoft/fix/opaque-reasoning-provenance-local
fix: namespace Copilot opaque reasoning provenance
2 parents d9f3505 + 383c726 commit 5040547

5 files changed

Lines changed: 212 additions & 7 deletions

File tree

‎amplifier_module_provider_github_copilot/sdk_adapter/extract.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,8 @@ def extract_event_fields(sdk_event: Any) -> dict[str, Any]:
8383

8484
# reasoning_opaque for extended thinking signature preservation
8585
# Contract: streaming-contract:ThinkingBlock:MUST:1
86-
# Anthropic models send encrypted extended thinking data that must be
87-
# returned verbatim in subsequent turns for multi-turn extended thinking.
86+
# Preserve Copilot's opaque bytes verbatim; the accumulator tags the
87+
# persisted block as Copilot state, not a native upstream signature.
8888
reasoning_opaque = getattr(sdk_data, "reasoning_opaque", None)
8989
if reasoning_opaque and "reasoning_opaque" not in event_dict:
9090
event_dict["reasoning_opaque"] = reasoning_opaque

‎amplifier_module_provider_github_copilot/streaming.py‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -325,14 +325,20 @@ def to_chat_response(self) -> "StreamingChatResponse":
325325
content_blocks.append(TextContent(text=block_text))
326326
elif btype == "thinking":
327327
thinking_text: str = block["text"]
328-
if thinking_text:
328+
signature = block.get("opaque") or self.reasoning_opaque
329+
# Only block-local opaque state can retain an empty block.
330+
if thinking_text or block.get("opaque"):
329331
# Contract: streaming-contract:ThinkingBlock:MUST:1
332+
# Copilot opaque state is not a native Anthropic/Gemini signature.
333+
# Keep its producer namespace through context persistence.
330334
content.append(
331335
ThinkingBlock(
332336
thinking=thinking_text,
333-
signature=block.get("opaque") or self.reasoning_opaque,
337+
signature=signature,
338+
thinking_provenance={"version": 1, "provider": PROVIDER_ID},
334339
)
335340
)
341+
if thinking_text:
336342
content_blocks.append(ThinkingContent(text=thinking_text))
337343
elif btype == "tool_call":
338344
tc = block["data"]

‎contracts/streaming-contract.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,9 @@ class ThinkingBlock(BaseModel):
6363
signature: str | None = None # Opaque extended thinking signature
6464
```
6565

66-
**streaming-contract:ThinkingBlock:MUST:1**: Provider MUST preserve `reasoning_opaque` from SDK events as `ThinkingBlock.signature`.
66+
**streaming-contract:ThinkingBlock:MUST:1**: Provider MUST preserve `reasoning_opaque` from SDK events as `ThinkingBlock.signature`, including when visible thinking is empty. Persisted thinking blocks MUST carry the per-block extra `thinking_provenance: {"version": 1, "provider": "github-copilot"}`. Opaque state MUST NOT be rendered as text or streaming UI content.
6767

68-
**Rationale:** Anthropic models send encrypted extended thinking data in `reasoning_opaque` which must be returned verbatim in subsequent turns for multi-turn extended thinking to work. The kernel's `ThinkingBlock.signature` field maps directly to the SDK's `reasoning_opaque`. GPT models do not use this field.
68+
**Rationale:** The SDK's `reasoning_opaque` is Copilot-owned state, not a native Anthropic or Gemini signature, even when Copilot hosts one of those models. The producer namespace lets native consumers distinguish foreign state after serialization or a provider switch; it does not bind state to a selected account or model. Copilot itself creates a fresh SDK session per request and renders only visible historical thinking through its text history adapter. Preserving and tagging opaque state does not change that SDK request behavior.
6969

7070
### ToolCallBlock (Pydantic) — for ChatResponse.content
7171
```python

‎tests/test_contract_streaming.py‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,22 @@
99
from __future__ import annotations
1010

1111
import logging
12+
from types import SimpleNamespace
1213
from typing import Any
1314

15+
import pytest
16+
from amplifier_core import ChatRequest, ChatResponse, ThinkingBlock, ThinkingContent
17+
from amplifier_core.message_models import Message
18+
from pydantic import BaseModel
19+
20+
from amplifier_module_provider_github_copilot.request_adapter import convert_chat_request
21+
from amplifier_module_provider_github_copilot.sdk_adapter import extract_event_fields
1422
from amplifier_module_provider_github_copilot.streaming import (
1523
DomainEvent,
1624
DomainEventType,
1725
StreamingAccumulator,
26+
load_event_config,
27+
translate_event,
1828
)
1929

2030

@@ -364,6 +374,134 @@ def test_thinking_block_handles_no_signature(self) -> None:
364374
# signature should be None (not missing, not error)
365375
assert thinking_block.signature is None
366376

377+
@pytest.mark.parametrize("empty_first", [False, True], ids=["empty-after", "empty-before"])
378+
@pytest.mark.parametrize("thinking", ["Visible reasoning", ""], ids=["visible", "opaque-only"])
379+
def test_empty_unsigned_block_does_not_inherit_retention_from_separate_block(
380+
self, empty_first: bool, thinking: str
381+
) -> None:
382+
"""Only locally captured opaque state can retain an empty thinking block."""
383+
unsigned = DomainEvent(
384+
type=DomainEventType.CONTENT_DELTA, data={"text": ""}, block_type="THINKING"
385+
)
386+
signed = DomainEvent(
387+
type=DomainEventType.CONTENT_DELTA,
388+
data={"text": thinking, "reasoning_opaque": "copilot-opaque-fixture"},
389+
block_type="THINKING",
390+
)
391+
separator = DomainEvent(type=DomainEventType.CONTENT_DELTA, data={"text": "Answer"})
392+
accumulator = StreamingAccumulator()
393+
events = [unsigned, separator, signed] if empty_first else [signed, separator, unsigned]
394+
for event in events:
395+
accumulator.add(event)
396+
397+
response = accumulator.to_chat_response()
398+
assert [block.type for block in response.content] == (
399+
["text", "thinking"] if empty_first else ["thinking", "text"]
400+
)
401+
blocks = [block for block in response.content if isinstance(block, ThinkingBlock)]
402+
assert len(blocks) == 1
403+
assert blocks[0].thinking == thinking
404+
assert blocks[0].signature == "copilot-opaque-fixture"
405+
assert response.content_blocks is not None
406+
ui_thinking = [
407+
block for block in response.content_blocks if isinstance(block, ThinkingContent)
408+
]
409+
assert [block.text for block in ui_thinking] == ([thinking] if thinking else [])
410+
411+
@pytest.mark.parametrize("object_event", [False, True], ids=["dict", "sdk-object"])
412+
@pytest.mark.parametrize("split", [False, True], ids=["single", "split"])
413+
@pytest.mark.parametrize(
414+
("thinking", "opaque"),
415+
[
416+
("Visible reasoning", "copilot-opaque-fixture"),
417+
("", "copilot-opaque-fixture"),
418+
("Visible reasoning", None),
419+
("", None),
420+
],
421+
ids=["signed-visible", "opaque-only", "unsigned-visible", "empty"],
422+
)
423+
def test_provenance_survives_capture_and_context_round_trip(
424+
self,
425+
object_event: bool,
426+
split: bool,
427+
thinking: str,
428+
opaque: str | None,
429+
caplog: pytest.LogCaptureFixture,
430+
) -> None:
431+
"""Tag producer state without rendering opaque bytes or changing SDK history."""
432+
caplog.set_level(logging.DEBUG)
433+
accumulator = StreamingAccumulator()
434+
# A signature-only trailing delta must consolidate with visible reasoning.
435+
chunks = [(thinking, None), ("", opaque)] if split else [(thinking, opaque)]
436+
config = load_event_config()
437+
for text, signature in chunks:
438+
if object_event:
439+
sdk_event = SimpleNamespace(
440+
type="assistant.reasoning_delta",
441+
data=SimpleNamespace(delta_content=text, reasoning_opaque=signature),
442+
)
443+
event = extract_event_fields(sdk_event)
444+
else:
445+
event = {
446+
"type": "assistant.reasoning_delta",
447+
"data": {"text": text, "reasoning_opaque": signature},
448+
}
449+
translated = translate_event(event, config)
450+
assert translated is not None
451+
accumulator.add(translated)
452+
453+
response = accumulator.to_chat_response()
454+
blocks = [block for block in response.content if isinstance(block, ThinkingBlock)]
455+
assert len(blocks) == int(bool(thinking or opaque))
456+
if blocks:
457+
assert blocks[0].thinking == thinking
458+
assert blocks[0].signature == opaque
459+
assert isinstance(blocks[0], BaseModel)
460+
assert blocks[0].model_dump()["thinking_provenance"] == {
461+
"version": 1,
462+
"provider": "github-copilot",
463+
}
464+
assert response.text is None
465+
if thinking:
466+
assert response.content_blocks is not None
467+
assert len(response.content_blocks) == 1
468+
ui_block = response.content_blocks[0]
469+
assert isinstance(ui_block, ThinkingContent)
470+
assert ui_block.text == thinking
471+
else:
472+
assert response.content_blocks is None # Opaque-only state is persistence-only.
473+
assert "copilot-opaque-fixture" not in caplog.text
474+
475+
# The loop persists response content via model_dump, then rebuilds Messages
476+
# and ChatRequests. Exercise both Python and JSON reloads without Core edits.
477+
# Local Core stubs omit Pydantic APIs; retain real runtime validation here.
478+
response_model: Any = ChatResponse
479+
request_model: Any = ChatRequest
480+
assert isinstance(response, BaseModel)
481+
restored = response_model.model_validate(response.model_dump())
482+
restored = response_model.model_validate_json(restored.model_dump_json())
483+
assistant = Message(role="assistant", content=restored.content)
484+
assert isinstance(assistant, BaseModel)
485+
request = request_model(
486+
messages=[assistant, Message(role="user", content="Continue")],
487+
)
488+
for _ in range(2):
489+
request = request_model.model_validate(request.model_dump())
490+
request = request_model.model_validate_json(request.model_dump_json())
491+
assert request.messages[0].model_dump()["content"] == assistant.model_dump()["content"]
492+
493+
# Copilot history deliberately serializes visible thinking only. Stripping
494+
# the new provenance must produce the exact same SDK prompt.
495+
untagged = request.model_dump()
496+
for block in untagged["messages"][0]["content"]:
497+
block.pop("thinking_provenance", None)
498+
prompt = convert_chat_request(request).prompt
499+
assert prompt == convert_chat_request(request_model.model_validate(untagged)).prompt
500+
assert "copilot-opaque-fixture" not in prompt
501+
assert "thinking_provenance" not in prompt
502+
if thinking:
503+
assert f"[Thinking: {thinking}]" in prompt
504+
367505

368506
# ---------------------------------------------------------------------------
369507
# T-4 Contract Test Gaps — EventRouter integration tests

‎tests/test_provider_coverage.py‎

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@
2222

2323
from __future__ import annotations
2424

25+
import logging
26+
from dataclasses import dataclass
2527
from typing import TYPE_CHECKING, Any
26-
from unittest.mock import MagicMock, patch
28+
from unittest.mock import AsyncMock, MagicMock, patch
2729

2830
import pytest
2931

@@ -98,6 +100,65 @@ def _create_mock_request(model: str = "gpt-4o") -> MagicMock:
98100
# =============================================================================
99101

100102

103+
@pytest.mark.parametrize("use_streaming", [False, True], ids=["complete", "streaming"])
104+
@pytest.mark.parametrize("thinking", ["", "Visible reasoning"], ids=["opaque-only", "visible"])
105+
async def test_complete_tags_copilot_opaque_state(
106+
use_streaming: bool,
107+
thinking: str,
108+
caplog: pytest.LogCaptureFixture,
109+
) -> None:
110+
"""Both complete() modes persist producer provenance without leaking opaque state."""
111+
from amplifier_core import ChatRequest, ThinkingBlock
112+
from amplifier_core.message_models import Message
113+
from pydantic import BaseModel
114+
115+
from amplifier_module_provider_github_copilot.provider import GitHubCopilotProvider
116+
117+
@dataclass
118+
class ReasoningData(SessionEventData):
119+
reasoning_opaque: str | None = None
120+
121+
client = MockCopilotClientWrapper(
122+
events=[
123+
SessionEvent(
124+
type=SessionEventType.ASSISTANT_REASONING_DELTA,
125+
data=ReasoningData(delta_content=thinking, reasoning_opaque="opaque-fixture"),
126+
),
127+
text_delta_event("Answer"),
128+
],
129+
)
130+
coordinator = MagicMock()
131+
coordinator.hooks.emit = AsyncMock()
132+
provider = GitHubCopilotProvider(
133+
client=client, # type: ignore[arg-type]
134+
coordinator=coordinator,
135+
config={"use_streaming": use_streaming},
136+
)
137+
caplog.set_level(logging.DEBUG)
138+
try:
139+
response = await provider.complete(
140+
ChatRequest(model="gpt-4o", messages=[Message(role="user", content="Test")]),
141+
)
142+
block = response.content[0]
143+
assert isinstance(block, ThinkingBlock)
144+
assert block.thinking == thinking
145+
assert block.signature == "opaque-fixture"
146+
assert isinstance(block, BaseModel)
147+
assert block.model_dump()["thinking_provenance"] == {
148+
"version": 1,
149+
"provider": "github-copilot",
150+
}
151+
assert response.content[1].type == "text"
152+
assert "opaque-fixture" not in caplog.text
153+
assert "opaque-fixture" not in str(coordinator.hooks.emit.call_args_list)
154+
emitted_names = [call.args[0] for call in coordinator.hooks.emit.call_args_list]
155+
assert ("llm:stream_block_delta" in emitted_names) is use_streaming
156+
assert client.session_instance is not None
157+
assert client.session_instance.disconnected
158+
finally:
159+
await provider.close()
160+
161+
101162
class TestRetryWithEventualSuccess:
102163
"""Test retry paths that succeed after initial failures."""
103164

0 commit comments

Comments
 (0)