feat(payment): clickhouse and dual querying - #35
Merged
Conversation
📝 WalkthroughWalkthroughAdds PAYMENT event support: ClickHouse ChangesPAYMENT Event Type Storage
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 1
🧹 Nitpick comments (1)
src/storage/adapter/clickhouse/handlers/queryEvents.ts (1)
290-290: ⚡ Quick winPrefer explicit table-to-eventType mapping over chained ternaries.
The nested ternary assumes exactly three tables and will default to
PAYMENTfor any unrecognized table name. Consider using a lookup object or switch statement for clearer intent and safer future maintenance.♻️ Proposed refactor using a lookup map
Define a constant at the top of the file:
+const TABLE_TO_EVENT_TYPE: Record<string, string> = { + sdk_call_events: "SDK_CALL", + ai_token_usage_events: "AI_TOKEN_USAGE", + payment_events: "PAYMENT", +};Then replace the ternary:
- `'${t === "sdk_call_events" ? "SDK_CALL" : t === "ai_token_usage_events" ? "AI_TOKEN_USAGE" : "PAYMENT"}' as group_value` + `'${TABLE_TO_EVENT_TYPE[t] ?? "UNKNOWN"}' as group_value`🤖 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 `@src/storage/adapter/clickhouse/handlers/queryEvents.ts` at line 290, Replace the nested ternary that produces group_value (the expression using t === "sdk_call_events" ? "SDK_CALL" : ...) with a clear lookup map (e.g., TABLE_TO_EVENT_TYPE) defined at the top of the module and use TABLE_TO_EVENT_TYPE[t] (with a safe default or explicit error) to produce the group_value; update any references in queryEvents.ts that use the variable t to use the lookup result so adding new tables is simple and avoids silently defaulting to "PAYMENT".
🤖 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 `@src/storage/adapter/clickhouse/ClickHouseAdapter.ts`:
- Line 16: Remove the cross-adapter import of handlePriceRequestPayment from the
Postgres handlers and instead create a ClickHouse-specific handler module under
src/storage/adapter/clickhouse/handlers/ (e.g., priceRequestPayment.ts) that
implements the same interface/behavior used by ClickHouseAdapter; then update
ClickHouseAdapter to import handlePriceRequestPayment from the new clickhouse
handlers module, keeping the existing call sites and factory usage unchanged
(reference symbols: handlePriceRequestPayment, ClickHouseAdapter).
---
Nitpick comments:
In `@src/storage/adapter/clickhouse/handlers/queryEvents.ts`:
- Line 290: Replace the nested ternary that produces group_value (the expression
using t === "sdk_call_events" ? "SDK_CALL" : ...) with a clear lookup map (e.g.,
TABLE_TO_EVENT_TYPE) defined at the top of the module and use
TABLE_TO_EVENT_TYPE[t] (with a safe default or explicit error) to produce the
group_value; update any references in queryEvents.ts that use the variable t to
use the lookup result so adding new tables is simple and avoids silently
defaulting to "PAYMENT".
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fe1cca8c-497f-43b7-833f-f138820fc30a
📒 Files selected for processing (9)
protosrc/factory/EventStorageAdapterFactory.tssrc/interface/storage/Storage.tssrc/storage/adapter/clickhouse/ClickHouseAdapter.tssrc/storage/adapter/clickhouse/handlers/addPayment.tssrc/storage/adapter/clickhouse/handlers/index.tssrc/storage/adapter/clickhouse/handlers/queryEvents.tssrc/storage/adapter/clickhouse/schema.tssrc/storage/adapter/postgres/handlers/queryEvents.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/storage/adapter/common/priceRequestPayment.ts (1)
6-62: ⚡ Quick winAdd operation logging for payment price calculation flow.
Please log success/failure with operation name and context for this handler path.
As per coding guidelines,
Use logger from errors/logger with logOperationInfo and logOperationError; include operation name and context.🤖 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 `@src/storage/adapter/common/priceRequestPayment.ts` around lines 6 - 62, The handler handlePriceRequestPayment is missing operation logging: import logOperationInfo and logOperationError from errors/logger, then add logOperationInfo at the start (operation: "handlePriceRequestPayment", context: { userId, beforeTimestamp }) and another logOperationInfo on successful completion before returning totalPrice with context { userId, beforeTimestamp, sdkPrice, aiPrice, totalPrice }; in the catch block call logOperationError with the same operation name and include the error and context (userId, beforeTimestamp) before rethrowing (preserve existing StorageError throws and still wrap non-StorageError errors via StorageError.priceCalculationFailed).
🤖 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 `@src/storage/adapter/common/priceRequestPayment.ts`:
- Around line 23-24: The current validation uses typeof + isNaN (e.g., if
(typeof sdkPrice !== "number" || isNaN(sdkPrice))) which accepts
Infinity/-Infinity; replace those checks with Number.isFinite(sdkPrice) (i.e.,
if (!Number.isFinite(sdkPrice)) throw StorageError.priceCalculationFailed(...))
and do the same replacement for the other price validation site in the same
module so both sdkPrice and the other price variable use Number.isFinite().
- Around line 48-53: Create a typed user-defined type guard to replace the `(e
as any)` cast in the catch logic: add a function like `isStorageError(e:
unknown): e is StorageError` that checks `e !== null`, `typeof e === "object"`,
`'type' in e`, and `'name' in e && typeof (e as { name?: unknown }).name ===
"string" && (e as { name: string }).name === "StorageError"`, then use `if
(isStorageError(e)) { ... }` in the catch block instead of the current `(e as
any)` pattern so the error narrowing is strict and no `any` is used.
---
Nitpick comments:
In `@src/storage/adapter/common/priceRequestPayment.ts`:
- Around line 6-62: The handler handlePriceRequestPayment is missing operation
logging: import logOperationInfo and logOperationError from errors/logger, then
add logOperationInfo at the start (operation: "handlePriceRequestPayment",
context: { userId, beforeTimestamp }) and another logOperationInfo on successful
completion before returning totalPrice with context { userId, beforeTimestamp,
sdkPrice, aiPrice, totalPrice }; in the catch block call logOperationError with
the same operation name and include the error and context (userId,
beforeTimestamp) before rethrowing (preserve existing StorageError throws and
still wrap non-StorageError errors via StorageError.priceCalculationFailed).
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 54666d90-704e-4009-8d58-3b5b604609ef
📒 Files selected for processing (5)
src/storage/adapter/clickhouse/ClickHouseAdapter.tssrc/storage/adapter/clickhouse/handlers/index.tssrc/storage/adapter/clickhouse/handlers/priceRequestPayment.tssrc/storage/adapter/common/priceRequestPayment.tssrc/storage/adapter/postgres/handlers/priceRequestPayment.ts
✅ Files skipped from review due to trivial changes (1)
- src/storage/adapter/clickhouse/handlers/priceRequestPayment.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/storage/adapter/clickhouse/handlers/index.ts
- src/storage/adapter/clickhouse/ClickHouseAdapter.ts
SteakFisher
force-pushed
the
feat/payment_clickhouse
branch
from
May 13, 2026 18:12
b6d6984 to
29860ae
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 `@src/storage/adapter/common/priceRequestPayment.ts`:
- Around line 30-39: Remove the corrupted validation block that references
aiPrice before it's declared and contains orphaned lines for sdkPrice;
specifically delete the early Number.isFinite(aiPrice) check and the stray lines
that look like a partial throw StorageError.priceCalculationFailed(...) so only
the proper aiPrice validation (the existing check around aiPrice assigned later)
remains; ensure no leftover orphaned references to aiPrice or sdkPrice exist and
that only the correct StorageError.priceCalculationFailed(...) usages remain.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 68db90ae-3779-475a-a22e-d3f15779ade0
📒 Files selected for processing (1)
src/storage/adapter/common/priceRequestPayment.ts
Comment on lines
+30
to
+39
| if (!Number.isFinite(aiPrice)) { | ||
| throw StorageError.priceCalculationFailed( | ||
| userId, | ||
| new Error(`Invalid AI price value returned: ${aiPrice}`) | ||
| ); | ||
| } | ||
| userId, | ||
| new Error(`Invalid SDK price value returned: ${sdkPrice}`) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Critical: Corrupted code block breaks compilation and references undefined variable.
Lines 30-39 contain two problems causing CI failures:
aiPriceis referenced on line 30 before it's declared on line 43- Lines 36-39 are orphaned code fragments (missing the
throw StorageError.priceCalculationFailed(prefix)
This entire block must be removed. The aiPrice validation already exists at lines 49-54 after aiPrice is properly assigned.
🐛 Proposed fix: Remove corrupted lines
if (!Number.isFinite(sdkPrice)) {
throw StorageError.priceCalculationFailed(
userId,
new Error(`Invalid SDK price value returned: ${sdkPrice}`)
);
}
-
- if (!Number.isFinite(aiPrice)) {
- throw StorageError.priceCalculationFailed(
- userId,
- new Error(`Invalid AI price value returned: ${aiPrice}`)
- );
- }
- userId,
- new Error(`Invalid SDK price value returned: ${sdkPrice}`)
- );
- }
const aiAdapter =📝 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
| if (!Number.isFinite(aiPrice)) { | |
| throw StorageError.priceCalculationFailed( | |
| userId, | |
| new Error(`Invalid AI price value returned: ${aiPrice}`) | |
| ); | |
| } | |
| userId, | |
| new Error(`Invalid SDK price value returned: ${sdkPrice}`) | |
| ); | |
| } | |
| if (!Number.isFinite(sdkPrice)) { | |
| throw StorageError.priceCalculationFailed( | |
| userId, | |
| new Error(`Invalid SDK price value returned: ${sdkPrice}`) | |
| ); | |
| } | |
| const aiAdapter = |
🧰 Tools
🪛 Biome (2.4.15)
[error] 38-38: Expected a statement but instead found ')'.
(parse)
🪛 GitHub Actions: Run Tests / 0_Test.txt
[error] 38-38: TypeScript (TS1128): Declaration or statement expected.
🪛 GitHub Actions: Run Tests / Test
[error] 38-38: TypeScript error TS1128: Declaration or statement expected.
🤖 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 `@src/storage/adapter/common/priceRequestPayment.ts` around lines 30 - 39,
Remove the corrupted validation block that references aiPrice before it's
declared and contains orphaned lines for sdkPrice; specifically delete the early
Number.isFinite(aiPrice) check and the stray lines that look like a partial
throw StorageError.priceCalculationFailed(...) so only the proper aiPrice
validation (the existing check around aiPrice assigned later) remains; ensure no
leftover orphaned references to aiPrice or sdkPrice exist and that only the
correct StorageError.priceCalculationFailed(...) usages remain.
This was referenced May 14, 2026
Merged
Merged
This was referenced May 18, 2026
Merged
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.
Summary by CodeRabbit
New Features
Bug Fixes
Chores