From ca0d1efda2b491205ea95014cff63cd16099145c Mon Sep 17 00:00:00 2001
From: gtarpenning <griffin.tarpenning@wandb.com>
Date: Mon, 9 Dec 2024 18:26:55 -0800
Subject: [PATCH 1/3] fix(ui): fix error state saving in human feedback form

---
 .../StructuredFeedback/HumanAnnotation.tsx    | 46 ++++++++-----------
 1 file changed, 20 insertions(+), 26 deletions(-)

diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/feedback/StructuredFeedback/HumanAnnotation.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/feedback/StructuredFeedback/HumanAnnotation.tsx
index 7facffe9556..ba560b219b7 100644
--- a/weave-js/src/components/PagePanelComponents/Home/Browse3/feedback/StructuredFeedback/HumanAnnotation.tsx
+++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/feedback/StructuredFeedback/HumanAnnotation.tsx
@@ -53,17 +53,11 @@ export const HumanAnnotationCell: React.FC<HumanAnnotationProps> = props => {
   const feedbackSpecRef = props.hfSpec.ref;
 
   useEffect(() => {
-    if (!props.readOnly) {
-      // We don't need to listen for feedback changes if the cell is editable
-      // it is being controlled by local state
-      return;
-    }
     return getTsClient().registerOnFeedbackListener(
       props.callRef,
       query.refetch
     );
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [props.callRef]);
+  }, [props.callRef, query.refetch]);
 
   useEffect(() => {
     if (foundFeedbackCallRef && props.callRef !== foundFeedbackCallRef) {
@@ -183,6 +177,14 @@ const FeedbackComponentSelector: React.FC<{
   }) => {
     const wrappedOnAddFeedback = useCallback(
       async (value: any) => {
+        if (value == null || value === foundValue || value === '') {
+          // Remove from unsaved changes if value is invalid
+          setUnsavedFeedbackChanges(curr => {
+            const {[feedbackSpecRef]: _, ...rest} = curr;
+            return rest;
+          });
+          return true;
+        }
         setUnsavedFeedbackChanges(curr => ({
           ...curr,
           [feedbackSpecRef]: () => onAddFeedback(value),
@@ -346,21 +348,10 @@ export const NumericalFeedbackColumn = ({
   focused?: boolean;
   isInteger?: boolean;
 }) => {
-  const debouncedFn = useMemo(
-    () =>
-      _.debounce((val: number | null) => onAddFeedback?.(val), DEBOUNCE_VAL),
-    [onAddFeedback]
-  );
-  useEffect(() => {
-    return () => {
-      debouncedFn.cancel();
-    };
-  }, [debouncedFn]);
-
   return (
     <NumericalTextField
       value={defaultValue}
-      onChange={debouncedFn}
+      onChange={value => onAddFeedback?.(value)}
       min={min}
       max={max}
       isInteger={isInteger}
@@ -446,7 +437,7 @@ export const EnumFeedbackColumn = ({
     }));
     return opts;
   }, [options]);
-  const [value, setValue] = useState<Option | undefined>(undefined);
+  const [value, setValue] = useState<Option | null>(null);
 
   useEffect(() => {
     const found = dropdownOptions.find(option => option.value === defaultValue);
@@ -457,17 +448,19 @@ export const EnumFeedbackColumn = ({
 
   const onValueChange = useCallback(
     (newValue: Option | null) => {
-      if (newValue == null || newValue.value === value?.value) {
-        return;
-      }
       setValue(newValue);
-      onAddFeedback?.(newValue.value);
+      onAddFeedback?.(newValue?.value ?? '');
     },
-    [value?.value, onAddFeedback]
+    [onAddFeedback]
   );
 
   return (
-    <Select options={dropdownOptions} value={value} onChange={onValueChange} />
+    <Select
+      autoFocus={focused}
+      options={dropdownOptions}
+      value={value}
+      onChange={onValueChange}
+    />
   );
 };
 
@@ -582,6 +575,7 @@ export const NumericalTextField: React.FC<NumericalTextFieldProps> = ({
         (max != null && parsedVal > max)
       ) {
         setError(true);
+        onChange(null);
         return;
       }
 

From eecf0fd94825c71f4232da74d8eb4a01280ac16d Mon Sep 17 00:00:00 2001
From: gtarpenning <griffin.tarpenning@wandb.com>
Date: Mon, 9 Dec 2024 18:29:32 -0800
Subject: [PATCH 2/3] chore(ui): fix save state in human annotations when null
 or invalid

---
 .../feedback/StructuredFeedback/HumanAnnotation.tsx        | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/feedback/StructuredFeedback/HumanAnnotation.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/feedback/StructuredFeedback/HumanAnnotation.tsx
index ba560b219b7..27342f88e9e 100644
--- a/weave-js/src/components/PagePanelComponents/Home/Browse3/feedback/StructuredFeedback/HumanAnnotation.tsx
+++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/feedback/StructuredFeedback/HumanAnnotation.tsx
@@ -57,7 +57,7 @@ export const HumanAnnotationCell: React.FC<HumanAnnotationProps> = props => {
       props.callRef,
       query.refetch
     );
-  }, [props.callRef, query.refetch]);
+  }, [props.callRef, query.refetch, getTsClient]);
 
   useEffect(() => {
     if (foundFeedbackCallRef && props.callRef !== foundFeedbackCallRef) {
@@ -180,7 +180,8 @@ const FeedbackComponentSelector: React.FC<{
         if (value == null || value === foundValue || value === '') {
           // Remove from unsaved changes if value is invalid
           setUnsavedFeedbackChanges(curr => {
-            const {[feedbackSpecRef]: _, ...rest} = curr;
+            const rest = {...curr};
+            delete rest[feedbackSpecRef];
             return rest;
           });
           return true;
@@ -191,7 +192,7 @@ const FeedbackComponentSelector: React.FC<{
         }));
         return true;
       },
-      [onAddFeedback, setUnsavedFeedbackChanges, feedbackSpecRef]
+      [onAddFeedback, setUnsavedFeedbackChanges, feedbackSpecRef, foundValue]
     );
 
     switch (type) {

From 774c55e09f04a8cb75d7a0a5cf61a3a9a2da7163 Mon Sep 17 00:00:00 2001
From: gtarpenning <griffin.tarpenning@wandb.com>
Date: Tue, 10 Dec 2024 09:20:20 -0800
Subject: [PATCH 3/3] set format errors to null as well

---
 .../Home/Browse3/feedback/StructuredFeedback/HumanAnnotation.tsx | 1 +
 1 file changed, 1 insertion(+)

diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/feedback/StructuredFeedback/HumanAnnotation.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/feedback/StructuredFeedback/HumanAnnotation.tsx
index 27342f88e9e..24dcc5f986b 100644
--- a/weave-js/src/components/PagePanelComponents/Home/Browse3/feedback/StructuredFeedback/HumanAnnotation.tsx
+++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/feedback/StructuredFeedback/HumanAnnotation.tsx
@@ -567,6 +567,7 @@ export const NumericalTextField: React.FC<NumericalTextFieldProps> = ({
       // If val is null but v isn't empty, there's a format error
       if (val === null) {
         setError(true);
+        onChange(null);
         return;
       }