Skip to content

Commit

Permalink
chore(weave): human annotation labeling review comments (#3020)
Browse files Browse the repository at this point in the history
  • Loading branch information
gtarpenning authored Nov 19, 2024
1 parent 35aeea4 commit 3fa8ebf
Show file tree
Hide file tree
Showing 10 changed files with 194 additions and 59 deletions.
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import {toast} from '@wandb/weave/common/components/elements/Toast';
import {useOrgName} from '@wandb/weave/common/hooks/useOrganization';
import {useViewerInfo} from '@wandb/weave/common/hooks/useViewerInfo';
import {useViewerUserInfo2} from '@wandb/weave/common/hooks/useViewerUserInfo';
import {Button} from '@wandb/weave/components/Button';
import {Icon} from '@wandb/weave/components/Icon';
import {Loading} from '@wandb/weave/components/Loading';
import {annotationsViewed} from '@wandb/weave/integrations/analytics/viewEvents';
import {makeRefCall} from '@wandb/weave/util/refs';
import React, {useState} from 'react';
import React, {useEffect, useState} from 'react';
import {useHistory} from 'react-router-dom';

import {useWeaveflowRouteContext} from '../../context';
Expand All @@ -14,13 +18,15 @@ import {tsHumanAnnotationSpec} from './humanAnnotationTypes';

type FeedbackSidebarProps = {
humanAnnotationSpecs: tsHumanAnnotationSpec[];
specsLoading: boolean;
callID: string;
entity: string;
project: string;
};

export const FeedbackSidebar = ({
humanAnnotationSpecs,
specsLoading,
callID,
entity,
project,
Expand All @@ -32,6 +38,11 @@ export const FeedbackSidebar = ({
Record<string, () => Promise<boolean>>
>({});

const {loading: viewerLoading, userInfo} = useViewerUserInfo2();
const {loading: orgNameLoading, orgName} = useOrgName({
entityName: entity,
});

const save = async () => {
setIsSaving(true);
try {
Expand All @@ -58,6 +69,28 @@ export const FeedbackSidebar = ({
}
};

useEffect(() => {
if (!viewerLoading && !orgNameLoading) {
annotationsViewed({
traceId: callID,
userId: userInfo.id,
organizationName: orgName,
entityName: entity,
projectName: project,
numAnnotationSpecs: humanAnnotationSpecs.length,
});
}
}, [
viewerLoading,
orgNameLoading,
userInfo,
orgName,
entity,
project,
humanAnnotationSpecs.length,
callID,
]);

return (
<div className="flex h-full flex-col bg-white">
<div className="justify-left flex w-full border-b border-moon-300 p-12">
Expand All @@ -75,7 +108,7 @@ export const FeedbackSidebar = ({
setUnsavedFeedbackChanges={setUnsavedFeedbackChanges}
/>
</div>
<div className="flex w-full border-t border-moon-300 p-6 pr-10">
<div className="flex w-full border-t border-moon-300 p-12 pr-18">
<Button
onClick={save}
variant="primary"
Expand All @@ -88,6 +121,10 @@ export const FeedbackSidebar = ({
</Button>
</div>
</>
) : specsLoading ? (
<div className="mt-12 w-full items-center justify-center">
<Loading centered />
</div>
) : (
<div className="mt-12 w-full items-center justify-center">
<Empty {...EMPTY_PROPS_ANNOTATIONS} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ const FeedbackComponentSelector: React.FC<{
jsonSchema: Record<string, any>;
focused: boolean;
onAddFeedback: (value: any) => Promise<boolean>;
foundValue: string | number | null;
foundValue: string | number | boolean | null;
feedbackSpecRef: string;
setUnsavedFeedbackChanges: React.Dispatch<
React.SetStateAction<Record<string, () => Promise<boolean>>>
Expand All @@ -194,12 +194,23 @@ const FeedbackComponentSelector: React.FC<{
);

switch (type) {
case 'integer':
return (
<NumericalFeedbackColumn
min={jsonSchema.minimum}
max={jsonSchema.maximum}
isInteger={true}
onAddFeedback={wrappedOnAddFeedback}
defaultValue={foundValue as number | null}
focused={focused}
/>
);
case 'number':
return (
<NumericalFeedbackColumn
min={jsonSchema.min}
max={jsonSchema.max}
isInteger={jsonSchema.is_integer}
min={jsonSchema.minimum}
max={jsonSchema.maximum}
isInteger={false}
onAddFeedback={wrappedOnAddFeedback}
defaultValue={foundValue as number | null}
focused={focused}
Expand All @@ -226,7 +237,7 @@ const FeedbackComponentSelector: React.FC<{
return (
<BinaryFeedbackColumn
onAddFeedback={wrappedOnAddFeedback}
defaultValue={foundValue as string | null}
defaultValue={foundValue as boolean | null}
focused={focused}
/>
);
Expand Down Expand Up @@ -513,35 +524,35 @@ export const BinaryFeedbackColumn = ({
defaultValue,
focused,
}: {
onAddFeedback?: (value: string) => Promise<boolean>;
defaultValue: string | null;
onAddFeedback?: (value: any) => Promise<boolean>;
defaultValue: boolean | null;
focused?: boolean;
}) => {
const [value, setValue] = useState<string | null>(defaultValue);
const [value, setValue] = useState<boolean | null>(defaultValue);

useEffect(() => {
setValue(defaultValue);
}, [defaultValue]);

const handleClick = (newValue: string) => {
const handleClick = (newValue: boolean) => {
// If clicking the same value, deselect it
const valueToSet = value === newValue ? null : newValue;
setValue(valueToSet);
onAddFeedback?.(valueToSet ?? '');
onAddFeedback?.(valueToSet);
};

return (
<Tailwind>
<div className="flex w-full justify-center gap-10">
<Button
variant={value === 'true' ? 'primary' : 'outline'}
onClick={() => handleClick('true')}
variant={value === true ? 'primary' : 'outline'}
onClick={() => handleClick(true)}
autoFocus={focused}>
True
</Button>
<Button
variant={value === 'false' ? 'primary' : 'outline'}
onClick={() => handleClick('false')}>
variant={value === false ? 'primary' : 'outline'}
onClick={() => handleClick(false)}>
False
</Button>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@ import {AnnotationSpec} from '../../pages/wfReactInterface/generatedBaseObjectCl
import {Feedback} from '../../pages/wfReactInterface/traceServerClientTypes';

export const HUMAN_ANNOTATION_BASE_TYPE = 'wandb.annotation';
export const FEEDBACK_TYPE_OPTIONS = ['string', 'number', 'boolean', 'enum'];
export const FEEDBACK_TYPE_OPTIONS = [
'string',
'number',
'boolean',
'enum',
'integer',
];
export type FeedbackSchemaType = (typeof FEEDBACK_TYPE_OPTIONS)[number];

export const makeAnnotationFeedbackType = (type: string) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {tsHumanAnnotationSpec} from './humanAnnotationTypes';
export const useHumanAnnotationSpecs = (
entity: string,
project: string
): tsHumanAnnotationSpec[] => {
): {humanAnnotationSpecs: tsHumanAnnotationSpec[]; specsLoading: boolean} => {
const req: TraceObjQueryReq = {
project_id: projectIdFromParts({entity, project}),
filter: {
Expand All @@ -31,20 +31,23 @@ export const useHumanAnnotationSpecs = (

return useMemo(() => {
if (cols == null) {
return [];
return {humanAnnotationSpecs: [], specsLoading: res.loading};
}
return cols.map(col => ({
...col.val,
// attach object refs to the columns
ref: objectVersionKeyToRefUri({
scheme: 'weave',
weaveKind: 'object',
entity,
project,
objectId: col.object_id,
versionHash: col.digest,
path: '',
}),
}));
}, [cols, entity, project]);
return {
humanAnnotationSpecs: cols.map(col => ({
...col.val,
// attach object refs to the columns
ref: objectVersionKeyToRefUri({
scheme: 'weave',
weaveKind: 'object',
entity,
project,
objectId: col.object_id,
versionHash: col.digest,
path: '',
}),
})),
specsLoading: res.loading,
};
}, [cols, entity, project, res.loading]);
};
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ const CallPageInnerVertical: FC<{
)
);
}, [currentRouter, history, path, showTraceTree, call, showFeedbackExpand]);
const humanAnnotationSpecs = useHumanAnnotationSpecs(
const {humanAnnotationSpecs, specsLoading} = useHumanAnnotationSpecs(
call.entity,
call.project
);
Expand Down Expand Up @@ -275,23 +275,21 @@ const CallPageInnerVertical: FC<{
<PaginationControls call={call} path={path} />
)}
<Box sx={{marginLeft: showPaginationContols ? 0 : 'auto'}}>
<Button
icon="marker"
tooltip={`${
showFeedbackExpand ? 'Hide' : 'Show'
} feedback sidebar`}
variant="ghost"
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}
/>
<Button
icon="marker"
tooltip={`${showFeedbackExpand ? 'Hide' : 'Show'} feedback`}
variant="ghost"
active={showFeedbackExpand ?? false}
onClick={onToggleFeedbackExpand}
className="ml-4"
/>
</Box>
</Box>
}
Expand All @@ -301,6 +299,7 @@ const CallPageInnerVertical: FC<{
<div className="flex h-full flex-col">
<FeedbackSidebar
humanAnnotationSpecs={humanAnnotationSpecs}
specsLoading={specsLoading}
callID={currentCall.callId}
entity={currentCall.entity}
project={currentCall.project}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@ import React, {FC, useCallback, useContext, useEffect} from 'react';
import {useHistory} from 'react-router-dom';

import {TableRowSelectionContext} from '../../../Browse3';
import {TRACETREE_PARAM, useWeaveflowCurrentRouteContext} from '../../context';
import {isEvaluateOp} from '../common/heuristics';
import {
FEEDBACK_EXPAND_PARAM,
TRACETREE_PARAM,
useWeaveflowCurrentRouteContext,
} from '../../context';
import {useURLSearchParamsDict} from '../util';
import {CallSchema} from '../wfReactInterface/wfDataModelHooksInterface';

Expand All @@ -21,9 +24,11 @@ export const PaginationControls: FC<{
const currentRouter = useWeaveflowCurrentRouteContext();
const query = useURLSearchParamsDict();
const showTraceTree =
TRACETREE_PARAM in query
? query[TRACETREE_PARAM] === '1'
: !isEvaluateOp(call.spanName);
TRACETREE_PARAM in query ? query[TRACETREE_PARAM] === '1' : false;
const showFeedbackExpand =
FEEDBACK_EXPAND_PARAM in query
? query[FEEDBACK_EXPAND_PARAM] === '1'
: false;

const onNextCall = useCallback(() => {
const nextCallId = getNextRowId?.(call.callId);
Expand All @@ -35,11 +40,20 @@ export const PaginationControls: FC<{
call.traceId,
nextCallId,
path,
showTraceTree
showTraceTree,
showFeedbackExpand
)
);
}
}, [call, currentRouter, history, path, showTraceTree, getNextRowId]);
}, [
call,
currentRouter,
history,
path,
showTraceTree,
getNextRowId,
showFeedbackExpand,
]);
const onPreviousCall = useCallback(() => {
const previousRowId = getPreviousRowId?.(call.callId);
if (previousRowId) {
Expand All @@ -50,11 +64,20 @@ export const PaginationControls: FC<{
call.traceId,
previousRowId,
path,
showTraceTree
showTraceTree,
showFeedbackExpand
)
);
}
}, [call, currentRouter, history, path, showTraceTree, getPreviousRowId]);
}, [
call,
currentRouter,
history,
path,
showTraceTree,
getPreviousRowId,
showFeedbackExpand,
]);
const handleKeyDown = useCallback(
(event: KeyboardEvent) => {
if (event.key === 'ArrowDown' && event.shiftKey) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,9 @@ function buildCallsTableColumns(
return row[c];
},
renderCell: (params: GridRenderCellParams<TraceCallSchema>) => {
if (typeof params.value === 'boolean') {
return <div>{params.value ? 'true' : 'false'}</div>;
}
return <div>{params.value}</div>;
},
};
Expand Down
Loading

0 comments on commit 3fa8ebf

Please sign in to comment.