fix(hermes): harden attribution and database reads - #66
Conversation
|
Warning Review limit reached
Next review available in: 27 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (15)
WalkthroughHermes 사용량 읽기를 SQLite 스냅샷 기반으로 변경했습니다. 모델별 토큰과 비용 귀속, 가격 산정 시각, 원장 baseline, 미귀속 이벤트 집계를 추가했습니다. 원격 스냅샷 비용 필드와 관련 회귀 테스트도 확장했습니다. ChangesHermes 사용량 파이프라인
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant HermesReader
participant HermesSQLiteConnection
participant HermesUsageResolver
participant HermesUsageLedger
participant RemoteUsageMapper
participant UsageReportModelStats
HermesReader->>HermesSQLiteConnection: SQLite 스냅샷에서 사용량 읽기
HermesSQLiteConnection-->>HermesReader: 최신 데이터베이스 행 반환
HermesReader->>HermesUsageResolver: 모델별 사용량과 pricingTimestamp 전달
HermesUsageResolver->>HermesUsageLedger: 모델별 비용과 카운터 적용
HermesUsageLedger-->>RemoteUsageMapper: 토큰·비용 이벤트 전달
RemoteUsageMapper-->>UsageReportModelStats: 미귀속 및 비용 전용 이벤트 전달
UsageReportModelStats-->>UsageReportModelStats: Mixed / Unattributed로 집계
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f803fbcf76
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
choi138
added
the
🤖 codex: needs work
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Toki/Domain/Usage/UsageReportModelStats.swift (1)
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win미귀속 모델 식별자를 공유 상수로 노출하세요.
unattributedModelID는 이 파일에서만private으로 정의됩니다. 동일한 문자열"Mixed / Unattributed"가TokiTests/UsageServiceBehaviorTests.swift(라인 251),Tests/TokiAgentTests/AgentSnapshotModelAttributionTests.swift(라인 41)에 하드코딩되어 있습니다. 값을 변경하면 테스트는 실패하지 않고 낡은 문자열을 계속 비교합니다. 이 식별자는 집계 키이면서 화면에 표시되는 문자열입니다. 공유 가능한 상수로 노출하고 테스트와 표시 계층이 그 상수를 참조하게 하세요.또한 표시 문자열을 데이터 키로 사용하면 실제 모델 ID와 충돌할 수 있습니다. 키와 표시 문자열을 분리하는 방안도 검토하세요.
Also applies to: 132-132
🤖 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 `@Toki/Domain/Usage/UsageReportModelStats.swift` at line 5, Expose the unattributed model identifier through a shared, non-private constant associated with UsageReportModelStats, and update all tests and presentation code to reference it instead of hardcoding the string. Separate the aggregation key from the user-facing “Mixed / Unattributed” label so a real model ID cannot collide with the display value.
🤖 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 `@TokiTests/HermesPricingRefreshTests.swift`:
- Line 437: Rename the test method
test_hermesReader_allocatesInitialDerivedCostsAtModelUsagePricingInstant to
reflect that HermesReader prices usage at read time, such as
test_hermesReader_allocatesInitialDerivedCostsAtReadTimePricingInstant. Keep the
test behavior and assertions unchanged.
---
Nitpick comments:
In `@Toki/Domain/Usage/UsageReportModelStats.swift`:
- Line 5: Expose the unattributed model identifier through a shared, non-private
constant associated with UsageReportModelStats, and update all tests and
presentation code to reference it instead of hardcoding the string. Separate the
aggregation key from the user-facing “Mixed / Unattributed” label so a real
model ID cannot collide with the display value.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c2d10f80-2009-482c-a276-60920415d929
📒 Files selected for processing (14)
Sources/TokiUsageReaders/HermesReader.swiftSources/TokiUsageReaders/HermesSQLiteConnection.swiftSources/TokiUsageReaders/HermesUsageAttribution.swiftSources/TokiUsageReaders/HermesUsageCostBreakdown.swiftSources/TokiUsageReaders/HermesUsageLedger.swiftSources/TokiUsageReaders/HermesUsageLedgerSupport.swiftSources/TokiUsageReaders/HermesUsageObservation.swiftSources/TokiUsageReaders/HermesUsageResolver.swiftTests/TokiAgentTests/AgentSnapshotModelAttributionTests.swiftToki/Domain/Usage/UsageReportModelStats.swiftTokiTests/HermesPricingRefreshTests.swiftTokiTests/HermesReaderTests.swiftTokiTests/HermesUsageLedgerTests.swiftTokiTests/UsageServiceBehaviorTests.swift
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 538acb8a8e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
choi138
removed
the
🤖 codex: needs work
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2bf7dfb0a0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
choi138
added
the
🤖 codex: needs work
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2bf7dfb0a0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
choi138
removed
the
🤖 codex: needs work
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/TokiUsageReaders/HermesUsageLedger.swift (1)
221-261: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win초기 zero-token 비용 관찰값을 기록하지 않도록 개선하십시오.
applyInitial이observation.counters.totalTokens > 0에서false를 반환하면 기준선과 미귀속 사용량 모두 저장되지 않습니다.validateHermesUsageObservation은cost >= 0만 요구하므로costs > 0인 zero-token 관찰값은 유효하지만, 현재는 이 비용이 Ledger와HermesUsageLedgerStatus.unattributedTokens에서 모두 누락됩니다. 첫 호출에서candidate.baselines[identifier] = currentBaseline을 먼저 저장한 뒤, cost-only 초기 사용량을 이벤트 또는 미귀속 경로로 처리하도록 맞추는 것이 필요합니다.🤖 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 `@Sources/TokiUsageReaders/HermesUsageLedger.swift` around lines 221 - 261, Update applyInitial so zero-token observations with positive cost are retained: store candidate.baselines[identifier] before the early-return logic, and allow cost-only initial observations to proceed through the existing dated event or addUnattributed path. Continue returning false only when the observation has neither token usage nor reportable cost, while preserving normal token-bearing behavior.
🤖 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.
Outside diff comments:
In `@Sources/TokiUsageReaders/HermesUsageLedger.swift`:
- Around line 221-261: Update applyInitial so zero-token observations with
positive cost are retained: store candidate.baselines[identifier] before the
early-return logic, and allow cost-only initial observations to proceed through
the existing dated event or addUnattributed path. Continue returning false only
when the observation has neither token usage nor reportable cost, while
preserving normal token-bearing behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 95363901-f5b9-4307-8b00-f2f2b6b555e4
📒 Files selected for processing (20)
Sources/TokiUsageCore/RawTokenUsage.swiftSources/TokiUsageReaders/HermesReader.swiftSources/TokiUsageReaders/HermesSQLiteConnection.swiftSources/TokiUsageReaders/HermesUsageAttribution.swiftSources/TokiUsageReaders/HermesUsageCostBreakdown.swiftSources/TokiUsageReaders/HermesUsageLedger.swiftSources/TokiUsageReaders/HermesUsageLedgerSupport.swiftSources/TokiUsageReaders/HermesUsageResolver.swiftTests/TokiAgentTests/AgentSnapshotModelAttributionTests.swiftToki/Domain/Usage/UsageData.swiftToki/Domain/Usage/UsageReportBuilder.swiftToki/Domain/Usage/UsageReportExport.swiftToki/Domain/Usage/UsageReportModelStats.swiftToki/Features/UsagePanel/PanelStatComponents.swiftTokiTests/HermesPricingRefreshTests.swiftTokiTests/HermesReaderTests.swiftTokiTests/UsageExportTests.swiftTokiTests/UsageOriginAggregationTests.swiftTokiTests/UsageProjectTimelineReportTests.swiftTokiTests/UsageServiceBehaviorTests.swift
🚧 Files skipped from review as they are similar to previous changes (7)
- Toki/Domain/Usage/UsageReportModelStats.swift
- Tests/TokiAgentTests/AgentSnapshotModelAttributionTests.swift
- Sources/TokiUsageReaders/HermesUsageAttribution.swift
- Sources/TokiUsageReaders/HermesUsageLedgerSupport.swift
- Sources/TokiUsageReaders/HermesUsageCostBreakdown.swift
- Sources/TokiUsageReaders/HermesUsageResolver.swift
- Sources/TokiUsageReaders/HermesReader.swift
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1522601472
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
choi138
added
the
🤖 codex: needs work
|
CodeRabbit의 outside-diff 지적도 |
choi138
removed
the
🤖 codex: needs work
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26e9966307
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
choi138
added
the
🤖 codex: needs work
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f82e2ecc5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
choi138
removed
the
🤖 codex: needs work
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06631362fb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let residual = counters.subtracting(combinedCounters) | ||
| if residual.totalTokens > 0 { | ||
| parts.append(HermesUsageEventPart(model: nil, counters: residual)) |
There was a problem hiding this comment.
When the session counters exceed a partial session_model_usage breakdown but the session and every detail row agree on one model, HermesUsageResolver preserves that model in observation.model, yet this branch assigns the residual counters to nil. For example, a 150-token session and a 100-token detail row both labeled model A are reported as A=100 and Mixed / Unattributed=50, also misassigning the residual cost and activity. Use the resolved model for the residual when it is non-nil, reserving nil for genuinely ambiguous sessions, and add focused partial-detail reader coverage.
AGENTS.md reference: AGENTS.md:L38-L40
Useful? React with 👍 / 👎.
choi138
added
the
🤖 codex: needs work
The custom billing provider reports kr/claude-opus-5 as its own catalog entry rather than as a provider prefix on claude-opus-5, so usage under that ID was left unpriced. Add an explicit exact key at the claude-opus-5 rates. Keep the entry exact-only so it cannot lend its rates to future tiers, and cover that slash-bearing keys stay independent of prefix stripping.
Pick up the lane-based Codex review loop from #68 so the skill files are tracked here instead of lingering as a stale untracked copy.
Hermes, omo, and senpi write task logs and plans into the working tree, which left dozens of untracked files in every status check and risked committing local agent state.
When session counters exceeded a partial session_model_usage breakdown, the residual was always emitted with a nil model. A 150-token session with a 100-token detail row on the same model was therefore reported as 100 tokens for that model and 50 as Mixed / Unattributed, and the residual cost followed the same wrong split. The resolver leaves the observation model non-nil only when the session and every detail row agree on one model, so use it for the residual and reserve nil for genuinely ambiguous sessions. Merge into the existing part for that model so its pricing counters are not charged twice, and keep emitting an unattributed carrier when reported cost belongs to no model.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
choi138
removed
the
🤖 codex: needs work
1 participant
Summary
Root cause
Hermes session totals can combine several session_model_usage rows, but the ledger previously kept only a single optional session model. It also priced every model with a catalog entry before distributing the reported-cost remainder, which distorted per-model costs. Separately, SQLite could fail to open a checkpointed database in a non-writable directory even though no live sidecars were required.
Impact
Hermes totals remain unchanged while model and cost breakdowns stay accurate. Read-only checkpointed databases and live WAL databases are handled without dropping current usage.
Validation
Summary by CodeRabbit