Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replaces the legacy Eleventy/Pug/D3 stack with a modern SPA in app/: - Vite + React 18 + TypeScript for the build and UI layer - Recharts for Sankey flow, Treemap, and BarChart visualizations - Tailwind CSS + custom shadcn-compatible UI components - PapaParse for client-side CSV parsing (replaces Python preprocessing) - React Router v6 preserving existing URL paths (/adopted-budget-flow, /adopted-budget-tree, /compare) - Single BudgetContext loads and caches the CSV once for all pages - Treemap drill-down, Compare diff table with Infinity sentinel, Flow page with REV_ORDER/EXP_ORDER preserved from legacy flow.js Also updates deploy.yml to build from app/ and deploy app/dist/ to gh-pages, with public/404.html for SPA routing fallback. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a new Vite + React + TypeScript SPA under app/ to replace/modernize the legacy _src/ implementation while preserving existing visualization routes and updating GitHub Pages deployment to publish the Vite build output.
Changes:
- Introduces a new React SPA (
app/) with routing, layout, and three visualization pages (flow/treemap/compare) plus supporting UI components. - Switches data loading to a single client-side PapaParse CSV fetch and new transform utilities for Sankey/Treemap/Compare views.
- Updates GitHub Actions deploy workflow to build and deploy
app/distinstead of the legacybuild/output.
Reviewed changes
Copilot reviewed 56 out of 60 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| app/vite.config.ts | Vite config with React plugin, path alias, and chunk splitting. |
| app/tsconfig.node.json | TypeScript config for Vite/node-side files. |
| app/tsconfig.json | TS project references wiring. |
| app/tsconfig.app.json | TypeScript config for the React app source. |
| app/tailwind.config.js | Tailwind theme + content scanning configuration. |
| app/postcss.config.js | PostCSS pipeline for Tailwind/autoprefixer. |
| app/eslint.config.js | ESLint flat config for TS/React. |
| app/package.json | App dependencies and build scripts. |
| app/package-lock.json | Dependency lockfile for reproducible installs. |
| app/.gitignore | App-local ignores (dist/node_modules/etc.). |
| app/README.md | Vite template README content. |
| app/index.html | Vite entry HTML for SPA. |
| app/public/vite.svg | Static public asset. |
| app/public/404.html | GitHub Pages SPA 404 fallback page (currently problematic). |
| app/public/data/City_of_Sacramento_Approved_Budgets.csv | Static CSV dataset served to the SPA. |
| app/src/main.tsx | React entrypoint mounting the app. |
| app/src/App.tsx | Router and top-level providers/layout. |
| app/src/index.css | Tailwind directives + base CSS variables/styles. |
| app/src/types/budget.ts | Shared domain/types for budget rows and view models. |
| app/src/context/BudgetContext.tsx | Data loading context/provider for CSV-backed budget rows. |
| app/src/lib/utils.ts | cn() helper for className merging. |
| app/src/lib/formatters.ts | Currency/percent formatting helpers. |
| app/src/lib/csvParser.ts | PapaParse CSV loader + normalization into BudgetRow. |
| app/src/lib/flowTransforms.ts | Sankey node/link derivation from budget rows. |
| app/src/lib/treemapTransforms.ts | Treemap and spreadsheet row derivation. |
| app/src/lib/compareTransforms.ts | Compare records + totals computation. |
| app/src/pages/HomePage.tsx | Landing page with visualization links. |
| app/src/pages/FlowPage.tsx | Sankey “Budget Overview” page. |
| app/src/pages/TreemapPage.tsx | Treemap drill-down + spreadsheet page. |
| app/src/pages/ComparePage.tsx | Compare UI wiring (selectors, tabs, summary). |
| app/src/pages/WhoWeArePage.tsx | About page content. |
| app/src/pages/NotFoundPage.tsx | In-app 404 page. |
| app/src/components/layout/PageLayout.tsx | Shared page scaffold (navbar/main/footer). |
| app/src/components/layout/Navbar.tsx | Desktop/mobile navigation and dropdowns. |
| app/src/components/layout/Footer.tsx | Footer content and links. |
| app/src/components/shared/YearSelector.tsx | Shared year select control. |
| app/src/components/shared/LoadingState.tsx | Skeleton-based loading UI. |
| app/src/components/shared/ErrorBoundary.tsx | Error boundary wrapper for runtime failures. |
| app/src/components/ui/select.tsx | Radix Select wrappers styled with Tailwind. |
| app/src/components/ui/tabs.tsx | Radix Tabs wrappers styled with Tailwind. |
| app/src/components/ui/card.tsx | Card primitives for layout/summary UI. |
| app/src/components/ui/button.tsx | Button component with variants/sizes. |
| app/src/components/ui/badge.tsx | Badge component variants. |
| app/src/components/ui/skeleton.tsx | Skeleton loading primitive. |
| app/src/components/flow/SankeyChart.tsx | Recharts Sankey container + tooltip binding. |
| app/src/components/flow/SankeyNode.tsx | Custom Sankey node renderer. |
| app/src/components/flow/SankeyLink.tsx | Custom Sankey link renderer. |
| app/src/components/flow/FlowTooltip.tsx | Tooltip content for nodes/links. |
| app/src/components/treemap/TreemapChart.tsx | Recharts Treemap container + tooltip binding. |
| app/src/components/treemap/TreemapContent.tsx | Custom Treemap cell renderer + click-to-drill. |
| app/src/components/treemap/Breadcrumb.tsx | Drill-down breadcrumb/reset UI. |
| app/src/components/treemap/SpreadsheetTable.tsx | Top-N table for treemap drill level. |
| app/src/components/compare/CompareHeader.tsx | Compare year selectors + $/% toggle UI. |
| app/src/components/compare/TotalSummary.tsx | Total summaries for both years + change card. |
| app/src/components/compare/BreakdownTabs.tsx | Table/bar tabs + dimension toggle UI. |
| app/src/components/compare/DiffTable.tsx | Compare diff table rendering (currently has sign-format bug). |
| app/src/components/compare/BarComparison.tsx | Recharts bar chart for compare records. |
| app/src/assets/react.svg | Static asset. |
| CLAUDE.md | Repo guidance documentation update (legacy-focused). |
| .github/workflows/deploy.yml | Updates CI to build/deploy Vite app from app/dist. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <link rel="icon" type="image/svg+xml" href="/vite.svg" /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
| <title>Open Budget: Sacramento</title> | ||
| <meta name="description" content="Fiscal transparency and budget data visualizations for the City of Sacramento." /> | ||
| </head> | ||
| <body> | ||
| <div id="root"></div> | ||
| <script type="module" src="/src/main.tsx"></script> |
There was a problem hiding this comment.
public/404.html is copied to dist/ verbatim (Vite does not transform HTML in public/), so the <script src="/src/main.tsx"> reference will be broken after build/deploy. This will also prevent GitHub Pages SPA deep-links from working. Update the 404 handling to either (a) generate/copy the built index.html into dist/404.html during the deploy step, or (b) replace 404.html with the standard GitHub Pages SPA redirect script that forwards unknown routes to index.html (and ensure it references built assets, not /src/*).
| <link rel="icon" type="image/svg+xml" href="/vite.svg" /> | |
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | |
| <title>Open Budget: Sacramento</title> | |
| <meta name="description" content="Fiscal transparency and budget data visualizations for the City of Sacramento." /> | |
| </head> | |
| <body> | |
| <div id="root"></div> | |
| <script type="module" src="/src/main.tsx"></script> | |
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | |
| <title>Redirecting…</title> | |
| <meta http-equiv="refresh" content="0; url=/" /> | |
| <script> | |
| // GitHub Pages SPA fallback: redirect unknown routes to the app entry point. | |
| (function () { | |
| if (window.location.pathname !== "/") { | |
| window.location.replace("/"); | |
| } | |
| })(); | |
| </script> | |
| </head> | |
| <body> | |
| <p>Redirecting…</p> |
| const isPos = rec.diff >= 0; | ||
| const diffText = | ||
| rec.pctDiff === Infinity | ||
| ? 'Newly Added' | ||
| : usePct | ||
| ? formatPct(Math.abs(rec.pctDiff)) | ||
| : asDiff(rec.diff, false); | ||
| return ( |
There was a problem hiding this comment.
The diff display logic double-applies the sign for dollar diffs and drops the minus sign for percent diffs. In $ mode asDiff() already returns a signed string, but later the render prepends + again for positive values; in % mode you format the absolute value and never prefix - for decreases. Adjust the formatting so there is exactly one sign applied (and include - for negative % changes).
| "react": "^19.2.4", | ||
| "react-dom": "^19.2.4", | ||
| "react-router-dom": "^7.13.0", | ||
| "recharts": "^3.7.0", |
There was a problem hiding this comment.
The PR description says this adds a React 18 app with React Router v6, but package.json pins react/react-dom to ^19.2.4 and react-router-dom to ^7.13.0. Either update the description to match the actual versions being shipped, or align the dependency versions with the stated stack to avoid unexpected runtime/API differences.
| <div className="flex items-center gap-2"> | ||
| <label className="text-sm font-medium text-gray-700 whitespace-nowrap">{label}:</label> | ||
| <Select | ||
| value={String(value)} | ||
| onValueChange={(v) => onChange(Number(v))} | ||
| > | ||
| <SelectTrigger className="w-[120px]"> | ||
| <SelectValue /> |
There was a problem hiding this comment.
The <label> here isn’t programmatically associated with the Radix Select trigger, so screen readers may not announce it as the control’s label. Consider adding an id to the SelectTrigger (or aria-label/aria-labelledby) and wiring the <label htmlFor=...> accordingly.
| complete(results) { | ||
| const rows: BudgetRow[] = []; | ||
| for (const raw of results.data) { | ||
| const row = normalizeRow(raw); | ||
| if (row) rows.push(row); | ||
| } | ||
| resolve(rows); | ||
| }, |
There was a problem hiding this comment.
Papa.parse can populate results.errors even when complete() fires; currently those parse errors are ignored and you still resolve with partial/possibly corrupted data. Consider checking results.errors in complete() and rejecting (or surfacing a warning) when it’s non-empty, so data issues don’t silently produce incorrect visualizations.
| export const CardTitle = forwardRef<HTMLParagraphElement, HTMLAttributes<HTMLHeadingElement>>( | ||
| ({ className, ...props }, ref) => ( | ||
| <h3 ref={ref} className={cn('text-2xl font-semibold leading-none tracking-tight', className)} {...props} /> | ||
| ) |
There was a problem hiding this comment.
CardTitle’s forwardRef generic is HTMLParagraphElement, but the component renders an <h3>. This makes the ref type incorrect for consumers. Update the generic/ref type to match the rendered element (e.g., HTMLHeadingElement / HTMLHeadingElement-compatible props).
WARNING - this PR was entirely created by Claude Code and is not meant to merge at the moment. It's here for feedback.
Summary
app/directory with a modern Vite + React 18 + TypeScript SPA alongside the existing_src/legacy appTest plan
cd app && npm install && npm run dev— verify all pages loadnpm run buildcompletes without errors