Skip to content

fix: consolidate all unmerged bug fixes from orphaned worktree branches#114

Closed
rpwallin wants to merge 17 commits intowshobson:mainfrom
rpwallin:claude/cool-lumiere
Closed

fix: consolidate all unmerged bug fixes from orphaned worktree branches#114
rpwallin wants to merge 17 commits intowshobson:mainfrom
rpwallin:claude/cool-lumiere

Conversation

@rpwallin
Copy link

@rpwallin rpwallin commented Mar 15, 2026

Summary

Consolidates unique fixes from 6 orphaned worktree branches that were each fixed independently but never merged to main — causing the same bugs to be re-fixed every session.

Bug fixes (17 fixes across 19 files)

  • Portfolio: correct metric key names, safe .get() access, correlation column fix
  • Chart: seaborn-v0_8 with fallback + Agg backend for headless
  • Screening: days_back default 1→30 (prevents empty results)
  • SQLAlchemy: wrap raw SQL with text() (3 files)
  • Sentiment: low confidence returns neutral
  • Health tools: handle Pydantic model_dump()
  • Data: normalize timezone-aware timestamps
  • Technical: missing pandas import
  • Performance: SQLite guard for PostgreSQL-only queries

New features

  • Finnhub provider as backup data source with fallback chain
  • Agent timeout protection (25s default, depth-based for research)
  • Auto-seed S&P 500 screening data on first startup
  • Market overview: graceful partial failure handling

Windows compatibility

  • testcontainers imports made optional
  • resource module guarded (Unix-only)
  • UTF-8 encoding for test file reads
  • Logging handler cleanup in conftest teardown

Source branches consolidated

Branch Unique fixes merged
dreamy-edison Auto-seed S&P 500, matplotlib Agg backend
dreamy-mahavira Windows test compat (testcontainers, resource, encoding)
silly-albattani Screening days_back 1→30
eager-hertz Timestamp normalization, sentiment, SQLAlchemy text(), Finnhub provider
thirsty-hertz Agent timeout protection, bind_tools guard
infallible-driscoll Correlation column fix

Test plan

  • make test passes
  • Portfolio endpoint works with correct metric keys
  • Chart generation works without matplotlib warnings
  • Screening returns results (days_back=30)
  • Agent calls timeout gracefully instead of hanging

🤖 Generated with Claude Code

rpwallin and others added 12 commits March 12, 2026 15:14
Screener enhancements:
- Regime-gated screener: auto-detect SPY regime and filter results
- EARS sector rotation scoring: relative strength vs SPY + sector ETF
- Multi-timeframe confirmation: daily+weekly confluence scoring

Portfolio & risk enhancements:
- Kelly criterion position sizer from backtest win/loss stats
- HRP portfolio optimizer via riskfolio-lib

Data & infrastructure:
- Pre-market cache warm-up tool for portfolio holdings
- Backtest persistence wiring with persist=True flag and history query

Agent & LLM enhancements:
- Haiku fast path for sentiment/quick-answer tasks
- Structured JSON output for market analysis agents
- Redis-backed agent session persistence with in-memory fallback

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add two major features:

1. WebSocket-Based Real-Time Price Streaming (#4):
   - PriceStreamManager singleton with asyncio polling loop
   - /ws/prices WebSocket endpoint with subscribe/unsubscribe protocol
   - 7 MCP tools (streaming_*) for stream control
   - Alert delta detection (only broadcasts state changes)
   - STDIO-compatible price snapshot fallback
   - Per-connection rate limiting and dead connection cleanup

2. Options Chain Analysis & Greeks Calculator (#5):
   - Full European Greeks (1st-3rd order) via blackscholes library
   - American option pricing via Barone-Adesi Whaley (OptionsPricerLib)
   - Options chain fetching with liquidity filtering (bid-ask spread, volume, OI)
   - IV skew/smile analysis and term structure
   - Multi-leg strategy P&L profiles (covered call, spreads, iron condor, etc.)
   - Unusual options activity detection (volume/OI spikes, put/call ratios)
   - Portfolio-aware hedging recommendations
   - 7 MCP tools (options_*) registered in tool registry
   - 56 unit tests covering core analysis, validation, and data provider

Also includes code quality improvements: removed deprecated DDD/parallel
router files, added CI workflow, watchlist monitor, sentiment analyzer,
and fundamental analysis modules.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add Finnhub free-tier integration providing earnings calendars, analyst
recommendations, institutional ownership (13F), company/market news,
economic calendar, and backup quotes. Finnhub company news also serves
as a fallback in the news sentiment pipeline when Tiingo is unavailable.

New files:
- providers/finnhub_data.py: FinnhubDataProvider with TTL cache and
  token-bucket rate limiter (60 calls/min)
- validation/finnhub.py: 8 Pydantic validation models
- api/routers/finnhub.py: 8 async MCP tool functions
- tests/test_finnhub_provider.py: 20 provider unit tests
- tests/test_finnhub_validation.py: 23 validation tests

Modified:
- settings.py: FinnhubSettings config class
- circuit_breaker_services.py: FINNHUB_CONFIG + FinnhubCircuitBreaker
- circuit_breaker_decorators.py: with_finnhub_circuit_breaker decorator
- tool_registry.py: register_finnhub_tools registration
- news_sentiment_enhanced.py: Finnhub as fallback news source
- pyproject.toml: finnhub-python dependency
- .env.example: FINNHUB_API_KEY entry

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Wrap raw SQL strings with text() in 3 locations to comply with
SQLAlchemy 2.0. Add credential masking via mask_url() for database
and Redis connection URLs. Protect API keys in agent initialization
error paths using `from None` exception chaining. Apply CSP and
Permissions-Policy headers in security middleware. Fix ticker
validation to accept dots/hyphens (BRK.B, BF-A).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add periodic background cache cleanup service (every 5 min) sweeping
YFinancePool, FinnhubDataProvider, and QuickCache for expired entries.
Register atexit handlers for SQLAlchemy engine disposal and all
ThreadPoolExecutor shutdowns to prevent resource leaks. Replace
imprecise asyncio.sleep rate limiting with aiolimiter AsyncLimiter
(leaky-bucket) for Finnhub, Tiingo, and general API calls. Bound
FinnhubDataProvider cache at 500 entries with expired-first eviction.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add 14 new test files covering all previously untested API routers and
core analysis modules to reduce regression risk and improve coverage:

Routers: tool_registry, data, screening, portfolio, performance,
health_tools, technical_enhanced, introspection, mcp_prompts,
news_sentiment_enhanced, intelligent_backtesting

Core modules: relative_strength (EARS scoring), regime_gate (market
regime detection), portfolio_optimization (HRP optimization)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add declarative Pandera schemas (OHLCV, OHLCVLowercase,
TechnicalIndicators) to validate DataFrames at 6 ingestion points:
yfinance fetch, cache write, StockDataFetchingService, optimized
provider, PriceCache, and technical indicators output.

Replaces ~80 lines of manual validation in DataValidator with Pandera
delegation. Non-strict by default (warns, doesn't crash); set
PANDERA_STRICT=true for strict mode. Includes 29 schema tests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace ~1,287 lines of custom circuit breaker implementation with the
battle-tested `circuitbreaker` library (v2.1.3). The adapter pattern
preserves all existing imports and APIs via backward-compatible facades.

Key changes:
- New `circuit_breaker_adapter.py`: Core adapter wrapping the library
  with MaverickCircuitBreaker, CircuitBreakerMetrics, and global registry
- `circuit_breaker.py`: Rewritten as facade (~310 lines, down from 945)
- `circuit_breaker_decorators.py`: Simplified (~120 lines, down from 366)
- `circuit_breaker_services.py`: Delegates to adapter via get_or_create_breaker
- Added circuitbreaker>=2.1.3 and tenacity>=9.1.4 as explicit dependencies
- Updated tests to work with adapter-based implementation (25 tests passing)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix column case bug in portfolio router (yfinance Title-case vs lowercase)
- Add division-by-zero guards for cost_basis and total_cost in P&L calculations
- Replace mutable dict defaults with Field(default_factory=dict) in agents/base.py
- Replace all eval() calls with json.loads() in 3 test files (17 occurrences)
- Add ticker validation and batch size limit (50) to data router
- Change price_caches relationship from eager to lazy loading
- Create utils/error_handling.py with safe_error_message() sanitization

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
M1: Apply safe_error_message() across all 16 routers (73 sites sanitized)
    - Prevents leaking internal paths, connection strings, and stack traces
    - Only data.py ValueError catches from TickerValidator kept as str(e)

M3: Deduplicate convert_param() in backtesting.py
    - Extracted 3 identical nested definitions to single module-level helper

M4: Add return type annotations to 24 functions
    - mcp_prompts.py: 11 prompt/resource handlers
    - monitoring.py: 13 route handlers and dependencies

M5: Add atexit cleanup to 2 ThreadPoolExecutors
    - data.py and technical_enhanced.py were missing shutdown hooks

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…de, update docs

Convert ~106 f-string logging calls to lazy % formatting across 17 router files
to avoid unnecessary string interpolation when log level is disabled. Add ruff G
rule to prevent regression. Remove 3 unused SSE variant files and commented-out
middleware block (451 lines of dead code). Fix 8 G201 violations by converting
.error(exc_info=True) to .exception(). Update README.md transport guidance to
match CLAUDE.md and document SSE monkey-patch removal criteria.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Use .get() for total_current_value to prevent KeyError when metrics are incomplete
- Replace deprecated "seaborn" style with "default" (removed in matplotlib 3.6+)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Merges unique fixes from 6 orphaned branches (dreamy-edison, dreamy-mahavira,
silly-albattani, eager-hertz, thirsty-hertz, infallible-driscoll) that were
each fixed independently but never landed on main.

Bug fixes:
- Portfolio: correct metric key names (total_value/total_pnl/total_pnl_percentage)
- Portfolio: safe .get() access to prevent KeyError on missing metrics
- Portfolio: fix correlation column name (close -> Close)
- Chart: seaborn-v0_8 with fallback to default + Agg backend for headless
- Screening: days_back default 1 -> 30 (prevents empty results)
- SQLAlchemy: wrap raw SQL strings with text() (3 files)
- Sentiment: low confidence returns neutral instead of false positive
- Health tools: handle Pydantic model_dump for resource_usage
- Data: normalize timezone-aware timestamps to UTC date-only
- Technical: add missing pandas import
- Performance: SQLite guard for PostgreSQL-only pg_stats queries
- Stock data: add exc_info to error logs

New features:
- Finnhub provider as backup data source with fallback chain
- Finnhub tools (quote, profile, earnings, financials)
- Agent timeout protection (25s default, depth-based for research)
- Agent bind_tools guard for FakeListLLM compatibility
- Agent creation error handling with helpful API key messages
- Market overview: graceful partial failure handling
- Auto-seed S&P 500 screening data on first startup

Windows compatibility:
- testcontainers imports made optional
- resource module import guarded (Unix-only)
- UTF-8 encoding for test file reads
- Logging handler cleanup in conftest teardown

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@rpwallin rpwallin changed the title fix: portfolio KeyError and deprecated matplotlib style fix: consolidate all unmerged bug fixes from orphaned worktree branches Mar 15, 2026
Applies all remaining improvements that were missed in the first pass:

From eager-hertz:
- RSI neutral zone (45-55) instead of binary bullish/bearish at 50
- Double bottom/top mutual exclusion (prevents contradictory signals)
- Trade duration: convert timedelta to days (float) for readable output
- SQLite guard in tools/performance_monitoring.py for pg_stat queries
- Moving averages (EMA 21, SMA 20/50/200) in technical analysis output

From thirsty-hertz:
- market_service.py: use existing get_market_overview_async() instead of
  calling 4 nonexistent methods (get_major_indices_async, etc.)
- Screening: add hint when tables are empty (3 endpoints)

From infallible-driscoll:
- Per-agent timeouts (25s) in compare_multi_agent_analysis
- Per-persona timeouts with error isolation in compare_personas_analysis

Also fixes:
- SQLAlchemy text() in test_production_validation.py and test_session_management.py

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment on lines +1476 to +1481
result = subprocess.run(
[sys.executable, "scripts/seed_sp500.py"],
capture_output=True,
text=True,
timeout=120,
)
Copy link

Choose a reason for hiding this comment

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

Bug: The subprocess call to scripts/seed_sp500.py uses a relative path, which will fail if the server is started from a directory other than the project root.
Severity: HIGH

Suggested Fix

Construct an absolute path to the script before calling subprocess.run. Use pathlib.Path and __file__ to determine the project root and build a full, reliable path to scripts/seed_sp500.py. This will ensure the script is found regardless of the current working directory.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: maverick_mcp/api/server.py#L1476-L1481

Potential issue: The auto-seed feature in `maverick_mcp/api/server.py` invokes a script
using `subprocess.run` with a relative path: `"scripts/seed_sp500.py"`. This path is
resolved relative to the current working directory. If the server is started from a
directory other than the project root, which is a common scenario for users invoking it
via `claude mcp add`, the subprocess will fail to find the script. The resulting
exception is caught and logged, causing the auto-seed feature to fail silently. This
leads to users seeing empty screening results, breaking a core feature of the pull
request.

…, CI)

Merge claude/keen-jones branch with comprehensive conflict resolution:
- Resolved 5 conflicts (agents, data, portfolio, screening, test_stress)
- Preserved portfolio metric key fix (.get("total_value") etc.)
- Preserved visualization seaborn-v0_8 fallback (no regression)
- Preserved days_back=30 default (no regression)
- Added empty-table hints to bear and breakout screening endpoints
- Kept keen-jones additions: finnhub router, options router, CI workflow,
  pandera validation, circuitbreaker, AgentFactory refactor, pre-commit

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
rpwallin and others added 2 commits March 15, 2026 12:27
From thirsty-hertz:
- Add asyncio.wait_for timeout protection to all 6 agent endpoints
  (25s default, depth-based for deep research: 30s-180s)
- Add timeout to market_data._run_in_executor (15s default)

From eager-hertz:
- Add UTC timestamp normalization in data.py (single + batch fetch)

From silly-albattani:
- Add exc_info=True to portfolio correlation warning log

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…k warning

- Rename first register_finnhub_tools to register_finnhub_core_tools to
  avoid shadowing by keen-jones version (both now register their tools)
- Enhance FakeListLLM warning with specific API key names and bind_tools note

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment on lines +1626 to +1630
weekly_data["Open"] = data["Open"].resample("W").first()
weekly_data["High"] = data["High"].resample("W").max()
weekly_data["Low"] = data["Low"].resample("W").min()
weekly_data["Close"] = data["Close"].resample("W").last()
weekly_data["Volume"] = data["Volume"].resample("W").sum()
Copy link

Choose a reason for hiding this comment

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

Bug: The run_backtest_mtf method will crash with a KeyError because it accesses DataFrame columns with Title-case names ("Open") after they have been converted to lowercase ("open").
Severity: CRITICAL

Suggested Fix

In the run_backtest_mtf method, change the column accessors to use lowercase names to match the data returned by get_historical_data. For example, change data["Open"] to data["open"] and weekly_data["Close"] to weekly_data["close"].

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: maverick_mcp/backtesting/vectorbt_engine.py#L1626-L1630

Potential issue: The `run_backtest_mtf` method calls `get_historical_data`, which
returns a pandas DataFrame with column names normalized to lowercase (e.g., `open`,
`close`). However, subsequent code in `run_backtest_mtf` attempts to access these
columns using Title-case names (e.g., `data["Open"]`, `data["Close"]`). This mismatch
will cause a `KeyError` every time the function is executed, preventing the
multi-timeframe backtesting functionality from working.

@rpwallin
Copy link
Author

Closing — merging locally instead.

@rpwallin rpwallin closed this Mar 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant