[cristim]

Summary

Pure domain code in internal/archera (stdlib + github.com/google/uuid) that associates one stored platform recommendation (AWS EC2 Instance Savings Plan) with at most one Archera plan line item. The join is exact: source account, provider_offering_id, product, native duration, payment and the exact stored hourly 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 between a platform recommendation and an Archera line item.

Deliberately unavailable

• Missing or ambiguous identity returns no match with a reason, never a guess.
• Candidate native metadata is not borrowed to fill gaps.
• Currency is null; no currency is assumed.
• No economic equivalence claim: the result carries unavailable_source_precision.

Evidence (synthetic domain fixtures, not real Archera data)

• go test ./internal/archera/ and -race: pass (68 tests). go vet, gofmt -l, go build ./...: clean.
• Mutation probe (scratch, reverted): using the recommendation's own source account in the line-item identity makes TestMatchRecommendationRefusesUnprovenAssociation fail.

Stack

PR 1 of 2. The alternatives PR is stacked on this branch.

Refs #799

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Added matching for AWS EC2 Instance Savings Plan recommendations to resources when their identity and current comparison details can be verified.
  • Recommendations with invalid or unverified details are marked unavailable, while cases with multiple matching line items are identified as ambiguous rather than associated with a resource.

[coderabbitai]

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: fa1f3cf4-e8ca-40a7-b1e4-7bc1ef12bdf8

📥 Commits

Reviewing files that changed from the base of the PR and between 2f41346 and 902586f.

📒 Files selected for processing: 2

• 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

Adds identity types and MatchRecommendation for matching AWS EC2 Instance Savings Plan recommendations to line items and current comparison rows. The matcher validates recommendation fields and references, then returns an association status and amount provenance.

Changes: AWS Savings Plan recommendation matching

Layer / File(s)	Summary
Identity validation and reference indexing internal/archera/recommendation_match.go	Adds identity and result types. Validates recommendation dimensions, payment, duration, commitment, source account, and UUID references. Indexes line-item and comparison references, with errors for duplicate references.
Association matching and outcome tests internal/archera/recommendation_match.go, internal/archera/recommendation_match_test.go	Matches recommendation identity against line items and current comparison rows. Returns association results with stored-decimal amount provenance when the current comparison basis matches. Tests cover matching, mismatches, duplicate entries, malformed references, and input preservation.

Priority: ⬇️ Low

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

Change: Feature

Merge Risk: ⚪ Minimal · up to 90258

This change adds recommendation-matching logic that is not yet reachable from production routes. No actionable merge-blocking risk was found.

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 17 functions across 2 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 EC2 Savings Plan identities in Archera.
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.

• Fix all pre-merge checks with AI

✨ 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 902586f: clean

Reviewer: fresh cold-briefed agent (non-author), dispatched by claude-cc-pr-merge-2.

Findings (all non-blocking)

1. The comparison row's CurrentPayment join (sameCurrentBasis) is untested: probe drop_row_payment survives. Item payment is joined and tested, so there is no false match today. Add one table row varying ComparisonIdentity.CurrentPayment.
2. The pinned client can't supply all of the inputs yet. insurance.OfferEntry deliberately does not map selected_amount (pkg/insurance/types.go:137-139, quote.go:220). The line-item fields (source_account, provider_offering_id, selected_commitment, duration) exist in no client type at the pinned version. The wiring PR needs a go-module change or a line-item decoder, with an Archera doc source per feat(insurance): per-recommendation Archera quotes (blocked on an Archera API mapping) #799. PaymentOption values (types.go:74-76) match validPayment.
3. The comparison row's contract term is not cross-checked against the line item's duration.
4. Style: validRecommendation uses sameAmount(x, x) as a nil/positive check. It works but reads oddly.

Refuted:

• Float equality: all amounts are *big.Rat.
• Ambiguity: returns ambiguous_line_items, and duplicate ids are an error.
• Cross-account and case changes are refused.
• Compute SP vs EC2 Instance SP is refused.
• "All Upfront" vs all_upfront is refused.
• Only 31536000 and 94608000 seconds are accepted.
• Mixed plan ids fail closed.

Source of truth checked: the #799 acceptance criteria (exact-match, unmatched and ambiguous tests are present), and cloud-commitments-go/pkg/insurance types.go and quote.go at the pinned version.

What I attacked: go vet and go test -race ./internal/archera/... pass. Files are 195 and 190 lines.

Probe (reverted)	Result
drop the duration join	killed
ambiguity returns first match	killed
amount compare loosened to <=	killed
drop the item amount join	killed
drop the source account join	killed
drop the IsCurrent check	killed
drop the row amount check	killed
drop the offer-id cross-reference	killed
drop the row CurrentPayment check	survived (finding 1)

Worktree cleaned: yes. Mutation probes capped per policy; the survivor is non-blocking.

[cristim]

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

• Independent adversarial review: clean at this SHA (comment above). 4 non-blocking notes go to the wiring PR.
• Local verification on macOS: go test -race ./internal/archera/... and go vet pass at this SHA. 9 hand probes on the join fields: 8 killed, 1 non-blocking survivor. Evidence is synthetic domain fixtures, not real Archera data, as the PR body states. The code is pure and not wired to a route, so there is no runtime path to exercise.
• CI: 26/26 green on this SHA.
• Labels mirror feat(insurance): per-recommendation Archera quotes (blocked on an Archera API mapping) #799 (Refs; does not close it). CodeRabbit: skipped (quota).
• Merging with squash and --match-head-commit. feat(archera): associate vendor comparison alternatives #945 is retargeted next.

[cristim]

Independent review at 902586f

Verdict: changes requested (test gaps only; no wrong behaviour found in the code as written).

Findings

All are mutation survivors in internal/archera/recommendation_match.go (line numbers at this head). Each guard exists and behaves correctly today, but no test would fail if it were deleted.

1. Medium, test gap, line 131-133 (sameResource, item region). Replacing want == got with the same comparison minus region survives. No test gives a line item a region different from the recommendation while the comparison row still carries the recommendation's region. missing_details_region only blanks both sides, which validDimensions rejects first. Fix: add a case with item.Region = "us-west-2" (rec unchanged).
2. Medium, test gap, line 136 (sameCurrentBasis, row.CurrentPayment == r.Payment). Replacing it with true survives. The payment case only changes the line item. Fix: add a case with row.CurrentPayment = "all_upfront", expecting comparison_basis_unproven.
3. Medium, test gap, line 186 (validLineReference, strings.EqualFold(planID, item.PlanID)). Replacing it with true survives. Only equal-but-malformed plan IDs are tested, never a valid but different plan ID on the line item (cross-plan item). Fix: add a case with a different valid PlanID.
4. Low, test gap, line 91 (r.Provider != "aws"). Removing it survives: a gcp recommendation with gcp item and row would still be refused only by luck of the service/plan-type check. Fix: add a case with provider gcp on all three sides.
5. Low, test gap, line 97 (validPayment(r.Payment)). Removing it survives when recommendation and item both carry an unknown or empty payment. Fix: add a case with both sides "".
6. Low, dead code, line 97 sameAmount(r.HourlyCommitment, r.HourlyCommitment). Removing it survives, including the missing_hourly and zero_hourly cases, because sameResource already requires a positive equal amount. It is also an odd self-comparison to read. Fix: delete it, or replace it with an explicit positive check if you want the early exit.
7. Low, dead code, line 186 validMatchUUID(item.OfferID). Removing it survives: validComparisonReference already requires EqualFold(row.CurrentOfferID, item.OfferID) with row.CurrentOfferID validated. Either delete it or document why a line item without a row must still have a valid offer ID.

Questions for R2 (not blockers)

• Stored AWS recommendations carry PaymentOption as all-upfront, partial-upfront and no-upfront (hyphens), and a float64 HourlyCommitment. This matcher takes no_upfront style payments and a *big.Rat. Nothing here maps them, so R2 needs one exact table (no trimming, no case folding) and an exact decimal conversion that does not go through float formatting, with tests. Term strings 1yr/3yr must map to 31536000/94608000, which matches the AWS provider's purchasecfg constants.
• LineItemIdentity.InstanceFamily is compared exactly, but the issue's list of offer fields does not include an instance family. I could not read the OpenAPI extracts named in the brief (they are not on disk), so I could not confirm where R2 gets this field. If it is not on the offer, every association will be unavailable; if R2 fills it from the recommendation, the guard is vacuous.
• MatchRecommendation returns an error for duplicate line-item or comparison references and unavailable for everything else. R2 should map the error to the same unavailable response, not a 500.

What I attacked

Identity dimensions one by one (provider, source account incl. case and control characters, provider_offering_id vs offer_id aliasing, product, plan type, region incl. top-level contradiction, native duration, payment, hourly amount via big.Rat with no float), uniqueness (two matching items, duplicate item or row references), UUID case-insensitivity and malformed IDs, nil recommendation, nil region, nil amounts, nil duration, input immutability (JSON snapshot before and after).

Mutation results (scratch copy, each reverted)

32 mutants, 25 killed, 7 survived (findings 1-7 above). Killed: service/plan-type guard, term allowlist, source usability, top-level region, offering and family non-empty, item type, item duration, item amount, is_current, row type, row provider, at-most-one, row offer match, row amount, 36-length check, payment/offering/source members of the identity struct, provider_offering_id swapped for offer_id.

Local verification (synthetic domain fixtures, not live Archera or AWS data)

GOWORK=off go test -race -count=1 ./internal/archera/ passes at this head. go vet clean, gofmt clean, gocyclo max 10 (MatchRecommendation), file under 500 lines. The package is not wired to a route, so I judged the API for the R2 handler: inputs and outputs are sufficient, subject to the questions above.

[cristim]

Review findings addressed in 52d390e. Each guard was removed in a scratch mutant and the named case failed; the mutants were reverted.

• Line-item region guard: new case line_item_region kills it.
• Row CurrentPayment check: new case current_payment kills it.
• Plan id EqualFold guard: new case other_plan (valid but different plan UUID) kills it.
• Provider != "aws" guard: new case non_aws_recommendation kills it.
• validPayment check: new case invalid_payment kills it.
• Dead sameAmount(r.HourlyCommitment, r.HourlyCommitment): removed. A missing or non-positive stored amount is already refused in sameResource, covered by the existing missing_hourly and zero_hourly cases.
• validMatchUUID(item.OfferID): removed. The comparison-row check already rejects a malformed offer id, and the existing malformed-offer cases still pass.

Local: go test -race ./internal/archera/, vet, gofmt and build pass.

[cristim]

Correction to the resolution comment above: #944 was squash-merged at 902586f2, before 52d390ee was pushed, so the review fixes are not in main yet. They land through #945, which the merge owner is rebasing onto main. 52d390ee gets its own independent review there.