[Budget] 월별 budget window를 Clock 기반으로 교정#49
Merged
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
HuitaePark
self-requested a review
July 22, 2026 05:24
Member
|
@coderabbitai full review |
✅ Action performedFull review finished. |
📝 WalkthroughSummary by CodeRabbit
Walkthrough예산 정책과 월별 Changes예산 식별자 기반 흐름
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
token-pilot-notification/src/main/java/io/tokenpilot/notification/BudgetNotificationService.java (1)
37-60: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift동일 key의 알림 선점을 원자화하세요.
동시 요청 두 건이 모두 이전 threshold를 읽고 각각 이벤트를 발행한 뒤 상태를 갱신할 수 있어, 동일
BudgetKey·threshold의 중복 알림이 발생합니다. 조회와 갱신을 분리한 현재 계약으로는 이를 막을 수 없습니다.
token-pilot-notification/src/main/java/io/tokenpilot/notification/BudgetNotificationService.java#L37-L60: 별도 조회·갱신 대신 원자적 claim 결과가 성공한 경우에만 이벤트를 발행하세요.token-pilot-notification/src/main/java/io/tokenpilot/notification/NotificationStateStore.java#L11-L16: 예:advanceIfHigher(BudgetKey, BudgetThreshold): boolean형태의 원자적 상태 전이 계약으로 교체하세요.token-pilot-notification/src/main/java/io/tokenpilot/notification/InMemoryNotificationStateStore.java#L16-L27:ConcurrentHashMap.compute등으로 threshold 비교와 갱신을 하나의 원자적 연산으로 구현하세요.token-pilot-notification/src/test/java/io/tokenpilot/notification/BudgetNotificationServiceTest.java#L25-L52: 같은 key·threshold를 동시에 호출해 handler가 한 번만 실행되는 회귀 테스트를 추가하세요.As per coding guidelines, 알림 중복 제거는 evaluator-resolved
BudgetKey를 사용하고 동일 window의 동등하거나 낮은 threshold를 억제해야 합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@token-pilot-notification/src/main/java/io/tokenpilot/notification/BudgetNotificationService.java` around lines 37 - 60, 동일 key·threshold 알림의 중복 발행을 막도록 원자적 claim 흐름으로 변경하세요. token-pilot-notification/src/main/java/io/tokenpilot/notification/BudgetNotificationService.java:37-60에서는 evaluator-resolved BudgetKey로 NotificationStateStore.advanceIfHigher(...)를 호출하고 성공한 경우에만 이벤트를 발행하세요. token-pilot-notification/src/main/java/io/tokenpilot/notification/NotificationStateStore.java:11-16의 계약을 해당 boolean 원자적 상태 전이로 교체하고, token-pilot-notification/src/main/java/io/tokenpilot/notification/InMemoryNotificationStateStore.java:16-27에서는 ConcurrentHashMap.compute 등으로 동일 window의 동등하거나 낮은 threshold를 억제하며 비교·갱신을 원자화하세요. token-pilot-notification/src/test/java/io/tokenpilot/notification/BudgetNotificationServiceTest.java:25-52에는 같은 key·threshold의 동시 호출 시 handler가 한 번만 실행되는 회귀 테스트를 추가하세요.Source: Coding guidelines
token-pilot-spring-ai/src/main/java/io/tokenpilot/springai/internal/DefaultLedgerAdvisor.java (2)
63-88: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
after()의 방어 조건 불일치 및 통화 불일치 시 전체 AI 응답이 깨질 수 있습니다.두 가지 결함이 결합되어 있습니다.
- 가드 조건이
budgetStateStore/costCalculator/pricingRegistry != null만 확인하고budgetEvaluator != null은 확인하지 않습니다.budgetEvaluator가 없으면before()에서 컨텍스트에 decision이 채워지지 않는데,after()는 여전히 예산 블록을 실행해extractBudgetDecision이IllegalStateException을 던지게 됩니다.costCalculator.calculate(...)는 pricing plan의 currency로Cost를 만드는데,decision.currency()는 budget policy의 currency입니다. 두 currency가 다르면(멀티-currency 가격 정책 구성 시 충분히 발생 가능)addCost가IllegalArgumentException을 던집니다.두 경우 모두
after()에서 잡히지 않은 예외가 advisor 체인 밖으로 전파되어, 실제 LLM 응답 자체가 실패하게 됩니다(장부 기록ledgerManager.record는 이미 끝난 뒤이므로 응답만 깨지는 것이 더 안타까운 지점). 최소한 이 블록을 방어적으로 처리(예: catch 후 로깅)해 예산/가격 설정 문제가 사용자 응답을 막지 않도록 하는 것을 권장합니다.🛡️ 방어적 처리 제안
if (budgetStateStore != null && costCalculator != null && pricingRegistry != null) { Optional<PricingPlan> plan = pricingRegistry.getPlan(modelId); if (plan.isPresent()) { Cost cost = costCalculator.calculate(usage, plan.get()); - BudgetDecision decision = extractBudgetDecision(response); - budgetStateStore.addCost( - decision.key(), - decision.limit(), - decision.currency(), - cost - ); + try { + BudgetDecision decision = extractBudgetDecision(response); + budgetStateStore.addCost( + decision.key(), + decision.limit(), + decision.currency(), + cost + ); + } catch (IllegalStateException | IllegalArgumentException e) { + // 예산 설정/통화 불일치가 실제 AI 응답을 막지 않도록 함 + log.warn("Skipping budget cost accumulation: {}", e.getMessage()); + } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@token-pilot-spring-ai/src/main/java/io/tokenpilot/springai/internal/DefaultLedgerAdvisor.java` around lines 63 - 88, Update the budget-processing block in DefaultLedgerAdvisor.after to also require budgetEvaluator before calling extractBudgetDecision, and defensively catch failures from decision extraction, cost calculation, or budgetStateStore.addCost so pricing or budget configuration errors are logged without preventing the original ChatClientResponse from returning. Preserve ledgerManager.record and always return response.
51-61: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
before()에서 BLOCK을 차단해야 합니다.budgetEvaluator.evaluate(tags)의 결과를 context에만 넣고request를 그대로 반환해서,BudgetState.BLOCK이어도 provider 호출이 계속됩니다. 예산 초과 시에는 여기서 예외를 던지거나 요청을 중단해야 합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@token-pilot-spring-ai/src/main/java/io/tokenpilot/springai/internal/DefaultLedgerAdvisor.java` around lines 51 - 61, DefaultLedgerAdvisor의 before()에서 budgetEvaluator.evaluate(tags) 결과가 BudgetState.BLOCK이면 provider 호출로 진행되지 않도록 요청을 중단하거나 예외를 던지도록 수정하세요. 차단되지 않은 결정만 기존처럼 BUDGET_DECISION_CONTEXT에 저장한 변형된 요청을 반환하고, evaluator가 없는 경우의 기존 동작은 유지하세요.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@token-pilot-budget/src/main/java/io/tokenpilot/budget/BudgetStateStore.java`:
- Around line 8-15: Redefine the BudgetStateStore contract so spend enforcement
is not split between getAccumulatedCost and addCost. Add an atomic reservation
operation, with idempotency support and concurrency-safe limit enforcement, and
update DefaultLedgerAdvisor.before() to reserve before provider invocation.
Reconcile the reservation with actual cost in after(), preserving final BLOCK
enforcement for concurrent requests.
In
`@token-pilot-budget/src/main/java/io/tokenpilot/budget/internal/DefaultBudgetEvaluator.java`:
- Around line 30-54: Update DefaultBudgetEvaluator’s evaluate methods and the
ledger flow used by DefaultLedgerAdvisor.after() so final spend control does not
rely on check-then-add: reserve the estimated cost atomically for each BudgetKey
before provider invocation, enforce BLOCK from the reservation result, and
ensure concurrent requests cannot exceed the policy limit. Make the cost-bearing
evaluation path used by before() include the current request estimate, and
preserve a clear reconciliation path for replacing the reservation with actual
cost and releasing any difference, with idempotency for retries.
---
Outside diff comments:
In
`@token-pilot-notification/src/main/java/io/tokenpilot/notification/BudgetNotificationService.java`:
- Around line 37-60: 동일 key·threshold 알림의 중복 발행을 막도록 원자적 claim 흐름으로 변경하세요.
token-pilot-notification/src/main/java/io/tokenpilot/notification/BudgetNotificationService.java:37-60에서는
evaluator-resolved BudgetKey로 NotificationStateStore.advanceIfHigher(...)를 호출하고
성공한 경우에만 이벤트를 발행하세요.
token-pilot-notification/src/main/java/io/tokenpilot/notification/NotificationStateStore.java:11-16의
계약을 해당 boolean 원자적 상태 전이로 교체하고,
token-pilot-notification/src/main/java/io/tokenpilot/notification/InMemoryNotificationStateStore.java:16-27에서는
ConcurrentHashMap.compute 등으로 동일 window의 동등하거나 낮은 threshold를 억제하며 비교·갱신을 원자화하세요.
token-pilot-notification/src/test/java/io/tokenpilot/notification/BudgetNotificationServiceTest.java:25-52에는
같은 key·threshold의 동시 호출 시 handler가 한 번만 실행되는 회귀 테스트를 추가하세요.
In
`@token-pilot-spring-ai/src/main/java/io/tokenpilot/springai/internal/DefaultLedgerAdvisor.java`:
- Around line 63-88: Update the budget-processing block in
DefaultLedgerAdvisor.after to also require budgetEvaluator before calling
extractBudgetDecision, and defensively catch failures from decision extraction,
cost calculation, or budgetStateStore.addCost so pricing or budget configuration
errors are logged without preventing the original ChatClientResponse from
returning. Preserve ledgerManager.record and always return response.
- Around line 51-61: DefaultLedgerAdvisor의 before()에서
budgetEvaluator.evaluate(tags) 결과가 BudgetState.BLOCK이면 provider 호출로 진행되지 않도록 요청을
중단하거나 예외를 던지도록 수정하세요. 차단되지 않은 결정만 기존처럼 BUDGET_DECISION_CONTEXT에 저장한 변형된 요청을
반환하고, evaluator가 없는 경우의 기존 동작은 유지하세요.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6935906c-ed89-4adf-a2a8-0ed2dbf0c1b5
📒 Files selected for processing (22)
AGENTS.mdtoken-pilot-autoconfigure/src/main/java/io/tokenpilot/autoconfigure/TokenPilotAutoConfiguration.javatoken-pilot-autoconfigure/src/main/java/io/tokenpilot/autoconfigure/TokenPilotProperties.javatoken-pilot-autoconfigure/src/test/java/io/tokenpilot/autoconfigure/TokenPilotAutoConfigurationTest.javatoken-pilot-budget/src/main/java/io/tokenpilot/budget/BudgetDecision.javatoken-pilot-budget/src/main/java/io/tokenpilot/budget/BudgetKey.javatoken-pilot-budget/src/main/java/io/tokenpilot/budget/BudgetPolicy.javatoken-pilot-budget/src/main/java/io/tokenpilot/budget/BudgetStateStore.javatoken-pilot-budget/src/main/java/io/tokenpilot/budget/BudgetWindow.javatoken-pilot-budget/src/main/java/io/tokenpilot/budget/internal/DefaultBudgetEvaluator.javatoken-pilot-budget/src/main/java/io/tokenpilot/budget/internal/InMemoryBudgetStateStore.javatoken-pilot-budget/src/main/java/io/tokenpilot/budget/internal/LedgerBudgetComponents.javatoken-pilot-budget/src/test/java/io/tokenpilot/budget/internal/DefaultBudgetEvaluatorTest.javatoken-pilot-budget/src/test/java/io/tokenpilot/budget/internal/InMemoryBudgetStateStoreTest.javatoken-pilot-notification/src/main/java/io/tokenpilot/notification/BudgetNotificationEvent.javatoken-pilot-notification/src/main/java/io/tokenpilot/notification/BudgetNotificationService.javatoken-pilot-notification/src/main/java/io/tokenpilot/notification/InMemoryNotificationStateStore.javatoken-pilot-notification/src/main/java/io/tokenpilot/notification/NotificationStateStore.javatoken-pilot-notification/src/test/java/io/tokenpilot/notification/BudgetNotificationServiceTest.javatoken-pilot-sample-app/src/main/java/io/tokenpilot/sample/SampleController.javatoken-pilot-spring-ai/src/main/java/io/tokenpilot/springai/internal/DefaultLedgerAdvisor.javatoken-pilot-spring-ai/src/test/java/io/tokenpilot/springai/internal/DefaultLedgerAdvisorTest.java
Comment on lines
8
to
+15
| /** | ||
| * 예산 판단을 위해 비용 누적 상태를 관리하는 저장소 인터페이스입니다. | ||
| * <p> | ||
| * 예산 식별자별(예: tenant)로 비용을 조회하고 | ||
| * 누적하는 책임을 가집니다. | ||
| * resolved {@link BudgetKey}별 누적 비용과 limit/currency snapshot을 관리합니다. | ||
| */ | ||
|
|
||
| public interface BudgetStateStore { | ||
|
|
||
| BigDecimal getAccumulatedCost(Map<String, String> tags); | ||
| BigDecimal getAccumulatedCost(BudgetKey key, BigDecimal limit, Currency currency); | ||
|
|
||
| void addCost(Map<String, String> tags, BigDecimal amount); | ||
| void addCost(BudgetKey key, BigDecimal limit, Currency currency, Cost amount); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
getAccumulatedCost/addCost는 여전히 check-then-add 구조입니다.
두 메서드가 분리되어 있어, 동시에 들어온 여러 요청이 모두 getAccumulatedCost로 한도 미달을 확인한 뒤 각각 addCost를 호출하면 월 한도를 초과할 수 있습니다. DefaultLedgerAdvisor에서도 before()의 평가와 after()의 addCost 사이에 실제 LLM 호출 시간만큼의 간격이 있어 동일한 위험이 그대로 전파됩니다. 원자적 예약(reserve)이나 idempotency 키 없이는 최종 지출 제어가 보장되지 않습니다.
이 PR의 범위(식별자/윈도우 정합성)를 벗어나는 것은 이해하지만, 이번 diff에서 인터페이스 계약 자체가 재정의되고 있어 명시적으로 짚어둡니다.
As per coding guidelines, token-pilot-budget/src/main/java/**/*.java: "Do not rely on check-then-add for final spend control; implement BLOCK enforcement before provider invocation, atomic reservation, idempotency, concurrency safety, and estimate/actual reconciliation as required MVP behavior."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@token-pilot-budget/src/main/java/io/tokenpilot/budget/BudgetStateStore.java`
around lines 8 - 15, Redefine the BudgetStateStore contract so spend enforcement
is not split between getAccumulatedCost and addCost. Add an atomic reservation
operation, with idempotency support and concurrency-safe limit enforcement, and
update DefaultLedgerAdvisor.before() to reserve before provider invocation.
Reconcile the reservation with actual cost in after(), preserving final BLOCK
enforcement for concurrent requests.
Source: Coding guidelines
Comment on lines
30
to
+54
| @Override | ||
| public BudgetDecision evaluate( | ||
| Map<String, String> tags, | ||
| BigDecimal costAmount | ||
| ) { | ||
|
|
||
| BigDecimal currentUsage = store.getAccumulatedCost(tags); | ||
| BigDecimal newUsage = currentUsage.add(costAmount); | ||
|
|
||
| BigDecimal halfThreshold = monthlyLimit.multiply(BigDecimal.valueOf(0.5)); | ||
| BigDecimal warningThreshold = monthlyLimit.multiply(BigDecimal.valueOf(0.8)); | ||
|
|
||
| if (newUsage.compareTo(monthlyLimit) >= 0) { | ||
| BudgetDecision decision = new BudgetDecision( | ||
| BudgetState.BLOCK, | ||
| BudgetThreshold.EXCEEDED, | ||
| "월 예산을 초과했습니다", | ||
| newUsage, | ||
| monthlyLimit | ||
| ); | ||
| public BudgetDecision evaluate(Map<String, String> tags, BigDecimal costAmount) { | ||
| BudgetKey key = resolveKey(tags); | ||
| BigDecimal usage = store.getAccumulatedCost( | ||
| key, | ||
| policy.monthlyLimit(), | ||
| policy.currency() | ||
| ).add(costAmount); | ||
| BudgetDecision decision = decide(key, usage); | ||
| if (decision.state() == BudgetState.BLOCK) { | ||
| throw new BudgetExceededException(decision); | ||
| } | ||
| return decision; | ||
| } | ||
|
|
||
| // ✅ 80% 이상 | ||
| if (newUsage.compareTo(warningThreshold) >= 0) { | ||
| return new BudgetDecision( | ||
| BudgetState.WARN, | ||
| BudgetThreshold.WARNING, | ||
| "월 예산의 80% 이상 사용", | ||
| newUsage, | ||
| monthlyLimit | ||
| ); | ||
| } | ||
| @Override | ||
| public BudgetDecision evaluate(Map<String, String> tags) { | ||
| BudgetKey key = resolveKey(tags); | ||
| BigDecimal usage = store.getAccumulatedCost( | ||
| key, | ||
| policy.monthlyLimit(), | ||
| policy.currency() | ||
| ); | ||
| return decide(key, usage); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift
check-then-add 방식의 예산 집행 – 동시성 하에 한도 초과 가능
evaluate(tags, costAmount)는 누적 비용을 조회해 예상치를 계산하고 BLOCK 여부만 판정할 뿐, 이 값을 store에 원자적으로 반영(예약)하지 않습니다. 실제 커밋은 응답 이후 별도의 addCost 호출(그래프 컨텍스트상 DefaultLedgerAdvisor.after())에서 이뤄지므로, 두 요청이 동시에 동일 key로 evaluate()를 통과한 뒤 각자 실제 비용을 더하면 총합이 한도를 초과할 수 있습니다(TOCTOU). 또한 before()에서 쓰는 evaluate(tags)(비용 없는 오버로드)는 현재 요청의 예상 비용조차 반영하지 않아, 매 요청이 "이전 누적치"만으로 판정됩니다.
추가된 동시성 테스트는 key 생성의 결정성만 검증하며, 실제 예산 집행의 원자성은 검증하지 않습니다.
As per coding guidelines, "Do not rely on check-then-add for final spend control; implement BLOCK enforcement before provider invocation, atomic reservation, idempotency, concurrency safety, and estimate/actual reconciliation as required MVP behavior."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@token-pilot-budget/src/main/java/io/tokenpilot/budget/internal/DefaultBudgetEvaluator.java`
around lines 30 - 54, Update DefaultBudgetEvaluator’s evaluate methods and the
ledger flow used by DefaultLedgerAdvisor.after() so final spend control does not
rely on check-then-add: reserve the estimated cost atomically for each BudgetKey
before provider invocation, enforce BLOCK from the reservation result, and
ensure concurrent requests cannot exceed the policy limit. Make the cost-bearing
evaluation path used by before() include the current request estimate, and
preserve a clear reconciliation path for replacing the reservation with actual
cost and releasing any difference, with idempotency for retries.
Source: Coding guidelines
Member
|
@Leejaewang03 고생하셨습니다! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
변경 내용
BudgetKey와BudgetWindow를 추가했습니다.Clock과 설정된ZoneId기준으로 월별 window를 결정합니다.