Skip to content

Conversation

@athul-rs
Copy link
Contributor

What

  • Added export reminder notification system for Prompt Studio projects that are actively used in deployments
  • Backend API endpoint to check if project is used in API Deployments, ETL Pipelines, Task Pipelines, or Manual Review
  • Frontend change tracking when users edit, add, or delete prompts
  • Yellow notification bar with immediate "Export Now" action
  • State management to clear notifications after successful export

Why

  • Users were confused when editing prompts in Prompt Studio for projects used in deployments, expecting changes to take effect immediately
  • By design, Prompt Studio projects must be exported as Tools for changes to take effect in deployments
  • No visual indication was provided to users that their changes needed to be exported

How

Backend Changes:

  • Extracted existing usage checking logic into reusable _check_tool_usage() helper method
  • Added check_deployment_usage API endpoint that checks all deployment types
  • Returns proper message with specific deployment types where the tool is used

Frontend Changes:

  • Added state management for hasUnsavedChanges, deploymentUsageInfo in custom-tool-store
  • Created ExportReminderBar component with yellow notification styling
  • Integrated change detection in DocumentParser for all prompt modifications
  • Added deployment usage checking in ToolIde component with proper orchestration
  • Ensured both header and notification exports clear the reminder

Can this PR break any existing features. If yes, please list possible items. If no, please explain why.

No, this PR will not break any existing features because:

  • It only adds new functionality without modifying existing APIs or data structures
  • All changes are additive - new state variables, new components, and new endpoints
  • Existing export functionality remains unchanged and continues to work as before
  • The notification system is purely visual and doesn't interfere with existing workflows
  • Proper error handling ensures graceful degradation if API calls fail

Database Migrations

  • N/A (No database schema changes required)

Env Config

  • N/A (No new environment variables required)

Relevant Docs

  • Feature addresses UN-1722 JIRA ticket requirements

Related Issues or PRs

  • UN-1722: Export reminder for Prompt Studio projects in use

Dependencies Versions

  • N/A (No new dependencies added)

Notes on Testing

  • ✅ Tested change detection for editing, adding, and deleting prompts
  • ✅ Verified API correctly identifies all deployment types (API, ETL, Task, Manual Review)
  • ✅ Confirmed notification only shows when both conditions are met (changes + deployed)
  • ✅ Tested export functionality from both notification bar and header
  • ✅ Verified proper state clearing after successful export
  • ✅ Tested edge cases: tool not exported, API errors, export errors
  • ✅ Confirmed proper state reset when switching between tools

Screenshots

[Yellow notification bar will appear at the top of Prompt Studio when changes are made to projects that are actively deployed]

Checklist

I have read and understood the Contribution Guidelines.

- Add backend API endpoint to check if project is used in deployments
- Implement frontend change tracking for prompt modifications
- Create yellow notification bar component with export action
- Track changes when editing, adding, or deleting prompts
- Clear notification after successful export
- Check usage in API Deployments, ETL/Task Pipelines, and Manual Review

This ensures users are reminded to export their Prompt Studio changes when
the project is actively being used in deployments, preventing confusion
about why changes don't take effect immediately.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 20, 2025

Summary by CodeRabbit

Release Notes

  • New Features
    • Added deployment usage checking to display which deployments are using exported custom tools.
    • Introduced unsaved changes tracking for custom tool prompts and notes.
    • Added export reminder bar that appears when tool changes are detected and the tool is actively used in deployments, encouraging timely exports to keep deployments in sync.
    • Enhanced tool deletion safeguards to prevent accidental removal of in-use tools with detailed usage information.

Walkthrough

Adds backend detection and a GET API to report whether a CustomTool is used by deployments and prevents deletion when used. Frontend tracks unsaved changes, records deployment usage, shows an export reminder bar for deployed tools, and provides an export action that clears the reminder.

Changes

Cohort / File(s) Change Summary
Backend - Tool usage & API
backend/prompt_studio/prompt_studio_core_v2/views.py
Added _check_tool_usage(instance) to inspect related ToolInstance objects and aggregate dependent workflow IDs. Refactored destroy() to use the helper and raise ToolDeleteError when used. Added check_deployment_usage GET endpoint that classifies usage into deployment types and returns { is_used, deployment_types, message }, with error handling and logging.
Frontend - Store (custom tools)
frontend/src/store/custom-tool-store.js
Added state: hasUnsavedChanges, lastExportedAt, deploymentUsageInfo. Added actions: setHasUnsavedChanges, setLastExportedAt, setDeploymentUsageInfo, markChangesAsExported. setCustomTool() resets hasUnsavedChanges and preserves deployment/export metadata when loading a tool.
Frontend - Export reminder component
frontend/src/components/custom-tools/export-reminder-bar/ExportReminderBar.jsx
frontend/src/components/custom-tools/export-reminder-bar/ExportReminderBar.css
New presentational component ExportReminderBar (props: message, onExport, isExporting) rendering a sticky warning banner with icon, message, and primary "Export Now" button. Added CSS for layout, pale warning background, and right-aligned action.
Frontend - Tool IDE integration
frontend/src/components/custom-tools/tool-ide/ToolIde.jsx
Adds deployment usage check logic (checkDeploymentUsage) triggered by unsaved changes, UI state (showExportReminder, isExporting, isCheckingUsageRef), handleExportFromReminder to POST export and clear reminder, and conditional rendering of ExportReminderBar. Now consumes hasUnsavedChanges, deploymentUsageInfo, setDeploymentUsageInfo, markChangesAsExported from the custom tool store.
Frontend - Header integration
frontend/src/components/custom-tools/header/Header.jsx
Uses markChangesAsExported after successful export and updated handleConfirmForceExport dependency array to include handleExport.
Frontend - Unsaved changes tracking
frontend/src/components/custom-tools/document-parser/DocumentParser.jsx
Marks hasUnsavedChanges true via setHasUnsavedChanges(true) when a prompt field is modified after validation.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant ToolIDE as Tool IDE (frontend)
    participant Store as CustomTool Store
    participant API as Backend API
    participant UI as ExportReminderBar

    User->>ToolIDE: Edit prompt
    ToolIDE->>Store: setHasUnsavedChanges(true)
    Store-->>ToolIDE: state updated

    alt ToolIDE detects unsaved changes
        ToolIDE->>API: GET /check_deployment_usage(pk)
        API->>API: _check_tool_usage(instance)
        API-->>ToolIDE: { is_used, deployment_types, message }
        ToolIDE->>Store: setDeploymentUsageInfo(data)
    end

    alt is_used == true and hasUnsavedChanges
        ToolIDE->>UI: render ExportReminderBar(message)
        UI-->>User: show reminder
        User->>UI: click "Export Now"
        UI->>ToolIDE: onExport()
        ToolIDE->>API: POST /export
        API-->>ToolIDE: success
        ToolIDE->>Store: markChangesAsExported()
        Store-->>ToolIDE: hasUnsavedChanges=false
        ToolIDE->>UI: hide reminder
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~45 minutes

  • Inspect backend _check_tool_usage() logic, lazy imports, classification mapping to deployment types, and ToolDeleteError handling.
  • Verify destroy() refactor and API endpoint error handling and message construction.
  • Validate store state transitions (unsaved flag, export timestamp) and usages in consumers.
  • Review ToolIde concurrency guard for usage checks, export flow, and cleanup of reminder state.
  • Check ExportReminderBar accessibility, prop-types, and CSS isolation.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main feature: adding an export reminder for Prompt Studio projects in active use.
Description check ✅ Passed The description comprehensively covers all required sections from the template with detailed information about what was added, why it was needed, how it was implemented, and thorough testing notes.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/UN-1722-export-reminder-prompt-studio

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Contributor

filepath function $$\textcolor{#23d18b}{\tt{passed}}$$ SUBTOTAL
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_logs}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_cleanup}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_cleanup\_skip}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_client\_init}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_image\_exists}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_image}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_container\_run\_config}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_container\_run\_config\_without\_mount}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_run\_container}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_image\_for\_sidecar}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_sidecar\_container}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{TOTAL}}$$ $$\textcolor{#23d18b}{\tt{11}}$$ $$\textcolor{#23d18b}{\tt{11}}$$

@sonarqubecloud
Copy link

Removed 10 trailing commas from 4 files that were causing the Docker
build to fail with Prettier violations:
- DocumentParser.jsx: 3 locations (lines 86, 124, 179)
- Header.jsx: 3 locations (lines 73, 176, 277)
- ToolIde.jsx: 2 locations (lines 97, 223)
- custom-tool-store.js: 2 locations (lines 83, 106)

These changes ensure the code passes ESLint/Prettier checks during
the build process without modifying any functionality.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
backend/prompt_studio/prompt_studio_core_v2/views.py (1)

716-724: Fix the deployment types copy

When more than one deployment type references the project, the reminder still renders “API Deployment or Task Pipeline”, which implies exclusivity even though we know both deployments exist. Please switch the separators to “and” for two (and “, and” for 3+) types so the notification reflects the actual state.

Apply this diff:

-                    types_text = f"{types_list[0]} or {types_list[1]}"
+                    types_text = f"{types_list[0]} and {types_list[1]}"
                 else:
-                    types_text = ", ".join(types_list[:-1]) + f", or {types_list[-1]}"
+                    types_text = ", ".join(types_list[:-1]) + f", and {types_list[-1]}"
frontend/src/components/custom-tools/tool-ide/ToolIde.jsx (1)

124-126: Consider simplifying useEffect dependencies.

The dependencies hasUnsavedChanges and details?.tool_id are redundant since they're already captured in the checkDeploymentUsage callback's dependency array. The effect only needs to depend on the callback itself.

Apply this diff:

   useEffect(() => {
     checkDeploymentUsage();
-  }, [checkDeploymentUsage, hasUnsavedChanges, details?.tool_id]);
+  }, [checkDeploymentUsage]);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to Reviews > Disable Cache setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between d40ea54 and afde3a8.

📒 Files selected for processing (7)
  • backend/prompt_studio/prompt_studio_core_v2/views.py (2 hunks)
  • frontend/src/components/custom-tools/document-parser/DocumentParser.jsx (1 hunks)
  • frontend/src/components/custom-tools/export-reminder-bar/ExportReminderBar.css (1 hunks)
  • frontend/src/components/custom-tools/export-reminder-bar/ExportReminderBar.jsx (1 hunks)
  • frontend/src/components/custom-tools/header/Header.jsx (3 hunks)
  • frontend/src/components/custom-tools/tool-ide/ToolIde.jsx (6 hunks)
  • frontend/src/store/custom-tool-store.js (5 hunks)
🧰 Additional context used
🪛 Ruff (0.14.4)
backend/prompt_studio/prompt_studio_core_v2/views.py

155-158: Avoid specifying long messages outside the exception class

(TRY003)


655-655: Unused method argument: request

(ARG002)


733-733: Do not catch blind exception: Exception

(BLE001)


734-734: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (4)
frontend/src/components/custom-tools/export-reminder-bar/ExportReminderBar.jsx (1)

1-43: LGTM! Clean and well-structured component.

The component is simple, follows React best practices, includes proper PropTypes validation, and appropriately uses Ant Design components. The implementation correctly handles the loading state and provides a clear user action.

frontend/src/components/custom-tools/tool-ide/ToolIde.jsx (3)

2-2: LGTM! Appropriate imports and state setup.

The new imports and state variables are properly introduced to support the export reminder feature. The integration with the custom tool store is clean.

Also applies to: 13-13, 51-54, 65-67


129-178: LGTM! Well-structured export handler.

The handleExportFromReminder function properly manages loading state, handles errors gracefully, updates the store, and includes appropriate PostHog tracking with error suppression. The implementation follows best practices.


289-295: LGTM! Correct conditional rendering.

The ExportReminderBar is properly displayed only when both showExportReminder is true and a deployment usage message exists. The component receives all required props correctly.

…ck pattern

Fixed two major issues identified by CodeRabbit review:

1. Fixed state carryover bug in custom-tool-store.js
   - When switching tools, deploymentUsageInfo and lastExportedAt now
     properly reset to null instead of carrying over from previous tool
   - Prevents incorrect export reminders showing for wrong projects

2. Fixed useCallback pattern issue in ToolIde.jsx
   - Replaced isCheckingUsage state in useCallback deps with useRef
   - Prevents unnecessary callback recreations and potential race conditions
   - Simplified useEffect dependencies to only depend on the callback
   - Removed unused isCheckingUsage state variable

These changes improve code quality and prevent potential bugs without
affecting functionality.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
frontend/src/components/custom-tools/tool-ide/ToolIde.jsx (2)

115-121: Optimize callback dependencies to reduce recreations.

Using details?.tool_id and sessionDetails?.orgId in the dependency array causes the callback to recreate whenever the parent objects change, even if the specific properties remain the same. While not breaking, this triggers unnecessary effect executions.

Destructure the needed values earlier or include the full objects:

 }, [
-  details?.tool_id,
+  details,
   hasUnsavedChanges,
   axiosPrivate,
-  sessionDetails?.orgId,
+  sessionDetails,
   setDeploymentUsageInfo,
 ]);

Then extract the values at function start (as shown in the previous comment).


169-177: Consider stabilizing callback dependencies.

Including entire objects (details, sessionDetails) in the dependency array causes the callback to recreate whenever those objects change reference, even when the specific properties used remain unchanged.

If these objects change frequently, consider extracting only the needed properties:

const { tool_id, created_by, tool_name } = details || {};
const { orgId, csrfToken, userId } = sessionDetails || {};

const handleExportFromReminder = useCallback(async () => {
  // ... use extracted values
}, [
  tool_id,
  created_by,
  tool_name,
  orgId,
  csrfToken,
  userId,
  axiosPrivate,
  setAlertDetails,
  handleException,
  markChangesAsExported,
  setPostHogCustomEvent,
]);

However, if the objects are relatively stable, the current approach is acceptable.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to Reviews > Disable Cache setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between afde3a8 and d151e6d.

📒 Files selected for processing (2)
  • frontend/src/components/custom-tools/tool-ide/ToolIde.jsx (6 hunks)
  • frontend/src/store/custom-tool-store.js (5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • frontend/src/store/custom-tool-store.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (5)
frontend/src/components/custom-tools/tool-ide/ToolIde.jsx (5)

2-2: LGTM!

The new imports are appropriate for the added functionality.

Also applies to: 13-13


51-54: LGTM!

The store properties are correctly destructured and used throughout the component to coordinate the export reminder flow.


65-67: LGTM!

The ref pattern for concurrency control is correctly implemented, addressing the past review comment. The isExporting state appropriately tracks the export operation for UI feedback.


123-125: LGTM!

The effect correctly triggers the deployment usage check when relevant dependencies change, with proper guarding inside the callback to prevent unnecessary API calls.


288-294: LGTM!

The conditional rendering correctly checks both the reminder flag and the presence of a usage message before showing the component. Props are appropriately passed to ExportReminderBar.

Comment on lines +88 to +121
// Check deployment usage when there are unsaved changes
const checkDeploymentUsage = useCallback(async () => {
if (!details?.tool_id || !hasUnsavedChanges || isCheckingUsageRef.current) {
return;
}

isCheckingUsageRef.current = true;
try {
const response = await axiosPrivate.get(
`/api/v1/unstract/${sessionDetails?.orgId}/prompt-studio/${details?.tool_id}/check_deployment_usage/`
);

const usageInfo = response.data;
setDeploymentUsageInfo(usageInfo);

// Show reminder if the tool is being used in deployments
if (usageInfo?.is_used && usageInfo?.message) {
setShowExportReminder(true);
} else {
setShowExportReminder(false);
}
} catch (error) {
console.error("Error checking deployment usage:", error);
setShowExportReminder(false);
} finally {
isCheckingUsageRef.current = false;
}
}, [
details?.tool_id,
hasUnsavedChanges,
axiosPrivate,
sessionDetails?.orgId,
setDeploymentUsageInfo,
]);
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Race condition: stale hasUnsavedChanges during API call.

If a user exports (from header or reminder bar) while the deployment-usage check is in-flight, the check completes and may set showExportReminder(true) even though hasUnsavedChanges is now false (after markChangesAsExported was called). The guard on line 90 only checks the flag at call time, not when the response arrives.

Consider capturing the current value in a local variable and verifying it hasn't changed:

 const checkDeploymentUsage = useCallback(async () => {
-  if (!details?.tool_id || !hasUnsavedChanges || isCheckingUsageRef.current) {
+  const toolId = details?.tool_id;
+  const hasChanges = hasUnsavedChanges;
+  if (!toolId || !hasChanges || isCheckingUsageRef.current) {
     return;
   }

   isCheckingUsageRef.current = true;
   try {
     const response = await axiosPrivate.get(
-      `/api/v1/unstract/${sessionDetails?.orgId}/prompt-studio/${details?.tool_id}/check_deployment_usage/`
+      `/api/v1/unstract/${sessionDetails?.orgId}/prompt-studio/${toolId}/check_deployment_usage/`
     );

     const usageInfo = response.data;
     setDeploymentUsageInfo(usageInfo);

     // Show reminder if the tool is being used in deployments
-    if (usageInfo?.is_used && usageInfo?.message) {
+    if (usageInfo?.is_used && usageInfo?.message && hasChanges) {
       setShowExportReminder(true);
     } else {
       setShowExportReminder(false);
     }
   } catch (error) {
     console.error("Error checking deployment usage:", error);
     setShowExportReminder(false);
   } finally {
     isCheckingUsageRef.current = false;
   }
 }, [
-  details?.tool_id,
+  details,
   hasUnsavedChanges,
   axiosPrivate,
-  sessionDetails?.orgId,
+  sessionDetails,
   setDeploymentUsageInfo,
 ]);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Check deployment usage when there are unsaved changes
const checkDeploymentUsage = useCallback(async () => {
if (!details?.tool_id || !hasUnsavedChanges || isCheckingUsageRef.current) {
return;
}
isCheckingUsageRef.current = true;
try {
const response = await axiosPrivate.get(
`/api/v1/unstract/${sessionDetails?.orgId}/prompt-studio/${details?.tool_id}/check_deployment_usage/`
);
const usageInfo = response.data;
setDeploymentUsageInfo(usageInfo);
// Show reminder if the tool is being used in deployments
if (usageInfo?.is_used && usageInfo?.message) {
setShowExportReminder(true);
} else {
setShowExportReminder(false);
}
} catch (error) {
console.error("Error checking deployment usage:", error);
setShowExportReminder(false);
} finally {
isCheckingUsageRef.current = false;
}
}, [
details?.tool_id,
hasUnsavedChanges,
axiosPrivate,
sessionDetails?.orgId,
setDeploymentUsageInfo,
]);
// Check deployment usage when there are unsaved changes
const checkDeploymentUsage = useCallback(async () => {
const toolId = details?.tool_id;
const hasChanges = hasUnsavedChanges;
if (!toolId || !hasChanges || isCheckingUsageRef.current) {
return;
}
isCheckingUsageRef.current = true;
try {
const response = await axiosPrivate.get(
`/api/v1/unstract/${sessionDetails?.orgId}/prompt-studio/${toolId}/check_deployment_usage/`
);
const usageInfo = response.data;
setDeploymentUsageInfo(usageInfo);
// Show reminder if the tool is being used in deployments
if (usageInfo?.is_used && usageInfo?.message && hasChanges) {
setShowExportReminder(true);
} else {
setShowExportReminder(false);
}
} catch (error) {
console.error("Error checking deployment usage:", error);
setShowExportReminder(false);
} finally {
isCheckingUsageRef.current = false;
}
}, [
details,
hasUnsavedChanges,
axiosPrivate,
sessionDetails,
setDeploymentUsageInfo,
]);
🤖 Prompt for AI Agents
frontend/src/components/custom-tools/tool-ide/ToolIde.jsx around lines 88-121,
the async checkDeploymentUsage function can set state based on a stale
hasUnsavedChanges value because the guard only runs before the await; capture
the current hasUnsavedChanges into a local const (e.g., const currentHasUnsaved
= hasUnsavedChanges) before the network call and, after the response arrives,
verify currentHasUnsaved is still true before calling setDeploymentUsageInfo and
setShowExportReminder (otherwise skip updating reminder/state); keep
isCheckingUsageRef handling as-is and ensure finally still clears the ref.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@athul-rs Thank you for addressing the race condition! 👍

Copy link
Contributor

@chandrasekharan-zipstack chandrasekharan-zipstack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@athul-rs can you also post a screenshot for the FE changes?

Comment on lines +674 to +677
# Import necessary models
from api_v2.models import APIDeployment
from pipeline_v2.models import Pipeline
from workflow_manager.endpoint_v2.models import WorkflowEndpoint
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@athul-rs move these imports to the top of the file

if APIDeployment.objects.filter(
workflow_id__in=workflow_ids, is_active=True
).exists():
deployment_types.add("API Deployment")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maintain this deployment_type as an enum


# Check API Deployments
if APIDeployment.objects.filter(
workflow_id__in=workflow_ids, is_active=True
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@athul-rs do you think we need to filter for active pipelines / deployments alone? This might cause an issue / drift in behaviour when they enable those pipelines/ deployments again

) -> Response:
instance: CustomTool = self.get_object()
# Checks if tool is exported
def _check_tool_usage(self, instance: CustomTool) -> tuple[bool, set]:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: Consider making the function name explicit itself. _check_prompt_studio_usage_in_wf / _check_tool_usage_in_wf

Comment on lines +735 to +738
return Response(
{"error": "Failed to check deployment usage"},
status=status.HTTP_500_INTERNAL_SERVER_ERROR,
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@athul-rs in case of an error, please raise an exception instead

else:
types_text = ", ".join(types_list[:-1]) + f", or {types_list[-1]}"

deployment_info["message"] = (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For better UX, I think we should attempt to display the actual workflows / deployments that its used in as clickable links which will take the user there. Not sure on the lift here but do you think you can give this a shot with any agent @athul-rs ?

@sonarqubecloud
Copy link

@github-actions
Copy link
Contributor

Test Results

Summary
  • Runner Tests: 11 passed, 0 failed (11 total)
  • SDK1 Tests: 66 passed, 0 failed (66 total)

Runner Tests - Full Report
filepath function $$\textcolor{#23d18b}{\tt{passed}}$$ SUBTOTAL
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_logs}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_cleanup}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_cleanup\_skip}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_client\_init}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_image\_exists}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_image}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_container\_run\_config}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_container\_run\_config\_without\_mount}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_run\_container}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_image\_for\_sidecar}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_sidecar\_container}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{TOTAL}}$$ $$\textcolor{#23d18b}{\tt{11}}$$ $$\textcolor{#23d18b}{\tt{11}}$$
SDK1 Tests - Full Report
filepath function $$\textcolor{#23d18b}{\tt{passed}}$$ SUBTOTAL
$$\textcolor{#23d18b}{\tt{tests/test\_platform.py}}$$ $$\textcolor{#23d18b}{\tt{TestPlatformHelperRetry.test\_success\_on\_first\_attempt}}$$ $$\textcolor{#23d18b}{\tt{2}}$$ $$\textcolor{#23d18b}{\tt{2}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_platform.py}}$$ $$\textcolor{#23d18b}{\tt{TestPlatformHelperRetry.test\_retry\_on\_connection\_error}}$$ $$\textcolor{#23d18b}{\tt{2}}$$ $$\textcolor{#23d18b}{\tt{2}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_platform.py}}$$ $$\textcolor{#23d18b}{\tt{TestPlatformHelperRetry.test\_non\_retryable\_http\_error}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_platform.py}}$$ $$\textcolor{#23d18b}{\tt{TestPlatformHelperRetry.test\_retryable\_http\_errors}}$$ $$\textcolor{#23d18b}{\tt{3}}$$ $$\textcolor{#23d18b}{\tt{3}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_platform.py}}$$ $$\textcolor{#23d18b}{\tt{TestPlatformHelperRetry.test\_post\_method\_retry}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_platform.py}}$$ $$\textcolor{#23d18b}{\tt{TestPlatformHelperRetry.test\_retry\_logging}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_prompt.py}}$$ $$\textcolor{#23d18b}{\tt{TestPromptToolRetry.test\_success\_on\_first\_attempt}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_prompt.py}}$$ $$\textcolor{#23d18b}{\tt{TestPromptToolRetry.test\_retry\_on\_errors}}$$ $$\textcolor{#23d18b}{\tt{2}}$$ $$\textcolor{#23d18b}{\tt{2}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_prompt.py}}$$ $$\textcolor{#23d18b}{\tt{TestPromptToolRetry.test\_wrapper\_methods\_retry}}$$ $$\textcolor{#23d18b}{\tt{4}}$$ $$\textcolor{#23d18b}{\tt{4}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestIsRetryableError.test\_connection\_error\_is\_retryable}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestIsRetryableError.test\_timeout\_is\_retryable}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestIsRetryableError.test\_http\_error\_retryable\_status\_codes}}$$ $$\textcolor{#23d18b}{\tt{3}}$$ $$\textcolor{#23d18b}{\tt{3}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestIsRetryableError.test\_http\_error\_non\_retryable\_status\_codes}}$$ $$\textcolor{#23d18b}{\tt{5}}$$ $$\textcolor{#23d18b}{\tt{5}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestIsRetryableError.test\_http\_error\_without\_response}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestIsRetryableError.test\_os\_error\_retryable\_errno}}$$ $$\textcolor{#23d18b}{\tt{5}}$$ $$\textcolor{#23d18b}{\tt{5}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestIsRetryableError.test\_os\_error\_non\_retryable\_errno}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestIsRetryableError.test\_other\_exception\_not\_retryable}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestCalculateDelay.test\_exponential\_backoff\_without\_jitter}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestCalculateDelay.test\_exponential\_backoff\_with\_jitter}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestCalculateDelay.test\_max\_delay\_cap}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestCalculateDelay.test\_max\_delay\_cap\_with\_jitter}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestRetryWithExponentialBackoff.test\_successful\_call\_first\_attempt}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestRetryWithExponentialBackoff.test\_retry\_after\_transient\_failure}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestRetryWithExponentialBackoff.test\_max\_retries\_exceeded}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestRetryWithExponentialBackoff.test\_max\_time\_exceeded}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestRetryWithExponentialBackoff.test\_retry\_with\_custom\_predicate}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestRetryWithExponentialBackoff.test\_no\_retry\_with\_predicate\_false}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestRetryWithExponentialBackoff.test\_exception\_not\_in\_tuple\_not\_retried}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestRetryWithExponentialBackoff.test\_delay\_would\_exceed\_max\_time}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestCreateRetryDecorator.test\_default\_configuration}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestCreateRetryDecorator.test\_environment\_variable\_configuration}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestCreateRetryDecorator.test\_invalid\_max\_retries}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestCreateRetryDecorator.test\_invalid\_max\_time}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestCreateRetryDecorator.test\_invalid\_base\_delay}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestCreateRetryDecorator.test\_invalid\_multiplier}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestCreateRetryDecorator.test\_jitter\_values}}$$ $$\textcolor{#23d18b}{\tt{2}}$$ $$\textcolor{#23d18b}{\tt{2}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestCreateRetryDecorator.test\_custom\_exceptions\_only}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestCreateRetryDecorator.test\_custom\_predicate\_only}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestCreateRetryDecorator.test\_both\_exceptions\_and\_predicate}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestCreateRetryDecorator.test\_exceptions\_match\_but\_predicate\_false}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestPreconfiguredDecorators.test\_retry\_platform\_service\_call\_exists}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestPreconfiguredDecorators.test\_retry\_prompt\_service\_call\_exists}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestPreconfiguredDecorators.test\_platform\_service\_decorator\_retries\_on\_connection\_error}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestPreconfiguredDecorators.test\_prompt\_service\_decorator\_retries\_on\_timeout}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestRetryLogging.test\_warning\_logged\_on\_retry}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestRetryLogging.test\_info\_logged\_on\_success\_after\_retry}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/utils/test\_retry\_utils.py}}$$ $$\textcolor{#23d18b}{\tt{TestRetryLogging.test\_exception\_logged\_on\_giving\_up}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{TOTAL}}$$ $$\textcolor{#23d18b}{\tt{66}}$$ $$\textcolor{#23d18b}{\tt{66}}$$

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (3)
backend/prompt_studio/prompt_studio_core_v2/views.py (3)

127-144: Consider more explicit function name.

The current name _check_tool_usage is acceptable, but a more explicit name like _check_tool_usage_in_workflows would better convey that this checks workflow-level usage specifically.


714-717: Move imports to the top of the file.

Lazy imports can make code harder to follow and may hide circular dependency issues. Consider moving these imports to the top of the file.


725-725: Maintain deployment_type as an enum.

The deployment type strings should be defined as constants or an enum for better maintainability and to avoid string typos.

🧹 Nitpick comments (2)
backend/prompt_studio/prompt_studio_core_v2/views.py (2)

157-160: Extract error message to a constant.

The error message is hardcoded in the raise statement. Consider defining it as a constant in the exceptions or constants module for better maintainability and consistency.

Example:

# In prompt_studio_core_v2/constants.py or exceptions.py
TOOL_IN_USE_DELETE_ERROR = "Failed to delete tool, its used in other workflows. Delete its usages first"

Then use it:

-            raise ToolDeleteError(
-                "Failed to delete tool, its used in other workflows. "
-                "Delete its usages first"
-            )
+            raise ToolDeleteError(TOOL_IN_USE_DELETE_ERROR)

773-778: Use logger.exception for exception logging.

When logging in an exception handler, use logger.exception() instead of logger.error() to automatically include the stack trace, which aids debugging.

Apply this diff:

         except Exception as e:
-            logger.error(f"Error checking deployment usage for tool {pk}: {e}")
+            logger.exception(f"Error checking deployment usage for tool {pk}: {e}")
             return Response(
                 {"error": "Failed to check deployment usage"},
                 status=status.HTTP_500_INTERNAL_SERVER_ERROR,
             )
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to Reviews > Disable Cache setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between d151e6d and b87a9db.

📒 Files selected for processing (1)
  • backend/prompt_studio/prompt_studio_core_v2/views.py (2 hunks)
🧰 Additional context used
🪛 Ruff (0.14.4)
backend/prompt_studio/prompt_studio_core_v2/views.py

157-160: Avoid specifying long messages outside the exception class

(TRY003)


695-695: Unused method argument: request

(ARG002)


773-773: Do not catch blind exception: Exception

(BLE001)


774-774: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

🔇 Additional comments (1)
backend/prompt_studio/prompt_studio_core_v2/views.py (1)

722-751: Good: Active deployment filtering implemented.

The code correctly filters for is_active=True when checking API Deployments and Pipelines, ensuring only currently active deployments are considered. This addresses the previous concern about potential drift in behavior when deployments are enabled/disabled.

Comment on lines +734 to +738
for pipeline_type in pipelines:
if pipeline_type == "ETL":
deployment_types.add("ETL Pipeline")
elif pipeline_type == "TASK":
deployment_types.add("Task Pipeline")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Use Pipeline model constants for pipeline types.

The string literals "ETL" and "TASK" should use the Pipeline model's enum or constants to avoid magic strings and ensure consistency with the model definition.

Verify that the Pipeline model defines these types and update the code accordingly:


🏁 Script executed:

#!/bin/bash
# Search for pipeline_type field definition in Pipeline model
rg -nP -A5 'class Pipeline.*:' --type=py | rg -A10 'pipeline_type'

Length of output: 1502


I need to search for the Pipeline model definition more specifically to verify if it defines constants for pipeline types.


Wait, let me get the actual Pipeline model:

---

🏁 Script executed:

```shell
#!/bin/bash
# Find Pipeline model definition and check for pipeline_type field definition
fd -e py -path "*/models*" --exec rg -l "class Pipeline" {} \;

Length of output: 168


🏁 Script executed:

#!/bin/bash
# Find and display Pipeline model class with pipeline_type field
rg -nP 'class Pipeline\b' --type=py -A 50 | head -100

Length of output: 3598


🏁 Script executed:

#!/bin/bash
# Check the exact code at the flagged lines and imports
rg -nP -B5 -A10 'for pipeline_type in pipelines:' backend/prompt_studio/prompt_studio_core_v2/views.py

Length of output: 818


🏁 Script executed:

#!/bin/bash
# Check imports in the views.py file
head -50 backend/prompt_studio/prompt_studio_core_v2/views.py | rg -n 'import|from'

Length of output: 1911


🏁 Script executed:

#!/bin/bash
# Check if Pipeline is imported anywhere in the file
rg -n 'Pipeline' backend/prompt_studio/prompt_studio_core_v2/views.py | head -20

Length of output: 379


🏁 Script executed:

#!/bin/bash
# Check all imports in the file more comprehensively
grep -n "^from\|^import" backend/prompt_studio/prompt_studio_core_v2/views.py | head -80

Length of output: 2514


Replace magic strings with Pipeline model constants.

At lines 735 and 737, replace "ETL" with Pipeline.PipelineType.ETL and "TASK" with Pipeline.PipelineType.TASK. The Pipeline model defines these as enum constants at backend/pipeline_v2/models.py:38-42, and Pipeline is already imported at line 716 within this function's scope.

🤖 Prompt for AI Agents
In backend/prompt_studio/prompt_studio_core_v2/views.py around lines 734 to 738,
replace the magic string checks for pipeline types with the Pipeline model enum
constants: change comparisons from "ETL" to Pipeline.PipelineType.ETL and from
"TASK" to Pipeline.PipelineType.TASK (Pipeline is already imported at line 716).
Update the branches so deployment_types.add(...) executes as before but uses
these enum values for comparison to avoid hard-coded strings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants