예산 임계치 기반 판단 로직 및 알림 중복 방지 기능 추가#18
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a budget notification system that triggers alerts at 50%, 80%, and 100% usage thresholds. It includes the new BudgetThreshold enum, a BudgetNotificationService to manage alert frequency, and updates to DefaultBudgetEvaluator to support these new levels. Feedback focuses on improving the robustness of the notification logic, specifically by including budget cycle information in the state store keys to prevent state carry-over between months. Additionally, it is recommended to avoid relying on Enum.ordinal() for threshold comparisons, pre-calculate threshold values to improve performance, and remove development-related comments from the production code.
| * 이미 알림을 보낸 임계치 이하의 상태인 경우 | ||
| * 중복 알림을 방지하기 위해 아무 작업도 하지 않습니다. | ||
| */ | ||
| public void notifyIfNeeded( |
There was a problem hiding this comment.
현재 알림 상태 저장소(NotificationStateStore)가 targetId만을 키로 사용하고 있어, 예산 주기가 바뀌어도(예: 다음 달) 이전 상태가 유지되는 문제가 있습니다. 주기가 바뀔 때 알림이 다시 발송될 수 있도록 키에 주기 정보(예: 연월)를 포함하거나 상태 초기화 메커니즘이 필요합니다.
| // 우리가 "의도한" 형태 | ||
| public record BudgetDecision( |
There was a problem hiding this comment.
이 주석(// 우리가 "의도한" 형태)은 개발 과정의 메모로 보입니다. 코드의 의도는 클래스 구조로 충분히 전달되므로, 프로덕션 코드의 깔끔함을 위해 삭제하는 것을 권장합니다.
public record BudgetDecision(| public enum BudgetThreshold { | ||
|
|
||
| NONE, // 임계치 미도달 | ||
| HALF, // 50% | ||
| WARNING, // 80% | ||
| EXCEEDED // 100% | ||
| } |
There was a problem hiding this comment.
ordinal()에 의존하는 대신, 각 임계치에 명시적인 순서나 비교 로직을 제공하는 것이 안전합니다. 이를 통해 향후 Enum 상수의 순서가 바뀌더라도 로직이 깨지는 것을 방지할 수 있습니다.
| public enum BudgetThreshold { | |
| NONE, // 임계치 미도달 | |
| HALF, // 50% | |
| WARNING, // 80% | |
| EXCEEDED // 100% | |
| } | |
| public enum BudgetThreshold { | |
| NONE(0), // 임계치 미도달 | |
| HALF(1), // 50% | |
| WARNING(2), // 80% | |
| EXCEEDED(3); // 100% | |
| private final int level; | |
| BudgetThreshold(int level) { | |
| this.level = level; | |
| } | |
| public boolean isHigherThan(BudgetThreshold other) { | |
| return this.level > other.level; | |
| } | |
| } |
| if (current.ordinal() <= lastNotified.ordinal()) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
ordinal()을 직접 비교하는 대신 BudgetThreshold에 비교 로직을 캡슐화하여 사용하는 것이 좋습니다(제공된 BudgetThreshold 관련 제안 참고). 또한, 현재 로직은 임계치 확인과 상태 갱신 사이에 원자성이 보장되지 않아 동시성 환경에서 중복 알림이 발생할 수 있는 구조입니다.
| if (current.ordinal() <= lastNotified.ordinal()) { | |
| return; | |
| } | |
| if (!current.isHigherThan(lastNotified)) { | |
| return; | |
| } |
| BigDecimal halfThreshold = monthlyLimit.multiply(BigDecimal.valueOf(0.5)); | ||
| BigDecimal warnThreshold = monthlyLimit.multiply(BigDecimal.valueOf(0.8)); |
There was a problem hiding this comment.
halfThreshold와 warnThreshold는 monthlyLimit이 결정되면 변하지 않는 값입니다. evaluate 메서드 호출 시마다 매번 계산하기보다 생성자에서 미리 계산하여 필드로 관리하는 것이 효율적입니다.
|
에러 해결 후 다시 올리겠습니다.. |
1 participant
📌 변경 요약
AI 호출 비용에 대해 예산 사용률 임계치(50% / 80% / 100%)를 기준으로
판단 및 알림을 처리하는 로직을 추가하고,
동일 임계치에 대해 알림이 중복 발송되지 않도록 개선했습니다.
✅ 주요 변경 사항
1. 예산 임계치 개념 도입 (budget 모듈)
BudgetDecision에 임계치(BudgetThreshold) 정보를 포함하도록 확장DefaultBudgetEvaluator에서 임계치 기반 상태 판단 로직 구현2. 알림 책임 분리 (notification 모듈)
BudgetDecision)를 기반으로 알림을 처리하는BudgetNotificationService를 추가했습니다.3. 알림 중복 방지 기능 구현
NotificationStateStore를 도입했습니다.InMemoryNotificationStateStore를 제공하여로컬/테스트 환경에서 사용 가능하도록 했습니다.
4. 단위 테스트 추가
DefaultBudgetEvaluatorTestBudgetNotificationServiceTest✅ 설계 의도
책임을 명확히 분리하여 확장성(이메일, Slack, Webhook 등)과 테스트 용이성을 고려했습니다.
🔍 테스트 방법