diff --git a/docs/perf-quality-2026-07-12/findings.md b/docs/perf-quality-2026-07-12/findings.md new file mode 100644 index 00000000..52c63b3a --- /dev/null +++ b/docs/perf-quality-2026-07-12/findings.md @@ -0,0 +1,761 @@ +# Findings — performance & code-quality pass (2026-07-12) + +Produced by the 10-dimension review workflow in [prompt.md](./prompt.md) (workflow run wf_d2e7bc6c-553, 90 agents). +Every finding was adversarially verified: high severity by two skeptics (reachability + impact/fix-safety lenses), medium/low by one. **73 confirmed, 1 refuted.** Skeptic severity adjustments and fix corrections are recorded inline; the implementation follows the corrected fixes. + +The exhaustive lint triage (all 63 warnings) is at the bottom; an adversarial audit of the behavior-sensitive lint fixes produced 1 correction (tour-context), incorporated below. + + +## perf-render-explore + +### 1. Home feed rows are not memoized — every append/filter interaction re-renders the entire feed + +- **id:** `home-feed-rows-not-memoized` · **track:** T1-home +- **severity / effort / regression risk:** medium (adjusted from high) / S / low +- **location:** `src/components/home/home-feed.tsx:758` +- **evidence:** `function HomeFeedRow({ event }: { event: HomeFeedEvent })` (line 758) and `function EndorsementGroupRow({ group }: { group: EndorsementGroupItem })` (line 796) are plain functions rendered from `items.map(...)` in HomeFeedBody (lines 408-418) with no `memo` wrapper — unlike ActivityCard, whose comment (activity-card.tsx:22-23) documents exactly this hazard: "loadMore replaces the activities array with a new identity, so every prior card would otherwise re-render." `useHomeFeed.loadMore` appends with `events: [...prev.events, ...append]` (use-home-feed.ts:364), so prior event object refs ARE stable, but without memo every append re-renders every prior card (FeedCardHead + useAuthorInfo hook + PreviewCard image subtree). Worse, the auto-load loop runs up to `MAX_AUTO_LOADS = 25` sequential loadMore cycles (home-feed.tsx:104, 354-363), re-rendering the full list each cycle; and any HomeFeed parent state change — opening the filter panel (`setFilterOpen`, line 275), ticking an evaluator checkbox (`setCustomEvaluators`) — re-renders all N cards because HomeFeedBody is also unmemoized. Feed can reach 1250 events (25 x PAGE_SIZE 50). +- **fix:** Wrap HomeFeedRow in `memo` (its `event` prop is referentially stable across appends). Wrap EndorsementGroupRow in `memo` with a custom comparator since groupConsecutiveEndorsements rebuilds group objects on every events change: `(a, b) => a.group.key === b.group.key && a.group.createdAt === b.group.createdAt && a.group.subjectDids.length === b.group.subjectDids.length` (a group only changes by absorbing more subjects or changing headline time). Optionally also wrap HomeFeedBody in `memo` so filter-panel open/close doesn't re-execute the list map at all. +- **skeptic fix correction:** memo(HomeFeedRow) and memo(HomeFeedBody) are safe as proposed (loadMore is useCallback-stable per use-home-feed.ts:317, other body props are primitives). But the custom comparator for EndorsementGroupRow is unsafe: comparing only key + createdAt + subjectDids.length ignores subjectDids content and actorProfile, so a filter-change reload that rebuilds the same consecutive run with different subject composition but identical first-URI/createdAt/length would leave a stale expanded subject list. Since groupConsecutiveEndorsements (src/lib/utils/group-feed.ts:31-64) rebuilds groups and subjectDids arrays fresh on every pass, use a full element-wise compare instead — it is O(n) only on re-render attempts and trivially cheap vs. the render it saves: (prev, next) => prev.group.key === next.group.key && prev.group.createdAt === next.group.createdAt && prev.group.actorProfile === next.group.actorProfile && prev.group.subjectDids.length === next.group.subjectDids.length && prev.group.subjectDids.every((d, i) => d === next.group.subjectDids[i]) + +### 2. Search-input keystroke state lives in ExploreMain/ExploreAllBlocks, re-rendering the whole chrome + results tree per keystroke + +- **id:** `explore-search-state-too-high` · **track:** T2-explore +- **severity / effort / regression risk:** medium (adjusted from medium) / S / low +- **location:** `src/components/explore-page/explore.tsx:694` +- **evidence:** `const [localQuery, setLocalQuery] = useState(search)` (line 694) sits in ExploreMain, the component that also renders SubPrefixDropdown, SegmentedControl, two popovers, EndorsementDegreeBar, ResultsArea, and LoadMoreSentinel (lines 726-944). Every keystroke (`onChange={(e) => setLocalQuery(e.target.value)}`, line 756) re-renders that entire subtree during the 350ms debounce window. The row memos hold (sortedUsers/sortedCerts memos at 1903-1920 keep stable refs), but React still re-executes ResultsArea, re-allocates all N `
  • ` elements and runs N memo comparisons per keystroke — O(list length) work per keypress on lists that grow 50/page unboundedly. The identical debounce block is copy-pasted in ExploreAllBlocks (lines 1148-1163), where a keystroke additionally re-renders four AllSection blocks. +- **fix:** Extract a small `ExploreSearchField` component owning `localQuery`, the `lastWroteToUrlRef` sync, and the 350ms debounce, receiving `search: string` and `onCommit: (q: string | null) => void` (the existing `setUrl` patch) as props. Use it in both ExploreMain and ExploreAllBlocks (deleting the duplicated ~25-line debounce block from each). Keystrokes then re-render only the input; the parent re-renders once per committed URL change. + +### 3. No windowing or content-visibility on home-feed / explore list items — offscreen rows are fully laid out and painted + +- **id:** `no-content-visibility-long-lists` · **track:** T11-css +- **severity / effort / regression risk:** medium (adjusted from medium) / S / medium +- **location:** `src/app/styles/home.css:390` +- **evidence:** `.home-feed__item { border-bottom: 1px solid var(--border-subtle); }` (home.css:390) and `.explore__list { ... overflow: hidden; }` (explore.css:700-708) / `.explore__grid { display: grid; gap: 12px; }` (explore.css:945-950) have no containment hints; `grep -rn "content-visibility|contain-intrinsic" src/app/styles/` returns nothing. Both surfaces grow unboundedly: explore appends PAGE_SIZE = 50 per load-more (use-explore.ts:136) and the home feed's auto-loop is sized for up to 1250 events (home-feed.tsx:97-104), each an image-bearing card. Without `content-visibility`, every offscreen card participates in layout/paint on each render and scroll, which is the dominant frame cost once lists pass a few hundred rows — especially the home feed's full-width image cards. +- **fix:** Add `content-visibility: auto; contain-intrinsic-size: auto 140px;` to `.home-feed__item`, and `content-visibility: auto; contain-intrinsic-size: auto 64px;` to `.explore__list > li` (row height ~56-64px) and `contain-intrinsic-size: auto 320px` to `.explore__grid > li` (card height). Tune the intrinsic sizes to each surface's real average row height to keep the scrollbar stable. Verify back-button scroll restoration on /explore (useScrollRestoration, explore.tsx:677) still lands correctly — content-visibility uses the intrinsic-size estimate for offscreen rows, so a badly-off estimate would make restored offsets drift; that is the reason for the medium regression rating. + +### 4. Expanding a grouped endorsement row mounts every endorsed account at once (design target: ~1000) + +- **id:** `endorsement-group-expand-unbounded` · **track:** T1-home +- **severity / effort / regression risk:** medium (adjusted from medium) / S / low +- **location:** `src/components/home/home-feed.tsx:828` +- **evidence:** `{group.subjectDids.map((did) => (
  • ))}` (lines 828-833) renders the full subject list in one commit when `expanded` flips. The grouping is explicitly sized for bursts of ~1000: "a single curator who batch-endorses ~1000 accounts can be fully absorbed into a single grouped row" (lines 97-100). Each EndorsedAccountLink calls `useAuthorInfo(did)` (line 865), which runs a per-instance effect + up to 3 setStates on resolve (use-author-info.ts:43-70) — so one click can mount ~1000 IdentityRows and fire ~1000 hook effects synchronously, a guaranteed multi-hundred-ms main-thread stall. +- **fix:** Window the expanded list: `const [visibleCount, setVisibleCount] = useState(50)` inside EndorsementGroupRow, render `group.subjectDids.slice(0, visibleCount)`, and append a "Show more (N remaining)" button that adds 50 per click (reset visibleCount when `expanded` flips off). Keeps the common case (groups of 2-20) identical while capping the pathological commit. + +### 5. Explore All view fetches 4 x 50 full records to render 5 per block + +- **id:** `all-view-overfetches-4x50-for-5` · **track:** T2-explore +- **severity / effort / regression risk:** medium (adjusted from medium) / M / low +- **location:** `src/components/explore-page/explore.tsx:1010` +- **evidence:** `/** How many results each block on the All view shows. The loader still fetches a full page per kind; we just render the head of each list. */ const ALL_VIEW_BLOCK_SIZE = 5` (lines 1010-1012). ExploreAllBlocks — the default /explore landing state — runs four `useExploreData` loaders (activities/projects/accounts/funding, lines 1168-1205), each requesting `first: PAGE_SIZE` = 50 (use-explore.ts:136, threaded into fetchIndexerActivities/fetchProjects/fetchNetworkActors/fetchFundingReceipts), then slices `.slice(0, ALL_VIEW_BLOCK_SIZE)` (lines 1207-1233). That is 200 full records (activity records with images/workScope, receipts with attestations) fetched, JSON-parsed, and held in state to paint 20 rows — and it re-runs on every committed search or quality-filter change on the busiest entry surface. +- **fix:** Add an optional `pageSize?: number` to `useExploreData` opts, default PAGE_SIZE, threaded through LoadArgs into each loader's `first:` argument (loadAccountsPage/loadProjectsPage/loadCertsPage/loadFundingPage). ExploreAllBlocks passes e.g. `pageSize: 10` (2x block size so a short server page still fills the block after any client-side trimming). Blocks mode never paginates ("Show all" switches to single-kind mode which uses its own loader), so cursor semantics are unaffected. +- **skeptic fix correction:** The fix as written ("threaded into each loader's first: argument") is under-specified and would regress client-side-filtered branches if applied literally. loadAccountsPage's follows/recent branch fetches first: 100 (use-explore.ts:959) and then intersects client-side against followedDids/recently-viewed (lines 966-975); threading pageSize: 10 there would make the All-view "Accounts I follow"/"Recently viewed" blocks render near-empty — an intersection, not the mild "trimming" the 2x buffer covers. Restrict the pageSize threading to the server-paginated first: PAGE_SIZE call sites only (use-explore.ts:998, 1120, 1319, 1360), leaving the first: 100 client-filter branch and the URI-keyed ma-earth/recent fan-outs untouched. Also note the funding block applies a client-side matchesConfirmedBy role filter (explore.tsx:1225-1233) that drops sender-only/third-party-only receipts, so whether 10 fills a 5-row block is data-dependent — use a larger pageSize (or keep PAGE_SIZE) for the funding loader specifically. + +## perf-render-detail + +### 6. Endorsement graph disables autoPauseRedraw, leaving a permanent full-canvas rAF repaint loop + +- **id:** `force-graph-perma-redraw-loop` · **track:** T4-graph +- **severity / effort / regression risk:** high (adjusted from high) / M / medium +- **location:** `src/components/visualization/endorsement-graph.tsx:767` +- **evidence:** Line 764-767: `// Keep repainting when idle so avatars appear as their images finish loading (the ref has no public refresh()).` / `autoPauseRedraw={false}`. In force-graph's render loop (node_modules/react-force-graph-2d/dist/react-force-graph-2d.js:12892) `var doRedraw = !state.autoPauseRedraw || ...` — with the prop false, every animation frame repaints the whole canvas (O(nodes+links) draws incl. per-node getInitials + text layout at endorsement-graph.tsx:590-642) forever, even after the simulation cools down and with no interaction. The graph page burns a core continuously for as long as it is open. The stated reason is only to make late-loading avatar images appear; the preload effect (lines 329-338) sets `img.src` but never hooks `onload`. +- **fix:** Remove `autoPauseRedraw={false}`. In the preload effect, set `img.onload = () => setAvatarEpoch(e => e + 1)` (and onerror) for each newly created Image; add `avatarEpoch` (a useState counter) to `paintNode`'s useCallback deps. force-graph marks `nodeCanvasObject` with `onChange: notifyRedraw` (force-graph.js:11346-11349), so the new function identity sets needsRedraw and triggers exactly one repaint per loaded avatar. Hover/zoom/drag already set needsRedraw internally, so highlighting keeps working. +- **skeptic fix correction:** The fix direction is right but incomplete: force-graph's backgroundColor onChange (force-graph.js:12082-12085) only sets canvas.style.background and does not trigger a canvas redraw, and the theme observer (endorsement-graph.tsx:296-309) updates colorsRef (a ref) without changing paintNode/linkColor identity. With autoPauseRedraw removed, flipping data-theme while the graph is open would leave nodes/links painted in stale theme colors until the next interaction, violating the project's dark-mode hard rule. Use a single repaintEpoch useState counter bumped (a) in each preloaded Image's onload/onerror and (b) in the theme observer's apply(), and add it to the useCallback deps of BOTH paintNode and linkColor (linkColor also reads colorsRef). Hover highlighting is unaffected since hoverId-derived highlightNodes is already in both deps. + +### 7. Force-config effect keyed on size.w re-applies forces and reheats the simulation on every container resize + +- **id:** `force-graph-reheat-on-resize` · **track:** T4-graph +- **severity / effort / regression risk:** medium (adjusted from medium) / S / low +- **location:** `src/components/visualization/endorsement-graph.tsx:490` +- **evidence:** Lines 451-490: the effect rebuilds collide/radial/gravity forces, resets charge/link strengths, and calls `fg.d3ReheatSimulation?.()`, with deps `[layout, data, size.w]`. `size` is fed by a ResizeObserver (lines 312-321) that fires on every 1px width change. Any window drag, sidebar toggle, fullscreen enter/exit, or mobile URL-bar show/hide re-heats the simulation — nodes jiggle and re-layout — and when the engine stops again `onEngineStop={() => fgRef.current?.zoomToFit(400, 40)}` (line 771) discards the user's zoom/pan. `size.w` is in the deps only to catch the mount transition (comment lines 447-449: 'once size.w flips positive — that's the render where the graph mounts'). +- **fix:** Replace the `size.w` dep with a boolean that only flips once: `const graphMounted = size.w > 0` and use `[layout, data, graphMounted]` as deps (early-return when `!graphMounted`). Boolean identity is stable for all subsequent resizes, so forces are re-applied only on mount, layout switch, or data change — resizes just update the width/height props without a reheat. + +### 8. useContextUpdates has no cache/coalescer, so activity- and project-detail fetch the same updates list twice per mount and the navbar count goes stale + +- **id:** `context-updates-duplicate-fetch` · **track:** T3-detail +- **severity / effort / regression risk:** medium (adjusted from medium) / M / medium +- **location:** `src/components/feed/activity-detail.tsx:290` +- **evidence:** activity-detail.tsx:290-293 `const { updates } = useContextUpdates(rkey ? \`at://${did}/...\` : null)` (count for the navbar title, mounted on every tab) AND the `` child (lines 1579-1586 overview, 1730-1738 updates tab) which calls `useContextUpdates(subjectUri)` itself (context-updates.tsx:92-93). use-context-updates.ts:56-102 is a plain per-instance effect fetch — no module cache or in-flight sharing — so every overview/updates mount issues two identical network requests, and each tab switch remounts the child and refetches. Same on project-detail.tsx: lines 297-301 (navUpdates, gated to the updates tab) plus lines 1465-1472 (ContextUpdates variant="full") double-fetch on the Updates tab. Bonus correctness bug: after deleting an update inside ContextUpdates (`removeUpdate` mutates only that instance's state), the outer instance's `updatesCount` — the mobile "Updates (N)" title — stays stale. +- **fix:** Add a module-level uri-keyed cache + in-flight coalescer to useContextUpdates, mirroring the useActivity pattern from the prior pass: `const cache = new Map` shared across instances, with `refetch`/`removeUpdate` invalidating/patching the shared entry and notifying subscribers (useSyncExternalStore or a version counter) so both instances converge. This collapses the duplicate requests, makes tab switches hit the cache, and keeps the navbar count in sync with deletes. +- **skeptic fix correction:** The shared-cache fix is right but incomplete as written: because update creation happens on a separate route (/[actor]/[type]/[rkey]/update/new), a session-long cache invalidated only through the hook's refetch/removeUpdate would serve the stale pre-create list when the user navigates back (today's per-mount refetch guarantees freshness). Mirror the useActivity pattern fully: export an invalidateContextUpdates(subjectUri) and call it from the update create/edit save paths (as the activity edit route calls invalidateActivity), and/or give cache entries a short TTL via fetchedAt so remounts revalidate. Also move the removed-URIs tombstone set (removedUrisRef) into the shared cache layer, otherwise a cache-hydrated second instance can resurrect a just-deleted update that the indexer is still serving. + +### 9. ProfileEndorsements calls uncached useGivenEndorsements twice with the same DID, doubling an already expensive fetch fan-out + +- **id:** `given-endorsements-duplicate-hook-call` · **track:** T5-profile +- **severity / effort / regression risk:** medium / S / low +- **location:** `src/components/profile/profile-endorsements.tsx:216` +- **evidence:** Line 131 `const given = useGivenEndorsements(did)` and lines 216-218 `const ownGivenForModal = useGivenEndorsements(canManage ? did : null)`. When `canManage` is true both arguments are the identical `did`; when false the second returns nothing — so `ownGivenForModal` is always either empty or an exact duplicate of `given`. use-endorsements.ts:60-143 has no module cache (unlike useReceivedEndorsements, use-received-endorsements.ts:234-245), and each instance runs `listDefinitions + listAwards + endorsementDefUriSet` (lines 85-103) — the last one fans out per foreign badge definition. Every owner visit to their Endorsements tab runs this whole chain twice in parallel; profile-overview.tsx:116 runs it again on every Overview<->Endorsements tab flip (tabs are conditionally mounted, [actor]/page.tsx:524-624). +- **fix:** Delete the second hook call: compute `ownAlreadyEndorsedDids` from `given.endorsements` (gated on `canManage`) and replace `ownGivenForModal.refetch()` in onCompleted (line 635) with the existing `given.refetch()`. Optionally (M) also add a did-keyed module cache + STALE_MS to useGivenEndorsements mirroring useReceivedEndorsements, so Overview/Endorsements tab flips stop re-running the fan-out; keep `refetch(force)` writing through the cache. + +### 10. useRouteRkey derives rkey in an effect, forcing a guaranteed second render of the 2300-line ActivityDetail on mount and delaying all rkey-gated work by a commit + +- **id:** `route-rkey-effect-double-render` · **track:** T3-detail +- **severity / effort / regression risk:** medium / S / low +- **location:** `src/components/feed/activity-detail.tsx:1828` +- **evidence:** Lines 1828-1845: `const [rkey, setRkey] = useState(null); useEffect(() => { ... setRkey(decodeURIComponent(last)) }, [])` — rkey is always null on the first client render and flips via setState, re-rendering the entire ActivityDetail tree. Everything keyed on rkey starts a commit late: `useActivityFunding(did, rkey)` (line 287), `useContextUpdates(rkey ? ... : null)` (290), `usePageRecordMenu(rkey ? {...} : null)` (706-730), `useCertProjects(did, rkey)` via CertHeadlineColumns (2154), and the title-row AddToListMenu (1105) which pops in on the second paint. The same component already calls `usePathname()` (line 410), which returns the path synchronously on the first render. +- **fix:** Replace the effect+state with a pure derivation: `const rkey = useMemo(() => { const segs = (pathname ?? "").split("/").filter(Boolean); const last = segs[segs.length - 1] ?? null; if (!last) return null; try { return decodeURIComponent(last) } catch { return last } }, [pathname])` using the existing `usePathname()` (hoist that call above line 273). Removes the mount double-render and starts the funding/updates/projects fetches and record-menu publication one commit earlier; delete useRouteRkey. + +### 11. Inline onToggle arrows defeat the memo on GivenListRow/ReceivedListRow, re-rendering every list row on each search keystroke + +- **id:** `endorsement-row-inline-ontoggle-defeats-memo` · **track:** T5-profile +- **severity / effort / regression risk:** low (adjusted from low) / S / low +- **location:** `src/components/profile/profile-endorsements.tsx:1195` +- **evidence:** Line 1195 `onToggle={() => onToggleOne(e.uri)}` (GivenList) and line 1329 (ReceivedList) hand each memoized row (`const GivenListRow = memo(...)` line 1206, `ReceivedListRow` line 1342) a fresh arrow every parent render. The component's own comment (lines 335-338) says stable callbacks were built precisely 'so the memoized cards/rows don't re-render on every keystroke just because a fresh inline arrow was handed down' — but the list view does exactly that: every keystroke in the search input (line 411 setQuery) re-renders all visible rows, each re-running useAuthorInfo/useReceivedIssuerInfo, EndorsementRowBody, ResponseMenu, and Checkbox. +- **fix:** Change the row props from `onToggle: () => void` to `onToggleOne: (uri: string) => void` (the already-stable useCallback at line 261) and call `onToggleOne(endorsement.uri)` inside the row. Rows then only re-render when their `selected` flag or endorsement data actually changes. + +## perf-data + +### 12. useReceivedEndorsements fires 3 concurrent identical indexer scans per cold profile visit (cache has no in-flight dedupe) + +- **id:** `received-endorsements-no-singleflight` · **track:** T6-hooks +- **severity / effort / regression risk:** high (adjusted from high) / S / low +- **location:** `src/hooks/use-received-endorsements.ts:400` +- **evidence:** doScan checks only the settled cache before scanning: `const cached = cache.get(did); if (cached && Date.now() - cached.fetchedAt < STALE_MS) {...} ... const data = await scanReceivedEndorsements(did, signal)` (lines 400-412). There is a module cache (line 245) but no in-flight promise map, and scanReceivedEndorsements paginates the ReceivedEndorsements indexer op at 100/page (lines 102-147). On /[actor] the hook mounts simultaneously in ProfileHeader (profile-header.tsx:84), ProfileSidebar (profile-sidebar.tsx:167) and ProfileOverview (profile-overview.tsx:115) — all three see a cold cache and each runs its own full scan (3x every POST /api/indexer page). The repo's own gold-standard pattern exists in use-profile-responses.ts (`inflightByDid` Map, lines 59+73-113). +- **fix:** Add a module-level `const inflightByDid = new Map>()`. In doScan, after the cache check: if an in-flight promise exists for `did`, await it and set state from its result; otherwise create the promise (NOT tied to any single caller's AbortSignal — mirror the useTypedLists comment at use-typed-lists.ts:112-117 so one unmount can't fail siblings), store it, delete in finally, and cache.set on resolve. Each hook instance keeps its own signal?.aborted guard before setState. + +### 13. useFollowers duplicates the full paginated follower walk across 3-4 simultaneous consumers + +- **id:** `followers-no-singleflight` · **track:** T6-hooks +- **severity / effort / regression risk:** medium (adjusted from medium) / S / low +- **location:** `src/hooks/use-followers.ts:146` +- **evidence:** doFetch (line 146) checks `cache.get(targetDid)` then runs `while (collected.length < 10_000) { const page = await fetchFollowersPage(targetDid, cursor, signal) ... }` (lines 170-176, 100/page). No in-flight map. Consumers mounting together on a profile: profile-header.tsx:82, profile-sidebar.tsx:161, profile-follow-endorse.tsx:60, plus profile-followers.tsx:75 on that tab — each cold mount runs its own multi-page indexer walk of the same DID. +- **fix:** Same singleflight as the received-endorsements fix: module-level `inflight = new Map>()` consulted in doFetch after the cache check; shared promise not bound to a caller signal; writeCache + delete-from-map on settle. refetch() (force=true) should bypass both cache and the in-flight map (abort semantics unchanged: per-instance signal guards stay). + +### 14. useFollowing duplicates the PDS listRecords page-walk across simultaneous consumers + +- **id:** `following-no-singleflight` · **track:** T6-hooks +- **severity / effort / regression risk:** medium (adjusted from medium) / S / low +- **location:** `src/hooks/use-following.ts:87` +- **evidence:** doFetch (line 87) checks the settled cache then `const result = await listFollowing(targetDid, signal, ...)` (line 108); listFollowing paginates /api/xrpc/com/atproto/repo/listRecords with a cursor loop (follow.ts:212-248). No in-flight dedupe. Mounted concurrently for the same DID by profile-sidebar.tsx:160 and :190 (viewed + viewer), profile-header.tsx:83, profile-follow-endorse.tsx:57, profile-followers.tsx:76, home-feed.tsx:135 — a cold own-profile visit runs the identical page-walk 2-4 times. +- **fix:** Add the same module-level singleflight Map keyed by DID in front of listFollowing, sharing one promise across instances (not bound to any caller's signal); cache.set on resolve; refetch(force) bypasses both. Preserve the `truncated` flag in the shared result. + +### 15. useGivenEndorsements has neither cache nor in-flight dedupe — 2-3 duplicate two-call PDS loads per own-profile view + +- **id:** `given-endorsements-no-cache` · **track:** T6-hooks +- **severity / effort / regression risk:** low (adjusted from medium) / M / medium +- **location:** `src/hooks/use-endorsements.ts:70` +- **evidence:** load (line 70) always fetches: `const [defs, awards] = await Promise.all([listDefinitions(did, signal, opts), listAwards(did, signal, opts)])` (lines 85-88) plus cross-repo def lookups via endorsementDefUriSet (line 98) — with zero module-level cache or coalescing (state is per-instance only). Consumers mounting simultaneously: profile-overview.tsx:116 (given(did)), profile-follow-endorse.tsx:224 (ownGiven — same DID on own profile), profile-endorsements.tsx:131 + 216, endorsements/page.tsx:264. Every one repeats both PDS listRecords walks and the cross-repo getRecord fan-out. +- **fix:** Add the module cache + singleflight pattern used by use-received-endorsements (5-min STALE_MS keyed by DID, plus an inflight Map). Keep refetch() force-bypassing cache + inflight with noCache:true (already passed as `opts`) so post-write freshness is unchanged. Invalidate the cache from the same mutation paths that call refetch today. +- **skeptic fix correction:** Do not add the 5-min module cache + singleflight. The hook's docstring (use-endorsements.ts:52-58) documents fresh-on-every-mount as a deliberate design ("fresh on every reload, no caching concerns, works for unauthenticated visitors"), and a module TTL cache creates cross-consumer staleness obligations: every mutation path (EndorseButton writes on foreign profiles, bulk revokes on /endorsements, Given-tab revokes in profile-endorsements) must invalidate it, and missing one yields up-to-5-min-stale endorsement state after a write. The only real duplication is inside profile-endorsements.tsx itself: ownGivenForModal (line 216) is always either null or the exact same DID as given (line 131). Delete the second hook instance and derive ownAlreadyEndorsedDids from given.endorsements (gated on canManage). If cross-component dedupe is ever wanted later, an inflight-only singleflight Map keyed by did (no TTL) preserves the documented freshness semantics while collapsing concurrent mounts. + +### 16. CONFIRMED: useHyperboard runs displayProfile fetches as a second global barrier after all identity resolutions + +- **id:** `hyperboard-displayprofile-waterfall` · **track:** T6-hooks +- **severity / effort / regression risk:** medium (adjusted from medium) / S / low +- **location:** `src/hooks/use-hyperboard.ts:133` +- **evidence:** Stage A (lines 115-126): `await Promise.all([...idStrings].map(async (s) => { const r = await loadResolvedProfile(s) ... }))`. Only after every resolution settles does Stage B run (lines 129-138): `for (const s of idStrings) if (isDid(s)) dids.add(s); ... await Promise.all([...dids].map(async (did) => { const dp = await fetchDisplayProfile(did) ... }))`. Identities that are already DIDs (line 131) need no resolution at all, yet their getRecord displayProfile fetch waits behind the slowest handle resolution plus the 16ms resolve-dids batch window (resolve-did-batch.ts:39). Total latency = max(resolve) + max(displayProfile) instead of max over per-identity pipelines. +- **fix:** Collapse into one Promise.all over idStrings that pipelines per identity: for bare-DID identities start `fetchDisplayProfile(s)` immediately (concurrent with `loadResolvedProfile(s)`); for handle identities chain `fetchDisplayProfile(r.did)` off the resolve result (and also fetch for r.did when it differs from s). Populate the same `resolved` and `displayProfiles` Maps; buildBoardEntries and state writes are untouched. Common case (contributorInfo identifiers are DIDs) drops from 2 sequential round-trips to 1. + +### 17. useEndorsementLists permanently bypasses its cache after the first list mutation in a session + +- **id:** `endorsement-lists-permanent-force` · **track:** T6-hooks +- **severity / effort / regression risk:** medium (adjusted from medium) / S / low +- **location:** `src/hooks/use-endorsement-lists.ts:215` +- **evidence:** The load effect computes `const force = listsVersion > 0; doFetch(did, controller.signal, force)` (lines 211-218). listsVersion is the module-level monotonically increasing version from endorsement-lists-cache and never resets, so after ANY list mutation, every subsequent mount of the hook — for the rest of the session, for any DID — passes force=true, which skips the cache check (lines 150-157) AND passes noCache:true to listAwards (line 163), defeating both the module cache and the proxy's HTTP cache on every mount. +- **fix:** Adopt the versioned-cache-key pattern already proven in use-typed-lists.ts:93 (`const cacheKey = `${targetDid}:${version}``): key the module cache by `${did}:${listsVersion}` (or store `version` in the CacheEntry and treat a mismatch as a miss). Then the effect never needs force; only user-invoked refetch() keeps force/noCache. A version bump naturally misses the cache exactly once. +- **skeptic fix correction:** Use the finding's option B, not the "effect never needs force" simplification: store `version` in CacheEntry, treat a version mismatch as a miss, AND pass force/noCache on exactly that version-driven miss. badges.ts:204-206 documents that post-write refetches need noCache to beat the proxy's 5s listRecords cache; a bump-driven refetch without noCache can capture a <=5s-stale listAwards response and pin it in the module cache for up to STALE_MS. If the `${did}:${version}` key variant is chosen instead, add LRU eviction (use-typed-lists.ts:34 MAX_CACHE_ENTRIES exists for exactly this leak) and migrate the mutation callbacks' cache.set(targetDid,...) and refetch's cache.delete(targetDid) to the versioned key. + +### 18. useCgsMemberships resolves each group via an individual GET /api/resolve-did instead of the batched coalescer + +- **id:** `cgs-memberships-resolve-n-plus-1` · **track:** T6-hooks +- **severity / effort / regression risk:** medium (adjusted from medium) / S / low +- **location:** `src/hooks/use-cgs-memberships.ts:69` +- **evidence:** Hydration maps per membership: `const res = await authFetch(`/api/resolve-did?did=${encodeURIComponent(m.groupDid)}`, { signal })` (lines 66-72) — one request per group. resolve-did-batch.ts:10-13 documents that this exact per-row GET pattern is what blew the route's 60/min rate limit and motivated the POST /api/resolve-dids batcher; loadResolvedProfile also gives session caching that this hook currently lacks (refresh/nonce refires all K requests). +- **fix:** Replace the per-DID authFetch with `loadResolvedProfile(m.groupDid)` inside the existing Promise.all (fields map 1:1: handle/displayName/description/avatar). K groups collapse into one coalesced POST and repeat mounts hit the module cache. Keep the signal.aborted guard before setState (loadResolvedProfile has no signal; the fetch completing in background only warms the shared cache). + +### 19. Ma Earth featured Projects filter fans out one PDS getRecord per curated URI; the indexer batch pattern used for certs is missing for collections + +- **id:** `ma-earth-projects-pds-fanout` · **track:** P15-ma-earth +- **severity / effort / regression risk:** medium (adjusted from medium) / M / medium +- **location:** `src/hooks/use-explore.ts:1048` +- **evidence:** `const res = await fetchProjectsByUris(itemUris, signal ?? undefined)` where fetchProjectsByUris does `await Promise.all(uris.map((u) => fetchOne(u, signal)))` (records-by-uri.ts:105-109) — one /api/xrpc getRecord per URI, uncached, re-fired on every filter activation. indexer.ts:224-231 notes curated sets 'can carry well over 100 URIs', and the certs-side Ma Earth path already solved this with the chunked indexer batch fetchIndexerActivitiesByUris (50 URIs/request). No CollectionsByUris op exists in the proxy (checked OPERATIONS in src/app/api/indexer/route.ts:138-1069). +- **fix:** Add a `CollectionsByUris` operation to the indexer proxy mirroring ActivitiesByUris (`orgHypercertsCollection(where: { uri: { in: $uris } })`, 50-URI chunks), a `fetchIndexerProjectsByUris` in indexer.ts reusing chunkArray, and use it in the MA_EARTH_FILTER projects branch. Keep the PDS fetchProjectsByUris path for the small 'recent' branch (its not-yet-indexed rationale at lines 1283-1286 still applies); for the curated featured set, indexer-ingestion parity with the certs featured path is already the accepted tradeoff. +- **skeptic fix correction:** The fix is directionally right but needs three additions to avoid regressions: (1) preserve curated display order — fetchProjectsByUris returns records in the curator's item order (Promise.all preserves input order), while the indexer returns indexed order; re-sort the merged result by an itemUris order map exactly like the recent branch does at use-explore.ts:1100-1103. (2) Carry authorLabels/excludeAuthorLabels through the new CollectionsByUris op (mirroring ActivitiesByUris) and update the now-stale "ignores the org-quality filter by design" comment at use-explore.ts:1030-1036 — the stated design reason (no label-filterable connection) disappears with this change. (3) Verify upstream magic-indexer actually supports where: { uri: { in } } on orgHypercertsCollection before shipping — this repo only proves the pattern on orgHypercertsClaimActivity; if unsupported it needs a magic-indexer change first (same cross-repo dependency pattern as PR #173). Also note legacy-shape curated records will render "Untitled project"/no banner via the indexer (route.ts:776-781, documented-intentional) — acceptable but worth flagging to the curator. + +### 20. buildGraph fetches NetworkActorsByDids chunks sequentially instead of in parallel + +- **id:** `endorsement-graph-sequential-chunks` · **track:** T4-graph +- **severity / effort / regression risk:** low (adjusted from low) / S / low +- **location:** `src/hooks/use-endorsement-graph.ts:270` +- **evidence:** `for (let i = 0; i < allDidList.length; i += PROFILE_CHUNK) { ... const resolved = await fetchNetworkActorsByDids(chunk, signal); ... }` (lines 270-281). Each 100-DID chunk is an independent indexer POST, but they run serially — a 600-participant graph pays 6 sequential round-trips before the canvas can render, on top of the two award scans. +- **fix:** Build all chunks first, then `const results = await Promise.all(chunks.map((c) => fetchNetworkActorsByDids(c, signal)))` and merge into `profiles` afterwards (one aborted check after the await). Order of merging is irrelevant — entries are keyed by DID. + +### 21. fetchDisplayProfile / fetchBoardForActivity cache only settled results — concurrent callers duplicate the fetch + +- **id:** `displayprofile-board-no-inflight` · **track:** T6-hooks +- **severity / effort / regression risk:** low (adjusted from low) / S / low +- **location:** `src/lib/atproto/hyperboard.ts:234` +- **evidence:** fetchDisplayProfile: `const cached = displayProfileCache.get(did); if (cached !== undefined) return cached` then fetches, with `displayProfileCache.set(did, result)` only after the await (lines 234-252). fetchBoardForActivity is identical (cache check line 195, set line 215) and its miss path is a full listRecords pagination walk (lines 155-177). Two concurrent callers for the same key (e.g. useDisplayProfile editor + useHyperboard tiles for the viewer's own DID, or the per-identity pipeline introduced by the waterfall fix resolving a handle back to an already-queued DID) each issue their own network request. Contrast use-org-marker.ts:28's `inFlight` Map, the established fix. +- **fix:** Add `const inFlight = new Map>()` in front of both functions (check cache, then inFlight, else create + store promise, delete in finally, cache on resolve) — the exact fetchOrgMarker pattern in use-org-marker.ts:52-123. Invalidate helpers should also clear the inFlight entry, as use-org-marker.ts:170-173 does. + +## perf-server-cache + +### 22. Indexer proxy is POST-only, so hot public zero-variable queries (network counts, graph scans) can never hit the Vercel edge cache + +- **id:** `indexer-cacheable-get-variant` · **track:** T7-server +- **severity / effort / regression risk:** high (adjusted from high) / M / low +- **location:** `src/app/api/indexer/route.ts:1712` +- **evidence:** The only handler is `export async function POST(request: NextRequest)` (line 1712) and the upstream response is returned with no Cache-Control at all (lines 1793-1799: `return new NextResponse(responseBody, { status: upstream.status, headers: { "Content-Type": ... } })`). POST responses are never edge-cached by Vercel. Meanwhile the five zero-variable count ops ProfileCount/OrganizationCount/ActivityCount/ProjectCount/AwardCount (lines 646-719) are fired by `fetchNetworkCounts` (src/lib/atproto/indexer.ts:1092, which maps over COUNT_SPECS at lines 984-996) as 5 parallel POSTs for EVERY /welcome visitor — 5 serverless invocations + 5 indexer GraphQL round-trips per anonymous page view, for numbers that change on the order of hours. Same story for `AllEndorsements` (line 346), a full-network paginated scan per /endorsement-graph visitor. This is the known gap 'cache-indexer-proxy-no-shared-cache'. +- **fix:** Add a `GET` handler in the same route file for an explicit CACHEABLE_OPS allowlist. Safe set: the five zero-variable count ops (variables always `{}`), plus optionally `OrganizationDids` and `AllEndorsements` (public, viewer-independent, staleness-tolerant; cache key = full query string so `op`/`badgeType`/`first`/`after` are part of the key). Handler: read `op` (+ optional `badgeType`/`first`/`after`) from searchParams, reject anything outside the allowlist with 400, reuse the existing OPERATIONS map + buildVariables, and return the upstream body with `Cache-Control: public, s-maxage=300, stale-while-revalidate=86400` for counts (use s-maxage=60, SWR=600 for AllEndorsements pages). Only set the cache header when the upstream returned 200 AND the body has no `errors` (parse once, cheap at this size) so a transient GraphQL error is not pinned for 5 minutes. NEVER add to the allowlist: FundingReceipts / FundingReceiptsForActivity (money-adjacent attestation state must not be stale-shared), ReceivedEndorsements / EvaluatorEndorsements (accept/reject `response.state` drives UX immediately after a user action), FollowerEvents / HydrateFeedPage / EndorsementClosure (viewer-derived variables, cache-key explosion), and any op invoked with `search`/label variance. No CSRF check needed on the GET (read-only, no credentials, allowlisted ops only); keep the IP rate limiter — edge hits never reach the function anyway. Client side: switch `fetchCount` (and the endorsement-graph loader if included) from POST to the GET form. + +### 23. Foreign blob proxy sets max-age but no s-maxage, so immutable CID-addressed blobs are re-streamed through a serverless function for every visitor + +- **id:** `foreign-blob-no-smaxage` · **track:** T7-server +- **severity / effort / regression risk:** high (adjusted from high) / S / low +- **location:** `src/app/api/xrpc/[...method]/route.ts:107` +- **evidence:** `const FOREIGN_BLOB_CACHE_HEADERS = { "Cache-Control": "public, max-age=3600, immutable" }` (lines 107-109), applied in `proxyPublicGetBlob` (line 371). Vercel's edge cache only stores function responses that carry `s-maxage` (or SWR); `max-age` alone is browser-only. So every avatar/banner/cert image (the single highest-fan-out asset class in the app — every feed row, profile row, explore card) invokes the function and re-streams up to MAX_FOREIGN_BLOB_SIZE = 10MB (line 91) from the origin PDS once per browser, per region, per hour. The content is keyed by `did`+`cid` in the query string and CIDs are content-addressed and immutable — the response literally cannot change. +- **fix:** Change FOREIGN_BLOB_CACHE_HEADERS to `"public, max-age=3600, s-maxage=86400, immutable"` (or add `Vercel-CDN-Cache-Control: max-age=31536000` to keep the shared TTL out of the browser directive). The cache key already varies on the full URL (did + cid), and the capStream byte-cap plus nosniff/CSP headers are preserved unchanged. Error responses (413/502/upstream non-OK) already go through NextResponse.json without these headers, so only successful streams get pinned. +- **skeptic fix correction:** The fix is correct but must also update src/app/api/xrpc/[...method]/__tests__/get-blob-foreign.test.ts line 118, which asserts the exact string "public, max-age=3600, immutable" — otherwise CI fails. If the Vercel-CDN-Cache-Control variant is chosen instead, add a matching assertion there too. +- **skeptic fix correction:** The s-maxage=86400 fix is right, with two adjustments: (1) src/app/api/xrpc/[...method]/__tests__/get-blob-foreign.test.ts lines 117-119 assert the exact string "public, max-age=3600, immutable" — update that assertion in the same commit or the test suite breaks. (2) Drop the alternative "Vercel-CDN-Cache-Control: max-age=31536000" option: blobs are immutable but deletable (avatar removal, takedown), and a year-long edge pin has no purge path besides redeploys; 24h s-maxage bounds takedown latency and is the safer primary. + +### 24. xrpc GET restores the full OAuth session for every signed-in request, including foreign-repo/blob reads that never use the agent + +- **id:** `xrpc-get-eager-oauth-restore` · **track:** T7-server +- **severity / effort / regression risk:** medium (adjusted from medium) / M / medium +- **location:** `src/app/api/xrpc/[...method]/route.ts:405` +- **evidence:** Lines 405-419: `let agent: Agent | null = null; if (did) { const client = await getOAuthClient(); const oauthSession = await client.restore(did); agent = new Agent(oauthSession) }` runs before the method switch, for every GET. But all three branches first check `repo !== did` / `blobDid !== did` (lines 439, 471, 521) and route foreign targets to the plain-fetch proxies that never touch `agent`. `client.restore(did)` costs at least one Upstash REST round-trip (RedisSessionStore.get, stores.ts:70-74) plus JWK/DPoP deserialization, and can trigger a full token refresh against the auth server. Feeds and profile pages fan out dozens of foreign getBlob/listRecords calls per page for signed-in users — each one pays this for nothing. +- **fix:** Compute `did` with getSessionDid() as today (needed for the same-repo comparison), but move agent construction into a lazily-invoked helper (`const getAgent = async () => { try { const client = await getOAuthClient(); return new Agent(await client.restore(did)) } catch (err) { logSafe(...); await deleteSession(); return null } }`) and call it only inside the same-session branches (repo === did / blobDid === did) and in the auth-required methods (getSession, listAppPasswords — return 401 when it yields null). Preserve the existing semantics: restore-failure still deletes the session, and same-session reads still fall back to the public proxy when the helper returns null. Non-public methods with a cookie but no restorable session must still 401 — keep that check inside the two auth-required cases. + +### 25. Group registration walks every group's member list even when the user's total membership count cannot reach the self-created-org limit + +- **id:** `register-org-limit-walk-no-early-exit` · **track:** T7-server +- **severity / effort / regression risk:** medium (adjusted from medium) / S / low +- **location:** `src/app/api/groups/register/route.ts:116` +- **evidence:** Lines 116-153: after collecting `allGroups` from membership.list, the route unconditionally runs the member-list walk — for each group in batches of 5, `createGroupClient(...).call("app.certified.group.member.list", ...)`, where every call first mints a fresh service-auth token via a PDS round-trip (`getServiceAuthToken` inside `callGroupServiceJson`, proxy-agent.ts:254; tokens are one-shot per service-auth.ts:14-15 so they cannot be batched). `selfCreatedCount` can never reach `MAX_SELF_CREATED_ORGS` (= 5, constants.ts:16) when `allGroups.length < 5`, so for the typical registrant with 0-4 memberships the entire walk — up to 4 × (PDS getServiceAuth + CGS member.list, possibly paginated) sequential-ish round-trips — is provably wasted latency on the registration critical path, and a wasted 503 risk (line 161-167 fails the whole registration if the walk throws). +- **fix:** Right after the membership pagination loop, add an early exit: `if (allGroups.length < MAX_SELF_CREATED_ORGS) { /* cannot hit the cap; skip the member walk */ }` — i.e. only run the addedBy walk when `allGroups.length >= MAX_SELF_CREATED_ORGS`. The boolean outcome is identical because selfCreatedCount <= allGroups.length. +- **skeptic fix correction:** The guard itself is behavior-correct, but applying it as-is breaks src/app/api/groups/register/__tests__/org-limit-member-walk.test.ts: that test's membership.list mock returns exactly one group (line 76) and asserts memberListCall was called exactly once (line 135); with the allGroups.length < MAX_SELF_CREATED_ORGS guard the walk is skipped and the mock is called 0 times. Apply the guard AND update the test: change its membership.list mock to return >= 5 groups so the page-level early exit (quality-056-authz-repo-4) is still exercised, and add a second test asserting createGroupClient/member.list is never called when the user has fewer than 5 memberships. Also drop the '503 risk' rationale from any commit message — the per-group try/catch already prevents the walk from throwing. + +### 26. Public group profile/metadata GETs return no Cache-Control, so per-row org fan-outs re-fetch the same PDS records on every page view + +- **id:** `groups-profile-metadata-no-cache-headers` · **track:** T7-server +- **severity / effort / regression risk:** low (adjusted from medium) / S / medium +- **location:** `src/app/api/groups/[groupDid]/profile/route.ts:61` +- **evidence:** profile GET returns `return NextResponse.json(data.value)` (line 61) and the absent-profile case `NextResponse.json(null, { status: 200 })` (lines 41, 55) with no Cache-Control; metadata GET does the same (`return NextResponse.json(data.value)`, metadata/route.ts:53). These are unauthenticated public PDS reads (the route comment itself says reads are public, line 44) hit once per org row: `resolveGroups` calls `getOrgProfile` per membership (lib/groups/api.ts:481), and per issue #156 'many feed/list rows reference group DIDs'. The xrpc route already established the precedent for exactly this class of read: `FOREIGN_READ_CACHE_HEADERS = { "Cache-Control": "private, max-age=30" }` (xrpc route lines 150-152). Without any header, every mount of every org row is a full function invocation + DID-doc resolve + PDS fetch. +- **fix:** Add cache headers to both GET success paths (including the 200+null absent case, which the route comment already notes is the hot expected case): minimum-risk option is the existing pattern `Cache-Control: private, max-age=30`; better is `public, s-maxage=30, stale-while-revalidate=300` since the records are definitionally public (unauthenticated PDS reads) — 30s shared staleness after an org-settings PUT is in line with the accepted xrpc foreign-read tradeoff. Apply the same header to metadata/route.ts GET. Do not cache the error paths. +- **skeptic fix correction:** Use only the `Cache-Control: private, max-age=30` variant (the existing FOREIGN_READ_CACHE_HEADERS pattern) on the GET success and 200+null paths. Do NOT use the proposed `public, s-maxage=30, stale-while-revalidate=300`: it contradicts the documented decision in the xrpc route ("Use private so we don't share between users via a CDN"), and Vercel's edge cache is not invalidated by a PUT, so an admin saving org profile/metadata (edit-profile/page.tsx:159-160 PUTs to the same URLs) could be served their own stale record for up to 30s plus the 300s SWR window, breaking the save-then-refetch flow the xrpc route's SAME_SESSION_NO_STORE_HEADERS comment explicitly guards against. The private variant is safe because the PUT targets the identical URL and browser caches invalidate it on a successful unsafe-method response (RFC 9111 4.4). + +### 27. xrpc GET performs the rate-limit INCR and session lookup as two sequential Upstash round-trips on every request + +- **id:** `xrpc-get-sequential-upstash-roundtrips` · **track:** T7-server +- **severity / effort / regression risk:** low (adjusted from low) / S / low +- **location:** `src/app/api/xrpc/[...method]/route.ts:392` +- **evidence:** `const rateDenied = await enforceRateLimit(GET_LIMITER, clientIp(request)); if (rateDenied) return rateDenied` (lines 392-393) then `const did = await getSessionDid()` (line 398). Both are independent Upstash REST calls (checkHttpRateLimit does `redis.incr`, rate-limit.ts:189; getSessionDid does `redis.get`, session.ts:66). On this 300/min hot fan-out route every request pays two serialized ~10-30ms REST round-trips where one suffices. +- **fix:** Run them concurrently: `const [rateDenied, did] = await Promise.all([enforceRateLimit(GET_LIMITER, clientIp(request)), getSessionDid()]); if (rateDenied) return rateDenied`. The session GET result is simply discarded on the (rare) denied path; no ordering semantics change because the limiter INCRs regardless. + +### 28. Own-repo getBlob responses carry no Cache-Control, so the browser re-downloads the user's own immutable avatar/banner on every mount + +- **id:** `same-session-blob-no-cache-header` · **track:** T7-server +- **severity / effort / regression risk:** low (adjusted from low) / S / low +- **location:** `src/app/api/xrpc/[...method]/route.ts:531` +- **evidence:** The same-session blob branch returns `new NextResponse(Buffer.from(blob), { headers: { "Content-Type": ..., "X-Content-Type-Options": "nosniff", "Content-Security-Policy": ... } })` (lines 531-538) — no Cache-Control, unlike the foreign branch which sets FOREIGN_BLOB_CACHE_HEADERS. Blobs are CID-addressed and immutable (the URL contains the cid), yet the signed-in user's own avatar — rendered in the navbar/workspace on effectively every page — is re-fetched through the OAuth agent (full restore + PDS round-trip, plus full in-memory buffering) each time. +- **fix:** Add `"Cache-Control": "private, max-age=3600, immutable"` to the same-session blob response headers (private because it went through the bound agent; immutable is safe because the cid in the URL changes whenever the blob changes). One-line change; foreign path is untouched. + +### 29. Direct CGS fetches in memberships and register lack an abort timeout, letting a slow group service pin serverless invocations to the platform ceiling + +- **id:** `cgs-fetches-missing-timeout` · **track:** T7-server +- **severity / effort / regression risk:** low (adjusted from low) / S / low +- **location:** `src/app/api/groups/memberships/route.ts:34` +- **evidence:** memberships/route.ts:34-38 `const res = await fetch(url.toString(), { headers: { Authorization: \`Bearer ${token}\` } })` and register/route.ts:102-104 (membership pagination loop) plus register/route.ts:176-186 (the registration call itself) all fetch the group service with no `signal`. Every other upstream in this surface is bounded — `groupServiceFetch` uses `AbortSignal.timeout(15_000)` (proxy-agent.ts:280), the PDS proxies use 10-15s timeouts. A hung CGS keeps these functions (and the register route's whole limit-check loop, which can iterate many pages) alive until Vercel's function timeout, burning duration and holding the client spinner. +- **fix:** Add `signal: AbortSignal.timeout(15_000)` to the three direct CGS fetch calls (memberships:34, register:102, register:176), matching groupServiceFetch. The surrounding try/catch paths already map aborts to the existing 502/503 error responses. + +## cq-duplication + +### 30. Indexer GraphQL POST wrapper hand-rolled at ~22 call sites with drifting error handling + +- **id:** `indexer-post-wrapper-duplicated` · **track:** P2a-indexer +- **severity / effort / regression risk:** medium (adjusted from medium) / M / medium +- **location:** `src/lib/atproto/workspace.ts:135` +- **evidence:** The identical `fetch(INDEXER_PROXY_URL, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ operationName, variables }), signal })` block is copy-pasted at: src/lib/atproto/indexer.ts:249,329,643,681,764,913,1011,1036,1264,1341; src/lib/atproto/workspace.ts:135,285,339,376,441; src/lib/atproto/follower-events.ts:165,478; src/lib/atproto/trusted-evaluators.ts:247; src/components/profile/profile-lists.tsx:1410; src/hooks/use-followers.ts:61; src/hooks/use-received-endorsements.ts:107; src/hooks/use-cert-projects.ts:59. Two sites even re-declare `const INDEXER_PROXY_URL = "/api/indexer"` locally (use-followers.ts:31, use-received-endorsements.ts:70, workspace.ts:3) instead of importing it from indexer.ts:33. The duplication has produced real behavioral drift: workspace.ts:159-160 `const json = (await res.json()) as NetworkActorsGraphQLResponse; const connection = json.data?.appCertifiedActorProfile` has NO res.ok check and NO errors[] check (indexer failure silently renders an empty actor list — the exact fail-soft gotcha documented for this repo), while use-followers.ts:70-79 throws `new Error(\`Indexer query failed: ${res.status}\`)` plus surfaces `json.errors[0].message`, follower-events.ts:181-196 throws a typed FollowerEventsError with parsed extension codes, and trusted-evaluators.ts:255 does `if (!res.ok) break` silently truncating pagination. +- **fix:** Add one helper in src/lib/atproto/indexer.ts: `export async function postIndexer(operationName: string, variables: Record, signal?: AbortSignal, opts?: { failSoft?: boolean }): Promise<{ data: T | null; error: string | null }>` that does the fetch, the res.ok check, JSON parse, and errors[]-array check in one place. Migrate all 22 sites mechanically, preserving each site's current throw/fail-soft semantics via the return shape (sites that throw today convert the returned error to a throw; deliberate fail-soft sites keep ignoring it, now explicitly). Delete the two local INDEXER_PROXY_URL re-declarations. Do NOT change workspace.ts fail-soft behavior in the same commit — flag it separately. +- **skeptic fix correction:** The helper's proposed return shape { data: T | null; error: string | null } is lossy and would break mechanical migration: follower-events.ts:194 needs errors[0].extensions?.code to construct its typed FollowerEventsError; use-followers.ts and trusted-evaluators.ts embed res.status in their throw messages; and indexer.ts:255-262 distinguishes "GraphQL errors present" (warn + return null) from "HTTP !ok with no errors" (throw). The helper should instead return the full context, e.g. { ok: boolean; status: number; data: T | null; errors: Array<{ message: string; extensions?: { code?: string } }> }, letting each site reconstruct its exact current throw/fail-soft/warn semantics. Drop the redundant opts.failSoft flag — the return shape already covers it. Also correct the migration notes: the silent-truncation site to flag separately is workspace.ts:291, not trusted-evaluators.ts, and there are three (not two) local INDEXER_PROXY_URL re-declarations to delete. + +### 31. Project card presentation parsing copy-pasted 6x with two conflicting image-precedence orders + +- **id:** `project-image-precedence-drift` · **track:** P2b-helpers +- **severity / effort / regression risk:** medium (adjusted from medium) / S / low +- **location:** `src/components/home/home.tsx:285` +- **evidence:** Three parallel copy-paste families for rendering an org.hypercerts.collection project: (1) `function asString(v: unknown)` re-declared in home.tsx:382, project-edit-route.tsx:66, project-detail.tsx:91, project-list-row.tsx:120, explore-project-card.tsx:87, profile-projects.tsx:441; (2) title chain `asString(value.title) || asString(value.name) || "Untitled project"` at home.tsx:280-283, home-feed.tsx:1188, project-detail.tsx:270, project-list-row.tsx:38, explore-project-card.tsx:31, profile-projects.tsx:252 and 446, activity-detail.tsx:2243; (3) image precedence that has DIVERGED: home-feed.tsx:1201, activity-detail.tsx:2250 and project-list-row.tsx:46 use `const rawImage = v.avatar ?? v.image ?? v.banner` (project-list-row comments "Mirrors the home feed's CollectionPreview precedence so the same project reads identically across surfaces"), but home.tsx:285-286 uses `(project.value as Record).banner ?? project.value.image` and explore-project-card.tsx:36 uses `(value as Record).banner ?? value.image` — no avatar, banner first. The /explore page itself shows different thumbnails for the same project depending on gallery view (explore-project-card, banner-first) vs list view (project-list-row, avatar-first): a user-visible inconsistency the duplication created. +- **fix:** Add `export function projectPresentation(value: CollectionValue): { title: string; rawImage: BlobLike | null }` (plus a shared `asString`) to src/lib/atproto/collection.ts encoding the avatar ?? image ?? banner order, and replace all 6+ derivations. This intentionally changes home.tsx and explore-project-card.tsx thumbnails for projects that have an avatar — that is the confirmed-defect fix, matching the documented canonical precedence. +- **skeptic fix correction:** Do not flatten all sites to a single avatar ?? image ?? banner order. The banner-first sites are mostly wide/hero slots where banner (LargeImage) is the intended asset: explore-project-card's full-width image-wrap, profile-projects ProjectBox ("Rendered much larger ... primary unit on the page", profile-projects.tsx:258-261), and project-detail's hero (project-detail.tsx:355-366, which also has object-URL preview logic a shared helper must not swallow). Forcing avatar-first there would render a SmallImage avatar in a banner slot — a visual regression. Instead add a slot-aware helper to src/lib/atproto/collection.ts, e.g. projectImage(value, slot: "thumb" | "banner") where thumb = avatar ?? image ?? banner and banner = banner ?? image, plus a shared asString and projectTitle(value, fallback) (home-feed.tsx:1183-1191 needs the fallback parameter for its "Untitled list"/"Untitled portfolio" variants). The one true drift fix is home.tsx:285 (a compact thumb using banner-first) -> switch it to the thumb order; that is behavior-neutral today since its fetchProjects data carries banner only. Whether explore gallery cards should also show avatar is a design decision to flag to the user, not a confirmed defect; note the divergence is also partly upstream — the Projects/UserProjects persisted queries never select avatar, so client-side reordering alone cannot make indexer-fed surfaces match the feed. + +### 32. ~640 lines of orphaned CSS across 23 BEM blocks with zero TSX/TS references + +- **id:** `dead-css-blocks` · **track:** T11-css +- **severity / effort / regression risk:** medium (adjusted from medium) / M / low +- **location:** `src/app/styles/feed.css:1852` +- **evidence:** Verified via per-block grep over all *.ts/*.tsx in src/ (0 hits each, including dynamic-template prefixes — cf. `cert-detail--tab-${activeTab}` at activity-detail.tsx:1326 which DOES hit and is excluded): feed.css — `.received-endorsement-card` (1852), `.endorser-chip` (1872), `.endorsement-preview` (1568), `.location-card`/`.location-list` (721/712), `.search-page` (5), `.page-tabs-bar` (941), `.page-section-heading` (984), `.feed-unfiltered-banner` (367); components.css — `.connected-apps` (1324), `.app-detail` (1492), `.handle-edit` (508), `.my-data` (1509), `.org-sync` (1561), `.more-link-row` (26), `.feedback-bottom-sheet`, `.email-section--form`; layout.css — `.personal-info` (975), `.profile-fallback-note` (995), `.signin-mark` (47); profile.css `.profile-panel` (302); landing.css `.app-card` (1777); pages.css `.org-create` (250); profile-inline-edit.css `.profile-edit-banner`; profile-edit.css:87 `.pe .profile-card__banner` whose comment says it tightens "BannerUpload's profile-card__banner reuse" but banner-upload.tsx:113-184 only emits `profile-banner-upload__*` classes. Rule-block line count for the 23 blocks: feed.css 240, components.css 213, layout.css 137, pages.css 19, landing.css 17, others 18 — ~644 lines shipped in globally-imported stylesheets on every page. +- **fix:** Delete the listed rule blocks (and the stale profile-edit.css comment). Mechanical, one commit per stylesheet if preferred. Re-run the block-name grep for each deletion as the guard, then a dark-mode + 800/1100/1300 breakpoint visual spot-check of feed, settings, and landing pages. +- **skeptic fix correction:** "Delete the listed rule blocks" is unsafe verbatim in two spots. (1) feed.css:941-942 is the compound `.page-tabs-bar, .endorsements-tabs-bar` and feed.css:953-954, 971-972, 976-977, 1561-1562 pair `.page-tabs-bar__new` with `.endorsements-new-btn`; both partners are LIVE (src/app/endorsements/page.tsx:354 and :372). For those five rules, remove only the dead `.page-tabs-bar`/`.page-tabs-bar__new` selectors from the selector lists — deleting the whole blocks would unstyle the endorsements tab bar and its New button. Standalone `.page-tabs-bar__*` rules (e.g. underline/active-tab rules) can be deleted whole. (2) `.app-card` (landing.css:1777) sits in the "SHARED, NON-LANDING — Do not remove with landing iterations" section whose comment at landing.css:1450 stale-claims `.app-card` backs /about, /terms, /privacy, /dsa — only `.app-page` is used there; update that comment when deleting, and prune the dead `.app-card__label` selector from the compound list at tokens.css:423 (selector only — the block is shared with live `.dash-card__title` etc.). Also note regressionRisk is "low" only for the current tree: unmerged branches/worktrees (e.g. fix/mobile-bottom-nav, feat/public-endorsements-groups) still render `page-tabs-bar` in their pre-ad7adf1 copies of src/app/groups/page.tsx and would resurrect the class without CSS if ever merged. + +### 33. Three hook files (223 lines) have zero production importers; two keep test suites alive + +- **id:** `dead-hooks-three-files` · **track:** T9-lint-dead +- **severity / effort / regression risk:** medium (adjusted from medium) / S / low +- **location:** `src/hooks/use-display-profile.ts:21` +- **evidence:** `export function useDisplayProfile(` (use-display-profile.ts:21, 55 lines) — grep for `useDisplayProfile|use-display-profile` across the whole repo matches only the definition file; zero importers, not even tests. `export function usePendingAwardsCount(` (use-pending-awards-count.ts:38, 71 lines) — only non-self reference is a stale doc comment at use-received-endorsements.ts:331 ("Used by `usePendingAwardsCount` on the nav rail") plus its own test file use-pending-awards-count-logged-out.test.tsx. `export function useUserActivities(` (use-user-activities.ts:11, 97 lines) — only importer is use-user-activities.test.tsx. Both tests exercise code no product path can reach. +- **fix:** Delete src/hooks/use-display-profile.ts, src/hooks/use-pending-awards-count.ts, src/hooks/use-user-activities.ts plus src/hooks/__tests__/use-pending-awards-count-logged-out.test.tsx and use-user-activities.test.tsx; fix the stale comment at use-received-endorsements.ts:331. fetchDisplayProfile/invalidateDisplayProfile stay (used by use-hyperboard.ts:135 and hyperboard.ts:499). If the pending-awards nav chip is planned to return, note it in the commit message — git history preserves the implementation. + +### 34. displayName/handle/href derivation from useAuthorInfo copy-pasted at ~20 sites + +- **id:** `identity-fallback-chain-duplicated` · **track:** P2b-helpers +- **severity / effort / regression risk:** medium / S / low +- **location:** `src/components/endorsements/endorsement-row.tsx:45` +- **evidence:** The 3-4 line block `const displayName = info?.displayName || info?.handle || ; const handle = info?.handle && info.handle !== info.did ? info.handle : null; const initials = getInitials(...); const href = profileUrl(info?.handle || did)` recurs at: endorsement-row.tsx:45-48, endorsement-lists.tsx:600-604, profile-endorsements.tsx:784, 908, 1107, 1226, 1366, profile-followers.tsx:477, profile-lists.tsx:559, person-card.tsx:46, activity-detail.tsx:2043, activity-author.tsx:46-48, account-list-row.tsx:41-48, explore-user-card.tsx:39-46, new-endorsement-panel.tsx:268, contributor-identity-card.tsx:38-41, sync-social-graph-section.tsx:617, step-graph.tsx:311, app/endorsements/page.tsx:114. The fallback tail varies arbitrarily (`did`, `"Anonymous"`, `"Unknown"`, `truncateDid(did)`) — e.g. activity-detail.tsx:2043 shows "Anonymous" while endorsement-lists.tsx:600 shows the raw DID for the same unresolved state. +- **fix:** Add `export function deriveIdentity(info: AuthorInfo | null, did: string): { displayName: string; handle: string | null; initials: string; profileHref: string }` next to useAuthorInfo (or in lib/utils/initials.ts), using truncateDid(did) as the single canonical fallback, and replace the ~20 call sites mechanically. Purely derivational — no markup/CSS changes, so desktop baseline is untouched except unresolved-DID text becoming consistently truncated (an improvement over raw 40-char DIDs). +- **skeptic fix correction:** The helper is right, but "replace ~20 call sites mechanically" is not literally true for 4 of them: account-list-row.tsx and explore-user-card.tsx put `actor.displayName`/`actor.avatarUrl` ahead of the resolved info, new-endorsement-panel.tsx inserts a caller-supplied `handle` prop into the chain, and contributor-identity-card.tsx keys off `identity` (handle-or-DID) via useContributorInfo. Give deriveIdentity an options bag (e.g. `{ preferredName?, preferredAvatarUrl?, fallbackLabel? }`) or let those 4 sites compose the helper output rather than blind-replacing, otherwise the explore cards lose their record-level display-name priority. Also note the unification is a deliberate visible change at the "Anonymous"/"Unknown" sites (unresolved state only); no tests pin those strings, but it should be called out in the commit message per the desktop-baseline convention. + +### 35. Three near-identical endorsement subject-row components (avatar + name + handle + note + date + revoke) + +- **id:** `endorsement-subject-row-triplicated` · **track:** T5-profile +- **severity / effort / regression risk:** medium (adjusted from medium) / M / medium +- **location:** `src/components/profile/endorsement-lists.tsx:597` +- **evidence:** Three components render the same row: (1) endorsement-row.tsx:36-93 `EndorsementRow` — useAuthorInfo → Skeleton-or-Avatar → `.endorsement-row__name`/`__handle`/`__note` → `