mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-09-30 08:03:15 +00:00
feat: add Slack reaction feedback to LangSmith (#1231)
* feat: add Slack reaction feedback to LangSmith Record Slack reaction feedback against explicitly mapped LangGraph runs so user ratings are idempotent and tied to the message they reacted to. * Address review feedback on Slack reaction → LangSmith feedback - langsmith.py: drop lru_cache on _build_langsmith_feedback_clients so rotated keys / late env hydration are picked up; dedupe by (key, url) tuple instead of key alone so the same key pointing at different endpoints (cloud + self-hosted) builds both clients. - langsmith.py: treat LangSmithNotFoundError on delete_feedback as success — out-of-order or redelivered reaction_removed events would otherwise loop forever on Slack's retry policy. - slack_feedback.py: include channel_id in _feedback_key so the same message_ts in two channels can't collide on the same feedback id. - slack_feedback.py: treat conflicting +/- reactions from one user as ambiguous (clear feedback) instead of averaging to a misleading 0.5. - slack_feedback.py + slack.py + webapp.py: gate reaction handling to the user who triggered the run (stored in the slack_run_map mapping alongside run_id). Prevents bystanders in shared channels from polluting eval feedback.
This commit is contained in:
parent
d151a118ae
commit
3f9dbb6597
9 changed files with 794 additions and 18 deletions
|
|
@ -138,6 +138,8 @@ For tasks that require code changes, follow this order:
|
|||
|
||||
**Strict requirement:** Never claim "PR updated/opened" unless `gh` returned success and you have the PR URL from command output or `GH_TOKEN=dummy gh pr view --json url --jq .url`. If push or PR creation fails, state that explicitly.
|
||||
|
||||
For Slack-triggered tasks, end your final `slack_thread_reply` completion summary by asking the user to react with `:+1:` or `:-1:` to provide feedback.
|
||||
|
||||
For questions or status checks (no code changes needed):
|
||||
|
||||
1. **Answer** — Gather the information needed to respond.
|
||||
|
|
|
|||
|
|
@ -1,9 +1,19 @@
|
|||
import asyncio
|
||||
import os
|
||||
from typing import Any
|
||||
|
||||
from langgraph.config import get_config
|
||||
from langgraph_sdk import get_client
|
||||
|
||||
from ..utils.slack import convert_mentions_to_slack_format, post_slack_thread_reply
|
||||
from ..utils.slack import (
|
||||
convert_mentions_to_slack_format,
|
||||
post_slack_thread_reply_with_ts,
|
||||
store_slack_message_run_mapping,
|
||||
)
|
||||
|
||||
LANGGRAPH_URL = os.environ.get("LANGGRAPH_URL") or os.environ.get(
|
||||
"LANGGRAPH_URL_PROD", "http://localhost:2024"
|
||||
)
|
||||
|
||||
|
||||
def slack_thread_reply(message: str) -> dict[str, Any]:
|
||||
|
|
@ -39,5 +49,13 @@ def slack_thread_reply(message: str) -> dict[str, Any]:
|
|||
return {"success": False, "error": "Message cannot be empty"}
|
||||
|
||||
message = convert_mentions_to_slack_format(message)
|
||||
success = asyncio.run(post_slack_thread_reply(channel_id, thread_ts, message))
|
||||
return {"success": success}
|
||||
message_ts = asyncio.run(_post_and_store_mapping(channel_id, thread_ts, message))
|
||||
return {"success": message_ts is not None}
|
||||
|
||||
|
||||
async def _post_and_store_mapping(channel_id: str, thread_ts: str, message: str) -> str | None:
|
||||
message_ts = await post_slack_thread_reply_with_ts(channel_id, thread_ts, message)
|
||||
if message_ts:
|
||||
langgraph_client = get_client(url=LANGGRAPH_URL)
|
||||
await store_slack_message_run_mapping(langgraph_client, channel_id, thread_ts, message_ts)
|
||||
return message_ts
|
||||
|
|
|
|||
|
|
@ -4,6 +4,11 @@ from __future__ import annotations
|
|||
|
||||
import logging
|
||||
import os
|
||||
import uuid
|
||||
from typing import Any
|
||||
|
||||
from langsmith import Client as LangSmithClient
|
||||
from langsmith.utils import LangSmithNotFoundError
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
|
@ -30,3 +35,94 @@ def get_langsmith_trace_url(thread_id: str) -> str | None:
|
|||
"Failed to build LangSmith trace URL for thread %s", thread_id, exc_info=True
|
||||
)
|
||||
return None
|
||||
|
||||
|
||||
def _build_langsmith_feedback_clients() -> tuple[LangSmithClient, ...]:
|
||||
"""Build feedback clients from current env. Re-read each call so rotated
|
||||
keys / late secret hydration are picked up."""
|
||||
clients: list[LangSmithClient] = []
|
||||
seen: set[tuple[str, str]] = set()
|
||||
|
||||
api_endpoint = os.environ.get("LANGSMITH_ENDPOINT", "https://api.smith.langchain.com")
|
||||
client_configs = (
|
||||
(
|
||||
os.environ.get("LANGSMITH_API_KEY") or os.environ.get("LANGCHAIN_API_KEY"),
|
||||
api_endpoint,
|
||||
),
|
||||
(
|
||||
os.environ.get("LANGSMITH_API_KEY_PROD"),
|
||||
os.environ.get("LANGSMITH_ENDPOINT_PROD", api_endpoint),
|
||||
),
|
||||
)
|
||||
|
||||
for api_key, api_url in client_configs:
|
||||
if not api_key or not api_url:
|
||||
continue
|
||||
identity = (api_key, api_url)
|
||||
if identity in seen:
|
||||
continue
|
||||
clients.append(LangSmithClient(api_key=api_key, api_url=api_url))
|
||||
seen.add(identity)
|
||||
|
||||
return tuple(clients)
|
||||
|
||||
|
||||
def _feedback_id(run_id: str, key: str) -> uuid.UUID:
|
||||
return uuid.uuid5(uuid.NAMESPACE_URL, f"langsmith-feedback:{run_id}:{key}")
|
||||
|
||||
|
||||
def create_langsmith_feedback(
|
||||
run_id: str,
|
||||
key: str,
|
||||
*,
|
||||
score: float,
|
||||
comment: str | None = None,
|
||||
source_info: dict[str, Any] | None = None,
|
||||
) -> bool:
|
||||
"""Create or update deterministic feedback on all configured LangSmith clients."""
|
||||
clients = _build_langsmith_feedback_clients()
|
||||
if not clients:
|
||||
logger.warning("No LangSmith API key configured, skipping feedback")
|
||||
return False
|
||||
|
||||
feedback_id = _feedback_id(run_id, key)
|
||||
any_success = False
|
||||
for client in clients:
|
||||
try:
|
||||
client.create_feedback(
|
||||
run_id=run_id,
|
||||
key=key,
|
||||
score=score,
|
||||
comment=comment,
|
||||
source_info=source_info,
|
||||
feedback_source_type="api",
|
||||
feedback_id=feedback_id,
|
||||
)
|
||||
any_success = True
|
||||
except Exception:
|
||||
try:
|
||||
client.update_feedback(feedback_id, score=score, comment=comment)
|
||||
any_success = True
|
||||
except Exception:
|
||||
logger.exception("Failed to create or update LangSmith feedback for run %s", run_id)
|
||||
return any_success
|
||||
|
||||
|
||||
def delete_langsmith_feedback(run_id: str, key: str) -> bool:
|
||||
"""Delete deterministic feedback from all configured LangSmith clients."""
|
||||
clients = _build_langsmith_feedback_clients()
|
||||
if not clients:
|
||||
logger.warning("No LangSmith API key configured, skipping feedback deletion")
|
||||
return False
|
||||
|
||||
feedback_id = _feedback_id(run_id, key)
|
||||
any_success = False
|
||||
for client in clients:
|
||||
try:
|
||||
client.delete_feedback(feedback_id)
|
||||
any_success = True
|
||||
except LangSmithNotFoundError:
|
||||
any_success = True
|
||||
except Exception:
|
||||
logger.exception("Failed to delete LangSmith feedback for run %s", run_id)
|
||||
return any_success
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@ from typing import Any
|
|||
from urllib.parse import urlparse
|
||||
|
||||
import httpx
|
||||
from langgraph_sdk.client import LangGraphClient
|
||||
|
||||
from agent.utils.langsmith import get_langsmith_trace_url
|
||||
|
||||
|
|
@ -333,10 +334,10 @@ async def set_slack_assistant_status(
|
|||
return False
|
||||
|
||||
|
||||
async def post_slack_thread_reply(channel_id: str, thread_ts: str, text: str) -> bool:
|
||||
"""Post a reply in a Slack thread."""
|
||||
async def post_slack_thread_reply_with_ts(channel_id: str, thread_ts: str, text: str) -> str | None:
|
||||
"""Post a reply in a Slack thread and return its Slack timestamp."""
|
||||
if not SLACK_BOT_TOKEN:
|
||||
return False
|
||||
return None
|
||||
|
||||
payload = {
|
||||
"channel": channel_id,
|
||||
|
|
@ -355,11 +356,17 @@ async def post_slack_thread_reply(channel_id: str, thread_ts: str, text: str) ->
|
|||
data = response.json()
|
||||
if not data.get("ok"):
|
||||
logger.warning("Slack chat.postMessage failed: %s", data.get("error"))
|
||||
return False
|
||||
return True
|
||||
return None
|
||||
message_ts = data.get("ts")
|
||||
return message_ts if isinstance(message_ts, str) and message_ts else None
|
||||
except httpx.HTTPError:
|
||||
logger.exception("Slack chat.postMessage request failed")
|
||||
return False
|
||||
return None
|
||||
|
||||
|
||||
async def post_slack_thread_reply(channel_id: str, thread_ts: str, text: str) -> bool:
|
||||
"""Post a reply in a Slack thread."""
|
||||
return await post_slack_thread_reply_with_ts(channel_id, thread_ts, text) is not None
|
||||
|
||||
|
||||
async def post_slack_ephemeral_message(
|
||||
|
|
@ -709,9 +716,139 @@ def _format_trace_reply(message: str, trace_url: str | None) -> str:
|
|||
|
||||
async def post_slack_trace_reply(
|
||||
channel_id: str, thread_ts: str, thread_id: str, message: str | None = None
|
||||
) -> None:
|
||||
"""Post a trace URL reply in a Slack thread."""
|
||||
) -> str | None:
|
||||
"""Post a trace URL reply in a Slack thread and return its Slack timestamp."""
|
||||
if message is None:
|
||||
message = random.choice(TRACE_REPLY_PHRASES)
|
||||
trace_url = get_langsmith_trace_url(thread_id)
|
||||
await post_slack_thread_reply(channel_id, thread_ts, _format_trace_reply(message, trace_url))
|
||||
return await post_slack_thread_reply_with_ts(
|
||||
channel_id, thread_ts, _format_trace_reply(message, trace_url)
|
||||
)
|
||||
|
||||
|
||||
_SLACK_RUN_MAP_NAMESPACE = "slack_run_map"
|
||||
_THREAD_RUN_KEY_PREFIX = "thread:"
|
||||
_MESSAGE_RUN_KEY_PREFIX = "message:"
|
||||
|
||||
|
||||
def _extract_run_id_from_store_item(item: dict[str, Any] | None) -> str | None:
|
||||
if not item:
|
||||
return None
|
||||
value = item.get("value")
|
||||
if not isinstance(value, dict):
|
||||
return None
|
||||
run_id = value.get("run_id")
|
||||
return run_id if isinstance(run_id, str) and run_id else None
|
||||
|
||||
|
||||
async def store_slack_run_mapping(
|
||||
langgraph_client: LangGraphClient,
|
||||
channel_id: str,
|
||||
thread_ts: str,
|
||||
run_id: str,
|
||||
*,
|
||||
message_ts: str | None = None,
|
||||
triggering_user_id: str | None = None,
|
||||
) -> None:
|
||||
"""Persist Slack thread/message to LangGraph run mapping."""
|
||||
namespace = (_SLACK_RUN_MAP_NAMESPACE, channel_id)
|
||||
value: dict[str, Any] = {"run_id": run_id, "thread_ts": thread_ts}
|
||||
if triggering_user_id:
|
||||
value["triggering_user_id"] = triggering_user_id
|
||||
try:
|
||||
await langgraph_client.store.put_item(
|
||||
namespace, f"{_THREAD_RUN_KEY_PREFIX}{thread_ts}", value
|
||||
)
|
||||
if message_ts:
|
||||
await langgraph_client.store.put_item(
|
||||
namespace,
|
||||
f"{_MESSAGE_RUN_KEY_PREFIX}{message_ts}",
|
||||
{**value, "message_ts": message_ts},
|
||||
)
|
||||
except Exception:
|
||||
logger.exception(
|
||||
"Failed to store Slack run mapping for channel=%s thread=%s run=%s",
|
||||
channel_id,
|
||||
thread_ts,
|
||||
run_id,
|
||||
)
|
||||
|
||||
|
||||
async def store_slack_message_run_mapping(
|
||||
langgraph_client: LangGraphClient,
|
||||
channel_id: str,
|
||||
thread_ts: str,
|
||||
message_ts: str,
|
||||
) -> None:
|
||||
"""Persist a Slack message mapping using the current thread's run mapping."""
|
||||
namespace = (_SLACK_RUN_MAP_NAMESPACE, channel_id)
|
||||
try:
|
||||
item = await langgraph_client.store.get_item(
|
||||
namespace, f"{_THREAD_RUN_KEY_PREFIX}{thread_ts}"
|
||||
)
|
||||
run_id = _extract_run_id_from_store_item(item)
|
||||
if not run_id:
|
||||
logger.debug(
|
||||
"No Slack thread run mapping found for channel=%s thread=%s",
|
||||
channel_id,
|
||||
thread_ts,
|
||||
)
|
||||
return
|
||||
triggering_user_id: str | None = None
|
||||
if isinstance(item, dict):
|
||||
value = item.get("value")
|
||||
if isinstance(value, dict):
|
||||
candidate = value.get("triggering_user_id")
|
||||
if isinstance(candidate, str) and candidate:
|
||||
triggering_user_id = candidate
|
||||
await store_slack_run_mapping(
|
||||
langgraph_client,
|
||||
channel_id,
|
||||
thread_ts,
|
||||
run_id,
|
||||
message_ts=message_ts,
|
||||
triggering_user_id=triggering_user_id,
|
||||
)
|
||||
except Exception:
|
||||
logger.exception(
|
||||
"Failed to store Slack message run mapping for channel=%s message=%s",
|
||||
channel_id,
|
||||
message_ts,
|
||||
)
|
||||
|
||||
|
||||
async def lookup_slack_run_mapping(
|
||||
langgraph_client: LangGraphClient,
|
||||
channel_id: str,
|
||||
message_ts: str,
|
||||
) -> dict[str, Any] | None:
|
||||
"""Return the stored mapping value for a Slack bot message, or None."""
|
||||
namespace = (_SLACK_RUN_MAP_NAMESPACE, channel_id)
|
||||
try:
|
||||
item = await langgraph_client.store.get_item(
|
||||
namespace, f"{_MESSAGE_RUN_KEY_PREFIX}{message_ts}"
|
||||
)
|
||||
except Exception:
|
||||
logger.exception(
|
||||
"Failed to look up Slack message run mapping for channel=%s message=%s",
|
||||
channel_id,
|
||||
message_ts,
|
||||
)
|
||||
return None
|
||||
if not item:
|
||||
return None
|
||||
value = item.get("value")
|
||||
return value if isinstance(value, dict) else None
|
||||
|
||||
|
||||
async def lookup_run_id_for_slack_message(
|
||||
langgraph_client: LangGraphClient,
|
||||
channel_id: str,
|
||||
message_ts: str,
|
||||
) -> str | None:
|
||||
"""Look up the LangGraph run mapped to a specific Slack bot message."""
|
||||
value = await lookup_slack_run_mapping(langgraph_client, channel_id, message_ts)
|
||||
if not value:
|
||||
return None
|
||||
run_id = value.get("run_id")
|
||||
return run_id if isinstance(run_id, str) and run_id else None
|
||||
|
|
|
|||
213
agent/utils/slack_feedback.py
Normal file
213
agent/utils/slack_feedback.py
Normal file
|
|
@ -0,0 +1,213 @@
|
|||
"""Slack reaction feedback handling."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import logging
|
||||
import os
|
||||
from typing import Any
|
||||
|
||||
from langgraph_sdk import get_client
|
||||
from langgraph_sdk.client import LangGraphClient
|
||||
|
||||
from .langsmith import create_langsmith_feedback, delete_langsmith_feedback
|
||||
from .slack import lookup_slack_run_mapping
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
LANGGRAPH_URL = os.environ.get("LANGGRAPH_URL") or os.environ.get(
|
||||
"LANGGRAPH_URL_PROD", "http://localhost:2024"
|
||||
)
|
||||
|
||||
FEEDBACK_REACTIONS: dict[str, float] = {
|
||||
"+1": 1.0,
|
||||
"thumbsup": 1.0,
|
||||
"-1": 0.0,
|
||||
"thumbsdown": 0.0,
|
||||
}
|
||||
|
||||
_REACTION_STATE_NAMESPACE = "slack_reaction_state"
|
||||
_REACTION_EVENT_NAMESPACE = "slack_reaction_events"
|
||||
|
||||
|
||||
def _read_active_reactions(item: dict[str, Any] | None) -> set[str]:
|
||||
if not item:
|
||||
return set()
|
||||
value = item.get("value")
|
||||
if not isinstance(value, dict):
|
||||
return set()
|
||||
reactions = value.get("reactions")
|
||||
if not isinstance(reactions, list):
|
||||
return set()
|
||||
return {reaction for reaction in reactions if isinstance(reaction, str)}
|
||||
|
||||
|
||||
def _feedback_key(channel_id: str, user_id: str, message_ts: str) -> str:
|
||||
return f"slack_reaction:{channel_id}:{user_id}:{message_ts}"
|
||||
|
||||
|
||||
def _reaction_state_key(run_id: str, user_id: str, message_ts: str) -> str:
|
||||
return f"{run_id}:{user_id}:{message_ts}"
|
||||
|
||||
|
||||
async def _event_was_processed(
|
||||
langgraph_client: LangGraphClient, channel_id: str, event_id: str
|
||||
) -> bool:
|
||||
if not event_id:
|
||||
return False
|
||||
item = await langgraph_client.store.get_item((_REACTION_EVENT_NAMESPACE, channel_id), event_id)
|
||||
return bool(item)
|
||||
|
||||
|
||||
async def _mark_event_processed(
|
||||
langgraph_client: LangGraphClient, channel_id: str, event_id: str
|
||||
) -> None:
|
||||
if not event_id:
|
||||
return
|
||||
await langgraph_client.store.put_item(
|
||||
(_REACTION_EVENT_NAMESPACE, channel_id), event_id, {"event_id": event_id}
|
||||
)
|
||||
|
||||
|
||||
async def _update_reaction_state(
|
||||
langgraph_client: LangGraphClient,
|
||||
*,
|
||||
channel_id: str,
|
||||
run_id: str,
|
||||
user_id: str,
|
||||
message_ts: str,
|
||||
reaction: str,
|
||||
added: bool,
|
||||
) -> set[str]:
|
||||
namespace = (_REACTION_STATE_NAMESPACE, channel_id)
|
||||
key = _reaction_state_key(run_id, user_id, message_ts)
|
||||
item = await langgraph_client.store.get_item(namespace, key)
|
||||
active_reactions = _read_active_reactions(item)
|
||||
|
||||
if added:
|
||||
active_reactions.add(reaction)
|
||||
else:
|
||||
active_reactions.discard(reaction)
|
||||
|
||||
await langgraph_client.store.put_item(
|
||||
namespace,
|
||||
key,
|
||||
{
|
||||
"run_id": run_id,
|
||||
"user_id": user_id,
|
||||
"message_ts": message_ts,
|
||||
"reactions": sorted(active_reactions),
|
||||
},
|
||||
)
|
||||
return active_reactions
|
||||
|
||||
|
||||
def _score_reactions(reactions: set[str]) -> float | None:
|
||||
scores = {
|
||||
FEEDBACK_REACTIONS[reaction] for reaction in reactions if reaction in FEEDBACK_REACTIONS
|
||||
}
|
||||
if not scores:
|
||||
return None
|
||||
if len(scores) > 1:
|
||||
# Conflicting positive + negative reactions from the same user — treat
|
||||
# as ambiguous and clear feedback rather than recording a misleading
|
||||
# average score.
|
||||
return None
|
||||
return next(iter(scores))
|
||||
|
||||
|
||||
async def process_slack_reaction(
|
||||
event: dict[str, Any],
|
||||
*,
|
||||
event_id: str = "",
|
||||
added: bool,
|
||||
) -> None:
|
||||
reaction = event.get("reaction")
|
||||
if not isinstance(reaction, str) or reaction not in FEEDBACK_REACTIONS:
|
||||
return
|
||||
|
||||
item = event.get("item")
|
||||
if not isinstance(item, dict) or item.get("type") != "message":
|
||||
return
|
||||
|
||||
channel_id = item.get("channel")
|
||||
message_ts = item.get("ts")
|
||||
user_id = event.get("user")
|
||||
if not (
|
||||
isinstance(channel_id, str)
|
||||
and channel_id
|
||||
and isinstance(message_ts, str)
|
||||
and message_ts
|
||||
and isinstance(user_id, str)
|
||||
and user_id
|
||||
):
|
||||
return
|
||||
|
||||
langgraph_client = get_client(url=LANGGRAPH_URL)
|
||||
if await _event_was_processed(langgraph_client, channel_id, event_id):
|
||||
return
|
||||
|
||||
mapping = await lookup_slack_run_mapping(langgraph_client, channel_id, message_ts)
|
||||
if not mapping:
|
||||
logger.debug(
|
||||
"No run mapping for Slack reaction on channel=%s message=%s",
|
||||
channel_id,
|
||||
message_ts,
|
||||
)
|
||||
return
|
||||
run_id_value = mapping.get("run_id")
|
||||
if not isinstance(run_id_value, str) or not run_id_value:
|
||||
return
|
||||
run_id = run_id_value
|
||||
|
||||
triggering_user_id = mapping.get("triggering_user_id")
|
||||
if isinstance(triggering_user_id, str) and triggering_user_id and triggering_user_id != user_id:
|
||||
# Only the user who triggered the run may give feedback on it. Other
|
||||
# reactors are ignored to keep eval signal clean in shared channels.
|
||||
logger.debug(
|
||||
"Ignoring Slack reaction from non-triggering user=%s on run=%s",
|
||||
user_id,
|
||||
run_id,
|
||||
)
|
||||
return
|
||||
|
||||
active_reactions = await _update_reaction_state(
|
||||
langgraph_client,
|
||||
channel_id=channel_id,
|
||||
run_id=run_id,
|
||||
user_id=user_id,
|
||||
message_ts=message_ts,
|
||||
reaction=reaction,
|
||||
added=added,
|
||||
)
|
||||
|
||||
key = _feedback_key(channel_id, user_id, message_ts)
|
||||
source_info = {
|
||||
"source": "slack_reaction",
|
||||
"channel_id": channel_id,
|
||||
"message_ts": message_ts,
|
||||
"user_id": user_id,
|
||||
}
|
||||
score = _score_reactions(active_reactions)
|
||||
if score is None:
|
||||
success = await asyncio.to_thread(delete_langsmith_feedback, run_id, key)
|
||||
else:
|
||||
success = await asyncio.to_thread(
|
||||
create_langsmith_feedback,
|
||||
run_id,
|
||||
key,
|
||||
score=score,
|
||||
comment=f"Slack reaction feedback from user {user_id}",
|
||||
source_info={**source_info, "reactions": sorted(active_reactions)},
|
||||
)
|
||||
|
||||
if success:
|
||||
await _mark_event_processed(langgraph_client, channel_id, event_id)
|
||||
|
||||
|
||||
async def process_slack_reaction_added(event: dict[str, Any], event_id: str = "") -> None:
|
||||
await process_slack_reaction(event, event_id=event_id, added=True)
|
||||
|
||||
|
||||
async def process_slack_reaction_removed(event: dict[str, Any], event_id: str = "") -> None:
|
||||
await process_slack_reaction(event, event_id=event_id, added=False)
|
||||
|
|
@ -66,9 +66,15 @@ from .utils.slack import (
|
|||
resolve_slack_links_in_context,
|
||||
select_slack_context_messages,
|
||||
set_slack_assistant_status,
|
||||
store_slack_run_mapping,
|
||||
strip_bot_mention,
|
||||
verify_slack_signature,
|
||||
)
|
||||
from .utils.slack_feedback import (
|
||||
FEEDBACK_REACTIONS,
|
||||
process_slack_reaction_added,
|
||||
process_slack_reaction_removed,
|
||||
)
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
|
@ -974,14 +980,32 @@ async def process_slack_mention(event_data: dict[str, Any], repo_config: dict[st
|
|||
_run_id_for_logging(run),
|
||||
thread_id,
|
||||
)
|
||||
run_id = run.get("run_id")
|
||||
if is_first_mention:
|
||||
await post_slack_trace_reply(channel_id, thread_ts, thread_id)
|
||||
trace_message_ts = await post_slack_trace_reply(channel_id, thread_ts, thread_id)
|
||||
await set_slack_assistant_status(channel_id, thread_ts)
|
||||
if isinstance(run_id, str) and run_id:
|
||||
await store_slack_run_mapping(
|
||||
langgraph_client,
|
||||
channel_id,
|
||||
thread_ts,
|
||||
run_id,
|
||||
message_ts=trace_message_ts,
|
||||
triggering_user_id=user_id,
|
||||
)
|
||||
else:
|
||||
logger.info(
|
||||
"Skipping Slack trace reply for thread %s — agent will reply when run completes",
|
||||
thread_id,
|
||||
)
|
||||
if isinstance(run_id, str) and run_id:
|
||||
await store_slack_run_mapping(
|
||||
langgraph_client,
|
||||
channel_id,
|
||||
thread_ts,
|
||||
run_id,
|
||||
triggering_user_id=user_id,
|
||||
)
|
||||
|
||||
|
||||
async def process_slack_pr_review_request(
|
||||
|
|
@ -1195,6 +1219,25 @@ async def slack_webhook(request: Request, background_tasks: BackgroundTasks) ->
|
|||
return {"status": "ignored", "reason": "Not an event callback"}
|
||||
|
||||
event = payload.get("event", {})
|
||||
|
||||
if event.get("type") == "reaction_added":
|
||||
reaction = event.get("reaction")
|
||||
if reaction in FEEDBACK_REACTIONS:
|
||||
background_tasks.add_task(
|
||||
process_slack_reaction_added, event, payload.get("event_id", "")
|
||||
)
|
||||
return {"status": "accepted", "message": "Reaction feedback queued"}
|
||||
return {"status": "ignored", "reason": "Reaction not tracked for feedback"}
|
||||
|
||||
if event.get("type") == "reaction_removed":
|
||||
reaction = event.get("reaction")
|
||||
if reaction in FEEDBACK_REACTIONS:
|
||||
background_tasks.add_task(
|
||||
process_slack_reaction_removed, event, payload.get("event_id", "")
|
||||
)
|
||||
return {"status": "accepted", "message": "Reaction removal queued"}
|
||||
return {"status": "ignored", "reason": "Reaction not tracked for feedback"}
|
||||
|
||||
if event.get("type") != "app_mention":
|
||||
message_text = event.get("text", "")
|
||||
has_username_mention = bool(
|
||||
|
|
|
|||
|
|
@ -11,7 +11,7 @@ from agent.utils import slack as slack_utils
|
|||
|
||||
def _ok_response() -> MagicMock:
|
||||
response = MagicMock()
|
||||
response.json.return_value = {"ok": True}
|
||||
response.json.return_value = {"ok": True, "ts": "1.0"}
|
||||
response.raise_for_status.return_value = None
|
||||
return response
|
||||
|
||||
|
|
|
|||
|
|
@ -227,10 +227,15 @@ def test_post_slack_trace_reply_picks_random_phrase_when_no_message(
|
|||
) -> None:
|
||||
posted: list[str] = []
|
||||
|
||||
async def fake_post_slack_thread_reply(channel_id: str, thread_ts: str, text: str) -> None:
|
||||
async def fake_post_slack_thread_reply_with_ts(
|
||||
channel_id: str, thread_ts: str, text: str
|
||||
) -> str | None:
|
||||
posted.append(text)
|
||||
return "1.1"
|
||||
|
||||
monkeypatch.setattr(slack_utils, "post_slack_thread_reply", fake_post_slack_thread_reply)
|
||||
monkeypatch.setattr(
|
||||
slack_utils, "post_slack_thread_reply_with_ts", fake_post_slack_thread_reply_with_ts
|
||||
)
|
||||
monkeypatch.setattr(slack_utils, "get_langsmith_trace_url", lambda thread_id: None)
|
||||
|
||||
asyncio.run(post_slack_trace_reply("C123", "1.0", "thread-id"))
|
||||
|
|
@ -247,10 +252,15 @@ def test_post_slack_trace_reply_uses_explicit_message_when_provided(
|
|||
) -> None:
|
||||
posted: list[str] = []
|
||||
|
||||
async def fake_post_slack_thread_reply(channel_id: str, thread_ts: str, text: str) -> None:
|
||||
async def fake_post_slack_thread_reply_with_ts(
|
||||
channel_id: str, thread_ts: str, text: str
|
||||
) -> str | None:
|
||||
posted.append(text)
|
||||
return "1.1"
|
||||
|
||||
monkeypatch.setattr(slack_utils, "post_slack_thread_reply", fake_post_slack_thread_reply)
|
||||
monkeypatch.setattr(
|
||||
slack_utils, "post_slack_thread_reply_with_ts", fake_post_slack_thread_reply_with_ts
|
||||
)
|
||||
monkeypatch.setattr(slack_utils, "get_langsmith_trace_url", lambda thread_id: None)
|
||||
|
||||
asyncio.run(post_slack_trace_reply("C123", "1.0", "thread-id", message="Taking a look..."))
|
||||
|
|
|
|||
257
tests/test_slack_feedback.py
Normal file
257
tests/test_slack_feedback.py
Normal file
|
|
@ -0,0 +1,257 @@
|
|||
import json
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
|
||||
from agent import webapp
|
||||
from agent.utils import slack_feedback
|
||||
from agent.utils.slack_feedback import (
|
||||
process_slack_reaction_added,
|
||||
process_slack_reaction_removed,
|
||||
)
|
||||
|
||||
|
||||
class _FakeStore:
|
||||
def __init__(self) -> None:
|
||||
self.items: dict[tuple[tuple[str, ...], str], dict[str, Any]] = {}
|
||||
|
||||
async def get_item(self, namespace: tuple[str, ...], key: str) -> dict[str, Any] | None:
|
||||
return self.items.get((namespace, key))
|
||||
|
||||
async def put_item(self, namespace: tuple[str, ...], key: str, value: dict[str, Any]) -> None:
|
||||
self.items[(namespace, key)] = {"value": value}
|
||||
|
||||
|
||||
class _FakeClient:
|
||||
def __init__(self) -> None:
|
||||
self.store = _FakeStore()
|
||||
|
||||
|
||||
class _FakeBackgroundTasks:
|
||||
def __init__(self) -> None:
|
||||
self.tasks: list[tuple[Any, tuple[Any, ...]]] = []
|
||||
|
||||
def add_task(self, func: Any, *args: Any) -> None:
|
||||
self.tasks.append((func, args))
|
||||
|
||||
|
||||
class _FakeRequest:
|
||||
def __init__(self, payload: dict[str, Any]) -> None:
|
||||
self.headers: dict[str, str] = {}
|
||||
self._body = json.dumps(payload).encode()
|
||||
|
||||
async def body(self) -> bytes:
|
||||
return self._body
|
||||
|
||||
|
||||
def _store_message_mapping(
|
||||
client: _FakeClient,
|
||||
channel_id: str,
|
||||
message_ts: str,
|
||||
*,
|
||||
triggering_user_id: str | None = "U123",
|
||||
) -> None:
|
||||
value: dict[str, Any] = {"run_id": "run-1", "thread_ts": "1.000"}
|
||||
if triggering_user_id:
|
||||
value["triggering_user_id"] = triggering_user_id
|
||||
client.store.items[(("slack_run_map", channel_id), f"message:{message_ts}")] = {"value": value}
|
||||
|
||||
|
||||
def _reaction_event(reaction: str = "thumbsup") -> dict[str, Any]:
|
||||
return {
|
||||
"type": "reaction_added",
|
||||
"reaction": reaction,
|
||||
"user": "U123",
|
||||
"item": {"type": "message", "channel": "C123", "ts": "2.000"},
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reaction_added_creates_feedback(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
client = _FakeClient()
|
||||
_store_message_mapping(client, "C123", "2.000")
|
||||
created: dict[str, Any] = {}
|
||||
|
||||
def fake_create_feedback(
|
||||
run_id: str,
|
||||
key: str,
|
||||
*,
|
||||
score: float,
|
||||
comment: str | None = None,
|
||||
source_info: dict[str, Any] | None = None,
|
||||
) -> bool:
|
||||
created.update(
|
||||
{
|
||||
"run_id": run_id,
|
||||
"key": key,
|
||||
"score": score,
|
||||
"comment": comment,
|
||||
"source_info": source_info,
|
||||
}
|
||||
)
|
||||
return True
|
||||
|
||||
monkeypatch.setattr(slack_feedback, "get_client", lambda url: client)
|
||||
monkeypatch.setattr(slack_feedback, "create_langsmith_feedback", fake_create_feedback)
|
||||
|
||||
await process_slack_reaction_added(_reaction_event(), event_id="Ev1")
|
||||
|
||||
assert created["run_id"] == "run-1"
|
||||
assert created["key"] == "slack_reaction:C123:U123:2.000"
|
||||
assert created["score"] == 1.0
|
||||
assert created["source_info"]["reactions"] == ["thumbsup"]
|
||||
assert (("slack_reaction_events", "C123"), "Ev1") in client.store.items
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reaction_added_skips_duplicate_event(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
client = _FakeClient()
|
||||
_store_message_mapping(client, "C123", "2.000")
|
||||
client.store.items[(("slack_reaction_events", "C123"), "Ev1")] = {"value": {"event_id": "Ev1"}}
|
||||
|
||||
def fail_create_feedback(*args: Any, **kwargs: Any) -> bool:
|
||||
raise AssertionError("duplicate event should not create feedback")
|
||||
|
||||
monkeypatch.setattr(slack_feedback, "get_client", lambda url: client)
|
||||
monkeypatch.setattr(slack_feedback, "create_langsmith_feedback", fail_create_feedback)
|
||||
|
||||
await process_slack_reaction_added(_reaction_event(), event_id="Ev1")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reaction_removed_deletes_feedback_when_last_reaction_removed(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
client = _FakeClient()
|
||||
_store_message_mapping(client, "C123", "2.000")
|
||||
client.store.items[(("slack_reaction_state", "C123"), "run-1:U123:2.000")] = {
|
||||
"value": {
|
||||
"run_id": "run-1",
|
||||
"user_id": "U123",
|
||||
"message_ts": "2.000",
|
||||
"reactions": ["thumbsup"],
|
||||
}
|
||||
}
|
||||
deleted: dict[str, str] = {}
|
||||
|
||||
def fake_delete_feedback(run_id: str, key: str) -> bool:
|
||||
deleted["run_id"] = run_id
|
||||
deleted["key"] = key
|
||||
return True
|
||||
|
||||
monkeypatch.setattr(slack_feedback, "get_client", lambda url: client)
|
||||
monkeypatch.setattr(slack_feedback, "delete_langsmith_feedback", fake_delete_feedback)
|
||||
|
||||
await process_slack_reaction_removed(_reaction_event(), event_id="Ev2")
|
||||
|
||||
assert deleted == {"run_id": "run-1", "key": "slack_reaction:C123:U123:2.000"}
|
||||
state = client.store.items[(("slack_reaction_state", "C123"), "run-1:U123:2.000")]
|
||||
assert state["value"]["reactions"] == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reaction_without_message_mapping_is_ignored(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
client = _FakeClient()
|
||||
|
||||
def fail_create_feedback(*args: Any, **kwargs: Any) -> bool:
|
||||
raise AssertionError("unmapped message should not create feedback")
|
||||
|
||||
monkeypatch.setattr(slack_feedback, "get_client", lambda url: client)
|
||||
monkeypatch.setattr(slack_feedback, "create_langsmith_feedback", fail_create_feedback)
|
||||
|
||||
await process_slack_reaction_added(_reaction_event(), event_id="Ev1")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reaction_from_non_triggering_user_is_ignored(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
client = _FakeClient()
|
||||
_store_message_mapping(client, "C123", "2.000", triggering_user_id="UTRIGGER")
|
||||
|
||||
def fail_create_feedback(*args: Any, **kwargs: Any) -> bool:
|
||||
raise AssertionError("non-triggering user should not create feedback")
|
||||
|
||||
monkeypatch.setattr(slack_feedback, "get_client", lambda url: client)
|
||||
monkeypatch.setattr(slack_feedback, "create_langsmith_feedback", fail_create_feedback)
|
||||
|
||||
await process_slack_reaction_added(_reaction_event(), event_id="Ev1")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_conflicting_reactions_clear_feedback(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
client = _FakeClient()
|
||||
_store_message_mapping(client, "C123", "2.000")
|
||||
client.store.items[(("slack_reaction_state", "C123"), "run-1:U123:2.000")] = {
|
||||
"value": {
|
||||
"run_id": "run-1",
|
||||
"user_id": "U123",
|
||||
"message_ts": "2.000",
|
||||
"reactions": ["thumbsup"],
|
||||
}
|
||||
}
|
||||
deleted: dict[str, str] = {}
|
||||
|
||||
def fail_create_feedback(*args: Any, **kwargs: Any) -> bool:
|
||||
raise AssertionError("conflicting reactions must not record a numeric score")
|
||||
|
||||
def fake_delete_feedback(run_id: str, key: str) -> bool:
|
||||
deleted["run_id"] = run_id
|
||||
deleted["key"] = key
|
||||
return True
|
||||
|
||||
monkeypatch.setattr(slack_feedback, "get_client", lambda url: client)
|
||||
monkeypatch.setattr(slack_feedback, "create_langsmith_feedback", fail_create_feedback)
|
||||
monkeypatch.setattr(slack_feedback, "delete_langsmith_feedback", fake_delete_feedback)
|
||||
|
||||
# User adds a thumbsdown alongside the existing thumbsup → conflicting.
|
||||
event = {**_reaction_event("thumbsdown"), "type": "reaction_added"}
|
||||
await process_slack_reaction_added(event, event_id="EvConflict")
|
||||
|
||||
assert deleted == {"run_id": "run-1", "key": "slack_reaction:C123:U123:2.000"}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_slack_webhook_queues_reaction_added(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
event = _reaction_event("+1")
|
||||
payload = {"type": "event_callback", "event_id": "Ev1", "event": event}
|
||||
background_tasks = _FakeBackgroundTasks()
|
||||
|
||||
monkeypatch.setattr(webapp, "verify_slack_signature", lambda **kwargs: True)
|
||||
|
||||
response = await webapp.slack_webhook(_FakeRequest(payload), background_tasks)
|
||||
|
||||
assert response == {"status": "accepted", "message": "Reaction feedback queued"}
|
||||
assert background_tasks.tasks == [(webapp.process_slack_reaction_added, (event, "Ev1"))]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_slack_webhook_queues_reaction_removed(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
event = {**_reaction_event("-1"), "type": "reaction_removed"}
|
||||
payload = {"type": "event_callback", "event_id": "Ev2", "event": event}
|
||||
background_tasks = _FakeBackgroundTasks()
|
||||
|
||||
monkeypatch.setattr(webapp, "verify_slack_signature", lambda **kwargs: True)
|
||||
|
||||
response = await webapp.slack_webhook(_FakeRequest(payload), background_tasks)
|
||||
|
||||
assert response == {"status": "accepted", "message": "Reaction removal queued"}
|
||||
assert background_tasks.tasks == [(webapp.process_slack_reaction_removed, (event, "Ev2"))]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_slack_webhook_ignores_untracked_reaction(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
event = _reaction_event("eyes")
|
||||
payload = {"type": "event_callback", "event_id": "Ev3", "event": event}
|
||||
background_tasks = _FakeBackgroundTasks()
|
||||
|
||||
monkeypatch.setattr(webapp, "verify_slack_signature", lambda **kwargs: True)
|
||||
|
||||
response = await webapp.slack_webhook(_FakeRequest(payload), background_tasks)
|
||||
|
||||
assert response == {"status": "ignored", "reason": "Reaction not tracked for feedback"}
|
||||
assert background_tasks.tasks == []
|
||||
Loading…
Add table
Reference in a new issue