Merged
Conversation
WalkthroughAdds a team-level Dora metrics surface: GraphQL schema and types, a new resolver and service method to compute per-team aggregates, extensive integration tests, frontend query/types updates, a new sortable Team Overview table component, page integration, and sandbox mock data. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/app/metrics/services/dora-metrics.integration.test.ts`:
- Around line 3850-3900: The test and service incorrectly compute
changeFailureRate by counting incidents (which can exceed deployments) resulting
in >100% (see test using getDoraTeamOverview and asserting changeFailureRate),
so update the test expectation to assert 100 instead of 200 and modify the
aggregation logic in getDoraTeamOverview (or the helper that computes CFR) to
count distinct failed deployment IDs (i.e., count unique deploymentId of
deployments that had any incident) divided by total deployments and clamp the
result to 0–100 before converting to percentage; keep MTTR (meanTimeToRecoverMs)
logic unchanged and still derive durations from resolved incidents
(seedIncident/resolvedAt).
In
`@apps/web/src/app/metrics-and-insights/dora/components/table-team-overview/table-team-overview.tsx`:
- Around line 137-140: The table row (<Table.Tr>) is only clickable via mouse,
so make it keyboard-accessible by adding focusability and keyboard activation:
give the row a tabIndex={0} and role="button" (or appropriate role), add an
onKeyDown handler that calls selectTeam(row.teamId) when Enter or Space is
pressed, and ensure any aria-label or aria-describedby uses the team identifier
for clarity; update references in the component where Table.Tr and selectTeam
are defined/used to include these attributes and handler.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ef38bd3a-d4ea-4f24-808c-dd03c11081a9
📒 Files selected for processing (14)
apps/api/src/app/metrics/resolvers/dora-metrics.schema.tsapps/api/src/app/metrics/resolvers/queries/dora-metrics.query.tsapps/api/src/app/metrics/services/dora-metrics.integration.test.tsapps/api/src/app/metrics/services/dora-metrics.service.tsapps/api/src/app/metrics/services/dora-metrics.types.tsapps/web/src/api/dora-metrics.api.tsapps/web/src/app/metrics-and-insights/dora/components/table-team-overview/index.tsapps/web/src/app/metrics-and-insights/dora/components/table-team-overview/table-team-overview.tsxapps/web/src/app/metrics-and-insights/dora/page.tsxapps/web/src/app/metrics-and-insights/dora/useDoraMetrics.tsxapps/web/src/sandbox/handlers/dora-metrics.tspackages/graphql-types/api.tspackages/graphql-types/frontend/gql.tspackages/graphql-types/frontend/graphql.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/web/src/app/metrics-and-insights/pr-flow/components/table-team-overview/table-team-overview.tsx">
<violation number="1" location="apps/web/src/app/metrics-and-insights/pr-flow/components/table-team-overview/table-team-overview.tsx:154">
P2: This change removes team-row click behavior, so users can no longer click a team to set the team filter from the overview table.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/web/src/app/metrics-and-insights/dora/components/table-team-overview/table-team-overview.tsx`:
- Line 88: The CFR cell currently uses `${row.changeFailureRate}%` which shows
inconsistent decimal places; update the value assigned to `cfr` in the
`TableTeamOverview` rendering logic (the expression using `hasDeploys` and
`row.changeFailureRate`) to format the numeric value to a fixed precision (e.g.,
one decimal) and append '%' (e.g., convert to Number/parseFloat, guard null/NaN,
then call toFixed(1) + '%') so all rows show a consistent "Failure Rate" format.
- Around line 58-71: The comparator for "leadTimeMs" and "meanTimeToRecoverMs"
treats missing values as -1 which causes null rows to sort to the top when
sortDirection === "asc"; update the comparator logic in the table's sort
function (the switch handling "leadTimeMs" and "meanTimeToRecoverMs", using the
dir variable) to explicitly detect missing/null values and push them to the
bottom regardless of sort direction — e.g., if a value is null/undefined treat
it as greater than any real value by returning 1 when a is missing and -1 when b
is missing (and return 0 if both missing), then only apply dir * numeric
difference for two real values.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 12628d0d-3f65-4fd7-92ab-5f431ea4ba28
📒 Files selected for processing (6)
apps/api/src/app/metrics/resolvers/dora-metrics.schema.tsapps/api/src/app/metrics/services/dora-metrics.integration.test.tsapps/api/src/app/metrics/services/dora-metrics.service.tsapps/web/src/app/metrics-and-insights/code-review-efficiency/components/table-team-overview/table-team-overview.tsxapps/web/src/app/metrics-and-insights/dora/components/table-team-overview/table-team-overview.tsxapps/web/src/app/metrics-and-insights/pr-flow/components/table-team-overview/table-team-overview.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/api/src/app/metrics/resolvers/dora-metrics.schema.ts
- apps/api/src/app/metrics/services/dora-metrics.service.ts
Comment on lines
+58
to
+71
| case "leadTimeMs": { | ||
| const av = a.leadTimeMs != null ? Number(a.leadTimeMs) : -1; | ||
| const bv = b.leadTimeMs != null ? Number(b.leadTimeMs) : -1; | ||
| return dir * (av - bv); | ||
| } | ||
| case "changeFailureRate": | ||
| return dir * (a.changeFailureRate - b.changeFailureRate); | ||
| case "meanTimeToRecoverMs": { | ||
| const av = | ||
| a.meanTimeToRecoverMs != null ? Number(a.meanTimeToRecoverMs) : -1; | ||
| const bv = | ||
| b.meanTimeToRecoverMs != null ? Number(b.meanTimeToRecoverMs) : -1; | ||
| return dir * (av - bv); | ||
| } |
There was a problem hiding this comment.
Null rows sort to the top under ascending sort.
Using -1 as the sentinel for missing leadTimeMs/meanTimeToRecoverMs means any team without data sorts before every team with data when sortDirection === "asc" (since -1 is less than any real duration). Users sorting "fastest lead time first" will see empty rows at the top, which is the opposite of what they'd expect.
Consider pushing nulls to the bottom regardless of direction (standard spreadsheet-style behavior):
♻️ Proposed fix
case "leadTimeMs": {
- const av = a.leadTimeMs != null ? Number(a.leadTimeMs) : -1;
- const bv = b.leadTimeMs != null ? Number(b.leadTimeMs) : -1;
- return dir * (av - bv);
+ const av = a.leadTimeMs != null ? Number(a.leadTimeMs) : null;
+ const bv = b.leadTimeMs != null ? Number(b.leadTimeMs) : null;
+ if (av == null && bv == null) return 0;
+ if (av == null) return 1; // nulls always last
+ if (bv == null) return -1;
+ return dir * (av - bv);
}
case "changeFailureRate":
return dir * (a.changeFailureRate - b.changeFailureRate);
case "meanTimeToRecoverMs": {
- const av =
- a.meanTimeToRecoverMs != null ? Number(a.meanTimeToRecoverMs) : -1;
- const bv =
- b.meanTimeToRecoverMs != null ? Number(b.meanTimeToRecoverMs) : -1;
- return dir * (av - bv);
+ const av =
+ a.meanTimeToRecoverMs != null ? Number(a.meanTimeToRecoverMs) : null;
+ const bv =
+ b.meanTimeToRecoverMs != null ? Number(b.meanTimeToRecoverMs) : null;
+ if (av == null && bv == null) return 0;
+ if (av == null) return 1;
+ if (bv == null) return -1;
+ return dir * (av - bv);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/web/src/app/metrics-and-insights/dora/components/table-team-overview/table-team-overview.tsx`
around lines 58 - 71, The comparator for "leadTimeMs" and "meanTimeToRecoverMs"
treats missing values as -1 which causes null rows to sort to the top when
sortDirection === "asc"; update the comparator logic in the table's sort
function (the switch handling "leadTimeMs" and "meanTimeToRecoverMs", using the
dir variable) to explicitly detect missing/null values and push them to the
bottom regardless of sort direction — e.g., if a value is null/undefined treat
it as greater than any real value by returning 1 when a is missing and -1 when b
is missing (and return 0 if both missing), then only apply dir * numeric
difference for two real values.
| hasDeploys && row.leadTimeMs != null | ||
| ? getAbbreviatedDuration(Number(row.leadTimeMs)) | ||
| : "–", | ||
| cfr: hasDeploys ? `${row.changeFailureRate}%` : "–", |
There was a problem hiding this comment.
Inconsistent CFR display precision.
${row.changeFailureRate}% will render raw values returned by SQL ROUND(..., 2), producing a mix of "50%", "33.33%", and "16.67%" across rows. Normalize to a fixed precision (e.g. 1 decimal) so the column aligns visually and the "Failure Rate" reads consistently.
♻️ Proposed fix
- cfr: hasDeploys ? `${row.changeFailureRate}%` : "–",
+ cfr: hasDeploys ? `${row.changeFailureRate.toFixed(1)}%` : "–",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| cfr: hasDeploys ? `${row.changeFailureRate}%` : "–", | |
| cfr: hasDeploys ? `${row.changeFailureRate.toFixed(1)}%` : "–", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/web/src/app/metrics-and-insights/dora/components/table-team-overview/table-team-overview.tsx`
at line 88, The CFR cell currently uses `${row.changeFailureRate}%` which shows
inconsistent decimal places; update the value assigned to `cfr` in the
`TableTeamOverview` rendering logic (the expression using `hasDeploys` and
`row.changeFailureRate`) to format the numeric value to a fixed precision (e.g.,
one decimal) and append '%' (e.g., convert to Number/parseFloat, guard null/NaN,
then call toFixed(1) + '%') so all rows show a consistent "Failure Rate" format.
sweetrdev
approved these changes
Apr 23, 2026
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
2 participants
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.
Greptile Summary
This PR adds a DORA team overview table that surfaces deployment count, lead time, CFR, and MTTR per team for the selected date range. The backend uses a single CTE-based SQL query (
getDoraTeamOverview) that correctly scopes deployments to each team via theTeamMemberjoin, with thorough integration test coverage for edge cases (archived entities, multi-team attribution, deployment-cohort anchoring for incidents).Confidence Score: 5/5
Safe to merge; all remaining findings are P2 style suggestions.
The backend SQL logic is well-structured and tested (16 integration tests covering archived entities, multi-team attribution, null authors, date range boundaries, and incident-cohort anchoring). The null-sentinel sort ordering issue was flagged in a previous review cycle. Only a minor CFR display formatting nit remains.
apps/web/src/app/metrics-and-insights/dora/components/table-team-overview/table-team-overview.tsx — null sentinel sort order (previous review) and CFR precision formatting.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[DoraPage] -->|teamOverview query| B[teamOverview resolver] B --> C[getDoraTeamOverview] C --> D{Build CTEs} D --> E[all_teams\nfiltered by workspace + teamIds] D --> F[deployments_scoped\ndate range + env + app filters] D --> G[deployment_lead_times\ndeployments with PRs] E --> H[team_deployments\nTeamMember join] F --> H H --> I[team_lead\nAVG lead time] H --> J[team_deploy_count\nCOUNT deployments] H --> K[team_cfr\nincidents / deployments x 100] H --> L[team_mttr\nAVG resolved incident duration] E --> M[Final SELECT\nLEFT JOIN all metrics] I --> M J --> M K --> M L --> M M --> N[TableTeamOverview\nsortable frontend table]Comments Outside Diff (2)
apps/web/src/app/metrics-and-insights/dora/components/table-team-overview/table-team-overview.tsx, line 1147-1151 (link)Rows with
nulllead time or MTTR are assigned a sentinel value of-1, which puts them at the top under an ascending sort and the bottom under a descending sort. Teams without data appear before teams with data when sorting ascending, which is counterintuitive — usually inactive teams should sort to the bottom regardless of direction. Consider usingInfinityfor the missing-data sentinel so null rows always land at the end:apps/web/src/app/metrics-and-insights/dora/components/table-team-overview/table-team-overview.tsx, line 1154-1159 (link)Same problem as
leadTimeMs:nullMTTR is assigned-1, which sorts those teams above teams with real (positive) MTTR values when sorting ascending.Reviews (3): Last reviewed commit: "fix: row click" | Re-trigger Greptile