Skip to content

refactor(ic-dashboard): remove legacy metric fetches - #208

Merged
aleksdotbar merged 2 commits into
mainfrom
fix/ic-dashboard-drop-legacy-fetches
Jul 21, 2026
Merged

refactor(ic-dashboard): remove legacy metric fetches#208
aleksdotbar merged 2 commits into
mainfrom
fix/ic-dashboard-drop-legacy-fetches

Conversation

@aleksdotbar

@aleksdotbar aleksdotbar commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

What

EngineeringDashboardV2 no longer issues the legacy /metrics/queries or /catalog/get_metrics requests. Every metric family is now served by the unified /v1/metric-results path, so the legacy batch and catalog results were fetched and immediately discarded — dead over-fetch on the IC dashboard load.

Changes

  • Drop useIcDashboardData / useCatalog from the v2 dashboard and the scaffolding they fed (LEGACY_GROUP_FEEDS, legacyRowsByGroup, legacyKpiTiles, legacyKpiLabels, legacyAttentionItems).
  • Sections and drilldowns iterate metricGroups(); the KPI row and the attention surface are unified-only.
  • Region-owned loading — KPI tiles, group cards, and the attention section each own their loading / error / empty states. Removed the page-level spinner, the total-error page, the empty-state, and the fetch-driven opacity dim.
  • Delete the now-orphaned helpers legacyKpiTiles, kpiRowTiles, legacyAttentionItems and their tests.

Shared LegacyGroup / KpiTileSource types and the useIcDashboardData / useCatalog hooks are kept — the v1 dashboard and the team view still consume them.

Summary by CodeRabbit

  • Updates
    • Simplified the Engineering Dashboard to use metric-based KPI tiles, attention items, and sections.
    • Improved loading, error, retry, and no-data states for KPI tiles.
    • Sections with no available results are now omitted.
    • Group cards open the relevant metric drilldown, while changing the selected person closes open drilldowns.
  • Bug Fixes
    • Removed legacy dashboard displays and inconsistent legacy error or empty-state handling.

@aleksdotbar
aleksdotbar requested a review from a team as a code owner July 21, 2026 14:14
@aleksdotbar aleksdotbar changed the title refactor(ic-dashboard): remove vestigial legacy metric fetches refactor(ic-dashboard): remove legacy metric fetches Jul 21, 2026
@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The dashboard now uses metric-collection data exclusively for KPI tiles, attention items, metric groups, and drilldowns. Legacy KPI and attention builders are removed, and new tests cover metric-driven rendering, loading/error states, omitted groups, retries, and drilldown behavior.

Changes

Metric dashboard migration

Layer / File(s) Summary
Metric insight helpers
src/lib/insight/attention.ts, src/lib/insight/attention.test.ts, src/lib/insight/kpi-row.ts, src/lib/insight/kpi-row.test.ts
Metric-derived attention items and KPI tiles remain as the active construction paths, while legacy builders, merging logic, and legacy attention tests are removed.
Metric-only dashboard flow
src/screens/ic-dashboard/engineering-dashboard-v2.tsx
EngineeringDashboardV2 derives dashboard content from metric collections, renders metric KPI states and groups unconditionally, and limits drilldowns to metric groups.
Dashboard behavior coverage
src/screens/ic-dashboard/engineering-dashboard-v2.test.tsx
Tests cover baseline rendering, KPI error/loading/placeholder states, retry behavior, omitted groups, drilldown opening, and closing drilldowns when personId changes.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: refactoring the IC dashboard to remove legacy metric fetches.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/ic-dashboard-drop-legacy-fetches

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

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

With every metric family on the unified /v1/metric-results path, the /metrics/queries and /catalog/get_metrics fetches were dead over-fetch.

Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
@aleksdotbar
aleksdotbar force-pushed the fix/ic-dashboard-drop-legacy-fetches branch from 9e0ada9 to 5c3cee3 Compare July 21, 2026 15:26

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/screens/ic-dashboard/engineering-dashboard-v2.test.tsx (1)

42-47: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Assert the lazy drilldown query contract.

These tests only verify the sheet’s open flag. Capture useMetricCollection calls and assert that closed sheets use the disabled target, opening selects the clicked group’s full collection, and changing people disables that query again.

Also applies to: 193-220

🤖 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/screens/ic-dashboard/engineering-dashboard-v2.test.tsx` around lines 42 -
47, Extend the tests around the mocked useMetricCollection and sheet
interactions to capture each query call and assert the lazy drilldown contract:
closed sheets pass the disabled target, opening a sheet queries the clicked
group’s full collection, and changing people disables the query again. Update
the related cases around the sheet assertions while preserving the existing
open-flag checks.
🤖 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.

Nitpick comments:
In `@src/screens/ic-dashboard/engineering-dashboard-v2.test.tsx`:
- Around line 42-47: Extend the tests around the mocked useMetricCollection and
sheet interactions to capture each query call and assert the lazy drilldown
contract: closed sheets pass the disabled target, opening a sheet queries the
clicked group’s full collection, and changing people disables the query again.
Update the related cases around the sheet assertions while preserving the
existing open-flag checks.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2d7b4a50-9d8b-45e2-9d02-ded61655ba7b

📥 Commits

Reviewing files that changed from the base of the PR and between 25091b6 and 5c3cee3.

📒 Files selected for processing (6)
  • src/lib/insight/attention.test.ts
  • src/lib/insight/attention.ts
  • src/lib/insight/kpi-row.test.ts
  • src/lib/insight/kpi-row.ts
  • src/screens/ic-dashboard/engineering-dashboard-v2.test.tsx
  • src/screens/ic-dashboard/engineering-dashboard-v2.tsx

@aleksdotbar
aleksdotbar merged commit 035eeef into main Jul 21, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants