feat: add Affinda plugin with 119 operations - #375
Conversation
Implements the Affinda document intelligence integration with Bearer API key auth, routes across documents, collections, extractors, annotations, and workspaces. Closes corsairdev#374.
|
@Ayush7614 is attempting to deploy a commit to the corsair Team on Vercel. A member of the Team first needs to authorize it. |
Greptile SummaryThe PR adds an Affinda plugin exposing 119 operations through shared route, schema, request, logging, and cache infrastructure.
Confidence Score: 5/5The PR appears safe to merge, with only a remaining non-blocking documentation issue in the existing test assertion thread. No blocking failure remains. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
Caller["Corsair caller"] --> Endpoint["Affinda endpoint group"]
Endpoint --> Factory["executeAffindaOperation"]
Factory --> Routes["Route metadata and schemas"]
Factory --> Client["makeAffindaRequest"]
Client --> API["Affinda API v3"]
API --> Cache["Optional entity cache sync"]
Factory --> Log["Operation event logging"]
Reviews (4): Last reviewed commit: "Merge branch 'main' into feat/affinda-pl..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Remove redundant TOKEN config, consolidate shared Zod schemas with comments, and add type assertion justification comments in tests.
Cache documents, collections, and workspaces via ctx.db upserts; rename endpoint files to kebab-case; remove generator scripts.
|
Warning Review limit reached
Next review available in: 31 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (34)
📝 WalkthroughWalkthroughAdds the ChangesAffinda integration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant AffindaEndpoint
participant executeAffindaOperation
participant makeAffindaRequest
participant AffindaAPI
participant AffindaCache
AffindaEndpoint->>executeAffindaOperation: pass context and input
executeAffindaOperation->>makeAffindaRequest: send resolved route, query, body, and headers
makeAffindaRequest->>AffindaAPI: send authenticated request
AffindaAPI-->>makeAffindaRequest: return response or error
makeAffindaRequest-->>executeAffindaOperation: return response
executeAffindaOperation->>AffindaCache: synchronize successful result
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
|
@greptile review |
Plugin PR scorecard —
|
| Check | Status | Notes |
|---|---|---|
| R1 — Scope: plugin files only | ✅ | |
| R2 — Tests with assertions | ✅ | |
| R3 — Description complete | ✅ | |
| R3 — Linked issue / claim | ✅ | |
| R4 — Demo video / recording | ✅ |
Rules: PLUGIN_PR_RULES.md · re-runs on every push
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
packages/affinda/client.ts (1)
62-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCollapse the duplicate catch branches.
ApiErrorextendsError, and both branches construct the sameAffindaAPIErrorwith the same arguments. The first branch is therefore unreachable in effect. TheApiErrordetail extraction already happens inside theAffindaAPIErrorconstructor at Lines 14-18.♻️ Proposed refactor
} catch (error) { - if (error instanceof ApiError) { - throw new AffindaAPIError(error.message, { cause: error }); - } if (error instanceof Error) { throw new AffindaAPIError(error.message, { cause: error }); } throw new AffindaAPIError('Unknown error'); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/affinda/client.ts` around lines 62 - 69, Collapse the duplicate error handling in the catch block by removing the separate ApiError branch and retaining a single Error-based branch that constructs AffindaAPIError with the original error as cause. Preserve the existing Unknown error fallback for non-Error values.packages/affinda/endpoints/routes.ts (1)
18-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a
satisfiesclause so the compiler validates all 119 route entries.
affindaRoutesdeclaresas constonly.AffindaRouteis therefore never applied to the literal, so a typo inmethod,riskLevel, or a missingdescriptionstays undetected until runtime.satisfieskeeps the narrow literal types thatAffindaRoutesdepends on while enforcing the declared shape.♻️ Proposed refactor
-export const affindaRoutes = [ +export const affindaRoutes = [Apply the clause at the end of the literal:
-] as const; +] as const satisfies readonly AffindaRoute[];Also applies to: 1500-1502
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/affinda/endpoints/routes.ts` at line 18, Update the affindaRoutes array declaration to add a satisfies clause against the intended route collection type after the existing as const assertion, so every route entry is validated while preserving narrow literal types used by AffindaRoutes.packages/affinda/endpoints/cache-sync.ts (2)
103-107: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSequential cache writes add latency proportional to the page size.
Each item is awaited in turn, and this runs before
executeAffindaOperationreturns the result to the caller. A list response of 100 documents therefore performs 100 sequential database round trips on the request path. Batch the writes, or bound the concurrency.♻️ Proposed refactor
- for (const item of cacheItems(response, rule)) { - const entityId = cacheEntityId(item, rule); - if (!entityId) continue; - await client.upsertByEntityId(entityId, item); - } + const upsert = client.upsertByEntityId; + const writes = cacheItems(response, rule).flatMap((item) => { + const entityId = cacheEntityId(item, rule); + return entityId ? [upsert(entityId, item)] : []; + }); + await Promise.all(writes);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/affinda/endpoints/cache-sync.ts` around lines 103 - 107, Update the cache synchronization loop around cacheItems and client.upsertByEntityId to avoid awaiting writes sequentially; batch the upserts or use bounded concurrency while preserving entityId filtering and ensuring all cache writes complete before executeAffindaOperation returns.
76-90: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the
ctx.dbshape and use the Corsair logger.Two smaller items in this block:
The inline
ctx.db as Record<...>cast declares the client shape at the use site. That hides any real type onAffindaContextand will not fail if the Corsair database surface changes. Declare a namedCacheClienttype, and derive it from the Corsair type if one is exported.The catch block writes to
console.warn.packages/affinda/endpoints/factory.tsalready useslogEventFromContextfromcorsair/corefor operation logging. Route this warning through the same mechanism so cache failures appear in the same place as operation events.Also applies to: 108-110
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/affinda/endpoints/cache-sync.ts` around lines 76 - 90, Define a named CacheClient type for the database client shape, deriving it from the exported Corsair type when available, and use it instead of the inline ctx.db cast in the cache-sync flow. In the catch block, replace console.warn with logEventFromContext from corsair/core, preserving the existing cache-failure warning details and routing them through the operation logger.packages/affinda/endpoints/factory.ts (2)
143-149: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueBuild a route lookup map once.
getRoutescans all 119 entries linearly. Every endpoint wrapper calls it at module load, so package import performs roughly 119 × 119 comparisons. The cost is small, but aMapkeyed bynameis simpler and constant-time.♻️ Proposed refactor
+const ROUTES_BY_NAME = new Map( + affindaRoutes.map((route) => [route.name, route as AffindaRoute]), +); + export function getRoute(name: string): AffindaRoute { - const route = affindaRoutes.find((candidate) => candidate.name === name); + const route = ROUTES_BY_NAME.get(name); if (!route) { throw new Error(`[affinda] missing route: ${name}`); } return route; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/affinda/endpoints/factory.ts` around lines 143 - 149, Replace the repeated linear search in getRoute with a module-level Map keyed by each affindaRoutes entry’s name, initialized once from affindaRoutes. Look up the requested name through that map while preserving the existing missing-route error and returned AffindaRoute behavior.
63-81: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPositional placeholder mapping is fragile.
resolvePathmaps the nth{...}placeholder toroute.pathParams[index]. The mapping is therefore correct only whenpathParamsis written in the same order as the placeholders, and it silently mis-maps when the order differs. The catalog already contains entries wherepathParamsdoes not match the placeholder names at all, for exampledeleteCollectioninpackages/affinda/endpoints/routes.ts(Line 376).Consider resolving by placeholder name and treating
pathParamsas a declared set to validate against, instead of an ordered lookup table. That makes an author mistake a startup failure rather than a wrong URL.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/affinda/endpoints/factory.ts` around lines 63 - 81, Update resolvePath to resolve each placeholder by its name rather than by its positional index in route.pathParams. Treat route.pathParams as a declared set: validate that every placeholder is declared and fail during route initialization when names are missing or inconsistent, while preserving the existing input key and camelToSnake fallback resolution.packages/affinda/endpoints/annotations.ts (1)
4-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a single wrapper factory for all 119 endpoints.
Every endpoint in this file, and in the other 23 endpoint files, repeats the same three lines: resolve the route by name, then delegate to
executeAffindaOperation. A small helper removes about 700 lines and gives one place to add input validation later.♻️ Proposed refactor
Add the helper to
packages/affinda/endpoints/factory.ts:export function defineAffindaEndpoint(name: string): AffindaEndpoint { const route = getRoute(name); return (ctx, input = {}) => executeAffindaOperation(ctx, input, route); }Then each wrapper becomes one line:
-const batchUpdateAnnotationsRoute = getRoute('batchUpdateAnnotations'); -export const batchUpdateAnnotations: AffindaEndpoint = async (ctx, input = {}) => { - return executeAffindaOperation(ctx, input, batchUpdateAnnotationsRoute); -}; +export const batchUpdateAnnotations = defineAffindaEndpoint('batchUpdateAnnotations');If a generator produces these files, apply the change in the generator template.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/affinda/endpoints/annotations.ts` around lines 4 - 27, Introduce a shared defineAffindaEndpoint factory in the endpoints factory module that resolves a route once and delegates to executeAffindaOperation, then replace the repeated route constants and wrapper functions such as batchUpdateAnnotations, createBatchAnnotations, and updateAnnotation with factory-based definitions across all endpoint files. If these wrappers are generated, update the generator template as the source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/affinda/client.ts`:
- Around line 44-48: Update the HEADERS construction in requestAffindaOperation
so the caller-provided headers are spread before the plugin-owned Content-Type
and Authorization values; keep those fixed headers last so input.headers cannot
override them.
In `@packages/affinda/endpoints/factory.ts`:
- Around line 188-194: Update the finally block in the operation wrapper around
logAffindaOperation so logging failures are caught and isolated rather than
propagated. Preserve the original error thrown from the catch block, including
AffindaAPIError details, while retaining the existing operation logging attempt
and status handling.
- Around line 127-141: Update the body construction logic in the endpoint
factory to preserve Affinda’s generated wire-format field names, including
targetUrl, instead of applying camelToSnake to every body key. Remove or limit
the global conversion while retaining snakeCaseKeys only where explicitly
required, and ensure schemas containing both camelCase and snake_case fields
remain unambiguous.
In `@packages/affinda/endpoints/routes.ts`:
- Around line 1211-1223: Update the removeTagFromDocuments route definition to
use riskLevel: 'write' and remove the irreversible: true flag, matching the
reversible behavior and the configuration of addTagToDocuments.
- Around line 550-562: Align each affected route’s path and pathParams with its
required schema field, removing those identifiers from queryParams: in
packages/affinda/endpoints/routes.ts, update deleteWorkspace (550-562) to
workspace_id, deleteResthookSubscription (511-523) to identifier, getTag
(1026-1037) to tag_id, getResthookSubscription (1002-1013) to identifier,
getUsageByWorkspace (1038-1049) to workspace_id, updateResthookSubscription
(1452-1463) to identifier, and updateWorkspace (1488-1499) to workspace_id. In
packages/affinda/endpoints/types.ts, retain only the required matching field in
DeleteWorkspaceInputSchema (763-769), GetResthookSubscriptionInputSchema
(1293-1300), GetTagInputSchema (1325-1332), GetUsageByWorkspaceInputSchema
(1337-1346), UpdateResthookSubscriptionInputSchema (1915-1925), and
UpdateWorkspaceInputSchema (1991-2005). Add one test for each route using only
its required identifier and assert the generated URL.
In `@packages/affinda/error-handlers.ts`:
- Around line 37-45: Update the SERVER_ERROR handler and its surrounding retry
configuration to avoid exponential retries for non-idempotent mutations such as
createResthookSubscription. Only return retry settings for operations proven
idempotent or carrying a supported idempotency key; otherwise disable retries
while preserving the existing 5xx status matching.
---
Nitpick comments:
In `@packages/affinda/client.ts`:
- Around line 62-69: Collapse the duplicate error handling in the catch block by
removing the separate ApiError branch and retaining a single Error-based branch
that constructs AffindaAPIError with the original error as cause. Preserve the
existing Unknown error fallback for non-Error values.
In `@packages/affinda/endpoints/annotations.ts`:
- Around line 4-27: Introduce a shared defineAffindaEndpoint factory in the
endpoints factory module that resolves a route once and delegates to
executeAffindaOperation, then replace the repeated route constants and wrapper
functions such as batchUpdateAnnotations, createBatchAnnotations, and
updateAnnotation with factory-based definitions across all endpoint files. If
these wrappers are generated, update the generator template as the source of
truth.
In `@packages/affinda/endpoints/cache-sync.ts`:
- Around line 103-107: Update the cache synchronization loop around cacheItems
and client.upsertByEntityId to avoid awaiting writes sequentially; batch the
upserts or use bounded concurrency while preserving entityId filtering and
ensuring all cache writes complete before executeAffindaOperation returns.
- Around line 76-90: Define a named CacheClient type for the database client
shape, deriving it from the exported Corsair type when available, and use it
instead of the inline ctx.db cast in the cache-sync flow. In the catch block,
replace console.warn with logEventFromContext from corsair/core, preserving the
existing cache-failure warning details and routing them through the operation
logger.
In `@packages/affinda/endpoints/factory.ts`:
- Around line 143-149: Replace the repeated linear search in getRoute with a
module-level Map keyed by each affindaRoutes entry’s name, initialized once from
affindaRoutes. Look up the requested name through that map while preserving the
existing missing-route error and returned AffindaRoute behavior.
- Around line 63-81: Update resolvePath to resolve each placeholder by its name
rather than by its positional index in route.pathParams. Treat route.pathParams
as a declared set: validate that every placeholder is declared and fail during
route initialization when names are missing or inconsistent, while preserving
the existing input key and camelToSnake fallback resolution.
In `@packages/affinda/endpoints/routes.ts`:
- Line 18: Update the affindaRoutes array declaration to add a satisfies clause
against the intended route collection type after the existing as const
assertion, so every route entry is validated while preserving narrow literal
types used by AffindaRoutes.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: dc3bc443-eb56-468a-8c72-829345fb5142
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (42)
demo/testing/package.jsondemo/testing/src/server/corsair.tspackages/affinda/api.test.tspackages/affinda/client.tspackages/affinda/endpoints/annotations.tspackages/affinda/endpoints/api-users.tspackages/affinda/endpoints/cache-sync.tspackages/affinda/endpoints/collections.tspackages/affinda/endpoints/data-point-choices.tspackages/affinda/endpoints/data-points.tspackages/affinda/endpoints/data-sources.tspackages/affinda/endpoints/document-splitters.tspackages/affinda/endpoints/document-types.tspackages/affinda/endpoints/documents.tspackages/affinda/endpoints/extractors.tspackages/affinda/endpoints/factory.tspackages/affinda/endpoints/index.tspackages/affinda/endpoints/indexes.tspackages/affinda/endpoints/invitations.tspackages/affinda/endpoints/job-description-search.tspackages/affinda/endpoints/mappings.tspackages/affinda/endpoints/occupation-groups.tspackages/affinda/endpoints/organization-memberships.tspackages/affinda/endpoints/organizations.tspackages/affinda/endpoints/resthooks.tspackages/affinda/endpoints/resume-search.tspackages/affinda/endpoints/routes.tspackages/affinda/endpoints/tags.tspackages/affinda/endpoints/types.tspackages/affinda/endpoints/validation-results.tspackages/affinda/endpoints/validation.tspackages/affinda/endpoints/workspace-memberships.tspackages/affinda/endpoints/workspaces.tspackages/affinda/error-handlers.tspackages/affinda/index.tspackages/affinda/jest.config.jsonpackages/affinda/package.jsonpackages/affinda/schema/database.tspackages/affinda/schema/index.tspackages/affinda/tsconfig.jsonpackages/affinda/tsup.config.tspackages/corsair/core/constants.ts
2 participants
Description
@corsair-dev/affindaplugin with all 119 Composio-mapped operationsAuthorization: Bearer <key>AFFINDA_API_KEYenv varCloses #374
Test plan
cd packages/affinda && pnpm typecheck— passescd packages/affinda && pnpm build— passes (~101 KB dist)cd packages/affinda && pnpm test— 4/4 tests passGET /v3/organizationsreturns 200 with valid keyScreenshots / Demos
Summary by CodeRabbit