Repository navigation
fix(aws): scope ladder baseline observations to account - #413
Conversation
|
@coderabbitai review |
|
|
Independent cold review: no actionable findings in either of two adversarial passes, performed by the same gpt-6.1-sol reviewer at MEDIUM effort. Reviewed head Pass 1 attacked the actual production caller, payer/member scope leakage, filter/page preservation, malformed/duplicate periods before map overwrite, missing observations versus explicit zero, and the independent sample/count/age guards. Pass 2 rechecked those conclusions against the pinned SDK, legacy API behavior, concrete parent failures and isolated mutations, plus completeness, correctness, security, bugs, reuse, scope and unnecessary machinery. The new scoped API has a current factory caller and reuses the existing numeric parser and sorter. It adds no purchases or cloud-backed verification. Checked source fields:
Source of truth: pinned Cost Explorer SDK v1.61.0 Fresh macOS verification, all with
Evidence is constructed CE-shaped HTTP/SDK-loopback fixtures, not recorded billing data or live AWS economic acceptance. Total-demand reconstruction, native normalization and Compute purchase enablement are outside this residual-only verdict. No new follow-up defect was found. |
Keep other accounts and incomplete daily observations out of the ladder residual percentile while preserving measured zero days. Refs #282
152fe91 to
1bf6d0e
Compare
|
Rebased 152fe91 to 1bf6d0e on parent c2a461a. The complete intervening main delta contains 11 files, all under providers/gcp, including its module pin update. AWS and shared pkg subtree bytes are unchanged, and range-diff confirms the AWS patch is identical. The original reviewed commit is preserved at backup/go413-reviewed-152fe913. Normal hooks on the eight changed AWS files, the standalone AWS build, and focused public SDK/collector race tests passed on the rebased candidate. The retained independent cold reviewer completed two clean candidate rebind passes. New-head CI and published-candidate consumer proof remain pending. Prior-head successful checks do not establish new-head CI success. |
|
@coderabbitai review |
|
Renewed independent verdict after the normal rebase: two adversarial passes, no actionable findings for head Pass 1 independently re-read the entire eight-file PR delta against the new parent and retraced configured account -> public AWS factory -> scoped collector -> every DAILY Cost Explorer page -> strict daily periods/duplicate rejection -> contiguous-prefix baseline. Pass 2 attacked rebase integration assumptions, dependency/base changes, legacy behavior, chronology/count/age boundaries, reuse, scope, security and unnecessary machinery. No new implementation defect was found. Independent rebind proof:
Checked implementation remains Fresh proof at this revision:
All fresh runs disabled ambient credentials, used synthetic credentials and constructed CE-shaped SDK-loopback responses, and serialized through retained-inode shared runner locks. This is local fixture integration evidence, not live AWS economic acceptance. No purchases, deployments or cloud calls were performed. CI/CR completion and the final normal merge gate remain separate checks. No new follow-up defect was found. |
|
|
Final gate evidence for head The renewed independent two-pass MEDIUM review found no actionable findings at this exact head. It independently verified identical AWS/shared bytes across the rebase and a fresh published AWS-only consumer with natural shared-module selection, without replacements or an initial shared pin. Native macOS build/race, actual-parent fail-before proof and seven killed/restored scratch mutations are recorded in the linked evidence. SDK responses are constructed loopback fixtures, not live billing observations. The completed CodeRabbit review explicitly covers the current head and all eight changed files, with no actionable comments. There are no inline review threads. The generated docstring-coverage warning is advisory: the new public scoped collector is documented; concise internal helpers and test functions follow the repository's comment policy. CI history is retained rather than described as uniformly green. The initial AWS lint failure was a schema-download timeout before code analysis; its bounded same-head retry passed. A separately carried integration failure read an HTTP 200 loopback response through a closed socket. Independent source tracing found no configured timeout, premature cleanup or shared transport mutation. Native 100 full-fixture race runs, 1,000 account race/coverage runs, the full affected package integration command, and a scratch 300ms delayed-body probe passed. The socket-closing actor remains unknown. A successful bounded Linux retry establishes nonrecurrence, not a proven root cause. Both workflows now have actual terminal SUCCESS at the unchanged reviewed head: CI - Build & Test, attempt 3 and pre-commit. The bounded integration retry and dependent CI Success gate succeeded; earlier successful jobs retained their actual successful conclusions. The failure did not recur on that Linux retry, and its root cause remains unknown. Final server inspection reports CLEAN/MERGEABLE, all checks SUCCESS, unchanged head/base, and no review threads. This is the scoped DAILY residual-observation prerequisite. Total-demand reconstruction, historical E/k unit provenance and compatible platform/executor adoption remain tracked by #282 and #349. This PR does not complete either issue or enable purchases. |
|
Merged #413 normally as Fresh published-main verification passed on macOS Go1.26.9: a new external module initially required only AWS Post-merge status is not all green: pre-commit passed, but main CI failed its unit job with the recurring HTTP 200 body-read socket error, this time in This merged leaf supplies scoped DAILY residual observations, not reconstructed total demand. Historical E/k units and cohort rates, total-demand observations, persisted tranche rates, compatible platform pin/in-flight accounting and executor adoption remain open under #282 and #349. No real purchases, deployments or runtime enablement were performed. Follow-up issues filed: #414 for the observed recurring Linux fixture failure. The remaining ladder work stays explicitly tracked by #282 and #349; neither parent issue is complete. |
The AWS ladder's residual baseline now queries DAILY Cost Explorer observations for its configured linked account. Each page retains the account, region, service and purchase-type filters. Scoped responses reject malformed periods and duplicate days; baseline construction requires a contiguous in-window prefix while preserving existing count and reporting-lag guards.
The legacy public unscoped collector remains available. Missing days are not filled with zero, and explicit zero observations keep their existing behavior.
Reviewed head:
1bf6d0e4a54c7aaf7e72c37ff1069cf024e2c69c, tree5022c3249df61174064eecda03077cb36289b98d.Validation includes native macOS build/race tests, actual-parent fail-before proof, seven killed/restored scratch mutations, and a fresh published AWS-only consumer with natural shared-module selection. The renewed independent two-pass review and exact-head evidence cover this revision. SDK transport evidence uses constructed loopback fixtures rather than live AWS billing responses. CI retry history and its verification limits are recorded in the final gate comment.
Refs #282 and #349. This ships the scoped residual-observation prerequisite. It does not reconstruct total demand, normalize historical E/k, update platform module pins, or enable an executor or purchases.