fix(review): resolve Neon quality comments

Author: Melly-999

app/db/readonly.py

@@ -65,8 +65,8 @@ def readonly_connection() -> Iterator[psycopg.Connection[Any]]:
         raise RuntimeError("DATABASE_URL is not configured.")
 
     with psycopg.connect(database_url, row_factory=dict_row) as conn:
-        conn.execute("SET default_transaction_read_only = on")
-        yield conn
+        with conn.transaction(read_only=True):
+            yield conn
 
 
 def fetch_all(sql: str, params: tuple[Any, ...] | None = None) -> list[dict[str, Any]]:

scripts/sync_vercel_neon_env.ps1

@@ -1,10 +1,10 @@
 # Sync Neon DATABASE_URL to Vercel environments for branch `vercel-dev`.
 # Requires: vercel CLI logged in, project linked (`vercel link`).
-# Usage: pwsh scripts/sync_vercel_neon_env.ps1 [-ProjectName <org/team scope>]
-# Note: `-ProjectName` maps to Vercel's `--scope` flag (team/org scope).
+# Usage: pwsh scripts/sync_vercel_neon_env.ps1 [-VercelScope <org/team scope>]
+# Note: `-VercelScope` maps to Vercel's `--scope` flag (team/org scope).
 
 param(
-    [string]$ProjectName = ""
+    [string]$VercelScope = ""
 )
 
 $ErrorActionPreference = "Stop"
@@ -19,6 +19,9 @@ Get-Content $envFile | ForEach-Object {
     if ($_ -match '^\s*([^#=]+)=(.*)$') {
         $name = $matches[1].Trim()
         $value = $matches[2].Trim()
+        if ($value -match '^"(.*)"$' -or $value -match "^'(.*)'$") {
+            $value = $matches[1]
+        }
         Set-Item -Path "env:$name" -Value $value
     }
 }
@@ -28,16 +31,16 @@ if (-not $env:DATABASE_URL_VERCEL_DEV) {
 }
 
 $vercelArgs = @("env", "add", "DATABASE_URL", "preview", "--force")
-if ($ProjectName) {
-    $vercelArgs += @("--scope", $ProjectName)
+if ($VercelScope) {
+    $vercelArgs += @("--scope", $VercelScope)
 }
 
 Write-Host "Setting Vercel preview DATABASE_URL from NEON vercel-dev branch..."
 $env:DATABASE_URL_VERCEL_DEV | vercel @vercelArgs
 
 $prodArgs = @("env", "add", "DATABASE_URL", "production", "--force")
-if ($ProjectName) {
-    $prodArgs += @("--scope", $ProjectName)
+if ($VercelScope) {
+    $prodArgs += @("--scope", $VercelScope)
 }
 
 if ($env:DATABASE_URL) {

tests/app/test_readonly_db.py

@@ -4,22 +4,26 @@
 
 import pytest
 
-from app.db.readonly import assert_select_only
+import app.db.readonly as readonly_module
+from app.db.readonly import (
+    assert_select_only,
+    fetch_all,
+    fetch_one,
+    readonly_connection,
+)
 
 
 def test_assert_select_only_allows_select():
     assert_select_only("SELECT 1")
 
 
 def test_assert_select_only_allows_with_and_leading_comments():
-    sql = textwrap.dedent(
-        """
+    sql = textwrap.dedent("""
         -- leading comment
         /* block comment */
         WITH x AS (SELECT 1 AS id)
         SELECT id FROM x
-        """
-    )
+        """)
     assert_select_only(sql)
 
 
@@ -52,3 +56,86 @@ def test_assert_select_only_rejects_writes():
 def test_assert_select_only_rejects_dangerous_and_multi_statement_sql(sql: str):
     with pytest.raises(ValueError, match="single read-only SELECT or WITH query"):
         assert_select_only(sql)
+
+
+def test_readonly_helpers_use_read_only_transaction_and_params(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    calls: list[tuple[str, object]] = []
+
+    class _FakeTransaction:
+        def __enter__(self) -> None:
+            calls.append(("transaction_enter", True))
+
+        def __exit__(self, exc_type, exc, tb) -> bool:
+            calls.append(("transaction_exit", exc_type))
+            return False
+
+    class _FakeCursor:
+        def __enter__(self) -> "_FakeCursor":
+            calls.append(("cursor_enter", None))
+            return self
+
+        def __exit__(self, exc_type, exc, tb) -> bool:
+            calls.append(("cursor_exit", exc_type))
+            return False
+
+        def execute(self, sql: str, params: tuple[object, ...]) -> None:
+            calls.append(("execute", (sql, params)))
+
+        def fetchall(self) -> list[dict[str, int]]:
+            return [{"id": 1}, {"id": 2}]
+
+    class _FakeConnection:
+        def __enter__(self) -> "_FakeConnection":
+            calls.append(("connect_enter", None))
+            return self
+
+        def __exit__(self, exc_type, exc, tb) -> bool:
+            calls.append(("connect_exit", exc_type))
+            return False
+
+        def transaction(self, read_only: bool = False) -> _FakeTransaction:
+            calls.append(("transaction", read_only))
+            return _FakeTransaction()
+
+        def cursor(self) -> _FakeCursor:
+            return _FakeCursor()
+
+    monkeypatch.setenv("DATABASE_URL", "postgresql://example")
+    monkeypatch.setattr(
+        readonly_module.psycopg,
+        "connect",
+        lambda *args, **kwargs: _FakeConnection(),
+    )
+
+    with readonly_connection() as conn:
+        assert isinstance(conn, _FakeConnection)
+
+    rows = fetch_all("  SELECT * FROM memories WHERE id = %s", (7,))
+    assert rows == [{"id": 1}, {"id": 2}]
+
+    first = fetch_one(
+        "WITH x AS (SELECT 1 AS id) SELECT id FROM x WHERE id = %s",
+        (1,),
+    )
+    assert first == {"id": 1}
+
+    assert ("transaction", True) in calls
+    assert (
+        "execute",
+        ("  SELECT * FROM memories WHERE id = %s", (7,)),
+    ) in calls
+    assert (
+        "execute",
+        ("WITH x AS (SELECT 1 AS id) SELECT id FROM x WHERE id = %s", (1,)),
+    ) in calls
+
+
+def test_readonly_connection_requires_database_url(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    monkeypatch.delenv("DATABASE_URL", raising=False)
+    with pytest.raises(RuntimeError, match="DATABASE_URL is not configured"):
+        with readonly_connection():
+            pass