feat: Polish — ship progress export (U11) - #3
Merged
Conversation
Set up the project's first test infrastructure: Vitest (jsdom + Testing Library) for unit/component tests, Playwright for E2E, and a GitHub Actions workflow gating PRs on lint + typecheck + test. Replace the no-op ESLint config with eslint-config-next's native flat core-web-vitals rules (no FlatCompat). Pre-existing react-hooks violations are downgraded to warnings so the gate is green from day one, to be burned down in the depth/polish pass. Seed one passing test per layer (unit, component, e2e); e2e runs locally and is not in the CI gate. Implements U2 of docs/plans/2026-06-17-001-feat-maximize-ironforge-plan.md Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tests Move the inline rule-of-thumb figures (burden %, $/ton, profit, bonding, etc.) into lib/estimator/constants.ts; clamp negative/NaN numeric inputs at the calculation boundary so a bypassed HTML min hint can't poison a bid; derive the bonding markup from PROFIT_PCT/CONTINGENCY_PCT so it can't drift from the profit/contingency constants. Add 13 tests: golden total, per-ton + compounding-order locks, monopolistic-WC and prevailing-wage behavior, and clamping edge cases. Implements U3 of docs/plans/2026-06-17-001-feat-maximize-ironforge-plan.md Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Make data provenance a first-class field: StateVerification (lastVerified / sourceUrl / effectiveDate) on StateData, backfilled for the hand-crafted WA and OR states. getStateVerification() resolves a well-formed view for every state — verified states carry their date, machine-generated states report unverified rather than a misleading one. A persistent, non-dismissable LegalDisclaimer renders on every wizard step: "not legal advice — verify with your state agency", plus the last-verified date for verified states or a "not independently verified" notice for the machine-generated ones. Implements U5 of docs/plans/2026-06-17-001-feat-maximize-ironforge-plan.md Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
getPhaseContent fell back to Washington's content for any state missing from the registry — silently presenting one state's regulatory facts as another's. Replace all six fallbacks with an explicit, labeled "data unavailable for <state>" phase that points the user at their own agency. Add a contract test asserting all 50 states x 7 phases resolve to valid non-empty steps, that a non-WA state gets generated (not Washington) content, that an unknown code gets the labeled phase (never Washington), and that the chat-route step lookup finds a phase's steps. Implements U4 of docs/plans/2026-06-17-001-feat-maximize-ironforge-plan.md Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The proxy enforced same-origin only on /api/chat. Generalize it to every state-changing API request (non-GET/HEAD on /api/*), so /api/bid-review and /api/onboarding are no longer open to cross-site calls. Move the full security-header set (CSP, HSTS, frame/permissions policy) into next.config.ts as the single source of truth — it covers static assets the middleware matcher excludes — and drop the duplicate weaker set. Narrow the proxy matcher to /api/*. Rate limiting stays in the route handlers (in-memory, per-instance; a durable limiter is plan U10). Implements U6 of docs/plans/2026-06-17-001-feat-maximize-ironforge-plan.md Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
From the foundation code review (no P0/P1; these are the high-value P2/P3s): - Clamp estimator inputs to MAX_FIELD_VALUE (1e9) so an absurd or tampered value (e.g. 1e307) can't overflow the bid to Infinity/NaN — the clamp's stated invariant was false for large finite values. - Add the flagged missing tests: overflow finiteness; estimator cost-line flags in the "on" direction, non-union burden, and cost-per-sqft; wizard navigation (getNextStep/getPrevStep across phase boundaries); and the unavailable / legal-federal / unknown-phase content paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CI's node 22 npm rejected the lockfile (Missing @swc/helpers@0.5.23) while node 24 (local dev + lockfile generator) accepts it via `npm ci`. Align CI to node 24 so the clean install matches the committed lockfile. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the disabled "Export Progress — coming soon" button on the summary dashboard with working JSON and CSV downloads (pure builders in lib/export/progress-export.ts, tested). Remove the dead "d.c." alias in the onboarding parser — DC isn't one of the 50 supported states, so it always fell through to not-found — and add the parser's first tests. Implements U11 of docs/plans/2026-06-17-001-feat-maximize-ironforge-plan.md Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (33)
📝 WalkthroughWalkthroughThis PR bootstraps a full test infrastructure (Vitest, Playwright, CI), refactors the Next.js middleware to a uniform CSRF same-origin gate with CSP/HSTS headers, extracts a shared SSE stream utility consumed by the wizard chat flow, introduces a ChangesPhase 1 Foundation
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser (Wizard Step)
participant proxy as proxy.ts Middleware
participant ChatAPI as /api/chat
participant readChatStream as readChatStream
Browser->>proxy: POST /api/chat (Origin: same-host)
proxy->>proxy: isSameOrigin(req) → true
proxy->>ChatAPI: NextResponse.next()
ChatAPI-->>Browser: 200 SSE stream (ReadableStream)
Browser->>readChatStream: readChatStream(res.body, setStreamingContent)
loop SSE frames
readChatStream->>readChatStream: parseSseChunk → extract text delta
readChatStream-->>Browser: onProgress(accumulated text)
end
readChatStream-->>Browser: final assembled message string
Browser->>Browser: append assistant message to chat history
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed3beac39a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| phasesComplete: summary.phasesComplete, | ||
| totalPhases: PHASE_DEFINITIONS.length, | ||
| startedAt: summary.startedAtIso, | ||
| phases: summary.phaseSummaries.map((p) => ({ |
There was a problem hiding this comment.
When a user downloads JSON to preserve or audit their wizard progress, this builder serializes summary.phaseSummaries rather than userState.progress, so the file only has per-phase counts and loses every visited step id and completedChecklist item id. Two different users can produce identical exports even though they checked different items, making the progress export unrecoverable/incomplete; include the step-level progress (or raw userState.progress) in the JSON payload.
Useful? React with 👍 / 👎.
Pull the ~25-line manual SSE parse loop out of the 617-line wizard step page into lib/chat/stream.ts (parseSseChunk + readChatStream), unit-tested in isolation: text frames, [DONE], malformed JSON, non-string text, cumulative progress, empty stream. Behavior-preserving — the page now calls readChatStream(res.body, setStreamingContent). Part of U12 of docs/plans/2026-06-17-001-feat-maximize-ironforge-plan.md Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
readChatStream fed each network read straight to parseSseChunk with no carry-over, so a `data:` frame split mid-line across reads dropped the delta silently (latent bug carried over from the pre-extraction inline code). Buffer until a newline, parse complete lines only, retain the trailing partial for the next read, and flush on stream end. Adds the coverage the review flagged: split-frame reassembly, multiple frames per chunk, no-trailing-newline flush, and CSV quote/newline escaping + exact-row assertions for the export. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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
1 participant
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.
First slice of Phase 3 (Polish) of the maximization roadmap. Stacked on PR #2 (foundation) → auto-retargets down the chain as the parent PRs merge.
U11 — finish/cull half-built features
lib/export/progress-export.ts, fully tested (valid JSON, CSV header/rows, comma-escaping, empty-progress well-formed)."d.c."state alias in the onboarding parser — DC isn't one of the 50 supported states, so it always fell through to not-found. Added the parser's first unit tests.Verification
tsc --noEmitclean · lint clean.Deferred (rest of Phase 3 — verification-gated, not done here)
ANTHROPIC_API_KEYto verify cache hits (cache_read_input_tokens); shipping it blind would be unverified. Best done where the key is available.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Chores