Skip to content

Commit 50107f1

Browse files
docs: address code review feedback
- Remove unused time import - Add documentation about cache growth and memory considerations - Document design decision not to cache error responses - Add comments explaining rationale for error handling approach Co-authored-by: merendamattia <[email protected]>
1 parent 027f691 commit 50107f1

File tree

1 file changed

+11
-1
lines changed

1 file changed

+11
-1
lines changed

src/tools/analyze_financial_asset.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
"""
77

88
import logging
9-
import time
109
from datetime import datetime, timedelta
1110
from typing import Any, Dict, Optional
1211

@@ -19,6 +18,12 @@
1918
logger = logging.getLogger(__name__)
2019

2120
# Cache for storing financial analysis results (session-level)
21+
# Note: This cache grows indefinitely. In production, consider implementing:
22+
# - Cache size limits (e.g., LRU eviction)
23+
# - TTL-based expiration for stale data
24+
# - Periodic cleanup in long-running applications
25+
# For typical Streamlit sessions, this is acceptable as the cache is cleared
26+
# on app restart and session_state provides the primary cache layer.
2227
_CACHE = {}
2328

2429

@@ -240,6 +245,11 @@ def analyze_financial_asset(
240245
result_json = response.model_dump_json()
241246

242247
# Cache the successful result
248+
# Note: Error responses are intentionally NOT cached because:
249+
# 1. Transient errors (network issues, API rate limits) should be retried
250+
# 2. Invalid tickers fail fast (symbol resolution is quick)
251+
# 3. Caching errors could hide resolved issues
252+
# If error caching is needed, implement with short TTL and proper invalidation
243253
if use_cache:
244254
_set_cached_analysis(ticker, years, result_json)
245255

0 commit comments

Comments
 (0)