Skip to content

Commit 5923214

Browse files
committed
review: address 3 Brad findings on PR #35
* `.github/workflows/tests.yml` — block comment above the typecheck job still described `continue-on-error: true` as the transitional policy, contradicting the actual code which removed it in this same PR. Updated the comment to reflect the enforced gate. * `api/logs.py` — out of issue #29's literal scope (issue lists only the three files I converted in the previous commit), but inconsistent with the "clean demo" spirit. Two `print()` error logs at lines 73 and 140 swapped for `_logger.exception(...)` matching the pattern in api/workspaces.py / api/search.py / api/composers.py. * `api/search.py:56` — `combined: list = []` is the minimum annotation that satisfies mypy, but `list[str]` is the actual shape: both content_parts and metadata_parts are typed `list[str]` upstream, the filter `(p for p in ... if p)` only adds truthy strings, and the consumer is `"\n\n".join(combined)` which requires strings. Tightened. Verified locally: - mypy --ignore-missing-imports --no-strict-optional . → Success - ruff check api/ utils/ scripts/export.py app.py → All checks passed - unittest discover tests → 178 / 178 OK
1 parent 7bbfe43 commit 5923214

3 files changed

Lines changed: 10 additions & 8 deletions

File tree

.github/workflows/tests.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,9 @@ jobs:
5555
# Codebase already has type hints across most of the surface (~70+ typed
5656
# functions). Mypy runs in lenient mode (--ignore-missing-imports for
5757
# untyped third-party deps; no strict-optional) so the gate isn't a wall
58-
# of false positives on first run. continue-on-error keeps findings as
59-
# warnings during the surface-cleanup phase; flip to required by removing
60-
# continue-on-error once the surface is clean.
58+
# of false positives. The transitional `continue-on-error: true` was
59+
# removed in #29 once mypy reached zero errors on this repo — type
60+
# failures now block merges.
6161
typecheck:
6262
name: Typecheck (mypy)
6363
runs-on: ubuntu-latest

api/logs.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
"""
55

66
import json
7+
import logging
78
import os
89
import re
910
import sqlite3
@@ -16,6 +17,7 @@
1617
from utils.path_helpers import to_epoch_ms
1718

1819
bp = Blueprint("logs", __name__)
20+
_logger = logging.getLogger(__name__)
1921

2022

2123
def _extract_chat_id_from_bubble_key(key: str) -> str | None:
@@ -69,8 +71,8 @@ def get_logs():
6971
"type": "chat",
7072
"messageCount": len(bubbles),
7173
})
72-
except Exception as e:
73-
print(f"Error reading global storage: {e}")
74+
except Exception:
75+
_logger.exception("Error reading global storage")
7476

7577
# Per-workspace (legacy)
7678
try:
@@ -136,6 +138,6 @@ def get_logs():
136138
logs.sort(key=lambda log: log.get("timestamp") or 0, reverse=True)
137139
return jsonify({"logs": logs})
138140

139-
except Exception as e:
140-
print(f"Failed to get logs: {e}")
141+
except Exception:
142+
_logger.exception("Failed to get logs")
141143
return jsonify({"error": "Failed to get logs", "logs": []}), 500

api/search.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ def _build_exclusion_searchable(
5353
metadata_parts: list[str] | None = None,
5454
) -> str:
5555
"""Build broad searchable text so exclusion rules cover visible output."""
56-
combined: list = []
56+
combined: list[str] = []
5757
if content_parts:
5858
combined.extend(p for p in content_parts if p)
5959
if metadata_parts:

0 commit comments

Comments
 (0)