feat(tooling): bump ruff 0.9.7 -> 0.15.20 (HYBIM-777)#66
Conversation
….26.1 -> 0.29.0 (HYBIM-777) openapi-python-client 0.26.x capped ruff<0.14; bump to 0.29.0 removes the conflict. Fix new violations introduced by ruff 0.13-0.15: - Remove unused timezone import (F401) - Add __hash__ to Message.__eq__ class (PLW1641) - Suppress PLC0415/RUF043/E402 in tests (importorskip pattern) - Suppress new rules in examples per-file-ignores (examples are docs) - Add PLC0415 suppression for experiment.py and metric.py (circular imports) - Add ruff: noqa comment in create_sample_logs.py (load_dotenv before imports)
fercor-cisco
left a comment
There was a problem hiding this comment.
🤖 This review was generated by the Astra agent (claude-opus-4-8). It may contain mistakes.
Verdict: approve — Well-scoped tooling bump; substantive src changes (StrEnum, datetime.UTC, hash, F401) are behavior-preserving and correctly scoped.
Follow-ups
Suggested follow-up work that could be tracked as Shortcut stories:
pyproject.toml:248-263: The newexamples/**/*.pyper-file-ignore list is broad — it silences whole rule families includingS(security),B(bugbear),E722(bare except), andASYNC. That's a defensible tradeoff for demo code, but wholesale-disabling security/bugbear linting means genuinely buggy example patterns (e.g. broken async, resource leaks) will never surface even when they mislead users copying the examples. Consider narrowing to the specific rule codes actually triggered rather than entire prefixes, so future real issues in examples still get flagged.
| def __hash__(self) -> int: | ||
| return hash((self.content, self.role, self.tool_call_id)) |
There was a problem hiding this comment.
🟡 minor (bug): __hash__ will raise TypeError: unhashable type: 'list' when content is a list (the multimodal list[ContentPart] / IngestMessageContent case that LoggedMessage explicitly supports). This isn't a regression — the class was fully unhashable before (defined __eq__ without __hash__) — but by adding __hash__ you invite callers to put Message in a set/dict key, which will then blow up only for multimodal messages. Consider normalizing list content to a hashable form so the method is total, e.g.:
def __hash__(self) -> int:
content = tuple(self.content) if isinstance(self.content, list) else self.content
return hash((content, self.role, self.tool_call_id))(and if elements aren't hashable, hash their serialized form). At minimum, worth a test covering a list-content Message to document the intended behavior.
| def __hash__(self) -> int: | |
| return hash((self.content, self.role, self.tool_call_id)) | |
| def __hash__(self) -> int: | |
| content = tuple(self.content) if isinstance(self.content, list) else self.content | |
| return hash((content, self.role, self.tool_call_id)) |
🤖 Generated by Astra
fercor-cisco
left a comment
There was a problem hiding this comment.
I'll create follow-up tickets for the 2 notes from the review agent.
Summary
0.9.7→0.15.20in.pre-commit-config.yamlandpyproject.tomlopenapi-python-client0.26.1→0.29.0(0.26.x cappedruff<0.14, blocking the upgrade)Violations fixed by new rules
F401timezoneimport fromutils/__init__.pyPLW1641__hash__toMessageclass (defines__eq__without__hash__)PLC0415insrc/experiment.pyandmetric.pyto per-file-ignores (circular import pattern)PLC0415/RUF043/E402intests/pytest.importorskipconditional imports)examples/examples/**/*.pyper-file-ignores (ANN,ASYNC,S, etc. — examples are docs not prod code)E402increate_sample_logs.py# ruff: noqa: E402(file-level,load_dotenv()must precede imports)Test plan
ruff checkpasses locally ✅ruff formatapplied ✅ (92 files reformatted)