galaxyproject / galaxyproject/brc-analytics

Umbrella: reconcile Cyclospora part 2 PyEuk walkthrough with the pinned commit (2aa6e74)

Open
#1,682 0 comments 0 reactions 1 assignee Claimed by @d-callan View on GitHub
content
Dominant language
TypeScript
Stars
7
Forks
11
Avg merge
2d 12h
Merged PRs (30d)
16

Description

## Summary

Umbrella / follow-up from the review of #1667. The Cyclospora part 2 post (`genotyping-cyclospora-from-haplotype-calls-to-outbreak-clusters.mdx`) pins PyEuk at commit `2aa6e74` and presents a step-by-step "reproducible walkthrough," but several descriptions don't match that commit's code — and one contradicts the post itself.

These were raised as Copilot's **suppressed** comments on #1667 (never surfaced as tracked threads, so never addressed), and I re-verified each directly against `spond/pyeuk@2aa6e74`. All eight check out. File/line references below are to the pinned PyEuk source unless noted as "post line".

**@d-callan** — assigning to you to coordinate the fix(s). This is an umbrella; please split into sub-issues as makes sense (or fix inline on the PR branch). For each item the fix is one of: **(a)** correct the prose to match `2aa6e74`, **(b)** re-pin to a later commit that actually implements the described behavior, or **(c)** soften the claim. Author's call per item.

## Highest priority — internal contradiction

- [ ] **1. MOI "paradox" contradicts the post's own worked example** (post line 524). The "Key Characteristics" bullet says the legacy `w = 1 + x` penalty "artificially inflated distances and falsely excluded co-infected patients." The worked example at post lines 247–251 already disproves exactly this: _"It does not. That reading takes `x` for the union … the code computes the sum, and the share-something case applies regardless."_ Rewrite the bullet so it stops re-asserting the claim the post debunks. (Self-contained; needs no code check.)

## Prose ↔ pinned-code mismatches (affect the "reproducible walkthrough" framing)

- [ ] **2. Tree-cut rule** (post line 620). Post: `rel_gap = (h_k − h_{k-1}) / h_k`. Code (`clustering.py:229-253`): ranks by **absolute** consecutive merge-height gap, then accepts the top candidate with `gap / root_tree_height ≥ 0.22` and min cluster size `≥ max(5, 10% N)`. The stated formula can select a different `k`, so the walkthrough isn't reproducible as written.
- [ ] **3. Supervised search range** (post line 632). Post: `k ∈ {1, …, 50}`. Code (`clustering.py:186-188`): `search_start = max(2, k_min)` — `k=1` is never considered.
- [ ] **4. Output table overstates the tool's output** (post lines 656–662). Post shows a "Surveillance Interpretation" column ("Verified Outbreak Case (Vendor A)", "Sporadic / Unassigned"). `find_clusters()` (`clustering.py:204-291`) writes only `Seq_ID` and a numeric `Assigned_cluster`; `-1` means low-completeness/excluded, not "sporadic." Relabel as an annotated/illustrative view, or trim to the real columns.
- [ ] **5. Completeness filter — invented condition** (post line 486). Post: dropped for <10% columns "or failing base diagnostic locus coverage." Code (`distance_engine.py:62-80`): filters on the 10% call fraction only. Drop the second clause.
- [ ] **6. Deterministic tie-breaking claim** (post line 567). Post: "enforces strict lexicographical index ordering … byte-for-byte identical dendrograms." Code (`clustering.py:171`): plain SciPy Ward, no custom tie-breaker — determinism is just SciPy on identical input. Claim only the measured repeated-run determinism.

## Wording / caveats

- [ ] **7. Pairwise-complete zero distance needs a caveat** (post line 525). "Pairs with no shared loci default to zero distance" is true (`distance_engine.py:45-48`) but treats *unknown* as *identical* (maximally similar) before the PSD projection, and the 10% per-specimen filter doesn't guarantee pairwise overlap. Flag as a caveat, not a neutral property.
- [ ] **8. Weight/frequency wording** (post line 508). "rare markers, rarely-amplifying loci, and artifacts receive identical weights" conflicts with the code's `p_j`, computed only over **called** specimens (`distance_engine.py:118-123`). Tighten the wording.

## Notes

- Verified against `spond/pyeuk@2aa6e74` (the commit the post pins).
- Related: #1667 (the PR), #1681 (the separate citation fix — the remaining open #1667 threads on ref-20 formatting and ensemble-paragraph placement are cosmetic nits).

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.