docs(bss): fx rate provider module#8
Open
bit4flip wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughThe PR adds product and technical design documents for a stateless FX Rate Provider adapter implementing ChangesFX rate-provider adapter
Estimated code review effort: 2 (Simple) | ~15 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gears/bss/rate-provider/docs/DESIGN.md`:
- Around line 275-280: Update the init() configuration validation so any
mismatch between sources[] order and the ledger’s fx.provider_order is rejected
as an initialization error rather than merely warned about. Keep provider
construction through build_source unchanged, and ensure initialization fails
before the composite can be used when the configured ordering differs.
- Around line 95-96: Update the composite fetch design to enforce a shared total
deadline or explicit per-source time budget across sequential fallback attempts,
rather than allowing each source to consume the full timeout independently.
Ensure the configured budgets align with the p95 ≤ 2 s target and
one-bounded-attempt behavior, and add coverage for the worst-case path where
both sources are unavailable.
- Around line 301-310: Update the SourceConfig initialization and outbound
request design to validate base_url before use, requiring HTTPS and rejecting
loopback, private, link-local, or otherwise disallowed destinations. Add
redirect handling that validates every target and prevents api_key credentials
from crossing hosts or schemes; keep secrets excluded from logs and fail
initialization for invalid endpoints.
- Around line 369-375: Resolve the mismatch between ECB’s EUR→X publication
format and the ProviderRate functional-per-unit-base contract: update the ECB
adapter and its contract tests so USD→EUR-style transaction-to-functional legs
are available, using deterministic inverse pairs where appropriate. Preserve
direct published rates, ensure the planned X→EUR→Y triangulation can obtain both
legs, and revise the design documentation to match the implemented ownership
decision.
- Around line 397-408: The http-json provider’s fetch_latest path must not
return a successful empty or misleading partial result when the document cannot
be mapped. Update the mapping flow used by fetch_latest so an entirely
unmappable document returns RateProviderError::Internal, and enforce the
configured all-or-nothing behavior for structural mapping failures if that rule
is strict; preserve counted skips only for valid documents with individually
unresolvable entries.
- Around line 417-422: Clarify the rate-to-rate_micro conversion specification
to require an exact decimal or integer-scaled representation before rounding,
avoiding binary floating-point behavior. Define half-even rounding for exact
halfway values, perform explicit overflow and non-finite validation, and map any
invalid or overflowing input to RateProviderError::Internal without truncation.
- Around line 49-52: Resolve the v1 scope in DESIGN.md by choosing either
implemented ECB-plus-bank/PSP fallback or ECB-only with fallback deferred, then
make the architecture overview, wiring section, decision O-10, factory, sample
configuration, and tests consistently reflect that choice. If fallback remains
deferred, describe CompositeRateProvider as a future extension and remove
normative v1 fallback claims; otherwise update the implementation and coverage
to include the ordered fallback behavior.
- Around line 327-334: Require the composite rate provider configuration to
contain at least one source before registration. Validate sources[] during the
composite provider setup, rejecting empty configurations before provider_id() or
fallback logic can access sources[0] or require a missing last error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 19a07908-fbdb-4db3-9150-9bbffc0947c7
📒 Files selected for processing (1)
gears/bss/rate-provider/docs/DESIGN.md
bit4flip
force-pushed
the
bss/fx-provider-design
branch
from
July 17, 2026 12:44
d6793e0 to
8f601c0
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gears/bss/rate-provider/docs/DESIGN.md`:
- Around line 437-446: Update the rate conversion flow described in DESIGN.md to
reject any parsed FX rate that is zero or negative before scaling and rounding.
Map non-positive values to RateProviderError::Internal, while preserving the
existing exact-decimal parsing, half-to-even rounding, and overflow handling for
strictly positive rates.
- Around line 560-566: Update the integration-test networking guidance around
the local HTTP server to use test-only HTTPS or a narrowly scoped
loopback/private-host allowlist. Ensure the exception is explicitly limited to
integration-test initialization and cannot alter production URL validation or
redirect credential policy, including the related sections at the referenced
locations.
- Around line 189-191: Define the canonical UTC conversion for date-only
provider publication dates in the `as_of` design, including the exact timezone
and time (or explicitly retain date-only precision), and apply it consistently
to the related sections. Add a golden test covering the conversion and resulting
ledger staleness behavior.
- Around line 41-43: Update the Status block in DESIGN.md to reflect that
decision O-12 was recorded on 2026-07-17, including a refreshed date or wording
that accurately represents the document’s current readiness state; preserve the
existing references to O-1, O-3, and the bss-ledger dependency.
In `@gears/bss/rate-provider/docs/PRD.md`:
- Around line 253-257: Update the posting-path isolation requirement in the PRD
to apply only to non-FX posts: state that provider outages must not affect
non-FX posting latency or availability, while explicitly allowing FX posts to
block through the ledger fail-safe. Align the threshold wording so it no longer
claims provider downtime cannot affect posting availability generally.
- Around line 61-62: Make the cross-currency business goal at
gears/bss/rate-provider/docs/PRD.md lines 61-62 conditional on supported
EUR-functional flows or the companion ledger capability, rather than claiming
unconditional success. Apply the same currency-scope precondition to the
acceptance criterion at gears/bss/rate-provider/docs/PRD.md lines 361-363,
preserving the direct-pair-only adapter scope.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fd7a88f3-c104-4f61-859b-1c5c5bcecd52
📒 Files selected for processing (2)
gears/bss/rate-provider/docs/DESIGN.mdgears/bss/rate-provider/docs/PRD.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit