diff --git a/docs/00_overview/MVP1_DASHBOARD.md b/docs/00_overview/MVP1_DASHBOARD.md index 5cc8ada4..85503558 100644 --- a/docs/00_overview/MVP1_DASHBOARD.md +++ b/docs/00_overview/MVP1_DASHBOARD.md @@ -95,7 +95,7 @@ _None._ | Feature | Type | One-liner | Depends on | Status | |---|---|---|---|---| | [chore_ci_prettier_check](../02_product/planned_features/chore_ci_prettier_check/idea.md) | Chore | `.github/workflows/pr.yml`'s `frontend` job runs: | — | Idea — surfaced during `/impl-execute --ad-hoc` of `infra_e2e_wire_seed_helper_into_studies_spec`. | -| [chore_data_table_primitive_followups](../02_product/planned_features/chore_data_table_primitive_followups/idea.md) | Chore | `feat_data_table_primitive` shipped with six known non-regression follow-up items captured only in chat transcripts. None block the PR but each is a real improvement that would otherwise evaporate whe | — | Partial — items 1, 2, 4, 6 shipped in PR (TBD) 2026-05-17. Items 3 + 5 remain open (larger refactors touching 8 DataTable consumers each). | +| [chore_data_table_columnvisibility_tanstack](../02_product/planned_features/chore_data_table_columnvisibility_tanstack/idea.md) | Chore | `feat_data_table_primitive` shipped with six known non-regression follow-up items captured only in chat transcripts. None block the PR but each is a real improvement that would otherwise evaporate whe | — | Partial — items 1, 2, 4, 6 shipped in PR #132 (squash commit `ee4c8d4`) 2026-05-17; item 3 closed 2026-05-19 (won't fix — flat-prop API locked as canonical, plan addendum required). Item 5 is the only residual implementation work: 1 primitive + (optionally) per-wrapper touch-ups. | | [chore_detail_page_shell_primitive](../02_product/planned_features/chore_detail_page_shell_primitive/idea.md) | Chore | Six of the seven `/{entity}/[id]` detail routes hand-roll the same three-state scaffold around their data query. The structure is **identical** down to the className strings, with two minor copy varia | — | Idea — surfaced during a UI-refactor audit run after [`chore_form_dropdown_primitive`](../../../00_overview/implemented_features/2026_05_18_chore_form_dropdown_primitive/feature_spec.md) (PR #136) and [`feat_data_table_primitive`](../../../00_overview/implemented_features/2026_05_16_feat_data_table_primitive/feature_spec.md) (PR #126) merged. | | [chore_extract_shadcn_select_test_mock](../02_product/planned_features/chore_extract_shadcn_select_test_mock/idea.md) | Chore | A factor-and-share refactor was attempted during `chore_form_dropdown_primitive` post-implementation. I extracted the mock to `ui/src/__tests__/helpers/shadcn-select-mock.tsx` exporting `mockShadcnSel | — | Idea — captured during `chore_form_dropdown_primitive` execution (PR pending). | | [chore_form_dropdown_guide_screenshot_refresh](../02_product/planned_features/chore_form_dropdown_guide_screenshot_refresh/idea.md) | Chore | Each affected guide has a Playwright spec at `ui/tests/e2e/guides/*.spec.ts` that captures screenshots when run against the real backend. The PR's UI changes produce different screenshots: | — | Idea — captured during `chore_form_dropdown_primitive` post-implementation guide-impact assessment. | diff --git a/docs/00_overview/implemented_features/2026_05_16_feat_data_table_primitive/implementation_plan.md b/docs/00_overview/implemented_features/2026_05_16_feat_data_table_primitive/implementation_plan.md index 8fadcb3f..2670182d 100644 --- a/docs/00_overview/implemented_features/2026_05_16_feat_data_table_primitive/implementation_plan.md +++ b/docs/00_overview/implemented_features/2026_05_16_feat_data_table_primitive/implementation_plan.md @@ -705,7 +705,9 @@ None — inline in `data-table-toolbar.tsx`. ### Story 2.6 — `useDataTableUrlState` hook with push/replace history (FR-8) — **lifted to the consumer** -**Outcome:** A reusable hook that owns the URL-state contract: cursor uses `router.push()`, filter/sort/q use `router.replace()`. Cursor resets on filter/sort/q change. Hydrates from URL on mount. **The hook lives at the page-level consumer**, not inside DataTable, so the consumer's TanStack Query hook receives the URL state and refetches accordingly. DataTable becomes a **controlled component** receiving `urlState` + setters as props (per spec §4 "Consumer-supplied data" principle). +> **Shipped contract addendum (2026-05-19).** The "Key interfaces" block below sketched `DataTableProps` gaining a `urlState: DataTableUrlState` + setter-bag (`setSort` / `setFilter` / `setQ` / `setCursor` / `setPageSize`) shape. The build-out across stories 2.2–2.5 produced **10 flat optional props** on `DataTableProps` instead: `sort? / onSortChange? / sortCodec? / filters? / onFilterChange? / q? / onQChange? / cursor? / onCursorChange? / onPageSizeChange?`. The flat-prop API is canonical; the aggregate-prop shape below is an early sketch retained for historical context. Rationale and decision log: [`chore_data_table_primitive_followups/idea.md` item 3 (closed)](../../../02_product/planned_features/chore_data_table_primitive_followups/idea.md). Verified in `ui/src/components/common/types.ts:181-216`. + +**Outcome:** A reusable hook that owns the URL-state contract: cursor uses `router.push()`, filter/sort/q use `router.replace()`. Cursor resets on filter/sort/q change. Hydrates from URL on mount. **The hook lives at the page-level consumer**, not inside DataTable, so the consumer's TanStack Query hook receives the URL state and refetches accordingly. DataTable becomes a **controlled component** receiving the flat URL-state value/onChange pairs as props (per spec §4 "Consumer-supplied data" principle). **New files** diff --git a/docs/00_overview/mvp1_dashboard.html b/docs/00_overview/mvp1_dashboard.html index b53fc158..cf9da97b 100644 --- a/docs/00_overview/mvp1_dashboard.html +++ b/docs/00_overview/mvp1_dashboard.html @@ -443,7 +443,7 @@

Idea 5

-
Data Table Primitive Followups
+
Data Table Columnvisibility Tanstack
Chore diff --git a/docs/02_product/planned_features/chore_data_table_primitive_followups/idea.md b/docs/02_product/planned_features/chore_data_table_columnvisibility_tanstack/idea.md similarity index 62% rename from docs/02_product/planned_features/chore_data_table_primitive_followups/idea.md rename to docs/02_product/planned_features/chore_data_table_columnvisibility_tanstack/idea.md index 5e889d6e..44093f48 100644 --- a/docs/02_product/planned_features/chore_data_table_primitive_followups/idea.md +++ b/docs/02_product/planned_features/chore_data_table_columnvisibility_tanstack/idea.md @@ -1,7 +1,7 @@ # DataTable primitive — review-cycle follow-ups **Date:** 2026-05-16 -**Status:** Partial — items 1, 2, 4, 6 shipped in PR (TBD) 2026-05-17. Items 3 + 5 remain open (larger refactors touching 8 DataTable consumers each). +**Status:** Partial — items 1, 2, 4, 6 shipped in PR #132 (squash commit `ee4c8d4`) 2026-05-17; item 3 closed 2026-05-19 (won't fix — flat-prop API locked as canonical, plan addendum required). Item 5 is the only residual implementation work: 1 primitive + (optionally) per-wrapper touch-ups. **Origin:** [`feat_data_table_primitive`](../../../00_overview/implemented_features/2026_05_16_feat_data_table_primitive/) review cycles. Specific cycle/finding references inline below. **Depends on:** `feat_data_table_primitive` (merged as PR #126). @@ -26,6 +26,8 @@ Four test files duplicate the same ~25-line `searchParamsSubscribers` + `applyUr Factor into `ui/src/__tests__/helpers/data-table-url-mock.ts` exporting `setupDataTableUrlMock()` that returns `{ setSearch, getLastReplace, getLastPush }`. Next consumer that adds a DataTable URL-state test pulls the helper instead of duplicating. +> *(The shipped helper exposes a `vi.mock`-friendly API instead — `makeNextNavigationMock` / `resetDataTableUrlMock` / `getDataTableUrlMockState` / `setMockedSearch` — see Resolution above. The `setupDataTableUrlMock` text here is the original sketch; trust the Resolution paragraph for the canonical API.)* + ### 2. `useLocalStorageSet` return shape ✅ DONE 2026-05-17 **Origin:** Epic 2 GPT-5.5 cycle 1 finding #14 (Low, deferred). @@ -34,13 +36,18 @@ Plan Story 2.10 specifies `useLocalStorageSet` returns `{ value: string[], add(i **Resolution:** Locked the shipped Set-based shape as canonical. The plan's `string[]` proposal was an early sketch — the actual consumer (`` at `ui/src/components/common/data-table.tsx`) uses `.has(c.id)` + `.toggle(id)`, which are Set-shaped operations. Set provides O(1) membership; array would force `.includes()` (O(n)). Updated the hook's docstring at `ui/src/hooks/use-local-storage-set.ts` to lock in the contract and reference this idea. No code change. -### 3. `DataTableProps` URL state aggregate prop +### 3. `DataTableProps` URL state aggregate prop — ✅ CLOSED 2026-05-19 (won't fix — doc the chosen shape) **Origin:** Epic 2 GPT-5.5 cycle 1 finding #1 (High, deferred). -Plan Story 2.6 key-interface block declares `DataTableProps` gaining required `urlState: DataTableUrlState` + `setSort`/`setFilter`/`setQ`/`setCursor`/`setPageSize` props after the URL hook lifts. The implementation kept the flat optional props (`sort?`, `onSortChange?`, etc.) from the 2.2-2.5 build-out — works as a controlled contract but doesn't match the plan's compact shape. +**Resolution:** Locked the shipped flat-prop API as canonical. The implemented plan's Story 2.6 key-interface block sketched `urlState: DataTableUrlState` + a `setSort`/`setFilter`/`setQ`/`setCursor`/`setPageSize` setter bag, but the build-out in stories 2.2-2.5 produced 10 flat optional props on [`DataTableProps`](../../../../ui/src/components/common/types.ts) (`sort?`, `onSortChange?`, `filters?`, `onFilterChange?`, `q?`, `onQChange?`, `cursor?`, `onCursorChange?`, `onPageSizeChange?`, `sortCodec?`). Rationale for closing: + +- The plan in `implemented_features/2026_05_16_feat_data_table_primitive/` is a historical record; the shipped code is the live contract that the 8 thin-wrapper consumers depend on. +- The flat-prop API is idiomatic React + aligns with shadcn primitives (consumer passes value + onChange pairs directly). +- All 8 consumers (`clusters/clusters-table.tsx`, `judgments/judgments-table.tsx`, `proposals/proposals-table.tsx`, `query-sets/queries-table.tsx`, `query-sets/query-sets-table.tsx`, `studies/studies-table.tsx`, `studies/trials-table.tsx`, `templates/templates-table.tsx`) work cleanly with the shipped shape. +- Refactor cost (~250 LOC across 9 files) doesn't justify the marginal API-aesthetics gain. -Refactor would touch every Epic 3 consumer (8 thin wrappers). The flat-prop API is also slightly more idiomatic React; the question is whether the plan should be updated to reflect the chosen design or the code should be updated to reflect the plan. +Companion patch to the historical plan: [`implemented_features/2026_05_16_feat_data_table_primitive/implementation_plan.md` Story 2.6 key-interface block](../../../00_overview/implemented_features/2026_05_16_feat_data_table_primitive/implementation_plan.md) gains a "Shipped contract" addendum noting the divergence from the sketched aggregate-prop shape and pointing back to this idea for the rationale. ### 4. `?limit=` coercion to `pageSizeOptions` allowlist ✅ DONE 2026-05-17 @@ -76,7 +83,13 @@ Direct URLs like `?status=invented` or `?sort=garbage:asc` currently pass throug Each item was reviewed and adjudicated "non-regression follow-up" during the parent PR's review cycles. None block correctness; each improves consistency, encapsulation, or defense-in-depth. Bundling into one chore folder so they ship together (or get dropped together) when picked up. +## Open questions for /spec-gen + +- **Q1 — Item 3 scope.** ✅ **Locked 2026-05-19: close item 3 (won't fix), lock the flat-prop API as canonical, add a "Shipped contract" addendum to the historical implementation plan's Story 2.6 key-interface block.** See item 3 above for the full rationale. +- **Q2 — Item 5 cleanup scope: just the primitive, or also the wrapper-level prop changes?** Migrating to TanStack `state.columnVisibility` is straightforward in [`data-table.tsx:259-267`](../../../../ui/src/components/common/data-table.tsx) (pre-filter → full columns + `state.columnVisibility` derived from the same `useLocalStorageSet`). Recommended default: **primitive-only.** The 8 thin wrappers don't touch column visibility today; the localStorage key + the `useLocalStorageSet` integration stay unchanged. Refactor is contained to the primitive's `useReactTable()` call and the `visibleColumns`/`columns` props passed downstream. + ## Relationship to other work -- **Parent:** `feat_data_table_primitive`. +- **Parent:** `feat_data_table_primitive` (PR #126). - **Adjacent deferred:** `feat_fts_rank_ordering_mvp2` (rank-ordered FTS) — different concern (backend ordering), no overlap. +- **Sibling check (clean):** `chore_form_dropdown_primitive` (shipped, PR #136), `chore_detail_page_shell_primitive` (idea-stage, PR #147) — both touch different surfaces. `chore_ci_prettier_check` is the only sibling that *could* affect a future PR for this chore (catches `prettier` drift in changed files), but doesn't conflict. diff --git a/state.md b/state.md index 8fbf5ffb..0c498812 100644 --- a/state.md +++ b/state.md @@ -25,7 +25,7 @@ - **2026-05-18 — `chore_detail_page_shell_primitive` idea captured** as PR #147 (squash commit `8854e47`). Docs-only PR. Third primitive-extraction candidate after PR #126 (DataTable) and PR #136 (EntitySelect) — 6 of 7 detail pages (`clusters/[id]`, `studies/[id]`, `proposals/[id]`, `query-sets/[id]`, `templates/[id]`, `judgments/[id]`; chat excluded as structurally different) hand-roll the same `isPending` → `isError` → data scaffold with identical className strings and copy. Latent UX inconsistency: only `proposals/[id]` distinguishes 404 from network error today; the primitive flattens this. Preflight surfaced 6 corrections from the initial draft (mostly API-shape fixes: discriminate by `errorCode` string per `ApiError`, not HTTP status; add optional `entityTitle` override for compound labels like "Judgment list"; correct LOC math from "150-200 saved" to "+170 LOC PR, ROI is consistency"). **Gemini Code Assist:** 3 Medium findings, all addressed (1 already-fixed-by-preflight, 2 accepted — explicit `docker compose stop ui` command + `entityTitle` optional prop + Suspense out-of-scope reasoning clarified). Next step is `/pipeline` to ship the actual implementation. -- **2026-05-18 — `chore_form_dropdown_primitive` merged into `main`** as PR #136 (squash commit `cb7d9ee`). Shipped via `/pipeline --auto` end-to-end (idea-preflight × 2 → spec-gen → impl-plan-gen → impl-execute → merge). The form-side peer to `DataTableFkSelect`: a generic `>` primitive at `ui/src/components/common/entity-select.tsx` consolidating four hand-rolled FK `` onto one shared API. Plus a sibling vitest lint guard (`form-select-discipline.test.tsx`) blocking inline `` for backend-validated enums in form components — companion to the Story 2.13 column-discipline guard from `feat_data_table_primitive`. **Migrations (4 components, 7 FK sites):** `create-query-set-modal.tsx` cluster_id (UUID `` → cluster dropdown with health-status dots; label renamed "Cluster ID" → "Cluster"; submit gains explicit required-field guard); `create-study-modal.tsx` 4 FK selects at lines 246 / 283 / 301 / 326 (cluster/query-set/judgment-list/template — child-field reset preserved verbatim); `register-cluster-modal.tsx` config-repo (conditional render → always-visible with empty-state CTA Link — UX improvement, operators previously missed the field's existence); `generate-judgments-dialog.tsx` template (preserves `gen-template` data-testid for the existing E2E spec). **Primitive shape:** generic over `T`, takes `query: UseQueryResult` directly (consumer calls the listing hook in its own scope — refactored mid-implementation from the original `useEntities: () => useHook()` callback shape after hitting rules-of-hooks for dynamic-param consumers like `useQuerySets({ cluster_id })`). Ships loading / error-with-retry / empty-with-CTA / disabled-subset-with-tooltip slots. Opt-in `getStatus` callback renders Unicode-dot status indicators (`●` wrapped `aria-hidden="true"` so screen readers get clean names) with stable green-first sort. Opt-in `inlineWarning` renders an amber `

` under the trigger matching the existing helper-text styling. The `unreachable → unknown` wire-value mapping is caller-side per `HEALTH_STATUS_VALUES` from `enums.ts`. **Modal-level testing landmine:** shadcn `` blocks + one UUID-paste `` onto one shared API. Plus a sibling vitest lint guard (`form-select-discipline.test.tsx`) blocking inline `` for backend-validated enums in form components — companion to the Story 2.13 column-discipline guard from `feat_data_table_primitive`. **Migrations (4 components, 7 FK sites):** `create-query-set-modal.tsx` cluster_id (UUID `` → cluster dropdown with health-status dots; label renamed "Cluster ID" → "Cluster"; submit gains explicit required-field guard); `create-study-modal.tsx` 4 FK selects at lines 246 / 283 / 301 / 326 (cluster/query-set/judgment-list/template — child-field reset preserved verbatim); `register-cluster-modal.tsx` config-repo (conditional render → always-visible with empty-state CTA Link — UX improvement, operators previously missed the field's existence); `generate-judgments-dialog.tsx` template (preserves `gen-template` data-testid for the existing E2E spec). **Primitive shape:** generic over `T`, takes `query: UseQueryResult` directly (consumer calls the listing hook in its own scope — refactored mid-implementation from the original `useEntities: () => useHook()` callback shape after hitting rules-of-hooks for dynamic-param consumers like `useQuerySets({ cluster_id })`). Ships loading / error-with-retry / empty-with-CTA / disabled-subset-with-tooltip slots. Opt-in `getStatus` callback renders Unicode-dot status indicators (`●` wrapped `aria-hidden="true"` so screen readers get clean names) with stable green-first sort. Opt-in `inlineWarning` renders an amber `

` under the trigger matching the existing helper-text styling. The `unreachable → unknown` wire-value mapping is caller-side per `HEALTH_STATUS_VALUES` from `enums.ts`. **Modal-level testing landmine:** shadcn `