From 0c4a941ca8ae8d53e0001ca6f4f0c6ef3f938ecd Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Wed, 5 Nov 2025 18:34:00 +0000
Subject: [PATCH] Fix critical double commit bug and Spanish holidays warning

CRITICAL FIX - Database Transaction:
- Removed duplicate commit logic from _store_in_db() inner function (lines 805-811)
- Prevents 'Method commit() can't be called here' error
- Now only outer scope handles commits (line 821 for new sessions, parent for parent sessions)
- Fixes issue where all 5 models trained successfully but failed to store in DB

MINOR FIX - Logging:
- Fixed Spanish holidays logger call (line 977-978)
- Removed invalid keyword arguments (region=, years=) from logger.info()
- Now uses f-string format consistent with rest of codebase
- Prevents 'Logger._log() got an unexpected keyword argument' warning

Impact:
- Training pipeline can now complete successfully
- Models will be stored in database after training
- No more cascading transaction failures
- Cleaner logs without warnings

Root cause: Double commit introduced during recent session management fixes
Related commits: 673108e, 74215d3, fd0a96e, b2de56e, e585e9f
---
 services/training/app/ml/prophet_manager.py | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/services/training/app/ml/prophet_manager.py b/services/training/app/ml/prophet_manager.py
index ebab84e7..aa6dcbd2 100644
--- a/services/training/app/ml/prophet_manager.py
+++ b/services/training/app/ml/prophet_manager.py
@@ -801,14 +801,11 @@ class BakeryProphetManager:
                 db_model.data_quality_score = float(training_metrics.get('data_quality_score')) if training_metrics.get('data_quality_score') is not None else None
 
             db_session.add(db_model)
-            
-            # Only commit if using a new session, not if using parent session
-            # Parent session commits will be handled by the calling method to prevent conflicts
-            if not use_parent_session:
-                await db_session.commit()
-                logger.info(f"Model {model_id} stored in database successfully")
-            else:
-                logger.debug(f"Added model {model_id} to parent session (commit deferred)")
+
+            # ✅ FIX: Don't commit here - let the outer scope handle commits
+            # This prevents double commit when using a new dedicated session
+            # Parent sessions will commit when parent completes; new sessions commit in outer scope
+            logger.debug(f"Added model {model_id} to session (commit handled by caller)")
 
         try:
             # ✅ FIX: Use parent session if provided, otherwise create new one
@@ -977,9 +974,8 @@ class BakeryProphetManager:
                 holidays_df = holidays_df.drop_duplicates(subset=['ds', 'holiday'])
                 holidays_df = holidays_df.sort_values('ds').reset_index(drop=True)
 
-                logger.info(f"Loaded {len(holidays_df)} Spanish holidays dynamically",
-                           region=region or 'National',
-                           years=f"{min(years)}-{max(years)}")
+                # ✅ FIX: Don't pass keyword args to logger - use f-string instead
+                logger.info(f"Loaded {len(holidays_df)} Spanish holidays dynamically (region={region or 'National'}, years={min(years)}-{max(years)})")
 
                 return holidays_df
             else: