Skip to content

Commit 67f068a

Browse files
committed
fix(search): no cached member rows with access off, no double enrollment, code-point-safe matching
Surfaces consume member-connector rows only while the feature is on; the Search page treats an awaited enrollment, including a first connect, as non-actionable; a refused members-mode sync rolls back only the member lists; result actions show on pointers without hover; route error causes go through the redacting describer; the citation template is valid JSON; term edges and snippet windows respect code points.
1 parent 3a15ed2 commit 67f068a

10 files changed

Lines changed: 82 additions & 24 deletions

File tree

apps/sim/app/workspace/[workspaceId]/home/components/knowledge-search-results/knowledge-search-results.tsx

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -199,10 +199,13 @@ export function KnowledgeSearchResults({
199199
* the viewer will see, and the list is not worth asking for.
200200
*/
201201
const memberAccessAvailable = features?.knowledgeMemberAccess === true
202-
const { data: memberConnectors = EMPTY_MEMBER_CONNECTORS } = useWorkspaceMemberConnectors(
203-
workspaceId,
204-
{ enabled: memberAccessAvailable }
205-
)
202+
const { data: memberConnectorRows } = useWorkspaceMemberConnectors(workspaceId, {
203+
enabled: memberAccessAvailable,
204+
})
205+
/** Rows cached before the feature went off are not this surface's to show. */
206+
const memberConnectors = memberAccessAvailable
207+
? (memberConnectorRows ?? EMPTY_MEMBER_CONNECTORS)
208+
: EMPTY_MEMBER_CONNECTORS
206209
const indexing = indexingSourceNames(memberConnectors, knowledgeBaseIds)
207210
const documents = useMemo(() => groupResultsByDocument(results ?? []), [results])
208211
const sourceTypes = useMemo(

apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/source-card/source-card.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ export function SourceCard({ source, query, onSummarize }: SourceCardProps) {
168168
</p>
169169
)}
170170
</div>
171-
<div className='flex flex-shrink-0 items-center gap-1 self-start opacity-0 transition-opacity group-focus-within/source:opacity-100 group-hover/source:opacity-100'>
171+
<div className='flex flex-shrink-0 items-center gap-1 self-start opacity-0 transition-opacity group-focus-within/source:opacity-100 group-hover/source:opacity-100 [@media(hover:none)]:opacity-100'>
172172
<CopyLinkAction url={source.url} />
173173
{onSummarize && (
174174
<Button variant='ghost' size='sm' onClick={() => onSummarize(source)}>

apps/sim/app/workspace/[workspaceId]/home/components/search-sources/search-sources.tsx

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,10 +145,13 @@ export function SearchSources({ workspaceId }: SearchSourcesProps) {
145145
* off, a connect is refused, so the chips say so instead of offering one.
146146
*/
147147
const memberAccessAvailable = features?.knowledgeMemberAccess === true
148-
const { data: memberConnectors = EMPTY_MEMBER_CONNECTORS } = useWorkspaceMemberConnectors(
149-
workspaceId,
150-
{ enabled: memberAccessAvailable }
151-
)
148+
const { data: memberConnectorRows } = useWorkspaceMemberConnectors(workspaceId, {
149+
enabled: memberAccessAvailable,
150+
})
151+
/** Rows cached before the feature went off are not this surface's to show. */
152+
const memberConnectors = memberAccessAvailable
153+
? (memberConnectorRows ?? EMPTY_MEMBER_CONNECTORS)
154+
: EMPTY_MEMBER_CONNECTORS
152155
const connectionByType = useMemo(
153156
() => simSearchConnectionsByType(memberConnectors),
154157
[memberConnectors]

apps/sim/app/workspace/[workspaceId]/search/search.test.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ vi.mock('@/hooks/use-member-enrollment', async () => {
140140
setupConnector: null,
141141
closeSetup: () => {},
142142
isAwaiting: () => false,
143+
isAwaitingSource: () => false,
143144
isPending: false,
144145
error: null,
145146
}),

apps/sim/app/workspace/[workspaceId]/search/search.tsx

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ function SourceRow({
9090
: connector.meta.description
9191
const description = unavailableReason ?? (personal ? state : NEEDS_KNOWLEDGE_BASE_SETUP)
9292
const connectable =
93-
!unavailable && personal && (!membership || CONNECTABLE_MEMBERSHIPS.has(membership))
93+
!unavailable && !waiting && personal && (!membership || CONNECTABLE_MEMBERSHIPS.has(membership))
9494
return (
9595
<SettingsResourceRow
9696
iconVariant='custom'
@@ -141,8 +141,14 @@ export function Search() {
141141
*/
142142
const setSearchTerm = useDebouncedSearchSetter(setSearchTermParam)
143143

144-
const { data: memberConnectors = EMPTY_MEMBER_CONNECTORS, isPending: connectionsPending } =
145-
useWorkspaceMemberConnectors(workspaceId, { enabled: memberAccessAvailable })
144+
const { data: memberConnectorRows, isPending: connectionsPending } = useWorkspaceMemberConnectors(
145+
workspaceId,
146+
{ enabled: memberAccessAvailable }
147+
)
148+
/** Rows cached before the feature went off are not this surface's to show. */
149+
const memberConnectors = memberAccessAvailable
150+
? (memberConnectorRows ?? EMPTY_MEMBER_CONNECTORS)
151+
: EMPTY_MEMBER_CONNECTORS
146152
useScrollRestoration(scrollContainerRef, {
147153
ready: !memberAccessAvailable || !connectionsPending,
148154
})
@@ -179,6 +185,7 @@ export function Search() {
179185
setupConnector,
180186
closeSetup,
181187
isAwaiting,
188+
isAwaitingSource,
182189
isPending,
183190
error,
184191
} = useMemberEnrollment({
@@ -237,7 +244,11 @@ export function Search() {
237244
integrationAvailability,
238245
memberAccessAvailable
239246
)}
240-
waiting={connection ? isAwaiting(connection.connectorId) : false}
247+
waiting={
248+
connection
249+
? isAwaiting(connection.connectorId)
250+
: isAwaitingSource(connector.type)
251+
}
241252
isPending={isPending}
242253
onConnect={() => connectSearchSource(workspaceId, connector, connection)}
243254
/>

apps/sim/hooks/queries/kb/connectors.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -524,10 +524,15 @@ export function useTriggerSync() {
524524
if (previous) {
525525
setCachedConnectorStatus(queryClient, knowledgeBaseId, connectorId, previous)
526526
}
527-
queryClient.invalidateQueries({ queryKey: connectorKeys.all(knowledgeBaseId) })
528-
/** The member-connector list took the same optimistic `pending`; a refetch is its rollback. */
527+
/**
528+
* The member-connector list took the same optimistic `pending`; a refetch
529+
* is its rollback, and the connector list's own status was restored above,
530+
* so it is not refetched over concurrent optimistic patches.
531+
*/
529532
if (previous && 'memberSyncStatus' in previous) {
530533
queryClient.invalidateQueries({ queryKey: memberConnectorKeys.lists() })
534+
} else {
535+
queryClient.invalidateQueries({ queryKey: connectorKeys.all(knowledgeBaseId) })
531536
}
532537
},
533538
/**

apps/sim/lib/copilot/tools/server/knowledge/knowledge-base.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ const DEFAULT_QUERY_TOP_K = 5
6666
* a source URL is quoted by name instead.
6767
*/
6868
const KNOWLEDGE_CITATION_INSTRUCTION =
69-
'Cite each result you use inline, right after the sentence it supports, as <source>{"url":<sourceUrl>,"title":<documentName>,"siteName":<knowledgeBaseName>,"connectorType":<connectorType>,"snippet":<the sentence or two of content you relied on>,"updatedAt":<sourceModifiedAt>,"author":<author>}</source>; leave out any optional field whose value is null or unknown, and omit the tag for a result whose sourceUrl is null and name the document instead.'
69+
'Cite each result you use inline, right after the sentence it supports, as <source>{"url":"<sourceUrl>","title":"<documentName>","siteName":"<knowledgeBaseName>","connectorType":"<connectorType>","snippet":"<the sentence or two of content you relied on>","updatedAt":"<sourceModifiedAt>","author":"<author>"}</source> with every value JSON-escaped; leave out any optional field whose value is null or unknown, and omit the tag for a result whose sourceUrl is null and name the document instead.'
7070

7171
/**
7272
* Resolves an environment-variable reference passed as a connector API key.

apps/sim/lib/core/utils/with-route-handler.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { createLogger, runWithRequestContext } from '@sim/logger'
2-
import { getErrorMessage, getPostgresErrorCode } from '@sim/utils/errors'
2+
import { describeError, getErrorMessage } from '@sim/utils/errors'
33
import type { NextRequest } from 'next/server'
44
import { NextResponse } from 'next/server'
55
import { getRateLimitHeaders } from '@/lib/api/server/rate-limit-context'
@@ -90,14 +90,15 @@ function traceIdFromTraceparent(header: string | null | undefined): string | und
9090
/**
9191
* What a wrapped error hides: a query failure from the database client carries
9292
* the driver's reason and the Postgres code on its cause, and only the outer
93-
* message names the query.
93+
* message names the query. The shared describer reads the deepest cause and
94+
* strips bound parameter values, so user data never reaches the log.
9495
*/
9596
function errorDetail(error: unknown): { cause?: string; code?: string } {
96-
const cause = error instanceof Error && error.cause !== undefined ? error.cause : undefined
97-
const code = getPostgresErrorCode(error)
97+
if (!(error instanceof Error) || error.cause === undefined) return {}
98+
const described = describeError(error)
9899
return {
99-
...(cause !== undefined ? { cause: getErrorMessage(cause) } : {}),
100-
...(code ? { code } : {}),
100+
cause: `${described.name}: ${described.message}`,
101+
...(described.code ? { code: described.code } : {}),
101102
}
102103
}
103104

apps/sim/lib/knowledge/search/snippet.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,10 @@ describe('findTermMatches', () => {
6060
expect(findTermMatches('東京の天気', ['天気'])).toEqual([{ index: 3, length: 2 }])
6161
})
6262

63+
it('reads whole characters beside a hit, not code units', () => {
64+
expect(findTermMatches('𝔘nicode volvo𝔘 volvo', ['volvo'])).toEqual([{ index: 17, length: 5 }])
65+
})
66+
6367
it('skips a hit glued to another word character', () => {
6468
expect(findTermMatches('subvolvo volvo_x volvo', ['volvo'])).toEqual([{ index: 17, length: 5 }])
6569
})
@@ -85,6 +89,14 @@ describe('matchSnippet', () => {
8589
expect(matchSnippet(german, 'Zürich')).toContain('nach Zürich')
8690
})
8791

92+
it('never splits a surrogate pair at a window edge', () => {
93+
const emoji = `${'🙂'.repeat(200)} volvo ${'🙂'.repeat(200)}`
94+
const loneSurrogate = /[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(?<![\uD800-\uDBFF])[\uDC00-\uDFFF]/
95+
for (const snippet of [matchSnippet(emoji, 'volvo'), matchSnippet(emoji, 'none')]) {
96+
expect(loneSurrogate.test(snippet)).toBe(false)
97+
}
98+
})
99+
88100
it('falls back to the opening when no term appears in the chunk', () => {
89101
const snippet = matchSnippet(EMAIL, 'unrelated')
90102
expect(snippet.startsWith('Thanks for your patience.')).toBe(true)

apps/sim/lib/knowledge/search/snippet.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,15 +68,35 @@ export function findTermMatches(text: string, terms: readonly string[]): TermMat
6868
const pattern = new RegExp(terms.map(escapeRegExp).join('|'), 'giu')
6969
const matches: TermMatch[] = []
7070
for (const match of text.matchAll(pattern)) {
71-
const before = text[match.index - 1]
72-
const after = text[match.index + match[0].length]
71+
const before = codePointBefore(text, match.index)
72+
const after = codePointAt(text, match.index + match[0].length)
7373
if (before !== undefined && WORD_CHARACTER.test(before)) continue
7474
if (after !== undefined && WORD_CHARACTER.test(after)) continue
7575
matches.push({ index: match.index, length: match[0].length })
7676
}
7777
return matches
7878
}
7979

80+
/** The whole character starting at a code-unit index, or undefined past the end. */
81+
function codePointAt(text: string, index: number): string | undefined {
82+
const code = text.codePointAt(index)
83+
return code === undefined ? undefined : String.fromCodePoint(code)
84+
}
85+
86+
/** The whole character ending just before a code-unit index, or undefined at the start. */
87+
function codePointBefore(text: string, index: number): string | undefined {
88+
if (index <= 0) return undefined
89+
const unit = text.charCodeAt(index - 1)
90+
const start = unit >= 0xdc00 && unit <= 0xdfff && index >= 2 ? index - 2 : index - 1
91+
return codePointAt(text, start)
92+
}
93+
94+
/** An index moved off the middle of a surrogate pair, so a slice never splits a character. */
95+
function alignToCodePoint(text: string, index: number): number {
96+
const unit = text.charCodeAt(index)
97+
return unit >= 0xdc00 && unit <= 0xdfff ? index - 1 : index
98+
}
99+
80100
/**
81101
* The passage of a document a search result shows: a window around the first
82102
* query term found, the way a search page shows why a document matched, and
@@ -94,11 +114,13 @@ export function matchSnippet(content: string, query?: string): string {
94114
const boundary = flat.indexOf(' ', start)
95115
if (boundary !== -1 && boundary - start < LEAD_LENGTH) start = boundary + 1
96116
}
117+
start = alignToCodePoint(flat, start)
97118
if (flat.length - start <= SNIPPET_LENGTH) {
98119
return `${start > 0 ? '…' : ''}${flat.slice(start)}`
99120
}
100121
let end = start + SNIPPET_LENGTH
101122
const lastSpace = flat.lastIndexOf(' ', end)
102123
if (lastSpace > start + SNIPPET_LENGTH / 2) end = lastSpace
124+
end = alignToCodePoint(flat, end)
103125
return `${start > 0 ? '…' : ''}${flat.slice(start, end).trimEnd()}…`
104126
}

0 commit comments

Comments
 (0)