Skip to content

Commit 266d26d

Browse files
committed
feat(history): enhance search functionality with state persistence and tests
- Implement persistent search state management across tab switches - Add useHistorySearch hook for centralized search state handling - Refactor search components to use controlled state pattern - Add comprehensive test coverage for history search features - Move search state responsibility to parent CodyPanel component - Update search bar with reset and submit handlers - Improve variable replacement in workflow execution - Enhance CLI command sanitization and execution safety - Bump version to 1.67.0+2 The changes improve search functionality in the history tab while maintaining state across navigation, add robust testing, and include important workflow execution safety improvements.
1 parent 11b3c45 commit 266d26d

File tree

6 files changed

+143
-44
lines changed

6 files changed

+143
-44
lines changed

pnpm-lock.yaml

+3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

vscode/package.json

+2-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"name": "cody-ai",
44
"private": true,
55
"displayName": "Cody: AI Code Assistant",
6-
"version": "1.67.0+1",
6+
"version": "1.67.0+2",
77
"publisher": "sourcegraph",
88
"license": "Apache-2.0",
99
"icon": "resources/sourcegraph.png",
@@ -1615,6 +1615,7 @@
16151615
"franc-min": "^6.2.0",
16161616
"fs-extra": "^11.2.0",
16171617
"fuzzysort": "^2.0.4",
1618+
"happy-dom": "^14.3.10",
16181619
"htmlnano": "^2.1.1",
16191620
"http-proxy-middleware": "^3.0.0",
16201621
"immer": "^10.1.1",

vscode/src/workflow/workflow-executor.ts

+20-18
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,10 @@ export async function executeWorkflow(
122122
switch (node.type) {
123123
case NodeType.CLI: {
124124
try {
125-
const inputs = combineParentOutputsByConnectionOrder(node.id, context).map(
125+
const inputs = combineParentOutputsByConnectionOrder(node.id, context)
126+
/*.map(
126127
output => sanitizeForShell(output)
127-
)
128+
)*/
128129
const command = (node as CLINode).data.content
129130
? replaceIndexedInputs((node as CLINode).data.content, inputs, context)
130131
: ''
@@ -386,31 +387,33 @@ export function replaceIndexedInputs(
386387
parentOutputs: string[],
387388
context?: IndexedExecutionContext
388389
): string {
389-
let result = template.replace(/\${(\d+)}/g, (_match, index) => {
390+
// Only replace numbered placeholders that match exactly ${1}, ${2}, etc.
391+
let result = template.replace(/\${(\d+)}(?!\w)/g, (_match, index) => {
390392
const adjustedIndex = Number.parseInt(index, 10) - 1
391393
return adjustedIndex >= 0 && adjustedIndex < parentOutputs.length
392394
? parentOutputs[adjustedIndex]
393395
: ''
394396
})
395397

396398
if (context) {
397-
// Replace loop variables
399+
// Only replace loop variables that are explicitly defined in the context
398400
for (const [, loopState] of context.loopStates) {
399401
result = result.replace(
400-
new RegExp(`\\$\{${loopState.variable}}`, 'g'),
402+
new RegExp(`\\$\{${loopState.variable}}(?!\\w)`, 'g'),
401403
String(loopState.currentIteration)
402404
)
403405
}
404-
result = result.replace(/\${(\w+)}/g, (_match, variableName) => {
405-
// Matches any word character after ${
406-
if (
407-
!/^\d+$/.test(variableName) &&
408-
!Object.values(context.loopStates).some(loopState => loopState.variable === variableName)
409-
) {
410-
return context.accumulatorValues?.get(variableName) || ''
411-
}
412-
return _match
413-
})
406+
407+
// Only replace accumulator variables that are explicitly defined
408+
const accumulatorVars = context.accumulatorValues
409+
? Array.from(context.accumulatorValues.keys())
410+
: []
411+
for (const varName of accumulatorVars) {
412+
result = result.replace(
413+
new RegExp(`\\$\{${varName}}(?!\\w)`, 'g'),
414+
context.accumulatorValues?.get(varName) || ''
415+
)
416+
}
414417
}
415418

416419
return result
@@ -488,14 +491,13 @@ export async function executeCLINode(
488491
result: `${filteredCommand}`,
489492
},
490493
} as ExtensionToWorkflow)
491-
492494
const approval = await approvalHandler(node.id)
493495
if (approval.command) {
494-
filteredCommand = approval.command
496+
filteredCommand = sanitizeForShell(approval.command)
495497
}
496498
}
497499

498-
if (commandsNotAllowed.some(cmd => filteredCommand.startsWith(cmd))) {
500+
if (commandsNotAllowed.some(cmd => sanitizeForShell(filteredCommand).startsWith(cmd))) {
499501
void vscode.window.showErrorMessage('Cody cannot execute this command')
500502
throw new Error('Cody cannot execute this command')
501503
}

vscode/webviews/CodyPanel.tsx

+5-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
} from '@sourcegraph/cody-shared'
1212
import { useExtensionAPI, useObservable } from '@sourcegraph/prompt-editor'
1313
import type React from 'react'
14-
import { type FunctionComponent, useEffect, useMemo, useRef } from 'react'
14+
import { type FunctionComponent, useEffect, useMemo, useRef, useState } from 'react'
1515
import type { ConfigurationSubsetForWebview, LocalEnv } from '../src/chat/protocol'
1616
import styles from './App.module.css'
1717
import { Chat } from './Chat'
@@ -109,6 +109,8 @@ export const CodyPanel: FunctionComponent<CodyPanelProps> = ({
109109
}
110110
}, [api.clientActionBroadcast])
111111

112+
const [historySearchQuery, setHistorySearchQuery] = useState('')
113+
112114
return (
113115
<TabViewContext.Provider value={useMemo(() => ({ view, setView }), [view, setView])}>
114116
<TabRoot
@@ -153,6 +155,8 @@ export const CodyPanel: FunctionComponent<CodyPanelProps> = ({
153155
setView={setView}
154156
webviewType={config.webviewType}
155157
multipleWebviewsEnabled={config.multipleWebviewsEnabled}
158+
searchQuery={historySearchQuery}
159+
onSearchQueryChange={setHistorySearchQuery}
156160
/>
157161
)}
158162
{view === View.Toolbox && config.webviewType === 'sidebar' && (
+60-3
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,79 @@
11
import { CodyIDE } from '@sourcegraph/cody-shared'
2-
import { render, screen } from '@testing-library/react'
2+
import { fireEvent, render, screen } from '@testing-library/react'
33
import { describe, expect, test, vi } from 'vitest'
44
import { AppWrapperForTest } from '../AppWrapperForTest'
55
import { HistoryTabWithData } from './HistoryTab'
66

77
describe('HistoryTabWithData', () => {
8+
const defaultProps = {
9+
IDE: CodyIDE.VSCode,
10+
setView: vi.fn(),
11+
searchQuery: '',
12+
onSearchQueryChange: vi.fn(),
13+
}
14+
815
test('renders empty state when there are no non-empty chats', () => {
9-
const mockSetView = vi.fn()
1016
const emptyChats = [
1117
{ id: '1', interactions: [], lastInteractionTimestamp: new Date().toISOString() },
1218
{ id: '2', interactions: [], lastInteractionTimestamp: new Date().toISOString() },
1319
]
1420

15-
render(<HistoryTabWithData IDE={CodyIDE.VSCode} setView={mockSetView} chats={emptyChats} />, {
21+
render(<HistoryTabWithData {...defaultProps} chats={emptyChats} />, {
1622
wrapper: AppWrapperForTest,
1723
})
1824

1925
expect(screen.getByText('You have no chat history')).toBeInTheDocument()
2026
expect(screen.getByText('Start a new chat')).toBeInTheDocument()
2127
})
28+
29+
test('search functionality works correctly', () => {
30+
const chats = [
31+
{
32+
id: '1',
33+
interactions: [
34+
{
35+
humanMessage: { text: 'test message', speaker: 'human' as const },
36+
assistantMessage: { text: 'response', speaker: 'assistant' as const },
37+
},
38+
],
39+
lastInteractionTimestamp: new Date().toISOString(),
40+
chatTitle: '',
41+
speaker: 'human' as const,
42+
},
43+
]
44+
45+
render(<HistoryTabWithData {...defaultProps} chats={chats} />, {
46+
wrapper: AppWrapperForTest,
47+
})
48+
49+
const searchInput = screen.getByPlaceholderText('Search in chat history...')
50+
fireEvent.change(searchInput, { target: { value: 'test' } })
51+
fireEvent.click(screen.getByTitle('Search'))
52+
53+
expect(defaultProps.onSearchQueryChange).toHaveBeenCalledWith('test')
54+
})
55+
56+
test('search can be cleared', () => {
57+
const chats = [
58+
{
59+
id: '1',
60+
interactions: [
61+
{
62+
humanMessage: { text: 'test message', speaker: 'human' as const },
63+
assistantMessage: { text: 'response', speaker: 'assistant' as const },
64+
},
65+
],
66+
lastInteractionTimestamp: new Date().toISOString(),
67+
chatTitle: '',
68+
speaker: 'human' as const,
69+
},
70+
]
71+
72+
render(<HistoryTabWithData {...defaultProps} chats={chats} searchQuery="test" />, {
73+
wrapper: AppWrapperForTest,
74+
})
75+
76+
fireEvent.click(screen.getByTitle('Clear search'))
77+
expect(defaultProps.onSearchQueryChange).toHaveBeenCalledWith('')
78+
})
2279
})

vscode/webviews/tabs/HistoryTab.tsx

+53-21
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
import { SearchIcon } from 'lucide-react'
1111
import { XCircleIcon } from 'lucide-react'
1212
import type React from 'react'
13-
import { memo, useCallback, useMemo, useRef, useState } from 'react'
13+
import { memo, useCallback, useEffect, useMemo, useRef, useState } from 'react'
1414
import type { WebviewType } from '../../src/chat/protocol'
1515
import { getRelativeChatPeriod } from '../../src/common/time-date'
1616
import { LoadingDots } from '../chat/components/LoadingDots'
@@ -26,6 +26,8 @@ interface HistoryTabProps {
2626
setView: (view: View) => void
2727
webviewType?: WebviewType | undefined | null
2828
multipleWebviewsEnabled?: boolean | undefined | null
29+
searchQuery: string
30+
onSearchQueryChange: (query: string) => void
2931
}
3032

3133
export const HistoryTab: React.FC<HistoryTabProps> = props => {
@@ -74,20 +76,20 @@ const filterChatsBySearch = (chats: SerializedChatTranscript[], term: string) =>
7476

7577
export const HistoryTabWithData: React.FC<
7678
HistoryTabProps & { chats: UserLocalHistory['chat'][string][] }
77-
> = ({ IDE, webviewType, multipleWebviewsEnabled, setView, chats }) => {
79+
> = ({
80+
IDE,
81+
webviewType,
82+
multipleWebviewsEnabled,
83+
setView,
84+
chats,
85+
searchQuery,
86+
onSearchQueryChange,
87+
}) => {
7888
const [editingId, setEditingId] = useState<string | null>(null)
7989
const [newTitle, setNewTitle] = useState('')
8090
const inputRef = useRef<HTMLInputElement>(null)
81-
const [debouncedSearchTerm, setDebouncedSearchTerm] = useState('')
8291

83-
const filteredChats = useMemo(
84-
() =>
85-
filterChatsBySearch(
86-
chats.filter(chat => chat.interactions.length > 0),
87-
debouncedSearchTerm
88-
),
89-
[chats, debouncedSearchTerm]
90-
)
92+
const { filteredChats, handleSearch } = useHistorySearch(chats, searchQuery)
9193

9294
const chatByPeriod = useMemo(
9395
() =>
@@ -153,12 +155,20 @@ export const HistoryTabWithData: React.FC<
153155
)
154156

155157
const SearchBar = memo(
156-
({ initialValue, onSearch }: { initialValue: string; onSearch: (term: string) => void }) => {
157-
const [inputValue, setInputValue] = useState(initialValue)
158+
({ value, onSearchSubmit }: { value: string; onSearchSubmit: (term: string) => void }) => {
159+
const [inputValue, setInputValue] = useState(value)
160+
161+
useEffect(() => {
162+
setInputValue(value)
163+
}, [value])
158164

159165
const handleReset = () => {
160166
setInputValue('')
161-
onSearch('')
167+
onSearchSubmit('') // Clear search by submitting empty term
168+
}
169+
170+
const submitSearch = () => {
171+
onSearchSubmit(inputValue) // Submit current input value
162172
}
163173

164174
return (
@@ -168,11 +178,7 @@ export const HistoryTabWithData: React.FC<
168178
placeholder="Search in chat history..."
169179
value={inputValue}
170180
onChange={e => setInputValue(e.target.value)}
171-
onKeyDown={e => {
172-
if (e.key === 'Enter') {
173-
onSearch(inputValue)
174-
}
175-
}}
181+
onKeyDown={e => e.key === 'Enter' && submitSearch()}
176182
className="tw-flex-1 tw-text-sm tw-text-muted-foreground [::placeholder:tw-text-sm] [::placeholder:tw-text-muted-foreground] tw-h-10"
177183
variant="search"
178184
/>
@@ -181,7 +187,7 @@ export const HistoryTabWithData: React.FC<
181187
<XCircleIcon size={14} strokeWidth={1.25} />
182188
</Button>
183189
)}
184-
<Button variant="secondary" onClick={() => onSearch(inputValue)} title="Search">
190+
<Button variant="secondary" onClick={submitSearch} title="Search">
185191
<SearchIcon size={14} strokeWidth={1.25} />
186192
</Button>
187193
</div>
@@ -191,7 +197,10 @@ export const HistoryTabWithData: React.FC<
191197

192198
return (
193199
<div className="tw-flex tw-flex-col tw-gap-6">
194-
<SearchBar initialValue={debouncedSearchTerm} onSearch={setDebouncedSearchTerm} />
200+
<SearchBar
201+
value={searchQuery}
202+
onSearchSubmit={term => onSearchQueryChange(handleSearch(term))} // Update searchQuery in parent
203+
/>
195204
{chatByPeriod.map(([period, chats]) => (
196205
<CollapsiblePanel
197206
id={`history-${period}`.replaceAll(' ', '-').toLowerCase()}
@@ -324,3 +333,26 @@ function useUserHistory(): UserLocalHistory | null | undefined {
324333
const userHistory = useExtensionAPI().userHistory
325334
return useObservable(useMemo(() => userHistory(), [userHistory])).value
326335
}
336+
337+
function useHistorySearch(chats: SerializedChatTranscript[], initialSearchTerm: string) {
338+
const activeSearchTerm = initialSearchTerm // Directly use the prop
339+
340+
const filteredChats = useMemo(
341+
() =>
342+
filterChatsBySearch(
343+
chats.filter(chat => chat.interactions.length > 0),
344+
activeSearchTerm
345+
),
346+
[chats, activeSearchTerm]
347+
)
348+
349+
// handleSearch now just updates the external search term (passed via callback)
350+
const handleSearch = (term: string) => {
351+
return term // Return the term, the parent component will handle state update
352+
}
353+
354+
return {
355+
filteredChats,
356+
handleSearch,
357+
}
358+
}

0 commit comments

Comments
 (0)