Admin social load-time instrumentation#111
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds end-to-end admin observability: a new ChangesAdmin Route Timing, Progress Status, and Load Sample Telemetry
Sequence Diagram(s)sequenceDiagram
participant Browser as Admin Browser
participant LandingRoute as /api/admin/social/landing GET
participant Repository as getSocialLandingPayloadResult
participant Backend as Backend Social Progress API
participant Attach as attachAdminRouteTiming
participant Storage as localStorage
Browser->>LandingRoute: GET /api/admin/social/landing
LandingRoute->>LandingRoute: create timingCollector, record routeStartedAt
LandingRoute->>Repository: getSocialLandingPayloadResult(ctx, timingCollector)
Repository->>Backend: fetch social progress rollup
Backend-->>Repository: payload { timing, stale, cache_status }
Repository->>Repository: normalize timing → timingCollector.recordBackendMs()
Repository-->>LandingRoute: { payload: { social_progress_status }, cacheable }
LandingRoute->>Attach: attachAdminRouteTiming(response, { cacheStatus, backendMs, databaseMs })
Attach->>Attach: append Server-Timing header
Attach->>Attach: set x-trr-admin-route-ms
Attach-->>Browser: NextResponse with timing headers + social_progress_status
Browser->>Browser: recordAdminLoadSample(duration, x-trr-cache)
Browser->>Storage: write sample to localStorage
Browser->>Browser: render SocialProgressStatusBadge(social_progress_status)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR adds end-to-end load-time instrumentation for the admin social landing + social profile surfaces, combining server-side Server-Timing / x-trr-* timing headers with client-side persistence of local load samples, and adds UI signaling when progress data is stale.
Changes:
- Introduces
attachAdminRouteTiming()to standardize Server-Timing and timing headers on admin API routes and TRR social proxy routes. - Persists local (per-browser) admin load samples for social landing/profile navigation + snapshot API calls.
- Extends social landing payload with progress freshness metadata and renders a stale/live progress badge; defers some secondary reads post-first-paint.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/web/src/lib/server/trr-api/social-profile-route-factory.ts | Wraps admin social proxy JSON responses with standardized timing instrumentation. |
| apps/web/src/lib/server/admin/social-landing-repository.ts | Adds progress status + timing fields for landing progress rollups and exposes timing collection hooks. |
| apps/web/src/lib/server/admin/admin-route-timing.ts | New helper to attach Server-Timing + x-trr-admin-* headers to admin responses. |
| apps/web/src/lib/admin/social-landing.ts | Adds SocialLandingProgressStatus type to represent progress freshness/staleness. |
| apps/web/src/lib/admin/api-references/generated/inventory.ts | Regenerated admin API inventory to match scanner output. |
| apps/web/src/lib/admin/admin-load-samples.ts | New client utility to record/read admin load samples in localStorage. |
| apps/web/src/components/admin/SocialAccountProfilePage.tsx | Records load samples, adjusts secondary-read gating, and defers non-essential reads until after first paint. |
| apps/web/src/app/api/admin/trr-api/social/profiles/[platform]/[handle]/snapshot/route.ts | Attaches admin route timing to the social profile snapshot API route. |
| apps/web/src/app/api/admin/trr-api/social/profiles/[platform]/[handle]/catalog/posts/route.ts | Attaches admin route timing to the catalog posts API route. |
| apps/web/src/app/api/admin/trr-api/social/ingest/queue-status/route.ts | Attaches admin route timing to the ingest queue status API route. |
| apps/web/src/app/api/admin/social/landing/route.ts | Adds timing collection + attaches admin route timing headers to landing GET/POST responses. |
| apps/web/src/app/admin/social/page.tsx | Records landing load samples and renders a progress stale/live badge on the admin social landing page. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const routeStartedAt = performance.now(); | ||
| let backendMs = 0; | ||
| let databaseMs = 0; | ||
| const timingCollector = { | ||
| recordBackendMs(durationMs: number) { | ||
| backendMs += durationMs; | ||
| }, | ||
| recordDbMs(durationMs: number) { | ||
| databaseMs += durationMs; | ||
| }, | ||
| }; |
| const routeStartedAt = performance.now(); | ||
| let backendMs = 0; | ||
| let databaseMs = 0; | ||
| const timingCollector = { | ||
| recordBackendMs(durationMs: number) { | ||
| backendMs += durationMs; | ||
| }, | ||
| recordDbMs(durationMs: number) { | ||
| databaseMs += durationMs; | ||
| }, | ||
| }; |
| platform !== "instagram" || | ||
| summaryLoading || | ||
| !summary | ||
| !summary || | ||
| !secondaryReadsGateOpen || | ||
| secondaryReadBudgetSlot < 2 |
| platform, | ||
| pauseSecondaryReadsForPressure, | ||
| secondaryReadBudgetSlot, | ||
| secondaryReadsGateOpen, | ||
| summary, |
| return attachAdminRouteTiming(NextResponse.json(data), { | ||
| routeFamily: "admin-social-profile", | ||
| routeName: "GET /api/admin/trr-api/social/ingest/queue-status", | ||
| cacheStatus: "miss", | ||
| startedAt: routeStartedAt, |
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/lib/server/trr-api/social-profile-route-factory.ts (1)
253-258:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse the handler start time for cached profile responses.
The cache helpers start timing after admin auth and param resolution, while the non-cache/error paths use
routeStartedAtfrom Line 329. Cache-hit/stale timings will underreport the same route. PassrouteStartedAtinto both cache helpers instead of creating a new timestamp inside them.Suggested shape
const respondWithRouteResponseCache = async ( input: SocialProfileProxyRouteInput, config: SocialProfileProxyRouteConfig, cache: RouteResponseCacheConfig, + startedAt: number, ): Promise<NextResponse> => { - const startedAt = performance.now(); const respondWithAdminSnapshotCache = async ( input: SocialProfileProxyRouteInput, config: SocialProfileProxyRouteConfig, cache: AdminSnapshotCacheConfig, + startedAt: number, ): Promise<NextResponse> => { - const startedAt = performance.now(); if (config.cache?.kind === "route-response") { - return await respondWithRouteResponseCache(input, config, config.cache); + return await respondWithRouteResponseCache(input, config, config.cache, routeStartedAt); } if (config.cache?.kind === "admin-snapshot") { - return await respondWithAdminSnapshotCache(input, config, config.cache); + return await respondWithAdminSnapshotCache(input, config, config.cache, routeStartedAt); }Also applies to: 304-309, 342-347
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/server/trr-api/social-profile-route-factory.ts` around lines 253 - 258, The cache helpers respondWithRouteResponseCache and other cache-related functions are creating their own timestamp via performance.now() instead of using the routeStartedAt value established earlier in the request lifecycle, causing inconsistent timing measurements across cached and non-cached response paths. Remove the startedAt timestamp creation from inside these cache helper functions and instead add routeStartedAt as a parameter to both respondWithRouteResponseCache and the other cache helpers mentioned in the related lines. Update all references to startedAt within these functions to use the passed-in routeStartedAt parameter instead, ensuring consistent timing measurement across all response paths.apps/web/src/app/api/admin/social/landing/route.ts (1)
528-538:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winWrap POST early returns with route timing too.
These validation/not-found branches still return raw
NextResponse.json, so 400/404 POST responses miss the newServer-Timingandx-trr-admin-route-msheaders. Use a small local helper to keep these branches consistent with the success/error paths.Suggested helper shape
export async function POST(request: NextRequest) { const routeStartedAt = performance.now(); let backendMs = 0; let databaseMs = 0; const timingCollector = { recordBackendMs(durationMs: number) { backendMs += durationMs; }, recordDbMs(durationMs: number) { databaseMs += durationMs; }, }; + const timedJson = ( + body: Record<string, unknown>, + init: ResponseInit, + cacheStatus = "error", + ) => + attachAdminRouteTiming(NextResponse.json(body, init), { + routeFamily: "admin-social-landing", + routeName: "POST /api/admin/social/landing", + cacheStatus, + startedAt: routeStartedAt, + backendMs, + databaseMs, + }); try {Then replace early returns like:
- return NextResponse.json({ error: "target_type is required" }, { status: 400 }); + return timedJson({ error: "target_type is required" }, { status: 400 });Also applies to: 542-543, 567-568, 633-634
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/api/admin/social/landing/route.ts` around lines 528 - 538, The early return statements that validate parameters (checking isLandingTargetType, targetId, isLandingPlatform, platform, and rawValue) are returning NextResponse.json() directly without the Server-Timing and x-trr-admin-route-ms headers that are applied elsewhere in the route. Create a small local helper function in the POST handler that wraps the error response creation and includes the necessary timing headers, then replace all early validation return statements (including the ones at 542-543, 567-568, and 633-634) to use this helper function instead of calling NextResponse.json() directly to ensure consistency across all response paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/app/admin/social/page.tsx`:
- Line 1045: The condition in the coerceLandingPayload function call that sets
progressStale only checks for the exact string "stale" using cacheStatus ===
"stale", but this misses other stale cache-status variants like
"uncacheable-stale". Update the condition to handle all stale cache-status
variants, such as by checking if cacheStatus includes the substring "stale" or
by explicitly checking for all known stale variant values, so that the
progressStale flag correctly identifies all cases where the data is stale.
In `@apps/web/src/components/admin/SocialAccountProfilePage.tsx`:
- Around line 4165-4168: The effect has overly broad dependencies on the full
`summary` object and `catalogActionMessage`, causing unnecessary re-runs for
unrelated updates, and it lacks a check for `secondaryReadsPressurePause` to
honor secondary-read pressure. Refactor the dependency array to depend only on
the specific properties of `summary` that are actually used in the effect logic
rather than the entire object, add a check for `!secondaryReadsPressurePause` to
the condition alongside the existing checks for `secondaryReadsGateOpen` and
`secondaryReadBudgetSlot`, and apply these same changes to the duplicate effect
pattern mentioned in the also-applies-to range.
- Around line 4324-4332: Multiple tab data fetch effects in
SocialAccountProfilePage.tsx are initiating requests while the summary is still
null and the first snapshot request is in flight, causing unnecessary
competition for resources. Apply the same `!hasSummary` guard condition
(matching the pattern shown in the provided diff with `summaryUninitialized`) to
the catalog fetch effect, hashtag fetch effect, hashtag timeline fetch effect,
and collaborators/tag fetch effects at the locations noted (around lines
4375-4384, 4448-4455, 4512-4521, 4620-4628). This ensures these secondary
requests are only initiated after the summary snapshot has been successfully
fetched, keeping the critical first-paint data fetch as the priority.
- Around line 8512-8520: The condition checking activeCommentsRunDateWindow
currently uses AND logic (!dateStart && !dateEnd) which only refreshes progress
when both boundaries are missing. Change this to OR logic (!dateStart ||
!dateEnd) so that the fetch of activeCommentsRunProgressNow and subsequent
readCommentsRunDateWindow call occurs whenever either dateStart or dateEnd is
missing, ensuring both boundaries are properly backfilled from the fresh
progress response.
- Around line 3325-3336: The useEffect hook that calls recordAdminLoadSample can
execute before the summary state is properly reset when platform or handle
changes, causing it to record navigation samples for the new route using the old
route's summary data. Add an additional condition to verify that the loaded
summary data's platform and handle match the current route parameters (the
platform and handle variables in the dependency array) before proceeding with
the recordAdminLoadSample call. This ensures that we only record a sample when
the summary state corresponds to the current route, not stale data from the
previous route.
In `@apps/web/src/lib/admin/admin-load-samples.ts`:
- Around line 91-93: The regex pattern in the conditional check is missing
support for admin-prefixed routes. Update the pattern to match both
`/social/{platform}/{handle}` and `/admin/social/{platform}/{handle}` paths by
making the `/admin` portion optional in the regex. This will ensure that when
callers provide admin-prefixed social profile paths, the function returns
"admin-social-profile" instead of null.
In `@apps/web/src/lib/server/admin/social-landing-repository.ts`:
- Around line 890-900: When the backend social progress rollup fails in the
catch block, the code returns a status with source set to "fallback" even though
it never actually executes the local SQL fallback (since adminContext returns
early later). This misleads the landing badge into displaying "local fallback"
when the backend is unavailable and no fallback has been attempted. Either move
the local SQL fallback logic before this early return so it executes on backend
failure, or change the status source to a distinct value like "unavailable" or
"degraded" to accurately reflect the degraded state without claiming a fallback
was used. Apply the same fix to the related error handling blocks at lines
922-923 and 2794-2799 for consistency.
---
Outside diff comments:
In `@apps/web/src/app/api/admin/social/landing/route.ts`:
- Around line 528-538: The early return statements that validate parameters
(checking isLandingTargetType, targetId, isLandingPlatform, platform, and
rawValue) are returning NextResponse.json() directly without the Server-Timing
and x-trr-admin-route-ms headers that are applied elsewhere in the route. Create
a small local helper function in the POST handler that wraps the error response
creation and includes the necessary timing headers, then replace all early
validation return statements (including the ones at 542-543, 567-568, and
633-634) to use this helper function instead of calling NextResponse.json()
directly to ensure consistency across all response paths.
In `@apps/web/src/lib/server/trr-api/social-profile-route-factory.ts`:
- Around line 253-258: The cache helpers respondWithRouteResponseCache and other
cache-related functions are creating their own timestamp via performance.now()
instead of using the routeStartedAt value established earlier in the request
lifecycle, causing inconsistent timing measurements across cached and non-cached
response paths. Remove the startedAt timestamp creation from inside these cache
helper functions and instead add routeStartedAt as a parameter to both
respondWithRouteResponseCache and the other cache helpers mentioned in the
related lines. Update all references to startedAt within these functions to use
the passed-in routeStartedAt parameter instead, ensuring consistent timing
measurement across all response paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fd470d66-1339-4b35-a369-d090777e332c
⛔ Files ignored due to path filters (1)
apps/web/src/lib/admin/api-references/generated/inventory.tsis excluded by!**/generated/**
📒 Files selected for processing (11)
apps/web/src/app/admin/social/page.tsxapps/web/src/app/api/admin/social/landing/route.tsapps/web/src/app/api/admin/trr-api/social/ingest/queue-status/route.tsapps/web/src/app/api/admin/trr-api/social/profiles/[platform]/[handle]/catalog/posts/route.tsapps/web/src/app/api/admin/trr-api/social/profiles/[platform]/[handle]/snapshot/route.tsapps/web/src/components/admin/SocialAccountProfilePage.tsxapps/web/src/lib/admin/admin-load-samples.tsapps/web/src/lib/admin/social-landing.tsapps/web/src/lib/server/admin/admin-route-timing.tsapps/web/src/lib/server/admin/social-landing-repository.tsapps/web/src/lib/server/trr-api/social-profile-route-factory.ts
Codex Exhaustive Code ReviewFindings
Validation I reviewed only the specified diff range and surrounding code. I attempted the focused Vitest run:
It could not run in this sandbox because |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/components/admin/SocialAccountProfilePage.tsx (1)
3320-3346:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse route-matched summary state before unlocking profile reads.
On a
platform/handlechange, these effects still see the previous route’ssummary,hasSummary, gate, and budget values until the reset effect’s state updates commit. That can start new-route tab/secondary requests before the new snapshot first-paint request finishes. Derive a route-scoped summary readiness flag and use it for these guards, including the review-queue path.Proposed direction
const shouldShowCatalogRunProgressCard = selectedTab !== "comments"; const hasSummary = summary !== null; + const summaryMatchesCurrentRoute = + summary?.platform === platform && + normalizeComparable(String(summary?.account_handle || "").replace(/^@+/, "")) === + normalizeComparable(handle.replace(/^@+/, "")); + const hasCurrentSummary = hasSummary && summaryMatchesCurrentRoute; useEffect(() => { profileLoadStartedAtRef.current = nowMs(); profileNavigationSampleRecordedRef.current = false; }, [handle, platform]); useEffect(() => { if (profileNavigationSampleRecordedRef.current) return; - if (!hasSummary && !summaryUninitialized) return; - if ( - hasSummary && - (summary?.platform !== platform || - String(summary?.account_handle || "") - .trim() - .replace(/^@+/, "") - .toLowerCase() !== handle.trim().replace(/^@+/, "").toLowerCase()) - ) { - return; - } + if (!hasCurrentSummary) return; profileNavigationSampleRecordedRef.current = true;const selectedTabPrimaryReadPending = - hasSummary && + hasCurrentSummary && !summaryUninitialized &&const shouldDeferSecondaryCatalogReads = - !hasSummary || summaryLoading || selectedTabPrimaryReadActive || selectedTabPrimaryReadPending || shouldPauseSecondaryReads; + !hasCurrentSummary || summaryLoading || selectedTabPrimaryReadActive || selectedTabPrimaryReadPending || shouldPauseSecondaryReads;Apply the same
!hasCurrentSummaryreplacement to the live-total, posts, catalog posts, hashtags, timeline, collaborators, review-queue, and cookie-health guards in this range.Also applies to: 3426-3446, 4168-4178, 4344-4351, 4397-4405, 4468-4478, 4537-4547, 4588-4598, 4647-4654, 7507-7526
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/admin/SocialAccountProfilePage.tsx` around lines 3320 - 3346, The useEffect blocks in SocialAccountProfilePage are using stale summary data from previous routes because hasSummary is checked before verifying the summary matches the current platform and handle. Create a new derived state variable (e.g., hasCurrentSummary) that checks both that summary is not null AND that the summary's platform and account_handle match the current route's platform and handle parameters. Then replace all instances of hasSummary with this new hasCurrentSummary flag in the guard conditions throughout all the useEffect dependency arrays and conditional checks at lines 3320-3346 and the other ranges (3426-3446, 4168-4178, 4344-4351, 4397-4405, 4468-4478, 4537-4547, 4588-4598, 4647-4654, 7507-7526) to prevent mismatched route reads.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/components/admin/SocialAccountProfilePage.tsx`:
- Around line 4241-4250: Replace the three-part condition checking
isBackendSaturationError, isBackendPressureCode, and hasBackendSaturationText
with a single call to isBackendPressureError(requestError) in the if statement
before pauseSecondaryReadsForPressure is called. This will use the shared
pressure detector which has broader coverage than the current narrower checks,
preventing timeout and upstream-status-only failures from skipping the pressure
pause logic.
---
Outside diff comments:
In `@apps/web/src/components/admin/SocialAccountProfilePage.tsx`:
- Around line 3320-3346: The useEffect blocks in SocialAccountProfilePage are
using stale summary data from previous routes because hasSummary is checked
before verifying the summary matches the current platform and handle. Create a
new derived state variable (e.g., hasCurrentSummary) that checks both that
summary is not null AND that the summary's platform and account_handle match the
current route's platform and handle parameters. Then replace all instances of
hasSummary with this new hasCurrentSummary flag in the guard conditions
throughout all the useEffect dependency arrays and conditional checks at lines
3320-3346 and the other ranges (3426-3446, 4168-4178, 4344-4351, 4397-4405,
4468-4478, 4537-4547, 4588-4598, 4647-4654, 7507-7526) to prevent mismatched
route reads.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9e4ee500-0177-48cf-a637-60163a890f4f
📒 Files selected for processing (7)
apps/web/src/app/admin/social/page.tsxapps/web/src/app/api/admin/social/landing/route.tsapps/web/src/components/admin/SocialAccountProfilePage.tsxapps/web/src/lib/admin/admin-load-samples.tsapps/web/src/lib/admin/social-landing.tsapps/web/src/lib/server/admin/social-landing-repository.tsapps/web/src/lib/server/trr-api/social-profile-route-factory.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/web/src/lib/admin/social-landing.ts
- apps/web/src/app/admin/social/page.tsx
- apps/web/src/lib/admin/admin-load-samples.ts
- apps/web/src/app/api/admin/social/landing/route.ts
- apps/web/src/lib/server/admin/social-landing-repository.ts
- apps/web/src/lib/server/trr-api/social-profile-route-factory.ts
Summary
Notes
apps/web/src/lib/admin/api-references/generated/inventory.tswas regenerated becausepnpm -C apps/web run validate:quickrequires the generated admin API inventory to match the current scanner output.Validation
pnpm -C apps/web exec vitest run -c vitest.config.mts tests/social-landing-repository.test.ts tests/social-account-profile-snapshot-route.test.ts tests/social-account-profile-page.runtime.test.tsx tests/social-account-summary-route.test.tspnpm -C apps/web run validate:quickpnpm -C apps/web run typecheckgit diff --checkSummary by CodeRabbit
Release Notes
New Features
Improvements
Tests