release: dev → prod — 2026-08-10 (dashboard perf campaign + production observability) - #1130
Merged
Conversation
chore: sync prod → dev (re-align release merge history)
…ads (#1101) Memoizes getSession with React.cache so nested layout gates share one Better Auth call per render, narrows the consultant home/appointments Prisma graphs, drops pending-approval nests that were fetched and discarded, and code-splits the onboarding steps, create-org wizard, EarningsTabs (recharts) and SafeUnifiedCalendar. Measured: /form/onboarding first-load JS 392 kB -> 319 kB against a control route that grew 5 kB over the same window. A first pass moved the layout gates and requireApiAuth onto the Better Auth cookie cache; that is reverted. customSession re-runs its Prisma enrichment on every getSession call regardless, so the cache skipped roughly one query in four, while lib/auth-guard.ts has no ban check of its own and relied on the forced read to catch bans, DPDP erasure and revoked sessions. requireAuth now force-reads and checks banned explicitly too. Also fixes correctness regressions the perf work introduced: Financial Summary counts derived from a truncated array, an approvals badge that contradicted NeedsYou on the same screen, Home ranking that could empty Today/Upcoming, an INNER JOIN that blanked Trending, and a StreamProvider element-type swap that remounted the whole dashboard subtree. Pinned by __tests__/dashboards/consultant-home-read-shape.test.ts, each test verified by reintroducing its bug. Deriving UserRoleEnum from Prisma removes five 'as never' casts in onboarding. No raw SQL remains in lib/data or app.
Splits the dashboard prefetch and NeedsYou roll-up into their own async components behind Suspense, and renders the page header as real text outside them. The server now emits a shell at ~500ms instead of producing nothing until all ~14 queries resolve. Measured on the deploy preview (same URL/account/route, warm): shell HTML moved from blocked-behind-data to ~500ms. FCP did NOT move (6240ms -> 6008ms), and that is expected: the dashboard layout is a client component that wraps children in StreamProvider with ssr:false, so the server renders a spinner instead of the subtree and <h1> never reaches the HTML. Suspense can only stream markup the server is willing to produce. Restoring that SSR is the follow-up; this change is its prerequisite, because restoring SSR while the page still blocks on every query would just move the 4.9s wait into the HTML. Also fixes a real bug: the header title came from the session, which belongs to the VIEWER, so an ADMIN/STAFF inspecting another consultant's dashboard was greeted by their own name. Inspectors now get a neutral title. The auth gate stays outside the boundaries, prefetch and dehydrate stay in one component, and the page stays first in the file so the textual ownership-order assertion in personal-dashboard-ssr-ownership.test.ts still holds.
StreamProvider wrapped {children} in a next/dynamic component with ssr:false,
which skips server rendering for the component AND its children. The connector
now renders null as a sibling and publishes state to a module store read via
useSyncExternalStore, so children sit in a fixed position.
That also removes the once-per-session remount documented in StreamProviderImpl
(children -> <StreamVideo> -> <Chat> as the sockets settled), which tore down
the whole dashboard — the storm behind 'I pressed Join ten times' (#248).
The SDK contexts move to the surfaces that consume them: <Chat> to the three
Messages tabs, <StreamVideo> to /meetings. A completeness sweep caught a latent
bug this scoping would otherwise have introduced: useEventActions:115 powers
Join on consultee and org appointments, routes that no longer mount
<StreamVideo>, so every Join there would have failed. It now reads the singleton
at click time via getGlobalVideoClient().
Does NOT improve FCP, and is not merged as a perf claim. Measured on the
preview, the server HTML still contains no <h1> and no layout nav: the client
layout returns PersonalDashboardShellSkeleton instead of children while its
queries load, which is always during SSR because consultant-data is never
server-prefetched. That is the dominant blocker and the next PR.
Verified on the preview: consultant Messages (channels render, unread badge
live, no console errors), consultant Appointments, and /meetings mounting
without a context crash.
#1105) Both personal dashboard layouts are client components that return PersonalDashboardShellSkeleton instead of children while their queries load — always true during SSR, because nothing prefetched them. So no dashboard markup reached the HTML at all. Two pieces were needed. app/dashboard/layout.tsx (already a server component) now seeds ['user-details', sessionUserId] and dehydrates it. That alone did nothing, because both layouts derive identity from useSession(), a client hook still pending during SSR — so their key was ['user-details', undefined] and the seed was unreachable. The layout therefore also publishes the server-resolved id through a small client context, and both layouts fall back to it: getEffectiveUserId(session) ?? serverUserId. MEASURED on the preview, streaming the document: - layout nav markup ('>Event Planner<'): ABSENT before, present at 1775ms after - <h1> markup: absent before, present after - server HTML: 59KB -> 107KB FCP is NOT improved (5432ms and 6456ms vs a 6004-6240ms baseline — within noise), and this is not merged as an FCP win. The page's own sequential awaits (requirePersonalProfileAccess then getSession(true)) still gate its <h1> until ~5.1s, which is the next thing to chase and is local to one file. Seeds user-details rather than the profile-specific query deliberately: it is the viewer's OWN row, keyed on the session user, never the profile id from the URL which belongs to someone else when an ADMIN/STAFF inspects. The consultant read carries public/private access levels, #726 plan-visibility filtering and PII narrowing — re-deriving that for a prefetch is how #946 happened. Consultee is included because it shares the gate and the client-derived id. The query moves to lib/data/user-details.ts so GET /api/user/[id] and the prefetch cannot drift.
useOrgScope read role and memberships from useSession(), pending during SSR, so it always resolved 'personal' there while the consultee home page computed 'all' for org members, admins and staff. The scope is part of the query key (['consultee-events', id, scopeKey]), so those users got a guaranteed hydration miss and then a second key flip once the session landed. Generalises the server-facts context added in #1105 to carry role and firstOrgId, and useOrgScope falls back to them. resolveDefaultScopeKey puts the default rule in one place so hook and pages cannot drift, and it honours ?orgScope= — which the consultee home page's local mirror did not, so every scope toggle prefetched the wrong scope and discarded the result. VERIFIED BY A/B on the previews, same OWNER account with both a consultee profile and an org membership, same page: #1107 preview (old hook): client refetches /events at 5819ms — the wasted hydration miss #1108 preview (fixed): no events fetch at all; hydration holds Scoped deliberately: I checked which clients actually key a query off the hook (consultee home, consultee payments). The consultant appointments page has a comment claiming its client uses useOrgScope — it does not — so its hard-coded 'personal' prefetch is left alone rather than 'fixed' on a stale comment, which would have inverted a mismatch that does not exist.
…andle (#1109) lucide-react, recharts and date-fns are already in Next's built-in optimizePackageImports list, so naming them was inert config that read as if it were doing something. @novu/react and @novu/nextjs ARE imported (components/notifications/NotificationInbox.tsx) and were absent. transpilePackages: ['date-fns'] left in place with a note rather than removed: date-fns is 4.1.0 and ships an exports map so the entry looks stale, but its comment claims it fixes a module-format conflict and that is unverified in both directions — confirming needs build:analyze, which is too RAM-heavy to run here. ISR deliberately excluded. explore/programs has 2 server-side auth references, so caching that route's HTML would serve one viewer's authenticated content to everyone. That needs a per-page audit across all 11 force-dynamic pages, which is its own PR rather than a switch bolted onto a config tidy.
Replaces the hardcoded switch(step)/switch(role) machine and its separate STEP_LABELS map with Record<OnboardingRole, OnboardingStep[]>, each entry carrying its own label and render. OnboardingRole is derived from the existing z.discriminatedUnion, so a new role branch fails to compile until it declares steps. Two flags absorb the leftover special cases: (was string-matching the label 'Availability') and , which is how the ORG_WORKSPACE escape hatch disappears — its second entry declares it paints its own chrome, so the shell steps aside exactly as the old early return did. All next/dynamic calls keep their inline options literals and loading fallbacks. Deletes the dead FormProvider: the only useFormContext in the repo is inside components/ui/form.tsx, which nothing in the onboarding tree imports. Fixes ORG_WORKSPACE role stranding by reverting on cancel rather than deferring the commit — deferring would have meant changing the shared wizard's contract or weakening the POST /api/organizations gate. The guard is the where clause of a single updateMany, so there is no read-then-write race: it matches only ORG_WORKSPACE + onboardingCompleted not true + no memberships. A real org owner always holds the owner Membership and can never be reverted. 221 suites / 2518 tests (up 4, covering the reset action's unauthenticated, cross-user, guard-clause and no-op cases).
…1112) * fix: harden event-id validation and restore reschedule heatmap paint Mock/hand-crafted appointment PKs were legal Prisma String ids but failed allocate Zod checks; keep UUID/CUID validation, fail closed on timings SSR, and restore consultant reschedule Being-moved paint, legend, and org Reschedule. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: unblock CI for org reschedule and repair script Pass consulteeId from SSR instead of next-auth/react (project uses better-auth), and stop logging DATABASE_URL so Sonar S8689 clears. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
) * fix(dashboard): stop document scroll past role shells into white space Lock every role shell to the PersonalDashboardShell overflow contract and stop the consultant Profile summary column from inflating a blank grid band. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(dashboard): lock document scroll so the window cannot pass the shell Shell overflow-hidden alone was not enough: the root body stays min-h-screen/overflow-visible, so the window could still scroll past the grey dashboard chrome into empty white space. Mount a dashboard-only scroll lock on html/body. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(dashboard): content-only org-workspace loading + stronger shell contract tests Part of #1113. Stop nesting a full CollapsibleSidebarSkeleton inside OrgWorkspaceShell, and tighten the overflow contract guardrails from review. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
…#1107) The org tree gates on orgDetailsQueryKey(orgId), which nothing prefetched, so useOrgRole failed closed to LEARNER during SSR and ~15 components returned null. layout.tsx becomes a server wrapper that seeds and dehydrates that query; the client body moves verbatim to OrgDashboardShell.tsx, carrying #1113's overflow chain. The seed is only written when the read returns non-null — caching null would also suppress the client's own fetch. orgDetailsInclude moves to a zero-import leaf module: sharing it from org-details-server.ts dragged auth-helpers into GET /api/organizations/[orgId] and 500'd it on every request. Also fixes org avatars, which have never displayed: the logo lives on the OrgBrandingProfile satellite (#768 lockdown #6) and this read never joined it. Verified on the preview against the dev branch deploy, same account and page: route 200 3/3, operator nav present as markup (>Members< 3, >Analytics< 2, >Billing< 2 vs 0/0/0), org home server-renders the operator view.
Every public page carried force-dynamic, which makes Next emit private/no-cache/no-store — uncacheable at every CDN by construction, so each visitor paid a Netlify ap-southeast-1 to Supabase ap-south-1 round trip behind a cold function boot. This gives the five viewer-independent public routes real ISR, and adds the guard that makes build-time prerendering safe to accept. The first shape of this PR did the opposite of what its comments claimed. The CI route table showed the three FIXED routes flipping to prerendered-at-build while the two [param] routes stayed dynamic with an EMPTY Revalidate column — their revalidate export was inert, and those were the two heaviest reads. Three fixes: 1. generateStaticParams returning an empty array on both [param] routes. Next renders a dynamic segment dynamically unless generateStaticParams exists, and silently ignores the interval when it does so. The empty array is the documented "all paths at runtime" shape: nothing prerendered at build (consultant cardinality is unbounded), each param rendered on first request and then cached. dynamicParams stays at its default of true. 2. The three fixed routes stay prerendered. A prerendered page is served as static HTML from the CDN with no function invocation at all, which sidesteps the cold start on the pages where LCP matters most. 3. A build-phase guard, because prerendering means those reads run inside next build — the #932 failure mode. fail-open degrades a transient pooler timeout to an empty section, which is right at request time and dangerous at build time, where the empty result gets baked into static HTML and served to everyone for a whole revalidate window. Both helpers now rethrow when NEXT_PHASE is the production build phase, turning a silent bad bake into a loud, retryable build failure, with a bounded build-only retry in front since #932 was a cold connect. Request-time behaviour is unchanged. Next resolves a route's revalidate to the MINIMUM of the segment value and every data cache entry read during the render, so the 120s unstable_cache windows were silently capping the pages regardless of what the segment asked for. Those windows were raised to match the declared intervals; the reads involved feed only these two pages. Freshness now comes from on-demand purges at the write sites rather than short intervals — verification flips, profile edits and deletes, org isPublic/status moves and slug renames (which purge the old path too), and review writes. Verified on CI and on the deploy preview: the three fixed routes prerender at 1h/5m/5m, both param routes moved from dynamic to SSG with nothing prerendered at build, and both now return a public, cacheable response with a climbing age against a still-dynamic control returning private/no-store. Part of #932 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…lves (#1116) The navbar rendered a skeleton for the entire /api/auth/get-session round trip. Measured warm on prod, that is about 350 ms logged out and about 1,000 ms logged in, and the session fetch already dispatches within 40 ms of FCP, so the delay is the round trip itself and there is no earlier moment to dispatch from. lib/auth-broadcast.ts already kept a familiarise.auth_authed flag in localStorage for cross-tab sync, written by AuthSyncProvider on every resolved session. It now also carries a display-only identity (name and image, nothing else), and hooks/useRememberedAuth.ts reads both in a layout effect - after commit, before paint - so the first painted frame shows "Sign in" or the real avatar. The server render and the first client render both still return the skeleton, so hydration is byte-identical; nothing is read from storage during render. Privacy: writeAuthedFlag(false) always removes the cached identity, the clear happens before the replacement write and again in the catch so a quota failure cannot strand the previous account's name next to a true flag, and signOut is wrapped in lib/auth-client.ts, which every sign-out path in the app imports. The flag is presentation only - /dashboard stays middleware-gated and server-guarded, and nothing branches on it except which chrome to paint. Measured on the deploy preview against the dev branch deploy, warm, same page, logged-out returning visitor: the FCP-to-affordance gap goes from 426/531/543 ms to 85/89/140 ms. In one preview run the navbar was correct at 700 ms while the session fetch did not return until 1,385 ms, so it is now 685 ms ahead of the answer instead of 16 ms behind it. With the remembered identity seeded, the authed shape painted at FCP+91 ms using the cached avatar, then reconciled to "Sign in" at 1,020 ms when the real session resolved null, clearing both the flag and the identity. CLS stayed at 0.0017, identical to dev, and no hydration warning appeared. A second change parallelising two customSession reads was written, measured over 20 order-balanced interleaved rounds, found to move nothing (1,142 ms median on dev against 1,176 ms on the preview) and reverted. PG_POOL_MAX=1 on every deployed context gives each function instance a single pg client, so concurrent Prisma queries serialise and Promise.all over them cannot win. Written up in #1117. Part of #636
…ill (#1118) * docs(skill): share the Next.js-on-Netlify rendering and caching skill Encodes what the 2026-07/08 dashboard-performance campaign measured rather than what the framework docs promise, so the next person does not re-derive it. Was untracked on one machine only. Part of #14 * perf: decompose the cold TTFB and record the connection budgets The 23s figure was misattributed to cold boot. Across 42 cold on-demand renders the distribution is bimodal with no sample between 6.8s and 30.8s: a new instance rendering sequentially costs ~1.9s, twelve concurrent renders against warm instances cost 3.3-5.9s, and the tail appears only where the two intersect — a new instance opening its first pooler connection while other new instances do the same. Slow-response count equals new-instance count per batch. One HTTP request is one function invocation (6 requests, 6 Duration lines, constant ~0.37s CDN delta), so nothing chains or retries at the platform level. The log has no Init Duration field; an empty-message info line marks an invocation start, and the Better Auth provider-warning pair marks a module load. Logs the resolved connect/query budgets once per instance boot, because the function log emits only Duration and Memory Usage and the deployed budget was otherwise unreadable — which is how a 3s connect timeout firing at t+29.8s went unnoticed. Part of #14 Part of #1119 Part of #1120 * perf: validate the decomposition claims against primary sources Softens two claims that adversarial validation weakened, and moves the boot diagnostic to a log an investigator can actually read. Init Duration: Netlify staff describe a full Lambda-style report line and the docs call it the cold-start discriminator, so "does not exist" was too strong. This function emits a reduced line, and a Netlify forum report describes the identical absence for the same handler. The dashboard UI was not checked. Platform ceiling: 10s default / 26s max on paid plans is documented with no supported extension, yet 26.4-31.9s invocations were logged. The observation stands; the mechanism does not, so it is now stated as unresolved. The boot line moves from Sentry.logger to console.warn: Sentry logs ship to Sentry, not to the function log that was missing the number, and Sentry is not readable with the access available here — an unverifiable diagnostic is not one. Also measured rather than assumed: the call costs 8.2us once per instance boot, 4e-4 % of a 1,900ms cold render. Part of #14 Part of #1120 * perf: drop the inert instrumentation, keep the finding that killed it The [Prisma:INIT] line was deployed and measured, not assumed. It produced zero log occurrences across a deploy where two module loads were independently confirmed by the Better Auth markers in the same window: compiler.removeConsole strips server-side console.* in production, not only client bundles. The Sentry.logger form survives that but ships to Sentry rather than the function log that was missing the number, and Sentry is unreadable with the access here. An unverifiable diagnostic is worse than none, so lib/prisma.ts goes back to byte-identical with dev and the failure is recorded as #1122 instead — ~993 console.* call sites under lib/ and app/api are silently inert in production. Adds the second-deploy replication (8 concurrent, all-new instances: 5 at 9.20-11.40s, 3 at 34.16-36.76s), which shows the fast mode is not a constant while the 30s+ slow mode always is. Part of #14 Part of #1122 * docs(skill): separate measured facts from inferred mechanism Addresses the review. The most substantive point was a real internal contradiction: the headline asserted the first DB connect as the cause while a later paragraph admitted the per-instance attribution was inference. The headline now states what the data indicates and labels each part — the ~30s plateau and bimodality measured, the connection read from prisma:error in the same invocation, the per-new-instance trigger inferred from counting. Also scoped or softened five claims that reached further than the evidence: one-request-one-invocation is now about anonymous document requests on this route rather than a platform rule; the build route table is authoritative for build-time classification, not on-demand ISR; the build-retry benefit is a hypothesis, not "very likely"; the static-page affordance says client-only, since a server dynamic island cannot work without PPR on our version; and the query-key rule is restated as both sides deriving identical values. Adds the shared-cache warning to the CDN override section — those headers on a session-dependent response serve one user's page to another, and it fails silently. Requires re-verification on any adapter/runtime change, not just majors. Part of #14
… 30s tail (#1123) #1119 is fixed. #1120 is answered, and the answer is that it is not a database problem and cannot be fixed from application code, so it is not closed here. #1119 — a degraded fail-open render was returned as HTTP 200 on an ISR route, so Netlify wrote it into the durable cache and replayed it to everyone for the rest of the revalidate window. Reproduced on the dev branch deploy: 10 of 68 concurrent cold renders of /explore/experts/[consultantId] came back as the 66 KB degraded shell with Cache-Status "Netlify Durable"; fwd=uri-miss; stored, and re-fetching them five minutes later served the same broken page in 0.30-0.64 s with a hit and age 318-350. Six of them were produced in under 3 s, so the broken page is also the fast one, which is why nobody noticed. Fail-open is now opt-in per call site via perRequest and defaults to rethrowing. The only opt-in is /explore/programs, which is force-dynamic. The ISR pages drop the wrappers entirely, [consultantId] no longer renders ConsultantUnavailable (deleted), and [orgSlug]'s generateMetadata no longer degrades to a generic title. Both halves of the framework behaviour were verified on a temporary ISR route: a thrown render returns 500 with "Netlify Durable"; fwd=bypass and stores nothing, and a thrown revalidation leaves the last good copy serving with age climbing past the window. Measured after: 0 degraded in 64 samples, including 15 slow renders, against 10 in 68 before. What rethrowing costs is stated rather than glossed. On / it replaces the whole landing page with app/error.tsx, acceptable only because / is prerendered at build so a cached copy almost always exists. On the two [param] routes generateStaticParams returns [], so a parameter rendering for the first time has no copy to fall back on and its visitor gets the error boundary. Still the better trade than poisoning the cache for everyone. #1120 — the 3 s connect budget is wired correctly. @prisma/adapter-pg passes the config straight to pg.Pool, and a black-holed connect fails locally in 3,003 ms with the exact error strings from production. It is not enforced because both pg timers are plain setTimeouts and the event loop is blocked. A diagnostic route that ran 400 ms of pure idle awaiting before touching the database at all measured 23.9-24.8 s stalls on brand-new instances at invocation 1, with the database query after the stall completing in under a second. No timeout value can bound this. The ~30 s tail is unchanged by this PR and is filed as #1124 with the numbers. A request-time retry was written, measured and REVERTED inside this PR. Its benefit rested on two diagnostic samples and could not be separated from the rethrow in the page-level A/B. Its cost is structural: retrying per read doubles the query count on pages issuing four of them, PG_POOL_MAX=1 serialises those, and a saturated pooler then pushes a failing render toward the Netlify function ceiling, where the response is a bare platform 500 with no error boundary and no Cache-Status at all. Reinstating it needs a per-render budget and fault-injected evidence. Review round, done by an independent adversarial pass because CodeRabbit's account-level rate limit never cleared: the consultant error boundary claimed the transient never reaches it and captured to Sentry unconditionally while onRequestError already reported the same error, so it now guards on the digest; sentry.shared.config.ts justified leaving pooler timeouts unfiltered on a premise this change invalidates; three page headers and one client rationale credited a fail-open guard those files no longer import; EMPTY_EXPERTS_METADATA lost its last importer. The tests pinned the helper default rather than the call-site wiring that actually regressed, so isr-routes-never-fail-open.test.ts now reads the route sources and asserts no route exporting revalidate opts into degrading. It is mutation-checked. Verified with scoped tsc, eslint on every touched file, and the full Jest suite (230 suites, 2582 tests). The build route table is unchanged. No schema change, no db push, no test data written to the shared database. Closes #1119 Not Closes for #1120: that half was diagnosed, not fixed, and the one mitigation that shipped for it has since been reverted. Refs #1120, #1124, #1125. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…teed-empty reads (#1126) * fix: restore production server diagnostics and stop paying for guaranteed-empty reads `compiler.removeConsole: true` was framed as a bundle-size measure, which reads as client-only. It is not — SWC applies the transform to the server layer too, and Next offers no client-only scoping, so every one of ~1,110 server-side diagnostics was silently absent from the deployed function. Proven rather than inferred: a console.warn at Prisma client construction produced ZERO log lines on preview 1118 across two independently confirmed module loads, while third-party output in the same window survived (node_modules is not compiled by SWC). It cost real time twice — the #1124 investigation had to return its findings through the HTTP response body. Narrowed to `{ exclude: ["error", "warn"] }`, the only documented knob. `log` stays stripped, which is the whole of the bundle saving the original comment was after. Sentry's consoleLoggingIntegration cannot substitute: it patches globalThis.console at runtime and this deletes the call expressions at compile time. `[Prisma:INIT]` now logs the resolved connect/query/pool budgets once per instance boot, so deployed values are readable from `netlify logs` instead of argued about, and it doubles as the A/B probe for this change. Separately, the consultant Home issued nine `WHERE id IN (NULL)` queries for any consultant with nothing upcoming. Verified first-hand in Sentry (FAMILIARISE_WEB-G): the operation measured 4,624ms in production. The empty selects cost 63-68ms each once warm — the bulk is pg-pool.connect and the first query queued behind it, the PG_POOL_MAX=1 serialisation of #1117 — so statement count is the lever and not issuing them is the cheapest way to pull it. Guarded per-query, not by early return: the other reads in that Promise.all are consultant-scoped and must still run. Three sibling unguarded `in:` sites on the now-ISR explore routes get the same guard, and HomePageClient's `staleTime: 0` override is gone — it marked the SSR-prefetched entry stale on mount and refetched the identical payload, doubling everything behind it. Finally, the #1119 guard could only ever see degradation that went through lib/data/fail-open.ts, because its detector is `/perRequest/.test(src)`. The two hand-rolled swallows in /explore/programs now route through fallbackOnTransientDbError (its first call sites) so a mapper or schema regression surfaces instead of rendering an empty page, the announcements degrade branch gets the `no-store` its sibling documented, and the guard gains a bare-catch assertion: a catch that binds nothing cannot report what it swallowed. Both new guards are mutation-checked, and the bare-catch detector is anchored on fixtures rather than real files so the #1125 sweep cannot quietly make it vacuous. Closes #1122 Closes #1121 Part of #1125 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(test): a comment must not switch off the bare-catch guard CodeRabbit review. `\s` does not match comments, so `catch /* transient only */ {` slipped past the detector — and a comment is precisely what someone writes while explaining why a swallow is fine, which makes it the likeliest way through. A guard a comment can disable is worse than no guard, because it still reads as coverage. Both comment forms added to the fixtures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor: name the empty-`in` guard instead of repeating it SonarCloud flagged two new CRITICAL cognitive-complexity smells (typescript:S3776) introduced by this PR: consultant-dashboard's getConsultantDashboard went 15 -> 16, and explore-programs' getCuratedPrograms went to 19, because each inline ternary counts against the enclosing function and the two in explore-programs sit three levels deep. The quality gate still passed, but both were mine. Extracting `readByIds` fixes the metric by moving the branch out of the callers, and it is better code besides: the reason an empty `in` is not free — Prisma renders it as `IN (NULL)`, round-trips, and pays one follow-up SELECT per nested relation — is now stated once where the guard lives, rather than as a comment repeated at four call sites with three different levels of detail. This reverses the "use the inline ternary, do not add a helper for four sites" call made earlier in this PR. Four sites plus a metric regression is where that trade flips. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…ity (#1129) * fix: report the silent swallows that lose money, access and availability #1125 asked for a sweep of catch-and-placeholder blocks. A six-batch census found 229 of them, so every HIGH-severity call was checked against source before anything was changed. Roughly a third survived: two claimed a swallow was unreported in code that already calls Sentry.captureException, and several described degradations that are correct and already return a failure to their caller. Only the survivors are fixed here; the full reviewed inventory is posted on the issue so the tail stays actionable without becoming an unreviewable diff. A theme runs through the ones that were real. Each carries a comment describing a case its own code already handles structurally, which is what made the catch look reasonable while it fired exclusively on genuine faults: * The invoice-refund wallet credit sat under "swallow if no wallet flow applies" — but `fundingSource === "WALLET"` already gates that. Twenty lines above, a note calls this credit "the guaranteed bookkeeping" for a refund. It was not guaranteed, and the console.warn saying so was deleted from production by #1122. Deliberately still not rethrown: the dispatcher stamps error=true on a throw and the sweeper only re-drives error=null, so rethrowing would roll back the refund booking AND retire the event permanently. Durability is #1128. * /r/[code] swallowed applyReferralCode "if it fails (already referred, self-referral, etc.)" — that function RETURNS NULL for every one of those and only throws on a real fault. The page then redirected to ?ref_applied=true regardless, asserting a reward the user might not have. Now conditional. * customSession's SSO enforcement recheck failed OPEN with "non-fatal — don't break session". Unlike the two fail-opens reasoned about directly above it, that was never a decision. * removeCollaborator awaited removeUserFromEventChannel without reading the { success } it returns instead of throwing, so a failed chat-access revocation was indistinguishable from a successful one and the outer catch never fired. formatSlotsForApi is the one behaviour change. It feeds a PUT body, so degrading there does not mean render less, it means SAVE less: a per-slot catch dropped the offending slot and an outer catch returned [] for the whole schedule, after which SettingsTab refetched and showed the wiped availability as "what was actually saved". Its three catches are gone and formatCustomSlot throws rather than returning a null the caller filtered away. An availability save is all-or-nothing; the caller already has a destructive toast to fail into, and now reports to Sentry. Five tests pin the contract, mutation-checked against a reinstated outer catch. Rate-limit reporting deliberately omits `identifier`: callers key on whatever identifies the caller and one passes a raw client IP, which would put PII in Sentry against this project's sendDefaultPii: false (#1127). It buys nothing — when Redis is down every limiter fails. Closes #1125 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: throw on a failed WEEKLY slot conversion, and throttle Redis outage reports CodeRabbit review on #1129. Two of the three were legit. formatWeeklySlot still returned [] when convertTimezoneToUtc failed, and the caller's flatMap absorbed it — the identical drop-a-slot-from-the-save-payload hazard this PR fixed in formatCustomSlot, missed in its twin. It is the worse half, because WEEKLY is the default schedule type. convertTimezoneToUtc returns "" for both an unparseable time and a caught conversion error, so the empty string could never be distinguished from a slot legitimately omitted. Two tests added that reach the conversion-failure path for real, via an unusable timezone — which is how a VALID slot gets there, since formatSlotsForApi filters on isValidTimeRange before formatting and the existing tests only reached the day-key and date-key guards. Mutation-checked: restoring `return []` fails the weekly test and nothing else. The Redis-outage report is now throttled to one per instance per minute. A Redis failure is total rather than per-caller, so the second capture of an outage carries nothing the first did not, while the volume burns quota and buries unrelated alerts. Per-instance rather than global on purpose: there is no shared state to coordinate through when the shared state is what is down. The third comment asked to confirm the failed-wallet-credit path has durable recovery. It does not, deliberately — the dispatcher retires an event on throw, so rethrowing would lose the whole refund booking. That trade and the fact that a later webhook hits the REFUNDED short-circuit are both written up in #1128. No code change. Part of #1125 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
✅ Deploy Preview for familiarise ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
17 commits,
c38afa06→75de05b9. prod holds no commits that dev does not, so this is a clean non-divergent release.Pre-flight
db pushrequiredALLOW_SHARED_DB_REPAIR/CONFIRM_INVALID_EVENT_ID_REPAIRappear in the diff but are guardrails insidescripts/appointments/repair-invalid-event-ids.ts, not app runtimeWhat ships
The dashboard performance campaign. Nothing under
/dashboardwas server-rendered — four stacked blockers, each hiding the next (#1105, #1103, #1102, #1101). Plus real ISR on the public routes (#1110), org dashboard SSR (#1107), and the optimistic navbar auth paint (#1116), whose logged-out FCP→auth-affordance went 426/531/543ms → 85/89/140ms.Production observability (#1126).
compiler.removeConsole: truewas deleting ~1,110 server-side diagnostics from the deployed function, because SWC applies the transform to the server layer and Next offers no client-only scoping. Now{ exclude: ["error", "warn"] }. Verified on the preview by A/B:[Prisma:INIT]appears 2 times against 0 on production, with thenode_modulesBetter Auth warnings as the control.Correctness (#1129, #1123, #1112, #1113). Silent swallows that could lose an org's wallet credit, pass SSO enforcement on a thrown lookup, disable every rate limiter without telemetry, or save a consultant's availability minus the slots that failed to format.
Two things to watch after deploy
console.error/console.warnsites become live in Netlify logs for the first time. That is the point of fix: restore production server diagnostics and stop paying for guaranteed-empty reads #1126 — but nothing constrains what those calls pass, and Sentry is deliberatelysendDefaultPii: false. Tracked in Audit what console.error/warn payloads serialise, now that they reach Netlify function logs #1127; worth a look at the logs shortly after release./,/explore/expertsor/explore/enterprise/organisationslook stale or empty, that is the first place to check.Known-unchanged and still open: #1124 (the ~24s cold-instance event-loop stall) is not fixed by this release and will still produce occasional 30s+ cold responses. #1123 mitigates the blast radius by serving from the ISR cache rather than invoking the function.
🤖 Generated with Claude Code