Merge pull request #14 from fathomnet/fix/taxonomy-lookup-bugs

Fix taxonomy lookup bugs

Author: hohonuuli

build.sbt

@@ -3,7 +3,7 @@ import Dependencies._
 Global / onChangedBuildSource := ReloadOnSourceChanges
 Laika / sourceDirectories     := Seq(baseDirectory.value / "docs")
 
-ThisBuild / scalaVersion     := "3.8.3"
+ThisBuild / scalaVersion     := "3.8.4"
 ThisBuild / organization     := "org.fathomnet"
 ThisBuild / organizationName := "MBARI"
 ThisBuild / startYear        := Some(2021)
@@ -68,6 +68,8 @@ lazy val root = project
         ),
         // Test suites share State.data (global mutable state); parallel execution causes races
         Test / parallelExecution := false,
+        // Fork so the Vert.x event-loop threads keep the JVM alive when running via `sbt run`
+        run / fork               := true,
         scalacOptions ++= Seq(
             "-deprecation", // Emit warning and location for usages of deprecated APIs.
             "-encoding",

project/Dependencies.scala

@@ -9,9 +9,9 @@ object Dependencies {
 
     lazy val jansi = "org.fusesource.jansi" % "jansi" % "2.4.3"
 
-    lazy val logback  = "ch.qos.logback"               % "logback-classic" % "1.5.32"
+    lazy val logback  = "ch.qos.logback"               % "logback-classic" % "1.5.34"
     lazy val methanol = "com.github.mizosoft.methanol" % "methanol"        % "1.9.0"
-    lazy val munit    = "org.scalameta"               %% "munit"           % "1.3.0"
+    lazy val munit    = "org.scalameta"               %% "munit"           % "1.3.3"
     lazy val picocli  = "info.picocli"                 % "picocli"         % "4.7.7"
 
     lazy val slf4jJdk = "org.slf4j" % "slf4j-jdk-platform-logging" % "2.0.18"
@@ -24,7 +24,7 @@ object Dependencies {
     lazy val tapirNetty       = "com.softwaremill.sttp.tapir"   %% "tapir-netty-server"      % tapirVersion
     lazy val tapirVertx       = "com.softwaremill.sttp.tapir"   %% "tapir-vertx-server"      % tapirVersion
 
-    lazy val typesafeConfig = "com.typesafe" % "config" % "1.4.8"
+    lazy val typesafeConfig = "com.typesafe" % "config" % "1.4.9"
     lazy val zio            = "dev.zio"     %% "zio"    % "2.1.26"
 
 }

project/build.properties

@@ -1 +1 @@
-sbt.version=1.11.7
+sbt.version=1.12.11

src/main/scala/org/fathomnet/worms/Data.scala

@@ -24,16 +24,22 @@ final case class Data(rootNode: WormsNode, wormsConcepts: Seq[WormsConcept]):
     private val log = System.getLogger(getClass.getName)
 
     /**
-     * Map of [nodeName, Node] for both the name and alternate name of the node.
+     * Multimap of [name, nodes] for both the primary name and alternate names of each node.
+     * A single name can map to multiple nodes when common/vernacular names are shared across
+     * different taxa (e.g. "limpets" appears as a vernacular name for both Patellidae and
+     * Acmaeidae). Storing all matching nodes prevents the last-writer-wins silent data loss
+     * that occurred with the previous SortedMap[String, WormsNode].
      */
-    lazy val namesMap: SortedMap[String, WormsNode] =
-        val map                        = SortedMap.newBuilder[String, WormsNode]
+    lazy val namesMap: SortedMap[String, Seq[WormsNode]] =
+        val map = scala.collection.mutable.Map.empty[String, Vector[WormsNode]]
+        def insert(name: String, node: WormsNode): Unit =
+            map.updateWith(name)(existing => Some(existing.getOrElse(Vector.empty) :+ node))
         def add(node: WormsNode): Unit =
-            map += node.name -> node
-            node.alternateNames.foreach(n => map += n -> node)
+            insert(node.name, node)
+            node.alternateNames.foreach(n => insert(n, node))
             node.children.foreach(add)
         add(rootNode)
-        map.result()
+        SortedMap.from(map)
 
     /**
      * All names used in WoRMS.
@@ -89,7 +95,23 @@ final case class Data(rootNode: WormsNode, wormsConcepts: Seq[WormsConcept]):
         add(rootNode)
         map.result()
 
-    def findNodeByName(name: String): Option[WormsNode] = namesMap.get(name)
+    /**
+     * Return all nodes associated with a name (primary or alternate).
+     * Returns an empty Seq when the name is not present in the tree.
+     */
+    def findNodesByName(name: String): Seq[WormsNode] =
+        namesMap.getOrElse(name, Vector.empty)
+
+    /**
+     * Return the single best node for a name. When multiple nodes share the same name,
+     * preference order is: (1) node whose primary name matches exactly, (2) any accepted
+     * node, (3) first in insertion order.
+     */
+    def findNodeByName(name: String): Option[WormsNode] =
+        namesMap.get(name).flatMap: nodes =>
+            nodes.find(_.name == name)
+                .orElse(nodes.find(_.isAccepted))
+                .orElse(nodes.headOption)
 
     def findNodeByChildName(name: String): Option[WormsNode] = parents.get(name)
 

src/main/scala/org/fathomnet/worms/StateController.scala

@@ -94,13 +94,18 @@ object StateController:
 
     def descendantNames(name: String, acceptedOnly: Boolean = false): Either[ErrorMsg, List[String]] =
         def search(data: Data): List[String] =
-            data.findNodeByName(name) match
-                case None       => Nil
-                case Some(node) =>
-                    if (acceptedOnly && node.isAccepted)
-                        node.descendants.filter(_.isAccepted).map(_.name).sorted.toList
-                    else
-                        node.descendantNames.sorted.toList
+            val nodes = data.findNodesByName(name)
+            if nodes.isEmpty then Nil
+            else
+                nodes
+                    .flatMap: node =>
+                        if acceptedOnly then
+                            node.descendants.filter(_.isAccepted).map(_.name)
+                        else
+                            node.descendantNames
+                    .distinct
+                    .sorted
+                    .toList
         runSearch(search).fold(
             e => Left(e),
             v =>
@@ -190,7 +195,9 @@ object StateController:
                         .rangeFrom(lower)
                         .takeWhile(_._1.startsWith(lower))
                         .values
-                        .flatMap(names => names.flatMap(data.findNodeByName))
+                        .flatMap(names => names.flatMap(data.findNodesByName))
+                        .toSeq
+                        .distinctBy(_.aphiaId)
                         .map(_.simple)
                         .toList
                 case Some(parent) =>
@@ -230,7 +237,7 @@ object StateController:
                     )
             names        <- synonyms(name)
             acceptedName <- names.headOption.toRight(NotFound(s"Unable to find `$name`"))
-            wormsConcept <- data.namesMap.get(acceptedName)
+            wormsConcept <- data.findNodeByName(acceptedName)
                                 .flatMap(node => data.wormsConceptsMap.get(node.aphiaId))
                                 .toRight(NotFound(s"Unable to find `$name` accepted name of `$acceptedName`"))
         yield WormsDetails

src/main/scala/org/fathomnet/worms/io/model.scala

@@ -20,35 +20,34 @@ import scala.util.{Success, Try, Using}
 def taxonIDToKey(taxonID: String): Long =
     taxonID.split(":").last.toLong
 
-def readFile[A](file: String, rowMapper: String => Option[A]): List[A] = {
+def readFile[A](file: String, rowMapper: String => Option[A]): List[A] =
     Using(Source.fromFile(file)) { source =>
         source.getLines().flatMap(rowMapper).toList
     } match
         case Success(list) => list
         case _             => List.empty[A]
 
-}
-
 final case class Taxon(
     taxonID: String,
     parentNameUsageID: Option[String],
     scientificName: String,
     rank: String,
     acceptedNameUsageID: Option[String]
 ):
-    val id: Long = taxonIDToKey(taxonID)
-    val parentId: Option[Long] = parentNameUsageID.map(taxonIDToKey)
+    val id: Long                 = taxonIDToKey(taxonID)
+    val parentId: Option[Long]   = parentNameUsageID.map(taxonIDToKey)
     val acceptedId: Option[Long] = acceptedNameUsageID.map(taxonIDToKey)
 
 object Taxon:
     def from(row: String): Option[Taxon] =
         Try {
-            val cols                = row.split("\t")
-            val taxonID             = cols(0)
-            val acceptedNameUsageID = if cols(2).isBlank then None else Some(cols(2))
-            val parentNameUsageID   = if cols(3).isBlank then None else Some(cols(3))
-            val scientificName      = cols(5)
-            val rank                = cols(19)
+            val cols                = row.split("\t", -1).toList
+            def get(i: Int): String = cols.applyOrElse(i, (_: Int) => "")
+            val taxonID             = get(0)
+            val acceptedNameUsageID = if get(2).isBlank then None else Some(get(2))
+            val parentNameUsageID   = if get(3).isBlank then None else Some(get(3))
+            val scientificName      = get(5)
+            val rank                = get(19)
             Taxon(taxonID, parentNameUsageID, scientificName, rank, acceptedNameUsageID)
         }.toOption
 
@@ -60,8 +59,9 @@ final case class VernacularName(taxonID: String, vernacularName: String):
 object VernacularName:
     def from(row: String): Option[VernacularName] =
         Try {
-            val cols = row.split("\t")
-            VernacularName(cols(0), cols(1))
+            val cols                = row.split("\t", -1).toList
+            def get(i: Int): String = cols.applyOrElse(i, (_: Int) => "")
+            VernacularName(get(0), get(1))
         }.toOption
 
     def read(file: String): List[VernacularName] = readFile(file, VernacularName.from)
@@ -84,8 +84,9 @@ object SpeciesProfile:
 
     def from(row: String): Option[SpeciesProfile] =
         Try {
-            val cols = row.split("\t")
-            SpeciesProfile(cols(0), toBool(cols(1)), toBool(cols(2)), toBool(cols(3)), toBool(cols(4)), toBool(cols(5)))
+            val cols                = row.split("\t", -1).toList
+            def get(i: Int): String = cols.applyOrElse(i, (_: Int) => "")
+            SpeciesProfile(get(0), toBool(get(1)), toBool(get(2)), toBool(get(3)), toBool(get(4)), toBool(get(5)))
         }.toOption
 
     def read(file: String): List[SpeciesProfile] = readFile(file, SpeciesProfile.from)

src/test/scala/org/fathomnet/worms/StateControllerSuite.scala

@@ -13,28 +13,31 @@ class StateControllerSuite extends munit.FunSuite:
     // ---- test tree --------------------------------------------------------
     //   Animalia (2, alternateNames=["animals"])
     //     Mollusca (51)
-    //       Polyplacophora (55, alternateNames=["chitons"])
+    //       Polyplacophora (55, alternateNames=["chitons", "shells"])  ← "shells" shared
     //     Arthropoda (100)
-    //       Decapoda (120, alternateNames=["decapods"])
+    //       Decapoda (120, alternateNames=["decapods", "shells"])      ← "shells" shared
     //     OldArthropoda (200, acceptedAphiaId=100)  ← synonym for Arthropoda
     // -----------------------------------------------------------------------
+    // "shells" is intentionally shared between Polyplacophora and Decapoda to verify
+    // that the namesMap multimap correctly preserves both nodes and that descendant
+    // lookups by a shared alternate name return the union of their descendants.
 
-    private val polyplacophora = WormsNode("Polyplacophora", "Class",   55,  55,  Seq("chitons"),  Nil)
-    private val mollusca       = WormsNode("Mollusca",       "Phylum",  51,  51,  Nil,             Seq(polyplacophora))
-    private val decapoda       = WormsNode("Decapoda",       "Order",   120, 120, Seq("decapods"), Nil)
-    private val arthropoda     = WormsNode("Arthropoda",     "Phylum",  100, 100, Nil,             Seq(decapoda))
-    private val oldArthropoda  = WormsNode("OldArthropoda",  "Phylum",  200, 100, Nil,             Nil)
-    private val root           = WormsNode("Animalia",       "Kingdom", 2,   2,   Seq("animals"),  Seq(mollusca, arthropoda, oldArthropoda))
+    private val polyplacophora = WormsNode("Polyplacophora", "Class",   55,  55,  Seq("chitons", "shells"),  Nil)
+    private val mollusca       = WormsNode("Mollusca",       "Phylum",  51,  51,  Nil,                       Seq(polyplacophora))
+    private val decapoda       = WormsNode("Decapoda",       "Order",   120, 120, Seq("decapods", "shells"),  Nil)
+    private val arthropoda     = WormsNode("Arthropoda",     "Phylum",  100, 100, Nil,                       Seq(decapoda))
+    private val oldArthropoda  = WormsNode("OldArthropoda",  "Phylum",  200, 100, Nil,                       Nil)
+    private val root           = WormsNode("Animalia",       "Kingdom", 2,   2,   Seq("animals"),            Seq(mollusca, arthropoda, oldArthropoda))
 
     private def cn(name: String, primary: Boolean = true) = WormsConceptName(name, primary)
 
     private val wormsConcepts = Seq(
-        WormsConcept(2,   2,   None,      Seq(cn("Animalia"), cn("animals", false)),       "Kingdom", isMarine = Some(true)),
-        WormsConcept(51,  51,  Some(2),   Seq(cn("Mollusca")),                             "Phylum",  isMarine = Some(true)),
-        WormsConcept(55,  55,  Some(51),  Seq(cn("Polyplacophora"), cn("chitons", false)), "Class",   isMarine = Some(true)),
-        WormsConcept(100, 100, Some(2),   Seq(cn("Arthropoda")),                           "Phylum"),
-        WormsConcept(120, 120, Some(100), Seq(cn("Decapoda"), cn("decapods", false)),      "Order"),
-        WormsConcept(200, 100, Some(2),   Seq(cn("OldArthropoda")),                        "Phylum"),
+        WormsConcept(2,   2,   None,      Seq(cn("Animalia"), cn("animals", false)),                               "Kingdom", isMarine = Some(true)),
+        WormsConcept(51,  51,  Some(2),   Seq(cn("Mollusca")),                                                     "Phylum",  isMarine = Some(true)),
+        WormsConcept(55,  55,  Some(51),  Seq(cn("Polyplacophora"), cn("chitons", false), cn("shells", false)),    "Class",   isMarine = Some(true)),
+        WormsConcept(100, 100, Some(2),   Seq(cn("Arthropoda")),                                                   "Phylum"),
+        WormsConcept(120, 120, Some(100), Seq(cn("Decapoda"), cn("decapods", false), cn("shells", false)),         "Order"),
+        WormsConcept(200, 100, Some(2),   Seq(cn("OldArthropoda")),                                               "Phylum"),
     )
 
     private val testData = Data(root, wormsConcepts)
@@ -55,12 +58,13 @@ class StateControllerSuite extends munit.FunSuite:
     // ---- findAllNames / countAllNames -------------------------------------
 
     test("findAllNames returns paginated results with correct total"):
-        // The tree has 9 distinct names: Animalia, animals, Arthropoda, chitons, Decapoda,
-        // decapods, Mollusca, OldArthropoda, Polyplacophora
+        // 10 distinct names: Animalia, animals, Arthropoda, chitons, Decapoda,
+        // decapods, Mollusca, OldArthropoda, Polyplacophora, shells
+        // ("shells" is shared between Polyplacophora and Decapoda but counts once)
         val page = StateController.findAllNames(3, 0).getOrElse(fail("expected Right"))
         assertEquals(page.limit, 3)
         assertEquals(page.offset, 0)
-        assertEquals(page.total, 9)
+        assertEquals(page.total, 10)
         assertEquals(page.items.size, 3)
 
     test("findAllNames honours offset"):
@@ -69,7 +73,7 @@ class StateControllerSuite extends munit.FunSuite:
         assertEquals(page.items.size, 3)
 
     test("countAllNames returns total number of distinct names"):
-        assertEquals(StateController.countAllNames().getOrElse(fail("expected Right")), 9)
+        assertEquals(StateController.countAllNames().getOrElse(fail("expected Right")), 10)
 
     // ---- queryNames -------------------------------------------------------
 
@@ -114,6 +118,26 @@ class StateControllerSuite extends munit.FunSuite:
     test("descendantNames returns NotFound for unknown name"):
         assert(StateController.descendantNames("Unknown").isLeft)
 
+    test("descendantNames via a shared alternate name returns union of descendants from all matching nodes"):
+        // "shells" is an alternate name for both Polyplacophora (55) and Decapoda (120).
+        // Before the multimap fix, whichever node was inserted last would silently overwrite
+        // the other; the "lost" node's descendants would be missing from the result.
+        val names = StateController.descendantNames("shells").getOrElse(fail("expected Right"))
+        assert(names.contains("Polyplacophora"), s"expected Polyplacophora in $names")
+        assert(names.contains("Decapoda"),       s"expected Decapoda in $names")
+
+    test("findNodesByName returns all nodes sharing a name"):
+        val nodes = State.data.get.findNodesByName("shells")
+        assertEquals(nodes.size, 2)
+        assert(nodes.map(_.name).toSet == Set("Polyplacophora", "Decapoda"))
+
+    test("findNodeByName on a shared alternate name returns an accepted node"):
+        val nodeOpt = State.data.get.findNodeByName("shells")
+        assert(nodeOpt.isDefined)
+        val node = nodeOpt.get
+        assert(node.name == "Polyplacophora" || node.name == "Decapoda")
+        assert(node.isAccepted)
+
     // ---- ancestorNames ----------------------------------------------------
 
     test("ancestorNames returns full path from root to node"):