feat: durable job queues for conversion, extraction and embedding (ADR) - #40
Open
amal66 wants to merge 1 commit into
Open
feat: durable job queues for conversion, extraction and embedding (ADR)#40amal66 wants to merge 1 commit into
amal66 wants to merge 1 commit into
Conversation
…xtraction Port of the fork's (amal66/mike) durable job-queue architecture: - lib/queue/: shared lazy Redis connection (BullMQ-safe options), the document-conversion and tabular-extraction queues with deterministic jobIds (dedupe on double submit), attempts:3 + exponential backoff, and the Redis pub/sub bridge (runProgress) that lets an SSE request tail a running extraction. - workers/: in-process BullMQ workers behind a declarative registry. Conversion mirrors the synchronous upload path's semantics (conversion failure is non-fatal; a permanently failed job flips the document to "error"). Extraction re-derives everything from the DB at run time (no secrets in Redis), is idempotent/retry-safe, and marks surviving cells "error" once retries are exhausted. - lib/tabular/: the extraction core factored out of routes/tabular.ts (extractDocumentColumns + LLM/text-extraction helpers + prepare guard) so the synchronous route and the async worker share one loop. - routes/tabular.ts: POST /:reviewId/generate gains the async path (enqueue + reconnectable tail) behind ASYNC_TABULAR_EXTRACTION, and a new GET /:reviewId/generate/stream resume endpoint. The inline path remains the default. - routes/documents.ts: upload defers DOCX->PDF conversion to the queue behind ASYNC_DOCUMENT_CONVERSION; default stays inline. - index.ts: workers start only when a flag is on; graceful shutdown closes the server, workers, queues and Redis. Both flags default "false": with no configuration nothing dials Redis and behavior is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CEguyEgXa9JjCciXCcVemC
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Design (ADR)
Summarized from the fork's
docs/adr/0003-default-synchronous-env-gated-async.md,docs/adr/0004-database-enforced-idempotency.mdanddocs/async-jobs.md.Context. Two workloads are expensive and can outlive a single request: DOCX→PDF conversion and tabular cell extraction. Running them inline is simple and needs no infrastructure, but the work dies with the request (a client disconnect or server restart loses it) and can't retry. Making them async needs Redis and workers — infrastructure a self-hoster kicking the tires should not be forced to run.
Decision. Every async-capable path is synchronous by default and opt-in to async via an
ASYNC_*env flag, each defaulting to"false":anyWorkerEnabled()is true, so the default process has no Redis dependency at all.attempts: 3, exponential). Bulkhead concurrency caps per worker (conversion 2, extraction 3) bound resource use.extractDocumentColumnsis called by the sync route and the async worker, differing only inmissing-cell policy — so async is a deployment choice, not a rewrite.jobIddeterministically from the work identity (versionIdfor conversion,(reviewId, documentId)for extraction), so a double submit (client retry, reconnect) collapses into the in-flight job instead of running twice. Durable state lives in DB rows (documents.status,tabular_cells), not queue history; extraction jobs re-read cell state and only process columns not alreadydone, so retries never redo finished work./generaterequest becomes a reconnectable view: it subscribes to the review's Redis progress channel before enqueuing, forwards the samecell_updateSSE frames the sync path emits, and a 3-second DB-poll backstop reconciles missed pub/sub frames so a dropped message can never leave the stream hung.GET /:reviewId/generate/streamlets a dropped client resume without re-triggering work.Consequences. Trivial default onboarding (fresh clone runs everything inline); durability and retries where they matter; two code paths kept honest by sharing one extraction core; workers run in-process by default (split into a dedicated process by calling
startWorkers()from a separate entrypoint when scaling apart).Alternatives considered. Always-async (rejected: forces Redis on every self-hoster); app-level dedupe/locking for double submits (rejected: racy across replicas — the queue's jobId uniqueness and the DB are the only single points of serialization); a cron/poller instead of a queue (no retry semantics, no backpressure, higher latency).
Summary
Fewer stuck documents and fewer lost review runs. Today, a large DOCX upload blocks its HTTP request on LibreOffice, and a tabular review generation dies if the browser tab closes or the server restarts mid-run — the grid is left with spinners that never resolve. With the flags on, uploads return immediately and convert in the background with retries, and review extraction survives disconnects and restarts, retries transient LLM/storage failures with backoff, and lets a client reconnect to a running generation and catch up. Failures become explicit terminal states (document
error, cellerror) instead of hangs.Changes
backend/src/lib/queue/— shared lazy Redis connection (BullMQ-safemaxRetriesPerRequest: null),conversionQueue,extractionQueue(deterministic jobIds, retry/backoff, bounded history), andrunProgress(Redis pub/sub bridge for live cell updates).backend/src/workers/—conversionWorker,extractionWorker, a declarativeWORKER_REGISTRY, andstartWorkers()/stopWorkers()lifecycle.backend/src/lib/tabular/— the extraction core factored out ofroutes/tabular.tsso the sync route and the async worker share one loop:tabular.extract.ts(LLM cell extraction + PDF/DOCX/Office text extraction),tabular.extractDoc.ts(extractDocumentColumns),tabular.generate.ts(prepareTabularGenerateguard),tabular.generateStream.ts(async enqueue + reconnectable SSE tail),tabular.shared.ts,tabular.prompt.ts.routes/tabular.tsnow imports these instead of its inline copies (moved, not changed — bodies are byte-identical apart from import/type-annotation mechanics).backend/src/routes/tabular.ts—POST /:reviewId/generategains the async path behindASYNC_TABULAR_EXTRACTION; newGET /:reviewId/generate/streamresume endpoint. Inline path remains the default.backend/src/routes/documents.ts— upload defers Office→PDF conversion to the queue behindASYNC_DOCUMENT_CONVERSION(doc staysprocessinguntil the worker flips it toready).backend/src/index.ts— start workers only when a flag is on; graceful SIGTERM/SIGINT shutdown (drain server, stop workers, close queues + Redis, 15s force-exit guard).backend/src/lib/sseHeartbeat.ts,backend/src/lib/pdfjs.ts— small helpers the above use (SSE keepalive comments; typed facade overpdfjs-dist).backend/.env.example,backend/tsconfig.json(exclude tests fromtsc, as the fork does),backend/package.json+ lockfile.lib/queue/__tests__/{conversionQueue,extractionQueue}.test.ts,workers/__tests__/{conversionWorker,extractionWorker}.test.ts,lib/tabular/__tests__/{tabular.extractDoc,tabular.generateStream}.test.ts.Why
Leads with the cost posture: both flags default
"false", and with them off the server never dials Redis — behavior and infrastructure requirements are exactly what they are today. A cost-sensitive firm on a £20 VPS changes nothing and pays nothing extra. Turning a queue on needs only a Redis container on the same box (REDIS_URL, defaults toredis://localhost:6379).New runtime deps (both shipped by the fork for this feature):
bullmq ^5.34.0(the queue) andioredis 5.10.1(its Redis client; the lockfile pins bullmq 5.79.2, matching the fork's lockfile, so a single deduped ioredis serves both). No other dependency, schema, or API change.Note on the title: the fork's third queue (document embedding) is not in this PR — its queue/worker import the RAG ingest module, so that wiring rides in the follow-up RAG PR where the dependency is real.
Testing
npm install && npm run build(tsc) green on the branch as committed.npx vitest run→ 7 test files, 39 tests, all passing (the 6 ported suites above plus the harness's existingdownloadTokenssuite)."true"(connection is lazy and only reached viaenqueue*/startWorkers).Provenance
All added lines are mechanical ports of amal66/mike@origin/main (b3166dd) — path moves (
apps/api/src/*→backend/src/*,modules/tabular/*→lib/tabular/*), import rewrites, and inlined types; exceptions:lib/env.tsmodule is not ported; env reads are inlined asprocess.env.Xwith the fork's defaults preserved (ASYNC_*default"false",REDIS_URLdefaultredis://localhost:6379).loggercalls are translated toconsole.*, and OpenTelemetry trace propagation (withTraceContext, theotel?: OtelCarrierpayload field,runWithRequestContext/withExtractedContextwrappers) is removed from queue payloads and worker callbacks. TheLogtype becomesPick<Console, "error">.tabular.prompt.tsports onlyformatPromptSuffix(identical to the inline copy this PR deletes fromroutes/tabular.ts);tabular.generate.tsports onlyprepareTabularGenerate;tabular.extract.tsomits the fork'sextractTabularAnnotationsand the redline-summary enrichment ofextractDocxMarkdown— those depend on fork-only chat/tracked-changes code, so the ported body matches upstream's existing inline function exactly.routes/tabular.tsdeletions are the inline copies of the moved helpers (verified byte-identical modulo import/type annotations); the new/generatebody is the fork's route body withreq.log→console.vi.mock("../../env", …)shims are dropped (no env module here) and mock import paths follow the path moves.bullmq/ioredisversion pins match the fork's package.json + lockfile resolution.Credits & prior art
🤖 Generated with Claude Code
https://claude.ai/code/session_01CEguyEgXa9JjCciXCcVemC