Post-MVP hardening: code-review cleanup, CI, and next-steps plan - #13
Merged
TimBest merged 3 commits intoMay 22, 2026
Merged
Conversation
- Extract formatMeters / formatBigUSD into lib/formatting.ts (used by IsoViz today, will be reused by future numeric displays) with dedicated tests. - Add isPositiveFinite type guard in lib/math.ts; reuse from urlState and FallbackForm to dedupe the Number.isFinite/> 0 check. - Split IsoViz into focused subcomponents (CommodityColumn, ReferenceStack, CompactReference, StackedReference) and cap the stacked-glyph DOM at 20 nodes — beyond that the compact x-multiplier form is used. - Add cache-control: public, max-age=3600 on /api/geocode and /api/footprint so deterministic lookups are CDN-friendly. - Use URLSearchParams consistently in clientFetchers buildUrl helper.
Captures what's shipping in PR #12, what's stubbed, and an ordered P0-P5 plan to finish the MVP. Lives in the repo so the plan survives across ephemeral sessions.
Runs on every push to master and on every pull request: - bun install --frozen-lockfile - bun run typecheck - bun test - bun run build Pins Bun 1.3.11 to match netlify.toml, disables Next telemetry, and cancels superseded runs on the same ref via concurrency groups.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Stacked on top of the MVP work (PR #12). This PR isolates the three post-MVP commits so they can be reviewed independently.
Base branch is
claude/commodity-visualizer-base(the MVP + Netlify baseline) rather thanmaster, so the diff shows exactly these three commits.masteris still the legacy Rust/Python code — CI can't run there. Once PR #12 lands, this PR can be retargeted tomaster.Commits
refactor: code review cleanup— applied findings from a three-agent review:formatMeters/formatBigUSDintolib/formatting.ts(+ tests)isPositiveFinitetype guard inlib/math.ts, reused inurlState.tsandFallbackForm.tsxIsoVizinto focused subcomponents; capped stacked-glyph DOM at 20 nodescache-controlheaders to/api/geocodeand/api/footprintclientFetchersviaURLSearchParamsdocs: next-steps plan—docs/NEXT_STEPS.md: MVP status snapshot and an ordered P0–P5 plan to finish the spec.ci: GitHub Actions workflow—.github/workflows/ci.yml: runsbun install --frozen-lockfile,bun run typecheck,bun test,bun run buildon every push tomasterand every PR. Pins Bun 1.3.11; cancels superseded runs via concurrency groups.Test plan
bun test— 78/78 passbun run typecheck— cleanbun run build— cleanNote
These three commits also exist on PR #12's branch (
claude/commodity-visualizer-display-netlify) — they were left there intentionally. Whichever PR merges first, git will no-op the duplicates in the other.https://claude.ai/code/session_01Df7Y187BB8uPFqRxkrJbiz
Generated by Claude Code