Skip to content

fix(kanban): pass skills from decomposer to child tasks - #28875

Open
szocpaul wants to merge 1 commit into
NousResearch:mainfrom
szocpaul:fix/kanban-decompose-skills
Open

fix(kanban): pass skills from decomposer to child tasks#28875
szocpaul wants to merge 1 commit into
NousResearch:mainfrom
szocpaul:fix/kanban-decompose-skills

Conversation

@szocpaul

Copy link
Copy Markdown

What

The Kanban decomposer LLM was never able to pass skill bundles (skills) to child tasks during decomposition. This was because:

  1. kanban_decompose.py did not extract skills from the decomposer's JSON response and did not include it in the children dict passed to the DB
  2. kanban_db.py decompose_triage_task() INSERT omitted the skills column entirely

This PR fixes both:

  • kanban_decompose.py: Adds skills to the decomposer system prompt schema so the LLM knows it can include skill bundles. Extracts and validates skills from the LLM response and includes it in the children dict.
  • kanban_db.py: Adds the skills column to the INSERT statement in decompose_triage_task(), JSON-serializes the skills list, and updates the docstring to document the new field.

Testing

All 167 kanban tests pass (9 decompose + 158 DB).

Impact

This is a bug fix — the skills field was always supported by create_task() but the decomposer path was bypassing it entirely. Existing tasks are unaffected; only new decompositions will now correctly propagate skill bundles to child tasks.

The decomposer LLM was never able to pass skill bundles to
child tasks because:

1. kanban_decompose.py did not extract `skills` from the LLM
   response and did not include it in the children dict
2. kanban_db.py `decompose_triage_task()` INSERT omitted the
   `skills` column entirely

This commit:
- Adds `skills` to the decomposer system prompt schema so the
  LLM knows it can include skill bundles in its response
- Extracts and validates `skills` from the decomposer JSON
- Inserts `skills` into the child task row (JSON-serialized)

All 167 kanban tests pass (9 decompose + 158 DB).

@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 the focused fix. I verified the premise on current main: the decomposer still drops skills before the DB call, while create_task/worker spawn already support per-task skills.

Problems

  • The PR's DB normalization is weaker than the existing create_task path. The added list comprehension at hermes_cli/kanban_db.py:3238 filters non-empty strings but stores the unstripped value and does not mirror create_task's dedupe/comma/toolset-name validation from current main hermes_cli/kanban_db.py:2118-2161.
  • There is no regression test for the actual fixed path. Current decompose tests cover fan-out and workspace behavior around tests/hermes_cli/test_kanban_decompose_db.py:35-230, but none assert that child skills round-trip after decompose_triage_task.
  • The DB hunk is stale against current main: the live INSERT at hermes_cli/kanban_db.py:4515-4530 preserves workspace_kind/workspace_path inheritance, while this diff edits an older scratch-only INSERT.

Suggested changes

  • Reuse or mirror create_task's skill normalization for decomposed children.
  • Add a decompose_triage_task regression that asserts kb.get_task(conn, child_id).skills equals the child skills list.
  • Apply the skills column to the current workspace-preserving INSERT path.

Automated hermes-sweeper review.

Comment thread hermes_cli/kanban_db.py
body = child.get("body")
assignee = _canonical_assignee(child.get("assignee"))
# Extract optional skills from the child dict.
raw_skills = child.get("skills")

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.

This should mirror create_task's skills normalization: strip the stored value, dedupe, reject comma-containing names, and reject known toolset names so decomposed children behave like tasks created through the normal path.

parents = []
# Clean parent indices: drop non-int and out-of-range.
clean_parents = [p for p in parents if isinstance(p, int) and 0 <= p < len(raw_tasks) and p != idx]
# Extract optional skills list from the decomposer response.

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.

This keeps the original string rather than s.strip(), so a decomposer response like " translation " would persist a skill name with spaces unless the DB layer normalizes it later.

@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 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins 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.

3 participants