Feature/proto driven sdk - #38
Merged
Merged
Conversation
Proto CreateAPIKeyRequest lacks a role field. Read role from the gRPC metadata header x-scrawn-role instead. Zod schema defaults to production if not specified. Only dashboard callers can create keys.
- SdkCallField, AiTokenField, PaymentField enums in query.proto - Per-table enums (UsersField, SessionsField, etc.) in data.proto - Regenerated proto stubs include new enum types
…eeping proto enums
…proto enums - query.ts + data.ts: OPERATOR_MAP, LOGICAL_MAP, AGGREGATION_TYPE_MAP now use proto Operator/LogicalOperator/AggregationType enum values - Remove hardcoded number literals — single source of truth is proto
📝 WalkthroughWalkthroughThe PR updates Zod schema validation modules in ChangesOperator and Logical Type Enum Mappings
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/zod/query.ts (1)
5-22: ⚡ Quick winMake enum validation key-driven across operator/logical/aggregation.
These maps are proto-keyed now, but schema checks still use fixed numeric bounds. That leaves room for map/validator drift and transform-to-
undefinedbehavior when enum values evolve.Proposed refactor
const OPERATOR_MAP = { [Operator.EQ]: "EQ", [Operator.GT]: "GT", [Operator.GTE]: "GTE", [Operator.LT]: "LT", [Operator.LTE]: "LTE", [Operator.NEQ]: "NEQ", } as const; @@ const AGGREGATION_TYPE_MAP = { [AggregationType.SUM]: "SUM", [AggregationType.COUNT]: "COUNT", } as const; @@ const LOGICAL_MAP = { [LogicalOperator.AND]: "AND", [LogicalOperator.OR]: "OR", } as const; + +const OPERATOR_CODES = new Set<number>(Object.keys(OPERATOR_MAP).map(Number)); +const AGGREGATION_CODES = new Set<number>(Object.keys(AGGREGATION_TYPE_MAP).map(Number)); +const LOGICAL_CODES = new Set<number>(Object.keys(LOGICAL_MAP).map(Number)); @@ operator: z .number() .int() - .min(1) - .max(6) - .transform((v) => OPERATOR_MAP[v as keyof typeof OPERATOR_MAP]), + .refine((v): v is keyof typeof OPERATOR_MAP => OPERATOR_CODES.has(v), { + error: "Invalid operator", + }) + .transform((v) => OPERATOR_MAP[v]), @@ logical: z .number() .int() - .min(0) - .max(2) - .transform( - (v) => LOGICAL_MAP[v as keyof typeof LOGICAL_MAP] - ), + .refine((v): v is keyof typeof LOGICAL_MAP => LOGICAL_CODES.has(v), { + error: "Invalid logical operator", + }) + .transform((v) => LOGICAL_MAP[v]), @@ type: z .number() .int() - .min(1) - .max(2) - .transform( - (v) => AGGREGATION_TYPE_MAP[v as keyof typeof AGGREGATION_TYPE_MAP] - ), + .refine( + (v): v is keyof typeof AGGREGATION_TYPE_MAP => AGGREGATION_CODES.has(v), + { error: "Invalid aggregation type" } + ) + .transform((v) => AGGREGATION_TYPE_MAP[v]),🤖 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/query.ts` around lines 5 - 22, The current operator/logical/aggregation validators use fixed numeric bounds which can drift from the maps; instead make validation key-driven by deriving allowed keys from OPERATOR_MAP, AGGREGATION_TYPE_MAP, and LOGICAL_MAP (e.g., use Object.keys or keyof typeof <MAP>) and build the Zod validators/enums from those derived keys so the schema always matches the transform maps; update any schema functions that reference numeric bounds to use these map-derived key sets (look for validations around Operator, AggregationType, LogicalOperator) and ensure transforms use the same maps to avoid producing undefined.src/zod/data.ts (1)
12-25: ⚡ Quick winDerive accepted codes from enum-key maps, not fixed numeric ranges.
OPERATOR_MAP/LOGICAL_MAPare now proto-driven, but the validators still use hardcoded.min()/.max()bounds. That can drift from enum values and allow transforms to returnundefinedfor unmapped codes.Proposed refactor
const OPERATOR_MAP = { [Operator.EQ]: "EQ", [Operator.GT]: "GT", [Operator.GTE]: "GTE", [Operator.LT]: "LT", [Operator.LTE]: "LTE", [Operator.NEQ]: "NEQ", [Operator.CONTAINS]: "CONTAINS", } as const; const LOGICAL_MAP = { [LogicalOperator.AND]: "AND", [LogicalOperator.OR]: "OR", } as const; + +const OPERATOR_CODES = new Set<number>(Object.keys(OPERATOR_MAP).map(Number)); +const LOGICAL_CODES = new Set<number>(Object.keys(LOGICAL_MAP).map(Number)); @@ operator: z .number() .int() - .min(1) - .max(7) - .transform((v) => OPERATOR_MAP[v as keyof typeof OPERATOR_MAP]), + .refine((v): v is keyof typeof OPERATOR_MAP => OPERATOR_CODES.has(v), { + error: "Invalid operator", + }) + .transform((v) => OPERATOR_MAP[v]), @@ logical: z .number() .int() - .min(0) - .max(2) - .transform((v) => LOGICAL_MAP[v as keyof typeof LOGICAL_MAP]), + .refine((v): v is keyof typeof LOGICAL_MAP => LOGICAL_CODES.has(v), { + error: "Invalid logical operator", + }) + .transform((v) => LOGICAL_MAP[v]),🤖 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/data.ts` around lines 12 - 25, Validators that currently rely on hardcoded .min()/.max() ranges should instead validate against the actual keys of OPERATOR_MAP and LOGICAL_MAP so unmapped/changed enum values don't slip through; replace the .min/.max() checks with a membership check built from the maps (e.g., compute allowed = new Set(Object.keys(OPERATOR_MAP).map(Number)) and use z.number().refine(v => allowed.has(v), { message: ... }) or use z.nativeEnum(Operator)/z.nativeEnum(LogicalOperator) where appropriate) so validation is driven from OPERATOR_MAP and LOGICAL_MAP rather than fixed numeric ranges.
🤖 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.
Nitpick comments:
In `@src/zod/data.ts`:
- Around line 12-25: Validators that currently rely on hardcoded .min()/.max()
ranges should instead validate against the actual keys of OPERATOR_MAP and
LOGICAL_MAP so unmapped/changed enum values don't slip through; replace the
.min/.max() checks with a membership check built from the maps (e.g., compute
allowed = new Set(Object.keys(OPERATOR_MAP).map(Number)) and use
z.number().refine(v => allowed.has(v), { message: ... }) or use
z.nativeEnum(Operator)/z.nativeEnum(LogicalOperator) where appropriate) so
validation is driven from OPERATOR_MAP and LOGICAL_MAP rather than fixed numeric
ranges.
In `@src/zod/query.ts`:
- Around line 5-22: The current operator/logical/aggregation validators use
fixed numeric bounds which can drift from the maps; instead make validation
key-driven by deriving allowed keys from OPERATOR_MAP, AGGREGATION_TYPE_MAP, and
LOGICAL_MAP (e.g., use Object.keys or keyof typeof <MAP>) and build the Zod
validators/enums from those derived keys so the schema always matches the
transform maps; update any schema functions that reference numeric bounds to use
these map-derived key sets (look for validations around Operator,
AggregationType, LogicalOperator) and ensure transforms use the same maps to
avoid producing undefined.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8ceff87f-505e-4294-86c7-cba8e1ff02cc
⛔ Files ignored due to path filters (4)
src/gen/data/v1/data_pb.d.tsis excluded by!**/gen/**src/gen/data/v1/data_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 (2)
src/zod/data.tssrc/zod/query.ts
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
2 participants
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