Skip to content

fix: type market-data sizes as Option<f64> - #718

Merged
wboayue merged 2 commits into
mainfrom
fix/716-decimal-size-types
Aug 7, 2026
Merged

fix: type market-data sizes as Option<f64>#718
wboayue merged 2 commits into
mainfrom
fix/716-decimal-size-types

Conversation

@wboayue

@wboayue wboayue commented Aug 6, 2026

Copy link
Copy Markdown
Owner

Closes #716. PR-B of two, following #717.

The bug

Historical tick and histogram sizes were typed i32, but IBKR models market-data sizes as Decimal and ships them as protobuf optional string. A fractional wire value failed integer parsing and fell through unwrap_or_default():

// before
size: parse_i32(&t.size),   // wire "0.5" -> parse::<i32>() fails -> 0

That is real data loss on crypto and fractional-share feeds, and it is what the issue reports.

What changed

Type Field Before After
TickMidpoint size i32 Option<f64>
TickLast size i32 Option<f64>
TickBidAsk size_bid, size_ask i32 Option<f64>
HistogramEntry size i32 Option<f64>
ContractDetails min_size, size_increment, suggested_size_increment f64 Option<f64>

All eight decode through parse_optional_decimal (added in #717): None means TWS sent no value — field absent, empty, or an "unset" sentinel — Some(0.0) is a real zero, and a malformed size surfaces as Error::Parse instead of decoding as 0.

The ContractDetails trio is included because contracts without size rules genuinely omit those fields, so the old 0.0 was indistinguishable from a real value, and a size_increment of 0.0 is nonsense rather than merely unlikely.

parse_i32 is deleted along with its last call sites.

Also in here

Allocation. The tick and histogram decoders build their Vec with an explicit capacity rather than collect::<Result<Vec<_>, _>>(), whose size_hint lower bound is 0 (the iterator may short-circuit) and so grows by doubling. This preserves the exact-capacity allocation the pre-change .collect() already had from an ExactSizeIterator — introducing ? is what would have lost it.

Fixture builders. The four historical fixture structs now hold the raw wire string rather than an i32, so fractional, sentinel, empty and absent sizes are all expressible. A size_wire setter keeps the edge cases to one line, and None omits the field entirely:

histogram_entry(125.5, 0.0).size_wire(Some("2147483647"))

Examples. Four async examples needed real changes, not just casts. histogram_data.rs's max_by_key(|e| e.size) no longer compiles (f64 isn't Ord) and becomes max_by + total_cmp; its bar-chart scale also had to stop being seeded with 1.0, which was a faithful port of .max().unwrap_or(1) but floors the scale and squashes every bar once sizes can be fractional. Option<f64> isn't Display, so display columns go through a small fmt_size helper that shows n/a rather than hiding a missing size behind a zero. That helper is duplicated across the four on purpose — each example must read and compile standalone.

Serialized shape. All five retyped structs derive Serialize/Deserialize and utoipa::ToSchema, so this also changes the JSON: a size is now 100.0 rather than 100, absent is null rather than 0, and the generated OpenAPI schema becomes a nullable number. That is the one break here with no compile-time signal, so it is called out in both the changelog and migration guide.

Tests

Regression tests pinning fractional sizes through all four decoders — these assert Some(0.5) where the old code produced 0. Plus malformed → Err per decoder, and ContractDetails absent-size-rules → None.

Sync and async end-to-end tests assert a malformed size fails the whole request through the public API. Sentinel and empty-string handling is not re-tested per decoder: parse_optional_decimal owns those semantics and covers them exhaustively in src/proto/decoders_tests.rs, so the domain tests only prove each decoder is wired to it.

Coverage on the touched modules: historical decoders 97.7%, contracts decoders 97.7%, proto/decoders.rs 84.9% (unchanged from #717).

Docs

CHANGELOG.md gains a Changed section and drops the "not yet covered" caveat #717 added. docs/migration-3.0.md §35 covers the field table, None semantics, and the three migration gotchas — the Ord break, integer accumulators, and {} formatting — with before/after snippets. The quick-migration checklist links to it. Grepped README.md, all docs/*.md and module rustdoc for the changed field names: no other references.

Full sweep green: cargo fmt, all three clippy configs, all three rustdoc configs, just test, cargo test --all-features, and both integration crates (build + clippy).

Known inconsistency

proto::HistoricalTickLast / HistoricalTickBidAsk are reused verbatim by the tick-by-tick path, so the identical wire field now surfaces as Option<f64> on the historical side (TickBidAsk.size_bid) and f64 on the realtime side (BidAsk.bid_size). The boundary is drawn by which struct the field lands in, not by wire semantics. The issue reported only the i32 truncation and the realtime types were already f64, so widening the break was out of scope — but it is a real seam, and the decimal quantity type below is where it should be resolved rather than by retyping realtime piecemeal.

Follow-up

Option<f64> is an intermediate. A dedicated decimal quantity type would let sizes round-trip the wire's decimal representation exactly instead of through binary floating point.

parse_decimal_or_zero's remaining call sites are not all provably safe — the C# client guards several of them with HasX ? StringToDecimal(..) : decimal.MaxValue, so upstream models them as "absent means unset" too. Bar::volume / Bar::wap are the clearest case: MIDPOINT, BID and ASK bars carry no volume, and EDecoderUtils.cs defaults them to decimal.MaxValue while defaulting open/high/low/close to 0 in the same function. Those are the next candidates; the helper's rustdoc names them.

Historical tick and histogram sizes were typed i32, but IBKR models them
as decimals and ships them as strings. A fractional wire value such as
"0.5" failed parse::<i32>() and silently decoded as 0 — real data loss
on crypto and fractional-share feeds, which is what issue #716 reports.

Retype TickMidpoint.size, TickLast.size, TickBidAsk.size_bid/size_ask
and HistogramEntry.size from i32 to Option<f64>, and ContractDetails
min_size / size_increment / suggested_size_increment from f64 to
Option<f64> — contracts without size rules omit those on the wire, where
0.0 was indistinguishable from a real value and a size_increment of 0.0
is nonsense. All eight now decode through parse_optional_decimal, so
None means "TWS sent no value" (absent, empty, or an unset sentinel),
Some(0.0) is a real zero, and a malformed size fails the request rather
than decoding as 0.

parse_i32 is deleted with its last call sites.

The tick and histogram decoders build their Vec with an explicit
capacity rather than collecting into Result<Vec<_>, _>, whose size_hint
lower bound is 0 and so grows by doubling — the same fix applied to the
bars path in #717.

Test fixture builders now hold the raw wire string, so fractional,
sentinel, empty and absent sizes are all expressible; edge cases use
struct-update syntax rather than new setters. Four async examples are
updated — histogram_data's max_by_key no longer compiles because f64
isn't Ord, and Option<f64> isn't Display.

Adds sync and async end-to-end tests asserting a malformed size fails
the request, since Error::Parse is terminal for a subscription.
@wboayue
wboayue force-pushed the fix/716-decimal-size-types branch from fc09587 to 770ef43 Compare August 6, 2026 06:19
The load-bearing fix is a doc claim I got wrong. parse_decimal_or_zero's
rustdoc asserted "every remaining call site is deliberate: the field is
always populated on real wire, so the 0.0 fallback is unreachable." The
C# reference client contradicts that: EDecoderUtils.cs guards volume and
wap with `HasX ? StringToDecimal(..) : decimal.MaxValue` while
defaulting open/high/low/close to 0 in the same function, and MIDPOINT /
BID / ASK bars carry no volume at all. Restore the hedged wording and
name Bar::volume / Bar::wap as follow-up candidates, so the docstring
stops being wrong guidance a future contributor would trust.

Document the serde and OpenAPI break in migration-3.0.md §35 and the
changelog. All five retyped structs derive Serialize/Deserialize and
utoipa::ToSchema, so a size now serializes as 100.0 rather than 100,
absent as null rather than 0, and the generated schema becomes a
nullable number — the one break in this PR with no compile-time signal.

examples/async/histogram_data.rs seeded its bar-chart scale with
`fold(1.0_f64, f64::max)`, a faithful port of `.max().unwrap_or(1)` that
is wrong now that sizes can be fractional: it floors the scale at 1.0,
so a histogram of sub-1.0 sizes renders every bar squashed. Reduce and
fall back only when empty. Its count column also printed a bare 0 for an
absent size, contradicting the fmt_size guidance this same PR publishes.

Drop two tests that re-ran parse_optional_decimal's own semantics
through a decoder — the same tier drift removed in #717's review. Use
assert_decimal_parse_error in the sync/async end-to-end pair rather than
a weaker hand-rolled match, and correct their comment: it named
process_decode_result, but histogram_data returns the decoder's Result
directly and never reaches that classification.

Also: add size_wire setters so malformed-size fixtures stop passing a
dead size argument, hoist 7 repeated import preambles, and fix two
comments left referring to the now-deleted parse_i32.
@wboayue wboayue closed this Aug 7, 2026
@wboayue wboayue reopened this Aug 7, 2026
@wboayue
wboayue merged commit d69a917 into main Aug 7, 2026
3 checks passed
@wboayue
wboayue deleted the fix/716-decimal-size-types branch August 7, 2026 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Preserve IBKR Decimal market-data sizes instead of decoding to f64/i32

1 participant