feat: add dora to metrics digest#81
Conversation
WalkthroughAdds DORA metrics fetching and rendering to the team digest flow (deployment frequency, lead time, change failure rate, MTTR), updates digest construction to fetch these in parallel with team metrics and conditionally render deployment-related controls; also removes the Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/api/src/app/digests/services/digest-team-metrics.service.ts (1)
233-236: Add a comment explaining the negation ofdeploymentFrequency.change.The sign inversion is semantically correct — higher deployment frequency is better, but the
getChangeLabel/getChangeEmojihelpers treat a positive delta as "worse". Without a comment this looks like a bug to the next reader.💡 Proposed clarification
- change: -doraMetrics.deploymentFrequency.change, + // Negated: higher deployment frequency is better, so we invert the sign + // so that an increase renders as "better" (green) rather than "worse" (orange). + change: -doraMetrics.deploymentFrequency.change,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/digests/services/digest-team-metrics.service.ts` around lines 233 - 236, Add an inline comment next to the negation of doraMetrics.deploymentFrequency.change (the `change: -doraMetrics.deploymentFrequency.change` entry in the deployments metric object) explaining that the sign is intentionally inverted because higher deployment frequency is better while getChangeLabel/getChangeEmoji interpret a positive delta as "worse", so we negate the value to preserve correct semantics for display; update the comment to reference getChangeLabel/getChangeEmoji so future readers understand why the inversion is required.
🤖 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/digests/services/digest-team-metrics.service.ts`:
- Around line 253-256: The Failure Rate metric is inconsistent: while the other
DORA metrics cast currentAmount via Number(), the Failure Rate uses a template
string which can preserve Decimal formatting; update the code that builds the
Failure Rate (the object using label "Failure Rate" and
doraMetrics.changeFailureRate.currentAmount) to coerce the value the same way as
the others (use Number(...) on doraMetrics.changeFailureRate.currentAmount) so
the displayed value matches the other metrics; locate the construction that
references doraMetrics.changeFailureRate.currentAmount and apply the Number(...)
cast to its value.
- Around line 47-51: The Promise.all call causes any error from getDoraMetrics
to short-circuit the whole digest; change the call so DORA failures degrade
gracefully: call getTeamMetrics and getDoraMetrics independently (e.g., await
getTeamMetrics(digest) and await getDoraMetrics(digest) inside a try/catch or
use Promise.allSettled for the pair), capture errors from getDoraMetrics and
replace with a safe fallback (null/empty DORA object and a logged warning)
instead of throwing, and then pass that fallback into getDigestMessageBlocks;
also update getDigestMessageBlocks to treat a null/empty doraMetrics by omitting
the DORA block (make the DORA block conditional) so sendTeamMetricsDigest still
sends the digest when DORA fails.
In `@apps/web/src/providers/app.provider.ts`:
- Around line 39-41: The persist middleware configuration for the store named
"app-store" removed the explicit version causing older clients with version: 1
in localStorage to be forced into a reset; restore migration handling by adding
an explicit version bump (e.g., version: 2) and provide a no-op migrate function
that returns the incoming state (or implement appropriate migration logic) so
existing persisted state isn’t discarded, or alternatively add a
clear-old-storage routine that explicitly removes the old "app-store" key on
deploy; update the persist config where "app-store" is defined to include the
chosen version and migrate handler.
---
Nitpick comments:
In `@apps/api/src/app/digests/services/digest-team-metrics.service.ts`:
- Around line 233-236: Add an inline comment next to the negation of
doraMetrics.deploymentFrequency.change (the `change:
-doraMetrics.deploymentFrequency.change` entry in the deployments metric object)
explaining that the sign is intentionally inverted because higher deployment
frequency is better while getChangeLabel/getChangeEmoji interpret a positive
delta as "worse", so we negate the value to preserve correct semantics for
display; update the comment to reference getChangeLabel/getChangeEmoji so future
readers understand why the inversion is required.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/digests/services/digest-team-metrics.service.ts`:
- Around line 290-354: The DORA metrics rich_text block should be rendered only
when deployments are configured; wrap the entire DORA block (the object that
uses doraMetrics and calls getMetricLineElements/formatMsDuration) in a
conditional that checks hasDeploymentIngestion and skip adding that block (and
its trailing divider) when hasDeploymentIngestion is false so users don't see
zero/N/A metrics or the setup prompt alongside empty data.
---
Duplicate comments:
In `@apps/api/src/app/digests/services/digest-team-metrics.service.ts`:
- Around line 329-332: The Failure Rate metric builds its value using
template-literal coercion for doraMetrics.changeFailureRate.currentAmount,
causing inconsistent precision compared to the other metrics; change the value
expression to wrap the currentAmount in Number(...) (i.e., use
Number(doraMetrics.changeFailureRate.currentAmount) when constructing the value
string with %), matching the pattern used for the other DORA metrics in
digest-team-metrics.service.ts so Decimal values are consistently coerced to
numbers.
- Around line 64-67: Promise.all currently uses getTeamMetrics(digest) and
getDoraMetrics(digest) so any failure in getDoraMetrics will reject the whole
Promise.all; change it to call getDoraMetrics(digest).catch(() => null) so the
DORA failure is swallowed and assign the results to const [metrics, doraMetrics]
as before, then make subsequent DORA processing conditional on doraMetrics !==
null (e.g., only merge or include DORA data when non-null). Ensure any
downstream use of doraMetrics safely handles the null case.
f7670fe
| { | ||
| type: "rich_text", | ||
| elements: [ | ||
| { | ||
| type: "rich_text_section", | ||
| elements: [ | ||
| { | ||
| type: "text", | ||
| text: "🚀 DORA Metrics", | ||
| style: { bold: true }, | ||
| }, | ||
| ], | ||
| }, | ||
| { | ||
| type: "rich_text_list", | ||
| elements: [ | ||
| { | ||
| type: "rich_text_section", | ||
| elements: getMetricLineElements({ | ||
| label: "Deployments", | ||
| value: `${Number(doraMetrics.deploymentFrequency.currentAmount)} deploys`, | ||
| change: -doraMetrics.deploymentFrequency.change, | ||
| }), | ||
| }, | ||
| { | ||
| type: "rich_text_section", | ||
| elements: getMetricLineElements({ | ||
| label: "Lead Time", | ||
| value: | ||
| formatMsDuration( | ||
| Number(doraMetrics.leadTime.currentAmount), | ||
| dateFormatter | ||
| ) || "N/A", | ||
| change: doraMetrics.leadTime.change, | ||
| }), | ||
| }, | ||
| { | ||
| type: "rich_text_section", | ||
| elements: getMetricLineElements({ | ||
| label: "Failure Rate", | ||
| value: `${Number(doraMetrics.changeFailureRate.currentAmount)}%`, | ||
| change: doraMetrics.changeFailureRate.change, | ||
| }), | ||
| }, | ||
| { | ||
| type: "rich_text_section", | ||
| elements: getMetricLineElements({ | ||
| label: "MTTR", | ||
| value: | ||
| formatMsDuration( | ||
| Number(doraMetrics.meanTimeToRecover.currentAmount), | ||
| dateFormatter | ||
| ) || "N/A", | ||
| change: doraMetrics.meanTimeToRecover.change, | ||
| }), | ||
| }, | ||
| ], | ||
| style: "bullet", | ||
| indent: 1, | ||
| }, | ||
| ], | ||
| }, |
There was a problem hiding this comment.
DORA metrics section is always displayed even when no deployments exist. Users without deployment ingestion will see "0 deploys" and "N/A" values. Consider conditionally rendering based on hasDeploymentIngestion.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Greptile Summary
This PR adds DORA (DevOps Research and Assessment) metrics to the team metrics digest sent via Slack. The digest now includes deployment frequency, lead time, change failure rate, and mean time to recover alongside existing PR metrics.
Key changes:
The DORA metrics section uses the same change indicator system as existing metrics (green for improvements, orange for regressions), with deployment frequency change inverted since higher deployments are better.
Confidence Score: 5/5
version: 1from the app store (noted in previous review thread), and that DORA metrics display even without deployment data (users will see zeros/N/A).Important Files Changed
version: 1from zustand persist configurationSequence Diagram
sequenceDiagram participant Client participant DigestService participant WorkspaceService participant SlackService participant MetricsService participant DoraService Client->>DigestService: sendTeamMetricsDigest(digest) DigestService->>WorkspaceService: findWorkspaceById() WorkspaceService-->>DigestService: workspace DigestService->>SlackService: getWorkspaceSlackClient() SlackService-->>DigestService: slackClient DigestService->>SlackService: joinSlackChannelOrThrow() SlackService-->>DigestService: slackChannel par Fetch Metrics in Parallel DigestService->>MetricsService: getTeamMetrics() MetricsService-->>DigestService: metrics (PR stats) and DigestService->>DoraService: getDoraMetrics() par Fetch DORA Metrics DoraService->>DoraService: getDeploymentFrequencyMetric() and DoraService->>DoraService: getLeadTimeMetric() and DoraService->>DoraService: getChangeFailureRateMetric() and DoraService->>DoraService: getMeanTimeToRecoverMetric() end DoraService-->>DigestService: doraMetrics end DigestService->>DigestService: getDigestMessageBlocks() Note over DigestService: Check hasDeploymentIngestion<br/>Build blocks with DORA section<br/>Add conditional buttons DigestService->>SlackService: sendSlackMessage(blocks) SlackService-->>Client: SuccessLast reviewed commit: f7670fe