move sync and check logic to another file and add unit tests

app/packages/core/src/components/Modal/Modal.tsx

@@ -7,7 +7,6 @@ import { useRecoilCallback, useRecoilValue } from "recoil";
 import styled from "styled-components";
 import { ModalActionsRow } from "../Actions";
 import Sidebar from "../Sidebar";
-import { useOnSidebarSelectionChange } from "../Sidebar/useOnSidebarSelectionChange";
 import ModalNavigation from "./ModalNavigation";
 import { ModalSpace } from "./ModalSpace";
 import { TooltipInfo } from "./TooltipInfo";

app/packages/core/src/components/Sidebar/syncAndCheckRefreshNeeded.test.ts

@@ -0,0 +1,96 @@
+import { beforeEach, describe, expect, it } from "vitest";
+import { syncAndCheckRefreshNeeded } from "./syncAndCheckRefreshNeeded";
+
+describe("syncAndCheckRefreshNeeded", () => {
+  let lut: Map<string, Set<string>>;
+
+  beforeEach(() => {
+    lut = new Map();
+  });
+
+  it("returns false if currentActiveLabelFields is empty", () => {
+    const id = "looker1";
+    const currentActiveLabelFields = new Set<string>();
+
+    const result = syncAndCheckRefreshNeeded(id, lut, currentActiveLabelFields);
+    expect(result).toBe(false);
+    // lut shoiuldn't be modified
+    expect(lut.has(id)).toBe(false);
+  });
+
+  it("returns true if lut is empty and there are active label fields", () => {
+    const id = "looker1";
+    const currentActiveLabelFields = new Set<string>(["segmentation1"]);
+
+    const result = syncAndCheckRefreshNeeded(id, lut, currentActiveLabelFields);
+    expect(result).toBe(true);
+
+    // lut should now have the newly added fields for looker1
+    const storedFields = lut.get(id);
+    expect(storedFields).toEqual(new Set(["segmentation1"]));
+  });
+
+  it("returns true if the looker ID does not exist in lut (new looker)", () => {
+    lut.set("existingLooker", new Set(["seg1"]));
+
+    const currentActiveLabelFields = new Set(["seg1", "heatmap1"]);
+
+    const newLookerId = "newLooker";
+    const result = syncAndCheckRefreshNeeded(
+      newLookerId,
+      lut,
+      currentActiveLabelFields
+    );
+    expect(result).toBe(true);
+
+    const storedFieldsNewLooker = lut.get(newLookerId);
+    expect(storedFieldsNewLooker).toEqual(new Set(["seg1", "heatmap1"]));
+
+    const storedFieldsExistingLooker = lut.get("existingLooker");
+    expect(storedFieldsExistingLooker).toEqual(new Set(["seg1"]));
+  });
+
+  it("returns false if lut already has the same fields", () => {
+    const id = "looker2";
+    lut.set(id, new Set(["seg1", "seg2"]));
+
+    const currentActiveLabelFields = new Set(["seg1", "seg2"]);
+    const result = syncAndCheckRefreshNeeded(id, lut, currentActiveLabelFields);
+
+    expect(result).toBe(false);
+    // lut remains the same, no changes
+    expect(lut.get(id)).toEqual(new Set(["seg1", "seg2"]));
+  });
+
+  it("returns true if lut has a subset of the current fields (some new ones are missing)", () => {
+    const id = "looker2";
+    // lut only has "field1"
+    lut.set(id, new Set(["field1"]));
+
+    // incoming fields: "field1" (already known) and "field2" (new)
+    const currentActiveLabelFields = new Set(["field1", "field2"]);
+    const result = syncAndCheckRefreshNeeded(id, lut, currentActiveLabelFields);
+
+    expect(result).toBe(true);
+
+    // lut should have been updated to include both fields
+    expect(lut.get(id)).toEqual(new Set(["field1", "field2"]));
+  });
+
+  it("does not keep returning true after lut is updated", () => {
+    const id = "looker3";
+    lut.set(id, new Set(["field1"]));
+    // This call should add "field2"
+    let result = syncAndCheckRefreshNeeded(
+      id,
+      lut,
+      new Set(["field1", "field2"])
+    );
+    expect(result).toBe(true);
+    expect(lut.get(id)).toEqual(new Set(["field1", "field2"]));
+
+    // second call with the same fields should now return false
+    result = syncAndCheckRefreshNeeded(id, lut, new Set(["field1", "field2"]));
+    expect(result).toBe(false);
+  });
+});

app/packages/core/src/components/Sidebar/syncAndCheckRefreshNeeded.ts

@@ -0,0 +1,51 @@
+import { CachedLabels, LookerId } from "./useShouldReloadSample";
+
+/**
+ * Checks if a given looker needs to be refreshed based on active label fields.
+ *
+ * @param lookerId - unique Looker ID
+ * @param lut - look-up table tracking which fields each looker has already rendered at least once (and therefore cached)
+ * @param currentActiveLabelFields - set of active label fields for the modal/looker
+ *
+ * @returns whether or not looker should be refreshed (true) or not (false)
+ */
+export function syncAndCheckRefreshNeeded(
+  lookerId: string,
+  lut: Map<LookerId, CachedLabels>,
+  currentActiveLabelFields: Set<string>
+): boolean {
+  // if no active fields, no refresh is needed
+  if (currentActiveLabelFields.size === 0) {
+    return false;
+  }
+
+  const thisLookerActiveFields = lut.get(lookerId);
+
+  // we only care about net-new fields.
+  let hasNewFields = false;
+
+  if (thisLookerActiveFields) {
+    for (const field of currentActiveLabelFields) {
+      if (!thisLookerActiveFields.has(field)) {
+        hasNewFields = true;
+        break;
+      }
+    }
+  } else {
+    // if `thisLookerActiveFields` is undefined, it means this looker has not been considered yet
+    hasNewFields = true;
+  }
+
+  // if no new fields, then no refresh is needed
+  if (!hasNewFields) {
+    return false;
+  }
+
+  // update cached labels set for this looker
+  lut.set(
+    lookerId,
+    new Set([...(thisLookerActiveFields ?? []), ...currentActiveLabelFields])
+  );
+
+  return true;
+}

app/packages/core/src/components/Sidebar/useShouldReloadSample.ts

@@ -1,9 +1,10 @@
 import { activeLabelFields } from "@fiftyone/state";
 import { useCallback } from "react";
 import { useRecoilValue } from "recoil";
+import { syncAndCheckRefreshNeeded } from "./syncAndCheckRefreshNeeded";
 
-type LookerId = string;
-type CachedLabels = Set<string>;
+export type LookerId = string;
+export type CachedLabels = Set<string>;
 
 export const gridActivePathsLUT = new Map<LookerId, CachedLabels>();
 
@@ -16,41 +17,11 @@ export const useShouldReloadSampleOnActiveFieldsChange = ({
 
   const shouldRefresh = useCallback(
     (id: string) => {
-      const thisLookerActiveFields = gridActivePathsLUT.get(id);
-      const currentActiveLabelFields = new Set(activeLabelFieldsValue);
-
-      if (currentActiveLabelFields.size === 0) {
-        return false;
-      }
-
-      // diff the two sets, we only care about net new fields
-      // if there are no new fields, we don't need to update the tracker
-      let hasNewFields = false;
-
-      if (thisLookerActiveFields) {
-        for (const field of currentActiveLabelFields) {
-          if (!thisLookerActiveFields.has(field)) {
-            hasNewFields = true;
-            break;
-          }
-        }
-      } else {
-        hasNewFields = true;
-      }
-
-      if (!hasNewFields) {
-        return false;
-      }
-
-      gridActivePathsLUT.set(
+      return syncAndCheckRefreshNeeded(
         id,
-        new Set([
-          ...(thisLookerActiveFields ?? []),
-          ...currentActiveLabelFields,
-        ])
+        gridActivePathsLUT,
+        new Set(activeLabelFieldsValue)
       );
-
-      return true;
     },
     [activeLabelFieldsValue]
   );