Skip to content

Conversation

@rogeriochaves
Copy link
Contributor

The callback argument was missing, preventing me to get callbacks to work properly when using it async

@vercel
Copy link

vercel bot commented Jun 22, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Jun 22, 2023 11:13am

Copy link

@QodoAI-Agent QodoAI-Agent left a comment

Choose a reason for hiding this comment

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

PR Analysis

  • 🎯 Main theme: This PR fixes an issue with callback forwarding in the async plan method for the OpenAI function agent.
  • 🔍 Description and title: yes
  • 📌 Type of PR: Bug fix
  • 🐞 Bug description: The callback argument was missing in the async plan method, which prevented callbacks from working properly when using the method asynchronously.
  • 🧪 Relevant tests added: no
  • ⚠️ Unrelated changes: no
  • Minimal and focused: yes

PR Feedback

  • 💡 Suggestions: Please add tests to verify that the callback functionality works as expected after the fix.

  • 🌱 Minor suggestions: Consider adding a docstring to the aplan method to explain its purpose and usage, including the callback functionality.

  • 🤖 Code Suggestions:

    • In file base.py: Add a test case in the test suite for the OpenAI function agent that checks if the callback is properly forwarded and executed when using the aplan method asynchronously. [important]

Copy link

@QodoAI-Agent QodoAI-Agent left a comment

Choose a reason for hiding this comment

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

PR Analysis

  • 🎯 Main theme: This PR fixes the missing callback argument in the async plan method for OpenAI function agent.
  • 🔍 Description and title: yes
  • 📌 Type of PR: Bug fix
  • 🐞 Bug description: The callback argument was missing in the async plan method, preventing callbacks from working properly when using it asynchronously.
  • 🧪 Relevant tests added: no
  • ⚠️ Unrelated changes: no
  • Minimal and focused: yes

PR Feedback

  • 💡 Suggestions: Please add a test case to demonstrate the bug and the fix.
  • 🤖 Code Suggestions:

@dev2049
Copy link
Contributor

dev2049 commented Jun 22, 2023

thanks @rogeriochaves!

@dev2049 dev2049 merged commit 3436da6 into langchain-ai:master Jun 22, 2023
tconkling added a commit to tconkling/langchain that referenced this pull request Jun 22, 2023
* master:
  MD header text splitter returns Documents (langchain-ai#6571)
  Fix callback forwarding in async plan method for OpenAI function agent (langchain-ai#6584)
  bump 209 (langchain-ai#6593)
  Clarifai integration (langchain-ai#5954)
  Add missing word in comment (langchain-ai#6587)
  Add AzureML endpoint LLM wrapper (langchain-ai#6580)
  Add OpenLLM wrapper(langchain-ai#6578)
  feat: interfaces for async embeddings, implement async openai (langchain-ai#6563)
  Upgrade the version of AwaDB and add some new interfaces (langchain-ai#6565)
  add motherduck docs (langchain-ai#6572)
  Detailed using the Twilio tool to send messages with 3rd party apps incl. WhatsApp (langchain-ai#6562)
  Change Data Loader Namespace (langchain-ai#6568)
  Remove duplicate databricks entries in ecosystem integrations (langchain-ai#6569)
  Fix whatsappchatloader - enable parsing new datetime format on WhatsApp chat (langchain-ai#6555)
  Wait for all futures (langchain-ai#6554)
  feat: faiss filter from list (langchain-ai#6537)
  update pr tmpl (langchain-ai#6552)
  Remove unintended double negation in docstring (langchain-ai#6541)
  Minor Grammar Fixes in Docs and Comments (langchain-ai#6536)
This was referenced Jun 25, 2023
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.

3 participants