[Core/Budget] Money 정밀도·통화 불변식과 최종 반올림 교정#50
Closed
Rigu1 wants to merge 13 commits into
Closed
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비용 모델이 통화 명시형 통화 기반 비용 및 예산
Suggested reviewers: 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 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: 6
🧹 Nitpick comments (2)
token-ledger-budget/src/main/java/io/tokenledger/budget/BudgetEvaluator.java (1)
20-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winJavadoc에
CURRENCY_MISMATCH반환 케이스가 누락됨
evaluate(tags, cost)가 이제BudgetState.CURRENCY_MISMATCH를 반환할 수 있는데, 인터페이스 문서는 여전히 "예산 초과 여부만 판단합니다(부수 효과 없음)"로만 설명되어 있어 호출자가 이 상태를 놓칠 위험이 있습니다.♻️ 제안
/** - * 이번 호출로 발생할 예상 비용을 포함하여 예산 초과 여부를 판단합니다. (부수 효과 없음) + * 이번 호출로 발생할 예상 비용을 포함하여 예산 초과 여부를 판단합니다. (부수 효과 없음) + * <p> + * {`@code` cost}의 통화가 예산 한도 통화와 다르면 {`@link` BudgetState#CURRENCY_MISMATCH} 상태를 반환합니다. */🤖 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-ledger-budget/src/main/java/io/tokenledger/budget/BudgetEvaluator.java` around lines 20 - 26, Update the Javadoc for the BudgetEvaluator.evaluate method to document that it can return BudgetState.CURRENCY_MISMATCH in addition to budget-exceeded outcomes. Preserve the existing side-effect-free behavior description and make the currency mismatch case explicit for callers.token-ledger-budget/src/test/java/io/tokenledger/budget/internal/DefaultBudgetEvaluatorTest.java (1)
65-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBLOCK 테스트가 예외 타입만 검증,
Cost값은 검증하지 않음다른 테스트들은
decision.currentUsage()/decision.limit()을Cost로 검증하는데, 이 테스트는BudgetExceededException타입만 확인합니다.BudgetExceededException이 내부BudgetDecision을 노출한다면nextUsage/monthlyLimit값도 함께 검증하는 것이 이번 PR의 통화/정밀도 회귀 테스트 취지에 더 부합합니다.🤖 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-ledger-budget/src/test/java/io/tokenledger/budget/internal/DefaultBudgetEvaluatorTest.java` around lines 65 - 79, The shouldThrowExceptionWhenUsageExceedsLimit test only checks the exception type; extend it to inspect the BudgetExceededException’s exposed BudgetDecision and assert its nextUsage and monthlyLimit Cost values match usd("120") and usd("100"), respectively. Preserve the existing exception assertion and inputs.
🤖 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-ledger-budget/src/main/java/io/tokenledger/budget/internal/DefaultBudgetEvaluator.java`:
- Around line 43-51: Update the currency validation used by
DefaultBudgetEvaluator to validate accumulated as well as cost against
monthlyLimit before calling accumulated.add(cost). Apply the same validation to
the secondary evaluation path around the referenced lines, and route any
mismatch through currencyMismatchDecision rather than allowing Cost.add to
throw. Reuse or extend hasCurrencyMismatch so both store-derived and incoming
costs are covered.
- Around line 10-15: DefaultBudgetEvaluator 클래스의 Javadoc에서 “판단 기준” 목록을 올바른 항목별
형식으로 정리하고, currency 불일치 시 CURRENCY_MISMATCH가 발생한다는 기준을 명시하세요. 목록과 이어 붙은 순수 함수
설명을 분리해 Javadoc 문장이 자연스럽게 렌더링되도록 수정하세요.
In
`@token-ledger-budget/src/main/java/io/tokenledger/budget/internal/InMemoryBudgetStateStore.java`:
- Around line 28-30: Update InMemoryBudgetStateStore#getAccumulatedCost and
addCost so the state key includes both the tenant tags and currency, using the
same key construction for reads and writes. Ensure requests for different
currencies cannot share or return the same accumulated Cost.
In `@token-ledger-core/src/main/java/io/tokenledger/core/domain/Cost.java`:
- Around line 11-12: Update the public Javadoc for the Cost constructor
parameters to remove the claim that currency defaults to USD and that amount is
limited or rounded to six decimal places. Document only the current behavior and
precision contract for amount and currency.
In `@token-ledger-core/src/test/java/io/tokenledger/core/domain/CostTest.java`:
- Around line 48-54: Update shouldRejectAddWithNullCost to invoke Cost.add(null)
instead of Cost.compareTo(null), so the test validates Cost.add’s null rejection
contract. Keep the existing Cost setup and NullPointerException assertion
unchanged.
In
`@token-ledger-core/src/test/java/io/tokenledger/core/internal/DefaultLedgerManagerTest.java`:
- Line 56: 기본 요금제 테스트에서 result.amount()만 검증하지 말고 결과의 통화 필드/접근자도 함께 검증하도록 수정하세요.
누락 요금제의 fallback 통화가 USD임을 명시적으로 확인하고, 기존 금액 검증은 유지하세요.
---
Nitpick comments:
In
`@token-ledger-budget/src/main/java/io/tokenledger/budget/BudgetEvaluator.java`:
- Around line 20-26: Update the Javadoc for the BudgetEvaluator.evaluate method
to document that it can return BudgetState.CURRENCY_MISMATCH in addition to
budget-exceeded outcomes. Preserve the existing side-effect-free behavior
description and make the currency mismatch case explicit for callers.
In
`@token-ledger-budget/src/test/java/io/tokenledger/budget/internal/DefaultBudgetEvaluatorTest.java`:
- Around line 65-79: The shouldThrowExceptionWhenUsageExceedsLimit test only
checks the exception type; extend it to inspect the BudgetExceededException’s
exposed BudgetDecision and assert its nextUsage and monthlyLimit Cost values
match usd("120") and usd("100"), respectively. Preserve the existing exception
assertion and inputs.
🪄 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: acdb0dda-fa76-43c5-be8f-3f329452ebe1
📒 Files selected for processing (17)
token-ledger-budget/src/main/java/io/tokenledger/budget/BudgetDecision.javatoken-ledger-budget/src/main/java/io/tokenledger/budget/BudgetEvaluator.javatoken-ledger-budget/src/main/java/io/tokenledger/budget/BudgetState.javatoken-ledger-budget/src/main/java/io/tokenledger/budget/BudgetStateStore.javatoken-ledger-budget/src/main/java/io/tokenledger/budget/internal/DefaultBudgetEvaluator.javatoken-ledger-budget/src/main/java/io/tokenledger/budget/internal/InMemoryBudgetStateStore.javatoken-ledger-budget/src/main/java/io/tokenledger/budget/internal/LedgerBudgetComponents.javatoken-ledger-budget/src/test/java/io/tokenledger/budget/internal/DefaultBudgetEvaluatorTest.javatoken-ledger-core/src/main/java/io/tokenledger/core/RoundingPolicy.javatoken-ledger-core/src/main/java/io/tokenledger/core/domain/Cost.javatoken-ledger-core/src/main/java/io/tokenledger/core/domain/PricingPlan.javatoken-ledger-core/src/main/java/io/tokenledger/core/internal/DefaultCostCalculator.javatoken-ledger-core/src/main/java/io/tokenledger/core/internal/DefaultLedgerManager.javatoken-ledger-core/src/test/java/io/tokenledger/core/RoundingPolicyTest.javatoken-ledger-core/src/test/java/io/tokenledger/core/domain/CostTest.javatoken-ledger-core/src/test/java/io/tokenledger/core/internal/DefaultCostCalculatorTest.javatoken-ledger-core/src/test/java/io/tokenledger/core/internal/DefaultLedgerManagerTest.java
Comment on lines
+28
to
+30
| public Cost getAccumulatedCost(Map<String, String> tags, Currency currency) { | ||
| return store.getOrDefault(key(tags), Cost.zero(currency)); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate relevant files
git ls-files | rg 'token-ledger-budget/src/main/java/io/tokenledger/budget/internal/InMemoryBudgetStateStore.java|token-ledger-budget/src/main/java/io/tokenledger/budget|Cost.java|DefaultBudgetEvaluator.java'
# Map the target file structure
ast-grep outline token-ledger-budget/src/main/java/io/tokenledger/budget/internal/InMemoryBudgetStateStore.java --view expanded
# Inspect the target file with line numbers
wc -l token-ledger-budget/src/main/java/io/tokenledger/budget/internal/InMemoryBudgetStateStore.java
cat -n token-ledger-budget/src/main/java/io/tokenledger/budget/internal/InMemoryBudgetStateStore.java
# Find the Cost implementation and evaluator
rg -n "class Cost|record Cost|interface Cost|currency\(\)|add\(|CURRENCY_MISMATCH|evaluate\(" token-ledger-budget/src/main/java -S
# Print the relevant Cost and evaluator files if found
for f in $(git ls-files 'token-ledger-budget/src/main/java/**' | rg 'Cost\.java|DefaultBudgetEvaluator\.java'); do
echo "===== $f ====="
wc -l "$f"
cat -n "$f"
doneRepository: tokenpliot/tokenpilot
Length of output: 8069
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect Cost and store contract
wc -l token-ledger-core/src/main/java/io/tokenledger/core/domain/Cost.java \
token-ledger-budget/src/main/java/io/tokenledger/budget/BudgetStateStore.java
cat -n token-ledger-core/src/main/java/io/tokenledger/core/domain/Cost.java
printf '\n===== BudgetStateStore =====\n'
cat -n token-ledger-budget/src/main/java/io/tokenledger/budget/BudgetStateStore.java
# Check whether addCost/getAccumulatedCost are used elsewhere in a way that depends on currency matching
rg -n "addCost\\(|getAccumulatedCost\\(" token-ledger-budget token-ledger-core -SRepository: tokenpliot/tokenpilot
Length of output: 5166
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect relevant tests and usages around currency mismatch
rg -n "CURRENCY_MISMATCH|currency mismatch|addCost\\(|getAccumulatedCost\\(" token-ledger-budget/src/test token-ledger-budget/src/main -S
# Show the DefaultBudgetEvaluator tests around the store interaction
wc -l token-ledger-budget/src/test/java/io/tokenledger/budget/internal/DefaultBudgetEvaluatorTest.java
cat -n token-ledger-budget/src/test/java/io/tokenledger/budget/internal/DefaultBudgetEvaluatorTest.java
# Probe JDK merge/remapping exception behavior with a tiny standalone program
cat >/tmp/merge_probe.java <<'EOF'
import java.util.concurrent.ConcurrentHashMap;
import java.util.*;
public class merge_probe {
public static void main(String[] args) {
var map = new ConcurrentHashMap<String, String>();
map.put("k", "USD");
try {
map.merge("k", "EUR", (a,b) -> { throw new IllegalArgumentException("boom"); });
System.out.println("no-throw");
} catch (Exception e) {
System.out.println(e.getClass().getName() + ":" + e.getMessage());
}
System.out.println(map.get("k"));
}
}
EOF
javac /tmp/merge_probe.java && java -cp /tmp merge_probeRepository: tokenpliot/tokenpilot
Length of output: 7744
InMemoryBudgetStateStore#getAccumulatedCost/addCost(28-35)에서 통화도 상태 키에 포함하세요
key(tags)가 tenant_id만 써서 같은 태그에 서로 다른 통화가 섞일 수 있습니다.
getAccumulatedCost(tags, currency)가currency를 무시해 요청한 통화와 다른Cost를 그대로 반환할 수 있습니다.- 그 상태에서
DefaultBudgetEvaluator.evaluate()의accumulated.add(cost)가Cost.add()의 통화 검증에서 예외를 던질 수 있습니다. addCost()만 검증해도 읽기 경로가 남으니, 통화를 키에 포함하거나 조회/추가 모두에서 동일하게 차단하세요.
🤖 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-ledger-budget/src/main/java/io/tokenledger/budget/internal/InMemoryBudgetStateStore.java`
around lines 28 - 30, Update InMemoryBudgetStateStore#getAccumulatedCost and
addCost so the state key includes both the tenant tags and currency, using the
same key construction for reads and writes. Ensure requests for different
currencies cannot share or return the same accumulated Cost.
| Cost result = manager.record("unknown-model", usage, Map.of()); | ||
|
|
||
| assertThat(result.value()).isEqualByComparingTo(BigDecimal.ZERO); | ||
| assertThat(result.amount()).isEqualByComparingTo(BigDecimal.ZERO); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
누락 요금제의 기본 통화도 검증하세요.
현재는 금액만 검증하므로 USD fallback이 다른 통화로 바뀌어도 테스트가 통과합니다. 명시적 통화는 이번 변경의 계약이므로 함께 확인해야 합니다.
수정 예시
+import java.util.Currency;
+
assertThat(result.amount()).isEqualByComparingTo(BigDecimal.ZERO);
+assertThat(result.currency()).isEqualTo(Currency.getInstance("USD"));📝 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
| assertThat(result.amount()).isEqualByComparingTo(BigDecimal.ZERO); | |
| assertThat(result.amount()).isEqualByComparingTo(BigDecimal.ZERO); | |
| assertThat(result.currency()).isEqualTo(Currency.getInstance("USD")); |
🤖 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-ledger-core/src/test/java/io/tokenledger/core/internal/DefaultLedgerManagerTest.java`
at line 56, 기본 요금제 테스트에서 result.amount()만 검증하지 말고 결과의 통화 필드/접근자도 함께 검증하도록 수정하세요.
누락 요금제의 fallback 통화가 USD임을 명시적으로 확인하고, 기존 금액 검증은 유지하세요.
Rigu1
force-pushed
the
fix/money-invariants-rounding
branch
from
July 23, 2026 12:00
6713330 to
59dc355
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
token-pilot-core/src/main/java/io/tokenpilot/core/domain/PricingPlan.java (1)
22-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win기본 통화 상수를 불변으로 만들고 모든 기본 생성 경로에서 재사용하세요.
현재
DEFAULT_CURRENCY는 변경 가능한 package-private 필드이며,Line 48의 3-인자 생성자는 여전히"USD"를 직접 사용합니다. 한 경로의 기본 통화만 변경되면 생성 방식에 따라 결과가 달라질 수 있습니다.수정 예시
- static String DEFAULT_CURRENCY = "USD"; + private static final String DEFAULT_CURRENCY = "USD"; - this(modelId, promptPricePerK, completionPricePerK, Currency.getInstance("USD")); + this(modelId, promptPricePerK, completionPricePerK, Currency.getInstance(DEFAULT_CURRENCY));🤖 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-core/src/main/java/io/tokenpilot/core/domain/PricingPlan.java` at line 22, PricingPlan의 DEFAULT_CURRENCY를 static final 상수로 변경하고, 3-인자 생성자를 포함한 모든 기본 생성 경로에서 직접 지정한 "USD" 대신 DEFAULT_CURRENCY를 재사용하세요.
🤖 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-core/src/main/java/io/tokenpilot/core/domain/PricingPlan.java`:
- Around line 24-25: Update the PricingPlan constructor to handle an empty rates
map safely, since PricingPlanProperties may provide Map.of() when rates are
omitted. Initialize the EnumMap with TokenType.class before copying rates, or
explicitly reject empty input if that matches the intended contract, while
preserving the unmodifiable-map behavior.
---
Nitpick comments:
In `@token-pilot-core/src/main/java/io/tokenpilot/core/domain/PricingPlan.java`:
- Line 22: PricingPlan의 DEFAULT_CURRENCY를 static final 상수로 변경하고, 3-인자 생성자를 포함한
모든 기본 생성 경로에서 직접 지정한 "USD" 대신 DEFAULT_CURRENCY를 재사용하세요.
🪄 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: 3ee4e023-a218-4c43-9822-291ac508e7b4
📒 Files selected for processing (18)
README.mdtoken-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-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/domain/PricingPlan.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-core/src/test/java/io/tokenpilot/core/internal/DefaultLedgerManagerTest.java
Comment on lines
24
to
25
| public PricingPlan { | ||
| rates = Collections.unmodifiableMap(new EnumMap<>(rates)); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
jshell <<'EOF'
import java.util.*;
Map<Thread.State, Integer> empty = Map.of();
try {
new EnumMap<Thread.State, Integer>(empty);
throw new AssertionError("IllegalArgumentException이 예상됩니다");
} catch (IllegalArgumentException expected) {
System.out.println("verified");
}
EOFRepository: tokenpliot/tokenpilot
Length of output: 203
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
git ls-files | rg 'token-pilot-core/src/main/java/io/tokenpilot/core/domain/PricingPlan.java|token-pilot-core/src/test|PricingPlan'Repository: tokenpliot/tokenpilot
Length of output: 981
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
sed -n '1,200p' token-pilot-core/src/main/java/io/tokenpilot/core/domain/PricingPlan.javaRepository: tokenpliot/tokenpilot
Length of output: 2953
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('token-pilot-core/src/main/java/io/tokenpilot/core/domain/PricingPlan.java')
print(p.exists(), p)
PYRepository: tokenpliot/tokenpilot
Length of output: 238
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
sed -n '1,220p' token-pilot-core/src/main/java/io/tokenpilot/core/domain/PricingPlan.javaRepository: tokenpliot/tokenpilot
Length of output: 2953
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '\n== PricingPlan usages ==\n'
rg -n --no-heading "new PricingPlan\\(" token-pilot-core token-pilot-autoconfigure || true
printf '\n== Prices properties / defaults ==\n'
sed -n '1,220p' token-pilot-autoconfigure/src/main/java/io/tokenpilot/autoconfigure/PricingPlanProperties.java
printf '\n== EnumMap empty-map probe ==\n'
cat >/tmp/EnumMapEmptyProbe.java <<'EOF'
import java.util.*;
public class EnumMapEmptyProbe {
enum K { A }
public static void main(String[] args) {
Map<K, Integer> empty = new HashMap<>();
try {
new EnumMap<K, Integer>(empty);
System.out.println("NO_EXCEPTION");
} catch (Throwable t) {
System.out.println(t.getClass().getName());
System.out.println(t.getMessage());
}
}
}
EOF
javac /tmp/EnumMapEmptyProbe.java && java -cp /tmp EnumMapEmptyProbeRepository: tokenpliot/tokenpilot
Length of output: 2551
빈 rates 입력을 처리하세요 PricingPlanProperties가 기본값으로 Map.of()를 넘기므로, 설정에서 rates를 생략하면 new EnumMap<>(rates)에서 예외가 납니다. TokenType.class로 초기화한 뒤 putAll 하거나 빈 맵을 명시적으로 거부하세요.
🤖 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-core/src/main/java/io/tokenpilot/core/domain/PricingPlan.java`
around lines 24 - 25, Update the PricingPlan constructor to handle an empty
rates map safely, since PricingPlanProperties may provide Map.of() when rates
are omitted. Initialize the EnumMap with TokenType.class before copying rates,
or explicitly reject empty input if that matches the intended contract, while
preserving the unmodifiable-map behavior.
Rigu1
force-pushed
the
fix/money-invariants-rounding
branch
from
July 23, 2026 12:19
59dc355 to
5cd6d85
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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-ledger-core/src/test/java/io/tokenledger/core/internal/DefaultCostCalculatorTest.java`:
- Around line 81-93: Update the `@DisplayName` and
shouldUseRateAsCostForOneThousandTokens test method name to describe a one-token
calculation, matching TokenUsage.from(1, 0) and the asserted cost while
preserving the test logic.
🪄 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: ebe216f5-161a-42f4-a95c-eea45eb40bbb
📒 Files selected for processing (18)
README.mdtoken-ledger-budget/src/main/java/io/tokenledger/budget/BudgetDecision.javatoken-ledger-budget/src/main/java/io/tokenledger/budget/BudgetEvaluator.javatoken-ledger-budget/src/main/java/io/tokenledger/budget/BudgetState.javatoken-ledger-budget/src/main/java/io/tokenledger/budget/BudgetStateStore.javatoken-ledger-budget/src/main/java/io/tokenledger/budget/internal/DefaultBudgetEvaluator.javatoken-ledger-budget/src/main/java/io/tokenledger/budget/internal/InMemoryBudgetStateStore.javatoken-ledger-budget/src/main/java/io/tokenledger/budget/internal/LedgerBudgetComponents.javatoken-ledger-budget/src/test/java/io/tokenledger/budget/internal/DefaultBudgetEvaluatorTest.javatoken-ledger-core/src/main/java/io/tokenledger/core/RoundingPolicy.javatoken-ledger-core/src/main/java/io/tokenledger/core/domain/Cost.javatoken-ledger-core/src/main/java/io/tokenledger/core/domain/PricingPlan.javatoken-ledger-core/src/main/java/io/tokenledger/core/internal/DefaultCostCalculator.javatoken-ledger-core/src/main/java/io/tokenledger/core/internal/DefaultLedgerManager.javatoken-ledger-core/src/test/java/io/tokenledger/core/RoundingPolicyTest.javatoken-ledger-core/src/test/java/io/tokenledger/core/domain/CostTest.javatoken-ledger-core/src/test/java/io/tokenledger/core/internal/DefaultCostCalculatorTest.javatoken-ledger-core/src/test/java/io/tokenledger/core/internal/DefaultLedgerManagerTest.java
Comment on lines
+81
to
+93
| @Test | ||
| @DisplayName("1,000 token은 1K 단가 그대로 계산되어야 한다") | ||
| void shouldUseRateAsCostForOneThousandTokens() { | ||
| PricingPlan plan = new PricingPlan( | ||
| "tiny-model", | ||
| new BigDecimal("0.0004"), | ||
| BigDecimal.ZERO | ||
| ); | ||
| TokenUsage usage = TokenUsage.from(1, 0); | ||
|
|
||
| Cost cost = calculator.calculate(usage, plan); | ||
|
|
||
| assertThat(cost.amount()).isEqualByComparingTo("0.0000004"); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
테스트 이름을 실제 입력과 맞추세요.
Line 89는 1 토큰을 계산하지만 표시명과 메서드명은 1,000 토큰을 검증한다고 설명합니다. 미세 비용 보존 회귀 테스트가 목적이라면 이름을 1 토큰 기준으로 바꾸세요.
수정 예시
- `@DisplayName`("1,000 token은 1K 단가 그대로 계산되어야 한다")
- void shouldUseRateAsCostForOneThousandTokens() {
+ `@DisplayName`("1 token 비용은 1K 단가의 1/1000으로 계산되어야 한다")
+ void shouldCalculateFractionalRateForSingleToken() {📝 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
| @Test | |
| @DisplayName("1,000 token은 1K 단가 그대로 계산되어야 한다") | |
| void shouldUseRateAsCostForOneThousandTokens() { | |
| PricingPlan plan = new PricingPlan( | |
| "tiny-model", | |
| new BigDecimal("0.0004"), | |
| BigDecimal.ZERO | |
| ); | |
| TokenUsage usage = TokenUsage.from(1, 0); | |
| Cost cost = calculator.calculate(usage, plan); | |
| assertThat(cost.amount()).isEqualByComparingTo("0.0000004"); | |
| `@Test` | |
| `@DisplayName`("1 token 비용은 1K 단가의 1/1000으로 계산되어야 한다") | |
| void shouldCalculateFractionalRateForSingleToken() { | |
| PricingPlan plan = new PricingPlan( | |
| "tiny-model", | |
| new BigDecimal("0.0004"), | |
| BigDecimal.ZERO | |
| ); | |
| TokenUsage usage = TokenUsage.from(1, 0); | |
| Cost cost = calculator.calculate(usage, plan); | |
| assertThat(cost.amount()).isEqualByComparingTo("0.0000004"); |
🤖 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-ledger-core/src/test/java/io/tokenledger/core/internal/DefaultCostCalculatorTest.java`
around lines 81 - 93, Update the `@DisplayName` and
shouldUseRateAsCostForOneThousandTokens test method name to describe a one-token
calculation, matching TokenUsage.from(1, 0) and the asserted cost while
preserving the test logic.
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.
변경 사항
Costcanonical constructor에서amount와currency의null을 거부하도록 정리했습니다.Cost불변식을 강화했습니다.Cost생성 시 자동scale 6, HALF_UP반올림을 제거해 내부 정밀도를 보존했습니다.Cost.zero()의 암묵적 USD 선택을 제거하고Cost.zero(currency)로 통화를 명시하도록 변경했습니다.Cost.add(...)와Cost.compareTo(...)는 같은 통화에서만 허용하도록 했습니다.Cost.equals/hashCode정책을 정리했습니다.DefaultCostCalculator의 중간 반올림을 제거하고 1K 단가 계산을rate.multiply(count).movePointLeft(3)방식으로 변경했습니다.RoundingPolicy.COST_BOUNDARY_ROUNDING을 추가했습니다.BigDecimal금액 경계를Cost기반 API로 이관했습니다.BudgetDecision,BudgetEvaluator,BudgetStateStore,DefaultBudgetEvaluator,InMemoryBudgetStateStore가 currency-aware 금액을 다루도록 변경했습니다.CURRENCY_MISMATCH를 반환하도록 했습니다.배경
기존 구현은 비용 계산 중간 또는
Cost생성 시점에 반올림을 적용했습니다. 이로 인해0.0000004 USD처럼 작은 요청 비용이0.000000으로 사라질 수 있고, 요청별 반올림 오차가 누적될 수 있었습니다.또한 Budget API가
BigDecimal만 사용해 금액의 통화를 고려하지 않는 Budget public API에 통화 없는 새 금액 경계가 생겼기 때문에, budget limit과 actual/reservation cost의 통화를 안전하게 비교하기 어려웠습니다.이번 변경은 Money ADR의 정밀도 정책에 맞춰 내부 금액은 반올림 없이 보존하고, 외부 경계에서만 명시적인 정책으로 최종 반올림하도록 정리합니다.
비용 계산 흐름:
수치 예시:
호환성
Cost.value()는Cost.amount()로 변경됩니다.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을 명시적으로 적용해야 합니다.체크리스트
필수 테스트
0.0000004비용이Cost생성 시0이 되지 않는다.HALF_UP이 한 번 적용된다.zero(currency)가 지정 통화를 보존한다.Acceptance Criteria
:token-ledger-core:test,:token-ledger-budget:test가 통과한다.Closes #22