Skip to content

Commit 1100e0d

Browse files
druvusclaude
andcommitted
fix(validation): wire consensus param + harden conda guard + audit fixes
Fixes surfaced while live-testing the both-method validation + consensus path on real multi-barcode data and a follow-up code audit. - parameter_mapping: pass generate_consensus / consensus_min_depth to the pipeline (gated on run_validation_enabled). The pipeline accepted these params but the GUI never sent them, so consensus was unreachable from the GUI. - nextflow_manager: harden the conda-env launch guard. _purge_broken_conda_envs now also purges envs that have a history marker but an empty bin/ (a build that wrote history then failed to install binaries -> "command not found", exit 127). Add _strip_appledouble_files to remove macOS AppleDouble (._*) sidecars from the conda cache before launch; they corrupt conda-meta and make conda abort env creation ("corrupted file: ._<pkg>.json"). - blast_validation_parser: guard parse_blast_per_read against an IndexError when a malformed TSV has an all-null subject column; drop unknown keys in ValidationResult.from_dict (mirrors the sibling from_dict guards). - validation_layout: remove an orphan consensus-summary-container Div (no writer). - consensus_helpers: default the consensus selector to an organism that actually produced a sequence, so the panel does not open on a "no consensus" warning. - Tests: empty-bin / AppleDouble-strip guard cases, all-null-subject per-read case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 0b70199 commit 1100e0d

7 files changed

Lines changed: 132 additions & 10 deletions

File tree

nanometa_live/app/layouts/validation_layout.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -577,8 +577,6 @@ def _create_consensus_tab() -> dbc.Tab:
577577
label="Consensus",
578578
tab_id="consensus-tab",
579579
children=html.Div([
580-
html.Div(id="consensus-summary-container", className="mb-3"),
581-
582580
# Empty state (no consensus artifacts on disk)
583581
html.Div(
584582
id="consensus-empty-message",

nanometa_live/app/tabs/consensus_helpers.py

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,24 @@ def build_consensus_selector_options(
6464
valid_values = {o["value"] for o in options if not o.get("disabled")}
6565
if current_value and current_value in valid_values:
6666
return options, current_value
67-
first_value = next(
68-
(o["value"] for o in options if not o.get("disabled")),
67+
# Prefer a species that actually produced a consensus for the default
68+
# selection, so the panel does not open on a "no consensus" warning.
69+
with_seq = {
70+
f"{r.get('sample_id', '')}_{r.get('taxid', '')}"
71+
for r in results
72+
if r.get("has_sequence", r.get("consensus_length", 0) > 0)
73+
}
74+
default_value = next(
75+
(o["value"] for o in options
76+
if not o.get("disabled") and o["value"] in with_seq),
6977
None,
7078
)
71-
return options, first_value
79+
if default_value is None:
80+
default_value = next(
81+
(o["value"] for o in options if not o.get("disabled")),
82+
None,
83+
)
84+
return options, default_value
7285

7386

7487
def find_consensus_result(

nanometa_live/core/config/parameter_mapping.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -610,6 +610,11 @@ def _build_base_params(config: Dict[str, Any], main_dir: str, kraken_db: str,
610610
"validation_identity_threshold": config.get("validation_identity_threshold", 90.0),
611611
"minimap2_preset": config.get("minimap2_preset", "map-ont"),
612612
"minimap2_min_mapq": config.get("minimap2_min_mapq", 10),
613+
# Consensus sequence generation (amplicon-focused). Off unless the
614+
# operator enables it; only meaningful when validation runs (it reuses
615+
# the extracted reads + reference genome).
616+
"generate_consensus": config.get("generate_consensus", False) and run_validation_enabled,
617+
"consensus_min_depth": config.get("consensus_min_depth", 10),
613618
# NOTE: ``min_reads_for_validation`` (default 50) is a CUMULATIVE reporting
614619
# threshold applied in the GUI/aggregation layer -- an organism needs this
615620
# many reads across the whole run to be treated as validated. It is

nanometa_live/core/parsers/blast_validation_parser.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,10 @@ def from_dict(cls, data: Dict[str, Any]) -> 'ValidationResult':
121121
raw = data['status']
122122
data = dict(data)
123123
data['status'] = ValidationStatus(_status_map.get(raw, raw))
124-
return cls(**data)
124+
# Drop unknown keys so a dict from a newer schema does not raise
125+
# TypeError (mirrors NCBIResult/GTDBResult.from_dict in this codebase).
126+
return cls(**{k: v for k, v in data.items()
127+
if k in cls.__dataclass_fields__})
125128

126129
def determine_status(self) -> ValidationStatus:
127130
"""Determine validation status based on metrics."""
@@ -847,8 +850,11 @@ def parse_blast_per_read(
847850
"mean_pident": round(float(r.mean_pident), 1)}
848851
for r in top.itertuples(index=False)
849852
]
853+
# Guard top.iloc[0]: a malformed TSV whose sseqid column is all-null
854+
# yields an empty groupby, so check not-empty as well as total_reads.
850855
subject_agreement = (
851-
float(top.iloc[0]['reads']) / total_reads if total_reads else 0.0
856+
float(top.iloc[0]['reads']) / total_reads
857+
if total_reads and not top.empty else 0.0
852858
)
853859

854860
# Per-read table: cap to top-N by bitscore for the DOM.

nanometa_live/core/workflow/nextflow_manager.py

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,15 @@ def _purge_broken_conda_envs(work_dir: str) -> list:
213213
if not os.path.isdir(env_path):
214214
continue
215215
history_marker = os.path.join(env_path, "conda-meta", "history")
216-
if os.path.isfile(history_marker):
216+
bin_dir = os.path.join(env_path, "bin")
217+
has_history = os.path.isfile(history_marker)
218+
# A history marker normally means "fully built". But a build that
219+
# failed midway (e.g. conda aborting on an AppleDouble-corrupted
220+
# conda-meta file) can leave the history file with an EMPTY bin/ --
221+
# the env then activates but every tool is "command not found"
222+
# (exit 127). Treat an env with no executables as broken too.
223+
has_binaries = os.path.isdir(bin_dir) and bool(os.listdir(bin_dir))
224+
if has_history and has_binaries:
217225
# Fully-built env; leave it alone.
218226
continue
219227
try:
@@ -226,6 +234,30 @@ def _purge_broken_conda_envs(work_dir: str) -> list:
226234
)
227235
return removed
228236

237+
@staticmethod
238+
def _strip_appledouble_files(work_dir: str) -> int:
239+
"""Delete macOS AppleDouble ``._*`` sidecars under ``<work_dir>/conda``.
240+
241+
macOS writes ``._<name>`` files carrying resource-fork/xattr data; when
242+
they land in a conda env's ``conda-meta/`` (observed from Spotlight or
243+
cloud-sync activity even on APFS), ``conda env create`` treats the env
244+
as corrupt (``corrupted file: ._<pkg>.json``) and fails. They are always
245+
safe to remove. Returns the count deleted.
246+
"""
247+
conda_cache = os.path.join(work_dir, "conda")
248+
if not os.path.isdir(conda_cache):
249+
return 0
250+
removed = 0
251+
for root, _dirs, files in os.walk(conda_cache):
252+
for fname in files:
253+
if fname.startswith("._"):
254+
try:
255+
os.remove(os.path.join(root, fname))
256+
removed += 1
257+
except OSError:
258+
pass
259+
return removed
260+
229261
def _parse_pipeline_source(self) -> Tuple[str, Optional[str]]:
230262
"""
231263
Parse the pipeline source configuration.
@@ -526,6 +558,19 @@ def start(
526558
len(purged),
527559
", ".join(os.path.basename(p) for p in purged),
528560
)
561+
# Strip macOS AppleDouble (._*) sidecars from the conda
562+
# cache. On some volumes (Spotlight/cloud-sync churn) these
563+
# appear inside env conda-meta/ dirs; conda then reports the
564+
# env as "corrupted file: ._<pkg>.json" and aborts trying to
565+
# recreate an env that already has a history marker -- so the
566+
# half-built-env purge above does not catch it. Removing them
567+
# lets the pre-built env activate cleanly.
568+
stripped = self._strip_appledouble_files(self.work_dir)
569+
if stripped:
570+
logging.warning(
571+
"Stripped %d macOS AppleDouble file(s) from the "
572+
"conda cache before launch.", stripped,
573+
)
529574

530575
# Parse pipeline source configuration
531576
pipeline_path, revision = self._parse_pipeline_source()

tests/test_blast_per_read_parser.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,23 @@ def test_12_column_file(tmp_path):
7878
assert all(r["qcovs"] == 0.0 for r in res["records"])
7979

8080

81+
def test_all_null_subject_does_not_crash(tmp_path):
82+
# A malformed TSV whose subject column is empty must not raise on the
83+
# top-subject access; it should still return per-read records.
84+
p = tmp_path / "s_taxid1.blast.tsv"
85+
rows = []
86+
for i in range(3):
87+
# sseqid (col 2) left empty
88+
rows.append("\t".join(str(v) for v in [
89+
f"r{i}", "", 96.0, 400, 2, 0, 1, 400, 1, 400, "1e-50", 700,
90+
450, 1900000, 90]))
91+
_write(p, rows)
92+
res = parse_blast_per_read(p, "s", 1)
93+
assert res["total_reads"] == 3
94+
# subject_agreement is well-defined (0.0..1.0), no IndexError
95+
assert 0.0 <= res["subject_agreement"] <= 1.0
96+
97+
8198
def test_empty_or_missing_file(tmp_path):
8299
missing = tmp_path / "nope.blast.tsv"
83100
res = parse_blast_per_read(missing, "s", 1)

tests/test_purge_broken_conda_envs.py

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,17 @@ def _make_env(parent: Path, name: str, *, complete: bool) -> Path:
2323
"""Build a fake conda env directory under parent.
2424
2525
A complete env carries a `conda-meta/history` file (which conda
26-
writes last). An incomplete env is missing that marker, exactly
27-
like a build that was killed by SIGTERM partway through.
26+
writes last) AND at least one executable under `bin/`. An incomplete
27+
env is missing the marker (build killed by SIGTERM partway through)
28+
or has an empty `bin/` (build that wrote history but installed no
29+
binaries, e.g. conda aborting on an AppleDouble-corrupted file).
2830
"""
2931
env = parent / name
3032
(env / "conda-meta").mkdir(parents=True)
3133
(env / "bin").mkdir()
3234
if complete:
3335
(env / "conda-meta" / "history").write_text("# fake history\n")
36+
(env / "bin" / "tool").write_text("#!/bin/sh\n")
3437
return env
3538

3639

@@ -106,3 +109,38 @@ def test_empty_env_dir_is_treated_as_broken(self, tmp_path):
106109

107110
assert removed == [str(env)]
108111
assert not env.exists()
112+
113+
def test_history_but_empty_bin_is_broken(self, tmp_path):
114+
# A build that wrote the history marker but installed no binaries
115+
# (e.g. conda aborting on an AppleDouble-corrupted conda-meta file):
116+
# the env activates but every tool is "command not found" (exit 127).
117+
cache = tmp_path / "conda"
118+
cache.mkdir()
119+
env = cache / "env-deadbeef"
120+
(env / "conda-meta").mkdir(parents=True)
121+
(env / "bin").mkdir()
122+
(env / "conda-meta" / "history").write_text("# fake history\n")
123+
124+
removed = NextflowManager._purge_broken_conda_envs(str(tmp_path))
125+
126+
assert removed == [str(env)]
127+
assert not env.exists()
128+
129+
130+
class TestStripAppleDoubleFiles:
131+
def test_no_op_when_no_conda_cache(self, tmp_path):
132+
assert NextflowManager._strip_appledouble_files(str(tmp_path)) == 0
133+
134+
def test_strips_appledouble_from_conda_meta(self, tmp_path):
135+
cache = tmp_path / "conda"
136+
env = cache / "env-abc" / "conda-meta"
137+
env.mkdir(parents=True)
138+
(env / "gzip-1.13.json").write_text("{}")
139+
(env / "._gzip-1.13.json").write_text("appledouble")
140+
(cache / "env-abc" / "._conda-meta").write_text("appledouble")
141+
142+
count = NextflowManager._strip_appledouble_files(str(tmp_path))
143+
144+
assert count == 2
145+
assert (env / "gzip-1.13.json").exists() # real file untouched
146+
assert not (env / "._gzip-1.13.json").exists()

0 commit comments

Comments
 (0)