Skip to content

Conversation

@the-dev-z
Copy link
Collaborator

Problem

When editing trader configuration, if system_prompt_template was set to an empty string (""), the UI would incorrectly treat it as falsy and overwrite it with 'default', losing the user's selection.

Root cause:

if (traderData && !traderData.system_prompt_template) {
  // ❌ This triggers for both undefined AND empty string ""
  setFormData({ system_prompt_template: 'default' });
}

JavaScript falsy values that trigger ! operator:

  • undefined ✅ Should trigger default
  • null ✅ Should trigger default
  • "" ❌ Should NOT trigger (user explicitly chose empty)
  • false, 0, NaN (less relevant here)

Solution

Change condition to explicitly check for undefined:

if (traderData && traderData.system_prompt_template === undefined) {
  // ✅ Only triggers for truly missing field
  setFormData({ system_prompt_template: 'default' });
}

Impact

  • ✅ Empty string selections are preserved
  • ✅ Legacy data (undefined) still gets default value
  • ✅ User's explicit choices are respected
  • ✅ No breaking changes to existing functionality

Testing

  • ✅ Code compiles
  • ⚠️ Requires manual UI testing:
    • Edit trader with empty system_prompt_template
    • Verify it doesn't reset to 'default'
    • Create new trader → should default to 'default'
    • Edit old trader (undefined field) → should default to 'default'

Code Changes

web/src/components/TraderConfigModal.tsx:
- Line 99: Changed !traderData.system_prompt_template → === undefined

📝 Description | 描述

English:
Fixed critical bug where AI was calculating position_size_usd incorrectly, treating it as margin requirement instead of notional value, causing frequent code=-2019 errors (insufficient margin).

中文:
修復關鍵 bug:AI 錯誤計算 position_size_usd,將其視為保證金需求而非名義價值,導致頻繁出現 code=-2019 錯誤(保證金不足)。


🎯 Type of Change | 變更類型

  • 🐛 Bug fix | 修復 Bug(不影響現有功能的修復)
  • ✨ New feature | 新功能(不影響現有功能的新增)
  • 💥 Breaking change | 破壞性變更(會導致現有功能無法正常工作的修復或功能)
  • 📝 Documentation update | 文檔更新
  • 🎨 Code style update | 代碼樣式更新(格式化、重命名等)
  • ♻️ Refactoring | 重構(無功能變更)
  • ⚡ Performance improvement | 性能優化
  • ✅ Test update | 測試更新
  • 🔧 Build/config change | 構建/配置變更
  • 🔒 Security fix | 安全修復

🔗 Related Issues | 相關 Issue

  • Fixes code=-2019 (insufficient margin) errors
  • Related to trading execution failures

📋 Changes Made | 具體變更

English:

  1. Updated AI prompts with correct formula (prompts/adaptive.txt, nof1.txt, default.txt):

    • Clarified that position_size_usd is notional value (includes leverage)
    • Added step-by-step calculation: Available Margin × Leverage = position_size_usd
    • Example: $500 cash × 5x leverage = $2,375 notional value (not $500)
  2. Added code-level validation (trader/auto_trader.go):

    • Validates required margin + fees ≤ available balance before opening positions
    • Prevents execution if insufficient funds
    • Returns clear error message with breakdown

中文:

  1. 更新 AI 提示詞公式 (prompts/adaptive.txt, nof1.txt, default.txt):

    • 明確說明 position_size_usd名義價值(包含杠杆)
    • 添加分步計算:可用保證金 × 杠杆 = position_size_usd
    • 示例:$500 資金 × 5倍杠杆 = $2,375 名義價值(非 $500)
  2. 新增代碼級驗證 (trader/auto_trader.go):

    • 開倉前驗證「所需保證金 + 手續費 ≤ 可用餘額」
    • 資金不足時阻止執行
    • 返回清晰的錯誤信息(含明細)

🧪 Testing | 測試

Manual Testing | 手動測試

  • Tested locally | 本地測試通過
  • Tested on testnet | 測試網測試通過(交易所集成相關)
  • Tested with different configurations | 測試了不同配置
  • Verified no existing functionality broke | 確認沒有破壞現有功能

Test Environment | 測試環境

  • OS | 操作系統: macOS
  • Go Version | Go 版本: 1.21+
  • Exchange | 交易所: Binance Futures

Test Results | 測試結果

✅ go build ./... - Compilation successful
⚠️  Requires live trading validation

✅ Checklist | 檢查清單

Code Quality | 代碼質量

  • My code follows the project's code style | 我的代碼遵循項目代碼風格
  • I have performed a self-review of my code | 我已進行代碼自查
  • I have commented my code, particularly in hard-to-understand areas | 我已添加代碼注釋
  • My changes generate no new warnings or errors | 我的變更沒有產生新的警告或錯誤
  • Code compiles successfully | 代碼編譯成功 (go build)

Testing | 測試

  • I have added tests that prove my fix is effective or that my feature works | 我已添加證明修復有效的測試
  • New and existing unit tests pass locally | 新舊單元測試在本地通過
  • I have tested on testnet (for trading/exchange changes) | 我已在測試網測試(交易/交易所變更)

Documentation | 文檔

  • I have updated the documentation accordingly | 我已相應更新文檔(提示詞文件)
  • I have added inline code comments where necessary | 我已在必要處添加代碼注釋

Git

  • My commits follow the conventional commits format | 我的提交遵循 Conventional Commits 格式 (fix:)
  • I have rebased my branch on the latest dev branch | 我已將分支 rebase 到最新的 dev 分支
  • There are no merge conflicts | 沒有合併衝突
  • Commit messages are clear and descriptive | 提交信息清晰明確

📋 PR Size Estimate | PR 大小估計

  • 🟢 Small (< 100 lines) | 小(< 100 行)
  • 🟡 Medium (100-500 lines) | 中(100-500 行)
  • 🔴 Large (> 500 lines) | 大(> 500 行)

🎯 Review Focus Areas | 審查重點

Please pay special attention to:
請特別注意:

  • Logic changes | 邏輯變更
  • Security implications | 安全影響
  • Performance optimization | 性能優化
  • API changes | API 變更

Specific areas:

  • Formula correctness in prompt files
  • Margin validation logic in executeOpenLong/ShortWithRecord

By submitting this PR, I confirm that:
提交此 PR,我確認:

  • I have read the Contributing Guidelines | 我已閱讀貢獻指南
  • My contribution is licensed under the AGPL-3.0 License | 我的貢獻遵循 AGPL-3.0 許可證
  • I understand this is a voluntary contribution | 我理解這是自願貢獻
  • I have the right to submit this code | 我有權提交此代碼

… string

## Problem
When editing trader configuration, if `system_prompt_template` was set to an empty string (""), the UI would incorrectly treat it as falsy and overwrite it with 'default', losing the user's selection.

**Root cause:**
```tsx
if (traderData && !traderData.system_prompt_template) {
  // ❌ This triggers for both undefined AND empty string ""
  setFormData({ system_prompt_template: 'default' });
}
```

JavaScript falsy values that trigger `!` operator:
- `undefined` ✅ Should trigger default
- `null` ✅ Should trigger default
- `""` ❌ Should NOT trigger (user explicitly chose empty)
- `false`, `0`, `NaN` (less relevant here)

## Solution

Change condition to explicitly check for `undefined`:

```tsx
if (traderData && traderData.system_prompt_template === undefined) {
  // ✅ Only triggers for truly missing field
  setFormData({ system_prompt_template: 'default' });
}
```

## Impact
- ✅ Empty string selections are preserved
- ✅ Legacy data (undefined) still gets default value
- ✅ User's explicit choices are respected
- ✅ No breaking changes to existing functionality

## Testing
- ✅ Code compiles
- ⚠️ Requires manual UI testing:
  - [ ] Edit trader with empty system_prompt_template
  - [ ] Verify it doesn't reset to 'default'
  - [ ] Create new trader → should default to 'default'
  - [ ] Edit old trader (undefined field) → should default to 'default'

## Code Changes
```
web/src/components/TraderConfigModal.tsx:
- Line 99: Changed !traderData.system_prompt_template → === undefined
```
@github-actions
Copy link

github-actions bot commented Nov 4, 2025

🤖 Advisory Check Results

These are advisory checks to help improve code quality. They won't block your PR from being merged.

📋 PR Information

Title Format: ✅ Good - Follows Conventional Commits
PR Size: 🟢 Small (2 lines: +1 -1)

🔧 Backend Checks

Go Formatting: ⚠️ Needs formatting

Files needing formatting
api/server.go
config/database.go
main.go
manager/trader_manager.go

Go Vet: ✅ Good
Tests: ✅ Passed

Fix locally:

go fmt ./...      # Format code
go vet ./...      # Check for issues
go test ./...     # Run tests

⚛️ Frontend Checks

Build & Type Check: ✅ Success

Fix locally:

cd web
npm run build  # Test build (includes type checking)

📖 Resources

Questions? Feel free to ask in the comments! 🙏


These checks are advisory and won't block your PR from being merged. This comment is automatically generated from pr-checks-run.yml.

@github-actions
Copy link

github-actions bot commented Nov 5, 2025

🤖 Advisory Check Results

These are advisory checks to help improve code quality. They won't block your PR from being merged.

📋 PR Information

Title Format: ✅ Good - Follows Conventional Commits
PR Size: 🟢 Small (4 lines: +2 -2)

🔧 Backend Checks

Go Formatting: ⚠️ Needs formatting

Files needing formatting
decision/engine.go
logger/decision_logger.go
logger/telegram_sender.go
mcp/client.go
trader/aster_trader.go
trader/hyperliquid_trader.go

Go Vet: ✅ Good
Tests: ✅ Passed

Fix locally:

go fmt ./...      # Format code
go vet ./...      # Check for issues
go test ./...     # Run tests

⚛️ Frontend Checks

Build & Type Check: ✅ Success

Fix locally:

cd web
npm run build  # Test build (includes type checking)

📖 Resources

Questions? Feel free to ask in the comments! 🙏


These checks are advisory and won't block your PR from being merged. This comment is automatically generated from pr-checks-run.yml.

@Icyoung Icyoung merged commit c859736 into NoFxAiOS:dev Nov 5, 2025
8 of 11 checks passed
ForeverInLaw pushed a commit to ForeverInLaw/nofx that referenced this pull request Nov 8, 2025
…erwrite

fix(margin): correct position sizing formula to prevent insufficient margin errors
@the-dev-z the-dev-z deleted the fix/trader-config-overwrite branch November 12, 2025 10:04
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