feat(ENGKNOW-4117): native LIFTOVER_ANNOTATE command and every LIFTOVER mode on the same engine by gmagnu · Pull Request #159 · gorpipe/gor

gmagnu · 2026-10-31T01:58:13Z

Summary

ENGKNOW-4117: safer and faster liftover annotation, with a single liftover engine.

New LIFTOVER_ANNOTATE pipe command: streams exactly one output row per input row and adds liftover columns (liftover_chrom, liftover_pos or liftover_start/liftover_stop, liftover_strand, liftover_status, plus allele, endpoint and size diagnostics per mode). Modes -snp, -var, -seg, -seg1. Statuses include mapped, ambiguous, unmapped, inverted, size_changed, mapped_across_gap, split_contig, start_unmapped/stop_unmapped, unsupported_contig and passthrough. See LIFTOVER_ANNOTATE.rst.
LIFTOVER is rebuilt on the same engine for every mode (-snp, -var, -seg, -bam, -all, nested <(gor …) chains). The old query-string expansion is deleted, so there is one lift formula and one status rule.
New Java package gorsat.liftover: per-contig in-memory chain index (primitive arrays, prefix-max, binary search), LiftEngine, chain loading/validation and the two analyses. The LIFTOVER command itself is now Java (Liftover.java).
LIFTOVER_LEGACY (temporary): the pre-change LIFTOVER, restored unchanged as LiftoverLegacy.scala, so results can be compared with the new LIFTOVER. Same arguments. To be removed in a later release.
Chains are cached per session (path|uniqueId|policy); nested <(gor …) chains are not cached; <(nor …) chains are rejected.

Behaviour changes in `LIFTOVER`

All listed under versionchanged:: 5.17.0 in LIFTOVER.rst:

Identical output rows are no longer collapsed.
A tie between equal-score blocks gives one ambiguous row instead of duplicates.
-var reverse-complements each allele separately.
-bam is 1-based inclusive: - strand reads move by one base, MPOS is lifted (0 when the mate lands elsewhere or MRNM is *), a 1-base read on a block's last base maps, and an MPOS that is not a positive integer passes through.
A span no block contains, with an endpoint in a tie, is ambiguous (overlapping chains only).
Chains are validated: unsorted, gapped, '0'-strand, empty or malformed chains fail.
end < start is an error; a zero-length interval is a junction. Empty or - coordinate cells and repeated options are errors.

Testing

Full local suite (./gradlew test :gortools:testScala): 4,815 JUnit tests, 96 skipped, and 683 ScalaTest tests. All pass except 3 in UTestOCIObjectStorageSourceType (no OCI credentials locally); this branch does not change drivers.
Characterization tests of the old LIFTOVER output were written first.
Differential test (slow): a test-only Java port of the old expansion compared against the new engine on ~8M random rows; every difference is classified into one of the nine approved categories, and every category occurs.
LIFTOVER_LEGACY reproduces all 18 frozen pre-change outputs and the 3 frozen errors, and in every differential run the test oracle's output equals LIFTOVER_LEGACY's.
JaCoCo: 100% line coverage on the new gorsat.liftover files and LiftoverAnnotate.java (measured before the Liftover.java port; its branches are covered by the LIFTOVER characterization and parsing tests).
Benchmark, 1M rows, new vs old: VAR 841 vs 3,600 ms, SEG 73 vs 316 ms, VAR -all 984 vs 2,821 ms, BAM -all 131 vs 454 ms.
Chains in clinops-dev (ref-gregor/managed/liftover) scanned: sorted, no overlaps, ties or bad strands.

Follow-ups

ENGKNOW-4141: merged ref-gregor chains bridge gaps and map chrM as identity.
Update gor-opensource-third compat baselines (only error text changes).
Tell Sequence Miner owners about the -bam and -all changes before release.

🤖 Generated with Claude Code

… tests Adds UTestGorLiftoverCharacterization, pinning the exact output of today's LIFTOVER (-snp/-var/-seg/-bam, -all, -ref, -build, overlaps, empty REF, zero strand chains, nested chain, option errors, column collisions). Expectations that the native engine changes are marked // P4-CHANGES and kept in *_LEGACY constants. Probed literals: zeroLengthSeg, zeroStrandChainAll and multiAllelicRevcompl matched the brief unchanged. allScoreAboveInt had no literal in the brief and takes the observed row: chr12 90834 A G a1 chr12 200000 + mapped 1.234103462E10 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…ading Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…s and multi-contig nested chains Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…onomy Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…aining block A span contained in best block i now reports endpoints through block i, targetSize = sourceSize and sizeRatio 1.0 also when i's target is outside -mapped (unsupported_contig), giving one rule for contained spans. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…d diag; count failon rows in stats Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…e the annotate step clone safely Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…verArgs, link retargeting and non-empty guards Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…he signatures Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

… prefix default Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…ation wording and examples Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…ples from the docs Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…iant list Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…ntial test Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…tial categories and classify unit tests Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…ase as D8 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…he oracle proof and widen the random inputs Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…ate bam pair categories on the new model Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…gine LIFTOVER -snp, -var, -seg, -bam, -all and nested-query chains now run on LiftEngine through LiftoverRelocateAnalysis followed by sort genome; the query-string expansion is deleted. Behaviour changes are the approved N9 list, each a P4-CHANGES test or a differential category D1-D9. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

… position An MPOS that is not an integer > 0 is kept unchanged on every row (R18); an integer > 0 keeps the N15 lift, and MRNM '*' still gives 0. The differential's new-model MPOS follows the same rule. Adds tests for the MPOS and MRNM edges, liftMate, empty chains (R17) and parse errors. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…re paths and clone handling Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…ER status projection Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…mate lift and the norrows outcome Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…rder and minus-strand indels Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…rows, split nested contigs and repeated options - LIFTOVER_ANNOTATE and LIFTOVER reject an empty or blank coordinate cell instead of reading it as 0, and name chrom:pos - chain loader rejects tStart < 0, qStart < 0, empty qChrom and empty tEnd/qStart/qEnd - a nested chain rejects a contig that reappears after another contig - LIFTOVER_ANNOTATE rejects a value option given more than once Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…n docs RowObj.colInt reads a lone '-' as 0, so a -seg start of '-' lifted [0,end) and a chain qStart of '-' became a valid block at 0. Input coordinates and chain tEnd/qStart/qEnd now reject it. LIFTOVER_ANNOTATE.rst no longer claims a reappearing file-chain contig uses its first run, or that a negative tStart gets the Liftover chain prefix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

gmagnu · 2026-11-02T09:48:55Z

+    /** Writes a GOR file; rows are sorted by (chrom, pos) on columns 0 and 1. */
+    private static String gorFile(String name, String header, List<String[]> rows) throws IOException {
+        List<String[]> sorted = new ArrayList<>(rows);
+        sorted.sort(Comparator.<String[], String>comparing(r -> r[0]).thenComparingInt(r -> Integer.parseInt(r[1])));


No change here: this is test code, and the value is a position the test itself generated (fixture rows or RandomChains input), so it is always an integer. If it ever were not, the uncaught NumberFormatException would fail the test with a stack trace pointing at this line, which is the behaviour we want; catching it would only hide a broken fixture.

gmagnu · 2026-11-02T09:48:57Z

+    /** NaiveLift's containing blocks for an input row under LIFTOVER's mode rule (N12, N14, N15). */
+    private static List<NaiveLift.Hit> naiveHits(String[] in, String mode, List<ChainBlock> blocks) {
+        NaiveLift nl = new NaiveLift(blocks);
+        int p = Integer.parseInt(in[1]);


No change here: this is test code, and the value is a position the test itself generated (fixture rows or RandomChains input), so it is always an integer. If it ever were not, the uncaught NumberFormatException would fail the test with a stack trace pointing at this line, which is the behaviour we want; catching it would only hide a broken fixture.

gmagnu · 2026-11-02T09:48:59Z

+            case "var":
+                return in[2].isEmpty() ? nl.junctionHits(p - 1) : nl.intervalHits(p, p + in[2].length() - 1);
+            case "seg": {
+                int end = Integer.parseInt(in[2]);


No change here: this is test code, and the value is a position the test itself generated (fixture rows or RandomChains input), so it is always an integer. If it ever were not, the uncaught NumberFormatException would fail the test with a stack trace pointing at this line, which is the behaviour we want; catching it would only hide a broken fixture.

gmagnu · 2026-11-02T09:49:01Z

+                return end == p ? nl.junctionHits(p) : nl.intervalHits(p + 1, end);
+            }
+            default: {
+                int end = Integer.parseInt(in[2]);


No change here: this is test code, and the value is a position the test itself generated (fixture rows or RandomChains input), so it is always an integer. If it ever were not, the uncaught NumberFormatException would fail the test with a stack trace pointing at this line, which is the behaviour we want; catching it would only hide a broken fixture.

gmagnu · 2026-11-02T09:49:03Z

+        Map<String, int[]> range = new LinkedHashMap<>();
+        for (String[] r : bam.table.rows) {
+            int[] mm = range.computeIfAbsent(r[bam.chrom], k -> new int[]{Integer.MAX_VALUE, 0});
+            mm[0] = Math.min(mm[0], Integer.parseInt(r[bam.pos]) - 1);


No change here: this is test code, and the value is a position the test itself generated (fixture rows or RandomChains input), so it is always an integer. If it ever were not, the uncaught NumberFormatException would fail the test with a stack trace pointing at this line, which is the behaviour we want; catching it would only hide a broken fixture.

gmagnu · 2026-11-02T09:49:05Z

+        for (String[] r : bam.table.rows) {
+            int[] mm = range.computeIfAbsent(r[bam.chrom], k -> new int[]{Integer.MAX_VALUE, 0});
+            mm[0] = Math.min(mm[0], Integer.parseInt(r[bam.pos]) - 1);
+            mm[1] = Math.max(mm[1], Integer.parseInt(r[bam.end]));


No change here: this is test code, and the value is a position the test itself generated (fixture rows or RandomChains input), so it is always an integer. If it ever were not, the uncaught NumberFormatException would fail the test with a stack trace pointing at this line, which is the behaviour we want; catching it would only hide a broken fixture.

gmagnu · 2026-11-02T09:49:07Z

+                                               List<String[]> matePairs) {
+        List<String> expected = new ArrayList<>();
+        for (String[] r : bam.table.rows) {
+            int s0 = Integer.parseInt(r[bam.pos]) - 1;


No change here: this is test code, and the value is a position the test itself generated (fixture rows or RandomChains input), so it is always an integer. If it ever were not, the uncaught NumberFormatException would fail the test with a stack trace pointing at this line, which is the behaviour we want; catching it would only hide a broken fixture.

gmagnu · 2026-11-02T09:49:10Z

+        List<String> expected = new ArrayList<>();
+        for (String[] r : bam.table.rows) {
+            int s0 = Integer.parseInt(r[bam.pos]) - 1;
+            int e0 = Integer.parseInt(r[bam.end]);


No change here: this is test code, and the value is a position the test itself generated (fixture rows or RandomChains input), so it is always an integer. If it ever were not, the uncaught NumberFormatException would fail the test with a stack trace pointing at this line, which is the behaviour we want; catching it would only hide a broken fixture.

gmagnu · 2026-11-02T09:49:12Z

+        Map<String, Integer> expectedPerKey = new HashMap<>();
+        for (String[] r : bam.table.rows) {
+            int n = Math.max(1, containing(chain, r[bam.chrom], Integer.parseInt(r[bam.pos]) - 1,
+                    Integer.parseInt(r[bam.end])).size());


No change here: this is test code, and the value is a position the test itself generated (fixture rows or RandomChains input), so it is always an integer. If it ever were not, the uncaught NumberFormatException would fail the test with a stack trace pointing at this line, which is the behaviour we want; catching it would only hide a broken fixture.

gmagnu · 2026-11-02T09:49:14Z

+        }
+        Map<String, Integer> expectedPerKey = new HashMap<>();
+        for (String[] r : bam.table.rows) {
+            int n = Math.max(1, containing(chain, r[bam.chrom], Integer.parseInt(r[bam.pos]) - 1,


No change here: this is test code, and the value is a position the test itself generated (fixture rows or RandomChains input), so it is always an integer. If it ever were not, the uncaught NumberFormatException would fail the test with a stack trace pointing at this line, which is the behaviour we want; catching it would only hide a broken fixture.

github-actions · 2026-10-31T02:08:15Z

Junit Tests - Summary

5 206 tests +4 983 5 036 ✅ +4 835 31m 34s ⏱️ + 29m 7s
526 suites + 502 170 💤 + 148
526 files + 502 0 ❌ ± 0

Results for commit 560c2a5. ± Comparison against base commit 2285213.

♻️ This comment has been updated with latest results.

… LIFTOVER_LEGACY Liftover.scala becomes Liftover.java with the same arguments, output and errors. The pre-ENGKNOW-4117 implementation is restored unchanged as LiftoverLegacy.scala, registered as LIFTOVER_LEGACY, so results can be compared with the new LIFTOVER. It is temporary and will be removed. LIFTOVER_LEGACY is listed with the commands that contain input sources, so its chain is in the used files and cache signatures. Tests check that it reproduces the frozen pre-change output and, in every differential run, that the test oracle equals it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Adds LIFTOVER tests for a bad -ref/-alt outside -var, -alt by name on a column other than #4, a case-insensitive output-name collision and a -bam input without SEQ. The LegacyLiftoverQuery javadoc now says that LIFTOVER_LEGACY holds the same expansion for now. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

gmagnu and others added 30 commits October 31, 2026 01:52

feat(ENGKNOW-4117): add per-contig chain index with lazy validated lo…

ea3304e

…ading Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

fix(ENGKNOW-4117): reject non-finite chain scores and cover big score…

00c3d96

…s and multi-contig nested chains Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

feat(ENGKNOW-4117): add pure liftover engine with interval status tax…

0a6c3c7

…onomy Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

feat(ENGKNOW-4117): add 1:1 streaming liftover annotate analysis

1dd6461

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

test(ENGKNOW-4117): cover non-default columns, absolute seg values an…

7a59b3b

…d diag; count failon rows in stats Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

feat(ENGKNOW-4117): add LIFTOVER_ANNOTATE command

3bf551f

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

fix(ENGKNOW-4117): reject options that do nothing in the mode and mak…

17da91d

…e the annotate step clone safely Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

test(ENGKNOW-4117): cover LIFTOVER_ANNOTATE options end to end, Lifto…

47a1066

…verArgs, link retargeting and non-empty guards Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

fix(ENGKNOW-4117): include liftover chain files in used files and cac…

b104918

…he signatures Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

docs(ENGKNOW-4117): document LIFTOVER_ANNOTATE and fix LIFTOVER build…

e3cf00a

… prefix default Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

docs(ENGKNOW-4117): correct LIFTOVER_ANNOTATE status order, empty-loc…

e58b363

…ation wording and examples Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

test(ENGKNOW-4117): pin LIFTOVER_ANNOTATE -seg1 and -diag prefix exam…

9cd9129

…ples from the docs Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

test(ENGKNOW-4117): add liftover vs liftover_annotate benchmark

cf073ae

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

test(ENGKNOW-4117): benchmark warm-up, repeated rotating runs and var…

b65598b

…iant list Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

test(ENGKNOW-4117): add legacy liftover oracle and old-vs-new differe…

7ccf83f

…ntial test Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

test(ENGKNOW-4117): add D7 endpoint-tie and D8 bam last-base differen…

b4a2728

…tial categories and classify unit tests Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

test(ENGKNOW-4117): accept a tied 1-base bam read on a block's last b…

8089930

…ase as D8 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

test(ENGKNOW-4117): model-check D8 and D2, split D9 from D6, freeze t…

ab47859

…he oracle proof and widen the random inputs Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

test(ENGKNOW-4117): self-check the old model against the oracle and g…

e9bbe3a

…ate bam pair categories on the new model Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

docs(ENGKNOW-4117): note the MPOS rule and empty-chain error in LIFTOVER

308c7a2

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

test(ENGKNOW-4117): add liftover consumer-shape and round-trip tests

32a1f09

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

test(ENGKNOW-4117): cover the remaining liftover guards, loader failu…

a2c29f7

…re paths and clone handling Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

fix(ENGKNOW-4117): drop the unreachable mapped branch from the LIFTOV…

ea00faf

…ER status projection Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

test(ENGKNOW-4117): pin hand-computed minus-strand anchors, real-BAM …

d987746

…mate lift and the norrows outcome Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

docs(ENGKNOW-4117): document D7, D8, the interval rules, chain sort o…

e543135

…rder and minus-strand indels Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

github-code-quality Bot found potential problems Oct 31, 2026

View reviewed changes

gmagnu and others added 2 commits October 31, 2026 03:08

bragnarsson approved these changes Nov 2, 2026

View reviewed changes

gmagnu merged commit f58747f into main Nov 2, 2026
14 checks passed

gmagnu deleted the ENGKNOW-4117-liftover-annotate-command branch November 2, 2026 10:35

Navigation Menu

Repository navigation

feat(ENGKNOW-4117): native LIFTOVER_ANNOTATE command and every LIFTOVER mode on the same engine - #159

feat(ENGKNOW-4117): native LIFTOVER_ANNOTATE command and every LIFTOVER mode on the same engine#159
gmagnu merged 33 commits into
mainfrom
ENGKNOW-4117-liftover-annotate-command

gmagnu commented Oct 31, 2026 •

edited

Loading

Uh oh!

gmagnu Nov 2, 2026

Uh oh!

gmagnu Nov 2, 2026

Uh oh!

gmagnu Nov 2, 2026

Uh oh!

gmagnu Nov 2, 2026

Uh oh!

gmagnu Nov 2, 2026

Uh oh!

gmagnu Nov 2, 2026

Uh oh!

gmagnu Nov 2, 2026

Uh oh!

gmagnu Nov 2, 2026

Uh oh!

gmagnu Nov 2, 2026

Uh oh!

gmagnu Nov 2, 2026

Uh oh!

github-actions Bot commented Oct 31, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

gmagnu commented Oct 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Behaviour changes in LIFTOVER

Testing

Follow-ups

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Junit Tests - Summary

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gmagnu commented Oct 31, 2026 •

edited

Loading

Behaviour changes in `LIFTOVER`

github-actions Bot commented Oct 31, 2026 •

edited

Loading