profitmaker icon indicating copy to clipboard operation
profitmaker copied to clipboard

refactor: centralize CCXT instance caching with ccxtInstanceManager

Open suenot opened this issue 6 months ago • 3 comments

Summary

This PR refactors the codebase to use ccxtInstanceManager everywhere for centralized CCXT instance caching, eliminating duplicate caching mechanisms.

Changes Made

🔧 Core Refactoring

  • TestChartWidget.tsx: Replaced direct new ccxt.binance() with ccxtInstanceManager.getExchangeInstanceForMarket()
  • TestTimeframes.tsx: Replaced direct CCXT instantiation with ccxtInstanceManager, includes fallback for comparison
  • ccxtBrowserProvider.ts: Major refactoring to delegate all caching to ccxtInstanceManager

🗑️ Duplicate Code Removal

  • Removed static instancesCache and marketsCache from CCXTBrowserProviderImpl
  • Removed duplicate helper methods: createInstanceKey, isInstanceValid, loadMarketsWithCache
  • Removed duplicate cleanup interval (ccxtInstanceManager already handles this)

🔄 Delegation Implementation

  • getCCXTInstance() now delegates to ccxtInstanceManager.getExchangeInstanceForMarket()
  • Cache management methods (clearCache, cleanup, getCacheStats) delegate to ccxtInstanceManager
  • Maintains API compatibility - existing debug components continue to work seamlessly

🧪 Testing & Validation

  • Added experiments/test-refactoring.js validation script that confirms:
    • ✅ ccxtInstanceManager is properly exported and used
    • ✅ Direct CCXT instantiations limited to fallback cases only
    • ✅ Duplicate caching code successfully removed
    • ✅ All components delegate to unified caching system

Technical Details

Before: Multiple independent caching systems

  • ccxtInstanceManager.ts - Unused centralized cache
  • ccxtBrowserProvider.ts - Own static cache with duplicate logic
  • Test components - Direct instantiation bypassing cache

After: Single unified caching system

  • ccxtInstanceManager - Central authority for all CCXT instances
  • ccxtBrowserProvider - Thin wrapper delegating to ccxtInstanceManager
  • Test components - Use ccxtInstanceManager with fallback for testing

Compatibility Notes

  • CCXT Pro: Maintains fallback logic until ccxtInstanceManager supports Pro version
  • Debug UI: DebugCCXTCache.tsx continues to work unchanged via delegated methods
  • API: All public interfaces remain the same, ensuring backward compatibility

Test Results

🧪 Testing CCXT Instance Manager Refactoring
============================================

✅ ccxtInstanceManager properly exported as singleton
✅ TestChartWidget imports and uses ccxtInstanceManager
✅ TestTimeframes imports and uses ccxtInstanceManager  
✅ ccxtBrowserProvider delegates to ccxtInstanceManager
✅ Duplicate static instancesCache removed
✅ Duplicate cleanup interval removed
✅ Direct CCXT instantiations limited to fallback cases only

🎯 Refactoring Summary:
- ccxtInstanceManager is now used as the central caching mechanism
- Test components use ccxtInstanceManager instead of direct instantiation
- ccxtBrowserProvider delegates to ccxtInstanceManager
- Duplicate caching code has been removed

Fixes #32

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • More reliable exchange setup with automatic fallbacks, improving market, symbol, and timeframe loading.
  • Refactor
    • Centralized exchange instance management and caching for improved stability and performance.
    • Delegated cache operations and cleanup to a single manager, removing duplicates.
    • Converted related flows to async with better error handling and sequential execution where needed.
  • Tests
    • Added an automated validation script that verifies the refactor end-to-end and reports results.

suenot avatar Sep 29 '25 21:09 suenot

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
profitmaker Ready Ready Preview Comment Sep 29, 2025 10:03pm

vercel[bot] avatar Sep 29 '25 21:09 vercel[bot]

Walkthrough

Adds a new experimental validation script and refactors components and the browser provider to centralize CCXT instance management via ccxtInstanceManager. Components adopt async flows with fallback to direct ccxt instantiation. The provider removes local caches and delegates instance/market handling, cache operations, and cleanup to ccxtInstanceManager.

Changes

Cohort / File(s) Summary of changes
Experimental validator script
experiments/test-refactoring.js
Adds automated checks verifying ccxtInstanceManager usage across files, ensuring singleton export, delegation patterns, removal of duplicate caching, and constrained direct ccxt instantiation (fallback-only).
Components integrate manager + async + fallback
src/components/TestChartWidget.tsx, src/components/TestTimeframes.tsx
Switch to async flows; import and use ccxtInstanceManager.getExchangeInstanceForMarket; add try/catch and fallback to direct ccxt.binance instantiation on manager failure; update button handler and test orchestration.
Provider delegates to manager; removes local caches
src/store/providers/ccxtBrowserProvider.ts
Replaces per-instance/markets caching with ccxtInstanceManager delegation; getCCXTInstance uses manager for non-Pro, retains Pro fallback; getMarketsForExchange/getSymbolsForExchange obtain instances via manager; cache ops (invalidate/clear/stats/cleanup) delegated; removes local cache logic and related logs.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant UI as Component (TestChartWidget/TestTimeframes)
  participant M as ccxtInstanceManager
  participant CCXT as ccxt (direct fallback)

  UI->>M: getExchangeInstanceForMarket(exchange, creds, sandbox)
  alt Manager success
    M-->>UI: exchangeInstance
    UI->>UI: Proceed with tests (fetch, timeframes, etc.)
  else Manager failure
    UI->>CCXT: new ccxt.binance(...)\n(fallback)
    CCXT-->>UI: exchangeInstance (direct)
    UI->>UI: Proceed with limited/fallback tests
  end
sequenceDiagram
  autonumber
  participant P as ccxtBrowserProvider
  participant M as ccxtInstanceManager
  participant Pro as getCCXTPro (fallback)
  participant Ex as Exchange Instance

  rect rgb(240,245,255)
  note over P: getCCXTInstance(exchange, opts)
  alt isPro
    P->>Pro: request CCXT Pro
    Pro-->>P: ExchangeClass / instance
    P-->>Ex: return Pro instance (wrapped as needed)
  else regular CCXT
    P->>M: getExchangeInstanceForMarket(exchange, opts)
    M-->>P: instance
    P-->>Ex: return instance
  end
  end

  rect rgb(245,255,245)
  note over P: getMarketsForExchange / getSymbolsForExchange
  P->>M: getExchangeInstanceForMarket(exchange, metadataOpts)
  M-->>P: instance (for markets/capabilities)
  P->>P: derive markets/symbols
  end

  rect rgb(255,248,235)
  note over P: cache ops
  P->>M: clear/cleanup/getStats/invalidate
  M-->>P: delegated results
  end

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

I twitch my nose at caches gone,
One burrow rules the instances on.
If manager slips, I hop fallback,
Still fetch the carrots from the stack.
With tidy trails and markets neat,
This refactor makes our garden sweet. 🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title Check ✅ Passed The title concisely describes the main change—centralizing CCXT instance caching via ccxtInstanceManager—and accurately reflects the core refactoring without extraneous detail or ambiguity.
Linked Issues Check ✅ Passed All code changes fully implement the objectives of issue #32 by centralizing CCXT instance creation in ccxtInstanceManager, removing duplicate caching logic, refactoring components to use the manager with fallbacks, and preserving compatibility for CCXT Pro, fulfilling each requirement.
Out of Scope Changes Check ✅ Passed The refactoring is limited to ccxtBrowserProvider, TestChartWidget.tsx, TestTimeframes.tsx, and the validation script, and no unrelated or extraneous modifications were introduced beyond the scope of issue #32.
Description Check ✅ Passed The pull request description clearly summarizes the refactoring purpose and scope, details all code changes, references the linked issue (#32), and removes the template boilerplate as required, providing technical context and test results for validation.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • [ ] 📝 Generate Docstrings
🧪 Generate unit tests
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment
  • [ ] Commit unit tests in branch issue-32-fd891031

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🧪 Early access (Sonnet 4.5): enabled

We are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience.

Note:

  • Public repositories are always opted into early access features.
  • You can enable or disable early access features from the CodeRabbit UI or by updating the CodeRabbit configuration file.

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Sep 29 '25 21:09 coderabbitai[bot]

🤖 Solution Draft Log

This log file contains the complete execution trace of the AI solution draft process.

📎 Log file uploaded as GitHub Gist (406KB) 🔗 View complete solution draft log


Log automatically attached by solve.mjs with --attach-logs option

suenot avatar Sep 29 '25 22:09 suenot