You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
* chore: clean linter warnings + remove mypy continue-on-error (closes#29)
Polish-only pass against the acceptance criteria of issue #29. No
production logic changed; behaviour is identical end-to-end (178/178
unittests pass, every endpoint smokes 200 with the same response sizes
as master).
What landed:
* mypy now exits zero on the repo. Three small annotation fixes
(scripts/export.py:54 `existing: dict = {}`, api/search.py:54
`combined: list = []`, utils/exclusion_rules.py:76 `tokens:
list[str | tuple[str, str]] = []`), one inner-loop variable rename
in scripts/export.py to dodge a `Assignment to variable "e" outside
except: block` clash with the surrounding `except … as e:`, and an
empty `scripts/__init__.py` so mypy stops finding the script under
two module names.
* `continue-on-error: true` removed from the mypy step in
`.github/workflows/tests.yml`. Type errors now fail the job.
Comment updated to explain why.
* Ruff + flake8 clean: 11 auto-fixed (F401 unused imports, F541 empty
f-strings, F841 unused local `db_path` in api/workspaces.py), 5
manual renames of ambiguous `l` → `ln` / `log` / `layout / entry`,
one F811 fix in app.py (duplicate `import sys`), one E402 import
moved to the top of api/workspaces.py, and `# noqa: E402` markers on
the scripts/export.py imports that legitimately come after
`sys.path.insert(0, …)`.
* Bare `print()` error logging in api/composers.py (2), api/search.py
(3) and api/workspaces.py (7) replaced with
`_logger = logging.getLogger(__name__)` calls. Endpoint-level
catches use `_logger.exception(...)` so the traceback survives; the
two per-row catches (`CLI: could not read session …` and
`Error parsing composer data for …`) use
`_logger.warning(..., exc_info=True)` because they're per-iteration,
not endpoint-level. Matches the convention already used in
scripts/export.py:42 and utils/exclusion_rules.py:191.
Verified locally:
- python3 -m mypy --ignore-missing-imports --no-strict-optional .
→ Success: no issues found in 35 source files
- ruff check api/ utils/ scripts/export.py app.py → All checks passed
- flake8 --select=F,E9,W6 api/ utils/ scripts/export.py app.py → clean
- python3 -m unittest discover tests → Ran 178 tests, OK
- python3 app.py boots clean; GET / + /api/workspaces +
/api/composers + /api/search?q=foo all return 200
* review: silence BLE001 on the two CodeRabbit-cited best-effort catches
api/workspaces.py:876 (per-CLI-session traverse_blobs catch) and
api/workspaces.py:1398 (per-composer-row tabs parse catch) are
intentionally broad — a single malformed session or composer must not
500 the whole endpoint, and `exc_info=True` on the .warning() call
already captures the concrete exception type for debugging.
Added `# noqa: BLE001` with a one-line rationale on each line so the
intent survives future readers. CodeRabbit's review on PR #35 cited
exactly these two sites; the other BLE001 hits in the file are
pre-existing and out of scope for issue #29 ("polish only, no logic
changes").
Verified: full suite still 178/178; ruff default rule set still clean.
* 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
print(f"CLI: could not read session {session_id}: {e}")
876
+
exceptException: # noqa: BLE001 — best-effort per-session skip; one corrupted session must not 500 the endpoint, and the failure mode is logged with exc_info so the concrete type is preserved.
877
+
_logger.warning("CLI: could not read session %s", session_id, exc_info=True)
print(f"Error parsing composer data for {composer_id}: {e}")
1398
+
exceptException: # noqa: BLE001 — best-effort per-composer skip in a read-many loop; one malformed row must not 500 the tabs endpoint, and exc_info captures the concrete type for debugging.
1399
+
_logger.warning("Error parsing composer data for %s", composer_id, exc_info=True)
1399
1400
1400
1401
# Sort tabs by timestamp descending (newest first)
0 commit comments