Status
The client-side half of this landed in LeanerCloud/cloud-commitments-cli#1520 via e551a02a3 and 66bcbfa31.
MultiSubscriptionRecommendationsClient now returns a
PartialSubscriptionFailureError carrying the succeeded/failed subscription
counts and per-subscription causes, and internal/scheduler/scheduler.go
routes all six recommendation-fetch call sites through a single
tolerateIncompleteSweep helper so a partial sweep keeps the subscriptions
that did answer instead of discarding them.
This issue is retargeted to the remaining half: making an incomplete sweep
visible outside the logs.
Remaining gap
tolerateIncompleteSweep records the incompleteness with logging.Warnf and
returns nil. The author documented the limitation at
internal/scheduler/scheduler.go:912-915:
NOTE: this records the incompleteness in the log only. Surfacing it in the
state table's last_collection_error (so the dashboard shows the sweep as
partial) needs a partial-note threaded through
collectProviderRecommendations and its three per-provider implementations,
which is left as follow-up work.
So the original complaint survives, relocated from the client layer to the
observability layer: an operator looking at the dashboard still cannot
distinguish a sweep that covered every subscription from one that covered 1 of
10. A partial sweep still presents as complete org-wide coverage, and the
recommendations it produced still look like the full picture. Someone reading a
shrunken savings figure has no way to tell an under-collected sweep from a
genuinely shrinking opportunity without going to the logs, which nobody does
routinely.
This matters most on the scheduled path, which runs unattended.
Proposed direction
Thread a partial-sweep note from tolerateIncompleteSweep through
collectProviderRecommendations and its three per-provider implementations,
and record it where the UI can see it:
- populate the state table's
last_collection_error (or an adjacent
partial-specific column) with the succeeded/attempted counts and the failed
subscription IDs;
- surface it in the dashboard so a partial sweep is visibly marked as such
rather than rendering identically to a complete one;
- keep the current behaviour of returning the partial data. The point is to
annotate the result, never to discard it.
The error type already carries everything needed (Succeeded, Attempted,
FailedSubscriptionIDs()), so this is a plumbing and presentation change
rather than new detection logic.
Acceptance
- A partial sweep is distinguishable from a complete one in the UI, without
reading logs.
- The successful subscriptions' recommendations are still stored and shown.
- A regression test covering the scheduled path: a sweep where a strict subset
of subscriptions fails records both the surviving recommendations and the
partial-sweep marker.
Reference
Original CodeRabbit finding on LeanerCloud/cloud-commitments-cli#1520:
LeanerCloud/cloud-commitments-cli#1520 (review)
Findings from the 2026-09-02 codebase audit
Added by an automated audit of 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd (tip of origin/main). Each item below was reported by one reviewer and independently confirmed by a second that did not write it. Full report: docs/audits/codebase-audit-2026-09-02.md.
A16-007 (high)
A coverage gap in the half of this that already landed. internal/scheduler/partial_sweep_eviction_test.go is a careful guard for "an incomplete sweep must not authorize stale-row eviction", but every case drives Azure: the provider stub, the three collectAzureRecommendations cases (lines 106, 128, 174) and the fanOutPerAccount case at 148 all pass "azure". collectGCPForAccount carries its own inlined copy of the same plumbing and is entirely uncovered (scheduler.go:883-912), as is collectAWSForAccount (752-770). Hardcode true in place of complete at scheduler.go:912 and a partial GCP sweep enters outcome.SucceededAccountIDs, which authorizes the DELETE ... WHERE collected_at < $1 AND (provider, account_key) IN (...) inside UpsertRecommendations -- deleting the previous cycle's recommendations for every project the sweep never queried -- while the whole Azure suite stays green. The shared helper is directly tested at scheduler_test.go:800-814; what is untested is the per-provider wiring of its complete return. Parameterising newPartialSweepScheduler over the provider name and rerunning the three cases would close it. (audit finding A16-007)
Status
The client-side half of this landed in LeanerCloud/cloud-commitments-cli#1520 via
e551a02a3and66bcbfa31.MultiSubscriptionRecommendationsClientnow returns aPartialSubscriptionFailureErrorcarrying the succeeded/failed subscriptioncounts and per-subscription causes, and
internal/scheduler/scheduler.goroutes all six recommendation-fetch call sites through a single
tolerateIncompleteSweephelper so a partial sweep keeps the subscriptionsthat did answer instead of discarding them.
This issue is retargeted to the remaining half: making an incomplete sweep
visible outside the logs.
Remaining gap
tolerateIncompleteSweeprecords the incompleteness withlogging.Warnfandreturns
nil. The author documented the limitation atinternal/scheduler/scheduler.go:912-915:So the original complaint survives, relocated from the client layer to the
observability layer: an operator looking at the dashboard still cannot
distinguish a sweep that covered every subscription from one that covered 1 of
10. A partial sweep still presents as complete org-wide coverage, and the
recommendations it produced still look like the full picture. Someone reading a
shrunken savings figure has no way to tell an under-collected sweep from a
genuinely shrinking opportunity without going to the logs, which nobody does
routinely.
This matters most on the scheduled path, which runs unattended.
Proposed direction
Thread a partial-sweep note from
tolerateIncompleteSweepthroughcollectProviderRecommendationsand its three per-provider implementations,and record it where the UI can see it:
last_collection_error(or an adjacentpartial-specific column) with the succeeded/attempted counts and the failed
subscription IDs;
rather than rendering identically to a complete one;
annotate the result, never to discard it.
The error type already carries everything needed (
Succeeded,Attempted,FailedSubscriptionIDs()), so this is a plumbing and presentation changerather than new detection logic.
Acceptance
reading logs.
of subscriptions fails records both the surviving recommendations and the
partial-sweep marker.
Reference
Original CodeRabbit finding on LeanerCloud/cloud-commitments-cli#1520:
LeanerCloud/cloud-commitments-cli#1520 (review)
Findings from the 2026-09-02 codebase audit
Added by an automated audit of
3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd(tip oforigin/main). Each item below was reported by one reviewer and independently confirmed by a second that did not write it. Full report:docs/audits/codebase-audit-2026-09-02.md.A16-007 (high)
A coverage gap in the half of this that already landed.
internal/scheduler/partial_sweep_eviction_test.gois a careful guard for "an incomplete sweep must not authorize stale-row eviction", but every case drives Azure: the provider stub, the threecollectAzureRecommendationscases (lines 106, 128, 174) and thefanOutPerAccountcase at 148 all pass"azure".collectGCPForAccountcarries its own inlined copy of the same plumbing and is entirely uncovered (scheduler.go:883-912), as iscollectAWSForAccount(752-770). Hardcodetruein place ofcompleteat scheduler.go:912 and a partial GCP sweep entersoutcome.SucceededAccountIDs, which authorizes theDELETE ... WHERE collected_at < $1 AND (provider, account_key) IN (...)insideUpsertRecommendations-- deleting the previous cycle's recommendations for every project the sweep never queried -- while the whole Azure suite stays green. The shared helper is directly tested at scheduler_test.go:800-814; what is untested is the per-provider wiring of itscompletereturn. ParameterisingnewPartialSweepSchedulerover the provider name and rerunning the three cases would close it. (audit finding A16-007)