Conversation
WalkthroughAdds Cal.ai features across the codebase: introduces a new CSS variable Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
keithwillcode
added
core
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
| <div className="flex items-center justify-between"> | ||
| <span className="w-full" data-testid={`select-option-${(props as unknown as ExtendedOption).value}`}> | ||
| {(props.data as unknown as ExtendedOption).isCalAi ? ( | ||
| <Badge startIcon="sparkles" variant="purple" className="mr-1 hidden md:inline-flex"> |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (3)
273-316: Avoid getting stuck after failed auto-create; await mutation and drop console.log.Same issue previously flagged; symptoms: CTA can remain disabled due to
hasAutoCreated.currentand errors aren’t caught frommutate. UsemutateAsync, await it, and reset flags on error. Remove the console.log.- if (isCreatingAgent.current || createAgentMutation.isPending) { - console.log("Agent creation already in progress, skipping..."); + if (isCreatingAgent.current || createAgentMutation.isPending) { return; } @@ - if (updatedStep?.id) { - createAgentMutation.mutate({ + if (updatedStep?.id) { + await createAgentMutation.mutateAsync({ teamId, workflowStepId: updatedStep.id, ...(templateWorkflowId && { templateWorkflowId }), }); } else { showToast(t("failed_to_get_workflow_step_id"), "error"); } } catch (error) { console.error("Failed to create agent:", error); showToast(t("failed_to_create_agent"), "error"); + // enable retry after failure + hasAutoCreated.current = false; } finally { isCreatingAgent.current = false; }Outside selected lines (mutation options), also consider:
// add to createAgentMutation options onSettled: (_data, error) => { if (error) hasAutoCreated.current = false; },
321-346: Auto-create guard should not rely onhasAutoCreatedto prevent retries.Rely on
isPending/isSuccessplus the in-flightisCreatingAgentref. Don’t sethasAutoCreatedbefore the async call completes; otherwise a failure blocks the CTA.- const shouldAutoCreate = + const shouldAutoCreate = autoCreateAgent === "true" && templateWorkflowId && step?.id && step.action === WorkflowActions.CAL_AI_PHONE_CALL && !stepAgentId && - !hasAutoCreated.current && !createAgentMutation.isPending && !createAgentMutation.isSuccess; @@ - if (shouldAutoCreate && onSaveWorkflow) { - hasAutoCreated.current = true; - handleCreateAgent(templateWorkflowId); - } + if (shouldAutoCreate && onSaveWorkflow) { + handleCreateAgent(templateWorkflowId); + }
780-783: Don’t disable CTA withhasAutoCreated; base on pending state and in-flight ref.Keeps the button retryable after failures and prevents accidental permanent disable.
- disabled={props.readOnly || isCreatingAgent.current || hasAutoCreated.current} + disabled={props.readOnly || isCreatingAgent.current || createAgentMutation.isPending} className="flex items-center justify-center" onClick={() => handleCreateAgent()} - loading={createAgentMutation.isPending || isCreatingAgent.current}> + loading={createAgentMutation.isPending || isCreatingAgent.current}>
🧹 Nitpick comments (3)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (3)
193-197: Preserve URL hash when cleaning up params; optionally suppress duplicate toast in auto-flow.
- Add url.hash to avoid dropping fragment on replace.
- In autoCreateAgent flow, consider suppressing the extra success toast here for a single final toast UX.
- router.replace(url.pathname + url.search); + router.replace(url.pathname + url.search + url.hash);Outside selected lines (for clarity), gate the toast:
// Inside onSuccess before deleting params const isAuto = new URL(window.location.href).searchParams.get("autoCreateAgent") === "true"; if (!isAuto) { showToast(t("agent_created_successfully"), "success"); }
258-258: Remove dead state or convert to ref.
_isSenderIsNeededstate is never read; only the setter is used to trigger re-renders. Either derive this fromactionor use a ref if you don’t need re-render semantics.
318-320: Minor: avoid capturing params; read at use-site.Reading
autoCreateAgent/templateWorkflowIddirectly insideonSuccessand the effect reduces stale-closure risk and keeps behavior consistent after navigation.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/web/public/static/locales/en/common.json(8 hunks)packages/features/ee/workflows/components/WorkflowStepContainer.tsx(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/public/static/locales/en/common.json
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
🧠 Learnings (7)
📓 Common learnings
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/WorkflowStepContainer.tsx:641-649
Timestamp: 2025-08-26T20:09:17.089Z
Learning: In packages/features/ee/workflows/components/WorkflowStepContainer.tsx, Cal.AI actions are intentionally filtered out/hidden for organization workflows when props.isOrganization is true. This restriction is by design per maintainer Udit-takkar in PR #22995, despite the broader goal of enabling Cal.AI self-serve.
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com workflows and AI agent components, variable insertion follows a consistent pattern of directly transforming the input variable with toUpperCase() and replace(/ /g, "_") to create tokens like {VARIABLE_NAME}. The AgentConfigurationSheet.tsx implementation correctly follows this same pattern as WorkflowStepContainer.tsx, per maintainer Udit-takkar in PR #22995.
📚 Learning: 2025-08-26T20:09:17.089Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/WorkflowStepContainer.tsx:641-649
Timestamp: 2025-08-26T20:09:17.089Z
Learning: In packages/features/ee/workflows/components/WorkflowStepContainer.tsx, Cal.AI actions are intentionally filtered out/hidden for organization workflows when props.isOrganization is true. This restriction is by design per maintainer Udit-takkar in PR #22995, despite the broader goal of enabling Cal.AI self-serve.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-15T00:27:33.280Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com workflows and AI agent components, variable insertion follows a consistent pattern of directly transforming the input variable with toUpperCase() and replace(/ /g, "_") to create tokens like {VARIABLE_NAME}. The AgentConfigurationSheet.tsx implementation correctly follows this same pattern as WorkflowStepContainer.tsx, per maintainer Udit-takkar in PR #22995.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-09-08T09:34:46.661Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23421
File: packages/features/ee/workflows/components/AddActionDialog.tsx:171-174
Timestamp: 2025-09-08T09:34:46.661Z
Learning: In packages/features/ee/workflows/components/AddActionDialog.tsx, the actionOptions prop is guaranteed to never be empty according to maintainer CarinaWolli, so defensive checks for empty arrays are not necessary.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-14T10:30:23.062Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/lib/schema.ts:41-43
Timestamp: 2025-08-14T10:30:23.062Z
Learning: In calcom/cal.com workflows with CAL_AI_PHONE_CALL actions, agentId can legitimately be null during initial setup. The workflow is: 1) save workflow to get step ID, 2) use step ID to create and link agent. Therefore, agentId should remain nullable in schemas to support this multi-step setup process (per maintainer Udit-takkar).
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-27T12:15:43.830Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/trpc/server/routers/viewer/aiVoiceAgent/testCall.handler.ts:41-44
Timestamp: 2025-08-27T12:15:43.830Z
Learning: In calcom/cal.com, the AgentService.getAgent() method in packages/features/calAIPhone/providers/retellAI/services/AgentService.ts does NOT include authorization checks - it only validates the agentId parameter and directly calls the repository without verifying user/team access. This contrasts with other methods like getAgentWithDetails() which properly use findByIdWithUserAccessAndDetails() for authorization. When reviewing updateToolsFromAgentId() calls, always verify both agent ownership and eventType ownership are checked.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
📚 Learning: 2025-08-08T09:27:23.896Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:195-216
Timestamp: 2025-08-08T09:27:23.896Z
Learning: In PR calcom/cal.com#22919, file packages/features/calAIPhone/providers/retellAI/services/AgentService.ts, the updateAgentConfiguration method intentionally does not persist the optional `name` parameter to the repository for now, per maintainer (Udit-takkar). Future reviews should not flag this unless requirements change.
Applied to files:
packages/features/ee/workflows/components/WorkflowStepContainer.tsx
🧬 Code graph analysis (1)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (1)
packages/features/ee/workflows/lib/actionHelperFunctions.ts (1)
isCalAIAction(37-39)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (3)
packages/features/ee/workflows/components/WorkflowStepContainer.tsx (3)
4-4: Good call adding useCallback.Keeps handler identity stable and prevents unnecessary re-renders.
641-642: Nice: exposeisCalAito the select option.Enables UI affordances (badges/labels) without re-deriving downstream.
766-767: Responsive layout improvement LGTM.Better mobile stacking and alignment in the CAL.AI setup block.
|
@keithwillcode i'll fix the e2e tests |
Udit-takkar
dismissed stale reviews from PeerRich and CarinaWolli
via
7950ddb
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/features/ee/workflows/components/AgentConfigurationSheet.tsx (2)
711-735: Hide/disable “Test agent” when no agentId to avoid inert UIClicking opens state but the dialog won’t render without
agentId, leading to a no‑op. Either hide the control or disable it when!agentId.Apply one of the following minimal fixes (preferred: hide):
Option A — hide when unavailable:
- <div className="mr-auto"> + {agentId && ( + <div className="mr-auto"> <Dropdown> <DropdownMenuTrigger asChild> <Button color="secondary" className="rounded-[10px]" - disabled={readOnly} + disabled={readOnly} EndIcon="chevron-down"> {t("test_agent")} </Button> </DropdownMenuTrigger> <DropdownMenuContent> <DropdownMenuItem> <DropdownItem type="button" StartIcon="monitor" onClick={() => { setIsWebCallDialogOpen(true); }}> {t("web_call")} </DropdownItem> </DropdownMenuItem> </DropdownMenuContent> </Dropdown> - </div> + </div> + )}Option B — keep visible but disable the trigger:
- disabled={readOnly} + disabled={readOnly || !agentId}
737-741:justify-centerlikely redundant on ButtonOur Button styles already center content. Dropping this reduces class churn.
- <Button - className="justify-center" + <Button type="button" color="secondary" onClick={() => onOpenChange(false)}> ... - <Button - type="button" - className="justify-center" + <Button + type="button" onClick={agentForm.handleSubmit(handleAgentUpdate)}Also applies to: 744-749
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/web/playwright/fixtures/workflows.ts(1 hunks)packages/features/ee/workflows/components/AgentConfigurationSheet.tsx(2 hunks)packages/features/ee/workflows/components/WorkflowCreationDialog.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/features/ee/workflows/components/WorkflowCreationDialog.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
apps/web/playwright/fixtures/workflows.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
apps/web/playwright/fixtures/workflows.tspackages/features/ee/workflows/components/AgentConfigurationSheet.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
apps/web/playwright/fixtures/workflows.tspackages/features/ee/workflows/components/AgentConfigurationSheet.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/ee/workflows/components/AgentConfigurationSheet.tsx
🧠 Learnings (5)
📓 Common learnings
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/WorkflowStepContainer.tsx:641-649
Timestamp: 2025-08-26T20:09:17.089Z
Learning: In packages/features/ee/workflows/components/WorkflowStepContainer.tsx, Cal.AI actions are intentionally filtered out/hidden for organization workflows when props.isOrganization is true. This restriction is by design per maintainer Udit-takkar in PR #22995, despite the broader goal of enabling Cal.AI self-serve.
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com workflows and AI agent components, variable insertion follows a consistent pattern of directly transforming the input variable with toUpperCase() and replace(/ /g, "_") to create tokens like {VARIABLE_NAME}. The AgentConfigurationSheet.tsx implementation correctly follows this same pattern as WorkflowStepContainer.tsx, per maintainer Udit-takkar in PR #22995.
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com, the variable insertion pattern in AgentConfigurationSheet.tsx follows the same implementation as existing workflow code. The current approach of building variable tokens directly from the input variable (with toUpperCase and space replacement) is the established pattern and doesn't need to be changed for locale-agnostic concerns, per maintainer Udit-takkar.
📚 Learning: 2025-08-15T00:27:33.280Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com, the variable insertion pattern in AgentConfigurationSheet.tsx follows the same implementation as existing workflow code. The current approach of building variable tokens directly from the input variable (with toUpperCase and space replacement) is the established pattern and doesn't need to be changed for locale-agnostic concerns, per maintainer Udit-takkar.
Applied to files:
packages/features/ee/workflows/components/AgentConfigurationSheet.tsx
📚 Learning: 2025-08-15T00:27:33.280Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com workflows and AI agent components, variable insertion follows a consistent pattern of directly transforming the input variable with toUpperCase() and replace(/ /g, "_") to create tokens like {VARIABLE_NAME}. The AgentConfigurationSheet.tsx implementation correctly follows this same pattern as WorkflowStepContainer.tsx, per maintainer Udit-takkar in PR #22995.
Applied to files:
packages/features/ee/workflows/components/AgentConfigurationSheet.tsx
📚 Learning: 2025-08-08T09:29:11.681Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts:118-143
Timestamp: 2025-08-08T09:29:11.681Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (TypeScript), the AIPhoneServiceAgentListItem is required to include user.email in listAgents responses (per maintainer Udit-takkar). Future reviews should not flag this as unnecessary PII unless requirements change.
Applied to files:
packages/features/ee/workflows/components/AgentConfigurationSheet.tsx
📚 Learning: 2025-08-08T09:27:23.896Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:195-216
Timestamp: 2025-08-08T09:27:23.896Z
Learning: In PR calcom/cal.com#22919, file packages/features/calAIPhone/providers/retellAI/services/AgentService.ts, the updateAgentConfiguration method intentionally does not persist the optional `name` parameter to the repository for now, per maintainer (Udit-takkar). Future reviews should not flag this unless requirements change.
Applied to files:
packages/features/ee/workflows/components/AgentConfigurationSheet.tsx
🧬 Code graph analysis (1)
packages/features/ee/workflows/components/AgentConfigurationSheet.tsx (2)
packages/ui/components/dropdown/Dropdown.tsx (5)
Dropdown(12-12)DropdownMenuTrigger(15-26)DropdownMenuContent(34-51)DropdownMenuItem(63-71)DropdownItem(161-181)packages/ui/components/button/Button.tsx (1)
Button(221-349)
🔇 Additional comments (3)
apps/web/playwright/fixtures/workflows.ts (1)
28-29: Make creation path configurable and add explicit waits to reduce flakiness.Hard-coding "scratch" narrows coverage (can’t exercise Cal.ai template) and can flake if the dialog renders slowly or under different flags/roles — add a startFrom prop (default "scratch") and explicit expects before clicks.
Location: apps/web/playwright/fixtures/workflows.ts — lines 28–29
- await page.getByTestId(`workflow-option-card-scratch`).click(); - await page.getByTestId("continue-button").click(); + const optionTestId = + startFrom === "calai" ? "workflow-option-card-calai-template" : "workflow-option-card-scratch"; + const optionCard = page.getByTestId(optionTestId); + await expect(optionCard).toBeVisible(); + await optionCard.click(); + const continueBtn = page.getByTestId("continue-button"); + await expect(continueBtn).toBeEnabled(); + await continueBtn.click();Additions (illustrative — adjust exact testId if different):
// extend props type CreateWorkflowProps = { name?: string; isTeam?: true; trigger?: WorkflowTriggerEvents; startFrom?: "scratch" | "calai"; }; // destructure with default const { name, isTeam, trigger, startFrom = "scratch" } = props;I could not find a "workflow-option-card-calai-template" test id in the repo — confirm the Cal.ai card test id before applying. Search example:
rg -n -C2 'workflow-option-card-[A-Za-z0-9_-]+' || truepackages/features/ee/workflows/components/AgentConfigurationSheet.tsx (2)
910-910: Theming LGTMSwitching
bg-white→bg-defaultaligns with tokenized theming and dark‑mode support.
711-735: ```shell
#!/bin/bash
set -euo pipefailecho "PWD: $(pwd)"
echoecho "1) Find AgentConfigurationSheet.tsx"
FILE=$(find . -type f -iname 'AgentConfigurationSheet.tsx' -print -quit || true)
if [ -n "$FILE" ]; then
echo "Found: $FILE"
echo
echo "2) Print lines 680-760 of the file:"
awk 'NR>=680 && NR<=760{printf("%6d: %s\n", NR, $0)}' "$FILE" || true
else
echo "AgentConfigurationSheet.tsx not found. Listing .tsx files under packages/features (up to 200):"
find packages/features -type f -iname '*.tsx' -maxdepth 6 -print | sed -n '1,200p' || true
fiecho
echo "3) Search for translation keys and test actions (test_agent, web_call, phone_call):"
grep -RIn --exclude-dir=node_modules -E 'test_agent|web_call|phone_call' . || trueecho
echo "4) Search for DropdownItem usages:"
grep -RIn --exclude-dir=node_modules -E 'DropdownItem' . || trueecho
echo "5) Search for StartIcon props matching monitor/phone/call:"
grep -RIn --exclude-dir=node_modules -E 'StartIcon\s*=' . | grep -Ei 'monitor|phone|telephone|call' || trueecho
echo "6) Search for phone-number / phone card components (PhoneNumber, PhoneCard, PhoneNumberCard):"
grep -RIn --exclude-dir=node_modules -E 'PhoneNumber|PhoneCard|PhoneNumberCard|phone-card|phoneNumber' . || trueecho
echo "Done."</blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/playwright/icons.e2e.ts (1)
5-8: Force a deterministic visual environment.Locks down common flake sources across CI.
test.use({ viewport: { width: 1265, height: 1320 }, // Match the expected dimensions + colorScheme: "light", + deviceScaleFactor: 1, + timezoneId: "UTC", + locale: "en-US", });
🧹 Nitpick comments (1)
apps/web/playwright/icons.e2e.ts (1)
12-15: Scope the screenshot to the icon gallery (optional).Reduces noise from headers/nav/footers if they change independently.
- await expect(page).toHaveScreenshot("icons.png", { - maxDiffPixelRatio: 0.05, - animations: "disabled", - fullPage: true, - }); + const gallery = page.locator("main"); // replace with a specific [data-testid="icons-gallery"] if available + await expect(gallery).toHaveScreenshot("icons.png", { + maxDiffPixelRatio: 0.05, + animations: "disabled", + });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
apps/web/playwright/icons.e2e.ts-snapshots/icons--calcom-web-darwin.pngis excluded by!**/*.png
📒 Files selected for processing (1)
apps/web/playwright/icons.e2e.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
apps/web/playwright/icons.e2e.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
apps/web/playwright/icons.e2e.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
apps/web/playwright/icons.e2e.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/WorkflowStepContainer.tsx:641-649
Timestamp: 2025-08-26T20:09:17.089Z
Learning: In packages/features/ee/workflows/components/WorkflowStepContainer.tsx, Cal.AI actions are intentionally filtered out/hidden for organization workflows when props.isOrganization is true. This restriction is by design per maintainer Udit-takkar in PR #22995, despite the broader goal of enabling Cal.AI self-serve.
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com workflows and AI agent components, variable insertion follows a consistent pattern of directly transforming the input variable with toUpperCase() and replace(/ /g, "_") to create tokens like {VARIABLE_NAME}. The AgentConfigurationSheet.tsx implementation correctly follows this same pattern as WorkflowStepContainer.tsx, per maintainer Udit-takkar in PR #22995.
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/ee/workflows/components/AgentConfigurationSheet.tsx:348-371
Timestamp: 2025-08-15T00:27:33.280Z
Learning: In calcom/cal.com, the variable insertion pattern in AgentConfigurationSheet.tsx follows the same implementation as existing workflow code. The current approach of building variable tokens directly from the input variable (with toUpperCase and space replacement) is the established pattern and doesn't need to be changed for locale-agnostic concerns, per maintainer Udit-takkar.
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (1)
apps/web/playwright/icons.e2e.ts (1)
12-15: Don’t widen visual diff tolerance; stabilize the capture and update the golden instead.0.15 risks hiding real icon regressions—use a stricter ratio, wait for fonts/network, disable animations, and re-record the snapshot.
File: apps/web/playwright/icons.e2e.ts (lines 12–15)
test("Icons render properly", async ({ page }) => { await page.goto("/icons"); + await page.waitForLoadState("networkidle"); + // Wait for fonts to settle to avoid antialiasing/layout diffs + await page.evaluate(() => (document as any).fonts?.ready); await expect(page).toHaveScreenshot("icons.png", { - maxDiffPixelRatio: 0.15, + maxDiffPixelRatio: 0.05, + animations: "disabled", fullPage: true, }); });Re-record snapshot: pnpm playwright test apps/web/playwright/icons.e2e.ts --update-snapshots
4 participants
What does this PR do?
I'll update the 'new' button in /workflows in a follow up PR. For now I have added basic version
Screen.Recording.2025-09-11.at.3.42.54.PM.mov
Screen.Recording.2025-09-11.at.3.42.26.PM.mov
Screen.Recording.2025-09-11.at.7.15.22.PM.mov
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?