Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 48 additions & 4 deletions PolyPilot.Tests/ModelSelectionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,42 @@ public void NormalizeToSlug_AllFallbackModels_AreAlreadySlugs()
}

[Fact]
public void FallbackModels_IncludeCurrentGptVariants()
public void BuildSelectionList_AppendsSelectionAndDefault_WhenDiscoveryIsEmpty()
{
Assert.Contains("gpt-5.4", ModelHelper.FallbackModels);
Assert.Contains("gpt-5.4-mini", ModelHelper.FallbackModels);
Assert.Contains("gpt-5.3-codex", ModelHelper.FallbackModels);
var models = ModelHelper.BuildSelectionList(Array.Empty<string>(), "Custom Preview", "claude-opus-4.6");

Assert.Equal(new[] { "custom-preview", "claude-opus-4.6" }, models);
}

[Fact]
public void BuildSelectionList_NormalizesDiscoveredModels_AndAvoidsDuplicates()
{
var models = ModelHelper.BuildSelectionList(
new[] { "Claude Opus 4.6", "claude-opus-4.6", "Custom Preview" },
"custom-preview",
"claude-opus-4.6");

Assert.Equal(new[] { "claude-opus-4.6", "custom-preview" }, models);
}

[Fact]
public void ShouldAcceptObservedModel_EmptyCurrentModel_AcceptsObserved()
{
Assert.True(ModelHelper.ShouldAcceptObservedModel("", "claude-opus-4.6"));
Assert.True(ModelHelper.ShouldAcceptObservedModel("resumed", "claude-opus-4.6"));
}

[Fact]
public void ShouldAcceptObservedModel_SameModel_AcceptsObserved()
{
Assert.True(ModelHelper.ShouldAcceptObservedModel("Claude Opus 4.6", "claude-opus-4.6"));
}

[Fact]
public void ShouldAcceptObservedModel_DifferentObservedModel_PreservesExplicitChoice()
{
Assert.False(ModelHelper.ShouldAcceptObservedModel("gpt-5.4", "claude-opus-4.6"));
Assert.False(ModelHelper.ShouldAcceptObservedModel("claude-haiku-4.5", "claude-opus-4.6"));
}

[Fact]
Expand Down Expand Up @@ -493,6 +524,19 @@ public void ChangeModel_DisplayNameFromDropdown_NormalizesToSlug()
}
}

[Fact]
public void BuildSelectionList_PreservesDiscoveryOrder_AndAppendsMissingRequiredModels()
{
var models = ModelHelper.BuildSelectionList(
new[] { "claude-sonnet-4.6", "Claude Opus 4.6", "gpt-5.4" },
"claude-opus-4.6",
"gpt-5-mini");

Assert.Equal(
new[] { "claude-sonnet-4.6", "claude-opus-4.6", "gpt-5.4", "gpt-5-mini" },
models);
}

// --- PrettifyModel tests ---
// The prettifier is duplicated in ExpandedSessionView.razor and ModelSelector.razor.
// We test the logic inline here to catch regressions like the "Opus-4.5" bug.
Expand Down
5 changes: 3 additions & 2 deletions PolyPilot.Tests/ScenarioReferenceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -153,13 +153,14 @@ public void Scenario_RefreshSessionsButton_HasUnitTestCoverage()

/// <summary>
/// Scenario: "create-session-model-picker-includes-gpt-5-4"
/// Unit test equivalents: ModelSelectionTests.FallbackModels_IncludeCurrentGptVariants
/// Unit test equivalents: ModelSelectionTests.BuildSelectionList_AppendsSelectionAndDefault_WhenDiscoveryIsEmpty,
/// ModelSelectionTests.BuildSelectionList_NormalizesDiscoveredModels_AndAvoidsDuplicates,
/// and EventsJsonlParsingTests.ExtractLatestModelFromEvents_LaterModelChangeWins
/// </summary>
[Fact]
public void Scenario_ModelPickerIncludesGpt54_HasUnitTestCoverage()
{
Assert.True(true, "See ModelSelectionTests.FallbackModels_IncludeCurrentGptVariants and EventsJsonlParsingTests.ExtractLatestModelFromEvents_LaterModelChangeWins");
Assert.True(true, "See ModelSelectionTests.BuildSelectionList_* and EventsJsonlParsingTests.ExtractLatestModelFromEvents_LaterModelChangeWins");
}

/// <summary>
Expand Down
7 changes: 4 additions & 3 deletions PolyPilot.Tests/Scenarios/mode-switch-scenarios.json
Original file line number Diff line number Diff line change
Expand Up @@ -530,9 +530,10 @@
{
"id": "create-session-model-picker-includes-gpt-5-4",
"name": "Create session model picker exposes GPT-5.4",
"description": "The create-session model selector should list GPT-5.4 so users can match the CLI even when the app falls back to its built-in model catalog.",
"description": "The create-session model selector should list GPT-5.4 from the Copilot CLI's discovered model list, without requiring PolyPilot to hardcode every supported model.",
"unitTestCoverage": [
"ModelSelectionTests.FallbackModels_IncludeCurrentGptVariants"
"ModelSelectionTests.BuildSelectionList_AppendsSelectionAndDefault_WhenDiscoveryIsEmpty",
"ModelSelectionTests.BuildSelectionList_NormalizesDiscoveredModels_AndAvoidsDuplicates"
],
"steps": [
{
Expand Down Expand Up @@ -1585,4 +1586,4 @@
]
}
]
}
}
24 changes: 18 additions & 6 deletions PolyPilot.Tests/StateChangeCoalescerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -70,19 +70,31 @@ public async Task SeparateBursts_FireSeparately()
{
var svc = CreateService();
int fireCount = 0;
svc.OnStateChanged += () => Interlocked.Increment(ref fireCount);
var firstBurstFired = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
var secondBurstFired = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
svc.OnStateChanged += () =>
{
var count = Interlocked.Increment(ref fireCount);
if (count >= 1)
firstBurstFired.TrySetResult();
if (count >= 2)
secondBurstFired.TrySetResult();
};

// First burst
for (int i = 0; i < 10; i++)
svc.NotifyStateChangedCoalesced();
// Wait well beyond the coalesce window (150ms) to ensure the timer fires,
// even under heavy CI/GC load. Previous 300ms was flaky under load.
await Task.Delay(800);
var firstBurstCompleted = await Task.WhenAny(firstBurstFired.Task, Task.Delay(5000));
Assert.Same(firstBurstFired.Task, firstBurstCompleted);

// Second burst after timer has fired
// Second burst after the first coalesced notification has actually fired
for (int i = 0; i < 10; i++)
svc.NotifyStateChangedCoalesced();
await Task.Delay(800);
var secondBurstCompleted = await Task.WhenAny(secondBurstFired.Task, Task.Delay(5000));
Assert.Same(secondBurstFired.Task, secondBurstCompleted);

// Small settle window for any extra coalesced fires.
await Task.Delay(100);

// Each burst should produce ~1 notification
Assert.InRange(fireCount, 2, 4);
Expand Down
7 changes: 6 additions & 1 deletion PolyPilot/Components/Layout/SessionListItem.razor
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@
<div class="model-picker-inline" @onclick:stopPropagation="true">
<select value="@(Meta?.PreferredModel ?? "")" @onchange="OnPreferredModelChanged">
<option value="">Default (@Session.Model)</option>
@foreach (var model in CopilotService.AvailableModels)
@foreach (var model in GetSelectableModels())
{
var strengths = ModelCapabilities.GetStrengths(model);
<option value="@model" title="@strengths">@model</option>
Expand Down Expand Up @@ -405,6 +405,11 @@
CopilotService.SetSessionPreferredModel(Session.Name, string.IsNullOrEmpty(slug) ? null : slug);
}

private IReadOnlyList<string> GetSelectableModels() =>
CopilotService.AvailableModels.Count > 0
? ModelHelper.BuildSelectionList(CopilotService.AvailableModels, Meta?.PreferredModel, Session.Model, CopilotService.DefaultModel)
: ModelHelper.BuildSelectionList(ModelHelper.FallbackModels, Meta?.PreferredModel, Session.Model, CopilotService.DefaultModel);

private static string GetShortPath(string path)
{
var parts = path.Split(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);
Expand Down
6 changes: 4 additions & 2 deletions PolyPilot/Components/Layout/SessionSidebar.razor
Original file line number Diff line number Diff line change
Expand Up @@ -1439,7 +1439,9 @@ else
}

private IReadOnlyList<string> availableModels =>
CopilotService.AvailableModels.Count > 0 ? CopilotService.AvailableModels : ModelHelper.FallbackModels;
CopilotService.AvailableModels.Count > 0
? ModelHelper.BuildSelectionList(CopilotService.AvailableModels, selectedModel, CopilotService.DefaultModel)
: ModelHelper.BuildSelectionList(ModelHelper.FallbackModels, selectedModel, CopilotService.DefaultModel);

private IEnumerable<PersistedSessionInfo> filteredPersistedSessions =>
(string.IsNullOrWhiteSpace(sessionFilter)
Expand Down Expand Up @@ -1468,7 +1470,7 @@ else
// Restore last selected model
var uiState = CopilotService.LoadUiState();
if (!string.IsNullOrEmpty(uiState?.SelectedModel))
selectedModel = uiState.SelectedModel;
selectedModel = ModelHelper.NormalizeToSlug(uiState.SelectedModel);
}

private void SaveSelectedModel()
Expand Down
4 changes: 3 additions & 1 deletion PolyPilot/Components/Pages/Dashboard.razor
Original file line number Diff line number Diff line change
Expand Up @@ -3353,7 +3353,9 @@
// === Model, plan mode, font, token helpers ===

private IReadOnlyList<string> availableModels =>
CopilotService.AvailableModels.Count > 0 ? CopilotService.AvailableModels : ModelHelper.FallbackModels;
CopilotService.AvailableModels.Count > 0
? ModelHelper.BuildSelectionList(CopilotService.AvailableModels, CopilotService.DefaultModel)
: ModelHelper.BuildSelectionList(ModelHelper.FallbackModels, CopilotService.DefaultModel);

private string GetSessionModel(AgentSessionInfo session)
{
Expand Down
56 changes: 56 additions & 0 deletions PolyPilot/Models/ModelHelper.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Text.Json;
using System.Text.RegularExpressions;

namespace PolyPilot.Models;

Expand All @@ -10,6 +11,15 @@ namespace PolyPilot.Models;
/// </summary>
public static class ModelHelper
{
private static readonly Regex ValidSlugPattern = new(@"^[a-z0-9][-a-z0-9.]*$", RegexOptions.Compiled);

/// <summary>
/// Returns true if the string looks like a valid model slug
/// (lowercase alphanumeric with hyphens and dots, e.g. "claude-opus-4.6").
/// </summary>
public static bool IsValidModelSlug(string? slug) =>
!string.IsNullOrEmpty(slug) && ValidSlugPattern.IsMatch(slug);

public static IReadOnlyList<string> FallbackModels { get; } = new[]
{
"claude-opus-4.6",
Expand All @@ -34,6 +44,35 @@ public static class ModelHelper
"gemini-3-pro-preview",
};

/// <summary>
/// Builds a model picker list from CLI-discovered models plus any required
/// fallback choices (current selection, current session model, default model).
/// All values are normalized to slugs and deduplicated case-insensitively.
/// </summary>
public static IReadOnlyList<string> BuildSelectionList(IEnumerable<string>? discoveredModels, params string?[] requiredModels)
{
var result = new List<string>();
var seen = new HashSet<string>(StringComparer.OrdinalIgnoreCase);

void Add(string? model)
{
var normalized = NormalizeToSlug(model);
if (!string.IsNullOrEmpty(normalized) && seen.Add(normalized))
result.Add(normalized);
}

if (discoveredModels != null)
{
foreach (var model in discoveredModels)
Add(model);
}

foreach (var model in requiredModels)
Add(model);

return result;
}

/// <summary>
/// Normalize any model string to its canonical slug form.
/// Handles display names like "Claude Opus 4.5", "GPT-5.1-Codex",
Expand Down Expand Up @@ -90,6 +129,23 @@ public static bool IsDisplayName(string? model)
return model.Any(char.IsUpper) || model.Contains(' ');
}

/// <summary>
/// Decide whether an observed SDK/CLI event model should replace the current session model.
/// Preserve an explicit user-selected model when metrics events report a different backend
/// model (for example after resume/abort flows that transiently report a default model).
/// </summary>
public static bool ShouldAcceptObservedModel(string? currentModel, string? observedModel)
{
var normalizedObserved = NormalizeToSlug(observedModel);
if (string.IsNullOrEmpty(normalizedObserved))
return false;

var normalizedCurrent = NormalizeToSlug(currentModel);
return string.IsNullOrEmpty(normalizedCurrent) ||
normalizedCurrent == "resumed" ||
normalizedCurrent == normalizedObserved;
}

/// <summary>
/// Converts a model slug to a human-friendly display name.
/// If displayNames dictionary is provided and contains the slug, uses the SDK's display name.
Expand Down
23 changes: 20 additions & 3 deletions PolyPilot/Services/CopilotService.Events.cs
Original file line number Diff line number Diff line change
Expand Up @@ -772,8 +772,15 @@ await notifService.SendNotificationAsync(
if (!string.IsNullOrEmpty(uModel))
{
var normalizedUModel = Models.ModelHelper.NormalizeToSlug(uModel);
Debug($"[UsageInfo] Updating model from event: {state.Info.Model} -> {normalizedUModel}");
state.Info.Model = normalizedUModel;
if (Models.ModelHelper.ShouldAcceptObservedModel(state.Info.Model, normalizedUModel))
{
Debug($"[UsageInfo] Updating model from event: {state.Info.Model} -> {normalizedUModel}");
state.Info.Model = normalizedUModel;
}
else
{
Debug($"[UsageInfo] Ignoring backend-reported model: {normalizedUModel} (keeping explicit session model: {state.Info.Model})");
}
}
if (uCurrentTokens.HasValue) state.Info.ContextCurrentTokens = uCurrentTokens;
if (uTokenLimit.HasValue) state.Info.ContextTokenLimit = uTokenLimit;
Expand Down Expand Up @@ -807,7 +814,17 @@ await notifService.SendNotificationAsync(
}
catch { }
if (!string.IsNullOrEmpty(aModel))
state.Info.Model = Models.ModelHelper.NormalizeToSlug(aModel);
{
var normalizedAModel = Models.ModelHelper.NormalizeToSlug(aModel);
if (Models.ModelHelper.ShouldAcceptObservedModel(state.Info.Model, normalizedAModel))
{
state.Info.Model = normalizedAModel;
}
else
{
Debug($"[AssistantUsage] Ignoring backend-reported model: {normalizedAModel} (keeping explicit session model: {state.Info.Model})");
}
}
if (aInput.HasValue) state.Info.TotalInputTokens += aInput.Value;
if (aOutput.HasValue) state.Info.TotalOutputTokens += aOutput.Value;
if (aInput.HasValue || aOutput.HasValue || aPremiumQuota != null)
Expand Down
Loading