Skip to content

Stabilize wavediff output for buffered file2-only changes#17

Merged
toddstrader merged 2 commits into
hudson-trading:mainfrom
jeet-dekivadia:stable-buffered-file2-diffs
Jun 9, 2026
Merged

Stabilize wavediff output for buffered file2-only changes#17
toddstrader merged 2 commits into
hudson-trading:mainfrom
jeet-dekivadia:stable-buffered-file2-diffs

Conversation

@jeet-dekivadia

Copy link
Copy Markdown
Contributor

Fixes #16.

Summary

This makes wavediff output deterministic for the file2-only rows that were buffered while reading ahead in file2.

The comparison was already detecting the mismatch correctly, but that reporting path iterated buffered2, a HashMap<(time, handle), OwnedSignalValue>. That meant the same logical diff could render in hash iteration order. The patch collects those buffered rows, renders their names, sorts by (time, name, value), and then writes them out.

I also added a tiny VCD regression where file2 has three intermediate changes at time 10 and file1 has a later time-20 change. That forces the read-ahead buffer and checks the user-facing order:

10 t.a 1 (only in file2)
10 t.b 1 (only in file2)
10 t.c 1 (only in file2)

Validation

cargo build --release
cargo test
cargo clippy -- -D warnings
git diff --check
cargo run --quiet --bin wavediff -- tests/data/buffered_file2_base.vcd tests/data/buffered_file2_extra.vcd

The manual CLI check exits 1, as expected for a real waveform difference, and prints the three buffered file2-only rows in stable order.

@toddstrader

Copy link
Copy Markdown
Collaborator

I'd propose changing to something like this in order to avoid the new string cloning:
a68ad36

@jeet-dekivadia

Copy link
Copy Markdown
Contributor Author

Thanks, agreed. I pushed 211e462 using that shape so the buffered values stay owned and the sorted rows carry an index instead of cloning the rendered value string per name.

Validation:

git diff --check
cargo test test_buffered_file2_only_diffs_are_sorted -- --nocapture
cargo run --quiet --bin wavediff -- tests/data/buffered_file2_base.vcd tests/data/buffered_file2_extra.vcd
cargo build --release
cargo test
cargo clippy -- -D warnings

@toddstrader toddstrader merged commit 113779a into hudson-trading:main Jun 9, 2026
1 check passed
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.

Stabilize wavediff output for buffered file2-only changes

2 participants