From 4edb1710a53e1a7995e19adae6f8cd46a63ce61b Mon Sep 17 00:00:00 2001
From: nendo <tmgast@gmail.com>
Date: Mon, 16 Mar 2026 10:59:49 +0900
Subject: [PATCH] fix: address review feedback on session handler and counter

- Restore NoResultFound behavior on update_session, complete_session,
  fail_session when row is missing (scalar returns None, old .one()
  raised -- silent None is a semantic regression)
- Remove redundant get_session call from _increment_session_counter;
  the atomic SQL increment is already a no-op on missing rows
- Log warning when passed session_id is not found in _sync_device
  instead of silently creating an orphan session
---
 backend/endpoints/saves.py                       | 14 +++++---------
 .../handler/database/sync_sessions_handler.py    | 16 +++++++++++++---
 backend/tasks/sync_push_pull_task.py             |  3 +++
 3 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/backend/endpoints/saves.py b/backend/endpoints/saves.py
index 4fe481b39..07c4e1d04 100644
--- a/backend/endpoints/saves.py
+++ b/backend/endpoints/saves.py
@@ -96,15 +96,11 @@ def _resolve_device(
     return device
 
 
-def _increment_session_counter(session_id: int, user_id: int) -> None:
+def _increment_session_counter(session_id: int) -> None:
     try:
-        session = db_sync_session_handler.get_session(
-            session_id=session_id, user_id=user_id
+        db_sync_session_handler.increment_operations_completed(
+            session_id=session_id,
         )
-        if session:
-            db_sync_session_handler.increment_operations_completed(
-                session_id=session_id,
-            )
     except Exception:
         log.warning(f"Failed to update sync session {session_id}", exc_info=True)
 
@@ -260,7 +256,7 @@ async def add_save(
         db_device_handler.update_last_seen(device_id=device.id, user_id=request.user.id)
 
     if session_id:
-        _increment_session_counter(session_id, request.user.id)
+        _increment_session_counter(session_id)
 
     if slot and autocleanup:
         slot_saves = db_save_handler.get_saves(
@@ -457,7 +453,7 @@ def download_save(
         db_device_handler.update_last_seen(device_id=device.id, user_id=request.user.id)
 
     if session_id:
-        _increment_session_counter(session_id, request.user.id)
+        _increment_session_counter(session_id)
 
     return FileResponse(path=str(file_path), filename=save.file_name)
 
diff --git a/backend/handler/database/sync_sessions_handler.py b/backend/handler/database/sync_sessions_handler.py
index 7e21afb1e..cf9e66644 100644
--- a/backend/handler/database/sync_sessions_handler.py
+++ b/backend/handler/database/sync_sessions_handler.py
@@ -2,6 +2,7 @@ from collections.abc import Sequence
 from datetime import datetime, timezone
 
 from sqlalchemy import select, update
+from sqlalchemy.exc import NoResultFound
 from sqlalchemy.orm import Session
 
 from decorators.database import begin_session
@@ -75,7 +76,10 @@ class DBSyncSessionsHandler(DBBaseHandler):
             .values(**data)
             .execution_options(synchronize_session="evaluate")
         )
-        return session.scalar(select(SyncSession).filter_by(id=session_id))
+        result = session.scalar(select(SyncSession).filter_by(id=session_id))
+        if not result:
+            raise NoResultFound(f"SyncSession {session_id} not found after update")
+        return result
 
     @begin_session
     def increment_operations_completed(
@@ -111,7 +115,10 @@ class DBSyncSessionsHandler(DBBaseHandler):
             )
             .execution_options(synchronize_session="evaluate")
         )
-        return session.scalar(select(SyncSession).filter_by(id=session_id))
+        result = session.scalar(select(SyncSession).filter_by(id=session_id))
+        if not result:
+            raise NoResultFound(f"SyncSession {session_id} not found after complete")
+        return result
 
     @begin_session
     def fail_session(
@@ -130,7 +137,10 @@ class DBSyncSessionsHandler(DBBaseHandler):
             )
             .execution_options(synchronize_session="evaluate")
         )
-        return session.scalar(select(SyncSession).filter_by(id=session_id))
+        result = session.scalar(select(SyncSession).filter_by(id=session_id))
+        if not result:
+            raise NoResultFound(f"SyncSession {session_id} not found after fail")
+        return result
 
     @begin_session
     def cancel_active_sessions(
diff --git a/backend/tasks/sync_push_pull_task.py b/backend/tasks/sync_push_pull_task.py
index 370d24bec..88cc018ef 100644
--- a/backend/tasks/sync_push_pull_task.py
+++ b/backend/tasks/sync_push_pull_task.py
@@ -80,6 +80,9 @@ async def _sync_device(device: Device, session_id: int | None = None) -> dict:
             session_id=session_id, user_id=device.user_id
         )
         if not sync_session:
+            log.warning(
+                f"Push-pull: session {session_id} not found, creating new session"
+            )
             sync_session = db_sync_session_handler.create_session(
                 device_id=device.id, user_id=device.user_id
             )