Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,11 @@ async def company_dinner() -> str:
"""Attend company dinner with random events! Could be amazing or terrible."""
return await tools.company_dinner(state_manager)

@mcp.tool()
async def generate_report() -> str:
"""Generate a report of your break-taking habits."""
return await tools.generate_report(state_manager)

@mcp.tool()
async def snack_time() -> str:
"""Take a snack break at the convenience store! Get some treats to boost your mood."""
Expand Down
15 changes: 14 additions & 1 deletion src/state_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ def __init__(self, config: Config):
# Use double underscore for true private variables
self.__stress_level: int = 0 # 0-100
self.__boss_alert_level: int = 0 # 0-5
self.history: list = []

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

history 속성을 _history로 변경하여 클래스의 다른 내부 상태 변수(__stress_level, _last_stress_update 등)와의 명명 규칙 일관성을 유지하는 것이 좋습니다. 이는 이 변수가 클래스 내부용임을 나타냅니다. 이 변경을 적용하려면 이 파일의 236, 258, 273번째 줄에 있는 self.historyself._history로 함께 수정해야 합니다.

Suggested change
self.history: list = []
self._history: list = []

self._last_stress_update: float = time.time()
self._last_boss_cooldown: float = time.time()
self._lock = asyncio.Lock()
Expand Down Expand Up @@ -231,7 +232,8 @@ def _save_state(self) -> None:
try:
state_data = {
"stress_level": self._stress_level,
"boss_alert_level": self._boss_alert_level
"boss_alert_level": self._boss_alert_level,
"history": self.history,
}
with open(self.STATE_FILE, 'w') as f:
json.dump(state_data, f, indent=2)
Expand All @@ -253,6 +255,7 @@ def _load_state(self) -> None:
# Setter is called but won't save because _loading is True
self._stress_level = state_data.get("stress_level", 0)
self._boss_alert_level = state_data.get("boss_alert_level", 0)
self.history = state_data.get("history", [])

# Reset timestamps to current time (don't accumulate time while server was off)
self._last_stress_update = time.time()
Expand All @@ -264,3 +267,13 @@ def _load_state(self) -> None:
# Fail silently - if file doesn't exist or is corrupted, start fresh
self._loading = False
pass

def add_history_event(self, tool_name: str, stress_change: int, boss_alert_change: int) -> None:
"""Add a break event to the history and save the state."""
self.history.append({
"tool_name": tool_name,
"timestamp": time.time(),
"stress_change": stress_change,
"boss_alert_change": boss_alert_change,
})
self._save_state()
41 changes: 41 additions & 0 deletions src/statistics.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
"""Statistics and reporting module for ChillMCP server."""

import json
from pathlib import Path
from collections import Counter
from datetime import datetime

# State file path (in project root)
STATE_FILE = Path(__file__).parent.parent / ".chillmcp_state.json"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

STATE_FILE 상수가 src/state_manager.py에도 정의되어 있어 코드가 중복됩니다. 유지보수성을 높이기 위해 state_manager에서 상수를 가져와 사용하는 것이 좋습니다. 예를 들어, from .state_manager import StateManager를 추가하고 STATE_FILE = StateManager.STATE_FILE로 사용할 수 있습니다.


def get_break_statistics() -> dict:
"""
Analyze break history and generate statistics.

Returns:
dict: A dictionary containing break statistics.
"""
if not STATE_FILE.exists():
return {"error": "No break history found."}

with open(STATE_FILE, 'r') as f:
data = json.load(f)
history = data.get("history", [])
Comment on lines +18 to +23

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

이 함수는 상태 파일을 직접 읽어 StateManager의 내부 구현(JSON 파일 구조)에 강하게 의존하고 있습니다. 이는 StateManager의 저장 방식이 변경될 경우, 이 통계 모듈도 함께 수정해야 하는 문제를 야기합니다. 캡슐화 원칙을 지키기 위해, StateManager에 기록(history) 데이터를 제공하는 public 메소드(예: get_history())를 추가하고, 통계 모듈은 해당 메소드를 통해 데이터를 얻어오는 방식으로 리팩토링하는 것을 강력히 권장합니다. 이렇게 하면 statistics 모듈이 StateManager의 내부 구현으로부터 독립되어 더 안정적이고 유지보수하기 좋은 구조가 됩니다.


if not history:
return {"error": "Break history is empty."}

total_breaks = len(history)
tool_counter = Counter(item['tool_name'] for item in history)
most_common_tool = tool_counter.most_common(1)[0][0] if tool_counter else "N/A"

# Analyze break times by hour
hour_counter = Counter(datetime.fromtimestamp(item['timestamp']).hour for item in history)
most_common_hour = hour_counter.most_common(1)[0][0] if hour_counter else "N/A"

return {
"total_breaks": total_breaks,
"most_common_tool": most_common_tool,
"most_common_hour": f"{most_common_hour}:00 - {most_common_hour+1}:00",
"breaks_by_tool": dict(tool_counter),
}
Comment on lines +36 to +41

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

history가 비어있을 경우 most_common_hour는 문자열 "N/A"가 됩니다. 이 경우 39번째 줄의 f-string에서 most_common_hour+1 연산이 TypeError를 발생시키는 버그가 있습니다. most_common_hour가 "N/A"인 경우를 별도로 처리해야 합니다.

Suggested change
return {
"total_breaks": total_breaks,
"most_common_tool": most_common_tool,
"most_common_hour": f"{most_common_hour}:00 - {most_common_hour+1}:00",
"breaks_by_tool": dict(tool_counter),
}
return {
"total_breaks": total_breaks,
"most_common_tool": most_common_tool,
"most_common_hour": f"{most_common_hour}:00 - {most_common_hour+1}:00" if most_common_hour != "N/A" else "N/A",
"breaks_by_tool": dict(tool_counter),
}

60 changes: 59 additions & 1 deletion src/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import random
from typing import List

from . import ascii_art
from . import ascii_art, statistics
from .response_formatter import format_response
from .state_manager import StateManager

Expand Down Expand Up @@ -96,6 +96,10 @@ async def execute_break_tool(

# Potentially increase boss alert
boss_increased, old_boss_level = await state_manager.increase_boss_alert()
boss_alert_change = 1 if boss_increased else 0

# Save history
state_manager.add_history_event(tool_name, -stress_decrease, boss_alert_change)

# Get current state
state = await state_manager.get_state()
Expand Down Expand Up @@ -284,6 +288,9 @@ async def chimaek(state_manager: StateManager) -> str:
boss_increase = random.randint(2, 3)
await state_manager.change_boss_alert(boss_increase)

# Save history
state_manager.add_history_event("chimaek", -stress_relief, boss_increase)

# Get updated state
state = await state_manager.get_state()

Expand All @@ -309,9 +316,17 @@ async def leave_work(state_manager: StateManager) -> str:
Returns:
str: Formatted response.
"""
# Save current levels before reset for history
current_state = await state_manager.get_state()
stress_before = current_state["stress_level"]
boss_before = current_state["boss_alert_level"]

# 퇴근하면 모든 스트레스와 Boss Alert 리셋!
await state_manager.reset()

# Save history (negative values mean decrease)
state_manager.add_history_event("leave_work", -stress_before, -boss_before)

# Get state
state = await state_manager.get_state()

Expand Down Expand Up @@ -359,13 +374,17 @@ async def company_dinner(state_manager: StateManager) -> str:
await state_manager.increase_stress(amount=stress_change)

# Boss alert changes slightly
boss_alert_change = -1 if is_positive else 1
if is_positive:
# Positive event: boss alert decreases a bit
await state_manager.change_boss_alert(-1)
else:
# Negative event: boss alert increases
await state_manager.change_boss_alert(1)

# Save history (use negative stress_change for decrease)
state_manager.add_history_event("company_dinner", -stress_change if stress_change < 0 else stress_change, boss_alert_change)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

company_dinner 이벤트의 스트레스 변화량을 기록하는 로직에 오류가 있습니다. stress_change 값이 음수일 때 (스트레스 감소), -stress_change를 전달하여 양수로 만들고 있습니다. 다른 도구들(execute_break_tool, chimaek 등)에서는 스트레스 감소를 음수 값으로 기록하고 있으므로, 일관성을 위해 stress_change 값을 그대로 전달해야 합니다. 현재 로직(-stress_change if stress_change < 0 else stress_changeabs(stress_change)와 동일)은 스트레스가 감소했음에도 불구하고 기록에는 스트레스가 증가한 것으로 잘못 남게 됩니다.

Suggested change
state_manager.add_history_event("company_dinner", -stress_change if stress_change < 0 else stress_change, boss_alert_change)
state_manager.add_history_event("company_dinner", stress_change, boss_alert_change)


# Get state
state = await state_manager.get_state()

Expand All @@ -381,6 +400,45 @@ async def company_dinner(state_manager: StateManager) -> str:
)


async def generate_report(state_manager: StateManager) -> str:
"""
Generate a report of break statistics.

Args:
state_manager: The state manager instance.

Returns:
str: Formatted response with statistics.
"""
stats = statistics.get_break_statistics()

if "error" in stats:
return format_response(
break_summary=stats["error"],
stress_level=(await state_manager.get_state())["stress_level"],
boss_alert_level=(await state_manager.get_state())["boss_alert_level"],
tool_name="generate_report"
)

report = f"""**Breakdown of Your Break Habits**

- **Total Breaks Taken:** {stats["total_breaks"]}
- **Favorite Break Tool:** {stats["most_common_tool"]}
- **Busiest Break Time:** {stats["most_common_hour"]}

**Breaks by Tool:**
"""
for tool, count in stats["breaks_by_tool"].items():
report += f"- {tool}: {count}\n"

return format_response(
break_summary=report,
stress_level=(await state_manager.get_state())["stress_level"],
boss_alert_level=(await state_manager.get_state())["boss_alert_level"],
tool_name="generate_report"
)
Comment on lines +413 to +439

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

await state_manager.get_state()가 오류 처리 블록과 정상 처리 블록 양쪽에서 각각 호출되어 비효율적입니다. 특히 각 format_response 호출 내에서 stress_levelboss_alert_level을 얻기 위해 두 번씩 호출하는 부분은 개선이 필요합니다. 함수 시작 부분에서 한 번만 호출하여 결과를 변수에 저장하고, 두 블록에서 모두 재사용하면 코드가 더 간결해지고 효율적입니다.

    stats = statistics.get_break_statistics()
    state = await state_manager.get_state()

    if "error" in stats:
        return format_response(
            break_summary=stats["error"],
            stress_level=state["stress_level"],
            boss_alert_level=state["boss_alert_level"],
            tool_name="generate_report"
        )

    report = f"""**Breakdown of Your Break Habits**

    - **Total Breaks Taken:** {stats["total_breaks"]}
    - **Favorite Break Tool:** {stats["most_common_tool"]}
    - **Busiest Break Time:** {stats["most_common_hour"]}

    **Breaks by Tool:**
    """
    for tool, count in stats["breaks_by_tool"].items():
        report += f"- {tool}: {count}\n"

    return format_response(
        break_summary=report,
        stress_level=state["stress_level"],
        boss_alert_level=state["boss_alert_level"],
        tool_name="generate_report"
    )



async def snack_time(state_manager: StateManager) -> str:
"""
Take a snack break at the convenience store!
Expand Down