Skip to content

Commit bfbaa23

Browse files
devm33Copilot
andauthored
Strongly type expAssignments session config across all SDKs (#2033)
* Strongly type expAssignments session config across all SDKs 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 * Address review: normalize required exp-assignment fields - 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 * Default ExpConfigEntry.Id to "" in Java 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 * Fix stale "opaque JSON" comment in Python exp wiring 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 * Fix nightly rustfmt import grouping in types.rs tests 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 --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 6822afb commit bfbaa23

22 files changed

Lines changed: 796 additions & 110 deletions

dotnet/src/Client.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2758,7 +2758,7 @@ internal record CreateSessionRequest(
27582758
IList<NamedProviderConfig>? Providers = null,
27592759
IList<ProviderModelConfig>? Models = null,
27602760
OptionsUpdateToolFilterPrecedence? ToolFilterPrecedence = null,
2761-
[property: JsonPropertyName("expAssignments")] JsonElement? ExpAssignments = null,
2761+
[property: JsonPropertyName("expAssignments")] CopilotExpAssignmentResponse? ExpAssignments = null,
27622762
[property: JsonPropertyName("enableManagedSettings")] bool? EnableManagedSettings = null,
27632763
bool? EnableGitHubTelemetryForwarding = null);
27642764
#pragma warning restore GHCP001
@@ -2864,7 +2864,7 @@ internal record ResumeSessionRequest(
28642864
IList<NamedProviderConfig>? Providers = null,
28652865
IList<ProviderModelConfig>? Models = null,
28662866
OptionsUpdateToolFilterPrecedence? ToolFilterPrecedence = null,
2867-
[property: JsonPropertyName("expAssignments")] JsonElement? ExpAssignments = null,
2867+
[property: JsonPropertyName("expAssignments")] CopilotExpAssignmentResponse? ExpAssignments = null,
28682868
[property: JsonPropertyName("enableManagedSettings")] bool? EnableManagedSettings = null,
28692869
bool? EnableGitHubTelemetryForwarding = null);
28702870
#pragma warning restore GHCP001

dotnet/src/Types.cs

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2833,6 +2833,64 @@ public struct SetModelOptions
28332833
public ModelCapabilitiesOverride? ModelCapabilities { get; set; }
28342834
}
28352835

2836+
/// <summary>
2837+
/// A single configuration entry in a <see cref="CopilotExpAssignmentResponse"/>.
2838+
/// Each entry carries an identifier and a bag of typed parameter values.
2839+
/// </summary>
2840+
public sealed class ExpConfigEntry
2841+
{
2842+
/// <summary>Identifier of the configuration entry.</summary>
2843+
[JsonPropertyName("Id")]
2844+
public string Id { get; set; } = string.Empty;
2845+
2846+
/// <summary>
2847+
/// Parameter values keyed by parameter name. Each value is a scalar string,
2848+
/// number, boolean, or <c>null</c>.
2849+
/// </summary>
2850+
[JsonPropertyName("Parameters")]
2851+
public IDictionary<string, JsonValue?> Parameters { get; set; } = new Dictionary<string, JsonValue?>();
2852+
}
2853+
2854+
/// <summary>
2855+
/// ExP ("flight") assignment data, in the same JSON shape the Copilot CLI
2856+
/// fetches from the experimentation service. Property names serialize as
2857+
/// PascalCase (<c>Features</c>, <c>Flights</c>, ...) to match the on-the-wire
2858+
/// contract consumed by the runtime.
2859+
/// </summary>
2860+
public sealed class CopilotExpAssignmentResponse
2861+
{
2862+
/// <summary>Enabled feature names.</summary>
2863+
[JsonPropertyName("Features")]
2864+
public IList<string> Features { get; set; } = new List<string>();
2865+
2866+
/// <summary>Assigned flights keyed by flight name.</summary>
2867+
[JsonPropertyName("Flights")]
2868+
public IDictionary<string, string> Flights { get; set; } = new Dictionary<string, string>();
2869+
2870+
/// <summary>Configuration entries carrying typed parameter values.</summary>
2871+
[JsonPropertyName("Configs")]
2872+
public IList<ExpConfigEntry> Configs { get; set; } = new List<ExpConfigEntry>();
2873+
2874+
/// <summary>Opaque parameter-group payload passed through untouched. Optional.</summary>
2875+
[JsonPropertyName("ParameterGroups")]
2876+
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
2877+
public JsonNode? ParameterGroups { get; set; }
2878+
2879+
/// <summary>Version of the flighting configuration. Optional.</summary>
2880+
[JsonPropertyName("FlightingVersion")]
2881+
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
2882+
public int? FlightingVersion { get; set; }
2883+
2884+
/// <summary>Impression identifier for the assignment. Optional.</summary>
2885+
[JsonPropertyName("ImpressionId")]
2886+
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
2887+
public string? ImpressionId { get; set; }
2888+
2889+
/// <summary>Assignment context string forwarded to CAPI and telemetry.</summary>
2890+
[JsonPropertyName("AssignmentContext")]
2891+
public string AssignmentContext { get; set; } = string.Empty;
2892+
}
2893+
28362894
/// <summary>
28372895
/// Shared configuration properties for creating or resuming a Copilot session.
28382896
/// Use <see cref="SessionConfig"/> when creating a new session, or
@@ -3339,7 +3397,7 @@ protected SessionConfigBase(SessionConfigBase? other)
33393397
/// completion. It is not part of the broadly advertised public surface.
33403398
/// </remarks>
33413399
[EditorBrowsable(EditorBrowsableState.Never)]
3342-
public JsonElement? ExpAssignments { get; set; }
3400+
public CopilotExpAssignmentResponse? ExpAssignments { get; set; }
33433401

33443402
/// <summary>
33453403
/// Opt-in: when <c>true</c>, the runtime self-fetches enterprise managed
@@ -4041,6 +4099,8 @@ public sealed class SystemMessageTransformRpcResponse
40414099
[JsonSerializable(typeof(AutoModeSwitchRequest))]
40424100
[JsonSerializable(typeof(AutoModeSwitchResponse))]
40434101
[JsonSerializable(typeof(CustomAgentConfig))]
4102+
[JsonSerializable(typeof(CopilotExpAssignmentResponse))]
4103+
[JsonSerializable(typeof(ExpConfigEntry))]
40444104
[JsonSerializable(typeof(ExitPlanModeRequest))]
40454105
[JsonSerializable(typeof(ExitPlanModeResult))]
40464106
[JsonSerializable(typeof(GetAuthStatusResponse))]

dotnet/test/Unit/SerializationTests.cs

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
*--------------------------------------------------------------------------------------------*/
44

55
using Xunit;
6+
using System.Collections.Generic;
67
using System.Text.Json;
78
#if !NET8_0_OR_GREATER
89
using System.Runtime.Serialization;
@@ -488,24 +489,28 @@ public void SessionRequests_CanSerializeExpAssignments_WithSdkOptions()
488489
{
489490
var options = GetSerializerOptions();
490491

491-
using var createAssignments = JsonDocument.Parse("""{"Configs":[{"Id":"exp-create"}]}""");
492492
var createRequestType = GetNestedType(typeof(CopilotClient), "CreateSessionRequest");
493493
var createRequest = CreateInternalRequest(
494494
createRequestType,
495495
("SessionId", "session-id"),
496-
("ExpAssignments", createAssignments.RootElement.Clone()));
496+
("ExpAssignments", new CopilotExpAssignmentResponse
497+
{
498+
Configs = new List<ExpConfigEntry> { new() { Id = "exp-create" } },
499+
}));
497500

498501
var createJson = JsonSerializer.Serialize(createRequest, createRequestType, options);
499502
using var createDocument = JsonDocument.Parse(createJson);
500503
var createRoot = createDocument.RootElement;
501504
Assert.Equal("exp-create", createRoot.GetProperty("expAssignments").GetProperty("Configs")[0].GetProperty("Id").GetString());
502505

503-
using var resumeAssignments = JsonDocument.Parse("""{"Configs":[{"Id":"exp-resume"}]}""");
504506
var resumeRequestType = GetNestedType(typeof(CopilotClient), "ResumeSessionRequest");
505507
var resumeRequest = CreateInternalRequest(
506508
resumeRequestType,
507509
("SessionId", "session-id"),
508-
("ExpAssignments", resumeAssignments.RootElement.Clone()));
510+
("ExpAssignments", new CopilotExpAssignmentResponse
511+
{
512+
Configs = new List<ExpConfigEntry> { new() { Id = "exp-resume" } },
513+
}));
509514

510515
var resumeJson = JsonSerializer.Serialize(resumeRequest, resumeRequestType, options);
511516
using var resumeDocument = JsonDocument.Parse(resumeJson);
@@ -540,38 +545,36 @@ public void SessionRequests_OmitExpAssignments_WhenUnset()
540545
[Fact]
541546
public void SessionConfigClone_PreservesExpAssignments()
542547
{
543-
using var assignments = JsonDocument.Parse("""{"Configs":[{"Id":"exp-create"}]}""");
544-
545548
var config = new SessionConfig
546549
{
547550
SessionId = "session-id",
548-
ExpAssignments = assignments.RootElement.Clone(),
551+
ExpAssignments = new CopilotExpAssignmentResponse
552+
{
553+
Configs = new List<ExpConfigEntry> { new() { Id = "exp-create" } },
554+
},
549555
};
550556

551557
var clone = config.Clone();
552558

553-
Assert.True(clone.ExpAssignments.HasValue);
554-
Assert.Equal(
555-
"exp-create",
556-
clone.ExpAssignments!.Value.GetProperty("Configs")[0].GetProperty("Id").GetString());
559+
Assert.NotNull(clone.ExpAssignments);
560+
Assert.Equal("exp-create", clone.ExpAssignments!.Configs[0].Id);
557561
}
558562

559563
[Fact]
560564
public void ResumeSessionConfigClone_PreservesExpAssignments()
561565
{
562-
using var assignments = JsonDocument.Parse("""{"Configs":[{"Id":"exp-resume"}]}""");
563-
564566
var config = new ResumeSessionConfig
565567
{
566-
ExpAssignments = assignments.RootElement.Clone(),
568+
ExpAssignments = new CopilotExpAssignmentResponse
569+
{
570+
Configs = new List<ExpConfigEntry> { new() { Id = "exp-resume" } },
571+
},
567572
};
568573

569574
var clone = config.Clone();
570575

571-
Assert.True(clone.ExpAssignments.HasValue);
572-
Assert.Equal(
573-
"exp-resume",
574-
clone.ExpAssignments!.Value.GetProperty("Configs")[0].GetProperty("Id").GetString());
576+
Assert.NotNull(clone.ExpAssignments);
577+
Assert.Equal("exp-resume", clone.ExpAssignments!.Configs[0].Id);
575578
}
576579

577580
[Fact]

go/client_test.go

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3181,9 +3181,13 @@ func TestStartCLIServer_StderrFieldSet(t *testing.T) {
31813181
}
31823182

31833183
func TestCreateSessionRequest_ExpAssignments(t *testing.T) {
3184-
assignments := map[string]any{
3185-
"Parameters": map[string]any{"copilot_exp_flag": "treatment"},
3186-
"AssignmentContext": "ctx-123",
3184+
assignments := &CopilotExpAssignmentResponse{
3185+
Features: []string{"copilot_exp_flag"},
3186+
Flights: map[string]string{"copilot_exp_flag": "treatment"},
3187+
Configs: []ExpConfigEntry{
3188+
{ID: "cfg-1", Parameters: map[string]ExpFlagValue{"threshold": 5, "enabled": true}},
3189+
},
3190+
AssignmentContext: "ctx-123",
31873191
}
31883192

31893193
t.Run("includes expAssignments in JSON when set", func(t *testing.T) {
@@ -3221,10 +3225,50 @@ func TestCreateSessionRequest_ExpAssignments(t *testing.T) {
32213225
})
32223226
}
32233227

3228+
func TestCopilotExpAssignmentResponse_MarshalNormalizesNilCollections(t *testing.T) {
3229+
// A response left with zero-value collections must still serialize the
3230+
// required fields as JSON arrays/objects, not null, so the runtime does not
3231+
// treat the payload as malformed.
3232+
data, err := json.Marshal(&CopilotExpAssignmentResponse{AssignmentContext: "ctx"})
3233+
if err != nil {
3234+
t.Fatalf("Failed to marshal: %v", err)
3235+
}
3236+
var m map[string]json.RawMessage
3237+
if err := json.Unmarshal(data, &m); err != nil {
3238+
t.Fatalf("Failed to unmarshal: %v", err)
3239+
}
3240+
for _, tc := range []struct{ key, want string }{
3241+
{"Features", "[]"},
3242+
{"Flights", "{}"},
3243+
{"Configs", "[]"},
3244+
{"AssignmentContext", `"ctx"`},
3245+
} {
3246+
if got := string(m[tc.key]); got != tc.want {
3247+
t.Errorf("Expected %s to serialize as %s, got %s", tc.key, tc.want, got)
3248+
}
3249+
}
3250+
3251+
// A nil Parameters map on an entry must likewise serialize as {}.
3252+
entryData, err := json.Marshal(ExpConfigEntry{ID: "cfg"})
3253+
if err != nil {
3254+
t.Fatalf("Failed to marshal entry: %v", err)
3255+
}
3256+
if err := json.Unmarshal(entryData, &m); err != nil {
3257+
t.Fatalf("Failed to unmarshal entry: %v", err)
3258+
}
3259+
if got := string(m["Parameters"]); got != "{}" {
3260+
t.Errorf("Expected Parameters to serialize as {}, got %s", got)
3261+
}
3262+
}
3263+
32243264
func TestResumeSessionRequest_ExpAssignments(t *testing.T) {
3225-
assignments := map[string]any{
3226-
"Parameters": map[string]any{"copilot_exp_flag": "treatment"},
3227-
"AssignmentContext": "ctx-456",
3265+
assignments := &CopilotExpAssignmentResponse{
3266+
Features: []string{"copilot_exp_flag"},
3267+
Flights: map[string]string{"copilot_exp_flag": "treatment"},
3268+
Configs: []ExpConfigEntry{
3269+
{ID: "cfg-1", Parameters: map[string]ExpFlagValue{"copilot_exp_flag": "treatment"}},
3270+
},
3271+
AssignmentContext: "ctx-456",
32283272
}
32293273

32303274
t.Run("includes expAssignments in JSON when set", func(t *testing.T) {

go/internal/e2e/client_options_e2e_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ func TestClientOptionsE2E(t *testing.T) {
272272
RequestExtensions: copilot.Bool(true),
273273
ExtensionSDKPath: &extensionSDKPath,
274274
ExtensionInfo: &copilot.ExtensionInfo{Source: "github-app", Name: "go-e2e-extension"},
275-
ExpAssignments: map[string]any{"feature": "enabled"},
275+
ExpAssignments: &copilot.CopilotExpAssignmentResponse{Flights: map[string]string{"feature": "enabled"}, AssignmentContext: "ctx"},
276276
})
277277
if err != nil {
278278
t.Fatalf("CreateSession failed: %v", err)
@@ -342,7 +342,7 @@ func TestClientOptionsE2E(t *testing.T) {
342342
if canvas["id"] != "canvas" || canvas["displayName"] != "Canvas" || canvas["description"] != "Canvas description" {
343343
t.Fatalf("Expected canvas declaration to be forwarded, got %#v", canvas)
344344
}
345-
if params["expAssignments"].(map[string]any)["feature"] != "enabled" {
345+
if params["expAssignments"].(map[string]any)["Flights"].(map[string]any)["feature"] != "enabled" {
346346
t.Fatalf("Expected expAssignments to be forwarded, got %#v", params["expAssignments"])
347347
}
348348
})
@@ -460,7 +460,7 @@ func TestClientOptionsE2E(t *testing.T) {
460460
RequestExtensions: copilot.Bool(true),
461461
ExtensionSDKPath: &extensionSDKPath,
462462
ExtensionInfo: &copilot.ExtensionInfo{Source: "github-app", Name: "go-e2e-extension"},
463-
ExpAssignments: map[string]any{"resumeFeature": "enabled"},
463+
ExpAssignments: &copilot.CopilotExpAssignmentResponse{Flights: map[string]string{"resumeFeature": "enabled"}, AssignmentContext: "ctx"},
464464
})
465465
if err != nil {
466466
t.Fatalf("ResumeSession failed: %v", err)
@@ -503,7 +503,7 @@ func TestClientOptionsE2E(t *testing.T) {
503503
if extensionInfo["source"] != "github-app" || extensionInfo["name"] != "go-e2e-extension" {
504504
t.Fatalf("Expected extensionInfo on resume, got %#v", extensionInfo)
505505
}
506-
if params["expAssignments"].(map[string]any)["resumeFeature"] != "enabled" {
506+
if params["expAssignments"].(map[string]any)["Flights"].(map[string]any)["resumeFeature"] != "enabled" {
507507
t.Fatalf("Expected resume expAssignments to be forwarded, got %#v", params["expAssignments"])
508508
}
509509
})

0 commit comments

Comments
 (0)