Skip to content

fix(aws): expiry adjustment divides an org-wide count by a per-rec share of demand #70

Closed
Closed
fix(aws): expiry adjustment divides an org-wide count by a per-rec share of demand#70
#177 (+2)
Labels
effort/sHoursimpact/manyAffects most userspriority/p1Next up; this sprintseverity/mediumModerate harmtriagedItem has been triagedtype/bugDefecturgency/this-sprintWithin the current sprint

Description

@cristim
cristim
opened on Sep 2, 2026

Summary

Coverage application rescales each recommendation's average instance count so that the recommendations in a pool sum to the org-wide average, but the expiring counts are keyed by the same pool and hold the org-wide total. Dividing the pool-wide expiring count by one recommendation's share therefore inflates the expiring percentage by roughly the number of recommendations sharing the pool. With three linked-account recommendations it comes out about three times too large, clamps existing coverage to zero, and --target-coverage sizes each of the three as if the pool were entirely uncovered.

Location

providers/aws/recommendations/expiry.go:83 at 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd

Failure scenario

ApplyCoverageMapToRecommendations (coverage.go:447) rescales each rec's AverageInstancesUsedPerHour so the recs in a pool sum to the org-wide average. expiringByPool is keyed by the same org-wide pool key and holds the org-wide expiring count. With three linked-account recs sharing one pool, each rec's avg is roughly one third of the pool demand while expCount is the full pool's expiring count, so expiringPct comes out about three times too large, clamps ExistingCoveragePct to zero, and --target-coverage sizes each of the three recs as if the pool were entirely uncovered.

Evidence

expCount, ok := expiringByPool[lookupPoolKey(recs[i])]
if !ok || expCount == 0 {
    continue
}
expiringPct := float64(expCount) / recs[i].AverageInstancesUsedPerHour * 100.0

Suggested fix

Compute expiringPct once per pool against the pool's total average (sum the recs' avgs, or carry the coverage map's AvgInstancesPerHour), then apply that single percentage to every rec in the pool.

Found by the 2026-09-02 codebase audit, finding A07-014, reported by one reviewer and independently confirmed by a second. Full report: docs/audits/codebase-audit-2026-09-02.md.

Activity

  1. cristim
    added
    triagedItem has been triaged
    priority/p1Next up; this sprint
    severity/mediumModerate harm
    urgency/this-sprintWithin the current sprint
    impact/manyAffects most users
    effort/sHours
    type/bugDefect
    on Sep 2, 2026
  2. cristim
    transferred this issue fromLeanerCloud/cloud-commitments-clion Sep 27, 2026

  3. cristim commented on Oct 4, 2026

    @cristim
    cristim
    on Oct 4, 2026
    MemberAuthor

    Checkpoint: CLI follow-ups #2131 and #2132 track separate downstream limitations: linked-account commitment inventory completeness and RDS family sizing preserving the expiry adjustment. Neither is fixed by the denominator change in this issue; both remain unverified through the real CLI dry-run path.

  4. cristim commented on Oct 4, 2026

    @cristim
    cristim
    on Oct 4, 2026
    MemberAuthor

    Pkg prerequisite checkpoint: PR #175, head e6c7eb87968a5bdc05e604cbc1e922d5ce97ce36, is published and open. It adds optional in-process exact coverage and strict rational RI count flooring. It does not complete or close this issue.

    Native author verification passed the five-module race suite (3869 test events), build, vet and lint. Independent committed-source review found no actionable findings and independently ran pkg tests, boundary/control probes, a parent-sizing scaffold, four behavioral mutants, build, vet and lint. The report records exact source integrity and limitations. GitHub CI is being watched separately.

    Remaining work: the AWS producer must apply the correct pool denominator, populate and clear precise coverage state, and pin the published pkg version. CLI must pin both compatible modules and verify real root-command CSV counts/costs. The real-scenario acceptance question remains unresolved, so the PR stays open without merge approval. Serialization omits precise state; fresh decoding loses it, while reused destinations require explicit clearing when coverage is replaced.

    Related limitations remain tracked by CLI #2131 (inventory completeness) and CLI #2132 (RDS family coverage overwrite).

  5. cristim commented on Oct 4, 2026

    @cristim
    cristim
    on Oct 4, 2026
    MemberAuthor

    Issue #70 prerequisite checkpoint: the AWS stack is published and remains OPEN.

    • Pkg prerequisite #175: e6c7eb87968a5bdc05e604cbc1e922d5ce97ce36.
    • AWS A #176: 4ec36b01793b19d2d57bd04bb82c714d5c3134e3, base main, 365 changed lines. This denominator layer retains the demonstrated 3-versus-4 integer-boundary defect and must not merge alone or become a CLI consumer pin.
    • AWS B #177: 1ee2e2fe578352f124e971a5abd0c282ab2ff4e7, stacked on A, 257 changed lines. It preserves exact expiry coverage/reset semantics and pins published pkg v0.0.0-20261004010532-e6c7eb87968a.

    The complete independent AWS verdict is posted verbatim on A and B. It reports no actionable findings for combined B, independently reproduced A's boundary defect with A source/new-pkg configuration, passed 32 focused and 1544 AWS race checks, killed six behavioral mutants, and passed AWS build/vet/lint. Raw evidence is durably retained at ~/.claude/projects/cudly-resume/evidence/go70-aws-astra-1ee2e2f/; verdict SHA-256 aa6a1be23efb39b0ee4f024da6b754c3642a924b441be280e7e2ec6b99c9849d. These are synthetic native macOS fixtures, not live-cloud or CLI command-entry proof.

    A's automatic CI runs are watched. B's stacked base does not match the existing PR workflow filters; an authorized manual Build & Test run covers its exact head and is watched, but is not yet passing. B's pre-commit CI remains outstanding because that workflow has no manual trigger. Local hooks are not a substitute.

    Remaining work: the CLI must consume compatible published pkg and combined B, migrate its real caller to the coverage-aware API, and prove root-command CSV counts/costs, missing-demand diagnostics, strict fractional boundaries and connected mutations. Actual module/binary binding, exact-head Linux CI and real-scenario acceptance remain required. The synthetic-acceptance question is unresolved. Keep all layers open and issue #70 open; publication does not authorize merge or imply delivery. After acceptance, actual final pkg/AWS merge SHAs require repinning, retesting and review before consumer merge.

    Separate existing follow-ups: linked-account inventory #2131 and RDS family sizing #2132. Neither is fixed by this stack.

  6. cristim
    added 2 commits that reference this issue on Oct 4, 2026

    chore(aws): integrate exact coverage prerequisite into layer A

    7107e13

    chore(aws): integrate published pkg ancestry into layer B

    d4b69ab

  7. cristim commented on Oct 4, 2026

    @cristim
    cristim
    on Oct 4, 2026
    MemberAuthor

    Issue #70 additive dependency checkpoint: the existing stack remains OPEN, with original commits preserved.

    • Pkg #175: e6c7eb87968a5bdc05e604cbc1e922d5ce97ce36.
    • AWS A #176: 7107e1386a43866d8808d42fd092f6ec9c4565e1, now based on pkg; unchanged365-line denominator diff.
    • AWS B #177: d4b69ab4f8b10b241ad93d4e78171a597364a5cc, based on A; unchanged257-line precision/reset/pin diff.

    The previous B CI failure was valid: checked-in workspace pkg lacked the required field. Additive ancestry now includes the actual published prerequisite source, with no CI/go.work/hook/pin changes. Fresh author-run macOS workspace and standalone gates pass: A2 each3,911unit+3,911integration, B2 each3,925unit+3,925integration; all five modules build/vet/lint pass. B2 focused86cases pass in each mode. Published pkg selection remains independently recorded without replacements.

    Complete new independent binding reports are posted verbatim on A2 and B2. These reviews inspected source/ancestry and author logs, with original independently executed baseline/mutation evidence retained. They did not run fresh integration commands. Merge hooks selected no conflict files and skipped all checks; this is not new scanner/tidy evidence. Reviewers qualified unchanged parent hook evidence plus combined native gates.

    Exact-head A2 Build & Test and B2 Build & Test are dispatched and watched. Feature-base pre-commit CI remains outstanding; local hooks do not replace it.

    A still recommends3 instead of4 at the known integer boundary without B and must not merge alone or become a CLI pin. CLI published-version wiring, root-command CSV/count/cost proof, diagnostics, fractional controls, connected mutations and source-built artifact binding remain separate work. Real-scenario/fixture acceptance remains unresolved. No merge or issue close is authorized; actual final integration SHAs require repinning, retesting and review.

    Existing follow-ups remain inventory #2131 and RDS family sizing #2132.

  8. cristim commented on Oct 4, 2026

    @cristim
    cristim
    on Oct 4, 2026
    MemberAuthor

    CLI delivery checkpoint: consumer prerequisite A is LeanerCloud/cloud-commitments-cli#2133 at 6007c4f4fdc767a2a296c09e70999d0f97c6aaae; its Linux build/test and pre-commit CI passed. Test-only B is LeanerCloud/cloud-commitments-cli#2134 at f756c6bb925e70b7b2061af28546505364dfe009, stacked on A. Fresh cold gpt-6-astra review found no actionable findings at both exact commits. B independently exercised six actual command/TLS-SDK/CSV scenarios, retained completeness, four behavioral mutants with passing controls, and restored green cases. These use synthetic provider responses, not live-cloud acceptance. Exact-B Linux CI passed after one dispatch; all eight jobs succeeded at f756c6bb925e70b7b2061af28546505364dfe009 and watcher 47969 was reaped at exit 0: https://github.com/LeanerCloud/cloud-commitments-cli/actions/runs/37184155221. B has no automatic pre-commit CI because its stacked feature base does not match that workflow and it has no dispatch trigger; local normal hooks and A CI do not fill that gap. Real-scenario acceptance and final-main dependency repins/reverification remain unresolved. Both PRs and this issue remain open; no issue closure or merge is requested. Separate CLI issues 2131 and 2132 are unchanged.

  9. cristim
    added a commit that references this issue on Oct 4, 2026

    fix(recfilter): preserve exact expiry coverage during RI sizing (#175)

    de46f76
  10. cristim
    closed this as completedin #175on Oct 4, 2026
  11. cristim
    added 4 commits that reference this issue on Oct 5, 2026

    fix(aws): use pool demand for expiry adjustment (layer A) (#176)

    b4c0644

    fix(aws): preserve exact expiry coverage (layer B) (#177)

    945a404

    test(recommendations): cover expiry through command output

    ef3a2dc

    test(recommendations): cover expiry through command output (#2143)

    bfdb4f6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    effort/sHoursimpact/manyAffects most userspriority/p1Next up; this sprintseverity/mediumModerate harmtriagedItem has been triagedtype/bugDefecturgency/this-sprintWithin the current sprint

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions