DRAFT: Feature/ssot #134

interim17 · 2025-11-05T21:56:36Z

Recipe strings are gone: recipe objects are in.
Local state and packing functionality lifted out of App -> packingService and store
Lot's of props removed in favor of calling store hooks in many components.
A number of components check for their own data and return null rather than handling conditional rendering in parent.
Tests added for store and packingService
Dropdown: defaultValue removed, value provided by prop.
Changed updatedRecipeString and updateRecipObj -> editRecipe

No storing original and default recipes in state: store default and updates, build the "current" on the fly.
Packing logic moved into store version of startPacking to get rid of call back pattern.
Firebase util getPackingInputsDict refactored as getRecipesFromFirebase, returns RecipeManifest data shape.
PackingData put in RecipeState

Redundancy of recipes and inputOptions collapsed and consolidated. PackingInputs type removed, RecipeManifest and PackingManifest added... naming?

Feedback I want:
naming good?
state branches?
too much in state? just right?

Could this be broken up?
Yes but its extra work to make stepping stone PRs that use the wrong or non-final shape of the store.
Changes to dropdown/value could be pulled out.

Problem

Closes #97 Closes #101

Solution

What I/we did to solve this problem

with @pairperson1

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update
This change requires updated or new tests

Change summary:

Tidy, well formulated commit message
Another great commit message
Something else I/we did

Steps to Verify:

A setup step / beginning state
What to do next
Any other instructions
Expected behavior
Suggestions for testing

Screenshots (optional):

Show-n-tell images/animations here

Keyfiles (delete if not relevant):

main file/entry point
other important file

Thanks for contributing!

…as single source of truth

…nto feature/ssot

github-actions · 2025-11-05T21:56:57Z

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	37.83%	614 / 1623
🔵	Statements	37.83%	614 / 1623
🔵	Functions	43.67%	38 / 87
🔵	Branches	75.29%	128 / 170

File Coverage

File	Stmts	% Branch	% Funcs	% Lines	Uncovered Lines
Changed Files
src/App.tsx	0%	0%	0%	0%	1-5, 7-8, 10, 12-16, 18-22, 24-37, 39, 41
src/components/Dropdown/index.tsx	0%	0%	0%	0%	1, 11-18, 20-27, 29, 31
src/components/ErrorLogs/index.tsx	0%	0%	0%	0%	1-5, 7-9, 11-13, 15-17, 19-21, 23-29, 31-35, 37, 39
src/components/GradientInput/index.tsx	0%	0%	0%	0%	1, 7-9, 16, 18-22, 24-28, 30, 32-35, 38-43, 45-50, 52-55, 57-99, 101, 103, 105
src/components/InputSwitch/index.tsx	0%	0%	0%	0%	1-2, 9-11, 27-40, 42-45, 49, 52-54, 56-66, 69, 72-74, 76-80, 82-87, 89-114, 116, 118-137, 139, 141-148, 150, 152, 154-166, 168-170, 172
src/components/JSONViewer/index.tsx	0%	0%	0%	0%	1-2, 8-11, 13-15, 17, 19-21, 23, 26, 28, 30-42, 45-55, 57-61, 65-68, 70-93, 95, 97
src/components/PackingInput/index.tsx	0%	0%	0%	0%	1-2, 10-14, 16-22, 24, 27-34, 36-38, 40-42, 44-63, 65, 67
src/components/RecipeForm/index.tsx	0%	0%	0%	0%	1-3, 8, 14-17, 19-39, 41-48, 50, 52, 54-55
src/components/StatusBar/index.tsx	0%	0%	0%	0%	1-7, 9-12, 14-18, 20, 22-24, 26-35, 37-45, 47, 49-50, 52, 54
src/components/Viewer/index.tsx	0%	0%	0%	0%	1-2, 5-13, 15, 17
src/state/constants.ts	100%	100%	100%	100%
src/state/store.ts	95.67%	76.19%	36.36%	95.67%	78, 123-124, 220-223
src/state/utils.ts	75%	50%	100%	75%	10-11
src/types/index.ts	0%	100%	100%	0%	8, 16, 34, 77, 113, 127, 148, 164
src/utils/firebase.ts	33.33%	62.5%	25%	33.33%	34-35, 40-41, 75-77, 80-85, 88-92, 95-100, 104-106, 109-117, 120-122, 125-127, 130-149, 152-157, 159-173, 187-188, 191-195, 197-204, 206, 208-210, 212-215
src/utils/packingService.ts	100%	93.33%	100%	100%

Generated in workflow #110

github-actions · 2025-11-05T21:57:08Z

PR Preview Action v1.6.2
🚀 View preview at https://AllenCell.github.io/cellpack-client/pr-preview/pr-134/
Built to branch `gh-pages` at 2025-11-05 21:57 UTC. Preview will be ready when the GitHub Pages deployment is complete.

interim17 · 2025-11-05T22:21:32Z

src/state/store.ts


-                for (const [path, value] of Object.entries(updates)) {
-                    lodashSet(obj, path, value);
+        getCurrentValue: (path) => {


getRecipeSettingValue

is path a series of keys?

interim17 · 2025-11-05T22:21:55Z

src/state/store.ts

+        },

-        getCurrentValue: (path) => {
+        getOriginalValue: (path) => {


same here, recipe setting value

meganrm · 2025-11-05T23:41:04Z

src/components/Dropdown/index.tsx

+    const { placeholder, options, value, onChange } = props;
+    const selectOptions = Object.entries(options).map(([key, value]) => (
        {
-            label: <span>{key}</span>,
+            label: <span>{value.displayName}</span>,
            value: key,
        }
    ));

    return (
        <Select
-            defaultValue={undefined}
+            value={value}


I'm pretty sure default value is sufficient here. Try using my changes for this file other than your name change from inputs to manifest

Changing it match your version

meganrm · 2025-11-05T23:41:48Z

src/components/ErrorLogs/index.tsx

@@ -1,19 +1,21 @@
 import { useState } from "react";
 import { Button, Drawer } from "antd";
+import { usePackingData } from "../../state/store";


this is still too imprecise for me. is it user changes, or the recipe settings or the results?

I wrote this as I was reading, now understand what it is, but that was my first impression

meganrm · 2025-11-05T23:42:15Z

src/components/ErrorLogs/index.tsx

-    const { errorLogs } = props;
+const ErrorLogs = (): JSX.Element => {
    const [viewErrorLogs, setViewErrorLogs] = useState<boolean>(true);
+    const {jobStatus, jobLogs: errorLogs} = usePackingData();


based on this I would say it's resultsMetadata?

meganrm · 2025-11-05T23:43:30Z

src/components/InputSwitch/index.tsx

+    const editRecipe = useEditRecipe();
    const getCurrentValue = useGetCurrentValue();
-    const recipeVersion = useCurrentRecipeString();
+    const recipeVersion = useRecipes();


this is a little confusing to me. I expect the recipeVersion to be a field in the recipe. and useRecipes() to give you every available recipe

This is just a holdover naming wise, recipes useRecipes does return all available recipes. I'll rename the local variable.

meganrm · 2025-11-05T23:45:08Z

src/components/JSONViewer/index.tsx


-const JSONViewer = (props: JSONViewerProps): JSX.Element | null => {
-    const { content } = props;
+    const currentRecipe = recipes[selectedRecipeId];


this should be a selector in state the returns the recipe data. This component should just get the recipe obj either as a prop or as a useCurrentRecipeData hook

meganrm · 2025-11-05T23:48:53Z

src/state/constants.ts

+
+// stable/frozen empty array to prevent re-renders
+export const EMPTY_FIELDS: readonly EditableField[] = Object.freeze([]);
+export const EMPTY_PACKING_DATA: PackingManifest = Object.freeze({


Looking at this again, I would call this packingStatusData, resultsMetaData, resultsStatus etc. just to make it clear this is AFTER you've hit pack and not having to do with inputs to a packing

or responseMetadata

meganrm · 2025-11-05T23:49:22Z

src/state/store.ts

+    loadAllRecipes: () => void;
+    selectRecipe: (recipeId: string) => Promise<void>;
+    editRecipe: (recipeId: string, updates: Record<string, string | number>) => void;
    restoreRecipeDefault: (recipeId: string) => void;
    getCurrentValue: (path: string) => string | number | undefined;
    getOriginalValue: (path: string) => string | number | undefined;
-    startPacking: (
-        callback: (recipeId: string, configId: string, recipeString: string) => Promise<void>
-    ) => Promise<void>;
+    startPacking: () => Promise<void>;
+    reset: () => void;


this set looks really clear to me now

(other than the ones we commented on together )

meganrm · 2025-11-05T23:50:59Z

src/state/store.ts

    isLoading: false,
    isPacking: false,


note of what we talked about: can this be one called requestStatus that's an enum of LOADING, PACKING or NONE

meganrm · 2025-11-05T23:51:30Z

src/state/store.ts

+                const recipes = await getRecipesFromFirebase();
+                set({ recipes });
+                // make an intial selection
+                const firstId = Object.keys(recipes)[0];
+                set(s => (!s.selectedRecipeId && firstId ? { selectedRecipeId: firstId } : {}));
            } finally {
                set({ isLoading: false });
            }


note to use my changes here in the merge

meganrm · 2025-11-05T23:52:10Z

src/state/store.ts

-            Object.values(inputOptions).forEach(opt => { if (opt?.recipe) ids.add(opt.recipe); });
-            const missing = [...ids].filter(id => !recipes[id]);
-            if (!missing.length) return;
+            const newEdits = { ...rec.edits };


wouldn't these been the currentEdits?

i'm surprised this isn't just one field at a time. how is a user able to send in multiple edits at once to state?

meganrm · 2025-11-05T23:59:34Z

src/state/store.ts

            try {
-                await callback(s.selectedRecipeId, configId, recipeString);
+                const { response, data } = await submitJob(selectedRecipeId, recipeString, rec.configId);
+                set((state) => ({ packingData: { ...state.packingData, jobStatus: JOB_STATUS.SUBMITTED } }));


shouldnt state.packingData always be empty at this point?

also, you don't have a storeResults action which means this functionality is a little hidden

meganrm · 2025-11-06T00:01:44Z

src/state/store.ts

+                const finalStatus = await pollForJobStatus(newJobId, (next) =>
+                    set(state => ({ packingData: { ...state.packingData, jobStatus: next } }))
+                );


this pattern makes me feel like jobStatus should be a top level piece of state so you dont have to keep re-writing the rest of this object. but conceptually i see why you grouped them together

src/state/store.ts

meganrm · 2025-11-06T00:03:52Z

src/state/store.ts

-                return undefined;
-            }
-        },
+        reset: () => set(() => ({ ...initialState })),


meganrm · 2025-11-06T00:04:31Z

src/state/utils.ts

+import { RecipeManifest } from "../types";
+import { set } from "lodash-es";
+
+/**
+ * Build a recipe from a default and a set of edits.
+ */
+export const buildCurrentRecipeObject = (recipe: RecipeManifest) => {
+    const clone = structuredClone(recipe.defaultRecipeData);
+    for (const [path, value] of Object.entries(recipe.edits)) {
+        set(clone, path, value);
+    }
+    return clone;
+};


why isn't this just in the selectors list?

meganrm · 2025-11-06T00:05:19Z

src/types/index.ts

+export interface RecipeManifest {
+    recipeId: string;
+    configId: string;
+    displayName: string;
+    editableFields: EditableField[];
+    defaultRecipeData: ViewableRecipe;
+    edits: Record<string, string | number>;
+}


i like this

meganrm · 2025-11-06T00:07:05Z

src/utils/packingService.ts

+}
+
+export const recipeHasChanged = async (recipeId: string, recipeString: string): Promise<boolean> => {
+    const originalRecipe = await getFirebaseRecipe(recipeId);


shouldn't this be in state?

meganrm · 2025-11-06T00:08:19Z

src/utils/packingService.ts

+    recipeString: string,
+    configId?: string
+) => {
+    let firebaseRecipe = "firebase:recipes/" + selectedRecipeId;


nit: firebaseRecipePath

meganrm

Looks great! I suggested a lot of name changes and a few places to move things around

meganrm · 2025-11-06T00:35:50Z

src/utils/firebase.ts

+    const querySnapshot = await queryDocumentById(FIRESTORE_COLLECTIONS.RESULTS, jobId);
+    return extractSingleDocumentData(querySnapshot, FIRESTORE_FIELDS.URL);
+};
+


this url comes with the status ( on line 114), why do you need to separate it out here?

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

src/state/store.ts

+                    }));
+                }
            } finally {
                set({ isPacking: false });


instead of combining this with isLoading like I suggested before, I think this should become packingStatus, and it should be the JOB_STATUS or IDLE

meganrm · 2025-11-06T18:13:04Z

src/state/store.ts

+// Derived selectors
+export const useEditableFields = () =>
+    useRecipeStore(s => {
+        const id = s.selectedRecipeId;


you should use a selector to get other simple parts of state in a compound selector

interim17 added 9 commits October 21, 2025 15:20

use JS object instead of string as single source of truth in store

17f7a63

tests for store

154a1bb

use RecipeManifest single source of truth in store

f944bf7

pull packing data into store and utils and stop using recipe strings …

ec478fe

…as single source of truth

update tests

15ce497

wip branch bring in line with main pre-merge

d6afb28

use object in json viewer, and clean up on types and utils

02f3145

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

77ee8cd

…nto feature/ssot

put logs in store on failed jobs

1acdf30

interim17 commented Nov 5, 2025

View reviewed changes

src/state/store.ts

},

getCurrentValue: (path) => {

getOriginalValue: (path) => {

Copy link

Contributor Author

interim17 Nov 5, 2025

Choose a reason for hiding this comment

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

same here, recipe setting value

meganrm reviewed Nov 5, 2025

View reviewed changes

meganrm reviewed Nov 6, 2025

View reviewed changes

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

meganrm reviewed Nov 6, 2025

View reviewed changes

src/state/store.ts

return undefined;

}

},

reset: () => set(() => ({ ...initialState })),

Copy link

Contributor

meganrm Nov 6, 2025

Choose a reason for hiding this comment

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

👍🏻

meganrm reviewed Nov 6, 2025

View reviewed changes

meganrm requested changes Nov 6, 2025

View reviewed changes

meganrm reviewed Nov 6, 2025

View reviewed changes

interim17 added 3 commits November 6, 2025 13:15

make dropdown component in line with main

23e1986

rename recipeVersion variable

6f9aa8c

use packing results type and add defaultResultPath to RecipeManifest

e264325

interim17 changed the base branch from main to feature/show-precomputed-results-tweaks November 7, 2025 23:09

update tests

f229c30

interim17 mentioned this pull request Nov 10, 2025

Feature/show precomputed results tweaks #146

Merged

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

interim17 closed this Nov 19, 2025

DRAFT: Feature/ssot #134

DRAFT: Feature/ssot #134

Uh oh!

Conversation

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

Uh oh!

Problem

Solution

Type of change

Change summary:

Steps to Verify:

Screenshots (optional):

Keyfiles (delete if not relevant):

Uh oh!

github-actions bot commented Nov 5, 2025

Coverage Report

Uh oh!

github-actions bot commented Nov 5, 2025

Built to branch gh-pages at 2025-11-05 21:57 UTC. Preview will be ready when the GitHub Pages deployment is complete.

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!

meganrm Nov 5, 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!

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!

meganrm left a comment

Choose a reason for hiding this comment

Uh oh!

interim17 commented Nov 5, 2025 •

edited

Loading

Built to branch `gh-pages` at 2025-11-05 21:57 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

meganrm Nov 5, 2025 •

edited

Loading