mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-29 18:46:59 +00:00
fix(photon): address review — U+FFFC before _record_last_inbound, MIME-based CAF promotion, add tests
- Move U+FFFC placeholder detection before _record_last_inbound() so the placeholder message id is not recorded as the reaction target - Cancel pending U+FFFC tasks in disconnect() to prevent task leaks - Check mimeType audio/x-caf in addition to filename for CAF→VOICE promotion, fixing unnamed attachments that default to '(unnamed)' - Add 7 Photon adapter tests: CAF named/unnamed promotion, U+FFFC no-dispatch, U+FFFC not recorded as last inbound, U+FFFC+attachment cancel, timeout fire, disconnect cleanup - Add 6 transcription tests: ffmpeg conversion, afconvert fallback, all-fail, CAF→WAV before Groq, conversion failure error, local provider skip
This commit is contained in:
Florian Valade
Teknium
parent
5277065260
commit
afab7ed46e
3 changed files with 391 additions and 19 deletions
|
|
@ -496,6 +496,11 @@ class PhotonAdapter(BasePlatformAdapter):
|
|||
except Exception:
|
||||
pass
|
||||
self._inbound_task = None
|
||||
# Cancel any pending U+FFFC placeholder tasks.
|
||||
for chat_key, (_, fffc_task) in list(self._pending_fffc.items()):
|
||||
if fffc_task and not fffc_task.done():
|
||||
fffc_task.cancel()
|
||||
self._pending_fffc.clear()
|
||||
await self._stop_sidecar()
|
||||
if self._http_client is not None:
|
||||
try:
|
||||
|
|
@ -689,8 +694,10 @@ class PhotonAdapter(BasePlatformAdapter):
|
|||
is_voice = payload.get("type") == "voice"
|
||||
name = payload.get("name") or ("voice" if is_voice else "(unnamed)")
|
||||
mime = payload.get("mimeType") or ""
|
||||
# Only .caf files are promoted to VOICE (iMessage voice notes use CAF).
|
||||
if not is_voice and name.lower().endswith(".caf"):
|
||||
# Promote CAF attachments to VOICE (iMessage voice notes use CAF).
|
||||
# Check both filename and MIME: the sidecar may send "(unnamed)"
|
||||
# when no name is supplied, so the MIME type is the reliable signal.
|
||||
if not is_voice and (name.lower().endswith(".caf") or mime == "audio/x-caf"):
|
||||
is_voice = True
|
||||
mtype = MessageType.VOICE if is_voice else _attachment_message_type(mime)
|
||||
cached = _cache_inbound_attachment(
|
||||
|
|
@ -762,6 +769,28 @@ class PhotonAdapter(BasePlatformAdapter):
|
|||
)
|
||||
)
|
||||
return
|
||||
# U+FFFC placeholder — wait for the real attachment instead of
|
||||
# dispatching. Detected before _record_last_inbound so the placeholder
|
||||
# is not recorded as the reaction target — the real attachment will be.
|
||||
if ctype == "text" and (content.get("text") or "").strip() == "\ufffc":
|
||||
chat_key = space_id
|
||||
prev = self._pending_fffc.pop(chat_key, None)
|
||||
if prev and prev[1] and not prev[1].done():
|
||||
prev[1].cancel()
|
||||
task = asyncio.create_task(
|
||||
self._fffc_timeout_handler(chat_key, event.get("messageId") or "")
|
||||
)
|
||||
self._pending_fffc[chat_key] = (time.monotonic(), task)
|
||||
logger.debug("[photon] U+FFFC placeholder received — waiting for attachment")
|
||||
return
|
||||
|
||||
# Cancel any pending U+FFFC timeout — the real message arrived.
|
||||
if ctype in {"attachment", "voice"}:
|
||||
prev = self._pending_fffc.pop(space_id, None)
|
||||
if prev and prev[1] and not prev[1].done():
|
||||
prev[1].cancel()
|
||||
logger.debug("[photon] attachment arrived — cancelling U+FFFC timeout")
|
||||
|
||||
# Anything past here is a real (reactable) message — remember it as
|
||||
# the chat's latest inbound so `add_reaction` can target it when the
|
||||
# caller doesn't pass an explicit message id. Recorded before the
|
||||
|
|
@ -769,25 +798,8 @@ class PhotonAdapter(BasePlatformAdapter):
|
|||
self._record_last_inbound(space_id, event.get("messageId"))
|
||||
if ctype == "text":
|
||||
text = content.get("text") or ""
|
||||
# U+FFFC placeholder — wait for the real attachment instead of dispatching.
|
||||
if text.strip() == "\ufffc":
|
||||
chat_key = space_id
|
||||
prev = self._pending_fffc.pop(chat_key, None)
|
||||
if prev and prev[1] and not prev[1].done():
|
||||
prev[1].cancel()
|
||||
task = asyncio.create_task(
|
||||
self._fffc_timeout_handler(chat_key, event.get("messageId") or "")
|
||||
)
|
||||
self._pending_fffc[chat_key] = (time.monotonic(), task)
|
||||
logger.debug("[photon] U+FFFC placeholder received — waiting for attachment")
|
||||
return
|
||||
mtype = MessageType.TEXT
|
||||
elif ctype in {"attachment", "voice"}:
|
||||
# Cancel any pending U+FFFC timeout — the real attachment arrived.
|
||||
prev = self._pending_fffc.pop(space_id, None)
|
||||
if prev and prev[1] and not prev[1].done():
|
||||
prev[1].cancel()
|
||||
logger.debug("[photon] attachment arrived — cancelling U+FFFC timeout")
|
||||
text, mtype, media_urls, media_types = _normalize_binary_payload(content)
|
||||
elif ctype == "group":
|
||||
text_parts: List[str] = []
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ sidecar-event parsing without spawning the Node sidecar or binding ports.
|
|||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import base64
|
||||
import json
|
||||
from pathlib import Path
|
||||
|
|
@ -362,3 +363,220 @@ def test_check_requirements_without_node(monkeypatch: pytest.MonkeyPatch) -> Non
|
|||
|
||||
monkeypatch.setattr(adapter_mod.shutil, "which", lambda _name: None)
|
||||
assert adapter_mod.check_requirements() is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# CAF attachment promotion + U+FFFC placeholder tests
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
_CAF_BYTES = b"caff" + b"\x00" * 60 # Minimal CAF header magic
|
||||
|
||||
|
||||
def _caf_attachment_event(
|
||||
content: Dict[str, Any], msg_id: str = "spc-msg-caf"
|
||||
) -> Dict[str, Any]:
|
||||
return {
|
||||
"messageId": msg_id,
|
||||
"space": {"id": "+155****4567", "type": "dm", "phone": "+155****4567"},
|
||||
"sender": {"id": "+155****4567"},
|
||||
"content": {"type": "attachment", **content},
|
||||
"timestamp": "2026-05-14T19:06:32.000Z",
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_caf_attachment_named_promoted_to_voice(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""A named .caf attachment is promoted to VOICE for STT routing."""
|
||||
adapter = _make_adapter(monkeypatch)
|
||||
captured = _capture(adapter, monkeypatch)
|
||||
|
||||
raw = _CAF_BYTES
|
||||
event = _caf_attachment_event(
|
||||
{
|
||||
"name": "voice_note.caf",
|
||||
"mimeType": "audio/x-caf",
|
||||
"size": len(raw),
|
||||
"data": base64.b64encode(raw).decode("ascii"),
|
||||
"encoding": "base64",
|
||||
}
|
||||
)
|
||||
await adapter._dispatch_inbound(event)
|
||||
|
||||
assert len(captured) == 1
|
||||
ev = captured[0]
|
||||
assert ev.message_type == MessageType.VOICE
|
||||
assert ev.media_types == ["audio/x-caf"]
|
||||
assert len(ev.media_urls) == 1
|
||||
cached = Path(ev.media_urls[0])
|
||||
try:
|
||||
assert cached.is_file()
|
||||
assert cached.read_bytes() == raw
|
||||
assert ev.text == "(voice)"
|
||||
finally:
|
||||
cached.unlink(missing_ok=True)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_caf_attachment_unnamed_promoted_via_mime(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""An unnamed attachment with mimeType audio/x-caf is promoted to VOICE.
|
||||
|
||||
The sidecar sends "(unnamed)" when no filename is supplied, so the MIME
|
||||
type must be the fallback signal for CAF promotion.
|
||||
"""
|
||||
adapter = _make_adapter(monkeypatch)
|
||||
captured = _capture(adapter, monkeypatch)
|
||||
|
||||
raw = _CAF_BYTES
|
||||
event = _caf_attachment_event(
|
||||
{
|
||||
"name": "(unnamed)",
|
||||
"mimeType": "audio/x-caf",
|
||||
"size": len(raw),
|
||||
"data": base64.b64encode(raw).decode("ascii"),
|
||||
"encoding": "base64",
|
||||
}
|
||||
)
|
||||
await adapter._dispatch_inbound(event)
|
||||
|
||||
assert len(captured) == 1
|
||||
ev = captured[0]
|
||||
assert ev.message_type == MessageType.VOICE
|
||||
assert ev.media_types == ["audio/x-caf"]
|
||||
cached = Path(ev.media_urls[0])
|
||||
try:
|
||||
assert cached.is_file()
|
||||
assert cached.suffix == ".caf"
|
||||
finally:
|
||||
cached.unlink(missing_ok=True)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fffc_placeholder_no_dispatch(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""A U+FFFC placeholder text does not trigger a message dispatch."""
|
||||
adapter = _make_adapter(monkeypatch)
|
||||
captured = _capture(adapter, monkeypatch)
|
||||
|
||||
event = _dm_event("\ufffc", msg_id="spc-msg-fffc")
|
||||
chat_key = event["space"]["id"]
|
||||
await adapter._dispatch_inbound(event)
|
||||
|
||||
assert len(captured) == 0
|
||||
assert chat_key in adapter._pending_fffc
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fffc_placeholder_not_recorded_as_last_inbound(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""The U+FFFC placeholder must not be recorded as the reaction target.
|
||||
|
||||
_record_last_inbound runs after the U+FFFC early-return, so the placeholder
|
||||
message id is never stored. A subsequent real message will be recorded.
|
||||
"""
|
||||
adapter = _make_adapter(monkeypatch)
|
||||
_capture(adapter, monkeypatch)
|
||||
|
||||
fffc_event = _dm_event("\ufffc", msg_id="spc-msg-fffc")
|
||||
chat_key = fffc_event["space"]["id"]
|
||||
await adapter._dispatch_inbound(fffc_event)
|
||||
assert chat_key not in adapter._last_inbound_by_chat
|
||||
|
||||
real_event = _dm_event("hello", msg_id="spc-msg-real")
|
||||
await adapter._dispatch_inbound(real_event)
|
||||
assert adapter._last_inbound_by_chat.get(chat_key) == "spc-msg-real"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fffc_then_attachment_cancels_timeout(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""When an attachment arrives after a U+FFFC placeholder, the pending
|
||||
timeout task is cancelled and the attachment is dispatched normally."""
|
||||
adapter = _make_adapter(monkeypatch)
|
||||
captured = _capture(adapter, monkeypatch)
|
||||
|
||||
fffc_event = _dm_event("\ufffc", msg_id="spc-msg-fffc")
|
||||
chat_key = fffc_event["space"]["id"]
|
||||
await adapter._dispatch_inbound(fffc_event)
|
||||
assert len(captured) == 0
|
||||
assert chat_key in adapter._pending_fffc
|
||||
|
||||
raw = _CAF_BYTES
|
||||
att_event = _caf_attachment_event(
|
||||
{
|
||||
"name": "voice.caf",
|
||||
"mimeType": "audio/x-caf",
|
||||
"size": len(raw),
|
||||
"data": base64.b64encode(raw).decode("ascii"),
|
||||
"encoding": "base64",
|
||||
},
|
||||
msg_id="spc-msg-att",
|
||||
)
|
||||
att_event["space"]["id"] = chat_key
|
||||
await adapter._dispatch_inbound(att_event)
|
||||
|
||||
assert len(captured) == 1
|
||||
assert captured[0].message_type == MessageType.VOICE
|
||||
assert chat_key not in adapter._pending_fffc
|
||||
assert adapter._last_inbound_by_chat.get(chat_key) == "spc-msg-att"
|
||||
|
||||
cached = Path(captured[0].media_urls[0])
|
||||
try:
|
||||
assert cached.is_file()
|
||||
finally:
|
||||
cached.unlink(missing_ok=True)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fffc_timeout_fires_when_no_attachment(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""When no attachment arrives within the timeout, the pending entry is
|
||||
cleaned up and a warning is logged."""
|
||||
import plugins.platforms.photon.adapter as adapter_mod
|
||||
|
||||
monkeypatch.setattr(adapter_mod, "_FFFC_WAIT_SECONDS", 0.1)
|
||||
|
||||
adapter = _make_adapter(monkeypatch)
|
||||
captured = _capture(adapter, monkeypatch)
|
||||
|
||||
fffc_event = _dm_event("\ufffc", msg_id="spc-msg-fffc")
|
||||
chat_key = fffc_event["space"]["id"]
|
||||
await adapter._dispatch_inbound(fffc_event)
|
||||
assert chat_key in adapter._pending_fffc
|
||||
|
||||
await asyncio.sleep(0.3)
|
||||
|
||||
assert chat_key not in adapter._pending_fffc
|
||||
assert len(captured) == 0
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_disconnect_cancels_pending_fffc_tasks(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""disconnect() cancels any pending U+FFFC placeholder tasks."""
|
||||
adapter = _make_adapter(monkeypatch)
|
||||
_capture(adapter, monkeypatch)
|
||||
|
||||
await adapter._dispatch_inbound(_dm_event("\ufffc", msg_id="spc-msg-fffc"))
|
||||
assert len(adapter._pending_fffc) == 1
|
||||
|
||||
async def _noop_stop_sidecar():
|
||||
pass
|
||||
|
||||
monkeypatch.setattr(adapter, "_stop_sidecar", _noop_stop_sidecar)
|
||||
monkeypatch.setattr(adapter, "_inbound_running", False)
|
||||
monkeypatch.setattr(adapter, "_inbound_task", None)
|
||||
monkeypatch.setattr(adapter, "_sidecar_health_task", None)
|
||||
monkeypatch.setattr(adapter, "_http_client", None)
|
||||
|
||||
await adapter.disconnect()
|
||||
|
||||
assert len(adapter._pending_fffc) == 0
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@ import struct
|
|||
import subprocess
|
||||
import types
|
||||
import wave
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock, call, patch
|
||||
|
||||
import pytest
|
||||
|
|
@ -2173,3 +2174,144 @@ class TestLocalBaseUrlNoApiKey:
|
|||
assert _is_local_or_private_url("http://stt.internal/v1")
|
||||
assert not _is_local_or_private_url("https://api.openai.com/v1")
|
||||
assert not _is_local_or_private_url("")
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# CAF (iMessage voice note) conversion tests
|
||||
# ============================================================================
|
||||
|
||||
class TestCafConversion:
|
||||
"""Tests for _convert_caf_to_wav and CAF dispatch in transcribe_audio."""
|
||||
|
||||
def test_convert_caf_with_ffmpeg(self, tmp_path, monkeypatch):
|
||||
"""_convert_caf_to_wav uses ffmpeg when available."""
|
||||
caf_path = tmp_path / "voice.caf"
|
||||
caf_path.write_bytes(b"caff\x00" * 20)
|
||||
wav_path = str(tmp_path / "voice.wav")
|
||||
|
||||
def fake_run(cmd, **kwargs):
|
||||
Path(wav_path).write_bytes(b"RIFF\x00\x00\x00\x00")
|
||||
return MagicMock(returncode=0)
|
||||
|
||||
monkeypatch.setattr(
|
||||
"tools.transcription_tools._find_ffmpeg_binary",
|
||||
lambda: "/usr/bin/ffmpeg",
|
||||
)
|
||||
monkeypatch.setattr(subprocess, "run", fake_run)
|
||||
|
||||
from tools.transcription_tools import _convert_caf_to_wav
|
||||
result = _convert_caf_to_wav(str(caf_path))
|
||||
assert result == wav_path
|
||||
assert Path(result).exists()
|
||||
|
||||
def test_convert_caf_fallback_to_afconvert(self, tmp_path, monkeypatch):
|
||||
"""When ffmpeg is not found, falls back to afconvert (macOS)."""
|
||||
caf_path = tmp_path / "voice.caf"
|
||||
caf_path.write_bytes(b"caff\x00" * 20)
|
||||
wav_path = str(tmp_path / "voice.wav")
|
||||
|
||||
call_count = {"n": 0}
|
||||
|
||||
def fake_run(cmd, **kwargs):
|
||||
call_count["n"] += 1
|
||||
if cmd[0] == "/usr/bin/ffmpeg":
|
||||
raise subprocess.CalledProcessError(1, cmd)
|
||||
Path(wav_path).write_bytes(b"RIFF\x00\x00\x00\x00")
|
||||
return MagicMock(returncode=0)
|
||||
|
||||
monkeypatch.setattr(
|
||||
"tools.transcription_tools._find_ffmpeg_binary",
|
||||
lambda: "/usr/bin/ffmpeg",
|
||||
)
|
||||
monkeypatch.setattr(subprocess, "run", fake_run)
|
||||
monkeypatch.setattr(
|
||||
"tools.transcription_tools.shutil.which",
|
||||
lambda name: "/usr/bin/afconvert" if name == "afconvert" else None,
|
||||
)
|
||||
|
||||
from tools.transcription_tools import _convert_caf_to_wav
|
||||
result = _convert_caf_to_wav(str(caf_path))
|
||||
assert result == wav_path
|
||||
assert call_count["n"] == 2
|
||||
|
||||
def test_convert_caf_all_converters_fail(self, tmp_path, monkeypatch):
|
||||
"""When both ffmpeg and afconvert are unavailable, returns None."""
|
||||
caf_path = tmp_path / "voice.caf"
|
||||
caf_path.write_bytes(b"caff\x00" * 20)
|
||||
|
||||
monkeypatch.setattr(
|
||||
"tools.transcription_tools._find_ffmpeg_binary", lambda: None
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"tools.transcription_tools.shutil.which", lambda name: None
|
||||
)
|
||||
|
||||
from tools.transcription_tools import _convert_caf_to_wav
|
||||
result = _convert_caf_to_wav(str(caf_path))
|
||||
assert result is None
|
||||
|
||||
def test_transcribe_caf_converted_before_groq(self, tmp_path, monkeypatch):
|
||||
"""transcribe_audio converts .caf to .wav before dispatching to Groq."""
|
||||
caf_path = tmp_path / "voice.caf"
|
||||
caf_path.write_bytes(b"caff\x00" * 20)
|
||||
wav_path = str(tmp_path / "voice.wav")
|
||||
|
||||
def fake_convert(file_path):
|
||||
Path(wav_path).write_bytes(b"RIFF\x00\x00\x00\x00")
|
||||
return wav_path
|
||||
|
||||
with patch("tools.transcription_tools._load_stt_config",
|
||||
return_value={"provider": "groq"}), \
|
||||
patch("tools.transcription_tools._get_provider",
|
||||
return_value="groq"), \
|
||||
patch("tools.transcription_tools._convert_caf_to_wav",
|
||||
side_effect=fake_convert) as mock_convert, \
|
||||
patch("tools.transcription_tools._transcribe_groq",
|
||||
return_value={"success": True, "transcript": "hello",
|
||||
"provider": "groq"}) as mock_groq:
|
||||
from tools.transcription_tools import transcribe_audio
|
||||
result = transcribe_audio(str(caf_path))
|
||||
|
||||
assert result["success"] is True
|
||||
mock_convert.assert_called_once_with(str(caf_path))
|
||||
mock_groq.assert_called_once()
|
||||
call_args = mock_groq.call_args
|
||||
sent_path = call_args[0][0] if call_args[0] else call_args[1].get("file_path")
|
||||
assert sent_path == wav_path
|
||||
|
||||
def test_transcribe_caf_conversion_failure_returns_error(
|
||||
self, tmp_path, monkeypatch
|
||||
):
|
||||
"""When CAF conversion fails, transcribe_audio returns an error."""
|
||||
caf_path = tmp_path / "voice.caf"
|
||||
caf_path.write_bytes(b"caff\x00" * 20)
|
||||
|
||||
with patch("tools.transcription_tools._load_stt_config",
|
||||
return_value={"provider": "groq"}), \
|
||||
patch("tools.transcription_tools._get_provider",
|
||||
return_value="groq"), \
|
||||
patch("tools.transcription_tools._convert_caf_to_wav",
|
||||
return_value=None):
|
||||
from tools.transcription_tools import transcribe_audio
|
||||
result = transcribe_audio(str(caf_path))
|
||||
|
||||
assert result["success"] is False
|
||||
assert "could not be converted" in result["error"]
|
||||
|
||||
def test_transcribe_caf_not_converted_for_local(self, tmp_path, monkeypatch):
|
||||
"""CAF conversion is skipped for local provider (native handling)."""
|
||||
caf_path = tmp_path / "voice.caf"
|
||||
caf_path.write_bytes(b"caff\x00" * 20)
|
||||
|
||||
with patch("tools.transcription_tools._load_stt_config",
|
||||
return_value={"provider": "local"}), \
|
||||
patch("tools.transcription_tools._get_provider",
|
||||
return_value="local"), \
|
||||
patch("tools.transcription_tools._convert_caf_to_wav") as mock_convert, \
|
||||
patch("tools.transcription_tools._transcribe_local",
|
||||
return_value={"success": True, "transcript": "hi"}):
|
||||
from tools.transcription_tools import transcribe_audio
|
||||
result = transcribe_audio(str(caf_path))
|
||||
|
||||
assert result["success"] is True
|
||||
mock_convert.assert_not_called()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue