moq-net(stats): take a StatsConfig value type in Stats::new

[kixelated]

Summary

Follow-up to #1517, which left moq_net::Stats::new taking 5 positional args (prefix, interval, retention, node, origin), two of them path-like and easy to misorder. This replaces it with a dedicated StatsConfig value type and bumps the patch versions for the (unreleased) stats API break.

What changed

• StatsConfig value type + Stats::new(config). StatsConfig::new(origin) plus chained with_prefix / with_interval / with_retention / with_node, each defaulting to sensible values (.stats, 1s, retention 1, no node), handed to Stats::new(config).
• Why not a by-value builder on Stats? Stats is a cheap-clone shared handle, so Stats::new(origin).with_foo() style setters would be a footgun: calling one after cloning or deriving tier handles would silently configure only one copy. A separate config value type sidesteps that entirely.
• #[non_exhaustive] on StatsConfig so new knobs can land without breaking call sites; construct via StatsConfig::new rather than a struct literal. origin lives in the config (it's required, so there's no Default).
• Internal restructure. The set-once config (prefix, node, interval, retention, enabled) lives in plain Stats fields; only genuinely shared runtime state (origin, entries, task, tick_counter) stays in Arc<StatsShared>. Stats::new destructures the config, normalizing an empty node to None there (so a directly-assigned field is handled too). advertised is derived lazily from prefix + node at task-spawn time; disabled detection is an explicit enabled flag. The snapshot task takes the config it needs by value at spawn.
• Single call site updated (moq-relay StatsConfig::build), referring to it as moq_net::StatsConfig to avoid colliding with the relay's own clap-derived StatsConfig. Plus all stats tests.
• Manual patch bumps: moq-net 0.1.4 -> 0.1.5, moq-relay 0.12.1 -> 0.12.2. The stats API is new with no external consumers, so the deliberate break is taken as a patch bump this time rather than the 0.x minor semver-checks would otherwise force. semver_check stays enabled so real violations are still caught.

Reviewer notes

• Base branch: targeted main because the moq-net(stats): aggregate per-node into a single gzipped broadcast #1517 rewrite this builds on is already on main and the patch release fires from main. Per the branch-targeting guide a breaking moq-net API change would normally go to dev; happy to retarget if preferred, but it would then diverge from the code being released.
• The API shape change is on top of unreleased code (baseline moq-net 0.1.4), so there's no released-API impact beyond what moq-net(stats): aggregate per-node into a single gzipped broadcast #1517 already introduced.
• Root pins moq-net = { version = "0.1", ... } (^0.1), so 0.1.5 satisfies every dependent without touching their requirements.

Test plan

• cargo test -p moq-net stats:: (17 passed)
• cargo test -p moq-relay stats (config merge tests passed)
• cargo clippy -p moq-net -p moq-relay --all-targets (clean)
• cargo check -p moq-net -p moq-relay after version bump (Cargo.lock refreshed)

(Written by Claude)

[coderabbitai]

Walkthrough

This pull request refactors the Stats type from a multi-parameter constructor to a builder-based API. A new public StatsConfig struct provides fluent configuration methods. The Stats struct now stores configuration fields directly (prefix, node, interval, retention, enabled) and maintains an Arc<StatsShared> for runtime state instead of Arc<StatsInner>. The background publisher task lifecycle is rewired to gate on the enabled flag and pass advertised path as a parameter. All tests are updated to use the builder pattern and reference shared state via stats.shared.entries. The moq-relay consumer is adapted to use the new API. Package versions are bumped accordingly.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: refactoring Stats::new to accept a StatsConfig value type instead of five positional arguments.
Description check	✅ Passed	The description comprehensively explains the motivation, implementation details, design decisions, and testing performed, directly relating to all changes in the pull request.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch claude/thirsty-sammet-d3c1fc

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

rs/moq-net/src/stats.rs (1)

855-890: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Guard against a zero interval so run_publisher can’t panic

tokio::time::interval panics when given a Duration::ZERO period, so ensure the task never constructs a ticker with a zero period.

🛡️ Proposed diff

-	let mut ticker = tokio::time::interval(interval);
+	// `tokio::time::interval` panics on a zero period; fall back to 1s so a
+	// misconfigured `with_interval(Duration::ZERO)` can't kill the task.
+	let period = if interval.is_zero() { Duration::from_secs(1) } else { interval };
+	let mut ticker = tokio::time::interval(period);

🤖 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 `@rs/moq-net/src/stats.rs` around lines 855 - 890, run_publisher currently
constructs a tokio::time::interval with the supplied interval which will panic
if interval == Duration::ZERO; guard against that by validating/clamping the
incoming interval before creating the ticker (inside run_publisher): check
interval.is_zero() and either return early with a warning via tracing::warn! or
replace it with a small non‑zero fallback (e.g., Duration::from_millis(1)) and
then call tokio::time::interval with the safe value; update the variable used to
create ticker and keep the existing ticker.set_missed_tick_behavior call.

🤖 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 `@rs/moq-net/src/stats.rs`:
- Around line 855-890: run_publisher currently constructs a
tokio::time::interval with the supplied interval which will panic if interval ==
Duration::ZERO; guard against that by validating/clamping the incoming interval
before creating the ticker (inside run_publisher): check interval.is_zero() and
either return early with a warning via tracing::warn! or replace it with a small
non‑zero fallback (e.g., Duration::from_millis(1)) and then call
tokio::time::interval with the safe value; update the variable used to create
ticker and keep the existing ticker.set_missed_tick_behavior call.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ae6519e3-4a0d-430b-b1ac-d8ebadf5b687

📥 Commits

Reviewing files that changed from the base of the PR and between b7bfc31 and 38e7d5b.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (4)

• rs/moq-net/Cargo.toml
• rs/moq-net/src/stats.rs
• rs/moq-relay/Cargo.toml
• rs/moq-relay/src/stats.rs