fix(vapi): add cursor-based pagination to propDefinitions#21135
Conversation
The phoneNumberId, assistantId, and squadId async options previously fetched only a single page. Add createdAtLt cursor pagination using the prevContext pattern so accounts with >1000 resources can load more. Closes PipedreamHQ#21085
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
There was a problem hiding this comment.
Hi @favzqn, thank you for you contribution! Since there are changes in the vapi.app.mjs file, the corresponding actions need their versions to be updated for them to be reviewed and tested properly. Can you resolve conflicts and update the package versions and do a round of testing for the list-* actions ?
Sharing our contribution guidelines for your reference: https://pipedream.com/docs/components/contributing/guidelines
Also, since the return shape has changed, the caller sites need to be modified to utilise the changed options return change aswell.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughCursor-based ChangesVapi Pagination and Version Updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/vapi/actions/list-phone-number-id-options/list-phone-number-id-options.mjs (1)
23-33: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winSame return-shape mismatch as the other list-*-options actions.
vapi.propDefinitions.phoneNumberId.optionsnow returns{ options, context }(components/vapi/vapi.app.mjslines 83-97), but this action treats the whole return value asoptions.options?.lengthis alwaysundefined→$summaryalways reports 0, andreturn optionsreturns the wrapper object instead of the array of{ label, value }entries. Thecontext.createdAtLtcursor for "load more" is also lost.🐛 Proposed fix
- const options = await vapi.propDefinitions.phoneNumberId.options.call(this.vapi, { + const { options, context } = await vapi.propDefinitions.phoneNumberId.options.call(this.vapi, { prevContext: { createdAtLt: this.createdAtLt, }, }); - $.export("$summary", `Successfully retrieved ${options?.length ?? 0} option${options?.length === 1 - ? "" - : "s"}`); + $.export("$summary", `Successfully retrieved ${options?.length ?? 0} option${options?.length === 1 + ? "" + : "s"}${context?.createdAtLt + ? `. Pass createdAtLt="${context.createdAtLt}" to load the next page.` + : ""}`); return options;🤖 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 `@components/vapi/actions/list-phone-number-id-options/list-phone-number-id-options.mjs` around lines 23 - 33, The action in list-phone-number-id-options.mjs is treating vapi.propDefinitions.phoneNumberId.options as the final array, but it now returns a wrapper object with options and context. Update run() to destructure the result, use the options array for the $.summary count, return the options array instead of the wrapper, and preserve the returned context (including createdAtLt) so the load-more cursor continues to work.components/vapi/actions/list-squad-id-options/list-squad-id-options.mjs (1)
23-33: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winSame return-shape mismatch as the other list-*-options actions.
vapi.propDefinitions.squadId.optionsnow returns{ options, context }(components/vapi/vapi.app.mjslines 53-65), but this action assigns the whole return value tooptions.options?.lengthis alwaysundefined→$summaryalways reports 0, andreturn optionsreturns the wrapper object rather than the array of{ label, value }entries. Thecontext.createdAtLtcursor is also lost, so pagination cannot actually be driven from this action.🐛 Proposed fix
- const options = await vapi.propDefinitions.squadId.options.call(this.vapi, { + const { options, context } = await vapi.propDefinitions.squadId.options.call(this.vapi, { prevContext: { createdAtLt: this.createdAtLt, }, }); - $.export("$summary", `Successfully retrieved ${options?.length ?? 0} option${options?.length === 1 - ? "" - : "s"}`); + $.export("$summary", `Successfully retrieved ${options?.length ?? 0} option${options?.length === 1 + ? "" + : "s"}${context?.createdAtLt + ? `. Pass createdAtLt="${context.createdAtLt}" to load the next page.` + : ""}`); return options;🤖 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 `@components/vapi/actions/list-squad-id-options/list-squad-id-options.mjs` around lines 23 - 33, The list-squad-id-options action is handling the new return shape incorrectly in run(); vapi.propDefinitions.squadId.options now returns an object with options and context, not the array directly. Update run() to destructure the response from vapi.propDefinitions.squadId.options, use the options array for $.export("$summary") and the return value, and propagate the returned context so the createdAtLt cursor can be preserved for pagination.
🤖 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
`@components/vapi/actions/list-assistant-id-options/list-assistant-id-options.mjs`:
- Around line 23-33: The `run` method in `list-assistant-id-options.mjs` is
treating the `vapi.propDefinitions.assistantId.options` result as the options
array, but it now returns a `{ options, context }` wrapper. Destructure the
returned value in `run` so you use the inner `options` array for
`$.export("$summary")` and `return`, and also preserve `context` by exposing the
pagination cursor (for example via an exported output or summary) so downstream
consumers still get the flat `{ label, value }` list and can continue loading
more using `createdAtLt`.
In `@components/vapi/vapi.app.mjs`:
- Around line 12-36: The cursor-based pagination logic is duplicated in the
options methods for assistantId, squadId, and phoneNumberId; extract it into a
shared private helper in vapi.app.mjs. Move the repeated
prevContext?.createdAtLt handling, params.createdAtLt assignment,
listAssistants-style fetch, and context.createdAtLt calculation based on
lastItem?.createdAt and LIMIT into a helper such as _paginatedOptions(fetchFn,
prevContext, mapFn), then have each propDefinition’s options() delegate to it
and keep only the field-specific mapping.
---
Outside diff comments:
In
`@components/vapi/actions/list-phone-number-id-options/list-phone-number-id-options.mjs`:
- Around line 23-33: The action in list-phone-number-id-options.mjs is treating
vapi.propDefinitions.phoneNumberId.options as the final array, but it now
returns a wrapper object with options and context. Update run() to destructure
the result, use the options array for the $.summary count, return the options
array instead of the wrapper, and preserve the returned context (including
createdAtLt) so the load-more cursor continues to work.
In `@components/vapi/actions/list-squad-id-options/list-squad-id-options.mjs`:
- Around line 23-33: The list-squad-id-options action is handling the new return
shape incorrectly in run(); vapi.propDefinitions.squadId.options now returns an
object with options and context, not the array directly. Update run() to
destructure the response from vapi.propDefinitions.squadId.options, use the
options array for $.export("$summary") and the return value, and propagate the
returned context so the createdAtLt cursor can be preserved for pagination.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: dd0d7223-b94e-4196-ae5e-9c75d3215dad
📒 Files selected for processing (9)
components/vapi/actions/create-call/create-call.mjscomponents/vapi/actions/list-assistant-id-options/list-assistant-id-options.mjscomponents/vapi/actions/list-phone-number-id-options/list-phone-number-id-options.mjscomponents/vapi/actions/list-squad-id-options/list-squad-id-options.mjscomponents/vapi/actions/update-assistant-settings/update-assistant-settings.mjscomponents/vapi/actions/upload-file/upload-file.mjscomponents/vapi/package.jsoncomponents/vapi/sources/new-conversation/new-conversation.mjscomponents/vapi/vapi.app.mjs
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@components/vapi/vapi.app.mjs`:
- Around line 63-84: The _paginatedOptions helper in vapi.app.mjs manually
guards params.createdAtLt with an if check, but this optional value should be
passed through directly. Update _paginatedOptions to build the request params
object with createdAtLt assigned from prevContext?.createdAtLt without a
conditional, relying on the axios helper to strip undefined values
automatically; keep the rest of the pagination logic unchanged.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e37eeab5-1a8f-45d8-8336-d473f0661087
📒 Files selected for processing (4)
components/vapi/actions/list-assistant-id-options/list-assistant-id-options.mjscomponents/vapi/actions/list-phone-number-id-options/list-phone-number-id-options.mjscomponents/vapi/actions/list-squad-id-options/list-squad-id-options.mjscomponents/vapi/vapi.app.mjs
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Eval results —
|
| # | Eval | Category | Targeted tool | Calls | Time | Result |
|---|---|---|---|---|---|---|
| 1 | List available assistant options | read | list-assistant-id-options |
1 | 7.2s | ✅ |
| 2 | List available squad options | read | list-squad-id-options |
1 | 6.2s | ✅ |
| 3 | List available phone number options | read | list-phone-number-id-options |
1 | 6.8s | ✅ |
Each action returned the new { options, context } shape correctly. Sample evidence from the run:
- Adding Mediums feed of updates. #1 returned 41 assistants (agent found: "Cosmo Kramer Receptionist")
- Remove JSON.parse from default http component #2 returned 2 squads including "Vandelay Industries Support Team"
- Improving readme #3 returned 1 phone number
The other files in the PR (create-call, update-assistant-settings, upload-file, sources/new-conversation) are version-bump-only; the framework's filterToMeaningfulFiles correctly narrowed the eval scope to the three actions with real behavior changes.
Closes #21085.
The three propDefinitions (phoneNumberId, assistantId, squadId) previously fetched only a single page of results. This adds
createdAtLtcursor pagination using theprevContextpattern so accounts with >1000 resources can load more results in the dropdown.Changes
vapi.app.mjs: Eachasync options()now acceptsprevContext, passescreatedAtLtas a query param for the next page, and returns{ options, context }to support load-morepackage.json: Version bump0.3.0->0.3.1Validation
pnpm eslint components/vapi— passes cleannode --check— passesnode scripts/build-components.mjs components/vapi— passesSummary by CodeRabbit