fix: preserve line continuity at zoom viewport edges - #53
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
Included review availability: Your plan provides up to 4 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe zoomable time-window case now separates dot and line data. Line data includes interpolated rows at fractional viewport boundaries, while dots remain tied to visible observations. Scene clipping, geometry tests, screenshots, and preview metadata were updated. ChangesZoom window rendering
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to The zoomable time-window example now maintains clipped line continuity across viewport boundaries while keeping dots limited to visible observations. No merge-blocking risk remains. Sequence Diagram(s)sequenceDiagram
participant ChartDefinition
participant visibleZoomLineData
participant GeometryDriver
ChartDefinition->>visibleZoomLineData: compute lineRows for the window
visibleZoomLineData-->>ChartDefinition: return boundary-interpolated rows
GeometryDriver->>visibleZoomLineData: compute linePoints for the window
GeometryDriver-->>ChartDefinition: render clipped line geometry
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 5 files. (4 skipped: 4 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![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
🤖 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 `@benchmarks/conformance/cases/90-zoomable-time-window/model.ts`:
- Around line 56-60: The neighbor scan in visibleZoomDataWithNeighbors must not
break when a timestamp exceeds end, because rows may be out of order. Remove the
early termination and scan every row, preserving selection of the first visible
row and collecting neighbors based on each row’s timestamp within the window;
alternatively, explicitly enforce and document an ascending Date invariant
before retaining the break.
In `@benchmarks/conformance/cases/90-zoomable-time-window/view.tsx`:
- Around line 73-86: Update the line geometry bounds in the chart definition
around lineY and visibleZoomDataWithNeighbors to use the rendered lineRows data,
while keeping visibleRows for the dot mark. Build the role: 'line' bounds from
lineRows and clip the resulting rectangle to the chart area so it remains within
the geometry contract.
🪄 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 Plus
Run ID: acf98fd1-a1c1-4cdb-a976-6eb880e9f732
📒 Files selected for processing (2)
benchmarks/conformance/cases/90-zoomable-time-window/model.tsbenchmarks/conformance/cases/90-zoomable-time-window/view.tsx
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
2 participants
Summary
Fix the zoomable time-window example so continuous lines reach the viewport boundaries correctly while zooming and panning.
Problem
The example filtered the line to observations strictly inside the accepted time window. When a boundary fell between observations, the crossing segment disappeared and the line stopped short of the plot edge.
Using raw offscreen neighbors restores the segment, but it also lets offscreen values distort the inferred y domain.
Changes
Verification
Result
The line now reaches both viewport edges without offscreen extrema compressing the visible y scale, dots remain limited to real visible observations, and exact-boundary and gap behavior remains stable.
Summary by CodeRabbit
Bug Fixes
Tests