[huang195]

What changed

record::emitter::tests::lines_arrive_in_order_with_token (praxis/filters/src/record/emitter.rs) fails sometimes with left: 2, right: 3. It failed on main and on #1765.

• Cause: the emitter adds 1 to rows_emitted only after the write of a row returns. The mock server can get line c before that, and the test read the count at once.
• Fix: the test polls e.stats().rows_emitted until it is 3, with a 2 s limit. The assert stays, so a count that never gets to 3 still fails.
• This changes only the test. The emitter does not change.

The other stats() asserts in this file do not have this race. Their counters change in submit, before the line is sent, or the test has no server.

How it was tested

• To show the race: a 50 ms sleep before the rows_emitted bump makes the old test fail with left: 2, as in CI. The new test passes with the same sleep. The sleep is not in this PR.
• From praxis/, with the rustup toolchain: cargo fmt --check, cargo clippy --all-targets --locked -- -D warnings and cargo test --locked pass.

Assisted-By: Claude (Anthropic AI) noreply@anthropic.com

[coderabbitai]

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 48 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 4117deeb-238b-405a-98f6-8c0b8b5de828

Commits

Reviewing files that changed from the base of the PR and between 87f73ca and 1c87093.

Files selected for processing: 1

• praxis/filters/src/record/emitter.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

• X
• Mastodon
• Reddit
• LinkedIn

[clawgenti]

Summary

The diagnosis is correct. In Emitter::stream, bump(&self.rows_emitted, ...) runs only after the last wr.write_all(b"\r\n") returns. The mock server's read_stream can read the whole chunk (read_exact of n + 2 bytes) and send "c\n" on the channel before the emitter thread gets to the bump, so reading rows_emitted right after the third recv could see 2. Polling up to a 2 s deadline and keeping the final assert_eq! fixes the race without hiding a real regression: a count that never reaches 3 still fails, and the 2 s limit sits well under the 10 s recv timeout. Instant/Duration are already imported in the test module, and the Relaxed load is enough here because the test only needs the value to show up eventually.

I checked the other stats() asserts in this file and agree they don't have this race. captures_dropped_line_cap, captures_dropped_queue_bytes and rows_dropped (queue full) are all bumped synchronously in submit, and the queue tests use closed_addr() with no server.

Nit (advisory, not blocking): the commit subject is 74 characters, so GitHub cuts it off (...lines_arrive_in_order_with_t…). Something like fix: Wait for rows_emitted in the emitter order test would fit.

Author: huang195 (MEMBER — maintainer)
Areas reviewed: Rust (test only), security
Agent/IDE config (.claude/.vscode): none
Commits: 1 commit, all signed-off: yes
CI status: passing (Rust (praxis) green; a few unrelated Go jobs still pending)