Actin 2295: Replace scoreValue/scoreValuePrefix with scoreLowerBound/scoreUpperBound by ayaahmedhmf · Pull Request #1504 · hartwigmedical/actin

ayaahmedhmf · 2026-04-10T12:05:25Z

Notes

Each module's changes are in a separate commit to make reviewing easier but will all be squashed and merged together.
Build will fail until actin-datamodel v3.0.0 is deployed (see hartwigmedical/actin-datamodel#23).
This is the counterpart to hartwigmedical/actin-clinical#383 which covers the clinical/curation side.
Pending testing on test-framework

Summary

Replace scoreValue and scoreValuePrefix with scoreLowerBound and scoreUpperBound in algo evaluation functions, database schema, and report formatting
Update ValueComparison with new bounds-based helpers (evaluateBoundsVersusMinValue, evaluateBoundsVersusMaxValue)
Update IHC classification, PD-L1 evaluation, and protein expression evaluation to use score bounds
Update database schema (ihcTest table) and ClinicalDAO to read/write new columns
Update IhcTestInterpreter to format score bounds as ranges in reports
Update example patient JSON fixtures
Bump actin-datamodel to 3.0.0

…ate test fixtures

…LowerBound/scoreUpperBound in schema and DAO

…s instead of single value

… as ranges

…re bounds

jbartletthmf · 2026-04-10T14:18:57Z

algo/src/main/kotlin/com/hartwig/actin/algo/evaluation/molecular/PDL1EvaluationFunctions.kt

+    private fun formatBounds(test: IhcTest) = when {
+        test.scoreLowerBound == test.scoreUpperBound -> "${test.scoreLowerBound}"
+        test.scoreLowerBound != null && test.scoreUpperBound != null -> "${test.scoreLowerBound}-${test.scoreUpperBound}"
+        else -> test.scoreLowerBound?.let { "> $it" } ?: test.scoreUpperBound?.let { "< $it" }


Should this be ">=" and "<="? I thought the range was inclusive.

jbartletthmf · 2026-04-10T14:22:20Z

...c/main/kotlin/com/hartwig/actin/algo/evaluation/molecular/ProteinExpressionByIhcFunctions.kt

+                when (roundedScoreLowerBound == roundedScoreUpperBound && roundedScoreLowerBound == referenceExpressionLevel) {
                    true -> EvaluationResult.PASS
                    false -> EvaluationResult.FAIL
                }


Could use if instead of when.

jbartletthmf · 2026-04-10T14:36:37Z

algo/src/test/kotlin/com/hartwig/actin/algo/evaluation/molecular/HasHER2ExpressionByIhcTest.kt

    private fun ihcTest(
-        scoreValue: Double? = null,
+        scoreLowerBound: Double? = null,
+        scoreUpperBound: Double? = scoreLowerBound,


Defaulting this to scoreLowerBound is somewhat surprising. What do you think about adding a score parameter that is the default for both bounds?

jbartletthmf · 2026-04-10T14:59:51Z

...rc/test/kotlin/com/hartwig/actin/algo/evaluation/tumor/BreastCancerReceptorsEvaluatorTest.kt

+            listOf(IhcTest("HER2", scoreLowerBound = 1.0, scoreUpperBound = 1.0, scoreValueUnit = "+")),
+            ReceptorType.HER2


Should be ER according to test name?

jbartletthmf · 2026-04-10T15:01:50Z

...lin/com/hartwig/actin/algo/evaluation/tumor/HasBreastCancerWithPositiveReceptorOfTypeTest.kt


    private fun createIhcTest(
-        item: String, scoreText: String = "Score", scoreValue: Double = 50.0, scoreValueUnit: String = "Unit"
+        item: String, scoreText: String = "Score", scoreLowerBound: Double = 50.0, scoreUpperBound: Double = scoreLowerBound, scoreValueUnit: String = "Unit"


Again, I think it's a bit surprising to set the upper bound to the lower bound by default.

jbartletthmf · 2026-04-10T15:02:15Z

...lin/com/hartwig/actin/algo/evaluation/tumor/HasBreastCancerWithPositiveReceptorOfTypeTest.kt

+        item = item, scoreText = scoreText, scoreLowerBound = scoreLowerBound, scoreUpperBound = scoreUpperBound,
        scoreValueUnit = scoreValueUnit, impliesPotentialIndeterminateStatus = false


If all arguments can't fit on the same line, then each should have its own line.

jbartletthmf · 2026-04-10T15:04:19Z

...src/test/kotlin/com/hartwig/actin/algo/evaluation/tumor/HasTripleNegativeBreastCancerTest.kt

+
    private fun createIhcTest(
-        item: String, scoreText: String = "Score", scoreValue: Double = 50.0, scoreValueUnit: String = "Unit"
+        item: String, scoreText: String = "Score", scoreLowerBound: Double = 50.0, scoreUpperBound: Double = scoreLowerBound, scoreValueUnit: String = "Unit"


I'll stop repeating myself. Maybe we could provide this functionality for all test classes with a shared TestIhcTestFactory?

jbartletthmf · 2026-04-10T15:06:59Z

report/src/main/kotlin/com/hartwig/actin/report/interpretation/IhcTestInterpreter.kt

+            formattedLowerValue.isNotEmpty() -> "> $formattedLowerValue"
+            formattedUpperValue.isNotEmpty() -> "< $formattedUpperValue"


Are the bounds included in the range? If not, then cases where lower == upper imply an empty range.

jbartletthmf · 2026-04-10T15:07:51Z

report/src/main/kotlin/com/hartwig/actin/report/interpretation/IhcTestInterpreter.kt

+        val formattedLowerValue = valueTest.scoreLowerBound?.let(Formats::twoDigitNumber).orEmpty()
+        val formattedUpperValue = valueTest.scoreUpperBound?.let(Formats::twoDigitNumber).orEmpty()


Why orEmpty() here? It seems like it would be easier to just do null checks below since we never use the empty string.

jbartletthmf · 2026-04-10T15:19:49Z

common/src/main/kotlin/com/hartwig/actin/algo/evaluation/util/ValueComparison.kt


+    fun evaluateBoundsVersusMinValue(lowerBound: Double?, upperBound: Double?, minValue: Double): EvaluationResult =
+        lowerBound?.takeIf { it >= minValue }?.let { PASS }
+            ?: upperBound?.takeIf { it < minValue }?.let { FAIL }


This makes sense if the range of included values includes the bounds. If the bounds are excluded, then an upper bound equal to the min value should also lead to FAIL since the true value could not equal the min.

ayaahmedhmf added 6 commits April 10, 2026 09:17

ACTIN-2295: Bump actin-datamodel to version 3.0.0

3f92f9d

ACTIN-2295: [common] Add bounds-based ValueComparison helpers and upd…

ba4d345

…ate test fixtures

ACTIN-2295: [database] Replace scoreValue/scoreValuePrefix with score…

7352710

…LowerBound/scoreUpperBound in schema and DAO

ACTIN-2295: [algo] Update IHC evaluation functions to use score bound…

ed16010

…s instead of single value

ACTIN-2295: [report] Update IhcTestInterpreter to format score bounds…

8db6d46

… as ranges

ACTIN-2295: [system] Update example patient JSON fixtures for IHC sco…

1b2d35d

…re bounds

ayaahmedhmf requested review from jbartletthmf and spdeleeuw April 10, 2026 12:05

jbartletthmf reviewed Apr 10, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Actin 2295: Replace scoreValue/scoreValuePrefix with scoreLowerBound/scoreUpperBound #1504

Actin 2295: Replace scoreValue/scoreValuePrefix with scoreLowerBound/scoreUpperBound #1504
ayaahmedhmf wants to merge 6 commits intomasterfrom
ACTIN-2295

ayaahmedhmf commented Apr 10, 2026

Uh oh!

jbartletthmf Apr 10, 2026 •

edited

Loading

Uh oh!

jbartletthmf Apr 10, 2026

Uh oh!

jbartletthmf Apr 10, 2026

Uh oh!

jbartletthmf Apr 10, 2026

Uh oh!

jbartletthmf Apr 10, 2026

Uh oh!

jbartletthmf Apr 10, 2026

Uh oh!

jbartletthmf Apr 10, 2026

Uh oh!

jbartletthmf Apr 10, 2026 •

edited

Loading

Uh oh!

jbartletthmf Apr 10, 2026

Uh oh!

jbartletthmf Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		listOf(IhcTest("HER2", scoreLowerBound = 1.0, scoreUpperBound = 1.0, scoreValueUnit = "+")),
		ReceptorType.HER2

		item = item, scoreText = scoreText, scoreLowerBound = scoreLowerBound, scoreUpperBound = scoreUpperBound,
		scoreValueUnit = scoreValueUnit, impliesPotentialIndeterminateStatus = false

		formattedLowerValue.isNotEmpty() -> "> $formattedLowerValue"
		formattedUpperValue.isNotEmpty() -> "< $formattedUpperValue"

		val formattedLowerValue = valueTest.scoreLowerBound?.let(Formats::twoDigitNumber).orEmpty()
		val formattedUpperValue = valueTest.scoreUpperBound?.let(Formats::twoDigitNumber).orEmpty()

Conversation

ayaahmedhmf commented Apr 10, 2026

Notes

Summary

Uh oh!

jbartletthmf Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

jbartletthmf Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jbartletthmf Apr 10, 2026 •

edited

Loading

jbartletthmf Apr 10, 2026 •

edited

Loading