Skip to content

feat(sidebar): sidebar grouping base with root-agnostic temp-workspace check - #832

Draft
tcp404 wants to merge 5 commits into
mainfrom
boii/feat/sidebar-base
Draft

feat(sidebar): sidebar grouping base with root-agnostic temp-workspace check#832
tcp404 wants to merge 5 commits into
mainfrom
boii/feat/sidebar-base

Conversation

@tcp404

@tcp404 tcp404 commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

PR-A / base branch for the sidebar redesign. Re-lands the sidebar grouping + user_order foundation (previously #820, reverted from main by #828) together with an improved temp-workspace classifier. PR-B/C/D branch off / merge into this base before the set lands on main.

Contents:

Why the temp-workspace change

The earlier container-anchored check (#825) still missed real historical layouts. Temp workspaces have shipped in six shapes over time — OS temp dir, bare <data_dir>/{leaf}, <data_dir>/tmp/{leaf}, and the conversations/ bare / dated / per-user shapes. The earliest layouts have no container segment at all, and migrated data-dirs carry extra.workspace baked under a previous root, so anchoring on a root or container strip-fails on genuine temp rows and renders them as projects in the sidebar.

The -temp- leaf ({label}-temp-{id} / team-temp-{team_id}) is the only signal common to every layout, so the read predicate keys on that. A user-selected project whose own directory name contains -temp- is an accepted false positive; the project row's kind (standard/temp) is the authoritative override.

The write/delete predicate is_auto_workspace_relative_path is intentionally left anchored on the current data-dir — precise cleanup wants root-anchoring, the read path wants maximum recall.

Test plan

  • is_temp_session_workspace rewritten as a table-driven unit test covering all six layouts, migrated-root variants, the negative guards, and the documented false positive.
  • Full pre-push gate green: fmt, clippy -D warnings, and the entire workspace suite (8535 passed).

tcp404 added 5 commits August 12, 2026 15:13
…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
@tcp404
tcp404 force-pushed the boii/feat/sidebar-base branch from 515b2ed to 9923f9d Compare August 12, 2026 07:13
@tcp404
tcp404 marked this pull request as draft August 12, 2026 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant