feat: add async polling for graph generation API #7
Conversation
The Supermodel API now returns async job envelopes instead of synchronous graph responses. This adds a polling loop that waits for job completion before extracting graph data. Closes #6
WalkthroughReplaced synchronous graph generation with an async poll-based workflow: initial API returns a status envelope; code polls until completion, extracts graph from Changes
Sequence DiagramsequenceDiagram
participant Client as Client/Test
participant API as Supermodel API
participant Poller as Poller
Client->>API: POST /generateSupermodelGraph (job request)
API-->>Client: {status: "pending", retryAfter: 5, id: jobId}
Poller->>Poller: wait retryAfter seconds
Poller->>API: GET /jobs/{jobId} (status)
API-->>Poller: {status: "processing", retryAfter: 5}
Poller->>Poller: wait retryAfter seconds
Poller->>API: GET /jobs/{jobId} (status)
API-->>Poller: {status: "completed", result: {graph: {...}}}
Poller-->>Client: return resolved graph (nodes + relationships)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
Dead Code HunterNo dead code found! Your codebase is clean. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/__tests__/integration.test.ts`:
- Around line 39-42: Tests are calling the wrong SDK method: replace all uses of
api.generateCallGraph with api.generateSupermodelGraph in the integration tests
(both the initial request and the polling/retry calls) so the tests exercise the
same production code path; also add the same 5-minute polling timeout/safety
check used in production to the test polling loops to prevent infinite waits
during retries.
In `@src/index.ts`:
- Around line 166-179: The polling loop exits on any non-pending/processing
status but doesn't verify success; update the code after the loop (the block
using response from api.generateSupermodelGraph with idempotencyKey and zipBlob)
to explicitly check that response.status === 'completed' and treat any other
status (e.g., 'failed', 'cancelled', or unknown values) as an error: log/throw a
clear error including response.status and response.jobId (and any
response.message) so failures/unknown statuses don't get treated as successful;
keep existing handling for 'failed' but ensure all non-'completed' cases are
handled consistently.
🧹 Nitpick comments (3)
src/index.ts (1)
156-160: Theas anytype cast bypasses TypeScript's safety.Hey! So this
as anycast works, but it's basically telling TypeScript "trust me, I know what I'm doing" - which means you lose all the nice autocomplete and type checking for the response.This is probably happening because the SDK types (v0.4.1) don't match the new async job envelope from the API (v0.6.0). That's totally understandable, but worth noting for later cleanup once the SDK types are updated.
Consider defining an interface for the expected response shape. Something like:
interface AsyncJobResponse { status: 'pending' | 'processing' | 'completed' | 'failed'; jobId?: string; retryAfter?: number; error?: string; result?: { graph?: GraphData }; graph?: GraphData; }This way you still get some type safety even if the SDK doesn't provide it yet.
src/__tests__/integration.test.ts (2)
45-53: Test polling loops have no timeout protection - could hang forever.So in the main code (
src/index.ts), you have this nice 5-minute timeout:const maxPollTime = 5 * 60 * 1000; if (Date.now() - startTime > maxPollTime) { throw new Error('Graph generation timed out...'); }But in the tests... nothing stops the while loop if the API never returns
completedorfailed. The test would just hang until Vitest's 120-second timeout kills it - which isn't as clean and doesn't give you a helpful error message.♻️ Add timeout protection to test polling
+ const maxPollTime = 90 * 1000; // 90 seconds for tests + const startTime = Date.now(); + while (response.status === 'pending' || response.status === 'processing') { + if (Date.now() - startTime > maxPollTime) { + throw new Error(`Test timed out waiting for job ${response.jobId}`); + } const waitSeconds = response.retryAfter || 5; console.log(`Job ${response.jobId} is ${response.status}, waiting ${waitSeconds}s...`); await new Promise(resolve => setTimeout(resolve, waitSeconds * 1000)); response = await api.generateCallGraph({ idempotencyKey, file: zipBlob, }) as any; }Also applies to: 75-82
44-53: Duplicate polling logic in both tests - consider extracting a helper.You've got basically the same polling loop copy-pasted in both tests. This is fine for now (2 occurrences), but if you add more tests or the polling logic changes, you'll have to update multiple places.
♻️ Extract a reusable polling helper
async function pollUntilComplete( api: DefaultApi, idempotencyKey: string, file: Blob, timeoutMs = 90000 ): Promise<any> { const startTime = Date.now(); let response = await api.generateSupermodelGraph({ idempotencyKey, file }) as any; while (response.status === 'pending' || response.status === 'processing') { if (Date.now() - startTime > timeoutMs) { throw new Error(`Polling timed out after ${timeoutMs}ms`); } const waitSeconds = response.retryAfter || 5; console.log(`Job ${response.jobId} is ${response.status}, waiting ${waitSeconds}s...`); await new Promise(resolve => setTimeout(resolve, waitSeconds * 1000)); response = await api.generateSupermodelGraph({ idempotencyKey, file }) as any; } return response; }Then your tests become much cleaner:
const response = await pollUntilComplete(api, idempotencyKey, zipBlob); expect(response.status).toBe('completed');Also applies to: 74-82
| while (response.status === 'pending' || response.status === 'processing') { | ||
| if (Date.now() - startTime > maxPollTime) { | ||
| throw new Error('Graph generation timed out after 5 minutes'); | ||
| } | ||
|
|
||
| const waitSeconds = response.retryAfter || 5; | ||
| core.info(`Job ${response.jobId} is ${response.status}, retrying in ${waitSeconds}s...`); | ||
| await new Promise(resolve => setTimeout(resolve, waitSeconds * 1000)); | ||
|
|
||
| response = await api.generateSupermodelGraph({ | ||
| idempotencyKey, | ||
| file: zipBlob, | ||
| }) as any; | ||
| } |
There was a problem hiding this comment.
Missing explicit check for 'completed' status after the polling loop exits.
So here's the thing - your while loop exits when status is NOT pending or processing. That means it could exit with:
completed✅ (what you want)failed✅ (you handle this on line 181)cancelled❓ (not handled)unknown_future_status❓ (not handled)
The code assumes "if not pending/processing/failed, we're good" - but that's a bit risky. If the API adds a new status like cancelled or expired, your code would silently treat it as success.
🔧 Suggested fix: Add explicit completed check
if (response.status === 'failed') {
throw new Error(`Graph generation failed: ${response.error || 'Unknown error'}`);
}
+ if (response.status !== 'completed') {
+ throw new Error(`Unexpected job status: ${response.status}`);
+ }🤖 Prompt for AI Agents
In `@src/index.ts` around lines 166 - 179, The polling loop exits on any
non-pending/processing status but doesn't verify success; update the code after
the loop (the block using response from api.generateSupermodelGraph with
idempotencyKey and zipBlob) to explicitly check that response.status ===
'completed' and treat any other status (e.g., 'failed', 'cancelled', or unknown
values) as an error: log/throw a clear error including response.status and
response.jobId (and any response.message) so failures/unknown statuses don't get
treated as successful; keep existing handling for 'failed' but ensure all
non-'completed' cases are handled consistently.
- Add explicit 'completed' status check after polling loop to guard against unexpected statuses (e.g. 'cancelled', 'expired') - Change integration tests to use generateSupermodelGraph (matches production code path) instead of generateCallGraph - Add polling timeout protection to test loops - Update idempotency key prefix from 'call' to 'supermodel'
|
Closing - SDK 0.6.0 will handle async polling internally, so no changes needed in dead-code-hunter beyond a version bump once the SDK is published. |
Summary
completedorfailedretryAfterfield for polling interval, with a 5-minute max timeoutresponse.result.graph(with fallback toresponse.graphfor backward compatibility)Test plan
npm run build)SUPERMODEL_API_KEYsetCloses #6
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.