From 864f5d22463c631d2450fa51cb319924758c6562 Mon Sep 17 00:00:00 2001
From: Hayden <154503486+groupthinking@users.noreply.github.com>
Date: Sun, 2 Aug 2026 07:49:08 -0500
Subject: [PATCH 1/3] feat(config): share validated env parsing for tunable
 concurrency constants

`TAG_WRITE_CONCURRENCY` in intelligent_cache.py was a hardcoded literal, so
tuning Redis tag-write fan-out for a given deployment required an application
release. Its sibling in firestore_state.py was already env-parsed, but the
parser was private to that module -- and had already been copy-pasted once
into cloud_ai/providers/aws_rekognition.py.

Extract the two parsers verbatim into youtube_extension/core/env_config.py and
have both call sites import them, then wire TAG_WRITE_CONCURRENCY through
positive_int_env().

Semantics are preserved exactly, including the deliberate split that the
merged firestore implementation settled on:

  - absent or blank falls back to the shipped default, because Compose and
    Helm routinely render an empty string for an unconfigured value; and
  - malformed or out-of-range fails fast at import rather than being clamped,
    so an operator typo surfaces at startup instead of silently running the
    process with a concurrency limit or deadline nobody chose.

The only behavioural change is the error text, which now names the offending
variable and echoes the input instead of surfacing int()'s built-in message.

core/ is chosen over core/config/ and utils/ because its __init__.py is empty:
importing the helper pulls in no logging or proxy stack, which matters for a
module read at import time. Imports are relative so the helper resolves under
either package root in use in this repo (youtube_extension.* and
src.youtube_extension.*) rather than loading a second copy of the package.

Verification:
  - tests/unit/test_env_config.py (new, 46 tests) covers unset, blank,
    whitespace, valid, zero, negative, non-numeric, inf and nan for both
    parsers, and asserts the messages are diagnosable.
  - Import-time wiring is proven in a subprocess rather than with
    importlib.reload, which would rebind module classes and leave the rest of
    the session holding stale references. With the env unset the constants
    resolve to exactly the shipped 8 / 16 / 30.0; with an override set they
    take the override; with an invalid value the import exits non-zero.
  - The 9 pre-existing parser tests in test_firestore_state.py were repointed
    at the re-exported names and still pass unchanged, which is what
    demonstrates the extraction is behaviour-preserving.
  - 247 passed across test_firestore_state.py and test_intelligent_cache.py;
    ruff clean; mypy --strict clean on the new module.

Deliberately out of scope: the duplicate parser in aws_rekognition.py, which
is already modified by open PR #1216 and would conflict; and
TAG_WRITE_POOL_RESERVE, which is a headroom allowance rather than a
concurrency limit.

Closes #1180

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---
 .../backend/services/intelligent_cache.py     |   6 +-
 src/youtube_extension/core/env_config.py      | 108 +++++++++
 .../services/cloud/firestore_state.py         |  43 +---
 tests/unit/test_env_config.py                 | 218 ++++++++++++++++++
 tests/unit/test_firestore_state.py            |  18 +-
 5 files changed, 344 insertions(+), 49 deletions(-)
 create mode 100644 src/youtube_extension/core/env_config.py
 create mode 100644 tests/unit/test_env_config.py

diff --git a/src/youtube_extension/backend/services/intelligent_cache.py b/src/youtube_extension/backend/services/intelligent_cache.py
index a0281de08..6e837e551 100644
--- a/src/youtube_extension/backend/services/intelligent_cache.py
+++ b/src/youtube_extension/backend/services/intelligent_cache.py
@@ -31,6 +31,8 @@
 
 import redis.asyncio as redis
 
+from ...core.env_config import positive_int_env
+
 # Configure logging
 logging.basicConfig(level=logging.INFO)
 logger = logging.getLogger(__name__)
@@ -39,7 +41,9 @@
 # redis-py's async connection pool defaults to max_connections=20 and each
 # in-flight command holds one connection, so an unbounded fan-out over a large
 # tag list could exhaust the pool.
-TAG_WRITE_CONCURRENCY = 8
+# Overridable so operators can tune tag-write fan-out against their own Redis
+# deployment without an application release; invalid values fail fast at import.
+TAG_WRITE_CONCURRENCY = positive_int_env("TAG_WRITE_CONCURRENCY", 8)
 
 # Connections deliberately left free for everything that is not a tag write:
 # the SET/SETEX and HSET issued by the same set() call, plus concurrent get()
diff --git a/src/youtube_extension/core/env_config.py b/src/youtube_extension/core/env_config.py
new file mode 100644
index 000000000..39aeec4a0
--- /dev/null
+++ b/src/youtube_extension/core/env_config.py
@@ -0,0 +1,108 @@
+"""Validated parsing of environment overrides for tunable runtime constants.
+
+Several services expose performance knobs -- worker-pool sizes, RPC deadlines --
+as module-level constants so they are cheap to read on hot paths. Making those
+knobs operator-tunable means reading ``os.environ`` at import time, and that is
+exactly where a bare ``int(os.getenv(...))`` is most dangerous: a typo in a
+deployment manifest stops being a configuration error and becomes an
+unimportable module, surfacing as a confusing traceback far from its cause.
+
+These helpers centralise that parsing so every tunable behaves identically:
+
+* **Absent or blank falls back.** Unset, empty, and whitespace-only values all
+  return the caller's default, so a deployment that sets nothing keeps the
+  shipped behaviour byte-for-byte. Blank is treated as unset deliberately --
+  Compose and Helm templates routinely render an empty string for an
+  unconfigured value, and that should mean "default", not "invalid".
+* **Malformed fails fast, loudly.** Anything else must parse and satisfy the
+  documented bound. Out-of-range values raise rather than being silently
+  clamped, because clamping hides an operator's typo behind behaviour they did
+  not ask for. The raised ``ValueError`` names the variable and echoes the
+  offending input, so the message is self-describing at startup.
+
+The fail-fast half is a deliberate trade: an invalid override takes the process
+down at import rather than running with a value nobody chose. For a
+concurrency limit or a deadline, running with a silently-substituted value is
+the worse outcome -- it is the kind of misconfiguration that only reveals
+itself under production load.
+"""
+
+from __future__ import annotations
+
+import math
+import os
+
+__all__ = ["positive_int_env", "positive_finite_float_env"]
+
+
+def _raw_override(name: str) -> str | None:
+    """Return the stripped override for ``name``, or ``None`` to use the default.
+
+    Blank and whitespace-only values are folded into ``None`` so that every
+    caller treats "rendered but empty" identically to "never set".
+    """
+    raw = os.getenv(name)
+    if raw is None or not raw.strip():
+        return None
+    return raw.strip()
+
+
+def positive_int_env(name: str, default: int) -> int:
+    """Read a positive integer override, failing fast on invalid configuration.
+
+    Args:
+        name: Environment variable to read.
+        default: Value used when the variable is unset or blank.
+
+    Returns:
+        The parsed override, or ``default`` when no override is present.
+
+    Raises:
+        ValueError: The variable is set to something that is not an integer, or
+            to an integer below 1.
+    """
+    raw = _raw_override(name)
+    if raw is None:
+        return default
+    try:
+        value = int(raw)
+    except ValueError:
+        raise ValueError(f"{name} must be an integer >= 1, got {raw!r}") from None
+    if value < 1:
+        raise ValueError(f"{name} must be >= 1, got {raw!r}")
+    return value
+
+
+def positive_finite_float_env(name: str, default: float) -> float:
+    """Read a positive, finite float override, failing fast on invalid configuration.
+
+    ``float()`` happily accepts ``inf``/``-inf``/``nan``. An infinite timeout
+    would silently remove a deadline (or be rejected downstream by gRPC timeout
+    validation), and ``nan`` compares false against every bound, so non-finite
+    values are rejected outright rather than clamped into range.
+
+    Args:
+        name: Environment variable to read.
+        default: Value used when the variable is unset or blank.
+
+    Returns:
+        The parsed override, or ``default`` when no override is present.
+
+    Raises:
+        ValueError: The variable is set to something that is not a number, or to
+            a value that is not both positive and finite.
+    """
+    raw = _raw_override(name)
+    if raw is None:
+        return default
+    try:
+        value = float(raw)
+    except ValueError:
+        raise ValueError(
+            f"{name} must be a positive, finite number of seconds, got {raw!r}"
+        ) from None
+    if not math.isfinite(value) or value <= 0:
+        raise ValueError(
+            f"{name} must be a positive, finite number of seconds, got {raw!r}"
+        )
+    return value
diff --git a/src/youtube_extension/services/cloud/firestore_state.py b/src/youtube_extension/services/cloud/firestore_state.py
index c16074772..ef94ae25d 100644
--- a/src/youtube_extension/services/cloud/firestore_state.py
+++ b/src/youtube_extension/services/cloud/firestore_state.py
@@ -9,12 +9,13 @@
 
 import asyncio
 import logging
-import math
 import os
 from dataclasses import asdict, dataclass
 from datetime import datetime, timezone
 from typing import Any, Optional
 
+from ...core.env_config import positive_finite_float_env, positive_int_env
+
 try:
     from google.cloud import firestore
     from google.cloud.firestore_v1 import AsyncClient
@@ -28,42 +29,6 @@
 
 logger = logging.getLogger(__name__)
 
-def _positive_int_env(name: str, default: int) -> int:
-    """Read a positive integer override, failing fast on invalid configuration.
-
-    An unset or blank variable falls back to ``default`` (blank is common when a
-    compose/Helm template renders an empty value). Anything else must parse to an
-    integer >= 1; out-of-range values raise rather than being silently clamped,
-    so an operator typo surfaces at startup instead of changing behaviour quietly.
-    """
-    raw = os.getenv(name)
-    if raw is None or not raw.strip():
-        return default
-    value = int(raw.strip())
-    if value < 1:
-        raise ValueError(f"{name} must be >= 1, got {raw!r}")
-    return value
-
-
-def _positive_finite_float_env(name: str, default: float) -> float:
-    """Read a positive, finite float override, failing fast on invalid configuration.
-
-    ``float()`` happily accepts ``inf``/``-inf``/``nan``. An infinite timeout would
-    silently remove the per-delete deadline (or be rejected downstream by gRPC
-    timeout validation), and ``nan`` compares false against every bound, so
-    non-finite values are rejected outright rather than clamped into range.
-    """
-    raw = os.getenv(name)
-    if raw is None or not raw.strip():
-        return default
-    value = float(raw.strip())
-    if not math.isfinite(value) or value <= 0:
-        raise ValueError(
-            f"{name} must be a positive, finite number of seconds, got {raw!r}"
-        )
-    return value
-
-
 # Worker-pool size and per-delete deadline used by cleanup_old_states().
 # Cleanup can match an unbounded number of documents, so deletes are pulled from
 # a shared iterator by this many workers rather than dispatched all at once.
@@ -71,8 +36,8 @@ def _positive_finite_float_env(name: str, default: float) -> float:
 # delete RPCs and the number of allocated task objects by the same constant.
 # Both controls are overridable so operators can tune cleanup independently of an
 # application deployment; invalid values fail fast during import.
-CLEANUP_DELETE_CONCURRENCY = _positive_int_env("CLEANUP_DELETE_CONCURRENCY", 16)
-CLEANUP_DELETE_TIMEOUT_SECONDS = _positive_finite_float_env(
+CLEANUP_DELETE_CONCURRENCY = positive_int_env("CLEANUP_DELETE_CONCURRENCY", 16)
+CLEANUP_DELETE_TIMEOUT_SECONDS = positive_finite_float_env(
     "CLEANUP_DELETE_TIMEOUT_SECONDS", 30.0
 )
 
diff --git a/tests/unit/test_env_config.py b/tests/unit/test_env_config.py
new file mode 100644
index 000000000..96a545527
--- /dev/null
+++ b/tests/unit/test_env_config.py
@@ -0,0 +1,218 @@
+"""Tests for the shared environment-override parsers.
+
+``tests/unit/test_firestore_state.py`` already exercises these helpers through
+the constants they back. This module covers them at their canonical location,
+plus the two things that can only be observed end to end:
+
+* the error messages name the offending variable, so a misconfiguration is
+  diagnosable from the startup log alone; and
+* the constants really are wired at **import time**, which is checked by
+  importing the module under test in a subprocess with the override set.
+  A subprocess is used deliberately: ``importlib.reload`` would rebind the
+  module's classes and leave the rest of the session holding stale references.
+"""
+
+from __future__ import annotations
+
+import os
+import subprocess
+import sys
+from pathlib import Path
+from unittest.mock import patch
+
+import pytest
+
+import youtube_extension
+from youtube_extension.core.env_config import (
+    positive_finite_float_env,
+    positive_int_env,
+)
+
+_VAR = "ENV_CONFIG_TEST_VALUE"
+
+# Directory that must be on PYTHONPATH for a subprocess to import the package.
+_SRC_ROOT = str(Path(youtube_extension.__file__).resolve().parent.parent)
+
+
+# ===========================================================================
+# positive_int_env
+# ===========================================================================
+
+
+class TestPositiveIntEnv:
+    def test_unset_falls_back_to_default(self):
+        os.environ.pop(_VAR, None)
+        assert positive_int_env(_VAR, 8) == 8
+
+    @pytest.mark.parametrize("raw", ["", "   ", "\t", "\n"])
+    def test_blank_falls_back_to_default(self, raw):
+        """Compose/Helm render an empty string for an unconfigured value."""
+        with patch.dict(os.environ, {_VAR: raw}, clear=False):
+            assert positive_int_env(_VAR, 8) == 8
+
+    @pytest.mark.parametrize(("raw", "expected"), [("1", 1), ("3", 3), (" 12 ", 12)])
+    def test_parses_valid_override(self, raw, expected):
+        with patch.dict(os.environ, {_VAR: raw}, clear=False):
+            assert positive_int_env(_VAR, 8) == expected
+
+    @pytest.mark.parametrize("raw", ["0", "-1", "-42"])
+    def test_rejects_non_positive(self, raw):
+        """Out of range raises rather than clamping, so a typo is not masked."""
+        with patch.dict(os.environ, {_VAR: raw}, clear=False):
+            with pytest.raises(ValueError, match=">= 1"):
+                positive_int_env(_VAR, 8)
+
+    @pytest.mark.parametrize("raw", ["abc", "1.5", "inf", "nan", "8x", "0x10"])
+    def test_rejects_malformed(self, raw):
+        with patch.dict(os.environ, {_VAR: raw}, clear=False):
+            with pytest.raises(ValueError, match="must be an integer >= 1"):
+                positive_int_env(_VAR, 8)
+
+    def test_error_names_variable_and_echoes_input(self):
+        with patch.dict(os.environ, {_VAR: "oops"}, clear=False):
+            with pytest.raises(ValueError) as excinfo:
+                positive_int_env(_VAR, 8)
+        message = str(excinfo.value)
+        assert _VAR in message
+        assert "oops" in message
+
+    def test_malformed_error_does_not_chain_raw_int_error(self):
+        """``from None`` keeps the confusing built-in message out of the log."""
+        with patch.dict(os.environ, {_VAR: "abc"}, clear=False):
+            with pytest.raises(ValueError) as excinfo:
+                positive_int_env(_VAR, 8)
+        assert excinfo.value.__cause__ is None
+
+
+# ===========================================================================
+# positive_finite_float_env
+# ===========================================================================
+
+
+class TestPositiveFiniteFloatEnv:
+    def test_unset_falls_back_to_default(self):
+        os.environ.pop(_VAR, None)
+        assert positive_finite_float_env(_VAR, 30.0) == 30.0
+
+    @pytest.mark.parametrize("raw", ["", "   "])
+    def test_blank_falls_back_to_default(self, raw):
+        with patch.dict(os.environ, {_VAR: raw}, clear=False):
+            assert positive_finite_float_env(_VAR, 30.0) == 30.0
+
+    @pytest.mark.parametrize(
+        ("raw", "expected"), [("12.5", 12.5), (" 0.25 ", 0.25), ("5", 5.0)]
+    )
+    def test_parses_valid_override(self, raw, expected):
+        with patch.dict(os.environ, {_VAR: raw}, clear=False):
+            assert positive_finite_float_env(_VAR, 30.0) == expected
+
+    @pytest.mark.parametrize(
+        "raw", ["inf", "Infinity", "-inf", "nan", "NaN", "0", "0.0", "-1"]
+    )
+    def test_rejects_non_finite_and_non_positive(self, raw):
+        """``float()`` accepts inf/nan; an infinite deadline is not a deadline."""
+        with patch.dict(os.environ, {_VAR: raw}, clear=False):
+            with pytest.raises(ValueError, match="positive, finite"):
+                positive_finite_float_env(_VAR, 30.0)
+
+    @pytest.mark.parametrize("raw", ["abc", "1.2.3", "12s"])
+    def test_rejects_malformed(self, raw):
+        with patch.dict(os.environ, {_VAR: raw}, clear=False):
+            with pytest.raises(ValueError, match="positive, finite"):
+                positive_finite_float_env(_VAR, 30.0)
+
+    def test_error_names_variable_and_echoes_input(self):
+        with patch.dict(os.environ, {_VAR: "later"}, clear=False):
+            with pytest.raises(ValueError) as excinfo:
+                positive_finite_float_env(_VAR, 30.0)
+        message = str(excinfo.value)
+        assert _VAR in message
+        assert "later" in message
+
+
+# ===========================================================================
+# Import-time wiring of the tunable constants
+# ===========================================================================
+
+
+def _import_constant(module: str, constant: str, override: str | None):
+    """Import ``module`` in a clean interpreter and report ``constant``.
+
+    Redis is not installed in the test environment, so the stub that
+    ``test_intelligent_cache.py`` installs is reproduced here for the child
+    process. Returns the ``CompletedProcess`` so callers can assert on both the
+    printed value and a fail-fast non-zero exit.
+    """
+    code = (
+        "import sys, types;"
+        "m = types.ModuleType('redis');"
+        "a = types.ModuleType('redis.asyncio');"
+        "a.Redis = object;"
+        "a.ConnectionPool = object;"
+        "a.from_url = lambda url, **kw: None;"
+        "m.asyncio = a;"
+        "sys.modules['redis'] = m;"
+        "sys.modules['redis.asyncio'] = a;"
+        f"import {module} as mod;"
+        f"print(mod.{constant})"
+    )
+    env = dict(os.environ)
+    env["PYTHONPATH"] = _SRC_ROOT + os.pathsep + env.get("PYTHONPATH", "")
+    env.pop(constant, None)
+    if override is not None:
+        env[constant] = override
+    return subprocess.run(
+        [sys.executable, "-c", code],
+        capture_output=True,
+        text=True,
+        env=env,
+        timeout=120,
+    )
+
+
+_CACHE_MODULE = "youtube_extension.backend.services.intelligent_cache"
+_FIRESTORE_MODULE = "youtube_extension.services.cloud.firestore_state"
+
+
+class TestTunableConstantWiring:
+    """The acceptance criterion that matters: unset env == shipped behaviour."""
+
+    @pytest.mark.parametrize(
+        ("module", "constant", "default"),
+        [
+            (_CACHE_MODULE, "TAG_WRITE_CONCURRENCY", "8"),
+            (_FIRESTORE_MODULE, "CLEANUP_DELETE_CONCURRENCY", "16"),
+            (_FIRESTORE_MODULE, "CLEANUP_DELETE_TIMEOUT_SECONDS", "30.0"),
+        ],
+    )
+    def test_unset_keeps_shipped_default(self, module, constant, default):
+        result = _import_constant(module, constant, None)
+        assert result.returncode == 0, result.stderr
+        assert result.stdout.strip() == default
+
+    @pytest.mark.parametrize(
+        ("module", "constant", "override"),
+        [
+            (_CACHE_MODULE, "TAG_WRITE_CONCURRENCY", "3"),
+            (_FIRESTORE_MODULE, "CLEANUP_DELETE_CONCURRENCY", "4"),
+            (_FIRESTORE_MODULE, "CLEANUP_DELETE_TIMEOUT_SECONDS", "2.5"),
+        ],
+    )
+    def test_override_is_applied_at_import(self, module, constant, override):
+        result = _import_constant(module, constant, override)
+        assert result.returncode == 0, result.stderr
+        assert result.stdout.strip() == override
+
+    @pytest.mark.parametrize(
+        ("module", "constant"),
+        [
+            (_CACHE_MODULE, "TAG_WRITE_CONCURRENCY"),
+            (_FIRESTORE_MODULE, "CLEANUP_DELETE_CONCURRENCY"),
+            (_FIRESTORE_MODULE, "CLEANUP_DELETE_TIMEOUT_SECONDS"),
+        ],
+    )
+    def test_invalid_override_fails_fast_at_import(self, module, constant):
+        """A bad value must stop the process, not run with a value nobody chose."""
+        result = _import_constant(module, constant, "0")
+        assert result.returncode != 0
+        assert constant in result.stderr
diff --git a/tests/unit/test_firestore_state.py b/tests/unit/test_firestore_state.py
index 440783cef..4f471ff9c 100644
--- a/tests/unit/test_firestore_state.py
+++ b/tests/unit/test_firestore_state.py
@@ -747,45 +747,45 @@ class TestCleanupConfigEnvParsing:
     def test_float_env_rejects_non_finite_and_non_positive(self, raw):
         with patch.dict(os.environ, {"CLEANUP_TEST_TIMEOUT": raw}, clear=False):
             with pytest.raises(ValueError, match="positive, finite"):
-                _mod._positive_finite_float_env("CLEANUP_TEST_TIMEOUT", 30.0)
+                _mod.positive_finite_float_env("CLEANUP_TEST_TIMEOUT", 30.0)
 
     def test_float_env_rejects_malformed_value(self):
         with patch.dict(os.environ, {"CLEANUP_TEST_TIMEOUT": "abc"}, clear=False):
             with pytest.raises(ValueError):
-                _mod._positive_finite_float_env("CLEANUP_TEST_TIMEOUT", 30.0)
+                _mod.positive_finite_float_env("CLEANUP_TEST_TIMEOUT", 30.0)
 
     @pytest.mark.parametrize("raw", ["", "   "])
     def test_float_env_blank_falls_back_to_default(self, raw):
         with patch.dict(os.environ, {"CLEANUP_TEST_TIMEOUT": raw}, clear=False):
-            assert _mod._positive_finite_float_env("CLEANUP_TEST_TIMEOUT", 30.0) == 30.0
+            assert _mod.positive_finite_float_env("CLEANUP_TEST_TIMEOUT", 30.0) == 30.0
 
     def test_float_env_unset_falls_back_to_default(self):
         os.environ.pop("CLEANUP_TEST_TIMEOUT", None)
-        assert _mod._positive_finite_float_env("CLEANUP_TEST_TIMEOUT", 30.0) == 30.0
+        assert _mod.positive_finite_float_env("CLEANUP_TEST_TIMEOUT", 30.0) == 30.0
 
     def test_float_env_parses_valid_override(self):
         with patch.dict(os.environ, {"CLEANUP_TEST_TIMEOUT": " 12.5 "}, clear=False):
-            assert _mod._positive_finite_float_env("CLEANUP_TEST_TIMEOUT", 30.0) == 12.5
+            assert _mod.positive_finite_float_env("CLEANUP_TEST_TIMEOUT", 30.0) == 12.5
 
     @pytest.mark.parametrize("raw", ["0", "-4"])
     def test_int_env_rejects_non_positive(self, raw):
         with patch.dict(os.environ, {"CLEANUP_TEST_POOL": raw}, clear=False):
             with pytest.raises(ValueError, match=">= 1"):
-                _mod._positive_int_env("CLEANUP_TEST_POOL", 16)
+                _mod.positive_int_env("CLEANUP_TEST_POOL", 16)
 
     @pytest.mark.parametrize("raw", ["abc", "1.5", "inf"])
     def test_int_env_rejects_malformed_value(self, raw):
         with patch.dict(os.environ, {"CLEANUP_TEST_POOL": raw}, clear=False):
             with pytest.raises(ValueError):
-                _mod._positive_int_env("CLEANUP_TEST_POOL", 16)
+                _mod.positive_int_env("CLEANUP_TEST_POOL", 16)
 
     def test_int_env_blank_falls_back_to_default(self):
         with patch.dict(os.environ, {"CLEANUP_TEST_POOL": ""}, clear=False):
-            assert _mod._positive_int_env("CLEANUP_TEST_POOL", 16) == 16
+            assert _mod.positive_int_env("CLEANUP_TEST_POOL", 16) == 16
 
     def test_int_env_parses_valid_override(self):
         with patch.dict(os.environ, {"CLEANUP_TEST_POOL": " 4 "}, clear=False):
-            assert _mod._positive_int_env("CLEANUP_TEST_POOL", 16) == 4
+            assert _mod.positive_int_env("CLEANUP_TEST_POOL", 16) == 4
 
     def test_module_defaults_are_positive_and_finite(self):
         assert _mod.CLEANUP_DELETE_CONCURRENCY >= 1

From 96c53e2d0adce2fb68730937719958bf6d2c30a2 Mon Sep 17 00:00:00 2001
From: Hayden <154503486+groupthinking@users.noreply.github.com>
Date: Sun, 2 Aug 2026 08:28:54 -0500
Subject: [PATCH 2/3] fix: make shared tunables fail safe and bounded

---
 .../backend/services/intelligent_cache.py     |   4 +-
 src/youtube_extension/core/env_config.py      | 105 ++++++------------
 .../services/cloud/firestore_state.py         |  10 +-
 tests/unit/test_env_config.py                 |  75 +++++--------
 tests/unit/test_firestore_state.py            |  45 +++-----
 5 files changed, 83 insertions(+), 156 deletions(-)

diff --git a/src/youtube_extension/backend/services/intelligent_cache.py b/src/youtube_extension/backend/services/intelligent_cache.py
index 6e837e551..860a7f98b 100644
--- a/src/youtube_extension/backend/services/intelligent_cache.py
+++ b/src/youtube_extension/backend/services/intelligent_cache.py
@@ -31,7 +31,7 @@
 
 import redis.asyncio as redis
 
-from ...core.env_config import positive_int_env
+from youtube_extension.core.env_config import positive_int_env
 
 # Configure logging
 logging.basicConfig(level=logging.INFO)
@@ -42,7 +42,7 @@
 # in-flight command holds one connection, so an unbounded fan-out over a large
 # tag list could exhaust the pool.
 # Overridable so operators can tune tag-write fan-out against their own Redis
-# deployment without an application release; invalid values fail fast at import.
+# deployment without an application release; invalid values log and use the default.
 TAG_WRITE_CONCURRENCY = positive_int_env("TAG_WRITE_CONCURRENCY", 8)
 
 # Connections deliberately left free for everything that is not a tag write:
diff --git a/src/youtube_extension/core/env_config.py b/src/youtube_extension/core/env_config.py
index 39aeec4a0..6ff12aa1e 100644
--- a/src/youtube_extension/core/env_config.py
+++ b/src/youtube_extension/core/env_config.py
@@ -1,108 +1,71 @@
-"""Validated parsing of environment overrides for tunable runtime constants.
+"""Safe parsing for operator-tunable environment values.
 
-Several services expose performance knobs -- worker-pool sizes, RPC deadlines --
-as module-level constants so they are cheap to read on hot paths. Making those
-knobs operator-tunable means reading ``os.environ`` at import time, and that is
-exactly where a bare ``int(os.getenv(...))`` is most dangerous: a typo in a
-deployment manifest stops being a configuration error and becomes an
-unimportable module, surfacing as a confusing traceback far from its cause.
-
-These helpers centralise that parsing so every tunable behaves identically:
-
-* **Absent or blank falls back.** Unset, empty, and whitespace-only values all
-  return the caller's default, so a deployment that sets nothing keeps the
-  shipped behaviour byte-for-byte. Blank is treated as unset deliberately --
-  Compose and Helm templates routinely render an empty string for an
-  unconfigured value, and that should mean "default", not "invalid".
-* **Malformed fails fast, loudly.** Anything else must parse and satisfy the
-  documented bound. Out-of-range values raise rather than being silently
-  clamped, because clamping hides an operator's typo behind behaviour they did
-  not ask for. The raised ``ValueError`` names the variable and echoes the
-  offending input, so the message is self-describing at startup.
-
-The fail-fast half is a deliberate trade: an invalid override takes the process
-down at import rather than running with a value nobody chose. For a
-concurrency limit or a deadline, running with a silently-substituted value is
-the worse outcome -- it is the kind of misconfiguration that only reveals
-itself under production load.
+Runtime tuning must not make a service unimportable. Unset or invalid overrides
+therefore use the shipped default and emit a warning that names the variable.
+Integer settings may also declare a hard maximum when an unbounded value would
+create unsafe resource fan-out.
 """
 
 from __future__ import annotations
 
+import logging
 import math
 import os
 
 __all__ = ["positive_int_env", "positive_finite_float_env"]
 
+logger = logging.getLogger(__name__)
 
-def _raw_override(name: str) -> str | None:
-    """Return the stripped override for ``name``, or ``None`` to use the default.
 
-    Blank and whitespace-only values are folded into ``None`` so that every
-    caller treats "rendered but empty" identically to "never set".
-    """
+def _raw_override(name: str) -> str | None:
+    """Return the stripped override, or ``None`` when it is unset."""
     raw = os.getenv(name)
     if raw is None or not raw.strip():
         return None
     return raw.strip()
 
 
-def positive_int_env(name: str, default: int) -> int:
-    """Read a positive integer override, failing fast on invalid configuration.
-
-    Args:
-        name: Environment variable to read.
-        default: Value used when the variable is unset or blank.
-
-    Returns:
-        The parsed override, or ``default`` when no override is present.
-
-    Raises:
-        ValueError: The variable is set to something that is not an integer, or
-            to an integer below 1.
-    """
+def _fallback(name: str, raw: str, default: int | float, requirement: str) -> int | float:
+    logger.warning(
+        "Ignoring invalid %s=%r; expected %s. Using default %r.",
+        name,
+        raw,
+        requirement,
+        default,
+    )
+    return default
+
+
+def positive_int_env(
+    name: str,
+    default: int,
+    *,
+    maximum: int | None = None,
+) -> int:
+    """Read a positive integer override, falling back safely when invalid."""
     raw = _raw_override(name)
     if raw is None:
         return default
     try:
         value = int(raw)
     except ValueError:
-        raise ValueError(f"{name} must be an integer >= 1, got {raw!r}") from None
+        return int(_fallback(name, raw, default, "an integer >= 1"))
     if value < 1:
-        raise ValueError(f"{name} must be >= 1, got {raw!r}")
+        return int(_fallback(name, raw, default, "an integer >= 1"))
+    if maximum is not None and value > maximum:
+        return int(_fallback(name, raw, default, f"an integer between 1 and {maximum}"))
     return value
 
 
 def positive_finite_float_env(name: str, default: float) -> float:
-    """Read a positive, finite float override, failing fast on invalid configuration.
-
-    ``float()`` happily accepts ``inf``/``-inf``/``nan``. An infinite timeout
-    would silently remove a deadline (or be rejected downstream by gRPC timeout
-    validation), and ``nan`` compares false against every bound, so non-finite
-    values are rejected outright rather than clamped into range.
-
-    Args:
-        name: Environment variable to read.
-        default: Value used when the variable is unset or blank.
-
-    Returns:
-        The parsed override, or ``default`` when no override is present.
-
-    Raises:
-        ValueError: The variable is set to something that is not a number, or to
-            a value that is not both positive and finite.
-    """
+    """Read a positive finite float override, falling back safely when invalid."""
     raw = _raw_override(name)
     if raw is None:
         return default
     try:
         value = float(raw)
     except ValueError:
-        raise ValueError(
-            f"{name} must be a positive, finite number of seconds, got {raw!r}"
-        ) from None
+        return float(_fallback(name, raw, default, "a positive, finite number"))
     if not math.isfinite(value) or value <= 0:
-        raise ValueError(
-            f"{name} must be a positive, finite number of seconds, got {raw!r}"
-        )
+        return float(_fallback(name, raw, default, "a positive, finite number"))
     return value
diff --git a/src/youtube_extension/services/cloud/firestore_state.py b/src/youtube_extension/services/cloud/firestore_state.py
index ef94ae25d..d28fe9401 100644
--- a/src/youtube_extension/services/cloud/firestore_state.py
+++ b/src/youtube_extension/services/cloud/firestore_state.py
@@ -14,7 +14,7 @@
 from datetime import datetime, timezone
 from typing import Any, Optional
 
-from ...core.env_config import positive_finite_float_env, positive_int_env
+from youtube_extension.core.env_config import positive_finite_float_env, positive_int_env
 
 try:
     from google.cloud import firestore
@@ -35,8 +35,12 @@
 # Sizing the pool -- rather than gating a full fan-out -- bounds the in-flight
 # delete RPCs and the number of allocated task objects by the same constant.
 # Both controls are overridable so operators can tune cleanup independently of an
-# application deployment; invalid values fail fast during import.
-CLEANUP_DELETE_CONCURRENCY = positive_int_env("CLEANUP_DELETE_CONCURRENCY", 16)
+# application deployment. Invalid values log and use the shipped default.
+# Bound the worker pool so a typo cannot allocate one task per queued document.
+CLEANUP_DELETE_CONCURRENCY_MAX = 64
+CLEANUP_DELETE_CONCURRENCY = positive_int_env(
+    "CLEANUP_DELETE_CONCURRENCY", 16, maximum=CLEANUP_DELETE_CONCURRENCY_MAX
+)
 CLEANUP_DELETE_TIMEOUT_SECONDS = positive_finite_float_env(
     "CLEANUP_DELETE_TIMEOUT_SECONDS", 30.0
 )
diff --git a/tests/unit/test_env_config.py b/tests/unit/test_env_config.py
index 96a545527..3373ba798 100644
--- a/tests/unit/test_env_config.py
+++ b/tests/unit/test_env_config.py
@@ -4,8 +4,7 @@
 the constants they back. This module covers them at their canonical location,
 plus the two things that can only be observed end to end:
 
-* the error messages name the offending variable, so a misconfiguration is
-  diagnosable from the startup log alone; and
+* invalid overrides emit a diagnostic warning and preserve service startup; and
 * the constants really are wired at **import time**, which is checked by
   importing the module under test in a subprocess with the override set.
   A subprocess is used deliberately: ``importlib.reload`` would rebind the
@@ -46,7 +45,6 @@ def test_unset_falls_back_to_default(self):
 
     @pytest.mark.parametrize("raw", ["", "   ", "\t", "\n"])
     def test_blank_falls_back_to_default(self, raw):
-        """Compose/Helm render an empty string for an unconfigured value."""
         with patch.dict(os.environ, {_VAR: raw}, clear=False):
             assert positive_int_env(_VAR, 8) == 8
 
@@ -55,33 +53,21 @@ def test_parses_valid_override(self, raw, expected):
         with patch.dict(os.environ, {_VAR: raw}, clear=False):
             assert positive_int_env(_VAR, 8) == expected
 
-    @pytest.mark.parametrize("raw", ["0", "-1", "-42"])
-    def test_rejects_non_positive(self, raw):
-        """Out of range raises rather than clamping, so a typo is not masked."""
+    @pytest.mark.parametrize("raw", ["0", "-1", "abc", "1.5", "inf", "nan"])
+    def test_invalid_logs_and_falls_back(self, raw, caplog):
         with patch.dict(os.environ, {_VAR: raw}, clear=False):
-            with pytest.raises(ValueError, match=">= 1"):
-                positive_int_env(_VAR, 8)
-
-    @pytest.mark.parametrize("raw", ["abc", "1.5", "inf", "nan", "8x", "0x10"])
-    def test_rejects_malformed(self, raw):
-        with patch.dict(os.environ, {_VAR: raw}, clear=False):
-            with pytest.raises(ValueError, match="must be an integer >= 1"):
-                positive_int_env(_VAR, 8)
+            assert positive_int_env(_VAR, 8) == 8
+        assert _VAR in caplog.text
+        assert raw in caplog.text
 
-    def test_error_names_variable_and_echoes_input(self):
-        with patch.dict(os.environ, {_VAR: "oops"}, clear=False):
-            with pytest.raises(ValueError) as excinfo:
-                positive_int_env(_VAR, 8)
-        message = str(excinfo.value)
-        assert _VAR in message
-        assert "oops" in message
+    def test_enforces_optional_maximum(self, caplog):
+        with patch.dict(os.environ, {_VAR: "65"}, clear=False):
+            assert positive_int_env(_VAR, 16, maximum=64) == 16
+        assert "between 1 and 64" in caplog.text
 
-    def test_malformed_error_does_not_chain_raw_int_error(self):
-        """``from None`` keeps the confusing built-in message out of the log."""
-        with patch.dict(os.environ, {_VAR: "abc"}, clear=False):
-            with pytest.raises(ValueError) as excinfo:
-                positive_int_env(_VAR, 8)
-        assert excinfo.value.__cause__ is None
+    def test_accepts_value_at_maximum(self):
+        with patch.dict(os.environ, {_VAR: "64"}, clear=False):
+            assert positive_int_env(_VAR, 16, maximum=64) == 64
 
 
 # ===========================================================================
@@ -107,27 +93,13 @@ def test_parses_valid_override(self, raw, expected):
             assert positive_finite_float_env(_VAR, 30.0) == expected
 
     @pytest.mark.parametrize(
-        "raw", ["inf", "Infinity", "-inf", "nan", "NaN", "0", "0.0", "-1"]
+        "raw", ["inf", "Infinity", "-inf", "nan", "0", "-1", "abc", "1.2.3"]
     )
-    def test_rejects_non_finite_and_non_positive(self, raw):
-        """``float()`` accepts inf/nan; an infinite deadline is not a deadline."""
-        with patch.dict(os.environ, {_VAR: raw}, clear=False):
-            with pytest.raises(ValueError, match="positive, finite"):
-                positive_finite_float_env(_VAR, 30.0)
-
-    @pytest.mark.parametrize("raw", ["abc", "1.2.3", "12s"])
-    def test_rejects_malformed(self, raw):
+    def test_invalid_logs_and_falls_back(self, raw, caplog):
         with patch.dict(os.environ, {_VAR: raw}, clear=False):
-            with pytest.raises(ValueError, match="positive, finite"):
-                positive_finite_float_env(_VAR, 30.0)
-
-    def test_error_names_variable_and_echoes_input(self):
-        with patch.dict(os.environ, {_VAR: "later"}, clear=False):
-            with pytest.raises(ValueError) as excinfo:
-                positive_finite_float_env(_VAR, 30.0)
-        message = str(excinfo.value)
-        assert _VAR in message
-        assert "later" in message
+            assert positive_finite_float_env(_VAR, 30.0) == 30.0
+        assert _VAR in caplog.text
+        assert raw in caplog.text
 
 
 # ===========================================================================
@@ -141,7 +113,7 @@ def _import_constant(module: str, constant: str, override: str | None):
     Redis is not installed in the test environment, so the stub that
     ``test_intelligent_cache.py`` installs is reproduced here for the child
     process. Returns the ``CompletedProcess`` so callers can assert on both the
-    printed value and a fail-fast non-zero exit.
+    printed value and the fallback diagnostic.
     """
     code = (
         "import sys, types;"
@@ -211,8 +183,11 @@ def test_override_is_applied_at_import(self, module, constant, override):
             (_FIRESTORE_MODULE, "CLEANUP_DELETE_TIMEOUT_SECONDS"),
         ],
     )
-    def test_invalid_override_fails_fast_at_import(self, module, constant):
-        """A bad value must stop the process, not run with a value nobody chose."""
+    def test_invalid_override_logs_and_uses_default(self, module, constant):
         result = _import_constant(module, constant, "0")
-        assert result.returncode != 0
+        assert result.returncode == 0, result.stderr
+        expected = "30.0" if constant.endswith("TIMEOUT_SECONDS") else (
+            "16" if constant == "CLEANUP_DELETE_CONCURRENCY" else "8"
+        )
+        assert result.stdout.strip() == expected
         assert constant in result.stderr
diff --git a/tests/unit/test_firestore_state.py b/tests/unit/test_firestore_state.py
index 4f471ff9c..b76abcb7f 100644
--- a/tests/unit/test_firestore_state.py
+++ b/tests/unit/test_firestore_state.py
@@ -736,48 +736,33 @@ async def test_list_states_applies_order_and_limit(self):
 
 
 class TestCleanupConfigEnvParsing:
-    """Tests for the env-override parsers backing the cleanup constants.
+    """Invalid overrides preserve startup and emit actionable diagnostics."""
 
-    ``float()`` accepts ``inf``/``-inf``/``nan``, so an unvalidated timeout
-    override could silently remove the per-delete deadline. These tests pin that
-    non-finite and non-positive values are rejected rather than clamped.
-    """
-
-    @pytest.mark.parametrize("raw", ["inf", "Infinity", "-inf", "nan", "0", "-1", "0.0"])
-    def test_float_env_rejects_non_finite_and_non_positive(self, raw):
+    @pytest.mark.parametrize("raw", ["inf", "-inf", "nan", "0", "-1", "abc"])
+    def test_float_env_invalid_falls_back(self, raw, caplog):
         with patch.dict(os.environ, {"CLEANUP_TEST_TIMEOUT": raw}, clear=False):
-            with pytest.raises(ValueError, match="positive, finite"):
-                _mod.positive_finite_float_env("CLEANUP_TEST_TIMEOUT", 30.0)
-
-    def test_float_env_rejects_malformed_value(self):
-        with patch.dict(os.environ, {"CLEANUP_TEST_TIMEOUT": "abc"}, clear=False):
-            with pytest.raises(ValueError):
-                _mod.positive_finite_float_env("CLEANUP_TEST_TIMEOUT", 30.0)
+            assert _mod.positive_finite_float_env("CLEANUP_TEST_TIMEOUT", 30.0) == 30.0
+        assert "CLEANUP_TEST_TIMEOUT" in caplog.text
 
     @pytest.mark.parametrize("raw", ["", "   "])
     def test_float_env_blank_falls_back_to_default(self, raw):
         with patch.dict(os.environ, {"CLEANUP_TEST_TIMEOUT": raw}, clear=False):
             assert _mod.positive_finite_float_env("CLEANUP_TEST_TIMEOUT", 30.0) == 30.0
 
-    def test_float_env_unset_falls_back_to_default(self):
-        os.environ.pop("CLEANUP_TEST_TIMEOUT", None)
-        assert _mod.positive_finite_float_env("CLEANUP_TEST_TIMEOUT", 30.0) == 30.0
-
     def test_float_env_parses_valid_override(self):
         with patch.dict(os.environ, {"CLEANUP_TEST_TIMEOUT": " 12.5 "}, clear=False):
             assert _mod.positive_finite_float_env("CLEANUP_TEST_TIMEOUT", 30.0) == 12.5
 
-    @pytest.mark.parametrize("raw", ["0", "-4"])
-    def test_int_env_rejects_non_positive(self, raw):
+    @pytest.mark.parametrize("raw", ["0", "-4", "abc", "1.5", "inf"])
+    def test_int_env_invalid_falls_back(self, raw, caplog):
         with patch.dict(os.environ, {"CLEANUP_TEST_POOL": raw}, clear=False):
-            with pytest.raises(ValueError, match=">= 1"):
-                _mod.positive_int_env("CLEANUP_TEST_POOL", 16)
+            assert _mod.positive_int_env("CLEANUP_TEST_POOL", 16) == 16
+        assert "CLEANUP_TEST_POOL" in caplog.text
 
-    @pytest.mark.parametrize("raw", ["abc", "1.5", "inf"])
-    def test_int_env_rejects_malformed_value(self, raw):
-        with patch.dict(os.environ, {"CLEANUP_TEST_POOL": raw}, clear=False):
-            with pytest.raises(ValueError):
-                _mod.positive_int_env("CLEANUP_TEST_POOL", 16)
+    def test_int_env_enforces_maximum(self, caplog):
+        with patch.dict(os.environ, {"CLEANUP_TEST_POOL": "65"}, clear=False):
+            assert _mod.positive_int_env("CLEANUP_TEST_POOL", 16, maximum=64) == 16
+        assert "between 1 and 64" in caplog.text
 
     def test_int_env_blank_falls_back_to_default(self):
         with patch.dict(os.environ, {"CLEANUP_TEST_POOL": ""}, clear=False):
@@ -787,8 +772,8 @@ def test_int_env_parses_valid_override(self):
         with patch.dict(os.environ, {"CLEANUP_TEST_POOL": " 4 "}, clear=False):
             assert _mod.positive_int_env("CLEANUP_TEST_POOL", 16) == 4
 
-    def test_module_defaults_are_positive_and_finite(self):
-        assert _mod.CLEANUP_DELETE_CONCURRENCY >= 1
+    def test_module_defaults_are_bounded_and_finite(self):
+        assert 1 <= _mod.CLEANUP_DELETE_CONCURRENCY <= _mod.CLEANUP_DELETE_CONCURRENCY_MAX
         assert math.isfinite(_mod.CLEANUP_DELETE_TIMEOUT_SECONDS)
         assert _mod.CLEANUP_DELETE_TIMEOUT_SECONDS > 0
 

From 1fbfe0aacc1064a13d99b37223d9f82565a42209 Mon Sep 17 00:00:00 2001
From: Hayden <154503486+groupthinking@users.noreply.github.com>
Date: Sun, 2 Aug 2026 08:41:14 -0500
Subject: [PATCH 3/3] style: restore lint baseline and re-add dropped
 invalid-input cases

The fail-safe rework left three lint regressions relative to the branch
point, and dropped six malformed-input cases from the parametrize lists.

- ruff I001: the absolute import in firestore_state.py is 89 chars, one
  over the limit, so the import block needed rewrapping.
- black: _fallback()'s signature and the expected-value ternary in
  test_env_config.py both exceeded 88 chars.
- Re-add the invalid inputs dropped in the rewrite: -42, 8x, 0x10 for
  int; NaN, 0.0, 12s for float. Each exercises a distinct rejection
  path (parse error, range check, finiteness check).

Pre-existing debt left untouched: firestore_state.py is already
black-dirty on origin/main, and the unused 'result' at
test_firestore_state.py:530 predates this branch.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
---
 src/youtube_extension/core/env_config.py      |  4 ++-
 .../services/cloud/firestore_state.py         |  5 +++-
 tests/unit/test_env_config.py                 | 28 ++++++++++++++++---
 3 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/src/youtube_extension/core/env_config.py b/src/youtube_extension/core/env_config.py
index 6ff12aa1e..362ec431f 100644
--- a/src/youtube_extension/core/env_config.py
+++ b/src/youtube_extension/core/env_config.py
@@ -25,7 +25,9 @@ def _raw_override(name: str) -> str | None:
     return raw.strip()
 
 
-def _fallback(name: str, raw: str, default: int | float, requirement: str) -> int | float:
+def _fallback(
+    name: str, raw: str, default: int | float, requirement: str
+) -> int | float:
     logger.warning(
         "Ignoring invalid %s=%r; expected %s. Using default %r.",
         name,
diff --git a/src/youtube_extension/services/cloud/firestore_state.py b/src/youtube_extension/services/cloud/firestore_state.py
index d28fe9401..c529822d2 100644
--- a/src/youtube_extension/services/cloud/firestore_state.py
+++ b/src/youtube_extension/services/cloud/firestore_state.py
@@ -14,7 +14,10 @@
 from datetime import datetime, timezone
 from typing import Any, Optional
 
-from youtube_extension.core.env_config import positive_finite_float_env, positive_int_env
+from youtube_extension.core.env_config import (
+    positive_finite_float_env,
+    positive_int_env,
+)
 
 try:
     from google.cloud import firestore
diff --git a/tests/unit/test_env_config.py b/tests/unit/test_env_config.py
index 3373ba798..9657d6e1c 100644
--- a/tests/unit/test_env_config.py
+++ b/tests/unit/test_env_config.py
@@ -53,7 +53,10 @@ def test_parses_valid_override(self, raw, expected):
         with patch.dict(os.environ, {_VAR: raw}, clear=False):
             assert positive_int_env(_VAR, 8) == expected
 
-    @pytest.mark.parametrize("raw", ["0", "-1", "abc", "1.5", "inf", "nan"])
+    @pytest.mark.parametrize(
+        "raw",
+        ["0", "-1", "-42", "abc", "1.5", "8x", "0x10", "inf", "nan"],
+    )
     def test_invalid_logs_and_falls_back(self, raw, caplog):
         with patch.dict(os.environ, {_VAR: raw}, clear=False):
             assert positive_int_env(_VAR, 8) == 8
@@ -93,7 +96,22 @@ def test_parses_valid_override(self, raw, expected):
             assert positive_finite_float_env(_VAR, 30.0) == expected
 
     @pytest.mark.parametrize(
-        "raw", ["inf", "Infinity", "-inf", "nan", "0", "-1", "abc", "1.2.3"]
+        "raw",
+        # "0.0" and "NaN" are spelling variants that ``float()`` accepts but the
+        # guard must still reject; "12s" is the unit-suffix typo a human writes.
+        [
+            "inf",
+            "Infinity",
+            "-inf",
+            "nan",
+            "NaN",
+            "0",
+            "0.0",
+            "-1",
+            "abc",
+            "1.2.3",
+            "12s",
+        ],
     )
     def test_invalid_logs_and_falls_back(self, raw, caplog):
         with patch.dict(os.environ, {_VAR: raw}, clear=False):
@@ -186,8 +204,10 @@ def test_override_is_applied_at_import(self, module, constant, override):
     def test_invalid_override_logs_and_uses_default(self, module, constant):
         result = _import_constant(module, constant, "0")
         assert result.returncode == 0, result.stderr
-        expected = "30.0" if constant.endswith("TIMEOUT_SECONDS") else (
-            "16" if constant == "CLEANUP_DELETE_CONCURRENCY" else "8"
+        expected = (
+            "30.0"
+            if constant.endswith("TIMEOUT_SECONDS")
+            else ("16" if constant == "CLEANUP_DELETE_CONCURRENCY" else "8")
         )
         assert result.stdout.strip() == expected
         assert constant in result.stderr