BasedHardware
diff --git a/‎backend/database/goals.py‎
Lines changed: 14 additions & 1 deletion b/‎backend/database/goals.py‎
Lines changed: 14 additions & 1 deletion
diff --git a/‎backend/database/users.py‎
Lines changed: 10 additions & 1 deletion b/‎backend/database/users.py‎
Lines changed: 10 additions & 1 deletion
diff --git a/‎backend/routers/developer.py‎
Lines changed: 5 additions & 1 deletion b/‎backend/routers/developer.py‎
Lines changed: 5 additions & 1 deletion
diff --git a/‎backend/tests/unit/test_dev_goals_limit_include_inactive.py‎
Lines changed: 178 additions & 0 deletions b/‎backend/tests/unit/test_dev_goals_limit_include_inactive.py‎
Lines changed: 178 additions & 0 deletions
diff --git a/‎backend/tests/unit/test_subscription_import_cycle.py‎
Lines changed: 62 additions & 0 deletions b/‎backend/tests/unit/test_subscription_import_cycle.py‎
Lines changed: 62 additions & 0 deletions
@@ -337,15 +337,28 @@ def get_all_goals(
     uid: str,
     include_inactive: bool = False,
     *,
+    limit: Optional[int] = None,
     firestore_client: Any = None,
 ) -> List[Dict[str, Any]]:
+    """Fetch a user's goals, newest first.
+
+    ``limit`` bounds the returned page after the in-Python newest-first sort. It is opt-in:
+    every existing caller omits it and keeps the full-list behaviour they rely on.
+
+    The bound is deliberately NOT pushed into the Firestore query: ``order_by('created_at')``
+    excludes documents that lack the field entirely, and legacy or manually created goals can
+    lack ``created_at`` — a query-level order+limit would silently drop them. Instead the full
+    stream is sorted here (goals without ``created_at`` coerce to ``datetime.min`` and sort
+    last) and the page is sliced, so the bound caps the response payload while dateless legacy
+    goals still appear once dated goals run out.
+    """
     collection = _get_db(firestore_client).collection(users_collection).document(uid).collection(goals_collection)
     query = collection if include_inactive else collection.where(filter=FieldFilter('is_active', '==', True))
     goals = [normalize_goal_storage(_goal_dict(doc), goal_id=doc.id) for doc in query.stream()]
     if not include_inactive:
         goals = [goal for goal in goals if goal['is_active']]
     goals.sort(key=_goal_created_at_sort_key, reverse=True)
-    return goals
+    return goals if limit is None else goals[:limit]
 
 
 def create_goal(
 
@@ -29,7 +29,6 @@
     SubscriptionStatus,
 )
 from models.other import Person
-from utils.subscription import get_default_basic_subscription
 import logging
 
 logger = logging.getLogger(__name__)
@@ -1547,6 +1546,11 @@ def subscription_payload(_snapshot: object) -> dict:
             return subscription
 
     # If subscription doesn't exist for the user, create and return a default free plan.
+    # Imported here, not at module scope: utils.subscription imports this module, so a
+    # module-level import makes utils.subscription unimportable on its own (see
+    # get_user_valid_subscription, which defers the same import for the same reason).
+    from utils.subscription import get_default_basic_subscription
+
     default_subscription = get_default_basic_subscription()
     # Strip dynamic fields before storing
     sub_to_store = default_subscription.model_dump()
@@ -1678,6 +1682,11 @@ def get_user_valid_subscription(
     quota must use that mode against the customer Firestore so a miss cannot
     stamp ``plan: basic`` onto a paying user.
     """
+    # Imported here, not at module scope: utils.subscription imports this module, and a
+    # module-level import back into it forms a cycle that makes utils.subscription raise
+    # ImportError whenever it is the first of the pair to be imported.
+    from utils.subscription import get_default_basic_subscription
+
     if provision:
         subscription = get_user_subscription(uid, firestore_client=firestore_client)
     else:
 
@@ -1942,7 +1942,11 @@ def get_goals(
     # oversized limit cannot stream the whole collection. Mirrors the GET /v3/memories hardening.
     limit = max(1, min(limit, 1000))
     if include_inactive:
-        goals = goals_db.get_all_goals(uid, include_inactive=True)
+        # Pass the clamp down so the response honours the documented limit. The bound is
+        # applied after the in-Python newest-first sort rather than at the query, because a
+        # Firestore order_by('created_at') would silently exclude legacy goals that lack the
+        # field; see get_all_goals.
+        goals = goals_db.get_all_goals(uid, include_inactive=True, limit=limit)
     else:
         goals = goals_db.get_user_goals(uid, limit=limit)
 
 
@@ -0,0 +1,178 @@
+"""Regression test: GET /v1/dev/user/goals must honour `limit` when include_inactive=true.
+
+routers.developer.get_goals clamps `limit` to [1, 1000] and then branches. The default branch
+calls goals_db.get_user_goals(uid, limit=limit), which is bounded. The include_inactive=True
+branch called goals_db.get_all_goals(uid, include_inactive=True), which had no limit parameter
+and streamed the whole goals collection, so the clamp was dead code on that branch and the
+endpoint returned every goal the user had ever created -- ignoring its own documented
+"**limit**: Maximum number of goals to return".
+
+The clamp is now passed down and applied after the in-Python newest-first sort. The bound is
+deliberately not pushed into the Firestore query: order_by('created_at') excludes documents
+missing the field, and legacy goals can lack created_at, so a query-level bound would silently
+drop them. These tests assert the bounded page is the NEWEST goals, that dateless legacy goals
+survive bounding (sorting last), and that the bounded path is taken by the route.
+
+get_all_goals stays fetch-everything by default for its other callers
+(/v1/dev/user/goals/{goal_id}, routers/goals.py::get_all_goals, and the MCP goal reads); the
+limit is opt-in and only this route passes it.
+"""
+
+import pytest
+
+import database.goals as goals_db_module
+import routers.developer as developer
+
+
+def _fake_goals(count):
+    return [{'id': f'g{index}', 'title': f'goal {index}'} for index in range(count)]
+
+
+# --- router: the clamp reaches the helper -----------------------------------------------
+
+
+def test_include_inactive_passes_the_clamp_down_to_the_query(monkeypatch):
+    captured = {}
+
+    def get_all_goals(uid, include_inactive=False, *, limit=None):
+        captured.update(uid=uid, include_inactive=include_inactive, limit=limit)
+        return _fake_goals(min(limit, 25))
+
+    monkeypatch.setattr(developer.goals_db, 'get_all_goals', get_all_goals)
+
+    result = developer.get_goals(uid='u1', limit=5, include_inactive=True)
+
+    # The bound is delegated, not applied after the fact.
+    assert captured == {'uid': 'u1', 'include_inactive': True, 'limit': 5}
+    assert len(result) == 5
+
+
+def test_include_inactive_delegates_the_clamp_ceiling(monkeypatch):
+    captured = {}
+
+    def get_all_goals(uid, include_inactive=False, *, limit=None):
+        captured.update(limit=limit)
+        return _fake_goals(limit)
+
+    monkeypatch.setattr(developer.goals_db, 'get_all_goals', get_all_goals)
+
+    result = developer.get_goals(uid='u1', limit=99999, include_inactive=True)
+
+    assert captured == {'limit': 1000}
+    assert len(result) == 1000
+
+
+def test_active_only_branch_still_delegates_the_limit(monkeypatch):
+    captured = {}
+
+    def get_user_goals(uid, limit):
+        captured.update(uid=uid, limit=limit)
+        return _fake_goals(3)
+
+    monkeypatch.setattr(developer.goals_db, 'get_user_goals', get_user_goals)
+
+    result = developer.get_goals(uid='u1', limit=3, include_inactive=False)
+
+    assert captured == {'uid': 'u1', 'limit': 3}
+    assert len(result) == 3
+
+
+# --- database: the page is the newest goals and legacy goals survive ---------------------
+
+
+class _FakeDoc:
+    def __init__(self, doc_id, payload):
+        self.id = doc_id
+        self._payload = payload
+
+    def to_dict(self):
+        return dict(self._payload)
+
+
+class _FakeCollection:
+    """Streams the given docs; records whether a query-level order/limit was pushed down
+    (it must NOT be — that is the legacy-goal-dropping shape this fix avoids)."""
+
+    def __init__(self, docs, calls):
+        self._docs = docs
+        self.calls = calls
+
+    def collection(self, _name):
+        return self
+
+    def document(self, _name):
+        return self
+
+    def where(self, **kwargs):
+        self.calls.append(('where', kwargs))
+        return self
+
+    def order_by(self, field, direction=None):
+        self.calls.append(('order_by', field, direction))
+        return self
+
+    def limit(self, count):
+        self.calls.append(('limit', count))
+        return self
+
+    def stream(self):
+        return iter(self._docs)
+
+
+def _docs(count):
+    from datetime import datetime, timedelta, timezone
+
+    base = datetime(2026, 1, 1, tzinfo=timezone.utc)
+    return [
+        _FakeDoc(
+            f'g{index}',
+            {'created_at': base + timedelta(days=index), 'is_active': True, 'status': 'background'},
+        )
+        for index in range(count)
+    ]
+
+
+def test_get_all_goals_bounded_page_is_the_newest_goals():
+    calls = []
+    client = _FakeCollection(_docs(50), calls)
+
+    result = goals_db_module.get_all_goals('u1', include_inactive=True, limit=5, firestore_client=client)
+
+    # Newest first: the docs were streamed oldest-first, so the page must be the LAST five
+    # ids in reverse creation order — proving the sort ran before the slice.
+    assert [g['id'] for g in result] == ['g49', 'g48', 'g47', 'g46', 'g45']
+    # And the bound was never pushed into the query, where order_by('created_at') would
+    # exclude legacy goals lacking the field.
+    assert not any(call[0] in ('order_by', 'limit') for call in calls)
+
+
+def test_get_all_goals_keeps_legacy_goals_without_created_at():
+    calls = []
+    dated = _docs(3)
+    legacy = _FakeDoc('legacy', {'is_active': True, 'status': 'background'})  # no created_at
+    client = _FakeCollection(dated + [legacy], calls)
+
+    result = goals_db_module.get_all_goals('u1', include_inactive=True, limit=10, firestore_client=client)
+
+    # The dateless legacy goal is retained (a query-level order_by would have dropped it)
+    # and sorts deterministically last.
+    assert [g['id'] for g in result] == ['g2', 'g1', 'g0', 'legacy']
+
+
+def test_get_all_goals_bounds_still_apply_over_legacy_goals():
+    client = _FakeCollection(_docs(2) + [_FakeDoc('legacy', {'is_active': True, 'status': 'background'})], [])
+
+    result = goals_db_module.get_all_goals('u1', include_inactive=True, limit=2, firestore_client=client)
+
+    # Dated goals fill the page first; the dateless one only appears when room remains.
+    assert [g['id'] for g in result] == ['g1', 'g0']
+
+
+def test_get_all_goals_stays_unbounded_for_existing_callers():
+    calls = []
+    client = _FakeCollection(_docs(50), calls)
+
+    result = goals_db_module.get_all_goals('u1', include_inactive=True, firestore_client=client)
+
+    assert not any(call[0] in ('limit', 'order_by') for call in calls)
+    assert len(result) == 50
@@ -0,0 +1,62 @@
+"""Regression: utils.subscription must be importable on its own.
+
+utils/subscription.py imports database.users, and database/users.py imported
+get_default_basic_subscription back from utils.subscription at module scope. That pair formed a
+cycle, so whichever of the two was imported first decided whether it worked: importing
+utils.subscription first raised
+
+    ImportError: cannot import name 'get_default_basic_subscription' from partially initialized
+    module 'utils.subscription' (most likely due to a circular import)
+
+while importing database.users first happened to succeed. The order-dependence is the bug, which
+is why it surfaces intermittently as unrelated test files change what gets imported first. The
+database -> utils edge is also the reverse of the documented backend layering
+(database/ -> utils/ -> routers/ -> main.py).
+
+A fresh interpreter is the only honest seam here: once pytest has imported either module, the
+order is already decided for the whole session, so an in-process assertion would pass either way.
+"""
+
+import os
+import subprocess
+import sys
+from pathlib import Path
+
+import pytest
+
+# Marked slow: each test boots a fresh interpreter that imports the full backend
+# module graph, which per tests/README.md belongs outside the PR fast lane
+# (`not integration and not slow`). A fresh interpreter is still the only honest
+# seam for import-order bugs, so the cost is the test's point, not an accident.
+
+BACKEND_ROOT = Path(__file__).resolve().parents[2]
+
+
+def _import_first_in_fresh_interpreter(module: str) -> subprocess.CompletedProcess:
+    env = dict(os.environ)
+    env.setdefault("ENCRYPTION_SECRET", "test_secret_for_ci_only_0123456789")
+    env.setdefault("OPENAI_API_KEY", "sk-fake")
+    env.setdefault("PINECONE_API_KEY", "fake")
+    return subprocess.run(
+        [sys.executable, "-c", f"import {module}"],
+        cwd=str(BACKEND_ROOT),
+        env=env,
+        capture_output=True,
+        text=True,
+        timeout=180,
+    )
+
+
+@pytest.mark.slow
+def test_utils_subscription_imports_standalone():
+    result = _import_first_in_fresh_interpreter("utils.subscription")
+
+    assert result.returncode == 0, f"utils.subscription is not importable on its own:\n{result.stderr}"
+
+
+@pytest.mark.slow
+def test_database_users_imports_standalone():
+    # The other side of the pair must keep working too.
+    result = _import_first_in_fresh_interpreter("database.users")
+
+    assert result.returncode == 0, f"database.users is not importable on its own:\n{result.stderr}"