fix(knowledge): report what a bulk document sweep did, and refuse tag slots it cannot show

A bulk document update dropped ids that matched nothing and said so only in
a server log, then answered a selection where nothing matched with a 404 —
refusing a request that had nothing to refuse. It was never atomic, so
there was no guarantee to preserve. It now reports unmatched ids the way
the chunk sweep beside it always has, and a selection that matches nothing
is the same answer as one that matches some of what was asked for.

Its count is renamed to match that sibling, which also corrects it: the
update returns every row it matched, so the old name promised a count of
what changed.

Uploading a document could write a value into a tag slot the knowledge base
has not defined. Nothing could then read it back — every filter refuses an
undefined tag, and the app renders a slot only when a definition covers it.
Refused now rather than defined automatically: a slot write carries no
display name, so inventing one would put a name the caller never chose into
the vocabulary every later reader sees. Clearing such a slot still works,
so anything already written can be removed.

Reading an archived knowledge base answers not-found, which is deliberate —
the same answer conceals a workspace the caller cannot reach, so the
message cannot say more without telling the two apart. The parameter now
says which lifecycle it addresses, which reaches both the reference and the
terminal.

Author: waleedlatif1

apps/sim/app/api/v2/knowledge/[knowledgeBaseId]/documents/collection.test.ts

@@ -209,6 +209,7 @@ describe('PATCH /api/v2/knowledge/[knowledgeBaseId]/documents', () => {
     mockBulkUpdate.mockResolvedValue({
       operation: 'disable',
       successCount: 2,
+      errors: [],
       updatedDocuments: [
         { id: 'doc-1', enabled: false },
         { id: 'doc-2', enabled: false },
@@ -227,6 +228,7 @@ describe('PATCH /api/v2/knowledge/[knowledgeBaseId]/documents', () => {
     mockBulkUpdate.mockResolvedValueOnce({
       operation: 'disable',
       successCount: 100_000,
+      errors: [],
       updatedDocuments: Array.from({ length: 100_000 }, (_, index) => ({
         id: `doc-${index}`,
         enabled: false,
@@ -241,7 +243,7 @@ describe('PATCH /api/v2/knowledge/[knowledgeBaseId]/documents', () => {
 
     expect(response.status).toBe(200)
     expect(await response.json()).toEqual({
-      data: { operation: 'disable', updatedCount: 100_000 },
+      data: { operation: 'disable', processed: 100_000, errors: [] },
     })
   })
 
@@ -257,7 +259,7 @@ describe('PATCH /api/v2/knowledge/[knowledgeBaseId]/documents', () => {
 
     expect(response.status).toBe(200)
     expect(await response.json()).toEqual({
-      data: { operation: 'disable', updatedCount: 2, documentIds: ['doc-1', 'doc-2'] },
+      data: { operation: 'disable', processed: 2, errors: [], documentIds: ['doc-1', 'doc-2'] },
     })
     expect(mockBulkUpdate).toHaveBeenCalledWith(
       expect.objectContaining({

apps/sim/app/api/v2/knowledge/[knowledgeBaseId]/documents/route.ts

@@ -156,12 +156,13 @@ export const PATCH = defineV2JsonRoute({
      * bounds. A `selectAll` request has no such bound: a knowledge base with
      * 100k documents would otherwise materialize and element-wise validate a
      * multi-megabyte identifier array nobody asked for. That caller reads
-     * `updatedCount` and re-lists if it needs the identifiers.
+     * `processed` and re-lists if it needs the identifiers.
      */
     return {
       data: {
         operation: result.operation,
-        updatedCount: result.successCount,
+        processed: result.successCount,
+        errors: result.errors,
         documentIds: result.selectAll
           ? undefined
           : result.updatedDocuments.map((document) => document.id),

apps/sim/lib/api/contracts/v2/__tests__/transport-neutral-descriptions.test.ts

@@ -49,10 +49,6 @@ const ALLOWED = new Map<string, string>([
     'ISO 8601 timestamp when the knowledge base was archived by `DELETE /knowledge/{knowledgeBaseId}`, or null while the knowledge base is active. Only `GET /knowledge?scope=archived` returns knowledge bases with a non-null value.',
     'not touched here: lives in v2/knowledge.ts',
   ],
-  [
-    'Which lifecycle set to list: `active` (default) for live knowledge bases, `archived` for knowledge bases a `DELETE` archived and `POST /knowledge/{knowledgeBaseId}/restore` can bring back. `folderPath` resolves against active folders only, so pairing it with `scope=archived` returns an empty page when the containing folder was archived too.',
-    'not touched here: lives in v2/knowledge.ts',
-  ],
   [
     'Structured tag filters, at most 10 of them. Every filter must hold, including two that name the same tag: repeating one tag narrows the result rather than widening it, matching `GET /api/v2/knowledge/{knowledgeBaseId}/documents`. To match either of two values for one tag, issue a search per value. Each filtered tag must resolve to the same slot and field type in every knowledge base selected; one missing from any of them, or defined inconsistently across them, is rejected rather than ignored, and those knowledge bases must be searched separately. List the available names with `GET /api/v2/knowledge/{knowledgeBaseId}/tags`.',
     'not touched here: lives in v2/knowledge.ts',

apps/sim/lib/api/contracts/v2/knowledge.ts

@@ -627,7 +627,7 @@ export const v2ListKnowledgeBasesQuerySchema = z
     scope: v2KnowledgeBaseScopeSchema
       .default('active')
       .describe(
-        'Which lifecycle set to list: `active` (default) for live knowledge bases, `archived` for knowledge bases a `DELETE` archived and `POST /knowledge/{knowledgeBaseId}/restore` can bring back. `folderPath` resolves against active folders only, so pairing it with `scope=archived` returns an empty page when the containing folder was archived too.'
+        'Which lifecycle set to list: `active` (default) for live knowledge bases, `archived` for knowledge bases a delete archived and the restore operation can bring back. The folder filter resolves against active folders only, so pairing it with `archived` returns an empty page when the containing folder was archived too.'
       ),
     folderPath: v2FolderPathInputSchema
       .optional()
@@ -755,10 +755,24 @@ export const v2CreateKnowledgeBaseContract = defineRouteContract({
   },
 })
 
+/**
+ * Reads the active lifecycle only, which is why the identifier redescribes
+ * itself here rather than reusing the shared params schema: an archived
+ * knowledge base answers `404` from this route the same way a nonexistent one
+ * does, and nothing else on the read surface says which of the two lifecycles
+ * it addresses. `GET /api/v2/knowledge?scope=archived` is what resolves an
+ * archived base, and `POST /knowledge/{knowledgeBaseId}/restore` brings it back.
+ */
+const v2GetKnowledgeBaseParamsSchema = v2KnowledgeBaseParamsSchema.extend({
+  knowledgeBaseId: v2KnowledgeBaseParamsSchema.shape.knowledgeBaseId.describe(
+    'Knowledge base to read. Active knowledge bases only: an archived one answers 404 here, is listed by `scope=archived`, and is brought back by the restore endpoint.'
+  ),
+})
+
 export const v2GetKnowledgeBaseContract = defineRouteContract({
   method: 'GET',
   path: '/api/v2/knowledge/[knowledgeBaseId]',
-  params: v2KnowledgeBaseParamsSchema,
+  params: v2GetKnowledgeBaseParamsSchema,
   query: v1KnowledgeWorkspaceQuerySchema
     .extend({
       workspaceId: v1KnowledgeWorkspaceQuerySchema.shape.workspaceId.describe(
@@ -1482,22 +1496,43 @@ export const v2BulkKnowledgeDocumentsBodySchema = z
   })
 export type V2BulkKnowledgeDocumentsBody = z.input<typeof v2BulkKnowledgeDocumentsBodySchema>
 
-/** Bulk update outcome — one object, not a page. */
+/**
+ * Bulk update outcome — one object, not a page.
+ *
+ * Best-effort, and reported the same way {@link v2BulkKnowledgeChunksDataSchema}
+ * reports it: an identifier naming no updatable document in the knowledge base
+ * lands in `errors` rather than failing the request, and a request whose every
+ * identifier missed answers `200` with `processed: 0` instead of `404`. The
+ * selection has always applied to what it matched, so the shared shape describes
+ * what the operation already did rather than changing it.
+ *
+ * `processed`, not `updatedCount`: the count is the documents the selection
+ * matched, and disabling a document that was already disabled counts it, so it
+ * is not a count of changes. That is the same quantity `processed` names on the
+ * chunk sibling, and naming it the same way keeps the two readable side by side.
+ */
 export const v2BulkKnowledgeDocumentsDataSchema = z
   .object({
     operation: z.enum(['enable', 'disable']).describe('Operation that was applied.'),
-    updatedCount: z
+    processed: z
       .number()
       .int()
       .nonnegative()
-      .describe('Number of documents the operation changed.')
+      .describe(
+        'Number of documents in this knowledge base the operation matched. Documents already in the requested state are counted too, so this is not a count of changes.'
+      )
       .meta({ examples: [42] }),
+    errors: z
+      .array(z.string())
+      .describe(
+        'Per-document failures, including any identifier that named no updatable document in the knowledge base. A populated array still answers 200, and a `selectAll` request reports an empty one.'
+      ),
     documentIds: z
       .array(z.string())
       .optional()
       .describe(
-        'Identifiers of the documents the operation changed. Present only for an explicit `documentIds` request, which is bounded to ' +
-          `${MAX_V2_BULK_KNOWLEDGE_DOCUMENTS} documents; a \`selectAll\` request omits it because the selection is unbounded, and reports \`updatedCount\` instead.`
+        'Identifiers of the documents the operation matched. Present only for an explicit `documentIds` request, which is bounded to ' +
+          `${MAX_V2_BULK_KNOWLEDGE_DOCUMENTS} documents; a \`selectAll\` request omits it because the selection is unbounded, and reports \`processed\` instead.`
       ),
   })
   .strict()

apps/sim/lib/api/contracts/v2/list-scope-folder-filter.test.ts

@@ -0,0 +1,31 @@
+/**
+ * @vitest-environment node
+ */
+import { describe, expect, it } from 'vitest'
+import { v2ListKnowledgeBasesQuerySchema } from '@/lib/api/contracts/v2/knowledge'
+import { v2ListTablesQuerySchema } from '@/lib/api/contracts/v2/tables'
+import { v2ListWorkflowsQuerySchema } from '@/lib/api/contracts/v2/workflows'
+
+/**
+ * A v2 description is read on two surfaces at once: the API reference, where a
+ * filter is `folderPath`, and `sim tables list --help`, where the same filter is
+ * `--folder`. Naming either spelling is wrong on the other surface, so the
+ * `scope` prose names the concept — "the folder filter" — the way the workflows
+ * sibling already does. Asserted on both so the pair cannot drift apart again.
+ */
+const SCOPE_DESCRIPTIONS = [
+  ['tables', v2ListTablesQuerySchema.shape.scope.description],
+  ['workflows', v2ListWorkflowsQuerySchema.shape.scope.description],
+  ['knowledge', v2ListKnowledgeBasesQuerySchema.shape.scope.description],
+] as const
+
+describe('v2 list scope descriptions', () => {
+  it.each(SCOPE_DESCRIPTIONS)(
+    'name the folder filter transport-neutrally on %s',
+    (_, described) => {
+      expect(described).toBeTypeOf('string')
+      expect(described).toContain('The folder filter resolves against active folders only')
+      expect(described).not.toContain('folderPath')
+    }
+  )
+})

apps/sim/lib/knowledge/application/documents.test.ts

@@ -30,6 +30,7 @@ const mocks = vi.hoisted(() => ({
   recordKnowledgeBaseFileOwnership: vi.fn(),
   recordAudit: vi.fn(),
   captureServerEvent: vi.fn(),
+  assertTagSlotsAreDefined: vi.fn(),
   getDocumentTagDefinitions: vi.fn(),
 }))
 
@@ -78,6 +79,7 @@ vi.mock('@/lib/knowledge/documents/service', () => ({
 }))
 
 vi.mock('@/lib/knowledge/tags/service', () => ({
+  assertTagSlotsAreDefined: mocks.assertTagSlotsAreDefined,
   getDocumentTagDefinitions: mocks.getDocumentTagDefinitions,
 }))
 

apps/sim/lib/knowledge/application/documents.ts

@@ -65,7 +65,7 @@ import {
   resolveKnowledgeTagFilters,
   toKnowledgeTagFilterConditions,
 } from '@/lib/knowledge/tags/filter-resolution'
-import { getDocumentTagDefinitions } from '@/lib/knowledge/tags/service'
+import { assertTagSlotsAreDefined, getDocumentTagDefinitions } from '@/lib/knowledge/tags/service'
 import { validateTagValue } from '@/lib/knowledge/tags/utils'
 import { StorageService } from '@/lib/uploads'
 import { generateKnowledgeBaseFileKey } from '@/lib/uploads/contexts/knowledge-base/knowledge-base-file-manager'
@@ -901,6 +901,13 @@ export const updateKnowledgeDocument = defineAuthorizedKnowledgeUseCase({
         updates,
         await resolveKnowledgeDocumentTagValueUpdates(context.knowledgeBaseId, input.tagValues)
       )
+    } else {
+      /**
+       * `tagValues` addresses definitions by id and cannot name an undefined
+       * slot; `input.updates` carries the raw `tag1`..`tag7` the API surfaces
+       * accept, and can.
+       */
+      await assertTagSlotsAreDefined(context.knowledgeBaseId, updates)
     }
     const updatedFields = Object.keys(updates).filter(
       (key) => updates[key as keyof typeof updates] !== undefined
@@ -962,6 +969,12 @@ export const bulkUpdateKnowledgeDocuments = defineAuthorizedKnowledgeUseCase({
     return {
       operation: input.operation,
       successCount: result.successCount,
+      /**
+       * Only an explicit selection can name an id that matched nothing, so a
+       * `selectAll` sweep reports an empty array rather than omitting the field:
+       * one response shape for both selections.
+       */
+      errors: result.errors,
       updatedDocuments: result.updatedDocuments,
       /**
        * Reported so a surface can tell a bounded selection from an unbounded

apps/sim/lib/knowledge/documents/bulk-document-operation.test.ts

@@ -0,0 +1,84 @@
+/**
+ * @vitest-environment node
+ */
+import { document } from '@sim/db/schema'
+import { dbChainMockFns, queueTableRows, resetDbChainMock } from '@sim/testing'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+vi.mock('@/lib/knowledge/documents/document-processor', () => ({
+  processDocument: vi.fn(),
+}))
+
+vi.mock('@/lib/knowledge/embedding-models', () => ({
+  EMBEDDING_DIMENSIONS: 1536,
+  getEmbeddingModelInfo: vi.fn(() => ({ tokenizerProvider: 'openai' })),
+}))
+
+vi.mock('@/lib/knowledge/embeddings', () => ({
+  generateEmbeddings: vi.fn(),
+}))
+
+import { bulkDocumentOperation } from '@/lib/knowledge/documents/service'
+
+const KNOWLEDGE_BASE_ID = 'knowledge-base-1'
+
+describe('bulkDocumentOperation unmatched-id reporting', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    resetDbChainMock()
+  })
+
+  it('reports an id that matched no updatable document without failing the request', async () => {
+    queueTableRows(document, [{ id: 'document-1', enabled: true }])
+    dbChainMockFns.returning.mockResolvedValueOnce([{ id: 'document-1', enabled: false }])
+
+    const result = await bulkDocumentOperation(
+      KNOWLEDGE_BASE_ID,
+      'disable',
+      ['document-1', 'document-missing'],
+      'request-1'
+    )
+
+    expect(result.successCount).toBe(1)
+    expect(result.errors).toEqual(['No matching documents found to disable: document-missing'])
+    expect(result.success).toBe(false)
+  })
+
+  it('answers a zero-match selection the same way, rather than throwing not-found', async () => {
+    queueTableRows(document, [])
+
+    const result = await bulkDocumentOperation(
+      KNOWLEDGE_BASE_ID,
+      'enable',
+      ['missing-1', 'missing-2'],
+      'request-2'
+    )
+
+    expect(result.successCount).toBe(0)
+    expect(result.updatedDocuments).toEqual([])
+    expect(result.errors).toEqual(['No matching documents found to enable: missing-1, missing-2'])
+    expect(result.success).toBe(false)
+  })
+
+  it('reports no errors when every requested document matched', async () => {
+    queueTableRows(document, [
+      { id: 'document-1', enabled: false },
+      { id: 'document-2', enabled: false },
+    ])
+    dbChainMockFns.returning.mockResolvedValueOnce([
+      { id: 'document-1', enabled: true },
+      { id: 'document-2', enabled: true },
+    ])
+
+    const result = await bulkDocumentOperation(
+      KNOWLEDGE_BASE_ID,
+      'enable',
+      ['document-1', 'document-2'],
+      'request-3'
+    )
+
+    expect(result.successCount).toBe(2)
+    expect(result.errors).toEqual([])
+    expect(result.success).toBe(true)
+  })
+})