Skip to content

fix: resolve all ruff lint errors — zero warnings#5

Merged
patrick-chinchill merged 1 commit into
mainfrom
fix-lint
Apr 6, 2026
Merged

fix: resolve all ruff lint errors — zero warnings#5
patrick-chinchill merged 1 commit into
mainfrom
fix-lint

Conversation

@patrick-chinchill
Copy link
Copy Markdown
Collaborator

Library users expect zero lint warnings. Fixed all 111 ruff errors.

Fixes: E501 (line length), UP007 (Union→|), UP017 (datetime.UTC),
I001 (import sorting), SIM102/SIM103/SIM108/SIM113/SIM117 (simplifications),
F401 (unused imports), E402 (import order)
@patrick-chinchill patrick-chinchill merged commit 9865090 into main Apr 6, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request modernizes the codebase by updating datetime references to use UTC, adopting the | operator for type unions, and refactoring conditional logic into ternary expressions. It also includes significant formatting updates, import cleanups, and an optimization of Postgres transaction handling. I have no feedback to provide.

patrick-chinchill pushed a commit that referenced this pull request May 9, 2026
Hardens the socket-mode port against the seven review findings on
PR #86:

* Add ``connect_timeout_s`` config (default 30s) and wrap the initial
  socket-mode handshake in ``asyncio.wait_for`` so a hung
  ``SocketModeClient.connect()`` can't make ``initialize()`` block
  forever (hazard #11). On timeout we tear the loop down before raising.
* Reject forwarded socket events whose ``timestamp`` field is outside
  the same 5-minute window ``_verify_signature`` enforces, so a captured
  forwarded payload can't be replayed indefinitely (hazard #12).
* Narrow the ``contextlib.suppress`` in ``stop_socket_mode`` to
  ``CancelledError`` only — surprising loop crashes are no longer
  silently swallowed during shutdown.
* Replace ``asyncio.get_event_loop().create_task`` in the socket-mode
  ``wrap_async`` helper with ``get_running_loop`` (Python 3.12+
  compatibility, hazard #5).
* Have the socket-mode interactive branch ack with
  ``response_action: errors`` instead of an empty ack when dispatch
  raises — an empty ack on ``view_submission`` silently closes the
  modal so the user sees no signal anything went wrong.
* Drop ``is_ext_shared_channel`` from the synthesized ``event_callback``
  payload in the socket-mode events_api branch so socket and webhook
  paths feed identical shapes into ``_process_event_payload`` (hazard
  #7).
* Add a regression test firing two concurrent ``_route_socket_event``
  events_api dispatches for different teams via ``asyncio.gather`` to
  pin down the existing ``copy_context()`` isolation against future
  drift (hazard #6).

Also fixes a pre-existing B010 lint warning in the test stub.

https://claude.ai/code/session_01FyMxQn2BEAzmwKS1GZczKj
patrick-chinchill pushed a commit that referenced this pull request May 22, 2026
Independent review surfaced three actionable items:

1. **Math/escape ordering** (review #1) -- `_strip_math_regions` ran
   BEFORE `_strip_escape_sequences`, so input like
   `\$opener *unclosed text closer\$` had its two escaped `$`s paired
   as math, eating the `*` opener inside. Reordered: escape strip
   first, then math strip. Verified the original bug repro now
   correctly produces a closing `*`.

2. **Empty task items** (review #5) -- `r"^\[([ xX])\]\s+(.*)"`
   required at least one whitespace AFTER `]`, so `- [ ]` with no
   trailing content silently fell through to plain text. Loosened to
   `r"^\[([ xX])\](?:\s+(.*))?$"` -- trailing whitespace+content is
   now optional. `- [ ]` and `- [x]` (no trailing) both produce
   `listItem(checked=False/True, children=[])`.

3. **Strikethrough test strengthened** (review #7) -- the existing
   `test_escaped_tilde_does_not_form_strikethrough` only asserted the
   absence of a `delete` node. A buggy impl that dropped the tildes
   entirely would have passed. Added a content-shape assertion that
   `~~not strike~~` appears in the text leaves.

Reviewer's #3 (the `r"a *b\* * c"` case) was investigated and
determined NOT to be a bug -- the trailing `*` between two spaces is
not a valid CommonMark closer (whitespace-flanked), so italic
correctly stays open. Pre-fix behavior happened to balance by ignoring
flanking; post-fix is CommonMark-correct. Pre-existing edge cases #4
(`\\*text*`) and #6 (link-text unescape implicit) are documented in
the existing code; not blockers.

Tests added:
- `test_empty_unchecked_task_item_no_content` and `_checked_` variant
- `test_remend_escaped_dollar_does_not_pair_with_unescaped_dollar`
  (the exact #1 repro)
- `test_remend_escaped_dollar_does_not_create_phantom_math_region`
- Strengthened `test_escaped_tilde_does_not_form_strikethrough`

3,625 pass / 1 pre-existing failure unrelated.
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.

1 participant