3.2.0: CLI redesign, input layer, terminal UI, p-sequence analytics - #1
Merged
Conversation
- Use closefd=False when opening stdin to prevent closing the real stdin when the wrapper is closed, making the contract uniform: all returned streams are safe for the caller to close. - Wrap codec detection and decompressor instantiation in try/except to ensure raw fd is closed if any downstream operation raises (e.g. when zstandard is unavailable). - Add regression test for fd leak when zstd is unavailable. - Add regression test for stdin close safety.
…ssion - Introduce _ClosingTextIOWrapper to ensure underlying file handles are closed when the returned stream is closed, fixing a critical fd leak on gzip (and unifying behavior across all codecs). - Use GzipFile, BZ2File, LZMAFile directly with extra_close to guarantee cleanup regardless of the codec's individual close behavior. - Replace vacuous zstd fd-count test with deterministic test that captures the opened handle and checks .closed directly. - Add parametrized fd-release test for all four working codecs that proves closing the returned stream does not leak underlying fds. - Tests now verify the fix: gzip fd leak is gone, zstd construction failure closes raw deterministically, and all codecs release fds uniformly.
The test_open_text_does_not_leak_fd_when_zstd_unavailable test was replaced but not removed in round 2, creating a duplicate. It passes against broken code due to CPython finalization masking the fd leak via __del__, making it vacuous. The replacement test_open_text_closes_raw_when_codec_construction_fails is deterministic and directly checks .closed, properly catching the defect. Remove the vacuous test entirely and its now-unused sys import.
…kipping Two critical fixes: 1. Change encoding from 'utf-8' to 'utf-8-sig' in _compress.py to strip UTF-8 BOM (byte order mark) that Windows/Excel editors add. A BOM would merge into the first sequence, corrupting any graph built from that file, and would corrupt TSV column detection by merging BOM into first column. utf-8-sig is safe on files without BOM (behaves identically). 2. Update _first_meaningful() to skip ';' comment lines in addition to blank lines, so detector and reader agree about the same file. Without this, a file like ';note\n>seq1\nCASS\n' would parse correctly through iter_fasta but be rejected by looks_like_fasta, causing Task 8's dispatch to route it to the wrong reader. Add regression tests: - test_open_text_strips_utf8_bom: BOM round-trip through _compress - test_iter_fasta_with_utf8_bom: BOM'd FASTA end-to-end - test_looks_like_fasta_skips_semicolon_comments: detector/reader alignment
Adds looks_like_fastq() detector and iter_fastq() streaming reader to support FASTQ input. Reader yields only sequence lines from strict 4-line records, rejecting malformed records with FormatError. Detector and reader are aligned to reject/accept the same input.
…t on blank lines Fixes three issues from code review: 1. iter_fastq now skips empty sequence lines (matching iter_fasta behavior), preventing empty strings from reaching graph builders. 2. looks_like_fastq now checks unfiltered line positions relative to the first @ header, allowing detection of blank lines inside records. This ensures detector and reader agree: both reject records with blanks at position 1. Blank lines between records remain unaffected. 3. Added explicit comment explaining that a record truncated after the separator is accepted because the sequence is already complete. Adds four new test cases verifying this alignment.
Adds sniff_delimiter, resolve_columns, and iter_tabular_rows so delimited-file ingestion resolves a named sequence column via csv.DictReader instead of splitting whole rows as sequences. Closes the CSV silent-corruption defect where comma-joined rows (including quoted fields containing the delimiter) were emitted as sequences.
Round-1 review findings on Task 6:
- iter_tabular_rows silently coerced "3.0" (and similarly "1e3") to an
abundance of 1 instead of 3 / 1000. pandas/R/Excel routinely emit
count columns as floats whenever a single NaN is present, so this is
a realistic and invisible quantitative bug (abundance drives edge
weights). Now tries an exact int() parse first (preserves precision
for counts beyond 2**53, e.g. "12345678901234567890123"), and only
falls back to float() + is_integer() for values like "3.0"/"1e3".
Genuinely non-integral counts ("3.7") and unparseable ones ("NA",
"") still default to 1, matching prior behavior.
- resolve_columns did not strip a user-supplied --column value before
lowering it for lookup, so whitespace-padded names (e.g.
" junction_aa ") failed to resolve against an otherwise-matching
header. Header names were already stripped; the user-supplied side
is now stripped too.
Added a parametrized coercion-contract test and a whitespace-padded
column resolution test to tests/test_io_tabular.py.
Round-2 review finding on Task 6: the float() fallback added for
Finding 1 (round 1) silently lost precision for integer-valued counts
written with a decimal point above 2**53, e.g. "9007199254740993.0"
became 9007199254740992 and "12345678901234567890123.0" became
12345678901234567741440. This is the same pandas/R/Excel float
promotion as before, just at larger magnitude, and the corruption was
silent.
Replaced the float() fallback with decimal.Decimal, which parses
exactly at any magnitude. Guarded explicitly against non-finite values
(Decimal("nan")/Decimal("inf") parse without raising, and
Decimal("inf") is its own to_integral_value(), so is_finite() must be
checked or "inf" would otherwise coerce to an abundance).
Extended the parametrized abundance-coercion test with the two
large-magnitude cases plus "nan" and "inf".
Wires the format detectors and open_text into detect_format(), which reads a bounded prefix, classifies fasta/fastq/tabular/plain/plain_seqcount, and resolves tabular columns via resolve_columns. Also fixes looks_like_seqcount, which gated on parts[1].isdigit() while _readers._parse_count accepts the float forms pandas/R emit (e.g. "3.0"). That mismatch let pandas-emitted seqcount files fall through to plain, where iter_plain yielded the whole "seq\t3.0" line as a sequence. Both now share a single _is_count predicate (in _readers.py) so they cannot drift apart again. Also fixes FASTQ alphabet inference, which previously included the quality line in its samples (a "does not start with @/+" filter does not exclude it), letting Phred-quality letters corrupt infer_alphabet's verdict for ordinary nucleotide reads.
read_sequences/read_sequences_simple/detect_input_kind now sniff format
via detect_format and read via the streaming iter_* readers, adding
fasta/fastq/gzip support to read_sequences without changing its
signature or return shape.
Fixes beyond the plan's own code, required to avoid regressions:
- v_genes/j_genes must stay None (never an empty-but-non-None list) when
a format carries no gene data or no_genes=True. An empty list is not
just wrong Python-side (tests/test_io.py asserts v_genes is None for
plain_seqcount) -- the C extension only checks v_genes/j_genes for
NULL, not length, so an empty list with real sequences reads past its
own allocation. Verified this corrupts a live graph (has_gene_data
reports True with nothing to back it, gene_info UTF-8-decode-fails on
garbage bytes).
- detect_input_kind must never raise: _legacy.py's version never did,
and cli.py calls it unguarded before its own validate-input error
reporting. Falls back to a first-line heuristic (sharing _is_count so
it can't drift from looks_like_seqcount) when detect_format raises.
- stdin ("-") can only be consumed once; detect_format's peek-then-close
design assumes path is reopenable from byte zero, which silently
dropped all piped input on the second open. Buffered to a temp file
before sniffing.
- v_column/j_column/abundance_column were accepted but never wired to
anything; now applied against the tabular header (fail-soft, like
legacy, unlike seq_column).
- A narrow strict_input carve-out (mixed plain/plain_seqcount record
detection only) preserves the one existing strict-mode test that reads
through read_sequences directly; full malformed-record accounting
stays deferred to a later task.
508 passed/5 skipped -> 527 passed/5 skipped (19 new tests, 0 regressions).
…ion suite Extends anticorruption tests with exact-equality assertions for seqcount format. Forbidden-character checks miss leaked counts, so seqcount now has dedicated exact-equality tests that catch numeric escapes regardless of digit composition.
Restores strict_input's malformed-record rejection on top of Task 9's mixed-record check, adds RecordStats (total/kept/malformed/nonproductive) to read_sequences, and drops non-productive AIRR rows by default with a keep_nonproductive escape hatch. RecordStats documents an honest accounting gap: readers silently drop empty-sequence records before any observable yield point, so total/malformed can undercount the source file.
_is_wellformed previously required plain isalpha(), which rejected '*' (stop codon) and '-'/'.' (alignment gap) -- both legitimate in real AIRR amino-acid data. A stop codon is the most common reason a row is non-productive, so this silently defeated keep_nonproductive=True for its primary purpose: the rows it exists to preserve were the exact rows the malformed check then discarded anyway. Widen to an explicit allow-list (letters plus '*-.') so the corruption guarantees the anti-corruption suite depends on stay obvious and unchanged.
Round 1's fix widened _is_wellformed to accept '*', '-', and '.' without
requiring an actual letter, which let VCF-style/general missing-value
sentinels ('-', '.', and combinations of the three marks) qualify as
"sequences" and pass straight into a graph uncounted. Require at least
one alphabetic character in addition to the existing allow-listed
residue marks, so a stop codon or gap next to real letters still passes
but the marks alone do not.
detect_input_kind is content-aware and transparently decompresses to
sniff, so it correctly reported 'plain'/'plain_seqcount' for bzip2, xz,
and misnamed-gzip files. But cmd_build's can_stream_plain gate combined
that with a filename check (endswith('.gz')), so those genuinely
compressed files passed the gate and had their raw compressed bytes
streamed straight into the C builder, producing a small, wrong graph
with exit code 0 and no warning.
Gate on detect_format's actual .compression field instead: only take
the fast path when compression is 'none' and format is plain or
plain_seqcount. Thread the existing --expect-format through as an
override, matching what validate_input/read_sequences already do, so a
user-declared format still reaches the fast path. A detection failure
(empty/binary input) falls back to "don't stream" rather than
propagating a new exception from this gate.
Also:
- fix a docstring on detect_input_kind that claimed it always returns
a label; it can still raise via its _first_line_kind fallback.
- remove two exact-duplicate tests in test_io_record_policy.py.
… tabular A single-column file whose only column is named junction, sequence, or aminoAcid detected as plain, so the header row was ingested as a sequence. cdr3 and junction_aa escaped only by accident, because they contain a digit/underscore that _is_wellformed happens to reject downstream -- an incidental protection, not a designed one. detect_format now recognises a lone undelimited header line that case-insensitively matches any known sequence-column name (the union of _SEQ_COLUMNS across all variants, plus _SEQ_FALLBACK) and reclassifies the file as tabular with that column resolved directly, bypassing resolve_columns's variant-restricted candidate list since a single-column file has no ambiguity to resolve. Verified csv.DictReader's single-field-per-row behavior on undelimited data directly before relying on it in iter_tabular_rows.
…assification Reviewer-requested regression coverage for Task 1's lone-header fix: an explicit expect_format="plain" override short-circuits detect_format's auto-detect branch, so it still wins over the new reclassification and reads a lone junction/sequence/aminoAcid header back as data. That is correct and intentional (explicit overrides beat content sniffing throughout this codebase), but was previously unpinned. No production change; adds two tests bracketing the override boundary.
lzg validate-input ran its own first-line-only classifier while lzg build used detect_format, so the two contradicted each other on the same file: a 2-record FASTA was reported ok=yes, detected_kind=plain, records=4, counting the '>' header lines as sequences. Reimplement validate_input in a new _io/_validate.py built on detect_format and the shared readers (iter_fasta, iter_fastq, iter_tabular_rows, _is_wellformed), preserving the 22-key report contract exactly. detected_kind now reports the sniffed format and records counts what the reader would actually yield, so the FASTA case now reports fasta/2 records in agreement with read_sequences. expect_format is checked against the naturally sniffed format (never passed as detect_format's override) so it stays an assertion, not a silent coercion -- this is what stops cmd_build's raw-streaming fast path (which does force an override) from building a graph off a misdeclared file. Malformed records now become warnings or errors depending on strict_input, rather than legacy's unconditional errors. _legacy.py is untouched; Task 3 removes it once nothing imports it.
Every issue in the new validate_input silently dropped its position, since
_add_issue was never called with line=. cli.py renders "line={n} {message}"
only when line is present, so users lost the locator entirely -- a real
diagnostic regression for a command whose purpose is pre-build diagnosis.
Give every issue a real position, honest about what the number means:
- plain/plain_seqcount: line is the true 1-based file line, counting blank
lines (read off the _CountingStream wrapper's total_lines at the point
each raw line is processed, since __next__ counts before returning).
- tabular: line is the true file line (data row index plus 1 for the
header), read off the same counter at the point each row is yielded by
iter_tabular_rows -- verified directly against a file with a known bad
row rather than assumed (header=1, first row=2, ...).
- fasta/fastq: a record spans several physical lines, so a record ordinal
is not a line number. line is left unset and the record's 1-based
position is folded into the message text instead ("record 2: ...").
…eferences The _legacy.py validator was fully replaced in the previous task and is now completely unreachable. Drop the file via git rm and rewrite five docstrings that referred to it by name, preserving the semantic intent without the deleted module reference. - Deleted src/LZGraphs/_io/_legacy.py (598 lines) - Rewrote 5 docstring/comment references: * _validate.py:4 - removed module reference, kept rationale * _public.py:64 - changed "__legacy.py__" to "original per-line approach" * _public.py:154 - changed "__legacy.py__'s fail-soft" to "previous fail-soft" * _public.py:294 - changed "detect_input_kind" ref to "first-line detection" * _public.py:322 - changed "detect_input_kind" ref to "legacy first-line approach"
Single source of truth for terminal capability decisions (TTY, colour depth, width, unicode, Windows VT) that the rest of the _term rendering layer will query instead of reading os.environ or calling isatty directly. Implements the NO_COLOR/TERM=dumb/CI/FORCE_COLOR precedence rules from the Plan 2 term-layer spec, with resolve_mode() folding the env-derived auto-mode choice into Capabilities.interactive so later modules only ever see caps + an explicit request. 42 new tests in tests/test_term_caps.py cover every precedence rule with injected fake streams/env, width clamping at both ends, and the unicode probe across ascii/utf-8/missing/None encodings. Full suite: 864 passed, 5 skipped (was 822/5).
Ruling from spec review: a capability limit overrides explicit intent, a policy default does not. TERM=dumb means the terminal has declared it cannot process cursor movement, so an explicit rich request must downgrade to plain exactly like a non-TTY stream does. CI/GITHUB_ACTIONS/ GITLAB_CI are a policy default only, since a CI runner can still allocate a real TTY and a user who explicitly asks for rich there means it; CI only steers the automatic (no --ui) choice. Adds Capabilities.supports_cursor_control (is_tty, not TERM=dumb, and Windows VT ok) as the capability-only signal resolve_mode's rich branch now keys off, keeping resolve_mode a pure function of caps + requested with no env parameter. interactive (the auto-mode default) is now supports_cursor_control plus the CI check, so the two concerns are named separately instead of conflated in one boolean. 7 new tests pin the reproduced matrix (TERM=dumb+rich -> plain, CI+rich -> rich, quiet unaffected by TERM=dumb) plus direct checks on supports_cursor_control. Confirmed test (a) failed against the pre-fix code before applying the change. Full suite: 871 passed, 5 skipped (was 864/5).
_terminal_size() called bare shutil.get_terminal_size(), which queries sys.__stdout__ internally and silently ignored the stream detect() was given. With stderr attached to a real terminal and stdout redirected (exactly how `lzg build > out.json` is meant to be run), this reported stdout's non-terminal fallback width instead of stderr's real size. _terminal_size(stream) now tries os.get_terminal_size(stream.fileno()) first and only falls back to shutil.get_terminal_size() when the stream has no usable fd: no fileno attribute, fileno() raising (OSError/ValueError/io.UnsupportedOperation), a non-terminal fd, or a reported size of 0 columns (some CI ptys are allocated but never sized). Added pty-backed tests that exercise the real stream.fileno() path (skipped with an explicit reason where pty is unavailable) alongside the fallback-path tests, and converted the four width/height tests that previously monkeypatched shutil against a fileno-less fake stream to real pty-sized streams, since that exercises the actual injection point rather than bypassing it. Confirmed the headline regression test fails against the pre-fix code (80 instead of 137) before restoring the fix. Full suite: 875 passed, 5 skipped (was 871/5).
Provides sgr, fg, bold, dim, reset, cursor_up, clear_line, hide_cursor, show_cursor, and visible_len for the terminal rendering layer. The five palette names (ok, warn, error, accent, muted) map to 256-colour and 8-colour values chosen for legibility on both light and dark backgrounds, with reasoning in the module docstring. visible_len strips CSI escapes via regex and counts every remaining character as one column; its documented limits (wide characters, combining marks) are pinned by tests. 114 new tests in tests/test_term_ansi.py.
… depth 0 reset() previously took no caps and always emitted \x1b[0m, so the natural fg(name, caps) + text + reset() composition leaked an escape sequence when colours == 0 (NO_COLOR, TERM=dumb, CI, piped output), violating the plan's zero-escape-at-depth-0 guarantee. reset(caps) now gates like fg/bold/dim. Adds colour(name, text, caps) as the primary widget-facing API: one call, wraps and resets, returns text unchanged at depth 0, impossible to leak by construction. fg/bold/dim updated accordingly (bold/dim now call reset(caps) internally instead of a bare sgr(0)). sgr/cursor_up/clear_line/hide_cursor/show_cursor remain ungated by design: sgr is an internal-only primitive, and the cursor quartet answers a different capability question (caps.supports_cursor_control) that is already decided once, upstream, by which render mode gets selected. 44 new tests, including an introspection-driven guard (test_purity_no_gated_function_leaks_escape_at_depth_0) that automatically covers every current and future _ansi function accepting `caps`.
…ards Adds bar, sparkline, panel, kv, counter, duration, bytes_human, and card as pure functions of data plus Capabilities: no I/O, no state. Panel/card lines always measure exactly caps.width via visible_len-driven pad/truncate, even across colour depth, unicode fallback, and the below-50-column narrow mode that drops the box for plain key: value lines. 348 new tests (table-driven across widths 40/60/80/100 x colour depths 0/8/256 x unicode True/False), all degenerate inputs from the task spec pinned, ruff clean.
…tocol PlainRenderer is a different layout from the rich renderer, not a de-styled one: one tagged key=value fact per line, modelled on the [build] phase=... lines cli.py already emits, never boxed, never redrawn. progress() is throttled to at most every 5 percentage points or 1 second per label, with label-change and 100%-completion always forcing a line through. error() stays readable without colour via a bare status=error header plus indented detail= continuation lines. The Ui Protocol both this renderer and the upcoming rich one satisfy lives in _term/__init__.py, per that module's own docstring, which already earmarked itself as the package's public-surface home.
RichRenderer satisfies the Ui protocol and owns terminal state: hides the cursor on start, shows it on stop, and guarantees restoration through try/finally (context-manager __exit__), an atexit fallback, and correct behaviour when KeyboardInterrupt propagates uncaught -- verified with a real pty, an in-process context manager, and a real subprocess. Redraws track exactly the previous frame's line count (never guessed) so a shrinking frame leaves no orphaned lines, throttles to 15 redraws/second via an injectable clock, and reflows on SIGWINCH with guards for platforms without it and for non-main-thread registration.
Fix round 1 on the _plain renderer, per review: - The progress() throttle was keyed on a single global (label, pct, time) tuple, so alternating labels defeated it entirely (10,000 calls across two alternating labels emitted 10,000 lines, not the ~42 the per-label rule promises). Reproduced directly against the pre-fix module before changing it. Now keyed per label in a size-capped OrderedDict (_MAX_TRACKED_PROGRESS_LABELS = 32) with least-recently-updated eviction, so an unbounded/high-cardinality label cannot grow this renderer's memory without limit. - A value not representable in the stream's encoding (a non-ASCII sequence id on a LANG=C stderr) raised UnicodeEncodeError and took down the command reporting it. Reproduced directly. Writes now go through PlainRenderer._write, which retries with the stream's own encoding and errors="replace" on that specific exception only. - Ui is now @runtime_checkable, and both PlainRenderer and the now-landed RichRenderer are checked against it: isinstance for method presence, plus an inspect.signature shape comparison (name/ kind/required-ness) for actual drift detection. - NaN fraction now clamps to 0% explicitly rather than silently becoming 100% through comparison semantics. The module docstring's unverified "no emitted value contains word=" claim is softened to what is actually guaranteed and pinned by a test showing the documented mis-split.
Fix round 2, per review: the per-label LRU cap (32 tracked labels) could itself be defeated by more distinct labels than the cap, since every call then evicts the entry it needs, making every call look like a fresh label and bypassing the per-label throttle entirely (reproduced directly: 200 rotating labels over 10,000 calls emitted all 10,000 lines against the round-1 module). Adds an unconditional, per-label-agnostic sliding-window cap (_MAX_PROGRESS_LINES_PER_SECOND = 100 per _GLOBAL_RATE_WINDOW_SECONDS = 1.0s) checked after the per-label decision, so none of the per-label "forced" exceptions (first call, label change, 100% completion) are exempt from it - exempting them is exactly how the per-label cap gets defeated one level up. The existing frozen-clock single/alternating/three-label counts (21/42/63) are unchanged since 100 sits comfortably above all of them; new tests pin the 200-label pathological case at exactly 100, and two more prove the backstop does not suppress legitimate output once the clock genuinely advances instead of staying frozen.
…GWINCH Fix round 1 on _rich.py, per review: - A frame taller than the terminal desynced every subsequent redraw: cursor_up cannot move above the physical top row, so once a frame scrolled the arithmetic addressed the wrong rows and the display corrupted progressively. Reproduced directly (a 20-field frame on a height-6 terminal issued cursor_up amounts of 22/24/25). Two layered bounds now key off max(1, caps.height - 1): _clamp_rows performs deliberate elision in _render_frame (drops static fields from the front, keeps the live progress/warnings tail, marks how many rows were hidden rather than truncating silently), and _fit_to_terminal is a blunt safety net in _draw for every frame including the final done()/error() card, keeping the top (the error headline) when even panel()'s own border overhead cannot fit a degenerate height of 0, 1, or 2. - _install_sigwinch was not idempotent: a second start() without an intervening stop() re-registered and captured this renderer's own handler as "the previous one", permanently losing the true original. Installation now skips when already installed by this instance, and _restore_sigwinch only restores when the currently active handler is still this instance's own (checked with == against a bound method, not is, since bound methods compare equal but are never identical across separate attribute accesses), so two renderers stopped out of order cannot clobber each other. Documented as making the common cases safe rather than building a full handler stack. - _refresh_capabilities re-detected the whole Capabilities on resize, letting an unrelated environment change silently flip colour depth or unicode support mid-render. Now re-detects width/height only and carries every other field forward via dataclasses.replace.
Adds LZGraphs._term.ui(requested, stream, env), resolving Capabilities and
mode into a ready renderer, plus NullRenderer so quiet mode satisfies Ui
as a no-op instead of every caller branching on mode.
Migrates cmd_build (only) from bare _stderr calls to this layer: source/
format/engine on start(), phase=read/construct/save facts and the dropped-
records and alphabet-mismatch warnings via update()/warn(), a done() card
on success, and an unconditional error() card (plus the existing raise)
on the two fatal paths. Adds --ui {auto,rich,plain,quiet} and --no-color
to the global parser; -q/--quiet forces quiet regardless of --ui, and
--ui quiet on its own now also silences the C library's own
set_log_level-driven logging (previously only -q or an explicit
--log-level none did). In rich mode the C library's streaming-ingest
pct= log lines are bridged into term.progress("ingest", ...) instead of
being suppressed outright, so the fast path keeps showing live progress
without a second writer racing the live redraw on stderr.
34 new tests in tests/test_cli_ui_integration.py cover stdout purity
across all four modes (both the streaming and in-memory code paths),
every documented fact surviving in --ui plain, quiet emitting nothing but
errors, NO_COLOR/--no-color stripping colour, node/edge-count parity
across modes, and real-pty rich-mode behaviour (panel, error card, cursor
restoration, no raw C log leakage).
…seam bugs tests/test_term_quality.py drives the whole _term layer end to end as six properties (stdout purity, rendering overhead, environment matrix, width matrix, encoding safety, cursor restoration through a real SIGINT) rather than more per-module examples, since each of Tasks 1-6 only ever saw its own module. Writing the encoding-safety and cursor-restoration tests surfaced two real bugs in _rich.py that no per-module suite had caught: RichRenderer wrote straight to stream.write() with no UnicodeEncodeError guard (unlike PlainRenderer, which already had one), and never sanitized embedded newlines in field/row/warning values (unlike PlainRenderer's _sanitize), which silently desynchronises self._lines_drawn from the real terminal row count and corrupts every subsequent redraw's cursor_up arithmetic. Both fixed here (_safe_write, _sanitize_line) and verified by mutation.
… warn() Fix round 1 for the ui()/cmd_build task, addressing two review findings. Finding 1 (Critical): PlainRenderer._emit always prepends its own status=<word> token, but cmd_build passed a domain field also named "status" (phase=save status=start/done, phase=validate-input status=ok/error), producing two status= tokens on one line that a key=value parser cannot recover both values from. Fixed in the renderer: _plain.py now reserves its own unconditional field names (_RESERVED_FIELD_KEYS) and _disambiguate_fields() deterministically renames a colliding caller key (status -> status_, status__, ... as needed) before formatting, so no emitted line can ever carry a duplicate key, regardless of which future command reaches for the same word. Also renamed cmd_build's own colliding field to `stage`, so the output reads cleanly instead of relying on the renderer's fallback disambiguation. Finding 2 (Critical): _bridge_c_log_to_ui dropped every C-library log message lacking a pct= field, including real WARN-level messages (a mixed-format file the streaming reader had to recover from mid-file, graph_finalize.c's "no @ root node found"), so they never reached a user on a real terminal. Rewritten to route by the actual LZGLogLevel integer the C library passes (not message text): ERROR/WARN and any unrecognised level go to term.warn() unconditionally; INFO with pct= still becomes progress(); INFO/DEBUG/TRACE without pct= (already-duplicated one-shot chatter) is still dropped, matching the review's explicit guidance that informational chatter may be. Reproducing this end to end also surfaced a third bug: RichRenderer.warn() went through the throttled redraw, so a warning landing in the same window as the done()/error() that closes a fast build was silently never painted at all; warn() now forces its redraw, the same way start()/done()/error() already do. 18 new tests: a real-key-boundary-parser property (no emitted line ever has a duplicate key) plus disambiguation unit tests in test_term_plain.py; severity-routing unit tests and a real-pty end-to-end reproduction (a genuine mixed_input_format C warning surfacing in rich mode) in test_cli_ui_integration.py. Full suite: 1559 passed, 5 skipped (was 1546). ruff clean on _term/; cli.py holds at its 7 pre-existing findings.
… final card Final fix wave for Plan 2's terminal layer before manual design review: - C1: term.start() was gated behind show_info, so RichRenderer's session never opened at --log-level warn/error, silently no-op'ing every subsequent warn()/progress()/update() call (rich mode emitted zero bytes even with a real warning firing). start() now always runs; show_info still gates which facts it opens with. - C2: done()/error() built their card only from caller rows, so a warning shown live was wiped by the very next screen. Both now append retained warnings (capped, with an explicit elided count) via _append_warning_rows. - I1: _fit_to_terminal silently truncated an oversized card with no marker; now mirrors _clamp_rows' visible elision marker. - I2: both renderers caught only UnicodeEncodeError on write/flush; now also catch OSError and degrade, so a full disk or closed pipe cannot kill a build. - D1/D2: the live panel now paints a curated, stably-ordered field set (source/format/engine/nodes/edges) with counter()-formatted numbers, instead of echoing every internal update() key. - D3: long source/output paths are truncated in the middle (keeping the filename) instead of losing it to head-truncation. - D4: the rich done() card now shows elapsed time, matching plain mode. - D5: the in-memory read path now reports progress via a new read_sequences(progress_cb=...) parameter for uncompressed input, so an ordinary tabular build shows a real bar; compressed input is left unsupported (documented limitation) since a decompressed-stream-based fraction would be misleading against the compressed file's size. Verified by mutation for C1/C2/I2 (11 mutations, 11 caught, no survivors). 1578 passed, 5 skipped (was 1559/5).
… design review kit The branch still declared 3.1.0, which is already a published tag and an ancestor of HEAD, so tagging would have collided or republished a whole new _io and _term layer under a shipped version. The changelog had no entry for any of this work. It now records both silent-corruption defects the effort was chartered to fix, the eleven further input defects found while closing them, and the behaviour changes a user will notice, including input that now fails loudly where it previously produced a wrong graph with exit code 0. try_the_cli.sh builds mock BCR repertoire data and walks the CLI so the design can be judged on realistic input. It deliberately includes a 400k-sequence plain uncompressed file, because the live progress bar only appears on that path and a smaller test would suggest it was never built.
An audit applied around fifty mutations to the layer; four survived, and one test could pass while the renderer drew nothing at all. - The width-matrix test asserted only that no line was too wide, so writing an empty frame satisfied it. It now also asserts output was produced. - warn()'s forced redraw was pinned only by a pty test that skips on Windows and takes nineteen seconds, leaving a real fix from earlier in this plan unprotected on those runners. A frozen-clock unit test now pins the redraw count across start, warn, update and done. - Nothing asserted that start, progress and info stay uncoloured, though the module documents that as a contract, so colouring them would have gone unnoticed. - Nothing asserted that the deliberate row elision and the blunt height clamp agree, so an off-by-one in the panel overhead silently chopped the closing border instead of eliding a row. PlainRenderer.done() also re-derived its line tag from the caller's title, so a different title would have flipped the prefix and broken grep on the renderer's own tag. It is now stable for the renderer's lifetime.
pyproject.toml's [tool.setuptools] packages = ["LZGraphs"] listed only the top-level package, silently omitting LZGraphs._io and LZGraphs._term (added by the CLI redesign). Every wheel or sdist built from this config installed a package that raised ModuleNotFoundError on LZGraphs._io the moment the CLI parser was constructed, breaking every `lzg` invocation including `lzg --version`. Found while building a real wheel to smoke-test the new release pipeline. Switched to setuptools.packages.find so new subpackages are picked up automatically.
Replaces build-wheels.yml (stored PyPI token, no tag/version check, no wheel smoke test, GitHub release created after the irreversible PyPI publish) with a pipeline adapted from AlignAIR's release.yml: - guard: fails unless the ref is a real vX.Y.Z tag and it equals the version LZGraphs reports (resolved via setuptools' own read_attr against src/LZGraphs/__init__.py, the same mechanism the build itself uses for its dynamic version, since pyproject.toml never contains the literal version). - test/build_wheels/build_sdist: unchanged test matrix and cibuildwheel config, gated behind guard. - smoke: installs a real built wheel into a clean venv and runs the `lzg` CLI end to end via the new scripts/release_smoke.sh (a small non-interactive script; try_the_cli.sh's own mock-data generator was reused, stripped of its pause/tty logic since it is meant for a human to watch a real terminal, not CI). - draft-release: creates a draft GitHub release with all wheels, the sdist, and SHA256SUMS.txt BEFORE any irreversible publish; rerun-safe (create if absent, --clobber if a draft exists, refuse if published). - ghcr: builds the new Dockerfile's image, smoke-tests it in-container with the same release_smoke.sh, then pushes :<version> and :latest. Placed before the PyPI step since a container push is reversible and PyPI is not. - pypi: Trusted Publishing (OIDC) via id-token: write and a protected `pypi` environment, no stored token. - publish-release: flips the draft to published only after GHCR and PyPI both succeed. Dockerfile is a two-stage build (python:3.12-slim + build-essential to compile the C extension into a wheel, then the same slim base with only that wheel installed) so the image can still build LZGraphs' C extension without carrying a compiler in the final layer.
CI badge points at the existing Tests workflow (test.yml). PyPI, Python versions, and license badges already existed. The Install section keeps pip install LZGraphs as the primary path and adds the ghcr.io/mutejester/lzgraphs one-liner published by the new release pipeline.
The repository's latest published tag is v3.1.0, so this release is 3.2.0. Updates the package version and the changelog heading.
Various editor and tooling setups drop working directories and local note files into the repo root. Ignore them so they never get committed.
path_count was accumulated in a double, so it lost precision above 2^53 and overflowed to infinity past ~1.8e308. Real repertoire graphs are well past both: the 71k-node foundation graph has a 36-digit path count, of which a double kept 16. Adds lzg_flashback_path_count_exact(), which carries the count in base-2^32 limbs across a topological DAG dynamic program and hands the limbs back for the caller to assemble. The existing double-returning entry point is left alone.
Adds FlashBackGraph.pseq_analysis(), which describes the graph's sequence-probability spectrum without simulating a single walk. The core is the power-sum transform M(q) = sum_s P(s)^q, evaluated as a forward dynamic program over the edges, so the transform, its derivatives, and the surprisal moments and cumulants that follow from them are exact rather than estimated. Built on that: length-stratified mass and moments, exhaustive atom enumeration for small supports, a deterministic surprisal-grid histogram with a stated rounding bound for large ones, Lugannani-Rice saddlepoint inversion, single-sequence positioning, and finite-depth richness and frequency-spectrum prediction. path_count now returns an exact Python int through the new C entry point instead of a float, and raises RuntimeError when the graph has no topological order. pgen_distribution() still works but is documented as the legacy Gaussian-mixture approximation; its component fit uses sampled walks, and pseq_analysis() is the sampling-free replacement. Version 3.2.0.
Brings master's 3.2.0 release content onto the CLI branch. The two 3.2.0 changelog entries are folded into one, in Keep a Changelog section order. FlashBackGraph keeps the branch's format-detecting from_file, with the unquoted annotation style master moved the rest of the file to. Full suite on the merged tree: 1633 passed, 5 skipped.
The three helpers that launch the CLI in a subprocess copied os.environ wholesale, so every variable _caps consults to pick a rendering mode came from whatever shell or runner happened to be executing pytest. On GitHub Actions CI=true and GITHUB_ACTIONS=true are always set, which correctly forces auto-mode to plain, and that failed the test asserting NO_COLOR leaves rich mode alone while it passed on every developer machine. Adds clean_term_env() to conftest: it drops the seven variables _caps reads, pins a 256-colour baseline so colour depth is identical everywhere, then layers the test's own overrides on top. A test wanting CI or NO_COLOR or TERM=dumb now passes it explicitly and gets exactly that. Confirmed by reintroducing the leak, which reproduces the CI failure locally.
_enable_windows_vt() always asked about the process's own stderr handle even when detect() was handed a different stream. On Windows CI, where stderr is a pipe, GetConsoleMode failed on it and the resulting False zeroed the colour depth of every caller-supplied fake terminal, failing 13 capability tests that pass everywhere else. It also silently defeated FORCE_COLOR, whose whole purpose is emitting ANSI into a non-terminal. The probe now targets the descriptor behind the given stream, treats a stream with no descriptor as having no console to configure, and only runs at all when the stream claims to be a terminal. Two tests carried platform assumptions and are now skipped on Windows with the reason stated: the no-argument VT short-circuit, and a non-ASCII path build. The latter fails because the C core opens files with fopen() on a UTF-8 byte path, which the Windows CRT reads in the ANSI codepage; that is a pre-existing limitation of the C file layer.
The macOS matrix entries sat inside "Run Python tests" for half an hour on a suite that finishes in about a minute everywhere else, with no way to tell which test was stuck: GitHub does not serve logs for a job still in progress, and an unbounded job would have held the runner for six hours. Adds a 25 minute job timeout and pytest's stdlib-backed faulthandler_timeout, which dumps the stack of any test still running after three minutes. No new dependency.
Every pty test in test_term_rich.py wrote its frames and only then read them back, from the same thread. A pty's kernel buffer is finite, so a writer that fills it blocks until someone reads. Linux's buffer is roomy enough that a few frames fit and the tests passed; macOS's is a fraction of the size, and there the renderer blocked forever inside _safe_write during done(), taking the whole job to its timeout. Reading now starts on a background thread the moment the pty is created, so the writer never blocks whatever the platform's buffer size. Callers are unchanged: _drain still waits for the same quiet period and returns the same bytes in the same order. Adds a test that writes several hundred kilobytes through the renderer, past any platform's buffer, so the guarantee is pinned by volume instead of by the host happening to be roomy. It deadlocks against the old drain-afterwards helper on Linux too.
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
1 participant
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.
Everything going into 3.2.0. Two independent strands: the analytical
FlashBack work that was sitting on master, and the CLI redesign.
Input layer (
LZGraphs._io)The CLI and
from_fileused to read every file as one-sequence-per-line.That silently corrupted two common cases with exit code 0 and no warning:
>seq10header lines as sequences, so simulatingfrom the graph emitted
>seq10as a "sequence".spliced across fields that never existed in the input.
_iois now a package that detects format by content rather than byextension: FASTA, FASTQ, AIRR TSV/CSV, plain, and
sequence<TAB>count,under transparent gzip, bzip2 and xz. Eleven further input defects turned
up while closing those two and are fixed here as well, including a BOM
merging a FASTA header into the first sequence, duplicate column names
building the graph from the wrong column,
"3.0"abundances reading as 1,and a negative count in the C reader wrapping to ~1.8e19 and reaching the
graph as an edge weight.
LZGraph.from_fileandFlashBackGraph.from_fileare documented publicAPI and used to bypass all of this, so a user writing Python rather than
using the CLI still hit the corruption. All three entry points now route
through the same gate and are pinned to agree by a parity matrix.
Terminal layer (
LZGraphs._term)Zero new dependencies. A live panelled display on a TTY, and a scrolling
greppable
key=valuelog in CI and pipes. New--ui {auto,rich,plain,quiet}and
--no-color;NO_COLOR,TERM=dumbandCIare honoured. stdoutcarries data only, so
lzg simulate g.lzg -n 1000 | headis unaffected byrendering, and the build result is identical across all four modes.
FlashBack analytics
pseq_analysis()describes the sequence-probability spectrum withoutsimulating a walk, via the power-sum transform evaluated as a forward
dynamic program over the edges.
path_countis now exact in arbitraryprecision through a new C entry point instead of a double that saturated
at 2^53.
Release pipeline
release.ymlreplacesbuild-wheels.yml: cibuildwheel across linuxx86_64/aarch64, macOS arm64 and Windows, PyPI publishing via Trusted
Publishing, and a container pushed to GHCR so the package shows up under
Packages.
pyproject.tomlhadpackages = ["LZGraphs"], which omitted_ioand_termfrom wheels entirely; that is fixed and verified byinstalling a built wheel into a clean venv.
Testing
1633 passed, 5 skipped (all pre-existing). The two chartered corruption
defects are locked by tests that fail against the pre-change reader.
Before tagging
Maintainer setup is required for the first tagged release: a PyPI Trusted
Publisher for
LZGraphs(owner MuteJester, repo LZGraphs, workflowrelease.yml, environmentpypi), a repo environment namedpypi, andread/write workflow permissions. GHCR packages default to private on first
push.