Add analyze_footage agent tool for TwelveLabs Pegasus visual queries - #2
Add analyze_footage agent tool for TwelveLabs Pegasus visual queries#2mohit-twelvelabs wants to merge 1 commit into
Conversation
…ries Lets the chat assistant ask TwelveLabs Pegasus a natural-language question about the visual content of an already-indexed clip (scenes, objects, on-screen action, or locating the moment that matches a description) before planning timeline edits. Reuses the videoId captured during upload indexing and is exposed as an opt-in tool that skips gracefully when no analyzer is configured, so default behavior is unchanged. Surfaces twelveLabsVideoId through get_library_assets_data so the agent can route visual questions to analyze_footage and transcript questions to investigate_transcription. Includes no-network unit tests.
Greptile SummaryThis PR adds an
Confidence Score: 3/5Safe to merge for deployments that already have 12LABS_API_KEY configured; deployments without the key will receive error results from the tool instead of the silent skip the PR description promises. The opt-in design intent is broken: footageAnalyzer is unconditionally assigned in createToolDependencies, so when 12LABS_API_KEY is absent the tool errors rather than skips. Additionally, TwelveLabsService is now instantiated twice, creating two separate instances with independent caches. Neither issue is a crash or data-loss scenario, but both represent real behavioural gaps relative to what the PR claims. tools.service.ts and tools.module.ts — the first contains the always-set footageAnalyzer that bypasses the skip path, and the second introduces the duplicate service registration. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Agent as AI Agent
participant GLT as get_library_assets_data
participant AFT as analyze_footage tool
participant TLS as TwelveLabsService
participant TL as TwelveLabs API (Pegasus)
Agent->>GLT: call (no args)
GLT-->>Agent: asset list with twelveLabsVideoId
Agent->>AFT: call(videoId, prompt)
AFT->>TLS: "analyzeVideo({videoId, prompt})"
alt API key configured
TLS->>TL: "client.analyze({videoId, prompt})"
TL-->>TLS: response.data
TLS-->>AFT: answer string
AFT-->>Agent: status completed with answer
else API key missing
TLS--xAFT: throws Missing 12LABS_API_KEY
AFT-->>Agent: status error with error message
end
Note over AFT: footageAnalyzer always set so skip path unreachable
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Agent as AI Agent
participant GLT as get_library_assets_data
participant AFT as analyze_footage tool
participant TLS as TwelveLabsService
participant TL as TwelveLabs API (Pegasus)
Agent->>GLT: call (no args)
GLT-->>Agent: asset list with twelveLabsVideoId
Agent->>AFT: call(videoId, prompt)
AFT->>TLS: "analyzeVideo({videoId, prompt})"
alt API key configured
TLS->>TL: "client.analyze({videoId, prompt})"
TL-->>TLS: response.data
TLS-->>AFT: answer string
AFT-->>Agent: status completed with answer
else API key missing
TLS--xAFT: throws Missing 12LABS_API_KEY
AFT-->>Agent: status error with error message
end
Note over AFT: footageAnalyzer always set so skip path unreachable
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
apps/server/src/ai-gateway/tools/tools.service.ts:120-122
**`footageAnalyzer` is always set — the "skip" path is unreachable in production**
`footageAnalyzer` is unconditionally assigned in `createToolDependencies`, so `deps.footageAnalyzer` in `analyze-footage.tools.ts` is never `undefined`. When `12LABS_API_KEY` is absent, `getClient()` throws `Error('Missing 12LABS_API_KEY')`, which the tool catches and returns as `status: 'error'` — not the `status: 'skipped'` the PR description promises. The "no new env vars / opt-in skip" guarantee stated in the PR is not upheld: any deployment without the key will see error results from this tool rather than silent skips.
### Issue 2 of 3
apps/server/src/ai-gateway/tools/tool-creators/analyze-footage.tools.ts:86
**`reason` (the agent's calling justification) leaked back as the result `note`**
`reason` is the LLM's internal rationale for invoking the tool — not a meaningful description of what the tool found. Using it as `note` in the result means the model's calling justification is reflected verbatim in the tool's output, which the model will then read as context. None of the other tools in this codebase do this. The constant fallback `'Footage analysis completed successfully.'` is the right default.
```suggestion
note: 'Footage analysis completed successfully.'
```
### Issue 3 of 3
apps/server/src/ai-gateway/tools/tools.module.ts:11
**Duplicate `TwelveLabsService` instances across modules**
`TwelveLabsService` is already registered as a provider in `AppModule` (for `VideoAnalysisService` and `UploadService`). Registering it again here creates a second independent instance with its own `client`, `cachedIndexIds`, and `cachedConfiguredIndexId`. For the current `analyzeVideo` call this is harmless, but if the service ever accumulates state (rate-limit counters, connection pools, etc.) the two instances won't share it. Consider moving `TwelveLabsService` into a dedicated module that both `AppModule` and `ToolsModule` can import.
Reviews (1): Last reviewed commit: "Add analyze_footage AI Gateway tool for ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| footageAnalyzer: { | ||
| analyzeVideo: (args) => this.twelveLabsService.analyzeVideo(args), | ||
| }, |
There was a problem hiding this comment.
footageAnalyzer is always set — the "skip" path is unreachable in production
footageAnalyzer is unconditionally assigned in createToolDependencies, so deps.footageAnalyzer in analyze-footage.tools.ts is never undefined. When 12LABS_API_KEY is absent, getClient() throws Error('Missing 12LABS_API_KEY'), which the tool catches and returns as status: 'error' — not the status: 'skipped' the PR description promises. The "no new env vars / opt-in skip" guarantee stated in the PR is not upheld: any deployment without the key will see error results from this tool rather than silent skips.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/server/src/ai-gateway/tools/tools.service.ts
Line: 120-122
Comment:
**`footageAnalyzer` is always set — the "skip" path is unreachable in production**
`footageAnalyzer` is unconditionally assigned in `createToolDependencies`, so `deps.footageAnalyzer` in `analyze-footage.tools.ts` is never `undefined`. When `12LABS_API_KEY` is absent, `getClient()` throws `Error('Missing 12LABS_API_KEY')`, which the tool catches and returns as `status: 'error'` — not the `status: 'skipped'` the PR description promises. The "no new env vars / opt-in skip" guarantee stated in the PR is not upheld: any deployment without the key will see error results from this tool rather than silent skips.
How can I resolve this? If you propose a fix, please make it concise.| status: 'completed', | ||
| videoId, | ||
| answer: trimmed.slice(0, MAX_ANSWER_CHARACTERS), | ||
| note: reason ?? 'Footage analysis completed successfully.', |
There was a problem hiding this comment.
reason (the agent's calling justification) leaked back as the result note
reason is the LLM's internal rationale for invoking the tool — not a meaningful description of what the tool found. Using it as note in the result means the model's calling justification is reflected verbatim in the tool's output, which the model will then read as context. None of the other tools in this codebase do this. The constant fallback 'Footage analysis completed successfully.' is the right default.
| note: reason ?? 'Footage analysis completed successfully.', | |
| note: 'Footage analysis completed successfully.' |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/server/src/ai-gateway/tools/tool-creators/analyze-footage.tools.ts
Line: 86
Comment:
**`reason` (the agent's calling justification) leaked back as the result `note`**
`reason` is the LLM's internal rationale for invoking the tool — not a meaningful description of what the tool found. Using it as `note` in the result means the model's calling justification is reflected verbatim in the tool's output, which the model will then read as context. None of the other tools in this codebase do this. The constant fallback `'Footage analysis completed successfully.'` is the right default.
```suggestion
note: 'Footage analysis completed successfully.'
```
How can I resolve this? If you propose a fix, please make it concise.| imports: [RealtimeModule, AudioModule], | ||
| controllers: [ToolsController], | ||
| providers: [ToolsService], | ||
| providers: [ToolsService, TwelveLabsService], |
There was a problem hiding this comment.
TwelveLabsService instances across modules
TwelveLabsService is already registered as a provider in AppModule (for VideoAnalysisService and UploadService). Registering it again here creates a second independent instance with its own client, cachedIndexIds, and cachedConfiguredIndexId. For the current analyzeVideo call this is harmless, but if the service ever accumulates state (rate-limit counters, connection pools, etc.) the two instances won't share it. Consider moving TwelveLabsService into a dedicated module that both AppModule and ToolsModule can import.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/server/src/ai-gateway/tools/tools.module.ts
Line: 11
Comment:
**Duplicate `TwelveLabsService` instances across modules**
`TwelveLabsService` is already registered as a provider in `AppModule` (for `VideoAnalysisService` and `UploadService`). Registering it again here creates a second independent instance with its own `client`, `cachedIndexIds`, and `cachedConfiguredIndexId`. For the current `analyzeVideo` call this is harmless, but if the service ever accumulates state (rate-limit counters, connection pools, etc.) the two instances won't share it. Consider moving `TwelveLabsService` into a dedicated module that both `AppModule` and `ToolsModule` can import.
How can I resolve this? If you propose a fix, please make it concise.1 participant
Hi! I'm Mohit, I work at TwelveLabs (@mohit-twelvelabs).
What this adds
A new opt-in AI Gateway tool,
analyze_footage, that lets the chat assistant ask TwelveLabs Pegasus a natural-language question about the visual content of an already-indexed clip — scenes, objects, people, on-screen action, on-screen text, or finding the moment that matches a description — before it plans timeline edits.Framedeck already indexes every uploaded video in TwelveLabs and captures a one-shot summary at upload. This tool lets the agent run a fresh, focused visual question against that footage at edit time, so prompts like "find the drone shot and cut to it" or "where does the speaker pick up the product?" can be answered visually instead of relying only on the transcript.
It complements the existing
investigate_transcriptiontool: visual questions route toanalyze_footage, spoken-word questions toinvestigate_transcription. To make that routing work,get_library_assets_datanow surfaces thetwelveLabsVideoIdfor each indexed asset.How it works
TwelveLabsService.analyzeVideo(Pegasus) and thevideoIdalready captured during upload indexing — no new pipeline, no new env vars beyond the existing12LABS_API_KEY.ToolsServicethe same way as the other tools (tool name intool-names.ts, creator intool-creators/, structuralFootageAnalyzerdependency to keep the tool-creators decoupled from the Nest module).Opt-in / non-breaking
The tool is additive. The analyzer is injected as an optional dependency: if it isn't configured the tool returns a
skippedresult and the assistant carries on. No defaults change and no existing tool or pipeline is touched.How it was tested
pnpm exec jest src/ai-gateway/tools/tool-creators— all 46 tests across 11 suites pass, including the newanalyze-footage.tools.spec.ts(no-network unit tests covering the skip, success, and error paths).pnpm exec tsc --noEmitonapps/server— clean.client.analyzeis present and a Marengomarengo3.0text embedding returns a 512-dim vector against the live API. The end-to-end Pegasus call against real footage is server-side and slow, so the tool's request wiring is unit-tested and the SDK path is confirmed; a full footage run is pending a real uploaded asset.You can grab a free API key at https://twelvelabs.io — there's a generous free tier.