refactor(stats): make late-reply sampling explicit #92

2026-08-14T18:18:09Z

Lochnair commented

2026-08-14 18:18:09 +00:00

(Migrated from github.com)

Summary

The TUI suppressed late-reply timing contributions by reconstructing the ClientEvent::LateReply with sent_at, rtt, server_timing, one_way, and received_stats erased before handing it to irtt-stats. That put statistics policy into event construction, and had a side effect nobody asked for: a matched late reply became an UntrackedLate event, so it was counted as untracked_late_replies instead of late_unique_replies.
StatsConfig now carries an explicit late_replies: LateReplyMode policy:
- Measure (default, and what finite(), continuous(), and default() select) — today's behavior for every existing caller.
- CountOnly — the matched late reply is still a late unique reply and still updates packet, byte, late, and server-reported receive counters, but contributes no timing, one-way, server-processing, or IPDV measurement, and reports contributed_sample == false.
Classification stays separate from measurement: a late reply the client could not match to retained send state is UntrackedLate in both modes.
Cumulative and rolling snapshots are recomputed under the same policy, so a rolling window cannot disagree with the cumulative snapshot about what a late reply measured.
The TUI selects CountOnly and passes the original event directly to StatsCollector. The synthetic degraded LateReply is gone.

Behavior

Library callers that build StatsConfig via finite()/continuous()/default() are unchanged. StatsConfig gained a public field, so callers constructing it with a struct literal must add late_replies (the two in-repo test call sites were updated).
The stream client keeps measured-late behavior.
TUI-visible change, and the intended fix: a matched late reply now increments late_unique_replies and unique_replies instead of untracked_late_replies, while its RTT/one-way/server-processing/IPDV samples remain excluded exactly as before. One TUI test that had encoded the old accidental classification was updated to assert the intended semantics.

Scope

Explicitly not changed:

Median capability, retained sample accounting, SampleMode, the rolling API, or general StatsConfig architecture.
Managed outcome counters in irtt-client — untouched.
ClientEvent — no variant, field, or construction path changed; nothing clones or mutates an event to express policy.
Stream client statistics behavior.

Verification

Run from the task worktree, all successful:

cargo fmt --all -- --check
cargo check --workspace --all-targets --all-features --locked
cargo test --workspace --all-targets --all-features --locked
cargo clippy --workspace --all-targets --all-features --locked -- -D warnings
RUSTDOCFLAGS="-D warnings" cargo doc --workspace --all-features --no-deps --locked
cargo check -p irtt-cli --no-default-features --features client --all-targets --locked
cargo check -p irtt-cli --no-default-features --features tui --all-targets --locked
cargo check -p irtt-cli --no-default-features --features server --all-targets --locked
git diff --check

New tests: matched/Measure, matched/CountOnly, CountOnly IPDV suppression, untracked under both modes, rolling under CountOnly (irtt-stats), plus TUI coverage that matched late replies count as late unique and untracked late replies stay untracked.

## Summary - The TUI suppressed late-reply timing contributions by reconstructing the `ClientEvent::LateReply` with `sent_at`, `rtt`, `server_timing`, `one_way`, and `received_stats` erased before handing it to `irtt-stats`. That put statistics policy into event construction, and had a side effect nobody asked for: a *matched* late reply became an `UntrackedLate` event, so it was counted as `untracked_late_replies` instead of `late_unique_replies`. - `StatsConfig` now carries an explicit `late_replies: LateReplyMode` policy: - `Measure` (default, and what `finite()`, `continuous()`, and `default()` select) — today's behavior for every existing caller. - `CountOnly` — the matched late reply is still a late unique reply and still updates packet, byte, late, and server-reported receive counters, but contributes no timing, one-way, server-processing, or IPDV measurement, and reports `contributed_sample == false`. - Classification stays separate from measurement: a late reply the client could not match to retained send state is `UntrackedLate` in *both* modes. - Cumulative and rolling snapshots are recomputed under the same policy, so a rolling window cannot disagree with the cumulative snapshot about what a late reply measured. - The TUI selects `CountOnly` and passes the original event directly to `StatsCollector`. The synthetic degraded `LateReply` is gone. ## Behavior - Library callers that build `StatsConfig` via `finite()`/`continuous()`/`default()` are unchanged. `StatsConfig` gained a public field, so callers constructing it with a struct literal must add `late_replies` (the two in-repo test call sites were updated). - The stream client keeps measured-late behavior. - TUI-visible change, and the intended fix: a matched late reply now increments `late_unique_replies` and `unique_replies` instead of `untracked_late_replies`, while its RTT/one-way/server-processing/IPDV samples remain excluded exactly as before. One TUI test that had encoded the old accidental classification was updated to assert the intended semantics. ## Scope Explicitly not changed: - Median capability, retained sample accounting, `SampleMode`, the rolling API, or general `StatsConfig` architecture. - Managed outcome counters in `irtt-client` — untouched. - `ClientEvent` — no variant, field, or construction path changed; nothing clones or mutates an event to express policy. - Stream client statistics behavior. ## Verification Run from the task worktree, all successful: - `cargo fmt --all -- --check` - `cargo check --workspace --all-targets --all-features --locked` - `cargo test --workspace --all-targets --all-features --locked` - `cargo clippy --workspace --all-targets --all-features --locked -- -D warnings` - `RUSTDOCFLAGS="-D warnings" cargo doc --workspace --all-features --no-deps --locked` - `cargo check -p irtt-cli --no-default-features --features client --all-targets --locked` - `cargo check -p irtt-cli --no-default-features --features tui --all-targets --locked` - `cargo check -p irtt-cli --no-default-features --features server --all-targets --locked` - `git diff --check` New tests: matched/`Measure`, matched/`CountOnly`, `CountOnly` IPDV suppression, untracked under both modes, rolling under `CountOnly` (`irtt-stats`), plus TUI coverage that matched late replies count as late unique and untracked late replies stay untracked.

👍 1

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Lochnair-Labs/irtt-rs#92