wip

wandb · Oct 29, 2024 · 1a9919a · 1a9919a
1 parent 0991082
commit 1a9919a
Show file tree

Hide file tree

Showing 5 changed files with 112 additions and 51 deletions.
diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/feedback/FeedbackGridInner.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/feedback/FeedbackGridInner.tsx
@@ -19,6 +19,7 @@ export const FeedbackGridInner = ({
   feedback,
   currentViewerId,
 }: FeedbackGridInnerProps) => {
+  console.log('feedback', feedback);
   const columns: GridColDef[] = [
     {
       field: 'feedback_type',
@@ -42,6 +43,11 @@ export const FeedbackGridInner = ({
         if (params.row.feedback_type === 'wandb.reaction.1') {
           return params.row.payload.emoji;
         }
+        if (params.row.feedback_type === 'wandb.human_annotation.1') {
+          // TODO: make this a helper function
+          const val = Object.values(Object.values(params.row.payload.value)[0])[0];
+          return <CellValueString value={JSON.stringify(val)} />;
+        }
         return <CellValueString value={JSON.stringify(params.row.payload)} />;
       },
     },

diff --git a/.../src/components/PagePanelComponents/Home/Browse3/feedback/HumanFeedback/HumanFeedback.tsx b/.../src/components/PagePanelComponents/Home/Browse3/feedback/HumanFeedback/HumanFeedback.tsx
@@ -23,6 +23,8 @@ import {
   FeedbackCreateSuccess,
 } from '../../pages/wfReactInterface/traceServerClientTypes';
 import { CategoricalFeedback, HumanAnnotationPayload, HumanFeedback,NumericalFeedback, tsFeedbackType} from './humanFeedbackTypes';
+import { parseRef } from '@wandb/weave/react';
+
 // Constants
 const STRUCTURED_FEEDBACK_TYPE = 'wandb.human_annotation.1';
 const MAGIC_FEEDBACK_TYPES = {
@@ -31,42 +33,38 @@ const MAGIC_FEEDBACK_TYPES = {
   BOOLEAN: 'BinaryFeedback',
   CATEGORICAL: 'CategoricalFeedback',
 }
-const DEBOUNCE_VAL = 100;
+const DEBOUNCE_VAL = 200;
 
 // Interfaces
-interface StructuredFeedbackProps {
-  sfData: tsFeedbackType;
-  callRef: string;
+interface HumanFeedbackProps {
   entity: string;
   project: string;
+  viewer: string | null;
+  sfData: tsFeedbackType;
+  callRef: string;
   readOnly?: boolean;
   focused?: boolean;
 }
 
 // Utility function for creating feedback request
 const createFeedbackRequest = (
-  props: StructuredFeedbackProps,
+  props: HumanFeedbackProps,
   value: any,
 ) => {
-  const parsedRef = parseRefMaybe(props.sfData.ref);
-  if (!parsedRef || !parsedRef?.artifactVersion) {
-    throw new Error('Invalid feedback ref');
-  }
-
-  const humanAnnotationPayload = {
+  const parsedRef = parseRef(props.sfData.ref);
+  const humanAnnotationPayload: HumanAnnotationPayload = {
     annotation_column_ref: props.sfData.ref,
     value: {
-      [props.sfData.object_id]: {
+      [parsedRef.artifactName]: {
         [parsedRef?.artifactVersion]: value
       }
     }
-  } as HumanAnnotationPayload;
-  console.log('humanAnnotationPayload', humanAnnotationPayload);
+  };
 
   const baseRequest = {
     project_id: `${props.entity}/${props.project}`,
     weave_ref: props.callRef,
-    creator: null,
+    creator: props.viewer,
     feedback_type: STRUCTURED_FEEDBACK_TYPE,
     payload: humanAnnotationPayload,
     sort_by: [{created_at: 'desc'}],
@@ -76,7 +74,7 @@ const createFeedbackRequest = (
 };
 
 const renderFeedbackComponent = (
-  props: StructuredFeedbackProps,
+  props: HumanFeedbackProps,
   onAddFeedback: (value: any) => Promise<boolean>,
   foundValue: string | number | null,
 ) => {
@@ -123,22 +121,22 @@ const renderFeedbackComponent = (
   }
 };
 
-export const StructuredFeedbackCell: React.FC<
-  StructuredFeedbackProps
+export const HumanFeedbackCell: React.FC<
+  HumanFeedbackProps
 > = props => {
   const {useFeedback} = useWFHooks();
   const query = useFeedback({
     entity: props.entity,
     project: props.project,
     weaveRef: props.callRef,
   });
-
   const [foundFeedback, setFoundFeedback] = useState<HumanFeedback[]>([]);
   const getTsClient = useGetTraceServerClientContext();
 
   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(
@@ -205,25 +203,60 @@ export const StructuredFeedbackCell: React.FC<
     setFoundFeedback(currFeedback);
   }, [query?.result, query?.loading, props.sfData]);
 
+  // userId -> objectId -> objectHash : value
+  const combinedFeedback = foundFeedback.reduce((acc, feedback) => {
+    return {
+      [feedback.creator ?? '']: feedback.payload.value,
+      ...acc,
+    };
+  }, {});
+
+  console.log('combinedFeedback', combinedFeedback);
+
+  // rawValues is an array of values from the feedback
+  const parsedRef = parseRef(props.sfData.ref);
+
+  const rawValues = useMemo(() => {
+    let values = [];
+    for (const payload of Object.values(combinedFeedback)) {
+      const pRecord = payload as Record<string, Record<string, string>>;
+      values.push(pRecord[parsedRef.artifactName]?.[parsedRef.artifactVersion]);
+    }
+    return values;
+  }, [combinedFeedback, parsedRef])
+
+  console.log('rawValues', rawValues);
+
+
   if (query?.loading) {
     return <LoadingDots />;
   }
 
-  console.log('foundFeedback', foundFeedback);
-
-  const values = foundFeedback?.map(feedback => feedback.payload.value[props.sfData.object_id][props.sfData.ref]);
-  console.log('values', values);
-
   if (props.readOnly) {
     // TODO: make this prettier, for now just join with commas
     return <div className="flex w-full justify-center">
-      <CellValueString value={values?.join(', ')} />
+      <CellValueString value={rawValues?.join(', ')} />
+    </div>;
+  }
+
+  // TODO: fix, we want only one callsite for renderFeedbackComponent
+  if (Object.keys(combinedFeedback).length === 0) {
+    return <div className="flex w-full justify-center">
+      {renderFeedbackComponent(props, onAddFeedback, null)}
     </div>;
   }
 
   return (
-    <div className="flex w-full justify-center">
-      {values?.map(val => renderFeedbackComponent(props, onAddFeedback, val))}
+    <div className="w-full py-4">
+      {rawValues?.map(val => renderFeedbackComponent(props, onAddFeedback, val))}
+      {/* {Object.entries(combinedFeedback)
+        .map(([userId, value]) => {
+          return renderFeedbackComponent(
+            props,
+            onAddFeedback,
+            value[0]?.[parsedRef.artifactName]?.[parsedRef.artifactVersion]
+          )
+        })} */}
     </div>
   );
 };
@@ -316,7 +349,7 @@ export const TextFeedbackColumn = ({
   };
 
   return (
-    <div className="w-full">
+    <div className="w-full pb-4">
       <TextField
         autoFocus={focused}
         value={value}

diff --git a/...mponents/PagePanelComponents/Home/Browse3/feedback/HumanFeedback/HumanFeedbackSidebar.tsx b/...mponents/PagePanelComponents/Home/Browse3/feedback/HumanFeedback/HumanFeedbackSidebar.tsx
@@ -3,8 +3,9 @@ import {makeRefCall} from '@wandb/weave/util/refs';
 import React, {useState} from 'react';
 
 import {Icon} from '../../../../../Icon';
-import {StructuredFeedbackCell} from './HumanFeedback';
+import {HumanFeedbackCell} from './HumanFeedback';
 import {tsHumanFeedbackSpec} from './humanFeedbackTypes';
+import { useViewerInfo } from '@wandb/weave/common/hooks/useViewerInfo';
 
 type HumanFeedbackSidebarProps = {
   feedbackOptions: tsHumanFeedbackSpec | null;
@@ -19,17 +20,21 @@ export const HumanFeedbackSidebar = ({
   entity,
   project,
 }: HumanFeedbackSidebarProps) => {
-  const feedbackFields = feedbackOptions?.feedback_fields;
-  const feedbackSpecRef = feedbackOptions?.ref;
   const callRef = makeRefCall(entity, project, callID);
+  const {loading: loadingUserInfo, userInfo} = useViewerInfo();
+
   const [isExpanded, setIsExpanded] = useState(true);
 
-  const feedbackCount = feedbackFields?.length ?? 0;
+  const feedbackFields = feedbackOptions?.feedback_fields;
+  const feedbackSpecRef = feedbackOptions?.ref;
+  const feedbackCellCount = feedbackFields?.length ?? 0;
 
-  if (!feedbackSpecRef) {
+  if (loadingUserInfo || !feedbackSpecRef) {
     return null;
   }
 
+  const viewer = userInfo ? userInfo.id : null;
+
   return (
     <Tailwind>
       <div className="flex h-full flex-col bg-white">
@@ -44,7 +49,7 @@ export const HumanFeedbackSidebar = ({
               <div className="flex items-center">
                 <span className="">Human scores</span>
                 <span className="text-gray-600 bg-gray-200 ml-4 mt-2 rounded-full px-2 text-xs font-medium">
-                  {feedbackCount}
+                  {feedbackCellCount}
                 </span>
               </div>
               <Icon name={isExpanded ? 'chevron-up' : 'chevron-down'} />
@@ -57,12 +62,13 @@ export const HumanFeedbackSidebar = ({
                       {field.display_name}
                     </h3>
                     <div className="pb-8 pl-6 pr-8 pt-2">
-                      <StructuredFeedbackCell
+                      <HumanFeedbackCell
                         focused={index === 0}
                         sfData={field}
                         callRef={callRef}
                         entity={entity}
                         project={project}
+                        viewer={viewer}
                         readOnly={false}
                       />
                     </div>

diff --git a/...src/components/PagePanelComponents/Home/Browse3/feedback/HumanFeedback/tsHumanFeedback.ts b/...src/components/PagePanelComponents/Home/Browse3/feedback/HumanFeedback/tsHumanFeedback.ts
@@ -5,6 +5,23 @@ import {objectVersionKeyToRefUri} from '../../pages/wfReactInterface/utilities';
 import {ObjectVersionSchema} from '../../pages/wfReactInterface/wfDataModelHooksInterface';
 import {tsHumanFeedbackSpec} from './humanFeedbackTypes';
 
+
+const useResolveTypeObjects = (typeRefs: string[]) => {
+  const {useRefsData} = useWFHooks();
+  const refsData = useRefsData(typeRefs);
+  return useMemo(() => {
+    if (refsData.loading || refsData.result == null) {
+      return null;
+    }
+    const refDataWithRefs = refsData.result.map((x, i) => ({
+      ...x,
+      ref: typeRefs[i],
+    }));
+    return refDataWithRefs;
+  }, [refsData.loading, refsData.result]);
+};
+
+
 export const useHumanFeedbackOptions = (
   entity: string,
   project: string
@@ -24,13 +41,12 @@ export const useHumanFeedbackOptions = (
     undefined, // limit
     false // metadataOnly
   );
-
   // TODO: this is not actually tsHumanFeedbackSpec, it's HumanFeedbackSpec
   // we need to add the refs for each of the feedback fields
-  const val: tsHumanFeedbackSpec | null = latestSpec?.val;
-  const feedbackFields = val?.feedback_fields;
+  const feedbackFieldRefs = latestSpec?.val?.feedback_fields ?? []
+  const feedbackFields = useResolveTypeObjects(feedbackFieldRefs);
 
-  // TODO: how do we get the refs for the sub-objects here?
+  console.log('feedbackFields', feedbackFields);
 
   useEffect(() => {
     if (humanFeedbackObjects.loading || humanFeedbackObjects.result == null) {

diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/CallPage.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/CallPage.tsx
@@ -223,23 +223,23 @@ const CallPageInnerVertical: FC<{
     <SimplePageLayoutWithHeader
       headerExtra={
         <Box>
-          <Button
-            icon="marker"
-            tooltip={`${showFeedbackExpand ? 'Hide' : 'Show'} feedback sidebar`}
-            variant="ghost"
-            active={showFeedbackExpand ?? false}
-            onClick={onToggleFeedbackExpand}
-            className="mr-4"
-          />
           {feedbackOptions && (
             <Button
-              icon="layout-tabs"
-              tooltip={`${showTraceTree ? 'Hide' : 'Show'} trace tree`}
+              icon="marker"
+              tooltip={`${showFeedbackExpand ? 'Hide' : 'Show'} feedback sidebar`}
               variant="ghost"
-              active={showTraceTree ?? false}
-              onClick={onToggleTraceTree}
+              active={showFeedbackExpand ?? false}
+              onClick={onToggleFeedbackExpand}
+              className="mr-4"
             />
           )}
+          <Button
+            icon="layout-tabs"
+            tooltip={`${showTraceTree ? 'Hide' : 'Show'} trace tree`}
+            variant="ghost"
+            active={showTraceTree ?? false}
+            onClick={onToggleTraceTree}
+          />
         </Box>
       }
       isSidebarOpen={showTraceTree}