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

langchain_cohere: Add Cohere multi hop agent #19650

Closed

Conversation

BeatrixCohere
Copy link
Contributor

  • Description: Adds the experimental version of multi-hop agent as a raw prompt

@efriis efriis added the partner label Mar 27, 2024
@efriis efriis self-assigned this Mar 27, 2024
@dosubot dosubot bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Mar 27, 2024
Copy link

vercel bot commented Mar 27, 2024

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

Name Status Preview Comments Updated (UTC)
langchain ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 29, 2024 8:30pm

@dosubot dosubot bot added Ɑ: agent Related to agents module 🔌: cohere Primarily related to Cohere integrations 🤖:enhancement A large net-new component, integration, or chain. Use sparingly. The largest features labels Mar 27, 2024
@dosubot dosubot bot added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Mar 28, 2024
@dosubot dosubot bot removed the size:XL This PR changes 500-999 lines, ignoring generated files. label Mar 29, 2024
@dosubot dosubot bot added the size:XXL This PR changes 1000+ lines, ignoring generated files. label Mar 29, 2024
@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:XXL This PR changes 1000+ lines, ignoring generated files. labels Mar 29, 2024
Copy link

@minjie-cohere minjie-cohere left a comment

Choose a reason for hiding this comment

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

Nice work 👏
Left some comments and suggestions re. more thoroughly addressing issues 4 & 5 identified by Patrick. Please fix 🫡

Comment on lines +91 to +93
description=param_definition.get("description")
if "description" in param_definition
else "",

Choose a reason for hiding this comment

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

Suggested change
description=param_definition.get("description")
if "description" in param_definition
else "",
description=param_definition.get("description") or ""

to avoid rendering param description as None (issue 5)? we should apply the same fix to the other cases as well

Comment on lines +189 to +190
def get_type(type_: str, is_optional: bool) -> str:
python_type = JSON_TO_PYTHON_TYPES.get(type_, type_)

Choose a reason for hiding this comment

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

The following will get us List[str] (or List[int], ...) instead of array, which addresses a popular and important case.

Suggested change
def get_type(type_: str, is_optional: bool) -> str:
python_type = JSON_TO_PYTHON_TYPES.get(type_, type_)
# https://json-schema.org/understanding-json-schema/reference/type
JSON_SCHEMA_TO_PYTHON_TYPES = {
"string": "str",
"number": "float",
"boolean": "bool",
"integer": "int",
"array": "List",
"object": "Dict",
}
def get_type(json_schema_type: Union[str, dict], is_optional: bool) -> str:
if json_schema_type == "object":
python_type = "Dict"
elif json_schema_type == "array":
item_type = JSON_SCHEMA_TO_PYTHON_TYPES.get(json_schema_type["items"]["type"], "")
python_type = f"List[{item_type}]" if item_type else "List"
else:
python_type = JSON_SCHEMA_TO_PYTHON_TYPES.get(json_schema_type) or str(json_schema_type)

type_ = get_type(
parameter_definition.get("type"), "default" in parameter_definition
)
description = parameter_definition.get("description")

Choose a reason for hiding this comment

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

Suggested change
description = parameter_definition.get("description")
description = parameter_definition.get("description") or ""

to avoid rendering None into param description

description=param_definition.get("description")
if "description" in param_definition
else "",
type=JSON_TO_PYTHON_TYPES.get(

Choose a reason for hiding this comment

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

reuse get_type from libs/partners/cohere/langchain_cohere/react_multi_hop/agent.py for consistency

@@ -11,7 +11,12 @@
from langchain_core.runnables import Runnable, RunnablePassthrough
from langchain_core.runnables.base import RunnableLambda
from langchain_core.tools import BaseTool
from langchain_core.utils.function_calling import convert_to_openai_function
from langchain_core.utils.function_calling import (
PYTHON_TO_JSON_TYPES,

Choose a reason for hiding this comment

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

this only handles primitive types (i.e. not array and object). see below for a more complete solution.

@@ -101,7 +110,9 @@ def _convert_to_cohere_tool(
parameter_definitions={
param_name: ToolParameterDefinitionsValue(
description=param_definition.get("description"),

Choose a reason for hiding this comment

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

ditto

@@ -121,7 +132,9 @@ def _convert_to_cohere_tool(
parameter_definitions={
param_name: ToolParameterDefinitionsValue(
description=param_definition.get("description"),

Choose a reason for hiding this comment

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

ditto

Comment on lines +135 to +137
type=JSON_TO_PYTHON_TYPES.get(
param_definition.get("type"), param_definition.get("type")
),

Choose a reason for hiding this comment

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

ditto

Comment on lines +113 to +115
type=JSON_TO_PYTHON_TYPES.get(
param_definition.get("type"), param_definition.get("type")
),

Choose a reason for hiding this comment

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

ditto

@harry-cohere
Copy link
Contributor

Thanks Minjie and Beatrix (and Erick for the review so far). We've reopened this PR in:

#19919

efriis added a commit that referenced this pull request Apr 2, 2024
**Description**: Adds an agent that uses Cohere with multiple hops and
multiple tools.

This PR is a continuation of
#19650 - which was
previously approved. Conceptually nothing has changed, but this PR has
extra fixes, documentation and testing.

---------

Co-authored-by: BeatrixCohere <128378696+BeatrixCohere@users.noreply.github.com>
Co-authored-by: Erick Friis <erickfriis@gmail.com>
Je-Cp pushed a commit to Je-Cp/jcp-langchain that referenced this pull request Apr 2, 2024
**Description**: Adds an agent that uses Cohere with multiple hops and
multiple tools.

This PR is a continuation of
langchain-ai/langchain#19650 - which was
previously approved. Conceptually nothing has changed, but this PR has
extra fixes, documentation and testing.

---------

Co-authored-by: BeatrixCohere <128378696+BeatrixCohere@users.noreply.github.com>
Co-authored-by: Erick Friis <erickfriis@gmail.com>
hinthornw pushed a commit that referenced this pull request Apr 26, 2024
**Description**: Adds an agent that uses Cohere with multiple hops and
multiple tools.

This PR is a continuation of
#19650 - which was
previously approved. Conceptually nothing has changed, but this PR has
extra fixes, documentation and testing.

---------

Co-authored-by: BeatrixCohere <128378696+BeatrixCohere@users.noreply.github.com>
Co-authored-by: Erick Friis <erickfriis@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ɑ: agent Related to agents module 🔌: cohere Primarily related to Cohere integrations 🤖:enhancement A large net-new component, integration, or chain. Use sparingly. The largest features lgtm PR looks good. Use to confirm that a PR is ready for merging. partner size:XL This PR changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants