Skip to content

refactor(perf): heavy screens + analytics - #1513

Merged
Wellington01 merged 92 commits into
mainfrom
perf/heavy-screens-refactor
Jul 13, 2026
Merged

refactor(perf): heavy screens + analytics#1513
Wellington01 merged 92 commits into
mainfrom
perf/heavy-screens-refactor

Conversation

@Wellington01

Copy link
Copy Markdown
Member

No description provided.

Wellington01 and others added 30 commits July 3, 2026 17:17
… session fetch

The client fetch wrappers (authorizedFetch, axiosAuthorized) called getJWTToken()
— a network GET /api/auth/session — before every API request to read the access
token. On a single page load that fanned out to ~6 concurrent session fetches
(each 0.5–5.8s in dev due to server-side queuing), and added a serial round-trip
in front of every call.

The /api/proxy route already resolves the token from the httpOnly session cookie
server-side (it did this for EventSource). Make it the single authority: always
inject the server-derived bearer and ignore any client-sent Authorization. The
client wrappers then stop fetching/attaching the token entirely — server-side
calls (which bypass the proxy) still attach it from auth().

Removes the /api/auth/session fan-out per load and the serial token round-trip on
every client API call. Also keeps a client-dictated token from reaching upstream.
Proxy handler spec unchanged (32/32).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ings hydration

Runtime-profiled (Playwright) + static audit against next/vercel best-practice rubrics.

- Collapse the app-shell 2-hop waterfall (auth+teams -> teamId -> layout data):
  speculatively fetch team-scoped data using the team cookie, in parallel with
  auth+teams; fall back to a refetch only when the cookie is stale.
- Parallelize independent sequential awaits in 5 server pages (organization/general,
  sso, cockpit/review-suggestions, plugins @modal, kody-rules @modal).
- Drop the redundant 30s PR executions poll where SSE already invalidates the query
  (opt-out via poll:false; PR detail view keeps polling). Also clean up the dead
  React Query v4 refetchPage option (v5 ignored it).
- Stop the per-page /api/auth/session fetch: the PR and dry-run SSE routed through
  /api/proxy already have the Bearer injected server-side, so the manual token
  header was redundant. Verified: session fetch gone, SSE still 200.
- Fix a real hydration mismatch on settings pages: SelectedTeamProvider initialized
  useState() from a client-only cookie read (undefined on server, real value on
  client), diverging the tree and shifting Radix useId()s. Thread the server-read
  cookie as initialTeamId so server and first client render match. Verified: 4
  hydration errors -> 0 on /settings/git.

React Compiler is on, so no manual memoization was added.
next.config.js optimizePackageImports left out (co-mingled with the uncommitted
Next 16 config).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- apps/cli/pnpm-workspace.yaml isolates it as its own pnpm root
- packageManager pnpm@11.9.0; internal script yarn refs → pnpm
- allowBuilds: bun=false (transitive, CLI only detects bun via user-agent)
- CI (cli-ci, cli-release): pnpm/action-setup + cache: pnpm
- validated: skills:validate + build + 744 tests pass

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Next 15.5 → 16.2.10; middleware.ts → proxy.ts (Node.js runtime; Auth.js
  `export const proxy`); edge-runtime warnings (axios/jose) gone
- React Compiler enabled (top-level reactCompiler; babel plugin — Rust port
  is canary-only); turbopackFileSystemCacheForDev; dropped unsupported eslint
  config block; build-analyze uses --webpack
- 21 default.tsx added to parallel-route slots (required by 16)
- cookies-next imports → /client; removed dead deps
  (react-query-next-experimental, react-sortablejs, sortablejs,
  @tiptap/extensions)
- package manager yarn → pnpm (isolated pnpm-workspace.yaml; Dockerfiles,
  vercel.json, scripts); validated: build (Turbopack+standalone) + SSO e2e
  Quick (prod image, cloud+self-hosted)
- docs: NEXT16-MIGRATION.md, FRONTEND-DATA-PATTERNS.md

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tials

createMCPCustomPlugin logged the full payload (apiKey/basicPassword/clientSecret)
to the browser console; the dry-run SSE handler logged every event + message
content. Removed; kept legitimate console.error/warn.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…dant MCP token fetch

- (setup)/layout.tsx: SelectedTeamProvider rendered SupportDropdown/SetupUserNav
  (Radix useId) without the server-read team cookie — same hydration-mismatch
  class fixed in the app layout. Pass initialTeamId from the server cookie.
- mcp-manager/utils.ts: client calls route through /api/proxy/mcp, which injects
  the Bearer server-side, so the manual getJWTToken() header was redundant and
  only cost an /api/auth/session round-trip. Dropped it (server-side direct calls
  keep their token); spec updated to assert the proxy owns the Bearer.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…on on mount

- filter.hook: the prNumber/developer effects depend on searchParams and
  call router.replace unconditionally; once the ?models= seed writes to the
  URL on mount they re-fire in an infinite loop (blank page, CPU pegged). Guard
  navigate() to skip semantically no-op navigations.
- chart: migrate from victory to recharts (BarChart + stackId + the shared
  RechartsTooltip), matching the cockpit charts. Same colours; the y-scale cap
  now uses domain + allowDataOverflow so the tooltip shows real values.
- page.client: render the content skeleton (extracted TokenUsageContentSkeleton)
  instead of null before mount, so SSR/pre-hydration shows the placeholder
  layout instead of an empty body that pops in the content.
Verified live: loop gone (CPU 121%->0.2%), chart renders, skeleton in SSR HTML.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The flow-metrics tab was dead code: hidden in the tab bar (`flow-metrics:
false`) and its data layer stubbed to an empty array (`fetchChartData`
just `setChartsData([])`). Its 7 charts were the only remaining consumers
of the heavy `victory` chart lib.

Delete the whole dead chain — the 7 chart components, the
`flow-metrics-tabs` client, the `@flowMetrics` parallel-route slot (feature
+ app-router), plus the layout/constants wiring — instead of porting it.
Keep `recharts-shared.tsx` (used by the live token-usage chart) and widen
its tooltip props to `Partial<TooltipContentProps>` so recharts' injected
props type-check.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`victory` no longer has any consumer (the dead flow-metrics charts were
just deleted) — drop it from the manifest and lockfile; only recharts'
internal `victory-vendor` remains. Also correct the
`@tiptap/e^3.27.2n-code-block` dependency name mangled by a botched
migration script (it blocked `pnpm install`) back to
`@tiptap/extension-code-block`, and add the matching release-age excludes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Kodus Review is the cockpit's default landing tab, so its charts gate
first paint — yet unlike the productivity-tab slots they loaded recharts
eagerly into the initial route bundle. Wrap the four recharts charts
(ReviewOperationalOutcomes, WeeklyImplementation, RateBySeverity,
FeedbackSection) in `next/dynamic({ ssr: false })` via a client barrel,
streaming a skeleton until they hydrate — same pattern the productivity
slots already use. RateByCategory (no recharts) stays eager.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…pnpm-store

Spread the flat-native `next/core-web-vitals` config directly instead of
through FlatCompat (which chokes on eslint-plugin-react's circular refs);
switch `jsx` to `react-jsx` for the automatic runtime; and extend the
`.pnpm-store` gitignore to nested workspace copies leaked via the dev
container bind-mount.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The token-usage route's chart compile OOM'd the web dev container at 2G;
raise its limit to 4G. Add the `/perf-debug` skill capturing the
front→back performance debugging loop (Playwright + API + DB) used
throughout this work.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Zero import sites across the web app — a dead dependency like victory.
Drop it from the manifest and lockfile.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move the 5 list filters (repo, title, PR number, suggestions, author
policy) from local useState into the URL via nuqs — same pattern the
Issues page uses — so a filtered view is shareable / deep-linkable and
survives reload. Text inputs write shallowly with history:replace (no
back-button spam); the query stays debounced. Surface the applied filters
as removable chips with a "Clear all", instead of only a count hidden in
the popover.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the centered "Loading suggestions…" spinner with a skeleton that
mirrors the review layout's shell (max-w-[1600px] + file-tree / diff /
sidebar grid), so the page paints its structure immediately instead of a
blank-ish spinner while suggestions load.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The enriched-PR dashboard query paginates in a while loop, re-fetching
executions whenever the author-policy filter drops a whole batch. It did so
with `skip: initialSkip + accumulatedExecutions` — an OFFSET that grows every
iteration, so under aggressive filtering a single request walked (and
discarded) thousands of rows, spiking DB time (issue #1432).

Add an optional keyset `cursor` (createdAt, uuid) to
findPullRequestExecutionsByOrganizationAndTeam: when present it replaces
`.offset(skip)` with a range predicate matching the existing
`ORDER BY createdAt DESC, uuid ASC`, so each page is an indexed range scan.
The use-case keeps the page offset only for the first batch and continues via
the cursor thereafter — same rows (deterministic total ordering), no
over-scan. The page-based API/response contract is unchanged; the count is
still taken once on the cursor-less first batch.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the centered spinner on the list with a table-shaped skeleton
(consistent with the PR detail skeleton), and make the empty state aware of
context: when filters are active it says "No pull requests match these
filters" with a Clear filters action, otherwise "No pull requests reviewed
yet". Both wrapped in the same bordered card as the table for visual
continuity.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add a "Review status" filter (success, error, partial_error, skipped,
in_progress, pending) to the PR list. Threads a `status` param through the
enriched-PR query DTO → use-case → service → repository, where it becomes a
`WHERE automation_execution.status = :status` (backed by the existing
team_automation_id+status index). On the client it joins the other nuqs
URL-synced filters — shareable, with its own removable chip — following the
same pattern as suggestions/author-policy.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The productivity TabsContent was force-mounted, so while the Kodus Review
tab (the default) was active the productivity charts rendered into a
display:none container. Their ResponsiveContainers use height="100%" (the
cards are expandable, so a fixed height isn't an option), which measured
0x0 and spammed recharts' "width(0) and height(0)" warning on every render.

Drop `forceMount` from that tab so its charts mount only when it's active —
silences the warnings and avoids rendering the heavy hidden charts on load.
The Kodus Review tab keeps forceMount (its charts have fixed heights, so
they don't warn).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add a "Review date" from/to filter to the PR list. Threads
createdAtFrom/createdAtTo through the enriched-PR query DTO → use-case →
service → repository, where they become
`automation_execution.createdAt >= :from` / `<= :to` (backed by the existing
createdAt index). The client uses two native date inputs in the filter
popover, URL-synced via nuqs (?from/?to) with a removable chip, and makes
the "to" bound inclusive of the whole selected day. Same pattern as the
status filter.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The list rendered every accumulated infinite-scroll row into the DOM, and
each PrListItem is heavy (tooltips, timeline). Virtualize it with
@tanstack/react-virtual so only visible rows mount.

content-visibility is a no-op on <tr> (browsers drop size containment on
table-internal elements), and measured virtualization of the expandable
variable-height rows can't work inside a real <table> — so convert the
table to a CSS-grid layout: a shared grid-template on the header and every
row, rows absolutely positioned by the virtualizer with measureElement
(ResizeObserver) handling both collapsed and expanded heights. Responsive
column hiding was dropped in favor of horizontal scroll on narrow
viewports. Expand, infinite scroll and column alignment verified live.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…points & filters

Add severity + category breakdowns to the delivered-suggestion aggregation,
surfaced on the enriched PR response. New read endpoints for the dashboard:
- GET /pull-requests/executions/summary (daily digest)
- GET /pull-requests/executions/facets (segment counts)
- GET /pull-requests/awaiting (open PRs with no Kody review)

Plus needsAttention (critical/high) and author=me filters on the enriched query.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Rework the list into PR-centric rows (title-dominant, metadata subline,
severity/status signals) over the virtualized grid. Replace the split
filters with a single toolbar: a title/number search toggle, severity &
status dropdowns, an Awaiting-review view, and a slimmed 'More filters'
popover. Adds the awaiting-PRs list and wires the digest/facets/awaiting
services.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The code-review general settings page fired two heavy queries on mount that
are only conditionally needed:
- dry-run sidebar fetched the closed-PR list (~4s) eagerly; now fetched via
  react-query gated on the PR picker being opened.
- analysis-types fetched 100 MCP integrations (~2.5s) just to warn on the
  business_logic toggle; now gated on business_logic being enabled.

Both preserve behavior — the picker still loads on open, the warning still
shows when business_logic is on with no task MCP connected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The cockpit layout awaited validateOrganizationLicense (which can be slow —
billing cold-start) before kicking off the analytics-status + metrics-visibility
fetches, running them in series. Fire all three together and await the license
first only for the tier-gate redirect; analytics latency now hides behind the
license round-trip. Shell TTFB ~2.6s -> ~1.8s locally.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
getIntegrations fetched the available-integrations catalog from every provider
(a slow external round-trip, ~3s warm / ~8s cold) on every call, even though
the catalog is static-ish. Cache it in-memory per (org, page, size, appName)
with a 5-min TTL, evicting expired entries and never caching a degraded result
from a failed provider. Connection state is still read live and merged, so
isConnected/connectionStatus stay accurate on a cache hit.

Measured: /mcp/integrations ~3.6s cold -> 34-375ms on a warm cache; plugins
page load ~4.4s -> ~3.0s; identical response payload.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The suggestions explorer computed its pagination total with a
`COUNT(*) OVER ()` window, which forced the planner to materialize and join
the ENTIRE matching set (~1M rows at prod scale, spilling the sort to disk)
just to return a 20-row page. Replaced it with a top-N page query (uses the
(org, created) index and stops after LIMIT) plus a separate COUNT(*) run in
parallel via Promise.all. Response shape (total/page/pageSize/items) is
unchanged.

Validated against a seeded 500k-PR / 2M-suggestion / 500k-feedback dataset
(EXPLAIN ANALYZE, same 1M-row scan): 5124ms -> ~532ms at the SQL level (~9.6x).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The segment-facet counts ("all" / "needs attention" / "mine") ran a
$unwind of files×suggestions and $group to count DISTINCT PRs with a delivered
suggestion — exploding each PR into (files × suggestions) rows and forcing a
full collection scan (no organizationId index existed). Replaced with a
document-level countDocuments (nested $elemMatch when severities are given, so
delivered+severity match the same suggestion) and added the multikey index
{organizationId, 'files.suggestions.deliveryStatus'}.

Validated: identical counts (1989 / 442 / 1768) and 1504ms -> 5ms at the DB
level (300x); facets endpoint ~1.5s -> ~90ms warm.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
findSuggestionsByRuleId matched only by organizationId and then unwound
files×suggestions across every PR in the org just to surface the few carrying
the rule. Added a document-level pre-$match on brokenKodyRulesIds (files and
prLevel aggregations) so only PRs referencing the rule are unwound, plus the
multikey index {organizationId, 'files.suggestions.brokenKodyRulesIds'}.

Validated: identical result and 2517ms -> 141ms (18x) at the DB level.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Wellington01 and others added 2 commits July 13, 2026 13:42
…safe)

The local sandbox repo-boundary checks decided containment with
`startsWith(repoReal + '/')` and detected absolute paths with
`startsWith('/')`. Both hard-code the POSIX separator, so on Windows
(where `path`/`fs.realpath` yield backslashes) every in-repo path read
as an escape — and in the write path that made the parent-symlink loop
`break` early and skip its symlink validation entirely.

- Add `isPathInside(root, child)` using `path.relative` (separator-agnostic;
  a different drive yields an absolute `relative` result → rejected) and
  route all four realpath boundary checks through it.
- Detect absolute paths with `path.isAbsolute` instead of `startsWith('/')`
  in `validatePath` and both path resolvers.
- Harden the command-arg traversal guard to catch backslash separators
  (`..\\etc`, `C:\\x`) via `isAbsolute` + `/(^|[/\\])\.\.($|[/\\])/`, keeping
  the POSIX `startsWith('/')` check as defense-in-depth.

Tests: new `isPathInside` suite (POSIX + explicit win32/posix predicate) and
command-arg traversal suite (POSIX absolute, `-n ..` after a valueless flag,
backslash `..`, clean arg). Also type the pre-existing realpath mock so the
file typechecks clean. 32 tests pass.

Follow-up to #1469; the parent-swap TOCTOU residual remains tracked in #1532.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a view switcher (DS Tabs) that scopes the dashboard by role: a
contributor lands on "My queue" (author=me, mine-scoped cards), everyone
who manages lands on "My team" (the existing team dashboard). The org
session only knows owner-vs-contributor, so the role seeds the default
and the switcher (URL ?scope) is the source of truth.

The facets endpoint takes an optional scope=mine that narrows the
actionable "Needs attention" count to the caller's own PRs, reusing
countDeliveredPullRequests' authorEmail (no new query).

Also fix the status column: a merged PR now reads as "Merged" (terminal),
no longer stacking the review-status chip next to it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@kody-ai

kody-ai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

Kody Guide: Usage and Configuration
Interacting with Kody
  • Request a Review: Ask Kody to review your PR manually by adding a comment with the `@kody start-review` command at the root of your PR.

  • Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Current Kody Configuration
Review Options

The following review options are enabled or disabled:

Options Enabled
Bug
Performance
Security
Business Logic

Kody Code Review — 6 suggested fixes.
Paste the prompt below to your agent and all review fixed at once!

🛠️ Open Agent Prompt
A code review identified the following issues in this pull request.
Each section describes what was found and includes a reference implementation where available.

Files involved:
- apps/web/src/app/(app)/pull-requests/_components/page.client.tsx:156
- apps/web/src/app/(app)/pull-requests/_components/page.client.tsx:156
- apps/web/src/app/(app)/pull-requests/_components/page.client.tsx:122
- libs/code-review/application/use-cases/dashboard/get-pull-requests-facets.use-case.ts:136
- libs/code-review/application/use-cases/dashboard/get-pull-requests-facets.use-case.ts:136
- libs/sandbox/infrastructure/providers/local-sandbox.service.ts:814

---

### [1/6] apps/web/src/app/(app)/pull-requests/_components/page.client.tsx:156
Issue identified during code review:
In `page.client.tsx`, the mine view pins the author filter to the sentinel `me`, but the backend applies author filtering in memory after fetching automation executions while pagination totals are computed from the unfiltered execution count. This produces incorrect `totalItems`, `totalPages`, and `hasNextPage` values, can load empty pages, and forces repeated database round-trips to collect the user's PRs, so push the `me` filter into the execution repository query so counts and pages are computed against the same filtered result set.

---

### [2/6] apps/web/src/app/(app)/pull-requests/_components/page.client.tsx:156
Issue identified during code review:
In `page.client.tsx`, the mine view pins the author to `me`, but this is filtered post-query after the backend has already fetched and paginated a batch of team-wide executions. Because contributors now default to this view, a contributor in a large team can receive pages containing none or only a few of their PRs and require multiple backend round-trips before the infinite list reaches 30 visible items, so push the `me` filter down to the Postgres/Mongo query so pagination happens after author scoping, or add a dedicated backend endpoint that scopes executions to the current user before applying LIMIT/OFFSET.
Reference implementation (from code review):

// apps/web/src/app/(app)/pull-requests/_components/page.client.tsx:156
// "Minha fila" pins the list to my own PRs. `me` is a backend sentinel the
    // enriched use-case resolves to the logged-in user's git identity, so it
    // overrides any free-text author search while this view is active.
    const effectiveAuthor = isMineView
        ? "me"
        : debouncedAuthor.trim() || undefined;
    // TODO: author='me' is filtered post-query in the enriched use-case;
    // consider pushing this filter to the execution query so pagination is
    // scoped before LIMIT/OFFSET and mine-view pages are not under-filled.

---

### [3/6] apps/web/src/app/(app)/pull-requests/_components/page.client.tsx:122
Issue identified during code review:
Move role-to-permission decisions into `role-policies.ts` instead of doing an inline role comparison in a component, and use a policy/permission helper to determine the default scope. Run `yarn permissions:matrix` to regenerate the docs matrix if any permissions change.

---

### [4/6] libs/code-review/application/use-cases/dashboard/get-pull-requests-facets.use-case.ts:136
Issue identified during code review:
In 'mine' scope, the needsAttention facet in `get-pull-requests-facets.use-case.ts` is scoped to the caller's email without a falsy-email guard, so a missing email causes it to count the whole team's open actionable PRs instead of zero. This makes the 'mine' view disagree with the mine facet, which already returns 0 when email is absent, so return 0 for needsAttention in 'mine' scope when email is undefined.
Reference implementation (from code review):

// libs/code-review/application/use-cases/dashboard/get-pull-requests-facets.use-case.ts:136
const email = this.request.user?.email;
            // In the "mine" scope the actionable card counts only my open PRs
            // with an unaddressed suggestion — same predicate, plus the author.
            // countDeliveredPullRequests ANDs authorEmail with the other flags,
            // so this reuses the existing query (no new scan).
            const needsAttentionAuthor =
                query.scope === 'mine' ? email : undefined;

            const [reviewedKeys, needsAttention, mine, awaitingKeys] =
                await Promise.all([
                    this.automationExecutionService.getDistinctReviewedPullRequestKeys(
                        { organizationAndTeamData, repositoryIds },
                    ),
                    // Needs attention = open PRs that still carry an unaddressed
                    // delivered suggestion (any severity, implementationStatus ≠
                    // implemented). Actionable "someone needs to look at this",
                    // not "ever delivered a crit/high" (which counted merged/
                    // resolved PRs too). Scoped to the caller in the "mine" view.
                    needsAttentionAuthor || query.scope !== 'mine'
                        ? this.pullRequestsService.countDeliveredPullRequests(
                              organizationId,
                              repositoryIds,
                              {
                                  unresolvedOnly: true,
                                  openOnly: true,
                                  authorEmail: needsAttentionAuthor,
                              },
                          )
                        : Promise.resolve(0),
                    email
                        ? this.pullRequestsService.countDeliveredPullRequests(
                              organizationId,
                              repositoryIds,
                              { authorEmail: email },
                          )
                        : Promise.resolve(0),

---

### [5/6] libs/code-review/application/use-cases/dashboard/get-pull-requests-facets.use-case.ts:136
Issue identified during code review:
In `get-pull-requests-facets.use-case.ts`, `needsAttentionAuthor` is set to `request.user?.email` in 'mine' scope without a falsy guard, so a missing email causes `countDeliveredPullRequests` to omit the author filter and count the whole team's open unresolved PRs. The 'Needs my attention' card in the mine view then shows team-wide counts instead of zero, unlike the existing mine facet that already returns 0 when email is missing, so guard the call to return 0 in 'mine' scope when the caller has no email.
Reference implementation (from code review):

// libs/code-review/application/use-cases/dashboard/get-pull-requests-facets.use-case.ts:136
const email = this.request.user?.email;
            // In the "mine" scope the actionable card counts only my open PRs
            // with an unaddressed suggestion — same predicate, plus the author.
            // countDeliveredPullRequests ANDs authorEmail with the other flags,
            // so this reuses the existing query (no new scan).
            const needsAttentionAuthor =
                query.scope === 'mine' && email ? email : undefined;

            const [reviewedKeys, needsAttention, mine, awaitingKeys] =
                await Promise.all([
                    this.automationExecutionService.getDistinctReviewedPullRequestKeys(
                        { organizationAndTeamData, repositoryIds },
                    ),
                    // Needs attention = open PRs that still carry an unaddressed
                    // delivered suggestion (any severity, implementationStatus ≠
                    // implemented). Actionable "someone needs to look at this",
                    // not "ever delivered a crit/high" (which counted merged/
                    // resolved PRs too). Scoped to the caller in the "mine" view.
                    needsAttentionAuthor
                        ? this.pullRequestsService.countDeliveredPullRequests(
                              organizationId,
                              repositoryIds,
                              {
                                  unresolvedOnly: true,
                                  openOnly: true,
                                  authorEmail: needsAttentionAuthor,
                              },
                          )
                        : Promise.resolve(0),
                    email
                        ? this.pullRequestsService.countDeliveredPullRequests(
                              organizationId,
                              repositoryIds,
                              { authorEmail: email },
                          )
                        : Promise.resolve(0),

---

### [6/6] libs/sandbox/infrastructure/providers/local-sandbox.service.ts:814
Issue identified during code review:
The break condition of the `while (true)` loop in `local-sandbox.service.ts` uses strict equality (`parent === current`), which is discouraged because the loop only stops when the comparison is met exactly. This check is also redundant because `parentStrictlyInside` already becomes `false` once the parent reaches the repo root or escapes it, so remove the equality operand and rely on the relative-boundary guard instead.

---

Review each issue in context, use the reference implementations as guidance, and apply fixes that are consistent with the surrounding codebase.

Access your configuration settings here.

Comment thread apps/web/src/app/(app)/pull-requests/_components/page.client.tsx
Comment thread apps/web/src/app/(app)/pull-requests/_components/page.client.tsx
Comment thread apps/web/src/app/(app)/pull-requests/_components/page.client.tsx
Comment thread libs/sandbox/infrastructure/providers/local-sandbox.service.ts

@kody-ai kody-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found critical issues please review the requested changes

The author autocomplete only shows name/username and matching is done
server-side, yet findDistinctAuthorsByRepositoryIds returned each author's
email all the way to the client — letting any user with Read PullRequests
enumerate colleague emails. Remove email from the aggregation, the repo/
service contracts, the use-case suggestion type and the web option type.

Author-search component hardening:
- clear the blur-close timer on unmount and on refocus (was leaking a
  setTimeout that could setState after unmount or close a re-opened list)
- scroll the keyboard-highlighted option into view within the max-h list
- make option React keys unique (username+name+index) to avoid collisions
- type the filter/map callbacks (drop pre-existing implicit-any)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@kody-ai

kody-ai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

Kody Guide: Usage and Configuration
Interacting with Kody
  • Request a Review: Ask Kody to review your PR manually by adding a comment with the `@kody start-review` command at the root of your PR.

  • Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Current Kody Configuration
Review Options

The following review options are enabled or disabled:

Options Enabled
Bug
Performance
Security
Business Logic

Kody Code Review — 1 suggested fix.
Paste the prompt below to your agent and all review fixed at once!

🛠️ Open Agent Prompt
A code review identified the following issues in this pull request.
Each section describes what was found and includes a reference implementation where available.

Files involved:
- apps/web/src/app/(app)/pull-requests/_components/pr-author-search.tsx:95

---

### [1/1] apps/web/src/app/(app)/pull-requests/_components/pr-author-search.tsx:95
Issue identified during code review:
The `onKeyDown` early return guards the `Escape` handler, so pressing Escape while the dropdown is open but empty does nothing, preventing users from keyboard-closing the autocomplete when the query returns no authors. Move the `Escape` branch before the `filtered.length === 0` guard, or remove `filtered.length === 0` from the guard and check it only for ArrowDown/ArrowUp/Enter.
Reference implementation (from code review):

// apps/web/src/app/(app)/pull-requests/_components/pr-author-search.tsx:95
onKeyDown={(event) => {
                    if (!open) return;
                    if (event.key === "Escape") {
                        setOpen(false);
                        return;
                    }
                    if (filtered.length === 0) return;
                    if (event.key === "ArrowDown") {

---

Review each issue in context, use the reference implementations as guidance, and apply fixes that are consistent with the surrounding codebase.

Access your configuration settings here.

…mail

In the "mine" scope the needsAttention facet was scoped to the caller's
email, but a missing email left authorEmail undefined — which drops the
author filter and counts the WHOLE team's open actionable PRs instead of
0. Guard so mine-scope-without-email returns 0 (matching the `mine`
facet); team scope still runs the team-wide count.

Also document (TODO) that author='me' is filtered post-query, so the
mine view's pagination is computed on the unfiltered team-wide batch and
can under-fill pages on large teams — the push-down fix is deferred.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… too

Adds the `&& email` guard on the needsAttentionAuthor assignment (not only
at the call site) so a mine-scope caller with no resolvable email never
falls back to an undefined authorEmail. Behaviorally identical — belt-and-
suspenders — and matches the shape the reviewer expects.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@kody-ai

kody-ai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

Kody Guide: Usage and Configuration
Interacting with Kody
  • Request a Review: Ask Kody to review your PR manually by adding a comment with the `@kody start-review` command at the root of your PR.

  • Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Current Kody Configuration
Review Options

The following review options are enabled or disabled:

Options Enabled
Bug
Performance
Security
Business Logic

Access your configuration settings here.

The onKeyDown early-return bailed on `filtered.length === 0`, so pressing
Escape while the dropdown showed "No authors match" did nothing. Handle
Escape before the empty-list guard so it always closes an open dropdown.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@kody-ai

kody-ai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

Kody Guide: Usage and Configuration
Interacting with Kody
  • Request a Review: Ask Kody to review your PR manually by adding a comment with the `@kody start-review` command at the root of your PR.

  • Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Current Kody Configuration
Review Options

The following review options are enabled or disabled:

Options Enabled
Bug
Performance
Security
Business Logic

Access your configuration settings here.

@Wellington01
Wellington01 merged commit be2c54d into main Jul 13, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant