Skip to content

Discrete charts line interpolation fix#4233

Merged
cemreinanc merged 2 commits into
mainfrom
3768-discrete-chart-pdf-fix
Feb 5, 2026
Merged

Discrete charts line interpolation fix#4233
cemreinanc merged 2 commits into
mainfrom
3768-discrete-chart-pdf-fix

Conversation

@cemreinanc

@cemreinanc cemreinanc commented Feb 4, 2026

Copy link
Copy Markdown
Contributor

closes #3768
check issue above for before screenshots. after:

CleanShot 2026-02-04 at 10 05 31@2x CleanShot 2026-02-04 at 10 05 15@2x CleanShot 2026-02-04 at 10 03 35@2x

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced area chart border line rendering for discrete data to ensure accurate alignment with data points, while preserving the current rendering behavior for continuous data sets.

@coderabbitai

coderabbitai Bot commented Feb 4, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Adds conditional step interpolation to the VictoryLine border in the minified continuous area chart. When data is discrete, interpolation is set to "step" to align horizontal steps with discrete data points; otherwise, it remains undefined.

Changes

Cohort / File(s) Summary
Chart Border Rendering
front_end/src/components/charts/minified_continuous_area_chart.tsx
Conditionally applies step interpolation to the upper edge border line based on whether data is discrete, ensuring proper alignment with discrete data points while maintaining existing behavior for continuous data.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Suggested reviewers

  • elisescu
  • lsabor
  • hlbmtc

Poem

🐰 A tiny tweak, so swift and neat,
Where steps align with data's beat,
Discrete or smooth, now each one's right,
The chart lines dance in perfect sight! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Linked Issues check ❓ Inconclusive The PR addresses issue #3768 by fixing how discrete charts render their line interpolation, but the linked issue describes removing continuous PDF overlays from discrete bar charts. Clarify whether the interpolation fix fully resolves the continuous PDF overlay issue, or if additional changes are needed to remove the PDF layer entirely for discrete charts.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Discrete charts line interpolation fix' accurately describes the main change of adding conditional step interpolation to chart border lines for discrete data.
Out of Scope Changes check ✅ Passed The change is narrowly scoped to adding interpolation logic to the MinifiedContinuousAreaChart component's border line, directly addressing the rendering behavior for discrete charts.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 3768-discrete-chart-pdf-fix

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0cbacbe and fdf3441.

📒 Files selected for processing (1)
  • front_end/src/components/charts/minified_continuous_area_chart.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Build Docker Image / Build Docker Image
  • GitHub Check: integration-tests
  • GitHub Check: Frontend Checks
  • GitHub Check: Backend Checks
🔇 Additional comments (1)
front_end/src/components/charts/minified_continuous_area_chart.tsx (1)

354-382: Good fix for the discrete chart interpolation issue.

Adding interpolation="step" for discrete charts correctly removes the continuous curve overlay and creates step-wise horizontal segments that better represent discrete data. This aligns with the PR objective to fix the continuous PDF overlay on discrete bar charts.

One consideration: Victory/d3-shape offers three step variants:

  • "step" — y changes midway between x-values
  • "stepBefore" — y changes at each x-value (step occurs before the point)
  • "stepAfter" — y changes at each x-value (step occurs after the point)

Since VictoryBar typically centers bars on x-values and the data points in generateNumericAreaGraph use offset positions ((index + 0.5) / (cdf.length - 1)), "step" should produce correct alignment. Please confirm the border line visually aligns with bar tops in the before/after testing.

[approve_code_changes, request_verification]

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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 and usage tips.

@cemreinanc cemreinanc requested review from lsabor and ncarazon February 4, 2026 09:22
@github-actions

github-actions Bot commented Feb 4, 2026

Copy link
Copy Markdown
Contributor

🚀 Preview Environment

Your preview environment is ready!

Resource Details
🌐 Preview URL https://metaculus-pr-4233-3768-discrete-chart-pdf-fix-preview.mtcl.cc
📦 Docker Image ghcr.io/metaculus/metaculus:3768-discrete-chart-pdf-fix-6212cf5
🗄️ PostgreSQL NeonDB branch preview/pr-4233-3768-discrete-chart-pdf-fix
Redis Fly Redis mtc-redis-pr-4233-3768-discrete-chart-pdf-fix

Details

  • Commit: 3d671205ef190bfa6f738b516ca34a063f165c3b
  • Branch: 3768-discrete-chart-pdf-fix
  • Fly App: metaculus-pr-4233-3768-discrete-chart-pdf-fix

ℹ️ Preview Environment Info

Isolation:

  • PostgreSQL and Redis are fully isolated from production
  • Each PR gets its own database branch and Redis instance
  • Changes pushed to this PR will trigger a new deployment

Limitations:

  • Background workers and cron jobs are not deployed in preview environments
  • If you need to test background jobs, use Heroku staging environments

Cleanup:

  • This preview will be automatically destroyed when the PR is closed

@ncarazon ncarazon left a comment

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.

LGTM

@cemreinanc cemreinanc merged commit 3d67120 into main Feb 5, 2026
5 checks passed
@cemreinanc cemreinanc deleted the 3768-discrete-chart-pdf-fix branch February 5, 2026 10:53
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.

Discrete numeric chart overlays a continuous pdf on top of the bar chart

2 participants