Skip to content

feat(kanban): add read-only task summary tree - #31362

Open
SatoDri wants to merge 1 commit into
NousResearch:mainfrom
SatoDri:feat/kanban-summary-tree-readonly
Open

feat(kanban): add read-only task summary tree#31362
SatoDri wants to merge 1 commit into
NousResearch:mainfrom
SatoDri:feat/kanban-summary-tree-readonly

Conversation

@SatoDri

@SatoDri SatoDri commented May 24, 2026

Copy link
Copy Markdown

Summary

Reopens the Kanban summary-tree work from #29247 as a clean, re-scoped branch — now rebased onto current origin/main and updated for the review feedback on this PR.

This PR only ships the read-only selected-task summary tree:

  • Adds GET /tasks/{id}/summary-tree for a task and its descendants.
  • Surfaces per-node task status, links, latest run summary/metadata, display result, comments, blocked context, and artifact metadata.
  • Renders the summary tree in the existing task drawer with a focused bundle delta.
  • Keeps artifacts read-only/contextual: no local file opener, no POST /artifacts/open endpoint.
  • Does not scrape paths from free-form comments; artifact listing comes from structured task/workspace/run metadata and first-class task attachments only.

Changes since the last review

Addressing the hermes-sweeper review (both inline comments):

  1. The tree now renders every field its payload promises. New SummaryNodeRun (latest run profile · outcome · age, with the run's raw metadata behind a read-only <details>), SummaryNodeComments (important comments win over the recent tail, full comment_count in the header), and SummaryNodeLinks (parent/child ids) render alongside the existing result/blocked/artifact sections. Covered by a new behavioral bundle test (test_dashboard_bundle_renders_summary_tree_comments_links_and_runs).
  2. First-class task attachments are in the payload. _attachments_for_many batch-loads task_attachments rows for all traversed ids in one query; each row becomes a read-only artifact record (kind="attachment", source="task.attachment", openable=False) with attachment_id so the UI can reuse the existing authenticated GET /attachments/{id} download route. Nodes also expose attachment_count. Covered by test_task_summary_tree_includes_descendant_uploaded_attachments, which uploads through the real endpoint and also covers the missing-blob → availability="missing" case. Still no local-file open path.

Also folded in while rebasing:

  • Rebased onto current origin/main; the drawer integration now composes with the Attachments section, the Done-card final-result block, and Child Results instead of the pre-attachments drawer layout.
  • Tree section strings go through the dashboard's tx()/useI18n() i18n helpers like the rest of the drawer.
  • The two summary-tree bundle tests read the bundle with encoding="utf-8" so they pass on Windows checkouts too (bare read_text() decodes as cp1252 there; the other bundle tests share that pre-existing issue and are left for a separate cleanup).
  • A truncation notice renders when the traversal hits the defensive node/depth caps.

Test Plan

  • git diff --check on the four touched files
  • node --check plugins/kanban/dashboard/dist/index.js
  • python -m pytest tests/plugins/test_kanban_dashboard_plugin.py -q -o 'addopts=' — 114 passed (the 16 remaining failures are the pre-existing bare-read_text() cp1252 issue on Windows runners only; they fail identically on origin/main)
  • python -m pytest tests/plugins/test_kanban_attachments.py -q -o 'addopts=' — passed
  • python -m pytest tests/hermes_cli/ tests/tools/test_kanban_tools.py -q -o 'addopts=' -k kanban — failure set identical to pristine origin/main on the same machine (no regressions from this diff)
  • ruff check on the touched Python files

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins labels May 24, 2026

@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 resubmission. It follows the #29247 guidance to keep this read-only and excludes the local-open and comment-regex mechanisms.

Problems

  • TaskSummaryTreeSection renders only result, blocked context, and artifacts (plugins/kanban/dashboard/dist/index.js PR lines 3027-3038). The payload also supplies comments, important comments, links, and latest-run metadata, but none of those fields is rendered. This falls short of the stated summary-tree contents.
  • The PR's artifact extraction only uses workspace and run metadata (plugins/kanban/dashboard/plugin_api.py PR lines 742-763). Current main now has first-class task attachments (plugins/kanban/dashboard/plugin_api.py:560), so uploaded artifacts on descendant tasks are missing from the tree.

Suggested changes

  • Render the promised comment/link/run fields and add a behavioral bundle test for them.
  • Batch-load descendant task attachments into the read-only payload and cover an uploaded attachment in the summary-tree tests; do not add a local-file open path.

Automated hermes-sweeper review.

),
result ? h(MarkdownBlock, { source: result, enabled: props.renderMarkdown }) : null,
h(BlockedSummaryPanel, { block: node.block }),
h(ArtifactList, { artifacts: node.artifacts || [] }),

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.

The payload includes comments, important_comments, links, and latest_run.metadata, but this renderer only consumes result, block, and artifacts. Please render the fields this summary tree claims to surface (or narrow that contract) and add a behavioral UI/bundle test.

workspace_kind=workspace_kind,
))

for run in runs:

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 artifact walk only considers run metadata. Current main also has first-class task attachments (get_task serializes kanban_db.list_attachments), so uploaded attachments on descendant tasks disappear from this tree. Batch-load attachment metadata for the traversed IDs and cover that case without introducing a local-file opener.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 13, 2026
Adds GET /tasks/{id}/summary-tree and renders the tree in the task
drawer: per-node status, links, latest run summary/metadata, display
result, comments, blocked context, and artifact metadata (run metadata
paths plus first-class task attachments), all read-only — no local
file opener and no comment-path scraping.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@SatoDri
SatoDri force-pushed the feat/kanban-summary-tree-readonly branch from 207422d to 110f3ca Compare July 13, 2026 21:10
@SatoDri

SatoDri commented Jul 14, 2026

Copy link
Copy Markdown
Author

Thanks for the review — both points are addressed in the updated branch (also rebased onto current main, so the conflict is gone):

1. Promised fields now render. TaskSummaryTreeSection gained three subcomponents: SummaryNodeRun (latest run profile · outcome · age · run count, the run summary when an explicit task result shadows it, and the raw metadata behind a read-only <details>), SummaryNodeComments (important comments, falling back to the recent tail, with the full comment_count in the header), and SummaryNodeLinks (parent/child ids). Behavioral bundle coverage: test_dashboard_bundle_renders_summary_tree_comments_links_and_runs.

While wiring the UI I noticed _important_comments back-filled with ordinary comments, which scrambled chronological order and made the payload's comments field redundant — it now returns only term-matching comments in chronological order, pinned by test_task_summary_tree_important_comments_stay_chronological_without_backfill.

2. First-class attachments are in the payload. _attachments_for_many batch-loads task_attachments rows for all traversed ids in one query; each becomes a read-only artifact record (kind="attachment", source="task.attachment", openable=False) carrying attachment_id so the UI can reuse the existing authenticated GET /attachments/{id} route. Nodes also expose attachment_count. Covered by test_task_summary_tree_includes_descendant_uploaded_attachments, which uploads through the real endpoint and also asserts the missing-blob → availability="missing" case. Still no local-file open path.

Also folded in during the rebase: _links_for_many now issues two N-parameter queries instead of one 2N-parameter query (2×500 ids exceeded the 999-variable default on SQLite < 3.32 exactly at the traversal cap), the tree section resets its state when the drawer switches tasks so a stale tree never renders under the new task, tree strings go through tx()/useI18n() like the rest of the drawer, and the two summary-tree bundle tests read the bundle with encoding="utf-8" so they pass on Windows checkouts (the other bundle tests share that pre-existing bare-read_text() issue — happy to fix those in a separate PR if wanted).

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-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants