bugfix/node result-view actively updates with latest snapshot by kleinwave · Pull Request #216 · qua-platform/qualibrate-app

kleinwave · 2025-07-14T12:43:17Z

https://quantum-machines.atlassian.net/jira/software/c/projects/QUAL/boards/173?assignee=712020%3A2a0fc73a-d6d7-4126-9576-6e44837c6a01&selectedIssue=QUAL-1454

kleinwave · 2025-07-14T13:51:21Z

 export const MeasurementHistory: React.FC<IMeasurementHistoryListProps> = ({ title = "Execution history" }) => {
  const { allMeasurements, trackLatest, setTrackLatest } = useGraphStatusContext();
-  const { trackLatestSidePanel, fetchOneSnapshot, setLatestSnapshotId, setResult, setDiffData } = useSnapshotsContext();
+  const { fetchOneSnapshot, setLatestSnapshotId } = useSnapshotsContext();


trackLatestSidePanel was dead code
setResult and setDiffData were removed because now we rely on fetchOneSnapshot to manage state resets internally

kleinwave · 2025-07-14T13:52:30Z

-      if (allMeasurements) {
-        const element = allMeasurements[0];
-        // if (element) {
+    if (trackLatest && allMeasurements && allMeasurements.length > 0) {


simplifying code readability combining if statements

kleinwave · 2025-07-14T13:54:12Z

+        setLatestId(current.id);
+        setLatestName(current.metadata?.name);
+        setSelectedItemName(current.metadata?.name);
+        setSelectedNodeNameInWorkflow(current.metadata?.name);
+        setLatestSnapshotId(current.id);


These lines make sure that the selected node in the UI is synced with the latest measurement

kleinwave · 2025-07-14T13:56:58Z

+        if (current.id !== undefined) {
+          if (prev && prev.id !== undefined && current.id !== prev.id) {
+            fetchOneSnapshot(current.id, prev.id, true, true);
          } else {
-            setResult({});
-            setDiffData({});
+            fetchOneSnapshot(current.id, undefined, true, true);


if there’s a previous snapshot and it’s different from the current, perform a diff fetch
else just fetch the current snapshot only

kleinwave · 2025-07-14T14:09:08Z

+  fetchingSnapshotId: number | undefined;
+  setFetchingSnapshotId: Dispatch<SetStateAction<number | undefined>>;


If we don't have this, fetchOneSnapshot could get called multiple times for the same snapshot in rapid succession, and can cause duplicate API calls or race conditions

kleinwave · 2025-07-14T14:10:03Z

  // -----------------------------------------------------------

  const fetchOneSnapshot = (snapshotId: number, snapshotId2?: number, updateResult = true, fetchUpdate = false) => {
+    if (fetchingSnapshotId === snapshotId) return;


prevents duplicate in-flight requests for the same snapshot

kleinwave · 2025-07-14T14:10:55Z

+    let id2 = snapshotId2 ? snapshotId2.toString() : snapshotId - 1 >= 0 ? (snapshotId - 1).toString() : "0";
+    if (id1 === id2) id2 = "0";


prevents self-comparison bugs in the diff-fetch API

kleinwave · 2025-07-16T09:31:56Z

In addition to co-pilot I asked chatgpt to varrify the correctness of my code and do an analysis to make sure it doesn't disrupt anything else in SnapshotsContext

kleinwave · 2025-07-17T12:15:18Z

-          if (element.id) {
-            setLatestSnapshotId(element.id);


current.id is is guaranteed to exist at this point in the code
So in the diff you dont need the conditional anymore

…earing result and diffData anymore when fetchSnapshotResult returns no result and fetchSnapshotUpdate returns no diff

…shot

Copilot

Pull Request Overview

This PR implements a bugfix to ensure that the node result view actively updates with the latest snapshot during graph execution. The changes add state management for tracking graph execution status and prevent snapshot fetches from interfering with active node results.

Key changes:

Added graph execution state tracking with graphIsRunning and related state variables
Implemented logic to prevent snapshot context resets during active graph execution
Enhanced the measurement history component to properly handle live updates during execution

Reviewed Changes

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

File	Description
frontend/src/modules/Snapshots/context/SnapshotsContext.tsx	Adds graph execution state management, prevents snapshot fetches during execution, and improves fetch deduplication logic
frontend/src/modules/GraphLibrary/components/GraphStatus/components/MeasurementHistory/MeasurementHistory.tsx	Updates measurement history to respect graph execution state and freeze mechanisms for consistent node result display

Copilot · 2025-07-21T09:18:29Z

-      console.log(`Max snapshot ID - previous=${odlMaxId}, latest=${newMaxId}`);
-      if (newMaxId !== odlMaxId! && allSnapshots.length !== 0) {
+      const oldMaxId = allSnapshots[0]?.id;
+      // console.log(`Max snapshot ID - previous=${odlMaxId}, latest=${newMaxId}`);


Variable name 'oldMaxId' appears to be a typo fix for 'odlMaxId' in the commented line above, but the variable naming is inconsistent with the comment on line 186 which still references 'odlMaxId'.

Suggested change

// console.log(`Max snapshot ID - previous=${odlMaxId}, latest=${newMaxId}`);

// console.log(`Max snapshot ID - previous=${oldMaxId}, latest=${newMaxId}`);

Copilot · 2025-07-21T09:18:30Z

+          console.log("Graph execution likely completed. Stopping tracking after debounce.");
+          setGraphIsRunning(false);
+          setFreezeLatestSnapshot(true); // Prevent snapshot context reset
+        }, 3000);


The hardcoded 3000ms timeout (line 224) should be extracted as a named constant to improve maintainability and make the debounce timing more explicit.

Suggested change

}, 3000);

}, DEBOUNCE_TIMEOUT_MS);

Copilot · 2025-07-21T09:18:30Z

      }
    }
-  }, [trackLatest, allMeasurements, latestId, latestName]);
+  }, [trackLatest, allMeasurements, graphIsRunning, latestId, latestName, freezeLatestSnapshot]);


The useEffect dependency array is missing trackLatestSidePanel which is used inside the effect (lines 41, 47). This could cause stale closures and incorrect behavior when trackLatestSidePanel changes.

Suggested change

}, [trackLatest, allMeasurements, graphIsRunning, latestId, latestName, freezeLatestSnapshot]);

}, [trackLatest, allMeasurements, graphIsRunning, latestId, latestName, freezeLatestSnapshot, trackLatestSidePanel]);

Copilot · 2025-07-21T09:18:31Z

      }
    }
-  }, [trackLatest, allMeasurements, latestId, latestName]);
+  }, [trackLatest, allMeasurements, graphIsRunning, latestId, latestName, freezeLatestSnapshot]);


The useEffect dependency array is missing several functions used within the effect: setSelectedItemName, setSelectedNodeNameInWorkflow, setLatestSnapshotId, and fetchOneSnapshot. These should be included to ensure the effect has access to the latest function references.

Suggested change

}, [trackLatest, allMeasurements, graphIsRunning, latestId, latestName, freezeLatestSnapshot]);

}, [trackLatest, allMeasurements, graphIsRunning, latestId, latestName, freezeLatestSnapshot, setSelectedItemName, setSelectedNodeNameInWorkflow, setLatestSnapshotId, fetchOneSnapshot]);

Copilot · 2025-07-21T09:18:31Z

+          fetchOneSnapshot(current.id); // Final snapshot without compare
+        }
+      } else {
+        console.log(`Skipping fetch, already displaying snapshot id ${current.id}`);


Multiple console.log statements throughout this component should be removed or replaced with a proper logging mechanism for production code.

kleinwave · 2025-07-28T09:40:13Z

When a calibration graph runs, snapshots intermittently fail to be fetched and displayed in the results section, because either the same snapshot was requested multiple times, creating race conditions. And jsonData would be cleared or even overwritten because of lack of fetch coordination between them.

fix:

first check in fetchOneSnapshot, do these conditionals:

// avoids duplicate fetching
if (fetchingSnapshotId === snapshotId) return;
// needs to update when nessesary (whenever snapshot is selected AND jsondata is set)
if (selectedSnapshotId === snapshotId && jsonData) return;

The bug with snapshot retreival was that jsonData was being fetched before it was even set
And the solution is that it will now only fetch the snapshot in the first place if it is set (whenever it’s selected)

manas-qm · 2025-07-29T14:39:13Z

    // const index2 = index - 1 >= 0 ? index - 1 : 0;
    // const index2 = selectedSnapshotId ? (selectedSnapshotId - 1 >= 0 ? selectedSnapshotId - 1 : 0) : 0;
+    if (fetchingSnapshotId === snapshotId) return;
+    if (selectedSnapshotId === snapshotId && jsonData) return;


Let's discuss this.

Fix node snapshot result not switching during graph execution

772fe8c

kleinwave self-assigned this Jul 14, 2025

kleinwave requested review from nulinspiratie and removed request for nulinspiratie July 14, 2025 12:43

small refactor

e310919

kleinwave requested a review from Copilot July 14, 2025 13:03

This comment was marked as outdated.

Sign in to view

further refactor

bddea46

kleinwave requested a review from Copilot July 14, 2025 13:23

This comment was marked as outdated.

Sign in to view

kleinwave commented Jul 14, 2025

View reviewed changes

kleinwave requested a review from nulinspiratie July 14, 2025 14:11

nulinspiratie requested a review from Copilot July 16, 2025 07:05

This comment was marked as outdated.

Sign in to view

kleinwave requested a review from Copilot July 16, 2025 09:16

This comment was marked as outdated.

Sign in to view

kleinwave force-pushed the bugfix/node-result-view-updates-with-latest-snapshot branch from 843b955 to bddea46 Compare July 16, 2025 13:14

kleinwave commented Jul 17, 2025

View reviewed changes

kleinwave added 5 commits July 17, 2025 15:46

small fixes + implementing co-pilot nitpik suggestion

4a8af8b

return undefined -> return

355085b

preventing false negatives in the results panel by not immediately cl…

c0d0ea8

…earing result and diffData anymore when fetchSnapshotResult returns no result and fetchSnapshotUpdate returns no diff

fixing mistake - undoing prev commit changes - not showing first snap…

6e87edb

…shot

fixing graph sync issues

5314a17

kleinwave requested a review from Copilot July 21, 2025 08:54

This comment was marked as outdated.

Sign in to view

fixing small things

861aaca

kleinwave requested a review from Copilot July 21, 2025 09:17

Copilot AI reviewed Jul 21, 2025

View reviewed changes

kleinwave added 4 commits July 21, 2025 12:26

commenting out logs for now

2cd1ef5

deleting unnessesary logs + small code refactor

d876230

small code refactor

a184b9d

minimal fix

d1109f9

manas-qm reviewed Jul 29, 2025

View reviewed changes

		fetchingSnapshotId: number \| undefined;
		setFetchingSnapshotId: Dispatch<SetStateAction<number \| undefined>>;

		let id2 = snapshotId2 ? snapshotId2.toString() : snapshotId - 1 >= 0 ? (snapshotId - 1).toString() : "0";
		if (id1 === id2) id2 = "0";

	// console.log(`Max snapshot ID - previous=${odlMaxId}, latest=${newMaxId}`);
	// console.log(`Max snapshot ID - previous=${oldMaxId}, latest=${newMaxId}`);

	}, [trackLatest, allMeasurements, graphIsRunning, latestId, latestName, freezeLatestSnapshot]);
	}, [trackLatest, allMeasurements, graphIsRunning, latestId, latestName, freezeLatestSnapshot, trackLatestSidePanel]);

Uh oh!

Conversation

kleinwave commented Jul 14, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

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!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

kleinwave commented Jul 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kleinwave Jul 17, 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!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

kleinwave commented Jul 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kleinwave commented Jul 16, 2025 •

edited

Loading

kleinwave Jul 17, 2025 •

edited

Loading