fix: address data mode review findings

Author: wesm

docs/superpowers/plans/2026-07-23-data-mode-review-fixes.md

@@ -0,0 +1,127 @@
+# Data Mode Review Fixes Implementation Plan
+
+> **For Codex:** Follow the repository's test-first and verification rules for
+> each task. Keep the final implementation in one focused follow-up commit.
+
+**Goal:** Resolve every verified PR #1166 review finding and restore merge
+readiness without changing Data mode's core architecture.
+
+**Architecture:** Preserve the existing SQLite/PostgreSQL/DuckDB shared data
+models and candidate pipeline. Extend inventory identity metadata to retain
+merged keys, keep filtered PostgreSQL publications privacy-safe, and reuse the
+incremental append's existing session read.
+
+**Tech stack:** Go, SQLite, PostgreSQL, DuckDB, Svelte 5, TypeScript, Paraglide,
+Vitest.
+
+______________________________________________________________________
+
+### Task 1: Lock down the critical regressions
+
+**Files:**
+
+- Modify: `internal/postgres/push_pgtest_test.go`
+- Modify: `internal/db/orphaned_test.go`
+- Modify: `internal/db/worktree_mappings_test.go`
+- Modify: `internal/sync/engine_bench_test.go`
+
+1. Add or tighten tests that reproduce the PostgreSQL INSERT failure, the v70
+   snapshot overwrite, normalized persisted paths, and incremental allocation
+   regression.
+1. Run each focused test or benchmark and confirm the expected failure.
+1. Repair the placeholder sequence and upgrade gate.
+1. Normalize cross-platform expectations.
+1. Remove the duplicate incremental session read and rerun the benchmark.
+
+### Task 2: Preserve merged project selection
+
+**Files:**
+
+- Modify: `internal/db/project_inventory.go`
+- Modify: `internal/db/project_inventory_test.go`
+- Modify: `internal/db/worktree_candidates.go`
+- Modify: `internal/db/worktree_candidates_test.go`
+- Modify: PostgreSQL and DuckDB parity implementations/tests as needed
+- Regenerate: frontend API models
+- Modify: `frontend/src/lib/stores/data.svelte.ts`
+- Modify: `frontend/src/lib/stores/data.test.ts`
+
+1. Add tests for colliding display labels with distinct project keys and a deep
+   link through a non-canonical key.
+1. Add `project_keys` while retaining the canonical `project_key`.
+1. Select candidate sessions for every key in a validated merged row.
+1. Update frontend row resolution and refresh behavior.
+1. Run backend parity and frontend store/component tests.
+
+### Task 3: Repair publication scope and query bounds
+
+**Files:**
+
+- Modify: `internal/postgres/worktree_mappings_push.go`
+- Modify: `internal/postgres/worktree_mappings_push_pgtest_test.go`
+- Modify: `internal/db/project_inventory.go`
+- Modify: `internal/db/project_inventory_test.go`
+- Modify: `internal/db/project_identity.go`
+- Modify: `internal/db/project_identity_test.go`
+
+1. Add a filtered-push test proving unrelated mapping paths are not published.
+1. Skip mapping publication for filtered pushes.
+1. Add cardinality tests over backend bind limits.
+1. Chunk machine candidate reads and aggregate rebuilds transactionally.
+1. Run SQLite and PostgreSQL focused suites.
+
+### Task 4: Align behavior and error handling
+
+**Files:**
+
+- Modify: `internal/db/worktree_candidates.go`
+- Modify: `internal/db/worktree_reclassification.go`
+- Modify: `internal/server/huma_routes_settings.go`
+- Modify: associated Go tests
+
+1. Add tests for zero-message candidates, writer-closed preview, legacy update
+   duplicate handling, and token changes.
+1. Align session predicates to visible sessions.
+1. Map writer closure and uniqueness errors correctly.
+1. Include `original_project` in tokens and split the evaluator helper.
+1. Run focused DB and server tests.
+
+### Task 5: Surface stale inventory and fix localized copy
+
+**Files:**
+
+- Modify: `frontend/src/lib/stores/data.svelte.ts`
+- Modify: `frontend/src/lib/components/data/DataPage.svelte`
+- Modify: associated frontend tests
+- Modify: all `frontend/messages/*.json`
+
+1. Add a component regression test for a failed foreground reload after a
+   successful load.
+1. Render the error and retry control above retained stale inventory.
+1. Replace the Activity-scoped no-candidate text with archive-wide wording in
+   every locale.
+1. Run `npm run i18n:compile`, focused Vitest tests, and `npm run check`.
+
+### Task 6: Finish parity and maintenance cleanup
+
+**Files:**
+
+- Modify: inventory timestamp handling and tests across backends
+- Modify: PostgreSQL mapping ordering and parity tests
+- Modify: stale comments identified by review
+
+1. Add malformed-timestamp and deterministic-order regressions.
+1. Make SQLite and PostgreSQL observable behavior match.
+1. Correct stale bind-limit and collation comments.
+1. Run focused parity suites.
+
+### Task 7: Verify and publish
+
+1. Run Go formatting and vet.
+1. Run targeted and broad Go tests, PostgreSQL integration tests, frontend
+   tests/checks, and benchmark comparison.
+1. Inspect the final diff and run the private-data scrub.
+1. Use the commit workflow to create one conventional follow-up commit.
+1. Push the current branch.
+1. Replace the PR body with a concise rationale-first summary with no test
+   section or checklist.

docs/superpowers/specs/2026-07-23-data-mode-review-fixes-design.md

@@ -0,0 +1,73 @@
+# Data Mode Review Fixes
+
+## Goal
+
+Make PR #1166 mergeable by repairing its verified correctness, upgrade,
+cross-platform, publication-scope, user-feedback, and performance regressions
+without changing the core Data mode architecture.
+
+## Decisions
+
+### Merged project inventory rows
+
+Keep one inventory row per sanitized display label. Add every underlying project
+identity key to the row while retaining `project_key` as the stable canonical
+key. Candidate lookup validates that the requested key belongs to the displayed
+row, then includes sessions for every raw project merged into that row. The
+frontend resolves deep links against both the canonical key and the complete key
+list.
+
+This preserves the intentional privacy-safe label merge while making every
+counted session reachable.
+
+### Filtered PostgreSQL pushes
+
+Do not publish worktree mappings during project-filtered pushes. Mapping rules
+are archive-scoped configuration and cannot be partitioned safely by project:
+dynamic rules have no fixed project, and independently filtered cursors share
+the same archive key in PostgreSQL. Publishing a partial set could either leak
+unrelated paths or delete rules belonging to another filtered scope.
+
+Unfiltered pushes continue to publish the complete mapping set.
+
+### Session eligibility
+
+Data inventory, candidate discovery, preview, and apply all operate on visible
+sessions (`deleted_at IS NULL`), including sessions with zero messages. This
+keeps counts consistent across the three screens.
+
+### Failed reloads
+
+A failed foreground reload keeps the last successful inventory visible but also
+renders the error and retry action. Background refresh failures continue to
+preserve the current view without interrupting the user.
+
+### Incremental append performance
+
+Return the post-mapping project from the existing single-session mapping path so
+`writeIncremental` does not perform a second `GetSession` point read. The
+benchmark gate remains unchanged; the implementation must recover enough
+allocations to pass it.
+
+## Other corrections
+
+- Repair the PostgreSQL session INSERT placeholder sequence.
+- Move the source-snapshot upgrade gate to data version 71 and cover a v70
+  source archive.
+- Normalize persisted mapping-path expectations in cross-platform tests.
+- Chunk inventory-machine and aggregate-rebuild `IN` queries.
+- Map writer shutdown to the established 503 response.
+- Map legacy-row uniqueness conflicts to the duplicate sentinel.
+- Include `original_project` in reclassification tokens.
+- Split the oversized mapping evaluator.
+- Fail consistently on malformed inventory timestamps.
+- Use byte-order collation for PostgreSQL mapping ordering.
+- Correct stale comments and localized archive-wide candidate copy.
+- Rewrite the PR description as a concise rationale-first summary.
+
+## Validation
+
+Use behavior-level regression tests before implementation. Run targeted Go,
+PostgreSQL integration, frontend, localization, formatting, vet, and benchmark
+checks, followed by the broadest practical suites. Scrub publication text and
+the final diff before pushing.

frontend/messages/en.json

@@ -917,7 +917,7 @@
   "data_reclassify_original": "Originally shown as {project}",
   "data_reclassify_candidates_loading": "Finding worktrees…",
   "data_reclassify_candidates_failed": "Failed to find worktrees",
-  "data_reclassify_no_candidates": "No worktree paths were found for this project in the selected Activity view.",
+  "data_reclassify_no_candidates": "No worktree paths were found for this project in the archive.",
   "data_reclassify_choose_worktree": "Choose a worktree",
   "data_reclassify_candidate_sessions": [
     { "declarations": ["input count", "local countPlural = count: plural", "local countNumber = count: number"], "selectors": ["countPlural"], "match": { "countPlural=one": "{countNumber} session", "countPlural=other": "{countNumber} sessions" } }

frontend/messages/fr.json

@@ -917,7 +917,7 @@
   "data_reclassify_original": "Initialement affiché comme {project}",
   "data_reclassify_candidates_loading": "Recherche des arbres de travail…",
   "data_reclassify_candidates_failed": "Impossible de trouver les arbres de travail",
-  "data_reclassify_no_candidates": "Aucun chemin d’arbre de travail trouvé pour ce projet dans la vue Activité sélectionnée.",
+  "data_reclassify_no_candidates": "Aucun chemin d’arbre de travail trouvé pour ce projet dans l’archive.",
   "data_reclassify_choose_worktree": "Choisir un arbre de travail",
   "data_reclassify_candidate_sessions": [
     { "declarations": ["input count", "local countPlural = count: plural", "local countNumber = count: number"], "selectors": ["countPlural"], "match": { "countPlural=one": "{countNumber} session", "countPlural=other": "{countNumber} sessions" } }

frontend/messages/ko.json

@@ -892,7 +892,7 @@
   "data_reclassify_original": "원래 표시 이름: {project}",
   "data_reclassify_candidates_loading": "워크트리 찾는 중…",
   "data_reclassify_candidates_failed": "워크트리를 찾지 못했습니다",
-  "data_reclassify_no_candidates": "선택한 활동 보기에서 이 프로젝트의 워크트리 경로를 찾지 못했습니다.",
+  "data_reclassify_no_candidates": "아카이브에서 이 프로젝트의 워크트리 경로를 찾지 못했습니다.",
   "data_reclassify_choose_worktree": "워크트리 선택",
   "data_reclassify_candidate_sessions": [
     { "declarations": ["input count", "local countPlural = count: plural", "local countNumber = count: number"], "selectors": ["countPlural"], "match": { "countPlural=other": "세션 {countNumber}개" } }

frontend/messages/zh-CN.json

@@ -890,7 +890,7 @@
   "data_reclassify_original": "原显示名称：{project}",
   "data_reclassify_candidates_loading": "正在查找工作树…",
   "data_reclassify_candidates_failed": "无法查找工作树",
-  "data_reclassify_no_candidates": "在所选活动视图中未找到此项目的工作树路径。",
+  "data_reclassify_no_candidates": "在存档中未找到此项目的工作树路径。",
   "data_reclassify_choose_worktree": "选择工作树",
   "data_reclassify_candidate_sessions": [
     { "declarations": ["input count", "local countPlural = count: plural", "local countNumber = count: number"], "selectors": ["countPlural"], "match": { "countPlural=other": "{countNumber} 个会话" } }

frontend/messages/zh-TW.json

@@ -890,7 +890,7 @@
   "data_reclassify_original": "原顯示名稱：{project}",
   "data_reclassify_candidates_loading": "正在尋找工作樹…",
   "data_reclassify_candidates_failed": "無法尋找工作樹",
-  "data_reclassify_no_candidates": "在所選活動檢視中找不到此項目的工作樹路徑。",
+  "data_reclassify_no_candidates": "在封存中找不到此專案的工作樹路徑。",
   "data_reclassify_choose_worktree": "選擇工作樹",
   "data_reclassify_candidate_sessions": [
     { "declarations": ["input count", "local countPlural = count: plural", "local countNumber = count: number"], "selectors": ["countPlural"], "match": { "countPlural=other": "{countNumber} 個工作階段" } }

frontend/src/lib/api/generated/models/DbProjectInventoryRow.ts

@@ -43,7 +43,7 @@
   }
 
   function closeWorkspace() {
-    const key = data.selectedProjectKey;
+    const key = data.selectedRow?.project_key ?? data.selectedProjectKey;
     data.clearSelection();
     requestAnimationFrame(() => {
       // Match on dataset instead of an attribute selector so arbitrary
@@ -95,9 +95,15 @@
       />
     {/key}
   {:else if data.inventory}
-    <!-- Inventory-first ordering: once inventory has loaded once it keeps
-         rendering through background reloads; loading/error below only
-         apply before that first successful load. -->
+    <!-- Keep the last successful inventory visible during reloads, while
+         surfacing foreground failures so stale data is never silent. -->
+    {#if data.error}
+      <div class="status error">
+        <span>{data.error}</span>
+        <button class="retry-btn" onclick={() => data.load()}>{m.shared_retry()}</button>
+      </div>
+    {/if}
+
     <div class="summary-strip">
       <span>{m.data_summary_projects({ count: data.inventory.total_projects })}</span>
       <span>{m.data_summary_sessions({ count: data.inventory.total_sessions })}</span>

frontend/src/lib/components/data/DataPage.svelte

@@ -174,6 +174,18 @@ describe("DataPage", () => {
     expect(document.body.textContent).toContain(m.data_summary_projects({ count: 1 }));
   });
 
+  it("shows a foreground reload error above the retained inventory", async () => {
+    data.inventory = makeInventory([makeRow({ project_key: "k1" })]);
+    api.getApiV1DataProjects.mockRejectedValueOnce(new Error("server unavailable"));
+
+    component = mount(DataPage, { target: document.body });
+    await flush();
+
+    expect(document.body.textContent).toContain("server unavailable");
+    expect(document.body.textContent).toContain(m.data_summary_projects({ count: 1 }));
+    expect(document.querySelector<HTMLButtonElement>(".retry-btn")).not.toBeNull();
+  });
+
   it("shows the empty message and no table when there are no projects", async () => {
     const inventory = makeInventory([]);
     api.getApiV1DataProjects.mockResolvedValueOnce(inventory);