fix: inject resolved instrument identity into prompts#820
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to resolve and inject deterministic instrument identity (such as company name, sector, and industry) into the agent state using yfinance. This context is now utilized across various analyst, researcher, and manager nodes to ensure consistency and prevent ticker substitution. The reviewer suggests implementing caching for the identity resolution function to improve performance and mitigate potential rate-limiting issues.
| import logging | ||
| from typing import Any, Mapping |
There was a problem hiding this comment.
Import functools to support caching of the instrument identity resolution results, which helps avoid redundant network calls.
| import logging | |
| from typing import Any, Mapping | |
| import logging | |
| import functools | |
| from typing import Any, Mapping |
| return cleaned | ||
|
|
||
|
|
||
| def resolve_instrument_identity(ticker: str) -> dict[str, str]: |
There was a problem hiding this comment.
Resolving instrument identity involves a network call to yfinance, which can be slow and subject to rate limits. Since ticker metadata (like company name, sector, and industry) is generally static, caching these results will significantly improve performance, especially during backtests or when running the graph multiple times for the same ticker across different dates.
Applying @functools.lru_cache here is a safe and effective optimization.
@functools.lru_cache(maxsize=128)
def resolve_instrument_identity(ticker: str) -> dict[str, str]:
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to resolve and inject deterministic instrument identity metadata—such as company name, sector, industry, and exchange—into the agent graph state using the yfinance library. The AgentState was updated to include an instrument_context field, and various analyst, researcher, and manager nodes were modified to utilize this precomputed context in their prompts and logic via the new get_instrument_context_from_state utility. Additionally, comprehensive unit tests were added to verify identity resolution, context building, and state propagation. I have no feedback to provide as no review comments were submitted.
|
Addressed the review suggestion in Testing:
|
1 participant
Summary
Resolves #814
Testing
uv run --with pytest python -m pytest tests/test_ticker_symbol_handling.py tests/test_memory_log.py::TestPortfolioManagerInjection -quv run --with pytest python -m pytest -q