From db48da678aea714b24b28aa73096c59739f2b9b5 Mon Sep 17 00:00:00 2001 From: Test Date: Mon, 5 Oct 2026 10:24:00 +0000 Subject: [PATCH] fix(ENGKNOW-3981): refbases below position 1 returns N instead of throwing RefSeqFromChromSeq.getBases(chr, 0, n) took the same-buffer fast path, because (pos - 1) / buffLength truncates toward zero for pos 0, and then indexed lastBuff(-1), throwing ArrayIndexOutOfBoundsException. getBase caught the error and returned 'N', so only multi-base reads failed, e.g. refbases(chrom,pos,pos+3) on a row with pos 0. Positions below 1 now read as 'N', like positions past the chromosome end. getBases emits 'N' for pos1..min(pos2,0) and continues from position 1; getBase returns 'N' for pos < 1 before any buffer lookup. Positions >= 1 are unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../gorsat/parser/UTestGenomicFunctions.java | 13 ++++++ .../gor/iterators/RefSeqFromChromSeq.scala | 8 ++++ .../iterators/UTestRefSeqFromChromSeq.java | 40 +++++++++++++++++++ 3 files changed, 61 insertions(+) diff --git a/gortools/src/test/java/gorsat/parser/UTestGenomicFunctions.java b/gortools/src/test/java/gorsat/parser/UTestGenomicFunctions.java index 96f2d66f5..b944554bd 100644 --- a/gortools/src/test/java/gorsat/parser/UTestGenomicFunctions.java +++ b/gortools/src/test/java/gorsat/parser/UTestGenomicFunctions.java @@ -89,6 +89,19 @@ public void testRefBases() { Assert.assertEquals("cct", TestUtils.getCalculatedWithArgs("refbases('chr1', 10101, 10103)", new String[]{"-config", "../tests/data/ref_mini/gor_config.txt"})); } + @Test + public void testRefBasesBelowPositionOne() { + String[] args = new String[]{"gorrow chr17,0 | calc r refbases(chrom,pos,pos+3)", "-config", "../tests/data/ref_mini/gor_config.txt"}; + String lines = TestUtils.runGorPipe(args); + Assert.assertEquals("chrom\tpos\tr\nchr17\t0\tNAAG\n", lines); + } + + @Test + public void testRefBasesWithBuildBelowPositionOne() { + String lines = TestUtils.runGorPipe("gorrow chr17,0 | calc r refbases_with_build(chrom,pos,pos+3,'../tests/data/ref_mini/chromSeq')"); + Assert.assertEquals("chrom\tpos\tr\nchr17\t0\tNAAG\n", lines); + } + @Test public void testRefBasesWithBuildUnQuoted() { TestUtils.assertCalculated("refbases_with_build('chr1', 10101, 10103, ../tests/data/ref_mini/chromSeq)", "cct"); diff --git a/model/src/main/scala/org/gorpipe/model/gor/iterators/RefSeqFromChromSeq.scala b/model/src/main/scala/org/gorpipe/model/gor/iterators/RefSeqFromChromSeq.scala index d17c78b57..1f38448ce 100644 --- a/model/src/main/scala/org/gorpipe/model/gor/iterators/RefSeqFromChromSeq.scala +++ b/model/src/main/scala/org/gorpipe/model/gor/iterators/RefSeqFromChromSeq.scala @@ -114,6 +114,8 @@ class RefSeqFromChromSeq(ipath : String, fileReader : FileReader) extends RefSeq def getBase(chr: String, pos: Int): Char = { if (noReferenceBuildFound) return 'N' + // Positions below 1 are outside the chromosome, same as positions past its end. + if (pos < 1) return 'N' try { val (buffKey, offset) = getKeyAndOffset(chr, pos) @@ -165,6 +167,12 @@ class RefSeqFromChromSeq(ipath : String, fileReader : FileReader) extends RefSeq } def getBases(chr: String, pos1: Int, pos2: Int): String = { + // Positions below 1 are outside the chromosome and read as 'N', same as positions past its end. + // Handle them before the buffer lookup: (pos - 1) / buffLength truncates toward zero, so pos 0 maps to buffer 0. + if (pos1 < 1) { + val leading = "N" * (math.min(pos2, 0) - pos1 + 1) + return if (pos2 < 1) leading else leading + getBases(chr, 1, pos2) + } if (pos1 == pos2) return getBase(chr, pos1).toString if ((pos1 - 1) / buffLength == (pos2 - 1) / buffLength) { val (buffKey, offset) = getKeyAndOffset(chr, pos1) diff --git a/model/src/test/java/org/gorpipe/model/gor/iterators/UTestRefSeqFromChromSeq.java b/model/src/test/java/org/gorpipe/model/gor/iterators/UTestRefSeqFromChromSeq.java index 8f0948458..ca45b28ac 100644 --- a/model/src/test/java/org/gorpipe/model/gor/iterators/UTestRefSeqFromChromSeq.java +++ b/model/src/test/java/org/gorpipe/model/gor/iterators/UTestRefSeqFromChromSeq.java @@ -79,6 +79,46 @@ public void testGetRefbases() { } + // Positions below 1 (e.g. pos 0 from an unmapped liftover row) read as 'N', like positions past the chromosome end. + @Test + public void testGetRefbasesBelowPositionOne() { + String path = "../tests/data/ref_mini/chromSeq"; + + // chr17 in ref_mini starts with real bases (AAGC...). + RefSeqFromChromSeq refseq = new RefSeqFromChromSeq(path, new DriverBackedFileReader("")); + Assert.assertEquals("NAAGC", refseq.getBases("chr17", 0, 4)); + + // Same, with the buffer already loaded. + Assert.assertEquals("NAAGC", refseq.getBases("chr17", 0, 4)); + + // Fresh refseq, no buffer loaded yet. + refseq = new RefSeqFromChromSeq(path, new DriverBackedFileReader("")); + Assert.assertEquals("NNNNN", refseq.getBases("chr17", -5, -1)); + + Assert.assertEquals("N", refseq.getBases("chr17", 0, 0)); + Assert.assertEquals('N', refseq.getBase("chr17", 0)); + Assert.assertEquals('N', refseq.getBase("chr17", -1)); + Assert.assertEquals('N', refseq.getBase("chr17", -10001)); + + // Starting below 1 and crossing the buffer boundary (10000). + String expectedTail = new RefSeqFromChromSeq(path, new DriverBackedFileReader("")).getBases("chr17", 1, 10002); + refseq = new RefSeqFromChromSeq(path, new DriverBackedFileReader("")); + String bases = refseq.getBases("chr17", -2, 10002); + Assert.assertEquals(10005, bases.length()); + Assert.assertEquals("NNN" + expectedTail, bases); + Assert.assertTrue(bases.startsWith("NNNAAGC")); + Assert.assertTrue(bases.endsWith("aactctt")); + + // Starting more than a buffer below 1. + refseq = new RefSeqFromChromSeq(path, new DriverBackedFileReader("")); + Assert.assertEquals("N".repeat(10006) + "AA", refseq.getBases("chr17", -10005, 2)); + + // Positive positions are unchanged. + Assert.assertEquals("AAGC", refseq.getBases("chr17", 1, 4)); + Assert.assertEquals('A', refseq.getBase("chr17", 1)); + Assert.assertEquals("aactcttgac", refseq.getBases("chr17", 9996, 10005)); + } + @Ignore("Run manually to test from same buffer optimization") @Test public void testGetRefbasesPerformance() {