Improve workflow visualizer functionality #1343

wenli-cai · 2025-06-11T15:33:02Z

Changed project structure and included new directories.

Two notable for added functionality

Instead of "opening" each node to provide more workflow-specific information, a side bar will open instead. This allows for much more information to be presented and will help to avoid cluttering up the main state diagram (but more basic information can still be tacked on later)
Since traces will have different traces to see how render passes affected the responsible workflows, a trace selection bar that allows you to jump between different snapshots will help in visualization.

Screen.Recording.2025-06-11.at.12.05.51.PM.mov

- This will provide a lot more information about each workflow node that's clicked, especially when it could contain a lot of information

The order of the main content being displayed was from parsing -> rendering, which would lead to complexity when adding in tabbing functionality. Having the order be Renderer -> parsing -> rendering was more clear. This commit also introduces the idea of a "trace", where we have input a json array instead of a singular json

…n trace

Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

rjrjr · 2025-06-11T16:05:32Z

...low-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/model/WorkflowNode.kt

+ *
+ * TBD what more metadata should be involved with each node, e.g. (props, states, # of render passes)
+ */
+public data class WorkflowNode(


Let's avoid reusing the term WorkflowNode, that's a very key object in the workflow runtime. Maybe just Node? Or ViewerNode?

Avoid data class in library APIs

A package with a single member is odd, unless you're very confident that there will be other classes here soon. Perhaps you should promote this class and just call it Model or NodeModel.

Be interested to hear @steve-the-edwards's take.

Could you explain (2)? Do you mean replace it with a normal class like a POJO (I guess POKO) ?

For (3), I'm thinking of taking all the mutable states in App.kt out and having them in a wrapper class, which would be inside model. Would it be better if it was also by itself, like a StatesModel?

I just mean don't use the word data. It generates a bunch of code that makes API evolution really difficult / impossible. If you need them go ahead and implement hash / equals manualy.

I don't have the context to answer WRT App.kt, and really the issue I raised is a small one. If you think more models are coming might as well keep the package.

I see, I've never looked deeply into data class, so thanks for pointing that out.

rjrjr · 2025-06-11T16:12:38Z

workflow-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/utils/JsonParser.kt

@@ -0,0 +1,48 @@
+package com.squareup.workflow1.traceviewer.utils


util rather than utils for the package name.

rjrjr · 2025-06-11T16:14:05Z

workflow-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/utils/JsonParser.kt

+  } catch (e: JsonDataException) {
+    throw JsonDataException("Failed to parse JSON: ${e.message}", e)
+  } catch (e: IOException) {
+    throw IOException("Malformed JSON: ${e.message}", e)
+  }


This wrap and rethrow behavior isn't really providing any value. I think you just want to let them fly and add an @throws line or two in the kdoc.

rjrjr · 2025-06-11T16:14:21Z

workflow-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/utils/JsonParser.kt

+ * All the caught exceptions should be handled by the caller, and appropriate UI feedback should be
+ * provided to user


Implicit, especially if you use @throws here instead

wardellbagby · 2025-06-11T16:34:36Z

workflow-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/App.kt

-  }
-}
+    // Top trace selector row
+    StateSelectTab(


Considering how many traces there are likely to be, you might consider using a dropdown instead.

Agreed, maybe even a combination of the 2, where with a dropdown you can instantly jump to a frame, and a scrolling bar could let you traverse locally. I think, though, this could be something I implement down the line as iterative improvements.

wardellbagby · 2025-06-11T16:43:10Z

...flow-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/ui/StateSelectTab.kt

+ */
+@Composable
+public fun StateSelectTab(
+  trace: List<WorkflowNode>,


Let's name this traces to make it clear that this is a list.

But also, the name traces is kinda confusing since this Composable is named StateSelectTab. Those should be consistent. I don't think trace is super clear, though. I think @rjrjr and @steve-the-edwards might have a better idea there.

Personally, I'm thinking something like snapshots? But snapshot also has a defined meaning in Workflow land so maybe not. My thought there is that since all a trace is is an export of a snapshot of a Workflow's state tree over time, naming this snapshots helps make it clear that this is effectively a list of all of the "root nodes" that the user has loaded. rootNodes is also a potential option there.

Regardless, let's name the Composable and this field the same.

Changed the naming schemes, let me know if this seems better. See:
422b94b#r2141079704

wardellbagby · 2025-06-11T16:46:35Z

...flow-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/ui/StateSelectTab.kt

+  Surface(
+    modifier = modifier
+      .padding(4.dp),
+    color = Color.White,


Nit: And this is really a nit but consider using the pre-existing MaterialTheme (that might require a new dependency on Material though...) for pulling colors like this. Just so things can be consistent going forward.

https://developer.android.com/develop/ui/compose/designsystems/material3

I'm saying this is totally optional but if you do, do it in a follow-up PR. In a perfect world, we'd be using existing Square design but someone (....me) still hasn't ported Market (our most recent Square design system) to be usable in Compose for Desktop so...

wardellbagby · 2025-06-11T16:48:46Z

...w-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/ui/WorkflowInfoPanel.kt

+      return@Column
+    }
+
+    Column(


A column with a single column child is a bit weird. These can probably be combined.

wardellbagby · 2025-06-11T16:50:10Z

workflow-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/ui/WorkflowTree.kt

+  onNodeSelect: (WorkflowNode) -> Unit,
+) {
+  var workflowNodes by remember { mutableStateOf<List<WorkflowNode>>(emptyList()) }
+  var isLoading by remember { mutableStateOf(true) }


Either put file in the key block for remember or set isLoading to true in your LaunchedEffect. Otherwise, it'll still think it's not loading when you start loading a new file.

Good catch, thanks!

wardellbagby · 2025-06-11T16:57:22Z

workflow-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/App.kt

-    var selectedFile by remember { mutableStateOf<PlatformFile?>(null) }
+  var selectedFile by remember { mutableStateOf<PlatformFile?>(null) }
+  var selectedNode by remember { mutableStateOf<WorkflowNode?>(null) }
+  var workflowTrace by remember { mutableStateOf<List<WorkflowNode>>(emptyList()) }


Kind of the same comment I make below here. snapshotIndex implies this is the index of a list of snapshots but it's really the index for workflowTrace, so those should be named similarly.

See:
422b94b#r2141079704

wenli-cai · 2025-06-11T20:55:16Z

workflow-trace-viewer/README.md

+**Frame**: Essentially a "snapshot" of the current "state" of the whole Workflow tree. It contains relevant information about the changes in workflow states and how props are passed throughout.
+
+- Note that "snapshot" and "state" are different from `snapshotState` and `State`, which are idiomatic to the Workflow library.
+


Set some standard for naming terminology in this project. Open to changing it though!

1. Made terminology and variable naming consistent (see README changes) 2. Fixed caught bugs in PR review 3. Right aligned inspection tab

Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

japplin · 2025-06-12T13:57:11Z

workflow-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/App.kt

-  }
-  val root = jsonString?.let { parseTrace(it) }
+    // Right side information panel
+    InfoPanel(selectedNode)


nit: instead of a comment here rename InfoPanel to RightInfoPanel.

japplin · 2025-06-12T13:58:58Z

...w-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/ui/WorkflowInfoPanel.kt

+      )
+    }
+
+    // based on open/close, display the node details (Column)


This code was already really easy to understand so good job! No need for the comment.

Understood!

japplin · 2025-06-12T14:00:27Z

...w-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/ui/WorkflowInfoPanel.kt

+  selectedNode: Node?,
+  modifier: Modifier = Modifier
+) {
+  // This row is ordered RTL


What is causing this row to be ordered right to left?

I think RTL was the wrong description, I've changed it now to say This row is aligned to the right of the screen.

The full-width spacer pushes everything to the end.

japplin · 2025-06-12T14:01:06Z

...w-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/ui/WorkflowInfoPanel.kt

+      return@Column
+    }
+
+    Text("only visible with a node selected")


Did you mean to leave this Text here?

Yeah just for now, it would be first to go once actual data starts coming in.

japplin · 2025-06-12T14:02:14Z

...w-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/ui/WorkflowInfoPanel.kt

+ * data class to see what information should be displayed.
+ */
+@Composable
+private fun PanelDetails(


nit: Instead of the comment perhaps NodeDetailsPanel

japplin · 2025-06-12T14:12:25Z

workflow-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/util/UploadFile.kt

    Button(
      onClick = { launcher.launch() },
      modifier = Modifier
-        .align(Alignment.BottomEnd),
+        .align(Alignment.BottomStart),


I noticed that there is a comment in your App method saying that this is a bottom left button, but if you instead passed set the alignment of the button in the modifier you're already passing in then the comment wouldn't be required. I also think it's more typical that the parent would specify the position of a button rather than the button itself specifying where it is on screen.

Makes sense, also a lot more self-explanatory when looking at parent containers. Fixed

japplin · 2025-06-12T14:13:21Z

workflow-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/util/UploadFile.kt

@@ -23,6 +23,7 @@ import io.github.vinceglb.filekit.dialogs.compose.rememberFilePickerLauncher
 */
 @Composable
 public fun UploadFile(
+  onReset: () -> Unit,


Can onReset be an implicit step executed every time onFileSelect is called instead of a separate param?

japplin · 2025-06-12T14:15:09Z

workflow-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/util/JsonParser.kt

+ * Parses the data from the given file and initiates the workflow tree
+ */
+public suspend fun fetchTrace(
+  file: PlatformFile?,


Why allow a nullable PlatformFile?

Mistake, fixed now.

japplin · 2025-06-12T14:17:24Z

workflow-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/util/JsonParser.kt

+/**
+ * Parses the data from the given file and initiates the workflow tree
+ */
+public suspend fun fetchTrace(


This method isn't doing much, and it's comment is saying that it parses the file which is what the parseTrace method is doing. Also parseTrace throws exceptions which is commented on parseTrace method but not here if they were one function that wouldn't be an issue.

True, I've combined them now

japplin · 2025-06-12T14:18:19Z

workflow-trace-viewer/src/jvmMain/kotlin/com/squareup/workflow1/traceviewer/util/JsonParser.kt

+
+  val workflowList = Types.newParameterizedType(List::class.java, Node::class.java)
+  val workflowAdapter: JsonAdapter<List<Node>> = moshi.adapter(workflowList)
+  val root = workflowAdapter.fromJson(json) ?: emptyList()


Let's return null on an error loading the list and then show an error message to the user that loading the trace was unsuccessful. That would also allow us to handle all of the exceptions here instead of throwing them.

1. Clarified component positions with modifiers 2. Removed unnecessary comments 3. Simplified JsonParser

Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

wenli-cai added 8 commits June 11, 2025 09:35

Include a detailed view of node via side panel

b30a565

- This will provide a lot more information about each workflow node that's clicked, especially when it could contain a lot of information

Clean up project structure

42eeef9

Clean up project structure

7db8039

Change file names.

6dea47e

Add scroll bar functionality to allow indexing different states withi…

d85d0ff

…n trace

Bugfix with switching between different files and UI change for top row

bdbc65f

Add missing Kdocs

6831535

wenli-cai requested review from zach-klippenstein and a team as code owners June 11, 2025 15:33

wenli-cai assigned wenli-cai and steve-the-edwards and unassigned wenli-cai Jun 11, 2025

Apply changes from apiDump

d2626f2

Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

rjrjr reviewed Jun 11, 2025

View reviewed changes

wardellbagby reviewed Jun 11, 2025

View reviewed changes

wenli-cai commented Jun 11, 2025

View reviewed changes

Fix for PR comments

6bdc3d4

1. Made terminology and variable naming consistent (see README changes) 2. Fixed caught bugs in PR review 3. Right aligned inspection tab

wenli-cai force-pushed the wenli/workflow-viewer-open-nodes branch from e55dc85 to 6bdc3d4 Compare June 12, 2025 01:32

Apply changes from apiDump

1a6d868

Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

japplin reviewed Jun 12, 2025

View reviewed changes

wenli-cai and others added 3 commits June 14, 2025 12:11

More fix for PR comments

4255a8f

1. Clarified component positions with modifiers 2. Removed unnecessary comments 3. Simplified JsonParser

Improve error handling

ebe259b

Apply changes from apiDump

1beba35

Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

		@@ -0,0 +1,48 @@
		package com.squareup.workflow1.traceviewer.utils

		* All the caught exceptions should be handled by the caller, and appropriate UI feedback should be
		* provided to user

		Frame: Essentially a "snapshot" of the current "state" of the whole Workflow tree. It contains relevant information about the changes in workflow states and how props are passed throughout.

		- Note that "snapshot" and "state" are different from `snapshotState` and `State`, which are idiomatic to the Workflow library.

Improve workflow visualizer functionality #1343

Are you sure you want to change the base?

Improve workflow visualizer functionality #1343

Conversation

wenli-cai commented Jun 11, 2025 • edited by rjrjr Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wenli-cai Jun 11, 2025 • 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!

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!

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!

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

wenli-cai commented Jun 11, 2025 •

edited by rjrjr

Loading

wenli-cai Jun 11, 2025 •

edited

Loading

japplin Jun 12, 2025 •

edited

Loading