Skip to content

Feat/introduce verify command - #202

Merged
noamteyssier merged 8 commits into
dev-0.5.12from
feat/introduce-verify-command
Jul 23, 2026
Merged

Feat/introduce verify command#202
noamteyssier merged 8 commits into
dev-0.5.12from
feat/introduce-verify-command

Conversation

@noamteyssier

Copy link
Copy Markdown
Collaborator

No description provided.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Comment thread src/commands/verify/mod.rs
Comment on lines +52 to +56
struct VerifyResult {
fields: FieldMask,
checksum: u64,
num_records: usize,
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

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.

Suggested change
struct VerifyResult {
fields: FieldMask,
checksum: u64,
num_records: usize,
}
struct VerifyResult {
fields: FieldMask,
mate: Mate,
checksum: u64,
num_records: usize,
}

Comment on lines +97 to +120
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(())
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

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(())
}

Comment thread src/commands/verify/processor.rs Outdated
Comment thread src/commands/verify/processor.rs Outdated
@noamteyssier
noamteyssier merged commit b2b1fa4 into dev-0.5.12 Jul 23, 2026
8 checks passed
@noamteyssier
noamteyssier deleted the feat/introduce-verify-command branch July 23, 2026 23:04
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