Skip to content

feat(ui): add priority sorting to the task sidebar - #4292

Open
sunheyi6 wants to merge 5 commits into
apache:mainfrom
sunheyi6:codex/issue-1737-priority-sort-pr
Open

feat(ui): add priority sorting to the task sidebar#4292
sunheyi6 wants to merge 5 commits into
apache:mainfrom
sunheyi6:codex/issue-1737-priority-sort-pr

Conversation

@sunheyi6

Copy link
Copy Markdown
Contributor

Summary

Older tasks waiting for attention can be buried by recently updated conversations. This adds an optional Priority order to the task sidebar while keeping Last updated as the default.

  • Put a compact sorting menu beside the existing grouping control and persist the selection locally.
  • Rank waiting-for-user, actionable blocked, live running, unread, then ordinary tasks; break ties by recency and ID.
  • Preserve existing pinned sections and Project/Runtime Host group boundaries. Sorting does not change selection, mark tasks read, or affect execution.

Refs #1737 (ordering only). Activity filtering, manual ordering, and child-to-parent attention aggregation are outside this PR.

Verification

Validated in an isolated checkout on Windows with Node 24.19.0:

  • npm run build:test, npm run build, npm run typecheck, npm run lint, npm run format:check — passed.
  • npx knip --workspace apps/desktop and npx knip --workspace packages/ui — passed.
  • UI suite: 278 passed; sidebar layout/status unit tests: 14 passed.
  • Electron E2E: sidebar-project-row.spec.ts and session-rail-render-contract.spec.ts — 4 passed, including sorting persistence after reload and the row-render budget.
  • Checked the Storybook sorting interaction and compact menu; comparison screenshots are supplied in a follow-up comment. The full desktop E2E suite was not run locally.

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex implemented the UI, tests, and documentation, ran validation, and prepared this PR and screenshot comment at the contributor's request. The commit includes a Generated-by: OpenAI Codex trailer.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No
中文说明

改动概述

较早就需要关注的任务容易被新更新的会话埋没。本 PR 为任务侧栏增加可选的“优先级”排序,默认仍为“最近更新”。

  • 排序菜单收成小图标,放在现有分组控件右侧,选择会保存在本地。
  • 依次排列待用户确认、需要处理的阻塞、实时运行中、未读和普通任务;同级按更新时间及 ID 排序。
  • 保留现有置顶分区及项目、Runtime Host 分组边界,不切换当前任务、不标记已读,也不改变执行顺序。

关联 #1737,仅实现排序部分。本次不包含活动筛选、手动排序或子任务关注状态向父任务汇总。

验证

在独立工作目录中,使用 Windows 和 Node 24.19.0 验证:

  • npm run build:testnpm run buildnpm run typechecknpm run lintnpm run format:check 全部通过。
  • desktop 和 ui 工作区的 knip 检查通过。
  • UI 测试 278 项通过;侧栏布局、状态相关单元测试 14 项通过。
  • Electron 端到端测试 sidebar-project-row.spec.tssession-rail-render-contract.spec.ts 共 4 项通过,包括刷新后恢复排序偏好和任务行渲染开销检查。
  • 已检查 Storybook 排序交互和紧凑菜单,对比截图放在后续评论中。未在本地运行完整 desktop 端到端测试集。

AI 使用说明

OpenAI Codex 参与了 UI、测试和文档实现,执行验证,并根据贡献者要求准备本 PR 及截图评论。提交包含 Generated-by: OpenAI Codex 标记。

检查清单

  • 测试覆盖改动,移除实现后会失败
  • lint、格式、类型检查及相关测试在本地通过
  • 存在行为变化,已在概述中说明

@github-actions github-actions Bot added the effort/M Under 500 readable lines label Aug 31, 2026
@sunheyi6

Copy link
Copy Markdown
Contributor Author

Visual comparison for commit 5d24c3c59 — captured from the Storybook fixture using the production sidebar components. These are synthetic tasks, not private conversations.

The same two-day-old task waiting for confirmation moves from the bottom to the first unpinned position in Priority mode. The pinned task and selected conversation stay unchanged.

Last updated (default) Priority
Last updated: recent ordinary tasks appear first Priority: waiting and actionable blocked tasks appear first

The sorting menu sits beside the grouping control, without taking a separate row. Hovering the icon identifies the current order; the menu marks the selected option.

Compact sorting menu beside the grouping control

Posted by OpenAI Codex at the contributor's request. Screenshot assets are stored on a separate branch in the contributor's fork and are not part of the code diff.

中文说明

以上是提交 5d24c3c59 的效果对比,使用正式侧栏组件从 Storybook 演示场景截图。任务均为模拟数据,不包含私人会话。

左图为默认的“最近更新”,右图为“优先级”。同一条两天前就等待确认的任务,从列表末尾升到非置顶任务的第一位;置顶任务和当前选中的会话保持不变。

第三张图展示排序菜单的位置:紧邻分组控件,不单独占一行。悬停图标可查看当前排序,菜单内标记已选项。

本评论由 OpenAI Codex 按贡献者要求发布。截图保存在贡献者 fork 的独立分支中,不包含在代码差异里。

Restore the existing Desktop blocked-state helper instead of adding a UI dependency to the legacy renderer module. Keep the sidebar priority ordering unchanged.

Generated-by: OpenAI Codex
Regenerate the Astryx surface inventory to include the sorting dropdown components.

Generated-by: OpenAI Codex

@hqhq1025 hqhq1025 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.

No P0-P3 findings on exact head 35930601d05d95969aef1707874b0ce2ec65682d.

This change adds a persisted Last updated / Priority ordering mode to the task sidebar. Priority ordering is a deterministic presentation-only projection: waiting-for-user, actionable blocked, live running/streaming, unread, then ordinary sessions, with timestamp and session ID tie-breakers. It preserves pinned membership, linked-child filtering, and project/runtime-host group boundaries. The follow-up commit also refreshes the generated Astryx surface inventory; npm run astryx:surface-inventory now passes.

I reviewed the full PR diff and the session-state, grouping, persistence, accessibility, and renderer paths. Local verification passed: clean npm ci, npm run build:test, full typecheck, UI tests (278/278), focused sorting/layout/status tests (24/24), renderer architecture checks (62/62), changed-file Biome, git diff --check, clean merge-tree against current main, Desktop build, and focused Electron E2E (4/4). GitHub's exact-head CI has passed its inventory, build, typecheck, static-analysis, and affected-workspace-test stages and is still running the broader Desktop E2E tail at review time.

Not independently verified: the full hosted E2E/Storybook tail is not yet terminal, and I did not test alternative desktop platforms beyond the Linux/Xvfb environment.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

@sunheyi6

Copy link
Copy Markdown
Contributor Author

@Astro-Han cc

@Astro-Han Astro-Han 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 clear Phase 1 scope. The display-only projection, stable sorting, persistence, grouping boundaries, and tests look coherent; I did not find a P0–P2 correctness issue. I left two P3 simplification/performance suggestions below. This was an AI-assisted review using Codex; I independently checked the production composition, tests, and exact-head CI. Please push back if these ownership boundaries are intentionally separate.

Comment thread packages/ui/src/session-status-presentation.ts Outdated

@hqhq1025 hqhq1025 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.

No P0-P3 findings on exact head a4d7e2491d26fe536cf9042c74227b46e7122112.

The latest commit resolves the two earlier maintainability concerns without changing the priority-ordering contract. The actionable-blocked classifier now has one shared owner in @maka/core/session, consumed by both Desktop normalization and UI priority sorting. AppShell now subscribes only to the rail's collapsed and width geometry, while grouping and sorting preferences remain local to the rail. The regression test confirms that sort/view changes do not rerender the shell reader and that geometry changes still do.

I re-reviewed the complete PR diff and the latest 9-file delta, including persisted sort state, priority buckets, pinned and project/runtime-host group boundaries, live-running projection, renderer ownership, and accessibility. Local verification passed: clean npm ci, npm run build:test, full workspace typecheck, UI tests (278/278), focused Desktop state/layout/navigation tests (25/25), Astryx inventory, renderer architecture fixtures (62/62) plus the live architecture check, AppShell hook-scope check, changed-file Biome, git diff --check, Desktop production build, clean merge-tree against current main, and the sidebar Electron E2E suite (3/3).

The full Desktop Node suite reported 1,727 passes and 8 cancellations in the unchanged MCP OAuth deadline tests under Node 22.22.1; those are the same environment-sensitive cancellations seen outside this PR's files, with no ordinary test failures. GitHub's windows_recovery check is green; the broader hosted test job is still running at review time. I did not test alternative desktop platforms beyond Linux/Xvfb.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

@Astro-Han Astro-Han 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 sorting addition. I used Codex as a review aid and independently checked this against the current Astryx component sources and Maka design rules.

Comment thread apps/desktop/src/renderer/styles/sidebar.css Outdated

@hqhq1025 hqhq1025 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.

No P0-P3 findings on exact head ff5b8390f4e36baee3522616cef8dbda7313447e.

The latest commit resolves the remaining Astryx-boundary concern without changing the sidebar sorting contract. The grouping control now uses the published layout="hug" composition, the product CSS no longer reaches into [role="radio"] to replace item padding, and the English visible label is shortened to All while retaining All tasks as its accessible description.

I re-reviewed the complete PR diff and the latest three-file delta, including priority buckets, pinned and project/runtime-host boundaries, persisted grouping/sort state, AppShell subscription scope, Astryx composition, and accessibility. Local verification passed: clean npm ci, npm run build:test, full workspace typecheck, UI tests (278/278), focused Desktop tests (25/25), Astryx surface inventory, renderer architecture fixtures (62/62) plus the live architecture check, changed-file Biome, git diff --check, Desktop production build, the sidebar Electron E2E suite (3/3), and a real Storybook browser probe at the 180px minimum rail width in Chinese and English. Both locales kept grouping and sorting on one row without overlap or clipping. The exact head also merges cleanly with current main, and GitHub's test and windows_recovery checks are green.

Not independently verified: native rendering on Windows or macOS; the browser and Electron visual checks ran on Linux/Xvfb.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

@github-actions github-actions Bot added effort/L Under 1000 readable lines and removed effort/M Under 500 readable lines labels Aug 31, 2026

@Astro-Han Astro-Han 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 exploring this. The underlying need—helping users find tasks that require attention—seems reasonable, but I think we should settle the product model in a Discussion before merging an implementation.

At the moment, the same row combines two different interaction patterns: an always-visible segmented control for grouping and an icon-only menu for sorting. The sorting button is difficult to understand without opening it, does not make the active mode visible, and feels inconsistent with the neighbouring control and the rest of the sidebar design language. “Priority” may also be mistaken for execution priority, while the implementation is actually a presentation-only ordering based on several hidden status buckets.

Could we first use #1737, or a linked Discussion, to agree on whether this should be priority sorting, an attention filter, or another model—and how the current state and promotion reason should be communicated? Once that decision is anchored, the implementation should be easier to review against a clear product contract.

This is a suggestion from an outside perspective, so please feel free to push back if an existing design decision or Astryx pattern already establishes this interaction. I am leaving a Review Comment rather than Request Changes.

This was an AI-assisted review using Codex. I independently verified exact head ff5b839, the implementation and current Astryx composition, and I own this review.

中文对照

谢谢你探索这个方向。帮助用户找到真正需要关注的任务,这个需求本身是合理的;但我认为,在合入具体实现之前,我们应该先通过 Discussion 明确产品模型。

目前同一行混合了两种不同的交互模式:分组使用始终可见的分段控件,而排序使用只有图标的菜单。用户不打开菜单就很难理解这个按钮,也看不到当前采用的排序方式;它与相邻控件以及侧栏其余部分的设计语言并不一致。“优先级”还容易被理解为任务执行优先级,但当前实现实际上只是根据多个隐藏状态层级进行展示排序。

建议先在 #1737 或关联的 Discussion 中确认:我们需要的是优先级排序、仅显示需关注任务的筛选,还是另一种模型;同时明确如何展示当前模式,以及用户如何知道某个任务为什么被提前。产品契约确定后,再审查具体实现会更清楚。

这只是站在外部评审视角给出的建议;如果已有设计决定或 Astryx 规范明确支持这种交互,也欢迎指出或 push back。本次仅提交 Review Comment,不 Request Changes。

本次审查使用 Codex 辅助;我独立核验了精确 head ff5b839、实现和当前 Astryx 组件组合,并对本次评审负责。

@sunheyi6

Copy link
Copy Markdown
Contributor Author

@Astro-Han thanks for the review — the product question is the part I can answer directly, so here is the intent behind the design:

Sorting, not filtering. This is deliberately an ordering mode rather than an attention filter: every session stays in the list, and the two modes cover two retrieval mentalities — "most recent" for what happened, and this mode for "what needs me now". Filtering would hide sessions and make users wonder whether something disappeared; sorting promotes the attention-worthy rows while keeping the list complete.

On the "Priority" name: agreed that it reads like execution priority, and I'm happy to change the visible label if maintainers prefer something like "Needs attention" — the ordering is presentation-only, driven by the same status vocabulary the rows already speak (waiting for user → actionable blocked → running → unread). But I'd push back on per-row "why is this promoted" cues: the StatusDot already carries the status, and this is a view-level ordering, not a per-task classification. Adding another label per row would make the sidebar noisier for a fact the user can already get by looking at the row.

On discoverability: the active mode is already visible via the button tooltip and the selected radio item in the menu; beyond that we can document the ordering buckets, and users can simply ask the in-app AI how the order is computed.

I'd suggest not letting the naming question block the merge — the feature is opt-in and client-side only, and the label can be settled in parallel, e.g. in #1737 or a linked Discussion. Happy to change it once there's a decision.

中文说明

@Astro-Han 感谢评审,产品问题我可以直接回答,说明如下:

  1. 这是排序而不是筛选:有意做成排序模式而非关注筛选——所有会话都保留在列表中,两种模式对应两种查找方式:"按时间"找最近发生的,"按需关注"找现在需要处理的。筛选会把会话藏起来,用户会担心"是不是有东西不见了";排序既把值得关注的行提到前面,又保持列表完整。

  2. 关于 "Priority" 命名:同意它容易被理解成执行优先级,如果维护者倾向 "Needs attention" 之类的名字,很乐意改——排序只影响展示,依据的是行上已有的状态词汇(等待用户 → 可行动的阻塞 → 运行中 → 未读)。但不认同每行加"为什么被提前"的提示:StatusDot 已经承载状态,这只是视图级排序,不是对任务的分类;再给每行加标签只会让侧栏更吵,而这个信息用户看行本身就能得到。

  3. 可发现性:当前模式已通过按钮 tooltip 和菜单中选中的 radio 项可见;此外可以写文档说明排序层级,用户也可以直接问应用内 AI 排序是怎么计算的。

建议不要让命名问题阻塞合入——该功能可选且纯客户端,标签可以在 #1737 或关联 Discussion 里并行确定,一旦有决定我就改。

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

Labels

effort/L Under 1000 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants