Skip to content

Commit c3cbcff

Browse files
fix: address PR review comments
- Use context managers for file reads in audit_test_quality.py (3 locations) - Remove no-op statement in test_emoji_parity.py - Fix fallback stream final edit sending raw text instead of remended content (P1) - Add hard cache size limits in Discord and GitHub adapters (evict oldest when full) - TimeoutError is correct for Python >=3.11 (our minimum) — no change needed - GChat pending subscription silent return matches TS behavior — no change needed - Linear token refresh already has double-checked locking — no change needed Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent e34e95b commit c3cbcff

5 files changed

Lines changed: 18 additions & 8 deletions

File tree

scripts/audit_test_quality.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,8 @@ def check_phantoms(test_files):
4747
"""Find tests whose only statement is `assert True`."""
4848
issues = []
4949
for fpath in test_files:
50-
tree = ast.parse(open(fpath).read())
50+
with open(fpath, encoding="utf-8") as f:
51+
tree = ast.parse(f.read())
5152
for node in ast.walk(tree):
5253
if not isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)):
5354
continue
@@ -74,7 +75,8 @@ def check_cross_file_duplicates(test_files):
7475
"""Find tests with identical bodies in different files."""
7576
bodies = collections.defaultdict(list)
7677
for fpath in test_files:
77-
source = open(fpath).read()
78+
with open(fpath, encoding="utf-8") as f:
79+
source = f.read()
7880
tree = ast.parse(source)
7981
for node in ast.walk(tree):
8082
if not isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)):
@@ -98,7 +100,8 @@ def check_magicmock_for_async(test_files):
98100
"""Find MagicMock assigned to known async method names."""
99101
issues = []
100102
for fpath in test_files:
101-
source = open(fpath).read()
103+
with open(fpath, encoding="utf-8") as f:
104+
source = f.read()
102105
for i, line in enumerate(source.split("\n"), 1):
103106
if "MagicMock" not in line or "AsyncMock" in line:
104107
continue

src/chat_sdk/adapters/discord/adapter.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -658,6 +658,11 @@ async def _handle_forwarded_reaction(
658658
expired = [k for k, v in self._thread_parent_cache.items() if v.get("expires_at", 0) <= now]
659659
for k in expired:
660660
del self._thread_parent_cache[k]
661+
# Hard limit: evict oldest if still over threshold
662+
if len(self._thread_parent_cache) > 1000:
663+
keys = list(self._thread_parent_cache.keys())
664+
for k in keys[: len(keys) - 1000]:
665+
del self._thread_parent_cache[k]
661666
except Exception as error:
662667
self._logger.error(
663668
"Failed to fetch thread parent for reaction",

src/chat_sdk/adapters/github/adapter.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -887,11 +887,15 @@ async def _get_installation_token(self, installation_id: int) -> str:
887887
cached token when still valid.
888888
"""
889889

890-
# Purge expired entries on access
890+
# Purge expired entries and enforce hard size limit
891891
now = time.time()
892892
expired_ids = [iid for iid, (_, exp) in self._installation_token_cache.items() if now >= exp]
893893
for iid in expired_ids:
894894
del self._installation_token_cache[iid]
895+
if len(self._installation_token_cache) > 100:
896+
keys = list(self._installation_token_cache.keys())
897+
for k in keys[: len(keys) - 100]:
898+
del self._installation_token_cache[k]
895899

896900
# Check cache
897901
cached = self._installation_token_cache.get(installation_id)

src/chat_sdk/thread.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -626,12 +626,12 @@ async def _edit_loop() -> None:
626626
await self.adapter.edit_message(
627627
thread_id_for_edits,
628628
msg.id,
629-
PostableMarkdown(markdown=accumulated),
629+
PostableMarkdown(markdown=final_content),
630630
)
631631

632632
sent = self._create_sent_message(
633633
msg.id,
634-
PostableMarkdown(markdown=accumulated),
634+
PostableMarkdown(markdown=final_content),
635635
thread_id_for_edits,
636636
)
637637
if self._message_history is not None:

tests/test_emoji_parity.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -268,8 +268,6 @@ def test_individual_emoji_parity(self, name: str):
268268
ts_entry = self.ts_map[name]
269269
assert name in DEFAULT_EMOJI_MAP, f"TS emoji {name!r} not in Python map"
270270

271-
DEFAULT_EMOJI_MAP[name]
272-
273271
# Check slack[0] matches
274272
if ts_entry["slack"]:
275273
py_slack = self.resolver.to_slack(name)

0 commit comments

Comments
 (0)