fix: exit non-zero when vv can't honour the request#86
Merged
Conversation
Several failures were reported on stderr — or not at all — while the process
still exited 0, so a pipeline had no way to tell a typo or a truncated read
from a clean run. All reproducible on the previous build:
$ vv t.parquet --select Chr,Scoree --tsv ; echo $?
unknown column(s) in --select: Chr,Scoree
0
$ vv --count a.bed b.bed # b.bed dropped silently
$ vv t.bed --validate ; echo $?
Not a valid Parquet file: ...
0
write_delimited() and write_markdown() fprintf'd and returned void while only
write_json() returned a string main() checked; print_table() and
print_vertical_table() did the same. All five now return "" or an error, so
--tsv/--csv/--delimiter/--md/table/vertical/--parquet/--arrow agree with
--json. The message gained a did-you-mean (a capped Levenshtein over the
schema, shared by every --select consumer via unknown_columns_error()).
The --json/--md and --parquet/--arrow dispatches never called read_status(),
unlike the --delimiter path, so a mid-file read error produced truncated
output and exit 0. They check it now.
Extra positionals only mean something in the interactive viewer, which gives
each file a tab; every other mode read cfg.path and dropped the rest without
a word. That is an error now.
--validate is a LociSSD v3 (Parquet) invariants check but ran its Parquet
reader against any path, so a BED reported "Not a valid Parquet file" and
exited 0 — and a v4 "colblock" .lociss, the writer's current default and not
Parquet at all, got the same message. Gated on the extension plus the LSB1
magic, with a message for each case.
evicted_any() was declared in vvcore.hpp, overridden by seven streaming
sources, and called from nowhere. Forward-only sources keep a bounded window
of decoded batches, so a search / sort / filter / stats pass started after
batches were released sees only part of the file and used to present that as
complete. note_full_pass() records it at the three full-pass sites; the
status bar shows [PARTIAL] and the stats popup gains a Scope row. Deliberately
not wired into --describe / --unique / --sample / --tail: those are single
forward passes that consume each batch as it is produced, so eviction there is
normal and a marker would be a false alarm.
Also: --parquet - and --arrow - spool through a temp file (both formats need
seekable writes) and hardcoded /tmp; they honour $TMPDIR now, which unbreaks
containers with a small or read-only /tmp.
Exit code 2 is left alone — it already means "invalid option argument".
Tests: 400 pass (was 375), including the same suite rerun under ASan+UBSan.
New coverage asserts exit 1 for a bad --select across all nine output modes
and for a bad --filter across three, that a valid --select still exits 0, the
suggestion text, both --validate cases, the $TMPDIR spool round-trip with no
leftovers in /tmp, and a pty harness (tui_partial_pass_check.py) that drives
G-then-sort over a multi-batch FASTQ with VV_STREAM_BATCH_CAP=1 and asserts
[PARTIAL] appears — and does NOT appear with the default window.
multifile_cli_accepts_extra_paths asserted the old silent behaviour and is
replaced by multifile_cli_rejects_extra_paths.
Happy-path output is byte-identical to main (--tsv, table, --json, --md).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
Several failures were reported on stderr — or not at all — while the process still exited 0. A pipeline had no way to tell a typo or a truncated read from a clean run. Every case below reproduces on
main:What changed
Writers report failure.
write_delimited()andwrite_markdown()fprintf'd and returnedvoidwhile onlywrite_json()returned a stringmain()checked;print_table()andprint_vertical_table()did the same. All five now return""or an error, so--tsv/--csv/--delimiter/--md/ table / vertical /--parquet/--arrowfinally agree with--json. The message also suggests the intended column:A capped Levenshtein over the schema, shared by every
--selectconsumer through a newunknown_columns_error().Truncated reads surface. The
--json/--mdand--parquet/--arrowdispatches never calledread_status(), unlike the--delimiterpath — so a mid-file read error yielded a truncated result and exit 0.Extra positionals. They only mean something in the interactive viewer, which gives each file a tab. Every other mode read the first path and dropped the rest without a word; that is an error now.
--validate. It is a LociSSD v3 (Parquet) invariants check, but ran its Parquet reader against any path. A BED got "Not a valid Parquet file" and exit 0; a v4 "colblock".lociss— the writer's current default, and not Parquet at all — got the same misleading message. Now gated on the extension plus theLSB1magic, with a distinct message for each case.evicted_any()finally has a caller. It was declared invvcore.hpp, overridden by seven streaming sources, and called from nowhere. Forward-only sources keep a bounded window of decoded batches, so a search / sort / filter / stats pass started after batches were released sees only part of the file — and presented that as complete. The status bar now shows[PARTIAL]and the stats popup gains aScope: PARTIAL (batches released)row.Deliberately not wired into
--describe/--unique/--sample/--tail: those are single forward passes that consume each batch as it is produced, so eviction there is normal and a marker would be a false alarm.$TMPDIR.--parquet -and--arrow -spool through a temp file (both formats need seekable writes) and hardcoded/tmp. Unbreaks containers with a small or read-only/tmp.Exit code 2 is left alone — it already means "invalid option argument".
Compatibility
This is a deliberate break for anyone scripting the previously-silent behaviour, and it is the point of the change. Called out in its own
### ChangedCHANGELOG entry, and the man page's EXIT STATUS section now describes the new cases.Verification
--selectacross all nine output modes and a bad--filteracross three; a valid--selectstill exits 0; the did-you-mean text; both--validatecases; the$TMPDIRspool round-trip with no leftovers in/tmp.tests/tui_partial_pass_check.pydrivesGthen sort over a multi-batch FASTQ withVV_STREAM_BATCH_CAP=1and asserts[PARTIAL]appears — and does not appear with the default retention window, so the marker can't become noise.multifile_cli_accepts_extra_pathsasserted the old silent behaviour; replaced bymultifile_cli_rejects_extra_pathsplus exit-code assertions.main(--tsv, table,--json,--mdchecksums compared against a stashed build).libvvcore+vvg+vvkdetest) builds unchanged.Not in this PR
-ris a silent no-op on BAM/CRAM, Arrow, ORC, FASTA, SQLite and XLSX —vv reads.bam -r chrZZZ:1-2 --countreturns the whole file with exit 0. Same class of bug, but it needs real region support inBamSource, so it is its own change.🤖 Generated with Claude Code