Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 125 additions & 1 deletion backend/app/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,26 @@
``os.getenv`` directly. On Railway/Vercel these values are injected by the
platform; locally they come from ``.env`` (see ``.env.example``).

Importing this module loads the repo-root ``.env`` into ``os.environ`` with
``override=False`` — see ``_load_dotenv_once()``. Every backend entry point
reaches configuration through here (``app.main``, ``app.db``,
``app.services.watsonx_client``, ``app.routes.diagnostics``), so that single
import is enough to give ``uvicorn app.main:app`` the same environment the
platform dashboards inject in production.

Secrets are NEVER logged or serialized. ``env_report()`` and
``missing_required()`` expose only booleans / names — never the values — so
they are safe to surface over the deploy-verification endpoint.
"""

from __future__ import annotations

import logging
import os
from dataclasses import dataclass
from pathlib import Path

logger = logging.getLogger(__name__)

# Secrets that MUST be injected on BOTH platforms for the app to be healthy.
# Keep in sync with .env.example and docs/DEPLOYMENT.md.
Expand All @@ -28,11 +39,97 @@

_DEFAULT_CORS_ORIGINS = "http://localhost:3000"

# backend/app/config.py -> parents[2] == repo root (where `.env` lives per
# README.md, CONTRIBUTING.md and docs/DEPLOYMENT.md §"Local parity").
_REPO_ROOT: Path = Path(__file__).resolve().parents[2]
_DOTENV_PATH: Path = _REPO_ROOT / ".env"


def _split_origins(raw: str) -> tuple[str, ...]:
return tuple(origin.strip() for origin in raw.split(",") if origin.strip())


# ---------------------------------------------------------------------------
# .env loading
# ---------------------------------------------------------------------------


def _load_dotenv_once() -> bool:
"""Load the repo-root ``.env`` into ``os.environ``; return True if read.

``override=False`` is load-bearing, not a stylistic default: on Railway and
Vercel the real secrets are injected into the process environment by the
platform, and a stale ``.env`` that happened to ship in the image must
never win over them. Only names that are *absent* from the environment are
filled in, so the platform always has the last word and this call is a
no-op in production (where no ``.env`` exists at all).

``python-dotenv`` is a declared runtime dependency (``pyproject.toml`` /
``requirements.txt``), but its absence is degraded rather than fatal: a
deploy that injects every variable through the dashboard does not need it,
and ``/health`` must keep answering either way.
"""
if not _DOTENV_PATH.is_file():
return False
try:
from dotenv import load_dotenv
except ImportError: # pragma: no cover - dependency is declared
logger.warning(
"python-dotenv is not installed; %s will NOT be read. "
"Configuration must come from the process environment.",
_DOTENV_PATH,
)
return False
load_dotenv(_DOTENV_PATH, override=False)
logger.info("Loaded local environment overlay from %s (override=False).", _DOTENV_PATH)
return True


_DOTENV_LOADED: bool = _load_dotenv_once()


# ---------------------------------------------------------------------------
# DATABASE_URL normalisation
# ---------------------------------------------------------------------------


def to_asyncpg_dsn(database_url: str) -> str:
"""Return *database_url* in the ``postgresql+asyncpg://`` form.

``.env.example`` — the canonical list of variable names and shapes — spells
``DATABASE_URL`` with the plain ``postgresql://`` scheme, because that is
the DSN Supabase's dashboard hands out and the one psycopg2 consumers
(``scripts/seed_corpus.py``, ``scripts/precompute_umap.py``) expect. The
RAG path is the opposite: ``autoria_ai.db._make_engine`` feeds the value to
SQLAlchemy's ``create_async_engine``, which rejects a driver-less
``postgresql://`` URL outright.

Rather than change the documented ``.env`` shape (which would break the
seeding scripts) or ``autoria_ai.db``'s contract (explicitly out of scope
for WO-06), the backend translates at the boundary — the same rule, in the
same direction, as ``scripts/seed_corpus.py:to_asyncpg_url``. Both plain
schemes are accepted; ``postgres://`` is the short form some hosts emit.

A DSN that already names a driver (``postgresql+asyncpg``,
``postgresql+psycopg``, ...) is returned untouched, as is any unrecognised
scheme — normalising is a convenience, not a validator, and swallowing an
unknown DSN here would replace one silent failure with another.
"""
if database_url.startswith("postgresql://"):
return "postgresql+asyncpg://" + database_url[len("postgresql://") :]
if database_url.startswith("postgres://"):
return "postgresql+asyncpg://" + database_url[len("postgres://") :]
return database_url


def _resolve_database_url() -> str | None:
"""Read ``DATABASE_URL`` and return it as an async-engine-ready DSN."""
raw = os.getenv("DATABASE_URL")
if not raw:
return None
return to_asyncpg_dsn(raw)


@dataclass(frozen=True)
class Settings:
"""Snapshot of process configuration, read once at import time."""
Expand All @@ -42,6 +139,9 @@ class Settings:
watsonx_project_id: str | None
supabase_url: str | None
supabase_key: str | None
#: Async DSN for the pgvector RAG lookup — always in the
#: ``postgresql+asyncpg://`` form (see ``to_asyncpg_dsn``), never the plain
#: ``postgresql://`` scheme written in ``.env``. ``None`` when unset.
database_url: str | None
cors_origins: tuple[str, ...]
# Passport — paths/ids only; never log the values of *_PATH vars.
Expand All @@ -59,7 +159,7 @@ def load_settings() -> Settings:
watsonx_project_id=os.getenv("WATSONX_PROJECT_ID"),
supabase_url=os.getenv("SUPABASE_URL"),
supabase_key=os.getenv("SUPABASE_KEY"),
database_url=os.getenv("DATABASE_URL"),
database_url=_resolve_database_url(),
cors_origins=_split_origins(os.getenv("AUTORIA_CORS_ORIGINS", _DEFAULT_CORS_ORIGINS)),
passport_private_key_path=os.getenv("PASSPORT_PRIVATE_KEY_PATH"),
passport_public_key_path=os.getenv("PASSPORT_PUBLIC_KEY_PATH"),
Expand All @@ -71,6 +171,30 @@ def load_settings() -> Settings:
settings = load_settings()


def _log_database_url_status() -> None:
"""State once, at import, whether RAG retrieval can work at all.

Without this the only symptom of an unset ``DATABASE_URL`` is a generic
``RAG retrieval failed`` warning emitted by
``autoria_ai.generator.orchestrate`` on *every* generation, while the
request still returns 200 with ``rag_sources: []`` — a configuration error
disguised as a per-request hiccup (issue #87).
"""
if settings.database_url:
return
logger.error(
"DATABASE_URL is not set: RAG retrieval is disabled for this process. "
"Every /api/generate will fall back to an unconditioned prompt "
"('(no example passages provided)') and issue a passport with "
"rag_sources: []. Set DATABASE_URL in %s (local) or in the Railway "
"service Variables (deploy). See .env.example.",
_DOTENV_PATH,
)


_log_database_url_status()


def env_report() -> dict[str, bool]:
"""Presence map for the required secrets — booleans only, never values.

Expand Down
10 changes: 9 additions & 1 deletion backend/app/routes/generate.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

from fastapi import APIRouter, HTTPException

from app.config import settings
from app.db import get_client
from app.schemas import GenerateRequest, GenerateResponse

Expand Down Expand Up @@ -161,7 +162,6 @@ async def generate_text(body: GenerateRequest) -> GenerateResponse:
try:
# Import here so the name is available for the except clause below;
# the module is already cached by _load_orchestrator's sys.path setup.
from app.config import settings
from app.services.watsonx_client import WatsonxError

result: dict[str, Any] = await orchestrate(
Expand All @@ -170,6 +170,14 @@ async def generate_text(body: GenerateRequest) -> GenerateResponse:
author_id=body.author_id,
author_uuid=author_uuid,
model_id=_MODEL_ID,
# Without this the orchestrator calls retrieve_top_k(database_url=
# None), autoria_ai.db falls back to os.environ["DATABASE_URL"],
# and the resulting KeyError is swallowed by generator.py's
# `except Exception` — so every generation silently ran with zero
# RAG passages and shipped a passport with rag_sources: [] (#87).
# settings.database_url is already the +asyncpg form that
# create_async_engine requires (see app.config.to_asyncpg_dsn).
database_url=settings.database_url,
verifier_url=settings.passport_verifier_url,
)
except WatsonxError as exc:
Expand Down
4 changes: 4 additions & 0 deletions backend/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ dependencies = [
"ibm-watsonx-ai>=1.0",
"python-jose[cryptography]>=3.3",
"jsonschema[format]>=4.18",
# Reads the repo-root `.env` in app/config.py so `uvicorn app.main:app`
# sees the same variables Railway/Vercel inject (loaded with
# override=False — the platform environment always wins).
"python-dotenv>=1.0",
]

[project.optional-dependencies]
Expand Down
3 changes: 3 additions & 0 deletions backend/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,6 @@ python-multipart>=0.0.9
ibm-watsonx-ai>=1.0
python-jose[cryptography]>=3.3
jsonschema[format]>=4.18
# Local `.env` overlay read by app/config.py (override=False — platform-injected
# variables always win). requirements-deploy.txt pulls this file in via `-r`.
python-dotenv>=1.0
114 changes: 114 additions & 0 deletions backend/tests/test_config.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
"""Tests for app.config env-var wiring."""

from pathlib import Path

import app.config as config
from app.config import (
REQUIRED_ENV_VARS,
env_report,
load_settings,
missing_required,
to_asyncpg_dsn,
)


Expand Down Expand Up @@ -43,3 +47,113 @@ def test_cors_origins_defaults_to_localhost(monkeypatch):
monkeypatch.delenv("AUTORIA_CORS_ORIGINS", raising=False)
settings = load_settings()
assert settings.cors_origins == ("http://localhost:3000",)


# ---------------------------------------------------------------------------
# DATABASE_URL — asyncpg DSN normalisation (issue #87 / WO-06)
#
# `.env.example` documents the plain `postgresql://` scheme (what Supabase
# hands out and what the psycopg2 seeding scripts consume), but the RAG path
# ends in SQLAlchemy's create_async_engine, which rejects a driver-less URL.
# config.py translates at the boundary, exactly like
# scripts/seed_corpus.py:to_asyncpg_url.
# ---------------------------------------------------------------------------

_PLAIN_DSN = "postgresql://postgres.abc:pw@aws-0-eu-central-1.pooler.supabase.com:5432/postgres"
_ASYNC_DSN = (
"postgresql+asyncpg://postgres.abc:pw@aws-0-eu-central-1.pooler.supabase.com:5432/postgres"
)


def test_to_asyncpg_dsn_rewrites_plain_postgresql_scheme():
assert to_asyncpg_dsn(_PLAIN_DSN) == _ASYNC_DSN


def test_to_asyncpg_dsn_rewrites_short_postgres_scheme():
assert to_asyncpg_dsn("postgres://u:p@h:5432/db") == "postgresql+asyncpg://u:p@h:5432/db"


def test_to_asyncpg_dsn_is_idempotent():
assert to_asyncpg_dsn(_ASYNC_DSN) == _ASYNC_DSN


def test_to_asyncpg_dsn_preserves_credentials_and_query_string():
raw = "postgresql://u:p%40ss@h:5432/db?sslmode=require"
assert to_asyncpg_dsn(raw) == "postgresql+asyncpg://u:p%40ss@h:5432/db?sslmode=require"


def test_to_asyncpg_dsn_leaves_other_drivers_untouched():
"""Normalisation is a convenience, not a validator — don't mangle inputs."""
raw = "postgresql+psycopg://u:p@h:5432/db"
assert to_asyncpg_dsn(raw) == raw


def test_settings_database_url_is_async_form(monkeypatch):
"""load_settings() exposes the DSN create_async_engine can actually open."""
monkeypatch.setenv("DATABASE_URL", _PLAIN_DSN)
assert load_settings().database_url == _ASYNC_DSN


def test_settings_database_url_none_when_unset(monkeypatch):
monkeypatch.delenv("DATABASE_URL", raising=False)
assert load_settings().database_url is None


def test_settings_database_url_none_when_empty(monkeypatch):
"""An empty value is a missing value — never hand '' to create_async_engine."""
monkeypatch.setenv("DATABASE_URL", "")
assert load_settings().database_url is None


# ---------------------------------------------------------------------------
# .env overlay — the platform environment must always win
# ---------------------------------------------------------------------------


def test_dotenv_path_is_repo_root():
"""The overlay is the repo-root `.env` documented in DEPLOYMENT.md."""
assert Path(config._REPO_ROOT, ".env") == config._DOTENV_PATH
assert (config._REPO_ROOT / ".env.example").is_file()


def test_dotenv_does_not_override_platform_variables(monkeypatch, tmp_path):
"""A stale `.env` must never clobber a Railway/Vercel-injected variable.

The regression this guards: loading with override=True would let a `.env`
baked into the deploy image replace the dashboard's value.
"""
dotenv_file = tmp_path / ".env"
dotenv_file.write_text('AUTORIA_WO06_PROBE="from-dotenv"\n', encoding="utf-8")
monkeypatch.setattr(config, "_DOTENV_PATH", dotenv_file)

# Simulates the platform injecting the variable into the process env.
monkeypatch.setenv("AUTORIA_WO06_PROBE", "from-platform")
assert config._load_dotenv_once() is True

import os

assert os.environ["AUTORIA_WO06_PROBE"] == "from-platform"


def test_dotenv_fills_in_variables_absent_from_the_environment(monkeypatch, tmp_path):
"""With nothing injected, the `.env` value is what local dev gets."""
import os

dotenv_file = tmp_path / ".env"
dotenv_file.write_text('AUTORIA_WO06_PROBE="from-dotenv"\n', encoding="utf-8")
monkeypatch.setattr(config, "_DOTENV_PATH", dotenv_file)
os.environ.pop("AUTORIA_WO06_PROBE", None)

try:
assert config._load_dotenv_once() is True
assert os.environ["AUTORIA_WO06_PROBE"] == "from-dotenv"
finally:
# load_dotenv writes straight to os.environ, outside monkeypatch's
# undo log — clean up so the name cannot leak into another test.
os.environ.pop("AUTORIA_WO06_PROBE", None)


def test_dotenv_load_is_a_noop_when_file_absent(monkeypatch, tmp_path):
"""No `.env` (the normal production case) is not an error."""
monkeypatch.setattr(config, "_DOTENV_PATH", tmp_path / "does-not-exist.env")
assert config._load_dotenv_once() is False
Loading
Loading