Skip to content

Commit 9c819fa

Browse files
committed
fix(knowledge): stabilize skipped connector documents
1 parent adbb9b6 commit 9c819fa

2 files changed

Lines changed: 42 additions & 10 deletions

File tree

apps/sim/lib/knowledge/connectors/sync-engine.test.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -504,14 +504,36 @@ describe('classifyExternalDoc', () => {
504504
).toEqual({ type: 'skip', existingId: 'doc-1' })
505505
})
506506

507-
it('rehydrates a content-less placeholder even when its listing hash is unchanged', async () => {
507+
it('keeps a stable listing-time skipped placeholder unchanged', async () => {
508+
const { classifyExternalDoc } = await import('@/lib/knowledge/connectors/sync-engine')
509+
510+
expect(
511+
classifyExternalDoc(
512+
{ ...base, content: '', skippedReason: 'too big' },
513+
{ id: 'doc-1', contentHash: 'h1', storageKey: null }
514+
)
515+
).toEqual({ type: 'unchanged' })
516+
})
517+
518+
it('keeps a stable deferred skipped placeholder unchanged', async () => {
508519
const { classifyExternalDoc } = await import('@/lib/knowledge/connectors/sync-engine')
509520

510521
expect(
511522
classifyExternalDoc(
512523
{ ...base, content: '', contentDeferred: true },
513524
{ id: 'doc-1', contentHash: 'h1', storageKey: null }
514525
)
526+
).toEqual({ type: 'unchanged' })
527+
})
528+
529+
it('rehydrates a deferred skipped placeholder carrying an explicit retry hash', async () => {
530+
const { classifyExternalDoc } = await import('@/lib/knowledge/connectors/sync-engine')
531+
532+
expect(
533+
classifyExternalDoc(
534+
{ ...base, content: '', contentDeferred: true },
535+
{ id: 'doc-1', contentHash: 'connector:retry:h1', storageKey: null }
536+
)
515537
).toEqual({ type: 'update', existingId: 'doc-1' })
516538
})
517539

apps/sim/lib/knowledge/connectors/sync-engine.ts

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,13 @@ type DocClassification =
454454
| { type: 'unchanged' }
455455
| { type: 'drop' }
456456

457+
function shouldReplaceWithSkippedState(
458+
existing: { storageKey?: string | null },
459+
skipped: Pick<ExternalDocument, 'skippedExistingDisposition'>
460+
): boolean {
461+
return existing.storageKey === null || skipped.skippedExistingDisposition === 'replace'
462+
}
463+
457464
/**
458465
* Decides what a listed external document becomes during reconciliation.
459466
*
@@ -462,9 +469,11 @@ type DocClassification =
462469
* content stays last-known-good unless the connector marks the skip authoritative.
463470
* - `drop`: empty, non-deferred content that cannot be indexed.
464471
* - `add` / `update` / `unchanged`: normal content reconciliation by content hash.
465-
* - A deferred listing always rehydrates an existing content-less placeholder,
466-
* even when its listing hash is unchanged, so a prior hydration-time skip can
467-
* recover when the source becomes indexable.
472+
* - A deferred listing rehydrates an existing content-less placeholder when its
473+
* listing hash changes. A connector whose skip must be retried independently
474+
* persists `skippedRetryContentHash`, which deliberately differs from the next
475+
* listing hash. Stable permanent skips therefore do not redownload or surface
476+
* as source failures on every sync.
468477
*
469478
* `forceRehydrate` (set on a full resync of a `rehydrateOnFullSync` connector) promotes
470479
* an otherwise-`unchanged` deferred document to `update` so its content is re-fetched —
@@ -487,7 +496,10 @@ export function classifyExternalDoc(
487496
): DocClassification {
488497
if (extDoc.skippedReason) {
489498
if (!existing) return { type: 'skip' }
490-
return existing.storageKey === null || extDoc.skippedExistingDisposition === 'replace'
499+
if (existing.storageKey === null && existing.contentHash === extDoc.contentHash) {
500+
return { type: 'unchanged' }
501+
}
502+
return shouldReplaceWithSkippedState(existing, extDoc)
491503
? { type: 'skip', existingId: existing.id }
492504
: { type: 'unchanged' }
493505
}
@@ -497,9 +509,6 @@ export function classifyExternalDoc(
497509
if (!existing) {
498510
return { type: 'add' }
499511
}
500-
if (existing.storageKey === null && extDoc.contentDeferred) {
501-
return { type: 'update', existingId: existing.id }
502-
}
503512
if (existing.contentHash !== extDoc.contentHash) {
504513
return { type: 'update', existingId: existing.id }
505514
}
@@ -2312,7 +2321,7 @@ export async function executeSync(
23122321
// verified-unchanged match — same as the deferred-hydration
23132322
// equivalent above. A genuine hash match never sets skippedReason,
23142323
// so this only fires for the short-circuited case.
2315-
if (extDoc.skippedReason && existing) {
2324+
if (extDoc.skippedReason && existing && existing.storageKey !== null) {
23162325
recordUnverifiedExistingRefresh(result, failedExternalIds, extDoc.externalId)
23172326
} else {
23182327
result.docsUnchanged++
@@ -2377,7 +2386,8 @@ export async function executeSync(
23772386
extDoc: mergeHydratedSkippedDocument(op.extDoc, fullDoc),
23782387
})
23792388
} else if (op.type === 'update') {
2380-
if (fullDoc.skippedExistingDisposition === 'replace') {
2389+
const existing = priorByExternalId.get(op.extDoc.externalId)
2390+
if (existing && shouldReplaceWithSkippedState(existing, fullDoc)) {
23812391
skipOps.push({
23822392
type: 'skip',
23832393
existingId: op.existingId,

0 commit comments

Comments
 (0)