Feat/introduce verify command - #202
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new verify command to compute order-independent checksums over BINSEQ files using xxh3-64 and a commutative wrapping sum. The reviewer provided valuable feedback to improve correctness and portability: first, when processing single-end files, the --mate flag should be overridden to Mate::Both to prevent empty checksums if -M 2 is specified, and the actual mate used should be propagated to the final report. Second, to ensure the checksum is platform-independent, field lengths and flag values should be written to the hasher using a fixed endianness (little-endian) rather than native endianness.
| struct VerifyResult { | ||
| fields: FieldMask, | ||
| checksum: u64, | ||
| num_records: usize, | ||
| } |
There was a problem hiding this comment.
To support accurate reporting when the --mate option is overridden/ignored (e.g., for single-end files), let's include the actual mate used in the VerifyResult struct.
| struct VerifyResult { | |
| fields: FieldMask, | |
| checksum: u64, | |
| num_records: usize, | |
| } | |
| struct VerifyResult { | |
| fields: FieldMask, | |
| mate: Mate, | |
| checksum: u64, | |
| num_records: usize, | |
| } |
| pub fn run(args: &VerifyCommand) -> Result<()> { | ||
| let result = compute(args)?; | ||
|
|
||
| if args.opts.json { | ||
| let report = VerifyReport { | ||
| path: args.input.path().to_string(), | ||
| algorithm: "xxh3-64/wrapping-sum", | ||
| fields: field_labels(result.fields), | ||
| mate: mate_label(args.opts.mate), | ||
| num_records: result.num_records, | ||
| checksum: format!("{:016x}", result.checksum), | ||
| }; | ||
| println!("{}", serde_json::to_string_pretty(&report)?); | ||
| } else { | ||
| println!( | ||
| "{:016x}\t{}\t{}", | ||
| result.checksum, | ||
| result.num_records, | ||
| args.input.path() | ||
| ); | ||
| } | ||
|
|
||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
Update the JSON report to use the actual mate used during the computation (result.mate) rather than the raw CLI option (args.opts.mate), ensuring the report is accurate when the option is overridden.
pub fn run(args: &VerifyCommand) -> Result<()> {
let result = compute(args)?;
if args.opts.json {
let report = VerifyReport {
path: args.input.path().to_string(),
algorithm: "xxh3-64/wrapping-sum",
fields: field_labels(result.fields),
mate: mate_label(result.mate),
num_records: result.num_records,
checksum: format!("{:016x}", result.checksum),
};
println!("{}", serde_json::to_string_pretty(&report)?);
} else {
println!(
"{:016x}\t{}\t{}",
result.checksum,
result.num_records,
args.input.path()
);
}
Ok(())
}
No description provided.