Skip to content

observability v2#1936

Open
akshaydeo wants to merge 1 commit intomainfrom
graph-fixes
Open

observability v2#1936
akshaydeo wants to merge 1 commit intomainfrom
graph-fixes

Conversation

@akshaydeo
Copy link
Copy Markdown
Contributor

@akshaydeo akshaydeo commented Mar 5, 2026

Summary

Increased the bar width in the provider latency chart to improve visual clarity and readability of latency metrics.

Changes

  • Increased barSize from 8 to 30 pixels for all latency bars (avg, p90, p95, p99) in single provider mode
  • Increased barSize from 8 to 30 pixels for provider comparison bars in multi-provider mode
  • This change makes the latency data more visually prominent and easier to read in the dashboard

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (Next.js)
  • Docs

How to test

Navigate to the workspace dashboard and verify that the provider latency chart displays with wider, more visible bars for both single provider and multi-provider views.

# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build

Screenshots/Recordings

If UI changes, add before/after screenshots or short clips.

Breaking changes

  • Yes
  • No

Related issues

Link related issues and discussions. Example: Closes #123

Security considerations

None - this is a visual styling change only.

Checklist

  • I read docs/contributing/README.md and followed the guidelines
  • I added/updated tests where appropriate
  • I updated documentation where needed
  • I verified builds succeed (Go and UI)
  • I verified the CI pipeline passes locally if applicable

Copy link
Copy Markdown
Contributor Author

akshaydeo commented Mar 5, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 5, 2026

📝 Walkthrough

Summary by CodeRabbit

  • Style
    • Increased the visual width of bars in the provider latency chart for enhanced clarity and improved readability when reviewing performance data.

Walkthrough

The pull request increases the visual width of bars in the provider latency chart component by changing the barSize parameter from 8 to 30. This affects both single-provider and multi-provider visualization modes. No data structures, logic, or functional behavior are altered—only the graphical representation of bars on the chart.

Changes

Cohort / File(s) Summary
Provider Latency Chart Styling
ui/app/workspace/dashboard/components/providerLatencyChart.tsx
Increased barSize parameter from 8 to 30 for Bar components in single-provider and multi-provider modes, making bars visually wider without altering chart data, structure, or interactive elements.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 The bars grew wide, a sight to see,
From eight to thirty, plain and free,
No logic broke, no data swayed,
Just wider charts, by rabbits made! 🎨

🚥 Pre-merge checks | ✅ 1 | ❌ 4

❌ Failed checks (3 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR changes are unrelated to the linked issue #123 (Files API Support), which requires implementing file upload endpoints for providers like OpenAI and Anthropic. Either link relevant issues related to dashboard UI improvements or remove the incorrect issue link #123 which concerns Files API implementation.
Out of Scope Changes check ⚠️ Warning The PR modifies only the provider latency chart bar width, which is a visual styling change unrelated to the linked issue #123 about Files API support. Clarify the scope by linking appropriate UI/dashboard issues or remove the incorrect Files API issue link that doesn't relate to this styling change.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'observability v2' is vague and generic, failing to clearly describe the specific change of increasing bar width in the provider latency chart. Use a more descriptive title like 'Increase provider latency chart bar width from 8 to 30 pixels' to clearly convey the main change.
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The PR description covers most required sections including summary, changes, type, affected areas, testing instructions, and breaking changes confirmation.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch graph-fixes

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

@akshaydeo akshaydeo changed the title security patches observability v2 Mar 5, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 5, 2026

🧪 Test Suite Available

This PR can be tested by a repository admin.

Run tests for PR #1936

@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@akshaydeo akshaydeo mentioned this pull request Mar 7, 2026
18 tasks
@akshaydeo akshaydeo marked this pull request as ready for review March 7, 2026 19:12
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ui/app/workspace/dashboard/components/providerLatencyChart.tsx`:
- Around line 165-168: The bars currently set barSize={30} in
providerLatencyChart.tsx (the <Bar dataKey="avg_latency">, "p90_latency",
"p95_latency", "p99_latency") which forces a fixed width and conflicts with
Recharts' responsive grouping behavior; change these props to use
maxBarSize={30} instead of barSize so Recharts can auto-shrink grouped bars but
never exceed 30px, ensuring consistent dashboard sizing with other charts.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4b51526f-70fc-42fc-9f7b-54f6cd4354ab

📥 Commits

Reviewing files that changed from the base of the PR and between d3cee58 and 3879d2f.

📒 Files selected for processing (1)
  • ui/app/workspace/dashboard/components/providerLatencyChart.tsx

Comment on lines +165 to +168
<Bar dataKey="avg_latency" fill={LATENCY_COLORS.avg} fillOpacity={0.9} barSize={30} radius={[2, 2, 0, 0]} />
<Bar dataKey="p90_latency" fill={LATENCY_COLORS.p90} fillOpacity={0.9} barSize={30} radius={[2, 2, 0, 0]} />
<Bar dataKey="p95_latency" fill={LATENCY_COLORS.p95} fillOpacity={0.9} barSize={30} radius={[2, 2, 0, 0]} />
<Bar dataKey="p99_latency" fill={LATENCY_COLORS.p99} fillOpacity={0.9} barSize={30} radius={[2, 2, 0, 0]} />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

In Recharts 3.1.2, how does a fixed barSizebehave in groupedBarCharts when the total bar widths exceed the category width, and is maxBarSizesupported onBarorBarChart as a safer cap?

💡 Result:

Fixed barSize in grouped BarChart (Recharts 3.1.2) when bars “don’t fit”

In grouped bars, once Recharts detects that the group’s requested total width (sum of the per-series barSize plus barGaps) is >= the category band (bandSize), it does not let the bars overflow into neighboring categories:

  1. It first forces barGap to 0 (removing inter-bar spacing) to try to make the group fit.
  2. If the bars still don’t fit, it ignores the requested barSize and switches to an equal “full” size per bar (bandSize / numberOfBars), then multiplies that by 0.9 so the group fits with a little padding, and centers it within the band. [1]

So a fixed barSize is treated as a preference, but it will be shrunk (and gaps dropped) rather than overflowing when the group is too wide. [1]

Is maxBarSize supported, and is it a safe cap?

Yes—maxBarSize is supported on both:

  • <Bar maxBarSize={...} /> [2]
  • <BarChart maxBarSize={...} /> [3]

But in 3.1.2 it is not a “cap” for an explicitly fixed barSize in grouped bars: maxBarSize is only applied in the branch where bar sizes are auto-computed (i.e., when barSize isn’t set / isn’t “well-behaved”). When you set a fixed barSize, the “doesn’t fit” fallback is the shrink-to-0.9 * (bandSize / n) behavior instead. [1]


🏁 Script executed:

cat -n ui/app/workspace/dashboard/components/providerLatencyChart.tsx | sed -n '150,190p'

Repository: maximhq/bifrost

Length of output: 2050


🏁 Script executed:

# Check if there's a git diff or stack context available
cd ui/app/workspace/dashboard/components && git log --oneline -5 providerLatencyChart.tsx 2>/dev/null || echo "No git history available"
git diff HEAD providerLatencyChart.tsx 2>/dev/null | head -100 || echo "No uncommitted changes"

Repository: maximhq/bifrost

Length of output: 84


🏁 Script executed:

# Look for any stack or related PRs mentioned in PR description or comments
find . -name "*.md" -o -name ".github" -type d | head -5
ls -la .github/PULL_REQUEST_TEMPLATE* 2>/dev/null || echo "No PR template found"

Repository: maximhq/bifrost

Length of output: 285


🏁 Script executed:

# Verify Recharts version in package.json and confirm maxBarSize support
grep -A 2 -B 2 "recharts" package.json 2>/dev/null || grep -r "recharts" . --include="*.json" | grep -i version | head -3

Repository: maximhq/bifrost

Length of output: 41


🏁 Script executed:

# Check if there are any other bar sizing references or maxBarSize usage elsewhere in the codebase
rg "maxBarSize|barSize" ui/app --type tsx --type ts -B 2 -A 2

Repository: maximhq/bifrost

Length of output: 86


🏁 Script executed:

# Find package.json and check Recharts version
find . -name "package.json" -type f | head -5
cat package.json 2>/dev/null | grep -A 2 -B 2 "recharts" || echo "Not found in root"

Repository: maximhq/bifrost

Length of output: 307


🏁 Script executed:

# Search for barSize and maxBarSize usage with correct rg syntax
rg "maxBarSize|barSize" ui/app -A 2 -B 2

Repository: maximhq/bifrost

Length of output: 8307


Verify barSize consistency across dashboard components.

The barSize={30} change aligns with the existing dashboard design pattern (logsVolumeChart, tokenUsageChart, costChart, etc. all use barSize={30}). However, note that Recharts will automatically shrink grouped bars if they exceed the category band width—it won't clip or overflow. In single-provider mode with four bars per bucket, Recharts will reduce each bar below 30px and remove inter-bar gaps to fit within the available space. If 30px is a strict design requirement and auto-shrinking is undesirable, consider using maxBarSize as a constraint, though it only applies to auto-computed sizes, not explicitly fixed barSize values. The current implementation defers to Recharts' responsive sizing.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ui/app/workspace/dashboard/components/providerLatencyChart.tsx` around lines
165 - 168, The bars currently set barSize={30} in providerLatencyChart.tsx (the
<Bar dataKey="avg_latency">, "p90_latency", "p95_latency", "p99_latency") which
forces a fixed width and conflicts with Recharts' responsive grouping behavior;
change these props to use maxBarSize={30} instead of barSize so Recharts can
auto-shrink grouped bars but never exceed 30px, ensuring consistent dashboard
sizing with other charts.

@akshaydeo akshaydeo mentioned this pull request Mar 8, 2026
14 tasks
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.

Files API Support

2 participants