Skip to content

Claude/switch branch 011 cv1v mm qct he vpbrjjs rz4#22

Merged
chpeu merged 5 commits into
start-ml1211from
claude/switch-branch-011CV1vMmQctHeVpbrjjsRZ4
Nov 12, 2025
Merged

Claude/switch branch 011 cv1v mm qct he vpbrjjs rz4#22
chpeu merged 5 commits into
start-ml1211from
claude/switch-branch-011CV1vMmQctHeVpbrjjsRZ4

Conversation

@chpeu

@chpeu chpeu commented Nov 12, 2025

Copy link
Copy Markdown
Owner

PR Type

Bug fix


Description

  • Fix critical bookDepth filter bug preventing depth=0 pairs from being filtered

  • Fix bid_vol/ask_vol falsy check allowing zero values to bypass validation

  • Fix SL/TP distance calculations to use absolute values for correct display

  • Fix Profit Factor formula to sum all trades instead of using best/worst trades


Diagram Walkthrough

flowchart LR
  A["Bug Fixes"] --> B["Core Logic"]
  A --> C["Frontend Metrics"]
  B --> D["bookDepth Filter"]
  B --> E["bid_vol/ask_vol Check"]
  C --> F["SL/TP Distance"]
  C --> G["Profit Factor"]
  D --> H["Prevent depth=0 pairs"]
  E --> I["Handle zero volumes"]
  F --> J["Use absolute values"]
  G --> K["Sum all trades"]
Loading

File Walkthrough

Relevant files
Bug fix
scanner.py
Add critical bookDepth validation filter                                 

core/scanner.py

  • Added book_depth <= 0 filter condition to prevent pairs with zero
    depth from passing validation
  • Enhanced filter logic to ensure all positions have valid depth for
    slippage calculation
  • Added clarifying comment about bookDepth requirement in v6.4.3
+3/-2     
position_manager.py
Fix volume validation to handle zero values                           

core/position_manager.py

  • Changed bid_vol/ask_vol check from truthy (if bid_vol and ask_vol) to
    explicit None check (if bid_vol is not None and ask_vol is not None)
  • Added total_vol calculation with zero-division protection
  • Allows zero volume values to be used in depth_factor calculation
    instead of falling back to depth
+5/-3     
position.js
Use absolute values for distance calculations                       

frontend/src/lib/stores/position.js

  • Wrapped slDistance calculation with Math.abs() to ensure always
    positive percentage
  • Wrapped tpDistance calculation with Math.abs() to ensure always
    positive percentage
  • Added clarifying comments that distances are always positive
    indicators
+6/-4     
stats.js
Fix Profit Factor calculation to sum all trades                   

frontend/src/lib/stores/stats.js

  • Replaced incorrect Profit Factor formula using best/worst trades with
    correct sum-based calculation
  • Changed data source from stats store to tradeHistory for accurate
    profit/loss summation
  • Filters trades into profitable and loss-making groups, then sums each
    group separately
  • Returns '0.00' instead of null for consistency when no trades exist
+15/-6   

chpeu and others added 4 commits November 12, 2025 22:24
🔴 BUG CRITIQUE #1 - Filtre bookDepth manquant
Fichier: core/scanner.py:128
Sévérité: CRITIQUE ⚠️

Problème:
- Paires avec bookDepth=0 pouvaient passer le filtre
- Position ouverte avec depth=0 → slippage calculé = 0.00%
- CAUSE RACINE du bug rapporté (logs: depth=0.0)

Avant (ligne 127):
```python
if math.isnan(spread) or spread <= 0.001 or spread > 0.02 or recent_volume < 100000 or balance_score < min:
    return 0.0
```

Après (ligne 128):
```python
if math.isnan(spread) or spread <= 0.001 or spread > 0.02 or book_depth <= 0 or recent_volume < 100000 or balance_score < min:
    return 0.0
```

Impact: Garantit que TOUTES les positions auront depth > 0 pour calcul slippage valide

---

🟡 BUG #2 - Vérification bid_vol/ask_vol ambiguë
Fichier: core/position_manager.py:694-696
Sévérité: MOYENNE

Problème:
```python
if bid_vol and ask_vol:  # Faux si bid_vol=0 (0 est falsy)
```

Si bid_vol=0 et ask_vol=100:
- Condition False alors que ask_vol existe
- Utilise depth au lieu de volumes réels
- Calcul slippage moins précis

Correction:
```python
if bid_vol is not None and ask_vol is not None:
    total_vol = bid_vol + ask_vol
    depth_factor = order_size / total_vol if total_vol > 0 else 0
```

Impact: Utilise volumes réels même si un côté = 0

---

📊 TESTS REQUIS:
1. Vérifier qu'aucune paire avec bookDepth=0 n'est dans top_pairs
2. Vérifier logs slippage: spread > 0 ET depth > 0
3. Ouvrir position → vérifier slippage != 0.00%

✅ Ces corrections résolvent définitivement le problème slippage=0.00%
Bug #1: SL/TP distance affichait des valeurs négatives
- frontend/src/lib/stores/position.js:17-28
- Problème: Pour LONG, SL distance était négative
- Problème: Pour SHORT, TP distance était négative
- Fix: Utiliser Math.abs() pour toujours afficher distance positive

Bug #2: Profit Factor calculé avec formule totalement fausse
- frontend/src/lib/stores/stats.js:92-107
- Ancienne formule FAUSSE: (wins × best_trade) / (losses × worst_trade)
- Nouvelle formule CORRECTE: Σ(tous profits) / |Σ(toutes pertes)|
- Le Profit Factor doit sommer TOUS les trades, pas juste best/worst
@qodo-code-review

qodo-code-review Bot commented Nov 12, 2025

Copy link
Copy Markdown

PR Compliance Guide 🔍

(Compliance updated until commit ac666e3)

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing Audit Logs: The new code paths (e.g., disabling when psycopg2 unavailable) and scanner filtering
changes add critical behavior but do not introduce or reference audit logging of these
actions, which may be acceptable if handled elsewhere but is not evident in this diff.

Referred Code
if not PSYCOPG2_AVAILABLE:
    logger.error("❌ psycopg2 non disponible - PostgreSQL DataLogger désactivé")
    self.enabled = False
    self.pool = None  # 🔥 FIX: Initialiser pool à None pour les tests
    return

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Input Validation Scope: While stricter filters were added for spread and book depth, the diff does not show
validation or sanitization of external pair fields beyond basic numeric checks, which may
be handled elsewhere but is not visible here.

Referred Code
# Filtres stricts - 🔥 v6.4.3: Spread entre 0.001% et 0.02% ET bookDepth > 0
# 🔥 FIX: Vérifier explicitement si spread est NaN car NaN > 0.02 retourne False
# 🔥 FIX: Ajouter spread minimum de 0.001% pour éviter slippage nul
# 🔥 FIX BUG CRITIQUE: Filtrer book_depth <= 0 pour garantir slippage valide
if math.isnan(spread) or spread <= 0.001 or spread > 0.02 or book_depth <= 0 or recent_volume < 100000 or balance_score < TRADING_CONFIG['balance_score_min']:
    return 0.0

# Ratio volatilité/spread (plus élevé = mieux)

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Previous compliance checks

Compliance check up to commit 5a1ad46
Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing auditing: New logic paths affecting trading calculations add no audit/event logging for critical
decision outcomes such as slippage estimation branches.

Referred Code
# 🔥 FIX BUG #2: Depth factor - Vérifier que bid_vol et ask_vol ne sont pas None (pas juste truthy)
# Car bid_vol=0 est falsy mais valide
if bid_vol is not None and ask_vol is not None:
    total_vol = bid_vol + ask_vol
    depth_factor = order_size / total_vol if total_vol > 0 else 0
else:
    depth_factor = order_size / depth if depth > 0 else 0

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Input validation: The new filter adds constraints on spread and book_depth but lacks explicit type/None
validation and contextual error handling for malformed pairs.

Referred Code
# Filtres stricts - 🔥 v6.4.3: Spread entre 0.001% et 0.02% ET bookDepth > 0
# 🔥 FIX: Vérifier explicitement si spread est NaN car NaN > 0.02 retourne False
# 🔥 FIX: Ajouter spread minimum de 0.001% pour éviter slippage nul
# 🔥 FIX BUG CRITIQUE: Filtrer book_depth <= 0 pour garantir slippage valide
if math.isnan(spread) or spread <= 0.001 or spread > 0.02 or book_depth <= 0 or recent_volume < 100000 or balance_score < TRADING_CONFIG['balance_score_min']:
    return 0.0

# Ratio volatilité/spread (plus élevé = mieux)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Unvalidated data: The profitFactor computation trusts trade fields without validating numeric types or
handling NaN/Infinity beyond zero-loss shortcut, which could propagate invalid values to
UI.

Referred Code
// Computed: Profit Factor (Somme profits / |Somme pertes|)
export const profitFactor = derived(tradeHistory, $trades => {
	if ($trades.length === 0) return '0.00';

	// 🔥 FIX BUG CRITIQUE: Profit Factor = Σ(profits) / |Σ(losses)|
	// Ancienne formule incorrecte: (wins × best_trade) / (losses × worst_trade)
	const grossProfit = $trades
		.filter(t => (t.net_pnl_usdt || t.pnl_usdt || 0) > 0)
		.reduce((sum, t) => sum + (t.net_pnl_usdt || t.pnl_usdt || 0), 0);

	const grossLoss = Math.abs($trades
		.filter(t => (t.net_pnl_usdt || t.pnl_usdt || 0) < 0)
		.reduce((sum, t) => sum + (t.net_pnl_usdt || t.pnl_usdt || 0), 0));

	if (grossLoss === 0) return grossProfit > 0 ? '∞' : '0.00';
	return (grossProfit / grossLoss).toFixed(2);
});

Learn more about managing compliance generic rules or creating your own custom rules

@qodo-code-review

qodo-code-review Bot commented Nov 12, 2025

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Frontend state recalculates backend data

Avoid recalculating backend-provided statistics on the frontend. Instead, use
the backend as the single source of truth to maintain data integrity and
simplify frontend logic.

Examples:

frontend/src/lib/stores/stats.js [22-65]
export const stats = derived([tradeHistory, backendStats], ([$trades, $backendStats]) => {
	if ($trades.length === 0) {
		// Si pas de trades, utiliser les stats du backend (pour les autres métriques)
		return $backendStats;
	}

	// Calculer depuis les trades du frontend
	const total = $trades.length;
	const wins = $trades.filter(t => (t.net_pnl_usdt || t.pnl_usdt || 0) > 0).length;
	const losses = total - wins;

 ... (clipped 34 lines)

Solution Walkthrough:

Before:

// frontend/src/lib/stores/stats.js

// Backend provides stats
const backendStats = writable({ ... });

// Frontend has its own list of trades
import { tradeHistory } from './trades';

// Frontend recalculates stats from its own trade list
export const stats = derived([tradeHistory, backendStats], ([$trades, $backendStats]) => {
  if ($trades.length === 0) {
    return $backendStats;
  }
  // Recalculate wins, losses, PnL, etc. from $trades
  const wins = $trades.filter(t => t.pnl_usdt > 0).length;
  const total_pnl_usdt = $trades.reduce((sum, t) => sum + t.pnl_usdt, 0);
  // ...
  return { wins, total_pnl_usdt, ... };
});

After:

// frontend/src/lib/stores/stats.js

// Backend provides stats, which are the single source of truth
const backendStats = writable({
  wins: 0,
  losses: 0,
  total_pnl_usdt: 0,
  // ... other stats
});

// The stats store directly uses the data from the backend
export const stats = derived(backendStats, $backendStats => $backendStats);

// Other derived values can still be computed, but from the backend's stats
export const avgPnl = derived(stats, $stats => {
  if ($stats.total_trades === 0) return 0;
  return ($stats.total_pnl_usdt / $stats.total_trades).toFixed(2);
});
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a potential architectural flaw where the frontend recalculates stats, creating two sources of truth and risking data inconsistency, which is a significant concern for a trading application.

Medium
General
Optimize profit factor calculation

Refactor the profit factor calculation to use a single reduce operation instead
of two separate filter and reduce chains to improve performance.

frontend/src/lib/stores/stats.js [91-107]

 // Computed: Profit Factor (Somme profits / |Somme pertes|)
 export const profitFactor = derived(tradeHistory, $trades => {
 	if ($trades.length === 0) return '0.00';
 
 	// 🔥 FIX BUG CRITIQUE: Profit Factor = Σ(profits) / |Σ(losses)|
 	// Ancienne formule incorrecte: (wins × best_trade) / (losses × worst_trade)
-	const grossProfit = $trades
-		.filter(t => (t.net_pnl_usdt || t.pnl_usdt || 0) > 0)
-		.reduce((sum, t) => sum + (t.net_pnl_usdt || t.pnl_usdt || 0), 0);
+	const { grossProfit, grossLoss } = $trades.reduce(
+		(acc, trade) => {
+			const pnl = trade.net_pnl_usdt || trade.pnl_usdt || 0;
+			if (pnl > 0) {
+				acc.grossProfit += pnl;
+			} else if (pnl < 0) {
+				acc.grossLoss += pnl;
+			}
+			return acc;
+		},
+		{ grossProfit: 0, grossLoss: 0 }
+	);
 
-	const grossLoss = Math.abs($trades
-		.filter(t => (t.net_pnl_usdt || t.pnl_usdt || 0) < 0)
-		.reduce((sum, t) => sum + (t.net_pnl_usdt || t.pnl_usdt || 0), 0));
+	const absoluteGrossLoss = Math.abs(grossLoss);
 
-	if (grossLoss === 0) return grossProfit > 0 ? '∞' : '0.00';
-	return (grossProfit / grossLoss).toFixed(2);
+	if (absoluteGrossLoss === 0) return grossProfit > 0 ? '∞' : '0.00';
+	return (grossProfit / absoluteGrossLoss).toFixed(2);
 });
  • Apply / Chat
Suggestion importance[1-10]: 4

__

Why: The suggestion provides a valid performance optimization by combining two array traversals into one, which is more efficient and slightly improves code readability.

Low
  • Update

Bug #1: test_init_psycopg2_unavailable échouait
- core/postgresql_datalogger.py:68
- Problème: Attribut 'pool' non initialisé quand psycopg2 indisponible
- Fix: Ajouter `self.pool = None` avant le return

Bugs #2-4: test_log_scan_error, test_log_market_context, test_log_trade
- tests/test_postgresql_datalogger.py:188,213,258
- Problème: Tests attendaient 1 appel execute, mais code fait 2 appels
  (1 pour INSERT session, 1 pour INSERT table cible)
- Fix: Changer de assert_called_once() à assert call_count == 2
@chpeu chpeu merged commit c1a94d2 into start-ml1211 Nov 12, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants