Skip to content

fix(scheduler): surface partial multi-subscription sweeps in collection state so the dashboard shows them as incomplete #87

Description

@cristim

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)

No activity

Activity on this issue will appear here.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions