feat: chart-type-aware row cap — chartRowCap(type) replaces flat CHART_ROW_CAP=500#116
Merged
Merged
Conversation
…T_ROW_CAP=500 Each chart shape has a different readable ceiling (pie legibility tops out ~30 slices; bar/column are bar-width bound; line/area are point- density bound and can plot thousands before blurring), so a single flat cap either truncates too aggressively (pie) or too conservatively (line/area — a 1.4K-row result only plotted its first 500 points). CHART_ROW_CAPS / chartRowCap(type) in src/core/chart-data.js replace the constant; buildChartData() and the results.js truncation note both key off cfg.type, and switching chart type re-slices + updates the note via the existing rerender() path. Closes #109 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01784dKCpk5W7rdpUAwcQnix
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.
What & why
Closes #109
CHART_ROW_CAPwas one flat constant (500) applied to every chart type. Each shape has a different readable ceiling:CHART_ROW_CAPS/chartRowCap(type)insrc/core/chart-data.jsreplace the constant:{ pie: 30, hbar: 500, bar: 1000, line: 5000, area: 5000 }, falling back to 500 for an unmapped type.buildChartData()and theresults.jstruncation note both key offcfg.type; switching chart type re-slices and updates the note via the existingrerender()path (no new plumbing needed there).Out of scope (tracked separately in #111, referenced in #109's body):
buildChartData()slices raw rows before aggregating, which is a correctness issue for un-pre-aggregated results, not a readability one — different fix shape, kept out of this same-day pure-lookup change.Verification
npm test— full 100/100/100/100 gate onchart-data.js, updated tests inresults.js.npm run test:e2e— 26 passed (Chromium + WebKit clean); 13 Firefox failures are all pre-existingunshare(CLONE_NEWPID): EPERMsandbox errors inbeforeEach(an environment limitation of this container), none touching chart code.antalyademo cluster) vianpm run local+ Playwright: ran a 2000-row query, cycled every chart type, confirmed the note and plotted-point count for each: Barfirst 500 of 2.0K rows, Piefirst 30 of 2.0K rows(visually ~30 legible slices), Columnfirst 1000 of 2.0K rows, Line/Area — no truncation (2000 < 5000 cap)./code-review(5 parallel finder angles + verify): one confirmed finding — a stale docstring onchartRowCapclaiming the fallback was "the bar/column default" when it's actually 500 (hbar's value, not bar's 1000) — fixed in this PR.Checklist
npm testpasses (the per-file coverage gate is non-negotiable)npm run buildsucceeds (single-filedist/sql.html)src/core/, network insrc/net/(injected fetch), DOM insrc/ui/CHANGELOG.md([Unreleased]) updated🤖 Generated with Claude Code
https://claude.ai/code/session_01784dKCpk5W7rdpUAwcQnix