Add some comments.

Author: zach-klippenstein

workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/compose/ComposeChildNode.kt

@@ -5,23 +5,43 @@ import androidx.compose.runtime.currentCompositeKeyHash
 import com.squareup.workflow1.ActionProcessingResult
 import com.squareup.workflow1.TreeSnapshot
 import com.squareup.workflow1.Workflow
+import com.squareup.workflow1.WorkflowIdentifier
+import com.squareup.workflow1.compose.ComposeWorkflow
+import com.squareup.workflow1.internal.AbstractWorkflowNode
 import com.squareup.workflow1.internal.WorkflowNodeId
 import kotlinx.coroutines.selects.SelectBuilder
 
+/**
+ * Represents a workflow inside a [ComposeWorkflowNodeAdapter], either another
+ * [ComposeWorkflow]/[ComposeWorkflowChildNode], or a
+ * [Workflow]/[TraditionalWorkflowAdapterChildNode].
+ */
 internal interface ComposeChildNode<PropsT, OutputT, RenderingT> {
 
+  /** See [AbstractWorkflowNode.id]. */
   val id: WorkflowNodeId
 
+  /**
+   * Called during workflow render passes to produce the rendering for this workflow.
+   *
+   * The compose analog to [AbstractWorkflowNode.render].
+   */
   @Composable fun produceRendering(
     workflow: Workflow<PropsT, OutputT, RenderingT>,
     props: PropsT
   ): RenderingT
 
+  /** See [AbstractWorkflowNode.snapshot]. */
   fun snapshot(): TreeSnapshot
 
+  /** See [AbstractWorkflowNode.onNextAction]. */
   fun onNextAction(selector: SelectBuilder<ActionProcessingResult>): Boolean
 }
 
+/**
+ * Returns a stable key identifying a call to [ComposeWorkflowChildNode.renderChild] in the
+ * composition, suitable for use with [WorkflowIdentifier].
+ */
 @OptIn(ExperimentalStdlibApi::class)
 @Composable
 internal fun rememberChildRenderKey(): String {

workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/compose/ComposeWorkflowChildNode.kt

@@ -45,7 +45,8 @@ import kotlin.coroutines.CoroutineContext
 private const val OUTPUT_QUEUE_LIMIT = 1_000
 
 /**
- * Representation and implementation of a single [ComposeWorkflow].
+ * Representation and implementation of a single [ComposeWorkflow] inside a
+ * [ComposeWorkflowNodeAdapter].
  */
 @OptIn(WorkflowExperimentalApi::class)
 internal class ComposeWorkflowChildNode<PropsT, OutputT, RenderingT>(
@@ -77,8 +78,10 @@ internal class ComposeWorkflowChildNode<PropsT, OutputT, RenderingT>(
   override val sessionId: Long = idCounter.createId()
 
   private var lastProps by mutableStateOf(initialProps)
-  private val saveableStateRegistry: SaveableStateRegistry
+
+  /** This does not need to be a snapshot state object, it's only set again by [snapshot]. */
   private var snapshotCache = snapshot?.childTreeSnapshots
+  private val saveableStateRegistry: SaveableStateRegistry
   private val childNodes = mutableVectorOf<ComposeChildNode<*, *, *>>()
 
   private val outputsChannel = Channel<OutputT>(capacity = OUTPUT_QUEUE_LIMIT)
@@ -121,7 +124,7 @@ internal class ComposeWorkflowChildNode<PropsT, OutputT, RenderingT>(
 
     val workflowSnapshot = snapshot?.workflowSnapshot
     var restoredRegistry: SaveableStateRegistry? = null
-    // Don't care about this return value, our state is separate.
+    // Don't care if the interceptor returns a state value, our state is stored in the composition.
     interceptor.onInitialState(
       props = initialProps,
       snapshot = workflowSnapshot,
@@ -273,7 +276,7 @@ internal class ComposeWorkflowChildNode<PropsT, OutputT, RenderingT>(
     } else {
       // We need to be able to explicitly request recomposition of this composable when an action
       // cascade results in this child changing state because traditional workflows don't know
-      // about Compose.
+      // about Compose. See comment in acceptChildActionResult for more info.
       val recomposeScope = currentRecomposeScope
       remember {
         TraditionalWorkflowAdapterChildNode(
@@ -288,6 +291,11 @@ internal class ComposeWorkflowChildNode<PropsT, OutputT, RenderingT>(
           interceptor = interceptor,
           idCounter = idCounter,
           acceptChildActionResult = { actionApplied ->
+            // If this child needs to be re-rendered on the next render pass and there are no other
+            // state changes in the compose runtime during this action cascade, if we don't
+            // explicitly invalidate the recompose scope then the recomposer will think it doesn't
+            // have anything to do and not recompose us, which means we wouldn't have a chance to
+            // re-render the traditional workflow.
             if (actionApplied.stateChanged) {
               recomposeScope.invalidate()
             }
@@ -334,20 +342,7 @@ internal class ComposeWorkflowChildNode<PropsT, OutputT, RenderingT>(
   ): ActionProcessingResult {
     log("handling child output: $appliedActionFromChild")
     val outputFromChild = appliedActionFromChild.output
-    // if (outputFromChild == null || onOutput == null) {
-    //   // The child didn't actually emit anything or we don't care, so we don't need to
-    //   // propagate anything to the parent. We halt the action cascade by simply returning
-    //   // here without calling emitAppliedActionToParent.
-    //   //
-    //   // NOTE: SubtreeManager has an additional case for PARTIAL_TREE_RENDERING, but we
-    //   // can just assume that using ComposeWorkflow at all implies that optimization.
-    //   //
-    //   // If our child state changed, we need to report that ours did too, as per the
-    //   // comment in StatefulWorkflowNode.applyAction.
-    //   return appliedActionFromChild.withOutput(null)
-    // }
-
-    // The child DID emit an output, so we need to call our handler, which will zero or
+    // The child emitted an output, so we need to call our handler, which will zero or
     // more of two things: (1) change our state, (2) emit an output.
 
     // If this workflow calls emitOutput while running child output handler, we don't want

workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/compose/ComposeWorkflowState.kt

@@ -1,6 +1,11 @@
 package com.squareup.workflow1.internal.compose
 
+import com.squareup.workflow1.WorkflowInterceptor
+import com.squareup.workflow1.compose.ComposeWorkflow
+
 /**
- * Fake state object passed to WorkflowInterceptors.
+ * Fake state object passed to [WorkflowInterceptor]s as the state for [ComposeWorkflow]s.
+ *
+ * If we need interceptors to be able to identify compose workflows, we can just make this public.
  */
 internal object ComposeWorkflowState

workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/compose/RecomposerDriver.kt

@@ -100,6 +100,10 @@ internal class RecomposerDriver(
     }
 
   suspend fun runRecomposeAndApplyChanges() {
+    // Note: This context is _only_ used for the actual recompose loop. Everything inside the
+    // composition (rememberCoroutineScope, LaunchedEffects, etc) will NOT see these, and will see
+    // only whatever context was passed into the Recomposer's constructor (plus the stuff it adds
+    // to that context itself, like the BroadcastFrameClock).
     withContext(dispatcher + frameClock) {
       recomposer.runRecomposeAndApplyChanges()
     }

workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/compose/TraditionalWorkflowAdapterChildNode.kt

@@ -17,6 +17,10 @@ import com.squareup.workflow1.internal.createWorkflowNode
 import kotlinx.coroutines.selects.SelectBuilder
 import kotlin.coroutines.CoroutineContext
 
+/**
+ * Entry point back into the Workflow runtime from a Compose runtime (i.e. a
+ * [ComposeWorkflowNodeAdapter]).
+ */
 internal class TraditionalWorkflowAdapterChildNode<PropsT, OutputT, RenderingT>(
   id: WorkflowNodeId,
   workflow: Workflow<PropsT, OutputT, RenderingT>,

workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/compose/coroutines/Synchronization.kt

@@ -1,13 +1,16 @@
 package com.squareup.workflow1.internal.compose.coroutines
 
-import kotlinx.coroutines.channels.Channel
+import kotlinx.coroutines.channels.SendChannel
 
 internal expect class Lock()
 
 internal expect inline fun <R> Lock.withLock(block: () -> R): R
 
-// TODO pull into separate file
-internal fun <T> Channel<T>.requireSend(element: T) {
+/**
+ * Tries to send [element] to this channel and throws an [IllegalStateException] if the channel is
+ * full or closed.
+ */
+internal fun <T> SendChannel<T>.requireSend(element: T) {
   val result = trySend(element)
   if (result.isClosed) {
     throw IllegalStateException(