Merge branch 'feat/error-diagnostics'

Shareable in-app error diagnostics: always-visible failure panel,
durable event log, provider request_id/HTTP-status capture, and a
redacted report users can download or send to the team.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

app/main.py

@@ -31,7 +31,7 @@ from app.ui import (
     analysis_screen,
     proposal_screen,
 )
-from app.ui import api_usage_panel, motion_runtime, theme
+from app.ui import api_usage_panel, diagnostics_panel, motion_runtime, theme
 
 
 # (step key, sidebar label) in flow order.
@@ -253,6 +253,12 @@ def _render_sidebar(show_debug: bool) -> None:
         st.session_state.theme_dark = dark_on
         st.rerun()
 
+    # Problem/diagnostics panel. NOT debug-gated: a user who hits an error is
+    # precisely the user who cannot flip SHOW_DEBUG_PANEL, so the explanation
+    # and the share button have to be reachable by default. Renders nothing
+    # when there are no failures, leaving a healthy run visually unchanged.
+    diagnostics_panel.render(key_suffix="sidebar")
+
     # Developer-only API usage panel. Hidden unless SHOW_DEBUG_PANEL=true.
     api_usage_panel.render(key_suffix="sidebar")
 

app/services/diagnostics.py

@@ -0,0 +1,450 @@
+"""Durable, shareable error diagnostics.
+
+The session-state usage log (:mod:`app.services.llm_client`) is the right
+surface for "what happened during *this* run", but it dies the moment the
+browser reloads — which is a user's first instinct when something breaks,
+so the evidence is usually gone before they think to report it. This
+module keeps a **durable** copy on disk and turns it into a single,
+redacted report a user can hand to the app team.
+
+Three responsibilities
+----------------------
+1. :func:`record_event` — append one call record to a bounded JSONL file
+   the instant it happens (never at share time).
+2. :func:`build_report` — assemble an envelope (app/runtime versions,
+   active provider/model, anonymous browser tag) plus the most recent
+   events into one JSON-serialisable dict.
+3. :func:`upload_report` — optionally push that report to a private
+   Supabase bucket so support can pull it by ``report_id``.
+
+What is never written
+---------------------
+The same contract :mod:`app.utils.logging_utils` already commits to:
+**metadata only**. No dossier text, no screenshot bytes, no job
+descriptions, no generated proposals, no API keys. Records arrive here
+already sanitised by ``llm_client``; :func:`_scrub` re-applies
+:func:`app.utils.redaction.redact` on the way in *and* on the way out
+because it is idempotent and the cost of a double-scrub is zero next to
+the cost of leaking a key into a file a user emails around.
+
+Fail-safe
+---------
+Every public function swallows its own errors and degrades to a no-op or
+an empty result. Diagnostics must never be the reason the app breaks —
+a logger that crashes the program it is instrumenting is worse than no
+logger at all.
+"""
+
+from __future__ import annotations
+
+import json
+import os
+import platform
+import secrets
+import sys
+from datetime import datetime, timezone
+from typing import Any, Optional
+
+from app.config import LOG_DIR
+from app.utils.redaction import redact
+
+
+# Report format version — bump when the envelope/event shape changes so a
+# support tool reading old reports can tell which schema it is looking at.
+SCHEMA_VERSION = 1
+
+# Where the durable log lives. Sits beside app.log in the same per-user
+# state dir (repo root in dev, %APPDATA% when packaged) — see app.paths.
+_EVENTS_FILENAME = "diagnostics.jsonl"
+
+# Bounded on-disk history. A long session with retries would otherwise grow
+# the file without limit. We append cheaply and only pay the rewrite cost
+# when the file crosses the size ceiling.
+_MAX_EVENTS_ON_DISK = 500
+_TRIM_WHEN_BYTES_EXCEED = 1_000_000  # ~1 MB
+
+# How many events a generated report carries. Deliberately smaller than the
+# on-disk cap: a report is for a human to read and a support engineer to
+# scan, not a full archive.
+_REPORT_EVENT_LIMIT = 50
+
+# Supabase bucket for uploaded reports. Separate from the `apikeys` bucket
+# used by key_store — different data, different retention, different blast
+# radius if either is ever misconfigured.
+_BUCKET = "diagnostics"
+_TIMEOUT = 8.0
+
+
+# ---------------------------------------------------------------------------
+# Paths
+# ---------------------------------------------------------------------------
+def events_path():
+    """Absolute path to the durable JSONL event log."""
+    return LOG_DIR / _EVENTS_FILENAME
+
+
+# ---------------------------------------------------------------------------
+# Scrubbing
+# ---------------------------------------------------------------------------
+def _scrub(value: Any) -> Any:
+    """Recursively redact secrets from any JSON-ish value.
+
+    ``redact`` is idempotent, so applying it to already-clean records is
+    free insurance rather than duplicated work.
+    """
+    if isinstance(value, str):
+        return redact(value)
+    if isinstance(value, dict):
+        return {k: _scrub(v) for k, v in value.items()}
+    if isinstance(value, (list, tuple)):
+        return [_scrub(v) for v in value]
+    return value
+
+
+# ---------------------------------------------------------------------------
+# Recording
+# ---------------------------------------------------------------------------
+def record_event(entry: dict) -> None:
+    """Append one call record to the durable log. Never raises.
+
+    Called from ``llm_client._finalize`` for *every* call — success and
+    failure alike — so a report can show what worked right up to the point
+    something didn't.
+    """
+    if not isinstance(entry, dict):
+        return
+    try:
+        LOG_DIR.mkdir(parents=True, exist_ok=True)
+        path = events_path()
+        line = json.dumps(_scrub(entry), ensure_ascii=False, default=str)
+        with open(path, "a", encoding="utf-8") as fh:
+            fh.write(line + "\n")
+        _trim_if_large(path)
+    except Exception:  # noqa: BLE001 - diagnostics must never break the app
+        return
+
+
+def _trim_if_large(path) -> None:
+    """Keep the newest ``_MAX_EVENTS_ON_DISK`` lines once the file is big.
+
+    Checking size first keeps the common append path O(1); the rewrite only
+    happens on the rare crossing of the ceiling.
+    """
+    try:
+        if path.stat().st_size <= _TRIM_WHEN_BYTES_EXCEED:
+            return
+        with open(path, "r", encoding="utf-8", errors="replace") as fh:
+            lines = fh.readlines()
+        keep = lines[-_MAX_EVENTS_ON_DISK:]
+        with open(path, "w", encoding="utf-8") as fh:
+            fh.writelines(keep)
+    except Exception:  # noqa: BLE001
+        return
+
+
+def load_events(limit: int = _REPORT_EVENT_LIMIT) -> list[dict]:
+    """Return the most recent ``limit`` events, oldest first. Never raises.
+
+    Malformed lines (a half-written record from a hard kill) are skipped
+    rather than failing the whole read.
+    """
+    try:
+        path = events_path()
+        if not path.exists():
+            return []
+        with open(path, "r", encoding="utf-8", errors="replace") as fh:
+            lines = fh.readlines()
+    except Exception:  # noqa: BLE001
+        return []
+
+    out: list[dict] = []
+    for raw in lines[-max(1, limit):]:
+        raw = raw.strip()
+        if not raw:
+            continue
+        try:
+            parsed = json.loads(raw)
+        except ValueError:
+            continue
+        if isinstance(parsed, dict):
+            out.append(parsed)
+    return out
+
+
+def clear_events() -> bool:
+    """Delete the durable log. Returns True when it is gone afterwards."""
+    try:
+        path = events_path()
+        if path.exists():
+            path.unlink()
+        return True
+    except Exception:  # noqa: BLE001
+        return False
+
+
+# ---------------------------------------------------------------------------
+# Report assembly
+# ---------------------------------------------------------------------------
+def new_report_id() -> str:
+    """Short, readable id a user can read aloud to support.
+
+    Format ``UPS-<YYYYMMDD>-<6 hex>`` — date-prefixed so support can find
+    the right day's uploads without opening every object.
+    """
+    stamp = datetime.now(timezone.utc).strftime("%Y%m%d")
+    return f"UPS-{stamp}-{secrets.token_hex(3).upper()}"
+
+
+def _package_version(name: str) -> Optional[str]:
+    try:
+        from importlib.metadata import version
+
+        return version(name)
+    except Exception:  # noqa: BLE001 - package absent or metadata unreadable
+        return None
+
+
+def _app_revision() -> Optional[str]:
+    """Best-effort build identifier.
+
+    Prefers an explicit ``APP_REVISION`` env var (which a deploy can set to
+    the commit SHA); falls back to reading ``.git/HEAD`` in a dev checkout.
+    Returns None when neither is available — a packaged build with no env
+    var simply reports no revision rather than guessing.
+    """
+    explicit = (os.getenv("APP_REVISION") or "").strip()
+    if explicit:
+        return explicit[:40]
+    try:
+        from app.paths import resource_base
+
+        head = resource_base() / ".git" / "HEAD"
+        if not head.exists():
+            return None
+        ref = head.read_text(encoding="utf-8").strip()
+        if ref.startswith("ref:"):
+            target = resource_base() / ".git" / ref.split(" ", 1)[1].strip()
+            if target.exists():
+                return target.read_text(encoding="utf-8").strip()[:40]
+            return None
+        return ref[:40]
+    except Exception:  # noqa: BLE001
+        return None
+
+
+def _browser_tag() -> Optional[str]:
+    """Anonymous, stable per-browser id — lets support correlate repeat
+    reports from one user without any login. Never identifies a person."""
+    try:
+        from app.services import browser_tag
+
+        return browser_tag.get_or_create_tag()
+    except Exception:  # noqa: BLE001
+        return None
+
+
+def _active_config() -> dict:
+    """Provider/model context for the report. Never includes the key."""
+    try:
+        from app.config import get_settings
+
+        settings = get_settings()
+        return {
+            "provider": getattr(settings, "active_provider", None)
+            or getattr(settings, "llm_provider", None),
+            "model": getattr(settings, "active_model", None),
+            "vision_model": getattr(settings, "active_vision_model", None),
+            "has_api_key": bool(getattr(settings, "has_api_key", False)),
+        }
+    except Exception:  # noqa: BLE001
+        return {}
+
+
+def _environment() -> dict:
+    try:
+        packaged = False
+        try:
+            from app.paths import is_packaged
+
+            packaged = bool(is_packaged())
+        except Exception:  # noqa: BLE001
+            pass
+        return {
+            "python": sys.version.split()[0],
+            "platform": platform.platform(),
+            "packaged": packaged,
+            "streamlit": _package_version("streamlit"),
+            "anthropic_sdk": _package_version("anthropic"),
+            "openai_sdk": _package_version("openai"),
+            "google_generativeai_sdk": _package_version("google-generativeai"),
+        }
+    except Exception:  # noqa: BLE001
+        return {}
+
+
+def build_report(
+    *,
+    report_id: Optional[str] = None,
+    limit: int = _REPORT_EVENT_LIMIT,
+    session_events: Optional[list[dict]] = None,
+) -> dict:
+    """Assemble the full, redacted diagnostics report.
+
+    ``session_events`` lets a caller pass the in-memory session log so a
+    report is complete even if the disk write was blocked (a read-only
+    volume, a locked file). Disk events win when both are present because
+    they span earlier sessions too.
+    """
+    events = load_events(limit=limit)
+    if not events and session_events:
+        events = list(session_events)[-limit:]
+
+    report = {
+        "report_id": report_id or new_report_id(),
+        "schema_version": SCHEMA_VERSION,
+        "generated_at": datetime.now(timezone.utc).isoformat(timespec="seconds"),
+        "app_revision": _app_revision(),
+        "browser_tag": _browser_tag(),
+        "config": _active_config(),
+        "environment": _environment(),
+        "event_count": len(events),
+        "events": events,
+        "contains": (
+            "Metadata only — no API key, dossier text, screenshots, job "
+            "descriptions, or generated proposals."
+        ),
+    }
+    return _scrub(report)
+
+
+def report_to_json(report: dict) -> str:
+    """Pretty-printed JSON for download / clipboard. Never raises."""
+    try:
+        return json.dumps(report, indent=2, ensure_ascii=False, default=str)
+    except Exception:  # noqa: BLE001
+        return json.dumps({"error": "report serialization failed"}, indent=2)
+
+
+def error_events(events: list[dict]) -> list[dict]:
+    """Filter ``events`` down to the ones that represent a real failure.
+
+    ``local_placeholder`` is excluded: it is a deliberate design choice
+    (a stage that ran without the API), not an error.
+    """
+    ok_statuses = {"ok", "local_placeholder"}
+    return [
+        e
+        for e in events
+        if isinstance(e, dict) and str(e.get("status") or "") not in ok_statuses
+    ]
+
+
+def unresolved_errors(events: list[dict]) -> list[dict]:
+    """Failures that occurred *after* the last successful API call.
+
+    The durable log deliberately outlives the session, which creates a trap
+    the session-only log never had: a user who fixes a bad key would keep
+    seeing yesterday's failure forever, and a permanent warning is one
+    users learn to ignore. A successful call is the app's own evidence that
+    whatever was broken now works, so anything before it is history rather
+    than a live problem.
+
+    Position is used rather than timestamps because ``events`` is already
+    append-ordered — no parsing, and no dependence on clock consistency
+    across sessions.
+    """
+    last_ok = -1
+    for idx, event in enumerate(events):
+        if isinstance(event, dict) and str(event.get("status") or "") == "ok":
+            last_ok = idx
+    return error_events(events[last_ok + 1:])
+
+
+# ---------------------------------------------------------------------------
+# Optional upload (same Supabase project as key_store, different bucket)
+# ---------------------------------------------------------------------------
+def _base_url() -> str:
+    return (os.getenv("SUPABASE_URL") or "").rstrip("/")
+
+
+def _service_key() -> str:
+    return os.getenv("SUPABASE_SERVICE_KEY") or ""
+
+
+def is_upload_configured() -> bool:
+    """True only when Supabase credentials are present in the environment."""
+    return bool(_base_url() and _service_key())
+
+
+def _auth_headers(extra: Optional[dict] = None) -> dict:
+    key = _service_key()
+    headers = {"Authorization": f"Bearer {key}", "apikey": key}
+    if extra:
+        headers.update(extra)
+    return headers
+
+
+def _http(method: str, url: str, headers: dict, content: Optional[bytes] = None):
+    """One HTTP request → ``(status_code, body_bytes)``.
+
+    Isolated so tests can replace it with an in-memory fake, mirroring
+    :mod:`app.services.key_store`.
+    """
+    import httpx
+
+    with httpx.Client(timeout=_TIMEOUT) as client:
+        resp = client.request(method, url, headers=headers, content=content)
+        return resp.status_code, resp.content
+
+
+def _ensure_bucket() -> None:
+    """Create the private bucket if missing. Idempotent; errors ignored."""
+    url = f"{_base_url()}/storage/v1/bucket"
+    body = json.dumps({"id": _BUCKET, "name": _BUCKET, "public": False}).encode()
+    try:
+        _http("POST", url, _auth_headers({"Content-Type": "application/json"}), body)
+    except Exception:  # noqa: BLE001 - best-effort; upload surfaces real failure
+        pass
+
+
+def upload_report(report: dict) -> bool:
+    """Upload ``report`` to the private diagnostics bucket. Never raises.
+
+    Returns False (rather than raising) when Supabase is not configured or
+    the network call fails, so the UI can fall back to "download and email
+    it instead" without any error handling of its own.
+    """
+    if not is_upload_configured() or not isinstance(report, dict):
+        return False
+    try:
+        _ensure_bucket()
+        report_id = str(report.get("report_id") or new_report_id())
+        # Safe object name: the id is generated by us, but never trust a
+        # value straight into a URL path.
+        safe_id = "".join(ch for ch in report_id if ch.isalnum() or ch in "-_")
+        payload = report_to_json(report).encode("utf-8")
+        url = f"{_base_url()}/storage/v1/object/{_BUCKET}/{safe_id}.json"
+        status, _ = _http(
+            "POST",
+            url,
+            _auth_headers({"Content-Type": "application/json", "x-upsert": "true"}),
+            payload,
+        )
+        return 200 <= status < 300
+    except Exception:  # noqa: BLE001 - fail-safe: never break the UI
+        return False
+
+
+__all__ = [
+    "SCHEMA_VERSION",
+    "record_event",
+    "load_events",
+    "clear_events",
+    "events_path",
+    "build_report",
+    "report_to_json",
+    "error_events",
+    "new_report_id",
+    "is_upload_configured",
+    "upload_report",
+]

app/services/llm_client.py

@@ -28,10 +28,17 @@ Every call returns an :class:`LLMCallResult` with::
     status: str             # "ok" | "no_api" | "invalid_key" | "quota" |
                             # "connection" | "model_missing" | "parse_error"
                             # | "unsupported_provider" | "skipped" | "exception"
+    request_id: str | None  # provider-side call id (req_…), for support
+    http_status: int | None # exact HTTP status from the SDK exception
+    error_type: str | None  # provider's machine-readable error code
 
 ``used_api`` is true only when bytes actually went over the wire (or were
 attempted). Callers that fall back to local logic set it to ``False`` via
 :func:`record_local_use`.
+
+Every finished call is mirrored to the durable diagnostics log (see
+:mod:`app.services.diagnostics`) so a failure survives the page reload that
+usually follows it and can be handed to the app team as one redacted report.
 """
 
 from __future__ import annotations
@@ -137,6 +144,15 @@ class LLMCallResult:
     estimated_output_tokens: Optional[int] = None
     status: str = STATUS_OK
     duration_seconds: Optional[float] = None
+    # Provider-side correlation fields. ``request_id`` is the single most
+    # useful value when escalating to a provider's support (Anthropic and
+    # OpenAI both return one per call); ``http_status`` and ``error_type``
+    # come straight from the SDK exception instead of being guessed from
+    # the message text, so "wrong key" (401) and "wrong model" (404) are
+    # distinguishable with certainty.
+    request_id: Optional[str] = None
+    http_status: Optional[int] = None
+    error_type: Optional[str] = None
 
     def to_log_entry(self) -> dict:
         """Return a dict that is safe to display in the UI / usage log."""
@@ -151,6 +167,9 @@ class LLMCallResult:
             "duration_seconds": self.duration_seconds,
             "estimated_input_tokens": self.estimated_input_tokens,
             "estimated_output_tokens": self.estimated_output_tokens,
+            "request_id": self.request_id,
+            "http_status": self.http_status,
+            "error_type": self.error_type,
             # response_text / response_json are intentionally NOT stored
             # in the usage log — that log is metadata-only.
         }
@@ -177,6 +196,21 @@ def _record_to_session(entry: dict) -> None:
         return
 
 
+def _record_to_disk(entry: dict) -> None:
+    """Mirror ``entry`` into the durable diagnostics log.
+
+    Imported lazily — same pattern as the streamlit import above — so this
+    module keeps working in a bare unit-test process where the diagnostics
+    service (and its config-dependent log directory) may not be importable.
+    """
+    try:
+        from app.services import diagnostics
+
+        diagnostics.record_event(entry)
+    except Exception:  # pragma: no cover - diagnostics must never break a call
+        return
+
+
 def record_local_use(task_name: str, *, note: str = "") -> None:
     """Record a non-API stage so the UI can show LOCAL PLACEHOLDER clearly.
 
@@ -194,8 +228,16 @@ def record_local_use(task_name: str, *, note: str = "") -> None:
         "duration_seconds": None,
         "estimated_input_tokens": None,
         "estimated_output_tokens": None,
+        # Keep the record shape identical to to_log_entry() so a report
+        # renders one uniform table instead of a ragged one.
+        "request_id": None,
+        "http_status": None,
+        "error_type": None,
     }
     _record_to_session(entry)
+    # Persisted too: knowing a stage fell back to local logic is part of the
+    # sequence support needs, even though it is not itself an error.
+    _record_to_disk(entry)
     _logger.info(
         "stage=%s used_api=false status=local_placeholder note=%s",
         task_name,
@@ -386,6 +428,56 @@ def _classify_exception(exc: BaseException) -> str:
     return STATUS_EXCEPTION
 
 
+def _error_metadata(exc: BaseException) -> dict:
+    """Pull structured correlation fields off a provider SDK exception.
+
+    Complements :func:`_classify_exception`, which has to guess from
+    substrings. These fields are exact:
+
+    * ``request_id`` — Anthropic (``req_…``) and OpenAI both attach one to
+      both responses and errors. It is what a provider's support team asks
+      for first, and without it a bug report can say "a call failed" but
+      not *which* call.
+    * ``http_status`` — 401 (bad key) vs 403 (no access to that model) vs
+      404 (model does not exist) vs 400 (bad parameter) are four different
+      user-facing problems that all read the same in a message string.
+    * ``error_type`` — the provider's own machine-readable code, e.g.
+      ``authentication_error`` / ``not_found_error`` / ``invalid_request_error``.
+
+    Everything is read defensively: SDKs differ, and a missing attribute
+    must degrade to ``None`` rather than mask the original error.
+    """
+    meta: dict = {"request_id": None, "http_status": None, "error_type": None}
+    try:
+        request_id = getattr(exc, "request_id", None)
+        if request_id:
+            meta["request_id"] = str(request_id)[:64]
+
+        status_code = getattr(exc, "status_code", None)
+        if isinstance(status_code, int):
+            meta["http_status"] = status_code
+        else:
+            # google-api-core exposes the numeric status as `code`.
+            code = getattr(exc, "code", None)
+            if isinstance(code, int):
+                meta["http_status"] = code
+
+        # Anthropic/OpenAI put a machine-readable code inside the parsed
+        # JSON body: {"error": {"type": "...", "message": "..."}}.
+        body = getattr(exc, "body", None)
+        if isinstance(body, dict):
+            err = body.get("error")
+            if isinstance(err, dict) and err.get("type"):
+                meta["error_type"] = str(err["type"])[:64]
+        if not meta["error_type"]:
+            # Fall back to the SDK's own exception class name, which is
+            # still more precise than the free-text message.
+            meta["error_type"] = exc.__class__.__name__[:64]
+    except Exception:  # noqa: BLE001 - never let diagnostics mask the real error
+        pass
+    return meta
+
+
 # Key/org-id patterns now live in app.utils.redaction (single source of
 # truth, imported above as _API_KEY_PATTERNS / _ORG_ID_PATTERNS) so the
 # sink-level logging Filter and this sanitizer can never drift apart.
@@ -548,6 +640,24 @@ def _prepare_vision_image(data: bytes, mime: str) -> tuple[bytes, str]:
 # ---------------------------------------------------------------------------
 
 
+def _response_meta(response: Any) -> dict:
+    """Correlation fields from a *successful* provider response.
+
+    Both the Anthropic and OpenAI SDKs expose the HTTP request id as
+    ``response._request_id`` — public despite the underscore, and the value
+    provider support asks for when investigating a specific call. Captured
+    on success as well as failure so a report can point at the exact call
+    that returned a bad-looking result, not only at calls that raised.
+    """
+    try:
+        request_id = getattr(response, "_request_id", None)
+        if request_id:
+            return {"request_id": str(request_id)[:64]}
+    except Exception:  # noqa: BLE001
+        pass
+    return {}
+
+
 def _anthropic_text(
     *,
     api_key: str,
@@ -556,7 +666,7 @@ def _anthropic_text(
     user_prompt: str,
     expected_json: bool,
     max_tokens: int,
-) -> tuple[str, Optional[int], Optional[int], Optional[str]]:
+) -> tuple[str, Optional[int], Optional[int], Optional[str], dict]:
     import anthropic  # type: ignore
 
     client = anthropic.Anthropic(api_key=api_key, timeout=_request_timeout())
@@ -577,7 +687,7 @@ def _anthropic_text(
     in_tokens = getattr(usage, "input_tokens", None) if usage else None
     out_tokens = getattr(usage, "output_tokens", None) if usage else None
     finish = getattr(response, "stop_reason", None)
-    return out_text, in_tokens, out_tokens, finish
+    return out_text, in_tokens, out_tokens, finish, _response_meta(response)
 
 
 def _openai_text(
@@ -588,7 +698,7 @@ def _openai_text(
     user_prompt: str,
     expected_json: bool,
     max_tokens: int,
-) -> tuple[str, Optional[int], Optional[int], Optional[str]]:
+) -> tuple[str, Optional[int], Optional[int], Optional[str], dict]:
     from openai import OpenAI  # type: ignore
 
     client = OpenAI(api_key=api_key, timeout=_request_timeout())
@@ -608,7 +718,7 @@ def _openai_text(
     in_tokens = getattr(usage, "prompt_tokens", None) if usage else None
     out_tokens = getattr(usage, "completion_tokens", None) if usage else None
     finish = getattr(response.choices[0], "finish_reason", None)
-    return out_text, in_tokens, out_tokens, finish
+    return out_text, in_tokens, out_tokens, finish, _response_meta(response)
 
 
 def _groq_text(
@@ -619,7 +729,7 @@ def _groq_text(
     user_prompt: str,
     expected_json: bool,
     max_tokens: int,
-) -> tuple[str, Optional[int], Optional[int], Optional[str]]:
+) -> tuple[str, Optional[int], Optional[int], Optional[str], dict]:
     """Groq text completion (text-only — Groq has no image input).
 
     Groq exposes an OpenAI-compatible Chat Completions API. We prefer the
@@ -656,7 +766,7 @@ def _groq_text(
     in_tokens = getattr(usage, "prompt_tokens", None) if usage else None
     out_tokens = getattr(usage, "completion_tokens", None) if usage else None
     finish = getattr(response.choices[0], "finish_reason", None)
-    return out_text, in_tokens, out_tokens, finish
+    return out_text, in_tokens, out_tokens, finish, _response_meta(response)
 
 
 def _gemini_response_text(response: Any) -> str:
@@ -770,7 +880,7 @@ def _anthropic_vision(
     images: list[tuple[bytes, str]],
     expected_json: bool,
     max_tokens: int,
-) -> tuple[str, Optional[int], Optional[int], Optional[str]]:
+) -> tuple[str, Optional[int], Optional[int], Optional[str], dict]:
     import anthropic  # type: ignore
 
     client = anthropic.Anthropic(api_key=api_key, timeout=_request_timeout())
@@ -805,7 +915,7 @@ def _anthropic_vision(
     in_tokens = getattr(usage, "input_tokens", None) if usage else None
     out_tokens = getattr(usage, "output_tokens", None) if usage else None
     finish = getattr(response, "stop_reason", None)
-    return out_text, in_tokens, out_tokens, finish
+    return out_text, in_tokens, out_tokens, finish, _response_meta(response)
 
 
 def _openai_vision(
@@ -817,7 +927,7 @@ def _openai_vision(
     images: list[tuple[bytes, str]],
     expected_json: bool,
     max_tokens: int,
-) -> tuple[str, Optional[int], Optional[int], Optional[str]]:
+) -> tuple[str, Optional[int], Optional[int], Optional[str], dict]:
     from openai import OpenAI  # type: ignore
 
     client = OpenAI(api_key=api_key, timeout=_request_timeout())
@@ -848,7 +958,7 @@ def _openai_vision(
     in_tokens = getattr(usage, "prompt_tokens", None) if usage else None
     out_tokens = getattr(usage, "completion_tokens", None) if usage else None
     finish = getattr(response.choices[0], "finish_reason", None)
-    return out_text, in_tokens, out_tokens, finish
+    return out_text, in_tokens, out_tokens, finish, _response_meta(response)
 
 
 def _gemini_vision(
@@ -889,20 +999,29 @@ def _gemini_vision(
 # ---------------------------------------------------------------------------
 
 
-def _normalize_adapter_return(ret: Any) -> tuple[str, Optional[int], Optional[int], Optional[str]]:
-    """Accept a 3- or 4-tuple from a provider adapter.
+def _normalize_adapter_return(
+    ret: Any,
+) -> tuple[str, Optional[int], Optional[int], Optional[str], dict]:
+    """Accept a 3-, 4- or 5-tuple from a provider adapter.
 
-    Real adapters return ``(text, in_tokens, out_tokens, finish_reason)``;
-    older/monkeypatched fakes return the legacy 3-tuple. This keeps both
-    working without forcing every test to add a finish reason.
+    Current adapters return ``(text, in_tokens, out_tokens, finish_reason,
+    meta)`` where ``meta`` carries provider correlation fields such as
+    ``request_id``. Adapters that predate ``meta`` return the 4-tuple, and
+    older/monkeypatched fakes return the legacy 3-tuple; both keep working
+    and simply report no request id. Widening here rather than at each call
+    site is what lets a fake stay a three-liner.
     """
+    if isinstance(ret, tuple) and len(ret) == 5:
+        text, in_toks, out_toks, finish, meta = ret
+        return text, in_toks, out_toks, finish, meta if isinstance(meta, dict) else {}
     if isinstance(ret, tuple) and len(ret) == 4:
-        return ret
+        text, in_toks, out_toks, finish = ret
+        return text, in_toks, out_toks, finish, {}
     if isinstance(ret, tuple) and len(ret) == 3:
         text, in_toks, out_toks = ret
-        return text, in_toks, out_toks, None
+        return text, in_toks, out_toks, None, {}
     # Unexpected shape — treat as an empty text response.
-    return str(ret or ""), None, None, None
+    return str(ret or ""), None, None, None, {}
 
 
 def _invoke_with_retry(adapter, *, api_key: str, **kwargs):
@@ -1012,7 +1131,7 @@ def call_text_llm(
             "groq": _groq_text,
             "gemini": _gemini_text,
         }[provider]
-        text, in_toks, out_toks, finish_reason = _invoke_with_retry(
+        text, in_toks, out_toks, finish_reason, call_meta = _invoke_with_retry(
             _text_adapter,
             api_key=api_key,
             model=model,
@@ -1035,6 +1154,7 @@ def call_text_llm(
         return result
     except Exception as exc:  # noqa: BLE001
         status = _classify_exception(exc)
+        err_meta = _error_metadata(exc)
         result = LLMCallResult(
             success=False,
             task_name=task_name,
@@ -1045,6 +1165,9 @@ def call_text_llm(
                 f"{exc.__class__.__name__}: {exc}", secrets=(api_key,)
             )[:200],
             status=status,
+            request_id=err_meta.get("request_id"),
+            http_status=err_meta.get("http_status"),
+            error_type=err_meta.get("error_type"),
         )
         _finalize(result, started)
         return result
@@ -1074,6 +1197,7 @@ def call_text_llm(
                 estimated_output_tokens=out_toks if out_toks is not None
                 else _estimate_tokens(text),
                 status=STATUS_TRUNCATED if truncated else STATUS_PARSE_ERROR,
+                request_id=call_meta.get("request_id"),
             )
             _finalize(result, started)
             return result
@@ -1091,6 +1215,7 @@ def call_text_llm(
         estimated_output_tokens=out_toks if out_toks is not None
         else _estimate_tokens(text),
         status=STATUS_OK,
+        request_id=call_meta.get("request_id"),
     )
     _finalize(result, started)
     return result
@@ -1195,7 +1320,7 @@ def call_vision_llm(
             "openai": _openai_vision,
             "gemini": _gemini_vision,
         }[provider]
-        text, in_toks, out_toks, finish_reason = _invoke_with_retry(
+        text, in_toks, out_toks, finish_reason, call_meta = _invoke_with_retry(
             _vision_adapter,
             api_key=api_key,
             model=model,
@@ -1219,6 +1344,7 @@ def call_vision_llm(
         return result
     except Exception as exc:  # noqa: BLE001
         status = _classify_exception(exc)
+        err_meta = _error_metadata(exc)
         result = LLMCallResult(
             success=False,
             task_name=task_name,
@@ -1229,6 +1355,9 @@ def call_vision_llm(
                 f"{exc.__class__.__name__}: {exc}", secrets=(api_key,)
             )[:200],
             status=status,
+            request_id=err_meta.get("request_id"),
+            http_status=err_meta.get("http_status"),
+            error_type=err_meta.get("error_type"),
         )
         _finalize(result, started)
         return result
@@ -1256,6 +1385,7 @@ def call_vision_llm(
                 estimated_output_tokens=out_toks if out_toks is not None
                 else _estimate_tokens(text),
                 status=STATUS_TRUNCATED if truncated else STATUS_PARSE_ERROR,
+                request_id=call_meta.get("request_id"),
             )
             _finalize(result, started)
             return result
@@ -1273,6 +1403,7 @@ def call_vision_llm(
         estimated_output_tokens=out_toks if out_toks is not None
         else _estimate_tokens(text),
         status=STATUS_OK,
+        request_id=call_meta.get("request_id"),
     )
     _finalize(result, started)
     return result
@@ -1291,8 +1422,15 @@ def _finalize(result: LLMCallResult, started_monotonic: float) -> None:
         result.error_message = sanitize_error_message(result.error_message)
     entry = result.to_log_entry()
     _record_to_session(entry)
+    # Durable copy, written the instant the call finishes rather than when
+    # the user clicks "share". A browser reload is the usual first reaction
+    # to a failure, and it wipes session state — so a report assembled only
+    # at share time would routinely arrive empty. Fail-safe by contract:
+    # `record_event` swallows its own errors.
+    _record_to_disk(entry)
     _logger.info(
-        "stage=%s used_api=%s status=%s provider=%s model=%s in_tokens=%s out_tokens=%s",
+        "stage=%s used_api=%s status=%s provider=%s model=%s in_tokens=%s "
+        "out_tokens=%s http_status=%s error_type=%s request_id=%s",
         result.task_name,
         str(result.used_api).lower(),
         result.status,
@@ -1300,6 +1438,9 @@ def _finalize(result: LLMCallResult, started_monotonic: float) -> None:
         result.model or "-",
         result.estimated_input_tokens if result.estimated_input_tokens is not None else "-",
         result.estimated_output_tokens if result.estimated_output_tokens is not None else "-",
+        result.http_status if result.http_status is not None else "-",
+        result.error_type or "-",
+        result.request_id or "-",
     )
 
 

app/ui/diagnostics_panel.py

@@ -0,0 +1,229 @@
+"""Always-visible error surface + one-click diagnostics hand-off.
+
+Companion to :mod:`app.ui.api_usage_panel`, and deliberately governed by a
+different rule:
+
+* **API Usage Status** is a developer view of *everything* that happened —
+  token counts, per-stage API-vs-local badges, the raw call table. It stays
+  behind ``SHOW_DEBUG_PANEL`` because it is noise for a normal user.
+* **This panel** shows only *failures*, and it is **never** debug-gated. A
+  user who hits an error is exactly the user who cannot turn on a debug
+  flag, and hiding the explanation from them is what turns "I picked a
+  retired model" into a support ticket that reads "the app is broken".
+
+It renders nothing at all when there are no failures, so a healthy run is
+visually unchanged.
+
+Privacy
+-------
+Everything shown and shared comes from the metadata-only records in
+:mod:`app.services.diagnostics` — no API key, dossier text, screenshot,
+job description, or generated proposal. The full payload is shown to the
+user *before* any upload, because a share button people cannot inspect is
+a share button people do not press.
+"""
+
+from __future__ import annotations
+
+from typing import Optional
+
+import streamlit as st
+
+from app.services import diagnostics
+
+
+# Plain-English meaning per status code, with the actionable next step. The
+# wording deliberately separates "no model is set" (a configuration gap)
+# from "the model you set was rejected" (a wrong/retired name) — collapsing
+# those two into one message is what previously sent users to re-check a
+# dropdown that already had a value in it.
+_STATUS_HELP: dict[str, tuple[str, str]] = {
+    "invalid_key": (
+        "API key rejected",
+        "Re-copy the whole key and confirm it belongs to the provider "
+        "selected in Setup (an OpenAI key will not work with Anthropic).",
+    ),
+    "model_missing": (
+        "Model not accepted by the provider",
+        "The model name was not recognised — it may be retired, misspelled, "
+        "or unavailable on your plan. Pick a different model in Setup.",
+    ),
+    "quota": (
+        "Out of credit or rate limited",
+        "Add credit in your provider dashboard, or wait a moment and retry.",
+    ),
+    "context_overflow": (
+        "Too much input for this model",
+        "Shorten the dossier or screenshot text and try again.",
+    ),
+    "connection": (
+        "Could not reach the provider",
+        "Check your internet connection, VPN, or firewall, then retry.",
+    ),
+    "parse_error": (
+        "Unreadable reply from the model",
+        "The model did not return valid JSON. Retrying usually fixes it; if "
+        "it repeats, try a different model.",
+    ),
+    "truncated": (
+        "Reply cut off by the token limit",
+        "Raise the output-token limit in Setup and retry.",
+    ),
+    "no_api": (
+        "No API key configured",
+        "Add your key in Setup and save the configuration.",
+    ),
+    "unsupported_provider": (
+        "Provider not configured",
+        "Choose a supported provider in Setup.",
+    ),
+    "exception": (
+        "Unexpected provider error",
+        "Share the diagnostics below with the app team.",
+    ),
+}
+
+# HTTP status refines the story where the SDK gave us one. These are exact
+# signals, unlike the message-text heuristics used to bucket the status.
+_HTTP_HELP: dict[int, str] = {
+    400: "The provider rejected the request itself (bad parameter) — this is "
+         "an app-side issue, not something you configured wrong.",
+    401: "The key was not accepted at all.",
+    403: "The key is valid but is not allowed to use this model.",
+    404: "The model name does not exist for this provider.",
+    429: "Rate limit or credit limit hit.",
+}
+
+
+def _describe(event: dict) -> tuple[str, str]:
+    """Return ``(headline, next_step)`` for one failed call."""
+    status = str(event.get("status") or "exception")
+    headline, action = _STATUS_HELP.get(
+        status, ("Something went wrong", "Share the diagnostics with the app team.")
+    )
+    http_status = event.get("http_status")
+    if isinstance(http_status, int) and http_status in _HTTP_HELP:
+        action = f"{_HTTP_HELP[http_status]} {action}"
+    return headline, action
+
+
+def _stage_label(event: dict) -> str:
+    task = str(event.get("task_name") or "unknown")
+    return task.replace("_", " ").strip().capitalize()
+
+
+def render(*, key_suffix: str = "", limit: int = 50) -> None:
+    """Render the diagnostics panel, or nothing when there is no failure.
+
+    Pass a unique ``key_suffix`` when calling from more than one screen so
+    Streamlit widget keys stay unique.
+    """
+    try:
+        session_events = list(st.session_state.get("api_usage_log") or [])
+    except Exception:  # noqa: BLE001 - outside a script run
+        session_events = []
+
+    # Disk first: it survives the page reload that usually follows a failure
+    # and spans earlier sessions. Session state is the fallback for the case
+    # where the disk write was blocked (read-only volume, locked file).
+    events = diagnostics.load_events(limit=limit) or session_events
+    # Only failures since the last successful call: a success is proof the
+    # problem is over, and a warning that never clears is one users learn
+    # to ignore.
+    failures = diagnostics.unresolved_errors(events)
+    if not failures:
+        return
+
+    latest = failures[-1]
+    headline, action = _describe(latest)
+
+    with st.expander(f"⚠️ Problem detected — {headline}", expanded=False):
+        st.markdown(f"**{headline}**")
+        st.caption(action)
+
+        st.markdown(
+            f"- **Stage:** {_stage_label(latest)}\n"
+            f"- **Provider / model:** `{latest.get('provider') or '—'}`"
+            f" · `{latest.get('model') or '—'}`\n"
+            f"- **When:** {latest.get('timestamp') or '—'}"
+        )
+
+        # Correlation ids: meaningless to the user, decisive for support.
+        req_id = latest.get("request_id")
+        http_status = latest.get("http_status")
+        err_type = latest.get("error_type")
+        if req_id or http_status or err_type:
+            bits = []
+            if http_status:
+                bits.append(f"HTTP `{http_status}`")
+            if err_type:
+                bits.append(f"type `{err_type}`")
+            if req_id:
+                bits.append(f"request `{req_id}`")
+            st.caption("Reference: " + " · ".join(bits))
+
+        if len(failures) > 1:
+            st.caption(f"{len(failures)} failed calls since the last success.")
+
+        st.divider()
+        _render_share(session_events, key_suffix=key_suffix)
+
+
+def _render_share(session_events: list[dict], *, key_suffix: str = "") -> None:
+    """Build-and-share controls: inspect, download, and optional upload."""
+    st.markdown("**Share with the app team**")
+    st.caption(
+        "Metadata only — no API key, dossier text, screenshots, job posts, "
+        "or generated proposals."
+    )
+
+    # One report id per session so the download, the upload, and whatever the
+    # user quotes in an email all refer to the same thing.
+    id_key = f"diag_report_id_{key_suffix}"
+    if id_key not in st.session_state:
+        st.session_state[id_key] = diagnostics.new_report_id()
+    report_id: str = st.session_state[id_key]
+
+    report = diagnostics.build_report(
+        report_id=report_id, session_events=session_events
+    )
+    payload = diagnostics.report_to_json(report)
+
+    st.download_button(
+        "Download diagnostics",
+        data=payload,
+        file_name=f"{report_id}.json",
+        mime="application/json",
+        use_container_width=True,
+        key=f"diag_dl_{key_suffix}",
+        help="Save the report and attach it to an email or chat message.",
+    )
+
+    # st.code ships a native copy button, so there is no need for custom JS
+    # (which the app's CSP-tight embed would complicate anyway).
+    with st.expander("View / copy report", expanded=False):
+        st.code(payload, language="json")
+
+    if diagnostics.is_upload_configured():
+        sent_key = f"diag_sent_{key_suffix}"
+        if st.button(
+            "Send to support",
+            use_container_width=True,
+            key=f"diag_send_{key_suffix}",
+            help="Uploads the report above so the team can look it up by id.",
+        ):
+            st.session_state[sent_key] = diagnostics.upload_report(report)
+
+        sent: Optional[bool] = st.session_state.get(sent_key)
+        if sent is True:
+            st.success(f"Sent. Quote reference **{report_id}** to the team.")
+        elif sent is False:
+            st.warning(
+                "Upload failed. Use **Download diagnostics** above and send "
+                "the file instead."
+            )
+    else:
+        st.caption(f"Reference id: **{report_id}**")
+
+
+__all__ = ["render"]