Skip to content

Commit 6cd7537

Browse files
committed
fix(files): address the paged file reads' second review round
- Only a serve URL is read as a storage key with a query: a stored name or path holding `?` or `#` is looked up by reference (and by key when it has a `/`), in search-replace and picker selections alike (`storageKeyOfStoredValue`). - The `@` menu reads as loading, not "No results", while the file search behind its query is settling: the mention store carries the picker's pending state alongside the items. - The Audio and Video filters match any `audio/*` or `video/*` type, as the Type label does, in SQL and JS for both browsers (`fileKindSql`, `fileBrowserKinds`); the parity test covers types the MIME table does not list.
1 parent 03d3162 commit 6cd7537

15 files changed

Lines changed: 156 additions & 55 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/mention/mention-list.tsx‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {
55
type SuggestionKeyDownHandler,
66
useSuggestionKeyboard,
77
} from '../menus/use-suggestion-keyboard'
8-
import type { MentionStore } from './mention-store'
8+
import type { MentionMenuData, MentionStore } from './mention-store'
99
import type { MentionItem } from './types'
1010

1111
/**
@@ -16,6 +16,15 @@ export function mentionItemMatches(item: MentionItem, query: string): boolean {
1616
return item.matchedQuery?.toLowerCase() === query || item.label.toLowerCase().includes(query)
1717
}
1818

19+
/**
20+
* What the menu says with nothing to list: still loading while it has no data yet or its file
21+
* search is settling — the other families alone can fill it, so an empty match is not yet
22+
* "no results" until the files answering the query have landed.
23+
*/
24+
export function mentionEmptyLabel(menu: MentionMenuData): string {
25+
return menu.items.length === 0 || menu.filesPending ? 'Loading…' : 'No results'
26+
}
27+
1928
export type MentionListHandle = SuggestionKeyDownHandler
2029

2130
interface MentionListProps {
@@ -58,7 +67,8 @@ export const MentionList = forwardRef<MentionListHandle, MentionListProps>(funct
5867
{ query, command, store, editor },
5968
ref
6069
) {
61-
const rawItems = useSyncExternalStore(store.subscribe, store.getSnapshot, store.getSnapshot)
70+
const menu = useSyncExternalStore(store.subscribe, store.getSnapshot, store.getSnapshot)
71+
const rawItems = menu.items
6272
const containerRef = useRef<HTMLDivElement>(null)
6373

6474
/**
@@ -109,7 +119,7 @@ export const MentionList = forwardRef<MentionListHandle, MentionListProps>(funct
109119
command={command}
110120
ariaLabel='Mentions'
111121
idPrefix='mention'
112-
emptyLabel={rawItems.length === 0 ? 'Loading…' : 'No results'}
122+
emptyLabel={mentionEmptyLabel(menu)}
113123
itemKey={(item) => `${item.kind}:${item.id}`}
114124
renderItem={(item) => {
115125
const Icon = item.icon

‎apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/mention/mention-matching.test.ts‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import { describe, expect, it } from 'vitest'
2-
import { mentionItemMatches } from '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/mention/mention-list'
2+
import {
3+
mentionEmptyLabel,
4+
mentionItemMatches,
5+
} from '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/mention/mention-list'
36

47
describe('mentionItemMatches', () => {
58
const file = { kind: 'file' as const, id: 'wf_a', label: 'Quarterly-Report.md', group: 'Files' }
@@ -13,3 +16,15 @@ describe('mentionItemMatches', () => {
1316
expect(mentionItemMatches(file, 'report')).toBe(true)
1417
})
1518
})
19+
20+
describe('mentionEmptyLabel', () => {
21+
const file = { kind: 'file' as const, id: 'wf_a', label: 'a.md', group: 'Files' }
22+
23+
it('reads as loading, not "No results", while the file search behind a query is settling', () => {
24+
expect(mentionEmptyLabel({ items: [file], filesPending: true })).toBe('Loading…')
25+
})
26+
27+
it('says "No results" once the files for the query have landed', () => {
28+
expect(mentionEmptyLabel({ items: [file], filesPending: false })).toBe('No results')
29+
})
30+
})

‎apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/mention/mention-store.ts‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,29 +3,37 @@ import type { MentionItem } from './types'
33
/**
44
* A tiny external store bridging React Query data (host component) into the `@` menu list, which is
55
* rendered by TipTap's `ReactRenderer` as a detached root with no access to the app's React context
6-
* providers. The host pushes the latest items via {@link MentionStore.set}; the list subscribes with
6+
* providers. The host pushes the latest items (and whether the file search is settling) via {@link MentionStore.set}; the list subscribes with
77
* `useSyncExternalStore` and re-renders when async data lands — so the menu populates live even if it
88
* was opened before the data finished loading. One store instance lives per editor (in extension
99
* storage).
1010
*/
1111
export interface MentionStore {
12-
getSnapshot: () => MentionItem[]
12+
getSnapshot: () => MentionMenuData
1313
subscribe: (listener: () => void) => () => void
14-
set: (items: MentionItem[]) => void
14+
set: (data: MentionMenuData) => void
1515
}
1616

17+
/** The menu's items, and whether the file search behind them is still settling. */
18+
export interface MentionMenuData {
19+
items: MentionItem[]
20+
filesPending: boolean
21+
}
22+
23+
const EMPTY_MENU: MentionMenuData = { items: [], filesPending: false }
24+
1725
export function createMentionStore(): MentionStore {
18-
let items: MentionItem[] = []
26+
let data = EMPTY_MENU
1927
const listeners = new Set<() => void>()
2028
return {
21-
getSnapshot: () => items,
29+
getSnapshot: () => data,
2230
subscribe: (listener) => {
2331
listeners.add(listener)
2432
return () => listeners.delete(listener)
2533
},
2634
set: (next) => {
27-
if (next === items) return
28-
items = next
35+
if (next === data) return
36+
data = next
2937
for (const listener of listeners) listener()
3038
},
3139
}

‎apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/mention/use-editor-mentions.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ export function useEditorMentions(
2222
): void {
2323
const [active, setActive] = useState(false)
2424
const [query, setQuery] = useState('')
25-
const items = useMarkdownMentions(workspaceId, { enabled: active, fileQuery: query })
25+
const menu = useMarkdownMentions(workspaceId, { enabled: active, fileQuery: query })
2626
const navigable = options?.navigable ?? false
2727
const disableTagging = options?.disableTagging ?? false
2828

@@ -40,6 +40,6 @@ export function useEditorMentions(
4040
}, [editor, workspaceId, navigable, disableTagging])
4141

4242
useEffect(() => {
43-
editor?.storage.mentionMenu.store.set(items)
44-
}, [editor, items])
43+
editor?.storage.mentionMenu.store.set(menu)
44+
}, [editor, menu])
4545
}

‎apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/mention/use-markdown-mentions.ts‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { useMemo } from 'react'
2+
import type { MentionMenuData } from '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/mention/mention-store'
23
import { listIntegrations } from '@/blocks/integration-matcher'
34
import { useKnowledgeBasesQuery } from '@/hooks/queries/kb/knowledge'
45
import { useSkills } from '@/hooks/queries/skills'
@@ -18,18 +19,24 @@ import type { MentionItem } from './types'
1819
* Files are the one family not listed whole: the menu offers the most recently updated, and once
1920
* `fileQuery` (the text after the `@`) settles, the files whose names match it anywhere.
2021
*/
22+
const NO_MENTION_ITEMS: MentionItem[] = []
23+
2124
export function useMarkdownMentions(
2225
workspaceId: string | undefined,
2326
options: { enabled: boolean; fileQuery?: string }
24-
): MentionItem[] {
27+
): MentionMenuData {
2528
const active = options.enabled && Boolean(workspaceId)
2629
// When inactive, `ws` is undefined and `wsStr` is '' so every query stays disabled until the first
2730
// `@`: the hooks that expose an `enabled` option get it explicitly; the rest (which take no options)
2831
// self-gate internally on the falsy workspaceId — both empty string and undefined read as disabled.
2932
const ws = active ? workspaceId : undefined
3033
const wsStr = ws ?? ''
3134

32-
const { files, matchedQuery } = useWorkspaceFilePickerOptions(wsStr, options.fileQuery ?? '', {
35+
const {
36+
files,
37+
matchedQuery,
38+
isPending: filesPending,
39+
} = useWorkspaceFilePickerOptions(wsStr, options.fileQuery ?? '', {
3340
enabled: active,
3441
})
3542
const folders = useWorkspaceFileFolders(wsStr, 'active')
@@ -50,8 +57,8 @@ export function useMarkdownMentions(
5057
}))
5158
}, [active])
5259

53-
return useMemo(() => {
54-
if (!active) return []
60+
const items = useMemo(() => {
61+
if (!active) return NO_MENTION_ITEMS
5562
const items: MentionItem[] = []
5663

5764
for (const file of files ?? [])
@@ -117,4 +124,7 @@ export function useMarkdownMentions(
117124
skills.data,
118125
integrationItems,
119126
])
127+
/** Files still answering an older query (or none yet) keep the menu from reading as empty. */
128+
const pending = active && filesPending
129+
return useMemo(() => ({ items, filesPending: pending }), [items, pending])
120130
}

‎apps/sim/hooks/queries/workflow-search-replace.test.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,4 +94,11 @@ describe('workflowSearchFileLookups', () => {
9494
}
9595
)
9696
})
97+
98+
it('looks a name or path holding `?` or `#` up by reference, never as a serve URL', () => {
99+
expect(workflowSearchFileLookups(['what?.md', 'docs/q&a #1?.md'])).toEqual({
100+
references: ['what?.md', 'docs/q&a #1?.md'],
101+
keyValues: ['docs/q&a #1?.md'],
102+
})
103+
})
97104
})

‎apps/sim/hooks/queries/workflow-search-replace.ts‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import {
3030
type SelectorKey,
3131
} from '@/lib/selectors/manifest'
3232
import type { SelectorOption, SelectorScope } from '@/lib/selectors/types'
33-
import { tryExtractStorageKey } from '@/lib/uploads/utils/file'
33+
import { isInternalFileUrl, storageKeyOfStoredValue } from '@/lib/uploads/utils/file'
3434
import { getWorkflowSearchMatchResourceGroupKey } from '@/lib/workflows/search-replace/resources'
3535
import type {
3636
WorkflowSearchMatch,
@@ -346,9 +346,10 @@ export function useWorkflowSearchTableDetails(
346346
}
347347

348348
/**
349-
* What to look a stored file value up by. It is an id, a storage key, a serve path, or a name: a
350-
* reference lookup answers an id (its first rule), a path, and a name; a storage key — every one
351-
* has a `/` — and a serve path also need the key lookup, which a plain name or id never would.
349+
* What to look a stored file value up by. It is an id, a storage key, a serve URL, or a name or
350+
* path: a serve URL is looked up by the key it carries (nothing, when its encoding is malformed);
351+
* anything else by reference, which answers an id (its first rule), a path, and a name — and also
352+
* by key when it has a `/`, as every storage key does. Only a serve URL reads `?` as a query.
352353
*/
353354
export function workflowSearchFileLookups(rawValues: readonly string[]): {
354355
references: string[]
@@ -357,11 +358,9 @@ export function workflowSearchFileLookups(rawValues: readonly string[]): {
357358
const references = new Set<string>()
358359
const keyValues = new Set<string>()
359360
for (const rawValue of rawValues) {
360-
const key = tryExtractStorageKey(rawValue)
361-
// A serve path with malformed percent-encoding names nothing a lookup can find.
362-
if (key === undefined) continue
363-
if (key !== rawValue) {
364-
keyValues.add(key)
361+
if (isInternalFileUrl(rawValue)) {
362+
const key = storageKeyOfStoredValue(rawValue)
363+
if (key) keyValues.add(key)
365364
continue
366365
}
367366
references.add(rawValue)
@@ -394,7 +393,7 @@ export function useWorkflowSearchFileDetails(matches: WorkflowSearchMatch[], wor
394393
() =>
395394
fileMatches.map((match) => {
396395
const file =
397-
byKey.byValue.get(tryExtractStorageKey(match.rawValue) ?? '') ??
396+
byKey.byValue.get(storageKeyOfStoredValue(match.rawValue) ?? '') ??
398397
byReference.byValue.get(match.rawValue)
399398
return {
400399
data: settled

‎apps/sim/hooks/queries/workspace-files.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ import { FileTextSizeError, readFileText } from '@/lib/uploads/client/text-conte
4141
import type { UploadProgressEvent } from '@/lib/uploads/client/types'
4242
import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace'
4343
import { extractEmbeddedFileRef, storedFileId } from '@/lib/uploads/utils/embedded-image-ref'
44-
import { tryExtractStorageKey } from '@/lib/uploads/utils/file'
44+
import { storageKeyOfStoredValue } from '@/lib/uploads/utils/file'
4545
import { SEARCH_DEBOUNCE_MS } from '@/lib/url-state'
4646
import type { WorkspaceFileSelection } from '@/lib/workspace-files/selection'
4747
import type { UserFile } from '@/executor/types'
@@ -421,7 +421,7 @@ export function useWorkspaceFileSelectionCandidates(
421421
else if (selection.key) keys.add(selection.key)
422422
else if (selection.path) {
423423
// A persisted path with malformed percent-encoding names no file; it stays unresolved.
424-
const key = tryExtractStorageKey(selection.path)
424+
const key = storageKeyOfStoredValue(selection.path)
425425
if (key) keys.add(key)
426426
} else if (selection.name.trim()) names.add(selection.name.trim())
427427
}

‎apps/sim/lib/uploads/utils/file.test.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
resolveEffectiveMimeType,
1111
resolveFileType,
1212
resolveTrustedFileContext,
13+
storageKeyOfStoredValue,
1314
} from '@/lib/uploads/utils/file'
1415

1516
const logger = createLogger('FileUtilsTest')
@@ -184,3 +185,23 @@ describe('observedMediaType', () => {
184185
expect(observedMediaType('IMAGE/PNG', 'photo.png')).toBe('image/png')
185186
})
186187
})
188+
189+
describe('storageKeyOfStoredValue', () => {
190+
it('reads a serve URL as the key it carries, its query dropped', () => {
191+
expect(
192+
storageKeyOfStoredValue('/api/files/serve/workspace%2Fws-1%2Fa.png?context=workspace')
193+
).toBe('workspace/ws-1/a.png')
194+
expect(storageKeyOfStoredValue('https://sim.ai/api/files/serve/s3/workspace%2Fb.md')).toBe(
195+
'workspace/b.md'
196+
)
197+
})
198+
199+
it('keeps any other value whole, `?` and `#` included', () => {
200+
expect(storageKeyOfStoredValue('workspace/ws-1/what?.md')).toBe('workspace/ws-1/what?.md')
201+
expect(storageKeyOfStoredValue('notes #1?.md')).toBe('notes #1?.md')
202+
})
203+
204+
it('names nothing for a serve URL whose encoding is malformed, instead of throwing', () => {
205+
expect(storageKeyOfStoredValue('/api/files/serve/workspace%E0%A4%A.png')).toBeUndefined()
206+
})
207+
})

‎apps/sim/lib/uploads/utils/file.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -687,12 +687,14 @@ export function extractStorageKey(filePath: string): string {
687687
}
688688

689689
/**
690-
* {@link extractStorageKey} for a path read back from storage or a document, which may be
691-
* malformed: `undefined` when its percent-encoding cannot be decoded, rather than throwing.
690+
* The storage key a stored file value names: a serve URL's key (see {@link isInternalFileUrl}),
691+
* else the value itself — a name or path may hold `?` or `#`, which only a URL reads as its query.
692+
* `undefined` for a serve URL whose percent-encoding cannot be decoded, rather than throwing.
692693
*/
693-
export function tryExtractStorageKey(filePath: string): string | undefined {
694+
export function storageKeyOfStoredValue(value: string): string | undefined {
695+
if (!isInternalFileUrl(value)) return value
694696
try {
695-
return extractStorageKey(filePath)
697+
return extractStorageKey(value)
696698
} catch {
697699
return undefined
698700
}

0 commit comments

Comments
 (0)