Skip to content
Draft
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
54 changes: 54 additions & 0 deletions src/Netclaw.Cli.Tests/Doctor/ContextWindowDoctorCheckTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,60 @@ public async Task NoExplicitContextWindow_DaemonOffline_ProviderReturnsNull_Retu
Assert.Contains("provider returned no context window", result.Message);
}

[Fact]
public async Task CatalogOverlay_ContextWindow_Passes()
{
// C2 regression: pre-fix the doctor only read Models.Main.ContextWindow
// directly from JSON, ignoring the catalog overlay that the daemon
// honors. An operator with Catalog['p/m'].ContextWindow set (and no
// inline value) saw the doctor fall through to auto-detection and
// print a misleading "set Models.Main.ContextWindow to pin a
// specific value" tip — for a value they had already pinned.
WriteConfig(new
{
configVersion = 1,
Models = new
{
Main = new { Provider = "p", ModelId = "m" },
Catalog = new Dictionary<string, object>
{
["p/m"] = new { ContextWindow = 200000 }
}
}
});
var check = CreateCheck(CreateOfflineDaemonApi());

var result = await check.RunAsync(TestContext.Current.CancellationToken);

Assert.Equal(DoctorSeverity.Pass, result.Severity);
Assert.Contains("200,000", result.Message);
}

[Fact]
public async Task InlineContextWindow_WinsOver_CatalogOverlay_InDoctor()
{
// Mirrors ApplyCatalogOverlays' inline-wins semantics so the doctor's
// diagnostic always matches what the daemon will actually use.
WriteConfig(new
{
configVersion = 1,
Models = new
{
Main = new { Provider = "p", ModelId = "m", ContextWindow = 100000 },
Catalog = new Dictionary<string, object>
{
["p/m"] = new { ContextWindow = 200000 }
}
}
});
var check = CreateCheck(CreateOfflineDaemonApi());

var result = await check.RunAsync(TestContext.Current.CancellationToken);

Assert.Equal(DoctorSeverity.Pass, result.Severity);
Assert.Contains("100,000", result.Message);
}

// ── Helpers ──────────────────────────────────────────────────────────

private ContextWindowDoctorCheck CreateCheck(
Expand Down
132 changes: 131 additions & 1 deletion src/Netclaw.Cli.Tests/Model/ModelCommandTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,19 @@ public async Task Set_MainModel_WritesConfig()
Assert.Equal("my-ollama", main.GetProperty("Provider").GetString());
Assert.Equal("qwen3:30b", main.GetProperty("ModelId").GetString());
Assert.Equal("Manual", main.GetProperty("Provenance").GetString());
Assert.Equal(32768, main.GetProperty("ContextWindow").GetInt32());

// Role records are identity-only; --context-window writes to Catalog
// keyed by "{provider}/{modelId}" so the override survives later role
// swaps. Catalog key contains ':' (Ollama's `qwen3:30b`) — this
// assertion also pins down the IConfiguration colon-split fix, since
// LoadModelSelection reads back via JsonSerializer.
Assert.False(main.TryGetProperty("ContextWindow", out _));
var catalog = models.GetProperty("Catalog");
var entry = catalog.GetProperty("my-ollama/qwen3:30b");
Assert.Equal(32768, entry.GetProperty("ContextWindow").GetInt32());

var selection = ModelCommand.LoadModelSelection(_paths)!;
Assert.Equal(32768, selection.Main.ContextWindow);
}

[Fact]
Expand Down Expand Up @@ -278,6 +290,124 @@ public async Task Set_FallbackModel_WritesCorrectRole()
Assert.Equal("qwen3:8b", fallback.GetProperty("ModelId").GetString());
}

[Fact]
public async Task Set_ContextWindowFlag_WritesToCatalog_NotInlineRole()
{
// The role record stays a pure identity pointer; explicit override
// intent (--context-window) lands in Models.Catalog so it survives
// role-pointer changes (the #1127 contract).
WriteConfig(new Dictionary<string, object>
{
["configVersion"] = 1,
["Providers"] = new Dictionary<string, object>
{
["p"] = new Dictionary<string, object> { ["Type"] = "ollama" }
}
});

await ModelCommand.RunAsync(
["model", "set", "main", "p", "m", "--context-window", "200000"],
_paths, output: _output);

var config = ReadConfigFile(_paths.NetclawConfigPath);
var models = config.RootElement.GetProperty("Models");
var main = models.GetProperty("Main");
Assert.False(main.TryGetProperty("ContextWindow", out _),
"Role record should not carry inline ContextWindow — overrides live in Catalog.");

var entry = models.GetProperty("Catalog").GetProperty("p/m");
Assert.Equal(200_000, entry.GetProperty("ContextWindow").GetInt32());

// Effective via overlay merge: the role sees ContextWindow=200000.
var selection = ModelCommand.LoadModelSelection(_paths)!;
Assert.Equal(200_000, selection.Main.ContextWindow);
}

[Fact]
public async Task Set_SwitchingRole_PointerDoesNotTouchCatalog()
{
// #1127 core invariant: changing Main from p/m1 to p/m2 does NOT
// disturb Models.Catalog. The hand-set override on p/m1 stays in
// place and re-applies the next time Main points back at p/m1.
WriteConfig(new Dictionary<string, object>
{
["configVersion"] = 1,
["Providers"] = new Dictionary<string, object>
{
["p"] = new Dictionary<string, object> { ["Type"] = "ollama" }
}
});

// Operator pins ContextWindow=200000 on p/m1.
await ModelCommand.RunAsync(
["model", "set", "main", "p", "m1", "--context-window", "200000"],
_paths, output: _output);

// Operator switches Main to p/m2 (no override on m2).
await ModelCommand.RunAsync(
["model", "set", "main", "p", "m2"],
_paths, output: _output);

var afterSwitch = ModelCommand.LoadModelSelection(_paths)!;
Assert.Equal("m2", afterSwitch.Main.ModelId);
Assert.Null(afterSwitch.Main.ContextWindow); // no override for m2

// Catalog still holds the m1 override untouched.
var config = ReadConfigFile(_paths.NetclawConfigPath);
var entry = config.RootElement
.GetProperty("Models").GetProperty("Catalog").GetProperty("p/m1");
Assert.Equal(200_000, entry.GetProperty("ContextWindow").GetInt32());

// Switching Main back to p/m1 re-applies the saved override.
await ModelCommand.RunAsync(
["model", "set", "main", "p", "m1"],
_paths, output: _output);

var afterReturn = ModelCommand.LoadModelSelection(_paths)!;
Assert.Equal(200_000, afterReturn.Main.ContextWindow);
}

[Fact]
public async Task Clear_LeavesCatalogIntact()
{
// Clearing a role removes the pointer; saved overrides survive.
WriteConfig(new Dictionary<string, object>
{
["configVersion"] = 1,
["Providers"] = new Dictionary<string, object>
{
["p"] = new Dictionary<string, object> { ["Type"] = "ollama" }
},
["Models"] = new Dictionary<string, object>
{
["Main"] = new Dictionary<string, object>
{
["Provider"] = "p",
["ModelId"] = "main-model"
},
["Fallback"] = new Dictionary<string, object>
{
["Provider"] = "p",
["ModelId"] = "fallback-model"
},
["Catalog"] = new Dictionary<string, object>
{
["p/fallback-model"] = new Dictionary<string, object>
{
["ContextWindow"] = 65536L
}
}
}
});

await ModelCommand.RunAsync(["model", "clear", "fallback"], _paths, output: _output);
await ModelCommand.RunAsync(
["model", "set", "fallback", "p", "fallback-model"], _paths, output: _output);

var selection = ModelCommand.LoadModelSelection(_paths)!;
Assert.Equal(65_536, selection.Fallback!.ContextWindow);
}

private void WriteConfig(Dictionary<string, object> data)
{
File.WriteAllText(_paths.NetclawConfigPath,
Expand Down
44 changes: 44 additions & 0 deletions src/Netclaw.Cli.Tests/Provider/ProviderRenamerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,50 @@ public void Rename_TrimsWhitespaceOnNewName()
Assert.True(providers.TryGetProperty("lab-a100", out _));
}

[Fact]
public void Rename_RewritesCatalogKeys_KeyedByOldProvider()
{
// C3 regression: pre-fix, Models.Catalog kept its keys keyed by the
// OLD provider name after a rename. The renamed role's
// ApplyCatalogOverlays lookup misses, and the operator's saved
// override silently disappears.
WriteConfig(new Dictionary<string, object>
{
["configVersion"] = 1,
["Providers"] = new Dictionary<string, object>
{
["old-p"] = new Dictionary<string, object> { ["Type"] = "openai-compatible" }
},
["Models"] = new Dictionary<string, object>
{
["Main"] = new Dictionary<string, object>
{
["Provider"] = "old-p",
["ModelId"] = "m"
},
["Catalog"] = new Dictionary<string, object>
{
["old-p/m"] = new Dictionary<string, object> { ["ContextWindow"] = 200000L },
["unrelated/k"] = new Dictionary<string, object> { ["ContextWindow"] = 50000L }
}
}
});

var result = ProviderRenamer.Rename(_paths, "old-p", "new-p");

Assert.True(result.Success);

using var config = JsonDocument.Parse(File.ReadAllText(_paths.NetclawConfigPath));
var catalog = config.RootElement.GetProperty("Models").GetProperty("Catalog");
Assert.False(catalog.TryGetProperty("old-p/m", out _),
"old key should have been rewritten");
Assert.True(catalog.TryGetProperty("new-p/m", out var renamedEntry),
"renamed key should be present");
Assert.Equal(200_000, renamedEntry.GetProperty("ContextWindow").GetInt32());
Assert.True(catalog.TryGetProperty("unrelated/k", out _),
"catalog entries for other providers should be untouched");
}

private void WriteConfig(Dictionary<string, object> data)
{
File.WriteAllText(_paths.NetclawConfigPath,
Expand Down
22 changes: 22 additions & 0 deletions src/Netclaw.Cli/Config/ConfigFileHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -121,4 +121,26 @@ internal static string DecryptIfEncrypted(Configuration.NetclawPaths paths, stri
var protector = SecretsProtection.CreateProtector(paths);
return protector.Unprotect(value);
}

/// <summary>
/// Write an operator-set <see cref="Configuration.ModelOverride"/> field
/// into <c>Models.Catalog["{provider}/{modelId}"]</c>. Used by CLI flags
/// (e.g. <c>--context-window</c>) that represent explicit override intent.
/// The catalog is the persistent override layer — values written here
/// survive role-pointer changes (picker swaps, role clears, role
/// re-selection) because the catalog key is independent of which role
/// currently references the model.
/// </summary>
internal static void SetCatalogOverride(
Dictionary<string, object> modelsSection,
string provider,
string modelId,
string fieldName,
object value)
{
var catalog = GetOrCreateSection(modelsSection, "Catalog");
var key = Configuration.ModelSelection.CatalogKey(provider, modelId);
var entry = GetOrCreateSection(catalog, key);
entry[fieldName] = value;
}
}
74 changes: 63 additions & 11 deletions src/Netclaw.Cli/Doctor/ContextWindowDoctorCheck.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,25 +50,77 @@ public async Task<DoctorCheckResult> RunAsync(CancellationToken cancellationToke
"Add a Models.Main section with ContextWindow to netclaw.json.");
}

var contextWindow = main["ContextWindow"];
var modelId = main["ModelId"]?.GetValue<string>() ?? "unknown";
var providerName = main["Provider"]?.GetValue<string>() ?? "local-ollama";

// Effective ContextWindow follows ModelSelection.ApplyCatalogOverlays:
// inline on the role wins; otherwise fall back to the catalog
// overlay keyed by "{provider}/{modelId}" (case-insensitive). The
// runtime daemon uses exactly this precedence, so the doctor must
// match it or it will report "no explicit setting" while the
// daemon honors an override.
var contextWindow = main["ContextWindow"]
?? TryReadCatalogContextWindow(models!, providerName, modelId);

if (contextWindow is null)
{
var modelId = main["ModelId"]?.GetValue<string>() ?? "unknown";
var providerName = main["Provider"]?.GetValue<string>() ?? "local-ollama";
return await ResolveEffectiveContextWindowAsync(modelId, providerName, cancellationToken);
}

if (contextWindow.GetValue<int>() is var cw and > 0)
if (!TryReadPositiveInt(contextWindow, out var cw))
{
return DoctorCheckResult.Pass(
return DoctorCheckResult.Error(
"Context Window",
$"Context window explicitly set to {cw:N0} tokens.");
$"ContextWindow for {providerName}/{modelId} is not a positive integer (got {DescribeJsonValue(contextWindow)}).",
"Edit netclaw.json so the ContextWindow value is a positive integer literal (no quotes, no decimals).");
}

return DoctorCheckResult.Error(
return DoctorCheckResult.Pass(
"Context Window",
"Models.Main.ContextWindow must be a positive integer.",
"Set Models.Main.ContextWindow to the effective runtime context window size in tokens.");
$"Context window explicitly set to {cw:N0} tokens.");
}

private static JsonNode? TryReadCatalogContextWindow(
JsonObject models, string providerName, string modelId)
{
var catalog = models["Catalog"] as JsonObject;
if (catalog is null) return null;
var key = ModelSelection.CatalogKey(providerName, modelId);
// Case-insensitive scan: ApplyCatalogOverlays does the same when the
// exact-case lookup misses, so the doctor must agree.
if (catalog[key] is JsonObject direct) return direct["ContextWindow"];
foreach (var kvp in catalog)
{
if (string.Equals(kvp.Key, key, StringComparison.OrdinalIgnoreCase)
&& kvp.Value is JsonObject entry)
return entry["ContextWindow"];
}
return null;
}

private static bool TryReadPositiveInt(JsonNode node, out int value)
{
value = 0;
try
{
// JsonValue.TryGetValue<T> avoids the InvalidOperationException
// GetValue<int>() throws on strings, doubles, etc. — exactly the
// shapes a hand-edited Catalog override is prone to.
if (node is JsonValue jv && jv.TryGetValue<int>(out var parsed) && parsed > 0)
{
value = parsed;
return true;
}
}
catch (Exception ex) when (ex is InvalidOperationException or FormatException)
{
// fall through to false
}
return false;
}

private static string DescribeJsonValue(JsonNode node)
{
var raw = node.ToJsonString();
return raw.Length > 40 ? raw[..37] + "..." : raw;
}

private async Task<DoctorCheckResult> ResolveEffectiveContextWindowAsync(
Expand Down
Loading
Loading