fix(azure): surface skipped consumption recommendations as a typed partial-result error

[cristim]

Closes #362

Root cause

providers/azure/internal/recommendations/consumption.go ExtractConsumptionOrSkip / AppendConsumptionVariants logged and returned nil for non-USD amounts and unpriceable SKUs. An EUR/GBP account got zero recommendations and a nil error.

Change

• SkipLog carried on the collection context (WithSkipLog); both skip paths record into it. ExtractConsumptionOrSkip now takes ctx (one-line call-site changes in the 7 service converters, plus a ctx param on synapse's converter).
• GetRecommendations returns the successful recommendations and a *azrecs.PartialResultError listing the skips.
• Request params and filters are unchanged (finding A1 untouched).

Choice of error variant

A typed partial-result error with the good rows retained, not a hard failure: one bad SKU must not discard the rest of a run, but must not be silent either. Callers use errors.As to keep the recs; callers that treat any error as failure see the problem. A fully skipped account returns empty recs plus the error. I could not inspect platform/MCP callers (code search returned nothing for those repos), so how they treat recs != nil && err != nil is unverified.

Evidence (fixture-based; modern payloads built from armconsumption types, no live Azure data)

Before (recommendations.go reverted, new test kept):

--- FAIL: TestGetRecommendations_NonUSDSkipIsSurfaced
    Messages: an all-EUR account must not look like zero savings: <nil>

After: go test ./... in providers/azure passes; go vet and GOWORK=off go build ./... clean; pre-commit hooks (gocyclo, vet) pass. golangci-lint could not run locally (installed binary built with go1.26, repo needs 1.27); relying on CI.

🤖 Generated with Claude Code

Update after review

• PartialResultError is now exported from providers/azure (type alias) with AsPartialResult(err) and a Partial() bool method, so it can satisfy the planned cross-provider AsPartial interface match (G6a-1 reshapes Partial() to whatever reccontract specifies).
• The multi-subscription merge keeps a subscription's recommendations when its error is a partial result and aggregates the skips (prefixed with the subscription ID) into one PartialResultError. A real subscription failure still yields PartialSubscriptionFailureError; if both occur in one sweep, the failure error wins and the skips are logged only.
• Test: TestMultiSubscriptionRecommendationsClient_PartialResultKeepsRecommendations reproduces the reviewer probe (2 subs, 1 rec + 1 skip each, now 2 recs and a partial error).

Caller changes needed (not in this PR; architect P1/C-steps)

• platform internal/scheduler/scheduler.go tolerateIncompleteSweep (~1019): also tolerate AsPartialResult (or reccontract.AsPartial).
• cli cmd/multi_service_helpers.go (~627): same.
• mcp search_recommendations.go (~266): same.
  Until then, a caller that only tolerates PartialSubscriptionFailureError treats a skip as a hard error for that run.

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 13 billable files and costs up to $3.25.}

• Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 7 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 61 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-go/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: 94225127-12e8-4e0e-9bbb-2e56c9ef128c

📥 Commits

Reviewing files that changed from the base of the PR and between 5b0fc26 and f1a9a88.

📒 Files selected for processing: 13

• providers/azure/internal/recommendations/consumption.go
• providers/azure/internal/recommendations/consumption_test.go
• providers/azure/recommendations.go
• providers/azure/recommendations_multi_subscription.go
• providers/azure/recommendations_multi_subscription_test.go
• providers/azure/recommendations_skips_test.go
• providers/azure/services/cache/client.go
• providers/azure/services/compute/client.go
• providers/azure/services/cosmosdb/client.go
• providers/azure/services/database/client.go
• providers/azure/services/managedredis/client.go
• providers/azure/services/search/client.go
• providers/azure/services/synapse/client.go

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

Independent review (claude-opus-5-5) of 2de6006: APPROVE

Re-review after my block of 83f14ed. Review only; no Azure calls needed.

The earlier blockers are fixed:

• PartialResultError is now reachable from outside the module: a type alias plus AsPartialResult in providers/azure/recommendations_multi_subscription.go.
• mergeSubscriptionResults keeps a subscription's recommendations on a partial result and aggregates the skips with the subscription ID as a prefix. A real failure still yields PartialSubscriptionFailureError.

Evidence (local, fresh clone, macOS, fixture-based):

• go vet ./... and go test ./... in providers/azure pass.
• TestMultiSubscriptionRecommendationsClient_PartialResultKeepsRecommendations reproduces my two-subscription probe. Disabling the new partial branch in mergeSubscriptionResults makes it fail; at 83f14ed the same probe returned 0 recs and "all 2 Azure subscriptions failed".
• CI: every check on this head is green.

Non-blocking:

1. Partial() bool (internal/recommendations/consumption.go:330): the comment says "the cross-provider partial-result check matches on this method", but nothing in this repo, cloud-commitments-platform, -cli or -mcp calls it. The approved plan reshapes it to Partial() *reccontract.PartialError in G6a-1, with the same name and a different signature, so any consumer that adopts interface{ Partial() bool } in the meantime would break. I suggest dropping the method (and its test assertion) until G6a-1 adds the real contract, or at least fixing the comment.
2. When a real failure and skips occur together, only the failure is returned and the skips are logged. I find this acceptable: the run is already flagged incomplete, and the platform withholds stale-row eviction for PartialSubscriptionFailureError. A Skipped field on that error would make the skips visible to callers too.
3. Consumers must handle the new error when they bump this module. platform internal/scheduler/scheduler.go tolerateIncompleteSweep, cli cmd/multi_service_helpers.go:627 and mcp tools/search_recommendations.go:266 treat it as a hard error today, so an account with one skip would fail its Azure collection instead of keeping its USD recommendations.

[cristim]

Independent review (claude-opus-5-5) of f1a9a88: APPROVE
Update-branch of the approved 2de6006: only main's commits came in, as clean merges with no conflict resolution; the PR's own diff content is unchanged; GOWORK=off vet and tests pass in: providers/azure.