[Core/Budget] Money 정밀도·통화 불변식과 최종 반올림 교정#51
Open
Rigu1 wants to merge 8 commits into
Open
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughChanges비용 모델과 예산 API의 금액 타입을 통화 인식 비용 및 예산 처리
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches⚔️ Resolve merge conflicts
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
🤖 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 17-19: Introduce an atomic reserve/reconcile contract accepting
the budget window and idempotency key in
token-pilot-budget/src/main/java/io/tokenpilot/budget/BudgetStateStore.java:17-19.
Update DefaultBudgetEvaluator in
token-pilot-budget/src/main/java/io/tokenpilot/budget/internal/DefaultBudgetEvaluator.java:27-38
to decide ALLOW/BLOCK from the reservation result, and implement synchronized
per-currency reservations, actual-cost reconciliation, window handling, and
duplicate-key prevention in InMemoryBudgetStateStore at
token-pilot-budget/src/main/java/io/tokenpilot/budget/internal/InMemoryBudgetStateStore.java:24-26.
Extend DefaultBudgetEvaluatorTest at
token-pilot-budget/src/test/java/io/tokenpilot/budget/internal/DefaultBudgetEvaluatorTest.java:95-104
to cover concurrency, budget windows, idempotency, and estimate/actual
reconciliation.
In
`@token-pilot-sample-app/src/main/java/io/tokenpilot/sample/SampleController.java`:
- Around line 89-94: In SampleController’s endpoint flow, move the initial
evaluator.evaluate call into the same try block as the subsequent
evaluator.evaluate call so both invocations are handled by the existing catch
logic. Keep the stateStore.addCost operation and both evaluation arguments
unchanged, and preserve the current response behavior for
BudgetExceededException.
🪄 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: 5e4614fb-8477-4f14-8a0e-3621883b0337
📒 Files selected for processing (25)
token-pilot-autoconfigure/src/main/java/io/tokenpilot/autoconfigure/TokenPilotAutoConfiguration.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/BudgetEvaluator.javatoken-pilot-budget/src/main/java/io/tokenpilot/budget/BudgetState.javatoken-pilot-budget/src/main/java/io/tokenpilot/budget/BudgetStateStore.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-core/src/main/java/io/tokenpilot/core/RoundingPolicy.javatoken-pilot-core/src/main/java/io/tokenpilot/core/domain/Cost.javatoken-pilot-core/src/main/java/io/tokenpilot/core/internal/DefaultCostCalculator.javatoken-pilot-core/src/main/java/io/tokenpilot/core/internal/DefaultLedgerManager.javatoken-pilot-core/src/test/java/io/tokenpilot/core/RoundingPolicyTest.javatoken-pilot-core/src/test/java/io/tokenpilot/core/domain/CostTest.javatoken-pilot-core/src/test/java/io/tokenpilot/core/internal/DefaultCostCalculatorTest.javatoken-pilot-notification/src/main/java/io/tokenpilot/notification/BudgetNotificationEvent.javatoken-pilot-notification/src/test/java/io/tokenpilot/notification/BudgetNotificationServiceTest.javatoken-pilot-sample-app/src/main/java/io/tokenpilot/sample/CostBoundaryFormatter.javatoken-pilot-sample-app/src/main/java/io/tokenpilot/sample/SampleController.javatoken-pilot-sample-app/src/test/java/io/tokenpilot/sample/SampleApplicationBudgetE2ETest.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
+17
to
+19
| Cost getAccumulatedCost(Map<String, String> tags, Currency currency); | ||
|
|
||
| void addCost(Map<String, String> tags, BigDecimal amount); | ||
| void addCost(Map<String, String> tags, Cost cost); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
원자적 예산 예약 계약을 도입하세요. 현재 조회·평가·누적이 분리되어 동시 요청이 같은 잔여 예산을 보고 모두 BLOCK을 우회할 수 있습니다.
token-pilot-budget/src/main/java/io/tokenpilot/budget/BudgetStateStore.java#L17-L19: 예산 창과 idempotency key를 받는 원자적 reserve/reconcile 계약을 추가하세요.token-pilot-budget/src/main/java/io/tokenpilot/budget/internal/DefaultBudgetEvaluator.java#L27-L38: 단순 조회 결과 대신 원자적 예약 결과로 허용·차단을 결정하세요.token-pilot-budget/src/main/java/io/tokenpilot/budget/internal/InMemoryBudgetStateStore.java#L24-L26: 통화별 상태에 예약·실제 비용 조정·중복 요청 방지를 구현하세요.token-pilot-budget/src/test/java/io/tokenpilot/budget/internal/DefaultBudgetEvaluatorTest.java#L95-L104: 동시 요청, 예산 창, 중복 요청, estimate/actual reconcile을 검증하도록 바꾸세요.
As per coding guidelines, “Budget enforcement must address BLOCK behavior, budget windows, atomic reservation, idempotency, concurrency, and estimate/actual reconciliation before being treated as complete.”
📍 Affects 4 files
token-pilot-budget/src/main/java/io/tokenpilot/budget/BudgetStateStore.java#L17-L19(this comment)token-pilot-budget/src/main/java/io/tokenpilot/budget/internal/DefaultBudgetEvaluator.java#L27-L38token-pilot-budget/src/main/java/io/tokenpilot/budget/internal/InMemoryBudgetStateStore.java#L24-L26token-pilot-budget/src/test/java/io/tokenpilot/budget/internal/DefaultBudgetEvaluatorTest.java#L95-L104
🤖 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 17 - 19, Introduce an atomic reserve/reconcile contract accepting
the budget window and idempotency key in
token-pilot-budget/src/main/java/io/tokenpilot/budget/BudgetStateStore.java:17-19.
Update DefaultBudgetEvaluator in
token-pilot-budget/src/main/java/io/tokenpilot/budget/internal/DefaultBudgetEvaluator.java:27-38
to decide ALLOW/BLOCK from the reservation result, and implement synchronized
per-currency reservations, actual-cost reconciliation, window handling, and
duplicate-key prevention in InMemoryBudgetStateStore at
token-pilot-budget/src/main/java/io/tokenpilot/budget/internal/InMemoryBudgetStateStore.java:24-26.
Extend DefaultBudgetEvaluatorTest at
token-pilot-budget/src/test/java/io/tokenpilot/budget/internal/DefaultBudgetEvaluatorTest.java:95-104
to cover concurrency, budget windows, idempotency, and estimate/actual
reconciliation.
Source: Coding guidelines
Comment on lines
89
to
+94
| Map<String, String> tags = Map.of("tenant_id", "budget-sample-tenant"); | ||
| BudgetDecision initialDecision = evaluator.evaluate(tags, new BigDecimal("0.001")); | ||
| stateStore.addCost(tags, new BigDecimal("0.0045")); | ||
| BudgetDecision initialDecision = evaluator.evaluate(tags, Cost.of(new BigDecimal("0.001"), USD)); | ||
| stateStore.addCost(tags, Cost.of(new BigDecimal("0.0045"), USD)); | ||
|
|
||
| try { | ||
| evaluator.evaluate(tags, new BigDecimal("0.001")); | ||
| evaluator.evaluate(tags, Cost.of(new BigDecimal("0.001"), USD)); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
첫 번째 evaluate 호출이 예외 처리 없이 실행되어 반복 호출 시 500 에러 위험.
stateStore는 호출마다 무조건 0.0045를 누적하고(Line 91), Line 90의 evaluator.evaluate(tags, ...)는 try 블록 밖에 있습니다. 실제 실행 환경에서 이 엔드포인트를 두 번째로 호출하면 이전 호출에서 누적된 비용 때문에 Line 90에서도 BudgetExceededException이 발생할 수 있는데, 이는 어디에서도 캐치되지 않아 그대로 500 에러로 전파됩니다. 현재 catch 블록은 Line 94의 두 번째 호출에만 대비되어 있어 비대칭적입니다.
두 evaluate 호출을 모두 같은 try 블록 안에서 처리하도록 재구성하는 것을 권장합니다.
🛡️ 제안하는 수정
Map<String, String> tags = Map.of("tenant_id", "budget-sample-tenant");
- BudgetDecision initialDecision = evaluator.evaluate(tags, Cost.of(new BigDecimal("0.001"), USD));
- stateStore.addCost(tags, Cost.of(new BigDecimal("0.0045"), USD));
-
try {
+ BudgetDecision initialDecision = evaluator.evaluate(tags, Cost.of(new BigDecimal("0.001"), USD));
+ stateStore.addCost(tags, Cost.of(new BigDecimal("0.0045"), USD));
evaluator.evaluate(tags, Cost.of(new BigDecimal("0.001"), USD));
return Map.of(
"enabled", "true",
"initialState", initialDecision.state().name(),
"blockedState", "NONE"
);
} catch (BudgetExceededException exception) {
BudgetDecision blockedDecision = exception.getDecision();
return Map.of(
"enabled", "true",
- "initialState", initialDecision.state().name(),
+ "initialState", blockedDecision.state().name(),
"blockedState", blockedDecision.state().name(),
"currentUsage", CostBoundaryFormatter.format(blockedDecision.currentUsage()),
"limit", CostBoundaryFormatter.format(blockedDecision.limit())
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Map<String, String> tags = Map.of("tenant_id", "budget-sample-tenant"); | |
| BudgetDecision initialDecision = evaluator.evaluate(tags, new BigDecimal("0.001")); | |
| stateStore.addCost(tags, new BigDecimal("0.0045")); | |
| BudgetDecision initialDecision = evaluator.evaluate(tags, Cost.of(new BigDecimal("0.001"), USD)); | |
| stateStore.addCost(tags, Cost.of(new BigDecimal("0.0045"), USD)); | |
| try { | |
| evaluator.evaluate(tags, new BigDecimal("0.001")); | |
| evaluator.evaluate(tags, Cost.of(new BigDecimal("0.001"), USD)); | |
| Map<String, String> tags = Map.of("tenant_id", "budget-sample-tenant"); | |
| try { | |
| BudgetDecision initialDecision = evaluator.evaluate(tags, Cost.of(new BigDecimal("0.001"), USD)); | |
| stateStore.addCost(tags, Cost.of(new BigDecimal("0.0045"), USD)); | |
| evaluator.evaluate(tags, Cost.of(new BigDecimal("0.001"), USD)); | |
| return Map.of( | |
| "enabled", "true", | |
| "initialState", initialDecision.state().name(), | |
| "blockedState", "NONE" | |
| ); | |
| } catch (BudgetExceededException exception) { | |
| BudgetDecision blockedDecision = exception.getDecision(); | |
| return Map.of( | |
| "enabled", "true", | |
| "initialState", blockedDecision.state().name(), | |
| "blockedState", blockedDecision.state().name(), | |
| "currentUsage", CostBoundaryFormatter.format(blockedDecision.currentUsage()), | |
| "limit", CostBoundaryFormatter.format(blockedDecision.limit()) | |
| ); | |
| } |
🤖 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-sample-app/src/main/java/io/tokenpilot/sample/SampleController.java`
around lines 89 - 94, In SampleController’s endpoint flow, move the initial
evaluator.evaluate call into the same try block as the subsequent
evaluator.evaluate call so both invocations are handled by the existing catch
logic. Keep the stateStore.addCost operation and both evaluation arguments
unchanged, and preserve the current response behavior for
BudgetExceededException.
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
1 participant
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.
변경 사항
Cost의 금액/통화 불변식을 Money ADR 기준에 맞게 정리했습니다.
Costcanonical constructor에서value와currencynull을 거부합니다.Cost생성 시 자동 scale 6 반올림을 제거해 내부 정밀도를 보존합니다.Cost.zero()의 암묵적 USD 선택을 제거하고Cost.zero(currency)로 통화를 명시합니다.Cost.add(...)와Cost.compareTo(...)는 같은 통화에서만 허용합니다.equals/hashCode를 정리했습니다.DefaultCostCalculator의 1K 단가 계산을rate.multiply(count).movePointLeft(3)방식으로 정리했습니다.RoundingPolicy.COST_BOUNDARY_ROUNDING을 추가했습니다.Cost기반 API로 이관했습니다.BudgetDecision,BudgetEvaluator,BudgetStateStore, notification event가 currency-aware 금액을 다루도록 변경했습니다.CURRENCY_MISMATCH를 반환합니다.InMemoryBudgetStateStore는 같은 tags라도 currency가 다르면 누적 상태를 분리합니다.CostBoundaryFormatter를 통해 표시 경계 반올림을 한 번만 적용합니다.배경
기존 구현은 비용 계산 중간 또는
Cost생성 시점에 반올림을 적용했습니다. 이로 인해0.0000004 USD처럼 작은 요청 비용이 사라질 수 있고, 요청별 반올림 오차가 누적될 수 있었습니다.또한 Budget API가 통화 없는
BigDecimal을 사용해 budget limit과 actual/reservation cost의 통화를 안전하게 비교하기 어려웠습니다.이번 변경은 Money ADR의 정밀도 정책에 맞춰 내부 금액은 반올림 없이 보존하고, 외부 표시/청구 경계에서만 명시적인 정책으로 최종 반올림하도록 정리합니다.
비용 계산 흐름
수치 예시
호환성
Cost.of(value)대신Cost.of(value, currency)를 사용해야 합니다.Cost.zero()대신Cost.zero(currency)를 사용해야 합니다.Cost는 더 이상 생성자에서scale 6 반올림을 수행하지 않습니다.Budget API의 금액 타입이BigDecimal에서Cost로 변경됩니다.BudgetDecision.currentUsage()와BudgetDecision.limit()은Cost를 반환합니다.BudgetEvaluator.evaluate(tags, cost)는Cost를 받습니다.BudgetStateStore.addCost(tags, cost)는Cost를 받습니다.BudgetStateStore.getAccumulatedCost(tags, currency)는 명시currency를 받아Cost를 반환합니다.RoundingPolicy.COST_BOUNDARY_ROUNDING을 명시적으로 적용해야 합니다.범위 메모
현재 코드에 존재하는 Budget operation은 evaluate입니다. reserve, commit, reconcile은 아직 구현된 API가 아니므로, 해당 통화 불일치 처리는 후속 reservation/reconciliation 이슈에서 이어가야 합니다.
체크리스트
필수 테스트
Acceptance Criteria
Closes #26