Repository navigation
feat(plans): plan steps resolve real recommendations and show real counts and prices (tier B of #609) #631
Copy link
Copy link
Open
Labels
effort/xlMulti-week / refactorMulti-week / refactorimpact/manyAffects most usersAffects most userspriority/p1Next up; this sprintNext up; this sprintseverity/highSignificant harmSignificant harmtriagedItem has been triagedItem has been triagedtype/featNew capabilityNew capabilityurgency/this-sprintWithin the current sprintWithin the current sprint
Description
Tier B of #609. The tier A stopgap (#618, #616, merged) made empty plan steps fail loudly and show "No recommendations attached". It did not make plans work: until this issue lands, no plan can complete a step, and "Add Purchases" always returns 409.
Goal
A plan created from recommendations (provider, service, term, payment, target accounts, target coverage, ramp) schedules steps that purchase real recommendations, and the planned-purchases list shows real counts, resource types, upfront cost and savings instead of placeholders.
Proposed shape
- Resolve the matching stored recommendations for the plan's provider, service, term, payment and target accounts at execution time, not at plan creation, so a step buys what is current when it runs.
- Size each step from the ramp:
PercentPerStepof the resolved set, per the target-coverage meaning chosen under D1. - Savings Plans steps are sized by
HourlyCommitment, not instance count. - Store the resolved recommendations on the execution, so approval, history and retry see the same rows.
- Planned-purchases preview: compute count, resource types, upfront and savings for pending steps from the current matching recommendations, labelled as an estimate until the step runs.
- Reuse
createPurchaseExecutionsTx(ramp lock #1861, creator stamp #950), which tier A kept without a production caller. Re-enable "Add Purchases" through it. - Decide the dropped
recommendationsfield ofPlanRequest: honour it (per D2) or stop sending it fromsavePlan(frontend/src/plans.ts). - Keep the tier A guards as the fallback: a step that resolves to nothing fails once and does not advance the ramp.
Open product decisions (block the design)
- D1. Does "target coverage 80%" mean "buy 80% of the matching recommendations" or "reach 80% account coverage"? The second needs a coverage source, which is unknown for Azure.
- D2. Is the snapshot sent by "Plan from N selected" binding, or does the plan filter win?
Known gaps to cover
- Recommendations with no account (Azure org-wide) never match a
cloud_account_id = ANY(...)filter (buildRecommendationFilter). The step resolution must decide how to treat them, and the Opportunities page applies suppressions, account overrides and provider/allowed-account filters on top of the stored list; the plan path must match. - Completed empty rows from before tier A already advanced the ramp. Count them with
status='completed' AND recommendations='[]' AND plan_id IS NOT NULL; they are left in place. - Auto-Purchase-OFF plans only reach execution through a manual approve/run.
- Dashboard still renders $0 for bare rows (fix(dashboard): upcoming purchases show zeros for plan steps with no recommendations #620).
- Azure costs feeding any preview depend on the go#267 unit fix being re-pinned (sec(azure): purchase spend caps and recorded Azure costs use lookback-window amounts (cap can be exceeded; stale rows after the fix) #612).
Acceptance
- A plan like the one reported (Azure compute, 1 year, 80% target coverage, weekly 25% ramp, one target account, Auto-Purchase on) produces steps with non-zero counts and real upfront and savings figures, and a scheduled step executes against real recommendations.
- A step with nothing to buy fails once, with one failed row, and the ramp does not advance.
- Regression tests use the real data shape of that plan and fail on current main.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
effort/xlMulti-week / refactorMulti-week / refactorimpact/manyAffects most usersAffects most userspriority/p1Next up; this sprintNext up; this sprintseverity/highSignificant harmSignificant harmtriagedItem has been triagedItem has been triagedtype/featNew capabilityNew capabilityurgency/this-sprintWithin the current sprintWithin the current sprint
Activity
fix(plans): point plan-step refusal messages at the open tracking iss…
fix(purchase): scope plan fan-out recommendations to each account (#793)
fix(purchase): release plan-step suppressions only when no purchase c…
test(purchase): pin that post-submission failures keep plan-step supp…
feat(purchase): atomic plan-step claim, shared suppressions and accou…
feat(planstep): resolve a plan step to sized, priced recommendations …
feat(purchase): per-plan step commitment cap, auto-purchase kill swit…
feat(purchase): make notified real and pin the scheduler claim to the…
23 remaining items
cristim commentedon Oct 14, 2026
The guarded planned-purchase creator is merged in PR873 as
5134af444026276c6cdbf7a135ca00e54d656ef0. It replaces the unconditional 409 refusal with authenticated, locked and atomic scheduling, retaining account scope, creator, cap, global-switch, requested-date, veto and unsupported-SP controls.Independent exact-head native PostgreSQL verification exercised real routed creation, stored recommendations and attachment, then manual/global-OFF and automatic/global-ON paths through recording providers to durable completion with 20 Azure units, a 2400 full-term commitment, frozen quantities, correct next-step dates and no repeat purchase. All 33 expanded cases passed, including shared-fixture pause/run-now consumers, caps, creator/scope, clock and SP guards. Parent versions failed the intended refusal/credential-dispatch assertions, and scope/anchor deletion controls failed their regressions. Authentication, credentials, email, settings, clocks and provider calls use explicit fixtures, with no live purchases performed.
All required checks passed on reviewed head
e25ae316057f67162b6ff0c3e2164fa22ac20024before normal merge. Automatic post-merge deployment and checks are being watched, with read-only DEV verification to follow. No live plan or purchase was created during this work.Existing #631 work still pending: the Add Purchases UI date default needs to respect the notification/veto window, and attached planned purchases need to show their summed units and known type/region instead of row counts and generic labels. Those reviewed follow-ups remain in the delivery queue, so this backend merge doesn't claim the whole parent issue is complete.
cristim commentedon Oct 14, 2026
The guarded creator from #873 is now verified in the running deployment. Public GET
/versionreturned HTTP 200 withgit_sha=5134af4and build time20261014233006. The running image digest matches the image tagged with that commit and timestamp; deployment run 38654357934 succeeded at full commit5134af444026276c6cdbf7a135ca00e54d656ef0. All four post-merge workflows completed successfully.This confirms the deployed source version. No live plan or purchase was created, no scheduler/configuration state was changed, and no cloud purchase was performed. Authenticated plan listing was not exercised because this verification lane had no existing API session. The real local API/PostgreSQL and recording-provider execution evidence remains recorded above.
The quantity/resource display correction continues in #876. The existing UI date-default improvement and permanent combined regression proof remain tracked under #631.
cristim commentedon Oct 15, 2026
Merged #876 normally as
9da13c6cefee4dbbdb9515c483ed92acaf5ae81afrom reviewed head8c6c54c1911e51b680ebe943febfc1d6485fa70b. The merged tree exactly matches the independently reviewed tree. All required checks passed, including the one targeted integration retry and its fresh summary; no protection bypass, force-cancel or source change was used.The planned-purchases API now sums attached recommendation quantities and reports resource type and region when consistently known. Real local API/PostgreSQL verification confirms 20 / Standard_D2s_v3 / westeurope for the Azure regression, with unchanged-parent failures and independent mutation controls recorded above. Provider-specific raw unit semantics and existing authorization, caps and execution controls are preserved.
All four automatic post-merge workflows have individual background watchers. Deployment and authenticated live-listing results are not claimed yet. The already verified 374-line combined API/execution regression will be retained in a separate test-only PR; the existing UI date-default correction is #877. No live purchase or manual deployment was performed.
cristim commentedon Oct 15, 2026
The running deployment now reports #876's source version: public GET
/versionreturned HTTP 200 withgit_sha=9da13c6and build time20261015011008. Lambda is Active/Successful, and its resolved image digest matches the image tagged with that commit and timestamp. Automatic deployment 38664043722 succeeded at full commit9da13c6cefee4dbbdb9515c483ed92acaf5ae81a.This is read-only deployed-source verification. Authenticated live planned-purchase listing was not exercised because this lane had no existing API session. The affected GET response and quantities were verified through the real local handler/PostgreSQL path recorded above. No live purchase, configuration change or manual deployment occurred.
Post-merge frontend and pre-commit workflows succeeded. The full main CI watcher remains active. The separate permanent combined regression-test leaf is committed locally and undergoing fresh independent verification; the existing UI date-default fix is #877.
cristim commentedon Oct 15, 2026
All four post-merge workflows for
9da13c6cefee4dbbdb9515c483ed92acaf5ae81aare now successful: full CI 38664043763, pre-commit 38664043741, frontend sentinel 38664043804 and automatic deployment 38664043722. The full CI watcher encountered a transport timeout; GitHub's actual terminal result was checked directly and confirmed successful, so no test retry or source change was needed.The read-only deployed-source verification is recorded above. The combined regression is now tracked in test-only #878; the existing UI date-default correction is #877. No live purchase or authenticated live planned-listing result is claimed.
cristim commentedon Oct 15, 2026
Permanent regression coverage is merged in #879 (f038e2d), following the guarded creator #873 and truthful quantity projection #876. Exact-SHA independent review, real local API/Postgres/resolver/manager tests with a recording-only provider, nil-assertion sensitivity, normal hooks and required CI passed. #878 is closed as superseded with its original history preserved. Post-merge workflows are watched and still pending. Existing UI default-date work remains tracked in #877; graph-editor work and long-term execution-retention concerns remain separately coordinated. No live purchase or authenticated production plan-creation write was performed.
cristim commentedon Oct 15, 2026
Post-merge verification:
149544fe7d57f1b7b0b4cf651d8fb9f8ba3c7920has parentf038e2deand the expected treeebd2a5b6, preserving fix(api): show attached planned purchase quantities #876 and test(api): retain guarded planned purchase execution proof #879./version, reportinggit_sha: 149544fand build20261015051348. The running Lambda and corresponding image digest matched the same build/source.The date-default fix is complete. The broader graph editor remains tracked with #631; #880 is the separate no-write preview leaf, while atomic graph scheduling is still being developed. This fix does not close that broader work.
feat(plans): preview draft schedules from the plan form (#880)
cristim commentedon Oct 15, 2026
PR #880 normally squash merged as
64ec8b3fe12de04964837a3c68b1957a2c7dccf4, with parent149544feand the exact independently reviewed treeb3514bf0. All required checks were green at the reviewed PR head before merge.All five post-merge workflows passed: full CI, automatic deployment, frontend E2E, frontend build sentinel and pre-commit. Every CI watcher and the completed PR merge watcher has been reaped.
Read-only deployed identity verification returned HTTP200 from
/version, reportinggit_sha: 64ec8b3and build20261015055918. The running Lambda and corresponding image digest matched the same build/source. This verifies deployed identity; no authenticated live plan or purchase was changed.The actual Create/Edit form preview was independently verified locally from the production bundle at desktop and phone widths, including dense schedules, keyboard interaction, focus trapping and restoration, and zero API writes. The browser APIs/authentication were fixtures. This leaf adds the disclosed no-write preview; existing Save and Add Purchases actions remain. Atomic graph scheduling and saved history integration remain tracked in #631 and are not claimed complete by this PR.
feat(plans): edit local draft schedule dates (#882)
cristim commentedon Oct 15, 2026
Reopening per owner decision (2026-10-16) to track the remaining laddering plan-editor work end to end: M2 managed-ramp chain, ramp-ledger retention, legacy-writer guards, M3 atomic save command, preview/Save API routes, frontend Save (E4), and the end-to-end proof. PRs will say "Refs #631" and mirror this issue's labels; the final route-exposure PR closes it.
test(plans): compare the legacy next date at Postgres microsecond pre…
fix(plans): lock the plan before completing a target-met step
test(plans): drop the total arg from the target-met clock fixture call