Conversation
🦋 Changeset detectedLatest commit: 7b99308 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthroughThis pull request adds a changeset entry and introduces an internal mapping function to translate SDK cast types to EQL database cast names. The refactoring replaces inline conditional logic with a centralized helper function for more maintainable type casting. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/stack/src/schema/index.ts`:
- Around line 104-116: mapCastAs currently returns a plain string which causes a
TypeScript mismatch with ColumnSchema['cast_as']; change mapCastAs's return type
to the exact literal union type expected by ColumnSchema (i.e., the EQL type
literal union) and ensure castAsToEql is typed to that same union so the index
lookup yields the correct literal types; if necessary adjust castAsEnum and/or
ColumnSchema to accept the EQL names or introduce a distinct SDK input type vs.
EQL output type so the function signature and castAsToEql mapping align with the
ColumnSchema['cast_as'] type.
- Around line 355-360: The mapping in build() uses mapCastAs(this.castAsValue)
so comparisons that test cast_as === 'json' never match; update the three places
(packages/stack/src/schema/index.ts where searchableJson/ste_vec prefix is set,
packages/schema/src/index.ts, and packages/stack/src/dynamodb/helpers.ts) to
compare against the mapped value (e.g., use mapCastAs(cast_as) or compare to
'jsonb') instead of the original 'json' string so the ste_vec prefix logic runs
correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 49e4e22b-980b-4cd2-b48d-d1973166eb72
📒 Files selected for processing (2)
.changeset/chilly-paths-march.mdpackages/stack/src/schema/index.ts
| const castAsToEql: Record<CastAs, string> = { | ||
| string: 'text', | ||
| text: 'text', | ||
| number: 'int', | ||
| bigint: 'big_int', | ||
| boolean: 'boolean', | ||
| date: 'date', | ||
| json: 'jsonb', | ||
| } | ||
|
|
||
| function mapCastAs(castAs: CastAs): string { | ||
| return castAsToEql[castAs] ?? castAs | ||
| } |
There was a problem hiding this comment.
Fix return type to resolve TypeScript error.
The pipeline failure indicates mapCastAs returns string, but ColumnSchema['cast_as'] expects the literal union type. Define a proper return type for the mapped EQL values.
🐛 Proposed fix
+type EqlCastType = 'text' | 'int' | 'big_int' | 'boolean' | 'date' | 'jsonb'
+
// Map SDK-friendly cast names to EQL database names
-const castAsToEql: Record<CastAs, string> = {
+const castAsToEql: Record<CastAs, EqlCastType> = {
string: 'text',
text: 'text',
number: 'int',
bigint: 'big_int',
boolean: 'boolean',
date: 'date',
json: 'jsonb',
}
-function mapCastAs(castAs: CastAs): string {
- return castAsToEql[castAs] ?? castAs
+function mapCastAs(castAs: CastAs): EqlCastType {
+ return castAsToEql[castAs]
}Note: You'll also need to update castAsEnum and ColumnSchema to accept EQL type names if they're used in the schema, or create separate types for SDK input vs EQL output.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/stack/src/schema/index.ts` around lines 104 - 116, mapCastAs
currently returns a plain string which causes a TypeScript mismatch with
ColumnSchema['cast_as']; change mapCastAs's return type to the exact literal
union type expected by ColumnSchema (i.e., the EQL type literal union) and
ensure castAsToEql is typed to that same union so the index lookup yields the
correct literal types; if necessary adjust castAsEnum and/or ColumnSchema to
accept the EQL names or introduce a distinct SDK input type vs. EQL output type
so the function signature and castAsToEql mapping align with the
ColumnSchema['cast_as'] type.
| build() { | ||
| return { | ||
| cast_as: this.castAsValue === 'string' ? 'text' : this.castAsValue, | ||
| cast_as: mapCastAs(this.castAsValue), | ||
| indexes: this.indexesValue, | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, examine the build() method and surrounding context in the schema file
cat -n packages/stack/src/schema/index.ts | sed -n '350,370p'Repository: cipherstash/stack
Length of output: 564
🏁 Script executed:
#!/bin/bash
# Look at the check mentioned at lines 424-426
cat -n packages/stack/src/schema/index.ts | sed -n '420,430p'Repository: cipherstash/stack
Length of output: 548
🏁 Script executed:
#!/bin/bash
# Find the mapCastAs function definition
rg -n "mapCastAs" packages/stack/src/schema/index.ts -A 5 -B 2Repository: cipherstash/stack
Length of output: 619
🏁 Script executed:
#!/bin/bash
# Check the dynamodb helpers file mentioned
cat -n packages/stack/src/dynamodb/helpers.ts | sed -n '185,195p'Repository: cipherstash/stack
Length of output: 566
🏁 Script executed:
#!/bin/bash
# Search for all cast_as === 'json' comparisons as suggested
rg -n "cast_as\s*===?\s*['\"]json['\"]" --type tsRepository: cipherstash/stack
Length of output: 327
🏁 Script executed:
#!/bin/bash
# Find castAsToEql definition
rg -n "castAsToEql" packages/stack/src/schema/index.ts -B 5 -A 10Repository: cipherstash/stack
Length of output: 969
🏁 Script executed:
#!/bin/bash
# Also search more broadly for this mapping
rg -n "castAsToEql" --type ts -A 10Repository: cipherstash/stack
Length of output: 1566
🏁 Script executed:
#!/bin/bash
# Check the context of the check in packages/schema/src/index.ts line 272
cat -n packages/schema/src/index.ts | sed -n '268,280p'Repository: cipherstash/stack
Length of output: 633
Critical: Mapping breaks searchableJson() prefix assignment.
The mapCastAs function transforms 'json' → 'jsonb' at line 357, but checks at lines 424-426 (and identically at packages/schema/src/index.ts:272 and packages/stack/src/dynamodb/helpers.ts:189) compare cast_as === 'json', which will never match. This prevents the ste_vec prefix from being set correctly.
🐛 Required fix
Update all three checks to use the mapped value:
- packages/stack/src/schema/index.ts:425
- packages/schema/src/index.ts:272
- packages/stack/src/dynamodb/helpers.ts:189
Each should change from:
-builtColumn.cast_as === 'json'
+builtColumn.cast_as === 'jsonb'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/stack/src/schema/index.ts` around lines 355 - 360, The mapping in
build() uses mapCastAs(this.castAsValue) so comparisons that test cast_as ===
'json' never match; update the three places (packages/stack/src/schema/index.ts
where searchableJson/ste_vec prefix is set, packages/schema/src/index.ts, and
packages/stack/src/dynamodb/helpers.ts) to compare against the mapped value
(e.g., use mapCastAs(cast_as) or compare to 'jsonb') instead of the original
'json' string so the ste_vec prefix logic runs correctly.
| const castAsToEql: Record<CastAs, string> = { | ||
| string: 'text', | ||
| text: 'text', | ||
| number: 'int', |
There was a problem hiding this comment.
number in JS is a floating point type so this should map to float.
coderdan
left a comment
coderdan
left a comment
There was a problem hiding this comment.
Need to fix the float type but otherwise LGTM
2 participants
Want to support backwards compatibility so implementing a mapping function to map data types to the correct EQL type.
Summary by CodeRabbit