Skip to content

Commit 4b42c2c

Browse files
committed
fix(search): safe result links, script-aware term matching, and honest header stripping
A result links only to an http(s) URL; term matching judges word edges by the surrounding characters instead of ASCII \b and strips quotes from a phrase, and the same matcher bolds the passage; a chunk that is nothing but fields keeps its content; the agent leaves unknown optional citation fields out and keeps the tool's published result count; route errors log the wrapped cause and Postgres code.
1 parent 4d88736 commit 4b42c2c

9 files changed

Lines changed: 139 additions & 54 deletions

File tree

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@ import {
1414
SOURCE_ROW_MARK_CLASSES,
1515
SourceCard,
1616
} from '@/app/workspace/[workspaceId]/home/components/message-content/components/source-card'
17-
import type { SourceTagData } from '@/app/workspace/[workspaceId]/home/components/message-content/components/special-tags'
17+
import {
18+
isHttpUrl,
19+
type SourceTagData,
20+
} from '@/app/workspace/[workspaceId]/home/components/message-content/components/special-tags'
1821
import { isIndexing } from '@/app/workspace/[workspaceId]/home/components/search-sources'
1922
import {
2023
resourceUrlKeys,
@@ -76,11 +79,12 @@ export function indexingSourceNames(
7679

7780
/**
7881
* A result as the source card renders it: the row's second line names the
79-
* source app, or the knowledge base for an upload. A document without a
80-
* source URL cannot be opened.
82+
* source app, or the knowledge base for an upload. A document without an
83+
* http(s) source URL cannot be opened, and a connector-supplied value of any
84+
* other scheme is never handed to the browser as a link.
8185
*/
8286
function toSource(result: WorkspaceKnowledgeSearchResult, query: string): SourceTagData | null {
83-
if (!result.sourceUrl) return null
87+
if (!isHttpUrl(result.sourceUrl)) return null
8488
return {
8589
url: result.sourceUrl,
8690
title: result.documentName ?? undefined,

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

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { createLogger } from '@sim/logger'
77
import { getErrorMessage } from '@sim/utils/errors'
88
import { formatDate } from '@sim/utils/formatting'
99
import { faviconUrl } from '@/lib/core/utils/favicon'
10+
import { findTermMatches, queryTerms } from '@/lib/knowledge/search/snippet'
1011
import {
1112
externalLinkHostname,
1213
handleExternalLinkClick,
@@ -21,8 +22,6 @@ import { BrandIcon } from '@/blocks/brand-icon'
2122

2223
const logger = createLogger('SourceCard')
2324

24-
/** Query terms shorter than this are too common to bold. */
25-
const MIN_HIGHLIGHT_TERM_LENGTH = 3
2625
/** How long the copied state shows on the copy-link action. */
2726
const COPIED_FEEDBACK_MS = 1_500
2827

@@ -37,35 +36,27 @@ export const SOURCE_ROW_CLASSES =
3736
/** The 16px mark slot, nudged to centre on the title's first line. */
3837
export const SOURCE_ROW_MARK_CLASSES = cn(chipIconSlotClass, 'mt-[3px]')
3938

40-
function escapeRegExp(value: string): string {
41-
return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
42-
}
43-
4439
/**
4540
* The snippet with every query term in bold, so the reader sees why the
46-
* document matched. Terms are matched as whole words, case-insensitively.
41+
* document matched. Terms are matched as whole words in any script,
42+
* case-insensitively, by the same rule the snippet was centred with.
4743
*/
4844
export function highlightTerms(text: string, query: string | undefined): ReactNode {
49-
const terms = [
50-
...new Set(
51-
(query ?? '')
52-
.split(/\s+/)
53-
.map((term) => term.trim())
54-
.filter((term) => term.length >= MIN_HIGHLIGHT_TERM_LENGTH)
55-
),
56-
]
57-
if (terms.length === 0) return text
58-
const pattern = new RegExp(`\\b(${terms.map(escapeRegExp).join('|')})\\b`, 'gi')
59-
const parts = text.split(pattern)
60-
return parts.map((part, index) =>
61-
index % 2 === 1 ? (
62-
<strong key={index} className='font-medium text-[var(--text-primary)]'>
63-
{part}
45+
const matches = findTermMatches(text, queryTerms(query))
46+
if (matches.length === 0) return text
47+
const parts: ReactNode[] = []
48+
let cursor = 0
49+
for (const match of matches) {
50+
if (match.index > cursor) parts.push(text.slice(cursor, match.index))
51+
parts.push(
52+
<strong key={match.index} className='font-medium text-[var(--text-primary)]'>
53+
{text.slice(match.index, match.index + match.length)}
6454
</strong>
65-
) : (
66-
part
6755
)
68-
)
56+
cursor = match.index + match.length
57+
}
58+
if (cursor < text.length) parts.push(text.slice(cursor))
59+
return parts
6960
}
7061

7162
function parseUpdatedAt(value: string | undefined): Date | null {

apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ export {
2525
CredentialDisplay,
2626
credentialTagHasVisibleCard,
2727
formatCredentialSubmissionMessage,
28+
isHttpUrl,
2829
PendingTagIndicator,
2930
parseCredentialSubmissionMessage,
3031
parseCredentialSubmissionProgress,

apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,7 @@ function isMothershipErrorTagData(value: unknown): value is MothershipErrorTagDa
567567
* a source. Parsed rather than pattern-matched so a malformed value such as
568568
* `https://?` — which a prefix check would accept — never becomes a dead link.
569569
*/
570-
function isHttpUrl(value: unknown): value is string {
570+
export function isHttpUrl(value: unknown): value is string {
571571
if (typeof value !== 'string' || /\s/.test(value)) return false
572572
try {
573573
const url = new URL(value)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,7 @@ describe('manage_knowledge_base trusted application delegation', () => {
401401
workspaceId: 'workspace-paid',
402402
knowledgeBaseIds: [KNOWLEDGE_BASE.id],
403403
query: '{{KB_QUERY}}',
404-
topK: 10,
404+
topK: 5,
405405
resultSecretRegistry: registry,
406406
})
407407
expect(mockReadKnowledgeBase).not.toHaveBeenCalled()

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,14 @@ import { projectResolvedSecretModelContent } from '@/executor/utils/resolved-sec
5959
const logger = createLogger('KnowledgeBaseServerTool')
6060

6161
/** Results a query returns unless the caller asks for a number. */
62-
const DEFAULT_QUERY_TOP_K = 10
62+
const DEFAULT_QUERY_TOP_K = 5
6363
/**
6464
* How the model cites a knowledge result in its reply. The `<source>` tag is
6565
* what the chat renders as a link back to the document, so a result without
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>; 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>; 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: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { createLogger, runWithRequestContext } from '@sim/logger'
2-
import { getErrorMessage } from '@sim/utils/errors'
2+
import { getErrorMessage, getPostgresErrorCode } 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'
@@ -87,6 +87,20 @@ function traceIdFromTraceparent(header: string | null | undefined): string | und
8787
return match[1]
8888
}
8989

90+
/**
91+
* What a wrapped error hides: a query failure from the database client carries
92+
* the driver's reason and the Postgres code on its cause, and only the outer
93+
* message names the query.
94+
*/
95+
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)
98+
return {
99+
...(cause !== undefined ? { cause: getErrorMessage(cause) } : {}),
100+
...(code ? { code } : {}),
101+
}
102+
}
103+
90104
/**
91105
* Wraps a Next.js API route handler with centralized error reporting.
92106
*
@@ -119,6 +133,7 @@ export function withRouteHandler<T>(
119133
} catch (error) {
120134
const duration = Date.now() - startTime
121135
const message = getErrorMessage(error, 'Unknown error')
136+
const detail = errorDetail(error)
122137
if (request.signal.aborted) {
123138
logger.info('Client closed request', { duration, status: 499 })
124139
response = options.clientAbortResponse
@@ -132,7 +147,12 @@ export function withRouteHandler<T>(
132147
if (typedError) {
133148
const typedStatus = typedError.statusCode
134149
if (typedStatus >= 500) {
135-
logger.error('Unhandled route error', { duration, status: typedStatus, error: message })
150+
logger.error('Unhandled route error', {
151+
duration,
152+
status: typedStatus,
153+
error: message,
154+
...detail,
155+
})
136156
} else {
137157
logger.warn('Typed route error', { duration, status: typedStatus, error: message })
138158
}
@@ -144,13 +164,13 @@ export function withRouteHandler<T>(
144164
}
145165

146166
if (options.unhandledErrorResponse) {
147-
logger.error('Unhandled route error', { duration, error: message })
167+
logger.error('Unhandled route error', { duration, error: message, ...detail })
148168
response = options.unhandledErrorResponse({ error, requestId })
149169
applyResponseHeaders(response, request, requestId)
150170
return response
151171
}
152172

153-
logger.error('Unhandled route error', { duration, error: message })
173+
logger.error('Unhandled route error', { duration, error: message, ...detail })
154174
response = NextResponse.json({ error: 'Internal server error', requestId }, { status: 500 })
155175
applyResponseHeaders(response, request, requestId)
156176
return response

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

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,10 @@
33
*/
44
import { describe, expect, it } from 'vitest'
55
import {
6+
findTermMatches,
67
matchSnippet,
8+
queryTerms,
79
SNIPPET_LENGTH,
8-
snippetTerms,
910
stripLeadingHeaders,
1011
} from '@/lib/knowledge/search/snippet'
1112

@@ -18,6 +19,10 @@ const EMAIL = [
1819
`${'Thanks for your patience. '.repeat(12)}The Volvo order shipped on Monday and the tracking number follows. ${'More text here. '.repeat(20)}`,
1920
].join('\n')
2021

22+
const EVENT = ['Title: Weekly sync', 'Organizer: Ada', 'When: Monday 9am', 'Where: Room 4'].join(
23+
'\n'
24+
)
25+
2126
describe('stripLeadingHeaders', () => {
2227
it('drops the header block a connector writes above an email body', () => {
2328
expect(stripLeadingHeaders(EMAIL).startsWith('\nThanks for your patience.')).toBe(true)
@@ -28,12 +33,35 @@ describe('stripLeadingHeaders', () => {
2833
'Plain prose: with a colon inside.'
2934
)
3035
})
36+
37+
it('keeps a chunk that is nothing but fields, such as a calendar event', () => {
38+
expect(stripLeadingHeaders(EVENT)).toBe(EVENT)
39+
expect(stripLeadingHeaders(`${EVENT}\n\n`)).toBe(`${EVENT}\n\n`)
40+
})
3141
})
3242

33-
describe('snippetTerms', () => {
43+
describe('queryTerms', () => {
3444
it('keeps distinct terms of three or more characters, longest first', () => {
35-
expect(snippetTerms('the Volvo invoice is volvo')).toEqual(['invoice', 'Volvo', 'volvo', 'the'])
36-
expect(snippetTerms(undefined)).toEqual([])
45+
expect(queryTerms('the Volvo invoice is volvo')).toEqual(['invoice', 'Volvo', 'volvo', 'the'])
46+
expect(queryTerms(undefined)).toEqual([])
47+
})
48+
49+
it('strips the quotes and punctuation around a term', () => {
50+
expect(queryTerms('"foo bar" (baz),')).toEqual(['foo', 'bar', 'baz'])
51+
})
52+
})
53+
54+
describe('findTermMatches', () => {
55+
it('matches whole words in any script', () => {
56+
expect(findTermMatches('Der Bericht über Zürich.', ['Zürich'])).toEqual([
57+
{ index: 17, length: 6 },
58+
])
59+
expect(findTermMatches('Reports on Zürichsee.', ['Zürich'])).toEqual([])
60+
expect(findTermMatches('東京の天気', ['天気'])).toEqual([{ index: 3, length: 2 }])
61+
})
62+
63+
it('skips a hit glued to another word character', () => {
64+
expect(findTermMatches('subvolvo volvo_x volvo', ['volvo'])).toEqual([{ index: 17, length: 5 }])
3765
})
3866
})
3967

@@ -51,6 +79,12 @@ describe('matchSnippet', () => {
5179
expect(snippet.length).toBeLessThanOrEqual(SNIPPET_LENGTH + 2)
5280
})
5381

82+
it('centres on a quoted phrase and on a non-ASCII term', () => {
83+
expect(matchSnippet(EMAIL, '"Volvo order"')).toContain('The Volvo order shipped')
84+
const german = `${'Einleitung. '.repeat(30)}Die Lieferung nach Zürich ist unterwegs. ${'Mehr. '.repeat(30)}`
85+
expect(matchSnippet(german, 'Zürich')).toContain('nach Zürich')
86+
})
87+
5488
it('falls back to the opening when no term appears in the chunk', () => {
5589
const snippet = matchSnippet(EMAIL, 'unrelated')
5690
expect(snippet.startsWith('Thanks for your patience.')).toBe(true)

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

Lines changed: 48 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,14 @@ const LEAD_LENGTH = 90
66
const MIN_TERM_LENGTH = 3
77
/** `Key: value` lines a connector writes above an email or ticket body. */
88
const HEADER_LINE = /^[A-Z][A-Za-z-]{1,15}: .*$/
9+
/**
10+
* A character that continues a word, so a term touching one on either side is
11+
* part of a longer word rather than a hit. Scripts written without spaces
12+
* (Han, kana, Hangul, Thai) have no such edges, so their letters never
13+
* disqualify a neighbouring match.
14+
*/
15+
const WORD_CHARACTER =
16+
/(?![\p{sc=Han}\p{sc=Hiragana}\p{sc=Katakana}\p{sc=Hangul}\p{sc=Thai}])[\p{L}\p{N}_]/u
917

1018
function escapeRegExp(value: string): string {
1119
return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
@@ -15,28 +23,60 @@ function escapeRegExp(value: string): string {
1523
* The document text without the header block some connectors prefix (the
1624
* `Subject:` / `From:` / `To:` lines of an email): the title already says
1725
* what the subject is, and a snippet spent on the header never shows why the
18-
* document matched.
26+
* document matched. Only a block the connector closed with a blank line
27+
* counts, and only when a body follows it: a chunk that is nothing but
28+
* `Key: value` fields, such as a calendar event, is the document.
1929
*/
2030
export function stripLeadingHeaders(content: string): string {
2131
const lines = content.split('\n')
2232
let index = 0
2333
while (index < lines.length && HEADER_LINE.test(lines[index].trim())) index += 1
24-
if (index === 0) return content
25-
return lines.slice(index).join('\n')
34+
if (index === 0 || index >= lines.length || lines[index].trim() !== '') return content
35+
const body = lines.slice(index).join('\n')
36+
return body.trim() ? body : content
2637
}
2738

28-
/** The query's terms worth anchoring on, longest first so the most specific one wins. */
29-
export function snippetTerms(query: string | undefined): string[] {
39+
/**
40+
* The query's terms worth matching, longest first so the most specific one
41+
* wins: quotes and other search syntax around a term are not part of it.
42+
*/
43+
export function queryTerms(query: string | undefined): string[] {
3044
return [
3145
...new Set(
3246
(query ?? '')
3347
.split(/\s+/)
34-
.map((term) => term.trim())
48+
.map((term) => term.replace(/^["'(]+|["'),.;:!?]+$/g, '').trim())
3549
.filter((term) => term.length >= MIN_TERM_LENGTH)
3650
),
3751
].sort((a, b) => b.length - a.length)
3852
}
3953

54+
export interface TermMatch {
55+
index: number
56+
length: number
57+
}
58+
59+
/**
60+
* Where the query terms occur in the text as whole words, in order and without
61+
* overlap. Word edges are judged by the characters around a hit rather than
62+
* by `\b`, which knows only ASCII letters, so a term in any script still
63+
* matches; a hit glued to another word character on either side is not a
64+
* word and is skipped.
65+
*/
66+
export function findTermMatches(text: string, terms: readonly string[]): TermMatch[] {
67+
if (terms.length === 0) return []
68+
const pattern = new RegExp(terms.map(escapeRegExp).join('|'), 'giu')
69+
const matches: TermMatch[] = []
70+
for (const match of text.matchAll(pattern)) {
71+
const before = text[match.index - 1]
72+
const after = text[match.index + match[0].length]
73+
if (before !== undefined && WORD_CHARACTER.test(before)) continue
74+
if (after !== undefined && WORD_CHARACTER.test(after)) continue
75+
matches.push({ index: match.index, length: match[0].length })
76+
}
77+
return matches
78+
}
79+
4080
/**
4181
* The passage of a document a search result shows: a window around the first
4282
* query term found, the way a search page shows why a document matched, and
@@ -48,13 +88,8 @@ export function matchSnippet(content: string, query?: string): string {
4888
const flat = stripLeadingHeaders(content).replace(/\s+/g, ' ').trim()
4989
if (flat.length <= SNIPPET_LENGTH) return flat
5090

51-
let start = 0
52-
for (const term of snippetTerms(query)) {
53-
const match = new RegExp(`\\b${escapeRegExp(term)}\\b`, 'i').exec(flat)
54-
if (!match) continue
55-
start = Math.max(0, match.index - LEAD_LENGTH)
56-
break
57-
}
91+
const first = findTermMatches(flat, queryTerms(query))[0]
92+
let start = first ? Math.max(0, first.index - LEAD_LENGTH) : 0
5893
if (start > 0) {
5994
const boundary = flat.indexOf(' ', start)
6095
if (boundary !== -1 && boundary - start < LEAD_LENGTH) start = boundary + 1

0 commit comments

Comments
 (0)