From b78adb1316ca283fe9eaa6973ca93dddb2a7562f Mon Sep 17 00:00:00 2001 From: Austen Stone Date: Thu, 20 Aug 2026 09:23:45 -0700 Subject: [PATCH 1/3] test: cover the import, avatar, and theme paths and restore coverage gates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #95. - import.ts had no tests at all. Covers CSV, ZIP, non-CSV skip, and per-entry failure isolation. The ZIP fixture is prebuilt because fflate's zipSync can't produce a valid archive under jsdom — its Uint8Array check fails across realms and it walks the byte array as a nested directory. The read path is unaffected, which is what the app actually uses on import. - The bot avatar code was untested because this jsdom setup exposes no localStorage, so cache hydration and persistence silently no-op'd. Installing an in-memory Storage exercises them, along with request dedupe, the concurrency queue, the 10-lookup batch cap, and failure handling. - buildGitHubChartTheme now covers both the CSS-variable path and the bundled-palette fallback. Thresholds go back to the 80/70/80/65 the issue asked for. Actual is 88.2 statements / 73.2 branches / 88.1 functions / 90.8 lines. Also corrects the zip.ts coverage-exclusion comment, which blamed File.arrayBuffer(); that works fine here, zipSync is the problem. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/lib/chart-theme.test.ts | 44 ++++++- src/lib/formatters.avatars.test.ts | 192 +++++++++++++++++++++++++++++ src/lib/import.test.ts | 130 +++++++++++++++++++ vitest.config.ts | 12 +- 4 files changed, 371 insertions(+), 7 deletions(-) create mode 100644 src/lib/formatters.avatars.test.ts create mode 100644 src/lib/import.test.ts diff --git a/src/lib/chart-theme.test.ts b/src/lib/chart-theme.test.ts index 9a31a5a..0f7bbed 100644 --- a/src/lib/chart-theme.test.ts +++ b/src/lib/chart-theme.test.ts @@ -1,5 +1,10 @@ -import { describe, it, expect } from 'vitest'; -import { buildColorMap, getModelIconUrl, GITHUB_COLORS_RESOLVED } from './chart-theme'; +import { afterEach, describe, it, expect } from 'vitest'; +import { + buildColorMap, + buildGitHubChartTheme, + getModelIconUrl, + GITHUB_COLORS_RESOLVED, +} from './chart-theme'; describe('buildColorMap', () => { it('assigns distinct branded colors per AI model family', () => { @@ -49,3 +54,38 @@ describe('getModelIconUrl', () => { expect(getModelIconUrl('CLAUDE')).toBe(getModelIconUrl('claude')); }); }); + +describe('buildGitHubChartTheme', () => { + afterEach(() => { + document.querySelector('[data-color-mode]')?.remove(); + }); + + it('falls back to the bundled palette when no Primer root is mounted', () => { + const theme = buildGitHubChartTheme() as { colors: string[] }; + + expect(theme.colors).toEqual(GITHUB_COLORS_RESOLVED.slice(0, theme.colors.length)); + }); + + it('reads live CSS variables from the Primer root when one exists', () => { + const root = document.createElement('div'); + root.setAttribute('data-color-mode', 'dark'); + root.style.setProperty('--data-blue-color-emphasis', 'rgb(1, 2, 3)'); + document.body.appendChild(root); + + const theme = buildGitHubChartTheme() as { colors: string[] }; + + expect(theme.colors[0]).toBe('rgb(1, 2, 3)'); + }); + + it('disables credits, accessibility, and animation for deterministic rendering', () => { + const theme = buildGitHubChartTheme() as { + credits: { enabled: boolean }; + accessibility: { enabled: boolean }; + chart: { animation: boolean }; + }; + + expect(theme.credits.enabled).toBe(false); + expect(theme.accessibility.enabled).toBe(false); + expect(theme.chart.animation).toBe(false); + }); +}); diff --git a/src/lib/formatters.avatars.test.ts b/src/lib/formatters.avatars.test.ts new file mode 100644 index 0000000..6ee2678 --- /dev/null +++ b/src/lib/formatters.avatars.test.ts @@ -0,0 +1,192 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const AVATAR_STORAGE_KEY = 'tbb:bot-avatars'; + +const okResponse = (avatarUrl: string) => + ({ ok: true, json: async () => ({ avatar_url: avatarUrl }) }) as unknown as Response; + +/** + * This jsdom setup exposes no localStorage, so the avatar cache's persistence + * paths silently no-op under test. Install a minimal in-memory implementation + * before importing the module so hydration and persistence are exercised. + */ +const memoryStorage = () => { + const store = new Map(); + return { + getItem: (k: string) => store.get(k) ?? null, + setItem: (k: string, v: string) => void store.set(k, String(v)), + removeItem: (k: string) => void store.delete(k), + clear: () => store.clear(), + key: (i: number) => [...store.keys()][i] ?? null, + get length() { + return store.size; + }, + } as Storage; +}; + +let storage: Storage; + +/** + * The avatar cache is module-level state seeded from localStorage at import + * time, so every test needs a fresh module registry to stay independent. + */ +const freshFormatters = async () => { + vi.resetModules(); + return import('./formatters'); +}; + +describe('bot avatar resolution', () => { + beforeEach(() => { + storage = memoryStorage(); + vi.stubGlobal('localStorage', storage); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it('never calls the API for a human username', async () => { + const fetchMock = vi.fn(); + vi.stubGlobal('fetch', fetchMock); + const { resolveBotAvatar } = await freshFormatters(); + + await expect(resolveBotAvatar('austenstone')).resolves.toBeNull(); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it('serves known bots from the built-in list without a request', async () => { + const fetchMock = vi.fn(); + vi.stubGlobal('fetch', fetchMock); + const { resolveBotAvatar } = await freshFormatters(); + + await expect(resolveBotAvatar('dependabot[bot]')).resolves.toContain( + 'avatars.githubusercontent.com', + ); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it('resolves an unknown bot from the API and persists it', async () => { + const fetchMock = vi.fn().mockResolvedValue(okResponse('https://example.test/a.png')); + vi.stubGlobal('fetch', fetchMock); + const { resolveBotAvatar } = await freshFormatters(); + + await expect(resolveBotAvatar('acme-ci[bot]')).resolves.toBe('https://example.test/a.png'); + expect(fetchMock).toHaveBeenCalledWith( + 'https://api.github.com/users/acme-ci%5Bbot%5D', + ); + + const stored = JSON.parse(storage.getItem(AVATAR_STORAGE_KEY) ?? '{}'); + expect(stored['acme-ci[bot]']).toBe('https://example.test/a.png'); + }); + + it('rehydrates persisted avatars on the next load', async () => { + storage.setItem( + AVATAR_STORAGE_KEY, + JSON.stringify({ 'acme-ci[bot]': 'https://example.test/cached.png' }), + ); + const fetchMock = vi.fn(); + vi.stubGlobal('fetch', fetchMock); + const { resolveBotAvatar } = await freshFormatters(); + + await expect(resolveBotAvatar('acme-ci[bot]')).resolves.toBe( + 'https://example.test/cached.png', + ); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it('returns null when the API rejects the lookup', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: false } as Response)); + const { resolveBotAvatar } = await freshFormatters(); + + await expect(resolveBotAvatar('missing[bot]')).resolves.toBeNull(); + expect(storage.getItem(AVATAR_STORAGE_KEY)).toBeNull(); + }); + + it('returns null when the request throws', async () => { + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('offline'))); + const { resolveBotAvatar } = await freshFormatters(); + + await expect(resolveBotAvatar('offline[bot]')).resolves.toBeNull(); + }); + + it('collapses concurrent lookups for the same bot into one request', async () => { + const fetchMock = vi.fn().mockResolvedValue(okResponse('https://example.test/b.png')); + vi.stubGlobal('fetch', fetchMock); + const { resolveBotAvatar } = await freshFormatters(); + + const results = await Promise.all([ + resolveBotAvatar('busy[bot]'), + resolveBotAvatar('busy[bot]'), + resolveBotAvatar('busy[bot]'), + ]); + + expect(results).toEqual(Array(3).fill('https://example.test/b.png')); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it('queues requests beyond the concurrency cap but still resolves them all', async () => { + const fetchMock = vi.fn().mockResolvedValue(okResponse('https://example.test/c.png')); + vi.stubGlobal('fetch', fetchMock); + const { resolveBotAvatar } = await freshFormatters(); + + const names = Array.from({ length: 8 }, (_, i) => `queued-${i}[bot]`); + const results = await Promise.all(names.map(resolveBotAvatar)); + + expect(results.every((r) => r === 'https://example.test/c.png')).toBe(true); + expect(fetchMock).toHaveBeenCalledTimes(8); + }); +}); + +describe('preloadBotAvatars', () => { + beforeEach(() => { + storage = memoryStorage(); + vi.stubGlobal('localStorage', storage); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it('does nothing when the dataset has no bots', async () => { + const fetchMock = vi.fn(); + vi.stubGlobal('fetch', fetchMock); + const { preloadBotAvatars } = await freshFormatters(); + + await expect(preloadBotAvatars(['austenstone', 'octocat'])).resolves.toBe(false); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it('skips bots that are already cached', async () => { + const fetchMock = vi.fn(); + vi.stubGlobal('fetch', fetchMock); + const { preloadBotAvatars } = await freshFormatters(); + + await expect(preloadBotAvatars(['dependabot[bot]'])).resolves.toBe(false); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it('reports true once at least one avatar resolves', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(okResponse('https://example.test/d.png'))); + const { preloadBotAvatars } = await freshFormatters(); + + await expect(preloadBotAvatars(['fresh-a[bot]', 'fresh-b[bot]'])).resolves.toBe(true); + }); + + it('reports false when every lookup fails', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: false } as Response)); + const { preloadBotAvatars } = await freshFormatters(); + + await expect(preloadBotAvatars(['nope-a[bot]', 'nope-b[bot]'])).resolves.toBe(false); + }); + + it('caps a single batch at ten API lookups', async () => { + const fetchMock = vi.fn().mockResolvedValue(okResponse('https://example.test/e.png')); + vi.stubGlobal('fetch', fetchMock); + const { preloadBotAvatars } = await freshFormatters(); + + const many = Array.from({ length: 25 }, (_, i) => `bulk-${i}[bot]`); + await preloadBotAvatars(many); + + expect(fetchMock).toHaveBeenCalledTimes(10); + }); +}); diff --git a/src/lib/import.test.ts b/src/lib/import.test.ts new file mode 100644 index 0000000..7c4defb --- /dev/null +++ b/src/lib/import.test.ts @@ -0,0 +1,130 @@ +import { describe, expect, it, vi } from 'vitest'; +import { importFiles, importRawCSVs } from './import'; +import type { ParsedReport } from './types'; + +const USAGE_CSV = [ + 'date,product,sku,quantity,unit_type,price_per_unit,gross_amount,discount_amount,net_amount,organization,repository,cost_center_name', + '2026-01-05,actions,actions_linux,120,minutes,0.008,0.96,0,0.96,acme,acme/api,', +].join('\n'); + +const collector = () => { + const reports: ParsedReport[] = []; + const addReport = vi.fn((report: ParsedReport) => reports.push(report) - 1); + return { reports, addReport }; +}; + +const csvFile = (name: string, content: string) => + new File([content], name, { type: 'text/csv' }); + +// zipSync can't build a valid archive under jsdom — fflate's Uint8Array check +// fails across realms and it walks the byte array as a nested directory. The +// read path (unzipSync) is unaffected, so use an archive built ahead of time. +// Contents: good.csv (valid usage report), bad.csv (unparseable), notes.txt. +const ZIP_B64 = + 'UEsDBBQAAAAIAHdKFF3n7nuMjQAAANEAAAAIAAAAZ29vZC5jc3Y9jOEKwjAMhP/7LFG7gUOfpoQu' + + 'jKBNapOC8+ldlfnn7kvuuBmdoFSdW3Kwe4NnQ3H2FZqwR19LjzlRLFRj/8FS1Sxi1iYOM1vqsN9C' + + 'f9S6oPAbnVWgUlFj17pCUvOYSHwbFMx0GMM4HcNwDBfA1Nu2e3ywtBcMY4C8kZNBOIVw3fQ2QfgZ' + + 'pkxfOWNh+ABQSwMEFAAAAAgAd0oUXaIsw5gJAAAABwAAAAcAAABiYWQuY3N2S9RJ4jLUMQIAUEsD' + + 'BBQAAAAIAHdKFF0smQQICQAAAAcAAAAJAAAAbm90ZXMudHh0K87OLFDITQUAUEsBAhQAFAAAAAgA' + + 'd0oUXefue4yNAAAA0QAAAAgAAAAAAAAAAAAAAAAAAAAAAGdvb2QuY3N2UEsBAhQAFAAAAAgAd0oU' + + 'XaIsw5gJAAAABwAAAAcAAAAAAAAAAAAAAAAAswAAAGJhZC5jc3ZQSwECFAAUAAAACAB3ShRdLJkE' + + 'CAkAAAAHAAAACQAAAAAAAAAAAAAAAADhAAAAbm90ZXMudHh0UEsFBgAAAAADAAMAogAAABEBAAAA' + + 'AA=='; + +const zipFile = (name: string) => + new File([Uint8Array.from(atob(ZIP_B64), (c) => c.charCodeAt(0))], name, { + type: 'application/zip', + }); + +describe('importFiles', () => { + it('parses a CSV file and reports it as succeeded', async () => { + const { reports, addReport } = collector(); + + const result = await importFiles([csvFile('usage.csv', USAGE_CSV)], addReport); + + expect(result).toEqual({ succeeded: 1, failed: [] }); + expect(reports[0].type).toBe('usage_report'); + expect(addReport).toHaveBeenCalledWith(expect.anything(), USAGE_CSV); + }); + + it('skips files that are neither CSV nor ZIP without failing them', async () => { + const { addReport } = collector(); + + const result = await importFiles( + [new File(['nope'], 'notes.txt', { type: 'text/plain' })], + addReport, + ); + + expect(result).toEqual({ succeeded: 0, failed: [] }); + expect(addReport).not.toHaveBeenCalled(); + }); + + it('records the filename when a CSV cannot be parsed', async () => { + const { addReport } = collector(); + + const result = await importFiles([csvFile('junk.csv', 'a,b,c\n1,2,3')], addReport); + + expect(result).toEqual({ succeeded: 0, failed: ['junk.csv'] }); + }); + + it('extracts CSVs from a ZIP and ignores non-CSV entries', async () => { + const { reports, addReport } = collector(); + + const result = await importFiles([zipFile('reports.zip')], addReport); + + expect(result.succeeded).toBe(1); + expect(reports[0].type).toBe('usage_report'); + }); + + it('fails only the bad entries inside a ZIP', async () => { + const { addReport } = collector(); + + const result = await importFiles([zipFile('mixed.zip')], addReport); + + expect(result.failed).toEqual(['bad.csv']); + }); + + it('keeps going after a failed file', async () => { + const { addReport } = collector(); + + const result = await importFiles( + [csvFile('junk.csv', 'a,b\n1,2'), csvFile('usage.csv', USAGE_CSV)], + addReport, + ); + + expect(result).toEqual({ succeeded: 1, failed: ['junk.csv'] }); + }); +}); + +describe('importRawCSVs', () => { + it('imports raw CSV strings', () => { + const { reports, addReport } = collector(); + + const result = importRawCSVs([{ name: 'usage.csv', content: USAGE_CSV }], addReport); + + expect(result).toEqual({ succeeded: 1, failed: [] }); + expect(reports[0].isSample).toBeUndefined(); + }); + + it('marks reports as samples when asked', () => { + const { reports, addReport } = collector(); + + importRawCSVs([{ name: 'usage.csv', content: USAGE_CSV }], addReport, { isSample: true }); + + expect(reports[0].isSample).toBe(true); + }); + + it('collects unparseable entries without throwing', () => { + const { addReport } = collector(); + + const result = importRawCSVs( + [ + { name: 'bad.csv', content: 'a,b\n1,2' }, + { name: 'usage.csv', content: USAGE_CSV }, + ], + addReport, + ); + + expect(result).toEqual({ succeeded: 1, failed: ['bad.csv'] }); + }); +}); diff --git a/vitest.config.ts b/vitest.config.ts index 0cf1457..c910ad2 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -19,14 +19,16 @@ export default defineConfig({ exclude: [ 'src/lib/sample-data.ts', // dynamic imports only, untestable 'src/lib/local-storage.ts', // IndexedDB not available in jsdom - 'src/lib/zip.ts', // File.arrayBuffer() not available in jsdom + // fflate's zipSync can't build an archive under jsdom (its Uint8Array + // check fails across realms); the read path is covered via import.test.ts + 'src/lib/zip.ts', ], reporter: ['text', 'lcov'], thresholds: { - lines: 74, - functions: 67, - statements: 72, - branches: 64, + lines: 80, + functions: 70, + statements: 80, + branches: 65, }, }, }, From 660c050fbc62cf866aef7ab8b968234cddd1e636 Mon Sep 17 00:00:00 2001 From: Austen Stone Date: Thu, 20 Aug 2026 09:28:37 -0700 Subject: [PATCH 2/3] fix: pick a usable groupBy when the schema default is missing too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Org-level Copilot seat exports have no Organization column, but the seat activity schema defaults to grouping by it. The fallback couldn't help because the fallback was the broken column, so the chart came up empty. Validate each candidate in turn — current column, schema default, then the report's primary dimension — instead of trusting the default. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/lib/report-schema.test.ts | 5 +++++ src/lib/report-schema.ts | 24 +++++++++++++----------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/lib/report-schema.test.ts b/src/lib/report-schema.test.ts index faec0e3..e2595ee 100644 --- a/src/lib/report-schema.test.ts +++ b/src/lib/report-schema.test.ts @@ -107,6 +107,11 @@ describe('resolveGroupByColumn', () => { expect(resolveGroupByColumn(report(REPORT_TYPES.ENTERPRISE_MEMBERS, []), '')).toBe('licenseType'); }); + it('skips a schema default that is also absent, as in org-level seat exports', () => { + const rows = Array.from({ length: 50 }, () => ({ login: 'ana-reyes', lastSurfaceUsed: 'vscode' })); + expect(resolveGroupByColumn(report(REPORT_TYPES.COPILOT_SEAT_ACTIVITY, rows), '')).toBe('login'); + }); + it('falls back for an empty report rather than keeping an unusable column', () => { expect(resolveGroupByColumn(report(REPORT_TYPES.DORMANT_USERS, []), 'username')).toBe('role'); }); diff --git a/src/lib/report-schema.ts b/src/lib/report-schema.ts index 024f5aa..ab11dbe 100644 --- a/src/lib/report-schema.ts +++ b/src/lib/report-schema.ts @@ -364,18 +364,20 @@ export function resolveGroupByColumn( currentColumn: string, ): string { const schema = getReportSchema(report.type); - if (!currentColumn) return schema.defaultGroupBy; - - const readColumn = (row: unknown): unknown => - (row as Record | undefined)?.[currentColumn]; - const rows: readonly unknown[] = report.rows; - const stride = Math.max(1, Math.floor(rows.length / GROUP_BY_SAMPLE_SIZE)); - for (let i = 0; i < rows.length; i += stride) { - const value = readColumn(rows[i]); - if (value !== undefined && value !== null && value !== '') return currentColumn; - } - return schema.defaultGroupBy; + + const hasValues = (column: string): boolean => { + if (!column) return false; + const stride = Math.max(1, Math.floor(rows.length / GROUP_BY_SAMPLE_SIZE)); + for (let i = 0; i < rows.length; i += stride) { + const value = (rows[i] as Record | undefined)?.[column]; + if (value !== undefined && value !== null && value !== '') return true; + } + return false; + }; + + const candidates = [currentColumn, schema.defaultGroupBy, schema.primaryDimension]; + return candidates.find(hasValues) ?? schema.defaultGroupBy; } /** Sidebar nav page identifiers derived from report types */ From 54fd6764aa1fbbb248ed55cf1cb343b50d8ed574 Mon Sep 17 00:00:00 2001 From: Austen Stone Date: Thu, 20 Aug 2026 09:40:53 -0700 Subject: [PATCH 3/3] fix: validate groupBy on page switches too, not just on upload Switching pages in the sidebar reset the grouping to the schema default without checking the active report actually has that column. On an org-level seat export, which has no Organization column, that put the page straight back into the empty-chart state the upload path had just avoided. Resolve against the report being switched to. Verified against a five-column org-level export: the chart now renders per-login seats instead of coming up blank. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/hooks/usePageNavigation.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/hooks/usePageNavigation.ts b/src/hooks/usePageNavigation.ts index 6874e5f..423580a 100644 --- a/src/hooks/usePageNavigation.ts +++ b/src/hooks/usePageNavigation.ts @@ -5,6 +5,7 @@ import { PAGE_REPORT_TYPES, pageTypeForReport, getReportSchema, + resolveGroupByColumn, type PageType, } from '../lib/report-schema'; import { readURLFilterState, writeURLFilterState } from '../lib/url-state'; @@ -51,13 +52,13 @@ export function usePageNavigation({ // Files page has no report types const reportTypes = PAGE_REPORT_TYPES[page]; if (!reportTypes || reportTypes.length === 0) return; - const targetReportType = reportTypes[0]; - const schema = getReportSchema(targetReportType); - setGroupByColumn(schema.defaultGroupBy); const matchIndex = reports.findIndex((r) => reportTypes.includes(r.type)); if (matchIndex !== -1) { setActiveReport(matchIndex); + setGroupByColumn(resolveGroupByColumn(reports[matchIndex], '')); + return; } + setGroupByColumn(getReportSchema(reportTypes[0]).defaultGroupBy); }, [setGroupByColumn, reports, setActiveReport]); // Sync active page to URL