docs(adr): network adapter stack design (ADR-0029–0031)#56
Merged
Conversation
Capture the request/reply (HTTP) net-stack design for the first Broker (IBKR Client Portal Web API), grilled end-to-end: - 0029 transport-split crates + compile-time Service/Layer composition; Service in net-http-api (request/reply isn't universal), Timer in the kernel, compile-time/no-dyn binding per ADR-0007. - 0030 HTTP transport contract: untyped Request<Bytes> -> Response<ResponseBody<B>> (serialization stays in the adapter, ADR-0003), streaming-by-composition, one HttpError, blanket HttpClient seam, hyper-util+rustls backend. - 0031 HTTP resilience & venue pacing: order-safe retry (retransmission deferred to Core reconciliation, ADR-0022/0006/0026), merged keyed RateLimit (token-bucket + concurrency-as-policy) grounded in the real IBKR pacing table, 3-state CircuitBreaker + 429 penalty-box backstop, Tracing -> ADR-0014 Telemetry. Design record only; implementation tracked separately. Refs #55 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThree ADRs are added for the network adapter stack design. ADR-0029 defines the transport-neutral kernel and compile-time composition model. ADR-0030 specifies the HTTP wire-bytes contract, body model, and backend composition. ADR-0031 defines the HTTP resilience middleware order and pacing behavior. ChangesNetwork Adapter Stack ADRs (0029–0031)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 3
🤖 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 `@docs/adr/0030-http-transport-contract-wire-bytes-streaming-composition.md`:
- Around line 133-137: The public constructor currently hard-codes Tok ioTimer
in build(), which leaks a concrete backend and breaks the timer seam. Update the
build() signature to accept a generic Timer parameter (for example impl Timer or
T: Timer) and thread that abstraction through the assembled HTTP stack, while
keeping HttpConfig as plain data. Use the build() symbol and the Timer trait as
the main reference points, and ensure tests or alternate clocks can inject their
own timer implementation without changing the rest of the constructor.
- Around line 93-105: The `HttpClient` contract is inconsistent with the claim
that both the raw leaf and layered backend fit the same seam, because the
blanket `impl` on `HttpClient` still hard-requires `Body::Error = HttpError`.
Update the `HttpClient` trait/blanket impl wording so the leaf backend is either
normalized before it reaches `HttpClient` or the contract explicitly allows the
backend’s native body error and translates it inside the backend; keep the
`HttpClient`, `send`, and blanket `impl<S, B>` symbols aligned with that
behavior.
In `@docs/adr/0031-http-resilience-venue-pacing.md`:
- Around line 89-92: The ADR wording for the bucket-resolution behavior should
be changed to fail closed instead of silently skipping missing buckets. Update
the text around the pacing resolution rules so that any referenced bucket absent
from the map is treated as a configuration error or as Throttled, rather than
“no limit,” and keep the guidance aligned with the existing terminology in the
pacing section (e.g. Global, Local, Both, None).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 020a7ed0-227e-44cf-b3ec-6ef04dbdb839
📒 Files selected for processing (3)
docs/adr/0029-network-adapter-stack-transport-split-compile-time-composition.mddocs/adr/0030-http-transport-contract-wire-bytes-streaming-composition.mddocs/adr/0031-http-resilience-venue-pacing.md
- 0030 §6/§7: the HttpClient leaf must normalize the backend body error to HttpError (HyperBody = MapErr<Incoming>), not raw Incoming — aligns the blanket-impl contract with the "leaf satisfies HttpClient" claim. - 0030 §8: build<T: Timer>(...) instead of a hardcoded TokioTimer, so the Timer seam stays mockable per ADR-0029/0031. - 0031 §3: a missing referenced RateLimit bucket fails closed (config error, validated at construction; rejected as Throttled at runtime) rather than silently fail-open into IBKR's 429 penalty box. Only explicit None is unlimited. Refs #55 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/adr/0030-http-transport-contract-wire-bytes-streaming-composition.md (1)
76-80: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winUse the backend-native error wording here.
hyper_util::client::legacy::Clientexposeshyper_util::client::legacy::Errorat the request boundary, whileIncomingstill maps body-side failures fromhyper::Error; avoid naming onlyhyper::Errorin this section.🤖 Prompt for 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. In `@docs/adr/0030-http-transport-contract-wire-bytes-streaming-composition.md` around lines 76 - 80, Update the error-mapping description in the HTTP transport contract ADR to use backend-native error wording instead of naming only hyper::Error. In the section describing the request boundary and body-side failures, reference hyper_util::client::legacy::Error for the client request path and keep hyper::Error only for Incoming body errors, using the existing HttpError and Body::Error discussion as the anchor.Source: MCP tools
🤖 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.
Outside diff comments:
In `@docs/adr/0030-http-transport-contract-wire-bytes-streaming-composition.md`:
- Around line 76-80: Update the error-mapping description in the HTTP transport
contract ADR to use backend-native error wording instead of naming only
hyper::Error. In the section describing the request boundary and body-side
failures, reference hyper_util::client::legacy::Error for the client request
path and keep hyper::Error only for Incoming body errors, using the existing
HttpError and Body::Error discussion as the anchor.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 25a62202-e729-4164-b3e8-6547769b4871
📒 Files selected for processing (2)
docs/adr/0030-http-transport-contract-wire-bytes-streaming-composition.mddocs/adr/0031-http-resilience-venue-pacing.md
✅ Files skipped from review due to trivial changes (1)
- docs/adr/0031-http-resilience-venue-pacing.md
This was referenced Jul 1, 2026
Merged
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.
Captures the request/reply (HTTP) net-stack design for the first Broker — IBKR Client Portal Web API — grilled end-to-end. Design record only; implementation is tracked separately.
ADRs
net-apikernel (composition +ErrorKind+Timer, std-only) →net-http-api/ futurenet-ws-api→ leaf backends.Servicelives innet-http-apibecause request/reply isn't universal (WS is subscribe→stream);Timeris a kernel contract; compile-time / no-dynbinding per ADR-0007.Request<Bytes>→Response<ResponseBody<B>>with serialization in the adapter (ADR-0003); streaming-by-composition viaResponseBody<B>(Either<Full, B>) + per-requestBufferModeso buffering sits inside the retry boundary; one concreteHttpErrorfor service and body; blanketHttpClientseam;hyper-util+rustlsbackend;build()default stack + data config + three-tier override.RateLimit(token-bucket + concurrency-as-policy,Scopestate space, frozen-map/per-bucket-lock) grounded in the real IBKR pacing table; 3-stateCircuitBreaker+ 429 penalty-box backstop;Tracing→ ADR-0014 Telemetry.Notes
CONTEXT.mduntouched — all implementation vocabulary, no domain terms changed.Closes #55
🤖 Generated with Claude Code
Summary by CodeRabbit