add more tests and pr comments

autogpt_platform/backend/backend/data/tally.py

@@ -53,20 +53,24 @@ def _mask_email(email: str) -> str:
 
 
 async def _fetch_tally_page(
+    client: Requests,
     form_id: str,
     page: int,
     limit: int = _PAGE_LIMIT,
     start_date: Optional[str] = None,
 ) -> dict:
     """Fetch a single page of submissions from the Tally API."""
-    settings = Settings()
-    api_key = settings.secrets.tally_api_key
-
     url = f"{TALLY_API_BASE}/forms/{form_id}/submissions?page={page}&limit={limit}"
     if start_date:
         url += f"&startDate={start_date}"
 
-    client = Requests(
+    response = await client.get(url)
+    return response.json()
+
+
+def _make_tally_client(api_key: str) -> Requests:
+    """Create a Requests client configured for the Tally API."""
+    return Requests(
         trusted_origins=[TALLY_API_BASE],
         raise_for_status=True,
         extra_headers={
@@ -74,8 +78,6 @@ async def _fetch_tally_page(
             "Accept": "application/json",
         },
     )
-    response = await client.get(url)
-    return response.json()
 
 
 async def _fetch_all_submissions(
@@ -84,12 +86,15 @@ async def _fetch_all_submissions(
     max_pages: int = _MAX_PAGES,
 ) -> tuple[list[dict], list[dict]]:
     """Paginate through all Tally submissions. Returns (questions, submissions)."""
+    settings = Settings()
+    client = _make_tally_client(settings.secrets.tally_api_key)
+
     questions: list[dict] = []
     all_submissions: list[dict] = []
     page = 1
 
     while True:
-        data = await _fetch_tally_page(form_id, page, start_date=start_date)
+        data = await _fetch_tally_page(client, form_id, page, start_date=start_date)
 
         if page == 1:
             questions = data.get("questions", [])
@@ -325,9 +330,9 @@ Fields:
 - additional_notes (string): any additional context
 
 Form data:
-{submission_text}
+"""
 
-Return ONLY valid JSON."""
+_EXTRACTION_SUFFIX = "\n\nReturn ONLY valid JSON."
 
 
 async def extract_business_understanding(
@@ -348,9 +353,7 @@ async def extract_business_understanding(
                 messages=[
                     {
                         "role": "user",
-                        "content": _EXTRACTION_PROMPT.format(
-                            submission_text=formatted_text
-                        ),
+                        "content": f"{_EXTRACTION_PROMPT}{formatted_text}{_EXTRACTION_SUFFIX}",
                     }
                 ],
                 response_format={"type": "json_object"},

autogpt_platform/backend/backend/data/tally_test.py

@@ -5,8 +5,11 @@ from unittest.mock import AsyncMock, MagicMock, patch
 import pytest
 
 from backend.data.tally import (
+    _EXTRACTION_PROMPT,
+    _EXTRACTION_SUFFIX,
     _build_email_index,
     _format_answer,
+    _make_tally_client,
     _mask_email,
     find_submission_by_email,
     format_submission_for_llm,
@@ -356,3 +359,60 @@ def test_mask_email():
 
 def test_mask_email_invalid():
     assert _mask_email("no-at-sign") == "***"
+
+
+# ── Prompt construction (curly-brace safety) ─────────────────────────────────
+
+
+def test_extraction_prompt_safe_with_curly_braces():
+    """User content with curly braces must not break prompt construction.
+
+    Previously _EXTRACTION_PROMPT.format(submission_text=...) would raise
+    KeyError/ValueError if the user text contained { or }.
+    """
+    text_with_braces = "Q: What tools do you use?\nA: We use {Slack} and {{Jira}}"
+    # This must not raise — the old .format() call would fail here
+    prompt = f"{_EXTRACTION_PROMPT}{text_with_braces}{_EXTRACTION_SUFFIX}"
+    assert text_with_braces in prompt
+    assert prompt.startswith("You are a business analyst.")
+    assert prompt.endswith("Return ONLY valid JSON.")
+
+
+def test_extraction_prompt_no_format_placeholders():
+    """_EXTRACTION_PROMPT must not contain Python format placeholders."""
+    assert "{submission_text}" not in _EXTRACTION_PROMPT
+    # Ensure no stray single-brace placeholders
+    # (double braces {{ are fine — they're literal in format strings)
+    import re
+
+    single_braces = re.findall(r"(?<!\{)\{[^{].*?\}(?!\})", _EXTRACTION_PROMPT)
+    assert single_braces == [], f"Found format placeholders: {single_braces}"
+
+
+# ── _make_tally_client ───────────────────────────────────────────────────────
+
+
+def test_make_tally_client_returns_configured_client():
+    """_make_tally_client should create a Requests client with auth headers."""
+    client = _make_tally_client("test-api-key")
+    assert "Bearer test-api-key" in str(client.extra_headers.get("Authorization", ""))
+
+
+@pytest.mark.asyncio
+async def test_fetch_tally_page_uses_provided_client():
+    """_fetch_tally_page should use the passed client, not create its own."""
+    from backend.data.tally import _fetch_tally_page
+
+    mock_response = MagicMock()
+    mock_response.json.return_value = {"submissions": [], "questions": []}
+
+    mock_client = AsyncMock()
+    mock_client.get.return_value = mock_response
+
+    result = await _fetch_tally_page(mock_client, "form123", page=1)
+
+    mock_client.get.assert_awaited_once()
+    call_url = mock_client.get.call_args[0][0]
+    assert "form123" in call_url
+    assert "page=1" in call_url
+    assert result == {"submissions": [], "questions": []}