From 2d3b1065b5ad534d6871dd179e39d1457cb915e5 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Mon, 23 Mar 2026 13:43:11 +0000
Subject: [PATCH] Address code review feedback: improve logging, fix redundant
 check, clarify docs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Log exceptions at warning level in ConfigService (not debug)
- Include exc_info=True for startup seed failure logging
- Remove redundant `is_secret is not None` guard
- Clarify ★ markers in README environment variables docs

Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
Agent-Logs-Url: https://github.com/christianlouis/pop_puller_to_gmail/sessions/ce88d4a8-d8c2-49b3-95a1-30592105a769
---
 README.md                              |  6 +++---
 backend/app/main.py                    |  2 +-
 backend/app/services/config_service.py | 15 +++++++++------
 3 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/README.md b/README.md
index b7e0385..2743127 100644
--- a/README.md
+++ b/README.md
@@ -146,7 +146,7 @@ Emails are forwarded to Gmail via SMTP. This is the legacy method and is used as
 | `POP3_ACCOUNT_N_PASSWORD` | Yes | — | POP3 password |
 | `POP3_ACCOUNT_N_USE_SSL` | No | `true` | Use SSL/TLS |
 
-#### SMTP Settings ★
+#### SMTP Settings (★ database-configurable)
 
 | Variable | Required | Default | Description |
 |----------|----------|---------|-------------|
@@ -156,7 +156,7 @@ Emails are forwarded to Gmail via SMTP. This is the legacy method and is used as
 | `SMTP_PASSWORD` | For SMTP | — | SMTP password (App Password) |
 | `SMTP_USE_TLS` | No | `true` | Use STARTTLS |
 
-#### Gmail API Settings ★
+#### Gmail API Settings (★ database-configurable)
 
 | Variable | Required | Default | Description |
 |----------|----------|---------|-------------|
@@ -164,7 +164,7 @@ Emails are forwarded to Gmail via SMTP. This is the legacy method and is used as
 | `GOOGLE_CLIENT_SECRET` | For Gmail API | — | Google OAuth2 client secret |
 | `GMAIL_API_ENABLED` | No | `true` | Enable Gmail API delivery |
 
-#### Processing Settings ★
+#### Processing Settings (★ database-configurable)
 
 | Variable | Required | Default | Description |
 |----------|----------|---------|-------------|
diff --git a/backend/app/main.py b/backend/app/main.py
index 7f6beae..c6b367d 100644
--- a/backend/app/main.py
+++ b/backend/app/main.py
@@ -37,7 +37,7 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]:
         async with async_session_maker() as db:
             await ConfigService.seed_defaults(db)
     except Exception as exc:
-        logger.warning("Could not seed default settings: %s", exc)
+        logger.warning("Could not seed default settings: %s", exc, exc_info=True)
 
     yield
     # Shutdown
diff --git a/backend/app/services/config_service.py b/backend/app/services/config_service.py
index 301fe28..142a997 100644
--- a/backend/app/services/config_service.py
+++ b/backend/app/services/config_service.py
@@ -190,8 +190,12 @@ class ConfigService:
                         setting.value,  # type: ignore[arg-type]
                         setting.value_type or "string",  # type: ignore[arg-type]
                     )
-            except Exception:
-                logger.debug("DB lookup failed for key=%s, falling back to env", key)
+            except Exception as exc:
+                logger.warning(
+                    "DB lookup failed for key=%s, falling back to env: %s",
+                    key,
+                    exc,
+                )
 
         env_val = os.getenv(key)
         if env_val is not None:
@@ -216,8 +220,8 @@ class ConfigService:
                         setting.value,  # type: ignore[arg-type]
                         setting.value_type or "string",  # type: ignore[arg-type]
                     )
-            except Exception:
-                logger.debug("DB bulk lookup failed, falling back to env")
+            except Exception as exc:
+                logger.warning("DB bulk lookup failed, falling back to env: %s", exc)
 
         for key in keys:
             if key not in result:
@@ -253,8 +257,7 @@ class ConfigService:
                 existing.value_type = value_type  # type: ignore[assignment]
             if description is not None:
                 existing.description = description  # type: ignore[assignment]
-            if is_secret is not None:
-                existing.is_secret = is_secret  # type: ignore[assignment]
+            existing.is_secret = is_secret  # type: ignore[assignment]
             if category is not None:
                 existing.category = category  # type: ignore[assignment]
             await db.commit()