feat: ship hosted parsing v1#10
Conversation
Correlate requests, jobs, workflows, provider executions, native parsers, and cleanup with structured wide events. Add optional PostHog replay and error tracking, Autumn metering, and branded web/API error handling.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Important Review skippedToo many files! This PR contains 110 files, which is 10 over the limit of 100. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (17)
📒 Files selected for processing (111)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@greptile-apps @coderabbitai review |
|
✅ Action performedReview finished.
|
Greptile SummaryThis PR ships hosted parsing v1 for FileRouter. The main changes are:
Confidence Score: 4/5One contained workflow issue should be fixed before hosted async providers are reliable. The native parser and SDK changes are broadly coherent. The workflow polling loop can replay stale status because repeated polls use the same step names. The issue is isolated to async provider polling. src/workflows/document-workflow.ts
 T-Rex LogsWhat T-Rex did
Important Files Changed
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6804d6f73f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| !params.requestId || | ||
| !params.userId || | ||
| params.providers.length === 0 || | ||
| params.outputs.length === 0 || | ||
| !validSource | ||
| !params.source.key |
There was a problem hiding this comment.
When this is deployed while any pre-change document workflow is still queued/running, its saved payload will not have requestId/userId, and URL-source jobs used the old { kind: "url", url } shape rather than a source.key. This new validation throws before the workflow enters the try block, so those durable instances fail without marking their D1 job failed or complete, leaving clients polling a stuck job. Please accept/upgrade the old payload shape or fail the job inside the existing error path.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
9 issues found across 109 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="services/native-parsers/src/index.ts">
<violation number="1" location="services/native-parsers/src/index.ts:70">
P3: Unexpected source/container failures return the runtime-generated error response without the assigned `X-Request-Id`, so callers cannot correlate those 5xx responses with `native_parser_request_completed` logs. Return a sanitized 5xx through `responseWithRequestId` after recording `failure`.</violation>
</file>
<file name="packages/filerouter/src/mistral.ts">
<violation number="1" location="packages/filerouter/src/mistral.ts:238">
P2: Successful OCR calls can omit `usage.costUsd` when Mistral returns its `-completion` response model ID, because rate lookup only recognizes request-style IDs. Normalize that suffix (or pass the resolved request model into normalization) before rate lookup.</violation>
</file>
<file name="src/components/api-keys.tsx">
<violation number="1" location="src/components/api-keys.tsx:178">
P3: API-key rows display `undefined...` when Better Auth is configured not to persist `start` (or for older rows). Keep a prefix/friendly fallback so keys remain identifiable.</violation>
</file>
<file name="src/workflows/document-workflow.ts">
<violation number="1" location="src/workflows/document-workflow.ts:122">
P1: A D1 failure while marking a job running now leaves its row `queued` after the workflow deletes its source, so `jobs.wait()` keeps polling a job that cannot recover. Include `queued` in the failure transition (or make the running transition retry/atomic) so this path becomes terminal.</violation>
</file>
<file name="src/workflows/document-source.ts">
<violation number="1" location="src/workflows/document-source.ts:63">
P2: Dead or invalid document URLs are fetched up to four times before the job fails because permanent response errors are ordinary `Error`s inside the retried storage step. Classify known validation/HTTP-response failures as `NonRetryableError` while retaining retries for transient fetch and R2 failures.</violation>
</file>
<file name="services/native-parsers/engines/pdf-inspector/Dockerfile">
<violation number="1" location="services/native-parsers/engines/pdf-inspector/Dockerfile:6">
P2: Build context includes unnecessary files — add a `.dockerignore` to exclude `node_modules/`, `.git/`, `.env`, and editor files from the Docker build context. Without one, the full `engines/` directory is sent to the daemon every build, slowing builds and risking inconsistent cache behavior when local `node_modules/` exists.</violation>
</file>
<file name="services/native-parsers/engines/liteparse/package.json">
<violation number="1" location="services/native-parsers/engines/liteparse/package.json:6">
P2: The `start` script runs `node server.ts` directly, but Node.js requires `--experimental-strip-types` to run `.ts` files before version 24. The project declares `node@>=22.14.0` as a dependency, so developers on Node 22 or 23 will see the server crash with an unrecognized extension/syntax error when running `pnpm start`. Production (Node 24 in Docker) is unaffected, but local dev and CI on <24 will break.
Recommend either: (a) adding `--experimental-strip-types` to the start command, or (b) adding an `.nvmrc`/engines field that signals the expected runtime.</violation>
</file>
<file name="src/api/app.ts">
<violation number="1" location="src/api/app.ts:93">
P3: Rejected API requests such as invalid job IDs and authentication failures omit `error_code` from `api_request_completed` telemetry. Record the `HttpError.code` in `api.onError` before returning its problem response, since Hono handles these errors there rather than propagating them to this middleware catch.</violation>
</file>
<file name="services/native-parsers/engines/shared/observability.ts">
<violation number="1" location="services/native-parsers/engines/shared/observability.ts:18">
P1: `serializeError` drops stack traces from error telemetry, making production debugging harder. Include the stack property when available so operational investigations have the full call path.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| } | ||
|
|
||
| export function serializeError(error: unknown) { | ||
| return error instanceof Error |
There was a problem hiding this comment.
P1: serializeError drops stack traces from error telemetry, making production debugging harder. Include the stack property when available so operational investigations have the full call path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At services/native-parsers/engines/shared/observability.ts, line 18:
<comment>`serializeError` drops stack traces from error telemetry, making production debugging harder. Include the stack property when available so operational investigations have the full call path.</comment>
<file context>
@@ -0,0 +1,21 @@
+}
+
+export function serializeError(error: unknown) {
+ return error instanceof Error
+ ? { error_message: error.message, error_type: error.name }
+ : { error_message: "Unknown error", error_type: "UnknownError" }
</file context>
|
|
||
| try { | ||
| const configured = providers(this.env) | ||
| const startedAt = await step.do("mark job running", async () => { |
There was a problem hiding this comment.
P1: A D1 failure while marking a job running now leaves its row queued after the workflow deletes its source, so jobs.wait() keeps polling a job that cannot recover. Include queued in the failure transition (or make the running transition retry/atomic) so this path becomes terminal.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/workflows/document-workflow.ts, line 122:
<comment>A D1 failure while marking a job running now leaves its row `queued` after the workflow deletes its source, so `jobs.wait()` keeps polling a job that cannot recover. Include `queued` in the failure transition (or make the running transition retry/atomic) so this path becomes terminal.</comment>
<file context>
@@ -72,45 +109,71 @@ export class DocumentWorkflow extends WorkflowEntrypoint<
try {
- const configured = providers(this.env)
+ const startedAt = await step.do("mark job running", async () => {
+ const timestamp = new Date()
+ await createDb(this.env.DB)
</file context>
| @@ -0,0 +1,14 @@ | |||
| FROM node:24-bookworm-slim@sha256:6f7b03f7c2c8e2e784dcf9295400527b9b1270fd37b7e9a7285cf83b6951452d | |||
There was a problem hiding this comment.
P2: Build context includes unnecessary files — add a .dockerignore to exclude node_modules/, .git/, .env, and editor files from the Docker build context. Without one, the full engines/ directory is sent to the daemon every build, slowing builds and risking inconsistent cache behavior when local node_modules/ exists.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At services/native-parsers/engines/pdf-inspector/Dockerfile, line 6:
<comment>Build context includes unnecessary files — add a `.dockerignore` to exclude `node_modules/`, `.git/`, `.env`, and editor files from the Docker build context. Without one, the full `engines/` directory is sent to the daemon every build, slowing builds and risking inconsistent cache behavior when local `node_modules/` exists.</comment>
<file context>
@@ -0,0 +1,14 @@
+WORKDIR /app
+ENV NODE_ENV=production
+
+COPY pdf-inspector/package.json pdf-inspector/package-lock.json ./pdf-inspector/
+RUN cd pdf-inspector && npm ci --omit=dev --no-audit --no-fund
+
</file context>
| return responseWithRequestId(response, requestId) | ||
| } catch (error) { | ||
| failure = error | ||
| throw error |
There was a problem hiding this comment.
P3: Unexpected source/container failures return the runtime-generated error response without the assigned X-Request-Id, so callers cannot correlate those 5xx responses with native_parser_request_completed logs. Return a sanitized 5xx through responseWithRequestId after recording failure.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At services/native-parsers/src/index.ts, line 70:
<comment>Unexpected source/container failures return the runtime-generated error response without the assigned `X-Request-Id`, so callers cannot correlate those 5xx responses with `native_parser_request_completed` logs. Return a sanitized 5xx through `responseWithRequestId` after recording `failure`.</comment>
<file context>
@@ -0,0 +1,197 @@
+ return responseWithRequestId(response, requestId)
+ } catch (error) {
+ failure = error
+ throw error
+ } finally {
+ const status = response?.status ?? 500
</file context>
| await next() | ||
| } catch (error) { | ||
| failure = error | ||
| if (error instanceof HttpError) { |
There was a problem hiding this comment.
P3: Rejected API requests such as invalid job IDs and authentication failures omit error_code from api_request_completed telemetry. Record the HttpError.code in api.onError before returning its problem response, since Hono handles these errors there rather than propagating them to this middleware catch.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/api/app.ts, line 93:
<comment>Rejected API requests such as invalid job IDs and authentication failures omit `error_code` from `api_request_completed` telemetry. Record the `HttpError.code` in `api.onError` before returning its problem response, since Hono handles these errors there rather than propagating them to this middleware catch.</comment>
<file context>
@@ -43,18 +77,73 @@ export const api = new OpenAPIHono<ApiBindings>({
+ await next()
+ } catch (error) {
+ failure = error
+ if (error instanceof HttpError) {
+ requestEvent.error_code = error.code ?? "http_error"
+ }
</file context>
| {key.start ?? key.prefix ?? FILEROUTER_API_KEY_PREFIX}... - | ||
| Created {dateFormatter.format(new Date(key.createdAt))} | ||
| <p className="truncate font-mono text-xs font-medium"> | ||
| {key.start}... |
There was a problem hiding this comment.
P3: API-key rows display undefined... when Better Auth is configured not to persist start (or for older rows). Keep a prefix/friendly fallback so keys remain identifiable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/components/api-keys.tsx, line 178:
<comment>API-key rows display `undefined...` when Better Auth is configured not to persist `start` (or for older rows). Keep a prefix/friendly fallback so keys remain identifiable.</comment>
<file context>
@@ -168,16 +174,18 @@ export function ApiKeys() {
- {key.start ?? key.prefix ?? FILEROUTER_API_KEY_PREFIX}... -
- Created {dateFormatter.format(new Date(key.createdAt))}
+ <p className="truncate font-mono text-xs font-medium">
+ {key.start}...
+ </p>
+ <p className="mt-1 truncate text-xs text-muted-foreground">
</file context>
| {key.start}... | |
| {key.start ?? key.prefix ?? "API key"}... |
1 participant
Summary
parse()andcompare()convenience APIs.Why
FileRouter v1 needs a reliable hosted parsing primitive before it can add intelligent routing. The previous path did not provide first-party hosted native parsers, recoverable SDK job handles, resilient Workflow startup, or production-grade execution telemetry.
This PR establishes those foundations while keeping provider choice explicit and preserving direct/BYOK execution.
Changes
client.jobs.create(),get(), andwait()with typed recoverable handles, stable idempotency keys, bounded transient retry, status callbacks, and total timeout accounting.FileRouterClient.parse()andcompare()as backward-compatible create-and-wait conveniences.Testing
pnpm typecheckpnpm test— 89 tests passedpnpm lintpnpm build:appgit diff --checkReview Notes
services/native-parsers/for the execution boundary.src/workflows/document-workflow.tsandsrc/lib/document-jobs.server.tsfor lifecycle and retry semantics.packages/filerouter/src/jobs.tsandpackages/filerouter/src/client.tsfor the public SDK contract.Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Ships hosted parsing v1 with private native parsers, durable/recoverable hosted jobs, and account-level hosted credits. Adds curated LiteParse, usage metering, and product analytics for better reliability, billing, and visibility.
New Features
liteparseandpdf-inspectorproviders running in private Cloudflare containers behind a native‑parser Worker with caps on bytes, pages, and concurrency; LiteParse supports curated managed OCR, Office conversion, image modes, complexity, and screenshots with explicit failures for unsupported settings.client.jobs.create(),get(), andwait()(stable idempotency keys, timeouts, bounded transient retry, status callbacks);parse()andcompare()remain convenience wrappers.autumn-js; records provider‑reported costs and estimates managed execution; returns402 Payment Required(insufficient_credits) when depleted and maps toPaymentRequiredin the SDK.posthog-js/posthog-node, branded web/API error pages and 404; docs and navigation updated; brand geometry refined.liteparseandpdf-inspector, keepslocalProviderIdslimited to directly configured providers for local comparisons, allows unnamed API keys; exportsselectParseOutputs.Migration
parse()andcompare()calls work as before.client.jobs.create(),client.jobs.get(), andclient.jobs.wait()from@file_router/sdk.AUTUMN_SECRET_KEYfor hosted credits and metering;POSTHOG_PROJECT_TOKENandPOSTHOG_HOSTfor analytics. A402 Payment Requiredwithinsufficient_creditsindicates the account needs credits (checkout from the dashboard).Written for commit 97f7c75. Summary will update on new commits.