feat: unified metrics architecture with AI family on /v1/metric-results - #190
Merged
Conversation
Metrics are now rendered from the self-describing /v1/metric-results endpoint through one composition path: a registry of metric groups (lib/insight/groups.ts) declares which metrics each group holds and how its drilldown is charted; one collection hook fetches with structurally coherent query keys and roster chunking under the backend row limit; generic renderers (trend, breakdown, peer comparison, peer story, group cards, drilldown templates) draw everything from response metadata — labels, units, formats, directions and explanations are server-owned. The AI adoption family (10 ai.* metrics) is fully cut over: KPI tiles with period-over-period deltas, group cards, needs-attention, personal and team drilldowns. Legacy groups render unchanged through the old path; shared surfaces consume computed intermediates fed by per-source selectors, and a group flips paths by changing its registry entry. Peer standings respect measurement honesty end to end: unmeasured people (null peer target_value) take no standing on any surface, server-suppressed thin-cohort percentiles render as no peer data, null timeseries points draw as gaps, and failed or stale previous-period queries yield no delta rather than a mispaired one. Removed: the legacy AI panel and its query hooks, per-vendor AI config entries, the compiled-in KPI ordering maps, and dead drilldown branches. The duplicated IC_BULLET_DELIVERY batch item now executes once and feeds both task delivery and code quality transforms. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (16)
📝 WalkthroughWalkthroughThis PR adds a typed metric-results pipeline, new metric drilldown and scoring widgets, and rewires the IC/team dashboards to consume group-based metric data. It also removes AI-specific registry entries, section ordering, and legacy panel wiring. ChangesMetrics Collection and Group UI
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
The page overlay signals that already-visible data is being replaced (period/range change). It keyed off bare isFetching for metric collections, so a collection's first load dimmed the whole page while its card showed its own spinner. Gate on revalidation instead (isFetching && !isPending) on both the personal and team screens. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
The loading state replaced the whole card with a generic placeholder that centered the group name beside the spinner. Render the real card chrome instead — name in the header, spinner in the body — so the card keeps its identity and the name isn't repeated. Applied to both the personal and team group cards. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
…ws lazily The section card only renders period + peer, but it was fetching the group's full collection — including the drilldown-only timeseries and tool breakdown — so the card waited on data it never shows while the KPI tiles (period + peer only) painted immediately. Cards now fetch the period+peer projection and the open drilldown fetches the full collection on demand, gated off while closed. Cards paint as fast as the KPI tiles, and heavy views are only fetched for a drilldown the user actually opens. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/widgets/metric-views/team-collection-drilldown.tsx (1)
1-136: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winMissing test coverage causing CI gate failure.
No test file accompanies this new component; diff-cover reports 0% coverage on changed lines, failing the required 80% gate.
🤖 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 `@src/components/widgets/metric-views/team-collection-drilldown.tsx` around lines 1 - 136, The new TeamCollectionDrilldown component lacks test coverage, causing the changed lines to fail the coverage gate. Add a test file for TeamCollectionDrilldown that exercises the key branches in this component: loading, error with retry, no metrics, no members, and the rendered table with member rows and formatted values. Use the TeamCollectionDrilldown, TeamCollectionDrilldownProps, and forEntity paths to locate the behavior to cover, and ensure the tests drive coverage high enough to satisfy the CI threshold.Source: Pipeline failures
🧹 Nitpick comments (5)
src/queries/metric-results.ts (1)
192-217: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnnecessary single-element
refetchesarray.
refetchesis always[query.refetch]— a single entry — so thefor...ofloop over it is just an indirect way of callingquery.refetch(). Simplifying removes a layer of indirection without changing behavior.♻️ Simplify refetch chaining
const existing = out.get(key); - const refetches = [query.refetch]; out.set(key, { byKey: new Map(), previousByKey: null, isPending: (existing?.isPending ?? false) || (query.isPending && enabled), isFetching: (existing?.isFetching ?? false) || query.isFetching, isError: (existing?.isError ?? false) || query.isError, refetch: existing ? () => { existing.refetch(); - for (const r of refetches) void r(); + void query.refetch(); } : () => { - for (const r of refetches) void r(); + void query.refetch(); }, });🤖 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 `@src/queries/metric-results.ts` around lines 192 - 217, The refetch chaining in the metric aggregation logic uses an unnecessary single-element refetches array, which adds indirection without behavior change. In the code that builds the MetricCollectionResult map, simplify the refetch handling around query.refetch so the generated refetch function directly invokes the current query’s refetch (while preserving the existing refetch call on any prior result) instead of looping over a one-item array.src/mocks/metric-results-factory.ts (1)
52-60: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueMock timeseries points don't respect the requested bucket granularity.
bucketStartsalways steps by one day, so aweek/monthbucket request returnsbucket: "week"/"month"metadata paired with daily-spaced points. Low risk since this is mock-only data, but it means UI code paths that branch on bucket granularity aren't meaningfully exercised locally.Also applies to: 101-125
🤖 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 `@src/mocks/metric-results-factory.ts` around lines 52 - 60, The mock timeseries helper bucketStarts is always incrementing by one day, so bucket metadata from the metric-results factory does not match the requested granularity. Update bucketStarts and the mock point generation in metric-results-factory so the step size matches the bucket type used by the caller (for example day, week, month), and ensure the existing mock builders that consume bucketStarts produce correctly spaced timestamps for all supported bucket values.src/lib/metrics/collection.ts (1)
233-265: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for asymmetric merge branches.
Pipeline flags lines 254-255 and 259-260 as uncovered — the case where one chunk's result has a
period/peerview and the accumulatedexistingentry doesn't yet (or vice versa). Current tests only exercise the "both sides have the view" path. Worth a test to lock in the merge behaves correctly if per-chunk view shapes ever diverge.🤖 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 `@src/lib/metrics/collection.ts` around lines 233 - 265, Add tests for mergeNormalizedResults to cover the asymmetric period/peer merge branches where the accumulated entry in out has no period/peer yet but a later map entry does, and the reverse case where existing already has the view and the new result does not. Use mergeNormalizedResults and NormalizedMetricResult fixtures to verify the values are copied or appended correctly in these one-sided paths, not just the both-sides-present path.Source: Pipeline failures
src/components/widgets/metric-views/metric-breakdown.tsx (1)
38-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winEmpty-state branch untested.
CI diff-cover flags line 39 ("No composition data yet" branch) as uncovered even though overall file coverage clears the 80% bar. Worth adding a test case where
forEntity(...).breakdownfilters down to zero rows.🤖 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 `@src/components/widgets/metric-views/metric-breakdown.tsx` around lines 38 - 47, The empty-state branch in metric-breakdown is uncovered, so add a test that exercises the zero-row path in the `MetricBreakdown` component by mocking `forEntity(...).breakdown` to return no rows after filtering. Make sure the test asserts the `CardDescription` empty-state content ("No composition data yet.") renders when `rows.length === 0`, so the `rows` check and its fallback branch are covered.Source: Pipeline failures
src/components/widgets/metric-views/metric-trend.tsx (1)
111-135: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNotable coverage gaps on core branches.
CI diff-cover reports several uncovered lines: the multi-metric merge path (115-119, 125, 129, 134), the multi-metric title join (197), the empty-state card (204), the tooltip
labelFormatter(246), and thechart === "line"render branch (287). These are exactly the branches most likely to regress silently (dimension folding across metrics, line-vs-bar rendering). Recommend adding test cases for: multiple metrics passed together,chart="line", and the no-data empty state.Also applies to: 193-303
🤖 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 `@src/components/widgets/metric-views/metric-trend.tsx` around lines 111 - 135, The uncovered branches in metric-trend.tsx are all in the main rendering path, so add tests that exercise those unique symbols: the multi-metric merge logic inside the metrics loop, the multi-metric title join, the empty-state card, the tooltip labelFormatter, and the chart === "line" render branch. Cover cases with multiple metrics passed together, line chart rendering, and a no-data state so the merge/fallback behavior is asserted and the branch-specific rendering paths are executed.Source: Pipeline failures
🤖 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 `@src/api/metric-results-client.ts`:
- Around line 122-139: The queryMetricResults function in metric-results-client
needs unit tests to raise diff coverage and satisfy the gate. Add tests that
exercise the happy path where fetchWithAuth returns an ok response and parsed
MetricResultsResponse is returned, the !res.ok branch where res.json() provides
an error body and AnalyticsApiError is thrown with the status/body, and the
invalid JSON branch where res.json() rejects on an ok response and
AnalyticsApiError is thrown with invalid_json.
In `@src/components/widgets/metric-views/dimension-series.ts`:
- Around line 39-46: `dimensionSeriesKey` can still produce ambiguous raw
strings before `safeSeriesKey` hashes them because `_` is used for both
key/value pairing and joining pairs. Update `dimensionSeriesKey` to use distinct
separators that won’t collide with typical dimension content, and keep the logic
centralized in this helper so the raw composition is unambiguous before hashing.
Use the `dimensionSeriesKey` and `safeSeriesKey` symbols to locate and adjust
the series key construction.
In `@src/components/widgets/metric-views/peer-comparison.tsx`:
- Around line 1-100: Add test coverage for PeerComparison to satisfy diff-cover
on the new rendering logic in PeerComparison and PeerComparisonProps. Create a
matching test that exercises the quartile strip and marker positioning across
the changed branches in PeerComparison, including the status-dependent
PEER_FILL/PEER_TEXT and higherIsBetter zone selection, so the new component’s
changed lines are covered.
In `@src/components/widgets/metric-views/peer-story.tsx`:
- Around line 1-493: Add tests for the new PeerStory rendering and helper
branches, since the changed helpers and conditional paths are not covered. Add
focused unit/component tests for formatGapPct, formatGap, SideCards,
OutlierChips, SupportingFold, and PeerStory using the existing
PeerStoryEntry/partitionPeerStory behavior to hit the hero, empty, neutral,
critical, rewards, chips, and folded states. Make sure the tests exercise the
key symbols in this file so the CI coverage gate is satisfied.
In `@src/components/widgets/metric-views/team-metric-group-card.tsx`:
- Around line 38-172: Add a dedicated team-metric-group-card.test.tsx for
TeamMetricGroupCard to cover all branches currently untested: pending loading,
error with refetch, scored versus no-peer-data states, preview rendering versus
fallback to top 3, and focus-mode/stripe class behavior. Use the component’s key
symbols (TeamMetricGroupCard, teamMetricStandings, applyFocusStatus,
sectionCounts, ComingSoon, Spinner) to locate the branching paths and mirror the
existing sibling metric-group-card test structure so CI diff-cover reaches the
missing lines.
In `@src/lib/insight/groups.ts`:
- Around line 124-173: Coverage is missing for the new group helpers in
groups.ts; add tests that exercise groupById’s successful lookup and unknown-id
throw path, legacyGroups returning only legacy-kind entries, and
groupIdForMetricKey returning null when no metric matches. Use the existing
exported symbols groupById, legacyGroups, and groupIdForMetricKey in the test
file so the uncovered branches in this diff are hit and CI coverage passes.
In `@src/lib/metrics/entity.ts`:
- Around line 1-9: Add test coverage for normalizePersonId in the corresponding
metrics entity test suite to satisfy the CI diff coverage gate. Create a focused
Vitest case that imports normalizePersonId from the entity module and verifies
it trims whitespace and lowercases the email input, matching the behavior of the
normalizePersonId function so line 8 is covered.
In `@src/lib/metrics/peer-story.ts`:
- Around line 62-68: Add tests covering the untested branching and sort behavior
in peerSpread and partitionPeerStory: exercise peerSpread’s IQR path, range
fallback, and constant-1 fallback, and verify the severity/sort comparators used
to pick the “hero” metric in partitionPeerStory across tied and outlier-heavy
PeerStats cases. Use the peerSpread helper and partitionPeerStory logic to
locate the code, and assert the chosen ordering/selection matches the intended
ranking rules.
In `@src/queries/metric-results.ts`:
- Around line 44-223: The entire metric-results query logic needs test coverage
to satisfy the coverage gate, especially the subtle pairing and chunk-merging
behavior in useMetricCollection and useMetricCollectionSet. Add unit tests that
exercise canonicalEntityIds/queryKeyFor indirectly through useMetricCollection,
covering current vs previous-period query pairing, placeholderData/error gating,
and refetch behavior. Also add tests for useMetricCollectionSet that verify
chunkEntityIds-driven splitting, mergeNormalizedResults output across chunks,
and multi-key refetch aggregation so the changed logic is executed in coverage.
---
Outside diff comments:
In `@src/components/widgets/metric-views/team-collection-drilldown.tsx`:
- Around line 1-136: The new TeamCollectionDrilldown component lacks test
coverage, causing the changed lines to fail the coverage gate. Add a test file
for TeamCollectionDrilldown that exercises the key branches in this component:
loading, error with retry, no metrics, no members, and the rendered table with
member rows and formatted values. Use the TeamCollectionDrilldown,
TeamCollectionDrilldownProps, and forEntity paths to locate the behavior to
cover, and ensure the tests drive coverage high enough to satisfy the CI
threshold.
---
Nitpick comments:
In `@src/components/widgets/metric-views/metric-breakdown.tsx`:
- Around line 38-47: The empty-state branch in metric-breakdown is uncovered, so
add a test that exercises the zero-row path in the `MetricBreakdown` component
by mocking `forEntity(...).breakdown` to return no rows after filtering. Make
sure the test asserts the `CardDescription` empty-state content ("No composition
data yet.") renders when `rows.length === 0`, so the `rows` check and its
fallback branch are covered.
In `@src/components/widgets/metric-views/metric-trend.tsx`:
- Around line 111-135: The uncovered branches in metric-trend.tsx are all in the
main rendering path, so add tests that exercise those unique symbols: the
multi-metric merge logic inside the metrics loop, the multi-metric title join,
the empty-state card, the tooltip labelFormatter, and the chart === "line"
render branch. Cover cases with multiple metrics passed together, line chart
rendering, and a no-data state so the merge/fallback behavior is asserted and
the branch-specific rendering paths are executed.
In `@src/lib/metrics/collection.ts`:
- Around line 233-265: Add tests for mergeNormalizedResults to cover the
asymmetric period/peer merge branches where the accumulated entry in out has no
period/peer yet but a later map entry does, and the reverse case where existing
already has the view and the new result does not. Use mergeNormalizedResults and
NormalizedMetricResult fixtures to verify the values are copied or appended
correctly in these one-sided paths, not just the both-sides-present path.
In `@src/mocks/metric-results-factory.ts`:
- Around line 52-60: The mock timeseries helper bucketStarts is always
incrementing by one day, so bucket metadata from the metric-results factory does
not match the requested granularity. Update bucketStarts and the mock point
generation in metric-results-factory so the step size matches the bucket type
used by the caller (for example day, week, month), and ensure the existing mock
builders that consume bucketStarts produce correctly spaced timestamps for all
supported bucket values.
In `@src/queries/metric-results.ts`:
- Around line 192-217: The refetch chaining in the metric aggregation logic uses
an unnecessary single-element refetches array, which adds indirection without
behavior change. In the code that builds the MetricCollectionResult map,
simplify the refetch handling around query.refetch so the generated refetch
function directly invokes the current query’s refetch (while preserving the
existing refetch call on any prior result) instead of looping over a one-item
array.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e5e31039-7a74-4f09-b15f-01fb347173ff
📒 Files selected for processing (54)
src/api/metric-registry.tssrc/api/metric-results-client.tssrc/components/widgets/metric-views/collection-drilldown.test.tsxsrc/components/widgets/metric-views/collection-drilldown.tsxsrc/components/widgets/metric-views/dimension-series.test.tssrc/components/widgets/metric-views/dimension-series.tssrc/components/widgets/metric-views/metric-breakdown.tsxsrc/components/widgets/metric-views/metric-group-card.test.tsxsrc/components/widgets/metric-views/metric-group-card.tsxsrc/components/widgets/metric-views/metric-trend.tsxsrc/components/widgets/metric-views/peer-comparison.tsxsrc/components/widgets/metric-views/peer-story.tsxsrc/components/widgets/metric-views/team-collection-drilldown.tsxsrc/components/widgets/metric-views/team-metric-group-card.tsxsrc/components/widgets/v2/ai-personal-panel.test.tsxsrc/components/widgets/v2/ai-personal-panel.tsxsrc/components/widgets/v2/collab-messaging-panel.test.tsxsrc/components/widgets/v2/collab-messaging-panel.tsxsrc/components/widgets/v2/group-drilldown-sheet.tsxsrc/components/widgets/v2/ic-needs-attention.test.tsxsrc/components/widgets/v2/ic-needs-attention.tsxsrc/components/widgets/v2/kpi-tile.stories.tsxsrc/components/widgets/v2/kpi-tile.test.tsxsrc/components/widgets/v2/kpi-tile.tsxsrc/components/widgets/v2/team-members-attention.tsxsrc/lib/format.tssrc/lib/insight/attention.test.tssrc/lib/insight/attention.tssrc/lib/insight/groups.tssrc/lib/insight/kpi-row.test.tssrc/lib/insight/kpi-row.tssrc/lib/insight/team-metrics.test.tssrc/lib/insight/team-metrics.tssrc/lib/insight/v2/bullet-defs.tssrc/lib/insight/v2/kpi-defs.tssrc/lib/insight/v2/metric-order.tssrc/lib/insight/v2/sections.tssrc/lib/metrics/collection.test.tssrc/lib/metrics/collection.tssrc/lib/metrics/delta.test.tssrc/lib/metrics/delta.tssrc/lib/metrics/entity.tssrc/lib/metrics/peer-story.test.tssrc/lib/metrics/peer-story.tssrc/mocks/handlers.tssrc/mocks/metric-results-factory.tssrc/mocks/metric-results-fixtures.tssrc/queries/ic-dashboard.tssrc/queries/metric-results.tssrc/queries/v2/ic-extras.test.tssrc/queries/v2/ic-extras.tssrc/queries/v2/team-extras.tssrc/screens/ic-dashboard/engineering-dashboard-v2.tsxsrc/screens/team-view-v2.tsx
💤 Files with no reviewable changes (7)
- src/components/widgets/v2/ai-personal-panel.test.tsx
- src/lib/insight/v2/sections.ts
- src/queries/v2/ic-extras.test.ts
- src/lib/insight/v2/kpi-defs.ts
- src/components/widgets/v2/ai-personal-panel.tsx
- src/lib/insight/v2/metric-order.ts
- src/lib/insight/v2/bullet-defs.ts
Comment on lines
+122
to
+139
| export async function queryMetricResults( | ||
| body: MetricResultsRequest, | ||
| ): Promise<MetricResultsResponse> { | ||
| const res = await fetchWithAuth(`${BASE}/metric-results`, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify(body), | ||
| }); | ||
| if (!res.ok) { | ||
| const errorBody = await res.json().catch(() => null); | ||
| throw new AnalyticsApiError(res.status, errorBody); | ||
| } | ||
| try { | ||
| return (await res.json()) as MetricResultsResponse; | ||
| } catch { | ||
| throw new AnalyticsApiError(res.status, { error: "invalid_json" }); | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add tests to satisfy the coverage gate.
The coverage pipeline reports 0% diff coverage on this function (lines 4, 125, 130-132, 134-135, 137), which will fail the CI gate (min 80%). Add unit tests covering the success path, the !res.ok branch, and the invalid-JSON branch.
🤖 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 `@src/api/metric-results-client.ts` around lines 122 - 139, The
queryMetricResults function in metric-results-client needs unit tests to raise
diff coverage and satisfy the gate. Add tests that exercise the happy path where
fetchWithAuth returns an ok response and parsed MetricResultsResponse is
returned, the !res.ok branch where res.json() provides an error body and
AnalyticsApiError is thrown with the status/body, and the invalid JSON branch
where res.json() rejects on an ok response and AnalyticsApiError is thrown with
invalid_json.
Source: Pipeline failures
Comment on lines
+1
to
+100
| import { ArrowUp } from "lucide-react"; | ||
|
|
||
| import { formatMetricValue } from "@/lib/format"; | ||
| import type { MetricFormat } from "@/api/metric-results-client"; | ||
| import { | ||
| PEER_FILL, | ||
| PEER_TEXT, | ||
| type PeerStats, | ||
| type PeerStatusWithNeutral, | ||
| } from "@/lib/peers"; | ||
| import { STATUS_SURFACE_CLASS } from "@/lib/status"; | ||
| import { cn } from "@/lib/utils"; | ||
|
|
||
| export interface PeerComparisonProps { | ||
| value: number; | ||
| stats: PeerStats; | ||
| status: PeerStatusWithNeutral; | ||
| higherIsBetter: boolean; | ||
| format: MetricFormat; | ||
| unit: string | null; | ||
| } | ||
|
|
||
| /** | ||
| * Quartile strip: cohort zones (bottom / interquartile pack / top, colored by | ||
| * direction), median tick, and the person's value marker. Binless — cohort | ||
| * percentiles come from the peer view. | ||
| */ | ||
| export function PeerComparison({ | ||
| value, | ||
| stats, | ||
| status, | ||
| higherIsBetter, | ||
| format, | ||
| unit, | ||
| }: PeerComparisonProps) { | ||
| const span = Math.max(1e-9, stats.max - stats.min); | ||
| const pct = (v: number) => | ||
| ((Math.max(stats.min, Math.min(stats.max, v)) - stats.min) / span) * 100; | ||
| const p25Left = pct(stats.p25); | ||
| const p50Left = pct(stats.p50); | ||
| const p75Left = pct(stats.p75); | ||
| const valueLeft = pct(value); | ||
| const bottomZone = higherIsBetter | ||
| ? STATUS_SURFACE_CLASS.bad | ||
| : STATUS_SURFACE_CLASS.good; | ||
| const topZone = higherIsBetter | ||
| ? STATUS_SURFACE_CLASS.good | ||
| : STATUS_SURFACE_CLASS.bad; | ||
|
|
||
| return ( | ||
| <div className="mt-4"> | ||
| <div className="relative h-3.5 w-full select-none"> | ||
| <div className="absolute inset-x-0 top-1/2 h-2 -translate-y-1/2 overflow-hidden rounded-sm"> | ||
| <div | ||
| className={cn("absolute inset-y-0 left-0", bottomZone)} | ||
| style={{ width: `${p25Left}%` }} | ||
| /> | ||
| <div | ||
| className="absolute inset-y-0 bg-muted" | ||
| style={{ left: `${p25Left}%`, width: `${p75Left - p25Left}%` }} | ||
| /> | ||
| <div | ||
| className={cn("absolute inset-y-0", topZone)} | ||
| style={{ left: `${p75Left}%`, right: 0 }} | ||
| /> | ||
| </div> | ||
| <div | ||
| className="absolute inset-y-0 w-px bg-foreground/60" | ||
| style={{ left: `${p50Left}%` }} | ||
| aria-hidden | ||
| /> | ||
| <div | ||
| className={cn( | ||
| "absolute inset-y-0 w-[3px] -translate-x-1/2 rounded-sm ring-2 ring-background", | ||
| PEER_FILL[status], | ||
| )} | ||
| style={{ left: `${valueLeft}%` }} | ||
| /> | ||
| </div> | ||
| <div className="relative h-5"> | ||
| <ArrowUp | ||
| className={cn( | ||
| "absolute top-1 size-4 -translate-x-1/2", | ||
| PEER_TEXT[status], | ||
| )} | ||
| style={{ left: `${valueLeft}%` }} | ||
| strokeWidth={3} | ||
| /> | ||
| </div> | ||
| <div className="mt-1 grid grid-cols-2 gap-3 text-[10px] tabular-nums"> | ||
| <span className="text-left text-muted-foreground"> | ||
| {formatMetricValue(stats.min, format, unit)} | ||
| </span> | ||
| <span className="text-right text-muted-foreground"> | ||
| {formatMetricValue(stats.max, format, unit)} | ||
| </span> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Missing test coverage causing CI gate failure.
The diff-cover pipeline reports 0% coverage on the changed lines (36-50) in this new component — no test file accompanies it in this batch.
🧰 Tools
🪛 GitHub Actions: CI / 0_Coverage summary & gate.txt
[error] 1-1: diff-cover: Changed lines coverage 0.0% (min 80%). Missing lines: 36-37, 39-43, 46, 50.
🪛 GitHub Actions: CI / Coverage summary & gate
[error] 36-50: diff-cover reported insufficient diff coverage (0.0% < 80%). Missing lines: 36-37, 39-43, 46, 50.
🤖 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 `@src/components/widgets/metric-views/peer-comparison.tsx` around lines 1 -
100, Add test coverage for PeerComparison to satisfy diff-cover on the new
rendering logic in PeerComparison and PeerComparisonProps. Create a matching
test that exercises the quartile strip and marker positioning across the changed
branches in PeerComparison, including the status-dependent PEER_FILL/PEER_TEXT
and higherIsBetter zone selection, so the new component’s changed lines are
covered.
Source: Pipeline failures
Comment on lines
+1
to
+493
| import { useState } from "react"; | ||
| import { ChevronDown, ChevronRight } from "lucide-react"; | ||
|
|
||
| import { | ||
| Card, | ||
| CardDescription, | ||
| CardHeader, | ||
| CardTitle, | ||
| } from "@/components/ui/card"; | ||
| import { | ||
| Tooltip, | ||
| TooltipContent, | ||
| TooltipTrigger, | ||
| } from "@/components/ui/tooltip"; | ||
| import { PeerComparison } from "@/components/widgets/metric-views/peer-comparison"; | ||
| import { useSettings } from "@/hooks/use-settings"; | ||
| import { | ||
| formatMetricNumber, | ||
| formatMetricValue, | ||
| metricDisplayUnit, | ||
| } from "@/lib/format"; | ||
| import { | ||
| partitionPeerStory, | ||
| type PeerStoryEntry, | ||
| } from "@/lib/metrics/peer-story"; | ||
| import { PEER_FILL, PEER_TEXT, type PeerCohortLabel } from "@/lib/peers"; | ||
| import { cn } from "@/lib/utils"; | ||
|
|
||
| interface PeerStoryProps { | ||
| entries: PeerStoryEntry[]; | ||
| cohortLabel?: PeerCohortLabel; | ||
| emptyLabel?: string; | ||
| className?: string; | ||
| } | ||
|
|
||
| function formatGapPct(gap: number): string { | ||
| const pct = Math.round(Math.abs(gap) * 100); | ||
| if (pct === 0) return "0%"; | ||
| return `${gap >= 0 ? "+" : "-"}${pct}%`; | ||
| } | ||
|
|
||
| function formatGap(entry: PeerStoryEntry): string { | ||
| if (entry.gapPct != null) return formatGapPct(entry.gapPct); | ||
| const sign = entry.gapDelta >= 0 ? "+" : "-"; | ||
| return `${sign}${formatMetricValue( | ||
| Math.abs(entry.gapDelta), | ||
| entry.format, | ||
| entry.unit, | ||
| )}`; | ||
| } | ||
|
|
||
| function outlierText(status: PeerStoryEntry["status"]): string { | ||
| return status === "bottom" ? "Bottom 25%" : "Top 25%"; | ||
| } | ||
|
|
||
| function HeroCard({ | ||
| entry, | ||
| cohortLabel, | ||
| }: { | ||
| entry: PeerStoryEntry; | ||
| cohortLabel: PeerCohortLabel; | ||
| }) { | ||
| const isBad = entry.status === "bottom"; | ||
| const color = isBad ? "bottom" : "top"; | ||
| const unit = metricDisplayUnit(entry.format, entry.unit); | ||
| return ( | ||
| <Card | ||
| className={cn( | ||
| "flex h-full min-h-72 flex-col gap-0 p-0", | ||
| isBad | ||
| ? "shadow-[inset_0_3px_0_0_var(--destructive)]" | ||
| : "shadow-[inset_0_3px_0_0_var(--success)]", | ||
| )} | ||
| > | ||
| <div className="flex flex-1 flex-col gap-3 p-5 sm:p-6"> | ||
| <div className="flex items-center gap-1.5"> | ||
| <span className={cn("size-1.5 rounded-full", PEER_FILL[color])} /> | ||
| <span | ||
| className={cn( | ||
| "text-[10px] font-semibold uppercase tracking-widest", | ||
| PEER_TEXT[color], | ||
| )} | ||
| > | ||
| {isBad ? "Top issue" : "Top win"} | ||
| </span> | ||
| </div> | ||
| <div> | ||
| <h3 className="text-xl font-semibold tracking-tight sm:text-2xl"> | ||
| {entry.label} | ||
| </h3> | ||
| {entry.sublabel ? ( | ||
| <p className="mt-1 text-sm text-muted-foreground">{entry.sublabel}</p> | ||
| ) : null} | ||
| </div> | ||
| <div className="flex flex-wrap items-baseline gap-x-5 gap-y-1.5"> | ||
| <span className="flex items-baseline gap-1"> | ||
| <span | ||
| className={cn( | ||
| "text-4xl font-semibold tabular-nums tracking-tight sm:text-[2.75rem]", | ||
| PEER_TEXT[color], | ||
| )} | ||
| > | ||
| {entry.format === "percent" | ||
| ? formatMetricValue(entry.value, entry.format, entry.unit) | ||
| : formatMetricNumber(entry.value, entry.format)} | ||
| </span> | ||
| {unit ? ( | ||
| <span className="text-base text-muted-foreground">{unit}</span> | ||
| ) : null} | ||
| </span> | ||
| {entry.stats ? ( | ||
| <span className="text-sm tabular-nums text-muted-foreground"> | ||
| gap{" "} | ||
| <span className={cn("font-normal", PEER_TEXT[color])}> | ||
| {formatGap(entry)} | ||
| </span>{" "} | ||
| from {cohortLabel} median{" "} | ||
| <span className="text-foreground"> | ||
| {formatMetricValue(entry.stats.p50, entry.format, entry.unit)} | ||
| </span> | ||
| </span> | ||
| ) : null} | ||
| </div> | ||
| {entry.stats && entry.stats.max > entry.stats.min ? ( | ||
| <PeerComparison | ||
| value={entry.value} | ||
| stats={entry.stats} | ||
| status={entry.status} | ||
| higherIsBetter={entry.higherIsBetter} | ||
| format={entry.format} | ||
| unit={entry.unit} | ||
| /> | ||
| ) : null} | ||
| <span className={cn("mt-auto text-xs font-medium", PEER_TEXT[color])}> | ||
| {isBad ? `Bottom 25% in ${cohortLabel}` : `Top 25% in ${cohortLabel}`} | ||
| </span> | ||
| </div> | ||
| </Card> | ||
| ); | ||
| } | ||
|
|
||
| function SideCards({ entries }: { entries: PeerStoryEntry[] }) { | ||
| if (entries.length === 0) return null; | ||
| const stretchCards = entries.length > 1; | ||
| return ( | ||
| <div | ||
| className={cn( | ||
| "grid gap-3", | ||
| stretchCards ? "h-full" : "content-start", | ||
| entries.length === 2 && "grid-rows-2", | ||
| entries.length === 3 && "grid-rows-3", | ||
| )} | ||
| > | ||
| {entries.map((entry) => ( | ||
| <SideCard key={entry.key} entry={entry} stretch={stretchCards} /> | ||
| ))} | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| function SideCard({ | ||
| entry, | ||
| stretch, | ||
| }: { | ||
| entry: PeerStoryEntry; | ||
| stretch: boolean; | ||
| }) { | ||
| const unit = metricDisplayUnit(entry.format, entry.unit); | ||
| return ( | ||
| <Card | ||
| className={cn( | ||
| "min-h-28 p-0", | ||
| stretch && "h-full", | ||
| "border-current/20", | ||
| PEER_TEXT[entry.status], | ||
| entry.status === "top" && "bg-success/5", | ||
| entry.status === "bottom" && "bg-destructive/5", | ||
| entry.status === "top" && "shadow-[inset_4px_0_0_0_var(--success)]", | ||
| entry.status === "bottom" && | ||
| "shadow-[inset_4px_0_0_0_var(--destructive)]", | ||
| )} | ||
| > | ||
| <div className="flex h-full"> | ||
| <div className="flex min-w-0 flex-1 flex-col justify-between gap-3 p-4"> | ||
| <div className="min-w-0"> | ||
| <div className="truncate text-sm font-semibold text-muted-foreground"> | ||
| {entry.label} | ||
| </div> | ||
| {entry.sublabel ? ( | ||
| <div className="mt-0.5 truncate text-xs text-muted-foreground"> | ||
| {entry.sublabel} | ||
| </div> | ||
| ) : null} | ||
| </div> | ||
| <div> | ||
| <div className="text-2xl font-semibold tabular-nums"> | ||
| {entry.format === "percent" | ||
| ? formatMetricValue(entry.value, entry.format, entry.unit) | ||
| : formatMetricNumber(entry.value, entry.format)} | ||
| {unit ? ( | ||
| <span className="ml-1 text-xs font-normal text-muted-foreground"> | ||
| {unit} | ||
| </span> | ||
| ) : null} | ||
| </div> | ||
| <div className="mt-1 truncate text-[11px] text-muted-foreground"> | ||
| {outlierText(entry.status)} | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </Card> | ||
| ); | ||
| } | ||
|
|
||
| function ChipTooltip({ | ||
| entry, | ||
| cohortLabel, | ||
| }: { | ||
| entry: PeerStoryEntry; | ||
| cohortLabel: PeerCohortLabel; | ||
| }) { | ||
| return ( | ||
| <span className="flex flex-col gap-1 leading-snug"> | ||
| <span className="font-medium">{entry.label}</span> | ||
| {entry.sublabel ? ( | ||
| <span className="text-background/70">{entry.sublabel}</span> | ||
| ) : null} | ||
| <span> | ||
| {formatMetricValue(entry.value, entry.format, entry.unit)} ·{" "} | ||
| {outlierText(entry.status)} | ||
| </span> | ||
| {entry.stats ? ( | ||
| <span className="text-background/70"> | ||
| gap {formatGap(entry)} · {cohortLabel} median{" "} | ||
| {formatMetricValue(entry.stats.p50, entry.format, entry.unit)} | ||
| </span> | ||
| ) : null} | ||
| </span> | ||
| ); | ||
| } | ||
|
|
||
| function OutlierChips({ | ||
| entries, | ||
| cohortLabel, | ||
| }: { | ||
| entries: PeerStoryEntry[]; | ||
| cohortLabel: PeerCohortLabel; | ||
| }) { | ||
| if (entries.length === 0) return null; | ||
| return ( | ||
| <div className="flex flex-wrap gap-1.5"> | ||
| {entries.map((entry) => ( | ||
| <Tooltip key={entry.key}> | ||
| <TooltipTrigger | ||
| render={ | ||
| <button | ||
| type="button" | ||
| className={cn( | ||
| "inline-flex cursor-help items-center gap-1 rounded-full border bg-transparent px-2.5 py-1 text-xs focus-visible:ring-1 focus-visible:ring-ring focus-visible:outline-none", | ||
| PEER_TEXT[entry.status], | ||
| )} | ||
| > | ||
| <span | ||
| className={cn("size-1.5 rounded-full", PEER_FILL[entry.status])} | ||
| /> | ||
| {entry.label} | ||
| <span className="font-mono tabular-nums"> | ||
| {formatMetricValue(entry.value, entry.format, entry.unit)} | ||
| </span> | ||
| </button> | ||
| } | ||
| /> | ||
| <TooltipContent side="top" className="max-w-64"> | ||
| <ChipTooltip entry={entry} cohortLabel={cohortLabel} /> | ||
| </TooltipContent> | ||
| </Tooltip> | ||
| ))} | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| function FlatGridCard({ entry }: { entry: PeerStoryEntry }) { | ||
| const unit = metricDisplayUnit(entry.format, entry.unit); | ||
| return ( | ||
| <Card className="p-4"> | ||
| <div className="min-w-0"> | ||
| <div className="truncate text-sm font-semibold text-muted-foreground"> | ||
| {entry.label} | ||
| </div> | ||
| {entry.sublabel ? ( | ||
| <div className="mt-0.5 truncate text-xs text-muted-foreground"> | ||
| {entry.sublabel} | ||
| </div> | ||
| ) : null} | ||
| </div> | ||
| <div className="mt-5 text-2xl font-semibold tabular-nums"> | ||
| {entry.format === "percent" | ||
| ? formatMetricValue(entry.value, entry.format, entry.unit) | ||
| : formatMetricNumber(entry.value, entry.format)} | ||
| {unit ? ( | ||
| <span className="ml-1 text-xs font-normal text-muted-foreground"> | ||
| {unit} | ||
| </span> | ||
| ) : null} | ||
| </div> | ||
| </Card> | ||
| ); | ||
| } | ||
|
|
||
| function FlatGrid({ | ||
| entries, | ||
| className, | ||
| }: { | ||
| entries: PeerStoryEntry[]; | ||
| className?: string; | ||
| }) { | ||
| if (entries.length === 0) return null; | ||
| return ( | ||
| <div | ||
| className={cn( | ||
| "grid gap-3 [grid-template-columns:repeat(auto-fit,minmax(260px,1fr))]", | ||
| className, | ||
| )} | ||
| > | ||
| {entries.map((entry) => ( | ||
| <FlatGridCard key={entry.key} entry={entry} /> | ||
| ))} | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| function EmptyState({ label }: { label: string }) { | ||
| return <Card className="p-6 text-sm text-muted-foreground">{label}</Card>; | ||
| } | ||
|
|
||
| function SupportingFold({ | ||
| entries, | ||
| cohortLabel, | ||
| }: { | ||
| entries: PeerStoryEntry[]; | ||
| cohortLabel: PeerCohortLabel; | ||
| }) { | ||
| const [open, setOpen] = useState(false); | ||
| if (entries.length === 0) return null; | ||
| const neutralCount = entries.filter( | ||
| (entry) => entry.status === "neutral", | ||
| ).length; | ||
| const trueOnParCount = entries.length - neutralCount; | ||
| const summaryLabel = | ||
| neutralCount === 0 | ||
| ? `${entries.length} on-par metric${entries.length === 1 ? "" : "s"}` | ||
| : trueOnParCount === 0 | ||
| ? `${entries.length} supporting metric${entries.length === 1 ? "" : "s"}` | ||
| : `${entries.length} supporting and on-par metric${ | ||
| entries.length === 1 ? "" : "s" | ||
| }`; | ||
| const summaryDescription = | ||
| neutralCount === 0 | ||
| ? `Metrics within the ${cohortLabel}'s normal range - no peer outlier` | ||
| : "Additional metrics without a visible peer outlier"; | ||
| return ( | ||
| <div className="rounded-md border"> | ||
| <button | ||
| type="button" | ||
| onClick={() => setOpen((value) => !value)} | ||
| className="flex w-full items-start justify-between gap-3 px-3 py-2 text-left transition-colors hover:bg-accent" | ||
| aria-expanded={open} | ||
| > | ||
| <div className="min-w-0 flex-1"> | ||
| <div className="text-sm font-medium"> | ||
| {open ? "Hide" : "Show"} {summaryLabel} | ||
| </div> | ||
| <div className="text-[11px] text-muted-foreground"> | ||
| {summaryDescription} | ||
| </div> | ||
| </div> | ||
| {open ? ( | ||
| <ChevronDown className="mt-0.5 size-4 shrink-0 text-muted-foreground" /> | ||
| ) : ( | ||
| <ChevronRight className="mt-0.5 size-4 shrink-0 text-muted-foreground" /> | ||
| )} | ||
| </button> | ||
| {open ? ( | ||
| <div className="border-t p-3"> | ||
| <div className="grid gap-x-8 gap-y-2 text-sm md:grid-cols-[minmax(180px,280px)_auto_1fr]"> | ||
| {entries.map((entry) => ( | ||
| <SupportingRow | ||
| key={entry.key} | ||
| entry={entry} | ||
| cohortLabel={cohortLabel} | ||
| /> | ||
| ))} | ||
| </div> | ||
| </div> | ||
| ) : null} | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| function SupportingRow({ | ||
| entry, | ||
| cohortLabel, | ||
| }: { | ||
| entry: PeerStoryEntry; | ||
| cohortLabel: PeerCohortLabel; | ||
| }) { | ||
| const unit = metricDisplayUnit(entry.format, entry.unit); | ||
| return ( | ||
| <div className="contents"> | ||
| <div className="font-medium">{entry.label}</div> | ||
| <div className="font-mono font-semibold tabular-nums"> | ||
| {entry.format === "percent" | ||
| ? formatMetricValue(entry.value, entry.format, entry.unit) | ||
| : formatMetricNumber(entry.value, entry.format)} | ||
| {unit ? ( | ||
| <span className="ml-1 font-sans text-xs font-normal text-muted-foreground"> | ||
| {unit} | ||
| </span> | ||
| ) : null} | ||
| </div> | ||
| <div className="text-muted-foreground"> | ||
| {!entry.stats ? ( | ||
| <span>no peer data</span> | ||
| ) : !entry.observed ? ( | ||
| <span> | ||
| no recorded activity · {cohortLabel} median:{" "} | ||
| {formatMetricValue(entry.stats.p50, entry.format, entry.unit)} | ||
| </span> | ||
| ) : entry.status === "in_pack" ? ( | ||
| <span> | ||
| on par · {cohortLabel} median:{" "} | ||
| {formatMetricValue(entry.stats.p50, entry.format, entry.unit)} | ||
| </span> | ||
| ) : ( | ||
| <span> | ||
| {cohortLabel} median:{" "} | ||
| {formatMetricValue(entry.stats.p50, entry.format, entry.unit)} | ||
| </span> | ||
| )} | ||
| </div> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| export function PeerStory({ | ||
| entries, | ||
| cohortLabel = "department", | ||
| emptyLabel = "No counter data yet.", | ||
| className, | ||
| }: PeerStoryProps) { | ||
| const { focusMode } = useSettings(); | ||
| const { hero, sideCards, chips, folded } = partitionPeerStory( | ||
| entries, | ||
| focusMode, | ||
| ); | ||
|
|
||
| if (entries.length === 0) { | ||
| return ( | ||
| <Card className={className}> | ||
| <CardHeader> | ||
| <CardTitle className="text-sm font-semibold">Counters</CardTitle> | ||
| <CardDescription>{emptyLabel}</CardDescription> | ||
| </CardHeader> | ||
| </Card> | ||
| ); | ||
| } | ||
|
|
||
| if (focusMode === "neutral") { | ||
| return <FlatGrid entries={entries} className={className} />; | ||
| } | ||
|
|
||
| if (focusMode === "all" && !hero) { | ||
| return <FlatGrid entries={entries} className={className} />; | ||
| } | ||
|
|
||
| return ( | ||
| <div className={cn("flex flex-col gap-4", className)}> | ||
| {hero ? ( | ||
| <div className="grid grid-cols-1 items-stretch gap-4 lg:grid-cols-2"> | ||
| <HeroCard entry={hero} cohortLabel={cohortLabel} /> | ||
| <SideCards entries={sideCards} /> | ||
| </div> | ||
| ) : focusMode === "critical" ? ( | ||
| <EmptyState label="No critical issues this period" /> | ||
| ) : focusMode === "rewards" ? ( | ||
| <EmptyState label="No standout wins this period" /> | ||
| ) : null} | ||
| <OutlierChips entries={chips} cohortLabel={cohortLabel} /> | ||
| <SupportingFold entries={folded} cohortLabel={cohortLabel} /> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Missing test coverage causing CI gate failure.
diff-cover reports only 22.9% coverage on the changed lines (min 80% required), with most of the new helper functions and component branches (formatGapPct, formatGap, SideCards, OutlierChips, SupportingFold, PeerStory itself) untested.
🧰 Tools
🪛 GitHub Actions: CI / 0_Coverage summary & gate.txt
[error] 1-1: diff-cover: Changed lines coverage 22.9% (min 80%). Missing lines: 37-39, 43-45, 53, 63-66, 143-145, 155, 168-169, 223, 250-251, 254, 334, 344-347, 349, 351, 359, 362, 366, 388, 408-409, 459, 470, 477.
🪛 GitHub Actions: CI / Coverage summary & gate
[error] 37-477: diff-cover reported insufficient diff coverage (22.9% < 80%). Missing lines: 37-39, 43-45, 53, 63-66, 143-145, 155, 168-169, 223, 250-251, 254, 334, 344-347, 349, 351, 359, 362, 366, 388, 408-409, 459, 470, 477.
🤖 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 `@src/components/widgets/metric-views/peer-story.tsx` around lines 1 - 493, Add
tests for the new PeerStory rendering and helper branches, since the changed
helpers and conditional paths are not covered. Add focused unit/component tests
for formatGapPct, formatGap, SideCards, OutlierChips, SupportingFold, and
PeerStory using the existing PeerStoryEntry/partitionPeerStory behavior to hit
the hero, empty, neutral, critical, rewards, chips, and folded states. Make sure
the tests exercise the key symbols in this file so the CI coverage gate is
satisfied.
Source: Pipeline failures
Comment on lines
+38
to
+172
| export function TeamMetricGroupCard({ | ||
| def, | ||
| data, | ||
| memberIds, | ||
| onOpen, | ||
| subtitle, | ||
| }: TeamMetricGroupCardProps) { | ||
| const { focusMode } = useSettings(); | ||
|
|
||
| if (data.isPending) { | ||
| // Keep the card's identity while it loads: the name in the header, a | ||
| // spinner in the body. Not interactive — nothing to open yet. | ||
| return ( | ||
| <Card className="border-l-2 border-l-border"> | ||
| <CardHeader className="pb-2"> | ||
| <CardTitle className="text-base font-semibold">{def.title}</CardTitle> | ||
| {subtitle ? ( | ||
| <CardDescription className="text-xs text-muted-foreground"> | ||
| {subtitle} | ||
| </CardDescription> | ||
| ) : null} | ||
| </CardHeader> | ||
| <CardContent className="flex items-center justify-center py-6"> | ||
| <Spinner | ||
| className="size-5 text-muted-foreground" | ||
| aria-label={`Loading ${def.title}`} | ||
| /> | ||
| </CardContent> | ||
| </Card> | ||
| ); | ||
| } | ||
| if (data.isError) { | ||
| return ( | ||
| <ComingSoon | ||
| variant="card" | ||
| state="error" | ||
| label={def.title} | ||
| onRetry={data.refetch} | ||
| /> | ||
| ); | ||
| } | ||
|
|
||
| const standings = teamMetricStandings(def, data.byKey, memberIds); | ||
| const scored = standings.filter((s) => s.scored > 0); | ||
| const scoredMetrics = standings.map((standing) => ({ | ||
| row: standing, | ||
| status: standing.status, | ||
| })); | ||
| const status = applyFocusStatus( | ||
| aggregateSectionStatus(scoredMetrics), | ||
| focusMode, | ||
| ); | ||
| const counts = sectionCounts(scoredMetrics); | ||
| const evaluated = counts.good + counts.warn + counts.bad; | ||
| const badgeText = | ||
| evaluated === 0 | ||
| ? "No peer data" | ||
| : `${counts.good} of ${evaluated} metrics ahead`; | ||
|
|
||
| const preview: TeamMetricStanding[] = def.card.preview | ||
| .map((key) => scored.find((s) => s.metric.metric_key === key)) | ||
| .filter((s): s is TeamMetricStanding => s != null); | ||
| const stripeClass = | ||
| status === "neutral" ? "border-l-border" : SECTION_STRIPE[status]; | ||
|
|
||
| return ( | ||
| <Card | ||
| render={ | ||
| <button | ||
| type="button" | ||
| onClick={onOpen} | ||
| aria-label={`Open ${def.title} details`} | ||
| /> | ||
| } | ||
| className={cn( | ||
| "border-l-2 text-left transition-colors hover:bg-accent/50", | ||
| stripeClass, | ||
| )} | ||
| > | ||
| <CardHeader className="pb-2"> | ||
| <CardTitle className="text-base font-semibold">{def.title}</CardTitle> | ||
| <CardDescription className="flex flex-col gap-1 text-xs"> | ||
| {subtitle ? ( | ||
| <span className="text-muted-foreground">{subtitle}</span> | ||
| ) : null} | ||
| <span className="flex items-center gap-1.5"> | ||
| <span | ||
| className={cn( | ||
| "size-1.5 shrink-0 rounded-full", | ||
| STATUS_BG_CLASS[status], | ||
| )} | ||
| aria-hidden | ||
| /> | ||
| <span className="tabular-nums">{badgeText}</span> | ||
| </span> | ||
| </CardDescription> | ||
| </CardHeader> | ||
| <CardContent className="flex flex-col gap-3 pt-0"> | ||
| {scored.length === 0 ? ( | ||
| <p className="text-sm text-muted-foreground"> | ||
| No metrics with peer data for this period. | ||
| </p> | ||
| ) : ( | ||
| <ul className="flex flex-col gap-1.5"> | ||
| {(preview.length > 0 ? preview : scored.slice(0, 3)).map( | ||
| (standing) => { | ||
| const rowStatus = applyFocusStatus(standing.status, focusMode); | ||
| return ( | ||
| <li | ||
| key={standing.metric.metric_key} | ||
| className="flex items-center gap-2 text-sm" | ||
| > | ||
| <span | ||
| className={cn( | ||
| "size-2 shrink-0 rounded-full", | ||
| STATUS_BG_CLASS[rowStatus], | ||
| )} | ||
| aria-hidden | ||
| /> | ||
| <span className="min-w-0 flex-1 truncate text-muted-foreground"> | ||
| {standing.metric.label} | ||
| </span> | ||
| <span className="shrink-0 font-medium tabular-nums"> | ||
| {standing.top} of {standing.scored} in top | ||
| </span> | ||
| </li> | ||
| ); | ||
| }, | ||
| )} | ||
| </ul> | ||
| )} | ||
| </CardContent> | ||
| </Card> | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Zero test coverage on this component.
CI diff-cover reports 0% coverage for the entire file (missing 45, 47, 50, 69-70, 80-82, 86, 90-91, 93, 97-99, 101, 103, 144-145), and no team-metric-group-card.test.tsx was included in this PR, unlike the sibling metric-group-card.test.tsx which thoroughly covers loading/error/scoring states. Given this component has comparable branching complexity (pending/error/scored/unscored/preview-fallback), it should get equivalent test coverage before merge.
🧰 Tools
🪛 GitHub Actions: CI / Coverage summary & gate
[error] 45-145: diff-cover reported insufficient diff coverage (0.0% < 80%). Missing lines: 45, 47, 50, 69-70, 80-82, 86, 90-91, 93, 97-99, 101, 103, 144-145.
🤖 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 `@src/components/widgets/metric-views/team-metric-group-card.tsx` around lines
38 - 172, Add a dedicated team-metric-group-card.test.tsx for
TeamMetricGroupCard to cover all branches currently untested: pending loading,
error with refetch, scored versus no-peer-data states, preview rendering versus
fallback to top 3, and focus-mode/stripe class behavior. Use the component’s key
symbols (TeamMetricGroupCard, teamMetricStandings, applyFocusStatus,
sectionCounts, ComingSoon, Spinner) to locate the branching paths and mirror the
existing sibling metric-group-card test structure so CI diff-cover reaches the
missing lines.
Source: Pipeline failures
Comment on lines
+124
to
+173
| export function groupById(id: GroupId): GroupDef { | ||
| const def = GROUPS.find((g) => g.id === id); | ||
| if (!def) throw new Error(`Unknown group: ${id}`); | ||
| return def; | ||
| } | ||
|
|
||
| export function metricGroups(): MetricGroup[] { | ||
| return GROUPS.filter((g): g is MetricGroup => g.kind === "metrics"); | ||
| } | ||
|
|
||
| export function legacyGroups(): LegacyGroup[] { | ||
| return GROUPS.filter((g): g is LegacyGroup => g.kind === "legacy"); | ||
| } | ||
|
|
||
| /** | ||
| * The "At a glance" KPI row: array order is display order. `legacy` tiles | ||
| * come from the legacy KPI batch; `metric` tiles come from the derived | ||
| * KPI collection below. Both render through the same display-ready tile | ||
| * intermediate — selectors own formatting and scoring. | ||
| */ | ||
| export type KpiTileSource = | ||
| | { kind: "legacy"; key: string; groupId: GroupId } | ||
| | { kind: "metric"; metricKey: string }; | ||
|
|
||
| export const KPI_ROW: readonly KpiTileSource[] = [ | ||
| { kind: "legacy", key: "tasks_closed", groupId: "task_delivery" }, | ||
| { kind: "legacy", key: "focus_time_pct", groupId: "collaboration" }, | ||
| { kind: "legacy", key: "prs_merged", groupId: "git_output" }, | ||
| { kind: "metric", metricKey: "ai.active_days" }, | ||
| { kind: "metric", metricKey: "ai.accepted_lines" }, | ||
| ]; | ||
|
|
||
| export const KPI_ROW_COLLECTION: MetricCollectionConfig = { | ||
| metrics: KPI_ROW.filter( | ||
| (t): t is Extract<KpiTileSource, { kind: "metric" }> => | ||
| t.kind === "metric", | ||
| ).map((t) => ({ | ||
| key: t.metricKey, | ||
| views: [{ view: "period" }, { view: "peer" }], | ||
| })), | ||
| }; | ||
|
|
||
| /** Metrics-backed KPI tiles navigate to the group that owns their metric. */ | ||
| export function groupIdForMetricKey(metricKey: string): GroupId | null { | ||
| for (const def of GROUPS) { | ||
| if (def.kind !== "metrics") continue; | ||
| if (def.collection.metrics.some((m) => m.key === metricKey)) return def.id; | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Coverage gate is failing for this file — add tests for groupById, legacyGroups, and the not-found path in groupIdForMetricKey.
The CI coverage gate reports 66.7% (< 80% minimum) for this diff, with lines 125-127 (groupById), 135 (legacyGroups), and 172 (groupIdForMetricKey returning null) uncovered. This will block the PR from passing CI.
// Example additions
it("groupById throws for unknown ids", () => {
expect(() => groupById("nonexistent" as GroupId)).toThrow();
});
it("legacyGroups returns only legacy groups", () => {
expect(legacyGroups().every((g) => g.kind === "legacy")).toBe(true);
});
it("groupIdForMetricKey returns null for unknown metric keys", () => {
expect(groupIdForMetricKey("not.a.real.metric")).toBeNull();
});🧰 Tools
🪛 GitHub Actions: CI / Coverage summary & gate
[error] 125-172: diff-cover reported insufficient diff coverage (66.7% < 80%). Missing lines: 125-127, 135, 172.
🤖 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 `@src/lib/insight/groups.ts` around lines 124 - 173, Coverage is missing for
the new group helpers in groups.ts; add tests that exercise groupById’s
successful lookup and unknown-id throw path, legacyGroups returning only
legacy-kind entries, and groupIdForMetricKey returning null when no metric
matches. Use the existing exported symbols groupById, legacyGroups, and
groupIdForMetricKey in the test file so the uncovered branches in this diff are
hit and CI coverage passes.
Source: Pipeline failures
Comment on lines
+1
to
+9
| /** | ||
| * Person entity ids are emails, matched case-insensitively by the backend's | ||
| * `normalize_entity_id` (trim + lowercase). Normalizing on the client keeps | ||
| * query keys and response lookups stable regardless of the casing a route | ||
| * param or identity record carries. | ||
| */ | ||
| export function normalizePersonId(email: string): string { | ||
| return email.trim().toLowerCase(); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Missing test coverage is failing the CI gate.
Both coverage jobs report 0% diff coverage on this file (line 8), below the 80% minimum — this blocks the pipeline even though the function itself is correct.
✅ Suggested test
import { describe, expect, it } from "vitest";
import { normalizePersonId } from "`@/lib/metrics/entity`";
describe("normalizePersonId", () => {
it("trims and lowercases", () => {
expect(normalizePersonId(" Alice@Example.COM ")).toBe("alice@example.com");
});
});🧰 Tools
🪛 GitHub Actions: CI / 0_Coverage summary & gate.txt
[error] 1-1: diff-cover: Changed lines coverage 0.0% (min 80%). Missing lines: 8.
🪛 GitHub Actions: CI / Coverage summary & gate
[error] 8-8: diff-cover reported insufficient diff coverage (0.0% < 80%). Missing lines: 8.
🤖 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 `@src/lib/metrics/entity.ts` around lines 1 - 9, Add test coverage for
normalizePersonId in the corresponding metrics entity test suite to satisfy the
CI diff coverage gate. Create a focused Vitest case that imports
normalizePersonId from the entity module and verifies it trims whitespace and
lowercases the email input, matching the behavior of the normalizePersonId
function so line 8 is covered.
Source: Pipeline failures
Comment on lines
+62
to
+68
| function peerSpread(stats: PeerStats): number { | ||
| const iqr = Math.abs(stats.p75 - stats.p25); | ||
| if (iqr > 1e-9) return iqr; | ||
| const range = Math.abs(stats.max - stats.min); | ||
| if (range > 1e-9) return range; | ||
| return 1; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Coverage gate is failing on the severity/sort logic.
Diff coverage is 78.4% (below the 80% gate), driven by peerSpread's IQR/range/constant-fallback branches (lines 63-67) and the severity-sort comparators in partitionPeerStory (lines 149, 152, 157) being untested. These aren't just defensive code — they determine which metric is picked as the peer-story "hero" — so beyond unblocking CI, tests here would catch real ranking regressions (e.g. a collection with multiple bottom/top outliers, and a metric whose peer stats have p25===p75 to exercise the range/constant-1 fallback).
Also applies to: 143-179
🤖 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 `@src/lib/metrics/peer-story.ts` around lines 62 - 68, Add tests covering the
untested branching and sort behavior in peerSpread and partitionPeerStory:
exercise peerSpread’s IQR path, range fallback, and constant-1 fallback, and
verify the severity/sort comparators used to pick the “hero” metric in
partitionPeerStory across tied and outlier-heavy PeerStats cases. Use the
peerSpread helper and partitionPeerStory logic to locate the code, and assert
the chosen ordering/selection matches the intended ranking rules.
Source: Pipeline failures
Comment on lines
+44
to
+223
| function canonicalEntityIds(entity: MetricCollectionEntity): string[] { | ||
| const ids = | ||
| entity.type === "person" | ||
| ? entity.ids.map(normalizePersonId) | ||
| : entity.ids.map((id) => id.trim()); | ||
| return [...new Set(ids.filter(Boolean))].sort(); | ||
| } | ||
|
|
||
| function queryKeyFor( | ||
| entity: MetricCollectionEntity, | ||
| ids: string[], | ||
| range: DateRange, | ||
| metrics: MetricRequest[], | ||
| ) { | ||
| // The derived `metrics` array rides in the key, so key and payload are | ||
| // provably coherent — no hand-maintained collection identity to forget to | ||
| // bump. TanStack hashes the key structurally. | ||
| return [ | ||
| "metric-results", | ||
| entity.type, | ||
| ids, | ||
| range.from, | ||
| range.to, | ||
| metrics, | ||
| ] as const; | ||
| } | ||
|
|
||
| export function useMetricCollection( | ||
| collection: MetricCollectionConfig, | ||
| entity: MetricCollectionEntity, | ||
| range: DateRange, | ||
| options?: MetricCollectionOptions, | ||
| ): MetricCollectionResult { | ||
| const ids = canonicalEntityIds(entity); | ||
| const request = buildMetricCollectionRequest( | ||
| collection, | ||
| { type: entity.type, ids }, | ||
| range, | ||
| ); | ||
| const enabled = ids.length > 0 && Boolean(range.from && range.to); | ||
|
|
||
| const current = useQuery({ | ||
| queryKey: queryKeyFor(entity, ids, range, request.metrics), | ||
| queryFn: () => queryMetricResults(request), | ||
| enabled, | ||
| placeholderData: keepPreviousData, | ||
| }); | ||
|
|
||
| const previousRange = options?.previousPeriod | ||
| ? previousPeriodRange(range, options.previousPeriod) | ||
| : null; | ||
| const previousRequest = previousRange | ||
| ? buildMetricCollectionRequest( | ||
| collection, | ||
| { type: entity.type, ids }, | ||
| previousRange, | ||
| ) | ||
| : null; | ||
| const previous = useQuery({ | ||
| // Sentinel key when no previous period is requested: the disabled twin | ||
| // must never alias the current query's cache entry. | ||
| queryKey: previousRequest | ||
| ? queryKeyFor(entity, ids, previousRange ?? range, previousRequest.metrics) | ||
| : (["metric-results", "previous-disabled"] as const), | ||
| queryFn: () => queryMetricResults(previousRequest ?? request), | ||
| enabled: enabled && previousRequest !== null, | ||
| placeholderData: keepPreviousData, | ||
| }); | ||
|
|
||
| const hasPrevious = previousRequest !== null; | ||
| const byKey = useMemo( | ||
| () => normalizeMetricResults(current.data?.metrics), | ||
| [current.data], | ||
| ); | ||
| // Deltas pair two periods; a failed or stale twin must yield "no delta" | ||
| // rather than a silently mispaired one (placeholderData keeps the OLD | ||
| // period's data around during a period switch). | ||
| const previousUsable = | ||
| hasPrevious && | ||
| !previous.isError && | ||
| !previous.isPlaceholderData && | ||
| !current.isPlaceholderData; | ||
| const previousData = previousUsable ? previous.data : undefined; | ||
| const previousByKey = useMemo( | ||
| () => (previousData ? normalizeMetricResults(previousData.metrics) : null), | ||
| [previousData], | ||
| ); | ||
|
|
||
| return { | ||
| byKey, | ||
| previousByKey, | ||
| isPending: current.isPending && enabled, | ||
| isFetching: | ||
| current.isFetching || (hasPrevious && previous.isFetching), | ||
| isError: current.isError, | ||
| refetch: () => { | ||
| void current.refetch(); | ||
| if (hasPrevious) void previous.refetch(); | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| export interface KeyedCollection { | ||
| key: string; | ||
| collection: MetricCollectionConfig; | ||
| } | ||
|
|
||
| /** | ||
| * One query per collection for a dynamic list (e.g. every metrics-backed | ||
| * group in the registry) — `useQueries`, so the list length can change | ||
| * without violating hook rules. No previous-period twin here; only the KPI | ||
| * row compares periods. | ||
| */ | ||
| export function useMetricCollectionSet( | ||
| collections: readonly KeyedCollection[], | ||
| entity: MetricCollectionEntity, | ||
| range: DateRange, | ||
| ): Map<string, MetricCollectionResult> { | ||
| const ids = canonicalEntityIds(entity); | ||
| const enabled = ids.length > 0 && Boolean(range.from && range.to); | ||
|
|
||
| // Large rosters are chunked so a period+peer collection over N entities | ||
| // never exceeds the backend's all-or-nothing projected-row limit; chunk | ||
| // results merge back into one collection result per key. | ||
| const requests = collections.flatMap(({ key, collection }) => { | ||
| const chunkSize = entityChunkSize(collection); | ||
| const chunks = | ||
| chunkSize === null ? [ids] : chunkEntityIds(ids, chunkSize); | ||
| return chunks.map((chunkIds) => ({ | ||
| key, | ||
| request: buildMetricCollectionRequest( | ||
| collection, | ||
| { type: entity.type, ids: chunkIds }, | ||
| range, | ||
| ), | ||
| chunkIds, | ||
| })); | ||
| }); | ||
|
|
||
| const results = useQueries({ | ||
| queries: requests.map(({ request, chunkIds }) => ({ | ||
| queryKey: queryKeyFor(entity, chunkIds, range, request.metrics), | ||
| queryFn: () => queryMetricResults(request), | ||
| enabled, | ||
| placeholderData: keepPreviousData, | ||
| })), | ||
| }); | ||
|
|
||
| const out = new Map<string, MetricCollectionResult>(); | ||
| const chunkMaps = new Map<string, Array<Map<string, NormalizedMetricResult>>>(); | ||
| requests.forEach(({ key }, index) => { | ||
| const query = results[index]; | ||
| if (!query) return; | ||
| const maps = chunkMaps.get(key) ?? []; | ||
| maps.push(normalizeMetricResults(query.data?.metrics)); | ||
| chunkMaps.set(key, maps); | ||
| const existing = out.get(key); | ||
| const refetches = [query.refetch]; | ||
| out.set(key, { | ||
| byKey: new Map(), | ||
| previousByKey: null, | ||
| isPending: (existing?.isPending ?? false) || (query.isPending && enabled), | ||
| isFetching: (existing?.isFetching ?? false) || query.isFetching, | ||
| isError: (existing?.isError ?? false) || query.isError, | ||
| refetch: existing | ||
| ? () => { | ||
| existing.refetch(); | ||
| for (const r of refetches) void r(); | ||
| } | ||
| : () => { | ||
| for (const r of refetches) void r(); | ||
| }, | ||
| }); | ||
| }); | ||
| for (const [key, maps] of chunkMaps) { | ||
| const entry = out.get(key); | ||
| if (entry) entry.byKey = mergeNormalizedResults(maps); | ||
| } | ||
| return out; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
CI coverage gate is failing for this entire file.
Diff-cover reports 0% coverage on nearly all changed lines (44-223), well under the 80% minimum required by the coverage gate. This is a new file implementing non-trivial logic (previous-period pairing, placeholder-data gating, chunk merging) with no tests currently covering it, which is both a CI blocker and a real risk given the subtlety of the delta-pairing logic around lines 118-130.
Please add unit tests for useMetricCollection (current/previous pairing, placeholder/error gating) and useMetricCollectionSet (chunking + merge, multi-key refetch) to satisfy the coverage gate.
🧰 Tools
🪛 GitHub Actions: CI / Coverage summary & gate
[error] 46-222: diff-cover reported insufficient diff coverage (0.0% < 80%). Missing lines: 46, 48-49, 61, 77-78, 83, 85, 87, 92, 95, 102, 108, 113-114, 122, 126-128, 132, 140-141, 162-163, 168-169, 171-172, 183-184, 186, 192-202, 210-211, 214, 218-220, 222.
🤖 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 `@src/queries/metric-results.ts` around lines 44 - 223, The entire
metric-results query logic needs test coverage to satisfy the coverage gate,
especially the subtle pairing and chunk-merging behavior in useMetricCollection
and useMetricCollectionSet. Add unit tests that exercise
canonicalEntityIds/queryKeyFor indirectly through useMetricCollection, covering
current vs previous-period query pairing, placeholderData/error gating, and
refetch behavior. Also add tests for useMetricCollectionSet that verify
chunkEntityIds-driven splitting, mergeNormalizedResults output across chunks,
and multi-key refetch aggregation so the changed logic is executed in coverage.
Source: Pipeline failures
Add behavior tests for the previously-uncovered units the PR introduced: the metric-results client (ok / non-ok / invalid-json), the collection hooks (current+previous pairing, error gating, empty-entity skip, roster chunk split + merge + aggregated refetch), the group registry helpers, entity normalization, peer-story peerSpread fallbacks, collection merge one-sided branches, and render tests for peer-comparison, peer-story, metric-trend, metric-breakdown, the team drilldown/card, and the group drilldown sheet dispatch. Changed-line coverage clears the 80% gate. Also fold in two review fixes: compose dimension series keys with :/| so the raw string is unambiguous before hashing, and drop the single-element refetch array in the collection set hook. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
This was referenced Jul 21, 2026
This was referenced Jul 28, 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
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.
What
Moves metric rendering onto the unified
POST /v1/metric-resultsendpoint and lays down the frontend architecture every metric family will use. The AI adoption family (10ai.*metrics) is the first fully migrated family — KPI tiles, group card, needs-attention, and the personal/team drilldowns all render from self-describing responses. Legacy groups render unchanged through the existing path until their families migrate.Architecture
src/lib/insight/groups.ts) — the single place a metrics-backed group is declared: aMetricGroupholds its metric collection (keys × views), card preview keys, and drilldown chart blocks.LegacyGroupentries mark groups still on the old path; both render through one dispatch. Adding the next family's section is one registry entry.src/lib/metrics/,src/queries/metric-results.ts) —useMetricCollection/useMetricCollectionSetfetch with query keys derived from the same arguments as the request (key/payload coherence by construction), an optional previous-period twin for deltas, and automatic roster chunking so team-scale requests stay under the backend's all-or-nothing projected-row limit.src/components/widgets/metric-views/) — trend (line/stacked-bar, dimension-split, multi-metric-ready), breakdown composition, peer-comparison strip, peer story (outlier hero/side cards/chips/supporting fold), group cards, and the drilldown templates. All consume normalized results; none contain metric- or family-specific branches.Measurement honesty
Peer standings distinguish "measured zero" from "unmeasured": a null peer
target_valuemeans no observations, and such entities take no standing anywhere — no bottom-quartile branding on cards, stories, attention lists, or team cells; the supporting fold says "no recorded activity" alongside the cohort median. Server-suppressed thin-cohort percentiles render as "no peer data". Null timeseries points draw as gaps, never fabricated zeros. Failed or stale previous-period queries produce no delta rather than a mispaired one, and rounded-to-zero deltas show no badge.Robustness
IC_BULLET_DELIVERYbatch item executes once and feeds both the task-delivery and code-quality transforms.Removed
The legacy AI personal panel and its three query hooks, five per-vendor AI query registry entries, AI entries in the compiled-in ordering/description maps, the KPI section side-map (
kpi-defs.ts— ownership now derives from the registry), dead drilldown toggles and branches, and a dead component.ai_loc_share_pctandai_sessionsleave the KPI row until the git family and a sessions measure exist.Validation
tsc,eslint --max-warnings 0, and both vitest projects green: 198 tests across 32 files (unit + Storybook browser), including wire-contract fixtures mirroring the backend serde shape test, selector parity between legacy and unified paths, roster-chunking math, and regression tests for every review finding fixed here.Follow-ups
🤖 Generated with Claude Code
Summary by CodeRabbit