fix: correct sample data, keep CSVs out of the JS bundle, and pick a valid groupBy - #106
Merged
Conversation
…valid groupBy Three fixes to first-run behaviour: - enterprise-members.csv was actually loading the dormant-users export, so the Enterprise Members page rendered empty in the demo. Load the real members file and add the dormant-users sample alongside it. - Samples were imported with ?raw, which inlines 15 MB of CSV into a JS chunk the engine has to parse as source. Switch to ?url + fetch so Vite emits them as static assets. - addReport hardcoded groupByColumn to 'username'. Summarized metered usage reports have no username column, so the first chart rendered empty. resolveGroupByColumn samples the parsed rows and falls back to the schema default. Also fixes activeReportIndex, which used the unfiltered report count and pointed past the end of the array once samples were auto-removed on a real import. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
tsconfig.json is a solution-style config — "files": [] with only project references — so `tsc --noEmit` resolved zero files and always exited 0. CI has been running a typecheck gate that could never fail. `tsc -b --noEmit` walks the referenced projects instead. Verified by planting a deliberate type error: the old script reported nothing, the new one reports it. This immediately caught a real error in resolveGroupByColumn, which only surfaced in the build step: the row union has no index signature, so it can't be cast straight to Record<string, unknown>. Read the column through an unknown-typed accessor instead. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The README pointed everyone at "Settings > Billing > Usage report", which isn't where any of these reports live anymore. Replace it with a per-report table matching the in-app instructions in FileDropzone, and note that metered usage exports come in detailed and summarized shapes now that both parse. The privacy section claimed "100% client-side, no uploads, no telemetry". Accurate for CSV data, but formatters.ts does look up avatars from api.github.com for bot accounts. Narrow the claim to what the code does. Also drops the last /tbb/ scaffold paths from share-state.test.ts. The test mocks its own location so it passed either way, but the fixture should match the real base path. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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.
Three first-run bugs, found by actually running the app against real exports.
The demo's Enterprise Members page was empty
sample-data.tsmappedenterprise-members.csvto the dormant users export. The real members file was never loaded, so the Enterprise Members page had nothing to render. Fixed, and the dormant-users sample is now loaded under its own (correct) name — 7 samples instead of 6.Enterprise Members now shows 164 licenses in the demo, where it was previously blank.
15 MB of CSV was being shipped as JavaScript
Samples were imported with
?raw, which inlines the file contents into a JS chunk. The build emitted a 15.4 MB JS chunk that the engine has to parse as source before anything renders.Switching to
?url+fetch()makes Vite emit the files as static assets. Same bytes over the wire, but they're now a plain.csvthe browser streams instead of a script it has to parse. Tests are unaffected — they readexamples/throughreadFileSync.The first chart rendered empty for summarized reports
addReporthardcodedgroupByColumnto'username'. GitHub's summarized metered usage export has nousernamecolumn (see #105), so uploading one landed you on a chart grouped by a column that doesn't exist — empty chart, no error, nothing to tell you why.resolveGroupByColumn()samples the parsed rows for a populated value and falls back to the schema default. It samples with a stride rather than from the head, becauseusernameis legitimately blank on leading storage rows.While in there:
activeReportIndexwas set fromprev.reports.lengthinstead of the post-filter count. When samples are auto-removed on a real import, that pointed past the end of the array. Currently masked, but wrong.Verified
Uploaded a 20,000-row summarized export over the demo:
groupBy=skuinstead of the emptyusername260 tests pass, 0 lint errors, 0 type errors.