Repository navigation
feat(video-analysis): add video-analysis support in neurolink - #824
Conversation
|
@rishikagudla is attempting to deploy a commit to the Sachin Sharma's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR adds end-to-end video analysis capabilities to NeuroLink, enabling automatic analysis of video files through Vertex AI and Gemini 2.0 Flash models. It includes frame extraction, intent-based feature selection, unified result formatting, core provider integration, comprehensive documentation, and usage examples. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client/SDK
participant BaseProvider as BaseProvider
participant VideoProcessor as VideoProcessor
participant VideoAnalyzer as VideoAnalyzer
participant Provider as AI Provider<br/>(Vertex AI/Gemini)
Client->>BaseProvider: generate(messages with video file)
BaseProvider->>VideoProcessor: extractFrames(video, interval)
VideoProcessor->>VideoProcessor: ffmpeg frame extraction
VideoProcessor-->>BaseProvider: frames[] with metadata
BaseProvider->>VideoAnalyzer: analyzeVideo(frames, options)
VideoAnalyzer->>VideoAnalyzer: buildConfig (standardized prompt)
VideoAnalyzer->>VideoAnalyzer: transformFramesToParts (base64/text)
VideoAnalyzer->>Provider: generateContent(message, config, model)
Provider-->>VideoAnalyzer: analysis result text
VideoAnalyzer-->>BaseProvider: analysis output
BaseProvider->>BaseProvider: merge into GenerateResult.content
BaseProvider-->>Client: GenerateResult with video analysis
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
374a64c to
596169c
Compare
|
@CodeRabbit please review my pr |
|
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/processors/media/VideoProcessor.ts (1)
725-765:⚠️ Potential issue | 🟠 MajorThe interval-based
selectfilter breaksextractFrameRangefor non-zerostartSec.The new select expression
isnan(prev_selected_t)+gte(t-prev_selected_t,${intervalSec}-0.001)always picks the first video frame (att≈0) and then selects everyintervalSecthereafter. ForextractKeyframes(full-video, starting at t=0) this is correct.However,
extractFrameRange(lines 1063-1143) targets a specific[startSec, endSec]window. With the sharedrunFfmpegFrameExtraction, frames will be selected starting at t=0, not atstartSec. For example, withstartSec=60, endSec=70, frameCount=5(interval ≈ 2.5s), the filter selects t=0, 2.5, 5, 7.5, 10 — entirely outside the desired range — then-frames:v 5stops output.Consider adding an
-ss startSecinput seek to the ffmpeg command whenstartSec > 0, or incorporating timestamp bounds into the select expression:Suggested approach: add seek and time-limited select
private runFfmpegFrameExtraction( videoPath: string, outputDir: string, timestamps: number[], intervalSec: number, + startSec: number = 0, ): Promise<void> { return new Promise((resolve, reject) => { - const selectExpr = `isnan(prev_selected_t)+gte(t-prev_selected_t,${intervalSec}-0.001)`; + // When startSec > 0, offset the select to skip early frames + const selectExpr = startSec > 0 + ? `gte(t,${startSec})*(isnan(prev_selected_t)+gte(t-prev_selected_t,${intervalSec}-0.001))` + : `isnan(prev_selected_t)+gte(t-prev_selected_t,${intervalSec}-0.001)`;Then pass
startSecfromextractFrameRange.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/processors/media/VideoProcessor.ts` around lines 725 - 765, The select filter currently picks frames relative to t=0 which breaks extractFrameRange; update runFfmpegFrameExtraction to accept a startSec (and optionally endSec) parameter and when startSec>0 add an input seek (-ss startSec) to ffmpegCommand (or alternatively incorporate time bounds into selectExpr using something like between(t, startSec, endSec) combined with your interval logic), then ensure extractFrameRange calls runFfmpegFrameExtraction with the startSec it computes so frames are sampled from the target window rather than from t≈0; keep the existing selectExpr behavior for startSec=0.
🧹 Nitpick comments (4)
examples/video-analysis.ts (2)
43-166: Consider wrappinggeneratecalls withwithTimeoututility.The coding guidelines for
*.tsfiles specify: "Wrap async operations with withTimeout utility." None of the sixneurolink.generate()calls are wrapped. Video analysis can be long-running, so timeouts would guard against indefinite hangs.As per coding guidelines,
**/*.ts: "Wrap async operations with withTimeout utility."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/video-analysis.ts` around lines 43 - 166, The six neurolink.generate calls (e.g., in Example 1..6 using neurolink.generate with VIDEO_PATH or fs.readFileSync) are not wrapped with the withTimeout utility; update each call to use withTimeout(...) so async operations will time out, e.g., wrap the Promise returned by neurolink.generate in withTimeout with an appropriate timeout value and handle timeout errors in the existing try/catch; ensure you replace each direct neurolink.generate invocation (including result1..result6) with the withTimeout-wrapped call and import or reference the withTimeout helper where it’s used.
162-166: A single failure aborts all remaining examples.The current try/catch wraps all six examples together, so if Example 2 fails, Examples 3–6 are skipped. For a demo script showcasing independent use cases, wrapping each example in its own try/catch would be more resilient and let users see results from the examples that do succeed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/video-analysis.ts` around lines 162 - 166, The top-level try/catch currently wrapping all six example runs causes one failure to abort the rest; refactor by enclosing each example invocation (the individual Example 1..6 call sites) in its own try { ... } catch (error) { console.error("\n❌ Example N failed:", error instanceof Error ? error.message : String(error)); } block, and remove or avoid calling process.exit(1) inside those per-example catch blocks so subsequent examples still run; keep the existing final process.exit only for a global fatal error if needed.src/lib/adapters/video/videoAnalyzer.ts (1)
116-125: No timeout onai.models.generateContent()— could hang indefinitely.The Gemini API call has no timeout protection. If the API is unresponsive,
generate()will block indefinitely. As per coding guidelines, async operations should be wrapped with a timeout utility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/adapters/video/videoAnalyzer.ts` around lines 116 - 125, The call to ai.models.generateContent in videoAnalyzer.ts can hang because it lacks a timeout; wrap the ai.models.generateContent(...) invocation in the project's timeout utility (e.g., withTimeout/runWithTimeout) so the operation is aborted after the configured limit, pass the same model/config/contents (buildConfig(), parts) into the timed call, and ensure you handle the timeout case by throwing/logging a clear error or returning a safe fallback from the enclosing function (video analysis flow) to avoid leaving the routine blocked.src/lib/utils/videoAnalysisProcessor.ts (1)
57-73: Duplicated provider-resolution logic diverges fromanalyzeVideodispatcher.Lines 57-65 pre-resolve the provider, then pass it to
analyzeVideo(videoAnalyzer.ts line 278), which has its own provider resolution. The two can diverge — e.g., hereGOOGLE_AIis selected whenprovider === AUTO && GOOGLE_AI_API_KEYexists, butanalyzeVideoroutesAUTOto Vertex AI unconditionally.Also, the
projectfield (line 69-71) is set toundefinedwhenoptions.regionis truthy, butgetVertexConfig()insideanalyzeVideoWithVertexAIre-reads it from env vars anyway, making this field effectively ignored.Consider removing the pre-resolution and passing
optionsthrough directly, lettinganalyzeVideoown the provider selection.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/utils/videoAnalysisProcessor.ts` around lines 57 - 73, The provider pre-resolution block (const provider = ...) and the custom project logic before calling analyzeVideo should be removed so analyzeVideo can own provider selection; instead pass the original options object (or at most options.provider) and options.model through to analyzeVideo unchanged. Specifically, delete the provider resolution and the conditional project/location override tied to options.region, and call analyzeVideo(messages[0], { ...options, model: options.model || "gemini-2.0-flash" }) so analyzeVideo (and its analyzeVideoWithVertexAI/getVertexConfig flow) uses a single source of truth for provider and project resolution.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/video-analysis.ts`:
- Around line 50-59: Fix the prompt string passed to neurolink.generate (the
input.text used when creating result1) to correct typographical issues: change
"whats" to "what's" and remove the double space before "it" (and optionally
clean up punctuation/capitalization for clarity) so the example prompt reads
polished and professional.
- Line 158: The header string logged by console.log("\n ERROR HANDLING REVIEW:")
is missing the emoji prefix used by other section headers; update that call
(console.log) to include a matching emoji prefix (e.g., "🐛 ERROR HANDLING
REVIEW:") so formatting is consistent with the other section headers like those
using "📋", "🕵️", "🔄", and "⚡".
- Around line 70-78: Fix the prompt typo and avoid synchronous full-file
buffering: update the prompt passed to neurolink.generate to replace "feedback
bond" with the intended phrase (e.g., "feedback" or "feedback loop") and replace
or annotate the fs.readFileSync(VIDEO_PATH) usage in the input.files for
neurolink.generate — either add a comment that synchronous Buffer loading is for
demonstration only or switch to a path-based approach (pass VIDEO_PATH as a
string or stream) to prevent loading large videos into memory; reference
neurolink.generate, VIDEO_PATH, and fs.readFileSync for the change.
In `@memory-bank/video-analysis-implementation-plan.md`:
- Around line 619-623: The example contains a duplicated call to
submitVideoAnalysis causing a redeclaration of const operation; remove the
redundant line so submitVideoAnalysis(request, config) is only called once and a
single const operation variable is declared (look for the two identical lines
referencing submitVideoAnalysis, const operation, request, and config and delete
the second occurrence).
- Around line 50-109: The document duplicates the "Architecture Overview" and
"Available analysis capabilities" sections (the block listing features like
`speech`, `ocr`, `labels`, `objects`, `shots`, `explicit`, `faces` and the
feature mapping table) and contains a typo "capabilitieVs"; remove the
duplicated block so only one canonical "Architecture Overview" and one
"Available analysis capabilities" list/table remain (keep the most complete
version under either the first occurrence or under "Core Components"), correct
the typo to "capabilities", and ensure the feature mapping table maps the
NeuroLink features (`speech`, `ocr`, `labels`, `objects`, `shots`, `explicit`,
`faces`) to Google Video Intelligence constants (`SPEECH_TRANSCRIPTION`,
`TEXT_DETECTION`, `LABEL_DETECTION`, `OBJECT_TRACKING`, `SHOT_CHANGE_DETECTION`,
`EXPLICIT_CONTENT_DETECTION`, `FACE_DETECTION`) so there's a single, consistent
place in the doc for this content.
In `@src/lib/adapters/video/videoAnalyzer.ts`:
- Around line 96-112: Extract the duplicate content-to-parts mapping used in
analyzeVideoWithVertexAI and analyzeVideoWithGeminiAPI into a single helper
(e.g., buildContentParts or convertFrameContentToParts) and replace both .map()
uses with that helper to remove duplication; in the helper, handle item types
"text" and "image" as before but explicitly detect Buffer or Uint8Array for
image payloads and convert them to base64 (fall back to validating strings and
throw a clear error if image data is neither string nor binary), ensure the
regex stripping of data URI prefixes still runs for string inputs, and preserve
the thrown Error for invalid item.type to keep existing validation.
- Around line 67-136: analyzeVideoWithVertexAI currently ignores options.project
and options.location because it always calls getVertexConfig(); update the
function to merge/override the config with the provided options by doing
something like: const config = await getVertexConfig(); const project =
options.project ?? config.project; const location = options.location ??
config.location; then use those merged values when constructing GoogleGenAI and
in the logger (keep options.model/default model logic unchanged); reference
analyzeVideoWithVertexAI and getVertexConfig to locate where to replace the
existing destructuring and ensure any logging reflects the final chosen
project/location.
- Around line 268-294: analyzeVideo currently routes AUTO to
analyzeVideoWithVertexAI unconditionally; change it to detect whether Vertex is
actually configured before calling analyzeVideoWithVertexAI and otherwise fall
back to Gemini when available. Specifically, in analyzeVideo check
AIProviderName.AUTO and call getVertexConfig() (or check the same condition used
inside getVertexConfig) to confirm Vertex credentials/config exist; if Vertex is
configured call analyzeVideoWithVertexAI(frames, options), otherwise if
process.env.GOOGLE_AI_API_KEY is present call analyzeVideoWithGeminiAPI(frames,
options), and only throw the final error if neither provider is available.
In `@src/lib/core/baseProvider.ts`:
- Around line 695-706: The current video-detection logic incorrectly triggers on
any image because hasVideoFrames checks for content parts with type === "image";
change the check to a more specific signal (e.g., require a video-specific
marker in message metadata or verify the message originated from
input.videoFiles / file-type video detection) so only video-extracted frames
trigger analysis (refer to hasVideoFrames and videoAnalysisProcessor.ts matching
logic). Also wrap the executeVideoAnalysis call with the withTimeout utility to
avoid hanging the generate() flow when Gemini is unresponsive (use withTimeout
around executeVideoAnalysis with an appropriate timeout value), and ensure
errors/timeouts are caught and handled/logged without replacing content when the
call fails.
- Around line 796-803: The current logic replaces the AI-generated content when
videoAnalysisResult is present, discarding executeGeneration output; update the
flow to either (A) add a new field videoAnalysis to the result type and payload
so enhancedResult keeps the original content and also includes videoAnalysis
(update GenerateResult in generateTypes.ts and set enhancedResult.videoAnalysis
= videoAnalysisResult), or (B) if video analysis should short-circuit
generation, move the videoAnalysisResult check before executeGeneration and
return early to avoid running the generation pipeline; modify the code around
executeGeneration, enhancedResult, and videoAnalysisResult accordingly to
implement one of these two behaviors.
In `@src/lib/types/fileTypes.ts`:
- Line 20: Remove the duplicate "video" literal from the FileType union
declaration so the union contains each file type only once; locate the FileType
type (the union of string literals that currently includes "video" twice) and
delete the redundant "video" entry to avoid confusion.
In `@src/lib/utils/videoAnalysisProcessor.ts`:
- Line 67: The current call to analyzeVideo only sends messages[0], dropping any
additional messages; update the logic in videoAnalysisProcessor so you collect
all relevant messages (e.g., filter messages for those containing
media/image/video fields or attachments) and either (a) call analyzeVideo with
the full array of filtered messages if analyzeVideo supports multiple inputs or
(b) iterate over the filtered messages and call analyzeVideo for each, then
merge/concatenate the results into videoAnalysisText. Ensure you update
references to analyzeVideo and the videoAnalysisText variable to handle an array
of inputs or aggregated results and preserve message order/context.
---
Outside diff comments:
In `@src/lib/processors/media/VideoProcessor.ts`:
- Around line 725-765: The select filter currently picks frames relative to t=0
which breaks extractFrameRange; update runFfmpegFrameExtraction to accept a
startSec (and optionally endSec) parameter and when startSec>0 add an input seek
(-ss startSec) to ffmpegCommand (or alternatively incorporate time bounds into
selectExpr using something like between(t, startSec, endSec) combined with your
interval logic), then ensure extractFrameRange calls runFfmpegFrameExtraction
with the startSec it computes so frames are sampled from the target window
rather than from t≈0; keep the existing selectExpr behavior for startSec=0.
---
Nitpick comments:
In `@examples/video-analysis.ts`:
- Around line 43-166: The six neurolink.generate calls (e.g., in Example 1..6
using neurolink.generate with VIDEO_PATH or fs.readFileSync) are not wrapped
with the withTimeout utility; update each call to use withTimeout(...) so async
operations will time out, e.g., wrap the Promise returned by neurolink.generate
in withTimeout with an appropriate timeout value and handle timeout errors in
the existing try/catch; ensure you replace each direct neurolink.generate
invocation (including result1..result6) with the withTimeout-wrapped call and
import or reference the withTimeout helper where it’s used.
- Around line 162-166: The top-level try/catch currently wrapping all six
example runs causes one failure to abort the rest; refactor by enclosing each
example invocation (the individual Example 1..6 call sites) in its own try { ...
} catch (error) { console.error("\n❌ Example N failed:", error instanceof Error
? error.message : String(error)); } block, and remove or avoid calling
process.exit(1) inside those per-example catch blocks so subsequent examples
still run; keep the existing final process.exit only for a global fatal error if
needed.
In `@src/lib/adapters/video/videoAnalyzer.ts`:
- Around line 116-125: The call to ai.models.generateContent in videoAnalyzer.ts
can hang because it lacks a timeout; wrap the ai.models.generateContent(...)
invocation in the project's timeout utility (e.g., withTimeout/runWithTimeout)
so the operation is aborted after the configured limit, pass the same
model/config/contents (buildConfig(), parts) into the timed call, and ensure you
handle the timeout case by throwing/logging a clear error or returning a safe
fallback from the enclosing function (video analysis flow) to avoid leaving the
routine blocked.
In `@src/lib/utils/videoAnalysisProcessor.ts`:
- Around line 57-73: The provider pre-resolution block (const provider = ...)
and the custom project logic before calling analyzeVideo should be removed so
analyzeVideo can own provider selection; instead pass the original options
object (or at most options.provider) and options.model through to analyzeVideo
unchanged. Specifically, delete the provider resolution and the conditional
project/location override tied to options.region, and call
analyzeVideo(messages[0], { ...options, model: options.model ||
"gemini-2.0-flash" }) so analyzeVideo (and its
analyzeVideoWithVertexAI/getVertexConfig flow) uses a single source of truth for
provider and project resolution.
596169c to
6bb932c
Compare
6bb932c to
9561315
Compare
|
🎉 This PR is included in version 9.9.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Pull Request
Description
What does this PR do?
Implements comprehensive video analysis support for NeuroLink using Gemini 2.0 Flash. This feature provides deep logical auditing of video sequences, focusing on the "Action-Reaction Chain" to identify silent failures, UI/UX bugs, and logical inconsistencies in recorded workflows.
Related Issues
Does this PR close any issues?
N/A - New feature implementation
Type of Change
Please select the type of change:
Motivation and Context
Why is this change needed? What problem does it solve?
Developers need to analyze screen recordings to debug UI/UX issues, identify workflow failures, and understand logical inconsistencies. Traditional video analysis focuses on visual description, but this implementation provides:
Use Cases:
Changes Made
What specific changes were made?
Core Implementation
src/lib/adapters/video/videoAnalyzer.tswith Gemini 2.0 Flash integration for both Vertex AI and Google AI providerssrc/lib/utils/videoAnalysisProcessor.tswith frame detection and analysis pipeline orchestrationsrc/lib/processors/media/VideoProcessor.tswith adaptive frame extraction using FFmpegsrc/lib/core/baseProvider.tsgeneration flowFrame Extraction Engine
prev_selected_tlogic in FFmpeg select filter for precise temporal frame distributionPrompting System
#,**) to ensure clean terminal outputDocumentation
docs/features/video-analysis.mdto match the new logic-first workflowExamples
examples/video-analysis.tswith 6 comprehensive examples:Breaking Changes
Does this PR introduce breaking changes?
This is an additive feature. Existing functionality remains unchanged. All video analysis is opt-in via providing video files in the input.
Testing
How has this been tested?
Test Coverage
Manual Testing Steps
Set up environment with credentials:
Test with example file:
Verify that all 6 examples execute successfully with detailed logical reports
Test CLI usage:
Confirm output contains:
Test Results:
result.content#or**markdown formatting)Code Quality
Have you followed code quality standards?
Documentation
Have you updated documentation?
docs/features/video-analysis.mdcompletely rewrittenexamples/video-analysis.tsrewritten with 6 comprehensive examplesCommit Message Format
Does your commit follow semantic commit conventions?
type(scope): descriptionSuggested commit message:
Dependencies
Does this PR add, update, or remove dependencies?
All required dependencies were already present:
fluent-ffmpeg- Already in package.json for video processing@google-cloud/vertexai- Already in package.json for Vertex AI@google/generative-ai- Already in package.json for Google AI StudioExternal dependency:
Performance Impact
Does this change affect performance?
Frame Extraction Performance:
Analysis Time (end-to-end):
Performance primarily depends on Gemini 2.0 Flash API latency.
Frame Extraction Speed:
Security Considerations
Are there any security implications?
Security Notes:
Deployment Notes
Special deployment instructions?
Required Environment Variables:
For Vertex AI:
For Google AI Studio:
System Requirements:
brew install ffmpegapt-get install ffmpegoryum install ffmpegScreenshots / Videos
If applicable, add screenshots or videos to demonstrate changes:
Example CLI Output:
Reviewer Checklist
For reviewers:
Additional Notes
Any additional information for reviewers:
Key Implementation Details:
FFmpeg Integration Deep Dive:
select='not(mod(n\,${frameInterval}))'- extracted same frameselect='isnan(prev_selected_t)+gte(t-prev_selected_t,${intervalSec}-0.001)'Provider Support:
gemini-2.0-flashmodel (vision-capable)Prompt Engineering Philosophy:
Terminal Compatibility:
# Headersand**bold**which broke terminal outputWhy Text Output Instead of Structured JSON?
VideoAnalysisResulttype with structured fieldsFuture Enhancement Ideas:
Known Limitations:
Questions for Reviewers:
--framesCLI option to override frame count?Pre-submission Checklist
Before submitting, ensure you have:
pnpm test- Manual testing completed, unit tests pendingpnpm buildpnpm run validate:alland all checks pass - To be run before mergeThank you for contributing to NeuroLink!
Summary by CodeRabbit
New Features
filesparameter to input options for automatic file type detection, including video files.Documentation