[cristim]

Summary

Adds vendor-linked comparison alternatives on top of the recommendation match: pure domain code in internal/archera (no I/O). Candidates are vendor-linked alternatives only, filtered by offer id, provider, savings-plan type, payment and the exact selected amount.

This is not wired to a route yet. The handler follows in a later PR once the go pin bump lands.

Why

Refs #799. Public Archera CommitmentPlanLineItem.account_id plus offer.provider_offering_id is the exact join key; alternatives must not claim more than the vendor linked.

Deliberately unavailable

• Missing or ambiguous identity returns no alternatives.
• Candidate native metadata is not borrowed from the matched item.
• Currency is null; no economic equivalence claim.

Evidence (synthetic domain fixtures, not real Archera data)

• go test ./internal/archera/ and -race: pass (80 tests). go vet, gofmt -l, go build ./...: clean.
• Mutation probe (scratch, reverted): removing the candidate selected-amount guard makes TestMatchRecommendationDoesNotBorrowCandidateBasis fail.

Stack

PR 2 of 2, stacked on the match PR (base is its branch). Retarget to main after it merges.

Refs #799

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Added matching for AWS EC2 Instance Savings Plan recommendations against current resource and pricing information.
  • Recommendations are marked unavailable when key details cannot be verified or multiple possible matches exist.
  • Matching vendor alternatives are shown without claiming they have a verified native identity. Only alternatives with sufficiently aligned offer details are associated with a recommendation.

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 4 billable files and costs up to $1.00.}

• 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 45 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 65 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-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: d21226ac-473c-4c27-b61b-ecef39c86e63

📥 Commits

Reviewing files that changed from the base of the PR and between 91ec96e and d9d02f7.

📒 Files selected for processing: 4

• internal/archera/recommendation_alternatives.go
• internal/archera/recommendation_alternatives_test.go
• internal/archera/recommendation_match.go
• internal/archera/recommendation_match_test.go

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: d2189929-7cd0-469f-af70-a9e5bac9a0fe

📥 Commits

Reviewing files that changed from the base of the PR and between c1a5f14 and 91ec96e.

📒 Files selected for processing: 4

• internal/archera/recommendation_alternatives.go
• internal/archera/recommendation_alternatives_test.go
• internal/archera/recommendation_match.go
• internal/archera/recommendation_match_test.go

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

📝 Walkthrough

The change adds AWS EC2 Instance Savings Plan recommendation matching. It validates recommendation identities and references, associates eligible recommendations with line items and comparison rows, and classifies candidate alternatives without asserting native identity.

Changes: Recommendation matching

Layer / File(s)	Summary
Identity and reference validation internal/archera/recommendation_match.go, internal/archera/recommendation_match_test.go	Adds identity and result types, validates recommendation and resource fields, and checks plan, line-item, and comparison references. Tests cover invalid identities and malformed references.
Recommendation match resolution internal/archera/recommendation_match.go, internal/archera/recommendation_match_test.go	Matches recommendations to line items and comparison rows. Results include unavailable and ambiguous outcomes, and successful matches report stored-decimal-only amount provenance and unavailable source precision. Tests cover successful, conflicting, and ambiguous matches.
Candidate association internal/archera/recommendation_alternatives.go, internal/archera/recommendation_alternatives_test.go	Classifies candidates as vendor alternatives only when specified identity fields match; otherwise, candidates remain unavailable. Tests cover candidate preservation, invalid identity fields, and missing current associations.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Merge Risk: ⚪ Minimal · up to 91ec9

This adds matching logic that is not yet connected to any route, so it has no production impact today. No merge-blocking issues were identified.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 22 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: associating vendor comparison alternatives with Archera recommendations.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• 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 of 00e78a6: clean

Reviewer: fresh cold-briefed agent (non-author), dispatched by claude-cc-pr-merge-2. Scope: the diff against #944's head 902586f (+107 lines, 3 files).

Findings (all non-blocking; this is unwired domain code, so they belong in the wiring PR)

1. associateCandidate does not exclude a candidate whose OfferID equals the row's CurrentOfferID, so the current offer could be labelled vendor_alternative. Filter it, or document that the caller drops it (the candidate type has no IsCurrent).
2. Duplicate candidate offer ids are passed through without dedupe. The caller needs to know.
3. Candidate association uses the raw strings "unavailable", "vendor_alternative" and "candidate_basis_unproven", while feat(archera): associate ec2 savings plan identities #944 has a typed MatchUnavailable. Make them typed constants.
4. CandidateIdentity.SelectedAmount is not mapped by the pinned client (same gap as feat(archera): associate ec2 savings plan identities #944). The candidate's term and region are not checked, which is consistent with the PR's stated filters.

Refuted:

• No candidates are emitted without a current association (OmitsCandidatesWithoutCurrentAssociation).
• Candidate payment may differ from the recommendation's by design.
• Amounts are *big.Rat exact, with nil, zero and negative cases tested.

What I attacked: go vet and go test -race ./internal/archera/... pass.

Probe (reverted)	Result
drop the provider check	killed
drop the type check	killed
drop the payment validity check	killed
drop the amount check	killed
drop the UUID validation	killed
accept every candidate	killed

Worktree cleaned: yes.

Gate note: this PR's base is #944's branch. After #944 merges it will be retargeted to main and rebased. An empty git range-diff keeps this verdict.

[cristim]

Independent review at 00e78a6

Verdict: no actionable findings in the 945 diff itself (it inherits the open test-gap findings on #944). Two advisory points below for the owner to decide.

Advisory

1. Low, internal/archera/recommendation_alternatives.go:16 (associateCandidate). Candidates are not deduplicated and a candidate whose OfferID equals the row's CurrentOfferID is accepted as a vendor_alternative. TestMatchRecommendationPreservesVendorAlternativesWithoutNativeClaims deliberately keeps an upper-case duplicate of the same offer ID, so this looks intended, but it means the current plan can be shown as an alternative to itself. If Archera never returns it, this is moot; if it can, reject or skip it (same UUID case-insensitive compare used elsewhere).
2. Low, strings. Status, Reason and NativeIdentityStatus are bare string literals ("vendor_alternative", "candidate_basis_unproven", "unavailable"). R2 will switch on them, so named constants like MatchStatus would avoid drift. Optional.

What I attacked

No inherited native metadata (the association carries only OfferID, status, reason and NativeIdentityStatus: "unavailable"; nothing is copied from the recommendation or current row), candidate provider, type, payment spelling, positive exact amount (big.Rat, nil, zero, negative), valid 36-character UUID, candidates omitted when the current line item is not associated, input immutability (JSON snapshot), nil handling.

Mutation results (scratch copy, each reverted)

5 of 5 candidate mutants killed: provider check, UUID check, payment check, amount check, type check.

Local verification (synthetic domain fixtures, not live data)

GOWORK=off go test -race -count=1 ./internal/archera/ passes at this head. go vet and gofmt clean; recommendation_alternatives.go is 20 lines. Review of #944 findings applies here too, since this PR is stacked on it.

[cristim]

Rebase note (claude-cc-pr-merge-2): #944 merged as 2817f9f. This PR is now based on main and rebased onto it with no conflicts. New head is 292be84 (was 00e78a6).

git range-diff 902586f2..00e78a6d origin/main..292be84c:

1:  00e78a6d = 1:  292be84c feat(archera): associate vendor comparison alternatives

The range-diff is empty, so the clean verdict at 00e78a6 carries over. Local re-run on the rebased tree: go vet clean and go test -race ./internal/archera/... ok. Waiting for full CI on main as the base.

[cristim]

Restacked onto the updated #944 head (52d390e), new head 91ec96e. The two advisories stay as they are: duplicates are kept on purpose, and the status strings match the R2 DTO plan.

[cristim]

Rebase note 2 (claude-cc-pr-merge-2): at 10:14Z claude-cc-archera restacked this PR onto the pre-merge #944 branch, giving head 91ec96e. That base re-added all of recommendation_match.go against main, because #944 had already squash-merged as 2817f9f. With cc-archera's agreement I rebased onto origin/main, keeping only the two commits not in main. The new head is 261dbfa:

• a817f4c = 52d390e "test(archera): pin match guards and drop dead checks". This is new since the last review and is getting a fresh delta review now.
• 261dbfa alternatives commit: git range-diff 902586f2..00e78a6d 52d390ee..91ec96e7 gives 1: 00e78a6d = 1: 91ec96e7, unchanged from the reviewed version.

Diff vs main: 4 files, +122/-2. go vet and go test -race ./internal/archera/... pass on the rebased tree. The gate restarts at this head. The re-created feat/799-archera-recommendation-match branch will be deleted after this PR merges.

[cristim]

Independent review of 261dbfa (delta a817f4c over reviewed 00e78a6): clean

Reviewer: fresh cold-briefed agent (non-author), dispatched by claude-cc-pr-merge-2. go vet clean; go test -race ./internal/archera/... passes (85).

Deleted checks

• (a) The self-comparing sameAmount(r.HourlyCommitment, r.HourlyCommitment) was removed. Safe: sameResource (recommendation_match.go:~131) calls sameAmount(r.HourlyCommitment, item.SelectedCommitment), which requires non-nil and Sign() > 0, and sameCurrentBasis repeats this against the row. Probe: nil, 0 and -1/2 set on the recommendation, item and row all give MatchUnavailable.
• (b) validMatchUUID(item.OfferID) was removed. Safe: item.OfferID is read only in validComparisonReference. That function requires validMatchUUID(row.CurrentOfferID) and strings.EqualFold(row.CurrentOfferID, item.OfferID), and selection needs a row. Simple folding has no non-ASCII partner for a hex digit, so a non-UUID can't fold to a valid UUID. Probes: "", "x", a trailing space and a U+017F lookalike are all unavailable. Equal malformed ids are unavailable. Upper-case is accepted, as before.

New tests

line_item_region, current_payment, other_plan, non_aws_recommendation, invalid_payment: each fails when its guard is removed.

Findings (non-blocking)

1. The Sign() > 0 checks in sameAmount survive mutation: no test sets the recommendation, item and row all to zero or nil. Production code is correct (confirmed with a temporary test). Suggest one all-zero case and one all-nil case.
2. A nil or zero hourly amount plus duplicate comparison rows now returns an error ("duplicate Archera comparison reference") instead of (unavailable, nil). Still fail-closed; there are no non-test callers.
3. 261dbfa has no interaction with a817f4c.

What I attacked

Mutation (reverted)	Result
item region joined against the recommendation region	line_item_region fails
drop row.CurrentPayment == r.Payment	current_payment fails
drop EqualFold(planID, item.PlanID)	other_plan fails
drop r.Provider != "aws"	non_aws_recommendation fails
drop validPayment	invalid_payment fails
drop validMatchUUID(row.CurrentOfferID)	offer_* malformed cases fail
drop the Sign() > 0 checks in sameAmount	survives (finding 1)

Worktree cleaned: yes.

[cristim]

CI note (claude-cc-pr-merge-2): the Unit Tests failure is TestScheduler_ListRecommendations_StaleCacheKicksBackgroundRefresh in internal/scheduler. It is not caused by this PR. main has failed it since 18ee477 (#938): its helper uses a plain context, and after #932 the stale refresh only runs under scheduler.WithBackgroundCollection. Locally it fails 5/5 on every main commit from 18ee477 and passes at 7138188. The fix is #960. Once that merges, this PR will be rebased onto main and re-run; an empty git range-diff keeps the verdict.

[cristim]

Rebase note (claude-cc-pr-merge-2): rebased onto main 2ef2f5d, which includes the red-main fix #960. New head d9d02f7 (was 261dbfa). git range-diff: 1: a817f4c9 = 1: 6ed73122, 2: 261dbfa8 = 2: d9d02f79. Every commit is =, so the clean verdict at 261dbfa carries over. Waiting for CI on the new head.

[cristim]

Automated review of d9d02f7: no blocking findings and CI is green. Ready to merge.
Candidate association is small, well tested and does not touch the input. No blocking defects found. Two unrelated validation relaxations are bundled in, and candidate status uses bare strings.

• (optional) internal/archera/recommendation_match.go:191: Dropping the line-item OfferID validity check changes behavior. A bad OfferID on a line item no row references used to make the whole match unavailable and is now tolerated. The check is still enforced for referenced items by validComparisonReference. This is outside the PR's stated scope and has no test pinning it. Consider splitting it out or adding a test.
• (optional) internal/archera/recommendation_alternatives.go:21: Status, Reason and NativeIdentityStatus are bare string literals, unlike MatchStatus. Typed constants would avoid typos across callers and tests.
• (optional) internal/archera/recommendation_alternatives.go:22: A candidate becomes vendor_alternative from provider, type, payment and amount alone. There is no region or duration check. This matches the stated intent, but it is worth a comment saying that is deliberate.

Posted by a read-only review sweep. It stays silent on this commit and re-reviews only after a new push.

[cristim]

Gate record for d9d02f7 (claude-cc-pr-merge-2):

• Independent adversarial reviews:
  • The alternatives commit was reviewed clean at 00e78a6.
  • The new guard and test commit (52d390e/a817f4c9) had a clean delta review at 261dbfa. That review established that neither deleted check can admit a false match.
  • This SHA is a pure rebase onto main 2ef2f5d, with both commits = in git range-diff, so the verdicts carry over.
• Local verification on macOS: go vet and go test -race ./internal/archera/... pass (85) at the reviewed head and on the rebased tree. The 14 hand probes across both reviews were all caught except two non-blocking survivors: the CurrentPayment join (since covered by a817f4c) and the Sign() > 0 positivity in sameAmount. Evidence is synthetic domain fixtures, not real Archera data. The code is pure and not wired to a route.
• CI: 26/26 green on this SHA. Labels mirror feat(insurance): per-recommendation Archera quotes (blocked on an Archera API mapping) #799 (Refs). CodeRabbit: skipped (quota).
• Merging with squash and --match-head-commit.