feat(041): saved forecast scenarios — persist, load, rename, delete named parameter sets#122
Conversation
…narios Plan drafted from the 036 deferral (forecast_scenarios table sketch) and challenged by a 5-lens adversarial review before implementation; revisions folded in (deterministic active-budget resolution, audit snapshots on update, 23505 mapping, state-model fixes, rename-only path, test feasibility). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- forecast_scenarios table (migration 0026): budget-scoped, case-insensitive unique names via expression index, jsonb ForecastInputs params, creator attribution (set null), FK + created_by indexes - CRUD server actions: deterministic active-budget resolution (fiscal_year DESC), Zod validation on write AND read (non-conforming rows skipped), PG 23505 mapped to the friendly duplicate-name error, full params snapshots in change_history on update/delete, 50-per-budget cap - Engine: pure inputsFromSaved() re-bases saved params onto the current dataset (extinct keys dropped, new tools defaulted); also re-bases surviving client state when the dataset prop refreshes - UI: persistent Saved scenarios card (load/rename/delete, recomputed figures vs saved ceiling, creator captions), save dialog with update-vs-new mode toggle, AlertDialog delete confirm, StatusText feedback; saved:* activates the Your-plan comparison row; preset clicks clear the update offer - Tests: engine merge incl. billing round-trip, validator matrix + type-level round trip, auth gating, real-DB integration round trip with audit/index/read-skip assertions (suite owns the highest seeded FY) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…shared resolvers, audit helper 4 parallel cleanup reviews (reuse / simplification / efficiency / altitude); fixes applied: - Drop the nameTaken pre-check: the unique expression index + 23505 catch is the single source of duplicate-name rejection (same friendly error, one fewer round trip, no race window). This exposed a real latent bug the pre-check had masked: drizzle wraps driver errors, so the 23505 code lives on error.cause — the detector now walks the cause chain (integration test exercises the catch path and passes). - Lift the deterministic active-budget rule into lib/budget-utils.ts (getActiveBudgetId) and align getActiveBudget()'s ordering with it — the chart and scenario list can no longer resolve different budgets. - Extract recordDeletion() into actions/history.ts (third copy of the delete-snapshot pattern); fold the delete's fetch into .returning(). - One shared dialogStatus instead of three; dialog targets hold the whole SavedForecastScenario; dead validator exports removed; key-set equality type assertion catches a forgotten optional lever where assignability cannot. 648 unit / 36 integration / typecheck / lint green after the pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Implements spec 041 by adding persistence for Budget / Cost Forecast Simulation “saved scenarios”: admins can save, load, rename, update, and delete named ForecastInputs parameter sets scoped to the active budget, with server-side validation and audit history.
Changes:
- Added
forecast_scenariostable (migration 0026) with case-insensitive per-budget name uniqueness and attribution. - Implemented CRUD server actions (admin-gated), plus UI to list/load/save/rename/delete scenarios and re-base saved params onto the current dataset.
- Added unit + integration tests covering validator boundaries, engine rebase logic, auth gating, and real-DB CRUD/audit/index behavior.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/scenarios/forecast-scenario-validators.test.ts | New unit coverage for Zod boundary schemas and type-level round-trip checks. |
| tests/unit/scenarios/budget-forecast.test.ts | Adds unit tests for inputsFromSaved rebase behavior and edge cases. |
| tests/unit/actions/forecast-scenarios-auth.test.ts | Verifies admin gating for scenario actions and “DB untouched” contract. |
| tests/integration/forecast-scenarios.test.ts | Real-DB CRUD/audit/index/cap tests for forecast scenarios. |
| src/lib/validators.ts | Adds forecast-scenario Zod schemas for params/name and CRUD inputs. |
| src/lib/scenarios/budget-forecast.ts | Adds inputsFromSaved() helper to re-base saved inputs onto current dataset. |
| src/lib/db/schema.ts | Adds forecast_scenarios table + relations and indexes. |
| src/lib/db/migrations/meta/_journal.json | Registers migration 0026 in the journal. |
| src/lib/db/migrations/0026_nebulous_shadow_king.sql | Creates forecast_scenarios table, FKs, indexes, and expression unique index. |
| src/lib/budget-utils.ts | Adds getActiveBudgetId() and related formatting changes. |
| src/app/scenarios/budget-forecast/page.tsx | Loads saved scenarios alongside the dataset and passes into client. |
| src/app/scenarios/budget-forecast/budget-forecast-client.tsx | Adds saved-scenario UI + dialogs and mutation flows. |
| src/actions/history.ts | Adds recordDeletion() helper for delete audit snapshots. |
| src/actions/forecast-scenarios.ts | New CRUD server actions for forecast scenarios (list/create/update/delete). |
| src/actions/budget.ts | Makes active budget resolution deterministic (order by fiscal year DESC). |
| specs/041-forecast-scenario-persistence/implementation-plan.html | Adds detailed implementation plan documentation for spec 041. |
| specs/041-forecast-scenario-persistence/implementation-notes.html | Adds running implementation notes and verification log. |
| CLAUDE.md | Notes spec 041 feature in “Recent Changes”. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function submitSave() { | ||
| const name = saveName.trim(); | ||
| if (!name) return; | ||
| startTransition(async () => { | ||
| const result = | ||
| saveMode === "update" && loadedScenario | ||
| ? await updateForecastScenario({ | ||
| id: loadedScenario.id, | ||
| name, | ||
| params: inputs, | ||
| }) | ||
| : await createForecastScenario({ name, params: inputs }); | ||
| if (result.success) { | ||
| setSaveOpen(false); | ||
| // The current inputs now match the saved row — mark it loaded/active. | ||
| setLoadedSavedId(result.data.id); | ||
| setActive(`saved:${result.data.id}`); | ||
| cardStatus.ok("SAVED"); | ||
| router.refresh(); | ||
| } else { | ||
| dialogStatus.error(result.error); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Addressed in d8271e5, with one correction on the premise. On React 19 (this repo runs 19.2.4), startTransition with an async function is a supported Action and isPending covers the whole round trip — per the current docs: "the isPending state switches to true at the first call to startTransition, and stays true until all Actions complete". Every submit/cancel/delete button here disables on isPending, so buttons cannot re-enable mid-request and duplicate submissions/double-deletes are not possible (this is also the established pattern in ingestion-filters-section.tsx). The second half of the comment is the documented React 19 caveat and is real, so d8271e5 adopts the docs' recommended shape: the post-await success-path state updates (router.refresh(), dialog close, setters) are now wrapped in an inner startTransition. The error path stays urgent deliberately — an inline error should render immediately.
| gte(budgetPeriods.endDate, invoiceDate), | ||
| ), | ||
| ) | ||
| .orderBy(desc(annualBudgets.createdAt)) |
There was a problem hiding this comment.
Fixed in d8271e5 — findActivePeriodForDate now tie-breaks multiple active budgets by fiscal_year DESC, the same deterministic rule as getActiveBudgetId() / getActiveBudget(). Good catch: under the multi-active state this could have routed an invoice to a different budget's period than the one the rest of the page resolves.
| .where( | ||
| sql`${budgetPeriods.startDate} <= ${invoiceDate} AND ${budgetPeriods.endDate} >= ${invoiceDate}` | ||
| sql`${budgetPeriods.startDate} <= ${invoiceDate} AND ${budgetPeriods.endDate} >= ${invoiceDate}`, | ||
| ) | ||
| .orderBy( | ||
| sql`CASE WHEN ${annualBudgets.status} = 'active' THEN 0 ELSE 1 END ASC`, | ||
| desc(annualBudgets.createdAt) | ||
| desc(annualBudgets.createdAt), | ||
| ) | ||
| .limit(1); |
There was a problem hiding this comment.
Fixed in d8271e5 — findPeriodForDate's secondary sort (after the active-first CASE) is now fiscal_year DESC, consistent with the contract this PR introduces.
| // other integration suites seed in parallel (budget-extensions: 2090-2098, | ||
| // invoice-sync: up to ~30,259). This suite must own the HIGHEST fiscal year | ||
| // in the whole integration run — keep this range above every other suite's. | ||
| const ACTIVE_FY = 999_000 + Math.floor(Math.random() * 999); |
There was a problem hiding this comment.
Fixed in d8271e5 — range widened from 999 to 900,000 values (FY 999,000–1,898,999). Still strictly above every other suite's seeded range (budget-extensions: 2090–2098, invoice-sync: ≤ ~30,259), so the highest-fiscal-year resolution contract keeps isolating this suite while the collision probability on the unique column becomes negligible for parallel/retried runs.
…-utils, wider test FY range, React 19 transition caveat - findActivePeriodForDate / findPeriodForDate now tie-break multiple active budgets by fiscal_year DESC, matching getActiveBudgetId()'s contract (previously createdAt — could resolve a different budget than the rest of the page under the multi-active state) - integration suite FY range widened from 999 to 900k values (fiscal_year is unique; avoids collision flakes on parallel/retried runs) - post-await state updates in the save/rename/delete handlers wrapped in an inner startTransition per the documented React 19 caveat (isPending already spans the whole async Action, so the double-submit concern did not apply — see in-thread reply) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Summary
Ships the follow-up spec 036 explicitly deferred: persistence for the Budget / Cost Forecast Simulation. An admin who has tuned a plan (levers, edited ceiling, Claude billing cadence) can save it under a name, reload it later from any session, rename it, update it after refinement, and delete it — shared across all admins, scoped to the active fiscal-year budget. Spec folder:
specs/041-forecast-scenario-persistence/(implementation plan — challenged by a 5-lens adversarial review before any code — plus running implementation notes covering design decisions, tradeoffs, and open questions).Data model (migration 0026)
One new table,
forecast_scenarios:budget_idFK (cascade),nameunique per budget case-insensitively via the repo's firstlower(name)expression index,paramsjsonb holding the fullForecastInputs,created_byattribution (set null), timestamps, FK indexes. Purely additive; reviewed by the drizzle-migration-reviewer agent (its one finding — a missingcreated_byindex — was fixed before the migration was committed).Server actions (
src/actions/forecast-scenarios.ts)listForecastScenarioscreateForecastScenarioActionResulterror (race-free single mechanism)updateForecastScenarioparamsoptional so rename never silently rewrites assumptions; full params snapshots inchange_history(overwrites are recoverable)deleteForecastScenariodelete().returning()+recordDeletion()snapshot (new shared helper inactions/history.ts)The active budget resolves deterministically (highest fiscal year wins) via a new shared
getActiveBudgetId()inlib/budget-utils.ts;getActiveBudget()now orders by the same rule so the chart and the scenario list can never resolve different budgets.Engine + UI
inputsFromSaved(ds, saved)re-bases any saved parameter set onto the current dataset (extinct tool keys dropped, new tools defaulted) — used for loading and for re-basing surviving client state when the dataset prop refreshes mid-session.saved:*activates the "Your plan" comparison row; preset-tile clicks clear the Update offer (no one-click overwrite of a shared row);StatusTextfeedback throughout (no Sonner).Process
Plan → 5-lens adversarial challenge (1 must-fix, 10 adopted should-fixes) → implement → 4-lens simplify pass. The simplify pass exposed a real latent bug: drizzle wraps driver errors, so the 23505 code lives on
error.cause— the pre-check it removed had been masking a detector that never fired. Fixed and now covered by the integration test.Verification
inputsFromSavedmerge matrix incl. yearly-billing round trip; validator accept/reject/strip matrix + type-level round trip and key-set equality (catches a forgotten optional lever atpnpm typecheck); auth gating (list →[], mutations → Unauthorized, DB untouched).change_historyassertions (create / rename+params snapshots / delete snapshot round-trip); case-insensitive duplicate rejected via the action and via direct insert (the expression index itself proven); malformed-jsonb row skipped by the read path; 50-cap via bulk seed. Suite seeds FY ≥ 999,000 so the deterministic resolution contract isolates it from every other suite's active budgets.🤖 Generated with Claude Code