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

Spike/langgraph #875

Merged
merged 9 commits into from
Jul 30, 2024
Merged

Spike/langgraph #875

merged 9 commits into from
Jul 30, 2024

Conversation

jamesrichards4
Copy link
Contributor

Context

We want to move to a more capable AI framework and concentrate all our AI work in the redbox library redbox-core

Changes proposed in this pull request

Use LangGraph to run the computation rather than manually executing chains.
Move all chains/AI code to redbox-core and make it available as a single graph which can be imported
Add new tests which allow easy creation of scenarios

Guidance to review

The current solution should be unchanged. This brings the new LangGraph code in alongside for now.

Things to check

  • I have added any new ENV vars in all deployed environments
  • I have tested any code added or changed
  • I have run integration tests

@jamesrichards4 jamesrichards4 requested a review from wpfl-dbt July 26, 2024 14:20
Copy link
Collaborator

@lmwilkigov lmwilkigov left a comment

Choose a reason for hiding this comment

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

Very clean code 🙌. My main thought is that I'm likely not fluent in LangGraph enough to spot anything needing a change. I'd be keen to get this up in Dev to test it out and then start documenting the Graph quite heavily

"reduce",
build_reduce_docs_step(
TokenTextSplitter(
model_name="gpt-4",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be gpt-4o instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, I'm not sure, this should be an env var so we switch it as needed

Copy link
Collaborator

Choose a reason for hiding this comment

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

Love the graph visualisation! Could we also serialise the images to the docs/assets directory so we can utilise them in the docs?

@jamesrichards4 jamesrichards4 merged commit 511d458 into main Jul 30, 2024
3 checks passed
@gecBurton gecBurton deleted the spike/langgraph branch October 29, 2024 15:48
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.

2 participants