fix(api): address PR review — json_error, typed session rows, test limits

Author: clean6378-max-it

.github/workflows/ci.yml

@@ -74,5 +74,8 @@ jobs:
       - name: Install dev dependencies
         run: pip install -r requirements-dev.txt
 
-      - name: Run mypy (strict)
+      - name: Run mypy (strict, production packages)
         run: mypy
+
+      - name: Run mypy on tests (non-strict smoke)
+        run: mypy tests --follow-imports skip

api/_flask_types.py

@@ -4,9 +4,17 @@
 
 from flask import Response, jsonify
 
+# Narrow scope: handlers here return Response or (Response, status) only.
+# Widen if adding (Response, int, headers) or plain-text tuples.
 FlaskReturn = Union[Response, tuple[Response, int]]
 
 
-def json_ok(*args: Any, **kwargs: Any) -> Response:
-    """Typed wrapper around :func:`flask.jsonify`."""
+def json_response(*args: Any, **kwargs: Any) -> Response:
+    """Typed wrapper around :func:`flask.jsonify` for JSON bodies."""
     return jsonify(*args, **kwargs)
+
+
+def json_error(payload: str | dict[str, Any], status: int) -> tuple[Response, int]:
+    """JSON error body with explicit HTTP status (avoids trailing `, 404` at call sites)."""
+    body: dict[str, Any] = {"error": payload} if isinstance(payload, str) else payload
+    return jsonify(body), status

api/export_api.py

@@ -9,7 +9,7 @@
 
 from flask import Blueprint, current_app, request, send_file
 
-from api._flask_types import FlaskReturn, json_ok
+from api._flask_types import FlaskReturn, json_error, json_response
 from models.export import ExportStateDict
 
 from utils.export_state_store import (
@@ -68,7 +68,7 @@ def _write_state(sessions_map: dict[str, float], count: int) -> None:
 def get_export_state() -> FlaskReturn:
     state = _read_state()
     n = state.get("exportedCount", 0)
-    return json_ok(
+    return json_response(
         {
             "last_export_time": state.get("lastExportTime"),
             # Sessions exported in the last completed bulk export (not a lifetime total).
@@ -84,11 +84,11 @@ def bulk_export() -> FlaskReturn:
     if body is None:
         body = {}
     if not isinstance(body, dict):
-        return json_ok({"error": "Invalid request body"}), 400
+        return json_error("Invalid request body", 400)
 
     since = body.get("since", "all")
     if since not in ("all", "last", "incremental"):
-        return json_ok({"error": "Invalid since mode", "since": since}), 400
+        return json_error({"error": "Invalid since mode", "since": since}, 400)
 
     base = (
         current_app.config.get("CLAUDE_PROJECTS_DIR")
@@ -230,15 +230,7 @@ def bulk_export() -> FlaskReturn:
         _write_state(new_sessions_map, count)
 
     if count == 0:
-        return (
-            json_ok(
-                {
-                    "error": "Nothing to export",
-                    "since": since,
-                }
-            ),
-            422,
-        )
+        return json_error({"error": "Nothing to export", "since": since}, 422)
 
     buf.seek(0)
     date_tag = datetime.now().strftime("%Y-%m-%d")
@@ -271,17 +263,17 @@ def export_session(project_name: str, session_id: str) -> FlaskReturn:
     try:
         filepath = safe_join(base, project_name, f"{session_id}.jsonl")
     except ValueError:
-        return json_ok({"error": "Invalid path"}), 400
+        return json_error("Invalid path", 400)
 
     if not os.path.isfile(filepath):
-        return json_ok({"error": "Session not found"}), 404
+        return json_error("Session not found", 404)
 
     fmt = request.args.get("format", "md")
     try:
         session = parse_session(filepath)
         rules = current_app.config.get("EXCLUSION_RULES") or []
         if is_session_excluded(rules, session, project_name):
-            return json_ok({"error": "Session not found"}), 404
+            return json_error("Session not found", 404)
         stats = compute_stats(session)
         title_slug = slugify(session["title"], default="session")
 
@@ -309,4 +301,4 @@ def export_session(project_name: str, session_id: str) -> FlaskReturn:
         current_app.logger.exception(
             "Failed to export session %s/%s", project_name, session_id
         )
-        return json_ok({"error": "Internal server error exporting session"}), 500
+        return json_error("Internal server error exporting session", 500)

api/projects.py

@@ -1,17 +1,44 @@
 """Project listing endpoints."""
 
-from typing import cast
+from flask import Blueprint, current_app
 
-from flask import Blueprint, current_app, jsonify
-
-from api._flask_types import FlaskReturn, json_ok
-from models.project import ProjectSessionRowDict
+from api._flask_types import FlaskReturn, json_error, json_response
+from models.project import ProjectSessionRowDict, SessionListItemDict
+from models.session import SessionDict
 from utils.session_path import get_claude_projects_dir, list_projects, list_sessions, safe_join
 from utils.exclusion_rules import is_session_excluded
 
 projects_bp = Blueprint("projects", __name__)
 
 
+def _session_row_ok(s: SessionListItemDict, parsed: SessionDict) -> ProjectSessionRowDict:
+    meta = parsed["metadata"]
+    models = meta.get("models_used", [])
+    return {
+        "id": s["id"],
+        "path": s["path"],
+        "size_bytes": s["size_bytes"],
+        "modified": s["modified"],
+        "title": parsed["title"],
+        "models": sorted(models) if isinstance(models, set) else list(models),
+        "tokens": meta["total_input_tokens"] + meta["total_output_tokens"],
+        "tool_calls": meta["total_tool_calls"],
+        "first_timestamp": meta["first_timestamp"],
+        "last_timestamp": meta["last_timestamp"],
+    }
+
+
+def _session_row_error(s: SessionListItemDict) -> ProjectSessionRowDict:
+    return {
+        "id": s["id"],
+        "path": s["path"],
+        "size_bytes": s["size_bytes"],
+        "modified": s["modified"],
+        "title": "Error parsing session",
+        "error": True,
+    }
+
+
 @projects_bp.route("/api/projects")
 def get_projects() -> FlaskReturn:
     base = current_app.config.get("CLAUDE_PROJECTS_DIR") or get_claude_projects_dir()
@@ -40,7 +67,7 @@ def get_projects() -> FlaskReturn:
         if latest_ts:
             project["last_modified"] = latest_ts
 
-    return json_ok(projects)
+    return json_response(projects)
 
 
 @projects_bp.route("/api/projects/<path:project_name>/sessions")
@@ -49,7 +76,7 @@ def get_project_sessions(project_name: str) -> FlaskReturn:
     try:
         project_dir = safe_join(base, project_name)
     except ValueError:
-        return json_ok([]), 400
+        return json_response([]), 400
     sessions = list_sessions(project_dir)
     # Add summary preview for each session
     from utils.jsonl_parser import parse_session
@@ -58,31 +85,17 @@ def get_project_sessions(project_name: str) -> FlaskReturn:
     for s in sessions:
         try:
             parsed = parse_session(s["path"])
-            meta = parsed["metadata"]
             # Skip untitled sessions (no real conversation)
             if parsed["title"] == "Untitled Session":
                 continue
             if is_session_excluded(rules, parsed, project_name):
                 continue
-            models = meta.get("models_used", [])
-            result.append(cast(ProjectSessionRowDict, {
-                **s,
-                "title": parsed["title"],
-                "models": sorted(models) if isinstance(models, set) else list(models),
-                "tokens": meta["total_input_tokens"] + meta["total_output_tokens"],
-                "tool_calls": meta["total_tool_calls"],
-                "first_timestamp": meta["first_timestamp"],
-                "last_timestamp": meta["last_timestamp"],
-            }))
+            result.append(_session_row_ok(s, parsed))
         except Exception:
             # Full detail (class, message, traceback) to the server log via
             # logger.exception. The per-session card carries only `error: True`
             # — the class-name+message string was a leak (issue #25). The
             # operator looks at the server log for triage.
             current_app.logger.exception("Failed to parse session %s", s["id"])
-            result.append(cast(ProjectSessionRowDict, {
-                **s,
-                "title": "Error parsing session",
-                "error": True,
-            }))
-    return json_ok(result)
+            result.append(_session_row_error(s))
+    return json_response(result)

api/search.py

@@ -4,7 +4,7 @@
 
 from flask import Blueprint, current_app, jsonify, request
 
-from api._flask_types import FlaskReturn, json_ok
+from api._flask_types import FlaskReturn, json_error, json_response
 from models.search import SearchHitDict
 from utils.session_path import get_claude_projects_dir, list_projects, list_sessions
 from utils.jsonl_parser import parse_session
@@ -32,12 +32,12 @@ def _parse_limit(raw: str | None, default: int = _DEFAULT_LIMIT) -> int:
 def search() -> FlaskReturn:
     query = request.args.get("q", "").strip().lower()
     if not query:
-        return json_ok([])
+        return json_response([])
 
     try:
         max_results = _parse_limit(request.args.get("limit"))
     except ValueError as e:
-        return json_ok({"error": str(e)}), 400
+        return json_error(str(e), 400)
     base = current_app.config.get("CLAUDE_PROJECTS_DIR") or get_claude_projects_dir()
     projects = list_projects(base)
 
@@ -76,4 +76,4 @@ def search() -> FlaskReturn:
                     if len(results) >= max_results:
                         break
 
-    return json_ok(results)
+    return json_response(results)

api/sessions.py

@@ -2,9 +2,9 @@
 
 import os
 
-from flask import Blueprint, current_app, jsonify
+from flask import Blueprint, current_app
 
-from api._flask_types import FlaskReturn, json_ok
+from api._flask_types import FlaskReturn, json_error, json_response
 
 from utils.session_path import get_claude_projects_dir, safe_join
 from utils.jsonl_parser import parse_session
@@ -20,25 +20,25 @@ def get_session(project_name: str, session_id: str) -> FlaskReturn:
     try:
         filepath = safe_join(base, project_name, f"{session_id}.jsonl")
     except ValueError:
-        return json_ok({"error": "Invalid path"}), 400
+        return json_error("Invalid path", 400)
 
     if not os.path.isfile(filepath):
-        return json_ok({"error": f"Session {session_id} not found"}), 404
+        return json_error(f"Session {session_id} not found", 404)
 
     try:
         session = parse_session(filepath)
         rules = current_app.config.get("EXCLUSION_RULES") or []
         if is_session_excluded(rules, session, project_name):
-            return json_ok({"error": "Session not found"}), 404
-        return json_ok(session)
+            return json_error("Session not found", 404)
+        return json_response(session)
     except Exception:
         # Full traceback (class name, message, stack) goes to the server log
         # via logger.exception. The HTTP body returns a stable, generic
         # message — never the class name or `e` itself, which would leak
         # internal field names, file paths, and user values to any client
         # (issue #25).
         current_app.logger.exception("Failed to parse session %s", session_id)
-        return json_ok({"error": "Failed to parse session"}), 500
+        return json_error("Failed to parse session", 500)
 
 
 @sessions_bp.route("/api/sessions/<path:project_name>/<session_id>/stats")
@@ -47,20 +47,20 @@ def get_session_stats(project_name: str, session_id: str) -> FlaskReturn:
     try:
         filepath = safe_join(base, project_name, f"{session_id}.jsonl")
     except ValueError:
-        return json_ok({"error": "Invalid path"}), 400
+        return json_error("Invalid path", 400)
 
     if not os.path.isfile(filepath):
-        return json_ok({"error": f"Session {session_id} not found"}), 404
+        return json_error(f"Session {session_id} not found", 404)
 
     try:
         session = parse_session(filepath)
         rules = current_app.config.get("EXCLUSION_RULES") or []
         if is_session_excluded(rules, session, project_name):
-            return json_ok({"error": "Session not found"}), 404
+            return json_error("Session not found", 404)
         stats = compute_stats(session)
-        return json_ok(stats)
+        return json_response(stats)
     except Exception:
         # Same pattern as get_session above — full detail to the server log,
         # generic message in the HTTP body (issue #25).
         current_app.logger.exception("Failed to compute stats for %s", session_id)
-        return json_ok({"error": "Failed to compute session stats"}), 500
+        return json_error("Failed to compute session stats", 500)

pyproject.toml

@@ -2,4 +2,9 @@
 python_version = "3.12"
 strict = true
 packages = ["api", "utils", "models"]
-exclude = ["tests/"]
+
+[[tool.mypy.overrides]]
+files = ["tests"]
+strict = false
+disallow_untyped_defs = false
+disallow_untyped_calls = false

tests/test_search.py

@@ -9,6 +9,7 @@
 import pytest
 
 REPO_ROOT = Path(__file__).resolve().parent.parent
+# TODO: drop sys.path hack when pyproject.toml gains a [project] table + pip install -e .
 sys.path.insert(0, str(REPO_ROOT))
 
 from flask import Flask  # noqa: E402
@@ -63,6 +64,20 @@ def test_limit_non_numeric_returns_400(self, client):
         assert "error" in body
         assert "limit" in body["error"].lower()
 
+    def test_limit_zero_returns_400(self, client):
+        resp = client.get("/api/search?q=test&limit=0")
+        assert resp.status_code == 400
+        body = resp.get_json()
+        assert "error" in body
+        assert "limit" in body["error"].lower()
+
+    def test_limit_negative_returns_400(self, client):
+        resp = client.get("/api/search?q=test&limit=-5")
+        assert resp.status_code == 400
+        body = resp.get_json()
+        assert "error" in body
+        assert "limit" in body["error"].lower()
+
     def test_limit_default_when_omitted(self, client, tmp_path):
         _write_searchable_session(tmp_path, "proj-a", "sess-1", "findme keyword here")
         resp = client.get("/api/search?q=findme")