Skip to content

Commit 866cf52

Browse files
anthony-nhsanthony-nhs
committed
another refactor
1 parent 62e20e4 commit 866cf52

File tree

4 files changed

+73
-68
lines changed

4 files changed

+73
-68
lines changed

‎packages/slackBotFunction/app/core/config.py‎

Lines changed: 37 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
Sets up all the AWS and Slack connections we need.
44
"""
55

6+
from dataclasses import dataclass
67
from functools import lru_cache
78
import os
89
import json
@@ -56,24 +57,42 @@ def get_ssm_params():
5657
return bot_token, signing_secret
5758

5859

59-
# Constants
60-
FEEDBACK_PREFIX = "feedback:"
61-
CONTEXT_TYPE_DM = "DM"
62-
CONTEXT_TYPE_THREAD = "thread"
63-
CHANNEL_TYPE_IM = "im"
64-
SESSION_SK = "session"
65-
DEDUP_SK = "dedup"
66-
EVENT_PREFIX = "event#"
67-
FEEDBACK_PREFIX_KEY = "feedback#"
68-
USER_PREFIX = "user#"
69-
DM_PREFIX = "dm#"
70-
THREAD_PREFIX = "thread#"
71-
NOTE_SUFFIX = "#note#"
72-
73-
# TTL constants (in seconds)
74-
TTL_EVENT_DEDUP = 3600 # 1 hour
75-
TTL_FEEDBACK = 7776000 # 90 days
76-
TTL_SESSION = 2592000 # 30 days
60+
@dataclass
61+
class Constants:
62+
FEEDBACK_PREFIX: str
63+
CONTEXT_TYPE_DM: str
64+
CONTEXT_TYPE_THREAD: str
65+
CHANNEL_TYPE_IM: str
66+
SESSION_SK: str
67+
DEDUP_SK: str
68+
EVENT_PREFIX: str
69+
FEEDBACK_PREFIX_KEY: str
70+
USER_PREFIX: str
71+
DM_PREFIX: str
72+
THREAD_PREFIX: str
73+
NOTE_SUFFIX: str
74+
TTL_EVENT_DEDUP: int
75+
TTL_FEEDBACK: int
76+
TTL_SESSION: int
77+
78+
79+
constants = Constants(
80+
FEEDBACK_PREFIX="feedback:",
81+
CONTEXT_TYPE_DM="DM",
82+
CONTEXT_TYPE_THREAD="thread",
83+
CHANNEL_TYPE_IM="im",
84+
SESSION_SK="session",
85+
DEDUP_SK="dedup",
86+
EVENT_PREFIX="event#",
87+
FEEDBACK_PREFIX_KEY="feedback#",
88+
USER_PREFIX="user#",
89+
DM_PREFIX="dm#",
90+
THREAD_PREFIX="thread#",
91+
NOTE_SUFFIX="#note#",
92+
TTL_EVENT_DEDUP=3600, # 1 hour
93+
TTL_FEEDBACK=7776000, # 90 days
94+
TTL_SESSION=2592000, # 30 days
95+
)
7796

7897

7998
@lru_cache

‎packages/slackBotFunction/app/slack/slack_events.py‎

Lines changed: 17 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,7 @@
1010
from botocore.exceptions import ClientError
1111
from slack_sdk import WebClient
1212
from app.core.config import (
13-
CONTEXT_TYPE_DM,
14-
CONTEXT_TYPE_THREAD,
15-
CHANNEL_TYPE_IM,
16-
SESSION_SK,
17-
FEEDBACK_PREFIX_KEY,
18-
USER_PREFIX,
19-
DM_PREFIX,
20-
THREAD_PREFIX,
21-
NOTE_SUFFIX,
22-
TTL_FEEDBACK,
23-
TTL_SESSION,
13+
constants,
2414
get_bot_messages,
2515
get_logger,
2616
)
@@ -78,7 +68,7 @@ def store_qa_pair(conversation_key, user_query, bot_response, message_ts, sessio
7868
"user_id": user_id,
7969
"message_ts": message_ts,
8070
"created_at": int(time.time()),
81-
"ttl": int(time.time()) + TTL_FEEDBACK,
71+
"ttl": int(time.time()) + constants.TTL_FEEDBACK,
8272
}
8373
store_state_information(item=item)
8474
logger.info("Stored Q&A pair", extra={"conversation_key": conversation_key, "message_ts": message_ts})
@@ -109,11 +99,11 @@ def _extract_conversation_context(event):
10999
"""Extract conversation key and thread context from event"""
110100
channel = event["channel"]
111101
# Determine conversation context: DM vs channel thread
112-
if event.get("channel_type") == CHANNEL_TYPE_IM:
113-
return f"{DM_PREFIX}{channel}", CONTEXT_TYPE_DM, None # DMs don't use threads
102+
if event.get("channel_type") == constants.CHANNEL_TYPE_IM:
103+
return f"{constants.DM_PREFIX}{channel}", constants.CONTEXT_TYPE_DM, None # DMs don't use threads
114104
else:
115105
thread_root = event.get("thread_ts", event["ts"])
116-
return f"{THREAD_PREFIX}{channel}#{thread_root}", CONTEXT_TYPE_THREAD, thread_root
106+
return f"{constants.THREAD_PREFIX}{channel}#{thread_root}", constants.CONTEXT_TYPE_THREAD, thread_root
117107

118108

119109
def _handle_session_management(
@@ -128,7 +118,7 @@ def _handle_session_management(
128118
kb_response["sessionId"],
129119
user_id,
130120
channel,
131-
thread_ts if context_type == CONTEXT_TYPE_THREAD else None,
121+
thread_ts if context_type == constants.CONTEXT_TYPE_THREAD else None,
132122
message_ts,
133123
)
134124
elif session_id:
@@ -283,26 +273,26 @@ def store_feedback(
283273
"""
284274
try:
285275
now = int(time.time())
286-
ttl = now + TTL_FEEDBACK
276+
ttl = now + constants.TTL_FEEDBACK
287277

288278
# Get latest bot message timestamp for feedback linking
289279
if not message_ts:
290280
message_ts = get_latest_message_ts(conversation_key)
291281

292282
if message_ts and feedback_type in ["positive", "negative"]:
293283
# Per-message feedback with deduplication for button votes only
294-
pk = f"{FEEDBACK_PREFIX_KEY}{conversation_key}#{message_ts}"
295-
sk = f"{USER_PREFIX}{user_id}"
284+
pk = f"{constants.FEEDBACK_PREFIX_KEY}{conversation_key}#{message_ts}"
285+
sk = f"{constants.USER_PREFIX}{user_id}"
296286
condition = "attribute_not_exists(pk) AND attribute_not_exists(sk)" # Prevent double-voting
297287
elif message_ts:
298288
# Text feedback allows multiple entries per user
299-
pk = f"{FEEDBACK_PREFIX_KEY}{conversation_key}#{message_ts}"
300-
sk = f"{USER_PREFIX}{user_id}{NOTE_SUFFIX}{now}"
289+
pk = f"{constants.FEEDBACK_PREFIX_KEY}{conversation_key}#{message_ts}"
290+
sk = f"{constants.USER_PREFIX}{user_id}{constants.NOTE_SUFFIX}{now}"
301291
condition = None
302292
else:
303293
# Fallback for conversation-level feedback
304-
pk = f"{FEEDBACK_PREFIX_KEY}{conversation_key}"
305-
sk = f"{USER_PREFIX}{user_id}{NOTE_SUFFIX}{now}"
294+
pk = f"{constants.FEEDBACK_PREFIX_KEY}{conversation_key}"
295+
sk = f"{constants.USER_PREFIX}{user_id}{constants.NOTE_SUFFIX}{now}"
306296
condition = None
307297

308298
feedback_item = {
@@ -381,7 +371,7 @@ def get_latest_message_ts(conversation_key):
381371
Get latest message timestamp from session
382372
"""
383373
try:
384-
response = get_state_information({"pk": conversation_key, "sk": SESSION_SK})
374+
response = get_state_information({"pk": conversation_key, "sk": constants.SESSION_SK})
385375
if "Item" in response:
386376
return response["Item"].get("latest_message_ts")
387377
return None
@@ -397,10 +387,10 @@ def store_conversation_session(
397387
Store new Bedrock session for conversation memory
398388
"""
399389
try:
400-
ttl = int(time.time()) + TTL_SESSION
390+
ttl = int(time.time()) + constants.TTL_SESSION
401391
item = {
402392
"pk": conversation_key,
403-
"sk": SESSION_SK,
393+
"sk": constants.SESSION_SK,
404394
"session_id": session_id,
405395
"user_id": user_id,
406396
"channel_id": channel_id,
@@ -426,7 +416,7 @@ def update_session_latest_message(conversation_key, message_ts):
426416
"""
427417
try:
428418
update_state_information(
429-
{"pk": conversation_key, "sk": SESSION_SK},
419+
{"pk": conversation_key, "sk": constants.SESSION_SK},
430420
"SET latest_message_ts = :ts",
431421
{":ts": message_ts},
432422
)

‎packages/slackBotFunction/app/slack/slack_handlers.py‎

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,7 @@
1515
get_bot_messages,
1616
get_bot_token,
1717
get_logger,
18-
FEEDBACK_PREFIX,
19-
CHANNEL_TYPE_IM,
20-
SESSION_SK,
21-
DM_PREFIX,
22-
THREAD_PREFIX,
18+
constants,
2319
)
2420
from app.services.dynamo import get_state_information
2521
from app.utils.handler_utils import is_duplicate_event, trigger_async_processing, respond_with_eyes
@@ -76,9 +72,9 @@ def _conversation_key_and_root(event):
7672
"""
7773
channel_id = event["channel"]
7874
root = event.get("thread_ts") or event.get("ts")
79-
if event.get("channel_type") == CHANNEL_TYPE_IM:
80-
return f"{DM_PREFIX}{channel_id}", root
81-
return f"{THREAD_PREFIX}{channel_id}#{root}", root
75+
if event.get("channel_type") == constants.CHANNEL_TYPE_IM:
76+
return f"{constants.DM_PREFIX}{channel_id}", root
77+
return f"{constants.THREAD_PREFIX}{channel_id}#{root}", root
8278

8379

8480
# ================================================================
@@ -103,7 +99,7 @@ def mention_handler(event, ack, body, client):
10399
conversation_key, thread_root = _conversation_key_and_root(event)
104100

105101
cleaned = _strip_mentions(event.get("text") or "")
106-
if cleaned.lower().startswith(FEEDBACK_PREFIX):
102+
if cleaned.lower().startswith(constants.FEEDBACK_PREFIX):
107103
feedback_text = cleaned.split(":", 1)[1].strip() if ":" in cleaned else ""
108104
try:
109105
store_feedback(
@@ -139,7 +135,7 @@ def dm_message_handler(event, event_id, client):
139135
- 'feedback:' prefix -> store as conversation-scoped additional feedback (no model call).
140136
- otherwise -> forward to async processing (Q&A).
141137
"""
142-
if event.get("channel_type") != CHANNEL_TYPE_IM:
138+
if event.get("channel_type") != constants.CHANNEL_TYPE_IM:
143139
return # not a DM; the channel handler will evaluate it
144140
bot_token = get_bot_token()
145141
respond_with_eyes(bot_token, event)
@@ -148,7 +144,7 @@ def dm_message_handler(event, event_id, client):
148144
conversation_key, thread_root = _conversation_key_and_root(event)
149145
user_id = event.get("user", "unknown")
150146

151-
if text.lower().startswith(FEEDBACK_PREFIX):
147+
if text.lower().startswith(constants.FEEDBACK_PREFIX):
152148
feedback_text = text.split(":", 1)[1].strip() if ":" in text else ""
153149
try:
154150
store_feedback(
@@ -186,7 +182,7 @@ def thread_message_handler(event, event_id, client):
186182
* 'feedback:' prefix -> store additional feedback.
187183
* otherwise -> treat as follow-up question (no re-mention needed) and forward to async.
188184
"""
189-
if event.get("channel_type") == CHANNEL_TYPE_IM:
185+
if event.get("channel_type") == constants.CHANNEL_TYPE_IM:
190186
return # handled in the DM handler
191187

192188
text = (event.get("text") or "").strip()
@@ -195,9 +191,9 @@ def thread_message_handler(event, event_id, client):
195191
if not thread_root:
196192
return # top-level message; require @mention to start
197193

198-
conversation_key = f"{THREAD_PREFIX}{channel_id}#{thread_root}"
194+
conversation_key = f"{constants.THREAD_PREFIX}{channel_id}#{thread_root}"
199195
try:
200-
resp = get_state_information({"pk": conversation_key, "sk": SESSION_SK})
196+
resp = get_state_information({"pk": conversation_key, "sk": constants.SESSION_SK})
201197
if "Item" not in resp:
202198
logger.info(f"No session found for thread: {conversation_key}")
203199
return # not a bot-owned thread; ignore
@@ -206,7 +202,7 @@ def thread_message_handler(event, event_id, client):
206202
logger.error(f"Error checking thread session: {e}")
207203
return
208204

209-
if text.lower().startswith(FEEDBACK_PREFIX):
205+
if text.lower().startswith(constants.FEEDBACK_PREFIX):
210206
feedback_text = text.split(":", 1)[1].strip() if ":" in text else ""
211207
user_id = event.get("user", "unknown")
212208
try:
@@ -247,7 +243,7 @@ def unified_message_handler(event, ack, body, client):
247243
return
248244

249245
# Route to appropriate handler based on message type
250-
if event.get("channel_type") == CHANNEL_TYPE_IM:
246+
if event.get("channel_type") == constants.CHANNEL_TYPE_IM:
251247
# DM handling
252248
dm_message_handler(event, event_id, client)
253249
else:
@@ -330,7 +326,7 @@ def setup_handlers(app):
330326
def _is_latest_message(conversation_key, message_ts):
331327
"""Check if message_ts is the latest bot message using session data"""
332328
try:
333-
response = get_state_information({"pk": conversation_key, "sk": SESSION_SK})
329+
response = get_state_information({"pk": conversation_key, "sk": constants.SESSION_SK})
334330
if "Item" in response:
335331
latest_message_ts = response["Item"].get("latest_message_ts")
336332
return latest_message_ts == message_ts

‎packages/slackBotFunction/tests/test_feedback.py‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ def test_store_feedback(mock_get_state_information, mock_store_state_information
1616
store_feedback("test-conversation", "positive", "U123", "C123")
1717

1818
mock_store_state_information.assert_called_once()
19-
call_args = mock_store_state_information.call_args[0][0]
19+
call_args = mock_store_state_information.call_args.kwargs["item"]
2020
assert call_args["feedback_type"] == "positive"
2121
assert call_args["user_id"] == "U123"
2222

@@ -32,7 +32,7 @@ def test_feedback_storage_with_additional_text(mock_get_state_information, mock_
3232
store_feedback("test-conversation", "additional", "U123", "C123", feedback_text="This is additional feedback")
3333

3434
mock_store_state_information.assert_called_once()
35-
call_args = mock_store_state_information.call_args[0][0]
35+
call_args = mock_store_state_information.call_args.kwargs["item"]
3636
assert call_args["feedback_type"] == "additional"
3737
assert call_args["user_id"] == "U123"
3838
assert call_args["feedback_text"] == "This is additional feedback"
@@ -157,9 +157,9 @@ def test_store_feedback_no_message_ts_fallback(mock_store_state_information, moc
157157
store_feedback("conv-key", "positive", "user-id", "channel-id")
158158
mock_store_state_information.assert_called_once()
159159
# Should use fallback pk/sk format without condition
160-
call_args = mock_store_state_information.call_args[0]
160+
call_args = mock_store_state_information.call_args.kwargs
161161
assert "ConditionExpression" not in call_args
162-
item = call_args[0]
162+
item = call_args["item"]
163163
assert item["pk"] == "feedback#conv-key"
164164
assert "#note#" in item["sk"]
165165

@@ -174,7 +174,7 @@ def test_store_conversation_session_with_thread(mock_get_state_information, mock
174174

175175
store_conversation_session("conv-key", "session-id", "user-id", "channel-id", "thread-123", "msg-456")
176176
mock_store_state_information.assert_called_once()
177-
item = mock_store_state_information.call_args[0][0]
177+
item = mock_store_state_information.call_args.kwargs["item"]
178178
assert item["thread_ts"] == "thread-123"
179179
assert item["latest_message_ts"] == "msg-456"
180180

@@ -189,7 +189,7 @@ def test_store_conversation_session_without_thread(mock_get_state_information, m
189189

190190
store_conversation_session("conv-key", "session-id", "user-id", "channel-id")
191191
mock_store_state_information.assert_called_once()
192-
item = mock_store_state_information.call_args[0][0]
192+
item = mock_store_state_information.call_args.kwargs["item"]
193193
assert "thread_ts" not in item
194194
assert "latest_message_ts" not in item
195195

0 commit comments

Comments
 (0)