Skip to content

ruff: replace 16 bare except: with except Exception:, drop E722#53

Open
racmac57 wants to merge 2 commits into
mainfrom
fix/bare-excepts-e722
Open

ruff: replace 16 bare except: with except Exception:, drop E722#53
racmac57 wants to merge 2 commits into
mainfrom
fix/bare-excepts-e722

Conversation

@racmac57
Copy link
Copy Markdown
Owner

Summary

Replaces 16 bare except: clauses with except Exception: and drops E722 from the ruff ignore list. Makes KeyboardInterrupt / SystemExit propagate correctly again — the bare form was silently swallowing Ctrl-C and programmatic exits inside the watcher main loop and around sqlite conn.close() cleanup paths.

Sites touched (all 16 are "recover from unexpected error" handlers; none intended to catch BaseException):

File Sites Context
chromadb_crud.py 1 json.loads fallback
chunker_cleanup.py 4 directory scan / winshell / tree walk cleanup
chunker_db.py 7 conn.close() in error paths
gui_app.py 1 streamlit metric fallback
notification_system.py 1 datetime.strptime fallback
watcher_splitter.py 2 nltk download fallback, stability-config defaults

Applied via a single sed that only matches ^\s*except:\s*$ (fully bare form, never except Foo:), so no collateral damage.

Stacking note

This branch was cut from the tip of ruff/shrink-ignores-auto-fix (PR #52). Targets main so GitHub's "branch contains commits already in target" auto-simplification kicks in once #52 merges. Merge order doesn't matter — if you merge this first, #52 rebases cleanly; if you merge #52 first, this PR simplifies to just the 7 files above.

Test plan

  • ruff check .All checks passed!
  • pytest tests/test_smoke.py → 3 passed
  • python -c "import chromadb_crud, chunker_cleanup, chunker_db, notification_system" → imports clean (catches any syntax regression from the sed)
  • CI test-fast green

Follow-ups

Remaining ignores in ruff.toml: E402 (53× module import not at top), E501 (long lines), E741 (ambiguous names). Each wants its own PR.


Generated by Claude Code

claude added 2 commits April 24, 2026 03:44
Applied `ruff check --fix` and targeted manual cleanup to drop
F401, F541, F811, F841, and W292 from the repo-wide ignore list.
Any new violation of those rules now fails CI on test-fast.

- 134 findings auto-fixed (F541 redundant f-strings, most F401 unused
  imports, F811 redefinitions, W292 missing trailing newlines).
- 11 remaining F401 sites annotated inline with `# noqa: F401` —
  all are intentional availability-probe imports inside
  try/except ImportError blocks (celery_tasks, langchain_rag_handler,
  ollama_integration, test_celery_integration).
- 10 F841 sites converted to `_var` prefix. Three flagged with
  inline TODO comments where the unused assignment hints at a
  real wiring gap (celery_tasks file_key, orchestrator auth_b64,
  watcher_splitter cloud_success — the retry-success signal is
  set but the caller never reads it).

Remaining ignores: E402 (53× intentional lazy imports), E501 (long
lines), E722 (16× bare except — separate cleanup), E741 (ambiguous
names). Each of those wants its own PR.
All 16 E722 sites in the repo were "recover from unexpected error"
handlers — none intended to catch KeyboardInterrupt, SystemExit, or
GeneratorExit. The bare form was swallowing those and making Ctrl-C
unreliable in the watcher main loop and anywhere a sqlite connection
was being closed in a finally-ish block.

Mechanical replacement across:
- chromadb_crud.py       (1 site: json.loads fallback)
- chunker_cleanup.py     (4 sites: cleanup/scan paths)
- chunker_db.py          (7 sites: conn.close() on error)
- gui_app.py             (1 site: streamlit metric fallback)
- notification_system.py (1 site: datetime parse fallback)
- watcher_splitter.py    (2 sites: nltk download fallback,
                          file-stability-config defaults)

E722 removed from ruff.toml ignore list; any new bare-except now
fails CI on test-fast.

Stacked on top of branch ruff/shrink-ignores-auto-fix (PR #52). The
changes don't line-conflict with #52; once #52 merges this should
become a clean delta on main.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants