Strongly type expAssignments session config across all SDKs#2033
Strongly type expAssignments session config across all SDKs#2033devm33 wants to merge 5 commits into
Conversation
Replace the opaque JSON typing of the internal `expAssignments` session-config field with a strongly-typed `CopilotExpAssignmentResponse` (plus `ExpConfigEntry`) in every SDK, mirroring the runtime contract. Wire keys remain PascalCase (Features, Flights, Configs, Id, Parameters, ...), optional fields are omitted when null, and the field keeps its internal/hidden posture in each language. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 43d7a02b-08ff-4e7d-a8c0-afa6dfbe94b0
There was a problem hiding this comment.
Pull request overview
Strongly types ExP assignment session configuration across all six SDKs while preserving PascalCase wire serialization.
Changes:
- Adds shared ExP response, configuration-entry, and flag-value types.
- Updates create/resume APIs and wire requests to use the new types.
- Revises serialization, clone, unit, and E2E tests.
Show a summary per file
| File | Description |
|---|---|
rust/tests/e2e/client_options.rs |
Updates ExP forwarding tests. |
rust/src/wire.rs |
Types create/resume wire fields. |
rust/src/types.rs |
Adds ExP types, builders, and tests. |
python/test_client.py |
Tests typed ExP serialization. |
python/copilot/client.py |
Adds dataclasses and wire conversion. |
python/copilot/__init__.py |
Exports ExP types. |
nodejs/test/client.test.ts |
Updates typed payload fixture. |
nodejs/src/types.ts |
Defines ExP TypeScript types. |
nodejs/src/index.ts |
Exports ExP types. |
java/src/test/java/com/github/copilot/SessionRequestBuilderTest.java |
Updates request and clone tests. |
java/src/main/java/com/github/copilot/rpc/SessionConfig.java |
Types create configuration. |
java/src/main/java/com/github/copilot/rpc/ResumeSessionRequest.java |
Types resume wire request. |
java/src/main/java/com/github/copilot/rpc/ResumeSessionConfig.java |
Types resume configuration. |
java/src/main/java/com/github/copilot/rpc/ExpConfigEntry.java |
Adds ExP configuration entries. |
java/src/main/java/com/github/copilot/rpc/CreateSessionRequest.java |
Types create wire request. |
java/src/main/java/com/github/copilot/rpc/CopilotExpAssignmentResponse.java |
Adds ExP response model. |
go/types.go |
Adds ExP types and request fields. |
go/internal/e2e/client_options_e2e_test.go |
Updates forwarding scenarios. |
go/client_test.go |
Updates request serialization tests. |
dotnet/test/Unit/SerializationTests.cs |
Updates serialization and clone tests. |
dotnet/src/Types.cs |
Adds ExP models and serializer metadata. |
dotnet/src/Client.cs |
Types create/resume request records. |
Review details
- Files reviewed: 22/22 changed files
- Comments generated: 6
- Review effort level: Medium
This comment has been minimized.
This comment has been minimized.
- Go: add MarshalJSON to CopilotExpAssignmentResponse/ExpConfigEntry so
nil Features/Flights/Configs/Parameters serialize as []/{} instead of
null, matching the Python/Rust/.NET reference behavior; add a test.
- Java: default AssignmentContext to "" so the required field is not
dropped by NON_NULL when unset.
- .NET: tighten ExpConfigEntry.Parameters value type from JsonNode? to
JsonValue? to constrain values to JSON scalars.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 43d7a02b-08ff-4e7d-a8c0-afa6dfbe94b0
This comment has been minimized.
This comment has been minimized.
The Id field is required by the wire contract, but defaulting to null let class-level NON_NULL drop the key for a zero-value entry. Default it to "" so the required key is always emitted, matching the Go and .NET defaults. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 43d7a02b-08ff-4e7d-a8c0-afa6dfbe94b0
There was a problem hiding this comment.
Review details
Comments suppressed due to low confidence (1)
python/copilot/client.py:3018
- The adjacent comment still describes this as “opaque JSON,” but this path now accepts and serializes a concrete
CopilotExpAssignmentResponse. Update the comment so it does not contradict the new API.
payload["expAssignments"] = _exp_assignment_response_to_dict(exp_assignments)
- Files reviewed: 22/22 changed files
- Comments generated: 1
- Review effort level: Medium
The expAssignments path now serializes a concrete CopilotExpAssignmentResponse, so drop the outdated "opaque JSON" wording from the adjacent comments. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 43d7a02b-08ff-4e7d-a8c0-afa6dfbe94b0
Move `use std::collections::HashMap;` into the std import block so `cargo fmt --check` with the nightly group_imports config passes in CI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 43d7a02b-08ff-4e7d-a8c0-afa6dfbe94b0
|
Vijay Rathor |
Cross-SDK Consistency ReviewThe PR successfully adds strongly-typed One minor inconsistency found: Python
|
Summary
Strongly types the internal
expAssignmentssession-config field across all six SDKs. Previously each SDK typed it as opaque JSON (Record<string, unknown>/any/dict/JsonElement/JsonNode/serde_json::Value). This replaces those with a concreteCopilotExpAssignmentResponsetype (plusExpConfigEntry) that mirrors the runtime contract fromcopilot-agent-runtime— the same shape the Copilot CLI fetches from the experimentation service.This closes a gap from #1750, which forwarded
expAssignmentsbut left it loosely typed even though the runtime's in-process type (sessionOptions.ts) is the concreteCopilotExpAssignmentResponse.Type shape
Wire JSON keys stay PascalCase (
Features,Flights,Configs,Id,Parameters, ...); optional fields are omitted when null; and the field keeps its@internal/hidden posture (not part of the broadly advertised public surface) in every language.Changes per SDK
Each SDK updates the type, the create/resume config field, the wire path, clone/copy logic, and tests.
nodejs/src/types.ts,index.tsgo/types.gopython/copilot/client.py,__init__.pyrust/src/types.rs,wire.rs-D warnings+ testdotnet/src/Types.cs,Client.csCopilotExpAssignmentResponse.java,ExpConfigEntry.java+rpc/*Testing
tsc --noEmitclean; vitest passinggo build/vet/gofmt/testpassingruff+pytestpassingcargo fmt --check,clippy --all-features --all-targets -- -D warnings,cargo testpassingdotnet buildclean; 46 serialization tests passingmvn spotless:check+ checkstyle clean; 85SessionRequestBuilderTesttests passing (JDK 25)