feat(ui): add priority sorting to the task sidebar - #4292
Conversation
Generated-by: OpenAI Codex
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
left a comment
There was a problem hiding this comment.
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.
|
@Astro-Han cc |
Astro-Han
left a comment
There was a problem hiding this comment.
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.
hqhq1025
left a comment
There was a problem hiding this comment.
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
left a comment
There was a problem hiding this comment.
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.
hqhq1025
left a comment
There was a problem hiding this comment.
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.
Astro-Han
left a comment
There was a problem hiding this comment.
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 组件组合,并对本次评审负责。
|
@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 感谢评审,产品问题我可以直接回答,说明如下:
建议不要让命名问题阻塞合入——该功能可选且纯客户端,标签可以在 #1737 或关联 Discussion 里并行确定,一旦有决定我就改。 |



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.
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/desktopandnpx knip --workspace packages/ui— passed.sidebar-project-row.spec.tsandsession-rail-render-contract.spec.ts— 4 passed, including sorting persistence after reload and the row-render budget.AI use
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 Codextrailer.Checklist
Does this PR entail a change in behavior?
中文说明
改动概述
较早就需要关注的任务容易被新更新的会话埋没。本 PR 为任务侧栏增加可选的“优先级”排序,默认仍为“最近更新”。
关联 #1737,仅实现排序部分。本次不包含活动筛选、手动排序或子任务关注状态向父任务汇总。
验证
在独立工作目录中,使用 Windows 和 Node 24.19.0 验证:
npm run build:test、npm run build、npm run typecheck、npm run lint、npm run format:check全部通过。sidebar-project-row.spec.ts和session-rail-render-contract.spec.ts共 4 项通过,包括刷新后恢复排序偏好和任务行渲染开销检查。AI 使用说明
OpenAI Codex 参与了 UI、测试和文档实现,执行验证,并根据贡献者要求准备本 PR 及截图评论。提交包含
Generated-by: OpenAI Codex标记。检查清单