fix(vegas): stop the width cap emitting fragments and stale windows - #446
fix(vegas): stop the width cap emitting fragments and stale windows#446ChuckBuilds wants to merge 1 commit into
Conversation
Two defects in the rotation that narrows an oversized plugin to its width budget. Both were found while investigating "cut off early / starts in the middle" reports and are the reason the cap is no longer on by default; they still bite anyone who sets one. A rotation's last window was whatever happened to be left over. Windows are placed by walking forward from the previous one, with nothing looking at the remainder, so a 1,840px stocks ticker against a 1,536px budget split 1,492 + 348 -- every other appearance showed seven seconds and cut. Absorb a remainder below half a budget into the window before it. That overruns the budget by at most half, which is the better trade: the budget guards against one plugin holding the panel for minutes, not against a 20% overshoot. The floor is measured against the budget rather than the panel because snapping to item boundaries already lands an ordinary window short of it -- a 512px budget over 182px-pitch items yields 348px windows, so an absolute floor merges windows that were never fragments. The stored offset also outlived the content it was recorded against. It was a pixel column, reused verbatim after the plugin re-rendered, so once anything ahead of it changed width the window pointed at unrelated items -- observed as news refreshing 9,793px -> 9,505px mid-rotation. Track the rotation as an index into the strip's item boundaries instead, since the Nth boundary survives a digit appearing in a price, and record alongside it what the offset indexes into: a row list, a boundary list, or a column in a gapless image. A mismatch restarts the rotation rather than reinterpreting the number, which also closes the case where one plugin's row index was read back as a pixel column after its content changed from several rows to one wide strip. Replaying the four plugins that actually hit the cap on a live 512px panel: no window is now a fragment, none exceeds 1.5 budgets, and every rotation still covers the whole strip. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
|
Warning Review limit reached
Next review available in: 39 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
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 |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 8 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
1 participant
Follow-up to #445, which called these out but left them. Both are in the rotation that narrows an oversized plugin to its width budget. They no longer fire at the default settings once #445 lands, but they still bite anyone who sets a cap.
1. The last window was whatever happened to be left over
Windows are placed by walking forward from the previous one, with nothing looking at the remainder. On a live 512px panel that split an 1,840px stocks ticker into 1,492 + 348 against a 1,536px budget — every other appearance showed seven seconds and cut, which reads as the display failing rather than as a rotation.
A remainder below half a budget is now absorbed into the window before it. That overruns the budget by at most half, which is the better trade: the budget guards against one plugin holding the panel for minutes, not against a 20% overshoot.
The floor is measured against the budget rather than the panel, and that distinction turned out to matter — snapping to item boundaries already lands an ordinary window short of the budget (a 512px budget over 182px-pitch items yields 348px windows), so an absolute floor merges windows that were never fragments. My first attempt used one screenful and the test caught it.
2. The offset outlived the content it was recorded against
The stored offset was a pixel column, reused verbatim after the plugin re-rendered. Once anything ahead of it changed width — a digit in a price, a shorter headline — the window pointed at unrelated items. Observed on the same panel as news refreshing 9,793px → 9,505px mid-rotation while its column kept advancing, which quietly breaks the "everything is seen eventually" promise for any ticker that refreshes.
Rotation is now an index into the strip's item boundaries, since the Nth boundary survives items changing width. Recorded alongside it is what the offset indexes into:
A mismatch restarts the rotation rather than reinterpreting the number. That also closes the latent unit collision from #445's description — one plugin's row index being read back as a pixel column after its content changed from several rows to one wide strip.
Verification
Replaying the four plugins that actually hit the cap on the live panel, at their logged widths:
No window is a fragment, none exceeds 1.5 budgets, and every rotation still covers the whole strip.
10 regression tests added across
TestTrailingRuntWindowandTestOffsetOutlivesItsContent, including the reported stocks case by its real numbers. 227 tests pass across the vegas and display-controller suites; the fulltest/run is 2372 passed with one pre-existing unrelated failure (test_install_lowmem.py::TestDiskBackedTmpdir, which fails identically on a clean tree whereverTMPDIRis set).Note this branch is cut from
main, not from #445 — they touch different files and can land in either order.🤖 Generated with Claude Code
https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5