Skip to content

Commit 241755e

Browse files
Python SDK: refactor types to idiomatic snake_case dataclasses
- ToolResult, ToolInvocation, ToolBinaryResult: TypedDict(camelCase) -> dataclass(snake_case) - PermissionRequest: remove handwritten TypedDict, re-export codegen'd dataclass - PermissionRequestResult: TypedDict -> dataclass with snake_case fields - Update all tests to use attribute access instead of dict access - Remove stale TestHandleToolCallRequest (tested removed v2 RPC method) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 193595a commit 241755e

9 files changed

Lines changed: 188 additions & 225 deletions

File tree

python/copilot/session.py

Lines changed: 21 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -317,47 +317,34 @@ async def _execute_tool_and_respond(
317317
) -> None:
318318
"""Execute a tool handler and send the result back via HandlePendingToolCall RPC."""
319319
try:
320-
invocation: ToolInvocation = {
321-
"session_id": self.session_id,
322-
"tool_call_id": tool_call_id,
323-
"tool_name": tool_name,
324-
"arguments": arguments,
325-
}
320+
invocation = ToolInvocation(
321+
session_id=self.session_id,
322+
tool_call_id=tool_call_id,
323+
tool_name=tool_name,
324+
arguments=arguments,
325+
)
326326

327327
result = handler(invocation)
328328
if inspect.isawaitable(result):
329329
result = await result
330330

331331
if result is None:
332332
result = ToolResult(
333-
textResultForLlm="Tool returned no result.",
334-
resultType="failure",
333+
text_result_for_llm="Tool returned no result.",
334+
result_type="failure",
335335
error="tool returned no result",
336-
toolTelemetry={},
336+
tool_telemetry={},
337337
)
338338

339-
# Build a structured ResultResult from the ToolResult
340-
text_result = ""
341-
error_str = None
342-
result_type = None
343-
tool_telemetry = None
344-
345-
if isinstance(result, dict):
346-
text_result = result.get("textResultForLlm", str(result))
347-
error_str = result.get("error")
348-
result_type = result.get("resultType")
349-
tool_telemetry = result.get("toolTelemetry")
350-
else:
351-
text_result = str(result)
352339

353340
await self.rpc.tools.handle_pending_tool_call(
354341
SessionToolsHandlePendingToolCallParams(
355342
request_id=request_id,
356343
result=ResultResult(
357-
text_result_for_llm=text_result,
358-
error=error_str,
359-
result_type=result_type,
360-
tool_telemetry=tool_telemetry,
344+
text_result_for_llm=result.text_result_for_llm,
345+
error=result.error,
346+
result_type=result.result_type,
347+
tool_telemetry=result.tool_telemetry,
361348
),
362349
)
363350
)
@@ -380,36 +367,18 @@ async def _execute_permission_and_respond(
380367
) -> None:
381368
"""Execute a permission handler and respond via RPC."""
382369
try:
383-
# Convert from generated PermissionRequest to dict for the handler
384-
if hasattr(permission_request, "to_dict"):
385-
perm_dict = permission_request.to_dict()
386-
elif isinstance(permission_request, dict):
387-
perm_dict = permission_request
388-
else:
389-
perm_dict = permission_request
390-
391-
result = handler(perm_dict, {"session_id": self.session_id})
370+
result = handler(permission_request, {"session_id": self.session_id})
392371
if inspect.isawaitable(result):
393372
result = await result
394373

395374
result = cast(PermissionRequestResult, result)
396-
default_kind = "denied-no-approval-rule-and-could-not-request-from-user"
397-
if isinstance(result, dict):
398-
kind_str = result.get("kind", default_kind)
399-
else:
400-
kind_str = getattr(result, "kind", default_kind)
401-
402-
def _get(key: str) -> Any:
403-
if isinstance(result, dict):
404-
return result.get(key)
405-
return getattr(result, key, None)
406375

407376
perm_result = SessionPermissionsHandlePendingPermissionRequestParamsResult(
408-
kind=Kind(kind_str),
409-
rules=_get("rules"),
410-
feedback=_get("feedback"),
411-
message=_get("message"),
412-
path=_get("path"),
377+
kind=Kind(result.kind),
378+
rules=result.rules,
379+
feedback=result.feedback,
380+
message=result.message,
381+
path=result.path,
413382
)
414383

415384
await self.rpc.permissions.handle_pending_permission_request(
@@ -509,7 +478,7 @@ async def _handle_permission_request(
509478

510479
if not handler:
511480
# No handler registered, deny permission
512-
return {"kind": "denied-no-approval-rule-and-could-not-request-from-user"}
481+
return PermissionRequestResult()
513482

514483
try:
515484
result = handler(request, {"session_id": self.session_id})
@@ -518,7 +487,7 @@ async def _handle_permission_request(
518487
return cast(PermissionRequestResult, result)
519488
except Exception: # pylint: disable=broad-except
520489
# Handler failed, deny permission
521-
return {"kind": "denied-no-approval-rule-and-could-not-request-from-user"}
490+
return PermissionRequestResult()
522491

523492
def _register_user_input_handler(self, handler: UserInputHandler | None) -> None:
524493
"""

python/copilot/tools.py

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ async def wrapped_handler(invocation: ToolInvocation) -> ToolResult:
122122
# Build args based on detected signature
123123
call_args = []
124124
if takes_params:
125-
args = invocation["arguments"] or {}
125+
args = invocation.arguments or {}
126126
if ptype is not None and _is_pydantic_model(ptype):
127127
call_args.append(ptype.model_validate(args))
128128
else:
@@ -141,11 +141,11 @@ async def wrapped_handler(invocation: ToolInvocation) -> ToolResult:
141141
# Don't expose detailed error information to the LLM for security reasons.
142142
# The actual error is stored in the 'error' field for debugging.
143143
return ToolResult(
144-
textResultForLlm="Invoking this tool produced an error. "
144+
text_result_for_llm="Invoking this tool produced an error. "
145145
"Detailed information is not available.",
146-
resultType="failure",
146+
result_type="failure",
147147
error=str(exc),
148-
toolTelemetry={},
148+
tool_telemetry={},
149149
)
150150

151151
return Tool(
@@ -185,19 +185,19 @@ def _normalize_result(result: Any) -> ToolResult:
185185
"""
186186
if result is None:
187187
return ToolResult(
188-
textResultForLlm="",
189-
resultType="success",
188+
text_result_for_llm="",
189+
result_type="success",
190190
)
191191

192-
# ToolResult passes through directly
193-
if isinstance(result, dict) and "resultType" in result and "textResultForLlm" in result:
192+
# ToolResult dataclass passes through directly
193+
if isinstance(result, ToolResult):
194194
return result
195195

196196
# Strings pass through directly
197197
if isinstance(result, str):
198198
return ToolResult(
199-
textResultForLlm=result,
200-
resultType="success",
199+
text_result_for_llm=result,
200+
result_type="success",
201201
)
202202

203203
# Everything else gets JSON-serialized (with Pydantic model support)
@@ -212,6 +212,6 @@ def default(obj: Any) -> Any:
212212
raise TypeError(f"Failed to serialize tool result: {exc}") from exc
213213

214214
return ToolResult(
215-
textResultForLlm=json_str,
216-
resultType="success",
215+
text_result_for_llm=json_str,
216+
result_type="success",
217217
)

python/copilot/types.py

Lines changed: 45 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@
99
from typing import Any, Literal, NotRequired, TypedDict
1010

1111
# Import generated SessionEvent types
12-
from .generated.session_events import SessionEvent
12+
from .generated.session_events import (
13+
PermissionRequest,
14+
SessionEvent,
15+
)
1316

1417
# SessionEvent is now imported from generated types
1518
# It provides proper type discrimination for all event types
@@ -100,29 +103,36 @@ class CopilotClientOptions(TypedDict, total=False):
100103
ToolResultType = Literal["success", "failure", "rejected", "denied"]
101104

102105

103-
class ToolBinaryResult(TypedDict, total=False):
104-
data: str
105-
mimeType: str
106-
type: str
107-
description: str
106+
@dataclass
107+
class ToolBinaryResult:
108+
"""Binary content returned by a tool."""
109+
110+
data: str = ""
111+
mime_type: str = ""
112+
type: str = ""
113+
description: str = ""
108114

109115

110-
class ToolResult(TypedDict, total=False):
116+
@dataclass
117+
class ToolResult:
111118
"""Result of a tool invocation."""
112119

113-
textResultForLlm: str
114-
binaryResultsForLlm: list[ToolBinaryResult]
115-
resultType: ToolResultType
116-
error: str
117-
sessionLog: str
118-
toolTelemetry: dict[str, Any]
120+
text_result_for_llm: str = ""
121+
result_type: ToolResultType = "success"
122+
error: str | None = None
123+
binary_results_for_llm: list[ToolBinaryResult] | None = None
124+
session_log: str | None = None
125+
tool_telemetry: dict[str, Any] | None = None
119126

120127

121-
class ToolInvocation(TypedDict):
122-
session_id: str
123-
tool_call_id: str
124-
tool_name: str
125-
arguments: Any
128+
@dataclass
129+
class ToolInvocation:
130+
"""Context passed to a tool handler when invoked."""
131+
132+
session_id: str = ""
133+
tool_call_id: str = ""
134+
tool_name: str = ""
135+
arguments: Any = None
126136

127137

128138
ToolHandler = Callable[[ToolInvocation], ToolResult | Awaitable[ToolResult]]
@@ -164,25 +174,26 @@ class SystemMessageReplaceConfig(TypedDict):
164174
SystemMessageConfig = SystemMessageAppendConfig | SystemMessageReplaceConfig
165175

166176

167-
# Permission request types
168-
class PermissionRequest(TypedDict, total=False):
169-
"""Permission request from the server"""
170-
171-
kind: Literal["shell", "write", "mcp", "read", "url", "custom-tool"]
172-
toolCallId: str
173-
# Additional fields vary by kind
177+
# Permission result types
174178

179+
PermissionRequestResultKind = Literal[
180+
"approved",
181+
"denied-by-rules",
182+
"denied-by-content-exclusion-policy",
183+
"denied-no-approval-rule-and-could-not-request-from-user",
184+
"denied-interactively-by-user",
185+
]
175186

176-
class PermissionRequestResult(TypedDict, total=False):
177-
"""Result of a permission request"""
178187

179-
kind: Literal[
180-
"approved",
181-
"denied-by-rules",
182-
"denied-no-approval-rule-and-could-not-request-from-user",
183-
"denied-interactively-by-user",
184-
]
185-
rules: list[Any]
188+
@dataclass
189+
class PermissionRequestResult:
190+
"""Result of a permission request."""
191+
192+
kind: PermissionRequestResultKind = "denied-no-approval-rule-and-could-not-request-from-user"
193+
rules: list[Any] | None = None
194+
feedback: str | None = None
195+
message: str | None = None
196+
path: str | None = None
186197

187198

188199
_PermissionHandlerFn = Callable[

python/e2e/test_multi_client.py

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,13 @@
1313
import pytest_asyncio
1414
from pydantic import BaseModel, Field
1515

16-
from copilot import CopilotClient, PermissionHandler, ToolInvocation, define_tool
16+
from copilot import (
17+
CopilotClient,
18+
PermissionHandler,
19+
PermissionRequestResult,
20+
ToolInvocation,
21+
define_tool,
22+
)
1723

1824
from .testharness import get_final_assistant_message
1925
from .testharness.proxy import CapiProxy
@@ -232,7 +238,7 @@ async def test_one_client_approves_permission(self, mctx: MultiClientContext):
232238
session1 = await mctx.client1.create_session(
233239
{
234240
"on_permission_request": lambda request, invocation: (
235-
permission_requests.append(request) or {"kind": "approved"}
241+
permission_requests.append(request) or PermissionRequestResult(kind="approved")
236242
),
237243
}
238244
)
@@ -279,9 +285,9 @@ async def test_one_client_rejects_permission(self, mctx: MultiClientContext):
279285
# Client 1 creates a session and denies all permission requests
280286
session1 = await mctx.client1.create_session(
281287
{
282-
"on_permission_request": lambda request, invocation: {
283-
"kind": "denied-interactively-by-user"
284-
},
288+
"on_permission_request": lambda request, invocation: (
289+
PermissionRequestResult(kind="denied-interactively-by-user")
290+
),
285291
}
286292
)
287293

0 commit comments

Comments
 (0)