Skip to content

fix: validate chunk_size and overlap to reject non-positive values#7789

Merged
alex-nork merged 1 commit into
mainfrom
swarm/chunk-vision-analysis/task-6
Feb 24, 2026
Merged

fix: validate chunk_size and overlap to reject non-positive values#7789
alex-nork merged 1 commit into
mainfrom
swarm/chunk-vision-analysis/task-6

Conversation

@alex-nork

@alex-nork alex-nork commented Feb 24, 2026

Copy link
Copy Markdown
Contributor

Addresses review feedback on #7787. Validates chunk_size >= 1 and overlap >= 0 at both the tool run() entry point and the core analyzeKeyframesForAsset() function, preventing empty-chunk behavior when chunk_size=0 passes through nullish coalescing.


Open with Devin

Co-Authored-By: Claude <noreply@anthropic.com>
@alex-nork alex-nork self-assigned this Feb 24, 2026
@alex-nork alex-nork merged commit 9a55b69 into main Feb 24, 2026
@alex-nork alex-nork deleted the swarm/chunk-vision-analysis/task-6 branch February 24, 2026 13:43

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 3 additional findings in Devin Review.

Open in Devin Review

Comment on lines +133 to +135
if (chunkSize !== undefined && chunkSize <= 0) {
throw new Error('chunk_size must be at least 1.');
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Validation in analyzeKeyframesForAsset uses <= 0 instead of < 1, allowing fractional chunkSize values that produce empty chunks

The validation at line 133 checks chunkSize <= 0, but the documented and intended minimum is 1 (matching the run() validation at line 289 which checks < 1, and the JSON schema which specifies minimum: 1). A fractional value like chunkSize = 0.5 passes the <= 0 check but causes buildChunks to produce empty chunks because keyframes.slice(i, i + 0.5) truncates to keyframes.slice(i, i) → an empty array.

Root Cause and Impact

analyzeKeyframesForAsset is an exported public function called both from run() and directly from assistant/src/memory/job-handlers/media-processing.ts:66. While current callers pass undefined (which defaults to 10), any future direct caller passing a fractional value between 0 and 1 (exclusive) would bypass validation and trigger empty-chunk processing.

The run() entry point correctly validates with chunkSizeInput < 1 at line 289, but the core function's own guard is weaker. Since the PR's stated goal is to "prevent empty-chunk behavior," the validation should be consistent: chunkSize < 1 instead of chunkSize <= 0.

Impact: Empty chunks passed to analyzeChunk() would send API requests with zero images, wasting API calls and potentially causing downstream errors.

Suggested change
if (chunkSize !== undefined && chunkSize <= 0) {
throw new Error('chunk_size must be at least 1.');
}
if (chunkSize !== undefined && chunkSize < 1) {
throw new Error('chunk_size must be at least 1.');
}
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant