Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -467,3 +467,4 @@ go-test-results.json
# Playwright generated
cmd/ui/playwright

.claude/
6 changes: 6 additions & 0 deletions cmd/ui/src/views/Explore/ExploreSearch/ExploreSearch.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,12 @@ const ExploreSearch: React.FC = () => {
if (!pathfindingSearchState.destinationSelectedItem) {
params.secondarySearch = null;
}
if (!pathfindingSearchState.nodes[2]?.selectedItem) {
params.tertiarySearch = null;
}
if (!pathfindingSearchState.nodes[3]?.selectedItem) {
params.quaternarySearch = null;
}
}
if (tab === 'cypher') {
if (!cypherSearchState.cypherQuery) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
//
// SPDX-License-Identifier: Apache-2.0

import { GraphResponse } from 'js-client-library';
import { apiClient } from '../../../utils';
import { ExploreQueryParams } from '../../useExploreParams';
import {
Expand All @@ -26,15 +27,48 @@ import {
sharedGraphQueryOptions,
} from './utils';

const mergeGraphResponses = (responses: GraphResponse[]): GraphResponse => {
return responses.reduce((merged, response) => ({
data: {
nodes: { ...merged.data.nodes, ...response.data.nodes },
edges: [...merged.data.edges, ...response.data.edges],
},
}));
};

export const pathfindingSearchGraphQuery = (paramOptions: Partial<ExploreQueryParams>): ExploreGraphQueryOptions => {
const { searchType, primarySearch, secondarySearch, pathFilters } = paramOptions;
const { searchType, primarySearch, secondarySearch, tertiarySearch, quaternarySearch, pathFilters } = paramOptions;

if (!primarySearch || !searchType || !secondarySearch || areFiltersEmpty(pathFilters)) {
return { enabled: false };
}

const filter = pathFilters?.length ? createPathFilterString(pathFilters) : undefined;

// Build ordered list of waypoints
const waypoints = [primarySearch, secondarySearch, tertiarySearch, quaternarySearch].filter(Boolean) as string[];
Comment on lines +48 to +49

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Don’t compact sparse waypoint params.

filter(Boolean) turns [primary, secondary, null, quaternary] into [primary, secondary, quaternary], so a sparse URL can query a different ordered path than the persisted node slots indicate. Since this PR adds URL persistence for extra destinations, disable or normalize sparse chains before querying.

Proposed guard
-    const waypoints = [primarySearch, secondarySearch, tertiarySearch, quaternarySearch].filter(Boolean) as string[];
+    if (!tertiarySearch && quaternarySearch) {
+        return { enabled: false };
+    }
+
+    const waypoints = [primarySearch, secondarySearch, tertiarySearch, quaternarySearch].filter(
+        (waypoint): waypoint is string => !!waypoint
+    );
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/javascript/bh-shared-ui/src/hooks/useExploreGraph/queries/pathfindingSearch.ts`
around lines 48 - 49, The waypoint list in pathfindingSearch is compacting
sparse inputs with filter(Boolean), which can change the intended slot order for
persisted URL destinations. Update the waypoints construction in
useExploreGraph/pathfindingSearch to preserve positional gaps or explicitly
reject/normalize sparse chains before calling the path query, so primarySearch,
secondarySearch, tertiarySearch, and quaternarySearch are interpreted in their
original order.


// Multi-leg pathfinding: fire parallel shortest-path queries for each consecutive pair and merge results
if (waypoints.length > 2) {
return {
...sharedGraphQueryOptions,
queryKey: [ExploreGraphQueryKey, searchType, ...waypoints, filter],
queryFn: async ({ signal }) => {
const legs = [];
for (let i = 0; i < waypoints.length - 1; i++) {
legs.push(
apiClient
.getShortestPathV2(waypoints[i], waypoints[i + 1], filter, { signal })
.then((res) => res.data as GraphResponse)
);
}
const results = await Promise.all(legs);
return mergeGraphResponses(results);
},
enabled: waypoints.length > 2,
};
}

return {
...sharedGraphQueryOptions,
queryKey: [ExploreGraphQueryKey, searchType, primarySearch, secondarySearch, filter],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,129 +16,213 @@

import { useEffect, useState } from 'react';
import { SearchValue } from '../../views/Explore/ExploreSearch/types';
import { useExploreParams } from '../useExploreParams';
import { ExploreQueryParams, useExploreParams } from '../useExploreParams';
import { useKeywordAndTypeValues, useSearch } from '../useSearch';

const MAX_NODES = 4;

const SEARCH_PARAM_KEYS = ['primarySearch', 'secondarySearch', 'tertiarySearch', 'quaternarySearch'] as const;

type PathfindingNode = {
searchTerm: string;
selectedItem: SearchValue | undefined;
};

const emptyNode = (): PathfindingNode => ({ searchTerm: '', selectedItem: undefined });

export const usePathfindingSearch = () => {
const [sourceSearchTerm, setSourceSearchTerm] = useState<string>('');
const [sourceSelectedItem, setSourceSelectedItem] = useState<SearchValue | undefined>(undefined);
const [destinationSearchTerm, setDestinationSearchTerm] = useState<string>('');
const [destinationSelectedItem, setDestinationSelectedItem] = useState<SearchValue | undefined>(undefined);
const [nodes, setNodes] = useState<PathfindingNode[]>([emptyNode(), emptyNode()]);
const [extraNodeCount, setExtraNodeCount] = useState(0);

const { primarySearch, secondarySearch, setExploreParams } = useExploreParams();
const { primarySearch, secondarySearch, tertiarySearch, quaternarySearch, setExploreParams } = useExploreParams();

// Wire up search queries. we should only recompute keywords when the param values change
const { keyword: sourceKeyword, type: sourceType } = useKeywordAndTypeValues(primarySearch);
const { keyword: destinationKeyword, type: destinationType } = useKeywordAndTypeValues(secondarySearch);
// Wire up search queries for each param
const { keyword: kw0, type: t0 } = useKeywordAndTypeValues(primarySearch);
const { keyword: kw1, type: t1 } = useKeywordAndTypeValues(secondarySearch);
const { keyword: kw2, type: t2 } = useKeywordAndTypeValues(tertiarySearch);
const { keyword: kw3, type: t3 } = useKeywordAndTypeValues(quaternarySearch);

const { data: sourceSearchData } = useSearch(sourceKeyword, sourceType);
const { data: destinationSearchData } = useSearch(destinationKeyword, destinationType);
const { data: data0 } = useSearch(kw0, t0);
const { data: data1 } = useSearch(kw1, t1);
const { data: data2 } = useSearch(kw2, t2);
const { data: data3 } = useSearch(kw3, t3);

// Watch query params and seperately sync them to each search field
// Sync URL params to node state
useEffect(() => {
if (primarySearch && sourceSearchData) {
const matchedNode = Object.values(sourceSearchData).find((node) => node.objectid === primarySearch);
syncNodeFromParam(0, primarySearch, data0);
}, [primarySearch, data0]);

Check warning on line 53 in packages/javascript/bh-shared-ui/src/hooks/useExploreGraph/usePathfindingSearch.ts

View workflow job for this annotation

GitHub Actions / build-ui

React Hook useEffect has a missing dependency: 'syncNodeFromParam'. Either include it or remove the dependency array

if (matchedNode) {
setSourceSearchTerm(matchedNode.name);
setSourceSelectedItem(matchedNode);
}
} else {
setSourceSearchTerm('');
setSourceSelectedItem(undefined);
useEffect(() => {
syncNodeFromParam(1, secondarySearch, data1);
}, [secondarySearch, data1]);

Check warning on line 57 in packages/javascript/bh-shared-ui/src/hooks/useExploreGraph/usePathfindingSearch.ts

View workflow job for this annotation

GitHub Actions / build-ui

React Hook useEffect has a missing dependency: 'syncNodeFromParam'. Either include it or remove the dependency array

useEffect(() => {
syncNodeFromParam(2, tertiarySearch, data2);
if (tertiarySearch && data2) {
setExtraNodeCount((prev) => Math.max(prev, 1));
}
}, [primarySearch, sourceSearchData]);
}, [tertiarySearch, data2]);

Check warning on line 64 in packages/javascript/bh-shared-ui/src/hooks/useExploreGraph/usePathfindingSearch.ts

View workflow job for this annotation

GitHub Actions / build-ui

React Hook useEffect has a missing dependency: 'syncNodeFromParam'. Either include it or remove the dependency array

useEffect(() => {
if (secondarySearch && destinationSearchData) {
const matchedNode = Object.values(destinationSearchData).find((node) => node.objectid === secondarySearch);
syncNodeFromParam(3, quaternarySearch, data3);
if (quaternarySearch && data3) {
setExtraNodeCount((prev) => Math.max(prev, 2));
}
}, [quaternarySearch, data3]);

Check warning on line 71 in packages/javascript/bh-shared-ui/src/hooks/useExploreGraph/usePathfindingSearch.ts

View workflow job for this annotation

GitHub Actions / build-ui

React Hook useEffect has a missing dependency: 'syncNodeFromParam'. Either include it or remove the dependency array
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

const syncNodeFromParam = (index: number, param: string | null, data: any) => {
if (param && data) {
const matchedNode = Object.values(data).find((node: any) => node.objectid === param) as
| SearchValue
| undefined;
if (matchedNode) {
setDestinationSearchTerm(matchedNode.name);
setDestinationSelectedItem(matchedNode);
updateNode(index, { searchTerm: matchedNode.name, selectedItem: matchedNode });
}
} else {
setDestinationSearchTerm('');
setDestinationSelectedItem(undefined);
} else if (!param) {
updateNode(index, emptyNode());
}
}, [secondarySearch, destinationSearchData]);
};

// Handle syncing each search field up to query params to trigger a graph query. Should trigger pathfinding if both have been selected and node
// if only one has been selected
const handleSourceNodeSelected = (selected?: SearchValue) => {
const objectId = selected?.objectid ?? '';
const term = selected?.name ?? objectId;
const updateNode = (index: number, update: Partial<PathfindingNode>) => {
setNodes((prev) => {
const next = [...prev];
while (next.length <= index) next.push(emptyNode());
next[index] = { ...next[index], ...update };
return next;
});
};

setSourceSelectedItem(selected);
setSourceSearchTerm(term);
const totalNodeCount = 2 + extraNodeCount;

// if i have the other node, set type to 'pathfinding' and search term to the objectid
// if not, set type to 'node' and clear out the opposing search term
if (secondarySearch && destinationSelectedItem) {
setExploreParams({
searchType: 'pathfinding',
primarySearch: objectId,
});
} else {
setExploreParams({
searchType: 'node',
primarySearch: objectId,
secondarySearch: null,
});
const getParamsFromNodes = (nodeList: PathfindingNode[]): Partial<ExploreQueryParams> => {
const params: Partial<ExploreQueryParams> = {};
SEARCH_PARAM_KEYS.forEach((key, i) => {
params[key] = nodeList[i]?.selectedItem?.objectid ?? null;
});
return params;
};

const triggerPathfinding = (params: Partial<ExploreQueryParams>) => {
const merged = { ...getParamsFromNodes(nodes), ...params };
const source = merged.primarySearch;
const dest = merged.secondarySearch;

if (source && dest) {
setExploreParams({ searchType: 'pathfinding', ...params });
} else if (source || dest) {
setExploreParams({ searchType: 'node', ...params });
}
};

const handleDestinationNodeSelected = (selected?: SearchValue) => {
// Handle node selection — triggers query
const handleNodeSelected = (index: number) => (selected?: SearchValue) => {
const objectId = selected?.objectid ?? '';
const term = selected?.name ?? objectId;

setDestinationSelectedItem(selected);
setDestinationSearchTerm(term);
updateNode(index, { searchTerm: term, selectedItem: selected });

if (primarySearch && sourceSelectedItem) {
setExploreParams({
searchType: 'pathfinding',
secondarySearch: objectId,
});
const paramKey = SEARCH_PARAM_KEYS[index];

if (index === 0) {
// Source node
if (secondarySearch && nodes[1]?.selectedItem) {
setExploreParams({ searchType: 'pathfinding', [paramKey]: objectId });
} else {
setExploreParams({ searchType: 'node', [paramKey]: objectId, secondarySearch: null });
}
} else if (index === 1) {
// First destination
if (primarySearch && nodes[0]?.selectedItem) {
setExploreParams({ searchType: 'pathfinding', [paramKey]: objectId });
} else {
setExploreParams({ searchType: 'node', [paramKey]: objectId, primarySearch: null });
}
} else {
setExploreParams({
searchType: 'node',
secondarySearch: objectId,
primarySearch: null,
});
// Extra destinations
triggerPathfinding({ [paramKey]: objectId });
}
};

// Handle text edit — does not trigger query
const handleNodeEdited = (index: number) => (edit: string) => {
updateNode(index, { searchTerm: edit, selectedItem: undefined });
};

const handleSwapPathfindingInputs = () => {
if (sourceSelectedItem && destinationSelectedItem) {
if (nodes[0]?.selectedItem && nodes[1]?.selectedItem) {
setExploreParams({
searchType: 'pathfinding',
primarySearch: destinationSelectedItem.objectid,
secondarySearch: sourceSelectedItem.objectid,
primarySearch: nodes[1].selectedItem.objectid,
secondarySearch: nodes[0].selectedItem.objectid,
});
}
};

// Handle changes internal to the search form that should not trigger a graph query. Each param should sync independently
const handleSourceNodeEdited = (edit: string) => {
setSourceSelectedItem(undefined);
setSourceSearchTerm(edit);
const handleReorderNodes = (fromIndex: number, toIndex: number) => {
const currentNodes = nodes.slice(0, totalNodeCount);
const [moved] = currentNodes.splice(fromIndex, 1);
currentNodes.splice(toIndex, 0, moved);

setNodes(currentNodes);

const params = getParamsFromNodes(currentNodes);
triggerPathfinding(params);
};

const handleDestinationNodeEdited = (edit: string) => {
setDestinationSelectedItem(undefined);
setDestinationSearchTerm(edit);
const handleAddNode = () => {
if (totalNodeCount < MAX_NODES) {
setExtraNodeCount((prev) => prev + 1);
setNodes((prev) => {
const next = [...prev];
while (next.length < 2 + extraNodeCount + 1) next.push(emptyNode());
return next;
});
}
};

const handleRemoveNode = (index: number) => {
if (index === 0 || totalNodeCount <= 2) return;

const currentNodes = nodes.slice(0, totalNodeCount);
currentNodes.splice(index, 1);
setExtraNodeCount((prev) => prev - 1);

// Pad back to at least 2
while (currentNodes.length < 2) currentNodes.push(emptyNode());
setNodes(currentNodes);

// Update URL params
const params = getParamsFromNodes(currentNodes);
// Clear any now-unused param
for (let i = currentNodes.length; i < MAX_NODES; i++) {
params[SEARCH_PARAM_KEYS[i]] = null;
}
triggerPathfinding(params);
};

// Build the return shape that PathfindingSearch.tsx expects
const sourceSearchTerm = nodes[0]?.searchTerm ?? '';
const sourceSelectedItem = nodes[0]?.selectedItem;
const destinationSearchTerm = nodes[1]?.searchTerm ?? '';
const destinationSelectedItem = nodes[1]?.selectedItem;

return {
sourceSearchTerm,
sourceSelectedItem,
destinationSearchTerm,
destinationSelectedItem,
handleSourceNodeEdited,
handleSourceNodeSelected,
handleDestinationNodeEdited,
handleDestinationNodeSelected,
nodes,
totalNodeCount,
maxNodes: MAX_NODES,
handleSourceNodeEdited: handleNodeEdited(0),
handleSourceNodeSelected: handleNodeSelected(0),
handleDestinationNodeEdited: handleNodeEdited(1),
handleDestinationNodeSelected: handleNodeSelected(1),
handleNodeEdited,
handleNodeSelected,
handleSwapPathfindingInputs,
handleReorderNodes,
handleRemoveNode,
handleAddNode,
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ export type ExploreQueryParams = {
exploreSearchTab: ExploreSearchTab | null;
primarySearch: string | null;
secondarySearch: string | null;
tertiarySearch: string | null;
quaternarySearch: string | null;
cypherSearch: string | null;
searchType: SearchType | null;
expandedPanelSections: string[] | null;
Expand Down Expand Up @@ -81,6 +83,8 @@ export const useExploreParams = (): UseExploreParamsReturn => {
exploreSearchTab: parseSearchTab(searchParams.get('exploreSearchTab')),
primarySearch: searchParams.get('primarySearch'),
secondarySearch: searchParams.get('secondarySearch'),
tertiarySearch: searchParams.get('tertiarySearch'),
quaternarySearch: searchParams.get('quaternarySearch'),
cypherSearch: searchParams.get('cypherSearch'),
searchType: parseSearchType(searchParams.get('searchType')),
expandedPanelSections: searchParams.getAll('expandedPanelSections'),
Expand All @@ -96,6 +100,8 @@ export const useExploreParams = (): UseExploreParamsReturn => {
'exploreSearchTab',
'primarySearch',
'secondarySearch',
'tertiarySearch',
'quaternarySearch',
'cypherSearch',
'searchType',
'expandedPanelSections',
Expand Down
Loading
Loading