Prune assembly graph before checking for cycles

src/main/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/readthreading/ReadThreadingAssembler.java

@@ -458,6 +458,11 @@ private AssemblyResult createGraph(final Iterable<GATKRead> reads,
         // actually build the read threading graph
         rtgraph.buildGraphIfNecessary();
 
+        // It's important to prune before recovering dangling ends so that we don't waste time recovering bad ends.
+        // It's also important to prune before checking for cycles so that sequencing errors don't create false cycles
+        // and unnecessarily abort assembly
+        chainPruner.pruneLowWeightChains(rtgraph);
+
         // sanity check: make sure there are no cycles in the graph
         if ( rtgraph.hasCycles() ) {
             if ( debug ) {
@@ -480,9 +485,6 @@ private AssemblyResult createGraph(final Iterable<GATKRead> reads,
     private AssemblyResult getAssemblyResult(final Haplotype refHaplotype, final int kmerSize, final ReadThreadingGraph rtgraph, final SmithWatermanAligner aligner) {
         printDebugGraphTransform(rtgraph, refHaplotype.getLocation() + "-sequenceGraph." + kmerSize + ".0.0.raw_readthreading_graph.dot");
 
-        // Prune before recovering dangling ends because the latter is expensive
-        chainPruner.pruneLowWeightChains(rtgraph);
-
         // look at all chains in the graph that terminate in a non-ref node (dangling sources and sinks) and see if
         // we can recover them by merging some N bases from the chain back into the reference
         if ( recoverDanglingBranches ) {

src/test/java/org/broadinstitute/hellbender/tools/walkers/haplotypecaller/HaplotypeCallerIntegrationTest.java

@@ -14,6 +14,7 @@
 import org.broadinstitute.barclay.argparser.CommandLineException;
 import org.broadinstitute.hellbender.CommandLineProgramTest;
 import org.broadinstitute.hellbender.cmdline.StandardArgumentDefinitions;
+import org.broadinstitute.hellbender.cmdline.argumentcollections.IntervalArgumentCollection;
 import org.broadinstitute.hellbender.engine.AssemblyRegionWalker;
 import org.broadinstitute.hellbender.engine.FeatureDataSource;
 import org.broadinstitute.hellbender.engine.ReadsDataSource;
@@ -1016,6 +1017,28 @@ public void test100PercentContaminationNoCallsInGVCFMode() throws Exception {
         }
     }
 
+    // check that a single, errorful, read that induces a cycle does not cause an assembly region to lose a real variant
+    @Test
+    public void testPrunedCycle() throws Exception {
+        final File output = createTempFile("output", ".vcf");
+
+        final String[] args = {
+                "-I", TEST_FILES_DIR + "pruned_cycle.bam",
+                "-R", b37Reference,
+                "-L", "1:169510380",
+                "--" + IntervalArgumentCollection.INTERVAL_PADDING_LONG_NAME, "100",
+                "-O", output.getAbsolutePath()
+        };
+        runCommandLine(args);
+
+        final Optional<VariantContext> het = VariantContextTestUtils.streamVcf(output)
+                .filter(vc -> vc.getStart() == 169510380)
+                .findFirst();
+
+        Assert.assertTrue(het.isPresent());
+        Assert.assertTrue(het.get().getGenotype(0).getAD()[1] > 100);
+    }
+
     @DataProvider
     public Object[][] getMaxAlternateAllelesData() {
         return new Object[][] {

src/test/java/org/broadinstitute/hellbender/tools/walkers/mutect/Mutect2IntegrationTest.java

@@ -634,7 +634,7 @@ public void testMitochondria() throws Exception {
                 "chrM:750-750 A*, [G]");
         Assert.assertTrue(expectedKeys.stream().allMatch(variantKeys::contains));
 
-        Assert.assertEquals(variants.get(0).getAttributeAsInt(GATKVCFConstants.ORIGINAL_CONTIG_MISMATCH_KEY, 0), 1513);
+        Assert.assertEquals(variants.get(0).getAttributeAsInt(GATKVCFConstants.ORIGINAL_CONTIG_MISMATCH_KEY, 0), 1517);
         Assert.assertEquals(variants.get(0).getGenotype("NA12878").getAnyAttribute(GATKVCFConstants.POTENTIAL_POLYMORPHIC_NUMT_KEY), "true");
     }
 
@@ -693,7 +693,7 @@ public void testMitochondrialRefConf() throws Exception {
         final List<String> variantKeys = new ArrayList<>(variantMap.keySet());
 
         final List<String> expectedKeys = Arrays.asList(
-                "chrM:152-152 T*, [<NON_REF>, C]",
+                "chrM:152-153 TA*, [<NON_REF>, CA, CC]",
                 "chrM:263-263 A*, [<NON_REF>, G]",
                 "chrM:297-297 A*, [<NON_REF>, AC, C]",  //alt alleles get sorted when converted to keys
                 //"chrM:301-301 A*, [<NON_REF>, AC, ACC]",