From 69a8da72f9341cb7dc32459db21abeec0f2941f5 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Mon, 16 Dec 2024 14:51:12 -0800 Subject: [PATCH 01/28] chore(ui): add frontend handling for deleted objects --- .../components/elements/ModifiedDropdown.tsx | 46 +-- .../Home/Browse2/SmallRef.tsx | 18 +- .../Browse3/pages/CallPage/ObjectViewer.tsx | 11 +- .../CallPage/ObjectViewerGroupingCell.tsx | 5 +- .../Home/Browse3/pages/ObjectVersionPage.tsx | 10 +- .../Home/Browse3/pages/OpVersionPage.tsx | 6 +- .../pages/PlaygroundPage/llmMaxTokens.ts | 23 -- .../PlaygroundPage/usePlaygroundState.ts | 11 +- .../Home/Browse3/pages/common/DeleteModal.tsx | 174 ++++++++++++ .../wfReactInterface/traceServerClient.ts | 8 + .../traceServerClientTypes.ts | 10 + .../traceServerDirectClient.ts | 5 + .../wfReactInterface/tsDataModelHooks.ts | 268 +++++++++++++++--- .../wfDataModelHooksInterface.ts | 12 +- 14 files changed, 500 insertions(+), 107 deletions(-) create mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx diff --git a/weave-js/src/common/components/elements/ModifiedDropdown.tsx b/weave-js/src/common/components/elements/ModifiedDropdown.tsx index 301abe035f75..814563cc14bc 100644 --- a/weave-js/src/common/components/elements/ModifiedDropdown.tsx +++ b/weave-js/src/common/components/elements/ModifiedDropdown.tsx @@ -45,28 +45,11 @@ type LabelCoord = { const ITEM_LIMIT_VALUE = '__item_limit'; -export function getAsValidRegex(s: string): RegExp | null { - try { - return new RegExp(s); - } catch (e) { - return null; - } -} - -export const simpleSearch = ( - options: DropdownItemProps[], - query: string, - config: { - allowRegexSearch?: boolean; - } = {} -) => { - const regex = config.allowRegexSearch ? getAsValidRegex(query) : null; - +const simpleSearch = (options: DropdownItemProps[], query: string) => { return _.chain(options) - .filter(o => { - const text = JSON.stringify(o.text).toLowerCase(); - return regex ? regex.test(text) : _.includes(text, query.toLowerCase()); - }) + .filter(o => + _.includes(JSON.stringify(o.text).toLowerCase(), query.toLowerCase()) + ) .sortBy(o => { const valJSON = typeof o.text === 'string' ? `"${query}"` : query; return JSON.stringify(o.text).toLowerCase() === valJSON.toLowerCase() @@ -86,17 +69,17 @@ const getOptionProps = (opt: Option, hideText: boolean) => { }; export interface ModifiedDropdownExtraProps { - allowRegexSearch?: boolean; debounceTime?: number; enableReordering?: boolean; - hideText?: boolean; itemLimit?: number; options: Option[]; - optionTransform?(option: Option): Option; resultLimit?: number; resultLimitMessage?: string; style?: CSSProperties; + hideText?: boolean; useIcon?: boolean; + + optionTransform?(option: Option): Option; } type ModifiedDropdownProps = Omit & @@ -115,11 +98,10 @@ const ModifiedDropdown: FC = React.memo( } = props; const { - allowAdditions, - allowRegexSearch, - enableReordering, itemLimit, optionTransform, + enableReordering, + allowAdditions, resultLimit = 100, resultLimitMessage = `Limited to ${resultLimit} items. Refine search to see other options.`, ...passProps @@ -148,13 +130,15 @@ const ModifiedDropdown: FC = React.memo( _.concat(currentOptions, search(propsOptions, query) as Option[]) ); } else { - const updatedOptions = currentOptions.concat( - simpleSearch(propsOptions, query, {allowRegexSearch}) as Option[] + setOptions( + _.concat( + currentOptions, + simpleSearch(propsOptions, query) as Option[] + ) ); - setOptions(updatedOptions); } }, debounceTime || 400), - [allowRegexSearch, debounceTime, multiple, propsOptions, search, value] + [multiple, propsOptions, search, value, debounceTime] ); const firstRenderRef = useRef(true); diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse2/SmallRef.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse2/SmallRef.tsx index 40555a00f690..c72283097178 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse2/SmallRef.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse2/SmallRef.tsx @@ -125,6 +125,11 @@ export const SmallRef: FC<{ } const objectVersion = useObjectVersion(objVersionKey); const opVersion = useOpVersion(opVersionKey); + + const isDeleted = + isObjDeleteError(objectVersion?.error) || + isObjDeleteError(opVersion?.error); + const versionIndex = objectVersion.result?.versionIndex ?? opVersion.result?.versionIndex; @@ -140,7 +145,6 @@ export const SmallRef: FC<{ rootType = {type: 'OpDef'}; } const {label} = objectRefDisplayName(objRef, versionIndex); - const rootTypeName = getTypeName(rootType); let icon: IconName = IconNames.CubeContainer; if (rootTypeName === 'Dataset') { @@ -177,13 +181,14 @@ export const SmallRef: FC<{ overflow: 'hidden', whiteSpace: 'nowrap', textOverflow: 'ellipsis', + textDecoration: isDeleted ? 'line-through' : 'none', }}> {label} )} ); - if (refTypeQuery.loading) { + if (refTypeQuery.loading || isDeleted) { return Item; } if (!isArtifactRef && !isWeaveObjRef) { @@ -200,3 +205,12 @@ export const SmallRef: FC<{ ); }; + +export const isObjDeleteError = (e: any): boolean => { + if (e == null) { + return false; + } + const errorStr = String(e); + const regex = /Obj .* was deleted at .*/; + return regex.test(errorStr); +}; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewer.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewer.tsx index e8f39ad28802..251483665172 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewer.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewer.tsx @@ -7,6 +7,7 @@ import { GridRowId, } from '@mui/x-data-grid-pro'; import {Button} from '@wandb/weave/components/Button'; +import {parseRef} from '@wandb/weave/react'; import _ from 'lodash'; import React, { Dispatch, @@ -21,6 +22,7 @@ import React, { import {parseRefMaybe} from '../../../../../../react'; import {LoadingDots} from '../../../../../LoadingDots'; import {Browse2OpDefCode} from '../../../Browse2/Browse2OpDefCode'; +import {objectRefDisplayName} from '../../../Browse2/SmallRef'; import {isWeaveRef} from '../../filters/common'; import {StyledDataGrid} from '../../StyledDataGrid'; import { @@ -161,10 +163,17 @@ export const ObjectViewer = ({ const refValues: RefValues = {}; for (const [r, v] of _.zip(refs, resolvedRefData)) { - if (!r || !v) { + if (!r) { // Shouldn't be possible continue; } + if (!v) { + // Value for ref not found, probably deleted + refValues[r] = { + _weave_is_deleted_ref: objectRefDisplayName(parseRef(r)).label, + }; + continue; + } let val = r; if (v == null) { console.error('Error resolving ref', r); diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewerGroupingCell.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewerGroupingCell.tsx index 2aeb6309afaa..37e1a3d5f7bc 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewerGroupingCell.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewerGroupingCell.tsx @@ -32,7 +32,9 @@ export const ObjectViewerGroupingCell: FC< event.stopPropagation(); }; + const deletedRef = row.value?._weave_is_deleted_ref; const tooltipContent = row.path.toString(); + const textContent = deletedRef ?? props.value; const box = ( - {props.value} + {textContent} } /> diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx index 085587a64d8c..14aafb9893c4 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx @@ -12,6 +12,7 @@ import {CustomWeaveTypeProjectContext} from '../typeViews/CustomWeaveTypeDispatc import {WeaveCHTableSourceRefContext} from './CallPage/DataTableView'; import {ObjectViewerSection} from './CallPage/ObjectViewerSection'; import {WFHighLevelCallFilter} from './CallsPage/callsTableFilter'; +import {DeleteObjectButtonWithModal} from './common/DeleteModal'; import { CallLink, CallsLink, @@ -94,7 +95,9 @@ export const ObjectVersionPage: React.FC<{ path: props.filePath, refExtra: props.refExtra, }); - if (objectVersion.loading) { + if (objectVersion.error) { + return ; + } else if (objectVersion.loading) { return ; } else if (objectVersion.result == null) { return ; @@ -212,7 +215,7 @@ const ObjectVersionPageInner: React.FC<{ } headerContent={ -
+

Name

@@ -253,6 +256,9 @@ const ObjectVersionPageInner: React.FC<{

{refExtra}

)} +
+ +
} diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionPage.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionPage.tsx index 36f4e44afc5d..13c878f0c8c6 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionPage.tsx @@ -5,6 +5,7 @@ import {LoadingDots} from '../../../../LoadingDots'; import {Tailwind} from '../../../../Tailwind'; import {NotFoundPanel} from '../NotFoundPanel'; import {OpCodeViewer} from '../OpCodeViewer'; +import {DeleteObjectButtonWithModal} from './common/DeleteModal'; import { CallsLink, opNiceName, @@ -76,7 +77,7 @@ const OpVersionPageInner: React.FC<{ title={opVersionText(opId, versionIndex)} headerContent={ -
+

Name

@@ -138,6 +139,9 @@ const OpVersionPageInner: React.FC<{

-

)}
+
+ +
} diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/PlaygroundPage/llmMaxTokens.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/PlaygroundPage/llmMaxTokens.ts index 405d7f021775..1fbb329b3f0d 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/PlaygroundPage/llmMaxTokens.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/PlaygroundPage/llmMaxTokens.ts @@ -233,21 +233,6 @@ export const LLM_MAX_TOKENS = { max_tokens: 8191, supports_function_calling: false, }, - 'amazon.nova-micro-v1:0': { - provider: 'bedrock', - max_tokens: 4096, - supports_function_calling: true, - }, - 'amazon.nova-lite-v1:0': { - provider: 'bedrock', - max_tokens: 4096, - supports_function_calling: true, - }, - 'amazon.nova-pro-v1:0': { - provider: 'bedrock', - max_tokens: 4096, - supports_function_calling: true, - }, 'amazon.titan-text-lite-v1': { provider: 'bedrock', max_tokens: 4000, @@ -368,12 +353,6 @@ export const LLM_MAX_TOKENS = { max_tokens: 4096, supports_function_calling: true, }, - - 'xai/grok-beta': { - max_tokens: 131072, - provider: 'xai', - supports_function_calling: true, - }, }; export type LLMMaxTokensKey = keyof typeof LLM_MAX_TOKENS; @@ -388,7 +367,6 @@ export const LLM_PROVIDERS = [ 'gemini', 'groq', 'bedrock', - 'xai', ]; export const LLM_PROVIDER_LABELS: Record< @@ -400,5 +378,4 @@ export const LLM_PROVIDER_LABELS: Record< gemini: 'Gemini', groq: 'Groq', bedrock: 'Bedrock', - xai: 'xAI', }; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/PlaygroundPage/usePlaygroundState.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/PlaygroundPage/usePlaygroundState.ts index ec50825cc23f..cbcc7c52fb80 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/PlaygroundPage/usePlaygroundState.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/PlaygroundPage/usePlaygroundState.ts @@ -150,17 +150,14 @@ export const getInputFromPlaygroundState = (state: PlaygroundState) => { model: state.model, temperature: state.temperature, max_tokens: state.maxTokens, - stop: state.stopSequences.length > 0 ? state.stopSequences : undefined, + stop: state.stopSequences, top_p: state.topP, frequency_penalty: state.frequencyPenalty, presence_penalty: state.presencePenalty, n: state.nTimes, - response_format: - state.responseFormat === PlaygroundResponseFormats.Text - ? undefined - : { - type: state.responseFormat, - }, + response_format: { + type: state.responseFormat, + }, tools: tools.length > 0 ? tools : undefined, }; }; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx new file mode 100644 index 000000000000..063008145ed1 --- /dev/null +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx @@ -0,0 +1,174 @@ +import { + Dialog as MaterialDialog, + DialogActions as MaterialDialogActions, + DialogContent as MaterialDialogContent, + DialogTitle as MaterialDialogTitle, +} from '@material-ui/core'; +import {Button} from '@wandb/weave/components/Button'; +import {Tailwind} from '@wandb/weave/components/Tailwind'; +import React, {useState} from 'react'; +import styled from 'styled-components'; + +import {useClosePeek} from '../../context'; +import {useWFHooks} from '../wfReactInterface/context'; +import { + ObjectVersionSchema, + OpVersionSchema, +} from '../wfReactInterface/wfDataModelHooksInterface'; + +interface DeleteModalProps { + open: boolean; + onClose: () => void; + onDelete: () => Promise; + deleteTargetStr: string; + onSuccess?: () => void; +} + +const Dialog = styled(MaterialDialog)` + .MuiDialog-paper { + min-width: 400px; + max-width: min(800px, 90vw); + width: auto !important; + } +`; + +export const DeleteModal: React.FC = ({ + open, + onClose, + onDelete, + deleteTargetStr, + onSuccess, +}) => { + const [deleteLoading, setDeleteLoading] = useState(false); + const [error, setError] = useState(null); + + const handleDelete = () => { + setDeleteLoading(true); + onDelete() + .then(() => { + onClose(); + onSuccess?.(); + }) + .catch(err => { + setError(err.message); + }) + .finally(() => { + setDeleteLoading(false); + }); + }; + + return ( + { + onClose(); + setError(null); + }}> + + Delete {deleteTargetStr} + +
+ {error != null ? ( +

{error}

+ ) : ( +

Are you sure you want to delete?

+ )} +
+ {deleteTargetStr} + + + + + + + + ); +}; + +const DialogContent = styled(MaterialDialogContent)` + padding: 0 32px !important; +`; +DialogContent.displayName = 'S.DialogContent'; + +const DialogTitle = styled(MaterialDialogTitle)` + padding: 32px 32px 16px 32px !important; + + h2 { + font-weight: 600; + font-size: 24px; + line-height: 30px; + } +`; +DialogTitle.displayName = 'S.DialogTitle'; + +const DialogActions = styled(MaterialDialogActions)<{$align: string}>` + justify-content: ${({$align}) => + $align === 'left' ? 'flex-start' : 'flex-end'} !important; + padding: 32px 32px 32px 32px !important; +`; +DialogActions.displayName = 'S.DialogActions'; + +export const DeleteObjectButtonWithModal: React.FC<{ + objVersionSchema: OpVersionSchema | ObjectVersionSchema; + overrideDisplayStr?: string; +}> = ({objVersionSchema, overrideDisplayStr}) => { + const {useObjectDeleteFunc} = useWFHooks(); + const closePeek = useClosePeek(); + const {opVersionDelete, objectVersionDelete} = useObjectDeleteFunc(); + const [deleteModalOpen, setDeleteModalOpen] = useState(false); + + const doDelete = () => { + if (versionSchemaIsOp(objVersionSchema)) { + return opVersionDelete(objVersionSchema); + } + return objectVersionDelete(objVersionSchema); + }; + + const deleteStr = + overrideDisplayStr ?? makeDefaultObjectDeleteStr(objVersionSchema); + + return ( + <> +
- +
diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx index 5b7b1110a9f0..7e450d95e4a4 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx @@ -122,23 +122,14 @@ const DialogActions = styled(MaterialDialogActions)<{$align: string}>` DialogActions.displayName = 'S.DialogActions'; export const DeleteObjectButtonWithModal: React.FC<{ - objVersionSchema: OpVersionSchema | ObjectVersionSchema; + objVersionSchema: ObjectVersionSchema; overrideDisplayStr?: string; }> = ({objVersionSchema, overrideDisplayStr}) => { const {useObjectDeleteFunc} = useWFHooks(); const closePeek = useClosePeek(); - const {opVersionDelete, objectVersionDelete} = useObjectDeleteFunc(); + const {objectVersionDelete} = useObjectDeleteFunc(); const [deleteModalOpen, setDeleteModalOpen] = useState(false); - const doDelete = () => { - if (versionSchemaIsOp(objVersionSchema)) { - return opVersionDelete(objVersionSchema); - } else if (versionSchemaIsObject(objVersionSchema)) { - return objectVersionDelete(objVersionSchema); - } - throw new Error('Invalid version schema'); - }; - const deleteStr = overrideDisplayStr ?? makeDefaultObjectDeleteStr(objVersionSchema); @@ -153,32 +144,47 @@ export const DeleteObjectButtonWithModal: React.FC<{ open={deleteModalOpen} onClose={() => setDeleteModalOpen(false)} deleteTargetStr={deleteStr} - onDelete={doDelete} + onDelete={() => objectVersionDelete(objVersionSchema)} onSuccess={closePeek} /> ); }; -function versionSchemaIsOp( - objVersionSchema: OpVersionSchema | ObjectVersionSchema -): objVersionSchema is OpVersionSchema { - return 'opId' in objVersionSchema; -} +export const DeleteOpButtonWithModal: React.FC<{ + opVersionSchema: OpVersionSchema; + overrideDisplayStr?: string; +}> = ({opVersionSchema, overrideDisplayStr}) => { + const {useObjectDeleteFunc} = useWFHooks(); + const closePeek = useClosePeek(); + const {opVersionDelete} = useObjectDeleteFunc(); + const [deleteModalOpen, setDeleteModalOpen] = useState(false); -function versionSchemaIsObject( - objVersionSchema: OpVersionSchema | ObjectVersionSchema -): objVersionSchema is ObjectVersionSchema { - return 'objectId' in objVersionSchema; + const deleteStr = + overrideDisplayStr ?? makeDefaultOpDeleteStr(opVersionSchema); + + return ( + <> + - ) : undefined; + const viewer = userInfo ? userInfo.id : null; + const isReadonly = !viewer || !userInfo?.teams.includes(props.entity); return ( + } tabs={[ { label: '', @@ -138,6 +152,52 @@ export const ObjectVersionsPage: React.FC<{ ); }; +const ObjectVersionsPageHeaderExtra: React.FC<{ + entity: string; + project: string; + objectName: string | null; + selectedVersions: string[]; + showDeleteButton?: boolean; + showCompareButton?: boolean; + onCompare: () => void; +}> = ({ + entity, + project, + objectName, + selectedVersions, + showDeleteButton, + showCompareButton, + onCompare, +}) => { + const compareButton = showCompareButton ? ( + + ) : undefined; + + const deleteButton = showDeleteButton ? ( +
+ +
+ ) : undefined; + + return ( +
+ {compareButton} + {deleteButton} +
+ ); +}; + export type WFHighLevelObjectVersionFilter = { objectName?: string | null; baseObjectClass?: KnownBaseObjectClassType | null; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx index 7e450d95e4a4..9ee7685f0913 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx @@ -6,6 +6,10 @@ import { } from '@material-ui/core'; import {Button} from '@wandb/weave/components/Button'; import {Tailwind} from '@wandb/weave/components/Tailwind'; +import { + maybePluralize, + maybePluralizeWord, +} from '@wandb/weave/core/util/string'; import React, {useState} from 'react'; import styled from 'styled-components'; @@ -20,7 +24,8 @@ interface DeleteModalProps { open: boolean; onClose: () => void; onDelete: () => Promise; - deleteTargetStr: string; + deleteTitleStr: string; + deleteBodyStrs?: string[]; onSuccess?: () => void; } @@ -36,7 +41,8 @@ export const DeleteModal: React.FC = ({ open, onClose, onDelete, - deleteTargetStr, + deleteTitleStr, + deleteBodyStrs, onSuccess, }) => { const [deleteLoading, setDeleteLoading] = useState(false); @@ -57,6 +63,8 @@ export const DeleteModal: React.FC = ({ }); }; + const deleteBodyStrRes = deleteBodyStrs ? deleteBodyStrs : [deleteTitleStr]; + return ( = ({ setError(null); }}> - Delete {deleteTargetStr} + Delete {deleteTitleStr}
{error != null ? ( @@ -74,14 +82,20 @@ export const DeleteModal: React.FC = ({

Are you sure you want to delete?

)}
- {deleteTargetStr} + + {deleteBodyStrRes.map((str, i) => ( +
+ {str} +
+ ))} + ) : undefined; const deleteButton = showDeleteButton ? ( -
- -
+ ) : undefined; return ( -
- {compareButton} - {deleteButton} -
+ +
+ {compareButton} + {deleteButton} +
+ ); }; From 03b3910da5d846c1fff50343dca689624afaee80 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Thu, 19 Dec 2024 11:17:47 -0800 Subject: [PATCH 07/28] add max to show for multi delete --- .../Home/Browse3/pages/OpVersionsPage.tsx | 1 + .../Home/Browse3/pages/common/DeleteModal.tsx | 19 ++++++++++++++----- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionsPage.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionsPage.tsx index c5272f8a8d92..f2ddddb696a8 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionsPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionsPage.tsx @@ -13,6 +13,7 @@ import {LoadingDots} from '../../../../LoadingDots'; import {Timestamp} from '../../../../Timestamp'; import {StyledDataGrid} from '../StyledDataGrid'; import {basicField} from './common/DataTable'; +import {DeleteObjectVersionsButtonWithModal} from './common/DeleteModal'; import {Empty} from './common/Empty'; import {EMPTY_PROPS_OPERATIONS} from './common/EmptyContent'; import { diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx index 9ee7685f0913..adffde259215 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx @@ -20,6 +20,8 @@ import { OpVersionSchema, } from '../wfReactInterface/wfDataModelHooksInterface'; +const MAX_DELETE_ROWS_TO_SHOW = 10; + interface DeleteModalProps { open: boolean; onClose: () => void; @@ -83,11 +85,18 @@ export const DeleteModal: React.FC = ({ )}
- {deleteBodyStrRes.map((str, i) => ( -
- {str} -
- ))} + {deleteBodyStrRes + .slice(0, MAX_DELETE_ROWS_TO_SHOW) + .map((str, i) => ( +
+ {str} +
+ ))} + {deleteBodyStrRes.length > MAX_DELETE_ROWS_TO_SHOW && ( +

+ and {deleteBodyStrRes.length - MAX_DELETE_ROWS_TO_SHOW} more... +

+ )} From 9a16ad58f0b6460d9f9b48bda285931ff8d167fe Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Fri, 3 Jan 2025 13:19:24 -0800 Subject: [PATCH 08/28] lint --- .../Home/Browse3/pages/ObjectVersionsPage.tsx | 5 +---- .../Home/Browse3/pages/OpVersionsPage.tsx | 1 - .../Home/Browse3/pages/common/DeleteModal.tsx | 7 ++----- .../Browse3/pages/wfReactInterface/traceServerClient.ts | 8 ++++---- .../Browse3/pages/wfReactInterface/tsDataModelHooks.ts | 4 ++-- 5 files changed, 9 insertions(+), 16 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionsPage.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionsPage.tsx index 343565321f71..30de2d2fb109 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionsPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionsPage.tsx @@ -34,10 +34,7 @@ import { } from '../context'; import {StyledDataGrid} from '../StyledDataGrid'; import {basicField} from './common/DataTable'; -import { - DeleteObjectButtonWithModal, - DeleteObjectVersionsButtonWithModal, -} from './common/DeleteModal'; +import {DeleteObjectVersionsButtonWithModal} from './common/DeleteModal'; import {Empty} from './common/Empty'; import { EMPTY_PROPS_ACTION_SPECS, diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionsPage.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionsPage.tsx index f2ddddb696a8..c5272f8a8d92 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionsPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionsPage.tsx @@ -13,7 +13,6 @@ import {LoadingDots} from '../../../../LoadingDots'; import {Timestamp} from '../../../../Timestamp'; import {StyledDataGrid} from '../StyledDataGrid'; import {basicField} from './common/DataTable'; -import {DeleteObjectVersionsButtonWithModal} from './common/DeleteModal'; import {Empty} from './common/Empty'; import {EMPTY_PROPS_OPERATIONS} from './common/EmptyContent'; import { diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx index adffde259215..3016026bd9ba 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx @@ -6,10 +6,7 @@ import { } from '@material-ui/core'; import {Button} from '@wandb/weave/components/Button'; import {Tailwind} from '@wandb/weave/components/Tailwind'; -import { - maybePluralize, - maybePluralizeWord, -} from '@wandb/weave/core/util/string'; +import {maybePluralizeWord} from '@wandb/weave/core/util/string'; import React, {useState} from 'react'; import styled from 'styled-components'; @@ -25,7 +22,7 @@ const MAX_DELETE_ROWS_TO_SHOW = 10; interface DeleteModalProps { open: boolean; onClose: () => void; - onDelete: () => Promise; + onDelete: () => Promise<{detail: string} | void>; deleteTitleStr: string; deleteBodyStrs?: string[]; onSuccess?: () => void; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts index 7f4605339321..bf782cbd38be 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts @@ -140,12 +140,12 @@ export class TraceServerClient extends CachingTraceServerClient { public objectDelete( req: TraceObjDeleteReq ): Promise { - const res = super.objectDelete(req).then(res => { - if (res && 'detail' in res) { - throw new Error(res.detail); + const res = super.objectDelete(req).then(r => { + if (r && 'detail' in r) { + throw new Error(r.detail); } this.onObjectListeners.forEach(listener => listener()); - return res; + return r; }); return res; } diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts index f168e5961cd8..4fe787148a9c 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts @@ -1165,8 +1165,8 @@ const useObjectDeleteFunc = () => { }); return getTsClient().objectDelete({ project_id: projectIdFromParts({ - entity: entity, - project: project, + entity, + project, }), object_id: objectId, digests, From 77ffe64f424b276ec5cfd0ec2a9d54c74dfd72fc Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Mon, 6 Jan 2025 08:57:07 -0800 Subject: [PATCH 09/28] clean --- .../PagePanelComponents/Home/Browse2/SmallRef.tsx | 11 ++--------- .../Home/Browse3/pages/wfReactInterface/utilities.ts | 9 +++++++++ 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse2/SmallRef.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse2/SmallRef.tsx index c72283097178..6f7075514acb 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse2/SmallRef.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse2/SmallRef.tsx @@ -13,6 +13,7 @@ import {Icon, IconName, IconNames} from '../../../Icon'; import {useWeaveflowRouteContext} from '../Browse3/context'; import {Link} from '../Browse3/pages/common/Links'; import {useWFHooks} from '../Browse3/pages/wfReactInterface/context'; +import {isObjDeleteError} from '../Browse3/pages/wfReactInterface/utilities'; import { ObjectVersionKey, OpVersionKey, @@ -145,6 +146,7 @@ export const SmallRef: FC<{ rootType = {type: 'OpDef'}; } const {label} = objectRefDisplayName(objRef, versionIndex); + const rootTypeName = getTypeName(rootType); let icon: IconName = IconNames.CubeContainer; if (rootTypeName === 'Dataset') { @@ -205,12 +207,3 @@ export const SmallRef: FC<{ ); }; - -export const isObjDeleteError = (e: any): boolean => { - if (e == null) { - return false; - } - const errorStr = String(e); - const regex = /Obj .* was deleted at .*/; - return regex.test(errorStr); -}; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/utilities.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/utilities.ts index 1e27e2c5c00e..af2ec8052865 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/utilities.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/utilities.ts @@ -287,6 +287,15 @@ export const objectVersionNiceString = (ov: ObjectVersionSchema) => { return result; }; +export const isObjDeleteError = (e: any): boolean => { + if (e == null) { + return false; + } + const errorStr = String(e); + const regex = /Obj .* was deleted at .*/; + return regex.test(errorStr); +}; + /// Hooks /// export const useParentCall = ( From eea58b0dbe2510db8cf5c0b17b4251a02e8f9ef8 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Mon, 6 Jan 2025 10:59:18 -0800 Subject: [PATCH 10/28] refactor for clarity --- .../Home/Browse3/pages/ObjectVersionPage.tsx | 37 ++++- .../Home/Browse3/pages/ObjectVersionsPage.tsx | 38 +++++- .../Home/Browse3/pages/OpVersionPage.tsx | 37 ++++- .../Home/Browse3/pages/common/DeleteModal.tsx | 128 ++---------------- 4 files changed, 116 insertions(+), 124 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx index 14aafb9893c4..a2b9cd3db69c 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx @@ -1,18 +1,20 @@ import Box from '@mui/material/Box'; +import {Button} from '@wandb/weave/components/Button'; import {useObjectViewEvent} from '@wandb/weave/integrations/analytics/useViewEvents'; -import React, {useMemo} from 'react'; +import React, {useMemo, useState} from 'react'; import {maybePluralizeWord} from '../../../../../core/util/string'; import {Icon, IconName} from '../../../../Icon'; import {LoadingDots} from '../../../../LoadingDots'; import {Tailwind} from '../../../../Tailwind'; import {Tooltip} from '../../../../Tooltip'; +import {useClosePeek} from '../context'; import {NotFoundPanel} from '../NotFoundPanel'; import {CustomWeaveTypeProjectContext} from '../typeViews/CustomWeaveTypeDispatcher'; import {WeaveCHTableSourceRefContext} from './CallPage/DataTableView'; import {ObjectViewerSection} from './CallPage/ObjectViewerSection'; import {WFHighLevelCallFilter} from './CallsPage/callsTableFilter'; -import {DeleteObjectButtonWithModal} from './common/DeleteModal'; +import {DeleteModal} from './common/DeleteModal'; import { CallLink, CallsLink, @@ -631,3 +633,34 @@ const OpVersionCallsLink: React.FC<{ ); }; + +const DeleteObjectButtonWithModal: React.FC<{ + objVersionSchema: ObjectVersionSchema; + overrideDisplayStr?: string; +}> = ({objVersionSchema, overrideDisplayStr}) => { + const {useObjectDeleteFunc} = useWFHooks(); + const closePeek = useClosePeek(); + const {objectVersionDelete} = useObjectDeleteFunc(); + const [deleteModalOpen, setDeleteModalOpen] = useState(false); + + const deleteStr = + overrideDisplayStr ?? + `${objVersionSchema.objectId}:v${objVersionSchema.versionIndex}`; + + return ( + <> +
)}
- + {showDeleteButton && ( + + )}
diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionsPage.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionsPage.tsx index 35f84ef6f753..244300aa1c07 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionsPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionsPage.tsx @@ -115,6 +115,7 @@ export const ObjectVersionsPage: React.FC<{ const hasComparison = filter.objectName != null; const viewer = userInfo ? userInfo.id : null; const isReadonly = !viewer || !userInfo?.teams.includes(props.entity); + const isAdmin = userInfo?.admin; return ( diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionPage.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionPage.tsx index 9664ffefde7d..105689875b7f 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionPage.tsx @@ -7,7 +7,7 @@ import {Tailwind} from '../../../../Tailwind'; import {useClosePeek} from '../context'; import {NotFoundPanel} from '../NotFoundPanel'; import {OpCodeViewer} from '../OpCodeViewer'; -import {DeleteModal} from './common/DeleteModal'; +import {DeleteModal, useShowDeleteButton} from './common/DeleteModal'; import { CallsLink, opNiceName, @@ -73,6 +73,7 @@ const OpVersionPageInner: React.FC<{ // that data available yet. return true; }, []); + const showDeleteButton = useShowDeleteButton(); return (
- + {showDeleteButton && ( + + )}
diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx index eeab9f912c53..d3ae8f6f3792 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx @@ -4,6 +4,7 @@ import { DialogContent as MaterialDialogContent, DialogTitle as MaterialDialogTitle, } from '@material-ui/core'; +import {useViewerInfo} from '@wandb/weave/common/hooks/useViewerInfo'; import {Button} from '@wandb/weave/components/Button'; import {Tailwind} from '@wandb/weave/components/Tailwind'; import React, {useState} from 'react'; @@ -102,6 +103,11 @@ export const DeleteModal: React.FC = ({ ); }; +export const useShowDeleteButton = () => { + const viewerInfo = useViewerInfo(); + return viewerInfo.loading ? false : viewerInfo.userInfo?.admin; +}; + const Dialog = styled(MaterialDialog)` .MuiDialog-paper { min-width: 400px; From a58565d255012bee442721efed99ff139bf9431a Mon Sep 17 00:00:00 2001 From: Griffin Tarpenning Date: Mon, 6 Jan 2025 13:17:30 -0800 Subject: [PATCH 12/28] chore(weave): soft deletion for objects server (#3259) --- tests/trace/test_obj_delete.py | 260 ++++++++++++++++++ tests/trace/test_objects_query_builder.py | 21 +- weave/trace_server/clickhouse_schema.py | 6 + .../clickhouse_trace_server_batched.py | 110 +++++++- weave/trace_server/errors.py | 12 + ...ternal_to_internal_trace_server_adapter.py | 4 + weave/trace_server/objects_query_builder.py | 28 +- weave/trace_server/sqlite_trace_server.py | 181 +++++++++--- weave/trace_server/trace_server_interface.py | 14 + .../remote_http_trace_server.py | 5 + 10 files changed, 588 insertions(+), 53 deletions(-) create mode 100644 tests/trace/test_obj_delete.py diff --git a/tests/trace/test_obj_delete.py b/tests/trace/test_obj_delete.py new file mode 100644 index 000000000000..a2e1c3ad3b07 --- /dev/null +++ b/tests/trace/test_obj_delete.py @@ -0,0 +1,260 @@ +import pytest + +import weave +from weave.trace.weave_client import WeaveClient +from weave.trace_server import trace_server_interface as tsi + + +def _objs_query(client: WeaveClient, object_id: str) -> list[tsi.ObjSchema]: + objs = client.server.objs_query( + tsi.ObjQueryReq( + project_id=client._project_id(), + filter=tsi.ObjectVersionFilter(object_ids=[object_id]), + sort_by=[tsi.SortBy(field="created_at", direction="asc")], + ) + ) + return objs.objs + + +def _obj_delete(client: WeaveClient, object_id: str, digests: list[str]) -> int: + return client.server.obj_delete( + tsi.ObjDeleteReq( + project_id=client._project_id(), + object_id=object_id, + digests=digests, + ) + ).num_deleted + + +def test_delete_object_versions(client: WeaveClient): + v0 = weave.publish({"i": 1}, name="obj_1") + v1 = weave.publish({"i": 2}, name="obj_1") + v2 = weave.publish({"i": 3}, name="obj_1") + + objs = _objs_query(client, "obj_1") + assert len(objs) == 3 + + num_deleted = _obj_delete(client, "obj_1", [v0.digest]) + assert num_deleted == 1 + + objs = _objs_query(client, "obj_1") + assert len(objs) == 2 + + # test deleting an already deleted digest + with pytest.raises(weave.trace_server.errors.NotFoundError): + _obj_delete(client, "obj_1", [v0.digest]) + + # test deleting multiple digests + digests = [v1.digest, v2.digest] + num_deleted = _obj_delete(client, "obj_1", digests) + assert num_deleted == 2 + + objs = _objs_query(client, "obj_1") + assert len(objs) == 0 + + +def test_delete_all_object_versions(client: WeaveClient): + weave.publish({"i": 1}, name="obj_1") + weave.publish({"i": 2}, name="obj_1") + weave.publish({"i": 3}, name="obj_1") + + num_deleted = _obj_delete(client, "obj_1", None) + assert num_deleted == 3 + + objs = _objs_query(client, "obj_1") + assert len(objs) == 0 + + with pytest.raises(weave.trace_server.errors.NotFoundError): + _obj_delete(client, "obj_1", None) + + +def test_delete_version_correctness(client: WeaveClient): + v0 = weave.publish({"i": 1}, name="obj_1") + v1 = weave.publish({"i": 2}, name="obj_1") + v2 = weave.publish({"i": 3}, name="obj_1") + + _obj_delete(client, "obj_1", [v1.digest]) + objs = _objs_query(client, "obj_1") + assert len(objs) == 2 + assert objs[0].digest == v0.digest + assert objs[0].val == {"i": 1} + assert objs[0].version_index == 0 + assert objs[1].digest == v2.digest + assert objs[1].val == {"i": 3} + assert objs[1].version_index == 2 + + v3 = weave.publish({"i": 4}, name="obj_1") + objs = _objs_query(client, "obj_1") + assert len(objs) == 3 + assert objs[0].digest == v0.digest + assert objs[0].val == {"i": 1} + assert objs[0].version_index == 0 + assert objs[1].digest == v2.digest + assert objs[1].val == {"i": 3} + assert objs[1].version_index == 2 + assert objs[2].digest == v3.digest + assert objs[2].val == {"i": 4} + assert objs[2].version_index == 3 + + _obj_delete(client, "obj_1", [v3.digest]) + objs = _objs_query(client, "obj_1") + assert len(objs) == 2 + assert objs[0].digest == v0.digest + assert objs[0].val == {"i": 1} + assert objs[0].version_index == 0 + assert objs[1].digest == v2.digest + assert objs[1].val == {"i": 3} + assert objs[1].version_index == 2 + + +def test_delete_object_max_limit(client: WeaveClient): + # Create more than MAX_OBJECTS_TO_DELETE objects + max_objs = 100 + digests = [] + for i in range(max_objs + 1): + digests.append(f"test_{i}") + + with pytest.raises( + ValueError, match=f"Please delete {max_objs} or fewer objects at a time" + ): + _obj_delete(client, "obj_1", digests) + + +def test_delete_nonexistent_object_id(client: WeaveClient): + with pytest.raises(weave.trace_server.errors.NotFoundError): + _obj_delete(client, "nonexistent_obj", None) + + +def test_delete_mixed_valid_invalid_digests(client: WeaveClient): + v0 = weave.publish({"i": 1}, name="obj_1") + v1 = weave.publish({"i": 2}, name="obj_1") + + invalid_digests = [v0.digest, "invalid-digest", v1.digest] + with pytest.raises( + weave.trace_server.errors.NotFoundError, + match="Delete request contains 3 digests, but found 2 objects to delete. Diff digests: {'invalid-digest'}", + ): + _obj_delete(client, "obj_1", invalid_digests) + + +def test_delete_duplicate_digests(client: WeaveClient): + v0 = weave.publish({"i": 1}, name="obj_1") + + num_deleted = _obj_delete(client, "obj_1", [v0.digest, v0.digest]) + assert num_deleted == 1 + + +def test_delete_with_digest_aliases(client: WeaveClient): + v0 = weave.publish({"i": 1}, name="obj_1") + weave.publish({"i": 2}, name="obj_1") + + num_deleted = _obj_delete(client, "obj_1", ["latest"]) + assert num_deleted == 1 + + objs = _objs_query(client, "obj_1") + assert len(objs) == 1 + assert objs[0].digest == v0.digest + assert objs[0].val == {"i": 1} + + num_deleted = _obj_delete(client, "obj_1", ["v0"]) + assert num_deleted == 1 + + objs = _objs_query(client, "obj_1") + assert len(objs) == 0 + + +def test_delete_and_recreate_object(client: WeaveClient): + # Create and delete initial object + v0 = weave.publish({"i": 1}, name="obj_1") + _obj_delete(client, "obj_1", [v0.digest]) + + # Create new object with same ID + v1 = weave.publish({"i": 2}, name="obj_1") + + objs = _objs_query(client, "obj_1") + assert len(objs) == 1 + assert objs[0].digest == v1.digest + assert objs[0].val == {"i": 2} + + +def test_read_deleted_object(client: WeaveClient): + weave.publish({"i": 1}, name="obj_1") + weave.publish({"i": 2}, name="obj_1") + obj1_v2 = weave.publish({"i": 3}, name="obj_1") + + _obj_delete(client, "obj_1", [obj1_v2.digest]) + + with pytest.raises(weave.trace_server.errors.ObjectDeletedError): + client.server.obj_read( + tsi.ObjReadReq( + project_id=client._project_id(), + object_id="obj_1", + digest=obj1_v2.digest, + ) + ) + + with pytest.raises(weave.trace_server.errors.NotFoundError): + client.server.refs_read_batch( + tsi.RefsReadBatchReq( + project_id=client._project_id(), + object_ids=["obj_1"], + refs=[obj1_v2.uri()], + ) + ) + + +def test_op_versions(client: WeaveClient): + @weave.op + def my_op(x: int) -> int: + return x + 1 + + my_op(1) + my_op(2) + + @weave.op() + def my_op(x: int, y: int) -> int: + return x + y + + my_op(1, 2) + + objs = _objs_query(client, "my_op") + assert len(objs) == 2 + + _obj_delete(client, "my_op", [objs[0].digest]) + + objs2 = _objs_query(client, "my_op") + assert len(objs2) == 1 + assert objs2[0].version_index == 1 + + _obj_delete(client, "my_op", ["latest"]) + + objs3 = _objs_query(client, "my_op") + assert len(objs3) == 0 + + +def test_read_deleted_op(client: WeaveClient): + @weave.op + def my_op(x: int) -> int: + return x + 1 + + op_ref = weave.publish(my_op, name="my_op") + + _obj_delete(client, "my_op", [op_ref.digest]) + + with pytest.raises(weave.trace_server.errors.ObjectDeletedError): + client.server.obj_read( + tsi.ObjReadReq( + project_id=client._project_id(), + object_id="my_op", + digest=op_ref.digest, + ) + ) + + with pytest.raises(weave.trace_server.errors.NotFoundError): + client.server.refs_read_batch( + tsi.RefsReadBatchReq( + project_id=client._project_id(), + object_ids=["my_op"], + refs=[op_ref.uri()], + ) + ) diff --git a/tests/trace/test_objects_query_builder.py b/tests/trace/test_objects_query_builder.py index 77662655b195..58321ad7ec6e 100644 --- a/tests/trace/test_objects_query_builder.py +++ b/tests/trace/test_objects_query_builder.py @@ -146,6 +146,7 @@ def test_object_query_builder_sort(): digest, version_index, is_latest, + deleted_at, version_count, is_op FROM ( @@ -153,6 +154,7 @@ def test_object_query_builder_sort(): project_id, object_id, created_at, + deleted_at, kind, base_object_class, refs, @@ -164,13 +166,20 @@ def test_object_query_builder_sort(): object_id ORDER BY created_at ASC ) - 1 AS version_index, - count(*) OVER (PARTITION BY project_id, kind, object_id) as version_count, - if(version_index + 1 = version_count, 1, 0) AS is_latest + count(*) OVER ( + PARTITION BY project_id, kind, object_id + ) as version_count, + row_number() OVER ( + PARTITION BY project_id, kind, object_id + ORDER BY (deleted_at IS NULL) DESC, created_at DESC + ) AS row_num, + if (row_num = 1, 1, 0) AS is_latest FROM ( SELECT project_id, object_id, created_at, + deleted_at, kind, base_object_class, refs, @@ -198,7 +207,8 @@ def test_object_query_builder_metadata_query_basic(): ) WHERE rn = 1 ) -WHERE is_latest = 1""" +WHERE ((is_latest = 1) AND (deleted_at IS NULL)) +ORDER BY created_at ASC""" assert query == expected_query assert parameters == {"project_id": "test_project"} @@ -225,7 +235,7 @@ def test_object_query_builder_metadata_query_with_limit_offset_sort(): ) WHERE rn = 1 ) -WHERE ((((digest = {{version_digest_0: String}}) OR (digest = {{version_digest_1: String}}) OR (version_index = {{version_index_2: Int64}}))) AND (base_object_class IN {{base_object_classes: Array(String)}})) +WHERE ((((digest = {{version_digest_0: String}}) OR (digest = {{version_digest_1: String}}) OR (version_index = {{version_index_2: Int64}}))) AND (base_object_class IN {{base_object_classes: Array(String)}}) AND (deleted_at IS NULL)) ORDER BY created_at DESC LIMIT 10 OFFSET 5""" @@ -255,7 +265,8 @@ def test_objects_query_metadata_op(): ) WHERE rn = 1 ) -WHERE ((is_op = 1) AND (version_index = {{version_index_0: Int64}}))""" +WHERE ((is_op = 1) AND (version_index = {{version_index_0: Int64}}) AND (deleted_at IS NULL)) +ORDER BY created_at ASC""" assert query == expected_query assert parameters == { diff --git a/weave/trace_server/clickhouse_schema.py b/weave/trace_server/clickhouse_schema.py index c02b5f7b3320..ac6507059fe4 100644 --- a/weave/trace_server/clickhouse_schema.py +++ b/weave/trace_server/clickhouse_schema.py @@ -136,6 +136,11 @@ class ObjCHInsertable(BaseModel): _refs = field_validator("refs")(validation.refs_list_validator) +class ObjDeleteCHInsertable(ObjCHInsertable): + deleted_at: datetime.datetime + created_at: datetime.datetime + + class SelectableCHObjSchema(BaseModel): project_id: str object_id: str @@ -147,3 +152,4 @@ class SelectableCHObjSchema(BaseModel): digest: str version_index: int is_latest: int + deleted_at: Optional[datetime.datetime] = None diff --git a/weave/trace_server/clickhouse_trace_server_batched.py b/weave/trace_server/clickhouse_trace_server_batched.py index e97e020b1abe..c4800df64a3c 100644 --- a/weave/trace_server/clickhouse_trace_server_batched.py +++ b/weave/trace_server/clickhouse_trace_server_batched.py @@ -59,6 +59,7 @@ CallStartCHInsertable, CallUpdateCHInsertable, ObjCHInsertable, + ObjDeleteCHInsertable, SelectableCHCallSchema, SelectableCHObjSchema, ) @@ -68,6 +69,8 @@ InsertTooLarge, InvalidRequest, MissingLLMApiKeyError, + NotFoundError, + ObjectDeletedError, RequestTooLarge, ) from weave.trace_server.feedback import ( @@ -129,10 +132,6 @@ MAX_CALLS_STREAM_BATCH_SIZE = 500 -class NotFoundError(Exception): - pass - - CallCHInsertable = Union[ CallStartCHInsertable, CallEndCHInsertable, @@ -533,12 +532,19 @@ def op_read(self, req: tsi.OpReadReq) -> tsi.OpReadRes: object_query_builder.add_is_op_condition(True) object_query_builder.add_digests_conditions(req.digest) object_query_builder.add_object_ids_condition([req.name], "op_name") + object_query_builder.set_include_deleted(include_deleted=True) objs = self._select_objs_query(object_query_builder) if len(objs) == 0: raise NotFoundError(f"Obj {req.name}:{req.digest} not found") - return tsi.OpReadRes(op_obj=_ch_obj_to_obj_schema(objs[0])) + op = objs[0] + if op.deleted_at is not None: + raise ObjectDeletedError( + f"Op {req.name}:v{op.version_index} was deleted at {op.deleted_at}" + ) + + return tsi.OpReadRes(op_obj=_ch_obj_to_obj_schema(op)) def ops_query(self, req: tsi.OpQueryReq) -> tsi.OpQueryRes: object_query_builder = ObjectMetadataQueryBuilder(req.project_id) @@ -584,12 +590,19 @@ def obj_read(self, req: tsi.ObjReadReq) -> tsi.ObjReadRes: object_query_builder = ObjectMetadataQueryBuilder(req.project_id) object_query_builder.add_digests_conditions(req.digest) object_query_builder.add_object_ids_condition([req.object_id]) + object_query_builder.set_include_deleted(include_deleted=True) objs = self._select_objs_query(object_query_builder) if len(objs) == 0: raise NotFoundError(f"Obj {req.object_id}:{req.digest} not found") - return tsi.ObjReadRes(obj=_ch_obj_to_obj_schema(objs[0])) + obj = objs[0] + if obj.deleted_at is not None: + raise ObjectDeletedError( + f"{req.object_id}:v{obj.version_index} was deleted at {obj.deleted_at}" + ) + + return tsi.ObjReadRes(obj=_ch_obj_to_obj_schema(obj)) def objs_query(self, req: tsi.ObjQueryReq) -> tsi.ObjQueryRes: object_query_builder = ObjectMetadataQueryBuilder(req.project_id) @@ -617,11 +630,77 @@ def objs_query(self, req: tsi.ObjQueryReq) -> tsi.ObjQueryRes: for sort in req.sort_by: object_query_builder.add_order(sort.field, sort.direction) metadata_only = req.metadata_only or False - + object_query_builder.set_include_deleted(include_deleted=False) objs = self._select_objs_query(object_query_builder, metadata_only) return tsi.ObjQueryRes(objs=[_ch_obj_to_obj_schema(obj) for obj in objs]) + def obj_delete(self, req: tsi.ObjDeleteReq) -> tsi.ObjDeleteRes: + """ + Delete object versions by digest, belonging to given object_id. + All deletion in this method is "soft". Deletion occurs by inserting + a new row into the object_versions table with the deleted_at field set. + Inserted rows share identical primary keys (order by) with original rows, + and will be combined by the ReplacingMergeTree engine at database merge + time. + If no digests are provided all versions will have their data overwritten with + an empty val_dump. + """ + MAX_OBJECTS_TO_DELETE = 100 + if req.digests and len(req.digests) > MAX_OBJECTS_TO_DELETE: + raise ValueError( + f"Object delete request contains {len(req.digests)} objects. Please delete {MAX_OBJECTS_TO_DELETE} or fewer objects at a time." + ) + + object_query_builder = ObjectMetadataQueryBuilder(req.project_id) + object_query_builder.add_object_ids_condition([req.object_id]) + metadata_only = True + if req.digests: + object_query_builder.add_digests_conditions(*req.digests) + metadata_only = False + + object_versions = self._select_objs_query(object_query_builder, metadata_only) + + delete_insertables = [] + now = datetime.datetime.now(datetime.timezone.utc) + for obj in object_versions: + original_created_at = _ensure_datetimes_have_tz_strict(obj.created_at) + delete_insertables.append( + ObjDeleteCHInsertable( + project_id=obj.project_id, + object_id=obj.object_id, + digest=obj.digest, + kind=obj.kind, + val_dump=obj.val_dump, + refs=obj.refs, + base_object_class=obj.base_object_class, + deleted_at=now, + # Keep the original created_at timestamp + created_at=original_created_at, + ) + ) + + if len(delete_insertables) == 0: + raise NotFoundError( + f"Object {req.object_id} ({req.digests}) not found when deleting." + ) + + if req.digests: + given_digests = set(req.digests) + found_digests = {obj.digest for obj in delete_insertables} + if len(given_digests) != len(found_digests): + raise NotFoundError( + f"Delete request contains {len(req.digests)} digests, but found {len(found_digests)} objects to delete. Diff digests: {given_digests - found_digests}" + ) + + data = [list(obj.model_dump().values()) for obj in delete_insertables] + column_names = list(delete_insertables[0].model_fields.keys()) + + self._insert("object_versions", data=data, column_names=column_names) + num_deleted = len(delete_insertables) + + return tsi.ObjDeleteRes(num_deleted=num_deleted) + def table_create(self, req: tsi.TableCreateReq) -> tsi.TableCreateRes: insert_rows = [] for r in req.table.rows: @@ -927,6 +1006,7 @@ def get_object_refs_root_val( conds: list[str] = [] object_id_conds: list[str] = [] parameters: dict[str, Union[str, int]] = {} + ref_digests: set[str] = set() for ref_index, ref in enumerate(refs): if ref.version == "latest": @@ -951,7 +1031,7 @@ def get_object_refs_root_val( object_id_conds.append(f"object_id = {{{object_id_param_key}: String}}") parameters[object_id_param_key] = ref.name parameters[version_param_key] = ref.version - + ref_digests.add(ref.version) if len(conds) > 0: conditions = [combine_conditions(conds, "OR")] object_id_conditions = [combine_conditions(object_id_conds, "OR")] @@ -962,6 +1042,11 @@ def get_object_refs_root_val( parameters=parameters, ) objs = self._select_objs_query(object_query_builder) + found_digests = {obj.digest for obj in objs} + if len(ref_digests) != len(found_digests): + raise NotFoundError( + f"Ref read contains {len(ref_digests)} digests, but found {len(found_digests)} objects. Diff digests: {ref_digests - found_digests}" + ) for obj in objs: root_val_cache[make_obj_cache_key(obj)] = json.loads(obj.val_dump) @@ -1777,6 +1862,15 @@ def _ensure_datetimes_have_tz( return dt +def _ensure_datetimes_have_tz_strict( + dt: datetime.datetime, +) -> datetime.datetime: + res = _ensure_datetimes_have_tz(dt) + if res is None: + raise ValueError(f"Datetime is None: {dt}") + return res + + def _nullable_dict_dump_to_dict( val: Optional[str], ) -> Optional[dict[str, Any]]: diff --git a/weave/trace_server/errors.py b/weave/trace_server/errors.py index b2014fc1a48d..06ffebe1feab 100644 --- a/weave/trace_server/errors.py +++ b/weave/trace_server/errors.py @@ -34,3 +34,15 @@ class MissingLLMApiKeyError(Error): def __init__(self, message: str, api_key_name: str): self.api_key_name = api_key_name super().__init__(message) + + +class NotFoundError(Error): + """Raised when a general not found error occurs.""" + + pass + + +class ObjectDeletedError(Error): + """Raised when an object has been deleted.""" + + pass diff --git a/weave/trace_server/external_to_internal_trace_server_adapter.py b/weave/trace_server/external_to_internal_trace_server_adapter.py index 1df739adbcdd..7718295d9d34 100644 --- a/weave/trace_server/external_to_internal_trace_server_adapter.py +++ b/weave/trace_server/external_to_internal_trace_server_adapter.py @@ -257,6 +257,10 @@ def objs_query(self, req: tsi.ObjQueryReq) -> tsi.ObjQueryRes: obj.project_id = original_project_id return res + def obj_delete(self, req: tsi.ObjDeleteReq) -> tsi.ObjDeleteRes: + req.project_id = self._idc.ext_to_int_project_id(req.project_id) + return self._ref_apply(self._internal_trace_server.obj_delete, req) + def table_create(self, req: tsi.TableCreateReq) -> tsi.TableCreateRes: req.table.project_id = self._idc.ext_to_int_project_id(req.table.project_id) return self._ref_apply(self._internal_trace_server.table_create, req) diff --git a/weave/trace_server/objects_query_builder.py b/weave/trace_server/objects_query_builder.py index a5675c4b5c10..b6a5f560ac5d 100644 --- a/weave/trace_server/objects_query_builder.py +++ b/weave/trace_server/objects_query_builder.py @@ -18,7 +18,8 @@ "digest", "version_index", "is_latest", - # columns not used in SelectableCHObjSchema(): + "deleted_at", + # columns not used in SelectableCHObjSchema: "version_count", "is_op", ] @@ -102,21 +103,25 @@ def __init__( conditions: Optional[list[str]] = None, object_id_conditions: Optional[list[str]] = None, parameters: Optional[dict[str, Any]] = None, + include_deleted: bool = False, ): self.project_id = project_id self.parameters: dict[str, Any] = parameters or {} if not self.parameters.get(project_id): self.parameters.update({"project_id": project_id}) - self._conditions: list[str] = conditions or [] self._object_id_conditions: list[str] = object_id_conditions or [] self._limit: Optional[int] = None self._offset: Optional[int] = None self._sort_by: list[tsi.SortBy] = [] + self._include_deleted: bool = include_deleted @property def conditions_part(self) -> str: - return _make_conditions_part(self._conditions) + _conditions = list(self._conditions) + if not self._include_deleted: + _conditions.append("deleted_at IS NULL") + return _make_conditions_part(_conditions) @property def object_id_conditions_part(self) -> str: @@ -124,6 +129,8 @@ def object_id_conditions_part(self) -> str: @property def sort_part(self) -> str: + if not self._sort_by: + return "ORDER BY created_at ASC" return _make_sort_part(self._sort_by) @property @@ -230,6 +237,9 @@ def set_offset(self, offset: int) -> None: raise ValueError("Offset can only be set once") self._offset = offset + def set_include_deleted(self, include_deleted: bool) -> None: + self._include_deleted = include_deleted + def make_metadata_query(self) -> str: columns = ",\n ".join(OBJECT_METADATA_COLUMNS) query = f""" @@ -240,6 +250,7 @@ def make_metadata_query(self) -> str: project_id, object_id, created_at, + deleted_at, kind, base_object_class, refs, @@ -251,13 +262,20 @@ def make_metadata_query(self) -> str: object_id ORDER BY created_at ASC ) - 1 AS version_index, - count(*) OVER (PARTITION BY project_id, kind, object_id) as version_count, - if(version_index + 1 = version_count, 1, 0) AS is_latest + count(*) OVER ( + PARTITION BY project_id, kind, object_id + ) as version_count, + row_number() OVER ( + PARTITION BY project_id, kind, object_id + ORDER BY (deleted_at IS NULL) DESC, created_at DESC + ) AS row_num, + if (row_num = 1, 1, 0) AS is_latest FROM ( SELECT project_id, object_id, created_at, + deleted_at, kind, base_object_class, refs, diff --git a/weave/trace_server/sqlite_trace_server.py b/weave/trace_server/sqlite_trace_server.py index 41d0e5813902..59bd19580e82 100644 --- a/weave/trace_server/sqlite_trace_server.py +++ b/weave/trace_server/sqlite_trace_server.py @@ -15,7 +15,11 @@ from weave.trace_server import refs_internal as ri from weave.trace_server import trace_server_interface as tsi from weave.trace_server.emoji_util import detone_emojis -from weave.trace_server.errors import InvalidRequest +from weave.trace_server.errors import ( + InvalidRequest, + NotFoundError, + ObjectDeletedError, +) from weave.trace_server.feedback import ( TABLE_FEEDBACK, validate_feedback_create_req, @@ -47,10 +51,6 @@ MAX_FLUSH_AGE = 15 -class NotFoundError(Exception): - pass - - _conn_cursor: contextvars.ContextVar[ Optional[tuple[sqlite3.Connection, sqlite3.Cursor]] ] = contextvars.ContextVar("conn_cursor", default=None) @@ -617,25 +617,18 @@ def obj_create(self, req: tsi.ObjCreateReq) -> tsi.ObjCreateRes: processed_val = processed_result["val"] json_val = json.dumps(processed_val) digest = str_digest(json_val) + project_id, object_id = req.obj.project_id, req.obj.object_id # Validate - object_id_validator(req.obj.object_id) + object_id_validator(object_id) - # TODO: version index isn't right here, what if we delete stuff? with self.lock: - cursor.execute("BEGIN TRANSACTION") - # Mark all existing objects with such id as not latest - cursor.execute( - """UPDATE objects SET is_latest = 0 WHERE project_id = ? AND object_id = ?""", - (req.obj.project_id, req.obj.object_id), - ) - # first get version count - cursor.execute( - """SELECT COUNT(*) FROM objects WHERE project_id = ? AND object_id = ?""", - (req.obj.project_id, req.obj.object_id), - ) - version_index = cursor.fetchone()[0] + if self._obj_exists(cursor, project_id, object_id, digest): + return tsi.ObjCreateRes(digest=digest) + cursor.execute("BEGIN TRANSACTION") + self._mark_existing_objects_as_not_latest(cursor, project_id, object_id) + version_index = self._get_obj_version_index(cursor, project_id, object_id) cursor.execute( """INSERT OR IGNORE INTO objects ( project_id, @@ -647,11 +640,22 @@ def obj_create(self, req: tsi.ObjCreateReq) -> tsi.ObjCreateRes: val_dump, digest, version_index, - is_latest - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)""", + is_latest, + deleted_at + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + ON CONFLICT(project_id, kind, object_id, digest) DO UPDATE SET + created_at = excluded.created_at, + kind = excluded.kind, + base_object_class = excluded.base_object_class, + refs = excluded.refs, + val_dump = excluded.val_dump, + version_index = excluded.version_index, + is_latest = excluded.is_latest, + deleted_at = excluded.deleted_at + """, ( - req.obj.project_id, - req.obj.object_id, + project_id, + object_id, datetime.datetime.now().isoformat(), get_kind(processed_val), processed_result["base_object_class"], @@ -660,28 +664,74 @@ def obj_create(self, req: tsi.ObjCreateReq) -> tsi.ObjCreateRes: digest, version_index, 1, + None, ), ) conn.commit() return tsi.ObjCreateRes(digest=digest) - def obj_read(self, req: tsi.ObjReadReq) -> tsi.ObjReadRes: - conds = [f"object_id = '{req.object_id}'"] - if req.digest == "latest": - conds.append("is_latest = 1") + def _obj_exists( + self, cursor: sqlite3.Cursor, project_id: str, object_id: str, digest: str + ) -> bool: + cursor.execute( + "SELECT COUNT(*) FROM objects WHERE project_id = ? AND object_id = ? AND digest = ? AND deleted_at IS NULL", + (project_id, object_id, digest), + ) + return_row = cursor.fetchone() + if return_row is None: + return False + return return_row[0] > 0 + + def _mark_existing_objects_as_not_latest( + self, cursor: sqlite3.Cursor, project_id: str, object_id: str + ) -> None: + """Mark all existing objects with such id as not latest. + We are creating a new object with the same id, all existing ones are no longer latest. + """ + cursor.execute( + "UPDATE objects SET is_latest = 0 WHERE project_id = ? AND object_id = ?", + (project_id, object_id), + ) + + def _get_obj_version_index( + self, cursor: sqlite3.Cursor, project_id: str, object_id: str + ) -> int: + """Get the version index for a new object with such id.""" + cursor.execute( + "SELECT COUNT(*) FROM objects WHERE project_id = ? AND object_id = ?", + (project_id, object_id), + ) + return_row = cursor.fetchone() + if return_row is None: + return 0 + return return_row[0] + + @staticmethod + def _make_digest_condition(digest: str) -> str: + if digest == "latest": + return "is_latest = 1" else: - (is_version, version_index) = digest_is_version_like(req.digest) + (is_version, version_index) = digest_is_version_like(digest) if is_version: - conds.append(f"version_index = '{version_index}'") + return f"version_index = '{version_index}'" else: - conds.append(f"digest = '{req.digest}'") + return f"digest = '{digest}'" + + def obj_read(self, req: tsi.ObjReadReq) -> tsi.ObjReadRes: + conds = [f"object_id = '{req.object_id}'"] + digest_condition = self._make_digest_condition(req.digest) + conds.append(digest_condition) objs = self._select_objs_query( req.project_id, conditions=conds, + include_deleted=True, ) if len(objs) == 0: raise NotFoundError(f"Obj {req.object_id}:{req.digest} not found") - + if objs[0].deleted_at is not None: + raise ObjectDeletedError( + f"{req.object_id}:v{objs[0].version_index} was deleted at {objs[0].deleted_at}" + ) return tsi.ObjReadRes(obj=objs[0]) def objs_query(self, req: tsi.ObjQueryReq) -> tsi.ObjQueryRes: @@ -716,6 +766,60 @@ def objs_query(self, req: tsi.ObjQueryReq) -> tsi.ObjQueryRes: return tsi.ObjQueryRes(objs=objs) + def obj_delete(self, req: tsi.ObjDeleteReq) -> tsi.ObjDeleteRes: + MAX_OBJECTS_TO_DELETE = 100 + if req.digests and len(req.digests) > MAX_OBJECTS_TO_DELETE: + raise ValueError( + f"Object delete request contains {len(req.digests)} objects. Please delete {MAX_OBJECTS_TO_DELETE} or fewer objects at a time." + ) + + # First, select the objects that match the query + select_query = """ + SELECT digest FROM objects + WHERE project_id = ? AND + object_id = ? AND + deleted_at IS NULL + """ + parameters = [req.project_id, req.object_id] + if req.digests: + digest_conditions = [ + self._make_digest_condition(digest) for digest in req.digests + ] + digest_conditions_str = " OR ".join(digest_conditions) + select_query += f"AND ({digest_conditions_str})" + + conn, cursor = get_conn_cursor(self.db_path) + cursor.execute(select_query, parameters) + matching_objects = cursor.fetchall() + + if len(matching_objects) == 0: + raise NotFoundError( + f"Object {req.object_id} ({req.digests}) not found when deleting." + ) + found_digests = {obj[0] for obj in matching_objects} + if req.digests: + given_digests = set(req.digests) + if len(given_digests) != len(found_digests): + raise NotFoundError( + f"Delete request contains {len(req.digests)} digests, but found {len(found_digests)} objects to delete. Diff digests: {given_digests - found_digests}" + ) + + # Create a delete query that will set the deleted_at field to now + delete_query = """ + UPDATE objects SET deleted_at = CURRENT_TIMESTAMP + WHERE project_id = ? AND + object_id = ? AND + digest IN ({}) + """.format(", ".join("?" * len(found_digests))) + delete_parameters = [req.project_id, req.object_id] + list(found_digests) + + with self.lock: + cursor.execute("BEGIN TRANSACTION") + cursor.execute(delete_query, delete_parameters) + conn.commit() + + return tsi.ObjDeleteRes(num_deleted=len(matching_objects)) + def table_create(self, req: tsi.TableCreateReq) -> tsi.TableCreateRes: conn, cursor = get_conn_cursor(self.db_path) insert_rows = [] @@ -1151,11 +1255,17 @@ def _select_objs_query( parameters: Optional[dict[str, Any]] = None, metadata_only: Optional[bool] = False, limit: Optional[int] = None, + include_deleted: bool = False, offset: Optional[int] = None, sort_by: Optional[list[tsi.SortBy]] = None, ) -> list[tsi.ObjSchema]: conn, cursor = get_conn_cursor(self.db_path) - pred = " AND ".join(conditions or ["1 = 1"]) + conditions = conditions or [] + if not include_deleted: + conditions.append("deleted_at IS NULL") + if not conditions: + conditions.append("1 = 1") + pred = " AND ".join(conditions) val_dump_part = "'{}' as val_dump" if metadata_only else "val_dump" query = f""" SELECT @@ -1167,10 +1277,10 @@ def _select_objs_query( {val_dump_part}, digest, version_index, - is_latest + is_latest, + deleted_at FROM objects - WHERE deleted_at IS NULL AND - project_id = ? AND {pred} + WHERE project_id = ? AND {pred} """ if sort_by: @@ -1212,6 +1322,7 @@ def _select_objs_query( digest=row[6], version_index=row[7], is_latest=row[8], + deleted_at=row[9], ) ) return result diff --git a/weave/trace_server/trace_server_interface.py b/weave/trace_server/trace_server_interface.py index c419a9e23e58..699e1e128c63 100644 --- a/weave/trace_server/trace_server_interface.py +++ b/weave/trace_server/trace_server_interface.py @@ -476,6 +476,19 @@ class ObjQueryReq(BaseModel): ) +class ObjDeleteReq(BaseModel): + project_id: str + object_id: str + digests: Optional[list[str]] = Field( + default=None, + description="List of digests to delete. If not provided, all digests for the object will be deleted.", + ) + + +class ObjDeleteRes(BaseModel): + num_deleted: int + + class ObjQueryRes(BaseModel): objs: list[ObjSchema] @@ -906,6 +919,7 @@ def cost_purge(self, req: CostPurgeReq) -> CostPurgeRes: ... def obj_create(self, req: ObjCreateReq) -> ObjCreateRes: ... def obj_read(self, req: ObjReadReq) -> ObjReadRes: ... def objs_query(self, req: ObjQueryReq) -> ObjQueryRes: ... + def obj_delete(self, req: ObjDeleteReq) -> ObjDeleteRes: ... def table_create(self, req: TableCreateReq) -> TableCreateRes: ... def table_update(self, req: TableUpdateReq) -> TableUpdateRes: ... def table_query(self, req: TableQueryReq) -> TableQueryRes: ... diff --git a/weave/trace_server_bindings/remote_http_trace_server.py b/weave/trace_server_bindings/remote_http_trace_server.py index b749af60d24a..966bf92a8151 100644 --- a/weave/trace_server_bindings/remote_http_trace_server.py +++ b/weave/trace_server_bindings/remote_http_trace_server.py @@ -352,6 +352,11 @@ def objs_query( "/objs/query", req, tsi.ObjQueryReq, tsi.ObjQueryRes ) + def obj_delete(self, req: tsi.ObjDeleteReq) -> tsi.ObjDeleteRes: + return self._generic_request( + "/obj/delete", req, tsi.ObjDeleteReq, tsi.ObjDeleteRes + ) + def table_create( self, req: Union[tsi.TableCreateReq, dict[str, Any]] ) -> tsi.TableCreateRes: From 6ce18b768c87ee99ec8bed1cd7ffc5df516f2309 Mon Sep 17 00:00:00 2001 From: Griffin Tarpenning Date: Mon, 6 Jan 2025 14:18:14 -0800 Subject: [PATCH 13/28] chore(weave): allow call.feedback.add() for annotations (#3323) --- tests/trace/test_annotation_feedback.py | 57 +++++++++++++++++++++++++ weave/trace/feedback.py | 34 ++++++++++++--- weave/trace/refs.py | 6 +++ 3 files changed, 92 insertions(+), 5 deletions(-) diff --git a/tests/trace/test_annotation_feedback.py b/tests/trace/test_annotation_feedback.py index 6a3d6b6589d2..bdb8d0c90e9d 100644 --- a/tests/trace/test_annotation_feedback.py +++ b/tests/trace/test_annotation_feedback.py @@ -456,3 +456,60 @@ class FeedbackModel(BaseModel): # Invalid cases should return False assert not enum_spec.value_is_valid("invalid_choice") assert not enum_spec.value_is_valid(123) + + +def test_annotation_feedback_sdk(client): + number_spec = AnnotationSpec( + name="Number Rating", + field_schema={ + "type": "number", + "minimum": 1, + "maximum": 5, + }, + ) + ref = weave.publish(number_spec, "number spec") + assert ref + + @weave.op() + def do_call(): + return 3 + + do_call() + do_call() + + calls = do_call.calls() + assert len(list(calls)) == 2 + + # Add annotation feedback + calls[0].feedback.add( + "wandb.annotation.number-spec", + {"value": 3}, + annotation_ref=ref.uri(), + ) + + # Query the feedback + feedback = calls[0].feedback.refresh() + assert len(feedback) == 1 + assert feedback[0].payload["value"] == 3 + assert feedback[0].annotation_ref == ref.uri() + + # no annotation_ref + with pytest.raises(ValueError): + calls[0].feedback.add("wandb.annotation.number_rating", {"value": 3}) + + # empty annotation_ref + with pytest.raises(ValueError): + calls[0].feedback.add( + "wandb.annotation.number_rating", {"value": 3}, annotation_ref="" + ) + + # invalid annotation_ref + with pytest.raises(ValueError): + calls[0].feedback.add("number_rating", {"value": 3}, annotation_ref="ssss") + + # no wandb.annotation prefix + with pytest.raises( + ValueError, + match="To add annotation feedback, feedback_type must conform to the format: 'wandb.annotation.'.", + ): + calls[0].feedback.add("number_rating", {"value": 3}, annotation_ref=ref.uri()) diff --git a/weave/trace/feedback.py b/weave/trace/feedback.py index 884c751c0594..77f07d2449c3 100644 --- a/weave/trace/feedback.py +++ b/weave/trace/feedback.py @@ -10,7 +10,7 @@ from weave.trace import util from weave.trace.context import weave_client_context as weave_client_context -from weave.trace.refs import parse_uri +from weave.trace.refs import parse_object_uri, parse_uri from weave.trace.rich import pydantic_util from weave.trace.rich.container import AbstractRichContainer from weave.trace.rich.refs import Refs @@ -188,7 +188,11 @@ def __init__(self, ref: str) -> None: self.weave_ref = ref def _add( - self, feedback_type: str, payload: dict[str, Any], creator: str | None + self, + feedback_type: str, + payload: dict[str, Any], + creator: str | None, + annotation_ref: str | None = None, ) -> str: freq = tsi.FeedbackCreateReq( project_id=f"{self.entity}/{self.project}", @@ -197,6 +201,14 @@ def _add( payload=payload, creator=creator, ) + if annotation_ref: + try: + parse_object_uri(annotation_ref) + except TypeError: + raise TypeError( + "annotation_ref must be a valid object ref, eg weave://///object/:" + ) + freq.annotation_ref = annotation_ref response = self.client.server.feedback_create(freq) self.feedbacks = None # Clear cache return response.id @@ -206,6 +218,7 @@ def add( feedback_type: str, payload: dict[str, Any] | None = None, creator: str | None = None, + annotation_ref: str | None = None, **kwargs: dict[str, Any], ) -> str: """Add feedback to the ref. @@ -213,12 +226,11 @@ def add( feedback_type: A string identifying the type of feedback. The "wandb." prefix is reserved. creator: The name to display for the originator of the feedback. """ - if feedback_type.startswith("wandb."): - raise ValueError('Feedback type cannot start with "wandb."') + _validate_feedback_type(feedback_type, annotation_ref) feedback = {} feedback.update(payload or {}) feedback.update(kwargs) - return self._add(feedback_type, feedback, creator) + return self._add(feedback_type, feedback, creator, annotation_ref) def add_reaction(self, emoji: str, creator: str | None = None) -> str: return self._add( @@ -258,6 +270,18 @@ def purge(self, feedback_id: str) -> None: self.feedbacks = None # Clear cache +def _validate_feedback_type(feedback_type: str, annotation_ref: str | None) -> None: + if feedback_type.startswith("wandb.") and not annotation_ref: + raise ValueError( + 'Feedback type cannot start with "wandb", it is reserved for annotation feedback.' + "Provide an annotation_ref to add annotation feedback." + ) + elif not feedback_type.startswith("wandb.annotation.") and annotation_ref: + raise ValueError( + "To add annotation feedback, feedback_type must conform to the format: 'wandb.annotation.'." + ) + + __docspec__ = [ Feedbacks, FeedbackQuery, diff --git a/weave/trace/refs.py b/weave/trace/refs.py index 9190368b5926..e3f635f90ced 100644 --- a/weave/trace/refs.py +++ b/weave/trace/refs.py @@ -305,3 +305,9 @@ def parse_op_uri(uri: str) -> OpRef: if not isinstance(parsed := parse_uri(uri), OpRef): raise TypeError(f"URI is not for an Op: {uri}") return parsed + + +def parse_object_uri(uri: str) -> ObjectRef: + if not isinstance(parsed := parse_uri(uri), ObjectRef): + raise TypeError(f"URI is not for an Object: {uri}") + return parsed From ee4dde1caf89a6dd3df97efc9e053f9cbcaf731c Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Tue, 7 Jan 2025 11:14:02 -0800 Subject: [PATCH 14/28] working --- .../Home/Browse3/pages/ObjectVersionsPage.tsx | 18 ++++++++++++------ .../pages/wfReactInterface/utilities.ts | 2 +- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionsPage.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionsPage.tsx index 244300aa1c07..ad4c427e4afb 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionsPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionsPage.tsx @@ -127,6 +127,7 @@ export const ObjectVersionsPage: React.FC<{ project={project} objectName={filter.objectName ?? null} selectedVersions={selectedVersions} + setSelectedVersions={setSelectedVersions} showDeleteButton={!isReadonly && isAdmin} showCompareButton={hasComparison} onCompare={onCompare} @@ -157,6 +158,7 @@ const ObjectVersionsPageHeaderExtra: React.FC<{ project: string; objectName: string | null; selectedVersions: string[]; + setSelectedVersions: (selected: string[]) => void; showDeleteButton?: boolean; showCompareButton?: boolean; onCompare: () => void; @@ -165,6 +167,7 @@ const ObjectVersionsPageHeaderExtra: React.FC<{ project, objectName, selectedVersions, + setSelectedVersions, showDeleteButton, showCompareButton, onCompare, @@ -174,14 +177,14 @@ const ObjectVersionsPageHeaderExtra: React.FC<{ Compare ) : undefined; - const deleteButton = showDeleteButton ? ( setSelectedVersions([])} /> ) : undefined; @@ -630,15 +633,17 @@ const DeleteObjectVersionsButtonWithModal: React.FC<{ entity: string; project: string; objectName: string; - objectDigests: string[]; + objectVersions: string[]; disabled?: boolean; -}> = ({entity, project, objectName, objectDigests, disabled}) => { + onSuccess: () => void; +}> = ({entity, project, objectName, objectVersions, disabled, onSuccess}) => { const {useObjectDeleteFunc} = useWFHooks(); const {objectVersionsDelete} = useObjectDeleteFunc(); const [deleteModalOpen, setDeleteModalOpen] = useState(false); - const numObjects = objectDigests.length; + const numObjects = objectVersions.length; const versionsStr = maybePluralizeWord(numObjects, 'version', 's'); + const objectDigests = objectVersions.map(v => v.split(':')[1]); return ( <> @@ -652,10 +657,11 @@ const DeleteObjectVersionsButtonWithModal: React.FC<{ open={deleteModalOpen} onClose={() => setDeleteModalOpen(false)} deleteTitleStr={`${numObjects} ${objectName} ${versionsStr}`} - deleteBodyStrs={objectDigests} + deleteBodyStrs={objectVersions} onDelete={() => objectVersionsDelete(entity, project, objectName, objectDigests) } + onSuccess={onSuccess} /> ); diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/utilities.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/utilities.ts index af2ec8052865..44d7f76b05d1 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/utilities.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/utilities.ts @@ -292,7 +292,7 @@ export const isObjDeleteError = (e: any): boolean => { return false; } const errorStr = String(e); - const regex = /Obj .* was deleted at .*/; + const regex = /Obj.* was deleted at .*/; return regex.test(errorStr); }; From 4796f401d5a1d81fb5b0905d42bec80683c6dc15 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Tue, 7 Jan 2025 16:50:03 -0800 Subject: [PATCH 15/28] review comments --- .../Browse3/pages/CallPage/ObjectViewer.tsx | 21 +++++++++++++++---- .../CallPage/ObjectViewerGroupingCell.tsx | 3 ++- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewer.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewer.tsx index 251483665172..672e90f0f96f 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewer.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewer.tsx @@ -168,10 +168,8 @@ export const ObjectViewer = ({ continue; } if (!v) { - // Value for ref not found, probably deleted - refValues[r] = { - _weave_is_deleted_ref: objectRefDisplayName(parseRef(r)).label, - }; + // Value for ref not found, must be deleted + refValues[r] = deletedRefValuePlaceholder(r); continue; } let val = r; @@ -734,3 +732,18 @@ const useTruncatedData = (data: Data) => { return {truncatedData, truncatedStore, setTruncatedData, setTruncatedStore}; }; + +// Placeholder value for deleted refs +const DELETED_REF_KEY = '_weave_deleted_ref'; +const deletedRefValuePlaceholder = ( + ref: string +): {[DELETED_REF_KEY]: string} => { + const parsedRef = parseRef(ref); + const refString = objectRefDisplayName(parsedRef).label; + return {[DELETED_REF_KEY]: refString}; +}; +export const maybeGetDeletedRefValuePlaceholderFromRow = ( + row: any +): string | undefined => { + return row.value?.[DELETED_REF_KEY]; +}; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewerGroupingCell.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewerGroupingCell.tsx index 37e1a3d5f7bc..2f6ada30cb4c 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewerGroupingCell.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewerGroupingCell.tsx @@ -6,6 +6,7 @@ import React, {FC, MouseEvent} from 'react'; import {Button} from '../../../../../Button'; import {Tooltip} from '../../../../../Tooltip'; import {CursorBox} from './CursorBox'; +import {maybeGetDeletedRefValuePlaceholderFromRow} from './ObjectViewer'; const INSET_SPACING = 40; @@ -32,7 +33,7 @@ export const ObjectViewerGroupingCell: FC< event.stopPropagation(); }; - const deletedRef = row.value?._weave_is_deleted_ref; + const deletedRef = maybeGetDeletedRefValuePlaceholderFromRow(row); const tooltipContent = row.path.toString(); const textContent = deletedRef ?? props.value; const box = ( From 119571b92279bae5a46c4295ced388d5b1ea8070 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Tue, 7 Jan 2025 16:51:41 -0800 Subject: [PATCH 16/28] dont regex error str --- .../Home/Browse3/pages/wfReactInterface/utilities.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/utilities.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/utilities.ts index 44d7f76b05d1..c19f8c97926d 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/utilities.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/utilities.ts @@ -291,9 +291,10 @@ export const isObjDeleteError = (e: any): boolean => { if (e == null) { return false; } - const errorStr = String(e); - const regex = /Obj.* was deleted at .*/; - return regex.test(errorStr); + if ('deleted_at' in e) { + return true; + } + return false; }; /// Hooks /// From 3fadc2d7a0aec75e06b567ce3cb2fce6f45c3e82 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Tue, 7 Jan 2025 17:16:25 -0800 Subject: [PATCH 17/28] better error --- .../Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts | 4 ++-- .../Home/Browse3/pages/wfReactInterface/utilities.ts | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts index 22de0a995fa9..fa18d2401cd2 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts @@ -898,7 +898,7 @@ const useObjectVersion = ( const cachedObjectVersion = key ? objectVersionCache.get(key) : null; const [objectVersionRes, setObjectVersionRes] = useState(null); - const [error, setError] = useState(null); + const [error, setError] = useState(null); const deepKey = useDeepMemo(key); useEffect(() => { if (deepKey) { @@ -916,7 +916,7 @@ const useObjectVersion = ( .then(res => { loadingRef.current = false; if (res.obj == null) { - setError(new Error(JSON.stringify(res))); + setError(res); // be conservative and unset the cache when there's an error if (deepKey) { objectVersionCache.del(deepKey); diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/utilities.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/utilities.ts index c19f8c97926d..d57a63fff169 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/utilities.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/utilities.ts @@ -287,11 +287,11 @@ export const objectVersionNiceString = (ov: ObjectVersionSchema) => { return result; }; -export const isObjDeleteError = (e: any): boolean => { - if (e == null) { +export const isObjDeleteError = (error: any): boolean => { + if (error == null) { return false; } - if ('deleted_at' in e) { + if ('deleted_at' in error) { return true; } return false; From f819f80beab6152d856d985d524ca6a5637d5385 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Tue, 7 Jan 2025 17:22:28 -0800 Subject: [PATCH 18/28] add number response --- .../pages/wfReactInterface/traceServerClient.ts | 6 +++--- .../wfReactInterface/wfDataModelHooksInterface.ts | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts index 5ac11c29a5ec..be2d6feeed26 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts @@ -145,10 +145,10 @@ export class TraceServerClient extends CachingTraceServerClient { public objectDelete( req: TraceObjDeleteReq - ): Promise { + ): Promise { const res = super.objectDelete(req).then(r => { - if (r && 'detail' in r) { - throw new Error(r.detail); + if (r && typeof r !== 'number') { + throw new Error(JSON.stringify(r)); } this.onObjectListeners.forEach(listener => listener()); return r; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/wfDataModelHooksInterface.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/wfDataModelHooksInterface.ts index c8e59c22e94a..3e085ab37b4c 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/wfDataModelHooksInterface.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/wfDataModelHooksInterface.ts @@ -261,20 +261,20 @@ export type WFDataModelHooksInterface = { useObjectDeleteFunc: () => { objectVersionDelete: ( key: ObjectVersionKey - ) => Promise; + ) => Promise; objectDeleteAllVersions: ( key: ObjectVersionKey - ) => Promise; + ) => Promise; objectVersionsDelete: ( entity: string, project: string, objectId: string, digests: string[] - ) => Promise; - opVersionDelete: (key: OpVersionKey) => Promise; + ) => Promise; + opVersionDelete: (key: OpVersionKey) => Promise; opDeleteAllVersions: ( key: OpVersionKey - ) => Promise; + ) => Promise; }; // `useRefsData` is in beta while we integrate Shawn's new Object DB useRefsData: (refUris: string[], tableQuery?: TableQuery) => Loadable; From 0e97e19dbfcf5e00da82bc29c3ef360d33f4f9f9 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Tue, 7 Jan 2025 17:43:42 -0800 Subject: [PATCH 19/28] cleanup --- .../Home/Browse3/pages/ObjectVersionPage.tsx | 11 ++++- .../Home/Browse3/pages/OpVersionPage.tsx | 11 ++++- .../Home/Browse3/pages/common/DeleteModal.tsx | 4 +- .../wfReactInterface/traceServerClient.ts | 9 ++-- .../traceServerClientTypes.ts | 3 +- .../traceServerDirectClient.ts | 10 +++-- .../wfReactInterface/tsDataModelHooks.ts | 41 ++++++++----------- .../wfDataModelHooksInterface.ts | 20 +++++---- 8 files changed, 60 insertions(+), 49 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx index b45af121a1e4..3c3f0cfaff1a 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx @@ -644,7 +644,7 @@ const DeleteObjectButtonWithModal: React.FC<{ }> = ({objVersionSchema, overrideDisplayStr}) => { const {useObjectDeleteFunc} = useWFHooks(); const closePeek = useClosePeek(); - const {objectVersionDelete} = useObjectDeleteFunc(); + const {objectVersionsDelete} = useObjectDeleteFunc(); const [deleteModalOpen, setDeleteModalOpen] = useState(false); const deleteStr = @@ -662,7 +662,14 @@ const DeleteObjectButtonWithModal: React.FC<{ open={deleteModalOpen} onClose={() => setDeleteModalOpen(false)} deleteTitleStr={deleteStr} - onDelete={() => objectVersionDelete(objVersionSchema)} + onDelete={() => + objectVersionsDelete( + objVersionSchema.project, + objVersionSchema.entity, + objVersionSchema.objectId, + [objVersionSchema.versionHash] + ) + } onSuccess={closePeek} /> diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionPage.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionPage.tsx index 105689875b7f..9fbd527aa8c7 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionPage.tsx @@ -188,7 +188,7 @@ const DeleteOpButtonWithModal: React.FC<{ }> = ({opVersionSchema, overrideDisplayStr}) => { const {useObjectDeleteFunc} = useWFHooks(); const closePeek = useClosePeek(); - const {opVersionDelete} = useObjectDeleteFunc(); + const {opVersionsDelete} = useObjectDeleteFunc(); const [deleteModalOpen, setDeleteModalOpen] = useState(false); const deleteStr = @@ -206,7 +206,14 @@ const DeleteOpButtonWithModal: React.FC<{ open={deleteModalOpen} onClose={() => setDeleteModalOpen(false)} deleteTitleStr={deleteStr} - onDelete={() => opVersionDelete(opVersionSchema)} + onDelete={() => + opVersionsDelete( + opVersionSchema.entity, + opVersionSchema.project, + opVersionSchema.opId, + [opVersionSchema.versionHash] + ) + } onSuccess={closePeek} /> diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx index d3ae8f6f3792..ded4e87973a7 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx @@ -10,12 +10,14 @@ import {Tailwind} from '@wandb/weave/components/Tailwind'; import React, {useState} from 'react'; import styled from 'styled-components'; +import {TraceObjDeleteRes} from '../wfReactInterface/traceServerClientTypes'; + const MAX_DELETE_ROWS_TO_SHOW = 10; interface DeleteModalProps { open: boolean; onClose: () => void; - onDelete: () => Promise<{detail: string} | void>; + onDelete: () => Promise; deleteTitleStr: string; deleteBodyStrs?: string[]; onSuccess?: () => void; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts index be2d6feeed26..8b3cd71581da 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts @@ -15,6 +15,7 @@ import { TraceObjCreateReq, TraceObjCreateRes, TraceObjDeleteReq, + TraceObjDeleteRes, TraceRefsReadBatchReq, TraceRefsReadBatchRes, } from './traceServerClientTypes'; @@ -143,12 +144,10 @@ export class TraceServerClient extends CachingTraceServerClient { return res; } - public objectDelete( - req: TraceObjDeleteReq - ): Promise { + public objectDelete(req: TraceObjDeleteReq): Promise { const res = super.objectDelete(req).then(r => { - if (r && typeof r !== 'number') { - throw new Error(JSON.stringify(r)); + if ('detail' in r) { + throw new Error(r.detail); } this.onObjectListeners.forEach(listener => listener()); return r; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClientTypes.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClientTypes.ts index bedd9eec0c3b..bbc6f44ca5b6 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClientTypes.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClientTypes.ts @@ -258,7 +258,8 @@ export type TraceObjDeleteReq = { }; export type TraceObjDeleteRes = { - num_deleted: number; + num_deleted?: number; + detail?: string; }; export type TraceRefsReadBatchReq = { diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerDirectClient.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerDirectClient.ts index fe30f5c04077..2254b05ebc84 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerDirectClient.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerDirectClient.ts @@ -43,6 +43,7 @@ import { TraceObjCreateReq, TraceObjCreateRes, TraceObjDeleteReq, + TraceObjDeleteRes, TraceObjQueryReq, TraceObjQueryRes, TraceObjReadReq, @@ -233,10 +234,11 @@ export class DirectTraceServerClient { return this.makeRequest('/obj/read', req); } - public objectDelete( - req: TraceObjDeleteReq - ): Promise { - return this.makeRequest('/obj/delete', req); + public objectDelete(req: TraceObjDeleteReq): Promise { + return this.makeRequest( + '/obj/delete', + req + ); } public readBatch(req: TraceRefsReadBatchReq): Promise { diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts index fa18d2401cd2..a80a8986a98a 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts @@ -1135,21 +1135,6 @@ const useObjectDeleteFunc = () => { refDataCache.del(ref); }, []); - const objectVersionDelete = useCallback( - (key: ObjectVersionKey) => { - updateObjectCaches(key); - return getTsClient().objectDelete({ - project_id: projectIdFromParts({ - entity: key.entity, - project: key.project, - }), - object_id: key.objectId, - digests: [key.versionHash], - }); - }, - [getTsClient, updateObjectCaches] - ); - const objectVersionsDelete = useCallback( (entity: string, project: string, objectId: string, digests: string[]) => { digests.forEach(digest => { @@ -1190,16 +1175,23 @@ const useObjectDeleteFunc = () => { [getTsClient, updateObjectCaches] ); - const opVersionDelete = useCallback( - (key: OpVersionKey) => { - updateOpCaches(key); + const opVersionsDelete = useCallback( + (entity: string, project: string, opId: string, digests: string[]) => { + digests.forEach(digest => { + updateOpCaches({ + entity, + project, + opId, + versionHash: digest, + }); + }); return getTsClient().objectDelete({ project_id: projectIdFromParts({ - entity: key.entity, - project: key.project, + entity, + project, }), - object_id: key.opId, - digests: [key.versionHash], + object_id: opId, + digests, }); }, [getTsClient, updateOpCaches] @@ -1221,10 +1213,9 @@ const useObjectDeleteFunc = () => { ); return { - objectVersionDelete, - objectDeleteAllVersions, objectVersionsDelete, - opVersionDelete, + objectDeleteAllVersions, + opVersionsDelete, opDeleteAllVersions, }; }; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/wfDataModelHooksInterface.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/wfDataModelHooksInterface.ts index 3e085ab37b4c..f37c85d817ee 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/wfDataModelHooksInterface.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/wfDataModelHooksInterface.ts @@ -259,22 +259,24 @@ export type WFDataModelHooksInterface = { opts?: {skip?: boolean; noAutoRefresh?: boolean} ) => LoadableWithError; useObjectDeleteFunc: () => { - objectVersionDelete: ( - key: ObjectVersionKey - ) => Promise; - objectDeleteAllVersions: ( - key: ObjectVersionKey - ) => Promise; objectVersionsDelete: ( entity: string, project: string, objectId: string, digests: string[] - ) => Promise; - opVersionDelete: (key: OpVersionKey) => Promise; + ) => Promise; + objectDeleteAllVersions: ( + key: ObjectVersionKey + ) => Promise; + opVersionsDelete: ( + entity: string, + project: string, + opId: string, + digests: string[] + ) => Promise; opDeleteAllVersions: ( key: OpVersionKey - ) => Promise; + ) => Promise; }; // `useRefsData` is in beta while we integrate Shawn's new Object DB useRefsData: (refUris: string[], tableQuery?: TableQuery) => Loadable; From 629287bf916ec9f68bb1e2f893b5dcfcae86c214 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Tue, 7 Jan 2025 17:45:47 -0800 Subject: [PATCH 20/28] lint --- .../Home/Browse3/pages/common/DeleteModal.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx index ded4e87973a7..e36fa70d189f 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/common/DeleteModal.tsx @@ -10,14 +10,12 @@ import {Tailwind} from '@wandb/weave/components/Tailwind'; import React, {useState} from 'react'; import styled from 'styled-components'; -import {TraceObjDeleteRes} from '../wfReactInterface/traceServerClientTypes'; - const MAX_DELETE_ROWS_TO_SHOW = 10; interface DeleteModalProps { open: boolean; onClose: () => void; - onDelete: () => Promise; + onDelete: () => Promise; deleteTitleStr: string; deleteBodyStrs?: string[]; onSuccess?: () => void; From d2c051aebf800280db65a0ded12b1b01a3f21987 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Wed, 8 Jan 2025 09:36:21 -0800 Subject: [PATCH 21/28] wip --- .../Browse3/pages/wfReactInterface/wfDataModelHooksInterface.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/wfDataModelHooksInterface.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/wfDataModelHooksInterface.ts index f37c85d817ee..e59b47de5ae9 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/wfDataModelHooksInterface.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/wfDataModelHooksInterface.ts @@ -28,7 +28,7 @@ export type Loadable = { export type LoadableWithError = { loading: boolean; result: T | null; - error: Error | null; + error: Error | {reason: string} | null; }; export type CallKey = { From 121cb3773b03fae202c37862f83772ebfebec6c8 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Wed, 8 Jan 2025 13:27:27 -0800 Subject: [PATCH 22/28] allow error handling to happen naturally --- .../Home/Browse3/pages/wfReactInterface/traceServerClient.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts index 8b3cd71581da..a433c0c087cd 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerClient.ts @@ -146,9 +146,6 @@ export class TraceServerClient extends CachingTraceServerClient { public objectDelete(req: TraceObjDeleteReq): Promise { const res = super.objectDelete(req).then(r => { - if ('detail' in r) { - throw new Error(r.detail); - } this.onObjectListeners.forEach(listener => listener()); return r; }); From 59c9a326d9e40d0826d38ed9defcadae874f0bac Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Thu, 9 Jan 2025 14:41:09 -0800 Subject: [PATCH 23/28] eval compare page robust to model delete --- .../pages/CompareEvaluationsPage/ecpTypes.ts | 2 +- .../EvaluationDefinition.tsx | 38 +++++++++++++++---- .../ScorecardSection/ScorecardSection.tsx | 6 +-- .../Home/Browse3/pages/ObjectVersionPage.tsx | 2 +- .../wfReactInterface/tsDataModelHooks.ts | 10 ++++- .../tsDataModelHooksEvaluationComparison.ts | 3 ++ 6 files changed, 47 insertions(+), 14 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/ecpTypes.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/ecpTypes.ts index df40b28a92df..a0f0eafc9b3e 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/ecpTypes.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/ecpTypes.ts @@ -25,7 +25,7 @@ export type EvaluationComparisonSummary = { // Models are the Weave Objects used to define the model logic and properties. models: { - [modelRef: string]: ModelObj; + [modelRef: string]: ModelObj | null; }; // ScoreMetrics define the metrics that are associated on each individual prediction diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx index 0de9421ba1a9..8f304ef0ee45 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx @@ -1,4 +1,4 @@ -import {Box} from '@material-ui/core'; +import {Box} from '@mui/material'; import React, {useMemo} from 'react'; import { @@ -9,9 +9,10 @@ import { import {hexToRGB} from '../../../../../../../../common/css/utils'; import {parseRef, WeaveObjectRef} from '../../../../../../../../react'; import {Icon, IconNames} from '../../../../../../../Icon'; -import {SmallRef} from '../../../../../Browse2/SmallRef'; +import {objectRefDisplayName, SmallRef} from '../../../../../Browse2/SmallRef'; import {CallLink, ObjectVersionLink} from '../../../common/Links'; import {useWFHooks} from '../../../wfReactInterface/context'; +import {isObjDeleteError} from '../../../wfReactInterface/utilities'; import {ObjectVersionKey} from '../../../wfReactInterface/wfDataModelHooksInterface'; import {EvaluationComparisonState} from '../../ecpState'; @@ -44,10 +45,10 @@ export const EvaluationModelLink: React.FC<{ const {useObjectVersion} = useWFHooks(); const evaluationCall = props.state.summary.evaluationCalls[props.callId]; const modelObj = props.state.summary.models[evaluationCall.modelRef]; - const objRef = useMemo( - () => parseRef(modelObj.ref) as WeaveObjectRef, - [modelObj.ref] - ); + const objRef = useMemo(() => { + return parseRef(evaluationCall.modelRef) as WeaveObjectRef; + }, [evaluationCall.modelRef]); + const objVersionKey = useMemo(() => { return { scheme: 'weave', @@ -69,10 +70,31 @@ export const EvaluationModelLink: React.FC<{ ]); const objectVersion = useObjectVersion(objVersionKey); + if (isObjDeleteError(objectVersion.error)) { + return ( + + + + {objectRefDisplayName(objRef).label} + + + ); + } + return ( { const model = props.state.summary.models[ref]; - Object.keys(model.properties).forEach(prop => { + Object.keys(model?.properties ?? {}).forEach(prop => { if (!propsRes[prop]) { propsRes[prop] = {}; } - propsRes[prop][ref] = model.properties[prop]; + propsRes[prop][ref] = model?.properties?.[prop]; }); }); @@ -104,7 +104,7 @@ export const ScorecardSection: React.FC<{ if (!propsRes.predict) { propsRes.predict = {}; } - propsRes.predict[ref] = model.predictOpRef; + propsRes.predict[ref] = model?.predictOpRef; }); return propsRes; diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx index 894149942147..d9a73f683d02 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx @@ -664,8 +664,8 @@ const DeleteObjectButtonWithModal: React.FC<{ deleteTitleStr={deleteStr} onDelete={() => objectVersionsDelete( - objVersionSchema.project, objVersionSchema.entity, + objVersionSchema.project, objVersionSchema.objectId, [objVersionSchema.versionHash] ) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts index a9048ee41a77..c9982cb60ceb 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooks.ts @@ -915,7 +915,15 @@ const useObjectVersion = ( }) .then(res => { loadingRef.current = false; - setObjectVersionRes(res); + if (res.obj == null) { + if ('deleted_at' in res) { + setError(new Error(JSON.stringify(res))); + } else { + setError(new Error('Object not found')); + } + } else { + setObjectVersionRes(res); + } }) .catch(err => { setError(new Error(JSON.stringify(err))); diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooksEvaluationComparison.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooksEvaluationComparison.ts index c321accd487e..5e707e5c0705 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooksEvaluationComparison.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooksEvaluationComparison.ts @@ -414,6 +414,9 @@ const fetchEvaluationSummaryData = async ( const ref = modelRefs[objNdx]; const parsed = parseRef(ref) as WeaveObjectRef; const objData = objVal; + if (objData == null) { + return [ref, null]; + } return [ ref, { From 5820182bfdcd66ebf7557a8a3cfe87fc26254bee Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Thu, 9 Jan 2025 14:56:38 -0800 Subject: [PATCH 24/28] add delete button to dataset page --- .../Home/Browse3/datasets/DatasetVersionPage.tsx | 11 +++++++++-- .../Home/Browse3/pages/ObjectVersionPage.tsx | 9 +++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/datasets/DatasetVersionPage.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/datasets/DatasetVersionPage.tsx index f3aed14e9efd..b07da77d6846 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/datasets/DatasetVersionPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/datasets/DatasetVersionPage.tsx @@ -13,6 +13,7 @@ import { ScrollableTabContent, SimplePageLayoutWithHeader, } from '../pages/common/SimplePageLayout'; +import {DeleteObjectButtonWithModal} from '../pages/ObjectVersionPage'; import {TabUseDataset} from '../pages/TabUseDataset'; import {useWFHooks} from '../pages/wfReactInterface/context'; import {objectVersionKeyToRefUri} from '../pages/wfReactInterface/utilities'; @@ -21,7 +22,8 @@ import {CustomWeaveTypeProjectContext} from '../typeViews/CustomWeaveTypeDispatc export const DatasetVersionPage: React.FC<{ objectVersion: ObjectVersionSchema; -}> = ({objectVersion}) => { + showDeleteButton?: boolean; +}> = ({objectVersion, showDeleteButton}) => { const {useRootObjectVersions, useRefsData} = useWFHooks(); const entityName = objectVersion.entity; const projectName = objectVersion.project; @@ -73,7 +75,7 @@ export const DatasetVersionPage: React.FC<{ } headerContent={ -
+

Name

Version

{objectVersionIndex}

+
+ {showDeleteButton && ( + + )} +
} diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx index d9a73f683d02..6fb0852c932e 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx @@ -208,7 +208,12 @@ const ObjectVersionPageInner: React.FC<{ } if (isDataset) { - return ; + return ( + + ); } return ( @@ -638,7 +643,7 @@ const OpVersionCallsLink: React.FC<{ ); }; -const DeleteObjectButtonWithModal: React.FC<{ +export const DeleteObjectButtonWithModal: React.FC<{ objVersionSchema: ObjectVersionSchema; overrideDisplayStr?: string; }> = ({objVersionSchema, overrideDisplayStr}) => { From 33b354d0b164db00802aa94ee0675a9c00a672cb Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Thu, 9 Jan 2025 14:57:48 -0800 Subject: [PATCH 25/28] lint --- .../ComparisonDefinitionSection/EvaluationDefinition.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx index 8f304ef0ee45..05cce3904519 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CompareEvaluationsPage/sections/ComparisonDefinitionSection/EvaluationDefinition.tsx @@ -44,7 +44,6 @@ export const EvaluationModelLink: React.FC<{ }> = props => { const {useObjectVersion} = useWFHooks(); const evaluationCall = props.state.summary.evaluationCalls[props.callId]; - const modelObj = props.state.summary.models[evaluationCall.modelRef]; const objRef = useMemo(() => { return parseRef(evaluationCall.modelRef) as WeaveObjectRef; }, [evaluationCall.modelRef]); From 0893dd40b3f5842bbf18c7eb117c28855915c925 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Thu, 9 Jan 2025 15:52:16 -0800 Subject: [PATCH 26/28] cosmetics --- .../Home/Browse3/datasets/DatasetVersionPage.tsx | 10 +++++----- .../Home/Browse3/pages/ObjectVersionPage.tsx | 10 +++++----- .../Home/Browse3/pages/ObjectVersionsPage.tsx | 9 ++++++--- .../Home/Browse3/pages/OpVersionPage.tsx | 10 +++++----- 4 files changed, 21 insertions(+), 18 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/datasets/DatasetVersionPage.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/datasets/DatasetVersionPage.tsx index b07da77d6846..1fd637d20d44 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/datasets/DatasetVersionPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/datasets/DatasetVersionPage.tsx @@ -75,7 +75,7 @@ export const DatasetVersionPage: React.FC<{ } headerContent={ -
+

Name

Version

{objectVersionIndex}

-
- {showDeleteButton && ( + {showDeleteButton && ( +
- )} -
+
+ )}
} diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx index 6fb0852c932e..d1a2543a63bc 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionPage.tsx @@ -230,7 +230,7 @@ const ObjectVersionPageInner: React.FC<{ } headerContent={ -
+

Name

@@ -271,11 +271,11 @@ const ObjectVersionPageInner: React.FC<{

{refExtra}

)} -
- {showDeleteButton && ( + {showDeleteButton && ( +
- )} -
+
+ )}
} diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionsPage.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionsPage.tsx index ad4c427e4afb..e9fee11f3e27 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionsPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/ObjectVersionsPage.tsx @@ -112,10 +112,12 @@ export const ObjectVersionsPage: React.FC<{ return ; } - const hasComparison = filter.objectName != null; + const filteredOnObject = filter.objectName != null; + const hasComparison = filteredOnObject; const viewer = userInfo ? userInfo.id : null; const isReadonly = !viewer || !userInfo?.teams.includes(props.entity); const isAdmin = userInfo?.admin; + const showDeleteButton = filteredOnObject && !isReadonly && isAdmin; return ( @@ -644,6 +646,7 @@ const DeleteObjectVersionsButtonWithModal: React.FC<{ const numObjects = objectVersions.length; const versionsStr = maybePluralizeWord(numObjects, 'version', 's'); const objectDigests = objectVersions.map(v => v.split(':')[1]); + const deleteTitleStr = `${numObjects} ${objectName} ${versionsStr}`; return ( <> @@ -656,7 +659,7 @@ const DeleteObjectVersionsButtonWithModal: React.FC<{ setDeleteModalOpen(false)} - deleteTitleStr={`${numObjects} ${objectName} ${versionsStr}`} + deleteTitleStr={deleteTitleStr} deleteBodyStrs={objectVersions} onDelete={() => objectVersionsDelete(entity, project, objectName, objectDigests) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionPage.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionPage.tsx index 9fbd527aa8c7..6148e5a5bc48 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionPage.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/OpVersionPage.tsx @@ -80,7 +80,7 @@ const OpVersionPageInner: React.FC<{ title={opVersionText(opId, versionIndex)} headerContent={ -
+

Name

@@ -142,11 +142,11 @@ const OpVersionPageInner: React.FC<{

-

)}
-
- {showDeleteButton && ( + {showDeleteButton && ( +
- )} -
+
+ )}
} From 59716f1606987722bb72a5e305bb5c93ed069f26 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Fri, 10 Jan 2025 10:45:24 -0800 Subject: [PATCH 27/28] handle eval compare page with deleted dataset --- .../tsDataModelHooksEvaluationComparison.ts | 35 ++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooksEvaluationComparison.ts b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooksEvaluationComparison.ts index 5e707e5c0705..5a918ed46d16 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooksEvaluationComparison.ts +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/tsDataModelHooksEvaluationComparison.ts @@ -500,25 +500,28 @@ const fetchEvaluationComparisonResults = async ( }); // 3.5 Populate the inputs - // We only ned 1 since we are going to effectively do an inner join on the rowDigest + // We only need 1 since we are going to effectively do an inner join on the rowDigest const datasetRef = Object.values(summaryData.evaluations)[0] .datasetRef as string; const datasetObjRes = await traceServerClient.readBatch({refs: [datasetRef]}); - const rowsRef = datasetObjRes.vals[0].rows; - const parsedRowsRef = parseRef(rowsRef) as WeaveObjectRef; - const rowsQuery = await traceServerClient.tableQuery({ - project_id: projectIdFromParts({ - entity: parsedRowsRef.entityName, - project: parsedRowsRef.projectName, - }), - digest: parsedRowsRef.artifactVersion, - }); - rowsQuery.rows.forEach(row => { - result.inputs[row.digest] = { - digest: row.digest, - val: row.val, - }; - }); + // If the dataset has not been deleted, fetch rows + if (datasetObjRes.vals[0] != null) { + const rowsRef = datasetObjRes.vals[0].rows; + const parsedRowsRef = parseRef(rowsRef) as WeaveObjectRef; + const rowsQuery = await traceServerClient.tableQuery({ + project_id: projectIdFromParts({ + entity: parsedRowsRef.entityName, + project: parsedRowsRef.projectName, + }), + digest: parsedRowsRef.artifactVersion, + }); + rowsQuery.rows.forEach(row => { + result.inputs[row.digest] = { + digest: row.digest, + val: row.val, + }; + }); + } // 4. Populate the predictions and scores const evalTraceRes = await evalTraceResProm; From 387d6c30d220a81741d7c3acb7c6606619756a58 Mon Sep 17 00:00:00 2001 From: gtarpenning Date: Fri, 10 Jan 2025 13:01:04 -0800 Subject: [PATCH 28/28] bump