Skip to content

Commit 87b1879

Browse files
dpark01claude
andcommitted
Address Copilot review comments
- Make plot labels generic ("Old"/"New") instead of hardcoded branch names - Fix SKILL.md example to match discover_pairs.py argparse interface - Fix stale reference to non-existent run_regression.py - Fix discover_pairs.py docstring (assembly_metadata, not assembly_stats) - Fix compare_sample_pair.py docstring (assembly_metadata TSV) - Sort attempt-N dirs numerically instead of lexicographically - Add pipefail to run_vadr.sh, use find instead of glob for model dir - Add docker.yml to container-vulns rule paths - Guard generate_markdown_report against empty DataFrames Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent ccaff74 commit 87b1879

6 files changed

Lines changed: 39 additions & 24 deletions

File tree

.agents/skills/regression-testing/SKILL.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,11 @@ Use `discover_pairs.py` to find all comparable old/new sample pairs by crawling
3131
GCS Cromwell output directories.
3232

3333
```bash
34-
python discover_pairs.py <workspace_name> \
34+
python discover_pairs.py \
3535
--bucket <workspace-bucket-id> \
3636
--old-sub <old-submission-id> \
3737
--new-sub <new-submission-id> \
38-
-o pairs.json
38+
--output pairs.json
3939
```
4040

4141
This produces a JSON mapping sample_name -> {old_tsv, new_tsv} for all samples
@@ -56,8 +56,9 @@ python compare_sample_pair.py \
5656
--output-json ./results/<sample>.json
5757
```
5858

59-
For batch processing, use `run_regression.py` (in the terra-regression/scripts/ directory)
60-
which orchestrates parallel execution across all pairs.
59+
For batch processing, iterate over all entries in `pairs.json` and invoke
60+
`compare_sample_pair.py` for each sample pair (e.g., via a small wrapper
61+
script using `concurrent.futures` or `xargs`/GNU `parallel`).
6162

6263
### Step 4: Generate Report
6364

.agents/skills/regression-testing/compare_sample_pair.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#!/usr/bin/env python3
22
"""Compare assembly outputs between old and new code for a single sample pair.
33
4-
Takes two GCS URIs pointing at assembly_stats_by_taxon_tsv files (old and new),
4+
Takes two GCS URIs pointing at assembly_metadata TSV files (old and new),
55
downloads them, compares metrics, aligns FASTAs with mafft, and outputs a JSON result.
66
"""
77
import argparse

.agents/skills/regression-testing/discover_pairs.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
#!/usr/bin/env python3
22
"""Discover comparable old/new sample pairs by crawling GCS Cromwell output directories.
33
4-
For each submission, finds assembly_stats_by_taxon_tsv files, extracts sample names
5-
from filenames, and outputs the intersection as a JSON mapping.
4+
For each submission, finds assembly_metadata TSV files named
5+
``assembly_metadata-<sample>.tsv``, extracts sample names from filenames,
6+
and outputs the intersection as a JSON mapping.
67
78
Usage:
89
python discover_pairs.py \
@@ -46,7 +47,11 @@ def find_tsv_in_call_dir(call_dir_uri):
4647
items = gcloud_ls(call_dir_uri)
4748

4849
# Check for attempt-N subdirectories
49-
attempt_dirs = sorted([i for i in items if '/attempt-' in i], reverse=True)
50+
def attempt_sort_key(path):
51+
match = re.search(r'/attempt-(\d+)', path)
52+
return int(match.group(1)) if match else 0
53+
attempt_dirs = sorted([i for i in items if '/attempt-' in i],
54+
key=attempt_sort_key, reverse=True)
5055
tsv_files = [i for i in items if i.endswith('.tsv')]
5156

5257
# If there are attempt dirs, check the highest attempt first

.agents/skills/regression-testing/generate_report.py

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,8 @@ def generate_plots(df, plot_dir):
124124
alpha=0.5, s=20)
125125
lims = [0, 105]
126126
ax.plot(lims, lims, 'r--', alpha=0.5, label='y=x')
127-
ax.set_xlabel('Old (main) % Reference Covered')
128-
ax.set_ylabel('New (FreeBayes) % Reference Covered')
127+
ax.set_xlabel('Old % Reference Covered')
128+
ax.set_ylabel('New % Reference Covered')
129129
ax.set_title('Percent Reference Covered: Old vs New')
130130
ax.set_xlim(lims)
131131
ax.set_ylim(lims)
@@ -148,8 +148,8 @@ def generate_plots(df, plot_dir):
148148
ax.plot([min_v, max_v], [min_v, max_v], 'r--', alpha=0.5, label='y=x')
149149
ax.set_xscale('log')
150150
ax.set_yscale('log')
151-
ax.set_xlabel('Old (main) Mean Coverage')
152-
ax.set_ylabel('New (FreeBayes) Mean Coverage')
151+
ax.set_xlabel('Old Mean Coverage')
152+
ax.set_ylabel('New Mean Coverage')
153153
ax.set_title('Mean Coverage: Old vs New')
154154
ax.legend()
155155
fig.tight_layout()
@@ -169,8 +169,8 @@ def generate_plots(df, plot_dir):
169169
max_v = max(df_len['old_assembly_length_unambiguous'].max(),
170170
df_len['new_assembly_length_unambiguous'].max()) * 1.05
171171
ax.plot([min_v, max_v], [min_v, max_v], 'r--', alpha=0.5, label='y=x')
172-
ax.set_xlabel('Old (main) Unambiguous Length')
173-
ax.set_ylabel('New (FreeBayes) Unambiguous Length')
172+
ax.set_xlabel('Old Unambiguous Length')
173+
ax.set_ylabel('New Unambiguous Length')
174174
ax.set_title('Assembly Length (Unambiguous): Old vs New')
175175
ax.legend()
176176
fig.tight_layout()
@@ -270,17 +270,24 @@ def generate_markdown_report(df, sample_df, workspace_name, report_dir, plot_dir
270270
pd, _ = get_deps()
271271

272272
total_samples = len(sample_df)
273-
samples_with_assemblies = len(sample_df[sample_df['old_assembly_count'] > 0])
274-
samples_count_match = len(sample_df[sample_df['assembly_count_match']])
273+
if sample_df.empty or 'old_assembly_count' not in sample_df.columns:
274+
samples_with_assemblies = 0
275+
samples_count_match = 0
276+
else:
277+
samples_with_assemblies = len(sample_df[sample_df['old_assembly_count'] > 0])
278+
samples_count_match = len(sample_df[sample_df['assembly_count_match']])
275279
samples_count_mismatch = total_samples - samples_count_match
276280

277281
total_assemblies = len(df)
278-
df_aln = df[df['alignment_identity'].notna()]
279-
280-
identical = len(df_aln[df_aln['alignment_identity'] >= 1.0])
281-
near_identical = len(df_aln[(df_aln['alignment_identity'] >= 0.999) & (df_aln['alignment_identity'] < 1.0)])
282-
minor_diff = len(df_aln[(df_aln['alignment_identity'] >= 0.99) & (df_aln['alignment_identity'] < 0.999)])
283-
significant_diff = len(df_aln[df_aln['alignment_identity'] < 0.99])
282+
if df.empty or 'alignment_identity' not in df.columns:
283+
df_aln = pd.DataFrame()
284+
else:
285+
df_aln = df[df['alignment_identity'].notna()]
286+
287+
identical = len(df_aln[df_aln['alignment_identity'] >= 1.0]) if len(df_aln) > 0 else 0
288+
near_identical = len(df_aln[(df_aln['alignment_identity'] >= 0.999) & (df_aln['alignment_identity'] < 1.0)]) if len(df_aln) > 0 else 0
289+
minor_diff = len(df_aln[(df_aln['alignment_identity'] >= 0.99) & (df_aln['alignment_identity'] < 0.999)]) if len(df_aln) > 0 else 0
290+
significant_diff = len(df_aln[df_aln['alignment_identity'] < 0.99]) if len(df_aln) > 0 else 0
284291

285292
with_snps = len(df_aln[df_aln['snp_count'] > 0])
286293
with_indels = len(df_aln[df_aln['indel_count_events'] > 0])

.agents/skills/regression-testing/run_vadr.sh

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,16 @@
1212
# ALERTS_TSV - file to write alerts TSV
1313
# VADR_TGZ - file to write full vadr output tarball
1414

15-
set -e
15+
set -euo pipefail
1616

1717
BASENAME=$(basename "${FASTA}" .fasta)
1818

1919
# Download and unpack VADR models
2020
if [ -n "${MODEL_URL}" ]; then
2121
mkdir -p vadr-untar
2222
curl -fsSL "${MODEL_URL}" | tar -C vadr-untar -xzf -
23-
ln -s vadr-untar/*/ vadr-models
23+
MODEL_DIR=$(find vadr-untar -mindepth 1 -maxdepth 1 -type d | head -1)
24+
ln -s "${MODEL_DIR}" vadr-models
2425
else
2526
ln -s /opt/vadr/vadr-models vadr-models
2627
fi

.claude/rules/container-vulns.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ paths:
55
- ".trivy-ignore-policy.rego"
66
- "vulnerability-mitigation-status.md"
77
- ".github/workflows/container-scan.yml"
8+
- ".github/workflows/docker.yml"
89
---
910

1011
For container vulnerability management guidance, see

0 commit comments

Comments
 (0)