Skip to content

Conversation

@lukecold
Copy link
Contributor

@lukecold lukecold commented Nov 29, 2025

📝 Pull Request Template

1. Related Issue

Closes # (issue number)

2. Type of Change (select one)

Type of Change: New Feature

3. Description

This pull request introduces 3 major enhancements to the portfolio initialization process:

  1. Initial Position Loading: The system now correctly loads and integrates any existing open positions into the PortfolioView at startup.
  2. Live Position Syncing: The system would sync the current position on each cycle for live tradings, to enable human interventions during the strategy execution.
  3. Robust Data Fetching: A retry mechanism has been implemented for fetching portfolio data to improve reliability against transient network or API errors.

4. Testing

  • I have tested this locally.
  • I have updated or added relevant tests.

5. Checklist

@lukecold lukecold force-pushed the main branch 2 times, most recently from c4fcc06 to ef21f30 Compare November 30, 2025 03:51
@vcfgv
Copy link
Collaborator

vcfgv commented Dec 1, 2025

Thank you for your contribution!

Could you please elaborate a bit on why the Initial Position Loading feature is needed?

We are concerned that introducing this feature might impact the accuracy of return rate calculations and the integrity of shared data. We would love to understand your use case better to ensure these metrics remain accurate.

Looking forward to your reply.

@lukecold
Copy link
Contributor Author

lukecold commented Dec 1, 2025

Thank you for your contribution!

Could you please elaborate a bit on why the Initial Position Loading feature is needed?

We are concerned that introducing this feature might impact the accuracy of return rate calculations and the integrity of shared data. We would love to understand your use case better to ensure these metrics remain accurate.

Looking forward to your reply.

This is meant to initialize the trading strategy with an existing position.

I'm also planning to add position synchronization along with the free cash sync, to enable human interventions while the auto-trade job is running.

Let me know your thoughts!

@vcfgv
Copy link
Collaborator

vcfgv commented Dec 2, 2025

This is meant to initialize the trading strategy with an existing position.

I'm also planning to add position synchronization along with the free cash sync, to enable human interventions while the auto-trade job is running.

Let me know your thoughts!

I understand your point now and it sounds like a great idea.

My initial concern was that human intervention might skew the return rate data shared on the leaderboard, meaning it wouldn't reflect results driven purely by AI decisions. However, from the perspective of personal use without sharing to the leaderboard, the feature you proposed is indeed very valuable.

There are just a few implementation details we need to consider:

  • Data Compatibility: InstrumentRef currently excludes the quote currency, so we need to figure out how to handle the data calculations compatibly.
  • Simulation Logic: If human intervention is introduced in live trading, we need to decide how the paper trading (simulation) mode should handle this.

If you'd like to discuss this in real-time, feel free to join our Discord group.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request enhances the portfolio initialization system to support loading existing positions at startup and improves data fetching reliability through retry mechanisms. The changes refactor the initial_capital concept into two distinct fields (initial_free_cash and initial_total_cash) to better represent the portfolio state when positions already exist.

Key Changes:

  • Splits initial_capital into initial_free_cash (available cash) and initial_total_cash (total cash including margin in positions)
  • Adds fetch_positions_from_gateway() function to load existing positions from exchange at startup
  • Implements retry logic with exponential backoff for both balance and position fetching
  • Updates portfolio initialization to include existing positions, exposures, and unrealized PnL

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
python/valuecell/server/services/strategy_service.py Updates performance calculation to use initial_total_cash instead of initial_capital for ROI computation
python/valuecell/server/services/strategy_persistence.py Refactors persistence functions to handle both initial_free_cash and initial_total_cash fields
python/valuecell/server/api/schemas/strategy.py Updates API schema to rename field from initial_capital to initial_total_cash
python/valuecell/server/api/routers/strategy.py Updates API documentation string for consistency
python/valuecell/agents/common/trading/utils.py Adds retry mechanism for balance/position fetching and new fetch_positions_from_gateway() function
python/valuecell/agents/common/trading/portfolio/in_memory.py Enhances portfolio initialization to support initial positions with calculated exposures and PnL
python/valuecell/agents/common/trading/models.py Extends TradingConfig model with new fields for free cash, total cash, and initial positions
python/valuecell/agents/common/trading/_internal/stream_controller.py Updates initial state persistence to handle both free and total cash values
python/valuecell/agents/common/trading/_internal/runtime.py Integrates position fetching at startup and initializes portfolio with existing positions
python/valuecell/agents/common/trading/_internal/coordinator.py Updates equity calculation fallback logic to use initial_total_cash
python/valuecell/agents/common/trading/README.md Updates documentation to reflect new initial capital fields
frontend/src/types/system.ts Updates TypeScript types to rename initial_capital to initial_total_cash
frontend/src/types/strategy.ts Extends strategy types with initial_free_cash and initial_total_cash fields
frontend/src/app/rank/board.tsx Updates UI to display initial_total_cash instead of initial_capital
frontend/src/app/agent/components/strategy-items/modals/create-strategy-modal.tsx Refactors form to use initial_capital_input and map to both cash fields
frontend/src/app/agent/components/strategy-items/forms/trading-strategy-form.tsx Updates form field name to initial_capital_input for consistency

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@lukecold lukecold requested a review from vcfgv December 4, 2025 08:34
@vcfgv
Copy link
Collaborator

vcfgv commented Dec 4, 2025

I've just commented in the unresolved conversation. Because we have specific dev cycles, please open an issue first for significant features in the future. We need to coordinate the merge timing with our iteration plan.

…etching Reliability

This pull request introduces two major enhancements to the portfolio initialization process:
1. Initial Position Loading: The system now correctly loads and integrates any existing open positions into the PortfolioView at startup.
2. Live Position Syncing: The system would sync the current position on each cycle for live tradings, to enable human interventions during the strategy execution.
3. Robust Data Fetching: A retry mechanism has been implemented for fetching portfolio data to improve reliability against transient network or API errors.
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.

2 participants