updated search filters and get dataconnectors api#86
Merged
Conversation
📝 WalkthroughWalkthroughThe OpenAPI specification is updated from version 0.6.3 to 0.6.4, introducing a new data connectors API endpoint with corresponding schemas, enhancing video info filtering with scope awareness, and extending segmentation query parameters with duration constraints and validation rules. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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
🧹 Nitpick comments (1)
spec/openapi.json (1)
767-771: Extract the repeatedfilterquery parameter into a shared component.The long filter contract text is duplicated in two operations, which will drift over time. Define it once under
components.parametersand$refit from both endpoints.♻️ Proposed refactor
+ "components": { + "parameters": { + "SearchFilterQueryParam": { + "name": "filter", + "in": "query", + "description": "JSON string containing filter criteria to constrain file search results. ...", + "required": false, + "schema": { "type": "string" } + } + }, + ... + } "/files": { "get": { "parameters": [ ... - { "name": "filter", "in": "query", ... } + { "$ref": "#/components/parameters/SearchFilterQueryParam" } ] } }, "/collections/{collection_id}/videos": { "get": { "parameters": [ ... - { "name": "filter", "in": "query", ... } + { "$ref": "#/components/parameters/SearchFilterQueryParam" } ] } }Also applies to: 2234-2238
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/openapi.json` around lines 767 - 771, Create a single reusable parameter under components.parameters (e.g., name it "Filter" or "filter") that contains the long description and schema { type: "string" } and required: false; then remove the duplicated inline parameter definitions in the two operations and replace them with a $ref to "#/components/parameters/Filter" so both endpoints share the same definition (update any occurrences that match the long "filter" query parameter to use components.parameters.Filter).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@spec/openapi.json`:
- Around line 8442-8456: The schema for the video filter items currently allows
incompatible combinations of "path" and "scope" (e.g.,
{"path":"has_audio","scope":"segment"}); update the JSON schema where the items
object defines "path" and "scope" enums to add conditional validation (oneOf or
if/then) that enforces valid pairs: require that if path == "has_audio" then
scope == "file", and if scope == "segment" then path == "duration_seconds"
(while preserving the existing enums and default for scope); apply the same
conditional schema to the other identical items block that defines the video
filter schema so invalid combinations fail contract validation early.
- Around line 3957-3984: The GET /data-connectors operation (operationId
listDataConnectors) returns the paginated schema DataConnectorList but has no
pagination query parameters; either add optional query parameters "limit" and
"offset" to the operation's parameters list (with appropriate types/integer
format and descriptions) so callers can control pagination, or change the
response schema to a non-paginated list type; update the OpenAPI path definition
for listDataConnectors to include the new parameters (or replace the response
$ref) and ensure the components schema reference (DataConnectorList) remains
consistent with the chosen approach.
---
Nitpick comments:
In `@spec/openapi.json`:
- Around line 767-771: Create a single reusable parameter under
components.parameters (e.g., name it "Filter" or "filter") that contains the
long description and schema { type: "string" } and required: false; then remove
the duplicated inline parameter definitions in the two operations and replace
them with a $ref to "#/components/parameters/Filter" so both endpoints share the
same definition (update any occurrences that match the long "filter" query
parameter to use components.parameters.Filter).
Comment on lines
+3957
to
+3984
| "get": { | ||
| "tags": ["Data Connectors"], | ||
| "summary": "List all data connectors", | ||
| "operationId": "listDataConnectors", | ||
| "description": "List all active data connectors configured for your account", | ||
| "responses": { | ||
| "200": { | ||
| "description": "A list of data connectors", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "$ref": "#/components/schemas/DataConnectorList" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "500": { | ||
| "description": "An unexpected error occurred on the server", | ||
| "content": { | ||
| "application/json": { | ||
| "schema": { | ||
| "$ref": "#/components/schemas/Error" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
/data-connectors uses a paginated response shape but exposes no pagination inputs.
DataConnectorList requires limit and offset, but this operation has no corresponding query params. Either add limit/offset inputs or switch to a non-paginated response schema.
🛠️ Proposed fix (add pagination params)
"/data-connectors": {
"get": {
"tags": ["Data Connectors"],
"summary": "List all data connectors",
"operationId": "listDataConnectors",
"description": "List all active data connectors configured for your account",
+ "parameters": [
+ {
+ "name": "limit",
+ "in": "query",
+ "description": "Maximum number of data connectors to return",
+ "required": false,
+ "schema": {
+ "type": "integer",
+ "minimum": 1,
+ "maximum": 100,
+ "default": 50
+ }
+ },
+ {
+ "name": "offset",
+ "in": "query",
+ "description": "Number of data connectors to skip",
+ "required": false,
+ "schema": {
+ "type": "integer",
+ "minimum": 0,
+ "default": 0
+ }
+ }
+ ],
"responses": {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@spec/openapi.json` around lines 3957 - 3984, The GET /data-connectors
operation (operationId listDataConnectors) returns the paginated schema
DataConnectorList but has no pagination query parameters; either add optional
query parameters "limit" and "offset" to the operation's parameters list (with
appropriate types/integer format and descriptions) so callers can control
pagination, or change the response schema to a non-paginated list type; update
the OpenAPI path definition for listDataConnectors to include the new parameters
(or replace the response $ref) and ensure the components schema reference
(DataConnectorList) remains consistent with the chosen approach.
Comment on lines
+8442
to
+8456
| "description": "Filter by video information. Use scope 'file' for source video properties or 'segment' for segment properties (e.g. segment duration).", | ||
| "items": { | ||
| "type": "object", | ||
| "properties": { | ||
| "path": { | ||
| "type": "string", | ||
| "enum": ["duration_seconds", "has_audio"], | ||
| "description": "JSON path on video_info object (e.g. 'has_audio', 'duration_seconds')" | ||
| }, | ||
| "scope": { | ||
| "type": "string", | ||
| "enum": ["file", "segment"], | ||
| "default": "file", | ||
| "description": "Scope of the filter. 'file' filters by source video properties, 'segment' filters by segment properties. Only duration_seconds is supported with segment scope." | ||
| }, |
There was a problem hiding this comment.
scope/path compatibility is documented but not enforced by schema.
Line 8455 and Line 10655 state has_audio is file-scope only, but the schema still allows { "scope": "segment", "path": "has_audio" }. Add conditional validation (oneOf) so invalid combinations fail contract validation early.
Also applies to: 10638-10656
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@spec/openapi.json` around lines 8442 - 8456, The schema for the video filter
items currently allows incompatible combinations of "path" and "scope" (e.g.,
{"path":"has_audio","scope":"segment"}); update the JSON schema where the items
object defines "path" and "scope" enums to add conditional validation (oneOf or
if/then) that enforces valid pairs: require that if path == "has_audio" then
scope == "file", and if scope == "segment" then path == "duration_seconds"
(while preserving the existing enums and default for scope); apply the same
conditional schema to the other identical items block that defines the video
filter schema so invalid combinations fail contract validation early.
amyxst
approved these changes
Mar 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
updates
Summary by CodeRabbit