feat(team): rebuild team view on the unified metric-results grid - #211
Merged
Conversation
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
…trailing list Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
…members grid Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
… spinner Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
The grid took the member name from the analytics row (BambooHR) while the sidebar and header use the identity-service name (Entra); for a person whose transliterations differ the same person_id showed two spellings. The roster is the identity source, so the grid now renders the identity name it already holds and analytics supplies only the metric values. Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
📝 WalkthroughWalkthroughThis PR migrates V2 team views to metric-backed member grids, adds shared loading, timeseries, formatting, and preview-card components, updates metric contracts and queries, and removes legacy heatmap, department-distribution, and per-tile loading implementations. ChangesMetric-backed member dashboard
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant TeamViewV2
participant useMemberGridData
participant MetricCollectionQueries
participant MembersOverview
TeamViewV2->>useMemberGridData: request current and previous member metrics
useMemberGridData->>MetricCollectionQueries: fetch period, peer, and previous-period views
MetricCollectionQueries-->>useMemberGridData: return keyed metric maps and query state
useMemberGridData-->>TeamViewV2: provide grid data and loading/error state
TeamViewV2->>MembersOverview: render members with metric maps
MembersOverview-->>TeamViewV2: render desktop grid or mobile triage list
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 |
… hook Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
This was
linked to
issues
Jul 22, 2026
Team grid shows a different name than the sidebar for the same person
constructorfabric/insight#1837
Closed
…he bar 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: 2
🧹 Nitpick comments (2)
src/components/widgets/metric-views/metric-timeseries-chrome.tsx (1)
58-78: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueClean extraction; export failures fail silently.
exportXlsxresetsisExportinginfinallybut has nocatch, so a rejection fromdownloadMetricTimeseriesXlsxbecomes an unhandled promise rejection with no user feedback beyond the spinner disappearing. Low risk given the export path is mostly local blob generation, so not blocking.Also applies to: 100-104
🤖 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-timeseries-chrome.tsx` around lines 58 - 78, Update TimeseriesExportMenu’s exportXlsx to catch downloadMetricTimeseriesXlsx failures, provide user-facing feedback using the component’s established notification/error mechanism, and retain the existing isExporting reset in finally.src/components/widgets/metric-views/metric-timeseries-csv.ts (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a shared
hasGrandTotal(model)predicate instead of triplicating the condition.The same
model.dimensions.length > 0 && model.grandTotals.some((value) => value != null)check now lives in three separate export/render paths. Since this PR already had to update all three in lockstep, a shared helper (e.g. exported frommetric-timeseries-model.ts) would prevent CSV/XLSX/table exports from silently drifting apart on a future change.
src/components/widgets/metric-views/metric-timeseries-csv.ts#L79-82: replace the inline condition with a call to the sharedhasGrandTotal(model)helper.src/components/widgets/metric-views/metric-timeseries-xlsx.ts#L197-200: replace the inlineif (...)condition withhasGrandTotal(model).src/components/widgets/metric-views/metric-timeseries-table.tsx#L193-194: replace the inline ternary condition withhasGrandTotal(model).♻️ Proposed helper
+export function hasGrandTotal(model: MetricTimeseriesModel): boolean { + return ( + model.dimensions.length > 0 && + model.grandTotals.some((value) => value != null) + ); +}🤖 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-timeseries-csv.ts` at line 1, Extract the repeated grand-total check into a shared hasGrandTotal(model) helper, preferably in metric-timeseries-model.ts, preserving the existing dimensions and non-null grandTotals logic. Update the CSV export, XLSX export, and table rendering paths to call this helper instead of duplicating the inline condition.
🤖 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/components/widgets/v2/members-grid.tsx`:
- Around line 206-210: Initialize the sort state in the members grid from
hasIssuesFacet: use "issues" when the issues facet is visible and "name" when it
is hidden. Preserve the existing reversed value and sorting behavior so the
visible Name control accurately reflects the initial ordering.
In `@src/queries/v2/member-grid.ts`:
- Around line 75-77: Update the member-grid query status mapping to include the
previous-period result: in the object containing isPending, isFetching, and
isError, combine cur and prev so pending remains true while either query is
pending and errors remain visible when either query fails. Preserve the existing
isFetching behavior and false defaults when neither result reports the status.
---
Nitpick comments:
In `@src/components/widgets/metric-views/metric-timeseries-chrome.tsx`:
- Around line 58-78: Update TimeseriesExportMenu’s exportXlsx to catch
downloadMetricTimeseriesXlsx failures, provide user-facing feedback using the
component’s established notification/error mechanism, and retain the existing
isExporting reset in finally.
In `@src/components/widgets/metric-views/metric-timeseries-csv.ts`:
- Line 1: Extract the repeated grand-total check into a shared
hasGrandTotal(model) helper, preferably in metric-timeseries-model.ts,
preserving the existing dimensions and non-null grandTotals logic. Update the
CSV export, XLSX export, and table rendering paths to call this helper instead
of duplicating the inline condition.
🪄 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: 726280f0-da3b-4f77-bf7a-e6e6d546cd02
📒 Files selected for processing (58)
src/api/metric-registry.tssrc/api/metric-results-client.tssrc/components/ui/preview-card.tsxsrc/components/widgets/centered-spinner.tsxsrc/components/widgets/metric-views/metric-timeseries-chrome.tsxsrc/components/widgets/metric-views/metric-timeseries-csv.tssrc/components/widgets/metric-views/metric-timeseries-model.tssrc/components/widgets/metric-views/metric-timeseries-presentation.tssrc/components/widgets/metric-views/metric-timeseries-table.tsxsrc/components/widgets/metric-views/metric-timeseries-view.tsxsrc/components/widgets/metric-views/metric-timeseries-xlsx.tssrc/components/widgets/metric-views/peer-comparison.tsxsrc/components/widgets/metric-views/team-collection-drilldown.test.tsxsrc/components/widgets/metric-views/team-collection-drilldown.tsxsrc/components/widgets/metric-views/team-metric-group-card.test.tsxsrc/components/widgets/metric-views/team-metric-group-card.tsxsrc/components/widgets/v2/group-drilldown-sheet.test.tsxsrc/components/widgets/v2/group-drilldown-sheet.tsxsrc/components/widgets/v2/kpi-tile.test.tsxsrc/components/widgets/v2/kpi-tile.tsxsrc/components/widgets/v2/members-grid.test.tsxsrc/components/widgets/v2/members-grid.tsxsrc/components/widgets/v2/members-heatmap/index.test.tsxsrc/components/widgets/v2/members-heatmap/index.tsxsrc/components/widgets/v2/members-heatmap/member-details-sheet.tsxsrc/components/widgets/v2/members-heatmap/triage-list.tsxsrc/components/widgets/v2/members-overview.test.tsxsrc/components/widgets/v2/members-overview.tsxsrc/components/widgets/v2/team-members-attention.test.tsxsrc/components/widgets/v2/team-members-attention.tsxsrc/components/widgets/v2/triage-list.test.tsxsrc/components/widgets/v2/triage-list.tsxsrc/lib/format.tssrc/lib/insight/groups.tssrc/lib/insight/kpi-row.tssrc/lib/insight/team-metrics.tssrc/lib/insight/v2/team-member-status.test.tssrc/lib/insight/v2/team-member-status.tssrc/lib/metrics/collection.tssrc/lib/metrics/delta.test.tssrc/lib/metrics/delta.tssrc/lib/metrics/gap.tssrc/lib/metrics/peer-story.tssrc/lib/peers.tssrc/mocks/factories.tssrc/mocks/handlers.tssrc/queries/ic-dashboard.tssrc/queries/metric-results.test.tsxsrc/queries/metric-results.tssrc/queries/team-view.tssrc/queries/v2/member-grid.test.tsxsrc/queries/v2/member-grid.tssrc/queries/v2/team-extras.tssrc/routes/__root.tsxsrc/screens/ic-dashboard/engineering-dashboard-v2.test.tsxsrc/screens/ic-dashboard/engineering-dashboard-v2.tsxsrc/screens/team-view-v2.test.tsxsrc/screens/team-view-v2.tsx
💤 Files with no reviewable changes (12)
- src/lib/insight/v2/team-member-status.test.ts
- src/components/widgets/v2/members-heatmap/index.test.tsx
- src/components/widgets/v2/members-heatmap/index.tsx
- src/lib/insight/v2/team-member-status.ts
- src/queries/v2/team-extras.ts
- src/components/widgets/v2/members-heatmap/triage-list.tsx
- src/components/widgets/v2/kpi-tile.test.tsx
- src/components/widgets/v2/kpi-tile.tsx
- src/components/widgets/v2/members-heatmap/member-details-sheet.tsx
- src/mocks/factories.ts
- src/lib/peers.ts
- src/mocks/handlers.ts
Comment on lines
+206
to
+210
| const hasIssuesFacet = showIssues ?? countsByMember != null; | ||
| const [sort, setSort] = useState<SortState>({ | ||
| key: "issues", | ||
| reversed: false, | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Default the initial sort to name when the issues facet is hidden.
When hasIssuesFacet is false, only the Name toggle renders on the Member header — the "Most behind" control is gone. But the initial sort key is still "issues", so rows load ordered by counts.bottom with no active-sort indicator and no way for the user to return to that ordering once they click Name. Seed the state from hasIssuesFacet so the visible control matches the actual order.
Proposed fix
const [sort, setSort] = useState<SortState>({
- key: "issues",
+ key: hasIssuesFacet ? "issues" : "name",
reversed: false,
});📝 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
| const hasIssuesFacet = showIssues ?? countsByMember != null; | |
| const [sort, setSort] = useState<SortState>({ | |
| key: "issues", | |
| reversed: false, | |
| }); | |
| const hasIssuesFacet = showIssues ?? countsByMember != null; | |
| const [sort, setSort] = useState<SortState>({ | |
| key: hasIssuesFacet ? "issues" : "name", | |
| reversed: false, | |
| }); |
🤖 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/v2/members-grid.tsx` around lines 206 - 210,
Initialize the sort state in the members grid from hasIssuesFacet: use "issues"
when the issues facet is visible and "name" when it is hidden. Preserve the
existing reversed value and sorting behavior so the visible Name control
accurately reflects the initial ordering.
Comment on lines
+75
to
+77
| isPending: cur?.isPending ?? false, | ||
| isFetching: (cur?.isFetching ?? false) || (prev?.isFetching ?? false), | ||
| isError: cur?.isError ?? false, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Wait for the previous-period query too.
isPending and isError ignore prev, so the grid can paint without trends or hide a previous-period failure when only the current request has settled.
Proposed fix
- isPending: cur?.isPending ?? false,
+ isPending: (cur?.isPending ?? false) || (prev?.isPending ?? false),
isFetching: (cur?.isFetching ?? false) || (prev?.isFetching ?? false),
- isError: cur?.isError ?? false,
+ isError: (cur?.isError ?? false) || (prev?.isError ?? false),📝 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
| isPending: cur?.isPending ?? false, | |
| isFetching: (cur?.isFetching ?? false) || (prev?.isFetching ?? false), | |
| isError: cur?.isError ?? false, | |
| isPending: (cur?.isPending ?? false) || (prev?.isPending ?? false), | |
| isFetching: (cur?.isFetching ?? false) || (prev?.isFetching ?? false), | |
| isError: (cur?.isError ?? false) || (prev?.isError ?? false), |
🤖 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/v2/member-grid.ts` around lines 75 - 77, Update the member-grid
query status mapping to include the previous-period result: in the object
containing isPending, isFetching, and isError, combine cur and prev so pending
remains true while either query is pending and errors remain visible when either
query fails. Preserve the existing isFetching behavior and false defaults when
neither result reports the status.
This was referenced Jul 28, 2026
This was referenced Aug 3, 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.
Summary
Rebuilds the team dashboard on the unified
/v1/metric-resultspath andunifies its surfaces around one semantic members grid. Also reworks dashboard
loading into a single page spinner and fixes a cross-surface name mismatch.
Team view
<table>(rows = members linking to their ICview, columns = metrics) replaces the old heatmap widget and the group
drilldown body. Each cell shows the member's own value, trend vs the previous
period, and colour from their standing vs their own department cohort.
One component, two call sites: the dashboard (fixed cross-family columns) and
the group drilldown (the group's full collection).
table-fixedwith aper-column min-width, so wide groups scroll instead of crushing headers.
attention list — "N behind / N ahead / on par" (behind wins), derived from
the shared
derivePeerStanding+ section scoring rather than per-surfacelogic.
the roster's standing in the shared vocabulary, "no peer data" when nobody is
rankable (rows never silently drop).
PreviewCard(hover-only, no click-pin):value, gap-vs-median magnitude, trend, standing.
short_label(full label in thetooltip); drilldown drops the peer view and computes a group-local standing
facet from its own cells.
Formatting
threshold as the vs-median gap, via a shared
formatMultiple— fixes absurd"+5460%" readouts on tiles and cells.
"department").
Loading
their queries load; a period change re-trips the same gate. No per-widget
spinners or partial paints.
pendingComponent), so thesidebar org tree, headers, and bottom identity mount complete — nothing pops
in. A subordinate's identity seeds from the cached viewer tree for an instant
named header; headers never fall back to a raw email.
Fixes
person header) instead of the analytics
display_name, so a person whosedirectory and HR name are different transliterations no longer appears under
two spellings on one screen.
Depends on
short_labeladdition (constructorfabric/insight) for the compactheaders; degrades to full labels without it.
Testing
tsc -b,pnpm lint,pnpm test— 321 passedSummary by CodeRabbit
New Features
Bug Fixes