-
Notifications
You must be signed in to change notification settings - Fork 141
Add support for Anthropic/Claude and other LLM providers #74
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
base: main
Are you sure you want to change the base?
Conversation
Refactored the agent construction to be provider-agnostic by replacing the OpenAI-specific tool message formatter and output parser with langchain's create_tool_calling_agent, which works with any chat model that implements tool calling. Changes: - Replace format_to_openai_tool_messages + OpenAIToolsAgentOutputParser with create_tool_calling_agent (provider-agnostic) - Widen ChatModel type alias to BaseChatModel so any langchain-compatible LLM can be passed in without type errors - Add langchain-anthropic as an optional dependency - Guard token usage tracking behind an isinstance check so it only runs the OpenAI callback for OpenAI models (avoids runtime errors with other providers) - Update turtle_agent example to support provider selection via LLM_PROVIDER env var (openai | anthropic | ollama) - Update .env with configuration for all three providers Backwards compatible: existing code using ChatOpenAI or AzureChatOpenAI will work exactly the same. No breaking changes to the public API. Relates to nasa-jpl#56
e530585 to
56adc8b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds support for multiple LLM providers (Anthropic/Claude, Ollama) to ROSA, which was previously locked to OpenAI only. The main architectural change replaces OpenAI-specific agent components (format_to_openai_tool_messages and OpenAIToolsAgentOutputParser) with langchain's provider-agnostic create_tool_calling_agent function.
Changes:
- Refactored the agent layer in ROSA to use provider-agnostic
create_tool_calling_agentinstead of OpenAI-specific components - Added provider selection logic to
turtle_agent/scripts/llm.pywith support for OpenAI, Anthropic, and Ollama - Updated the
ChatModeltype from a Union of specific models toBaseChatModelfor broader compatibility - Added conditional token usage tracking that only works for OpenAI-based models
- Updated dependencies and environment configuration to support multiple providers
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| src/rosa/rosa.py | Refactored agent creation to use create_tool_calling_agent, updated type annotations to accept any BaseChatModel, and added conditional token usage tracking for OpenAI models |
| src/turtle_agent/scripts/llm.py | Added provider switching logic based on LLM_PROVIDER environment variable with support for OpenAI, Anthropic, and Ollama |
| pyproject.toml | Added langchain-anthropic~=0.3.12 as a dependency |
| .env | Added configuration sections for all three providers with model names and API keys |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def get_llm(streaming: bool = False): | ||
| """A helper function to get the LLM instance.""" | ||
| """A helper function to get the LLM instance. | ||
|
|
||
| Supports OpenAI (default), Anthropic and Ollama models. | ||
| Set the LLM_PROVIDER env variable to switch between providers: | ||
| - "openai" (default): uses OPENAI_API_KEY | ||
| - "anthropic": uses ANTHROPIC_API_KEY | ||
| - "ollama": uses local Ollama instance | ||
| """ | ||
| dotenv.load_dotenv(dotenv.find_dotenv()) | ||
|
|
||
| llm = ChatOpenAI( | ||
| api_key=get_env_variable("OPENAI_API_KEY"), | ||
| model="gpt-5.1", | ||
| streaming=streaming, | ||
| ) | ||
| provider = os.getenv("LLM_PROVIDER", "openai").lower() | ||
|
|
||
| if provider == "anthropic": | ||
| try: | ||
| from langchain_anthropic import ChatAnthropic | ||
| except ImportError: | ||
| raise ImportError( | ||
| "langchain-anthropic is required for Anthropic support. " | ||
| "Install it with: pip install langchain-anthropic" | ||
| ) | ||
| llm = ChatAnthropic( | ||
| api_key=get_env_variable("ANTHROPIC_API_KEY"), | ||
| model=os.getenv("ANTHROPIC_MODEL", "claude-sonnet-4-20250514"), | ||
| streaming=streaming, | ||
| ) | ||
| elif provider == "ollama": | ||
| from langchain_ollama import ChatOllama | ||
| llm = ChatOllama( | ||
| model=os.getenv("OLLAMA_MODEL", "llama3"), | ||
| base_url=os.getenv("OLLAMA_BASE_URL", "http://localhost:11434"), | ||
| ) | ||
| else: | ||
| llm = ChatOpenAI( | ||
| api_key=get_env_variable("OPENAI_API_KEY"), | ||
| model=os.getenv("OPENAI_MODEL", "gpt-4o"), | ||
| streaming=streaming, | ||
| ) | ||
|
|
||
| return llm |
Copilot
AI
Jan 31, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new multi-provider functionality lacks test coverage. The codebase has comprehensive test coverage for tools and other components, but there are no tests validating that the different LLM providers (Anthropic, Ollama) can be instantiated correctly, or that the provider switching logic works as expected. Consider adding tests that verify the get_llm function with different LLM_PROVIDER values and mock the provider-specific imports.
pyproject.toml
Outdated
| "langchain-core~=0.3.52", | ||
| "langchain-openai~=0.3.14", | ||
| "langchain-ollama~=0.3.2", | ||
| "langchain-anthropic~=0.3.12", |
Copilot
AI
Jan 31, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The langchain-anthropic package is listed as a required dependency in pyproject.toml (line 33), but the code in llm.py treats it as optional with a try-except ImportError block. This creates an inconsistency: either make langchain-anthropic an optional dependency in pyproject.toml, or remove the try-except block since the package will always be installed. The current approach will always install langchain-anthropic even if users only want to use OpenAI or Ollama, which increases installation size and dependencies unnecessarily.
src/rosa/rosa.py
Outdated
| from langchain_core.language_models import BaseChatModel | ||
| from langchain_core.messages import AIMessage, HumanMessage | ||
| from langchain_core.prompts import ChatPromptTemplate | ||
| from langchain_ollama import ChatOllama |
Copilot
AI
Jan 31, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ChatOllama import is no longer used in this file after refactoring to use BaseChatModel. The import can be safely removed as ChatOllama instances will be passed in as BaseChatModel types. Only ChatOpenAI and AzureChatOpenAI are needed for the token callback isinstance check.
| from langchain_ollama import ChatOllama |
src/rosa/rosa.py
Outdated
| try: | ||
| from langchain_anthropic import ChatAnthropic | ||
| except ImportError: | ||
| pass |
Copilot
AI
Jan 31, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TYPE_CHECKING import block for ChatAnthropic will fail silently if the import fails, but this won't provide useful type hints since the import is wrapped in a try-except. Consider removing the try-except within the TYPE_CHECKING block, as import errors during type checking should be reported to developers so they know to install the appropriate dependencies for type checking. The runtime import protection is already handled in the turtle_agent/scripts/llm.py file.
| try: | |
| from langchain_anthropic import ChatAnthropic | |
| except ImportError: | |
| pass | |
| from langchain_anthropic import ChatAnthropic |
src/rosa/rosa.py
Outdated
| ros_version (Literal[1, 2]): The version of ROS that the agent will interact with. | ||
| llm (Union[AzureChatOpenAI, ChatOpenAI, ChatOllama]): The language model to use for generating responses. | ||
| llm (BaseChatModel): Any langchain chat model that supports tool calling. Tested with | ||
| ChatOpenAI, AzureChatOpenAI, ChatOllama, and ChatAnthropic. |
Copilot
AI
Jan 31, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The llm parameter documentation should mention that token usage tracking via show_token_usage is only available for ChatOpenAI and AzureChatOpenAI models. When using other providers like ChatAnthropic or ChatOllama, token usage will not be displayed even if show_token_usage is enabled.
| ChatOpenAI, AzureChatOpenAI, ChatOllama, and ChatAnthropic. | |
| ChatOpenAI, AzureChatOpenAI, ChatOllama, and ChatAnthropic. Note that token usage | |
| tracking via `show_token_usage` is only supported for ChatOpenAI and AzureChatOpenAI; | |
| other providers (e.g., ChatAnthropic, ChatOllama) will not display token usage even | |
| when `show_token_usage` is enabled. |
src/turtle_agent/scripts/llm.py
Outdated
| streaming=streaming, | ||
| ) | ||
| elif provider == "ollama": | ||
| from langchain_ollama import ChatOllama |
Copilot
AI
Jan 31, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The langchain_ollama import is not wrapped in a try-except block like the langchain_anthropic import, creating inconsistent error handling between providers. Since both are listed as required dependencies in pyproject.toml, the ImportError handling for Anthropic is unnecessary. However, if the intention is to make these providers optional, then both should have try-except blocks with helpful error messages, and both should be made optional dependencies in pyproject.toml.
| from langchain_ollama import ChatOllama | |
| try: | |
| from langchain_ollama import ChatOllama | |
| except ImportError: | |
| raise ImportError( | |
| "langchain-ollama is required for Ollama support. " | |
| "Install it with: pip install langchain-ollama" | |
| ) |
| from langchain_ollama import ChatOllama | ||
| llm = ChatOllama( | ||
| model=os.getenv("OLLAMA_MODEL", "llama3"), | ||
| base_url=os.getenv("OLLAMA_BASE_URL", "http://localhost:11434"), |
Copilot
AI
Jan 31, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The streaming parameter is not passed to ChatOllama, unlike ChatOpenAI and ChatAnthropic. This inconsistency means that streaming won't work properly when using Ollama. Add the streaming parameter to maintain consistent behavior across all providers.
| base_url=os.getenv("OLLAMA_BASE_URL", "http://localhost:11434"), | |
| base_url=os.getenv("OLLAMA_BASE_URL", "http://localhost:11434"), | |
| streaming=streaming, |
| from langchain.agents.output_parsers.openai_tools import OpenAIToolsAgentOutputParser | ||
| import logging | ||
| from contextlib import contextmanager | ||
| from typing import TYPE_CHECKING, Any, AsyncIterable, Dict, Literal, Optional, Union |
Copilot
AI
Jan 31, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'Union' is not used.
| from typing import TYPE_CHECKING, Any, AsyncIterable, Dict, Literal, Optional, Union | |
| from typing import TYPE_CHECKING, Any, AsyncIterable, Dict, Literal, Optional |
RobRoyce
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! Overall this is a great addition and solves open requests for Anthropic model support. There are a few minor tweaks needed to ensure there is no confusion about which model providers are supported (especially when we move from hard coding to using env vars).
.env
Outdated
|
|
||
| # Anthropic Configuration | ||
| ANTHROPIC_API_KEY= | ||
| ANTHROPIC_MODEL=claude-sonnet-4-20250514 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likely to break as model snapshots are retired, prefer leaving blank (placeholders are fine).
| ANTHROPIC_MODEL=claude-sonnet-4-20250514 | |
| ANTHROPIC_MODEL=claude-sonnet-4-5 |
src/rosa/rosa.py
Outdated
| logger = logging.getLogger(__name__) | ||
|
|
||
| # Runtime-safe type alias: accepts any BaseChatModel, covering OpenAI, Azure, | ||
| # Anthropic, Ollama and any future langchain provider that implements tool calling. | ||
| ChatModel = BaseChatModel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The strict Union was removed in favor of a generic BaseChatModel, but get_llm()in llm.py still effectively hard-codes a strict union.
I am in favor of allowing any tool calling LLM, but that means we need the ability to construct a generic chat model without enumerating all possibilities.
Recommend either reverting back to Union with ChatAnthropic included and remove "any tool calling model" language, or find a good solution for constructing generic tool calling Chat models. Either fix is fine with me 👍
| @contextmanager | ||
| def _token_callback(self): | ||
| """Context manager for token usage tracking. | ||
|
|
||
| Uses the OpenAI callback when the LLM is an OpenAI-based model, | ||
| otherwise yields None so the rest of the flow is unaffected. | ||
| """ | ||
| if isinstance(self.__llm, (ChatOpenAI, AzureChatOpenAI)): | ||
| with get_openai_callback() as cb: | ||
| yield cb | ||
| else: | ||
| yield None | ||
|
|
||
| def _print_usage(self, cb): | ||
| """Print the token usage if show_token_usage is enabled.""" | ||
| if cb and self.__show_token_usage: | ||
| print(f"[bold]Prompt Tokens:[/bold] {cb.prompt_tokens}") | ||
| print(f"[bold]Completion Tokens:[/bold] {cb.completion_tokens}") | ||
| print(f"[bold]Total Cost (USD):[/bold] ${cb.total_cost}") | ||
| if cb is None or not self.__show_token_usage: | ||
| return | ||
| print(f"[bold]Prompt Tokens:[/bold] {cb.prompt_tokens}") | ||
| print(f"[bold]Completion Tokens:[/bold] {cb.completion_tokens}") | ||
| print(f"[bold]Total Cost (USD):[/bold] ${cb.total_cost}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Silent inaction is undesirable when the user explicitly enables __show_token_usage (default is False). Prefer a small warning message and/or failing early (i.e. during __init__).
Question: does LangChain provide the ability to view tokens for arbitrary chat models? Since TikToken is already a dependency, perhaps we can use it to find token count for prompt + response?
.env
Outdated
| OPENAI_API_VERSION= | ||
| OPENAI_API_TYPE= No newline at end of file | ||
| OPENAI_API_TYPE= | ||
| OPENAI_MODEL=gpt-4o |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might as well go with the latest if we're adding defaults :)
| OPENAI_MODEL=gpt-4o | |
| OPENAI_MODEL=gpt-5.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the LLM_PROVIDER env var idea, but I don't think it should default to OpenAI if it doesn't match anthropic or ollama. If we are going to hard-code these names, prefer also hard-coding openai in the if/else and failing if there is no match to avoid confusion (error message should include the supported options).
| model="gpt-5.1", | ||
| streaming=streaming, | ||
| ) | ||
| provider = os.getenv("LLM_PROVIDER", "openai").lower() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the get_env_variable function and remove defaults (likely to cause confusion if this doesn't fail early with explicit error messages).
…ic dep, token usage warnings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if provider == "openai": | ||
| llm = ChatOpenAI( | ||
| api_key=get_env_variable("OPENAI_API_KEY"), | ||
| model=get_env_variable("OPENAI_MODEL"), |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR description claims this change is "backwards compatible" and that "Nothing breaks," but the change from a hardcoded model to requiring the OPENAI_MODEL environment variable is a breaking change for turtle_agent users. Existing users who upgrade will encounter errors if they don't update their .env file. This should be documented as a breaking change in the PR description or a migration guide should be provided.
| def _get_agent(self): | ||
| """Create and return an agent for processing user inputs and generating responses.""" | ||
| agent = ( | ||
| { | ||
| "input": lambda x: x["input"], | ||
| "agent_scratchpad": lambda x: format_to_openai_tool_messages( | ||
| x["intermediate_steps"] | ||
| ), | ||
| "chat_history": lambda x: x["chat_history"], | ||
| } | ||
| | self.__prompts | ||
| | self.__llm_with_tools | ||
| | OpenAIToolsAgentOutputParser() | ||
| """Create and return an agent for processing user inputs and generating responses. | ||
|
|
||
| Uses create_tool_calling_agent which is provider-agnostic and works with | ||
| any LLM that supports tool calling (OpenAI, Anthropic, Ollama, etc). | ||
| """ | ||
| agent = create_tool_calling_agent( | ||
| llm=self.__llm, | ||
| tools=self.__tools.get_tools(), | ||
| prompt=self.__prompts, | ||
| ) | ||
| return agent |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The refactoring from OpenAI-specific agent pipeline to create_tool_calling_agent is a significant architectural change, but there are no tests to verify that it works correctly with different providers. Consider adding integration tests that validate the agent works with mock ChatOpenAI, ChatAnthropic, and ChatOllama instances to ensure the provider-agnostic implementation behaves correctly across different LLM backends.
| if TYPE_CHECKING: | ||
| from langchain_anthropic import ChatAnthropic | ||
|
|
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ChatAnthropic import under TYPE_CHECKING is not used in any type annotations, only in the docstring. Since the code now uses BaseChatModel for all type hints, this import can be removed to reduce dependencies and avoid potential confusion.
| if TYPE_CHECKING: | |
| from langchain_anthropic import ChatAnthropic |
| [project.optional-dependencies] | ||
| anthropic = ["langchain-anthropic~=0.3.12"] |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding langchain-ollama to the optional-dependencies section similar to langchain-anthropic, since it's no longer imported directly in the ROSA core code and is only used in the turtle_agent example with lazy import. This would reduce the installation footprint for users who don't need Ollama support. For example: ollama = ["langchain-ollama~=0.3.2"]
src/rosa/rosa.py
Outdated
| if self.__show_token_usage: | ||
| logger.warning("Token usage tracking is only supported for OpenAI and Azure models.") |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The warning message is only logged once when the context manager is entered, but if a user repeatedly calls invoke() with show_token_usage=True on a non-OpenAI model, they will see this warning every time. Consider adding a flag to log this warning only once, or document this limitation in the constructor's docstring more prominently to set user expectations upfront.
src/rosa/rosa.py
Outdated
| if isinstance(self.__llm, (ChatOpenAI, AzureChatOpenAI)): | ||
| with get_openai_callback() as cb: | ||
| yield cb | ||
| else: | ||
| if self.__show_token_usage: | ||
| logger.warning("Token usage tracking is only supported for OpenAI and Azure models.") |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The _token_callback method uses isinstance() to check if the LLM is OpenAI-based, but this check happens at runtime on every invocation. Since the LLM type doesn't change after initialization, consider checking this once in init and storing a boolean flag (e.g., self.__supports_token_tracking) to avoid repeated isinstance calls. This would be more efficient and clearer.
| if isinstance(self.__llm, (ChatOpenAI, AzureChatOpenAI)): | |
| with get_openai_callback() as cb: | |
| yield cb | |
| else: | |
| if self.__show_token_usage: | |
| logger.warning("Token usage tracking is only supported for OpenAI and Azure models.") | |
| # Lazily determine whether the current LLM supports OpenAI-style token tracking. | |
| if not hasattr(self, "_ROSA__supports_token_tracking"): | |
| self.__supports_token_tracking = isinstance( | |
| self.__llm, (ChatOpenAI, AzureChatOpenAI) | |
| ) | |
| if self.__supports_token_tracking: | |
| with get_openai_callback() as cb: | |
| yield cb | |
| else: | |
| if self.__show_token_usage: | |
| logger.warning( | |
| "Token usage tracking is only supported for OpenAI and Azure models." | |
| ) |
|
hey @RobRoyce, thanks for the detailed review! pushed the fixes: Union vs BaseChatModel: went with a Token usage: moved the warning to btw, been using ROSA with a hexapod robot i have (JetHEXA -- Jetson Nano B01, ROS Melodic, 18 DOF with IK, LIDAR, depth camera, IMU). the multi-provider support is really usefull here -- been testing with both OpenAI and Claude to compare how they handle ROS service calls and tool chaining. the Ollama support is actually the one im most excited about tho. the whole point for me is running inference directly on the Jetson so the robot can operate fully offline -- no cloud dependency, no latency, just edge AI. think autonomous decision-making beyond what traditional nav stacks give you (SLAM, AMCL, move_base). similar to how JPL does it with AutoNav on the Mars rovers or AEGIS for autonomous target selection -- the robot needs to reason on its own when theres no link back home. im also planing a custom ROSA tool called let me know if the fixes look good |
Hey, been using ROSA for some robotics projects and ran into the limitation of it being locked to OpenAI only. Saw that other people had the same problem (#56, #65) so I decided to refactor the agent layer to support multiple providers.
What I did
The main change is swapping out the OpenAI-specific agent pipeline (
format_to_openai_tool_messages+OpenAIToolsAgentOutputParser) for langchain'screate_tool_calling_agent, which works with any model that has tool calling support. This means you can now use Claude, Gemini, local models through Ollama, etc.I also:
ChatModeltype toBaseChatModelso it accepts any langchain chat modellangchain-anthropicas dependency and updated .env with config for all providersHow to use it
Backwards compatible
Nothing breaks. If you're already using ChatOpenAI or AzureChatOpenAI everything stays the same.
Relates to #56, #65