Skip to content

fix(kanban): add --model to create + fix dispatcher -m placement (#67459) - #67518

Closed
Enough1122 wants to merge 2 commits into
NousResearch:mainfrom
Enough1122:fix/67459-kanban-model-override
Closed

fix(kanban): add --model to create + fix dispatcher -m placement (#67459)#67518
Enough1122 wants to merge 2 commits into
NousResearch:mainfrom
Enough1122:fix/67459-kanban-model-override

Conversation

@Enough1122

Copy link
Copy Markdown
Contributor

Fixes #67459.

Two independent bugs made kanban's model_override feature unusable:

1. No write surface

hermes kanban create had no --model option, and create_task() had no model_override parameter. The column existed in the schema but nothing set it.

Fix: add --model to the CLI create subparser (hermes_cli/kanban.py), pass it through _cmd_createcreate_task(), and include it in the INSERT statement (hermes_cli/kanban_db.py).

2. Dispatcher -m placement discarded by argparse

_default_spawn placed -m <model> BEFORE the chat subcommand. The chat subparser redefines -m/--model with a None default, which on Python argparse overwrites the top-level value. So even a manually-set model_override reached the worker as None.

Fix: move -m AFTER the chat subcommand in the argv list, so the chat subparser's default is the effective value and user-provided -m wins.

Test

$ python -m pytest tests/agent/test_kanban_stop.py -q
9 passed in 0.61s

Imports confirmed: from hermes_cli.kanban_db import create_task and from hermes_cli.kanban import build_parser both work.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for wiring the missing CLI write path through to task persistence. That part addresses a real current-main gap.

Problems

  • The dispatcher change is based on stale parser behavior. hermes_cli/_parser.py:280-295 already uses argparse.SUPPRESS for the chat subparser's inherited -m, and tests/hermes_cli/test_kanban_worker_spawn_toolsets.py:128-170 verifies the existing pre-chat worker argv preserves the override. The new comment in hermes_cli/kanban_db.py therefore describes behavior main no longer has.
  • Please add coverage for the new CLI-to-DB path. The existing worker-spawn test manually assigns Task.model_override and does not exercise kanban create --model, _cmd_create, or the INSERT persistence.

Suggested changes

  • Keep the CLI/DB write-surface change, but remove the redundant dispatcher relocation or update its rationale.
  • Add an isolated-HERMES_HOME test that creates a task with --model and asserts the stored task has that override.

Automated hermes-sweeper review.

Comment thread hermes_cli/kanban.py
@@ -1368,6 +1373,7 @@ def _cmd_create(args: argparse.Namespace) -> int:
max_runtime_seconds=max_runtime,
skills=getattr(args, "skills", None) or None,
max_retries=max_retries,
model_override=getattr(args, "model", None),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add an isolated-HERMES_HOME regression test for this new CLI-to-DB path: parse/create with --model, then reload the task and assert model_override. The existing spawn test assigns the field directly, so it does not cover this forwarding call.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 19, 2026
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cron Cron scheduler and job management needs-decision Awaiting maintainer decision before any implementation labels Jul 19, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #67459: this is the fix PR for the missing Kanban model-override write path and dispatch argument placement.

@Enough1122

Copy link
Copy Markdown
Contributor Author

cc @teknium1 — addressed both sweeper findings in commit 8089ec72b (now at PR head):

  1. Stale dispatcher relocation — reverted. hermes_cli/_parser.py:280-295 already uses argparse.SUPPRESS for the chat subparser's inherited -m, and tests/hermes_cli/test_kanban_worker_spawn_toolsets.py:128-170 already covers the pre-chat worker argv preserves the override. The dispatcher hunk in hermes_cli/kanban_db.py is gone; only the rationale comment for the write-surface change remains.

  2. CLI-to-DB coverage — added test_run_slash_create_with_model_override_persists in tests/hermes_cli/test_kanban_cli.py. The test uses the existing kanban_home fixture (isolated HERMES_HOME), runs hermes kanban create 'test-model-task' --assignee alice --model gpt-5.6-sol --json, parses the returned task ID, then reads back via kb.get_task(conn, task_id) and asserts task.model_override == "gpt-5.6-sol".

Final diff vs main:

  • hermes_cli/kanban.py--model on create subparser, plumbed through _cmd_createcreate_task()
  • hermes_cli/kanban_db.pymodel_override column added to the INSERT statement
  • tests/hermes_cli/test_kanban_cli.py — new test_run_slash_create_with_model_override_persists (+14)

Local: new test passes; the existing kanban_worker_spawn_toolsets regression (pre-chat argv preserves -m) is unchanged on this branch. Ready for another sweep pass.

— written by Hermes Agent on behalf of @Enough1122

…sResearch#67459)

Two independent bugs made kanban's model_override feature unusable:

1. **No write surface.** `hermes kanban create` had no `--model` option,
   and `create_task()` had no `model_override` parameter. The column
   existed in the schema but nothing set it. Fix: add `--model` to the
   CLI create subparser, pass it through `_cmd_create` → `create_task()`,
   and include it in the INSERT statement.

2. **Dispatcher `-m` placement discarded by argparse.** `_default_spawn`
   placed `-m <model>` BEFORE the `chat` subcommand. The `chat`
   subparser redefines `-m/--model` with a `None` default, which on
   Python argparse overwrites the top-level value. Fix: move `-m` AFTER
   the `chat` subcommand in the argv list, so the chat subparser's
   default is the effective value and user-provided `-m` wins.

Now `hermes kanban create --model openrouter/anthropic/claude-sonnet-4 ...`
actually works, and the dispatcher passes the override to the worker.

Fixes NousResearch#67459.
…ousResearch#67518 sweeper)

Reverts the dispatcher -m relocation (main already uses
argparse.SUPPRESS for the chat subparser's inherited -m, so the
model_override reaches the worker correctly via the pre-chat position).

Adds a test that creates a task with --model and asserts the stored
task has the override in the DB.

Addresses: NousResearch#67518 hermes-sweeper review (teknium1, 2026-07-19).
@Enough1122
Enough1122 force-pushed the fix/67459-kanban-model-override branch from 8089ec7 to 6b983d6 Compare July 22, 2026 10:21
@Enough1122

Copy link
Copy Markdown
Contributor Author

cc @teknium1 — rebased onto current upstream main. The fix is unchanged in content (3 files, +25/-8).

Both sweeper findings from 2026-07-19 still addressed:

  1. Stale dispatcher relocation reverted — hermes_cli/_parser.py already uses argparse.SUPPRESS for the chat subparser's -m.
  2. CLI-to-DB test test_run_slash_create_with_model_override_persists (added in 8089ec72b) covers the new kanban create --model write path.

tests/hermes_cli/test_kanban_cli.py 48 passed. git diff --check clean. Ready for another sweep pass.

— written by Hermes Agent on behalf of @Enough1122

@teknium1

Copy link
Copy Markdown
Contributor

Thanks @Enough1122 — this correctly identified and fixed the missing write surface for tasks.model_override, and your follow-ups addressing the sweeper findings were on point.

The gap is now closed on main by #69876 (c1b0f6f), which shipped a superset of this fix: kanban create --model plus a --provider pair (so cross-provider overrides resolve against the right backend), a set-model subcommand, the dashboard model dropdown + PATCH/bulk API, and agent-tool params. You were the first submitter on the core write-surface gap — credit to you for flagging and fixing it first in #67459/#67518.

Closing since the change is now redundant against main. Sorry we couldn't land your commits directly — the broader feature branch was already in flight when the overlap surfaced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cron Cron scheduler and job management needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tasks.model_override: no write surface, and the dispatcher's -m is discarded by the chat subparser

3 participants