Repository navigation
feat(kanban): per-task model override write-path - #39
Conversation
Finishes the half-wired `tasks.model_override` column. The column, its migration, the task-dataclass load, `kanban show`, and the `_default_spawn` `-m` append already existed — but nothing could SET the value. This wires the write path. WHAT: - kanban_db.py: `create_task(model_override=...)` kwarg + INSERT; new `set_task_model(conn, task_id, model)` setter returning rowcount (0 on a nonexistent id — never a silent success); structured spawn log line `kanban spawn task=%s assignee=%s model_override=%s` for cost attribution. - kanban.py: `kanban create --model MODEL`; `kanban edit --model MODEL` / `--clear-model` (None sentinel = "leave untouched", explicit clear flag — no `--model ""` footgun; the two are mutually exclusive). - kanban_tools.py: create action accepts model_override (agent-facing). - Tests: tests/hermes_cli/test_kanban_model_override.py (20) + tests/tools/test_kanban_tools.py (3 new). WHY: Let one task run on a different model than its assignee profile's default without cloning a whole profile (what daedalus-opus required). Spec: ~/.hermes/plans/2026-06-13_kanban-per-task-model-override-SPEC.md (v1.0, 3 review passes). No gateway/run.py change — spawn already honors the column. VERIFIED (Apollo, not just the worker's report): - New file 20 passed; kanban_tools 87 passed; core kanban DB+CLI 233 passed. - Proved the 11 failures in the broad `-k kanban` run are PRE-EXISTING: same 11 fail identically on a clean baseline (stash diff → re-run). With diff: 683 passed; baseline: 663. The diff adds 20 passing tests, breaks nothing. DEFERRED to a separate live-gateway verify (needs gateway reload = approval- gated): blackbox billing proof that an override task bills model X while a sibling bills the profile default; the Phase-0 live fallback probe. See ~/.hermes/plans/2026-06-13_kanban-model-override-LIVE-VERIFY-SPEC.md.
🔎 Lint report:
|
| Rule | Count |
|---|---|
unresolved-attribute |
2 |
invalid-argument-type |
2 |
unresolved-import |
1 |
First entries
tests/hermes_cli/test_kanban_model_override.py:20: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
tests/tools/test_kanban_tools.py:954: [unresolved-attribute] unresolved-attribute: Attribute `model_override` is not defined on `None` in union `Task | None`
tests/hermes_cli/test_kanban_model_override.py:230: [unresolved-attribute] unresolved-attribute: Attribute `model_override` is not defined on `None` in union `Task | None`
tests/hermes_cli/test_kanban_model_override.py:332: [invalid-argument-type] invalid-argument-type: Argument to function `resolve_workspace` is incorrect: Expected `Task`, found `Task | None`
tests/hermes_cli/test_kanban_model_override.py:335: [invalid-argument-type] invalid-argument-type: Argument to function `_default_spawn` is incorrect: Expected `Task`, found `Task | None`
✅ Fixed issues: none
Unchanged: 5123 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
|
| Filename | Overview |
|---|---|
| hermes_cli/kanban.py | Adds --model/--clear-model to create and edit subcommands. The _cmd_edit refactor introduces a dead rc variable and allows partial success when both --model and --result are provided together — model commits but the command exits with code 1 if result backfill fails. Also makes --result optional, which is correct but means --summary/--metadata alone produce a confusing "nothing to edit" error. |
| hermes_cli/kanban_db.py | Adds model_override column to the create_task INSERT and introduces set_task_model with correct rowcount-based error detection. Spawn log line added alongside the existing -m extension. The int(cur.rowcount or 0) pattern is slightly redundant but harmless. |
| tools/kanban_tools.py | Adds model_override to the agent-tool create path with correct type validation (non-string → error). Does not guard against empty string, meaning model_override: "" passes validation, stores "" in the DB, and is silently skipped at spawn time due to the truthiness check in _default_spawn. |
| tests/hermes_cli/test_kanban_model_override.py | Comprehensive new test file covering DB write/clear, CLI flags, argv injection round-trips (including metacharacter safety), retry/re-read path, and observability logging. |
| tests/tools/test_kanban_tools.py | Adds three tests for the kanban_tools create path: persist, null-default, and non-string rejection. |
Sequence Diagram
sequenceDiagram
participant CLI as kanban CLI / tool
participant CMD as _cmd_create / _cmd_edit
participant DB as kanban_db
participant SPAWN as _default_spawn
Note over CLI,SPAWN: Create path
CLI->>CMD: kanban create --model MODEL
CMD->>DB: "create_task(model_override=MODEL)"
DB-->>CMD: task_id
CMD-->>CLI: Created t_xxx
Note over CLI,SPAWN: Edit path
CLI->>CMD: kanban edit TASK_ID --model MODEL
CMD->>DB: set_task_model(conn, task_id, MODEL)
DB-->>CMD: rowcount
alt "rowcount == 0"
CMD-->>CLI: error cannot set model (exit 1)
else "rowcount == 1"
CMD-->>CLI: Set model override on TASK_ID
end
CLI->>CMD: kanban edit TASK_ID --clear-model
CMD->>DB: set_task_model(conn, task_id, None)
DB-->>CMD: rowcount
CMD-->>CLI: Cleared model override on TASK_ID
Note over CLI,SPAWN: Spawn path (pre-existing, unchanged)
SPAWN->>DB: get_task(conn, task_id)
DB-->>SPAWN: task with model_override
alt task.model_override is truthy
SPAWN->>SPAWN: "cmd += [-m, task.model_override]"
SPAWN->>SPAWN: "_log.info(kanban spawn model_override=...)"
end
SPAWN->>SPAWN: subprocess.Popen(cmd)
Reviews (1): Last reviewed commit: "feat(kanban): per-task model override wr..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| rc = 0 | ||
| with kb.connect_closing() as conn: | ||
| if not kb.edit_completed_task_result( | ||
| conn, | ||
| args.task_id, | ||
| result=args.result, | ||
| summary=getattr(args, "summary", None), | ||
| metadata=metadata, | ||
| ): | ||
| print( | ||
| f"cannot edit {args.task_id} (unknown id or task is not done)", | ||
| file=sys.stderr, | ||
| ) | ||
| return 1 | ||
| print(f"Edited {args.task_id}") | ||
| return 0 | ||
| if do_model: | ||
| # --clear-model writes NULL; --model X writes X literally. The | ||
| # None sentinel ("--model omitted") never reaches here. | ||
| new_model = None if clear_model else model_override | ||
| affected = kb.set_task_model(conn, args.task_id, new_model) | ||
| if affected == 0: | ||
| print( | ||
| f"cannot set model on {args.task_id} (unknown id)", | ||
| file=sys.stderr, | ||
| ) | ||
| return 1 | ||
| if clear_model: | ||
| print(f"Cleared model override on {args.task_id}") | ||
| else: | ||
| print(f"Set model override on {args.task_id}: {new_model}") | ||
| if do_result: | ||
| if not kb.edit_completed_task_result( | ||
| conn, | ||
| args.task_id, | ||
| result=args.result, | ||
| summary=getattr(args, "summary", None), | ||
| metadata=metadata, | ||
| ): | ||
| print( | ||
| f"cannot edit {args.task_id} (unknown id or task is not done)", | ||
| file=sys.stderr, | ||
| ) | ||
| return 1 | ||
| print(f"Edited {args.task_id}") | ||
| return rc |
There was a problem hiding this comment.
rc is initialized but never modified
rc = 0 is set once and only ever reaches return rc when all operations succeed — every failure path exits early via return 1. This means the variable is dead code. If the intent was to allow partial-success tracking (e.g., model edit committed but result backfill failed), rc never gets set to a non-zero value, so a caller checking exit code when both --model and --result are supplied can see exit 1 even though the model write already committed and was printed as successful.
| if not do_result and not do_model: | ||
| print( | ||
| "kanban: nothing to edit (pass --result, --model, or --clear-model)", | ||
| file=sys.stderr, | ||
| ) | ||
| return 2 |
There was a problem hiding this comment.
--summary / --metadata
do_result is only set when --result is provided, so kanban edit TASK_ID --summary 'foo' or kanban edit TASK_ID --metadata '{}' alone would hit this branch and print "nothing to edit (pass --result, --model, or --clear-model)" without mentioning the flags the user actually passed. Adding --summary and --metadata to the message (or to do_result) would prevent confusion.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| if goal_bool_error: | ||
| return tool_error(goal_bool_error) | ||
| goal_max_turns = args.get("goal_max_turns") | ||
| model_override = args.get("model_override") | ||
| if model_override is not None and not isinstance(model_override, str): | ||
| return tool_error( |
There was a problem hiding this comment.
model_override passes validation but is silently dropped at spawn
The guard rejects non-string values but allows "". Storing an empty string causes _default_spawn's truthiness check (if task.model_override:) to evaluate as False, so no -m flag is emitted and the spawn silently uses the profile default. The same check in show means show also hides it. An agent or tool caller passing "model_override": "" may believe the override is set when it is not. Adding or model_override == "" to the guard (treating it the same as omitting the field) or documenting it as unsupported would close the gap.
…39, #35 sibling) - #37 import map is module-level; function-local imports bind only in their function - #38 sink_dotted honoured when sink_names is None - #39 a function-local import shadows a same-file def - offload-call args walked (eager), lambda args still deferred New precision arms: 4 red on base, 5/5 green. Consumer gates 32/32; the widened walker surfaced 2 pre-existing telegram get_label->requests.get reaches (same shape as baselined matrix entry), added to REACHABLE_BASELINE.
…39, #35 sibling) - #37 import map is module-level; function-local imports bind only in their function - #38 sink_dotted honoured when sink_names is None - #39 a function-local import shadows a same-file def - offload-call args walked (eager), lambda args still deferred New precision arms: 4 red on base, 5/5 green. Consumer gates 32/32; the widened walker surfaced 2 pre-existing telegram get_label->requests.get reaches (same shape as baselined matrix entry), added to REACHABLE_BASELINE.
1 participant
Finishes the half-wired
tasks.model_overridecolumn (the column + spawn-mappend already existed; nothing could SET it). Addscreate_task(model_override=),set_task_model()(rowcount, no silent success),kanban create/edit --model+--clear-model(None sentinel, explicit clear — no--model ''footgun), agent-tool create support, and a spawn log line for cost attribution.Spec:
~/.hermes/plans/2026-06-13_kanban-per-task-model-override-SPEC.md(v1.0, 3 Opus review passes).No
gateway/run.pychange —_default_spawnalready honors the column.Verified: 20 new tests pass; kanban_tools 87; core kanban DB+CLI 233. The 11 failures in the broad
-k kanbanrun are PRE-EXISTING (identical on a clean baseline: stash diff → same 11 fail; 683 vs 663 passed — the diff adds 20 passing tests, breaks nothing).Deferred (approval-gated gateway reload): live blackbox billing proof + Phase-0 fallback probe — see
2026-06-13_kanban-model-override-LIVE-VERIFY-SPEC.md.