From 94ddb1f2464a7005778a65697b7d6fec2990e6e1 Mon Sep 17 00:00:00 2001 From: tfenne Date: Thu, 29 Oct 2026 17:30:31 -0600 Subject: [PATCH 1/2] Keep a BAM record's other tags when one of its tags is repeated readTags appends tags in sorted order through a tail pointer and sends any other tag through SAMBinaryTagAndValue.insert. A repeat of the tail's tag replaced that node in the list, but tail still pointed at the node it replaced, so every later tag that sorted above it was appended to a node no longer in the list and silently lost: XA:A:a XA:A:b XB:A:c decoded as XA=b alone. The replacement now becomes the tail. The later value of a repeated tag is still the one kept, as SAM text keeps it. --- CHANGELOG.md | 10 +++++ .../java/htsjdk/samtools/BinaryTagCodec.java | 2 + .../htsjdk/samtools/BinaryTagCodecTest.java | 37 +++++++++++++++++++ 3 files changed, 49 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b1dd32e894..83ab6772f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,16 @@ early infrastructure for a plugin-based codec framework and resource bundles. --- +## 6.0.1 + +Patch release on the 6.0 line. + +### Bug fixes + +- **A BAM record with a repeated tag keeps its other tags** ([#1946](https://github.com/samtools/htsjdk/issues/1946)). When a repeated tag sorted highest of the tags decoded so far, as in `XA:A:a XA:A:b XB:A:c`, every later tag that sorted above it was silently dropped. The later value of the repeated tag is still the one kept, as in SAM text. + +--- + ## 6.0.0 Major release. It includes every change in 5.0.2. Before upgrading, read [Upgrading from 5.x](#upgrading-from-5x); to explain differences from files 5.x wrote, see [Output that differs from 5.x](#output-that-differs-from-5x). diff --git a/src/main/java/htsjdk/samtools/BinaryTagCodec.java b/src/main/java/htsjdk/samtools/BinaryTagCodec.java index 728d3983ce..18d7a40952 100644 --- a/src/main/java/htsjdk/samtools/BinaryTagCodec.java +++ b/src/main/java/htsjdk/samtools/BinaryTagCodec.java @@ -306,6 +306,8 @@ public static SAMBinaryTagAndValue readTags( tail = tmp; } else { head = head.insert(tmp); + // A repeat of the last tag replaces it, so the replacement is the new tail + if (tmp.next == null) tail = tmp; } } diff --git a/src/test/java/htsjdk/samtools/BinaryTagCodecTest.java b/src/test/java/htsjdk/samtools/BinaryTagCodecTest.java index 323d6eaecc..09fd668c35 100644 --- a/src/test/java/htsjdk/samtools/BinaryTagCodecTest.java +++ b/src/test/java/htsjdk/samtools/BinaryTagCodecTest.java @@ -1,6 +1,8 @@ package htsjdk.samtools; import htsjdk.HtsjdkTest; +import java.util.LinkedHashMap; +import java.util.Map; import org.testng.Assert; import org.testng.annotations.Test; @@ -13,4 +15,39 @@ public void testCharTagAbove7FIsReadAsTheCharItWasWrittenFrom() { final SAMBinaryTagAndValue decoded = BinaryTagCodec.readTags(tag, 0, tag.length, ValidationStringency.STRICT); Assert.assertEquals(decoded.value, 'é'); } + + // A record may carry a tag twice. As in SAM text, the later value is kept, and the record's other tags must be too. + + /** Decodes a run of A-type tags, each given as its two-letter name followed by its one-character value. */ + private static Map decodeCharTags(final String... tagsAndValues) { + final byte[] bytes = new byte[tagsAndValues.length * 4]; + for (int i = 0; i < tagsAndValues.length; i++) { + bytes[i * 4] = (byte) tagsAndValues[i].charAt(0); + bytes[i * 4 + 1] = (byte) tagsAndValues[i].charAt(1); + bytes[i * 4 + 2] = 'A'; + bytes[i * 4 + 3] = (byte) tagsAndValues[i].charAt(2); + } + final Map decoded = new LinkedHashMap<>(); + for (SAMBinaryTagAndValue tag = BinaryTagCodec.readTags(bytes, 0, bytes.length, ValidationStringency.STRICT); + tag != null; + tag = tag.getNext()) { + decoded.put(SAMTag.makeStringTag(tag.tag), tag.value); + } + return decoded; + } + + @Test + public void testTagsAfterADuplicateOfTheLastTagAreKept() { + Assert.assertEquals(decodeCharTags("XAa", "XAb", "XBc"), Map.of("XA", 'b', "XB", 'c')); + } + + @Test + public void testTagsAfterADuplicateOfTheLastTagAreKeptWhenTagsAreOutOfOrder() { + Assert.assertEquals(decodeCharTags("XBb", "XAa", "XBc", "XCd"), Map.of("XA", 'a', "XB", 'c', "XC", 'd')); + } + + @Test + public void testADuplicateOfAnEarlierTagKeepsTheLaterValue() { + Assert.assertEquals(decodeCharTags("XAa", "XCc", "XAb", "XDd"), Map.of("XA", 'b', "XC", 'c', "XD", 'd')); + } } From 4755e8d70c76806a92d2f0b59c5bad2e05c3f9e2 Mon Sep 17 00:00:00 2001 From: tfenne Date: Thu, 29 Oct 2026 18:48:23 -0600 Subject: [PATCH 2/2] Say that later tags could be dropped, not that every later tag was --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 83ab6772f0..84c5234798 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ Patch release on the 6.0 line. ### Bug fixes -- **A BAM record with a repeated tag keeps its other tags** ([#1946](https://github.com/samtools/htsjdk/issues/1946)). When a repeated tag sorted highest of the tags decoded so far, as in `XA:A:a XA:A:b XB:A:c`, every later tag that sorted above it was silently dropped. The later value of the repeated tag is still the one kept, as in SAM text. +- **A BAM record with a repeated tag keeps its other tags** ([#1946](https://github.com/samtools/htsjdk/issues/1946)). When a repeated tag sorted highest of the tags decoded so far, later tags could be silently dropped: `XA:A:a XA:A:b XB:A:c` decoded as `XA=b` alone. The later value of the repeated tag is still the one kept, as in SAM text. ---