Fixed bug in short path of AbstractFilteredListCell.computeValueAndEvent.

cell/src/commonMain/kotlin/world/phantasmal/cell/ChangeObserver.kt

@@ -14,4 +14,7 @@ open class ChangeEvent<out T>(
     val value: T,
 ) {
     operator fun component1() = value
+
+    override fun toString(): String =
+        "ChangeEvent($value)"
 }

cell/src/commonMain/kotlin/world/phantasmal/cell/list/AbstractFilteredListCell.kt

@@ -40,6 +40,21 @@ abstract class AbstractFilteredListCell<E>(
 
     private fun computeValueAndEvent() {
         if (!valid) {
+            // Reuse the same list of changes during a mutation.
+            val event = _changeEvent
+            val filteredChanges: MutableList<ListChange<E>>
+
+            if (event == null || changesMutationId != MutationManager.currentMutationId) {
+                changesMutationId = MutationManager.currentMutationId
+                listChangeIndex = 0
+                filteredChanges = mutableListOf()
+                _changeEvent = ListChangeEvent(elements, filteredChanges)
+            } else {
+                // This cast is safe because we know we always instantiate our change event
+                // with a mutable list.
+                filteredChanges = unsafeCast(event.changes)
+            }
+
             val hasDependents = dependents.isNotEmpty()
 
             if (predicateInvalidated || !hasDependents) {
@@ -49,26 +64,10 @@ abstract class AbstractFilteredListCell<E>(
                 ignoreOtherChanges()
                 recompute()
 
-                _changeEvent = ListChangeEvent(
-                    elements,
-                    listOf(ListChange(index = 0, prevSize = removed.size, removed, elements)),
+                filteredChanges.add(
+                    ListChange(index = 0, prevSize = removed.size, removed, elements),
                 )
             } else {
-                // Reuse the same list of changes during a mutation.
-                val event = _changeEvent
-                val filteredChanges: MutableList<ListChange<E>>
-
-                if (event == null || changesMutationId != MutationManager.currentMutationId) {
-                    changesMutationId = MutationManager.currentMutationId
-                    listChangeIndex = 0
-                    filteredChanges = mutableListOf()
-                    _changeEvent = ListChangeEvent(elements, filteredChanges)
-                } else {
-                    // This cast is safe because we know we always instantiate our change event
-                    // with a mutable list.
-                    filteredChanges = unsafeCast(event.changes)
-                }
-
                 val listChangeEvent = list.changeEvent
 
                 if (listInvalidated && listChangeEvent != null) {

cell/src/commonMain/kotlin/world/phantasmal/cell/list/ListChangeObserver.kt

@@ -5,7 +5,10 @@ import world.phantasmal.cell.ChangeEvent
 class ListChangeEvent<out E>(
     value: List<E>,
     val changes: List<ListChange<E>>,
-) : ChangeEvent<List<E>>(value)
+) : ChangeEvent<List<E>>(value) {
+    override fun toString(): String =
+        "ListChangeEvent($value, changes=$changes)"
+}
 
 /**
  * Represents a structural change to a list cell. E.g. an element is inserted or removed.
@@ -20,6 +23,9 @@ class ListChange<out E>(
 ) {
     /** True when this change resulted in the removal of all elements from the list. */
     val allRemoved: Boolean get() = removed.size == prevSize
+
+    override fun toString(): String =
+        "ListChange(index=$index, prevSize=$prevSize, removed=$removed, inserted=$inserted)"
 }
 
 typealias ListChangeObserver<E> = (ListChangeEvent<E>) -> Unit

cell/src/commonTest/kotlin/world/phantasmal/cell/list/AbstractFilteredListCellTests.kt

@@ -3,12 +3,9 @@ package world.phantasmal.cell.list
 import world.phantasmal.cell.Cell
 import world.phantasmal.cell.ImmutableCell
 import world.phantasmal.cell.SimpleCell
+import world.phantasmal.cell.mutate
 import world.phantasmal.cell.test.CellTestSuite
-import kotlin.test.Test
-import kotlin.test.assertEquals
-import kotlin.test.assertNotNull
-import kotlin.test.assertNull
-import kotlin.test.assertTrue
+import kotlin.test.*
 
 /**
  * Tests that apply to all filtered list implementations.
@@ -307,4 +304,63 @@ interface AbstractFilteredListCellTests : CellTestSuite {
             assertTrue(c.inserted.isEmpty())
         }
     }
+
+    /**
+     * This tests the short-circuit path where a filtered list's predicate changes.
+     */
+    @Test
+    fun dependent_filtered_list_value_changes_and_emits_correctly_when_predicate_changes() = test {
+        val list = mutableListCell(1, 2, 3, 4, 5, 6)
+        val predicate: SimpleCell<(Int) -> Boolean> = SimpleCell { it % 2 == 0 }
+        val filteredList = createFilteredListCell(list, predicate)
+        val dependentList = filteredList.filtered { true }
+
+        var event: ListChangeEvent<Int>? = null
+
+        disposer.add(dependentList.observeListChange {
+            assertNull(event)
+            event = it
+        })
+
+        assertEquals(listOf(2, 4, 6), dependentList.value)
+
+        mutate {
+            // Trigger long path.
+            list.add(10)
+            dependentList.value
+
+            // Trigger long path again.
+            list.add(20)
+            dependentList.value
+
+            // Trigger short path.
+            predicate.value = { it % 2 != 0 }
+        }
+
+        assertEquals(listOf(1, 3, 5), dependentList.value)
+
+        val e = event
+        assertNotNull(e)
+        assertEquals(listOf(1, 3, 5), e.value)
+
+        assertEquals(3, e.changes.size)
+
+        val c0 = e.changes[0]
+        assertEquals(3, c0.index)
+        assertEquals(3, c0.prevSize)
+        assertEquals(emptyList(), c0.removed)
+        assertEquals(listOf(10), c0.inserted)
+
+        val c1 = e.changes[1]
+        assertEquals(4, c1.index)
+        assertEquals(4, c1.prevSize)
+        assertEquals(emptyList(), c1.removed)
+        assertEquals(listOf(20), c1.inserted)
+
+        val c2 = e.changes[2]
+        assertEquals(0, c2.index)
+        assertEquals(5, c2.prevSize)
+        assertEquals(listOf(2, 4, 6, 10, 20), c2.removed)
+        assertEquals(listOf(1, 3, 5), c2.inserted)
+    }
 }