feat(sidebar): add move endpoint for reordering pinned items - #831
Draft
tcp404 wants to merge 6 commits into
Draft
feat(sidebar): add move endpoint for reordering pinned items#831tcp404 wants to merge 6 commits into
tcp404 wants to merge 6 commits into
Conversation
…820) ## Description Backend half of PR-A: the ordering base + sidebar grouping/paging API that moves the left conversation list off "pull everything and group in the browser" onto a server-driven, paged model with a real source of truth for pins. ### Migration 038 - New `user_order` table — the pin ordering source of truth (v1 scene `'pinned'`; `order_key` ascending = most-recently pinned first). - `conversations.archived_at` column + partial indexes (`idx_user_order_scene`, archived indexes) landed here so PR-B (archive) needs no further migration. - No backfill: historical `extra.pinned` is intentionally NOT migrated into `user_order` (consistent with team-pin localStorage not migrating — preference data is never migrated). ### aionui-db - `IUserOrderStore` + `SqliteUserOrderStore`: pin (`order_key = scene global min − 1000`, empty scene → `1000`), unpin, keyset paging on `(order_key, item_type, item_id)`, `BEGIN IMMEDIATE` to serialize read-min-then-insert against concurrent pins. - Sidebar read store: thin-row query + batched hydration (no N+1), anti-join against `user_order` for the unpinned side (never reads the deprecated `pinned` column). ### aionui-sidebar (new crate) - `GET /api/sidebar` — first screen: pinned → projects (real project groups + dir pseudo-groups) → chats. - `GET /api/sidebar/items` — per-group `+10` keyset paging; five-case classification + path merge live in the display layer only (no `resolve_existing`, no fs touch, no writes). - `PUT`/`DELETE /api/order/pinned/{item_type}/{item_id}` — pin/unpin, idempotent, scene enum validated (unknown → 400). - Group order = render order; in-group item order = render order. ### Cascade (§4.3, best-effort) - Conversation delete, team delete, and a conversation becoming a live team member each drop the matching `user_order` rows. Orphans self-heal on read. ### Wiring - Routes / state / service wired into `aionui-app` mirroring the project module. ## Tests - `EXPLAIN QUERY PLAN` asserts the hot pinned reads (base + expanded keyset predicate) ride `idx_user_order_scene` and never full-scan `user_order`. - Keyset continuity (no repeat / no gap), concurrent-pin serialization into distinct rows, per-user scoping. - Sidebar classification matrix (five cases × conversation/team), path-merge does not write the DB, dangling `project_id`, join-team read-side member exclusion. - Cascade coverage across the three paths. - Full workspace gate green: `just push` = 8526 passed, 47 skipped. ## Notes - Pairs with AionUi `boii/feat/sidebar` (PR #3969). Frontend requires this branch running. - Not yet live-verified end-to-end in a running desktop instance — unit/integration only. - Follow-up (separate PR): `removeProject` greenfield (BR-19/D13). Conclusion: single global transaction is infeasible — conversation delete fires hooks (agent-process kill, cron clear) and removes the fs workspace dir, none of which can live in a DB tx. It will mirror `remove_team`: per-entity best-effort orchestration, localized atomicity only. - Deprecated `conversations.pinned/pinned_at` columns are left in place (not dropped); no read/write path touches them after this PR.
## Description
Make the "is this a temporary/auto-provisioned session workspace?" check
**root-agnostic**, fixing a historical-debt bug where long-time users —
whose conversation directories were migrated across data-dir layouts —
had their temporary sessions wrongly rendered as **projects** in the
sidebar.
### Root cause
Both backends that decide "is this a temp workspace?" anchored on the
*current* `data_dir` / `work_dir` root:
1. `aionui-conversation/src/service.rs` `is_temp_session_workspace` did
`workspace.strip_prefix(work_dir.join("conversations"))` — after a
migration `extra.workspace` holds an **absolute path under the old
root**, so the strip failed and it returned `false`.
2. `aionui-conversation/src/convert.rs` badge `is_temporary_workspace`
did `Path::new(ws).starts_with(data_dir)` — same current-root anchor,
same false negative for old-root workspaces.
Symptom chain (new sidebar `classify_unit`): a temp session with no
`project_id` → path branch → `is_temp_session_workspace` false → not
folded into Chats → `canonicalize` misses (old dir may still exist
physically) → `GroupKey::Dir` → **rendered as a project**.
### Fix
The auto/temp directory leaf has carried a `-temp-` marker across
**every** historical layout (`{agent}-temp-{ts}`, dated
`YYYY/MM/DD/{label}-temp-{id}`, `team-temp-{team_id}`). That marker is
root-agnostic, so:
- `is_temp_session_workspace` now scans the path components for the
**last** `conversations` segment and matches the relative tail, dropping
the `work_dir` prefix dependency. Each `is_auto_workspace_relative_path`
arm is **tightened** to additionally require `leaf.contains("-temp-")`,
so a real user project like `/x/conversations/myproj` is not
misclassified.
- The `convert.rs` badge now delegates to the same
`is_temp_session_workspace` predicate instead of a raw
`starts_with(data_dir)`.
- The sidebar call site drops the now-removed `work_dir` argument.
This covers the migration case with negligible false-positive risk (user
projects almost never have a `-temp-` leaf) and keeps the heuristic as
the foundation — no schema migration / backfill needed.
## Testing
```bash
cargo fmt --all
cargo test -p aionui-conversation -p aionui-sidebar
```
Added regression tests deliberately mixing an **old-root temp
workspace** with a **user project**, per our verification discipline:
- `is_temp_session_workspace`: current-root temp → true (no regression);
**migrated old-root** temp → true (the fix); team temp → true; legacy
bare leaf → true; user project `/home/me/conversations/myproj` →
**false**; no `conversations` segment → false; bad date → false.
- `convert.rs` badge: old-root workspace →
`is_temporary_workspace=true`; user project under data_dir → false.
- sidebar `classify_unit`: no `project_id` + old-root temp workspace →
`GroupKey::Chats`, no longer `GroupKey::Dir`.
All green.
## Cross-platform
Path handling uses component iteration (no hardcoded separators); tests
exercise Unix-style absolute paths. No platform-specific branches
introduced.
- Recognize temp sessions by the `-temp-` leaf marker alone instead of anchoring on a data-dir root or a conversations/tmp container segment - Covers every historical auto-workspace layout the previous container-anchored check (#825) still missed: OS temp dir, bare `<data_dir>/{leaf}`, and `<data_dir>/tmp/{leaf}`, alongside the conversations bare/dated/per-user shapes - Stays root-agnostic so migrated data-dirs whose `extra.workspace` was baked under a previous root still classify correctly on the read path - Leaves the write/delete predicate `is_auto_workspace_relative_path` untouched; it still anchors on the current data-dir for precise cleanup - Accepts a user project whose own leaf contains `-temp-` as a false positive, overridden by the project row's `kind` - Rewrite the unit test as a single table-driven case covering all layouts, the negative guards, and the documented false positive
- OrderScene has a single variant, so the roundtrip loop tripped clippy::single_element_loop; inline it to a direct binding
- Add a #[cfg(windows)] twin of the layout matrix using drive-letter and backslash paths so Windows exercises native Path::file_name splitting, which POSIX literals cannot on that platform - Gate the existing matrix with #[cfg(unix)] so each OS runs its own native path forms rather than shared portable literals - Extract the assertion loop into a shared assert_temp_layout_matrix helper reused by both platform tests
- Add IUserOrderStore::move_item with fractional order_key placement:
insert-at-top uses min-PIN_GAP; move-after-X takes the midpoint
between the anchor and its successor, falling back to a whole-scene
rebalance (1000,2000,...) when the gap drops below the threshold
- Run the read-modify-write in a single BEGIN IMMEDIATE txn to avoid
TOCTOU under concurrent moves
- Add POST /api/order/{scene}/move: parse scene/moved/after, reject a
self-anchor as 400, map MoveOutcome to Ok / 404 ScopeGone / 400
- Cover move, rebalance, boundaries and stale-anchor mapping in tests
tcp404
force-pushed
the
boii/feat/sidebar-sort
branch
from
August 12, 2026 07:20
f5b7981 to
f012d09
Compare
tcp404
marked this pull request as draft
August 12, 2026 08:06
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Description
Adds
POST /api/order/{scene}/move, the server-owned reorder primitive backing the sidebar pinned drag-and-drop (paired AionUi PR #3993). A move carries an anchor-only payload — the moved row plus its new predecessor (after, ornullfor top-of-list) — and the backend assigns a fractionalorder_key:min(order_key) - PIN_GAP1000 / 2000 …spreadThe whole move runs inside a single
BEGIN IMMEDIATEtransaction so concurrent moves serialize withoutorder_keycollisions (TOCTOU-safe). Outcomes map throughMoveOutcome→200 Ok/404(scope gone) /400(stale or invalid anchor). Conversations and teams share one scene and are addressed by composite${item_type}:${item_id}ids, so a mixed pinned list reorders as one sequence.Tests
cargo test -p aionui-db -p aionui-sidebar— green. New coverage:move_after_anchor_lands_at_midpointmove_rebalances_when_neighbour_gap_is_exhaustedmove_to_top_places_below_current_minmove_mixes_conversation_and_team_rowsconcurrent_moves_serialize_without_key_collisionmove_order_validates_and_maps_stale_anchors(service layer)Full
just pushgate (migration-check → lint → fmt → test) passed before push.Runtime Verification
No AI signatures.