diff --git a/CLAUDE.md b/CLAUDE.md index 272636f5..d7eeb973 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -57,7 +57,7 @@ Reach for these before grinding through tasks manually: - `Explore` for any codebase search that would take > 3 queries - `feature-dev:code-explorer` / `code-architect` / `code-reviewer` for new feature work - **Doctrine**: skills override default behavior except user instructions in this file. If a CLAUDE.md rule conflicts with a skill, this file wins. -- **Manual UAT — single master doc**: all multi-day Playwright + React Grab UAT findings go into `docs/superpowers/audits/alpha-uat-master.md` (the cross-cutting "alpha" audit that spans many sessions). Append to it as findings land in chat — don't create per-day files. Buckets: Quick fixes (<15min), Medium (15min–2h), Features/larger (>2h), Bugs (severity-tagged), Cross-references to the active full-codebase audit. Don't ask for the format each time. +- **Manual UAT — currently active doc**: `docs/superpowers/audits/active-uat.md` is the **live** findings doc. Every UAT finding the user surfaces in chat lands here regardless of which session captures it. Persists across sessions until the user explicitly says to wrap the round and archive — at which point rename to `YYYY-MM-DD-uat.md` and start a fresh `active-uat.md`. Buckets: Quick fixes (<15min), Medium (15min–2h), Features/larger (>2h), Bugs (severity-tagged). Tag every entry with status: `OPEN | IN PROGRESS | SHIPPED in | QUEUED | BLOCKED`. Don't ask the format each time. Historical: `alpha-uat-master.md` was the previous master through 2026-05-26 drain. ## Tech stack (non-obvious choices) diff --git a/docs/superpowers/audits/active-uat.md b/docs/superpowers/audits/active-uat.md new file mode 100644 index 00000000..b6960b9e --- /dev/null +++ b/docs/superpowers/audits/active-uat.md @@ -0,0 +1,613 @@ +# Active UAT — running findings + +> **THIS IS THE CURRENTLY ACTIVE AUDIT DOC.** All new UAT findings land here regardless of which session captures them. Persists across sessions until the user explicitly says "wrap this round up and start a fresh one" — at which point archive this file with a date stamp (`YYYY-MM-DD-uat.md`) and start a new `active-uat.md`. +> +> Started 2026-05-26 after the drain commit `e9509dc` cleared the prior `alpha-uat-master.md` long tail. This file is the home for findings surfaced as the user walks through the running app. Append every item as a discrete entry — even premature / aspirational ones — so nothing gets dropped. +> +> **Methodology:** user drives the live CRM at `http://localhost:3000`, surfaces issues in chat (with screenshot + React-grab anchor when applicable). Each finding lands here in the matching bucket with file:line evidence and a status tag. +> +> **Status legend:** +> +> - `OPEN` — captured, not started +> - `IN PROGRESS` — currently being worked on this session +> - `SHIPPED in ` — committed; commit message has detail +> - `QUEUED` — not for this session; deliberately deferred +> - `BLOCKED` — waiting on user input / external repo / clarification +> +> **Severity** (for bugs only): `critical | high | medium | low`. + +--- + +## Bucket 1 — Quick fixes (<15 min) + +### Dialog primitive default too narrow → bump platform-wide + +- **`OPEN`** — _src/components/ui/dialog.tsx_ (DialogContent base default). +- **Symptom:** Dialog primitive's default is `sm:max-w-lg` (512px), which is far too narrow for most content (forms, file previews, signing details). Even the earlier per-dialog `lg:max-w-4xl` bump only fixed the dialogs I explicitly migrated; everything still using the default — including FilePreviewDialog (which overrides to `max-w-4xl` but PDFs are unreadable at that width) — stays cramped on desktop. +- **Fix:** bump the Dialog primitive base to `sm:max-w-2xl lg:max-w-4xl` so every Dialog gets a sane wide-screen default. Per-dialog overrides ride on top for cases that need wider (PDF preview) or narrower (confirm dialogs). + +### FilePreviewDialog cramped for PDFs + +- **`IN PROGRESS`** — _src/components/files/file-preview-dialog.tsx:109_. +- **Symptom:** opening a PDF lands in a `max-w-4xl` (896px) container on a 1920px+ desktop; PDF renders in a thin column with massive empty bands on both sides. Screenshot 2026-05-26. +- **Fix applied:** bumped DialogContent to `w-[min(95vw,1400px)] sm:max-w-none lg:max-w-none h-[85vh]` so PDFs get viewport-sized rendering capped at 1400px. Reference for "correct" width is the documents-tab preview which the user confirmed reads correctly. + +### CreateDocumentWizard — doc-type labels lowercased + +- **`SHIPPED locally (not yet committed)`** — _src/components/documents/create-document-wizard.tsx_ + _src/lib/constants.ts_. +- **Symptom:** doc-type dropdown renders `eoi`, `nda`, `reservation agreement`, `other` — lowercase, looks unfinished. Naive `.replace(/_/g, ' ')` doesn't capitalize. +- **Fix applied:** added `DOCUMENT_TYPE_LABELS` Record alongside the enum (`EOI`, `Contract`, `NDA`, `Reservation Agreement`, `Other`). Wizard reads from the map. + +### CreateDocumentWizard — "Other" hint added + +- **`SHIPPED locally (not yet committed)`** — _src/components/documents/create-document-wizard.tsx_. +- **Decision:** kept schema unchanged. Added an inline hint under the type selector when `other` is selected: "Use the Title below to describe the document — that's how it'll appear everywhere it's referenced." + +### FlatFolderListing — needs padding above the list + +- **`SHIPPED locally (not yet committed)`** — _src/components/documents/documents-hub.tsx_ FlatFolderListing. +- **Symptom:** the flat list sat flush against the subfolders UI above it — no vertical breathing room. +- **Fix applied:** wrapped FlatFolderListing's returned tree in `
` so all three sub-sections (search/chip row, subfolders grid, documents list) get consistent vertical spacing. + +### FlatFolderListing — root folder doesn't show uploaded files + +- **`SHIPPED locally (not yet committed)`** — _src/components/documents/documents-hub.tsx_ FlatFolderListing + _src/lib/services/files.ts_ (listFiles) + _src/lib/validators/files.ts_ (already had folderId; service was ignoring it). +- **Root cause:** documents table (signature workflows) and files table (raw uploads) are separate; FlatFolderListing queried documents only. +- **Fix applied:** went with option B (parallel files query + client-side merge). `listFiles` now honours the `folderId` filter that was already accepted by the validator. FlatFolderListing runs a sibling `useQuery` against `/api/v1/files?folderId=X` and merges both sources into a unified `HubRow` list sorted by `createdAt desc`. New `renderFileRow` renders files with an "Uploaded file" type pill + "Stored" status pill, links the filename to the download URL. Existing FolderDropZone invalidation (`['files']` prefix) already covers the new query, so drag-drop AND New-document-menu uploads both refresh the list without a page reload. + +### FlatFolderListing — chevron does nothing when no signers + +- **`SHIPPED locally (not yet committed)`** — _src/components/documents/documents-hub.tsx:359+_. +- **React-grab anchor:** `` in FlatFolderListing. +- **Symptom:** every row renders a chevron button that's meant to expand signers detail. For docs with zero signers (manually uploaded, or signature docs that were cancelled/voided before recipients were added), clicking does nothing — the button toggles state but no signers panel exists to render. +- **Fix applied:** chevron button only renders when `totalSigners > 0`. Layout column kept (transparent placeholder span) so grid alignment doesn't jump. + +### Interest drawer — inline client create + +- **`SHIPPED locally (not yet committed)`** — _src/components/interests/interest-form.tsx_ + _src/components/clients/client-form.tsx_. +- **Symptom:** rep starts a new interest, realises the client isn't on file, has to close the drawer + navigate to Clients + create + come back. Yacht create was already inline ("Add new" button next to YachtPicker); client create wasn't. +- **Fix applied:** ClientForm gains an `onCreated(id)` callback; the create-branch mutation now returns `{ id }`. InterestForm renders an "Add new" Button next to the Client label (create-mode only — hidden on edit), opens the ClientForm Sheet, and auto-selects the newly-created client into the interest draft on success. + +### InterestForm reset path dropped source='manual' + +- **`SHIPPED locally (not yet committed)`** — _src/components/interests/interest-form.tsx_. +- **Symptom:** `defaultValues` set `source: 'manual'`, but the `!interest && open` reset path didn't include it. Reopening the drawer for a new interest landed on an unselected source dropdown. +- **Fix applied:** reset() block now includes `source: 'manual'` alongside the other create-mode defaults. + +### UploadForSigningDialog — recipients show only one name, no email differentiator + role + +- **`SHIPPED locally (not yet committed)`** +- **Files touched:** _src/components/documents/upload-for-signing-dialog.tsx_ (RECIPIENT_ROLE_META + RecipientRoleBadge helpers + placement-step sidebar render + FieldSidePanel dropdown). +- **React-grab anchor:** `
` in `FieldPlacementStep` in `DialogBody`. +- **Symptom:** placement-step's recipients sidebar (and the FieldSidePanel's "Assign this field to" dropdown) displayed only the recipient's NAME — no email, no role. UAT screenshot showed 4 recipients all literally named "matt 1, matt 2, matt 3, matt 4" with no way to distinguish them; reps editing real docs with duplicate names (e.g. multiple family members on a yacht purchase) hit the same problem. Worse: the failure of the "missing recipientId" error (separate finding below) is silently caused by which-email-maps-to-which-recipient confusion that the rep can't see. +- **Root cause:** the recipient rows in both surfaces were rendered as `r.name || r.email || #signingOrder` — falling back to email ONLY when name was blank. With non-blank names, email never showed. Role was tracked in state (`'SIGNER' | 'APPROVER' | 'CC'` on the Recipient interface) but never rendered. +- **Fix applied:** + 1. New `RECIPIENT_ROLE_META` constant maps each role to display label + tint (Signer blue, Approver amber, CC slate). New `RecipientRoleBadge` component renders the pill. + 2. Sidebar list rewritten as a two-line layout: line 1 is name + role badge, line 2 is the email (or "no email set" placeholder so the row doesn't shift). Email is also surfaced via `title` for hover-truncation tolerance. + 3. FieldSidePanel dropdown SelectItem rebuilt as a stacked layout — name + role badge on top, email muted below — so reps differentiating duplicate-named recipients can pick the right one without expanding the dropdown. +- **Alternatives considered + rejected:** + - Showing only email and dropping name — rejected because the cleaner display people want is "Matthew Ciaccio · matt@gmail.com (Signer)", not pure email. + - Color-coded chip strip instead of a dropdown — rejected for the same density reason captured in the prior "Assign this field to" finding. +- **Effort:** ~30 min (helpers + two render-site rewrites + tsc). +- **Cross-refs:** pairs with the "Assign this field to" label fix (just above). Both ship the same UAT round. +- **Acceptance criteria:** placement-step sidebar shows {color-dot, name, role badge, email} per recipient; FieldSidePanel dropdown options show {#order, name, role badge, email} per option; duplicate-named recipients are visually distinguishable by email. + +### Documenso upload — silent partial-state when field placement fails + +- **`PARTIALLY SHIPPED (rollback gap fixed; comprehensive audit still queued)`** +- **Files touched (this fix):** _src/lib/services/custom-document-upload.service.ts_ (~line 400, placeFields try/catch). _src/components/documents/upload-for-signing-dialog.tsx_ (recipient UI sibling fix shipped separately). +- **Symptom:** rep uploads a PDF, places fields, hits Send. Error toast surfaces: `Documenso response missing recipientId for matt.ciaccio@gmail.com - cannot place fields`. Document appears in the CRM's signing UI AND in Documenso, recipients + roles are wired, but **all placed fields are missing**. The signing UI on the receiving end has no boxes to fill, which means a signer who receives the invite via email lands on a useless page. +- **Root cause:** in `placeFieldsFromUpload`, the placements were built via `fields.map(f => { if (!recipientId) throw ConflictError(...) ...})` BEFORE the surrounding try/catch. The synchronous throw from `map()` bubbled past the catch-and-rollback block that wraps `placeFields()`, so when the recipient lookup missed: + 1. Documenso envelope: already created + distributed (`sendDoc` succeeded earlier in the flow). + 2. Recipients: created with correct roles, signing URLs issued. + 3. Fields: never placed (the throw fired BEFORE the placeFields call). + 4. CRM document row: stuck in `'sent'` status because the rollback only fired inside the try/catch that the throw skipped over. + Result: the partial state the user described. +- **Fix applied (this session):** + 1. The placements `map()` is now INSIDE the same try/catch that wraps `placeFields()`. Any throw — sync or async — triggers the rollback (Document row → cancelled, Documenso envelope → voided). + 2. Pre-throw `logger.error(...)` captures diagnostic state: the missed email, every email the Documenso response DID return. Future "why didn't this match" investigations have something to grep instead of guesswork. + 3. Comment block explaining the dedupe semantic (Documenso de-dupes by email at the envelope level, so duplicate emails across CRM recipient rows all map to the same Documenso recipientId — that's expected behaviour, not a bug). +- **Still open — comprehensive audit:** user explicitly asked for a full pass on the upload-for-signing flow because "we need to do a comprehensive audit of the uploading to be sent through documenso to ensure all fields are wired up correctly and won't cause issues." The rollback gap is one example of multi-step orchestration that fails silently; the wider audit should cover: + 1. **Pre-flight validation BEFORE envelope creation.** Validate every recipient row has a usable email; validate every placed field's `recipientIndex` resolves to a recipient that has a non-empty email; validate at least one field exists when documentType requires one. Failing pre-flight returns a 400 with field-level errors, no Documenso side effects. + 2. **Per-step rollback hardening.** Currently rollback paths exist after `documensoCreate`, `documensoSend`, and `placeFields`, but they're independent try/catches. Refactor into a single sequenced state machine with an idempotent `rollbackTo(step)` helper so future inserts (e.g. metadata write between steps) inherit the rollback automatically. + 3. **Recipient → Documenso identity reconciliation.** Today the code assumes Documenso's response will echo every input email. When dedupe happens (same email → one recipient), the code WORKS correctly (mapping just collapses), but if Documenso silently DROPS a recipient (e.g. invalid email), the lookup throws and we get the silent-partial-state again. Add explicit reconciliation: after `sendDoc`, verify that every distinct email we sent IS present in `sentDoc.recipients`; raise a specific error type if not. + 4. **Idempotency on retry.** If the rep hits Send twice (network blip), do we double-create envelopes? Verify the document row's `documensoDocumentId` is checked before another `documensoCreate` call. + 5. **End-to-end tests covering every failure path.** vitest integration suite for: empty fields list, recipient with blank email, recipient with duplicate email, Documenso 4xx on create, Documenso 4xx on send, Documenso 4xx on place-fields, network timeout mid-flow. + 6. **Audit-log every milestone.** Currently we log on the final success / final failure. Per-step audit entries (`create_envelope`, `send_envelope`, `place_fields`) would make post-mortem investigation tractable. +- **Effort for comprehensive audit:** ~6–10h. ~2h pre-flight validation, ~3h state-machine refactor + tests, ~1h reconciliation logic, ~1h idempotency, ~2h test coverage, ~1h audit-log richness. Bundle as one PR because the pieces interleave. +- **Cross-refs:** + - The `/documents/new` wizard refactor (Bucket 3 — wizard refactor finding) touches the same end-to-end flow — bundle the two so the same audit doesn't re-investigate the upload-for-signing service twice. + - This is the SECOND time a multi-step Documenso flow has had a rollback gap — the first was the EOI auto-cancel/replace flow (fixed earlier in `65ff596`). Pattern: every multi-step orchestration that touches Documenso needs end-to-end rollback OR pre-flight validation. The audit doc's broader "activity feed comprehensive copy" finding mentioned a similar discipline gap; both should land before more multi-step features ship. +- **Open questions for the user:** + 1. **Are you okay with the comprehensive audit being one larger PR (~1-2 days focused), or should it ship as discrete sub-PRs (pre-flight + state-machine + tests)?** Trade-off: single PR is faster but harder to review; sub-PRs are reviewable but you'd see intermediate states. + 2. **Should the pre-flight validation block the dialog Submit button entirely, or surface an inline error and let the rep submit anyway (with "I know there's a missing email" override)?** Default proposal: hard block — Documenso's API can't recover from missing emails, so submitting anyway is guaranteed-to-fail. + +### BerthRecommenderPanel — hide entirely when no desired dimensions set + +- **`SHIPPED locally (not yet committed)`** +- **Files touched:** _src/components/interests/interest-tabs.tsx_ (~line 1467 Overview inline render + ~line 1577 dedicated tab entry + ~line 1521 hasDesiredDims gate variable + ~line 711 OverviewTab inner gate). +- **React-grab anchor:** `
` in `Card` in `BerthRecommenderPanel`. +- **Symptom:** the recommender card rendered even when the rep hadn't entered any desired dimensions on the interest — surfacing only the "Set desired dimensions to see recommendations." guidance subtitle. User flagged that the card AND the dedicated "Berth Recommendations" tab should both be hidden in that state so reps aren't distracted by an empty placeholder. +- **Root cause:** previous design intentionally kept the panel always-mounted with inline guidance ("plan §5.3 — always-mounted card driven by the interest's desired dimensions"). User-experience preference now flips that to hide-entirely. +- **Fix applied:** + 1. Computed `hasDesiredDims = toNum(interest.desiredLengthFt) !== null` once near the top of the InterestTabs component, and once inside OverviewTab (because the Overview's inline render lives inside the child). + 2. Overview tab's BerthRecommenderPanel mount wrapped in `{hasDesiredDims ? : null}` — disappears entirely until length is captured. + 3. Dedicated "Berth Recommendations" tab object spread conditionally into the tabs array (`...(hasDesiredDims ? [tabObject] : [])`) so the tab strip's tab itself vanishes — not just the content. Rep doesn't get a dead-end tab. +- **Why gate on length only (not all three dimensions):** length is the primary ranking input in the recommender's SQL; width / draft fall back to length when missing. Requiring all three would hide the panel for partial-data interests where the recommender still has signal. +- **Alternatives considered + rejected:** + - Show the panel but collapsed by default — rejected because reps still see the empty card; defeats the user's "hide entirely" ask. + - Keep the dedicated tab but show the empty-state inside — rejected for the same reason; the user wants the tab gone too. +- **Effort:** ~15 min. +- **Cross-refs:** related to the Bucket 3 wizard refactor / OverviewTab inheritance finding — both touch what gets shown to a rep on the Overview tab as a function of what data is present. +- **Acceptance criteria:** an interest with `desiredLengthFt = NULL` shows no recommender card on Overview AND no "Berth Recommendations" tab in the strip. Setting desired length via the inline editor causes both to appear immediately (TanStack Query refetch). + +### Per-berth public-map flag — should inherit on subsequent surfaces + +- **`OPEN — needs user clarification on which surface specifically`** +- **React-grab anchor:** `