Skip to content

Correct extract motif metadata and output accounting - #698

Open
SuhasSrinivasan wants to merge 2 commits into
nanoporetech:masterfrom
SuhasSrinivasan:codex/fix-extract-output-accounting
Open

Correct extract motif metadata and output accounting#698
SuhasSrinivasan wants to merge 2 commits into
nanoporetech:masterfrom
SuhasSrinivasan:codex/fix-extract-output-accounting

Conversation

@SuhasSrinivasan

Copy link
Copy Markdown
Contributor

Fixes #670.

Summary

  • Annotate valid reference coordinate zero with its motif instead of treating zero as “no reference position.”
  • Treat --cpg as a motif source for the documented full/calls schemas and motif-dependent annotate/mask options.
  • Count only rows actually emitted after pass-only filtering.
  • Add exact coordinate-zero, schema, option-combination, and accounting regressions.

Severity

Severity: Medium — scientific metadata, output schema, and reproducibility

Rationale: Coordinate zero is valid and common on short transcriptome contigs; mislabeling it loses scientific motif context. The --cpg column-count and option behavior contradicted documented schemas. The row-counter issue is Low alone but belongs to the same output contract.

Root cause

Reference coordinate was represented as a bare integer and zero doubled as a sentinel. Motif-source validation recognized explicit motifs but not the --cpg shorthand, and row accounting was incremented before pass-only suppression.

Implementation

  • Represent reference position presence explicitly and allow coordinate zero.
  • Route --cpg through the same motif metadata/schema capability as an explicit CG 0 motif.
  • Increment emitted-row counts only after a record is successfully written.

Preserved behavior

  • Extracted call values and pass-only TSV bytes are unchanged.
  • Explicit motif behavior and output column meanings are unchanged.

Non-goals

  • No threshold-population, sampling, or general writer-finalization change.
  • No change to motif matching outside extract output metadata.

Behavior before and after

Case Before After Oracle
Valid motif at reference position 0 Motif field was “.” Correct motif and offset Same result as equivalent nonzero coordinate
--cpg full/calls output 21/23 columns Documented 22/24 columns Schema contract
--cpg with annotate/mask Rejected Accepted like explicit CG 0 Shorthand equivalence
pass-only one-row output Log reported two rows Log reports one row Emitted-row count

Testing

Environment: macOS 26.6 arm64; rustc/cargo 1.90.0; installed modkit 0.6.4 reference; ignored test-only Cargo.lock SHA-256 78876ab4a98da30caad167744d1c8a0875c27edad7b0b3ad5f8a1891f78604ea resolving hts-sys 2.2.0. Cargo.lock is not in the diff.

  • Revision/tree: f552734 / 2f028f8b21ea52c2b0d2e94dd1bf4827c43ac6ad; clean tracked worktree.
  • Parent-red installed 0.6.4 reproduced coordinate-zero “.”, 21/23 --cpg columns, rejected option combinations, and the two-versus-one row-count mismatch.
  • cargo test --offline --locked -p mod_kit extract -- --test-threads=1: 4 passed, 0 failed.
  • cargo test --offline --locked -p modkit --test test_extract -- --test-threads=1: 17 passed, 0 failed.
  • cargo test --offline --locked --workspace --all-targets -- --test-threads=1: 182 active tests passed, 14 declared ignored, 0 failed.
  • Sixteen-run --cpg/explicit-motif matrix covered full/calls, annotate/mask, pass-only, and soft-mask controls.
  • Installed/fixed pass-only TSVs were byte-identical: 371 bytes, SHA-256 beginning 47b6836e; only the corrected reported row count changed.
  • git diff --check upstream/master...HEAD passed; the worktree remained clean.
  • Repository-wide stable cargo fmt --all -- --check reports unchanged upstream formatting plus nightly-only settings; no unrelated rewrite was made.
  • Test setup note: a first concurrent full-suite attempt collided with other worktrees on shared temporary BAM filenames and failed test_adjust_canonical. The same exact revision passed the complete suite when rerun serially.

Tests not performed

  • cargo clippy was not run.
  • The large direct-RNA slice was not used because the four-base fixture supplies exact coordinate, schema, and emitted-row oracles.

Scientific validation

  • Reference coordinate zero is retained as a real coordinate.
  • --cpg and explicit CG 0 select the same motif metadata behavior.
  • Row counts equal successfully serialized rows.
  • Existing call probabilities and pass-only output bytes are conserved.

Output and compatibility

  • Affected metadata and documented column counts intentionally change.
  • CLI options become consistent with the documented --cpg shorthand; no option is removed.
  • Pass-only scientific bytes remain unchanged.

Reviewer guide

  1. Review reference-position representation and motif annotation in extract/writer.rs.
  2. Review --cpg capability validation in extract/args.rs and subcommand.rs.
  3. Rerun the two focused commands above.

Checklist

  • The issue contains reproducible observed and expected behavior.
  • The change is limited to extract metadata/schema/accounting.
  • Parent-red and fix-green evidence is recorded.
  • All material tests and setup failures are listed.
  • Scientific metadata and byte-identical controls are checked.
  • Diff hygiene passed; unrelated format findings are disclosed.
  • No private data, generated lockfile, or unrelated change is included.

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.

extract can omit documented motif metadata and overreport emitted rows

1 participant