Skip to content

Conversation

@wbrennan899
Copy link
Contributor

@wbrennan899 wbrennan899 commented Jul 3, 2025

Description

Added two new video analysis tools using TwelveLabs' AI models:

  1. search_video: Semantic video search using TwelveLabs' Marengo model for natural language queries against video content
  2. chat_video: Interactive video analysis using TwelveLabs' Pegasus model for Q&A about video content

Both tools support multi-modal analysis (visual and audio), with chat_video offering video upload capabilities and caching.

Related Issues

Documentation PR

Documentation PR not made

Type of Change

  • Bug fix
  • New Tool
  • Breaking change
  • Other (please describe):

Testing

Comprehensive testing implemented for both video tools:

  • Added test suites covering success cases, error handling, and edge cases
  • Mock TwelveLabs API interactions for reliable testing
  • Video upload caching functionality tested
  • All video tool tests pass consistently (20/20)
  • hatch fmt --linter ✅ (line length issues resolved)
  • hatch fmt --formatter
  • hatch test --all ✅ (All tests passing - 20/20 video tool tests now pass)

Checklist

  • I have read the CONTRIBUTING document

  • I have added tests that prove my fix is effective or my feature works

  • I have updated the documentation accordingly

  • I have added an appropriate example to the documentation to outline the feature

  • My changes generate no new warnings

  • Any dependent changes have been merged and published

  • By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@wbrennan899 wbrennan899 requested a review from a team as a code owner July 3, 2025 21:39
@jer96
Copy link
Contributor

jer96 commented Jul 9, 2025

hi @wbrennan899 thank you for the contribution. can you please rebase the PR? also regarding testing do you have any details you can share about testing outside of unit tests that you've done?

@jer96
Copy link
Contributor

jer96 commented Jul 11, 2025

hi @wbrennan899 following up once more as i've been able to pull down the code and run a couple tests.

i can't seem to get the basic functionality working when following the instructions in this PR.

for example:

(with TWELVELABS_API_KEY set to a valid api key)

import uuid

from strands import Agent

from strands_tools import chat_video

agent = Agent(tools=[chat_video])
result = agent.tool.chat_video(
    prompt="Describe what happens in this video",
    video_path="/Users/jerebill/Desktop/test.mp4",
    index_id=str(uuid.uuid4()),
)
print(result)
{'toolUseId': 'tooluse_chat_video_674640038', 'status': 'error', 'content': [{'text': "Error chatting with video: Error code: 400 - {'code': 'parameter_invalid', 'message': 'The index_id parameter is invalid. 6c587d27-1355-4d60-a32f-721f65f4c67e is not a valid id type'}\n\nMake sure the index_id is valid and you have access to it. For video uploads, index_id is required."}]}

i've also tested with the index_id as a fixed string and the same error arises. can you please advise on the correct way to interact with the twelve labs API? i believe the tool will require modification to function correctly.

@wbrennan899
Copy link
Contributor Author

@jer96 Your example looks good. You need to create a pegasus index on the twelve labs dashboard (https://playground.twelvelabs.io/indexes) and use that as your index_id.

I have a python script I used to test all the functionality. Would you like me to add that in a comment?

@jer96
Copy link
Contributor

jer96 commented Aug 6, 2025

hey @wbrennan899 sorry for the delay here, i've confirmed everything is working as expected. it may be worth dropping your test script in the just for documentation purposes. otherwise, the PR looks good, it just needs to be rebased (i tried to do this myself but i don't have access to the forked repo). thank you for the contribution

@james-le-twelve-labs
Copy link

@jer96 The PR has been rebased as seen in @wbrennan899's recent commits. Would be great if you can merge the PR by early next week!

jer96
jer96 previously approved these changes Aug 12, 2025
@jer96
Copy link
Contributor

jer96 commented Aug 12, 2025

hi @james-le-twelve-labs @wbrennan899 thank you for rebasing and apologies for the delay. i'm happy to merge once the unit tests are passing: https://github.com/strands-agents/tools/actions/runs/16815620070/job/47912920461?pr=120

@james-le-twelve-labs
Copy link

@jer96 All tests are now passing on our side. Can you approve the workflow to run them on GitHub?

@james-le-twelve-labs
Copy link

@jer96 Checking in here. Can you provide an update on the approval status for @wbrennan899 PR?

@cagataycali
Copy link
Member

Hi James, looks like one test failing:

FAILED tests/test_chat_video.py::TestChatVideoTool::test_chat_upload_failure - AssertionError: assert 'success' == 'error'

https://github.com/strands-agents/tools/actions/runs/17051069618/job/49424605102?pr=120

@james-le-twelve-labs
Copy link

Hi James, looks like one test failing:

FAILED tests/test_chat_video.py::TestChatVideoTool::test_chat_upload_failure - AssertionError: assert 'success' == 'error'

https://github.com/strands-agents/tools/actions/runs/17051069618/job/49424605102?pr=120

@cagataycali All tests were passing locally for us. @wbrennan899 made a change that should fix the issue when you run the CI tests. Please confirm if things work on your end.

@james-le-twelve-labs
Copy link

@cagataycali @jer96 We are still waiting for your final approval. It'd be great if this can be done quickly as this PR review has been dragging over 2 months.

@cagataycali cagataycali requested review from a team and cagataycali September 14, 2025 16:40
@yonib05
Copy link
Member

yonib05 commented Sep 17, 2025

Hey @james-le-twelve-labs,

It seems the test is still failing.

FAILED tests/test_chat_video.py::TestChatVideoTool::test_chat_upload_failure - AssertionError: assert 'success' == 'error'

https://github.com/strands-agents/tools/actions/runs/17413489152/job/50606369127?pr=120

@james-le-twelve-labs
Copy link

@wbrennan899 Can you look at the issue above? #120 (comment)

@cagataycali @yonib05 It appears that recent changes in the Strands repo over the past 2 weeks lead to breaking changes with our PR. After Will fix the issue, can you make sure to approve the PR in a timely manner (in < 3 days at most)?

@wbrennan899
Copy link
Contributor Author

@cagataycali All tests are still passing on my side, but this change might fix it

@yonib05
Copy link
Member

yonib05 commented Sep 18, 2025

Re-running the tests now

@wbrennan899
Copy link
Contributor Author

@yonib05 Any idea how I can replicate the tests properly on my machine? They all pass locally

@yonib05
Copy link
Member

yonib05 commented Sep 18, 2025

Is there any infrastructure setup or non-included dependencies required for the unit test? The GitHub action creates a clean environment, downloads all dependencies, and runs. I am taking a closer look now at the test code to see if I can figure out why it is failing.

@james-le-twelve-labs
Copy link

@yonib05 @cagataycali Since this thread has been going for a while, I think it'd be best if we can have a Zoom call to figure out the root cause of the issues. Without visibility on the logs that you ran that lead to these failing checks, it'd be challenging for us to fix them successfully.

Let me know if that works for you and we can schedule time.

@yonib05
Copy link
Member

yonib05 commented Sep 25, 2025

Hey @james-le-twelve-labs,

I have observed something similar: that locally tests pass, but tests are still failing on GitHub. We looking into it now to root cause the issue to understand where the flakiness could be coming from.

@james-le-twelve-labs
Copy link

@yonib05 Has there been an update on your review?

@mkmeral
Copy link
Contributor

mkmeral commented Sep 30, 2025

hey folks, I have been investigating this issue, and I could replicate it locally using python -m pytest tests/test_chat_video.py -v

The issue seems to be due to caching implemented in upload_and_index_video. Since same video is used in multiple unit tests, the order in which these tests are run impacts the results.

Some of these tests also use temp_video_file, causing the video to be "uploaded" and cached as uploaded in VIDEO_CACHE . As a result, in consequent upload calls, the task creation is not even called.

If you update tool use such that video path is unique, the test passes. You need to update it to something like

    @patch.dict("os.environ", {"TWELVELABS_API_KEY": "test-api-key", "TWELVELABS_PEGASUS_INDEX_ID": "test-index"})
    @patch("strands_tools.chat_video.TwelveLabs")
    def test_chat_upload_failure(self, mock_twelvelabs, tmp_path):
        """Test handling of upload failures."""
        ...

        tool_use = {"toolUseId": "test-chat-9", "input": {"prompt": "Test prompt", "video_path": temp_video_file(tmp_path, "test_new_video_with_unique_name.mp4")}}

        ...

using the updated temp_video_file

@pytest.fixture
def temp_video_file(tmp_path, filename = "test_video.mp4"):
    """Create a temporary video file path."""
    video_path = tmp_path / filename
    video_path.write_bytes(b"fake video content")
    return str(video_path)

@wbrennan899
Copy link
Contributor Author

Thanks @mkmeral! I was able to reproduce the error locally. I fixed the broken test and pushed an update.

@yonib05 yonib05 merged commit b65dd11 into strands-agents:main Sep 30, 2025
16 checks passed
@james-le-twelve-labs
Copy link

@mkmeral Thanks for your insight!

@cagataycali @yonib05 This PR finally got merged! Thanks for your support throughout the process. My team will create a lot more tutorials showing how to use TwelveLabs models via Strands Agents over the upcoming months :)

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.

6 participants