fix(recfilter): dedupe Redis OSS and Valkey ElastiCache nodes directionally

[cristim]

What

pkg/recfilter duplicate detection mapped valkey to redis for both recent ElastiCache reservations and recommendations. Both engines therefore shared one budget, so a recent Valkey reservation could suppress an uncovered Redis OSS recommendation (the issue's minimal scenario: Valkey cache.r6g.large reservation, Redis recommendation for the same node type and region; the Redis recommendation was filtered).

This PR keeps each engine in its own budget and adds one directional rule, coveringElastiCacheEngines:

• Valkey recommendation: consumes Valkey, then Redis OSS, then unknown-engine reservations.
• Redis OSS, Memcached and any other engine: own engine, then unknown-engine reservations (unchanged).
• Unknown-engine recommendation: unknown-engine reservations only (unchanged).

Provider/service scoping, deployment and region keys, single-use budget accounting and the fail-closed wildcard for reservations with a missing engine are unchanged.

Why (authoritative contract)

AWS ElastiCache reserved nodes, Upgrading nodes from Redis OSS to Valkey:

Redis OSS reserved nodes can additionally apply across engines to cover running Valkey nodes. Valkey reserved nodes apply only to running Valkey nodes within the same instance family.

How verified

dedupe_test.go: the old case valkey covers redis asserted the reverse direction AWS does not offer; it is replaced by valkey does not cover redis, with the doc link. New cases: valkey covers valkey, valkey and wildcard partial redis, valkey before redis for valkey, redis rec first keeps valkey for valkey, valkey overflow onto redis. Existing Memcached, wildcard and family consumed once cases still pass unchanged.

Before the fix (new tests on origin/main code):

--- FAIL: TestElastiCacheEngineBudgets/valkey_and_wildcard_partial_redis
--- FAIL: TestElastiCacheEngineBudgets/valkey_does_not_cover_redis

Mutations, each reverted:

• Valkey recommendation consumes Redis OSS before Valkey: --- FAIL: .../valkey_before_redis_for_valkey
• Redis recommendation may consume Valkey (symmetric): --- FAIL: .../valkey_does_not_cover_redis, .../valkey_and_wildcard_partial_redis

After, with GOTOOLCHAIN=go1.26.6 GOFLAGS='-p=2 -count=1' AWS_EC2_METADATA_DISABLED=true:

• pkg (GOWORK=off): go build ./..., go vet ./..., go test -race ./...: 1082 passed in 13 packages.
• providers/aws (workspace mode, pkg is unpublished v0.0.0): go test -race ./...: 1585 passed in 14 packages, including the ElastiCache client tests that drive AdjustRecommendationsForExisting.
• golangci-lint run ./recfilter/... in pkg: no issues.
• Not run locally: providers/gcp. The machine ran out of disk at the link step. GCP computeengine implements FilterRecommendationsForRecentCommitments and never reaches this code path. CI covers it.

Not run: the CLI duplicate-check path against controlled provider responses, which the issue asks for. The CLI pins a pkg revision, so that check belongs with the CLI bump.

Out of scope

• Normalized-unit conversion: one Redis OSS node covers 1.25 Valkey nodes of the same size. Counts stay 1:1, which can only under-count coverage, never drop a needed purchase.
• Size-flexible matching within a node family (fix(money): zero-vs-absent conflation in providers/azure and pkg/config,pkg/scorer [go part] #62, fix(recfilter): invalid exact coverage input leaves the full recommendation count in the purchase list #195).

Closes #189

Summary by CodeRabbit

• Bug Fixes
  • Redis OSS and Valkey commitments are now tracked separately. Redis commitments can cover Valkey recommendations, but Valkey commitments do not cover Redis recommendations.
  • Recommendations now use matching commitments first, with defined fallback behavior for Redis and unknown-engine commitments.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-go/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: ec81eb7a-d77d-4414-b5d6-901d72ba8fe0

📥 Commits

Reviewing files that changed from the base of the PR and between 8727842 and bfea9ee.

📒 Files selected for processing (2)

• pkg/recfilter/dedupe.go
• pkg/recfilter/dedupe_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 2 reviews per hour.

---

📝 Walkthrough

Walkthrough

ElastiCache reservation deduplication now distinguishes Redis, Valkey, and unknown engines. Matching uses engine-specific coverage order, while other providers and non-cache services retain their existing normalization behavior.

Changes

ElastiCache Deduplication

Layer / File(s)	Summary
Directional engine matching pkg/recfilter/dedupe.go, pkg/recfilter/dedupe_test.go	ElastiCache keys distinguish Redis, Valkey, and unknown engines. Valkey recommendations consume Valkey, then Redis, then unknown reservations; Redis recommendations do not consume Valkey reservations. Tests cover matching direction, partial and full coverage, wildcard allocation, ordering, and capacity overflow.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to bfea9

The change makes ElastiCache reservation matching directional between Redis OSS and Valkey, and no concrete merge-blocking risk remains. The CLI duplicate-check path and the GCP provider tests were not run locally, so rely on CI for those.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	#189’s directional coverage requirements are implemented in pkg/recfilter/dedupe.go: Redis OSS reservations can cover Valkey recommendations, but Valkey reservations cannot cover Redis recommendatio…	Exercise the real CLI duplicate-check path with controlled provider responses, including a failing pre-fix control, and provide the test results before closing #189.
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the change: directional deduplication of Redis OSS and Valkey ElastiCache nodes.
Out of Scope Changes check	✅ Passed	The changes are limited to ElastiCache reservation matching in pkg/recfilter/dedupe.go and its regression tests. They directly implement #189. The PR reports no unrelated changes.

Full details: Linked Issues check

Explanation

#189’s directional coverage requirements are implemented in pkg/recfilter/dedupe.go: Redis OSS reservations can cover Valkey recommendations, but Valkey reservations cannot cover Redis recommendations. The tests cover mixed demand order, single-use allocation, unknown-engine behavior, and Memcached isolation. The required real CLI duplicate-check exercise with controlled provider responses and a failing pre-fix control is not demonstrated; the PR states that this check was not run and defers it to a CLI bump.

• 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

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

---

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