From e6384e65b91e42a8a6ac99499c663907885682e2 Mon Sep 17 00:00:00 2001 From: elpresidank Date: Tue, 2 Jun 2026 01:39:04 -0500 Subject: [PATCH] Replace librarian throw helpers with Effect failures --- ts/EFFECT_NATIVE_REWRITE_AUDIT.md | 34 ++- .../src/__tests__/librarian-service.test.ts | 2 +- ts/packages/flow/src/librarian/service.ts | 211 ++++++++++-------- 3 files changed, 146 insertions(+), 101 deletions(-) diff --git a/ts/EFFECT_NATIVE_REWRITE_AUDIT.md b/ts/EFFECT_NATIVE_REWRITE_AUDIT.md index aa6e7fc5..305d94a6 100644 --- a/ts/EFFECT_NATIVE_REWRITE_AUDIT.md +++ b/ts/EFFECT_NATIVE_REWRITE_AUDIT.md @@ -12,12 +12,12 @@ Verified source roots: - Effect v4 subtree: `/home/elpresidank/YeeBois/projects/beep-effect2/.repos/effect-v4` - Installed Effect beta used by this workspace: `ts/node_modules/effect` -Current signal counts from `ts/packages` after the 2026-06-02 Librarian -schema/assertion cleanup slice: +Current signal counts from `ts/packages` after the 2026-06-02 Librarian tagged +operation helper slice: | Signal | Count | | --- | ---: | -| `Effect.runPromise` | 204 | +| `Effect.runPromise` | 209 | | `Map<` | 74 | | `WebSocket` | 47 | | `new Map` | 56 | @@ -270,8 +270,7 @@ Notes: loading, and schema-backed metadata triple normalization. - Remaining: - Librarian still has the dynamic `AsyncProcessorRuntime & Record` service object and sync throw helper paths. Keep it as the next P0 - state/ref-backed migration. + any>` service object. Keep it as the next P0 state/ref-backed migration. - Verification: - `bun run --cwd ts/packages/base build` - `bun run --cwd ts/packages/flow build` @@ -282,6 +281,31 @@ Notes: - `cd ts && bun run test` - `git diff --check` +### 2026-06-02: Librarian Tagged Operation Helper Slice + +- Status: migrated and root-verified. +- Completed: + - Removed the librarian `throwLibrarianServiceError` helper. + - `get-document-metadata`, `list-children`, `upload-chunk`, + `get-upload-status`, and `abort-upload` now dispatch through local + Effect-returning helpers that fail with `LibrarianServiceError`. + - Compatibility methods for those operations now return Promise facades + backed by `Effect.runPromise`. + - The librarian tests now await the Promise compatibility facade for upload + status. +- Remaining: + - Librarian still has the dynamic service object, mutable maps/handles on + that object, and a raw `while (service.running)` poll loop. That remains + the next P0 state/ref-backed migration. +- Verification: + - `bun run --cwd ts/packages/flow test -- src/__tests__/librarian-service.test.ts` + - `bun run --cwd ts/packages/flow build` + - `cd ts && bun run check` + - `bun run --cwd ts/packages/flow test` + - `cd ts && bun run build` + - `cd ts && bun run test` + - `git diff --check` + ## Subagent Findings To Preserve - MCP/workbench: diff --git a/ts/packages/flow/src/__tests__/librarian-service.test.ts b/ts/packages/flow/src/__tests__/librarian-service.test.ts index 6827757d..f2c64c8b 100644 --- a/ts/packages/flow/src/__tests__/librarian-service.test.ts +++ b/ts/packages/flow/src/__tests__/librarian-service.test.ts @@ -73,7 +73,7 @@ describe("LibrarianService schema-backed boundaries", () => { "chunk-size": 4, }); const uploadId = response["upload-id"]; - const status = service.getUploadStatus({ + const status = await service.getUploadStatus({ operation: "get-upload-status", "upload-id": uploadId, }); diff --git a/ts/packages/flow/src/librarian/service.ts b/ts/packages/flow/src/librarian/service.ts index dfaf6694..264c9129 100644 --- a/ts/packages/flow/src/librarian/service.ts +++ b/ts/packages/flow/src/librarian/service.ts @@ -79,10 +79,6 @@ const librarianServiceError = (operation: string, cause: unknown): LibrarianServ message: errorMessage(cause), }); -function throwLibrarianServiceError(operation: string, cause: unknown): never { - throw librarianServiceError(operation, cause); -} - function resolveDataDir(config: LibrarianServiceConfig): string { return config.dataDir ?? Effect.runSync( Config.string("LIBRARIAN_DATA_DIR").pipe(Config.withDefault("./data/librarian")), @@ -162,6 +158,107 @@ export function makeLibrarianService(config: LibrarianServiceConfig): LibrarianS service.colProducer = null; service.dataDir = resolveDataDir(config); service.persistPath = joinPath(service.dataDir, "librarian-state.json"); + + const getDocumentMetadataEffect = (request: LibrarianRequest): Effect.Effect => + Effect.gen(function* () { + const id = service.documentId(request); + if (id === undefined || id.length === 0) { + return yield* librarianServiceError("get-document-metadata", "get-document-metadata requires documentId"); + } + + const doc = service.documents.get(id); + if (doc === undefined) { + return yield* librarianServiceError("get-document-metadata", `Document not found: ${id}`); + } + + return service.documentResponse(doc); + }); + + const listChildrenEffect = (request: LibrarianRequest): Effect.Effect => + Effect.gen(function* () { + const parentId = service.documentId(request); + if (parentId === undefined || parentId.length === 0) { + return yield* librarianServiceError("list-children", "list-children requires documentId"); + } + + const children: DocumentMetadata[] = []; + for (const doc of service.documents.values()) { + if (doc.parentId === parentId) { + children.push(doc); + } + } + + return service.documentsResponse(children); + }); + + const uploadChunkEffect = (request: LibrarianRequest): Effect.Effect => + Effect.gen(function* () { + const req = service.requestRecord(request); + const uploadId = optionalString(req["upload-id"]); + if (uploadId === undefined) { + return yield* librarianServiceError("upload-chunk", "upload-chunk requires upload-id"); + } + const session = service.uploads.get(uploadId); + if (session === undefined) { + return yield* librarianServiceError("upload-chunk", `Upload not found: ${uploadId}`); + } + const chunkIndex = typeof req["chunk-index"] === "number" ? req["chunk-index"] : -1; + if (!Number.isInteger(chunkIndex) || chunkIndex < 0 || chunkIndex >= session.totalChunks) { + return yield* librarianServiceError("upload-chunk", "upload-chunk requires a valid chunk-index"); + } + const content = optionalString(req.content); + if (content === undefined) { + return yield* librarianServiceError("upload-chunk", "upload-chunk requires content"); + } + session.chunks.set(chunkIndex, content); + + const bytesReceived = [...session.chunks.values()].reduce((sum, chunk) => sum + chunk.length, 0); + return { + "upload-id": uploadId, + "chunk-index": chunkIndex, + "chunks-received": session.chunks.size, + "total-chunks": session.totalChunks, + "bytes-received": bytesReceived, + "total-bytes": session.totalSize, + }; + }); + + const getUploadStatusEffect = (request: LibrarianRequest): Effect.Effect => + Effect.gen(function* () { + const uploadId = optionalString(service.requestRecord(request)["upload-id"]); + if (uploadId === undefined) { + return yield* librarianServiceError("get-upload-status", "get-upload-status requires upload-id"); + } + const session = service.uploads.get(uploadId); + if (session === undefined) { + return yield* librarianServiceError("get-upload-status", `Upload not found: ${uploadId}`); + } + const receivedChunks = [...session.chunks.keys()].sort((a, b) => a - b); + const receivedSet = new Set(receivedChunks); + const missingChunks = Array.from({ length: session.totalChunks }, (_, i) => i).filter((i) => !receivedSet.has(i)); + const bytesReceived = [...session.chunks.values()].reduce((sum, chunk) => sum + chunk.length, 0); + return { + "upload-id": uploadId, + "upload-state": "in-progress", + "chunks-received": session.chunks.size, + "total-chunks": session.totalChunks, + "received-chunks": receivedChunks, + "missing-chunks": missingChunks, + "bytes-received": bytesReceived, + "total-bytes": session.totalSize, + }; + }); + + const abortUploadEffect = (request: LibrarianRequest): Effect.Effect => + Effect.gen(function* () { + const uploadId = optionalString(service.requestRecord(request)["upload-id"]); + if (uploadId === undefined) { + return yield* librarianServiceError("abort-upload", "abort-upload requires upload-id"); + } + service.uploads.delete(uploadId); + return {}; + }); + Object.assign(service, { @@ -508,10 +605,7 @@ export function makeLibrarianService(config: LibrarianServiceConfig): LibrarianS catch: (cause) => librarianServiceError("list-documents", cause), }); case "get-document-metadata": - return yield* Effect.try({ - try: () => service.getDocumentMetadata(request), - catch: (cause) => librarianServiceError("get-document-metadata", cause), - }); + return yield* getDocumentMetadataEffect(request); case "get-document-content": return yield* Effect.tryPromise({ try: () => service.getDocumentContent(request), @@ -523,10 +617,7 @@ export function makeLibrarianService(config: LibrarianServiceConfig): LibrarianS catch: (cause) => librarianServiceError("add-child-document", cause), }); case "list-children": - return yield* Effect.try({ - try: () => service.listChildren(request), - catch: (cause) => librarianServiceError("list-children", cause), - }); + return yield* listChildrenEffect(request); case "add-processing": return yield* Effect.tryPromise({ try: () => service.addProcessing(request), @@ -548,25 +639,16 @@ export function makeLibrarianService(config: LibrarianServiceConfig): LibrarianS catch: (cause) => librarianServiceError("begin-upload", cause), }); case "upload-chunk": - return yield* Effect.try({ - try: () => service.uploadChunk(request), - catch: (cause) => librarianServiceError("upload-chunk", cause), - }); + return yield* uploadChunkEffect(request); case "complete-upload": return yield* Effect.tryPromise({ try: () => service.completeUpload(request), catch: (cause) => librarianServiceError("complete-upload", cause), }); case "get-upload-status": - return yield* Effect.try({ - try: () => service.getUploadStatus(request), - catch: (cause) => librarianServiceError("get-upload-status", cause), - }); + return yield* getUploadStatusEffect(request); case "abort-upload": - return yield* Effect.try({ - try: () => service.abortUpload(request), - catch: (cause) => librarianServiceError("abort-upload", cause), - }); + return yield* abortUploadEffect(request); case "list-uploads": return yield* Effect.tryPromise({ try: () => service.listUploads(request), @@ -737,16 +819,8 @@ export function makeLibrarianService(config: LibrarianServiceConfig): LibrarianS - getDocumentMetadata: function(this: LibrarianService, request: LibrarianRequest): LibrarianResponse { - const id = this.documentId(request); - if (id === undefined || id.length === 0) { - throwLibrarianServiceError("get-document-metadata", "get-document-metadata requires documentId"); - } - - const doc = this.documents.get(id); - if (doc === undefined) throwLibrarianServiceError("get-document-metadata", `Document not found: ${id}`); - - return this.documentResponse(doc); + getDocumentMetadata: function(this: LibrarianService, request: LibrarianRequest): Promise { + return Effect.runPromise(getDocumentMetadataEffect(request)); }, @@ -833,20 +907,8 @@ export function makeLibrarianService(config: LibrarianServiceConfig): LibrarianS - listChildren: function(this: LibrarianService, request: LibrarianRequest): LibrarianResponse { - const parentId = this.documentId(request); - if (parentId === undefined || parentId.length === 0) { - throwLibrarianServiceError("list-children", "list-children requires documentId"); - } - - const children: DocumentMetadata[] = []; - for (const doc of this.documents.values()) { - if (doc.parentId === parentId) { - children.push(doc); - } - } - - return this.documentsResponse(children); + listChildren: function(this: LibrarianService, request: LibrarianRequest): Promise { + return Effect.runPromise(listChildrenEffect(request)); }, @@ -969,29 +1031,8 @@ export function makeLibrarianService(config: LibrarianServiceConfig): LibrarianS - uploadChunk: function(this: LibrarianService, request: LibrarianRequest): LibrarianResponse { - const req = this.requestRecord(request); - const uploadId = optionalString(req["upload-id"]); - if (uploadId === undefined) throwLibrarianServiceError("upload-chunk", "upload-chunk requires upload-id"); - const session = this.uploads.get(uploadId); - if (session === undefined) throwLibrarianServiceError("upload-chunk", `Upload not found: ${uploadId}`); - const chunkIndex = typeof req["chunk-index"] === "number" ? req["chunk-index"] : -1; - if (!Number.isInteger(chunkIndex) || chunkIndex < 0 || chunkIndex >= session.totalChunks) { - throwLibrarianServiceError("upload-chunk", "upload-chunk requires a valid chunk-index"); - } - const content = optionalString(req.content); - if (content === undefined) throwLibrarianServiceError("upload-chunk", "upload-chunk requires content"); - session.chunks.set(chunkIndex, content); - - const bytesReceived = [...session.chunks.values()].reduce((sum, chunk) => sum + chunk.length, 0); - return { - "upload-id": uploadId, - "chunk-index": chunkIndex, - "chunks-received": session.chunks.size, - "total-chunks": session.totalChunks, - "bytes-received": bytesReceived, - "total-bytes": session.totalSize, - }; + uploadChunk: function(this: LibrarianService, request: LibrarianRequest): Promise { + return Effect.runPromise(uploadChunkEffect(request)); }, @@ -1034,35 +1075,15 @@ export function makeLibrarianService(config: LibrarianServiceConfig): LibrarianS - getUploadStatus: function(this: LibrarianService, request: LibrarianRequest): LibrarianResponse { - const uploadId = optionalString(this.requestRecord(request)["upload-id"]); - if (uploadId === undefined) throwLibrarianServiceError("get-upload-status", "get-upload-status requires upload-id"); - const session = this.uploads.get(uploadId); - if (session === undefined) throwLibrarianServiceError("get-upload-status", `Upload not found: ${uploadId}`); - const receivedChunks = [...session.chunks.keys()].sort((a, b) => a - b); - const receivedSet = new Set(receivedChunks); - const missingChunks = Array.from({ length: session.totalChunks }, (_, i) => i).filter((i) => !receivedSet.has(i)); - const bytesReceived = [...session.chunks.values()].reduce((sum, chunk) => sum + chunk.length, 0); - return { - "upload-id": uploadId, - "upload-state": "in-progress", - "chunks-received": session.chunks.size, - "total-chunks": session.totalChunks, - "received-chunks": receivedChunks, - "missing-chunks": missingChunks, - "bytes-received": bytesReceived, - "total-bytes": session.totalSize, - }; + getUploadStatus: function(this: LibrarianService, request: LibrarianRequest): Promise { + return Effect.runPromise(getUploadStatusEffect(request)); }, - abortUpload: function(this: LibrarianService, request: LibrarianRequest): LibrarianResponse { - const uploadId = optionalString(this.requestRecord(request)["upload-id"]); - if (uploadId === undefined) throwLibrarianServiceError("abort-upload", "abort-upload requires upload-id"); - this.uploads.delete(uploadId); - return {}; + abortUpload: function(this: LibrarianService, request: LibrarianRequest): Promise { + return Effect.runPromise(abortUploadEffect(request)); },