Repository navigation
fix(interop): declare a timescale on the rust tail track - #5359
Conversation
#5083 made an undeclared Rust timescale mean untimed, so the tail client's Timestamp::ZERO writes failed with TimestampMismatch. The origin/narrow benches had the same untimed track with timestamped writes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review (Written by Claude Opus 5.5) |
|
Warning Review limit reachedYou'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 19 minutes. View limit details
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. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: 0bb8cd7
No actionable bugs found. Declaring a millisecond timescale in test/interop/clients/rust/tail.rs:27–30 and reusing timed() in rs/moq-net/benches/origin.rs:709 correctly matches their existing Timestamp::ZERO writes. This is a focused fix that retains timed-frame interop coverage; switching those writes to untimed frames would change that coverage.
Verification: static review of both changes, surrounding publish/subscribe and benchmark paths, and the track/group timestamp validation code. I did not run tests or benchmarks or independently verify the PR's reported test results.
Problem
#5083 made an undeclared Rust timescale mean untimed. The Rust tail interop client still created its track with
create_track("tail", None)and wroteTimestamp::ZERO, so everytail rust -> {rust, js-native-node, js-native-bun}cell failed with "frame timestamp doesn't match track timescale".The
origin/narrow*benches had the same pattern (an untimedvideotrack written withTimestamp::ZERO) and panic under the nightlycargo bench -- --testsmoke.Approach
Declare
Timescale::MILLIon both tracks, the same pattern #5083 used elsewhere (timed()in the bench).A sweep of the other interop clients, examples, benches and doc snippets found no other untimed track with timestamped writes. The JS tail client writes untimed frames, which is consistent.
Impact
Verification
just test interop --tail: fails on main (3 cells), passes with the fix.just test interop --all: 65 pass, 0 fail.origin/narrow_deliverypanics without the bench fix and passes with it.just checkpasses.Alternatives
None) in the Rust tail client to mirror the JS one. Declaring a timescale keeps the cells covering timed frames, as before feat(net)!: an undeclared Rust timescale means untimed #5083.Follow-ups
origin/reprice_duplicatepanic andorigin/handoffhang in the same bench binary come from feat(net)!: restart announce consumers on a replaced broadcast, keep subscriptions sticky #5087 and are fixed in fix: repair nightly-only compile, clippy, and bench breaks #5340.(Written by Claude Opus 5.5)
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.