From c099ed030eb5e84b2b3f5adc466b1cd74b76420a Mon Sep 17 00:00:00 2001 From: Patrick Buckley Date: Mon, 15 Jun 2026 21:26:01 -0700 Subject: [PATCH] fix(attachments): address PR review feedback (Copilot + code-quality) - TextDecoder in the text-preview stream now flushes on completion/cancel, so a multibyte UTF-8 char split across a chunk boundary isn't dropped (Copilot). - send() clears self._wire_part_cache in a finally so the per-send memo (which can hold large rasterized PDF page-images) is released at send end instead of retained on an idle session until the next send (Copilot + fix-review). - Make the implicit byte-string concatenation in _minimal_pdf explicit (+) in test_pdf.py and test_thumbnails.py so it can't read as a missing comma (CodeQL / github-code-quality). --- tests/test_pdf.py | 2 +- tests/test_thumbnails.py | 2 +- turnstone/core/session.py | 12 +++++++++--- turnstone/shared_static/composer_attachments.js | 1 + 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/tests/test_pdf.py b/tests/test_pdf.py index 233769ec..310095a6 100644 --- a/tests/test_pdf.py +++ b/tests/test_pdf.py @@ -12,7 +12,7 @@ def _minimal_pdf(text: str = "Hello PDF") -> bytes: b"<>", b"<>", b"<>>>>>", + + b"/Contents 4 0 R/Resources<>>>>>", b"<>\nstream\n%s\nendstream" % (len(stream), stream), b"<>", ] diff --git a/tests/test_thumbnails.py b/tests/test_thumbnails.py index da9aff4d..0b5fa12f 100644 --- a/tests/test_thumbnails.py +++ b/tests/test_thumbnails.py @@ -20,7 +20,7 @@ def _minimal_pdf(text: str = "Hi") -> bytes: b"<>", b"<>", b"<>>>>>", + + b"/Contents 4 0 R/Resources<>>>>>", b"<>\nstream\n%s\nendstream" % (len(stream), stream), b"<>", ] diff --git a/turnstone/core/session.py b/turnstone/core/session.py index 046123d0..fecdfd61 100644 --- a/turnstone/core/session.py +++ b/turnstone/core/session.py @@ -2969,9 +2969,9 @@ class ChatSession: # re-rasterized / a blob re-base64'd once per round-trip. Key on # (id, caps-signature): the same stored blob materializes differently per # capability set, and a fallback to a different-caps model can resolve - # within one send. The cache is refreshed per send (set in send()); the - # wire resolver runs only during a send, so a stale value between sends is - # never read. A None cache disables memoization (the original behavior). + # within one send. Set in send() and cleared in its finally, so it is + # None outside a send; the wire resolver runs only during a send. A None + # cache disables memoization (the original behavior). cache = self._wire_part_cache caps_sig = (caps.supports_pdf, caps.supports_vision, caps.supports_audio_input) out: dict[str, Any] = {} @@ -4600,6 +4600,12 @@ class ChatSession: self._drain_pending_advisories() self._record_fatal_error(exc) raise + finally: + # Release the per-send wire-part memo (it can hold large rasterized + # PDF page-images) so it is GC'd at send end rather than retained on + # an idle session until the next send. Restores the "None outside a + # send" invariant on every exit (success, cancel, or error). + self._wire_part_cache = None def _drain_pending_advisories(self) -> None: """Drop every pending nudge regardless of channel. diff --git a/turnstone/shared_static/composer_attachments.js b/turnstone/shared_static/composer_attachments.js index 5edb7053..89f4c17f 100644 --- a/turnstone/shared_static/composer_attachments.js +++ b/turnstone/shared_static/composer_attachments.js @@ -139,6 +139,7 @@ export function buildAttachmentPreview(opts) { if (res && res.value) acc += dec.decode(res.value, { stream: true }); if (res.done || acc.length >= 240) { + acc += dec.decode(); // flush bytes buffered across a chunk boundary try { reader.cancel(); } catch (e) {