Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a SQL-like caseWhen operator: public API and type inference, ConditionalSelect IR, builder/compiler support (including guarded includes), group-by handling, live-query nested resultPath materialization, traversal updates, and comprehensive tests. ChangescaseWhen Conditional Expression Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 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 |
More templates
@tanstack/angular-db
@tanstack/browser-db-sqlite-persistence
@tanstack/capacitor-db-sqlite-persistence
@tanstack/cloudflare-durable-objects-db-sqlite-persistence
@tanstack/db
@tanstack/db-ivm
@tanstack/db-sqlite-persistence-core
@tanstack/electric-db-collection
@tanstack/electron-db-sqlite-persistence
@tanstack/expo-db-sqlite-persistence
@tanstack/node-db-sqlite-persistence
@tanstack/offline-transactions
@tanstack/powersync-db-collection
@tanstack/query-db-collection
@tanstack/react-db
@tanstack/react-native-db-sqlite-persistence
@tanstack/rxdb-db-collection
@tanstack/solid-db
@tanstack/svelte-db
@tanstack/tauri-db-sqlite-persistence
@tanstack/trailbase-db-collection
@tanstack/vue-db
commit: |
Contributor
|
Size Change: +3.11 kB (+2.72%) Total Size: 117 kB 📦 View Changed
ℹ️ View Unchanged
|
Contributor
|
Size Change: 0 B Total Size: 4.24 kB ℹ️ View Unchanged
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
packages/db/tests/query/case-when.test-d.ts (1)
31-49: ⚡ Quick winAdd explicit return types to test helper factories.
Line 31 and Line 41 rely on inferred return types; please annotate both function return types explicitly to keep type tests stable against inference drift.
Proposed diff
-function createUsers() { +function createUsers(): ReturnType<typeof createCollection<User>> { return createCollection( mockSyncCollectionOptions<User>({ id: `case-when-type-users`, getKey: (user) => user.id, initialData: [], }), ) } -function createPosts() { +function createPosts(): ReturnType<typeof createCollection<Post>> { return createCollection( mockSyncCollectionOptions<Post>({ id: `case-when-type-posts`, getKey: (post) => post.id, initialData: [], }), ) }As per coding guidelines, "
**/*.{ts,tsx}: Provide proper type annotations for function return values instead of relying on implicit types" and "Provide precise return types for functions; avoidunknownoranyreturn types unless absolutely necessary".🤖 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 `@packages/db/tests/query/case-when.test-d.ts` around lines 31 - 49, The helper factory functions createUsers and createPosts rely on inferred return types; add explicit return type annotations to both (for example, the concrete collection type returned by createCollection for User and Post or ReturnType<typeof createCollection> specialized for each) so the test helpers have stable, precise return types; update the function signatures for createUsers and createPosts to include those explicit return types.packages/db/tests/query/case-when.test.ts (2)
384-583: ⚡ Quick winAdd an explicit empty-include corner-case assertion.
The conditional include materialization tests are strong, but they don’t explicitly assert behavior when the included child query resolves to an empty array (
[]) in an active branch.As per coding guidelines, "**/*.test.{ts,tsx,js,jsx}: Test corner cases including: empty collections, single elements, undefined vs null, resolved promises, race conditions, limit/offset edge cases".
🤖 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 `@packages/db/tests/query/case-when.test.ts` around lines 384 - 583, Add an assertion that explicitly covers the empty-include corner case by ensuring a branch that should include child rows returns an empty array when there are no matching child rows: modify one of the conditional projection tests (e.g., the tests using createUsersCollection/createPostsCollection and the LiveQuery named query) to either include a user with no posts or adapt an existing user to have no posts, call query.preload(), then assert that the included field (postTitles or profile.posts) for that user is exactly [] using query.toArray (optionally wrapped by stripVirtualPropsAndSymbols or childRows for Collection includes) so the test verifies an empty-array include is handled correctly.
59-77: ⚡ Quick winReplace
anyin test helpers with safer types.Both helpers use
anyfor parameters and return types, which removes compile-time type checking even in tests.Suggested typed rewrite
-function stripVirtualPropsAndSymbols(value: any): any { +function stripVirtualPropsAndSymbols(value: unknown): unknown { if (Array.isArray(value)) { return value.map((entry) => stripVirtualPropsAndSymbols(entry)) } if (value && typeof value === `object`) { - const out: Record<string, any> = {} - for (const [key, entry] of Object.entries(stripVirtualProps(value))) { + const out: Record<string, unknown> = {} + for (const [key, entry] of Object.entries(stripVirtualProps(value as object))) { out[key] = stripVirtualPropsAndSymbols(entry) } return out } return value } -function childRows(collection: any): Array<any> { - return [...collection.toArray].map((row) => stripVirtualPropsAndSymbols(row)) +function childRows( + collection: { toArray: Iterable<unknown> }, +): Array<unknown> { + return [...collection.toArray].map((row) => stripVirtualPropsAndSymbols(row)) }Per coding guidelines: "**/*.{ts,tsx}: Avoid using
anytypes; useunknowninstead when type is truly unknown and provide type guards to narrow safely".🤖 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 `@packages/db/tests/query/case-when.test.ts` around lines 59 - 77, Replace the unsafe any usages in the test helpers by typing stripVirtualPropsAndSymbols(value: unknown): unknown (and arrays as unknown[]) and childRows(collection: { toArray: Iterable<unknown> }): unknown[]; keep the existing runtime checks (Array.isArray, typeof value === 'object' && value !== null) as the type guards, change the intermediate out to Record<string, unknown>, and treat entries/returned values from stripVirtualProps as unknown before recursing so you avoid any while preserving the current logic in stripVirtualPropsAndSymbols, childRows, and the use of collection.toArray.packages/db/src/query/compiler/select.ts (1)
204-245: ⚡ Quick winExtract
isCaseWhenConditionTrue()into a shared helper.This truthiness rule now exists here and again in
compiler/evaluators.ts. If one side changes, scalarcaseWhen()and projectioncaseWhen()will pick different branches for the same row.As per coding guidelines, "Extract common logic into reusable utility functions when duplicated across multiple places".
🤖 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 `@packages/db/src/query/compiler/select.ts` around lines 204 - 245, The truthiness logic in isCaseWhenConditionTrue is duplicated; extract it into a single shared helper (e.g., export function isCaseWhenConditionTrue from a new/shared utility module) and update compileConditionalSelect to import and use that helper instead of its local implementation, then update the other occurrence in compiler/evaluators.ts (and any caseWhen implementations) to import the same helper so scalar and projection caseWhen use identical logic; ensure the helper is exported with the same name and update imports where isCaseWhenConditionTrue was previously defined or referenced.packages/db/src/query/compiler/group-by.ts (1)
656-666: ⚡ Quick winUse proper types from the IR module.
The inline type assertion doesn't reference the actual
ConditionalSelecttype from the IR module. This creates a maintenance risk if the structure changes.♻️ Refactor to use imported types
Import
ConditionalSelectfrom the IR module (if not already imported), then refine the type check:if (expr.type === `conditionalSelect` && `branches` in expr) { + const conditionalExpr = expr as ConditionalSelect return ( - expr.branches as Array<{ - condition: BasicExpression - value: BasicExpression | Aggregate | Select | { type: string } - }> + conditionalExpr.branches ).some( (branch) => containsAggregate(branch.condition) || containsAggregate(branch.value), ) }Note: Ensure
ConditionalSelectis imported at the top of the file alongside other IR types.As per coding guidelines, avoid using
anytypes and provide proper type annotations instead of relying on type assertions.🤖 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 `@packages/db/src/query/compiler/group-by.ts` around lines 656 - 666, Replace the inline ad-hoc type assertion for conditional selects with the actual ConditionalSelect IR type: import ConditionalSelect from the IR module (or add it to the existing IR import), then narrow expr using that type (e.g., treat expr as ConditionalSelect after the type check `expr.type === 'conditionalSelect'`) and access expr.branches typed as ConditionalSelect['branches'] instead of an anonymous cast; update the import section at the top of group-by.ts accordingly and adjust the containsAggregate checks to operate on the properly typed branch elements.
🤖 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 `@packages/db/src/query/builder/index.ts`:
- Around line 901-905: buildNestedSelect currently calls
buildConditionalSelect(obj, parentAliases) but buildConditionalSelect lowers
branches with buildNestedSelect(args[i * 2 + 1], parentAliases) which loses the
current select key/alias and prevents includes (toArray/IncludesSubquery) from
being produced; update the API so the destination/alias is threaded through:
add/pass a destination identifier (e.g. destKey or current select key) from
buildNestedSelect into buildConditionalSelect and ensure buildConditionalSelect
calls buildNestedSelect(branchValue, parentAliasesWithDest) (or
parentAliases.concat(dest)) when lowering each branch; adjust buildNestedSelect
signature and all call sites (including the initial call sites around
CaseWhenWrapper and the other occurrences noted) so branch values retain the
select alias and are treated as include nodes.
In `@packages/db/src/query/compiler/group-by.ts`:
- Around line 656-666: The conditionalSelect branch checker currently tests only
each branch's condition and value for aggregates but misses the optional
expr.defaultValue; update the logic in group-by.ts (inside the conditionalSelect
handling) to also call containsAggregate(expr.defaultValue) and include that
result in the overall some/any check so that aggregates present only in the
defaultValue are detected; reference the conditionalSelect `expr`, the helper
`containsAggregate`, and the branch structure when making this change.
---
Nitpick comments:
In `@packages/db/src/query/compiler/group-by.ts`:
- Around line 656-666: Replace the inline ad-hoc type assertion for conditional
selects with the actual ConditionalSelect IR type: import ConditionalSelect from
the IR module (or add it to the existing IR import), then narrow expr using that
type (e.g., treat expr as ConditionalSelect after the type check `expr.type ===
'conditionalSelect'`) and access expr.branches typed as
ConditionalSelect['branches'] instead of an anonymous cast; update the import
section at the top of group-by.ts accordingly and adjust the containsAggregate
checks to operate on the properly typed branch elements.
In `@packages/db/src/query/compiler/select.ts`:
- Around line 204-245: The truthiness logic in isCaseWhenConditionTrue is
duplicated; extract it into a single shared helper (e.g., export function
isCaseWhenConditionTrue from a new/shared utility module) and update
compileConditionalSelect to import and use that helper instead of its local
implementation, then update the other occurrence in compiler/evaluators.ts (and
any caseWhen implementations) to import the same helper so scalar and projection
caseWhen use identical logic; ensure the helper is exported with the same name
and update imports where isCaseWhenConditionTrue was previously defined or
referenced.
In `@packages/db/tests/query/case-when.test-d.ts`:
- Around line 31-49: The helper factory functions createUsers and createPosts
rely on inferred return types; add explicit return type annotations to both (for
example, the concrete collection type returned by createCollection for User and
Post or ReturnType<typeof createCollection> specialized for each) so the test
helpers have stable, precise return types; update the function signatures for
createUsers and createPosts to include those explicit return types.
In `@packages/db/tests/query/case-when.test.ts`:
- Around line 384-583: Add an assertion that explicitly covers the empty-include
corner case by ensuring a branch that should include child rows returns an empty
array when there are no matching child rows: modify one of the conditional
projection tests (e.g., the tests using
createUsersCollection/createPostsCollection and the LiveQuery named query) to
either include a user with no posts or adapt an existing user to have no posts,
call query.preload(), then assert that the included field (postTitles or
profile.posts) for that user is exactly [] using query.toArray (optionally
wrapped by stripVirtualPropsAndSymbols or childRows for Collection includes) so
the test verifies an empty-array include is handled correctly.
- Around line 59-77: Replace the unsafe any usages in the test helpers by typing
stripVirtualPropsAndSymbols(value: unknown): unknown (and arrays as unknown[])
and childRows(collection: { toArray: Iterable<unknown> }): unknown[]; keep the
existing runtime checks (Array.isArray, typeof value === 'object' && value !==
null) as the type guards, change the intermediate out to Record<string,
unknown>, and treat entries/returned values from stripVirtualProps as unknown
before recursing so you avoid any while preserving the current logic in
stripVirtualPropsAndSymbols, childRows, and the use of collection.toArray.
🪄 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
Run ID: 069e5d1e-ffc5-4ecc-b6ca-f024c435c362
📒 Files selected for processing (14)
packages/db/src/query/builder/functions.tspackages/db/src/query/builder/index.tspackages/db/src/query/builder/ref-proxy.tspackages/db/src/query/builder/types.tspackages/db/src/query/compiler/evaluators.tspackages/db/src/query/compiler/group-by.tspackages/db/src/query/compiler/index.tspackages/db/src/query/compiler/select.tspackages/db/src/query/index.tspackages/db/src/query/ir.tspackages/db/src/query/live/collection-config-builder.tspackages/db/src/query/live/utils.tspackages/db/tests/query/case-when.test-d.tspackages/db/tests/query/case-when.test.ts
Thread conditional include destinations through caseWhen branches and tighten aggregate handling for grouped conditional projections. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/db/src/query/compiler/select.ts (1)
231-249: ⚡ Quick winExtract shared CASE truthiness evaluation.
isCaseWhenConditionTrue()is now duplicated here and inpackages/db/src/query/compiler/group-by.ts. Keeping both copies in sync is part ofcaseWhen()correctness now, so this should live in one shared helper.As per coding guidelines, Extract common logic into reusable utility functions when duplicated across multiple places.
🤖 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 `@packages/db/src/query/compiler/select.ts` around lines 231 - 249, Duplicate CASE truthiness logic exists in isCaseWhenConditionTrue (in select.ts) and in group-by.ts; extract this logic into a single shared utility (e.g., create a new helper function name like evaluateCaseTruthiness or keep isCaseWhenConditionTrue) in a common utilities module and replace both local definitions with imports from that module, update references in caseWhen and any callers (select.ts's isCaseWhenConditionTrue and the duplicate in group-by.ts) to use the shared function, and remove the duplicated implementation so there is a single source of truth.
🤖 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 `@packages/db/src/query/compiler/group-by.ts`:
- Around line 766-852: The branch conditions/values compiled in
compileGroupedConditionalSelect (and any grouped select/value compilation paths
used by compileGroupedSelectObject) still reference original namespaced refs
(e.g., users.status) but the grouped evaluator runs against a row containing
only group-backed namespaces like $selected/group-key, so refs must be lowered
before compiling; update compileGroupedConditionalSelect to transform
branch.condition and branch.value refs into group-scoped refs (or wrap them)
prior to calling compileExpression/compileGroupedSelectValue (i.e., add a helper
that rewrites BasicExpression PropRef nodes that point to grouped tables into
PropRefs under $selected or the group-key namespace and apply it to each
branch.condition and branch.value), and apply the same lowering when creating
entries in compileGroupedSelectObject for non-aggregate ref expressions so the
compiled functions will read from the reconstructed grouped row rather than
undefined original namespaces.
- Around line 746-759: The branch that handles isNestedSelectObject currently
blindly runs extractAndReplaceAggregates over every property, which converts
IncludesSubquery IR into a plain object and breaks compileGroupedSelectValue's
includesSubquery handling; fix this by detecting IncludesSubquery nodes while
iterating the object (check value.type === 'includesSubquery' or the project's
IncludesSubquery predicate) and preserve them as-is into transformed[key] (skip
calling extractAndReplaceAggregates), otherwise continue extracting aggregates
as before (keep using extractAndReplaceAggregates and merging into
allExtracted), so compileGroupedSelectValue can still match value.type ===
'includesSubquery'.
---
Nitpick comments:
In `@packages/db/src/query/compiler/select.ts`:
- Around line 231-249: Duplicate CASE truthiness logic exists in
isCaseWhenConditionTrue (in select.ts) and in group-by.ts; extract this logic
into a single shared utility (e.g., create a new helper function name like
evaluateCaseTruthiness or keep isCaseWhenConditionTrue) in a common utilities
module and replace both local definitions with imports from that module, update
references in caseWhen and any callers (select.ts's isCaseWhenConditionTrue and
the duplicate in group-by.ts) to use the shared function, and remove the
duplicated implementation so there is a single source of truth.
🪄 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
Run ID: 40e73b2a-e80c-481d-930b-d0072bc10f93
📒 Files selected for processing (6)
.changeset/tender-mugs-hear.mdpackages/db/src/query/builder/functions.tspackages/db/src/query/builder/index.tspackages/db/src/query/compiler/group-by.tspackages/db/src/query/compiler/select.tspackages/db/tests/query/case-when.test.ts
✅ Files skipped from review due to trivial changes (1)
- .changeset/tender-mugs-hear.md
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/db/tests/query/case-when.test.ts
- packages/db/src/query/builder/functions.ts
kevin-dp
requested changes
May 18, 2026
Contributor
kevin-dp
left a comment
kevin-dp
left a comment
There was a problem hiding this comment.
I left some comments.
There's quite some duplication. I found 4 copies of the same function spread across the codebase. There are probably other functions that are duplicated too that i didn't spot. Please have an agent go through it to find all duplicated functions and extract them into a single shared function.
Use one compiler helper for scalar and projection caseWhen branch truthiness so every evaluation path stays consistent. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@packages/db/src/query/compiler/select.ts`:
- Around line 174-180: Replace the bare instanceof ConditionalSelect guard with
a single reusable predicate (e.g. isConditionalSelect(value)) and use that
predicate in both compileSelectValue and the addFromObject branch (the spots
currently using instanceof and the type==='conditionalSelect' check). Implement
isConditionalSelect to return true for either an actual ConditionalSelect
instance or an object with type === 'conditionalSelect' (and keep existing
containsAggregate/compileConditionalSelect logic unchanged). Then update the two
checks to call isConditionalSelect(value) so nested caseWhen values and plain
projection objects are classified consistently.
- Around line 171-188: In compileSelectValue, guard against nullish
SelectValueExpression before accessing its .type: update the early branches (the
ConditionalSelect and ValClass checks remain) and add a nullish check (e.g., if
value == null) that returns the intended literal fallback (null) before the line
that reads value.type; ensure the check is placed before the existing `if
(value.type === 'includesSubquery')` so that calling compileSelectValue with
null or undefined (or caseWhen(..., null)) will return the fallback instead of
throwing.
🪄 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
Run ID: 840c8b4e-2d30-446f-8ea5-84cb6fecdfd2
📒 Files selected for processing (5)
packages/db/src/query/builder/types.tspackages/db/src/query/compiler/evaluators.tspackages/db/src/query/compiler/group-by.tspackages/db/src/query/compiler/index.tspackages/db/src/query/compiler/select.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/db/src/query/compiler/evaluators.ts
- packages/db/src/query/builder/types.ts
- packages/db/src/query/compiler/index.ts
- packages/db/src/query/compiler/group-by.ts
Keep precise caseWhen typings to five branches to reduce API surface while preserving runtime behavior. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/db/src/query/builder/functions.ts (1)
693-698: ⚡ Quick winInconsistent wrapper pattern: use
declareandunknownto match sibling wrappers.
CaseWhenWrapperdiffers fromToArrayWrapperandConcatToArrayWrapperin two ways:
- Default type parameter is
anyinstead ofunknown_resultis a real optional property instead of a phantom (declare) propertyWithout
declare, instances will have an actual_resultproperty at runtime (set toundefined), unlike the other wrappers.♻️ Proposed fix to align with sibling wrappers
-export class CaseWhenWrapper<_T = any> { +export class CaseWhenWrapper<_T = unknown> { readonly __brand = `CaseWhenWrapper` as const declare readonly _type: `caseWhen` - readonly _result?: _T + declare readonly _result: _T constructor(public readonly args: Array<CaseWhenValue>) {} }🤖 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 `@packages/db/src/query/builder/functions.ts` around lines 693 - 698, CaseWhenWrapper currently uses a default type parameter of any and defines _result as a real optional property; change it to match the sibling wrappers (ToArrayWrapper, ConcatToArrayWrapper) by making the generic default unknown (CaseWhenWrapper<_T = unknown>) and turning _result into a phantom declaration (declare readonly _result?: _T) so instances do not get a runtime _result property.
🤖 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 `@packages/db/src/query/builder/functions.ts`:
- Around line 693-698: CaseWhenWrapper currently uses a default type parameter
of any and defines _result as a real optional property; change it to match the
sibling wrappers (ToArrayWrapper, ConcatToArrayWrapper) by making the generic
default unknown (CaseWhenWrapper<_T = unknown>) and turning _result into a
phantom declaration (declare readonly _result?: _T) so instances do not get a
runtime _result property.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e312db9d-b579-49b7-9d47-957f41d09575
📒 Files selected for processing (1)
packages/db/src/query/builder/functions.ts
Preserve include nodes during aggregate extraction and handle null projection branches without misclassifying conditional selects. Co-authored-by: Cursor <cursoragent@cursor.com>
Rewrite grouped refs inside aggregate-wrapped conditional projections so branch conditions and values evaluate against grouped keys. Co-authored-by: Cursor <cursoragent@cursor.com>
Allow longer caseWhen calls without reintroducing verbose precise overloads past five branches. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/db/tests/query/case-when.test-d.ts (1)
91-124: ⚡ Quick winThe fallback-overload type check is currently too weak.
toExtendstill passes iffallbackOverloadregresses toany, so this case does not actually protect the 6-branch inference path. Please add an exact or explicit non-anyassertion around that property before treating this as coverage for the broad overload.🤖 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 `@packages/db/tests/query/case-when.test-d.ts` around lines 91 - 124, The test's type assertion using toExtend is too permissive and won't fail if fallbackOverload becomes any; update the assertions to explicitly assert a non-any exact type for the fallbackOverload property: locate the result variable (from query.toArray[0]!) and replace or augment the toExtend check with a direct type equality/non-any check referencing fallbackOverload (e.g., use expectTypeOf(result.fallbackOverload) toEqualTypeOf or expectTypeOf(result.fallbackOverload).not.toBeAny()) so the caseWhen six-branch inference is strictly validated; keep the other OutputWithVirtualKeyed assertion if desired but ensure fallbackOverload is asserted exactly and not allowed to be any.
🤖 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 `@packages/db/src/query/builder/functions.ts`:
- Around line 753-773: isExpressionValue is currently too permissive and treats
plain projection objects like {type:'val', label: ...} as IR nodes; update
isExpressionValue to validate the full discriminated shape (or a private brand)
rather than only checking 'type'. Concretely, inside isExpressionValue add extra
checks per discriminant: if value.type === 'val' require the actual payload
property (e.g. 'value' or the IR's value-field), if value.type === 'func' ensure
expected fields like 'name' and 'args', if value.type === 'agg' ensure
agg-specific fields, and if value.type === 'ref' ensure reference-specific
fields (or alternatively check for a dedicated internal brand/symbol present on
IR nodes). This change in isExpressionValue (used by caseWhen / CaseWhenWrapper)
will prevent projection objects from being misclassified as IR nodes.
- Around line 570-596: The fallback overload for caseWhen currently returns any
and accepts ...rest: Array<CaseWhenValue>, losing alternating condition/value
typing and the inferred return union; update the signature for the variadic
fallback of caseWhen to use a generic variadic-tuple type (e.g. T extends
readonly [...(ExpressionLike | CaseWhenValue)[]]) that enforces alternating
condition/value entries and compute the resulting return union from those tuple
elements (using conditional/mapped types) instead of any; preserve existing
fixed overloads for 1–5 pairs, then implement the generic overload that
constrains length to an even number and derives the return type, referencing the
caseWhen function, ExpressionLike, and CaseWhenValue so callers with 6+ pairs
keep proper compile-time checks.
---
Nitpick comments:
In `@packages/db/tests/query/case-when.test-d.ts`:
- Around line 91-124: The test's type assertion using toExtend is too permissive
and won't fail if fallbackOverload becomes any; update the assertions to
explicitly assert a non-any exact type for the fallbackOverload property: locate
the result variable (from query.toArray[0]!) and replace or augment the toExtend
check with a direct type equality/non-any check referencing fallbackOverload
(e.g., use expectTypeOf(result.fallbackOverload) toEqualTypeOf or
expectTypeOf(result.fallbackOverload).not.toBeAny()) so the caseWhen six-branch
inference is strictly validated; keep the other OutputWithVirtualKeyed assertion
if desired but ensure fallbackOverload is asserted exactly and not allowed to be
any.
🪄 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
Run ID: abb29ecd-9fb2-4155-97e4-de2a9b815d1a
📒 Files selected for processing (5)
packages/db/src/query/builder/functions.tspackages/db/src/query/compiler/group-by.tspackages/db/src/query/compiler/select.tspackages/db/tests/query/case-when.test-d.tspackages/db/tests/query/case-when.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/db/src/query/compiler/select.ts
- packages/db/tests/query/case-when.test.ts
- packages/db/src/query/compiler/group-by.ts
Avoid treating projection objects with type fields as IR expressions when selecting conditional projections. Co-authored-by: Cursor <cursoragent@cursor.com>
kevin-dp
approved these changes
May 19, 2026
Contributor
kevin-dp
left a comment
kevin-dp
left a comment
There was a problem hiding this comment.
Thanks for addressing the comments. The type overload with the var args is great and avoids the need for too many overloads 💯
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
caseWhen(...)as a unified conditional query helper inspired by SQLCASE WHEN, with scalar expression support inselect,where,orderBy,groupBy,having, and equality join operands.caseWhensupport forselect()branch values, including nested projection objects, ref spreads,toArray(...), and Collection includes.caseWhenbranches are not materialized for that parent row.caseWhenin the live query guide and adds a changeset for the new operator.Why This Is Useful
caseWhenlets queries express conditional logic without dropping down to functional callbacks, so the query compiler can keep optimizing and incrementally maintaining the result.A common scalar use case is categorizing rows while preserving literal result types:
Because scalar
caseWhenis a normal query expression, it can also drive filtering, grouping, sorting, and join operands:Projection-valued branches make optional result shapes easier to model. For example, a joined row can project an optional nested object only when the joined source exists:
It also enables guarded includes, where child collections are materialized only for rows whose branch is active:
Design
caseWhenis one public API with two internal paths selected by branch value shape, not call location:Func('caseWhen', ...). The evaluator walks condition/value pairs left to right, returns the first matching branch, evaluates only the active value branch, and returnsnullwhen there is no default.CaseWhenWrapperthat is lowered byselect()intoConditionalSelectIR. The select compiler evaluates branch conditions lazily and returnsundefinedwhen there is no matching projection branch and no default.ConditionalSelectbranches carry guard metadata through include extraction/routing. Parent keys are emitted only when the guard chain matches, which keeps inactive branch includes from loading or attaching.Full design notes: https://gist.github.com/samwillis/60d8d0d937cf23267d0f8383bea3af92
Test plan
pnpm exec vitest --run tests/query/case-when.test.ts tests/query/case-when.test-d.tsfrompackages/dbpnpm testfrompackages/dbpnpm testfrom the repo root was also run;packages/dbpassed, but the workspace run failed in unrelated@tanstack/expo-db-sqlite-persistencetype-checking for untrackedtests/helpers/expo-runtime-node-preload.cjs.Made with Cursor
Summary by CodeRabbit
New Features
Includes / Materialization
Aggregation / Grouping
Tests
Made with Cursor