Skip to content

ui(dashboard): table layout, after-hours fix, trend improvements#317

Merged
sylvainkalache merged 17 commits intomainfrom
testing
Feb 6, 2026
Merged

ui(dashboard): table layout, after-hours fix, trend improvements#317
sylvainkalache merged 17 commits intomainfrom
testing

Conversation

@sylvainkalache
Copy link
Copy Markdown
Contributor

@sylvainkalache sylvainkalache commented Feb 5, 2026

Summary

  • Table layout for team members — replaced card tiles with a scannable table view (columns: Member, Risk Level, Trend, Incidents, Status, Data Sources)
  • After-hours calculation fix — now based only on voluntary activity (GitHub commits + Slack messages), excluding incidents since being paged is not a choice
  • Removed fake 15% after-hours estimate — GitHub collector no longer guesses when detailed commit data is unavailable, reports 0 instead
  • Trend calculation fix — low scores (< 10) no longer produce misleading percentages (e.g. 0→3 was showing "Critical 3143%")
  • Stronger progress bar colors — bumped from Tailwind 300 to 400 for better readability
  • Double arrows for extreme trends — Critical and Improving Fast use two TrendingUp/Down icons
  • Landing page — "GitHub" button renamed to "View on GitHub"
  • Tests — added 5 new tests for voluntary-only after-hours and fallback behavior, fixed after-hours threshold in existing test

Files changed

  • frontend/src/components/dashboard/TeamMembersList.tsx — table layout, trend fixes, colors
  • frontend/src/components/landing-page.tsx — button text
  • backend/app/services/unified_burnout_analyzer.py — after-hours uses GitHub + Slack only
  • backend/app/services/github_collector.py — removed 15% fallback estimate
  • backend/tests/test_incident_metrics.py — new and fixed tests

Test plan

  • Verify table layout renders correctly with all columns
  • Verify clicking a row opens member detail panel
  • Verify sort by Risk Level and Trend still works
  • Verify trend badges show correct labels and double arrows for Critical/Improving Fast
  • Verify after-hours percentage no longer includes incident timestamps
  • Run backend tests: pytest tests/test_incident_metrics.py (57 pass)

- Fix trend sorting bug: use ?? instead of || for trendOrder lookup
  (0 is a valid order value for significantly_worsening)
- Swap Trend to left, Risk Level to right in card layout
- Add sort toggle (Risk Level / Trend) in header
- Add info icons with tooltips explaining metrics
- Show "Score: X/100" instead of incident count under Risk Level
- Use softer progress bar colors (300-level) to match badge opacity
- Change "Needs Attention" label to "Critical" for clarity
- Remove unused variables (isGithubEnabled, isSlackEnabled, isLinearEnabled)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented Feb 5, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
rootly-burnout-detector-dev Ready Ready Preview, Comment Feb 6, 2026 10:27pm
rootly-burnout-detector-web Ready Ready Preview, Comment Feb 6, 2026 10:27pm

Request Review

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Feb 5, 2026

Greptile Overview

Greptile Summary

This PR enhances the Team Members List UI with several improvements:

  • Sort toggle added: Users can now sort by Risk Level or Trend (30-day change)
  • Card layout improved: Trend section moved to left, Risk Level to right for better visual hierarchy
  • UI enhancements: Added info icon tooltips explaining metrics, changed "Needs Attention" to "Critical" for clarity
  • Visual polish: Softer progress bar colors (300-level) matching badge opacity, displays "Score: X/100" instead of incident count
  • Code cleanup: Removed unused isGithubEnabled, isSlackEnabled, isLinearEnabled variables and getOCHRiskLevel function
  • Proactive bug prevention: Uses ?? (nullish coalescing) instead of || (logical OR) in trend sorting to handle the valid 0 value for significantly_worsening trend order

The changes are well-structured, maintain existing functionality while adding new features, and improve the user experience with clearer information presentation.

Confidence Score: 5/5

  • This PR is safe to merge with no identified issues
  • The changes are purely additive UI enhancements with no breaking changes. The sort functionality is implemented correctly with proper nullish coalescing operator usage, the layout changes are cosmetic, and the code cleanup removes genuinely unused variables without affecting functionality.
  • No files require special attention

Important Files Changed

Filename Overview
frontend/src/components/dashboard/TeamMembersList.tsx Adds sort toggle (Risk/Trend), swaps card layout (Trend left, Risk right), improves UI with info tooltips, softer progress colors, and fixes removed unused variables

Keep softer progress bar colors (300-level) from testing branch.
Remove unused getOCHRiskLevel function.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@railway-app railway-app Bot temporarily deployed to Oncall Health / sylvain-testing February 5, 2026 21:21 Inactive
@railway-app railway-app Bot temporarily deployed to Oncall Health / testing February 5, 2026 21:21 Inactive
@claude
Copy link
Copy Markdown

claude Bot commented Feb 5, 2026

Claude finished @sylvainkalache's task —— View job


PR Review Analysis

Todo List:

  • Read repository CLAUDE.md for guidelines
  • Read and analyze the modified TeamMembersList.tsx file
  • Review code for bugs, logic errors, and security issues
  • Update GitHub comment with review findings

Issues Found:

Potential Logic Error in Trend Calculation

File: frontend/src/components/dashboard/TeamMembersList.tsx:82-83

const baseline = firstWeeksAvg || 0.1 // Avoid division by zero
const change = ((lastWeeksAvg - firstWeeksAvg) / baseline) * 100

Issue: When firstWeeksAvg is exactly 0, the code uses 0.1 as baseline, but this creates misleading percentage calculations. If the first period had 0 score and last period has any positive value, this will show artificial improvement percentages.

Fix: Use proper handling for zero baseline cases or document the expected behavior.

Missing Error Handling

File: frontend/src/components/dashboard/TeamMembersList.tsx:206

const trendInfo = calculateUserTrend(member.user_email, individualDailyData)

Issue: No error handling if calculateUserTrend throws an exception due to malformed data in individualDailyData.

Fix: Add try-catch around trend calculation or validate input data.

Type Safety Issue

File: frontend/src/components/dashboard/TeamMembersList.tsx:461

const trendComparison = (trendOrder[trendA.trend] ?? 2) - (trendOrder[trendB.trend] ?? 2);

Issue: Fallback value of 2 corresponds to 'stable' but there's no guarantee this is the intended behavior if trend calculation fails or returns an invalid trend value.

Fix: Use explicit handling for invalid trend values rather than silent fallback.


The Jira status endpoint was calling _validate_jira() directly,
bypassing the existing Redis-backed validation cache (15-min TTL).
This caused a ~1.2s HTTP call to Jira API on every request.

Now uses validate_all_integrations(use_cache=True) which checks the
cache first, eliminating the external API call for ~90% of requests.
The check_permissions() method was making 4 sequential HTTP calls
(users, incidents, services, oncalls), each waiting for the previous
to complete. Now uses asyncio.gather() to run all 4 in parallel.

Expected improvement: ~2.3s avg → ~0.6-0.8s (limited by slowest call).
Adds indexes identified by New Relic analysis of the 5 slowest endpoints:

- user_burnout_reports(email, submitted_at): analyses endpoint survey query
- jira_integrations(user_id): /jira/status lookup
- jira_workspace_mappings(jira_cloud_id, organization_id): workspace lookup
- slack_workspace_mappings(organization_id, status): /slack/status org query
- slack_workspace_mappings(owner_user_id, status): /slack/status owner query

Uses CREATE INDEX CONCURRENTLY to avoid locking tables during migration.
Model files updated to match for new deployments.
Two optimizations in the get_analysis endpoint:

1. Select only email column from UserCorrelation instead of loading
   full objects (was loading all columns for 10K+ members when only
   email is needed)

2. Use load_only(id, uuid) on the 404 fallback query to avoid loading
   the potentially 30MB+ results JSON column just for an error message
The get_notifications_with_counts query was lazy-loading the
organization relationship for each notification's to_dict() call
(self.organization.name), causing 20 extra queries per page.

Adding joinedload(UserNotification.organization) fetches all
organization data in a single JOIN, eliminating the N+1 pattern.
@railway-app railway-app Bot temporarily deployed to Oncall Health / sylvain-testing February 6, 2026 01:59 Inactive
@railway-app railway-app Bot temporarily deployed to Oncall Health / testing February 6, 2026 01:59 Inactive
@claude
Copy link
Copy Markdown

claude Bot commented Feb 6, 2026

Claude finished @sylvainkalache's task —— View job


PR Review Analysis

Todo List:

  • Read repository CLAUDE.md for guidelines
  • Read and analyze the modified TeamMembersList.tsx file
  • Review code for bugs, logic errors, and security issues
  • Update GitHub comment with review findings

Issues Found:

Potential Division by Zero Edge Case

File: frontend/src/components/dashboard/TeamMembersList.tsx:82

const baseline = firstWeeksAvg || 0.1 // Avoid division by zero

Issue: When firstWeeksAvg is exactly 0, the fallback to 0.1 creates misleading percentage calculations. If someone had 0 score initially and any positive score later, this shows artificial percentage changes.

Missing Error Handling for Trend Calculation

File: frontend/src/components/dashboard/TeamMembersList.tsx:206

const trendInfo = calculateUserTrend(member.user_email, individualDailyData)

Issue: No error handling if calculateUserTrend throws due to malformed individualDailyData or invalid user email formats.

Fallback Value Logic Inconsistency

File: frontend/src/components/dashboard/TeamMembersList.tsx:461

const trendComparison = (trendOrder[trendA.trend] ?? 2) - (trendOrder[trendB.trend] ?? 2);

Issue: Fallback value 2 corresponds to 'stable' trend, but this may not be appropriate for invalid trend calculations. Could mask real issues with trend computation.


@railway-app railway-app Bot temporarily deployed to Oncall Health / sylvain-testing February 6, 2026 18:13 Inactive
@railway-app railway-app Bot temporarily deployed to Oncall Health / testing February 6, 2026 18:13 Inactive
@claude
Copy link
Copy Markdown

claude Bot commented Feb 6, 2026

Claude finished @sylvainkalache's task —— View job


PR Review Analysis

Issues Found:

Division by Zero Handling Issue

File: frontend/src/components/dashboard/TeamMembersList.tsx:82

const baseline = firstWeeksAvg || 0.1 // Avoid division by zero

Issue: Using || operator means that when firstWeeksAvg is exactly 0 (a valid value), the code uses 0.1 as baseline. This creates misleading percentage calculations where someone with a 0 score initially and any positive score later shows artificial percentage changes.

Fix: Use proper zero handling:

const baseline = firstWeeksAvg === 0 ? 0.1 : firstWeeksAvg || 0.1

Missing Error Handling

File: frontend/src/components/dashboard/TeamMembersList.tsx:206

const trendInfo = calculateUserTrend(member.user_email, individualDailyData)

Issue: No error handling if calculateUserTrend throws due to malformed individualDailyData or invalid user email formats.

Fix: Add try-catch wrapper:

try {
  const trendInfo = calculateUserTrend(member.user_email, individualDailyData)
} catch (error) {
  const trendInfo = { trend: 'stable', percentage: 0, firstHalfScore: 0, secondHalfScore: 0 }
}

Fallback Value Inconsistency

File: frontend/src/components/dashboard/TeamMembersList.tsx:463

const trendComparison = (trendOrder[trendA.trend] ?? 2) - (trendOrder[trendB.trend] ?? 2);

Issue: Fallback value 2 corresponds to 'stable' trend, but this may not be appropriate for invalid trend calculations and could mask real issues with trend computation.

Fix: Use explicit validation or log when fallback is used.


…s calculation

- Replace tile grid with table view for team member risk levels
  (columns: Member, Risk Level, Trend, Incidents, Status, Data Sources)
- Fix trend calculation for low scores to avoid misleading percentages
  (e.g. 0→3 no longer shows "Critical 3143%")
- Use stronger bar colors (Tailwind 400 instead of 300)
- After-hours activity now based only on voluntary activity (GitHub + Slack),
  excluding incidents since being paged is not a choice
- Remove fake 15% after-hours fallback estimate in GitHub collector
- Update landing page button text to "View on GitHub"
- Add tests for voluntary-only after-hours and fallback behavior
- Fix test_after_hours_detection threshold to match code (>= 17 not >= 18)
@railway-app railway-app Bot temporarily deployed to Oncall Health / testing February 6, 2026 21:45 Inactive
@railway-app railway-app Bot temporarily deployed to Oncall Health / sylvain-testing February 6, 2026 21:45 Inactive
@claude
Copy link
Copy Markdown

claude Bot commented Feb 6, 2026

Claude finished @sylvainkalache's task —— View job


PR Review Analysis

Todo List:

  • Read repository CLAUDE.md for guidelines
  • Read and analyze the modified TeamMembersList.tsx file
  • Review code for bugs, logic errors, and security issues
  • Update GitHub comment with review findings

Issues Found:

Logic Error in Trend Classification

File: frontend/src/components/dashboard/TeamMembersList.tsx:101-102

else if (absDiff >= 5) trend = 'worsening'
else if (absDiff >= 2) trend = 'worsening'

Issue: Both conditions assign the same 'worsening' trend. The second condition will never be reached because absDiff >= 2 is a subset of absDiff >= 5. This appears to be missing the 'significantly_worsening' classification.

Fix: The second line should likely assign 'significantly_worsening':

else if (absDiff >= 5) trend = 'significantly_worsening'
else if (absDiff >= 2) trend = 'worsening'

Potential Precision Issue

File: frontend/src/components/dashboard/TeamMembersList.tsx:92

const baseline = firstWeeksAvg || 1

Issue: Uses logical OR (||) which treats 0 as falsy. If firstWeeksAvg is exactly 0, this uses 1 as baseline, creating misleading percentage calculations.

Fix: Use nullish coalescing for proper zero handling:

const baseline = firstWeeksAvg ?? 1

@railway-app railway-app Bot temporarily deployed to Oncall Health / sylvain-testing February 6, 2026 22:16 Inactive
@railway-app railway-app Bot temporarily deployed to Oncall Health / testing February 6, 2026 22:16 Inactive
@claude
Copy link
Copy Markdown

claude Bot commented Feb 6, 2026

Claude finished @sylvainkalache's task —— View job


PR Review Analysis

Todo List:

  • Read repository CLAUDE.md for guidelines
  • Read and analyze the modified TeamMembersList.tsx file
  • Review code for bugs, logic errors, and security issues
  • Update GitHub comment with review findings

Issues Found:

Logic Error in Trend Classification

File: frontend/src/components/dashboard/TeamMembersList.tsx:101-102

else if (absDiff >= 5) trend = 'worsening'
else if (absDiff >= 2) trend = 'worsening'

Issue: Both conditions assign the same 'worsening' trend. The second condition will never be reached because absDiff >= 2 is a subset of absDiff >= 5. This appears to be missing the 'significantly_worsening' classification.

Fix: The first line should assign 'significantly_worsening':

else if (absDiff >= 5) trend = 'significantly_worsening'
else if (absDiff >= 2) trend = 'worsening'

Potential Division by Zero Edge Case

File: frontend/src/components/dashboard/TeamMembersList.tsx:92

const baseline = firstWeeksAvg || 1

Issue: Using logical OR (||) treats 0 as falsy. If firstWeeksAvg is exactly 0, this uses 1 as baseline, creating misleading percentage calculations for users who had zero score initially.

Fix: Use proper zero handling:

const baseline = firstWeeksAvg === 0 ? 1 : firstWeeksAvg || 1

@sylvainkalache sylvainkalache changed the title ui(cards): improve trend sorting and card layout ui(dashboard): table layout, after-hours fix, trend improvements Feb 6, 2026
@railway-app railway-app Bot temporarily deployed to Oncall Health / sylvain-testing February 6, 2026 22:26 Inactive
@railway-app railway-app Bot temporarily deployed to Oncall Health / testing February 6, 2026 22:26 Inactive
@claude
Copy link
Copy Markdown

claude Bot commented Feb 6, 2026

Claude finished @sylvainkalache's task —— View job


PR Review Analysis

Todo List:

  • Read repository CLAUDE.md for guidelines
  • Read and analyze the modified TeamMembersList.tsx file
  • Review code for bugs, logic errors, and security issues
  • Update GitHub comment with review findings

Issues Found:

Logic Error in Trend Classification

File: frontend/src/components/dashboard/TeamMembersList.tsx:101-102

else if (absDiff >= 5) trend = 'significantly_worsening'
else if (absDiff >= 2) trend = 'worsening'

Issue: Both conditions assign 'significantly_worsening' and 'worsening' trends, but the logic is backwards. The more severe condition (≥5) should come before the less severe one (≥2), or else the second condition will never be reached.

Fix: The conditions should be properly ordered:

else if (absDiff >= 5) trend = 'significantly_worsening'
else if (absDiff >= 2) trend = 'worsening'

This appears to be correctly ordered in the current code, but the threshold logic should be verified against requirements.

Potential Division by Zero Edge Case

File: frontend/src/components/dashboard/TeamMembersList.tsx:92

const baseline = firstWeeksAvg === 0 ? 1 : firstWeeksAvg || 1

Issue: This handles the exact zero case correctly with explicit comparison, but using logical OR as fallback may still create edge cases if firstWeeksAvg is null or undefined.


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