Conversation
Use the coverage-aware AWS API and report missing-demand skips. Pin the published exact-coverage prerequisites and test helper sizing. This is the consumer prerequisite for issue 70; command-level proof is retained in the separate verification layer. Real-scenario acceptance and final dependency repins remain outstanding.
cristim
added
triaged
|
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
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
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. 📝 WalkthroughWalkthroughExpiry sizing now uses coverage data when adjusting recommendations. When pool demand is unavailable, the adjustment is skipped and coverage remains unchanged. Tests cover sizing cases, missing demand, and zero-demand rows. Two Go dependencies use newer pseudo-versions. ChangesReservation expiry sizing
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~15 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The change makes expiry sizing use pool demand and skips adjustment, with a warning, when demand is unavailable. No merge-blocking issue was found in the supplied context. The author's reported CI and integration checks remain pending. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
This branch has not been deployed
1 participant
Scope
Use authoritative pool demand when adjusting expiry coverage, and report how many recommendations retain their coverage because pool demand is unavailable. Pin the published shared-package and AWS prerequisites.
Refs LeanerCloud/cloud-commitments-go#70.
This is partial consumer prerequisite A: four files, 153 changed lines. The separate B verification layer will add the six root-command/TLS-SDK/CSV regressions. This PR does not close issue 70 or independently establish CLI delivery.
Exact revision and dependencies
6007c4f4fdc767a2a296c09e70999d0f97c6aaae652fc94af4593273946ad8d5eafc2ad430496f1e7d68b349ec0e71203c426842c636bd30d84449f7v0.0.0-20261004010532-e6c7eb87968a, full commite6c7eb87968a5bdc05e604cbc1e922d5ce97ce36(prerequisite)v0.0.0-20261004034603-d4b69ab4f8b1, full commitd4b69ab4f8b10b241ad93d4e78171a597364a5cc(combined producer)Both modules were resolved from published commits with normal checksum verification,
GOWORK=off, and noreplacedirective.Local evidence
Native macOS arm64, Go 1.26.6, published dependencies. The exact four source blobs were verified before and after the precommit run; normal hooks committed the unchanged reviewed tree.
/Users/cristi/go/bin/golangci-lint; version: 2.10.1.-buildvcs=falseand external source/tree binding, not a misleading embedded ancestor revision. Artifact SHA-256:df769e8440da24cc143cd3e9608ec2e701de43472a89fa4cea2f8445509b5211;--helppassed.02735c8da9750fa803febdaae3221e22fea737839fe5d68cdc389d6076df4851.The three uncovered existing tests are
TestRunTool,TestGetAccountAliasRealFunction, andTestGetAllAWSRegions/Integration_test. No real credentials, cloud calls, purchases, deployments or Windows runs were used.The preserved aggregate checkout separately passed six synthetic SDK-to-CSV cases and seven isolated mutation probes. Those B tests are not in A. Parent-compatible aggregate tests reproduced incorrect baseline counts/costs, and the unchanged treatment expectations passed. This is synthetic local evidence, not live acceptance or committed-B qualification.
Independent committed-head review
Fresh gpt-6-astra review of exact commit
6007c4f4fdc767a2a296c09e70999d0f97c6aaaefound no actionable findings. The reviewer checked the published AWS/pkg source contracts and independently ran five synthetic sizing scenarios, the 17 committed helper tests, build, vet and lint on native macOS with Go 1.26.6. Published module versions, sums and origins matched, withGOWORK=offand no replacements.The independent baseline overlaid the parent CLI helper onto the new dependencies: missing-demand assertions failed while four controls passed. This isolates the consumer call and warning, not historical old-dependency behavior. Three connected mutations independently failed the intended denominator, warning and precision assertions while their controls passed. Treatment passed. The initial sandbox cache-access failure was retained as a setup failure; the approved retry passed using the same existing caches.
The reviewer inspected, but did not independently rerun, the author's broad suites, artifact/help and hook evidence above. Independent probes were helper-only, not command or real-cloud acceptance. Verdict SHA-256:
b554fe320841dc56a64b159043030176fd82e0f946a30241a2110af8f3a2bbec.Holds
6007c4f4fdc767a2a296c09e70999d0f97c6aaae. Both owned watchers terminated successfully. This does not resolve the remaining holds below.Existing separate CLI issues #2131 (linked inventory) and #2132 (RDS family expiry overwrite) are unchanged. Remaining issue-70 delivery is tracked on the parent issue and approved stack; no duplicate follow-up issue is needed.