Feature/ssot refactor #150

interim17 · 2025-11-10T19:08:57Z

Problem

Closes #97 Closes #101

Abandoned some pieces of the original SSOT branch, and deferred some.
Part two of this is #151

Solution

Recipe strings ===> RecipeData which is the default recipe and an edits object.
updateRecipeString and updateRecipOb ====> editRecipe

This is a streamlined version of old branch which got out of hand.
Did not try to refactor App.tsx, did not try to consolidate recipe data and input options, did not try to get rid of callback pattern in startPacking which is a reason to avoid testing. I think we should streamline that before putting effort into writing tests for it.

RecipeData and RecipeManifest are redundant, the plan is to use something like RecipeData and RecipeMetadata. Getting that `00% finished is out of scope. Here the focus is on getting rid of recipe strings.

…hub.com/mesoscope/cellpack-client into feature/ssot-refactor

github-actions · 2025-11-10T19:09:20Z

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	20.38%	367 / 1800
🔵	Statements	20.38%	367 / 1800
🔵	Functions	40%	24 / 60
🔵	Branches	71.05%	81 / 114

File Coverage

File	Stmts	% Branch	% Funcs	% Lines	Uncovered Lines
Changed Files
src/components/Dropdown/index.tsx	0%	0%	0%	0%	1-2, 12-17, 19-26, 28, 30
src/components/GradientInput/index.tsx	0%	0%	0%	0%	1, 7-9, 16, 18-22, 24-28, 30, 32-40, 42-46, 48-51, 53-86, 88, 90, 92
src/components/InputSwitch/index.tsx	0%	0%	0%	0%	1-2, 9-11, 27-28, 30-33, 37, 40-42, 44-55, 58, 61-63, 65-68, 70-74, 76-82, 84-110, 112, 114-133, 135, 137-144, 146, 148, 150-162, 164-166, 168
src/components/JSONViewer/index.tsx	0%	0%	0%	0%	1-2, 8-9, 17-18, 20-22, 25, 27, 29-41, 44-54, 56-60, 64-67, 69-92, 94, 96
src/components/PackingInput/index.tsx	0%	0%	0%	0%	1-2, 13-17, 27-32, 34-37, 39-42, 45-47, 49-51, 53-55, 57-79, 81, 83
src/state/store.ts	0%	0%	0%	0%	1, 45, 47-54, 56-58, 60-68, 70-79, 81-82, 84-104, 106-108, 110-112, 114-117, 119-127, 129-140, 142-153, 156-158, 160, 162-167, 169-178, 181-184, 187-193, 196-199, 201-202, 204-210, 212-226, 228-242, 244-245, 249-256, 258-261, 263-268, 270-274, 276-280, 282, 284-287, 289-292, 294-297, 299-302, 304-307, 309-320, 323-338
src/state/utils.ts	0%	0%	0%	0%	1, 12-18
src/types/index.ts	100%	100%	100%	100%
src/utils/firebase.ts	26.76%	62.5%	25%	26.76%	35-36, 41-42, 76-78, 81-91, 94-98, 101-109, 113-126, 129-134, 137-139, 142-166, 170-176, 179-184, 186-202, 216-217, 220-230, 232-241, 243, 245-249, 251-256

Generated in workflow #163

github-actions · 2025-11-10T19:09:32Z

PR Preview Action v1.6.2
Preview removed because the pull request was closed.
2025-11-13 19:37 UTC

interim17 · 2025-11-10T19:38:31Z

src/components/JSONViewer/index.tsx

-    content: string;
-    isEditable: boolean;
-    onChange: (value: string) => void;
+    content?: ViewableRecipe;


The onChange and isEditable props were unused, so I removed them.

The recipe content doesn't come down as string anymore but the selector can return undefined, so made it optional with the truthy check on ln21-22 retained.

interim17 · 2025-11-11T03:40:27Z

src/utils/firebase.ts

+// TODO get data and metadata separately
+const getRecipeDataFromFirebase = async (recipeId: string): Promise<RecipeData> => {
+    const defaultRecipeData = await getFirebaseRecipe(recipeId);
+    return {
+        recipeId,
+        defaultRecipeData,
+        edits: {}
+    }
+}
+
+const getRecipesFromFirebase = async (): Promise<Dictionary<RecipeManifest>> => {
+    const docs = await getAllDocsFromCollection(FIRESTORE_COLLECTIONS.PACKING_INPUTS);
    const inputsDict: Dictionary<RecipeManifest> = {};
    for (const doc of docs) {
        const displayName = doc[FIRESTORE_FIELDS.NAME];
        const config = doc[FIRESTORE_FIELDS.CONFIG];
-        const recipe = doc[FIRESTORE_FIELDS.RECIPE];
-        const editableFields = await getEditableFieldsList(
-            doc[FIRESTORE_FIELDS.EDITABLE_FIELDS] || []
-        );
-        const result = doc[FIRESTORE_FIELDS.RESULT_PATH] || "";
-        if (config && recipe) {
-            inputsDict[recipe] = {
-                [FIRESTORE_FIELDS.NAME]: displayName,
-                [FIRESTORE_FIELDS.CONFIG]: config,
-                [FIRESTORE_FIELDS.RECIPE]: recipe,
-                [FIRESTORE_FIELDS.EDITABLE_FIELDS]: editableFields,
+        const recipeId = doc[FIRESTORE_FIELDS.RECIPE];
+
+        if (displayName && config && recipeId) {
+            const editableFields = await getEditableFieldsList(doc[FIRESTORE_FIELDS.EDITABLE_FIELDS] || []);
+            const recipe = await getFirebaseRecipe(recipeId);
+            const result = doc[FIRESTORE_FIELDS.RESULT_PATH] || "";
+            inputsDict[recipeId] = {
+                recipeId: recipeId,
+                configId: config,
+                displayName,
+                editableFields: editableFields ?? [],
+                defaultRecipeData: recipe,
+                edits: {},


interim17 · 2025-11-11T03:40:38Z

src/types/index.ts

+// TODO further refine difference between data and metadata, decide how
+// to store them, and when to load them.
+
+export interface RecipeData {
+    recipeId: string;
+    defaultRecipeData: ViewableRecipe;
+    edits: Record<string, string | number>;
+}
+
+export interface RecipeManifest {
+    recipeId: string;
+    configId: string;
+    displayName: string;
+    editableFields: EditableField[];
    defaultResultPath?: string;
-    editable_fields?: EditableField[];
-};
+    defaultRecipeData: ViewableRecipe;
+    edits: Record<string, string | number>;
+}
+


Copilot

Pull Request Overview

This PR refactors the recipe data management to use a single source of truth (SSOT) pattern by replacing recipe strings with a RecipeData structure that separates default recipe data from user edits.

Key changes:

Replaced string-based recipe storage with RecipeData objects containing defaultRecipeData and edits
Consolidated updateRecipeString and updateRecipeObj into a single editRecipe function
Updated type definitions to use RecipeManifest and RecipeData interfaces with clearer property names

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/types/index.ts	Defined new `RecipeData` and updated `RecipeManifest` interfaces with clearer property names
src/utils/firebase.ts	Renamed functions and updated to return new data structures with recipe objects instead of strings
src/state/utils.ts	Added utility function to build current recipe from defaults and edits
src/state/store.ts	Refactored state management to use `RecipeData` structure and simplified edit operations
src/components/PackingInput/index.tsx	Updated to use recipe objects instead of strings
src/components/JSONViewer/index.tsx	Modified to accept objects directly instead of parsing strings
src/components/InputSwitch/index.tsx	Updated to use new `editRecipe` function and recipe structure
src/components/GradientInput/index.tsx	Simplified to use new `editRecipe` function
src/components/Dropdown/index.tsx	Updated property references to match new interface names

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/components/GradientInput/index.tsx

src/state/store.ts

+
+export const useCurrentRecipeObject = () => {
+    const recipe = useCurrentRecipeData();
+   return recipe ? buildCurrentRecipeObject(recipe) : undefined;


meganrm · 2025-11-11T18:12:37Z

src/state/store.ts

        },

+
        getCurrentValue: (path) => {


could have a clearer name of what the value is

getRecipeValueAtPath?
getValueForRecipeField?

meganrm · 2025-11-11T18:13:07Z

src/state/store.ts

+            return undefined;
+        },
+
+        getOriginalValue: (path) => {


also could have a clearer name

src/state/utils.ts

…nto feature/ssot-refactor

ascibisz · 2025-11-12T00:10:13Z

I got this error when I tried to go to the preview link? It doesn't happen on the preview link for #151 so I'm not too concerned, but I just want to make sure we don't get this error in main by merging this without #151

interim17 · 2025-11-12T17:22:42Z

I got this error when I tried to go to the preview link? It doesn't happen on the preview link for #151 so I'm not too concerned, but I just want to make sure we don't get this error in main by merging this without #151

@ascibisz Megan and I were workshopping some changes over the past 24 hours and these errors should be resolved momentarily!

ascibisz · 2025-11-12T18:57:05Z

src/state/utils.ts

+import { set as lodashSet } from "lodash-es";
+import { RecipeData, ViewableRecipe } from "../types";
+
+export const buildRecipeObject = (recipe: RecipeData): ViewableRecipe => {


could you add a comment describing what this function does?

src/state/store.ts

ascibisz

Left some very minor comments, but it looks good and works well for me!

I refrained from commenting on your edits to types/index.ts and firebase.ts because you cover all of the improvements I would like with #151 !

ascibisz · 2025-11-12T19:06:26Z

src/state/store.ts

+                if (typeof editedValue === "string" || typeof editedValue === "number") {
+                    return editedValue;
+                }
                return undefined;


I can't think of a time where we'd actually hit this return undefined; case, so perhaps it doesn't really matter, but I feel like we would want to try to continue on to trying to get the default value rather than just returning undefined?

meganrm · 2025-11-12T22:16:48Z

src/state/utils.ts

+ * @returns A deep-cloned ViewableRecipe with all edits applied.
+ * Typically represents the "current" version of the selected recipe in state.
+ */
+export const buildRecipeObject = (recipe: RecipeData): ViewableRecipe => {


sorta nitty: since this is a utility, I wouldn't be sending in such a typed object.

my suggestion:
mergeChangesIntoNestedObject(originalObject, edits) that returns the same type as the originalObject

or mergeNestedObjects

I think this also makes it clearer what the function is doing without having to read comments

or: applyChangesToNestedObject

Ok changed it.

…nto feature/ssot-refactor

interim17 added 2 commits November 10, 2025 11:06

use object instead of string as single source of truth

e07c9c6

Merge branch 'feature/show-precomputed-results-tweaks' of https://git…

2decf90

…hub.com/mesoscope/cellpack-client into feature/ssot-refactor

interim17 mentioned this pull request Nov 10, 2025

Feature/show precomputed results tweaks #146

Merged

use displayName

3a02c07

interim17 commented Nov 10, 2025

View reviewed changes

interim17 mentioned this pull request Nov 11, 2025

RecipeMetadata and prefetching optimizations #151

Merged

interim17 commented Nov 11, 2025

View reviewed changes

interim17 requested a review from Copilot November 11, 2025 03:41

Copilot AI reviewed Nov 11, 2025

View reviewed changes

src/components/GradientInput/index.tsx Show resolved Hide resolved

src/state/store.ts Outdated

export const useCurrentRecipeObject = () => {

const recipe = useCurrentRecipeData();

return recipe ? buildCurrentRecipeObject(recipe) : undefined;

This comment was marked as resolved.

Sign in to view

add second call of editRecipe for gradient value

ed152a6

interim17 marked this pull request as ready for review November 11, 2025 03:53

interim17 requested review from ascibisz, meganrm and rugeli November 11, 2025 03:54

meganrm reviewed Nov 11, 2025

View reviewed changes

src/state/store.ts

return undefined;

},

getOriginalValue: (path) => {

Copy link

Contributor

meganrm Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also could have a clearer name

meganrm reviewed Nov 11, 2025

View reviewed changes

src/state/utils.ts Outdated Show resolved Hide resolved

move building current recipe object from util into selector

9de4abf

interim17 requested a review from meganrm November 11, 2025 20:06

Base automatically changed from feature/show-precomputed-results-tweaks to main November 11, 2025 21:49

Merge branch 'main' of https://github.com/mesoscope/cellpack-client i…

656b14a

…nto feature/ssot-refactor

revert to using utility to build recipe objects

faf266d

ascibisz reviewed Nov 12, 2025

View reviewed changes

src/state/store.ts Outdated Show resolved Hide resolved

add documenting comment to buildRecipeObject

8f5f11a

ascibisz approved these changes Nov 12, 2025

View reviewed changes

meganrm reviewed Nov 12, 2025

View reviewed changes

interim17 added 2 commits November 12, 2025 19:46

Merge branch 'main' of https://github.com/mesoscope/cellpack-client i…

98e37c5

…nto feature/ssot-refactor

rename and adjust typing of recipe object utility

f6f6dca

interim17 requested a review from meganrm November 13, 2025 04:09

meganrm approved these changes Nov 13, 2025

View reviewed changes

Merge branch 'main' of https://github.com/AllenCell/cellpack-client i…

67eecdf

…nto feature/ssot-refactor

meganrm merged commit 8698017 into main Nov 13, 2025
2 checks passed

meganrm deleted the feature/ssot-refactor branch November 13, 2025 19:37

Feature/ssot refactor #150

Feature/ssot refactor #150

Uh oh!

Conversation

interim17 commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Uh oh!

github-actions bot commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report

Uh oh!

github-actions bot commented Nov 10, 2025 • edited 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

This comment was marked as resolved.

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!

Uh oh!

ascibisz commented Nov 12, 2025

Uh oh!

interim17 commented Nov 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ascibisz left a comment

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

interim17 commented Nov 10, 2025 •

edited

Loading

github-actions bot commented Nov 10, 2025 •

edited

Loading

github-actions bot commented Nov 10, 2025 •

edited

Loading