fix(render): stop an oversized chart from aborting the build - #129
Merged
Conversation
`glyf build` did not fail when a chart was too large to draw -- it died. glyf hands every mark to a renderer that holds them all in memory at once, and when that memory runs out the process is killed rather than returning an error: exit 133, a stack trace from inside the renderer where a traceback should be, no build output, and in a multi-chart build no way to tell which query was responsible. Nothing stood in front of it, because `execution.max_rows` is unset by default. `render.max_marks` bounds what a chart may draw and, unlike `max_rows`, applies to a project that has configured nothing. A chart over the budget now fails the way anything else does, naming the chart and saying what to change. The check sits beside the `max_rows` one, after execution and before rendering, so nothing is drawn and no artifact is left behind. The default of 500,000 is the largest single-series chart observed to render, on one machine. The real limit belongs to the renderer's memory rather than to glyf, so it moves with the platform and the chart: it is raisable, and `null` removes it along with the only thing standing in front of the abort. Measured with `bench/downsampling.py ceiling`, which is also why no test renders a chart that large -- proving the crash costs minutes and a 138 MB artifact. The tests prove the guard fires, that it fires before the renderer is reached, and that validate mode never trips it.
This was referenced Sep 3, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the crash found while measuring render cost:
glyf builddid not fail on a chart too large to draw, it died.The problem
glyf hands every mark to a renderer that holds them all in memory at once. When that memory runs out the process is killed rather than returning an error — exit 133, a stack trace from inside the renderer where a traceback should be, no build output, and in a multi-chart build no way to tell which query was responsible.
Measured on
v0.5.0, one-series line chart, 800x400:execution.max_rowsis unset by default, so nothing stood in front of it.The fix
render.max_marks, default500000, checked after execution and before rendering — beside the existingmax_rowscheck inrender_project, which is the same shape of guard:Unlike
max_rowsit applies with nothing configured, which is the point — the projects that hit this are the ones that never set a bound.nullremoves it, and the docs say plainly what that gives up.The default is the largest single-series chart observed to render, on one machine. The real limit belongs to the renderer's memory rather than to glyf, so it is documented as movable rather than as a constant.
Not in this PR
No downsampling. A chart over the budget fails; it is not redrawn smaller. That is deliberate — reducing marks changes what the artifact publishes (aggregates instead of rows), which is a separate decision.
Testing
uv run pytest— 404 passed, 6 skipped. 13 new tests covering the guard firing, the message naming the chart and the setting, no artifacts written when it fires, a chart exactly at the budget still rendering, the default applying with no config, validate mode never tripping it, and config parsing includingnullversus absent.No test renders a chart large enough to actually crash: that costs minutes and a 138 MB artifact, and
bench/downsampling.py ceilingalready covers it.Verified through the CLI as well — exit 1 with the named error above the budget, exit 0 and artifacts written at it.
Docs
render.max_marksin the Render table, a section explaining why it defaults on wheremax_rowsdoes not, and a table contrasting the two bounds so they are not mistaken for each other.