Repository navigation
feat(desktop): re-land sidebar filter/sort with activity-based ordering #6005
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
a78ee9b
Reapply "feat(desktop): filter and sort projects in the dashboard sid…
AviPeltz 2b2f601
fix(desktop): coerce string timestamps so sidebar sort can't crash
AviPeltz 7434d67
feat(desktop): sort workspaces within projects in non-manual sidebar …
AviPeltz ae38cbb
feat(desktop): rank sidebar 'Last updated' sort by real agent activity
AviPeltz be67fa8
docs(desktop): document sidebar activity-sort design and alternatives
AviPeltz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,175 @@ | ||
| # Sidebar "Last updated" sort: activity signal design | ||
|
|
||
| Status: shipped with the re-land of the sidebar filter/sort feature | ||
| (`reland-sidebar-sort-filter`, reapplying #5956 after revert #5996). | ||
| Covers what signal drives the "Last updated" sort, why, the current | ||
| polling implementation, and the alternatives considered — including the | ||
| ones that are "more correct" and what it would take to move to them. | ||
|
|
||
| ## The problem | ||
|
|
||
| The dashboard sidebar offers three sort modes for projects and their | ||
| workspaces: manual, date created, and last updated. The first shipped | ||
| version of "Last updated" sorted on `v2_workspaces.updatedAt` and had | ||
| two defects, one crash and one semantic: | ||
|
|
||
| 1. **Crash**: rows reaching the sidebar through the Electric cloud | ||
| fallback carry ISO **strings** in `updatedAt`/`createdAt` at runtime | ||
| (the Electric client's default parser handles int/bool/float but not | ||
| `timestamptz`, and we pass no custom `parser`), while the Drizzle | ||
| `SelectV2Workspace` type claims `Date`. `workspace.updatedAt.getTime()` | ||
| threw and took the whole sidebar down for anyone with remote | ||
| workspaces. Host-served rows are unaffected (superjson revives real | ||
| `Date`s over tRPC). | ||
| 2. **Semantics**: the host only writes `v2_workspaces.updatedAt` on | ||
| metadata patches — rename, PR linkage, branch changes. Sending an | ||
| agent a message never touches it, so the one thing "Last updated" | ||
| obviously should respond to didn't reorder anything. This is what got | ||
| the feature reverted. | ||
|
|
||
| ## What "activity" means now | ||
|
|
||
| The host-service already tracks exactly the signal we want. Every agent | ||
| hook event POSTs to `notifications.hook`, which normalizes the event | ||
| (`UserPromptSubmit`/`PostToolUse` → `Start`, `Stop`/turn-complete → | ||
| `Stop`, `PreToolUse`/`Notification` → `PermissionRequest`, session | ||
| start/end → `Attached`/`Detached`, agent-agnostic across | ||
| Claude/Codex/OpenCode/Grok) and records it on the terminal's agent | ||
| binding as `lastEventAt` (epoch ms, persisted in the host's SQLite via | ||
| `terminal-agents/persistence.ts`, so it survives host restarts). The | ||
| same feed drives the sidebar working/idle status dots. | ||
|
|
||
| A workspace's effective activity timestamp is: | ||
|
|
||
| ``` | ||
| max(updatedAt, newest lastEventAt across the workspace's agent bindings) | ||
| ``` | ||
|
|
||
| so metadata updates still count, and both directions of a conversation | ||
| count (prompt submit bumps immediately; tool use and turn completion | ||
| keep bumping). Things that intentionally do **not** count: plain | ||
| (non-agent) terminal typing, and merely viewing a workspace. | ||
|
|
||
| Sorting applies the effective timestamp at every level, stably | ||
| (name → id tie-breaks), with NaN-safe coercion for the string-date rows: | ||
|
|
||
| - workspaces within a project and within sections | ||
| (`sortDashboardSidebarProjectChildren`), | ||
| - sections rank among loose workspaces by their newest member, | ||
| - the local main workspace stays pinned first, | ||
| - projects rank by their most-active workspace | ||
| (`getProjectActivityTimestamp`). | ||
|
|
||
| ## Current implementation: per-host polling | ||
|
|
||
| `useDashboardSidebarData` reuses the pull-request query targets (one | ||
| entry per reachable host, local or relay-routed) and runs one | ||
| react-query per host against `terminalAgents.list`: | ||
|
|
||
| - `enabled` only while the sort mode is `"updated"` — manual/created | ||
| users generate zero extra traffic; | ||
| - `refetchInterval: 10_000`; | ||
| - results collapse into `Map<workspaceId, newest lastEventAt>`, | ||
| JSON-fingerprinted for stable identity, then threaded onto each | ||
| `DashboardSidebarWorkspace` as `lastAgentActivityAt` by the builders. | ||
|
|
||
| Properties: | ||
|
|
||
| - **Latency**: 0–10s from agent event to reorder. Measured live: a | ||
| synthetic `UserPromptSubmit` moved an idle workspace to the top of its | ||
| project in ~4s. | ||
| - **Cost**: one cheap host round-trip per host per 10s, only while the | ||
| mode is active and the window focused (react-query pauses intervals in | ||
| background, refetches on focus). | ||
| - **Failure mode**: an unreachable host contributes nothing; its | ||
| workspaces fall back to `updatedAt`. Nothing crashes, order degrades | ||
| gracefully. | ||
|
|
||
| Why polling first: it's stateless, self-healing by construction, and | ||
| identical in shape to the adjacent PR-status queries, so it was the | ||
| lowest-risk way to re-land a previously-reverted feature. The latency is | ||
| its only real cost. | ||
|
|
||
| ## Alternative 1 (recommended next): event-bus push + slow-poll self-heal | ||
|
|
||
| The renderer already holds a WebSocket to each host's event bus (it | ||
| powers the status dots), `notifications.hook` broadcasts | ||
| `agent:lifecycle` on that bus *before* it writes the store, and the bus | ||
| client supports wildcard subscriptions — `bus.on("agent:lifecycle", "*", | ||
| cb)` receives events for every workspace on the host over the existing | ||
| connection. | ||
|
|
||
| Design: | ||
|
|
||
| - one `"*"` listener per reachable host; on event, patch the activity | ||
| map in place via `queryClient.setQueryData` (the payload carries | ||
| `workspaceId` + `occurredAt` — no refetch round trip needed); | ||
| - keep the poll as reconciliation at a long interval (~60s) plus the | ||
| default refetch-on-focus, to catch events missed while a WS was down | ||
| (host restart, laptop sleep). Pure push with no reconciliation drifts | ||
| silently — `useTerminalAgentBindings` documents exactly this failure | ||
| mode and uses the same event+staleTime hybrid one level down. | ||
|
|
||
| Result: effectively instant reordering with *less* steady-state traffic | ||
| than the 10s poll. Contained change inside `useDashboardSidebarData`. | ||
| The only reason it isn't in the initial re-land is sequencing: land the | ||
| proven-simple version first. | ||
|
|
||
| ## Alternative 2 (most correct long-term): persist activity to the cloud | ||
|
|
||
| Both approaches above only rank hosts you can currently reach. The | ||
| fully correct model is server-side: the host (already the writer of | ||
| workspace rows) debounce-bumps a `lastActivityAt` column on | ||
| `v2_workspaces` (or a small activity table) on agent events, and | ||
| Electric pushes it to every device reactively — this is arguably what | ||
| `updatedAt` should have meant all along. | ||
|
|
||
| Gains: cross-device correctness (see that your desktop at home is busy, | ||
| from your laptop), offline-host ranking from last-known cloud state, no | ||
| renderer fan-out at all. | ||
|
|
||
| Costs / open questions, which is why it's deferred: | ||
|
|
||
| - write volume: agent events are chatty; needs debouncing (e.g. ≥30s | ||
| per workspace) and an answer for hosts that are offline mid-burst; | ||
| - schema + API surface: a migration, host write path, and backfill; | ||
| - it must *not* be conflated with `updatedAt`, or every activity bump | ||
| looks like a metadata edit to other consumers; | ||
| - the Electric string-date issue applies to any new timestamp column: | ||
| either fix the missing `parser` in our shape options first, or the new | ||
| column arrives as strings too and every consumer needs coercion. | ||
|
|
||
| If/when this lands, the renderer side collapses: the sort reads | ||
| `max(updatedAt, lastActivityAt)` straight off the row and both the poll | ||
| and the push wiring get deleted. | ||
|
|
||
| ## Alternatives rejected | ||
|
|
||
| - **`chat_sessions.lastActiveAt`** (cloud table, Electric-synced, has | ||
| `v2WorkspaceId`): only chat panes write it. Terminal agent panes — the | ||
| dominant way people talk to Claude in v2 — never touch it. | ||
| - **Local interaction tracking** (bump a localStorage timestamp when the | ||
| user opens/messages a workspace): per-device only, misses the | ||
| "agent finished while I was elsewhere" bump entirely, and adds a | ||
| second source of truth next to the host's binding store. | ||
| - **Faster polling**: trivially shrinks latency but scales traffic | ||
| inversely and still isn't instant; strictly dominated by push. | ||
|
|
||
| ## Known limitations of the shipped version | ||
|
|
||
| - 0–10s reorder latency (see Alternative 1). | ||
| - Unreachable hosts fall back to `updatedAt` ranking. | ||
| - Pinned rows keep pin order — the Pinned section ignores sort modes by | ||
| design. | ||
| - A messaged workspace ranks by newest event, it does not pin to #1: an | ||
| agent actively working elsewhere (its `PostToolUse` events keep | ||
| bumping) can legitimately out-rank it moments later. | ||
| - The `SelectV2Workspace` string-vs-`Date` type lie is still only | ||
| patched at the sidebar entry points (`mergeHostWorkspaces` | ||
| normalization + NaN-safe `toTime` in the sort). The root fix — an | ||
| Electric `parser` for timestamp columns in the shared shape options — | ||
| is intentionally out of scope here because it changes runtime types | ||
| for every consumer of every Electric collection and needs its own | ||
| audited PR. Note that persisted IndexedDB snapshots written before | ||
| that fix will still contain strings, so defensive coercion in sort | ||
| paths stays necessary regardless. |
98 changes: 98 additions & 0 deletions
98
...ktop/src/renderer/hooks/host-workspaces/useHostWorkspaces/useHostWorkspaces.utils.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| import { describe, expect, it } from "bun:test"; | ||
| import type { SelectV2Workspace } from "@superset/db/schema"; | ||
| import { | ||
| type HostWorkspaceRow, | ||
| type HostWorkspacesQueryTarget, | ||
| mergeHostWorkspaces, | ||
| } from "./useHostWorkspaces.utils"; | ||
|
|
||
| function makeTarget(machineId: string): HostWorkspacesQueryTarget { | ||
| return { | ||
| machineId, | ||
| organizationId: "org-1", | ||
| hostUrl: `http://host/${machineId}`, | ||
| isLocal: false, | ||
| }; | ||
| } | ||
|
|
||
| function makeHostRow(overrides: Partial<HostWorkspaceRow>): HostWorkspaceRow { | ||
| return { | ||
| id: "w-host", | ||
| organizationId: "org-1", | ||
| projectId: "p-1", | ||
| hostId: "host-a", | ||
| name: "host workspace", | ||
| branch: "main", | ||
| type: "worktree", | ||
| createdByUserId: null, | ||
| taskId: null, | ||
| createdAt: new Date("2026-07-01"), | ||
| updatedAt: new Date("2026-07-02"), | ||
| worktreePath: "/tmp/w", | ||
| worktreeExists: true, | ||
| ...overrides, | ||
| }; | ||
| } | ||
|
|
||
| // Electric rows carry ISO strings at runtime (the shape parser doesn't | ||
| // handle timestamptz), despite the SelectV2Workspace type saying Date. | ||
| function makeCloudRow( | ||
| overrides: Partial<Record<keyof SelectV2Workspace, unknown>>, | ||
| ): SelectV2Workspace { | ||
| return { | ||
| id: "w-cloud", | ||
| organizationId: "org-1", | ||
| projectId: "p-1", | ||
| hostId: "host-b", | ||
| name: "cloud workspace", | ||
| branch: "main", | ||
| type: "worktree", | ||
| createdByUserId: null, | ||
| taskId: null, | ||
| createdAt: "2026-06-01T00:00:00.000Z", | ||
| updatedAt: "2026-06-02T00:00:00.000Z", | ||
| ...overrides, | ||
| } as SelectV2Workspace; | ||
| } | ||
|
|
||
| describe("mergeHostWorkspaces", () => { | ||
| it("converts cloud-fallback rows' string timestamps into real Dates", () => { | ||
| const merged = mergeHostWorkspaces({ | ||
| hostResults: [], | ||
| cloudRows: [makeCloudRow({})], | ||
| }); | ||
|
|
||
| expect(merged).toHaveLength(1); | ||
| const row = merged[0]; | ||
| expect(row?.source).toBe("cloud"); | ||
| expect(row?.createdAt).toBeInstanceOf(Date); | ||
| expect(row?.updatedAt).toBeInstanceOf(Date); | ||
| expect(row?.createdAt.getTime()).toBe( | ||
| new Date("2026-06-01T00:00:00.000Z").getTime(), | ||
| ); | ||
| expect(row?.updatedAt.getTime()).toBe( | ||
| new Date("2026-06-02T00:00:00.000Z").getTime(), | ||
| ); | ||
| }); | ||
|
|
||
| it("keeps host-served rows authoritative over cloud rows for the same host", () => { | ||
| const target = makeTarget("host-a"); | ||
| const merged = mergeHostWorkspaces({ | ||
| hostResults: [ | ||
| { | ||
| target, | ||
| rows: [makeHostRow({ id: "w-1", hostId: "host-a" })], | ||
| reachable: true, | ||
| }, | ||
| ], | ||
| cloudRows: [ | ||
| makeCloudRow({ id: "w-stale", hostId: "host-a" }), | ||
| makeCloudRow({ id: "w-other", hostId: "host-b" }), | ||
| ], | ||
| }); | ||
|
|
||
| expect(merged.map((row) => row.id).sort()).toEqual(["w-1", "w-other"]); | ||
| const other = merged.find((row) => row.id === "w-other"); | ||
| expect(other?.updatedAt).toBeInstanceOf(Date); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: superset-sh/superset
Length of output: 50377
🏁 Script executed:
Repository: superset-sh/superset
Length of output: 29475
🏁 Script executed:
Repository: superset-sh/superset
Length of output: 21665
🌐 Web query:
@tanstack/react-db useLiveQuery isReady isLoading hydration cache data💡 Result:
In the context of the TanStack ecosystem,
@tanstack/react-dband@tanstack/react-queryserve different primary purposes, though TanStack DB is built on top of TanStack Query [1]. useLiveQuery is a hook provided by@tanstack/react-dbfor managing reactive, database-like collections [2][3][1]. It automatically re-runs when the underlying data in a collection changes, eliminating the need for manual cache invalidation [1]. Key concepts regarding your query include: Loading States (isLoading) In useLiveQuery, the isLoading property is a boolean that indicates whether the query is currently performing its initial fetch [3]. Like TanStack Query's useQuery, this state is typically used to render loading indicators [2][3]. Hydration and Cache Because TanStack DB uses TanStack Query's caching and synchronization mechanisms under the hood, it interacts with the same underlying infrastructure [1]. 1. Hydration: This is the process of transferring prefetched state from the server to the client [4][5]. It uses the same hydrate and dehydrate utilities as TanStack Query to restore state into the QueryClient cache [4][5]. 2. Cache: Data fetched by useLiveQuery is stored in the TanStack Query cache [1]. When hydrating, TanStack Query intelligently merges this state with any existing data in the cache based on timestamps [4][5]. isReady There is no standard isReady property in the primary TanStack Query or TanStack DB hook return objects. If you are seeing this, it may be a custom implementation, a legacy property, or a misidentification of states like isPending or isSuccess [6]. The core statuses used by the TanStack ecosystem to determine if data is available are isPending, isError, and isSuccess [6]. Compatibility Note If you are migrating from@tanstack/react-queryto@tanstack/react-db, note that while they share concepts, the APIs differ [7]. useLiveQuery replaces the standard useQuery pattern for collection management [7]. When dealing with SSR and hydration, ensure you are prefetching data on the server and using the HydrationBoundary or hydrate utilities on the client to avoid state mismatches [8][9][4]. Top Results: [2], [3], [1], [4], [5]Citations:
🌐 Web query:
docs TanStack DB useLiveQuery API return { data, isReady isLoading }💡 Result:
Yes, the useLiveQuery hook in TanStack DB returns an object that includes the properties { data, isReady, isLoading }, among others [1][2][3]. Specifically, the hook returns an object with the following shape [1][2]: - data: The result of the query. Its type is InferResultType (or an array, depending on the query) and it can be undefined if the query is not yet ready or is disabled [4][1][5]. - isLoading: A boolean indicating if the query is in a loading state [1][2][3]. - isReady: A boolean indicating if the query has finished its initial load and is ready to display [1][2]. In addition to these, the returned object typically includes several other useful properties for managing query state [1][2]: - status: A string representing the current status of the query (e.g., 'loading', 'error', 'success', 'disabled') [1][2][3]. - isError: A boolean indicating if the query encountered an error [1][2]. - isIdle: A boolean indicating if the query is idle [1][2]. - isCleanedUp: A boolean indicating if the query has been cleaned up [1][2]. - collection: The collection object associated with the query [1][2]. - state: A Map containing the reactive state of the query [1][2]. You can destructure these properties directly from the result of the hook [2][3]. Returning undefined or null from the useLiveQuery callback function will disable the query, in which case the status will be 'disabled' and data will be undefined [2][3].
Citations:
Wait for strict live-query readiness before writing user preferences.
useLiveQueryis only destructured fordata, soisReadyis ignored whencollections.v2UserPreferences.get(...)returns no row while hydration is in progress. In that state,setSidebarProjectSortModecan insert a default preference before the persisted row is available. Gate the insert onisReady(and otherwise defer the write) so a concurrent server/provided row cannot collide or be lost.🤖 Prompt for AI Agents
Source: Coding guidelines