feat(ai): add multi-provider AI support with enhanced extraction#547
feat(ai): add multi-provider AI support with enhanced extraction#547kelvinkipruto wants to merge 2 commits intomainfrom
Conversation
- Introduce provider catalog and registry to support multiple AI providers (Google, OpenAI, Anthropic, etc.) - Add provider credentials management in settings with environment variable fallbacks - Implement two-pass extraction with chunking and tool-based normalization - Add force re-extract capability for existing documents - Update dependencies to latest AI SDK versions and Zod 4.3.6 - Deprecate legacy Google-only model configuration in favor of new provider system
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb9861ef8d
ℹ️ 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".
| for (const extraction of existingExtractions) { | ||
| await payload.delete({ | ||
| collection: "ai-extractions", | ||
| id: extraction.id, | ||
| }); |
There was a problem hiding this comment.
Delay deleting existing extractions until re-extract succeeds
When forceReextract is enabled, this loop deletes all prior ai-extractions before the new AI run is validated; if any later step fails (API outage, parsing error, empty text, etc.), the catch block marks the document failed but the previous extraction data is already gone. This introduces avoidable data loss and can leave documents without any extraction even though they had valid historical results.
Useful? React with 👍 / 👎.
| if ( | ||
| APICallError.isInstance(error) || | ||
| NoObjectGeneratedError.isInstance(error) | ||
| ) { | ||
| continue; |
There was a problem hiding this comment.
Fail document when all chunk extractions error out
These errors are swallowed per chunk and execution continues, but there is no later check that at least one chunk actually produced candidates. If every chunk hits APICallError/NoObjectGeneratedError (for example due transient provider issues or unsupported structured output), the task can still proceed and eventually mark the document as "Analysed by AI (No promises found)" with fullyProcessed: true, which records a false negative instead of a recoverable failure.
Useful? React with 👍 / 👎.
|
@claude review |
|
Claude finished @kelvinkipruto's task in 2m 19s —— View job Code Review
Overall this is a well-structured feature. The provider catalog/registry split is clean, backward compatibility with legacy config is handled gracefully, and the two-pass extraction with chunking is a meaningful improvement. Below are issues ranging from critical to minor. P1 — Bugs / Data Loss1. Data deleted before new extraction succeeds ( When // Existing extractions deleted here...
for (const extraction of existingExtractions) {
await payload.delete({ collection: "ai-extractions", id: extraction.id });
}
// ...but extraction can still fail after this point
const extractedCandidates = await extractPromisesFromChunks({...});Fix: Defer deletion until after the new 2. All-chunk failures produce false negatives (
} catch (error) {
// ...logged as warn...
if (APICallError.isInstance(error) || NoObjectGeneratedError.isInstance(error)) {
continue; // ← silently drops chunk failure
}
throw error;
}Fix: After the chunk loop, check if P2 — Security / Design Concerns3. API keys stored as plaintext in database ( The Consider using a password/secret field type, environment-variable-only configuration for production, or at minimum documenting this limitation prominently. 4. Speculative model IDs in catalog ( The catalog includes model IDs that don't exist at the time of this PR (as of early 2026):
Including non-existent model IDs will fail silently at configuration time (they pass 5. return createProviderRegistry(providers as never);This completely disables TypeScript's type checking for the registry creation and hides any type incompatibilities between the provider objects and what P3 — Minor / UX Issues6. All 14 providers instantiated per extraction (
7. Silent candidate truncation ( When more than 220 candidates are found, the list is silently truncated (first 220 kept, rest dropped). There's a 8. admin: {
condition: (_, siblingData) => siblingData?.provider === "ollama",
...
},OpenAI-compatible providers and Azure OpenAI deployments commonly need custom base URLs. The 9. Ollama URL normalization is fragile ( const sanitized = baseURL.replace(/\/+$/, "");
return sanitized.endsWith("/v1") ? sanitized : `${sanitized}/v1`;This will incorrectly handle URLs like 10. If the model hits 40 steps without calling Nits
Summary: The architecture is solid and the backward compatibility path is well thought out. The two P1 issues (data loss during force re-extract and false-negative on all-chunk-failure) should be addressed before merge. The security concern around plaintext API key storage and the speculative model IDs in the catalog are also worth resolving or at least explicitly acknowledging. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b1afc646b
ℹ️ 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".
| finalizeExtraction, | ||
| }, | ||
| toolChoice: "required", | ||
| stopWhen: [hasToolCall("finalizeExtraction"), stepCountIs(MAX_TOOL_STEPS)], |
There was a problem hiding this comment.
Guard against silent truncation when tool step cap is hit
The normalization pass can silently return incomplete results because stepCountIs(MAX_TOOL_STEPS) stops generation after 40 steps even if finalizeExtraction was never called. On long documents (or when the model emits one recordPromise call per step), this returns a partial normalizedPromises list and the task still proceeds to mark the document as fully processed, which can undercount promises without surfacing a failure.
Useful? React with 👍 / 👎.
Description
Fixes #538
Type of change
Screenshots
Checklist: