diff --git a/docs/open.md b/docs/open.md index f87a49cd..4dd410e2 100644 --- a/docs/open.md +++ b/docs/open.md @@ -50,7 +50,7 @@ Alles gebaut + unit-getestet, aber **nicht visuell verifiziert** — Mika prüft Offene Entscheidungen dazu: - Interaktive ConPTY-Sessions bekommen `--effort`, aber **kein** `--model` — die Session läuft weiter unter dem Modell aus Mikas Claude-Config, der Effort kommt aus dem Preset des Modells, das ClaudeDo für die Task auflösen würde. Falls ClaudeDo auch interaktiv das Modell erzwingen soll, ist das ein Folge-Task. -- `AppSettings.DefaultMaxTurns` ist nur noch Fallback für ein Modell ohne Preset-Zeile und hat keinen Editor mehr. Spalte könnte später entfallen. +- `AppSettings.DefaultMaxTurns` ist jetzt tatsächlich verdrahtet (`ModelPresets.For(..., global.DefaultMaxTurns)` in `TaskRunner.ResolveConfigAsync`): reiner Fallback für ein Modell, das auch nach `ModelRegistry.TryNormalizeAlias` auf keine Preset-Zeile trifft — vorher war das Feld tot (hartkodierte 30). Hat weiterhin keinen eigenen Editor mehr (nur die Preset-Tabelle pro Modell); Spalte könnte später entfallen, falls das nie zutrifft. ## Beobachtung (offen — Entscheidung Mika) diff --git a/src/ClaudeDo.Data/CLAUDE.md b/src/ClaudeDo.Data/CLAUDE.md index 760d194d..b56e9d7b 100644 --- a/src/ClaudeDo.Data/CLAUDE.md +++ b/src/ClaudeDo.Data/CLAUDE.md @@ -14,7 +14,8 @@ Shared data layer: models, repositories, SQLite infrastructure, and git operatio - **WeekReportEntity** — Id, StartDate/EndDate (DateOnly), Markdown, GeneratedAt → table `week_reports`, unique index on (start_date, end_date) - **TaskAttachmentEntity** — Id, TaskId (FK to tasks, ON DELETE CASCADE), FileName, ByteSize, CreatedAt → table `task_attachments` - **AppSettingsEntity** also carries `ReportExcludedPaths` (string?, JSON array of excluded path prefixes, column `report_excluded_paths`), `StandupWeekday` (int DayOfWeek, default Wednesday, column `standup_weekday`), and `DailyPrepMaxTasks` (int, default 5, column `daily_prep_max_tasks` — hard cap on how many open tasks the daily-prep / "Prime Claude" feature may place in MyDay) and `ModelPresets` (string?, JSON array of `ModelPreset` rows, column `model_presets`) -- **ModelPresets** / **ModelPreset** — per-model run defaults (`Model`, `Effort`, `MaxTurns`): one row per `ModelRegistry.Aliases` entry, supplying the **global** effort and max-turns defaults. `Parse`/`Serialize` normalize (unknown models dropped, missing aliases filled from `Defaults`, effort validated, turns clamped 1–200) and never throw — a malformed settings row must not stop a run. `For(presets, model)` always returns a usable row. Ship defaults: haiku medium/20, sonnet high/30, opus high/40, fable high/25. +- **ModelPresets** / **ModelPreset** — per-model run defaults (`Model`, `Effort`, `MaxTurns`): one row per `ModelRegistry.Aliases` entry, supplying the **global** effort and max-turns defaults. `Parse`/`Serialize` normalize (unknown models dropped, missing aliases filled from `Defaults`, effort validated, turns clamped 1–200) and never throw — a malformed settings row must not stop a run. `For(presets, model, fallbackMaxTurns = 30)` always returns a usable row: `model` is resolved through `ModelRegistry.TryNormalizeAlias` first, so a full CLI model id (e.g. `claude-sonnet-4-6`) still hits its alias's preset row instead of missing every lookup and falling through; only a model that normalizes to nothing recognized falls back to a synthesized row (`EffortRegistry.DefaultLevel` + `fallbackMaxTurns` — callers pass `AppSettings.DefaultMaxTurns` here so that setting has a real effect instead of a hardcoded number). Ship defaults: haiku medium/20, sonnet high/30, opus high/40, fable high/25. +- **ModelRegistry.TryNormalizeAlias** — non-throwing counterpart to `NormalizeAlias` for the run path: exact alias match, then substring match against a full model id, else `null`. Never throws, unlike `NormalizeAlias` (which stays the strict, throwing validator for `add_task`/planning model input). - **EffortRegistry** — the `--effort` levels (`low|medium|high|xhigh|max`) + `NormalizeLevel` (blank → null = don't pass the flag) - **SubtaskEntity**, **AppSettingsEntity**, **AgentInfo** — existing helpers / settings / record for scanned agent files diff --git a/src/ClaudeDo.Data/Models/ModelPresets.cs b/src/ClaudeDo.Data/Models/ModelPresets.cs index 4d06b482..a585a43e 100644 --- a/src/ClaudeDo.Data/Models/ModelPresets.cs +++ b/src/ClaudeDo.Data/Models/ModelPresets.cs @@ -46,20 +46,26 @@ public static class ModelPresets } /// The preset for , or the default row for it. Never null so - /// callers don't have to special-case an unconfigured or unknown model. - public static ModelPreset For(IReadOnlyList presets, string? model) + /// callers don't have to special-case an unconfigured or unknown model. + /// is resolved through first, so a full CLI model + /// id (e.g. "claude-sonnet-4-6") still hits its alias's preset row instead of falling through. + /// If normalizes to nothing recognized, + /// (typically AppSettings.DefaultMaxTurns) is used instead of a hardcoded number, so a + /// truly unknown model is no harsher than the configured global default. + public static ModelPreset For(IReadOnlyList presets, string? model, int fallbackMaxTurns = 30) { - var alias = (model ?? ModelRegistry.DefaultAlias).Trim(); + var alias = ModelRegistry.TryNormalizeAlias(model) ?? (model ?? ModelRegistry.DefaultAlias).Trim(); foreach (var p in presets) if (string.Equals(p.Model, alias, StringComparison.OrdinalIgnoreCase)) return p; foreach (var p in Defaults) if (string.Equals(p.Model, alias, StringComparison.OrdinalIgnoreCase)) return p; - return new ModelPreset(alias, EffortRegistry.DefaultLevel, 30); + return new ModelPreset(alias, EffortRegistry.DefaultLevel, fallbackMaxTurns); } - public static ModelPreset For(string? json, string? model) => For(Parse(json), model); + public static ModelPreset For(string? json, string? model, int fallbackMaxTurns = 30) + => For(Parse(json), model, fallbackMaxTurns); // One row per known alias, canonical order, clamped values — so an edited or partial list // still round-trips into something every consumer can rely on. diff --git a/src/ClaudeDo.Data/Models/ModelRegistry.cs b/src/ClaudeDo.Data/Models/ModelRegistry.cs index 57a45c7c..dd2f3cc1 100644 --- a/src/ClaudeDo.Data/Models/ModelRegistry.cs +++ b/src/ClaudeDo.Data/Models/ModelRegistry.cs @@ -26,4 +26,22 @@ public static class ModelRegistry return alias; throw new ArgumentException($"Unknown model '{model}'. Allowed: {string.Join(", ", Aliases)}."); } + + /// + /// Best-effort alias lookup for run-path model resolution, where a full CLI model id + /// (e.g. "claude-sonnet-4-6") is just as likely as a bare alias and a run must never be + /// blocked by an unrecognized one. Null/blank/unrecognized → null; never throws. + /// + public static string? TryNormalizeAlias(string? model) + { + var m = model?.Trim(); + if (string.IsNullOrEmpty(m)) return null; + foreach (var alias in Aliases) + if (string.Equals(alias, m, StringComparison.OrdinalIgnoreCase)) + return alias; + foreach (var alias in Aliases) + if (m.Contains(alias, StringComparison.OrdinalIgnoreCase)) + return alias; + return null; + } } diff --git a/src/ClaudeDo.Worker/CLAUDE.md b/src/ClaudeDo.Worker/CLAUDE.md index cdee7bb8..e6c31fe3 100644 --- a/src/ClaudeDo.Worker/CLAUDE.md +++ b/src/ClaudeDo.Worker/CLAUDE.md @@ -122,7 +122,7 @@ A child that hits a roadblock (fails, or reports `CLAUDEDO_BLOCKED` roadblocks) 1. Load task + list metadata from DB; resolve config from `list_config` + task-level overrides (model, system_prompt, agent_path) 2. Create worktree (if `WorkingDir` set) or sandbox directory 3. Mark task "running", broadcast `TaskStarted` -4. Resolve the effective model (task → list → `AppSettings.DefaultModel`), then take its `ModelPresets` row: the preset supplies `--effort` and the **global** max-turns default (task/list `MaxTurns` overrides still win). Build CLI args via `ClaudeArgsBuilder`; inject attachment absolute paths via `TaskPromptComposer.Compose` (appends a read-only "## Reference files" section); invoke `ClaudeProcess` with task prompt +4. Resolve the effective model (task → list → `AppSettings.DefaultModel`), then take its `ModelPresets` row via `ModelPresets.For(global.ModelPresets, model, global.DefaultMaxTurns)`: the model string is resolved through `ModelRegistry.TryNormalizeAlias` first, so a full CLI model id (e.g. `claude-sonnet-4-6`, not just the bare `sonnet`/`opus`/`haiku`/`fable` aliases) still hits its alias's preset row; only a truly unrecognized model falls back to a synthesized row using `AppSettings.DefaultMaxTurns` (never a hardcoded number, and never throws — an unknown model must not block a run). The preset supplies `--effort` and the **global** max-turns default (task/list `MaxTurns` overrides still win). Build CLI args via `ClaudeArgsBuilder`; inject attachment absolute paths via `TaskPromptComposer.Compose` (appends a read-only "## Reference files" section); invoke `ClaudeProcess` with task prompt 5. Stream NDJSON output through `StreamAnalyzer`; lines forwarded to log file and SignalR (`TaskMessage`) 6. On success: auto-commit changes (worktree only), store run record, mark "done" 7. On failure: retry once if session ID available (`--resume`), then mark "failed" diff --git a/src/ClaudeDo.Worker/Runner/TaskRunner.cs b/src/ClaudeDo.Worker/Runner/TaskRunner.cs index 611e3ebc..9f32a51d 100644 --- a/src/ClaudeDo.Worker/Runner/TaskRunner.cs +++ b/src/ClaudeDo.Worker/Runner/TaskRunner.cs @@ -519,7 +519,7 @@ public sealed class TaskRunner // The model decides the global effort/turn defaults: one preset row per model alias // (Settings → General). List- and task-level max-turns overrides still win. var model = task.Model ?? listConfig?.Model ?? global.DefaultModel; - var preset = Data.Models.ModelPresets.For(global.ModelPresets, model); + var preset = Data.Models.ModelPresets.For(global.ModelPresets, model, global.DefaultMaxTurns); return new ClaudeRunConfig( Model: model, diff --git a/tests/ClaudeDo.Data.Tests/ModelPresetsTests.cs b/tests/ClaudeDo.Data.Tests/ModelPresetsTests.cs index 3aed384c..a96eeec0 100644 --- a/tests/ClaudeDo.Data.Tests/ModelPresetsTests.cs +++ b/tests/ClaudeDo.Data.Tests/ModelPresetsTests.cs @@ -65,6 +65,44 @@ public class ModelPresetsTests Assert.True(row.MaxTurns > 0); } + [Fact] + public void For_unknown_model_uses_the_supplied_fallback_max_turns_instead_of_a_hardcoded_number() + { + var row = ModelPresets.For(ModelPresets.Defaults, "some-future-model", fallbackMaxTurns: 100); + + Assert.Equal(100, row.MaxTurns); + } + + [Fact] + public void For_a_full_model_id_resolves_to_its_alias_preset_row_not_the_unknown_fallback() + { + var sonnetPreset = ModelPresets.For(ModelPresets.Defaults, "sonnet"); + + var row = ModelPresets.For(ModelPresets.Defaults, "claude-sonnet-4-6"); + + Assert.Equal(sonnetPreset.Effort, row.Effort); + Assert.Equal(sonnetPreset.MaxTurns, row.MaxTurns); + } + + [Fact] + public void For_a_full_model_id_honors_a_customized_preset_row_for_its_alias() + { + var parsed = ModelPresets.Parse(ModelPresets.Serialize(new[] { new ModelPreset("sonnet", "xhigh", 77) })); + + var row = ModelPresets.For(parsed, "claude-sonnet-4-6"); + + Assert.Equal("xhigh", row.Effort); + Assert.Equal(77, row.MaxTurns); + } + + [Fact] + public void For_unknown_model_never_throws_and_still_starts_a_run() + { + var exception = Record.Exception(() => ModelPresets.For(ModelPresets.Defaults, "totally-unrecognized-model")); + + Assert.Null(exception); + } + [Fact] public void For_null_model_uses_the_default_alias() { diff --git a/tests/ClaudeDo.Data.Tests/ModelRegistryTests.cs b/tests/ClaudeDo.Data.Tests/ModelRegistryTests.cs index 3aab3ba0..f38382aa 100644 --- a/tests/ClaudeDo.Data.Tests/ModelRegistryTests.cs +++ b/tests/ClaudeDo.Data.Tests/ModelRegistryTests.cs @@ -33,4 +33,28 @@ public class ModelRegistryTests { Assert.Equal(new[] { "haiku", "sonnet", "opus" }, ModelRegistry.ByCostAscending); } + + [Theory] + [InlineData("sonnet", "sonnet")] + [InlineData("OPUS", "opus")] + [InlineData(" haiku ", "haiku")] + [InlineData("claude-sonnet-4-6", "sonnet")] + [InlineData("claude-opus-4-6", "opus")] + [InlineData("claude-haiku-4-5-20251001", "haiku")] + [InlineData("us.anthropic.claude-sonnet-4-6-v1:0", "sonnet")] + public void TryNormalizeAlias_matches_aliases_and_full_model_ids(string input, string expected) + { + Assert.Equal(expected, ModelRegistry.TryNormalizeAlias(input)); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + [InlineData("gpt-4")] + [InlineData("some-future-model")] + public void TryNormalizeAlias_never_throws_and_returns_null_when_unrecognized(string? input) + { + Assert.Null(ModelRegistry.TryNormalizeAlias(input)); + } } diff --git a/tests/ClaudeDo.Worker.Tests/Runner/ModelResolutionWireTests.cs b/tests/ClaudeDo.Worker.Tests/Runner/ModelResolutionWireTests.cs new file mode 100644 index 00000000..50f55b52 --- /dev/null +++ b/tests/ClaudeDo.Worker.Tests/Runner/ModelResolutionWireTests.cs @@ -0,0 +1,111 @@ +using ClaudeDo.Data; +using ClaudeDo.Data.Git; +using ClaudeDo.Data.Models; +using ClaudeDo.Data.Repositories; +using ClaudeDo.Worker.Config; +using ClaudeDo.Worker.Hub; +using ClaudeDo.Worker.Runner; +using ClaudeDo.Worker.Tests.Infrastructure; +using Microsoft.Extensions.Logging.Abstractions; +using TaskStatus = ClaudeDo.Data.Models.TaskStatus; +using Xunit; + +namespace ClaudeDo.Worker.Tests.Runner; + +/// Verifies ResolveConfigAsync's model → preset resolution reaches the CLI args: a full +/// model id must hit its alias's preset row (not the hardcoded unknown-model fallback), and +/// an unknown model must still start a run, falling back no harder than AppSettings.DefaultMaxTurns. +public sealed class ModelResolutionWireTests : IDisposable +{ + private readonly DbFixture _db = new(); + private readonly string _tempDir; + private readonly WorkerConfig _cfg; + + public ModelResolutionWireTests() + { + _tempDir = Path.Combine(Path.GetTempPath(), $"cd_modelwire_{Guid.NewGuid():N}"); + Directory.CreateDirectory(_tempDir); + _cfg = new WorkerConfig { SandboxRoot = _tempDir, LogRoot = _tempDir }; + } + + public void Dispose() { _db.Dispose(); try { Directory.Delete(_tempDir, true); } catch { } } + + private (TaskRunner Runner, IReadOnlyList Args) BuildRunnerCapturingArgs(out Func> getArgs) + { + var dbFactory = _db.CreateFactory(); + var state = TaskStateServiceBuilder.Build(dbFactory).State; + var wt = new WorktreeManager(new GitService(), dbFactory, _cfg, NullLogger.Instance); + IReadOnlyList? captured = null; + var fake = new FakeClaudeProcess((_, _, args, _, _) => + { + captured = args; + return Task.FromResult(new RunResult { ExitCode = 0, ResultMarkdown = "ok" }); + }); + var runner = new TaskRunner(fake, dbFactory, new HubBroadcaster(new CapturingHubContext()), wt, + new ClaudeArgsBuilder(), _cfg, NullLogger.Instance, state, new TaskRunTokenRegistry(), + new AttachmentStore(), new FakeSessionSkillSeeder()); + getArgs = () => captured!; + return (runner, Array.Empty()); + } + + private async Task SeedTaskAsync(string listModel) + { + using var ctx = _db.CreateContext(); + ctx.Lists.Add(new ListEntity { Id = "l1", Name = "L", WorkingDir = null, CreatedAt = DateTime.UtcNow }); + ctx.ListConfigs.Add(new ListConfigEntity { ListId = "l1", Model = listModel }); + ctx.Tasks.Add(new TaskEntity + { + Id = "t1", ListId = "l1", Title = "Task", Status = TaskStatus.Idle, CreatedAt = DateTime.UtcNow, + }); + await ctx.SaveChangesAsync(); + } + + [Fact] + public async Task Full_model_id_resolves_to_its_alias_preset_row() + { + await SeedTaskAsync(listModel: "claude-sonnet-4-6"); + var (runner, _) = BuildRunnerCapturingArgs(out var getArgs); + + using (var ctx = _db.CreateContext()) + await runner.RunAsync((await new TaskRepository(ctx).GetByIdAsync("t1"))!, "slot-1", CancellationToken.None); + + var args = getArgs().ToList(); + var sonnetPreset = ModelPresets.For(ModelPresets.Defaults, "sonnet"); + + Assert.Contains("--effort", args); + Assert.Equal(sonnetPreset.Effort, args[args.IndexOf("--effort") + 1]); + Assert.Contains("--max-turns", args); + Assert.Equal(sonnetPreset.MaxTurns.ToString(), args[args.IndexOf("--max-turns") + 1]); + } + + [Fact] + public async Task Unknown_model_still_starts_a_run_and_uses_the_global_default_max_turns() + { + await SeedTaskAsync(listModel: "totally-unrecognized-model"); + var (runner, _) = BuildRunnerCapturingArgs(out var getArgs); + + using (var ctx = _db.CreateContext()) + { + var settingsRepo = new AppSettingsRepository(ctx); + var settings = await settingsRepo.GetAsync(); + settings.DefaultMaxTurns = 100; + await ctx.SaveChangesAsync(); + } + + Exception? thrown = null; + try + { + using var ctx = _db.CreateContext(); + await runner.RunAsync((await new TaskRepository(ctx).GetByIdAsync("t1"))!, "slot-1", CancellationToken.None); + } + catch (Exception ex) + { + thrown = ex; + } + + Assert.Null(thrown); + var args = getArgs().ToList(); + Assert.Contains("--max-turns", args); + Assert.Equal("100", args[args.IndexOf("--max-turns") + 1]); + } +}