Repository navigation
feat(common): preserve proven commitment currency metadata - #410
Conversation
Keep commitment currency separate from savings and baseline amounts. Preserve unknown legacy values and currency metadata through scaling.
|
Independent committed review covers exact head Two retained implementation/staged review passes and two fresh committed cold passes found no actionable findings. The fresh reviewer inspected the exact archived source and ran an independent native Go consumer importing the actual public common package. It checked legacy absent and explicit unknown JSON omission, known EUR, scaling ratios 0 / 0.5 / 2, nil versus known-zero recurring cost, and pointer isolation. The original parent combined with the new regression compiled and failed on the intended known-EUR runtime assertion. Separate scratch mutations clearing currency in the actual scaler and removing JSON omitempty also failed the intended assertions and were restored. The fresh archive passed standalone This verifies the shared public contract only. Provider source-leg completeness, provider population, platform persistence, displays, currency-safe guardrails and live economics are subsequent work. No default currency, FX, historical backfill, provider or platform code changes are included. The CSV tag follows existing shared metadata conventions; no CSV serialization framework or CLI writer was added. Follow-up issues filed: none for this 39-line contract leaf, confirmed against the exit checklist. Remaining currency-loss class work is explicitly tracked by #124; provider and platform transport follow after the merged standalone shared module is published. CI must pass on this exact SHA before a normal merge. |
|
CI diagnosis: the GCP lint job failed before source analysis during golangci-lint 2.10.1 config verify. Fetching https://golangci-lint.run/jsonschema/golangci.v2.10.jsonschema.json timed out while awaiting response headers. The shared pkg and other provider lint jobs passed. Evidence: https://github.com/LeanerCloud/cloud-commitments-go/actions/runs/38707989074/job/116332062840 . No code, linter configuration or bypass change is indicated. After this workflow reaches a terminal state, one targeted retry of the failed job and its normal dependents will be attempted; CI remains required. |
|
Workflow attempt 1 is terminal: Unit Tests, Integration Tests, security, shared pkg lint and other provider lint all passed. Only GCP schema-fetch lint and its dependent CI Success aggregate failed. Verified the actual job database ID and unchanged head 9eea7a0, then requested one targeted rerun of job 116332062840 and its normal dependencies in run 38707989074. A separate 30-second background watcher tracks the retry. No source, linter configuration, rebase or protection-bypass changes were made. |
|
Final gate at exact9eea7a039874d3168f8b36ea4790e86ccdb7e8a7/basea3fb25103204341399c624a0ce7b242fc89d3b15: all CI is SUCCESS after the single targeted schema-timeout retry, pre-commit SUCCESS, live CLEAN/MERGEABLE, zero review threads. Automatic CodeRabbit completed on this exact head and explicitly reported no actionable comments. Its generic docstring-coverage warning is dismissed: this change adds no production function, the new field has its semantic comment, and the test name and assertions explain the test; adding restatement comments conflicts with the rationale-only comment convention. No source change or review waiver is needed. Independent two-pass committed review and actual public-consumer evidence are recorded above. Proceeding with normal protected matched-head squash merge, without bypasses. |
|
PR #410 merged normally at main Published module: This completes the shared commitment-currency contract for #409. Provider modules can now pin the published API before populating it. No provider source completeness, platform transport, currency-safe guardrail, live cloud economics, purchase or deployment result is claimed. Broader work remains in #124. Follow-up issues filed: none for this contract leaf, confirmed against the exit checklist; remaining scope is explicitly recorded on #124. Post-merge main build/test run |
|
Post-merge main CI is terminal green at 498b74f: build/test run 38709401903 and pre-commit run 38709401920 completed SUCCESS. All PR, retry, CR, merge and main-run watchers are reaped. The exact published standalone module and separate pinned public-consumer verification are recorded above. Merged worktree and local/remote feature branch cleanup is complete. |
Recommendation currently drops a proven commitment currency during JSON round trips. Add optional CommitmentCurrency metadata for CommitmentCost and RecurringMonthlyCost, with empty meaning unknown and presence requiring source-proven amounts and currency for every required payment leg.
The field deliberately makes no claim about savings or on-demand baseline currency. Existing value-copy scaling preserves it without new helper logic. No provider population, platform transport, default currency, FX, historical backfill or dependency movement is included. Publish the standalone shared module before provider modules reference the new API.
Validation: known EUR, explicit unknown and legacy absence through JSON decode/encode and actual ScaleRecommendationCosts at ratios 1 and 2, with money scaling and typed round trips. The test failed by runtime assertion before the field existed. Separate scratch mutations removing omitempty and clearing metadata during scaling also fail by the intended assertions and were restored.
Two retained independent source passes and the same-byte staged review approved tree
43719e7adf4dc3d567433fd7333dd0e46f7128b2. Fresh committed cold review completed two passes with no findings at9eea7a039874d3168f8b36ea4790e86ccdb7e8a7. Its independent public consumer additionally covers ratios 0 / 0.5 / 2, nil and known-zero recurring cost, and pointer isolation; parent plus new regression and separate semantic mutations fail as expected.Standalone pkg full short race suite, build, official golangci-lint 2.10.1 and normal local commit hooks passed with Go 1.26.9, GOWORK=off and ambient cloud credentials disabled. The cold reviewer independently repeated the full suite and build from an owned archive. Added comments: 2 / 38 nonblank lines (5.3%).
Closes #409
Refs #124
Prerequisite #408 is merged and published. Provider population, platform transport and currency-safe enforcement remain subsequent work tracked by the broader issue; this contract does not make existing currency-blind guardrails safe.
Summary by CodeRabbit
New Features
Bug Fixes