Skip to content

Commit 96b8de5

Browse files
druvusclaude
andcommitted
fix(validation): null-safety, status, cache fingerprint, blank coverage tab
Audit of the validation step (BLAST + minimap2) found bugs that silently broke the Validation tab: - parse_nanometanf_aggregate_json: a single JSON null on a numeric field made None<=1.0 / float(None) raise, which the catch-all except swallowed, dropping the WHOLE validation_results.json and blanking the Validation tab. Coerce every numeric with `or 0`/`or 0.0` and clamp percent_validated to <=100 (covers both the blast and the expanded minimap2 result). - determine_status: reads classified to an organism but 0 validated now reads as LOW_CONFIDENCE (examined-and-negative), not NO_DATA -- "checked, not confirmed" must not look identical to "not yet checked". - _validation_dir_fingerprint: validation_dir resolves to validation/blast, but the authoritative aggregate JSON is one level up at validation/validation_results.json; fold its mtime in so realtime rewrites invalidate the cache instead of serving stale results. - update_coverage_plots: the no-PAF branch returned the warning Alert into coverage-stats-container while hiding its parent coverage-plots-section, blanking the tab; keep the section visible so the warning shows. Tests: null-safety, hit-rate clamp, examined-but-0-hits, cache-fingerprint- tracks-aggregate, no-PAF-shows-warning-visible; four tests that encoded the old behaviour updated. Full suite 2622 passed; verified live in the browser with zero callback 500s. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 04ae1c0 commit 96b8de5

5 files changed

Lines changed: 149 additions & 29 deletions

File tree

nanometa_live/app/tabs/validation_tab.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -743,11 +743,15 @@ def update_coverage_plots(selected_key, min_mapq, depth_threshold,
743743
title="No coverage data",
744744
message=no_paf_msg,
745745
)
746+
# Keep the section VISIBLE: coverage-stats-container (where this
747+
# warning lands) and the placeholder figures live inside
748+
# coverage-plots-section, so hiding the section would blank the tab
749+
# and swallow the explanation.
746750
return no_paf(), no_paf(), no_paf(), dbc.Alert(
747751
no_paf_msg,
748752
color="warning",
749753
className="text-center",
750-
), hidden
754+
), visible
751755

752756
depth_fig = create_coverage_depth_figure(coverage, threshold=threshold)
753757
cum_fig = create_cumulative_coverage_figure(coverage)

nanometa_live/core/parsers/blast_validation_parser.py

Lines changed: 53 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,12 @@ def determine_status(self) -> ValidationStatus:
132132
return ValidationStatus.FAILED
133133
if self.validated_reads == 0 and self.total_reads == 0:
134134
return ValidationStatus.NO_DATA
135+
# Examined but nothing validated (reads were classified to this organism
136+
# but BLAST/minimap2 confirmed none): a negative result, NOT "no data".
137+
# Distinguishing the two matters clinically -- "checked, not confirmed"
138+
# must not look identical to "not yet checked".
139+
if self.validated_reads == 0 and self.total_reads > 0:
140+
return ValidationStatus.LOW_CONFIDENCE
135141
if self.percent_validated >= 80 and self.percent_identity_mean >= 90:
136142
return ValidationStatus.CONFIRMED
137143
if self.percent_validated >= 50:
@@ -214,6 +220,23 @@ def _validation_dir_fingerprint(self) -> Optional[float]:
214220
latest = m
215221
except OSError:
216222
continue
223+
# The authoritative aggregate JSON is written one level up at
224+
# validation/validation_results.json (the loader prefers it), but
225+
# validation_dir often resolves to validation/blast or
226+
# validation/minimap2. Without folding the aggregate's mtime in, a
227+
# realtime rewrite of validation_results.json never advances this
228+
# fingerprint and the cache serves stale results.
229+
for agg in (
230+
self.validation_dir.parent / "validation_results.json",
231+
self.results_dir / "validation" / "validation_results.json",
232+
):
233+
try:
234+
if agg.exists():
235+
m = agg.stat().st_mtime
236+
if m > latest:
237+
latest = m
238+
except OSError:
239+
continue
217240
return latest
218241
except OSError:
219242
return None
@@ -444,21 +467,29 @@ def parse_nanometanf_aggregate_json(
444467

445468
method = entry.get('validation_method', method_default)
446469

447-
# Map nanometanf fields to ValidationResult
448-
kraken_reads = entry.get('kraken_reads', 0)
449-
hit_rate = entry.get('hit_rate', 0.0)
450-
validated = entry.get('blast_hits', entry.get('mapped_reads', 0))
470+
# Map nanometanf fields to ValidationResult. A JSON ``null``
471+
# makes ``.get(key, default)`` return None (the default only
472+
# applies when the key is ABSENT), and ``None <= 1.0`` /
473+
# ``float(None)`` would raise -- caught by the catch-all
474+
# except below, which would silently drop the WHOLE aggregate
475+
# and blank the Validation tab. Coerce every numeric to a real
476+
# number with ``or 0`` before arithmetic.
477+
kraken_reads = entry.get('kraken_reads', 0) or 0
478+
hit_rate = entry.get('hit_rate', 0.0) or 0.0
479+
validated = entry.get('blast_hits', entry.get('mapped_reads', 0)) or 0
451480

452481
result = ValidationResult(
453482
sample_id=sample_id,
454483
taxid=tid,
455484
species=entry.get('species', ''),
456-
total_reads=kraken_reads,
457-
validated_reads=validated,
458-
percent_validated=hit_rate * 100 if hit_rate <= 1.0 else hit_rate,
459-
percent_identity_mean=float(entry.get('avg_identity', 0.0)),
460-
coverage_breadth=float(entry.get('avg_coverage', 0.0)),
461-
avg_mapq=float(entry.get('avg_mapq', 0.0)),
485+
total_reads=int(kraken_reads),
486+
validated_reads=int(validated),
487+
percent_validated=min(
488+
100.0, hit_rate * 100 if hit_rate <= 1.0 else hit_rate
489+
),
490+
percent_identity_mean=float(entry.get('avg_identity', 0.0) or 0.0),
491+
coverage_breadth=float(entry.get('avg_coverage', 0.0) or 0.0),
492+
avg_mapq=float(entry.get('avg_mapq', 0.0) or 0.0),
462493
# ref_name / ref_length are emitted by nanometanf but were
463494
# previously dropped here; surface the reference identity
464495
# and genome size in the GUI.
@@ -472,20 +503,23 @@ def parse_nanometanf_aggregate_json(
472503

473504
# If 'both' method, check for minimap2 fields on a BLAST entry
474505
if entry.get('minimap2_mapped') is not None:
506+
mm2_hit_rate = entry.get('minimap2_hit_rate', 0.0) or 0.0
475507
mm2_result = ValidationResult(
476508
sample_id=sample_id,
477509
taxid=tid,
478510
species=entry.get('species', ''),
479-
total_reads=kraken_reads,
480-
validated_reads=int(entry.get('minimap2_mapped', 0)),
481-
percent_validated=(
482-
entry.get('minimap2_hit_rate', 0.0) * 100
483-
if entry.get('minimap2_hit_rate', 0.0) <= 1.0
484-
else entry.get('minimap2_hit_rate', 0.0)
511+
total_reads=int(kraken_reads),
512+
validated_reads=int(entry.get('minimap2_mapped', 0) or 0),
513+
percent_validated=min(
514+
100.0,
515+
mm2_hit_rate * 100 if mm2_hit_rate <= 1.0 else mm2_hit_rate,
516+
),
517+
percent_identity_mean=float(entry.get('minimap2_identity', 0.0) or 0.0),
518+
coverage_breadth=float(
519+
entry.get('minimap2_coverage',
520+
entry.get('avg_coverage', 0.0)) or 0.0
485521
),
486-
percent_identity_mean=float(entry.get('minimap2_identity', 0.0)),
487-
coverage_breadth=float(entry.get('minimap2_coverage', entry.get('avg_coverage', 0.0))),
488-
avg_mapq=float(entry.get('avg_mapq', 0.0)),
522+
avg_mapq=float(entry.get('avg_mapq', 0.0) or 0.0),
489523
reference_accession=entry.get('ref_name', '') or '',
490524
reference_length=int(entry.get('ref_length', 0) or 0),
491525
validation_method='minimap2',

tests/test_blast_validation_parser.py

Lines changed: 85 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
"""
1717

1818
import json
19+
import time
1920
from pathlib import Path
2021

2122
import pytest
@@ -104,13 +105,45 @@ def test_missing_file_is_no_data(self, parser, tmp_path):
104105
assert result.status == ValidationStatus.NO_DATA
105106

106107

108+
class TestCacheFingerprintTracksAggregate:
109+
"""The cache fingerprint must advance when the authoritative aggregate JSON
110+
(validation/validation_results.json) is rewritten -- it sits one level above
111+
validation_dir (validation/blast), so a naive iterdir misses it and the
112+
realtime Validation tab would serve stale results."""
113+
114+
def test_aggregate_rewrite_advances_fingerprint(self, tmp_path):
115+
import os
116+
from nanometa_live.core.parsers.blast_validation_parser import ValidationParser
117+
vdir = tmp_path / "validation" / "blast"
118+
vdir.mkdir(parents=True)
119+
(vdir / "barcode01_taxid562.blast.tsv").write_text("x\n")
120+
agg = tmp_path / "validation" / "validation_results.json"
121+
agg.write_text("{}")
122+
old = time.time() - 100
123+
os.utime(vdir, (old, old))
124+
os.utime(vdir / "barcode01_taxid562.blast.tsv", (old, old))
125+
os.utime(agg, (old, old))
126+
127+
parser = ValidationParser(str(tmp_path))
128+
assert parser.validation_dir.name == "blast"
129+
fp1 = parser._validation_dir_fingerprint()
130+
131+
# Rewrite only the aggregate, one level above validation_dir.
132+
newer = time.time()
133+
agg.write_text('{"results": {}}')
134+
os.utime(agg, (newer, newer))
135+
fp2 = parser._validation_dir_fingerprint()
136+
assert fp2 > fp1
137+
138+
107139
class TestDetermineStatusBoundaries:
108140
"""Exact-threshold behaviour of ValidationResult.determine_status().
109141
110142
Authoritative rule (blast_validation_parser.py): CONFIRMED iff
111143
percent_validated >= 80 AND percent_identity_mean >= 90; PARTIAL iff
112-
percent_validated >= 50; LOW_CONFIDENCE iff > 0; else NO_DATA; FAILED on
113-
errors. The boundaries are inclusive on the >= side.
144+
percent_validated >= 50; LOW_CONFIDENCE iff > 0 OR (validated_reads == 0 but
145+
total_reads > 0, i.e. examined-and-negative); NO_DATA only when nothing was
146+
examined; FAILED on errors. Boundaries are inclusive on the >= side.
114147
"""
115148

116149
@pytest.mark.parametrize("pv,ident,expected", [
@@ -120,7 +153,8 @@ class TestDetermineStatusBoundaries:
120153
(79.9, 99.0, ValidationStatus.PARTIAL), # validated just below 80
121154
(50.0, 99.0, ValidationStatus.PARTIAL), # 50 boundary inclusive
122155
(49.9, 99.0, ValidationStatus.LOW_CONFIDENCE),
123-
(0.0, 99.0, ValidationStatus.NO_DATA), # no reads validated
156+
# Examined (reads exist) but nothing validated -> low, NOT no_data.
157+
(0.0, 99.0, ValidationStatus.LOW_CONFIDENCE),
124158
])
125159
def test_threshold_boundaries(self, pv, ident, expected):
126160
# validated/total kept > 0 so the (0,0) early NO_DATA branch is not hit
@@ -133,6 +167,15 @@ def test_threshold_boundaries(self, pv, ident, expected):
133167
)
134168
assert r.determine_status() == expected
135169

170+
def test_examined_zero_hits_is_low_not_no_data(self):
171+
# A watchlist organism Kraken classified but BLAST/minimap2 confirmed
172+
# none must read as "checked, not confirmed", distinct from "not run".
173+
r = ValidationResult(
174+
sample_id="s", taxid=1, total_reads=120, validated_reads=0,
175+
percent_validated=0.0, validation_method="blast",
176+
)
177+
assert r.determine_status() == ValidationStatus.LOW_CONFIDENCE
178+
136179
def test_errors_force_failed_even_with_high_metrics(self):
137180
r = ValidationResult(
138181
sample_id="s", taxid=1, total_reads=100, validated_reads=100,
@@ -176,6 +219,41 @@ def test_single_blast_entry(self, parser, tmp_path):
176219
assert r.percent_validated == pytest.approx(90.0)
177220
assert r.status == ValidationStatus.CONFIRMED
178221

222+
def test_null_numeric_fields_do_not_drop_the_file(self, parser, tmp_path):
223+
# A JSON null on a numeric field must not make None<=1.0 / float(None)
224+
# raise and silently empty the whole aggregate (and the Validation tab).
225+
f = tmp_path / "validation_results.json"
226+
f.write_text(json.dumps(_aggregate({
227+
"barcode01": {
228+
"562": {
229+
"species": "Escherichia coli",
230+
"kraken_reads": 100,
231+
"blast_hits": None,
232+
"hit_rate": None,
233+
"avg_identity": None,
234+
"avg_coverage": None,
235+
"avg_mapq": None,
236+
}
237+
}
238+
})))
239+
results = parser.parse_nanometanf_aggregate_json(f)
240+
assert len(results) == 1
241+
r = results[0]
242+
assert r.validated_reads == 0
243+
assert r.percent_validated == 0.0
244+
assert r.percent_identity_mean == 0.0
245+
246+
def test_hit_rate_above_100_is_clamped(self, parser, tmp_path):
247+
# A pre-multiplied hit_rate that overshoots 100 (the >1.0 passthrough
248+
# branch) must be clamped so the UI never shows e.g. 130%.
249+
f = tmp_path / "validation_results.json"
250+
f.write_text(json.dumps(_aggregate({
251+
"barcode01": {"562": {"species": "E. coli", "kraken_reads": 10,
252+
"blast_hits": 13, "hit_rate": 130.0,
253+
"avg_identity": 98.0}}})))
254+
r = parser.parse_nanometanf_aggregate_json(f)[0]
255+
assert r.percent_validated == 100.0
256+
179257
def test_both_method_expands_to_two_results(self, parser, tmp_path):
180258
f = tmp_path / "validation_results.json"
181259
f.write_text(json.dumps(_aggregate({
@@ -259,8 +337,9 @@ def test_both_method_without_minimap2_fields_does_not_expand(self, parser, tmp_p
259337

260338
def test_minimap2_mapped_zero_still_expands(self, parser, tmp_path):
261339
# ``minimap2_mapped: 0`` is a real (mapped-nothing) outcome, distinct from
262-
# an absent key — it must expand to a NO_DATA minimap2 result, not be
263-
# silently dropped.
340+
# an absent key — it must expand to a result, not be silently dropped.
341+
# Reads existed (kraken_reads=100) but none mapped, so the status is
342+
# LOW_CONFIDENCE (examined-and-negative), not NO_DATA.
264343
f = tmp_path / "validation_results.json"
265344
f.write_text(json.dumps(_aggregate({
266345
"barcode01": {
@@ -275,7 +354,7 @@ def test_minimap2_mapped_zero_still_expands(self, parser, tmp_path):
275354
assert len(results) == 2
276355
mm2 = next(r for r in results if r.validation_method == "minimap2")
277356
assert mm2.validated_reads == 0
278-
assert mm2.status == ValidationStatus.NO_DATA
357+
assert mm2.status == ValidationStatus.LOW_CONFIDENCE
279358

280359
def test_mixed_methods_across_taxids_split_correctly(self, parser, tmp_path):
281360
# One taxid is "both" (-> 2 results), another is blast-only (-> 1). Total 3.

tests/test_validation_tab_callbacks.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,10 +193,13 @@ def test_batch_paf_renders(self, validation_app, enabled_config):
193193
"barcode01_1773", 0, 10, "batch", "1", enabled_config)
194194
assert style == {"display": "block"}
195195

196-
def test_missing_paf_returns_warning_hidden(self, validation_app, enabled_config):
196+
def test_missing_paf_shows_warning_visible(self, validation_app, enabled_config):
197+
# No PAF: the section must stay VISIBLE so the warning Alert (which lives
198+
# inside coverage-plots-section) is actually shown, not swallowed.
197199
depth, cum, hist, stats, style = self._fn(validation_app)(
198200
"barcode99_9999", 0, 10, "cumulative", None, enabled_config)
199-
assert style == {"display": "none"}
201+
assert style == {"display": "block"}
202+
assert "No PAF file" in str(stats)
200203

201204
def test_negative_depth_threshold_sanitized(self, validation_app, enabled_config):
202205
# A negative or None threshold must not raise; it clamps internally.

tests/validation/generate_synthetic_data.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ def _minimap2_stats_dict(sample, taxid, species, total_reads, mapped_reads,
359359
("barcode01", 1773, "minimap2", "confirmed"), # expanded minimap2 side
360360
("barcode01", 1280, "blast", "partial"), # S. aureus
361361
("barcode02", 1639, "minimap2", "confirmed"), # L. monocytogenes
362-
("barcode03", 562, "blast", "no_data"), # E. coli (low, no hits)
362+
("barcode03", 562, "blast", "low"), # E. coli: reads classified, no BLAST hits -> examined-and-negative
363363
("barcode05", 263, "minimap2", "confirmed"), # F. tularensis TUL4 amplicon
364364
]
365365

0 commit comments

Comments
 (0)