Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ai: Update llm pipeline #3336

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

ad-astra-video
Copy link
Collaborator

@ad-astra-video ad-astra-video commented Dec 29, 2024

What does this pull request do? Explain your changes. (required)

Updates go-livepeer for LLM pipeline updates for new response format.

Adds in validation of LLM request to gateway provided in #3204

Note: checked box to allow edits by maintainers to help expedite with timezone difference. Feel free to push any updates needed to branch.

Requires ai-worker livepeer/ai-worker#387 to be merged
Specific updates (required)

  • Updates needed for ai-worker changes to LLM request and response formats

How did you test each of these updates (required)

Built docker image locally for livepeer/ai-runner:llm and livepeer/go-livepeer to test streaming and non-streaming responses

Does this pull request close any open issues?

No

Checklist:

@github-actions github-actions bot added dependencies Pull requests that update a dependency file go Pull requests that update Go code AI Issues and PR related to the AI-video branch. labels Dec 29, 2024
@ad-astra-video
Copy link
Collaborator Author

I will update go.mod when we get the second PR merged on ai-worker and new version tagged so can pull in final updates from ai-worker.

go.mod Outdated Show resolved Hide resolved
Copy link
Contributor

@leszko leszko left a comment

Choose a reason for hiding this comment

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

Added 2 comments. Please also check if CI passes.

server/ai_worker_test.go Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
leszko
leszko previously requested changes Jan 3, 2025
Copy link
Contributor

@leszko leszko left a comment

Choose a reason for hiding this comment

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

Copy link
Member

@victorges victorges left a comment

Choose a reason for hiding this comment

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

LGTM but remember to update go.mod before merging

core/ai_worker.go Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
server/ai_mediaserver.go Outdated Show resolved Hide resolved
Copy link

codecov bot commented Jan 14, 2025

Codecov Report

Attention: Patch coverage is 3.57143% with 54 lines in your changes missing coverage. Please review.

Project coverage is 33.69713%. Comparing base (3acf4ea) to head (0b9cdc7).

Files with missing lines Patch % Lines
server/ai_process.go 0.00000% 30 Missing ⚠️
server/ai_http.go 0.00000% 9 Missing ⚠️
server/ai_mediaserver.go 0.00000% 8 Missing ⚠️
core/ai_worker.go 0.00000% 4 Missing ⚠️
server/ai_worker.go 40.00000% 2 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                 @@
##              master       #3336         +/-   ##
===================================================
+ Coverage   33.68997%   33.69713%   +0.00716%     
===================================================
  Files            141         141                 
  Lines          37480       37475          -5     
===================================================
+ Hits           12627       12628          +1     
+ Misses         24131       24125          -6     
  Partials         722         722                 
Files with missing lines Coverage Δ
core/ai.go 58.51852% <ø> (ø)
server/rpc.go 66.66667% <ø> (ø)
server/ai_worker.go 49.89059% <40.00000%> (ø)
core/ai_worker.go 31.06796% <0.00000%> (+0.12136%) ⬆️
server/ai_mediaserver.go 7.96117% <0.00000%> (+0.03080%) ⬆️
server/ai_http.go 10.03135% <0.00000%> (ø)
server/ai_process.go 0.59932% <0.00000%> (+0.00154%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3acf4ea...0b9cdc7. Read the comment docs.

Files with missing lines Coverage Δ
core/ai.go 58.51852% <ø> (ø)
server/rpc.go 66.66667% <ø> (ø)
server/ai_worker.go 49.89059% <40.00000%> (ø)
core/ai_worker.go 31.06796% <0.00000%> (+0.12136%) ⬆️
server/ai_mediaserver.go 7.96117% <0.00000%> (+0.03080%) ⬆️
server/ai_http.go 10.03135% <0.00000%> (ø)
server/ai_process.go 0.59932% <0.00000%> (+0.00154%) ⬆️

@ad-astra-video
Copy link
Collaborator Author

@rickstaa @kyriediculous Updated PR for latest updates in LLM pipeline. Please let me know if anything want changed. Built and tested locally end-to-end with no issues.

Copy link
Member

@rickstaa rickstaa left a comment

Choose a reason for hiding this comment

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

LGTM, will run one production test and merge.

@rickstaa rickstaa force-pushed the update_llm_pipeline branch from fc3530e to 0b9cdc7 Compare January 27, 2025 12:09
@rickstaa rickstaa requested a review from leszko January 27, 2025 12:28
@rickstaa rickstaa dismissed leszko’s stale review January 27, 2025 12:28

Addressed changes and OOO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AI Issues and PR related to the AI-video branch. dependencies Pull requests that update a dependency file go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants