Skip to content

Commit

Permalink
fix: Fixes issues with special characters in object name (#2156)
Browse files Browse the repository at this point in the history 
* more work

* adding

* linting

* some of the UI fixes

* tests - rough

* lint complete

* test improvement

* done with tests

* ok, basically done

* Final

* Final

* Final

* Final

* Final

* i think complete

* test fix

* removed client check

* added checks

* another little fix

* done

* lint

* comments
  • Loading branch information
@tssweeney
tssweeney authored Aug 28, 2024
1 parent c984aea commit b1a6d80
Show file tree
Hide file tree
Showing 15 changed files with 368 additions and 152 deletions.
47 changes: 30 additions & 17 deletions weave-js/src/components/PagePanelComponents/Home/Browse3.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ const Browse3Mounted: FC<{

const MainPeekingLayout: FC = () => {
const {baseRouter} = useWeaveflowRouteContext();
const params = useParams<Browse3Params>();
const params = useParamsDecoded<Browse3Params>();
const baseRouterProjectRoot = baseRouter.projectUrl(':entity', ':project');
const generalProjectRoot = browse2Context.projectUrl(':entity', ':project');
const query = useURLSearchParamsDict();
Expand Down Expand Up @@ -406,7 +406,7 @@ const MainPeekingLayout: FC = () => {
};

const ProjectRedirect: FC = () => {
const {entity, project} = useParams<Browse3ProjectMountedParams>();
const {entity, project} = useParamsDecoded<Browse3ProjectMountedParams>();
const {baseRouter} = useWeaveflowRouteContext();
const url = baseRouter.tracesUIUrl(entity, project);
return <Redirect to={url} />;
Expand Down Expand Up @@ -497,7 +497,7 @@ const Browse3ProjectRoot: FC<{

// TODO(tim/weaveflow_improved_nav): Generalize this
const ObjectVersionRoutePageBinding = () => {
const params = useParams<Browse3TabItemVersionParams>();
const params = useParamsDecoded<Browse3TabItemVersionParams>();
const query = useURLSearchParamsDict();

const history = useHistory();
Expand Down Expand Up @@ -538,7 +538,7 @@ const ObjectVersionRoutePageBinding = () => {

// TODO(tim/weaveflow_improved_nav): Generalize this
const OpVersionRoutePageBinding = () => {
const params = useParams<Browse3TabItemVersionParams>();
const params = useParamsDecoded<Browse3TabItemVersionParams>();
const history = useHistory();
const routerContext = useWeaveflowCurrentRouteContext();
useEffect(() => {
Expand Down Expand Up @@ -573,7 +573,7 @@ const useCallPeekRedirect = () => {
// This is a "hack" since the client doesn't have all the info
// needed to make a correct peek URL. This allows the client to request
// such a view and we can redirect to the correct URL.
const params = useParams<Browse3TabItemParams>();
const params = useParamsDecoded<Browse3TabItemParams>();
const {baseRouter} = useWeaveflowRouteContext();
const history = useHistory();
const {useCall} = useWFHooks();
Expand Down Expand Up @@ -618,10 +618,23 @@ const useCallPeekRedirect = () => {
]);
};

const useParamsDecoded = <T extends object>() => {
// Handle the case where entity/project (old) have spaces
const params = useParams<T>();
return useMemo(() => {
return Object.fromEntries(
Object.entries(params).map(([key, value]) => [
key,
decodeURIComponent(value),
])
);
}, [params]);
};

// TODO(tim/weaveflow_improved_nav): Generalize this
const CallPageBinding = () => {
useCallPeekRedirect();
const params = useParams<Browse3TabItemParams>();
const params = useParamsDecoded<Browse3TabItemParams>();
const query = useURLSearchParamsDict();

return (
Expand All @@ -636,7 +649,7 @@ const CallPageBinding = () => {

// TODO(tim/weaveflow_improved_nav): Generalize this
const CallsPageBinding = () => {
const {entity, project, tab} = useParams<Browse3TabParams>();
const {entity, project, tab} = useParamsDecoded<Browse3TabParams>();
const query = useURLSearchParamsDict();
const initialFilter = useMemo(() => {
if (tab === 'evaluations') {
Expand Down Expand Up @@ -769,7 +782,7 @@ const CallsPageBinding = () => {

// TODO(tim/weaveflow_improved_nav): Generalize this
const ObjectVersionsPageBinding = () => {
const {entity, project, tab} = useParams<Browse3TabParams>();
const {entity, project, tab} = useParamsDecoded<Browse3TabParams>();
const query = useURLSearchParamsDict();
const filters: WFHighLevelObjectVersionFilter = useMemo(() => {
let queryFilter: WFHighLevelObjectVersionFilter = {};
Expand Down Expand Up @@ -815,7 +828,7 @@ const ObjectVersionsPageBinding = () => {

// TODO(tim/weaveflow_improved_nav): Generalize this
const OpVersionsPageBinding = () => {
const params = useParams<Browse3TabParams>();
const params = useParamsDecoded<Browse3TabParams>();

const query = useURLSearchParamsDict();
const filters = useMemo(() => {
Expand Down Expand Up @@ -851,7 +864,7 @@ const OpVersionsPageBinding = () => {

// TODO(tim/weaveflow_improved_nav): Generalize this
const BoardPageBinding = () => {
const params = useParams<Browse3TabItemVersionParams>();
const params = useParamsDecoded<Browse3TabItemVersionParams>();

return (
<BoardPage
Expand All @@ -865,7 +878,7 @@ const BoardPageBinding = () => {

// TODO(tim/weaveflow_improved_nav): Generalize this
const ObjectPageBinding = () => {
const params = useParams<Browse3TabItemVersionParams>();
const params = useParamsDecoded<Browse3TabItemVersionParams>();
return (
<ObjectPage
entity={params.entity}
Expand All @@ -876,7 +889,7 @@ const ObjectPageBinding = () => {
};

const OpPageBinding = () => {
const params = useParams<Browse3TabItemVersionParams>();
const params = useParamsDecoded<Browse3TabItemVersionParams>();
return (
<OpPage
entity={params.entity}
Expand All @@ -887,7 +900,7 @@ const OpPageBinding = () => {
};

const CompareEvaluationsBinding = () => {
const {entity, project} = useParams<Browse3TabParams>();
const {entity, project} = useParamsDecoded<Browse3TabParams>();
const query = useURLSearchParamsDict();
const evaluationCallIds = useMemo(() => {
return JSON.parse(query.evaluationCallIds);
Expand All @@ -902,19 +915,19 @@ const CompareEvaluationsBinding = () => {
};

const OpsPageBinding = () => {
const params = useParams<Browse3TabItemParams>();
const params = useParamsDecoded<Browse3TabItemParams>();

return <OpsPage entity={params.entity} project={params.project} />;
};

const BoardsPageBinding = () => {
const params = useParams<Browse3TabItemParams>();
const params = useParamsDecoded<Browse3TabItemParams>();

return <BoardsPage entity={params.entity} project={params.project} />;
};

const TablesPageBinding = () => {
const params = useParams<Browse3TabItemParams>();
const params = useParamsDecoded<Browse3TabItemParams>();

return <TablesPage entity={params.entity} project={params.project} />;
};
Expand All @@ -934,7 +947,7 @@ const AppBarLink = (props: ComponentProps<typeof RouterLink>) => (
);

const Browse3Breadcrumbs: FC = props => {
const params = useParams<Browse3Params>();
const params = useParamsDecoded<Browse3Params>();
const query = useURLSearchParamsDict();
const filePathParts = query.path?.split('/') ?? [];
const refFields = query.extra?.split('/') ?? [];
Expand Down
2 changes: 1 addition & 1 deletion weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewer.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ const buildBaseRef = (
const parts = path.toPath().slice(-startIndex);
parts.forEach(part => {
if (typeof part === 'string') {
baseRef += '/' + OBJECT_ATTR_EDGE_NAME + '/' + part;
baseRef += '/' + OBJECT_ATTR_EDGE_NAME + '/' + encodeURIComponent(part);
} else if (typeof part === 'number') {
baseRef += '/' + LIST_INDEX_EDGE_NAME + '/' + part.toString();
} else {
Expand Down
6 changes: 5 additions & 1 deletion ...mponents/PagePanelComponents/Home/Browse3/pages/common/tabularListViews/columnBuilder.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,11 @@ function replaceTableRefsInFlattenedData(flattened: Record<string, any>) {
if (parentRef) {
const newVal: ExpandedRefWithValueAsTableRef =
makeRefExpandedPayload(
parentRef + '/' + OBJECT_ATTR_EDGE_NAME + '/' + attr,
parentRef +
'/' +
OBJECT_ATTR_EDGE_NAME +
'/' +
encodeURIComponent(attr),
val
);
return [key, newVal];
Expand Down
16 changes: 1 addition & 15 deletions weave-js/src/react.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe('parseRef', () => {
});
it('parses a ref with spaces in entity', () => {
const parsed = parseRef(
'weave:///Entity%20Name/project/object/artifact-name:artifactversion'
'weave:///Entity Name/project/object/artifact-name:artifactversion'
);
expect(parsed).toEqual({
artifactName: 'artifact-name',
Expand Down Expand Up @@ -102,20 +102,6 @@ describe('parseRef', () => {
weaveKind: 'object',
});
});
it('parses a ref with escaped spaces in name and projectName', () => {
const parsed = parseRef(
'weave:///entity/project%20with%20spaces/object/artifact%20name%20with%20spaces:artifactversion'
);
expect(parsed).toEqual({
artifactName: 'artifact name with spaces',
artifactRefExtra: '',
artifactVersion: 'artifactversion',
entityName: 'entity',
projectName: 'project with spaces',
scheme: 'weave',
weaveKind: 'object',
});
});
});
it('parses a weave table ref', () => {
const parsed = parseRef(
Expand Down
105 changes: 53 additions & 52 deletions weave-js/src/react.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import {
useState,
} from 'react';

import {WEAVE_REF_PREFIX} from './components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/constants';
import {PanelCompContext} from './components/Panel2/PanelComp';
import {usePanelContext} from './components/Panel2/PanelContext';
import {toWeaveType} from './components/Panel2/toWeaveType';
Expand All @@ -64,7 +65,6 @@ import {
getChainRootVar,
isConstructor,
} from './core/mutate';
import {trimStartChar} from './core/util/string';
import {UseNodeValueServerExecutionError} from './errors';
import {useDeepMemo} from './hookUtils';
import {consoleLog} from './util';
Expand Down Expand Up @@ -505,6 +505,7 @@ export const isWeaveObjectRef = (ref: ObjectRef): ref is WeaveObjectRef => {
// Unfortunately many teams have been created that violate this.
const PATTERN_ENTITY = '([^/]+)';
const PATTERN_PROJECT = '([^\\#?%:]{1,128})'; // Project name
const PATTERN_REF_EXTRA = '([a-zA-Z0-9_.~/%-]*)'; // Optional ref extra (valid chars are result of python urllib.parse.quote and javascript encodeURIComponent)
const RE_WEAVE_OBJECT_REF_PATHNAME = new RegExp(
[
'^', // Start of the string
Expand All @@ -518,7 +519,7 @@ const RE_WEAVE_OBJECT_REF_PATHNAME = new RegExp(
':',
'([*]|[a-zA-Z0-9]+)', // Artifact version, allowing '*' for any version
'/?', // Ref extra portion is optional
'([a-zA-Z0-9_/]*)', // Optional ref extra
PATTERN_REF_EXTRA, // Optional ref extra
'$', // End of the string
].join('')
);
Expand All @@ -531,7 +532,7 @@ const RE_WEAVE_TABLE_REF_PATHNAME = new RegExp(
'/table/',
'([a-f0-9]+)', // Digest
'/?', // Ref extra portion is optional
'([a-zA-Z0-9_/]*)', // Optional ref extra
PATTERN_REF_EXTRA, // Optional ref extra
'$', // End of the string
].join('')
);
Expand All @@ -544,7 +545,7 @@ const RE_WEAVE_CALL_REF_PATHNAME = new RegExp(
'/call/',
'([0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12})', // Call UUID
'/?', // Ref extra portion is optional
'([a-zA-Z0-9_/]*)', // Optional ref extra
PATTERN_REF_EXTRA, // Optional ref extra
'$', // End of the string
].join('')
);
Expand All @@ -561,7 +562,7 @@ export const parseRef = (ref: string): ObjectRef => {
} else if (isLocalArtifact) {
splitLimit = 2;
} else if (isWeaveRef) {
splitLimit = 4;
return parseWeaveRef(ref);
} else {
throw new Error(`Unknown protocol: ${url.protocol}`);
}
Expand Down Expand Up @@ -598,58 +599,58 @@ export const parseRef = (ref: string): ObjectRef => {
artifactPath,
};
}
throw new Error(`Unknown protocol: ${url.protocol}`);
};

if (isWeaveRef) {
const trimmed = trimStartChar(decodedUri, '/');
const tableMatch = trimmed.match(RE_WEAVE_TABLE_REF_PATHNAME);
if (tableMatch !== null) {
const [entity, project, digest] = tableMatch.slice(1);
return {
scheme: 'weave',
entityName: entity,
projectName: project,
weaveKind: 'table' as WeaveKind,
artifactName: '',
artifactVersion: digest,
artifactRefExtra: '',
};
}
const callMatch = trimmed.match(RE_WEAVE_CALL_REF_PATHNAME);
if (callMatch !== null) {
const [entity, project, callId] = callMatch.slice(1);
return {
scheme: 'weave',
entityName: entity,
projectName: project,
weaveKind: 'call' as WeaveKind,
artifactName: callId,
artifactVersion: '',
artifactRefExtra: '',
};
}
const match = trimmed.match(RE_WEAVE_OBJECT_REF_PATHNAME);
if (match === null) {
throw new Error('Invalid weave ref uri: ' + ref);
}
const [
entityName,
projectName,
weaveKind,
artifactName,
artifactVersion,
artifactRefExtra,
] = match.slice(1);
const parseWeaveRef = (ref: string): WeaveObjectRef => {
const trimmed = ref.slice(WEAVE_REF_PREFIX.length);
const tableMatch = trimmed.match(RE_WEAVE_TABLE_REF_PATHNAME);
if (tableMatch !== null) {
const [entity, project, digest] = tableMatch.slice(1);
return {
scheme: 'weave',
entityName,
projectName,
weaveKind: weaveKind as WeaveKind,
artifactName,
artifactVersion,
artifactRefExtra: artifactRefExtra ?? '',
entityName: entity,
projectName: project,
weaveKind: 'table' as WeaveKind,
artifactName: '',
artifactVersion: digest,
artifactRefExtra: '',
};
}
throw new Error(`Unknown protocol: ${url.protocol}`);
const callMatch = trimmed.match(RE_WEAVE_CALL_REF_PATHNAME);
if (callMatch !== null) {
const [entity, project, callId] = callMatch.slice(1);
return {
scheme: 'weave',
entityName: entity,
projectName: project,
weaveKind: 'call' as WeaveKind,
artifactName: callId,
artifactVersion: '',
artifactRefExtra: '',
};
}
const match = trimmed.match(RE_WEAVE_OBJECT_REF_PATHNAME);
if (match === null) {
throw new Error('Invalid weave ref uri: ' + ref);
}
const [
entityName,
projectName,
weaveKind,
artifactName,
artifactVersion,
artifactRefExtra,
] = match.slice(1);
return {
scheme: 'weave',
entityName,
projectName,
weaveKind: weaveKind as WeaveKind,
artifactName,
artifactVersion,
artifactRefExtra: artifactRefExtra ?? '',
};
};

export const objectRefWithExtra = (
Expand Down
4 changes: 2 additions & 2 deletions weave/integrations/langchain/langchain_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,12 @@ def test_simple_chain_invoke(
)
prompt = PromptTemplate.from_template("1 + {number} = ")
long_str = (
"really_massive_name_that_is_longer_than_max_characters_which_would_be_crazy_"
"really_massive_name_that_is_longer_than_max_characters_which_would_be_crazy"
)
name = long_str + long_str
prompt.name = name

exp_name = "really_massive_name_that_is_longer_than_max_characte_9ad6_t_is_longer_than_max_characters_which_would_be_crazy_"
exp_name = "really_massive_name_that_is_longer_than_max_characte_ff6e_at_is_longer_than_max_characters_which_would_be_crazy"

llm_chain = prompt | llm
_ = llm_chain.invoke({"number": 2})
Expand Down
Loading

0 comments on commit b1a6d80

Please sign in to comment.