refactor(cleanup): Rename from sdk to basic - #42
Conversation
📝 WalkthroughWalkthroughThis PR renames SDK_CALL terminology to BASIC_USAGE across the event system. Input schemas now accept ChangesSDK_CALL to BASIC_USAGE terminology refactor
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
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/zod/event.ts`:
- Line 41: The transform for BasicUsageDataSchema (the transform that returns {
basicUsageType: v.basicusagetype, debitAmount, metadata: ... }) and the
analogous transform in AITokenUsageDataSchema must not call JSON.parse directly;
instead, wrap the JSON.parse call in a try/catch inside the Zod transform
callback, and on parse failure call ctx.addIssue(...) to report a validation
error and return z.NEVER so the failure flows through Zod; update both
transforms to use the ctx parameter, catch SyntaxError from JSON.parse, add an
appropriate issue via ctx.addIssue, and return z.NEVER when metadata is
malformed, otherwise return the parsed metadata as before.
🪄 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: 890a4a46-efd4-4c4f-9c7d-7b390107953d
⛔ Files ignored due to path filters (4)
src/gen/event/v1/event_pb.d.tsis excluded by!**/gen/**src/gen/event/v1/event_pb.jsis excluded by!**/gen/**src/gen/query/v1/query_pb.d.tsis excluded by!**/gen/**src/gen/query/v1/query_pb.jsis excluded by!**/gen/**
📒 Files selected for processing (11)
src/routes/gRPC/query/queryEvents.tssrc/storage/adapter/clickhouse/ClickHouseAdapter.tssrc/storage/adapter/clickhouse/handlers/addBasicUsage.tssrc/storage/adapter/clickhouse/handlers/index.tssrc/storage/adapter/clickhouse/handlers/priceRequestBasicUsage.tssrc/storage/adapter/postgres/handlers/addBasicUsage.tssrc/storage/adapter/postgres/handlers/index.tssrc/storage/adapter/postgres/handlers/priceRequestBasicUsage.tssrc/storage/adapter/postgres/postgres.tssrc/utils/eventHelpers.tssrc/zod/event.ts
| debitAmount = v.amount; | ||
| } | ||
| return { basicUsageType: v.sdkcalltype, debitAmount, metadata: v.metadata ? JSON.parse(v.metadata) as Record<string, unknown> : undefined }; | ||
| return { basicUsageType: v.basicusagetype, debitAmount, metadata: v.metadata ? JSON.parse(v.metadata) as Record<string, unknown> : undefined }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the file to understand the context around line 41
head -50 src/zod/event.ts | cat -nRepository: ScrawnDotDev/Scrawn
Length of output: 2100
🏁 Script executed:
# Get more context around line 41
sed -n '35,50p' src/zod/event.ts | cat -nRepository: ScrawnDotDev/Scrawn
Length of output: 833
🏁 Script executed:
# Check the full transform function to understand the scope
sed -n '20,60p' src/zod/event.ts | cat -nRepository: ScrawnDotDev/Scrawn
Length of output: 1803
🏁 Script executed:
# Search for ctx.addIssue usage in transforms
rg "ctx\.addIssue" --type ts --type tsxRepository: ScrawnDotDev/Scrawn
Length of output: 91
🏁 Script executed:
# Search for z.NEVER usage
rg "z\.NEVER" --type ts --type tsxRepository: ScrawnDotDev/Scrawn
Length of output: 91
🏁 Script executed:
# Search for other .transform patterns with ctx parameter
rg "\.transform\s*\(\s*\(.*,\s*ctx\)" --type ts --type tsx -A 3Repository: ScrawnDotDev/Scrawn
Length of output: 91
🏁 Script executed:
# Check if there are other JSON.parse calls in Zod schemas
rg "JSON\.parse" --type ts --type tsx -B 3 -A 1Repository: ScrawnDotDev/Scrawn
Length of output: 91
🏁 Script executed:
# Check the Zod version in package.json
cat package.json | grep -A 2 -B 2 '"zod"'Repository: ScrawnDotDev/Scrawn
Length of output: 138
🏁 Script executed:
# Search for ctx.addIssue usage in transforms (TS files only)
rg "ctx\.addIssue" --type ts -B 2 -A 2Repository: ScrawnDotDev/Scrawn
Length of output: 45
🏁 Script executed:
# Search for z.NEVER usage
rg "z\.NEVER" --type tsRepository: ScrawnDotDev/Scrawn
Length of output: 45
🏁 Script executed:
# Search for other .transform patterns with ctx parameter
rg "\.transform\s*\(" --type ts -A 1 | head -40Repository: ScrawnDotDev/Scrawn
Length of output: 1739
🏁 Script executed:
# Check if there are other JSON.parse calls in Zod schemas
rg "JSON\.parse" --type ts -B 3 -A 1Repository: ScrawnDotDev/Scrawn
Length of output: 616
🏁 Script executed:
# Check how errors are handled in async transforms
rg "\.transform\(async" --type ts -A 15 | head -60Repository: ScrawnDotDev/Scrawn
Length of output: 1749
🏁 Script executed:
# Search for catch blocks in transforms or Zod usage
rg "catch" --type ts -B 3 -A 3 | grep -A 5 -B 5 "transform"Repository: ScrawnDotDev/Scrawn
Length of output: 45
🏁 Script executed:
# Check Zod error handling patterns in the codebase
rg "ZodError|parseAsync|\.parse\(" --type ts -B 2 -A 2 | head -50Repository: ScrawnDotDev/Scrawn
Length of output: 2723
🏁 Script executed:
# Look for other potential error sources in transforms
rg "fetchTagAmount|parseAndEvaluateExpr" --type ts -B 2 -A 5Repository: ScrawnDotDev/Scrawn
Length of output: 5667
🏁 Script executed:
# Verify Zod transform signature with ctx parameter in documentation/examples
# Check if there are any .refine patterns that might show error handling
rg "\.refine" --type ts -A 3 | head -30Repository: ScrawnDotDev/Scrawn
Length of output: 825
🏁 Script executed:
# Check the full AITokenUsageDataSchema transform to see if it also has JSON.parse
sed -n '44,100p' src/zod/event.ts | cat -nRepository: ScrawnDotDev/Scrawn
Length of output: 2320
🏁 Script executed:
# Look for how errors are currently being caught from the Zod schemas
rg "parseAsync\|\.parse\(" --type ts -B 2 -A 5 | head -50Repository: ScrawnDotDev/Scrawn
Length of output: 45
🏁 Script executed:
# Get the rest of the AITokenUsageDataSchema transform
sed -n '44,90p' src/zod/event.ts | cat -nRepository: ScrawnDotDev/Scrawn
Length of output: 1947
🏁 Script executed:
# Check how the schemas are actually used - look for parseAsync calls
rg "EventSchema\|BasicUsageDataSchema\|AITokenUsageDataSchema" --type ts -B 2 -A 5 | head -80Repository: ScrawnDotDev/Scrawn
Length of output: 45
🏁 Script executed:
# Search for actual error handling around schema validation
rg "try\|catch" --type ts src/routes -A 3 -B 1 | head -60Repository: ScrawnDotDev/Scrawn
Length of output: 45
🏁 Script executed:
# Check what happens when schemas are parsed - look at the event handler or route
rg "EventSchema" --type ts -A 10 | head -50Repository: ScrawnDotDev/Scrawn
Length of output: 3325
🏁 Script executed:
# Check if there's a pattern for using parseAsync instead of parse
rg "parseAsync" --type ts -B 3 -A 3Repository: ScrawnDotDev/Scrawn
Length of output: 995
🏁 Script executed:
# Find where these schemas are parsed/validated
fd -t f "\.ts$" | xargs grep -l "BasicUsageDataSchema\|AITokenUsageDataSchema" | head -10Repository: ScrawnDotDev/Scrawn
Length of output: 80
🏁 Script executed:
# Check a handler to see error flow
head -80 src/routes/http/addEvent.ts 2>/dev/null || head -80 src/routes/*/addEvent.ts 2>/dev/null || find src -name "*addEvent*" -type f | head -1 | xargs head -80Repository: ScrawnDotDev/Scrawn
Length of output: 1536
🏁 Script executed:
# Check the full registerEvent handler to see if there's error handling
cat -n src/routes/gRPC/events/registerEvent.ts | head -50Repository: ScrawnDotDev/Scrawn
Length of output: 2188
🏁 Script executed:
# Check if there's error handling wrapper around parseAsync calls
sed -n '1,70p' src/routes/gRPC/events/registerEvent.ts | cat -nRepository: ScrawnDotDev/Scrawn
Length of output: 2188
Add error handling to JSON.parse in both schema transforms to comply with validation guidelines.
The JSON.parse() calls in BasicUsageDataSchema (line 41) and AITokenUsageDataSchema will throw SyntaxError for invalid JSON, bypassing Zod's validation error path. Per the coding guidelines, Zod schemas must catch and convert all validation failures properly. Wrap the JSON.parse() calls in ctx.addIssue() with z.NEVER return to ensure malformed metadata is reported as a schema validation issue.
Proposed fix for BasicUsageDataSchema
- .transform(async (v): Promise<BasicUsageEventData> => {
+ .transform(async (v, ctx): Promise<BasicUsageEventData> => {
let debitAmount: number;
if (v.tag) {
debitAmount = await fetchTagAmount(v.tag, `Tag not found: ${v.tag}`);
} else if (v.expr) {
debitAmount = await parseAndEvaluateExpr(v.expr);
} else {
debitAmount = v.amount;
}
- return { basicUsageType: v.basicusagetype, debitAmount, metadata: v.metadata ? JSON.parse(v.metadata) as Record<string, unknown> : undefined };
+ let metadata: Record<string, unknown> | undefined;
+ if (v.metadata) {
+ try {
+ metadata = JSON.parse(v.metadata) as Record<string, unknown>;
+ } catch {
+ ctx.addIssue({
+ code: z.ZodIssueCode.custom,
+ path: ["metadata"],
+ message: "metadata must be valid JSON",
+ });
+ return z.NEVER;
+ }
+ }
+ return { basicUsageType: v.basicusagetype, debitAmount, metadata };
});Apply the same fix to AITokenUsageDataSchema's transform.
🤖 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/zod/event.ts` at line 41, The transform for BasicUsageDataSchema (the
transform that returns { basicUsageType: v.basicusagetype, debitAmount,
metadata: ... }) and the analogous transform in AITokenUsageDataSchema must not
call JSON.parse directly; instead, wrap the JSON.parse call in a try/catch
inside the Zod transform callback, and on parse failure call ctx.addIssue(...)
to report a validation error and return z.NEVER so the failure flows through
Zod; update both transforms to use the ctx parameter, catch SyntaxError from
JSON.parse, add an appropriate issue via ctx.addIssue, and return z.NEVER when
metadata is malformed, otherwise return the parsed metadata as before.
Summary by CodeRabbit
Bug Fixes
Refactor