Support Weave tracer in TracerTraceToTriplet#415
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for the Weave tracer in the TracerTraceToTriplet adapter, enabling Weave-based traces to be converted to reinforcement learning triplets. The changes extend attribute extraction to support both AgentOps and Weave tracer formats.
- Refactored test suite to use pytest fixtures for parametrized agent testing across different tracers
- Enhanced
TracerTraceToTripletadapter with multi-tracer attribute extraction supporting both AgentOps and Weave formats - Improved WeaveTracer with callback deduplication and better error handling
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/tracer/test_integration.py | Refactored tests to use pytest fixtures, removed conditional imports, updated reward emission to use new API, added debug output for triplets |
| agentlightning/adapter/triplet.py | Added helper functions for multi-tracer attribute extraction, extended agent_name and reward detection for Weave, updated span_to_triplet with Weave-specific attribute paths |
| agentlightning/tracer/weave.py | Added callback invocation tracking to prevent duplicate calls, added clear() method for cleanup |
| agentlightning/utils/otel.py | Added force parameter to sanitize functions for flexible JSON serialization |
| agentlightning/semconv.py | Added AGL_REWARD constant for standardized reward span naming |
| agentlightning/instrumentation/weave.py | Added recursive instrumentation detection and warning |
| pyproject.toml | Added langchain pytest marker |
| .gitignore | Extended debug file patterns to include JSON files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "store: tests for agentlightning.store module", | ||
| "prometheus: tests that require Prometheus", | ||
| "utils: tests for utility functions", | ||
| "langchain: tests that require LangChain", |
There was a problem hiding this comment.
Missing pytest marker definition for 'langgraph'. The test fixture at line 685 in tests/tracer/test_integration.py uses pytest.mark.langgraph, but this marker is not defined in the markers list. This will cause pytest to issue warnings about unknown markers. Add 'langgraph: tests that require LangGraph' to the markers list.
| "langchain: tests that require LangChain", | |
| "langchain: tests that require LangChain", | |
| "langgraph: tests that require LangGraph", |
tests/tracer/test_integration.py
Outdated
| from langchain.agents import create_agent # pyright: ignore[reportUnknownVariableType] | ||
| from langchain.chat_models import init_chat_model | ||
| from langchain_community.agent_toolkits import SQLDatabaseToolkit | ||
| from langchain_community.utilities import SQLDatabase | ||
| from langchain_core.messages import AIMessage | ||
| from langchain_core.output_parsers import StrOutputParser | ||
| from langchain_core.prompts import ChatPromptTemplate | ||
| from langchain_core.tools import tool # pyright: ignore[reportUnknownVariableType] | ||
| from langchain_openai import ChatOpenAI | ||
| from langgraph.graph import END, START, MessagesState, StateGraph |
There was a problem hiding this comment.
Unconditional LangChain/LangGraph imports will cause ImportError when these packages are not installed. The previous code had conditional imports with TYPE_CHECKING and LANGCHAIN_INSTALLED flag to handle optional dependencies. While pytest markers have been added to skip tests, the imports at module level will still fail before tests can be skipped. Consider wrapping these imports in a try-except block or making them conditional imports, or ensure these packages are listed as test dependencies.
| from langchain.agents import create_agent # pyright: ignore[reportUnknownVariableType] | |
| from langchain.chat_models import init_chat_model | |
| from langchain_community.agent_toolkits import SQLDatabaseToolkit | |
| from langchain_community.utilities import SQLDatabase | |
| from langchain_core.messages import AIMessage | |
| from langchain_core.output_parsers import StrOutputParser | |
| from langchain_core.prompts import ChatPromptTemplate | |
| from langchain_core.tools import tool # pyright: ignore[reportUnknownVariableType] | |
| from langchain_openai import ChatOpenAI | |
| from langgraph.graph import END, START, MessagesState, StateGraph | |
| # Optional LangChain and LangGraph imports | |
| LANGCHAIN_INSTALLED = False | |
| LANGGRAPH_INSTALLED = False | |
| try: | |
| from langchain.agents import create_agent # pyright: ignore[reportUnknownVariableType] | |
| from langchain.chat_models import init_chat_model | |
| from langchain_community.agent_toolkits import SQLDatabaseToolkit | |
| from langchain_community.utilities import SQLDatabase | |
| from langchain_core.messages import AIMessage | |
| from langchain_core.output_parsers import StrOutputParser | |
| from langchain_core.prompts import ChatPromptTemplate | |
| from langchain_core.tools import tool # pyright: ignore[reportUnknownVariableType] | |
| from langchain_openai import ChatOpenAI | |
| LANGCHAIN_INSTALLED = True | |
| except ImportError: | |
| pass | |
| try: | |
| from langgraph.graph import END, START, MessagesState, StateGraph | |
| LANGGRAPH_INSTALLED = True | |
| except ImportError: | |
| pass |
tests/tracer/test_integration.py
Outdated
| AGENTOPS_EXPECTED_REWARDS[name] = [None, None, None, 1.0, None, None] # type: ignore | ||
|
|
||
| async with MockOpenAICompatibleServer() as settings: |
There was a problem hiding this comment.
Modifying shared test configuration dictionaries during test execution creates test interdependency and state pollution. AGENTOPS_EXPECTED_TRIPLETS_NUMBER and AGENTOPS_EXPECTED_REWARDS are module-level constants that should not be mutated. When tests run in parallel or in different orders, this mutation will cause unpredictable failures. Create a local copy or use a separate configuration for Weave-specific expectations instead of modifying the shared constant.
| class EvalHook(AgentHooks): | ||
| @reward | ||
| def evaluate(self, context: Any, agent: Agent, output: Any): | ||
| # Custom reward logic: reward if the answer contains 'homework' | ||
| return 1.0 if output and "no" in str(output).lower() else 0.0 | ||
|
|
||
| async def on_end(self, context: Any, agent: Agent, output: Any): | ||
| nonlocal final_reward | ||
| final_reward = final_reward or self.evaluate(context, agent, output) | ||
| # Custom reward logic: reward if the answer contains 'no' | ||
| final_reward = 1.0 if output and "no" in str(output).lower() else 0.0 | ||
| emit_reward(final_reward) | ||
| final_rewards.append(final_reward) |
There was a problem hiding this comment.
Variable 'final_rewards' is referenced in the EvalHook.on_end method at line 430 before it is defined at line 455. This will cause a NameError when the on_end hook is executed. The variable should be defined before the class definition or passed through the class initialization.
agentlightning/adapter/triplet.py
Outdated
| if (isinstance(attributes[key], list) or isinstance(attributes[key], tuple)) and all( | ||
| isinstance(x, int) for x in attributes[key] | ||
| ): | ||
| return attributes[key] |
There was a problem hiding this comment.
The return type annotation indicates this function returns Optional[List[int]], but at line 46 when an attribute is found, it returns attributes[key] which could be either a list or tuple. This creates a type inconsistency since tuple is not List[int]. Consider converting tuples to lists before returning, or update the return type to Optional[Union[List[int], Tuple[int, ...]]].
| return attributes[key] | |
| return list(attributes[key]) |
|
/ci |
|
🚀 CI Watcher for correlation id-3660589983-mj8mv5v8 triggered by comment 3660589983
✅ All runs completed. |
1 participant
No description provided.