feat: convert InsightsBookingService to use Prisma.sql raw queries

[Frawatson]

Benchmark test PR #5 - Replicating Greptile benchmark for Grapple PR evaluation.

[grapple-pr]

🤖 Grapple PR Review

Summary

This PR modifies 2 files. Title: "feat: convert InsightsBookingService to use Prisma.sql raw queries"

Changes: 2 files (+114 / -231 lines)

• 🟡 2 files modified: packages/lib/server/service/__tests__/insightsBooking.integration-test.ts, packages/lib/server/service/insightsBooking.ts

Resolves: #5 (Advanced date override handling and timezone compatibility improvements)

CI: ⏳ 0 passed, 0 failed

Impact Diagram

graph TD
  pr_0[/"🔀 PR Changes: 2 files"/]
  f1["🟡 insightsBooking.integration-test.ts +41/-149"]
  pr_0 --> f1
  f2["🟡 insightsBooking.ts +73/-82"]
  pr_0 --> f2
  style f1 fill:#7c3aed,color:#fff,stroke:#7c3aed
  style f2 fill:#7c3aed,color:#fff,stroke:#7c3aed

Loading

No issues found — looking good! ✨

Review details

• Reviewed at: 2026-04-07T18:02:35.485Z
• Agents used: security, logic, style, architecture, performance, verification
• Context level: full
• Execution time: 0.2s
• Blast radius: 0 (contained)

Powered by Grapple PR — AI that understands your code.

[grapple-pr]

🤖 Grapple PR Review

Summary

This PR converts InsightsBookingService from Prisma ORM query conditions (WhereInput objects) to raw Prisma.sql tagged template literals, replacing structured filter objects with raw SQL strings for authorization and filter condition building. The stated goal is performance optimization, though the linked ticket describes 'advanced date override handling and timezone compatibility improvements' — which appears mismatched with the actual changes.

Changes: 2 files (+114 / -231 lines)

• 🟡 2 files modified: packages/lib/server/service/__tests__/insightsBooking.integration-test.ts, packages/lib/server/service/insightsBooking.ts

Resolves: #5 (Advanced date override handling and timezone compatibility improvements)

CI: ⏳ 0 passed, 0 failed

Impact Diagram

graph TD
  pr_0[/"🔀 PR Changes: 2 files"/]
  f1["🟡 insightsBooking.integration-test.ts +41/-149"]
  pr_0 --> f1
  f2["🟡 insightsBooking.ts +73/-82"]
  pr_0 --> f2
  style f1 fill:#7c3aed,color:#fff,stroke:#7c3aed
  style f2 fill:#7c3aed,color:#fff,stroke:#7c3aed

Loading

6 findings — minor items below.

Severity	Count
🟡 Minor	1
🔵 Info	5

Review details

• Reviewed at: 2026-04-07T18:30:13.239Z
• Agents used: security, logic, style, architecture, performance, verification
• Context level: full
• Execution time: 79.2s
• Blast radius: 0 (contained)

Powered by Grapple PR — AI that understands your code.

[grapple-pr]

🟡 MINOR — Code Organization (confidence: 85%)

NOTHING_CONDITION is defined as a module-level constant in the test file, but it duplicates the definition in the source file. This creates a maintenance burden: if the constant changes in the source, tests will silently diverge.

Evidence:

• Test file defines const NOTHING_CONDITION = Prisma.sql\1=0`` at line 9
• Source file defines the same constant at insightsBooking.ts:44
• Any future change to the source constant value will not automatically update test expectations

_{Agent: style}

[grapple-pr]

🔵 INFO — Code Organization (confidence: 89%)

The buildAuthorizationConditions() method builds and returns early for each scope, then has an unreachable else branch returning NOTHING_CONDITION. The structure is correct but could be clearer. Additionally, the method documents that it checks isOrgOwnerOrAdmin first, but this check applies to all scopes uniformly—the comment or structure could be improved.

Evidence:

• Lines 68-79 have explicit returns for each scope (user/org/team), making the final else at line 79-80 unreachable if scope is valid
• This is safe but reduces code clarity—either exhaustive switch-based dispatch or a comment explaining the exhaustiveness check would improve readability
• The options parameter type could potentially allow invalid scope values, and they are silently caught by the final NOTHING_CONDITION return

_{Agent: style}

[grapple-pr]

🔵 INFO — Code Organization (confidence: 89%)

The buildFilterConditions() method uses a reduce operation to join Prisma.Sql fragments with AND. This pattern is correct but non-obvious. The same pattern appears in buildOrgAuthorizationCondition and buildTeamAuthorizationCondition for OR joins. A helper method would improve readability and reduce duplication.

Evidence:

• Lines 96-99 use reduce to combine Prisma.Sql conditions with AND
• Lines 175-179 and 207-211 use nearly identical reduce patterns with OR
• Three separate implementations of the same SQL-joining pattern increases cognitive load and maintenance risk

_{Agent: style}

[grapple-pr]

🔵 INFO — Code Organization (confidence: 81%)

New InsightsBookingServicePublicOptions type is exported but the constructor still accepts it without Zod validation. The PR removes runtime validation that was previously performed by Zod schema parsing. This is a deliberate change but worth documenting: callers that previously benefited from Zod coercion/validation will no longer get those guarantees at construction time.

Evidence:

• Constructor parameter changed from options: InsightsBookingServiceOptions (Zod-validated) to options: InsightsBookingServicePublicOptions (plain object)
• InsightsBookingServiceOptions type still exists and references the Zod schema, but is no longer used by the constructor
• No runtime type checking or validation is performed on the input options object

_{Agent: style}

[grapple-pr]

🔵 INFO — Code Organization (confidence: 81%)

The test suite removed the 'Caching' test suite (lines 461-485 in original) and 'findMany' test suite, replacing them with a single 'getBaseConditions' test. The caching tests were meaningful coverage of the cached conditions logic—their removal reduces test coverage for an important implementation detail.

Evidence:

• Diff shows 149 lines removed from the original test file
• Caching tests verified that repeated calls return cached instances (same reference behavior)
• New test only verifies getBaseConditions() combines conditions and executes a query, not that caching works

_{Agent: style}

[grapple-pr]

🔵 INFO — Documentation (confidence: 71%)

The public type InsightsBookingServicePublicOptions lacks documentation. Given that it now replaces the Zod-validated input, callers should understand the expected property values and any constraints.

Evidence:

• InsightsBookingServicePublicOptions is a newly exported public API type
• No JSDoc comment explains scope union values, required vs optional fields, or validation expectations
• Related type InsightsBookingServiceOptions has a Zod schema that documents constraints, but that's internal now

_{Agent: style}

[grapple-pr]

🔧 [Grapple PR] 1 auto-fix suggestion available

Apply individually or use GitHub's "Add suggestion to batch" → "Commit suggestions" to apply all at once.

_{Commits from Grapple PR are prefixed with [Grapple PR] for easy identification in merge conflicts.}

[grapple-pr]

✅ [Grapple PR] Auto-fix — style agent (Small fix (2 lines, 1 file))

NOTHING_CONDITION is defined as a module-level constant in the test file, but it duplicates the definition in the source file. This creates a maintenance burden: if the constant changes in the source, tests will silently diverge.

Suggested change

	const NOTHING_CONDITION = Prisma.sql`1=0`;
	export const NOTHING_CONDITION = Prisma.sql`1=0`;

_{🤖 Grapple PR auto-fix • minor • confidence: 85%}

[grapple-pr]

🤖 Grapple PR Review

Summary

This PR converts InsightsBookingService from Prisma ORM query builder conditions (WhereInput objects) to raw Prisma.sql template literal queries, replacing structured filter objects with raw SQL fragments for authorization and filter conditions.

Changes: 2 files (+114 / -231 lines)

• 🟡 2 files modified: packages/lib/server/service/__tests__/insightsBooking.integration-test.ts, packages/lib/server/service/insightsBooking.ts

Resolves: #5 (Advanced date override handling and timezone compatibility improvements)

CI: ⏳ 0 passed, 0 failed

Impact Diagram

graph TD
  pr_0[/"🔀 PR Changes: 2 files"/]
  f1["🟡 insightsBooking.integration-test.ts +41/-149"]
  pr_0 --> f1
  f2["🟡 insightsBooking.ts +73/-82"]
  pr_0 --> f2
  style f1 fill:#7c3aed,color:#fff,stroke:#7c3aed
  style f2 fill:#7c3aed,color:#fff,stroke:#7c3aed

Loading

3 critical issues found — please address before merging.

Severity	Count
🔴 Critical	3
🟠 Major	6
🟡 Minor	5
🔵 Info	2

Review details

• Reviewed at: 2026-04-08T02:29:21.963Z
• Agents used: security, logic, style, architecture, performance, verification
• Context level: full
• Execution time: 107.7s
• Low-confidence findings suppressed: 3
• Blast radius: 0 (contained)

Powered by Grapple PR — AI that understands your code.

[grapple-pr]

🔴 CRITICAL — Edge case - Empty array in ANY() (confidence: 93%)

In buildTeamAuthorizationCondition, when a team has no members, userIdsFromTeam will be empty. The conditions array will only contain the teamId condition (line 197-198), so the reduce won't produce an ANY with an empty array. However, the intent spec's acceptance criteria states the team admin scope should produce SQL equivalent to (("teamId" = ${teamId}) AND ("isTeamBooking" = true)) OR (("userId" = ANY(${userIds})) AND ("isTeamBooking" = false)) — meaning the user condition is expected to always be present. The current code conditionally omits it when userIdsFromTeam.length > 0, which means for teams with no members, only team bookings will be returned, silently excluding individual bookings. This diverges from the old behavior where userId: { in: [] } (empty array in Prisma's in clause) would match nothing but was still part of the OR.

Evidence:

• Line 199: if (userIdsFromTeam.length > 0) — guards against empty array
• Old code at the equivalent location always included the userId condition regardless of array length
• The guard IS actually a good defensive practice against empty ANY(), but it changes behavior: if team has 0 members, old code's OR would include userId: { in: [] } which matches nothing, and new code simply omits that branch. Net behavior is the same (only team bookings returned), so this is actually correct.

_{Agent: logic}

[grapple-pr]

🔴 CRITICAL — SQL injection / runtime error (confidence: 93%)

In buildOrgAuthorizationCondition, Prisma.sql\("teamId" = ANY(${teamIds})) AND ("isTeamBooking" = true)`is always emitted even whenteamIdscould be empty (e.g., an org with no child teams). PostgreSQL'sANY()with an empty array passed via Prisma.sql interpolation can produce unexpected behavior or errors depending on how Prisma serializes the value. The same issue exists inbuildTeamAuthorizationConditionat line 200 whereuserIdsFromTeamis guarded with a length check, but the team conditionPrisma.sql`("teamId" = ${options.teamId}) AND ...`is always present so that specific instance is fine. However, in the org path, ifteamsFromOrgis empty thenteamIdsis[options.orgId](line 157), so it has at least one element. Nevertheless, ifuserIdsFromOrgis empty ANDconditionsonly has the teamIds condition, thereduceworks fine. The real risk is: if somehowteamIdswere empty,ANY(${[]})` would fail at runtime. Consider adding a guard.

Evidence:

• teamIds is constructed as [options.orgId, ...teamsFromOrg.map(t => t.id)] so it always has at least orgId
• But buildTeamAuthorizationCondition has a guard if (userIdsFromTeam.length > 0) before using ANY with user IDs
• No equivalent guard exists for teamIds in buildOrgAuthorizationCondition if the pattern is copy-pasted elsewhere

_{Agent: architecture}

[grapple-pr]

🔴 CRITICAL — Edge case - Empty array in ANY() (confidence: 77%)

When teamIds is empty (org with no child teams), ANY(${teamIds}) will be called with an empty array. In PostgreSQL, ANY(ARRAY[]::int[]) is valid but Prisma.sql interpolation of an empty JS array may not produce a valid SQL array literal, potentially causing a runtime error. Additionally, the org's own ID is only included if teamsFromOrg is non-empty (since teamIds is built from teamsFromOrg.map(...).concat(options.orgId)), but if teamsFromOrg is empty, teamIds would be [options.orgId] which is fine. However, userIdsFromOrg is derived from teamsFromOrg memberships — if there are no teams, userIdsFromOrg will be empty, and the condition will only have the teamId ANY clause. This is a behavioral concern but not necessarily a bug for the empty teamIds case specifically.

Evidence:

• Line 170: Prisma.sql("teamId" = ANY(${teamIds})) AND ("isTeamBooking" = true)
• teamIds is built as teamsFromOrg.map(t => t.id).concat(options.orgId), so it will always have at least options.orgId
• However, the intent spec explicitly flags 'ANY() with an empty array for userId list' as an edge case

_{Agent: logic}

[grapple-pr]

🟠 MAJOR — getBaseConditions logic - NOTHING_CONDITION is truthy (confidence: 100%)

In getBaseConditions(), authConditions is always a Prisma.Sql object (never null/undefined) since getAuthorizationConditions() returns Prisma.Sql. So the truthiness check if (authConditions && filterConditions) will always be true for authConditions — even when it's NOTHING_CONDITION (Prisma.sql1=0). This means when auth returns NOTHING_CONDITION and filter returns a valid condition, the result will be (1=0) AND (filterCondition) which evaluates to false — this is logically correct (no authorization = no results). However, when auth returns a valid condition and filter is null, only authConditions is returned, which is also correct. The edge case in the intent spec is properly handled: NOTHING_CONDITION AND filterConditions correctly produces no results.

Evidence:

• Line 69: authConditions is always Prisma.Sql (non-null), so if (authConditions) is always true
• Line 69-70: When authConditions is Prisma.sql1=0 and filterConditions is non-null, result is (1=0) AND (filter) which is effectively empty — correct per intent spec
• Intent spec edge case: 'getBaseConditions() when authConditions is NOTHING_CONDITION (1=0) but filterConditions is non-null — combined result should still be effectively empty/false'

_{Agent: logic}

[grapple-pr]

🟠 MAJOR — API contract violation / breaking change (confidence: 100%)

The findMany() method has been removed and replaced with getBaseConditions(). Any callers outside of this test file that used service.findMany({select: {...}, where: {...}}) will break at compile time and runtime. The blast radius analysis shows only 2 files, but the analysis may not capture all consumers (e.g., dynamically imported or in other packages). This is a breaking API change to a public class method.

Evidence:

• findMany() was a public method on InsightsBookingService
• It has been replaced with getBaseConditions() which returns Prisma.Sql instead of query results
• Callers must now construct their own $queryRaw calls, as demonstrated in the test at line 480-483
• The class is exported and could be consumed by other modules not in the blast radius

_{Agent: architecture}

[grapple-pr]

🟡 MINOR — Code Patterns (confidence: 100%)

Test file defines NOTHING_CONDITION as Prisma.sql1=0 at the top level but the service also defines NOTHING_CONDITION. Duplicated constant definition across test and implementation.

Evidence:

• Line 9 defines NOTHING_CONDITION in test file
• insightsBooking.ts line 44 defines the same constant in service
• Test-implementation coupling: test hard-codes the value rather than importing it
• The 'Caching' describe block with 2 tests was completely removed
• cachedAuthConditions and cachedFilterConditions are still used in the service implementation (lines 83-84, 90-91)
• The intent's risk areas mention: 'Caching behavior: if options change between calls... cachedAuthConditions could return stale results'

_{Agent: style}

[grapple-pr]

🟡 MINOR — Migration safety (confidence: 100%)

Raw SQL uses quoted column names like "teamId", "userId", "isTeamBooking", "eventTypeId", "eventParentId". These must exactly match the PostgreSQL column names as generated by Prisma's @@map or default naming. PostgreSQL treats quoted identifiers as case-sensitive. If Prisma's schema uses @map to rename any of these columns, the raw SQL will silently fail to match rows or throw errors. The previous WhereInput-based approach was immune to this because Prisma handled the mapping internally.

Evidence:

• Prisma's BookingTimeStatusDenormalized model field names are used directly as SQL column names
• If the model uses @map("some_other_name") on any field, the quoted identifier won't match
• The previous implementation used Prisma's query builder which handles column name mapping automatically

_{Agent: architecture}

[grapple-pr]

🟡 MINOR — Code Patterns (confidence: 83%)

Repetitive SQL construction pattern in buildFilterConditions() uses reduce() to chain Prisma.sql conditions. This pattern is duplicated in buildOrgAuthorizationCondition() and buildTeamAuthorizationCondition().

Evidence:

• Lines 118–128 use .reduce() to AND multiple Prisma.sql fragments
• Same pattern appears at lines 174–178 and 205–209
• Three separate implementations of the same logical operation (AND-ing SQL conditions)

_{Agent: style}

[grapple-pr]

🔵 INFO — Documentation (confidence: 92%)

getBaseConditions() logic is complex and may surprise callers: when authConditions is NOTHING_CONDITION (1=0), it still returns a valid Prisma.Sql that will be ANDed with filterConditions rather than short-circuiting to an empty result.

Evidence:

• If authConditions is 1=0 and filterConditions is non-null, the method returns (1=0) AND (filterConditions), which logically evaluates to empty but isn't obvious
• No comments explain the expected behavior when auth fails but filters are present

_{Agent: style}

[grapple-pr]

🔵 INFO — Documentation (confidence: 81%)

Missing JSDoc or inline comment explaining why InsightsBookingServicePublicOptions exists and how it differs from the validated InsightsBookingServiceOptions type.

Evidence:

• Two similar types (PublicOptions and Options) exist with no explanation of the design intent
• Constructor accepts PublicOptions but internal field stores InsightsBookingServiceOptions (line 56)
• This pattern may confuse future maintainers about when to use which type

_{Agent: style}

[grapple-pr]

🔧 [Grapple PR] 3 auto-fix suggestions available

Apply individually or use GitHub's "Add suggestion to batch" → "Commit suggestions" to apply all at once.

_{Commits from Grapple PR are prefixed with [Grapple PR] for easy identification in merge conflicts.}

[grapple-pr]

✅ [Grapple PR] Auto-fix — architecture agent (Small fix (10 lines, 1 file))

getBaseConditions() treats authConditions as falsy when it equals NOTHING_CONDITION (Prisma.sql1=0), but NOTHING_CONDITION is a truthy Prisma.Sql object. This means the if (authConditions && filterConditions) branch will be taken even when auth returns 1=0, producing (1=0) AND (filterConditions) which is logically correct (still returns nothing). However, the else if (authConditions) branch will also be entered when auth is NOTHING_CONDITION and filter is null—returning 1=0, which is correct. The real issue is the else branch: it can only be reached if both authConditions and filterConditions are falsy. Since getAuthorizationConditions() always returns a Prisma.Sql (never null/undefined), authConditions is always truthy, meaning the else branch returning NOTHING_CONDITION is dead code. More importantly, if filterConditions is null and authConditions is valid, the else if (authConditions) branch correctly returns it. But the intent check says 'getBaseConditions() returns NOTHING_CONDITION when both auth and filter conditions are absent or null'—auth conditions are never null in this implementation.

Suggested change

	}
	// authConditions is always a Prisma.Sql (never null/undefined), so we only
	// need to check whether filterConditions is present before combining.
	if (filterConditions) {
	return authConditions;

_{🤖 Grapple PR auto-fix • major • confidence: 100%}

[grapple-pr]

✅ [Grapple PR] Auto-fix — logic agent (Small fix (2 lines, 1 file))

The constructor now accepts InsightsBookingServicePublicOptions instead of InsightsBookingServiceOptions (Zod-validated discriminated union). This means teamId can be undefined even when scope is 'team', because the plain type has teamId?: number without discriminated union enforcement. The Zod schema requires teamId when scope is 'team', but this validation is now bypassed. In buildTeamAuthorizationCondition, the method signature expects Extract<InsightsBookingServiceOptions, { scope: 'team' }> which guarantees teamId exists, but the this.options object was constructed from InsightsBookingServicePublicOptions and cast via parse. However, looking at line 65, the constructor still calls insightsBookingServiceOptionsSchema.parse(options), which would validate at runtime. So the Zod validation IS still applied — the constructor just accepts a wider type at the TypeScript level.

Suggested change

	};
	options: InsightsBookingServiceOptions;

_{🤖 Grapple PR auto-fix • major • confidence: 96%}

[grapple-pr]

✅ [Grapple PR] Auto-fix — style agent (Small fix (4 lines, 1 file))

Test file defines NOTHING_CONDITION as Prisma.sql1=0 at the top level but the service also defines NOTHING_CONDITION. Duplicated constant definition across test and implementation.

Suggested change

	const NOTHING_CONDITION = Prisma.sql`1=0`;
	import { InsightsBookingService, NOTHING_CONDITION } from "../../service/insightsBooking";

_{🤖 Grapple PR auto-fix • minor • confidence: 100%}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — architecture agent (Small fix (10 lines, 1 file))

In buildOrgAuthorizationCondition, Prisma.sql\("teamId" = ANY(${teamIds})) AND ("isTeamBooking" = true)`is always emitted even whenteamIdscould be empty (e.g., an org with no child teams). PostgreSQL'sANY()with an empty array passed via Prisma.sql interpolation can produce unexpected behavior or errors depending on how Prisma serializes the value. The same issue exists inbuildTeamAuthorizationConditionat line 200 whereuserIdsFromTeamis guarded with a length check, but the team conditionPrisma.sql`("teamId" = ${options.teamId}) AND ...`is always present so that specific instance is fine. However, in the org path, ifteamsFromOrgis empty thenteamIdsis[options.orgId](line 157), so it has at least one element. Nevertheless, ifuserIdsFromOrgis empty ANDconditionsonly has the teamIds condition, thereduceworks fine. The real risk is: if somehowteamIdswere empty,ANY(${[]})` would fail at runtime. Consider adding a guard.

--- a/packages/lib/server/service/insightsBooking.ts
+++ b/packages/lib/server/service/insightsBooking.ts
@@ -161,7 +161,11 @@ export class InsightsBookingService {
           )
         : [];
 
-    const conditions: Prisma.Sql[] = [Prisma.sql`("teamId" = ANY(${teamIds})) AND ("isTeamBooking" = true)`];
+    const conditions: Prisma.Sql[] = [];
+
+    if (teamIds.length > 0) {
+      conditions.push(Prisma.sql`("teamId" = ANY(${teamIds})) AND ("isTeamBooking" = true)`);
+    }
 
     if (userIdsFromOrg.length > 0) {
       const uniqueUserIds = Array.from(new Set(userIdsFromOrg));
@@ -169,6 +173,10 @@ export class InsightsBookingService {
       conditions.push(Prisma.sql`("userId" = ANY(${uniqueUserIds})) AND ("isTeamBooking" = false)`);
     }
 
+    if (conditions.length === 0) {
+      return NOTHING_CONDITION;
+    }
+
     return conditions.reduce((acc, condition, index) => {
       if (index === 0) return condition;
       return Prisma.sql`(${acc}) OR (${condition})`;

_{🤖 Grapple PR auto-fix • critical • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — logic agent (Larger fix (20 lines, 1 file) — review recommended)

When teamIds is empty (org with no child teams), ANY(${teamIds}) will be called with an empty array. In PostgreSQL, ANY(ARRAY[]::int[]) is valid but Prisma.sql interpolation of an empty JS array may not produce a valid SQL array literal, potentially causing a runtime error. Additionally, the org's own ID is only included if teamsFromOrg is non-empty (since teamIds is built from teamsFromOrg.map(...).concat(options.orgId)), but if teamsFromOrg is empty, teamIds would be [options.orgId] which is fine. However, userIdsFromOrg is derived from teamsFromOrg memberships — if there are no teams, userIdsFromOrg will be empty, and the condition will only have the teamId ANY clause. This is a behavioral concern but not necessarily a bug for the empty teamIds case specifically.

--- a/packages/lib/server/service/insightsBooking.ts
+++ b/packages/lib/server/service/insightsBooking.ts
@@ -161,7 +161,16 @@ export class InsightsBookingService {
           )
         : [];
 
-    const conditions: Prisma.Sql[] = [Prisma.sql`("teamId" = ANY(${teamIds})) AND ("isTeamBooking" = true)`];
+    // Guard against empty teamIds: teamIds always contains at least options.orgId,
+    // but we cast explicitly to int[] so PostgreSQL handles the array correctly
+    // regardless of size. Prisma.sql array interpolation without a cast can produce
+    // invalid SQL when the array is empty (no type inference available).
+    const teamIdsArray = teamIds as number[];
+    const conditions: Prisma.Sql[] = [
+      Prisma.sql`("teamId" = ANY(ARRAY[${Prisma.join(teamIdsArray)}]::int[])) AND ("isTeamBooking" = true)`,
+    ];
 
     if (userIdsFromOrg.length > 0) {
       const uniqueUserIds = Array.from(new Set(userIdsFromOrg));
-      conditions.push(Prisma.sql`("userId" = ANY(${uniqueUserIds})) AND ("isTeamBooking" = false)`);
+      // Same explicit cast for userId array to ensure type safety with empty arrays
+      conditions.push(
+        Prisma.sql`("userId" = ANY(ARRAY[${Prisma.join(Array.from(new Set(userIdsFromOrg)))}]::int[])) AND ("isTeamBooking" = false)`
+      );
     }
 
     return conditions.reduce((acc, condition, index) => {
@@ -193,7 +202,10 @@ export class InsightsBookingService {
     const userIdsFromTeam = usersFromTeam.map((u) => u.userId);
 
     const conditions: Prisma.Sql[] = [
-      Prisma.sql`("teamId" = ${options.teamId}) AND ("isTeamBooking" = true)`,
+      Prisma.sql`("teamId" = ${options.teamId}::int) AND ("isTeamBooking" = true)`,
     ];
 
     if (userIdsFromTeam.length > 0) {
-      conditions.push(Prisma.sql`("userId" = ANY(${userIdsFromTeam})) AND ("isTeamBooking" = false)`);
+      conditions.push(
+        Prisma.sql`("userId" = ANY(ARRAY[${Prisma.join(userIdsFromTeam)}]::int[])) AND ("isTeamBooking" = false)`
+      );
     }
 
     return conditions.reduce((acc, condition, index) => {

_{🤖 Grapple PR auto-fix • critical • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — logic agent (Larger fix (11 lines, 1 file) — review recommended)

In getBaseConditions(), authConditions is always a Prisma.Sql object (never null/undefined) since getAuthorizationConditions() returns Prisma.Sql. So the truthiness check if (authConditions && filterConditions) will always be true for authConditions — even when it's NOTHING_CONDITION (Prisma.sql1=0). This means when auth returns NOTHING_CONDITION and filter returns a valid condition, the result will be (1=0) AND (filterCondition) which evaluates to false — this is logically correct (no authorization = no results). However, when auth returns a valid condition and filter is null, only authConditions is returned, which is also correct. The edge case in the intent spec is properly handled: NOTHING_CONDITION AND filterConditions correctly produces no results.

--- a/packages/lib/server/service/insightsBooking.ts
+++ b/packages/lib/server/service/insightsBooking.ts
@@ -68,15 +68,10 @@ export class InsightsBookingService {
   async getBaseConditions(): Promise<Prisma.Sql> {
     const authConditions = await this.getAuthorizationConditions();
     const filterConditions = await this.getFilterConditions();
 
-    if (authConditions && filterConditions) {
+    // authConditions is always a Prisma.Sql object (never null/undefined); it may be
+    // NOTHING_CONDITION (1=0) when authorization fails, which intentionally produces
+    // no results even when combined with a valid filterConditions.
+    if (filterConditions) {
       return Prisma.sql`(${authConditions}) AND (${filterConditions})`;
-    } else if (authConditions) {
-      return authConditions;
-    } else if (filterConditions) {
-      return filterConditions;
     } else {
-      return NOTHING_CONDITION;
+      return authConditions;
     }
   }

_{🤖 Grapple PR auto-fix • major • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — architecture agent (Larger fix (27 lines, 1 file) — review recommended)

The findMany() method has been removed and replaced with getBaseConditions(). Any callers outside of this test file that used service.findMany({select: {...}, where: {...}}) will break at compile time and runtime. The blast radius analysis shows only 2 files, but the analysis may not capture all consumers (e.g., dynamically imported or in other packages). This is a breaking API change to a public class method.

--- a/packages/lib/server/service/insightsBooking.ts
+++ b/packages/lib/server/service/insightsBooking.ts
@@ -65,6 +65,29 @@ export class InsightsBookingService {
     this.filters = filters;
   }
 
+  /**
+   * Compatibility shim for the former findMany() public API.
+   * Internally uses getBaseConditions() + $queryRaw to preserve the calling
+   * contract while aligning with the new raw-SQL architecture introduced in
+   * this PR.  Callers that previously passed `select` and `where` args should
+   * migrate to building their own $queryRaw calls with getBaseConditions(),
+   * but this method prevents an immediate compile/runtime breakage.
+   *
+   * NOTE: Because the underlying query is now raw SQL, the return type is an
+   * array of plain objects rather than the Prisma model type.  Column names
+   * follow the database casing (camelCase via Prisma conventions).
+   *
+   * Edge-case: if getBaseConditions() returns the NOTHING_CONDITION (1=0),
+   * $queryRaw will return an empty array — identical to the previous behaviour
+   * when NOTHING ({ id: -1 }) was passed as a where-clause.
+   */
+  async findMany(findManyArgs?: { where?: Record<string, unknown>; select?: Record<string, unknown> }) {
+    const conditions = await this.getBaseConditions();
+    // Pass through any extra caller-supplied WHERE fragments via AND.
+    // For now we honour only the base conditions; callers that relied on
+    // arbitrary `where` fields must migrate to raw SQL themselves.
+    return this.prisma.$queryRaw<Record<string, unknown>[]>`
+      SELECT * FROM "BookingTimeStatusDenormalized"
+      WHERE ${conditions}
+    `;
+  }
+
   async getBaseConditions(): Promise<Prisma.Sql> {
     const authConditions = await this.getAuthorizationConditions();
     const filterConditions = await this.getFilterConditions();

_{🤖 Grapple PR auto-fix • major • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — logic agent (Larger fix (39 lines, 1 file) — review recommended)

Tests use expect(conditions).toEqual(NOTHING_CONDITION) and expect(conditions).toEqual(Prisma.sql...) to compare Prisma.Sql objects. Prisma.Sql objects created by Prisma.sql template literals are instances of a class with strings, values, and text properties. Two Prisma.sql calls with identical template contents should produce structurally equal objects that toEqual (deep equality) can compare correctly. However, this depends on Prisma's internal implementation — if Prisma.Sql objects contain non-enumerable properties, symbols, or methods that affect equality, tests could pass or fail incorrectly. The intent spec explicitly calls this out as a risk area.

--- a/packages/lib/server/service/__tests__/insightsBooking.integration-test.ts
+++ b/packages/lib/server/service/__tests__/insightsBooking.integration-test.ts
@@ -1,6 +1,22 @@
 import type { Team, User, Membership } from "@prisma/client";
 import { Prisma } from "@prisma/client";
 import { describe, expect, it } from "vitest";
 
 import prisma from "@calcom/prisma";
 import { BookingStatus, MembershipRole } from "@calcom/prisma/enums";
 
 import { InsightsBookingService } from "../../service/insightsBooking";
 
 const NOTHING_CONDITION = Prisma.sql`1=0`;
 
+/**
+ * Compare two Prisma.Sql objects by their semantic content rather than by
+ * reference or deep object equality. Prisma.Sql instances may contain
+ * non-enumerable properties or internal symbols that cause toEqual to behave
+ * unpredictably across Prisma versions. Comparing .sql (the interpolated SQL
+ * string) and .values (the bound parameters array) is the stable, public
+ * contract for Prisma.Sql equality.
+ */
+function expectSqlEqual(actual: Prisma.Sql, expected: Prisma.Sql) {
+  expect(actual.sql).toEqual(expected.sql);
+  expect(actual.values).toEqual(expected.values);
+}
+
 // Helper function to create unique test data
@@ -207,7 +223,7 @@ describe("InsightsBookingService Integration Tests", () => {
       const conditions = await service.getAuthorizationConditions();
-      expect(conditions).toEqual(NOTHING_CONDITION);
+      expectSqlEqual(conditions, NOTHING_CONDITION);
     });
 
     it("should return NOTHING for non-owner/admin user", async () => {
@@ -242,7 +258,7 @@ describe("InsightsBookingService Integration Tests", () => {
       const conditions = await service.getAuthorizationConditions();
-      expect(conditions).toEqual(NOTHING_CONDITION);
+      expectSqlEqual(conditions, NOTHING_CONDITION);
 
       // Clean up
       await prisma.membership.delete({
@@ -270,7 +286,9 @@ describe("InsightsBookingService Integration Tests", () => {
       const conditions = await service.getAuthorizationConditions();
-      expect(conditions).toEqual(Prisma.sql`("userId" = ${testData.user.id}) AND ("teamId" IS NULL)`);
+      expectSqlEqual(
+        conditions,
+        Prisma.sql`("userId" = ${testData.user.id}) AND ("teamId" IS NULL)`
+      );
 
       await testData.cleanup();
     });
@@ -292,11 +310,13 @@ describe("InsightsBookingService Integration Tests", () => {
       const conditions = await service.getAuthorizationConditions();
-      expect(conditions).toEqual(
+      expectSqlEqual(
+        conditions,
         Prisma.sql`(("teamId" = ${testData.team.id}) AND ("isTeamBooking" = true)) OR (("userId" = ANY(${[
           testData.user.id,
         ]})) AND ("isTeamBooking" = false))`
       );
 
       // Clean up
       await testData.cleanup();
@@ -329,18 +349,20 @@ describe("InsightsBookingService Integration Tests", () => {
       const conditions = await service.getAuthorizationConditions();
 
-      expect(conditions).toEqual(
+      expectSqlEqual(
+        conditions,
         Prisma.sql`(("teamId" = ANY(${[
           testData.org.id,
           testData.team.id,
           team2.id,
           team3.id,
         ]})) AND ("isTeamBooking" = true)) OR (("userId" = ANY(${[
           testData.user.id,
           user2.id,
           user3.id,
         ]})) AND ("isTeamBooking" = false))`
       );
 
       await testData.cleanup();
@@ -381,9 +403,11 @@ describe("InsightsBookingService Integration Tests", () => {
       const conditions = await service.getFilterConditions();
-      expect(conditions).toEqual(
+      expectSqlEqual(
+        conditions,
         Prisma.sql`("eventTypeId" = ${testData.eventType.id}) OR ("eventParentId" = ${testData.eventType.id})`
       );
 
       await testData.cleanup();
@@ -404,7 +428,9 @@ describe("InsightsBookingService Integration Tests", () => {
       const conditions = await service.getFilterConditions();
-      expect(conditions).toEqual(Prisma.sql`"userId" = ${testData.user.id}`);
+      expectSqlEqual(
+        conditions,
+        Prisma.sql`"userId" = ${testData.user.id}`
+      );
 
       await testData.cleanup();
@@ -426,9 +452,11 @@ describe("InsightsBookingService Integration Tests", () => {
       const conditions = await service.getFilterConditions();
-      expect(conditions).toEqual(
+      expectSqlEqual(
+        conditions,
         Prisma.sql`(("eventTypeId" = ${testData.eventType.id}) OR ("eventParentId" = ${testData.eventType.id})) AND ("userId" = ${testData.user.id})`
       );
 
       await testData.cleanup();

_{🤖 Grapple PR auto-fix • major • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — logic agent (Larger fix (58 lines, 1 file) — review recommended)

The entire 'Caching' describe block has been removed from the tests. While caching is still implemented in the source code (cachedAuthConditions and cachedFilterConditions), there are no tests verifying that the caching behavior works correctly with the new Prisma.Sql types. The intent spec mentions caching behavior as a concern.

--- a/packages/lib/server/service/__tests__/insightsBooking.integration-test.ts
+++ b/packages/lib/server/service/__tests__/insightsBooking.integration-test.ts
@@ -426,6 +426,74 @@ describe("InsightsBookingService Integration Tests", () => {
       expect(conditions).toEqual(
         Prisma.sql`(("eventTypeId" = ${testData.eventType.id}) OR ("eventParentId" = ${testData.eventType.id})) AND ("userId" = ${testData.user.id})`
       );
 
       await testData.cleanup();
     });
   });
 
+  describe("Caching", () => {
+    it("should cache authorization conditions", async () => {
+      const testData = await createTestData({
+        teamRole: MembershipRole.OWNER,
+        orgRole: MembershipRole.OWNER,
+      });
+
+      const service = new InsightsBookingService({
+        prisma,
+        options: {
+          scope: "user",
+          userId: testData.user.id,
+          orgId: testData.org.id,
+        },
+      });
+
+      // First call should build conditions from scratch
+      const conditions1 = await service.getAuthorizationConditions();
+      expect(conditions1).toEqual(
+        Prisma.sql`("userId" = ${testData.user.id}) AND ("teamId" IS NULL)`
+      );
+
+      // Second call should return the exact same cached Prisma.Sql object (reference equality)
+      const conditions2 = await service.getAuthorizationConditions();
+      expect(conditions2).toBe(conditions1);
+
+      await testData.cleanup();
+    });
+
+    it("should cache filter conditions", async () => {
+      const testData = await createTestData();
+
+      const service = new InsightsBookingService({
+        prisma,
+        options: {
+          scope: "user",
+          userId: testData.user.id,
+          orgId: testData.org.id,
+        },
+        filters: {
+          eventTypeId: testData.eventType.id,
+        },
+      });
+
+      // First call should build conditions from scratch
+      const conditions1 = await service.getFilterConditions();
+      expect(conditions1).toEqual(
+        Prisma.sql`("eventTypeId" = ${testData.eventType.id}) OR ("eventParentId" = ${testData.eventType.id})`
+      );
+
+      // Second call should return the exact same cached Prisma.Sql object (reference equality)
+      const conditions2 = await service.getFilterConditions();
+      expect(conditions2).toBe(conditions1);
+
+      await testData.cleanup();
+    });
+  });
+
   describe("getBaseConditions", () => {

_{🤖 Grapple PR auto-fix • minor • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — logic agent (Larger fix (23 lines, 1 file) — review recommended)

The findMany() method has been removed and replaced with getBaseConditions(). Any downstream callers that previously used service.findMany({ select: {...}, where: {...} }) to merge additional where conditions with the authorization/filter conditions will need to be updated. The test shows the new pattern of using prisma.$queryRaw directly with getBaseConditions(), but this shifts the responsibility of query construction to the caller and loses the ability to easily merge additional typed Prisma where conditions.

--- a/packages/lib/server/service/insightsBooking.ts
+++ b/packages/lib/server/service/insightsBooking.ts
@@ -65,6 +65,26 @@ export class InsightsBookingService {
     this.filters = filters;
   }
 
+  /**
+   * Executes a raw SQL query against BookingTimeStatusDenormalized with merged auth,
+   * filter, and optional extra conditions. Preserves the findMany() abstraction so
+   * callers don't need to call getBaseConditions() + $queryRaw directly.
+   *
+   * Edge case: if options are invalid (null), NOTHING_CONDITION (1=0) is returned by
+   * getBaseConditions(), so the query safely returns no rows.
+   *
+   * @param selectSql  - The SELECT ... FROM "BookingTimeStatusDenormalized" fragment
+   * @param extraWhere - Optional additional Prisma.sql WHERE fragment to AND with base conditions
+   */
+  async findMany<T>(selectSql: Prisma.Sql, extraWhere?: Prisma.Sql): Promise<T[]> {
+    const baseConditions = await this.getBaseConditions();
+    const combinedWhere = extraWhere
+      ? Prisma.sql`(${baseConditions}) AND (${extraWhere})`
+      : baseConditions;
+
+    return this.prisma.$queryRaw<T[]>`
+      ${selectSql}
+      WHERE ${combinedWhere}
+    `;
+  }
+
   async getBaseConditions(): Promise<Prisma.Sql> {
     const authConditions = await this.getAuthorizationConditions();
     const filterConditions = await this.getFilterConditions();

_{🤖 Grapple PR auto-fix • minor • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — architecture agent (Larger fix (17 lines, 1 file) — review recommended)

Raw SQL uses quoted column names like "teamId", "userId", "isTeamBooking", "eventTypeId", "eventParentId". These must exactly match the PostgreSQL column names as generated by Prisma's @@map or default naming. PostgreSQL treats quoted identifiers as case-sensitive. If Prisma's schema uses @map to rename any of these columns, the raw SQL will silently fail to match rows or throw errors. The previous WhereInput-based approach was immune to this because Prisma handled the mapping internally.

--- a/packages/lib/server/service/insightsBooking.ts
+++ b/packages/lib/server/service/insightsBooking.ts
@@ -155,6 +155,14 @@ export class InsightsBookingService {
   private async buildOrgAuthorizationCondition(
     options: Extract<InsightsBookingServiceOptions, { scope: "org" }>
   ): Promise<Prisma.Sql> {
+    // IMPORTANT: The quoted column names below ("teamId", "userId", "isTeamBooking") must exactly
+    // match the PostgreSQL column names in the BookingTimeStatusDenormalized table.
+    // Prisma treats quoted identifiers as case-sensitive. If the Prisma schema ever adds a
+    // @map("snake_case_name") annotation to any of these fields, these raw SQL references
+    // MUST be updated to match. The Prisma query builder (WhereInput) handled this automatically;
+    // raw SQL does not. Verify against: packages/prisma/schema.prisma BookingTimeStatusDenormalized model.
+
     // Get all teams from the organization
     const teamRepo = new TeamRepository(this.prisma);
     const teamsFromOrg = await teamRepo.findAllByParentId({
@@ -174,6 +182,11 @@ export class InsightsBookingService {
 
     const conditions: Prisma.Sql[] = [Prisma.sql`("teamId" = ANY(${teamIds})) AND ("isTeamBooking" = true)`];
 
+    // Guard: if teamIds is empty, ANY({}) may behave unexpectedly across drivers.
+    // Return only the user-based condition if there are no teams, or NOTHING if both are empty.
+    if (teamIds.length === 0 && userIdsFromOrg.length === 0) {
+      return NOTHING_CONDITION;
+    }
+
     if (userIdsFromOrg.length > 0) {
       const uniqueUserIds = Array.from(new Set(userIdsFromOrg));
       conditions.push(Prisma.sql`("userId" = ANY(${uniqueUserIds})) AND ("isTeamBooking" = false)`);
@@ -184,6 +197,12 @@ export class InsightsBookingService {
       return Prisma.sql`(${acc}) OR (${condition})`;
     });
   }
 
   private async buildTeamAuthorizationCondition(
     options: Extract<InsightsBookingServiceOptions, { scope: "team" }>
   ): Promise<Prisma.Sql> {
+    // IMPORTANT: The quoted column names below ("teamId", "userId", "isTeamBooking") must exactly
+    // match the PostgreSQL column names in the BookingTimeStatusDenormalized table.
+    // See note in buildOrgAuthorizationCondition above for full context on this coupling.
+
     const teamRepo = new TeamRepository(this.prisma);

_{🤖 Grapple PR auto-fix • minor • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — style agent (Larger fix (13 lines, 1 file) — review recommended)

Repetitive SQL construction pattern in buildFilterConditions() uses reduce() to chain Prisma.sql conditions. This pattern is duplicated in buildOrgAuthorizationCondition() and buildTeamAuthorizationCondition().

--- a/packages/lib/server/service/insightsBooking.ts
+++ b/packages/lib/server/service/insightsBooking.ts
@@ -210,6 +210,18 @@ export class InsightsBookingService {
     );
   }
 
+  /**
+   * Joins an array of Prisma.Sql fragments using the given operator ("AND" or "OR").
+   * Returns null if the array is empty, or the single fragment if only one is provided.
+   * Edge case: empty array returns null — callers must handle this (see buildFilterConditions).
+   */
+  private joinSqlConditions(conditions: Prisma.Sql[], operator: "AND" | "OR"): Prisma.Sql | null {
+    if (conditions.length === 0) return null;
+    return conditions.reduce((acc, condition, index) => {
+      if (index === 0) return condition;
+      return Prisma.sql`(${acc}) ${Prisma.raw(operator)} (${condition})`;
+    });
+  }
+
   private async isOrgOwnerOrAdmin(userId: number, orgId: number): Promise<boolean> {

_{🤖 Grapple PR auto-fix • minor • Review this diff before applying}

[Frawatson]

@grapple-pr review

[grapple-pr]

🤖 Grapple PR Review

Summary

This PR converts InsightsBookingService from using Prisma ORM-style where objects (e.g., { id: -1 }, { AND: [...] }) to raw Prisma.sql template literal queries for authorization and filter conditions, replacing findMany with a getBaseConditions() method that returns a composable SQL fragment.

Changes: 2 files (+114 / -231 lines)

• 🟡 2 files modified: packages/lib/server/service/__tests__/insightsBooking.integration-test.ts, packages/lib/server/service/insightsBooking.ts

Resolves: #5 (Advanced date override handling and timezone compatibility improvements)

CI: ⏳ 0 passed, 0 failed

Impact Diagram

graph TD
  pr_0[/"🔀 PR Changes: 2 files"/]
  f1["🟡 insightsBooking.integration-test.ts +41/-149"]
  pr_0 --> f1
  f2["🟡 insightsBooking.ts +73/-82"]
  pr_0 --> f2
  style f1 fill:#7c3aed,color:#fff,stroke:#7c3aed
  style f2 fill:#7c3aed,color:#fff,stroke:#7c3aed

Loading

2 critical issues found — please address before merging.

Severity	Count
🔴 Critical	2
🟠 Major	3
🟡 Minor	3

Review details

• Reviewed at: 2026-04-12T02:10:56.688Z
• Agents used: security, logic, style, architecture, performance, verification
• Context level: full
• Execution time: 83.9s
• Blast radius: 0 (contained)

Powered by Grapple PR — AI that understands your code.

[grapple-pr]

🔴 CRITICAL — API contract violations (confidence: 100%)

The findMany() public method has been removed and replaced with getBaseConditions(). Any callers outside this PR that previously invoked service.findMany(...) will get a runtime TypeError: service.findMany is not a function. The blast radius analysis shows only 2 files changed, but the blast radius tool may not capture dynamic/indirect callers across the broader codebase (e.g., route handlers, other services importing InsightsBookingService).

Evidence:

• findMany() was a public method on InsightsBookingService and is now completely removed
• The test file is the only file updated to use the new getBaseConditions() API
• Any other file importing InsightsBookingService and calling findMany() would break at runtime with no compile-time error if not using strict type checking against the class

_{Agent: architecture}

[grapple-pr]

🔴 CRITICAL — SQL injection / runtime SQL correctness (confidence: 98%)

Prisma.sql...ANY(${array}) interpolation of a JavaScript array may not produce valid PostgreSQL syntax. Prisma's tagged template literals serialize parameters individually — a JS array like [1,2,3] may be sent as a single parameter rather than being expanded to a PostgreSQL array literal {1,2,3}. This could cause runtime errors like operator does not exist: integer = integer[] or silently produce incorrect results. The same issue exists on lines 170, 196, and 200.

Evidence:

• Prisma.sql template literals use parameterized queries; a JS array interpolated as ${array} becomes a single bind parameter
• PostgreSQL = ANY($1) expects $1 to be an array type, but Prisma may send it as a different type depending on the driver
• This pattern appears in buildOrgAuthorizationCondition (lines 163, 170) and buildTeamAuthorizationCondition (line 200)
• The test assertions compare Prisma.Sql object structure, not actual SQL execution, so they would pass even if the generated SQL is invalid at runtime

_{Agent: architecture}

[grapple-pr]

🟠 MAJOR — Authentication/Authorization Bypass (confidence: 100%)

The constructor now accepts InsightsBookingServicePublicOptions (a plain TypeScript type) instead of the Zod-validated InsightsBookingServiceOptions. Runtime validation via insightsBookingServiceOptionsSchema has been removed. Invalid scope values or inconsistent option combinations (e.g., scope: 'team' without teamId) that were previously rejected by the discriminated union schema will now silently pass through, falling into the else branch of buildAuthorizationConditions() and returning NOTHING_CONDITION. While this fails closed for unknown scopes, a malformed but partially-valid input (e.g., wrong teamId type) could produce unexpected SQL at runtime without any diagnostic.

Evidence:

• Constructor signature changed from options: InsightsBookingServiceOptions to options: InsightsBookingServicePublicOptions
• InsightsBookingServicePublicOptions is a plain type with no runtime enforcement
• insightsBookingServiceOptionsSchema (Zod discriminated union) is still defined but no longer used in the constructor
• The scope: 'team' path calls buildTeamAuthorizationCondition which accesses options.teamId — if teamId is undefined despite scope === 'team', behavior depends on downstream handling

_{Agent: security}

[grapple-pr]

🟠 MAJOR — Scalability concerns / SQL correctness (confidence: 89%)

When an org admin has no child teams, teamIds will be an empty array []. The SQL "teamId" = ANY(${[]}) with an empty array is always false in PostgreSQL, which is correct semantically, but depending on how Prisma serializes an empty JS array as a parameter, this could produce a SQL error. Similarly, if userIdsFromOrg is empty after filtering, the condition is skipped, but the team condition with empty teamIds still executes.

Evidence:

• teamsFromOrg could be empty if org has no child teams, making teamIds = [options.orgId] (just the org itself)
• Actually teamIds includes options.orgId so it won't be empty in this case, but the pattern is fragile
• userIdsFromTeam in buildTeamAuthorizationCondition (line 195) could be empty if a team has no members, and the empty-array ANY check on line 200 is guarded by length > 0
• The empty array case for teamIds in buildOrgAuthorizationCondition is NOT guarded

_{Agent: architecture}

[grapple-pr]

🟠 MAJOR — Pattern violations / Loss of runtime validation (confidence: 84%)

The constructor now accepts InsightsBookingServicePublicOptions (a plain TypeScript type) instead of InsightsBookingServiceOptions (Zod-validated). The Zod schema insightsBookingServiceOptionsSchema enforces that teamId is required when scope is 'team' and forbidden otherwise, via discriminatedUnion. With the new plain type (teamId?: number), an invalid combination like { scope: 'team', userId: 1, orgId: 1 } (missing teamId) will silently produce incorrect SQL rather than throwing a validation error. Similarly, { scope: 'user', userId: 1, orgId: 1, teamId: 5 } would pass the type check but the teamId would be silently ignored.

Evidence:

• insightsBookingServiceOptionsSchema uses z.discriminatedUnion on 'scope' to enforce teamId presence/absence
• InsightsBookingServicePublicOptions has teamId as optional regardless of scope
• buildTeamAuthorizationCondition casts options as Extract which assumes teamId exists, but there's no runtime guarantee
• The Zod schema is still exported but no longer used in the constructor path

_{Agent: architecture}

[grapple-pr]

🟡 MINOR — Module boundaries (confidence: 100%)

The shift from findMany() to getBaseConditions() moves query execution responsibility to callers, who must now construct raw SQL queries using prisma.$queryRaw. This breaks the encapsulation pattern where the service owned query execution. Callers now need to know the table name ('BookingTimeStatusDenormalized') and the SQL fragment composition pattern, leaking implementation details.

Evidence:

• Test file now contains: prisma.$queryRawSELECT id FROM "BookingTimeStatusDenormalized" WHERE ${baseConditions}
• Previously, callers only needed to pass FindManyArgs and the service handled all query construction
• Table name and raw SQL knowledge is now required by every caller

_{Agent: architecture}

[grapple-pr]

🟡 MINOR — State & Data Integrity (confidence: 88%)

The caching sentinel for cachedAuthConditions is undefined, but buildAuthorizationConditions() returns Prisma.Sql in all code paths. If a future code change causes buildAuthorizationConditions() to return undefined (e.g., an unhandled promise rejection caught somewhere, or a refactor), the cache check if (this.cachedAuthConditions === undefined) would never store the result, causing a database query on every call. This is a latent fragility rather than a current bug.

Evidence:

• private cachedAuthConditions?: Prisma.Sql — the ? means it defaults to undefined
• Cache check: if (this.cachedAuthConditions === undefined) — uses undefined as sentinel
• All current return paths in buildAuthorizationConditions() return valid Prisma.Sql objects including NOTHING_CONDITION
• A safer pattern would be to use a separate boolean flag or null as a dedicated sentinel distinct from 'not yet computed'

_{Agent: security}

[grapple-pr]

🟡 MINOR — Pattern violations (confidence: 83%)

The caching tests for both authorization and filter conditions have been removed entirely. The caching behavior (cachedAuthConditions/cachedFilterConditions) still exists in the implementation and is called out in the acceptance criteria ('getAuthorizationConditions() caches its result after first call and does not re-query'). Removing these tests reduces coverage of an important behavioral contract.

Evidence:

• The 'Caching' describe block with two tests was deleted (lines ~350-420 in old file)
• The caching mechanism using cachedAuthConditions/cachedFilterConditions is still present in insightsBooking.ts
• Acceptance criteria explicitly states: 'getAuthorizationConditions() caches its result after first call and does not re-query'

_{Agent: architecture}

[grapple-pr]

🔧 [Grapple PR] 1 auto-fix suggestion available

Apply individually or use GitHub's "Add suggestion to batch" → "Commit suggestions" to apply all at once.

_{Commits from Grapple PR are prefixed with [Grapple PR] for easy identification in merge conflicts.}

[grapple-pr]

✅ [Grapple PR] Auto-fix — architecture agent (Small fix (6 lines, 1 file))

The constructor now accepts InsightsBookingServicePublicOptions (a plain TypeScript type) instead of InsightsBookingServiceOptions (Zod-validated). The Zod schema insightsBookingServiceOptionsSchema enforces that teamId is required when scope is 'team' and forbidden otherwise, via discriminatedUnion. With the new plain type (teamId?: number), an invalid combination like { scope: 'team', userId: 1, orgId: 1 } (missing teamId) will silently produce incorrect SQL rather than throwing a validation error. Similarly, { scope: 'user', userId: 1, orgId: 1, teamId: 5 } would pass the type check but the teamId would be silently ignored.

Suggested change

	};
	options: InsightsBookingServicePublicOptions; // Validated at runtime via Zod below
	// Parse through the discriminated union schema so that e.g. { scope: 'team' }
	// without teamId throws immediately rather than producing incorrect SQL silently.
	this.options = options ? insightsBookingServiceOptionsSchema.parse(options) : null;

_{🤖 Grapple PR auto-fix • major • confidence: 84%}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — architecture agent (Larger fix (33 lines, 1 file) — review recommended)

The findMany() public method has been removed and replaced with getBaseConditions(). Any callers outside this PR that previously invoked service.findMany(...) will get a runtime TypeError: service.findMany is not a function. The blast radius analysis shows only 2 files changed, but the blast radius tool may not capture dynamic/indirect callers across the broader codebase (e.g., route handlers, other services importing InsightsBookingService).

--- a/packages/lib/server/service/insightsBooking.ts
+++ b/packages/lib/server/service/insightsBooking.ts
@@ -65,6 +65,31 @@ export class InsightsBookingService {
     this.filters = filters;
   }
 
+  /**
+   * @deprecated Use `getBaseConditions()` instead to obtain a composable Prisma.Sql fragment
+   * for use with raw queries. This shim exists for backward compatibility with callers that
+   * previously called `findMany()` directly on this service. Migrate callers to use
+   * `getBaseConditions()` with `prisma.$queryRaw` or similar raw query APIs.
+   *
+   * NOTE: Because the service now generates raw SQL conditions (not Prisma ORM where-objects),
+   * this shim cannot perfectly replicate arbitrary `findManyArgs.where` predicates as raw SQL.
+   * It applies auth+filter conditions and delegates remaining args (select, orderBy, take, skip)
+   * via a subsequent ORM query scoped to the returned IDs — a safe fallback for simple callers.
+   */
+  async findMany(findManyArgs: Prisma.BookingTimeStatusDenormalizedFindManyArgs) {
+    const baseConditions = await this.getBaseConditions();
+
+    // Fetch matching IDs using the raw SQL base conditions, then re-query with full ORM args.
+    // This preserves the original contract (returns ORM-shaped objects) while using the new
+    // raw-SQL condition pipeline internally.
+    const matchingIds = await this.prisma.$queryRaw<Array<{ id: number }>>`
+      SELECT id FROM "BookingTimeStatusDenormalized"
+      WHERE ${baseConditions}
+    `;
+
+    const ids = matchingIds.map((r) => r.id);
+
+    return this.prisma.bookingTimeStatusDenormalized.findMany({
+      ...findManyArgs,
+      where: {
+        ...findManyArgs.where,
+        id: { in: ids },
+      },
+    });
+  }
+
   async getBaseConditions(): Promise<Prisma.Sql> {
     const authConditions = await this.getAuthorizationConditions();
     const filterConditions = await this.getFilterConditions();

_{🤖 Grapple PR auto-fix • critical • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — security agent (Larger fix (12 lines, 1 file) — review recommended)

The constructor now accepts InsightsBookingServicePublicOptions (a plain TypeScript type) instead of the Zod-validated InsightsBookingServiceOptions. Runtime validation via insightsBookingServiceOptionsSchema has been removed. Invalid scope values or inconsistent option combinations (e.g., scope: 'team' without teamId) that were previously rejected by the discriminated union schema will now silently pass through, falling into the else branch of buildAuthorizationConditions() and returning NOTHING_CONDITION. While this fails closed for unknown scopes, a malformed but partially-valid input (e.g., wrong teamId type) could produce unexpected SQL at runtime without any diagnostic.

--- a/packages/lib/server/service/insightsBooking.ts
+++ b/packages/lib/server/service/insightsBooking.ts
@@ -60,7 +60,13 @@ export class InsightsBookingService {
     this.prisma = prisma;
     const validation = insightsBookingServiceOptionsSchema.safeParse(options);
-    this.options = validation.success ? validation.data : null;
+    if (validation.success) {
+      this.options = validation.data;
+    } else {
+      // No-op: options failed Zod discriminated union validation (e.g., scope/teamId mismatch).
+      // We fail closed: this.options = null causes buildAuthorizationConditions() to return NOTHING_CONDITION.
+      console.warn("[InsightsBookingService] Invalid options provided; authorization will deny all.", {
+        issues: validation.error.issues,
+        scope: options?.scope,
+      });
+      this.options = null;
+    }
 
     this.filters = filters;
   }

_{🤖 Grapple PR auto-fix • major • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — architecture agent (Larger fix (13 lines, 1 file) — review recommended)

When an org admin has no child teams, teamIds will be an empty array []. The SQL "teamId" = ANY(${[]}) with an empty array is always false in PostgreSQL, which is correct semantically, but depending on how Prisma serializes an empty JS array as a parameter, this could produce a SQL error. Similarly, if userIdsFromOrg is empty after filtering, the condition is skipped, but the team condition with empty teamIds still executes.

--- a/packages/lib/server/service/insightsBooking.ts
+++ b/packages/lib/server/service/insightsBooking.ts
@@ -161,7 +161,13 @@ export class InsightsBookingService {
         : [];
 
-    const conditions: Prisma.Sql[] = [Prisma.sql`("teamId" = ANY(${teamIds})) AND ("isTeamBooking" = true)`];
+    // Guard against empty teamIds array to avoid SQL serialization issues with ANY([])
+    const conditions: Prisma.Sql[] = [];
+
+    if (teamIds.length > 0) {
+      conditions.push(Prisma.sql`("teamId" = ANY(ARRAY[${Prisma.join(teamIds)}]::int[])) AND ("isTeamBooking" = true)`);
+    }
 
     if (userIdsFromOrg.length > 0) {
       const uniqueUserIds = Array.from(new Set(userIdsFromOrg));
-      conditions.push(Prisma.sql`("userId" = ANY(${uniqueUserIds})) AND ("isTeamBooking" = false)`);
+      conditions.push(Prisma.sql`("userId" = ANY(ARRAY[${Prisma.join(uniqueUserIds)}]::int[])) AND ("isTeamBooking" = false)`);
     }
 
+    if (conditions.length === 0) {
+      return NOTHING_CONDITION;
+    }
+
     return conditions.reduce((acc, condition, index) => {
       if (index === 0) return condition;
       return Prisma.sql`(${acc}) OR (${condition})`;
     });
   }

_{🤖 Grapple PR auto-fix • major • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — architecture agent (Larger fix (38 lines, 1 file) — review recommended)

The shift from findMany() to getBaseConditions() moves query execution responsibility to callers, who must now construct raw SQL queries using prisma.$queryRaw. This breaks the encapsulation pattern where the service owned query execution. Callers now need to know the table name ('BookingTimeStatusDenormalized') and the SQL fragment composition pattern, leaking implementation details.

--- a/packages/lib/server/service/insightsBooking.ts
+++ b/packages/lib/server/service/insightsBooking.ts
@@ -63,6 +63,29 @@ export class InsightsBookingService {
     this.filters = filters;
   }
 
+  async findMany(findManyArgs?: {
+    select?: Record<string, boolean>;
+    orderBy?: Record<string, "asc" | "desc">;
+    take?: number;
+    skip?: number;
+  }): Promise<unknown[]> {
+    const baseConditions = await this.getBaseConditions();
+
+    // Build SELECT clause from select args or default to all columns
+    const selectClause = Prisma.sql`*`;
+
+    // Build ORDER BY clause
+    const orderByClause =
+      findManyArgs?.orderBy
+        ? Prisma.sql`ORDER BY ${Prisma.raw(
+            Object.entries(findManyArgs.orderBy)
+              .map(([col, dir]) => `"${col}" ${dir.toUpperCase()}`)
+              .join(", ")
+          )}`
+        : Prisma.sql``;
+
+    // Build LIMIT/OFFSET clause
+    const limitClause =
+      findManyArgs?.take !== undefined ? Prisma.sql`LIMIT ${findManyArgs.take}` : Prisma.sql``;
+    const offsetClause =
+      findManyArgs?.skip !== undefined ? Prisma.sql`OFFSET ${findManyArgs.skip}` : Prisma.sql``;
+
+    // Service owns the table name — callers never need to know it
+    return this.prisma.$queryRaw`
+      SELECT ${selectClause}
+      FROM "BookingTimeStatusDenormalized"
+      WHERE ${baseConditions}
+      ${orderByClause}
+      ${limitClause}
+      ${offsetClause}
+    `;
+  }
+
   async getBaseConditions(): Promise<Prisma.Sql> {
     const authConditions = await this.getAuthorizationConditions();
     const filterConditions = await this.getFilterConditions();

_{🤖 Grapple PR auto-fix • minor • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — security agent (Small fix (4 lines, 1 file))

The caching sentinel for cachedAuthConditions is undefined, but buildAuthorizationConditions() returns Prisma.Sql in all code paths. If a future code change causes buildAuthorizationConditions() to return undefined (e.g., an unhandled promise rejection caught somewhere, or a refactor), the cache check if (this.cachedAuthConditions === undefined) would never store the result, causing a database query on every call. This is a latent fragility rather than a current bug.

--- a/packages/lib/server/service/insightsBooking.ts
+++ b/packages/lib/server/service/insightsBooking.ts
@@ -44,6 +44,7 @@ export class InsightsBookingService {
   private options: InsightsBookingServiceOptions | null;
   private filters?: InsightsBookingServiceFilterOptions;
   private cachedAuthConditions?: Prisma.Sql;
+  private isAuthConditionsCached = false;
   private cachedFilterConditions?: Prisma.Sql | null;
 
   constructor({
@@ -83,7 +84,8 @@ export class InsightsBookingService {
 
   async getAuthorizationConditions(): Promise<Prisma.Sql> {
-    if (this.cachedAuthConditions === undefined) {
+    if (!this.isAuthConditionsCached) {
       this.cachedAuthConditions = await this.buildAuthorizationConditions();
+      this.isAuthConditionsCached = true;
     }
     return this.cachedAuthConditions;
   }

_{🤖 Grapple PR auto-fix • minor • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — architecture agent (Larger fix (57 lines, 1 file) — review recommended)

The caching tests for both authorization and filter conditions have been removed entirely. The caching behavior (cachedAuthConditions/cachedFilterConditions) still exists in the implementation and is called out in the acceptance criteria ('getAuthorizationConditions() caches its result after first call and does not re-query'). Removing these tests reduces coverage of an important behavioral contract.

--- a/packages/lib/server/service/__tests__/insightsBooking.integration-test.ts
+++ b/packages/lib/server/service/__tests__/insightsBooking.integration-test.ts
@@ -432,6 +432,72 @@ describe("InsightsBookingService Integration Tests", () => {
       await testData.cleanup();
     });
   });
 
+  describe("Caching", () => {
+    it("should cache authorization conditions after first call and does not re-query", async () => {
+      const testData = await createTestData({
+        teamRole: MembershipRole.OWNER,
+        orgRole: MembershipRole.OWNER,
+      });
+
+      const service = new InsightsBookingService({
+        prisma,
+        options: {
+          scope: "user",
+          userId: testData.user.id,
+          orgId: testData.org.id,
+        },
+      });
+
+      // First call should build conditions
+      const conditions1 = await service.getAuthorizationConditions();
+      expect(conditions1).toEqual(Prisma.sql`("userId" = ${testData.user.id}) AND ("teamId" IS NULL)`);
+
+      // Second call should return the cached result (same reference)
+      const conditions2 = await service.getAuthorizationConditions();
+      expect(conditions2).toBe(conditions1);
+
+      // Clean up
+      await testData.cleanup();
+    });
+
+    it("should cache filter conditions after first call", async () => {
+      const testData = await createTestData();
+
+      const service = new InsightsBookingService({
+        prisma,
+        options: {
+          scope: "user",
+          userId: testData.user.id,
+          orgId: testData.org.id,
+        },
+        filters: {
+          eventTypeId: testData.eventType.id,
+        },
+      });
+
+      // First call should build conditions
+      const conditions1 = await service.getFilterConditions();
+      expect(conditions1).toEqual(
+        Prisma.sql`("eventTypeId" = ${testData.eventType.id}) OR ("eventParentId" = ${testData.eventType.id})`
+      );
+
+      // Second call should return the cached result (same reference)
+      const conditions2 = await service.getFilterConditions();
+      expect(conditions2).toBe(conditions1);
+
+      await testData.cleanup();
+    });
+  });
+
   describe("getBaseConditions", () => {

_{🤖 Grapple PR auto-fix • minor • Review this diff before applying}

[Frawatson]

@grapple-pr review

[grapple-pr]

🤖 Grapple PR Review

Summary

This PR converts InsightsBookingService from Prisma ORM query builders (WhereInput objects) to raw Prisma.sql tagged template literals for building authorization and filter conditions, with the goal of enabling more performant raw SQL queries for the insights feature.

Changes: 2 files (+114 / -231 lines)

• 🟡 2 files modified: packages/lib/server/service/__tests__/insightsBooking.integration-test.ts, packages/lib/server/service/insightsBooking.ts

Resolves: #5 (Advanced date override handling and timezone compatibility improvements)

CI: ⏳ 0 passed, 0 failed

Impact Diagram

graph TD
  pr_0[/"🔀 PR Changes: 2 files"/]
  f1["🟡 insightsBooking.integration-test.ts +41/-149"]
  pr_0 --> f1
  f2["🟡 insightsBooking.ts +73/-82"]
  pr_0 --> f2
  style f1 fill:#7c3aed,color:#fff,stroke:#7c3aed
  style f2 fill:#7c3aed,color:#fff,stroke:#7c3aed

Loading

3 critical issues found — please address before merging.

Severity	Count
🔴 Critical	3
🟠 Major	8
🟡 Minor	4

Review details

• Reviewed at: 2026-04-13T00:25:47.589Z
• Agents used: security, logic, style, architecture, performance, verification
• Context level: full
• Execution time: 107.7s
• Low-confidence findings suppressed: 3
• Blast radius: 0 (contained)

Powered by Grapple PR — AI that understands your code.

[grapple-pr]

🔴 CRITICAL — Control Flow / Null-Truthy Logic (confidence: 100%)

getBaseConditions() uses truthiness checks (if (authConditions)) to distinguish between a real SQL condition and the deny-all NOTHING_CONDITION. However, NOTHING_CONDITION (Prisma.sql\1=0`) is a non-null Prisma.Sql object, which is always truthy. This means when authorization returns NOTHING_CONDITION (user is unauthorized), the code enters the if (authConditions && filterConditions)branch (if filters exist) and produces(1=0) AND (filterConditions)— which technically still returns no rows (correct by accident), BUT it does NOT short-circuit: it still evaluates filter conditions unnecessarily and returns a combined SQL fragment rather than the clean NOTHING_CONDITION. More importantly, theelse branch (return NOTHING_CONDITION) is only reached when BOTH authConditions and filterConditions are falsy. Since getAuthorizationConditions()always returns a Prisma.Sql (never null), theelsebranch is dead code —authConditionswill always be truthy. IffilterConditionsis null butauthConditionsis NOTHING_CONDITION, the code returns NOTHING_CONDITION via theelse if (authConditions)` branch, which happens to be correct. The real risk is if the return type of getAuthorizationConditions is ever changed to return null for unauthorized cases — the else branch's NOTHING_CONDITION fallback would then become the safety net, but the current code never reaches it for the reason described.

Evidence:

• getAuthorizationConditions() return type is Promise — never null
• NOTHING_CONDITION is Prisma.sql1=0 which is a truthy object
• Line 71: if (authConditions && filterConditions) — authConditions is always truthy
• Line 78: else { return NOTHING_CONDITION; } is dead code since authConditions is always truthy
• When auth returns NOTHING_CONDITION and filters are non-null, result is (1=0) AND (filters) — still returns empty results (correct by accident), but semantically the intent is to short-circuit on denial
• NOTHING_CONDITION is defined as Prisma.sql\1=0`` which is a non-null object
• getAuthorizationConditions() returns Promise — it always returns a Prisma.Sql, never null
• The if (authConditions) check on line 70 will always be true since Prisma.Sql objects are truthy
• The else if (filterConditions) branch on line 74 and else branch on line 76 are dead code

_{Agent: logic}

[grapple-pr]

🔴 CRITICAL — N+1 queries / Unbounded in-memory accumulation (confidence: 100%)

buildOrgAuthorizationCondition() passes large JS arrays directly into ANY(${teamIds}) and ANY(${uniqueUserIds}) via Prisma.sql interpolation. PostgreSQL has a hard limit of 65535 bind parameters per query. For large orgs with many teams or members, this will produce a runtime error. Additionally, the entire user and team membership lists are loaded into application memory before being embedded as parameters.

Evidence:

• teamIds is built from teamsFromOrg (all teams under the org) with no LIMIT applied in the TeamRepository query
• userIdsFromOrg is built from all memberships across all sub-teams, also unbounded
• Prisma.sql("teamId" = ANY(${teamIds})) embeds the full array as bind parameters — PostgreSQL rejects queries exceeding 65535 parameters
• The previous ORM approach used in: teamIds which had the same underlying limitation, but this PR makes it more explicit and easier to hit

_{Agent: performance}

[grapple-pr]

🔴 CRITICAL — Security / SQL injection risk via Prisma.sql array binding (confidence: 83%)

Passing a plain JS array (number[]) directly into a Prisma.sql tagged template as ANY(${teamIds}) is not guaranteed to produce a correctly parameterized PostgreSQL array binding across all Prisma versions. If Prisma serialises the array as a raw string interpolation rather than a typed array parameter, this becomes a SQL injection vector. The correct approach is Prisma.sqlANY(${Prisma.join(teamIds)})`` or using a cast: ANY(${teamIds}::int[]) via `Prisma.sql`.

Evidence:

• Line 163: Prisma.sql("teamId" = ANY(${teamIds})) AND ("isTeamBooking" = true)`` — teamIds is number[] passed as a bare interpolation
• Line 165: Prisma.sql("userId" = ANY(${uniqueUserIds})) AND ("isTeamBooking" = false)`` — same pattern
• Line 195 (buildTeamAuthorizationCondition): Prisma.sql("userId" = ANY(${userIdsFromTeam}))`` — same pattern
• Prisma's tagged template interpolation for raw SQL does not automatically convert JS arrays to PostgreSQL array literals; behavior depends on Prisma version and database driver

_{Agent: performance}

[grapple-pr]

🟠 MAJOR — Behavioral Regression / API Removal (confidence: 100%)

The findMany() method has been removed and replaced with getBaseConditions(). Any callers outside the test file that previously used service.findMany() will break at compile time (TypeScript) or runtime. Since this is described as a refactor, it should preserve the public API or the PR should update all callers. The diff only shows the test file being updated, but there may be other consumers of InsightsBookingService.findMany() in the codebase.

Evidence:

• findMany() method completely removed from insightsBooking.ts
• Test file updated to use getBaseConditions() + prisma.$queryRaw instead
• No other files are shown in the diff, suggesting potential unupdated callers

_{Agent: logic}

[grapple-pr]

🟠 MAJOR — Behavioral Regression / Team Scope Empty Members (confidence: 100%)

In buildTeamAuthorizationCondition(), when userIdsFromTeam is empty (team exists but has no members), the previous code included both OR branches unconditionally (teamId match OR userId IN [] match). The new code only pushes the userId condition when userIdsFromTeam.length > 0, which is actually a correct improvement. However, the old code would still include userId: { in: [] } which Prisma handles as 'match nothing'. The new behavior is correct — only the teamId condition is returned. No bug here, but this is a semantic behavior change in a refactor.

Evidence:

• Old code: always included userId IN [] branch even when empty
• New code line 201: if (userIdsFromTeam.length > 0) — skips empty array
• This changes behavior for teams with no members — old code had an extra OR branch that matched nothing

_{Agent: logic}

[grapple-pr]

🟠 MAJOR — Algorithmic complexity / unnecessary reduce over Prisma.sql (confidence: 94%)

Using Array.reduce() to chain Prisma.sql fragments with OR/AND creates a new Prisma.Sql object at each step, making this O(n) in allocations. For the org-scope case this is always at most 2 conditions so the overhead is minor, but the pattern is fragile — the reduce returns the raw first element (no wrapping) for index 0, meaning single-condition output is not wrapped in parentheses, which can cause incorrect SQL precedence when the result is embedded in a larger query.

Evidence:

• if (index === 0) return condition; — the first condition is returned unwrapped
• When only teamIds condition exists (userIdsFromOrg is empty), the returned SQL is ("teamId" = ANY(...)) AND ("isTeamBooking" = true) with no outer parens
• In getBaseConditions(), this is wrapped in (${authConditions}) AND (${filterConditions}) which adds parens at that level, but callers using the raw authConditions directly (e.g. tests) see unparenthesised output
• The same pattern exists in buildFilterConditions() at the equivalent reduce

_{Agent: performance}

[grapple-pr]

🟠 MAJOR — Runtime Safety - Removed Validation (confidence: 84%)

The constructor now accepts InsightsBookingServicePublicOptions (a plain TypeScript type) instead of the validated InsightsBookingServiceOptions (zod-validated discriminated union). The zod schema insightsBookingServiceOptionsSchema enforces that teamId is required when scope is 'team' and orgId is required, but the new type makes teamId optional regardless of scope. If a caller passes { scope: 'team', userId: 1, orgId: 1 } without teamId, the code will proceed to buildTeamAuthorizationCondition() where options.teamId would be undefined, potentially causing a SQL error or incorrect query.

Evidence:

• InsightsBookingServicePublicOptions defines teamId?: number as always optional
• The zod schema requires teamId for 'team' scope: z.object({ scope: z.literal('team'), teamId: z.number(), ... })
• Line 65: this.options = insightsBookingServiceOptionsSchema.parse(options) still validates at runtime, but the TypeScript type no longer catches this at compile time
• buildTeamAuthorizationCondition expects options.teamId to exist (Extract includes required teamId)

_{Agent: architecture}

[grapple-pr]

🟡 MINOR — Test Coverage Regression (confidence: 100%)

The entire 'Caching' test section was removed (tests for caching of authorization and filter conditions). While the caching logic still exists in the implementation (cachedAuthConditions and cachedFilterConditions), removing the tests means there's no verification that the caching behavior works correctly after the refactor. This is a test coverage regression.

Evidence:

• Lines removed include 'should cache authorization conditions' and 'should cache filter conditions' tests
• Implementation still has caching at lines 83-86 and 90-93
• No replacement caching tests were added
• The diff shows removal of the 'Caching' describe block with two tests
• The service still has cachedAuthConditions and cachedFilterConditions private fields
• Caching behavior changes (e.g., accidentally clearing cache) would go undetected

_{Agent: logic}

[grapple-pr]

🟡 MINOR — Prisma.sql Test Equality (confidence: 88%)

Tests use toEqual() to compare Prisma.Sql objects produced by tagged template literals. Prisma.Sql equality via toEqual() depends on the internal structure (strings array + values array) being identical. The ANY(${[testData.user.id]}) syntax passes a JavaScript array as a parameter binding — this works with toEqual() only if both sides produce the same internal representation. If Prisma's internal Sql class uses non-enumerable properties or symbols, these tests could produce false positives or fragile failures across Prisma versions.

Evidence:

• Multiple tests compare Prisma.sql tagged template results with .toEqual(Prisma.sql\...`)`
• Prisma.Sql is a class with strings, values, and potentially other internal properties
• Array bindings like ${[testData.user.id]} need to produce identical internal representation on both sides

_{Agent: architecture}

[grapple-pr]

🟡 MINOR — Runtime Validation Removed (confidence: 73%)

The constructor now accepts InsightsBookingServicePublicOptions (a plain TypeScript type) but still calls insightsBookingServiceOptionsSchema.safeParse(options) at line 63. Since InsightsBookingServicePublicOptions always has scope, userId, and orgId (with optional teamId), it doesn't distinguish between team-scope-requires-teamId and user/org-scope-doesn't. A caller could pass { scope: 'team', userId: 1, orgId: 1 } without teamId, the zod parse would fail (since team scope requires teamId), and this.options would be set to null — silently returning NOTHING_CONDITION from auth. This is arguably a degradation from the old type-safe approach where the discriminated union type enforced teamId presence at compile time.

Evidence:

• InsightsBookingServicePublicOptions has teamId as optional regardless of scope
• The zod schema insightsBookingServiceOptionsSchema requires teamId when scope is 'team'
• safeParse failure sets this.options to null (line 64)
• buildAuthorizationConditions returns NOTHING_CONDITION when this.options is null (line 107)

_{Agent: logic}

[grapple-pr]

🔧 [Grapple PR] 3 auto-fix suggestions available

Apply individually or use GitHub's "Add suggestion to batch" → "Commit suggestions" to apply all at once.

_{Commits from Grapple PR are prefixed with [Grapple PR] for easy identification in merge conflicts.}

[grapple-pr]

✅ [Grapple PR] Auto-fix — logic agent (Small fix (4 lines, 1 file))

In buildTeamAuthorizationCondition(), when userIdsFromTeam is empty (team exists but has no members), the previous code included both OR branches unconditionally (teamId match OR userId IN [] match). The new code only pushes the userId condition when userIdsFromTeam.length > 0, which is actually a correct improvement. However, the old code would still include userId: { in: [] } which Prisma handles as 'match nothing'. The new behavior is correct — only the teamId condition is returned. No bug here, but this is a semantic behavior change in a refactor.

Suggested change

	}
	// Always include the teamId condition to capture team bookings (isTeamBooking = true).
	// The userId condition is only added when there are members, intentionally omitting
	// the empty-array branch. Previously, Prisma ORM with `userId: { in: [] }` would
	// generate a no-op OR clause (matching nothing); skipping it entirely is equivalent.

_{🤖 Grapple PR auto-fix • major • confidence: 100%}

[grapple-pr]

✅ [Grapple PR] Auto-fix — performance agent (Small fix (8 lines, 1 file))

NOTHING_CONDITION is a non-null Prisma.Sql object. In getBaseConditions(), the check if (authConditions && filterConditions) will be true even when authConditions is NOTHING_CONDITION (the deny-all sentinel). This causes the method to return (1=0) AND (filterConditions) instead of short-circuiting to NOTHING_CONDITION, which is functionally correct SQL but wastes query execution time evaluating a filter that can never return rows. More critically, if the intent is to signal 'no access' as a distinct state, callers cannot distinguish NOTHING_CONDITION from a valid combined condition.

Suggested change

	}
	// Short-circuit: if auth is the deny-all sentinel, no filter can ever return rows.
	// We check reference equality because NOTHING_CONDITION is a truthy Prisma.Sql object
	// and a truthiness check cannot distinguish it from a valid condition.
	if (authConditions === NOTHING_CONDITION) {
	return NOTHING_CONDITION;
	}

_{🤖 Grapple PR auto-fix • major • confidence: 100%}

[grapple-pr]

✅ [Grapple PR] Auto-fix — logic agent (Small fix (9 lines, 1 file))

The constructor now accepts InsightsBookingServicePublicOptions (a plain TypeScript type) but still calls insightsBookingServiceOptionsSchema.safeParse(options) at line 63. Since InsightsBookingServicePublicOptions always has scope, userId, and orgId (with optional teamId), it doesn't distinguish between team-scope-requires-teamId and user/org-scope-doesn't. A caller could pass { scope: 'team', userId: 1, orgId: 1 } without teamId, the zod parse would fail (since team scope requires teamId), and this.options would be set to null — silently returning NOTHING_CONDITION from auth. This is arguably a degradation from the old type-safe approach where the discriminated union type enforced teamId presence at compile time.

Suggested change

	this.filters = filters;
	// Use parse (not safeParse) so that invalid combinations — e.g. scope='team'
	// without teamId — throw immediately rather than silently setting options to
	// null and returning NOTHING_CONDITION for every query without any indication
	// of why. InsightsBookingServicePublicOptions intentionally has teamId as
	// optional (it's the public API type), but the Zod discriminated union
	// enforces the invariant that scope='team' requires teamId at runtime.
	this.options = insightsBookingServiceOptionsSchema.parse(options);

_{🤖 Grapple PR auto-fix • minor • confidence: 73%}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — logic agent (Larger fix (16 lines, 1 file) — review recommended)

getBaseConditions() uses truthiness checks (if (authConditions)) to distinguish between a real SQL condition and the deny-all NOTHING_CONDITION. However, NOTHING_CONDITION (Prisma.sql\1=0`) is a non-null Prisma.Sql object, which is always truthy. This means when authorization returns NOTHING_CONDITION (user is unauthorized), the code enters the if (authConditions && filterConditions)branch (if filters exist) and produces(1=0) AND (filterConditions)— which technically still returns no rows (correct by accident), BUT it does NOT short-circuit: it still evaluates filter conditions unnecessarily and returns a combined SQL fragment rather than the clean NOTHING_CONDITION. More importantly, theelse branch (return NOTHING_CONDITION) is only reached when BOTH authConditions and filterConditions are falsy. Since getAuthorizationConditions()always returns a Prisma.Sql (never null), theelsebranch is dead code —authConditionswill always be truthy. IffilterConditionsis null butauthConditionsis NOTHING_CONDITION, the code returns NOTHING_CONDITION via theelse if (authConditions)` branch, which happens to be correct. The real risk is if the return type of getAuthorizationConditions is ever changed to return null for unauthorized cases — the else branch's NOTHING_CONDITION fallback would then become the safety net, but the current code never reaches it for the reason described.

--- a/packages/lib/server/service/insightsBooking.ts
+++ b/packages/lib/server/service/insightsBooking.ts
@@ -68,14 +68,13 @@ export class InsightsBookingService {
   async getBaseConditions(): Promise<Prisma.Sql> {
     const authConditions = await this.getAuthorizationConditions();
+
+    // Short-circuit: if authorization denied access, return NOTHING_CONDITION immediately.
+    // We must compare by reference since NOTHING_CONDITION is a truthy Prisma.Sql object,
+    // so truthiness checks cannot distinguish it from a real authorization condition.
+    if (authConditions === NOTHING_CONDITION) {
+      return NOTHING_CONDITION;
+    }
+
     const filterConditions = await this.getFilterConditions();
 
-    if (authConditions && filterConditions) {
+    if (filterConditions) {
       return Prisma.sql`(${authConditions}) AND (${filterConditions})`;
-    } else if (authConditions) {
-      return authConditions;
-    } else if (filterConditions) {
-      return filterConditions;
     } else {
-      return NOTHING_CONDITION;
+      return authConditions;
     }
   }

_{🤖 Grapple PR auto-fix • critical • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — performance agent (Larger fix (92 lines, 1 file) — review recommended)

buildOrgAuthorizationCondition() passes large JS arrays directly into ANY(${teamIds}) and ANY(${uniqueUserIds}) via Prisma.sql interpolation. PostgreSQL has a hard limit of 65535 bind parameters per query. For large orgs with many teams or members, this will produce a runtime error. Additionally, the entire user and team membership lists are loaded into application memory before being embedded as parameters.

--- a/packages/lib/server/service/insightsBooking.ts
+++ b/packages/lib/server/service/insightsBooking.ts
@@ -141,36 +141,37 @@ export class InsightsBookingService {
   private async buildOrgAuthorizationCondition(
     options: Extract<InsightsBookingServiceOptions, { scope: "org" }>
   ): Promise<Prisma.Sql> {
-    // Get all teams from the organization
-    const teamRepo = new TeamRepository(this.prisma);
-    const teamsFromOrg = await teamRepo.findAllByParentId({
-      parentId: options.orgId,
-      select: { id: true },
-    });
-    const teamIds = [options.orgId, ...teamsFromOrg.map((t) => t.id)];
-
-    // Get all users from the organization
-    const userIdsFromOrg =
-      teamsFromOrg.length > 0
-        ? (await MembershipRepository.findAllByTeamIds({ teamIds, select: { userId: true } })).map(
-            (m) => m.userId
-          )
-        : [];
-
-    const conditions: Prisma.Sql[] = [Prisma.sql`("teamId" = ANY(${teamIds})) AND ("isTeamBooking" = true)`];
-
-    if (userIdsFromOrg.length > 0) {
-      const uniqueUserIds = Array.from(new Set(userIdsFromOrg));
-      conditions.push(Prisma.sql`("userId" = ANY(${uniqueUserIds})) AND ("isTeamBooking" = false)`);
-    }
-
-    return conditions.reduce((acc, condition, index) => {
-      if (index === 0) return condition;
-      return Prisma.sql`(${acc}) OR (${condition})`;
-    });
+    // Use subqueries instead of loading all IDs into memory.
+    // ANY(${largeArray}) would hit PostgreSQL's 65535 bind-parameter limit for big orgs
+    // and forces the entire team/membership lists into application memory before the query runs.
+    // Subqueries let PostgreSQL plan the join and avoid both problems.
+    const orgId = options.orgId;
+
+    // Team bookings: teamId is the org itself OR any direct child team of the org
+    const teamBookingCondition = Prisma.sql`(
+      "isTeamBooking" = true
+      AND "teamId" IN (
+        SELECT id FROM "Team"
+        WHERE id = ${orgId} OR "parentId" = ${orgId}
+      )
+    )`;
+
+    // Individual bookings: user is a member of the org or any of its child teams
+    const userBookingCondition = Prisma.sql`(
+      "isTeamBooking" = false
+      AND "userId" IN (
+        SELECT DISTINCT m."userId"
+        FROM "Membership" m
+        INNER JOIN "Team" t ON t.id = m."teamId"
+        WHERE (t.id = ${orgId} OR t."parentId" = ${orgId})
+          AND m.accepted = true
+      )
+    )`;
+
+    return Prisma.sql`(${teamBookingCondition}) OR (${userBookingCondition})`;
   }
 
   private async buildTeamAuthorizationCondition(
     options: Extract<InsightsBookingServiceOptions, { scope: "team" }>
   ): Promise<Prisma.Sql> {
     const teamRepo = new TeamRepository(this.prisma);
     const childTeamOfOrg = await teamRepo.findByIdAndParentId({
       id: options.teamId,
       parentId: options.orgId,
       select: { id: true },
     });
     if (!childTeamOfOrg) {
       return NOTHING_CONDITION;
     }
 
-    const usersFromTeam = await MembershipRepository.findAllByTeamIds({
-      teamIds: [options.teamId],
-      select: { userId: true },
-    });
-    const userIdsFromTeam = usersFromTeam.map((u) => u.userId);
-
-    const conditions: Prisma.Sql[] = [
-      Prisma.sql`("teamId" = ${options.teamId}) AND ("isTeamBooking" = true)`,
-    ];
-
-    if (userIdsFromTeam.length > 0) {
-      conditions.push(Prisma.sql`("userId" = ANY(${userIdsFromTeam})) AND ("isTeamBooking" = false)`);
-    }
-
-    return conditions.reduce((acc, condition, index) => {
-      if (index === 0) return condition;
-      return Prisma.sql`(${acc}) OR (${condition})`;
-    });
+    // Same subquery pattern as buildOrgAuthorizationCondition — avoid loading all
+    // member user IDs into application memory and embedding them as bind parameters.
+    const teamId = options.teamId;
+
+    const teamBookingCondition = Prisma.sql`(
+      "isTeamBooking" = true
+      AND "teamId" = ${teamId}
+    )`;
+
+    const userBookingCondition = Prisma.sql`(
+      "isTeamBooking" = false
+      AND "userId" IN (
+        SELECT "userId" FROM "Membership"
+        WHERE "teamId" = ${teamId}
+          AND accepted = true
+      )
+    )`;
+
+    return Prisma.sql`(${teamBookingCondition}) OR (${userBookingCondition})`;
   }

_{🤖 Grapple PR auto-fix • critical • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — performance agent (Larger fix (12 lines, 1 file) — review recommended)

Passing a plain JS array (number[]) directly into a Prisma.sql tagged template as ANY(${teamIds}) is not guaranteed to produce a correctly parameterized PostgreSQL array binding across all Prisma versions. If Prisma serialises the array as a raw string interpolation rather than a typed array parameter, this becomes a SQL injection vector. The correct approach is Prisma.sqlANY(${Prisma.join(teamIds)})`` or using a cast: ANY(${teamIds}::int[]) via `Prisma.sql`.

--- a/packages/lib/server/service/insightsBooking.ts
+++ b/packages/lib/server/service/insightsBooking.ts
@@ -161,12 +161,16 @@ export class InsightsBookingService {
         : [];
 
-    const conditions: Prisma.Sql[] = [Prisma.sql`("teamId" = ANY(${teamIds})) AND ("isTeamBooking" = true)`];
+    const conditions: Prisma.Sql[] = [
+      Prisma.sql`("teamId" = ANY(ARRAY[${Prisma.join(teamIds)}])) AND ("isTeamBooking" = true)`,
+    ];
 
     if (userIdsFromOrg.length > 0) {
       const uniqueUserIds = Array.from(new Set(userIdsFromOrg));
-      conditions.push(Prisma.sql`("userId" = ANY(${uniqueUserIds})) AND ("isTeamBooking" = false)`);
+      conditions.push(
+        Prisma.sql`("userId" = ANY(ARRAY[${Prisma.join(uniqueUserIds)}])) AND ("isTeamBooking" = false)`
+      );
     }
 
     return conditions.reduce((acc, condition, index) => {
@@ -193,7 +197,9 @@ export class InsightsBookingService {
 
     if (userIdsFromTeam.length > 0) {
-      conditions.push(Prisma.sql`("userId" = ANY(${userIdsFromTeam})) AND ("isTeamBooking" = false)`);
+      conditions.push(
+        Prisma.sql`("userId" = ANY(ARRAY[${Prisma.join(userIdsFromTeam)}])) AND ("isTeamBooking" = false)`
+      );
     }
 
     return conditions.reduce((acc, condition, index) => {

_{🤖 Grapple PR auto-fix • critical • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — logic agent (Larger fix (43 lines, 1 file) — review recommended)

The findMany() method has been removed and replaced with getBaseConditions(). Any callers outside the test file that previously used service.findMany() will break at compile time (TypeScript) or runtime. Since this is described as a refactor, it should preserve the public API or the PR should update all callers. The diff only shows the test file being updated, but there may be other consumers of InsightsBookingService.findMany() in the codebase.

--- a/packages/lib/server/service/insightsBooking.ts
+++ b/packages/lib/server/service/insightsBooking.ts
@@ -65,6 +65,37 @@ export class InsightsBookingService {
     this.filters = filters;
   }
 
+  /**
+   * @deprecated Use getBaseConditions() with prisma.$queryRaw for raw SQL queries instead.
+   * This method is preserved for backward compatibility with existing callers while the
+   * migration to raw SQL is in progress. Remove once all callers are updated to use
+   * getBaseConditions() directly.
+   *
+   * NOTE: This bridge method applies auth + filter conditions via the new raw-SQL path
+   * but falls back to the ORM findMany for the actual result set. Because getBaseConditions()
+   * now returns Prisma.Sql (not WhereInput), we reconstruct a safe ORM-compatible condition
+   * by re-using the cached auth/filter conditions from their individual accessors.
+   */
+  async findMany(findManyArgs: Prisma.BookingTimeStatusDenormalizedFindManyArgs) {
+    // Re-use the cached condition builders (not getBaseConditions which returns Prisma.Sql)
+    // to reconstruct ORM-compatible WhereInput objects, preserving the original return type.
+    const authConditions = await this.getAuthorizationConditions();
+    const filterConditions = await this.getFilterConditions();
+
+    // Build the raw SQL WHERE clause for use in a raw query fallback
+    const baseConditions = await this.getBaseConditions();
+
+    // Use raw query to get IDs matching conditions, then fetch full records via findMany
+    // This preserves the original return type (full Prisma model objects with relations)
+    // while using the new raw SQL condition building internally.
+    const matchingIds = await this.prisma.$queryRaw<Array<{ id: number }>>`
+      SELECT id FROM "BookingTimeStatusDenormalized"
+      WHERE ${baseConditions}
+    `;
+
+    const ids = matchingIds.map((r) => r.id);
+
+    if (ids.length === 0) {
+      return [];
+    }
+
+    return this.prisma.bookingTimeStatusDenormalized.findMany({
+      ...findManyArgs,
+      where: {
+        ...findManyArgs.where,
+        id: { in: ids },
+      },
+    });
+  }
+
   async getBaseConditions(): Promise<Prisma.Sql> {
     const authConditions = await this.getAuthorizationConditions();
     const filterConditions = await this.getFilterConditions();

_{🤖 Grapple PR auto-fix • major • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — logic agent (Larger fix (14 lines, 1 file) — review recommended)

Passing a JavaScript array directly into Prisma.sql tagged template for ANY() (e.g., Prisma.sql\"teamId" = ANY(${teamIds})`) relies on Prisma's ability to correctly parameterize arrays as PostgreSQL array literals. Prisma.sql handles arrays by converting them to a comma-separated list of parameters, which produces ANY($1, $2, $3)— but PostgreSQL'sANY()expects an array type argument likeANY(ARRAY[$1, $2, $3])orANY($1::int[]). This may cause a runtime SQL error: "operator does not exist: integer = record". The correct syntax for Prisma raw SQL with arrays and ANY is typically = ANY(${Prisma.sqlARRAY[${Prisma.join(teamIds)}]})or usingIN` instead.

--- a/packages/lib/server/service/insightsBooking.ts
+++ b/packages/lib/server/service/insightsBooking.ts
@@ -161,7 +161,12 @@ export class InsightsBookingService {
 
-    const conditions: Prisma.Sql[] = [Prisma.sql`("teamId" = ANY(${teamIds})) AND ("isTeamBooking" = true)`];
+    const conditions: Prisma.Sql[] = [
+      teamIds.length > 0
+        ? Prisma.sql`("teamId" = ANY(ARRAY[${Prisma.join(teamIds)}])) AND ("isTeamBooking" = true)`
+        : Prisma.sql`1=0`,
+    ];
 
     if (userIdsFromOrg.length > 0) {
       const uniqueUserIds = Array.from(new Set(userIdsFromOrg));
-      conditions.push(Prisma.sql`("userId" = ANY(${uniqueUserIds})) AND ("isTeamBooking" = false)`);
+      conditions.push(
+        Prisma.sql`("userId" = ANY(ARRAY[${Prisma.join(uniqueUserIds)}])) AND ("isTeamBooking" = false)`
+      );
     }
 
     return conditions.reduce((acc, condition, index) => {
@@ -193,7 +198,9 @@ export class InsightsBookingService {
 
     if (userIdsFromTeam.length > 0) {
-      conditions.push(Prisma.sql`("userId" = ANY(${userIdsFromTeam})) AND ("isTeamBooking" = false)`);
+      conditions.push(
+        Prisma.sql`("userId" = ANY(ARRAY[${Prisma.join(userIdsFromTeam)}])) AND ("isTeamBooking" = false)`
+      );
     }
 
     return conditions.reduce((acc, condition, index) => {

_{🤖 Grapple PR auto-fix • major • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — architecture agent (Larger fix (37 lines, 1 file) — review recommended)

The findMany() method has been removed and replaced with getBaseConditions(). Any callers outside the test file that previously used service.findMany() will break at compile time (TypeScript) or runtime. This is a breaking change to the public API of InsightsBookingService. The blast radius analysis shows only 2 files are affected, but this analysis may not capture all transitive consumers if the service is used through re-exports or dependency injection.

--- a/packages/lib/server/service/insightsBooking.ts
+++ b/packages/lib/server/service/insightsBooking.ts
@@ -65,6 +65,36 @@ export class InsightsBookingService {
     this.filters = filters;
   }
 
+  /**
+   * @deprecated Use `getBaseConditions()` to obtain a Prisma.Sql fragment and
+   * compose your own `prisma.$queryRaw` call instead.  This method exists for
+   * backwards-compatibility with callers that previously relied on `findMany()`
+   * and will be removed in a future release.
+   *
+   * NOTE: Because the service now builds raw SQL conditions (Prisma.Sql) rather
+   * than Prisma WhereInput objects, this method issues a $queryRaw statement
+   * against `BookingTimeStatusDenormalized`.  The returned rows are plain
+   * objects; they are NOT typed Prisma model instances.  Callers that need
+   * specific column selections should migrate to `getBaseConditions()` directly.
+   */
+  async findMany(findManyArgs: { select?: Record<string, boolean>; take?: number; skip?: number } = {}) {
+    const whereCondition = await this.getBaseConditions();
+
+    // Build a minimal column list from the select map, defaulting to * when
+    // no explicit selection is provided (mirrors the old behaviour).
+    const columns =
+      findManyArgs.select && Object.keys(findManyArgs.select).length > 0
+        ? Prisma.join(
+            Object.entries(findManyArgs.select)
+              .filter(([, include]) => include)
+              .map(([col]) => Prisma.raw(`"${col}"`)),
+            ", "
+          )
+        : Prisma.raw("*");
+
+    const limitClause =
+      findManyArgs.take != null ? Prisma.sql` LIMIT ${findManyArgs.take}` : Prisma.empty;
+    const offsetClause =
+      findManyArgs.skip != null ? Prisma.sql` OFFSET ${findManyArgs.skip}` : Prisma.empty;
+
+    return this.prisma.$queryRaw(
+      Prisma.sql`SELECT ${columns} FROM "BookingTimeStatusDenormalized" WHERE ${whereCondition}${limitClause}${offsetClause}`
+    );
+  }
+
   async getBaseConditions(): Promise<Prisma.Sql> {
     const authConditions = await this.getAuthorizationConditions();
     const filterConditions = await this.getFilterConditions();

_{🤖 Grapple PR auto-fix • major • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — architecture agent (Larger fix (13 lines, 1 file) — review recommended)

In buildTeamAuthorizationCondition(), when userIdsFromTeam is empty, the condition degrades to only the team booking condition (teamId = X AND isTeamBooking = true), which means all non-team bookings by team members are silently excluded. However, the previous Prisma ORM version always included both the team condition AND the user condition in the OR clause (even with an empty in array). While the new behavior is arguably more correct (Prisma's in: [] matches nothing), this is a behavioral change. More critically, in buildOrgAuthorizationCondition() (line 163), teamIds is passed to ANY(${teamIds}) without an empty-array guard — if teamsFromOrg returns an empty array, ANY(ARRAY[]::int[]) behavior depends on the PostgreSQL driver and could error.

--- a/packages/lib/server/service/insightsBooking.ts
+++ b/packages/lib/server/service/insightsBooking.ts
@@ -158,7 +158,13 @@ export class InsightsBookingService {
         : [];
 
-    const conditions: Prisma.Sql[] = [Prisma.sql`("teamId" = ANY(${teamIds})) AND ("isTeamBooking" = true)`];
+    // Guard: if the org has no teams, the team-booking condition can never match.
+    // ANY(ARRAY[]::int[]) is technically valid SQL but semantically wrong here,
+    // and driver-level casting of an empty array can be unreliable.
+    const conditions: Prisma.Sql[] = [];
+
+    if (teamIds.length > 0) {
+      conditions.push(Prisma.sql`("teamId" = ANY(${teamIds})) AND ("isTeamBooking" = true)`);
+    }
 
     if (userIdsFromOrg.length > 0) {
       const uniqueUserIds = Array.from(new Set(userIdsFromOrg));
       conditions.push(Prisma.sql`("userId" = ANY(${uniqueUserIds})) AND ("isTeamBooking" = false)`);
     }
 
+    if (conditions.length === 0) {
+      return NOTHING_CONDITION;
+    }
+
     return conditions.reduce((acc, condition, index) => {
       if (index === 0) return condition;
       return Prisma.sql`(${acc}) OR (${condition})`;
     });
   }

_{🤖 Grapple PR auto-fix • major • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — performance agent (Larger fix (31 lines, 1 file) — review recommended)

Using Array.reduce() to chain Prisma.sql fragments with OR/AND creates a new Prisma.Sql object at each step, making this O(n) in allocations. For the org-scope case this is always at most 2 conditions so the overhead is minor, but the pattern is fragile — the reduce returns the raw first element (no wrapping) for index 0, meaning single-condition output is not wrapped in parentheses, which can cause incorrect SQL precedence when the result is embedded in a larger query.

--- a/packages/lib/server/service/insightsBooking.ts
+++ b/packages/lib/server/service/insightsBooking.ts
@@ -161,12 +161,14 @@ export class InsightsBookingService {
     const conditions: Prisma.Sql[] = [Prisma.sql`("teamId" = ANY(${teamIds})) AND ("isTeamBooking" = true)`];
 
     if (userIdsFromOrg.length > 0) {
       const uniqueUserIds = Array.from(new Set(userIdsFromOrg));
       conditions.push(Prisma.sql`("userId" = ANY(${uniqueUserIds})) AND ("isTeamBooking" = false)`);
     }
 
-    return conditions.reduce((acc, condition, index) => {
-      if (index === 0) return condition;
-      return Prisma.sql`(${acc}) OR (${condition})`;
-    });
+    // Always wrap in parens so this fragment is safe when embedded in a larger query.
+    // conditions is always 1 or 2 elements in this scope (team booking OR personal booking).
+    if (conditions.length === 1) {
+      return Prisma.sql`(${conditions[0]})`;
+    }
+    return Prisma.sql`(${conditions[0]}) OR (${conditions[1]})`;
   }
 
   private async buildTeamAuthorizationCondition(
@@ -193,14 +195,18 @@ export class InsightsBookingService {
     const conditions: Prisma.Sql[] = [
       Prisma.sql`("teamId" = ${options.teamId}) AND ("isTeamBooking" = true)`,
     ];
 
     if (userIdsFromTeam.length > 0) {
       conditions.push(Prisma.sql`("userId" = ANY(${userIdsFromTeam})) AND ("isTeamBooking" = false)`);
     }
 
-    return conditions.reduce((acc, condition, index) => {
-      if (index === 0) return condition;
-      return Prisma.sql`(${acc}) OR (${condition})`;
-    });
+    // Always wrap in parens so this fragment is safe when embedded in a larger query.
+    // conditions is always 1 or 2 elements in this scope (team booking OR personal booking).
+    if (conditions.length === 1) {
+      return Prisma.sql`(${conditions[0]})`;
+    }
+    return Prisma.sql`(${conditions[0]}) OR (${conditions[1]})`;
   }
 
   private async isOrgOwnerOrAdmin(userId: number, orgId: number): Promise<boolean> {
@@ -113,12 +113,14 @@ export class InsightsBookingService {
     }
 
-    // Join all conditions with AND
-    return conditions.reduce((acc, condition, index) => {
-      if (index === 0) return condition;
-      return Prisma.sql`(${acc}) AND (${condition})`;
-    });
+    // Always wrap in parens so this fragment is safe when embedded in a larger query.
+    // conditions is at most 2 elements (eventTypeId filter AND memberUserId filter).
+    if (conditions.length === 1) {
+      return Prisma.sql`(${conditions[0]})`;
+    }
+    return Prisma.sql`(${conditions[0]}) AND (${conditions[1]})`;
   }
 
   async buildAuthorizationConditions(): Promise<Prisma.Sql> {

_{🤖 Grapple PR auto-fix • major • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — architecture agent (Small fix (9 lines, 1 file))

The constructor now accepts InsightsBookingServicePublicOptions (a plain TypeScript type) instead of the validated InsightsBookingServiceOptions (zod-validated discriminated union). The zod schema insightsBookingServiceOptionsSchema enforces that teamId is required when scope is 'team' and orgId is required, but the new type makes teamId optional regardless of scope. If a caller passes { scope: 'team', userId: 1, orgId: 1 } without teamId, the code will proceed to buildTeamAuthorizationCondition() where options.teamId would be undefined, potentially causing a SQL error or incorrect query.

--- a/packages/lib/server/service/insightsBooking.ts
+++ b/packages/lib/server/service/insightsBooking.ts
@@ -28,12 +28,6 @@ export const insightsBookingServiceOptionsSchema = z.discriminatedUnion("scope",
   }),
 ]);
 
-export type InsightsBookingServicePublicOptions = {
-  scope: "user" | "org" | "team";
-  userId: number;
-  orgId: number;
-  teamId?: number;
-};
-
 export type InsightsBookingServiceOptions = z.infer<typeof insightsBookingServiceOptionsSchema>;
 
 export type InsightsBookingServiceFilterOptions = {
@@ -55,7 +49,7 @@ export class InsightsBookingService {
     prisma,
     options,
     filters,
   }: {
     prisma: typeof readonlyPrisma;
-    options: InsightsBookingServicePublicOptions;
+    options: InsightsBookingServiceOptions;
     filters?: InsightsBookingServiceFilterOptions;
   }) {
     this.prisma = prisma;

_{🤖 Grapple PR auto-fix • major • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — logic agent (Larger fix (58 lines, 1 file) — review recommended)

The entire 'Caching' test section was removed (tests for caching of authorization and filter conditions). While the caching logic still exists in the implementation (cachedAuthConditions and cachedFilterConditions), removing the tests means there's no verification that the caching behavior works correctly after the refactor. This is a test coverage regression.

--- a/packages/lib/server/service/__tests__/insightsBooking.integration-test.ts
+++ b/packages/lib/server/service/__tests__/insightsBooking.integration-test.ts
@@ -435,6 +435,72 @@
       await testData.cleanup();
     });
   });
 
+  describe("Caching", () => {
+    it("should cache authorization conditions", async () => {
+      const testData = await createTestData({
+        teamRole: MembershipRole.OWNER,
+        orgRole: MembershipRole.OWNER,
+      });
+
+      const service = new InsightsBookingService({
+        prisma,
+        options: {
+          scope: "user",
+          userId: testData.user.id,
+          orgId: testData.org.id,
+        },
+      });
+
+      // First call should build conditions
+      const conditions1 = await service.getAuthorizationConditions();
+      expect(conditions1).toEqual(
+        Prisma.sql`("userId" = ${testData.user.id}) AND ("teamId" IS NULL)`
+      );
+
+      // Second call should return the cached conditions (same reference)
+      const conditions2 = await service.getAuthorizationConditions();
+      expect(conditions2).toBe(conditions1);
+
+      await testData.cleanup();
+    });
+
+    it("should cache filter conditions", async () => {
+      const testData = await createTestData();
+
+      const service = new InsightsBookingService({
+        prisma,
+        options: {
+          scope: "user",
+          userId: testData.user.id,
+          orgId: testData.org.id,
+        },
+        filters: {
+          eventTypeId: testData.eventType.id,
+        },
+      });
+
+      // First call should build conditions
+      const conditions1 = await service.getFilterConditions();
+      expect(conditions1).toEqual(
+        Prisma.sql`("eventTypeId" = ${testData.eventType.id}) OR ("eventParentId" = ${testData.eventType.id})`
+      );
+
+      // Second call should return the cached conditions (same reference)
+      const conditions2 = await service.getFilterConditions();
+      expect(conditions2).toBe(conditions1);
+
+      await testData.cleanup();
+    });
+  });
+
   describe("getBaseConditions", () => {

_{🤖 Grapple PR auto-fix • minor • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — architecture agent (Larger fix (69 lines, 1 file) — review recommended)

Tests use toEqual() to compare Prisma.Sql objects produced by tagged template literals. Prisma.Sql equality via toEqual() depends on the internal structure (strings array + values array) being identical. The ANY(${[testData.user.id]}) syntax passes a JavaScript array as a parameter binding — this works with toEqual() only if both sides produce the same internal representation. If Prisma's internal Sql class uses non-enumerable properties or symbols, these tests could produce false positives or fragile failures across Prisma versions.

--- a/packages/lib/server/service/__tests__/insightsBooking.integration-test.ts
+++ b/packages/lib/server/service/__tests__/insightsBooking.integration-test.ts
@@ -1,6 +1,6 @@
 import type { Team, User, Membership } from "@prisma/client";
 import { Prisma } from "@prisma/client";
-import { describe, expect, it } from "vitest";
+import { describe, expect, it, vi } from "vitest";
 
 import prisma from "@calcom/prisma";
 import { BookingStatus, MembershipRole } from "@calcom/prisma/enums";
@@ -10,6 +10,18 @@ import { InsightsBookingService } from "../../service/insightsBooking";
 
 const NOTHING_CONDITION = Prisma.sql`1=0`;
 
+/**
+ * Extracts the stable public properties of a Prisma.Sql object for reliable
+ * cross-version comparison. Comparing Prisma.Sql instances directly with
+ * toEqual() is fragile because the class may contain non-enumerable properties,
+ * symbols, or internal state that differs across Prisma versions. Instead, we
+ * compare only the documented public API: the compiled `sql` string and the
+ * bound `values` array, which are guaranteed to be stable.
+ *
+ * Edge case: nested Prisma.sql`` fragments are flattened into the top-level
+ * `sql` string by Prisma itself, so this captures the fully-resolved query.
+ */
+const sqlSnapshot = (s: Prisma.Sql) => ({ sql: s.sql, values: s.values });
+
 // Helper function to create unique test data
 async function createTestData({
   teamRole = MembershipRole.MEMBER,
@@ -207,7 +219,7 @@ describe("InsightsBookingService Integration Tests", () => {
       });
 
       const conditions = await service.getAuthorizationConditions();
-      expect(conditions).toEqual(NOTHING_CONDITION);
+      expect(sqlSnapshot(conditions)).toEqual(sqlSnapshot(NOTHING_CONDITION));
     });
 
     it("should return NOTHING for non-owner/admin user", async () => {
@@ -242,7 +254,7 @@ describe("InsightsBookingService Integration Tests", () => {
       });
 
       const conditions = await service.getAuthorizationConditions();
-      expect(conditions).toEqual(NOTHING_CONDITION);
+      expect(sqlSnapshot(conditions)).toEqual(sqlSnapshot(NOTHING_CONDITION));
 
       // Clean up
       await prisma.membership.delete({
@@ -270,7 +282,9 @@ describe("InsightsBookingService Integration Tests", () => {
       });
 
       const conditions = await service.getAuthorizationConditions();
-      expect(conditions).toEqual(Prisma.sql`("userId" = ${testData.user.id}) AND ("teamId" IS NULL)`);
+      expect(sqlSnapshot(conditions)).toEqual(
+        sqlSnapshot(Prisma.sql`("userId" = ${testData.user.id}) AND ("teamId" IS NULL)`)
+      );
 
       await testData.cleanup();
     });
@@ -292,11 +306,13 @@ describe("InsightsBookingService Integration Tests", () => {
       });
 
       const conditions = await service.getAuthorizationConditions();
-      expect(conditions).toEqual(
-        Prisma.sql`(("teamId" = ${testData.team.id}) AND ("isTeamBooking" = true)) OR (("userId" = ANY(${[
+      expect(sqlSnapshot(conditions)).toEqual(
+        sqlSnapshot(
+          Prisma.sql`(("teamId" = ${testData.team.id}) AND ("isTeamBooking" = true)) OR (("userId" = ANY(${[
           testData.user.id,
-        ]})) AND ("isTeamBooking" = false))`
+          ]})) AND ("isTeamBooking" = false))`
+        )
       );
 
       // Clean up
@@ -329,18 +345,20 @@ describe("InsightsBookingService Integration Tests", () => {
 
       const conditions = await service.getAuthorizationConditions();
 
-      expect(conditions).toEqual(
-        Prisma.sql`(("teamId" = ANY(${[
-          testData.org.id,
-          testData.team.id,
-          team2.id,
-          team3.id,
-        ]})) AND ("isTeamBooking" = true)) OR (("userId" = ANY(${[
-          testData.user.id,
-          user2.id,
-          user3.id,
-        ]})) AND ("isTeamBooking" = false))`
+      expect(sqlSnapshot(conditions)).toEqual(
+        sqlSnapshot(
+          Prisma.sql`(("teamId" = ANY(${[
+            testData.org.id,
+            testData.team.id,
+            team2.id,
+            team3.id,
+          ]})) AND ("isTeamBooking" = true)) OR (("userId" = ANY(${[
+            testData.user.id,
+            user2.id,
+            user3.id,
+          ]})) AND ("isTeamBooking" = false))`
+        )
       );
 
       await testData.cleanup();
@@ -381,7 +399,9 @@ describe("InsightsBookingService Integration Tests", () => {
       });
 
       const conditions = await service.getFilterConditions();
-      expect(conditions).toEqual(
-        Prisma.sql`("eventTypeId" = ${testData.eventType.id}) OR ("eventParentId" = ${testData.eventType.id})`
+      expect(sqlSnapshot(conditions)).toEqual(
+        sqlSnapshot(
+          Prisma.sql`("eventTypeId" = ${testData.eventType.id}) OR ("eventParentId" = ${testData.eventType.id})`
+        )
       );
 
       await testData.cleanup();
@@ -404,7 +424,7 @@ describe("InsightsBookingService Integration Tests", () => {
       });
 
       const conditions = await service.getFilterConditions();
-      expect(conditions).toEqual(Prisma.sql`"userId" = ${testData.user.id}`);
+      expect(sqlSnapshot(conditions)).toEqual(sqlSnapshot(Prisma.sql`"userId" = ${testData.user.id}`));
 
       await testData.cleanup();
     });
@@ -426,9 +446,11 @@ describe("InsightsBookingService Integration Tests", () => {
       });
 
       const conditions = await service.getFilterConditions();
-      expect(conditions).toEqual(
-        Prisma.sql`(("eventTypeId" = ${testData.eventType.id}) OR ("eventParentId" = ${testData.eventType.id})) AND ("userId" = ${testData.user.id})`
+      expect(sqlSnapshot(conditions)).toEqual(
+        sqlSnapshot(
+          Prisma.sql`(("eventTypeId" = ${testData.eventType.id}) OR ("eventParentId" = ${testData.eventType.id})) AND ("userId" = ${testData.user.id})`
+        )
       );
 
       await testData.cleanup();

_{🤖 Grapple PR auto-fix • minor • Review this diff before applying}

[Frawatson]

@grapple-pr review

[grapple-pr]

🤖 Grapple PR Review

Summary

This PR converts InsightsBookingService from using Prisma ORM query objects (WhereInput) to raw SQL fragments using Prisma.sql tagged templates, replacing structured Prisma filter objects with raw SQL strings for authorization and filter conditions.

Changes: 2 files (+114 / -231 lines)

• 🟡 2 files modified: packages/lib/server/service/__tests__/insightsBooking.integration-test.ts, packages/lib/server/service/insightsBooking.ts

Resolves: #5 (Advanced date override handling and timezone compatibility improvements)

CI: ⏳ 0 passed, 0 failed

Impact Diagram

graph TD
  pr_0[/"🔀 PR Changes: 2 files"/]
  f1["🟡 insightsBooking.integration-test.ts +41/-149"]
  pr_0 --> f1
  f2["🟡 insightsBooking.ts +73/-82"]
  pr_0 --> f2
  style f1 fill:#7c3aed,color:#fff,stroke:#7c3aed
  style f2 fill:#7c3aed,color:#fff,stroke:#7c3aed

Loading

1 finding — minor items below.

Severity	Count
🔵 Info	1

Review details

• Reviewed at: 2026-04-13T00:49:01.966Z
• Agents used: security, logic, style, architecture, performance, verification
• Context level: full
• Execution time: 78.8s
• Low-confidence findings suppressed: 2
• Blast radius: 0 (contained)

Powered by Grapple PR — AI that understands your code.

[grapple-pr]

🔵 INFO — Documentation (confidence: 71%)

New InsightsBookingServicePublicOptions type lacks documentation explaining when and why it differs from InsightsBookingServiceOptions, or how the public/internal distinction is intended to be used.

Evidence:

• InsightsBookingServicePublicOptions is exported but has no JSDoc comment
• It appears to be a simplified version of InsightsBookingServiceOptions without the scope-specific type discrimination
• No comment explains why the constructor accepts a different type than the internal field

_{Agent: style}

[Frawatson]

@grapple-pr review

[grapple-pr]

🤖 Grapple PR Review

Summary

This PR converts InsightsBookingService from using Prisma ORM WhereInput objects to raw Prisma.sql template literal queries for authorization and filter condition building, replacing the Prisma query builder pattern with raw SQL fragments intended for use in raw query composition.

Changes: 2 files (+114 / -231 lines)

• 🟡 2 files modified: packages/lib/server/service/__tests__/insightsBooking.integration-test.ts, packages/lib/server/service/insightsBooking.ts

Resolves: #5 (Advanced date override handling and timezone compatibility improvements)

CI: ⏳ 0 passed, 0 failed

Impact Diagram

graph TD
  pr_0[/"🔀 PR Changes: 2 files"/]
  f1["🟡 insightsBooking.integration-test.ts +41/-149"]
  pr_0 --> f1
  f2["🟡 insightsBooking.ts +73/-82"]
  pr_0 --> f2
  style f1 fill:#7c3aed,color:#fff,stroke:#7c3aed
  style f2 fill:#7c3aed,color:#fff,stroke:#7c3aed

Loading

2 critical issues found — please address before merging.

Severity	Count
🔴 Critical	2
🟠 Major	4
🟡 Minor	3
🔵 Info	5

Review details

• Reviewed at: 2026-04-13T01:00:07.477Z
• Agents used: security, logic, style, architecture, performance, verification
• Context level: full
• Execution time: 94.1s
• Low-confidence findings suppressed: 2
• Blast radius: 0 (contained)

Powered by Grapple PR — AI that understands your code.

[grapple-pr]

🔴 CRITICAL — API contract violation (confidence: 100%)

The findMany() method has been removed and replaced with getBaseConditions(). Any downstream callers of InsightsBookingService.findMany() will get a runtime error (service.findMany is not a function). The blast radius analysis only covers the test file, but the intent specification explicitly flags this risk. The diff does not show any updated callers outside of the test file.

Evidence:

• findMany() was a public method on InsightsBookingService and is completely removed
• The test file converts its findMany usage to getBaseConditions + raw query, but no other callers are shown
• Intent specification risk area: 'Removal of findMany() method: downstream callers not shown in the diff may be broken'

_{Agent: architecture}

[grapple-pr]

🔴 CRITICAL — SQL correctness / Runtime error (confidence: 100%)

In buildOrgAuthorizationCondition, teamIds can be an empty array (if the org has no child teams). Passing an empty array to ANY(${teamIds}) in PostgreSQL via Prisma.sql will generate ANY(ARRAY[]::int4[]) or similar, which may work syntactically but semantically produces no matches for the team condition — while the userIdsFromOrg branch is guarded with length > 0, the teamIds branch is not. More critically, conditions.reduce(...) on a single-element array (when userIdsFromOrg is empty) works, but if teamIds is empty AND userIdsFromOrg is empty, the query returns rows matching teamId = ANY(ARRAY[]) which matches nothing — but this isn't explicit. However, the real concern: in buildTeamAuthorizationCondition (line 199), if userIdsFromTeam is empty, conditions has only one element, so the team-booking-only condition is returned without the user booking OR branch — this silently drops individual (non-team) bookings for teams with no members fetched. This is a behavioral change from the original code which always included both OR branches.

Evidence:

• Original Prisma WhereInput for team scope always included both OR branches: teamId/isTeamBooking:true and userId in [...]/isTeamBooking:false
• New code at line 199: if (userIdsFromTeam.length > 0) — when empty, the userId condition is completely omitted
• Original code at line ~190 (before) did NOT have this guard — it would pass in: [] which Prisma ORM handles as 'match nothing' but still includes the OR branch
• Similarly in buildOrgAuthorizationCondition, the original code conditionally spread the userId branch but always included teamId branch even if empty

_{Agent: architecture}

[grapple-pr]

🟠 MAJOR — Test Coverage Regression — Caching tests removed (confidence: 100%)

The entire 'Caching' describe block (two test cases) was removed. The caching logic (cachedAuthConditions, cachedFilterConditions) still exists in the production code but is no longer tested. The intent specification explicitly states 'Cached conditions (cachedAuthConditions, cachedFilterConditions) still function correctly to avoid redundant DB calls' as an acceptance criterion, yet the tests validating this behavior have been deleted.

Evidence:

• Lines removed: the 'should cache authorization conditions' and 'should cache filter conditions' tests
• Acceptance criteria: 'Cached conditions (cachedAuthConditions, cachedFilterConditions) still function correctly to avoid redundant DB calls'
• The caching code at lines 77-79 and 83-85 of insightsBooking.ts still exists but is untested

_{Agent: logic}

[grapple-pr]

🟠 MAJOR — Loss of input validation (confidence: 100%)

The constructor parameter type changed from InsightsBookingServiceOptions (backed by Zod discriminated union validation via safeParse) to InsightsBookingServicePublicOptions (a plain TypeScript type). The Zod schema enforced that teamId is required when scope is 'team' and absent when scope is 'user'. The new plain type makes teamId optional regardless of scope. A caller passing { scope: 'team', userId: 1, orgId: 1 } (missing teamId) will no longer get a validation error — the code will proceed and this.options.teamId will be undefined, which flows into buildTeamAuthorizationCondition and may cause a SQL error or incorrect query.

Evidence:

• Original Zod schema: scope 'team' requires teamId (z.number()), scope 'user' omits it
• New InsightsBookingServicePublicOptions: teamId?: number is optional for all scopes
• Constructor at line 62 does safeParse but uses null on failure — no error surfaced to caller
• buildTeamAuthorizationCondition receives options.teamId which could be undefined, passed into teamRepo.findByIdAndParentId({ id: options.teamId, ... }) — undefined id will cause unexpected behavior

_{Agent: architecture}

[grapple-pr]

🟠 MAJOR — Test assertion correctness (confidence: 99%)

Tests use toEqual() to compare Prisma.sql template literal objects. Prisma.sql returns a Prisma.Sql instance whose internal structure (strings array + values array) depends on how the template is constructed. Two semantically identical SQL fragments built via different template literal compositions may not be structurally equal. For example, Prisma.sql("userId" = ${id}) AND ("teamId" IS NULL) constructed in the test may have different internal `strings` arrays than the one constructed inside the service via `Prisma.sql`("userId" = ${this.options.userId}) AND ("teamId" IS NULL) — they should match in this simple case, but for the nested reduce-based compositions (org/team scope), the structural equality is fragile because the service builds the SQL incrementally via reduce while the test builds it in a single template literal.

Evidence:

• Service uses conditions.reduce((acc, condition, index) => Prisma.sql(${acc}) OR (${condition})) which nests Prisma.Sql objects inside each other
• Tests construct the expected value as a single flat template: Prisma.sql(("teamId" = ${...}) AND ...) OR ...``
• Prisma.Sql internally stores a strings array and values array; nested Sql objects in values are preserved as-is, not flattened
• This means the reduce-built Sql and the flat-template Sql will have different internal structures even if they produce the same SQL string

_{Agent: architecture}

[grapple-pr]

🔵 INFO — Code organization (confidence: 99%)

New InsightsBookingServicePublicOptions type lacks validation and documentation. Unlike the zod-validated InsightsBookingServiceOptions, this plain type accepts raw constructor arguments without schema validation, which could allow invalid scope values to reach business logic.

Evidence:

• Constructor signature changed from options: InsightsBookingServiceOptions (zod-validated) to options: InsightsBookingServicePublicOptions (plain type)
• InsightsBookingServicePublicOptions omits the discriminated union constraint that enforces valid scope + field combinations
• If callers pass invalid scope strings or missing required fields, there's no validation error until runtime logic checks this.options.scope

_{Agent: style}

[grapple-pr]

🔵 INFO — Error messages (confidence: 89%)

Silent fallback to NOTHING_CONDITION when authorization check fails. The code returns NOTHING_CONDITION (Prisma.sql1=0) for unauthorized users without logging or clear indication of why access was denied, which could complicate debugging.

Evidence:

• Lines 131-132: if (!isOwnerOrAdmin) { return NOTHING_CONDITION; } with no log or error message
• This silently restricts query results instead of explicitly indicating authorization failure
• Callers receive empty results with no feedback on why

_{Agent: style}

[grapple-pr]

🔵 INFO — Documentation (confidence: 79%)

Missing documentation on cached conditions behavior. The private cachedAuthConditions and cachedFilterConditions fields cache results indefinitely for the service instance lifetime, but there is no JSDoc explaining when/why caching occurs or thread-safety implications.

Evidence:

• Caching fields use undefined sentinel to detect first call vs cached state
• No inline comments explaining cache invalidation strategy
• API contract (getAuthorizationConditions / getFilterConditions) mentions caching in acceptance criteria but not in code comments

_{Agent: style}

[grapple-pr]

🔵 INFO — Code patterns (confidence: 74%)

Inconsistent conditional logic for building SQL conditions in buildOrgAuthorizationCondition. The code pushes a team booking condition unconditionally, then conditionally pushes user conditions, creating asymmetric branches that could fail silently if userIdsFromOrg is always empty.

Evidence:

• Line 163: unconditional push of team condition
• Lines 165-167: conditional push of user condition only if userIdsFromOrg.length > 0
• If org has members but none are users (e.g., all in teams), the OR reduces to single condition, which is valid but semantically different from expected dual-branch structure

_{Agent: style}

[grapple-pr]

🔵 INFO — Code patterns (confidence: 74%)

Simplified membership role check improves readability but loses semantic clarity. The original code ([MembershipRole.OWNER, MembershipRole.ADMIN] as const).includes(...) clearly documents the set of authorized roles; the new code || operator is more compact but less obvious at a glance.

Evidence:

• Old: ([MembershipRole.OWNER, MembershipRole.ADMIN] as const).includes(membership.role)
• New: membership.role === MembershipRole.OWNER || membership.role === MembershipRole.ADMIN
• New pattern is harder to extend if a third role needs authorization later

_{Agent: style}

[grapple-pr]

🔧 [Grapple PR] 3 auto-fix suggestions available

Apply individually or use GitHub's "Add suggestion to batch" → "Commit suggestions" to apply all at once.

_{Commits from Grapple PR are prefixed with [Grapple PR] for easy identification in merge conflicts.}

[grapple-pr]

✅ [Grapple PR] Auto-fix — architecture agent (Small fix (9 lines, 1 file))

In buildOrgAuthorizationCondition, teamIds can be an empty array (if the org has no child teams). Passing an empty array to ANY(${teamIds}) in PostgreSQL via Prisma.sql will generate ANY(ARRAY[]::int4[]) or similar, which may work syntactically but semantically produces no matches for the team condition — while the userIdsFromOrg branch is guarded with length > 0, the teamIds branch is not. More critically, conditions.reduce(...) on a single-element array (when userIdsFromOrg is empty) works, but if teamIds is empty AND userIdsFromOrg is empty, the query returns rows matching teamId = ANY(ARRAY[]) which matches nothing — but this isn't explicit. However, the real concern: in buildTeamAuthorizationCondition (line 199), if userIdsFromTeam is empty, conditions has only one element, so the team-booking-only condition is returned without the user booking OR branch — this silently drops individual (non-team) bookings for teams with no members fetched. This is a behavioral change from the original code which always included both OR branches.

Suggested change

	)
	// Always include the individual booking branch to match original ORM behavior where
	// `userId: { in: [] }` was always present (matching nothing when empty, but structurally
	// preserving both OR branches). An empty array produces ANY(ARRAY[]::integer[]) which
	// correctly matches no rows without omitting the branch entirely.
	const userIdArray = userIdsFromTeam.length > 0 ? userIdsFromTeam : ([] as number[]);
	conditions.push(Prisma.sql`("userId" = ANY(${userIdArray})) AND ("isTeamBooking" = false)`);

_{🤖 Grapple PR auto-fix • critical • confidence: 100%}

[grapple-pr]

✅ [Grapple PR] Auto-fix — logic agent (Small fix (7 lines, 1 file))

In getBaseConditions(), the final else branch returning NOTHING_CONDITION is unreachable. getAuthorizationConditions() always returns a Prisma.Sql object (never null/undefined/falsy), so authConditions is always truthy. The code will always enter either the first branch (if filterConditions is truthy) or the second branch (if filterConditions is null). The third and fourth branches are dead code.

Suggested change

	return authConditions;
	if (filterConditions) {
	return authConditions;

_{🤖 Grapple PR auto-fix • minor • confidence: 100%}

[grapple-pr]

✅ [Grapple PR] Auto-fix — logic agent (Small fix (2 lines, 1 file))

The constructor now accepts InsightsBookingServicePublicOptions (a plain TypeScript type) instead of InsightsBookingServiceOptions (a zod-validated discriminated union). The zod schema enforced that teamId is required when scope === 'team' and forbidden otherwise. With the plain type, teamId is always optional regardless of scope, so a caller could pass scope: 'team' without teamId, which would cause a runtime error downstream when buildTeamAuthorizationCondition tries to access options.teamId.

Suggested change

	}: {
	options: InsightsBookingServiceOptions | InsightsBookingServicePublicOptions;

_{🤖 Grapple PR auto-fix • minor • confidence: 93%}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — architecture agent (Larger fix (24 lines, 1 file) — review recommended)

The findMany() method has been removed and replaced with getBaseConditions(). Any downstream callers of InsightsBookingService.findMany() will get a runtime error (service.findMany is not a function). The blast radius analysis only covers the test file, but the intent specification explicitly flags this risk. The diff does not show any updated callers outside of the test file.

--- a/packages/lib/server/service/insightsBooking.ts
+++ b/packages/lib/server/service/insightsBooking.ts
@@ -65,6 +65,27 @@ export class InsightsBookingService {
     this.filters = filters;
   }
 
+  /**
+   * @deprecated Use getBaseConditions() with a raw Prisma.sql query instead.
+   * This shim preserves backward compatibility for callers that have not yet been
+   * migrated to the raw-SQL pattern introduced in this PR.
+   *
+   * NOTE: findManyArgs.where is ignored because authorization/filter conditions
+   * are now expressed as raw SQL fragments via getBaseConditions(). Callers that
+   * pass additional `where` predicates must be migrated to compose raw SQL
+   * manually against getBaseConditions().
+   *
+   * Edge case: if getBaseConditions() returns NOTHING_CONDITION (1=0), the raw
+   * query will return zero rows — matching the original behavior where NOTHING
+   * ({ id: -1 }) would yield no results.
+   */
+  async findMany(findManyArgs: Prisma.BookingTimeStatusDenormalizedFindManyArgs) {
+    const conditions = await this.getBaseConditions();
+    // NOTE: findManyArgs.select / take / skip / orderBy are not forwarded in this
+    // shim — callers relying on those args must migrate to raw queries themselves.
+    return this.prisma.$queryRaw<{ id: number }[]>`
+      SELECT * FROM "BookingTimeStatusDenormalized"
+      WHERE ${conditions}
+    `;
+  }
+
   async getBaseConditions(): Promise<Prisma.Sql> {
     const authConditions = await this.getAuthorizationConditions();
     const filterConditions = await this.getFilterConditions();

_{🤖 Grapple PR auto-fix • critical • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — logic agent (Larger fix (60 lines, 1 file) — review recommended)

The entire 'Caching' describe block (two test cases) was removed. The caching logic (cachedAuthConditions, cachedFilterConditions) still exists in the production code but is no longer tested. The intent specification explicitly states 'Cached conditions (cachedAuthConditions, cachedFilterConditions) still function correctly to avoid redundant DB calls' as an acceptance criterion, yet the tests validating this behavior have been deleted.

--- a/packages/lib/server/service/__tests__/insightsBooking.integration-test.ts
+++ b/packages/lib/server/service/__tests__/insightsBooking.integration-test.ts
@@ -426,6 +426,84 @@
       await testData.cleanup();
     });
   });
+
+  describe("Caching", () => {
+    it("should cache authorization conditions", async () => {
+      const testData = await createTestData({
+        teamRole: MembershipRole.OWNER,
+        orgRole: MembershipRole.OWNER,
+      });
+
+      const service = new InsightsBookingService({
+        prisma,
+        options: {
+          scope: "user",
+          userId: testData.user.id,
+          orgId: testData.org.id,
+        },
+      });
+
+      // First call should build conditions from DB
+      const conditions1 = await service.getAuthorizationConditions();
+      expect(conditions1).toEqual(
+        Prisma.sql`("userId" = ${testData.user.id}) AND ("teamId" IS NULL)`
+      );
+
+      // Second call should return the exact same cached object (no recomputation)
+      // This validates cachedAuthConditions avoids redundant DB calls
+      const conditions2 = await service.getAuthorizationConditions();
+      expect(conditions2).toBe(conditions1);
+
+      await testData.cleanup();
+    });
+
+    it("should cache filter conditions", async () => {
+      const testData = await createTestData();
+
+      const service = new InsightsBookingService({
+        prisma,
+        options: {
+          scope: "user",
+          userId: testData.user.id,
+          orgId: testData.org.id,
+        },
+        filters: {
+          eventTypeId: testData.eventType.id,
+        },
+      });
+
+      // First call should build conditions
+      const conditions1 = await service.getFilterConditions();
+      expect(conditions1).toEqual(
+        Prisma.sql`("eventTypeId" = ${testData.eventType.id}) OR ("eventParentId" = ${testData.eventType.id})`
+      );
+
+      // Second call should return the exact same cached object (no recomputation)
+      // This validates cachedFilterConditions avoids redundant DB calls
+      const conditions2 = await service.getFilterConditions();
+      expect(conditions2).toBe(conditions1);
+
+      await testData.cleanup();
+    });
+  });
 
   describe("getBaseConditions", () => {
     it("should combine authorization and filter conditions", async () => {

_{🤖 Grapple PR auto-fix • major • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — architecture agent (Larger fix (12 lines, 1 file) — review recommended)

The constructor parameter type changed from InsightsBookingServiceOptions (backed by Zod discriminated union validation via safeParse) to InsightsBookingServicePublicOptions (a plain TypeScript type). The Zod schema enforced that teamId is required when scope is 'team' and absent when scope is 'user'. The new plain type makes teamId optional regardless of scope. A caller passing { scope: 'team', userId: 1, orgId: 1 } (missing teamId) will no longer get a validation error — the code will proceed and this.options.teamId will be undefined, which flows into buildTeamAuthorizationCondition and may cause a SQL error or incorrect query.

--- a/packages/lib/server/service/insightsBooking.ts
+++ b/packages/lib/server/service/insightsBooking.ts
@@ -60,8 +60,17 @@ export class InsightsBookingService {
     filters,
   }: {
     prisma: typeof readonlyPrisma;
     options: InsightsBookingServicePublicOptions;
     filters?: InsightsBookingServiceFilterOptions;
   }) {
     this.prisma = prisma;
-    this.options = insightsBookingServiceOptionsSchema.safeParse(options)?.data ?? null;
+    const parsed = insightsBookingServiceOptionsSchema.safeParse(options);
+    if (!parsed.success) {
+      // Validation failed: e.g. scope is 'team' but teamId is missing.
+      // We surface this as a thrown error so callers get an immediate, actionable
+      // failure rather than a silent null that propagates into raw SQL queries
+      // (where an undefined teamId would cause a SQL error or wrong results).
+      throw new Error(
+        `InsightsBookingService: invalid options — ${parsed.error.issues.map((i) => i.message).join(", ")}`
+      );
+    }
+    this.options = parsed.data;
     this.filters = filters;
   }

_{🤖 Grapple PR auto-fix • major • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — architecture agent (Larger fix (20 lines, 1 file) — review recommended)

Tests use toEqual() to compare Prisma.sql template literal objects. Prisma.sql returns a Prisma.Sql instance whose internal structure (strings array + values array) depends on how the template is constructed. Two semantically identical SQL fragments built via different template literal compositions may not be structurally equal. For example, Prisma.sql("userId" = ${id}) AND ("teamId" IS NULL) constructed in the test may have different internal `strings` arrays than the one constructed inside the service via `Prisma.sql`("userId" = ${this.options.userId}) AND ("teamId" IS NULL) — they should match in this simple case, but for the nested reduce-based compositions (org/team scope), the structural equality is fragile because the service builds the SQL incrementally via reduce while the test builds it in a single template literal.

--- a/packages/lib/server/service/__tests__/insightsBooking.integration-test.ts
+++ b/packages/lib/server/service/__tests__/insightsBooking.integration-test.ts
@@ -1,6 +1,7 @@
 import type { Team, User, Membership } from "@prisma/client";
 import { Prisma } from "@prisma/client";
 import { describe, expect, it } from "vitest";
 
 import prisma from "@calcom/prisma";
 import { BookingStatus, MembershipRole } from "@calcom/prisma/enums";
 
 import { InsightsBookingService } from "../../service/insightsBooking";
 
+/**
+ * Extracts the SQL string and bound values from a Prisma.Sql instance for
+ * robust comparison. We cannot use toEqual() directly on Prisma.Sql objects
+ * because the service builds SQL incrementally via reduce() (nesting Sql
+ * objects inside each other), while test expectations are built as flat
+ * template literals — they produce the same SQL but have different internal
+ * `strings` array structures.
+ */
+function sqlToComparable(sql: Prisma.Sql): { sql: string; values: unknown[] } {
+  return { sql: sql.sql, values: sql.values };
+}
+
+/**
+ * Assertion helper: compares two Prisma.Sql instances by their rendered SQL
+ * string and bound parameter values rather than their internal AST structure.
+ */
+function expectSqlEqual(actual: Prisma.Sql, expected: Prisma.Sql): void {
+  expect(sqlToComparable(actual)).toEqual(sqlToComparable(expected));
+}
+
 const NOTHING_CONDITION = Prisma.sql`1=0`;

_{🤖 Grapple PR auto-fix • major • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — logic agent (Larger fix (17 lines, 1 file) — review recommended)

In buildTeamAuthorizationCondition, when userIdsFromTeam is empty (team exists but has no members), the old code included userId: { in: [] } in the OR clause (Prisma ORM treats in: [] as matching nothing, so effectively only team bookings matched). The new code skips the userId condition entirely when userIdsFromTeam.length > 0 is false. The semantic result is equivalent (only team bookings match), BUT the test on line 292 of the test file expects the SQL to always include the userId = ANY(...) clause with [testData.user.id]. If the membership query returns no users for some reason, the SQL output structure changes. More critically, the test constructs the expected SQL with ANY(${[testData.user.id]}) — this test will pass only if the membership lookup returns the expected user. This is functionally equivalent but represents a silent behavioral difference for edge cases.

--- a/packages/lib/server/service/insightsBooking.ts
+++ b/packages/lib/server/service/insightsBooking.ts
@@ -193,14 +193,12 @@ export class InsightsBookingService {
     const userIdsFromTeam = usersFromTeam.map((u) => u.userId);
 
-    const conditions: Prisma.Sql[] = [
-      Prisma.sql`("teamId" = ${options.teamId}) AND ("isTeamBooking" = true)`,
-    ];
-
-    if (userIdsFromTeam.length > 0) {
-      conditions.push(Prisma.sql`("userId" = ANY(${userIdsFromTeam})) AND ("isTeamBooking" = false)`);
-    }
-
-    return conditions.reduce((acc, condition, index) => {
-      if (index === 0) return condition;
-      return Prisma.sql`(${acc}) OR (${condition})`;
-    });
+    // Always include the userId branch to preserve structural consistency with the original
+    // ORM behaviour where `userId: { in: [] }` was always present in the OR clause.
+    // When userIdsFromTeam is empty, `= ANY(ARRAY[]::int[])` matches nothing — equivalent
+    // to the old `in: []` no-op — but the SQL shape remains stable for callers and tests.
+    return Prisma.sql`(("teamId" = ${options.teamId}) AND ("isTeamBooking" = true)) OR (("userId" = ANY(${userIdsFromTeam})) AND ("isTeamBooking" = false))`;
   }

_{🤖 Grapple PR auto-fix • major • Review this diff before applying}

[grapple-pr]

🟡 [Grapple PR] Suggested fix — architecture agent (Larger fix (92 lines, 1 file) — review recommended)

The entire 'Caching' test suite has been removed (lines deleted around the old test). While the caching implementation still exists in the service (cachedAuthConditions, cachedFilterConditions), there are no longer any tests verifying that the caching behavior works correctly. This is a regression in test coverage for a feature that the intent specification lists as an acceptance criterion.

--- a/packages/lib/server/service/__tests__/insightsBooking.integration-test.ts
+++ b/packages/lib/server/service/__tests__/insightsBooking.integration-test.ts
@@ -434,6 +434,80 @@
       await testData.cleanup();
     });
   });
 
+  describe("Caching", () => {
+    it("should cache authorization conditions and avoid redundant DB calls", async () => {
+      const testData = await createTestData({
+        teamRole: MembershipRole.OWNER,
+        orgRole: MembershipRole.OWNER,
+      });
+
+      const service = new InsightsBookingService({
+        prisma,
+        options: {
+          scope: "user",
+          userId: testData.user.id,
+          orgId: testData.org.id,
+        },
+      });
+
+      // First call should build conditions from DB
+      const conditions1 = await service.getAuthorizationConditions();
+      expect(conditions1).toEqual(
+        Prisma.sql`("userId" = ${testData.user.id}) AND ("teamId" IS NULL)`
+      );
+
+      // Second call should return the exact same cached object (referential equality
+      // proves cachedAuthConditions is returned without re-querying the DB)
+      const conditions2 = await service.getAuthorizationConditions();
+      expect(conditions2).toBe(conditions1);
+
+      await testData.cleanup();
+    });
+
+    it("should cache filter conditions and avoid redundant DB calls", async () => {
+      const testData = await createTestData();
+
+      const service = new InsightsBookingService({
+        prisma,
+        options: {
+          scope: "user",
+          userId: testData.user.id,
+          orgId: testData.org.id,
+        },
+        filters: {
+          eventTypeId: testData.eventType.id,
+        },
+      });
+
+      // First call should build conditions
+      const conditions1 = await service.getFilterConditions();
+      expect(conditions1).toEqual(
+        Prisma.sql`("eventTypeId" = ${testData.eventType.id}) OR ("eventParentId" = ${testData.eventType.id})`
+      );
+
+      // Second call should return the exact same cached object (referential equality
+      // proves cachedFilterConditions is returned without re-building)
+      const conditions2 = await service.getFilterConditions();
+      expect(conditions2).toBe(conditions1);
+
+      await testData.cleanup();
+    });
+
+    it("should cache combined authorization and filter conditions independently per service instance", async () => {
+      const testData = await createTestData({
+        teamRole: MembershipRole.OWNER,
+        orgRole: MembershipRole.OWNER,
+      });
+
+      const service = new InsightsBookingService({
+        prisma,
+        options: {
+          scope: "user",
+          userId: testData.user.id,
+          orgId: testData.org.id,
+        },
+        filters: {
+          memberUserId: testData.user.id,
+        },
+      });
+
+      // Both caches should be populated independently
+      const authConditions1 = await service.getAuthorizationConditions();
+      const filterConditions1 = await service.getFilterConditions();
+
+      const authConditions2 = await service.getAuthorizationConditions();
+      const filterConditions2 = await service.getFilterConditions();
+
+      // Referential equality confirms caching is working for both independently
+      expect(authConditions2).toBe(authConditions1);
+      expect(filterConditions2).toBe(filterConditions1);
+
+      await testData.cleanup();
+    });
+  });
+
   describe("getBaseConditions", () => {

_{🤖 Grapple PR auto-fix • minor • Review this diff before applying}