Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/app/api/sources/[sourceId]/chunks/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,7 @@ describe("GET /api/sources/[sourceId]/chunks", () => {
revisionKey: "job_1",
page: 1,
pageSize: 1,
assetUrlPolicy: "durable",
})
})

Expand Down Expand Up @@ -659,6 +660,7 @@ describe("GET /api/sources/[sourceId]/chunks", () => {
revisionKey: "job_result_1",
page: 1,
pageSize: 1,
assetUrlPolicy: "durable",
})
})
})
Expand Down
4 changes: 2 additions & 2 deletions src/domains/chunks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,8 @@ export function toParsedChunkView(

/**
* Map an SDK `KnowledgeReadChunk` (from `knowledge.readChunks`) to the view
* model. Display reads do not request durable asset URL hardening; chat
* hardens the specific assets it returns separately.
* model. Display reads request durable asset URLs from the SDK; chat still
* hardens only the specific assets it returns separately.
*/
export function toParsedChunkViewFromReadChunk(
chunk: KnowledgeReadChunk,
Expand Down
11 changes: 10 additions & 1 deletion src/domains/chunks/read.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ function makeReadChunk(overrides: Record<string, unknown> = {}) {
}

describe("readSourceChunkPage", () => {
it("reads a page without durable asset hardening and maps chunks to the view model", async () => {
it("reads a page with durable asset URLs and maps chunks to the view model", async () => {
const readChunks = vi.fn(async () => ({
document: { localDocumentId: "doc_1" },
chunks: [
Expand Down Expand Up @@ -48,6 +48,7 @@ describe("readSourceChunkPage", () => {
revisionKey: "rev_1",
page: 2,
pageSize: 50,
assetUrlPolicy: "durable",
})
expect(result.pagination).toEqual({
page: 2,
Expand Down Expand Up @@ -84,6 +85,7 @@ describe("readSourceChunkPage", () => {
documentId: "doc_1",
page: 1,
pageSize: 50,
assetUrlPolicy: "durable",
})
})
})
Expand Down Expand Up @@ -116,6 +118,13 @@ describe("readAllSourceChunks", () => {
})

expect(readChunks).toHaveBeenCalledTimes(2)
expect(readChunks).toHaveBeenNthCalledWith(1, {
documentId: "doc_1",
revisionKey: "rev_1",
page: 1,
pageSize: 200,
assetUrlPolicy: "durable",
})
expect(chunks.map((chunk) => chunk.parserChunkId)).toEqual(["c1", "c2"])
})
})
6 changes: 4 additions & 2 deletions src/domains/chunks/read.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ type ReadableSource = {
/**
* Read a single display page of parsed chunks through the SDK. The SDK serves
* from configured Blob storage when fresh and falls back to Knowhere remote
* otherwise. Display reads intentionally do not request durable asset URLs:
* chat hardens only the specific assets it sends back to the user.
* otherwise. Display reads request durable asset URLs so media chunks remain
* usable whether the page came from Blob or the remote Knowhere fallback.
*/
export async function readSourceChunkPage(input: {
readonly knowledge: Knowledge
Expand All @@ -29,6 +29,7 @@ export async function readSourceChunkPage(input: {
...(input.source.revisionKey ? { revisionKey: input.source.revisionKey } : {}),
page: input.params.page,
pageSize: input.params.pageSize,
assetUrlPolicy: "durable",
})

const chunks = response.chunks.map((chunk) =>
Expand Down Expand Up @@ -68,6 +69,7 @@ export async function readAllSourceChunks(input: {
: {}),
page,
pageSize: loadAllPageSize,
assetUrlPolicy: "durable",
})
for (const chunk of response.chunks) {
chunks.push(
Expand Down
86 changes: 34 additions & 52 deletions src/domains/sources/parsed-sync-route-workflow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@ const mocks = vi.hoisted(() => ({
makeKnowhereClientWithParsedStorage: vi.fn(),
releaseSyncCapacity: vi.fn(),
updateSyncStatus: vi.fn(),
findInWorkspace: vi.fn(),
markFailed: vi.fn(),
markReady: vi.fn(),
markSourceReadyAfterReconciliation: vi.fn(),
loggerInfo: vi.fn(),
loggerError: vi.fn(),
}))
Expand All @@ -20,16 +16,9 @@ vi.mock("@/integrations/knowhere", () => ({
vi.mock("./workflow-runtime", () => ({
sourceWorkflowRuntime: {
updateSyncStatus: mocks.updateSyncStatus,
findInWorkspace: mocks.findInWorkspace,
markFailed: mocks.markFailed,
markReady: mocks.markReady,
},
}))

vi.mock("./source-reconcile-workflow", () => ({
markSourceReadyAfterReconciliation: mocks.markSourceReadyAfterReconciliation,
}))

vi.mock("./parsed-document-sync-capacity", () => ({
parsedDocumentSyncCapacityGuard: {
acquire: mocks.acquireSyncCapacity,
Expand Down Expand Up @@ -73,16 +62,13 @@ describe("parsedSyncRouteWorkflow.runParsedSyncWorkflow", () => {
},
})
mocks.releaseSyncCapacity.mockResolvedValue(undefined)
mocks.markSourceReadyAfterReconciliation.mockResolvedValue({
status: "ready",
})
})

afterEach(() => {
vi.clearAllMocks()
})

it("marks the source ready when sync completes in one segment", async () => {
it("records completion when sync completes in one segment", async () => {
const syncParsedDocument = vi.fn(async () => ({
documentId: "doc_1",
revisionKey: "rev_1",
Expand All @@ -105,18 +91,13 @@ describe("parsedSyncRouteWorkflow.runParsedSyncWorkflow", () => {
"source_1",
{ revisionKey: "rev_1", syncStatus: "completed" },
)
expect(mocks.markSourceReadyAfterReconciliation).toHaveBeenCalledWith({
workspaceId: "workspace_1",
sourceId: "source_1",
documentId: "doc_1",
})
expect(mocks.releaseSyncCapacity).toHaveBeenCalledWith({
leaseToken: "lease_1",
releaseReason: "completed",
})
})

it("marks ready and triggers a continuation when sync is incomplete", async () => {
it("triggers a continuation when sync is incomplete", async () => {
const syncParsedDocument = vi.fn(async () => ({
documentId: "doc_1",
revisionKey: "rev_1",
Expand Down Expand Up @@ -145,11 +126,6 @@ describe("parsedSyncRouteWorkflow.runParsedSyncWorkflow", () => {
restore()
}

expect(mocks.markSourceReadyAfterReconciliation).toHaveBeenCalledWith({
workspaceId: "workspace_1",
sourceId: "source_1",
documentId: "doc_1",
})
expect(triggered).toHaveLength(1)
expect(triggered[0]?.segmentIndex).toBe(1)
expect(triggered[0]?.workflowRunId).toBe("doc_1-sync-rev_1-1")
Expand Down Expand Up @@ -249,16 +225,45 @@ describe("parsedSyncRouteWorkflow.runParsedSyncWorkflow", () => {
])
expect(mocks.releaseSyncCapacity).not.toHaveBeenCalled()
})

it("does not release capacity when Upstash aborts during a planned step", async () => {
const workflowAbort = new Error("planned workflow step")
workflowAbort.name = "WorkflowAbort"
const syncParsedDocument = vi.fn(async () => ({
documentId: "doc_1",
revisionKey: "rev_1",
completed: true,
}))
mocks.makeKnowhereClientWithParsedStorage.mockReturnValue({
client: {},
knowledge: { syncParsedDocument },
})
const run: RunStep = async (id, task) => {
if (id === "sync-0-0") throw workflowAbort
return task()
}

await expect(
parsedSyncRouteWorkflow.runParsedSyncWorkflow({
context: {
run,
url: "https://notebook.example/api/sources/parsed-sync",
},
payload: basePayload,
}),
).rejects.toThrow("planned workflow step")

expect(mocks.releaseSyncCapacity).not.toHaveBeenCalled()
expect(mocks.updateSyncStatus).not.toHaveBeenCalled()
})
})

describe("parsedSyncRouteWorkflow.markSyncFailedAfterWorkflowFailure", () => {
afterEach(() => {
vi.clearAllMocks()
})

it("fails a parsing source with failure_stage storage_sync", async () => {
mocks.findInWorkspace.mockResolvedValue({ id: "source_1", status: "parsing" })

it("records failed sync metadata without failing the source", async () => {
await parsedSyncRouteWorkflow.markSyncFailedAfterWorkflowFailure(
{ ...basePayload, revisionKey: "rev_1" },
"boom",
Expand All @@ -269,28 +274,5 @@ describe("parsedSyncRouteWorkflow.markSyncFailedAfterWorkflowFailure", () => {
"source_1",
{ revisionKey: "rev_1", syncStatus: "failed", syncError: "boom" },
)
expect(mocks.markFailed).toHaveBeenCalledWith(
"workspace_1",
"source_1",
expect.stringContaining("storage sync failed"),
"parsing",
"storage_sync",
)
})

it("does not fail an already-ready source, only records sync_status", async () => {
mocks.findInWorkspace.mockResolvedValue({ id: "source_1", status: "ready" })

await parsedSyncRouteWorkflow.markSyncFailedAfterWorkflowFailure(
basePayload,
"boom",
)

expect(mocks.updateSyncStatus).toHaveBeenCalledWith(
"workspace_1",
"source_1",
{ revisionKey: undefined, syncStatus: "failed", syncError: "boom" },
)
expect(mocks.markFailed).not.toHaveBeenCalled()
})
})
67 changes: 26 additions & 41 deletions src/domains/sources/parsed-sync-route-workflow.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import "server-only"

import { Client, type WorkflowContext } from "@upstash/workflow"
import { Client, WorkflowAbort, type WorkflowContext } from "@upstash/workflow"
import type { KnowledgeSyncParsedDocumentResponse } from "@ontos-ai/knowhere-sdk"

import { makeKnowhereClientWithParsedStorage } from "@/integrations/knowhere"
Expand All @@ -10,7 +10,6 @@ import {
type ParsedSyncPayload,
} from "./parsed-document-sync-scheduler"
import { parsedDocumentSyncCapacityGuard } from "./parsed-document-sync-capacity"
import { markSourceReadyAfterReconciliation } from "./source-reconcile-workflow"
import { sourceWorkflowRuntime } from "./workflow-runtime"

type ParsedSyncWorkflowContext = Pick<
Expand Down Expand Up @@ -72,20 +71,10 @@ async function runParsedSyncWorkflow(input: {
workspaceId,
})

const preSyncReady = await context.run(
`source-ready-before-sync-${payload.segmentIndex}`,
async () =>
markSourceReadyAfterReconciliation({
workspaceId,
sourceId,
documentId,
}),
)
if (preSyncReady.status === "gone") return

let revisionKey = payload.revisionKey
let completed = false
let releaseReason: SyncLeaseReleaseReason = "incomplete"
let shouldReleaseLease = true

const capacity = await context.run(
`acquire-sync-capacity-${payload.segmentIndex}`,
Expand Down Expand Up @@ -196,24 +185,29 @@ async function runParsedSyncWorkflow(input: {
return
}
} catch (error) {
if (isWorkflowControlAbort(error)) {
shouldReleaseLease = false
throw error
}
releaseReason = "failed"
throw error
} finally {
await context.run(`release-sync-capacity-${payload.segmentIndex}`, async () =>
releaseCapacityLease({
leaseToken: capacity.leaseToken,
releaseReason,
sourceId,
documentId,
}),
)
if (shouldReleaseLease) {
await context.run(`release-sync-capacity-${payload.segmentIndex}`, async () =>
releaseCapacityLease({
leaseToken: capacity.leaseToken,
releaseReason,
sourceId,
documentId,
}),
)
}
}

logger.info("parsed-sync: parsed document sync finished", {
sourceId,
documentId,
revisionKey,
status: preSyncReady.status,
})
}

Expand Down Expand Up @@ -261,10 +255,9 @@ async function markSyncFailedAfterWorkflowFailure(
const normalized = normalizeParsedSyncPayload(payload)
const reason = getSafeFailureReason(failResponse)

// Record the storage-sync failure. A source still `parsing` is failed with
// failure_stage=storage_sync so a retry resumes sync without reparsing; an
// already-ready source is left ready (it still serves via remote fallback),
// only its sync_status is marked failed for observability.
// Parsed storage is a cache/read model. Exhausted sync failure is recorded for
// observability, but the source remains ready and SDK reads can fall back to
// Knowhere remote.
await sourceWorkflowRuntime.updateSyncStatus(
normalized.workspaceId,
normalized.sourceId,
Expand All @@ -275,26 +268,11 @@ async function markSyncFailedAfterWorkflowFailure(
},
)

const source = await sourceWorkflowRuntime.findInWorkspace(
normalized.workspaceId,
normalized.sourceId,
)
if (source?.status === "parsing") {
await sourceWorkflowRuntime.markFailed(
normalized.workspaceId,
normalized.sourceId,
`Parsed document storage sync failed: ${reason}`,
"parsing",
"storage_sync",
)
}

logger.error("parsed-sync: marked sync failed after workflow failure", {
workspaceId: normalized.workspaceId,
sourceId: normalized.sourceId,
documentId: normalized.documentId,
segmentIndex: normalized.segmentIndex,
sourceStatus: source?.status,
})
}

Expand All @@ -314,6 +292,13 @@ function setContinuationTriggerForTesting(
}
}

function isWorkflowControlAbort(error: unknown): boolean {
return (
(error instanceof WorkflowAbort && error.constructor === WorkflowAbort) ||
(error instanceof Error && error.name === "WorkflowAbort")
)
}

export const parsedSyncRouteWorkflow = {
markSyncFailedAfterWorkflowFailure,
normalizeParsedSyncPayload,
Expand Down
Loading
Loading