Skip to content

feat(net)!: an undeclared Rust timescale means untimed - #5083

Merged
kixelated merged 10 commits into
mainfrom
quest/m1/rust-untimed-default
Oct 12, 2026
Merged

kixelated merged 10 commits into
mainfrom
quest/m1/rust-untimed-default

Conversation

@kixelated

@kixelated kixelated commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

moq_net::track::Info::default() (and so create_track(name, None) and accept(None)) silently gave a track a millisecond timescale, while @moq/net treats an omitted timescale as untimed (#4968). The publish-timestamp and lite-07 untimed quests need each track to say whether it is timed before publishers stop inventing timestamps.

Approach

  • moq-net: removed impl Default for Timescale; Info::default() now has timescale: None. The lite encoder (TRACK_INFO, frame and datagram send times) and the lite datagram/fetch decoders name Timescale::MILLI for an untimed track, as does moq-audio's decode quantum floor.
  • Every Rust publisher that stamps frames declares milliseconds explicitly: hang catalog (Catalog::default_track_info, also used for MSF now), moq-mux timeline, TS SI tables (ts/si.rs, found by the moq-cli import/export tests), moq-boy status, moq-room chat, moq-stats JSON tracks, moq-bench, moq-c data_track, moq-ffi json/flate tracks, the moq-tokio and moq-json examples, and the moq-mux doc examples. Media tracks already pin their scale (hang::container::track_info, moq-c/moq-ffi raw tracks at microseconds).
  • Tests that write timestamps on a track created with None now use a millisecond track (Info::timed() inside moq-net, a local timed() helper elsewhere), preserving what they tested before. New regression test an_undeclared_timescale_is_untimed; broadcast_moq_lite_05_default_timescale became broadcast_moq_lite_05_untimed_track.
  • Docs: doc/setup/upgrade.md (Unreleased), doc/lib/js/net.md, rustdoc on Info, Info::with_timescale, create_track, hang::container::track_info. Quest file deleted and references updated; publish-timestamp and lite-untimed are now unblocked.
  • Follow-up on this branch: writers that still stamped a frame after create_track(name, None) now declare Timescale::MILLI. That includes the io_uring session and WebTransport tests, the relay io_uring runtime tests, and the benches and loom tests that write timestamps. Tracks that never stamp a frame stay untimed. Merged origin/main again (through quest(m1): plan #4349 follow-ups after the reporter's re-run #5325): main's new tests that stamp frames on a None track (moq-net auth/restart/route_change/finite_relay/late_lower_group/empty_group, the lite and ietf serve tests, moq-bench drain, moq-boy input, moq-relay auth_lifetime, moq-hls routes, the moq-mux TS SI and fMP4 clock-join tests) now declare milliseconds too. moq-ffi's JSON/flate entry points moved to the adopt constructors on main; their tracks come from the binding's create_track, which pins microseconds.

Decision

CI failed because lite_session_over_the_worker writes Timestamp::ZERO on a track whose timescale is now untimed.

  • ✅ Keep the TimestampMismatch refusal. These are transport and bench paths, not untimed-track tests, so the track gets an explicit millisecond timescale, which None used to mean.
  • Skip the test, or accept a timestamp on an untimed track. Rejected.

Left for the maintainer, unchanged:

  1. Convenience constructor for timed tracks. Recommended: keep the with_timescale call; no new public API.
  2. The stale rs/moq-mux/AGENTS.md line. Agent files stay untouched unless a maintainer asks.
  3. TS SI tables stay on milliseconds. The shared-clock quest has since landed (feat(mux)!: importers shift onto a catalog clock already in use #5082) without changing them; a finer scale is a possible follow-up.

Impact

  • Breaking (Rust API): moq_net::Timescale no longer implements Default.
  • Breaking (Rust behavior): track::Info::default(), broadcast::Producer::create_track(name, None), and track::Request::accept(None) create an untimed track; writing a timestamp to it fails with TimestampMismatch. Declare one with Info::default().with_timescale(Timescale::MILLI).
  • This follow-up adds no public API. The millisecond helpers are private to the tests and benches.
  • moq-ffi / moq-c / bindings: no API change. moq-c data_track stays millisecond-timed; moq-ffi tracks, including those adopted by its JSON and flate producers, come from its create_track, which already pins microseconds.
  • Wire: none. An untimed track on lite-05+ still declares milliseconds and carries send times, as before.

Alternatives

  • A public convenience such as track::Info::timed(scale) or a Timescale-to-Info conversion, so timed publishers don't write Info::default().with_timescale(...). Not added: no consumer needed new API to land this, and it can come with the publish-timestamp quest if wanted.
  • Rewriting tests that use Timestamp::ZERO as a placeholder to write untimed frames instead. Rejected: many depend on timestamps (expiry, retention), and a timed track keeps each test's original meaning.

Follow-ups

Tested

  • cargo nextest run --locked -p moq-uring --no-fail-fast --test session --test web -j 1: 13 passed, including lite_session_over_the_worker.
  • cargo nextest run --locked -p moq-relay --features io-uring --no-fail-fast --test runtime_uring -j 1: 6 passed.
  • quest check: 418 documents ok, after merging origin/main.
  • After the second origin/main merge: cargo nextest run --workspace 6008 + 459 + 169 + 97 passed; cargo nextest run -p moq-net -p moq-ffi -p moq-cli 2243 passed.
  • just check: passed on the timestamp fixes (5615 + 458 + 157 + 97 Rust tests, JS, Python). The later origin/main merge did not touch those writers.

(Written by Grok 4.7 and Claude Opus 5.5)

kixelated and others added 2 commits October 8, 2026 17:17
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`track::Info::default()`, `create_track(name, None)`, and `accept(None)`
now leave `timescale` as `None`, matching `@moq/net`. `Timescale` loses
`Default`; the lite encoder and datagram decoder name `Timescale::MILLI`
for an untimed track's send times, as does moq-audio's quantum floor.

Every publisher that stamps frames declares milliseconds explicitly: the
hang catalog, MSF and timeline tracks, TS SI tables, moq-boy status,
moq-room chat, moq-stats, moq-bench, moq-c and moq-ffi data tracks, and
the examples. Tests that write timestamps use a millisecond track.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Quest outcome: implemented quest/m1/rust-untimed-default.md in full; left as a draft for the maintainer's decisions below.

Open decisions:

  1. Convenience constructor for timed tracks. Every timed publisher now writes Info::default().with_timescale(Timescale::MILLI), and about 60 test modules carry a local timed() helper.
    • (a) Keep as is. Recommended: no consumer needs new API yet.
    • (b) Add a public track::Info::timed(scale) (or similar) and swap the helpers for it.
    • (c) Revisit in the publish-timestamp quest, which touches the same publishers.
  2. Stale line in rs/moq-mux/AGENTS.md ("Info::default() is milliseconds"). The advice to pin with with_timescale still holds, but the reason is wrong now. Recommended: change it to "is untimed" in this PR once approved.
  3. TS SI table timescale. These tables keep milliseconds, which is today's behavior, while their PTS is finer. Recommended: keep milliseconds here and settle it with the shared-clock quest.

Issues found along the way: the TS SI tracks (rs/moq-mux/src/container/ts/si.rs) were a production path that relied on the old default, and only the moq-cli import/export integration tests caught it. Local moq-uring tests fail on RLIMIT_MEMLOCK (environment only).

(Written by Claude Opus 5.5)

kixelated and others added 3 commits October 9, 2026 11:17
…-default

Co-authored-by: Grok 4.7 <noreply@x.ai>
An undeclared timescale is untimed, and a timestamp on that track is TimestampMismatch. These session tests and benches are not untimed-track coverage, so the track declares Timescale::MILLI, which None used to mean.

Co-Authored-By: Grok 4.7 <noreply@x.ai>
kixelated and others added 2 commits October 11, 2026 13:31
# Conflicts:
#	quest/m1/README.md
#	quest/m1/rust-untimed-default.md
#	rs/moq-ffi/src/flate.rs
#	rs/moq-ffi/src/json.rs
#	rs/moq-flate/src/stream/mod.rs
#	rs/moq-net/benches/track.rs
#	rs/moq-net/src/lite/publisher.rs
#	rs/moq-net/src/model/origin.rs
#	rs/moq-net/tests/track_abort_over_session.rs
#	rs/moq-relay/tests/goaway_cluster.rs
Tests merged from main created tracks with an undeclared timescale and then
wrote timestamps, which this branch makes a TimestampMismatch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated marked this pull request as ready for review October 11, 2026 22:23
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-12T01:24:14.953851Z a1e423a Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 40 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 2945c7f8-2114-46b9-a461-45f2e0bf3d4b

📥 Commits

Reviewing files that changed from the base of the PR and between 6e1fffa and a1e423a.


📒 Files selected for processing (121)
  • doc/lib/js/net.md
  • doc/setup/upgrade.md
  • js/net/src/lite/track.test.ts
  • quest/m1/README.md
  • quest/m1/lite-untimed.md
  • quest/m1/publish-timestamp.md
  • quest/m1/rust-untimed-default.md
  • quest/m3/teleop-mavlink.md
  • quest/m3/teleop-robot.md
  • rs/hang/src/catalog/root.rs
  • rs/hang/src/container/frame.rs
  • rs/moq-audio/src/decode/consumer.rs
  • rs/moq-bench/src/connection.rs
  • rs/moq-boy/src/input.rs
  • rs/moq-boy/src/status.rs
  • rs/moq-c/src/publish.rs
  • rs/moq-c/src/test.rs
  • rs/moq-cli/src/fetch.rs
  • rs/moq-e2ee/benches/protect.rs
  • rs/moq-e2ee/src/tests.rs
  • rs/moq-e2ee/tests/transport.rs
  • rs/moq-ffi/src/test.rs
  • rs/moq-flate/src/snapshot/mod.rs
  • rs/moq-flate/src/stream/mod.rs
  • rs/moq-hls/src/export/mod.rs
  • rs/moq-hls/src/server/routes.rs
  • rs/moq-json/examples/telemetry.rs
  • rs/moq-json/src/snapshot/mod.rs
  • rs/moq-json/src/stream/mod.rs
  • rs/moq-json/src/window/mod.rs
  • rs/moq-mux/src/binary.rs
  • rs/moq-mux/src/catalog/producer.rs
  • rs/moq-mux/src/catalog/tracks.rs
  • rs/moq-mux/src/container/consumer.rs
  • rs/moq-mux/src/container/flv/export_test.rs
  • rs/moq-mux/src/container/fmp4/export_test.rs
  • rs/moq-mux/src/container/fmp4/import_test.rs
  • rs/moq-mux/src/container/fmp4/muxer.rs
  • rs/moq-mux/src/container/group.rs
  • rs/moq-mux/src/container/producer.rs
  • rs/moq-mux/src/container/ts/export_test.rs
  • rs/moq-mux/src/container/ts/si.rs
  • rs/moq-mux/src/json.rs
  • rs/moq-mux/src/timeline.rs
  • rs/moq-net/benches/group.rs
  • rs/moq-net/benches/origin.rs
  • rs/moq-net/benches/session.rs
  • rs/moq-net/benches/track.rs
  • rs/moq-net/benches/viewers.rs
  • rs/moq-net/src/fuzz.rs
  • rs/moq-net/src/ietf/publisher.rs
  • rs/moq-net/src/ietf/subscriber.rs
  • rs/moq-net/src/lite/publisher.rs
  • rs/moq-net/src/lite/subscriber.rs
  • rs/moq-net/src/lite/track.rs
  • rs/moq-net/src/model/bandwidth.rs
  • rs/moq-net/src/model/broadcast.rs
  • rs/moq-net/src/model/front.rs
  • rs/moq-net/src/model/group.rs
  • rs/moq-net/src/model/origin.rs
  • rs/moq-net/src/model/resume.rs
  • rs/moq-net/src/model/time.rs
  • rs/moq-net/src/model/track.rs
  • rs/moq-net/tests/announce_to_serve.rs
  • rs/moq-net/tests/auth.rs
  • rs/moq-net/tests/datagram.rs
  • rs/moq-net/tests/drained_claim.rs
  • rs/moq-net/tests/empty_group.rs
  • rs/moq-net/tests/fetch_cancel.rs
  • rs/moq-net/tests/fetch_only.rs
  • rs/moq-net/tests/finished_broadcast_mock.rs
  • rs/moq-net/tests/finite_relay.rs
  • rs/moq-net/tests/goaway.rs
  • rs/moq-net/tests/group_charge.rs
  • rs/moq-net/tests/head_fetch.rs
  • rs/moq-net/tests/history_groups.rs
  • rs/moq-net/tests/late_lower_group.rs
  • rs/moq-net/tests/loom.rs
  • rs/moq-net/tests/multi_cdn.rs
  • rs/moq-net/tests/quiet_catalog.rs
  • rs/moq-net/tests/rejoin.rs
  • rs/moq-net/tests/restart.rs
  • rs/moq-net/tests/route_change.rs
  • rs/moq-net/tests/session_close.rs
  • rs/moq-net/tests/settled_end_survives_abort.rs
  • rs/moq-net/tests/subscribe_tracks.rs
  • rs/moq-net/tests/subscription_end_integrity.rs
  • rs/moq-net/tests/track_abort_over_session.rs
  • rs/moq-net/tests/track_stream_demand.rs
  • rs/moq-net/tests/track_tail.rs
  • rs/moq-net/tests/untimed.rs
  • rs/moq-net/tests/wide_sequence.rs
  • rs/moq-relay/src/cluster.rs
  • rs/moq-relay/src/internal.rs
  • rs/moq-relay/tests/auth_lifetime.rs
  • rs/moq-relay/tests/cluster_unknown.rs
  • rs/moq-relay/tests/drills.rs
  • rs/moq-relay/tests/embed.rs
  • rs/moq-relay/tests/goaway_cluster.rs
  • rs/moq-relay/tests/public_rules.rs
  • rs/moq-relay/tests/runtime_uring.rs
  • rs/moq-relay/tests/runtime_workers.rs
  • rs/moq-relay/tests/smoke.rs
  • rs/moq-relay/tests/upstream_links.rs
  • rs/moq-room/src/chat.rs
  • rs/moq-rtc/src/codec/bitstream_test.rs
  • rs/moq-stats/benches/aggregate.rs
  • rs/moq-stats/src/aggregate.rs
  • rs/moq-stats/src/consume.rs
  • rs/moq-stats/src/produce.rs
  • rs/moq-tokio/examples/chat.rs
  • rs/moq-tokio/examples/clock.rs
  • rs/moq-tokio/src/client.rs
  • rs/moq-tokio/src/connection.rs
  • rs/moq-tokio/src/server.rs
  • rs/moq-tokio/tests/backend.rs
  • rs/moq-tokio/tests/broadcast.rs
  • rs/moq-tokio/tests/subscription_end_integrity.rs
  • rs/moq-uring/benches/session_lite.rs
  • rs/moq-uring/tests/session.rs
  • rs/moq-uring/tests/web.rs

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Comment @coderabbitai help to get the list of available commands.

@kixelated

Copy link
Copy Markdown
Collaborator Author

@codex review

(Written by Claude Opus 5.5)

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: 9b4e961ca2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@kixelated

Copy link
Copy Markdown
Collaborator Author

@codex review

The new head merges main again and declares milliseconds on two more test tracks.

(Written by Claude Opus 5.5)

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🎉

Reviewed commit: c7a95a1fae

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit: c7a95a1

[P2] Migrate the newly merged datagram benchmark too — rs/moq-net/benches/track.rs:411–415. bench_datagram_subscriber_join still calls create_track("bench", None), then appends timestamped datagrams. With this PR's untimed default, every nonzero buffered case returns TimestampMismatch and panics at unwrap(), so the benchmark cannot run. Use the file's existing timed() helper here, and verify with cargo bench -p moq-net --bench track -- track_datagram_subscriber_join.

Overall direction is sound: explicit publisher scales preserve intended timing, the Lite millisecond fallback preserves compatibility, and private helpers avoid unnecessary public API. Full PR reviewed, including the latest main-merge interactions; no other concrete findings. No prior review from this reviewer to classify as fixed/open.

Verification limits: GitHub-only static inspection; no builds, tests, or benchmarks run locally. Head CI is still incomplete. State, head, and reviews rechecked before submission.

(Written by OpenAI)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Fixed the [P2] datagram bench finding in a1e423a: bench_datagram_subscriber_join now uses timed(). cargo test -p moq-net --bench track (plus session, priority, stats, viewers, and the fuzz-gated benches) runs clean. --bench origin panics at origin/reprice_duplicate (an announce Restart where it expects Update), which comes from main and is unrelated to timescales.

@codex review

(Written by Claude Opus 5.5)

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed commit: a1e423a

Fixed: the previous P2 at rs/moq-net/benches/track.rs:411. The datagram benchmark now uses timed(), which explicitly declares milliseconds and matches its timestamped writes. No prior findings remain open, and no new actionable findings in this one-line delta from c7a95a1; the base is unchanged.

Direction: this is the minimal fix and reuses the existing helper without changing production behavior or public API.

Verification limits: GitHub-only static review of the delta and helper/call-site context; I did not run tests or benchmarks. The author's reported bench checks were not independently reproduced; current-head CI remains pending. State, head, and reviews rechecked before submission.

(Written by OpenAI)

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Another round soon, please!

Reviewed commit: a1e423a9c9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@kixelated

Copy link
Copy Markdown
Collaborator Author

Merge summary for a1e423a:

  • Merged main twice. Resolved the conflicts by keeping main's moved and rewritten code: the moq-ffi JSON/flate adopt constructors, the moq-flate refusal tests, and the origin epoch-carryover tests. Then I declared milliseconds wherever those tests stamp frames. Main's leftover Timescale::default() uses became Timescale::MILLI.
  • Tests that main added since this branch started (auth, restart, multi_cdn, route_change, finite_relay, late_lower_group, empty_group, the lite/ietf serve tests, moq-bench drain, moq-boy input, moq-relay auth_lifetime, moq-hls routes, the moq-mux TS SI and fMP4 clock-join tests) create a None track and then write timestamps, so they now declare milliseconds. There are no production changes beyond the original PR.
  • Review: Codex and the OpenAI review are clean on this head. I fixed the one OpenAI finding (the datagram join bench).
  • Public API: Timescale no longer implements Default. track::Info::default(), create_track(name, None), and accept(None) now create untimed tracks. Wire: unchanged. js/net already treats an omitted timescale as untimed and declares milliseconds on lite-05+, so it matches.
  • Open items keep their recommended defaults: no public Info::timed helper; the rs/moq-mux/AGENTS.md line stays until a maintainer asks; TS SI tables stay on milliseconds.

(Written by Claude Opus 5.5)

@kixelated
kixelated added this pull request to the merge queue Oct 12, 2026
Merged via the queue into main with commit 30ec9b9 Oct 12, 2026
15 checks passed
@kixelated
kixelated deleted the quest/m1/rust-untimed-default branch October 12, 2026 02: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.

1 participant