Skip to content

Conversation

@hari-kuriakose
Copy link
Contributor

@hari-kuriakose hari-kuriakose commented Jan 9, 2026

What

  • Add dynamic plugin loading support to OSS codebase
  • Enable enterprise components to be loaded at runtime without modifying tracked files

Why

  • Enterprise code was overwriting git-tracked OSS files causing dirty git state
  • Need clean separation between OSS and enterprise codebases
  • OSS should work independently without enterprise components

How

  • unstract_migrations.py: Uses try/except ImportError to load from pluggable_apps.migrations_ext
  • api_hub_usage_utils.py: Uses try/except ImportError to load from plugins.verticals_usage
  • utils.py: Uses try/except ImportError to load from pluggable_apps.manual_review_v2 and plugins.workflow_manager.workflow_v2.rule_engine
  • backend.Dockerfile: Conditional install of requirements.txt if present

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

  • No. The changes add optional plugin loading that gracefully falls back to default behavior when plugins are not present. Existing OSS functionality is preserved.

Database Migrations

  • None

Env Config

  • None

Relevant Docs

  • None

Related Issues or PRs

  • None

Dependencies Versions

  • None

Notes on Testing

  • OSS build: Verify app starts and works without enterprise plugins
  • Enterprise build: Verify plugins are loaded and function correctly

Screenshots

  • N/A (backend changes only)

Checklist

I have read and understood the Contribution Guidelines.

🤖 Generated with Claude Code

## What

- Add dynamic plugin loading support to OSS codebase
- Enable enterprise components to be loaded at runtime without modifying tracked files

## Why

- Enterprise code was overwriting git-tracked OSS files causing dirty git state
- Need clean separation between OSS and enterprise codebases
- OSS should work independently without enterprise components

## How

- `unstract_migrations.py`: Uses try/except ImportError to load from `pluggable_apps.migrations_ext`
- `api_hub_usage_utils.py`: Uses try/except ImportError to load from `plugins.verticals_usage`
- `utils.py`: Uses try/except ImportError to load from `pluggable_apps.manual_review_v2` and `plugins.workflow_manager.workflow_v2.rule_engine`
- `backend.Dockerfile`: Conditional install of `requirements.txt` if present

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

- No. The changes add optional plugin loading that gracefully falls back to default behavior when plugins are not present. Existing OSS functionality is preserved.

## Database Migrations

- None

## Env Config

- None

## Relevant Docs

- None

## Related Issues or PRs

- None

## Dependencies Versions

- None

## Notes on Testing

- OSS build: Verify app starts and works without enterprise plugins
- Enterprise build: Verify plugins are loaded and function correctly

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

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 9, 2026

Summary by CodeRabbit

  • New Features

    • Implemented manual review functionality for workflows with file selection and rule validation.
    • Enhanced API hub usage tracking for improved analytics and monitoring.
    • Added extensible migration support for better version upgrade capabilities.
  • Chores

    • Updated Docker build process to automatically install additional Python dependencies when available.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

Adds an extensible migrations class, replaces OSS no-op API Hub usage functions with plugin-driven implementations, implements workflow utility functions that delegate to pluggable helpers, and conditionally installs extra Python deps in the backend Docker production stage. (≤50 words)

Changes

Cohort / File(s) Summary
Migration System
backend/migrating/v2/unstract_migrations.py
Adds UnstractMigration (extends MigrationQuery) and implements get_public_schema_migrations and get_organization_migrations. Both attempt to load extended migrations from pluggable_apps.migrations_ext.migrations and fall back to core migrations on ImportError. Adds logger and docstrings.
Workflow API Hub Usage
backend/plugins/workflow_manager/workflow_v2/api_hub_usage_utils.py
Replaces previous OSS no-ops with plugin-driven logic using verticals_usage plugin. Methods now build headers_cache and usage_tracker, extract/cache headers, store usage, and return success booleans; exceptions are logged and return False. Docstrings updated.
Workflow Utilities
backend/plugins/workflow_manager/workflow_v2/utils.py
Implements deterministic manual-review sampling (_mrq_files), selection helpers (get_q_no_list), file destination updates (add_file_destination_filehash), and rule validation (validate_rule_engine, validate_db_rule). Methods delegate to pluggable_apps helpers where available and handle ImportError fallbacks. get_hitl_ttl_seconds returns `int
Docker Build Configuration
docker/dockerfiles/backend.Dockerfile
Adds a conditional RUN in the final production stage to install additional Python dependencies from requirements.txt if the file exists (uv pip install -r requirements.txt).

Sequence Diagram(s)

sequenceDiagram
    participant APIHubUtil as APIHubUsageUtil
    participant Plugin as verticals_usage plugin
    participant HeadersCache as headers_cache_class
    participant UsageTracker as usage_tracker

    APIHubUtil->>Plugin: obtain plugin (verticals_usage)
    APIHubUtil->>Plugin: create headers_cache and usage_tracker
    APIHubUtil->>Plugin: call extract_api_hub_headers(request)
    Plugin-->>APIHubUtil: headers (or None / raises)
    alt headers present
        APIHubUtil->>HeadersCache: store_headers(headers)
        HeadersCache-->>APIHubUtil: success/failure
        APIHubUtil->>UsageTracker: store usage(headers, metadata)
        UsageTracker-->>APIHubUtil: success/failure
        APIHubUtil-->>Caller: return True
    else no headers or error
        APIHubUtil-->>Caller: return False
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title accurately summarizes the primary change: adding dynamic plugin loading for enterprise components, which is the core objective of this refactoring.
Description check ✅ Passed The pull request description is comprehensive and addresses all critical sections of the template with appropriate detail and clarity.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
backend/plugins/workflow_manager/workflow_v2/api_hub_usage_utils.py (1)

79-106: Remove unused ttl_seconds parameter or pass it to store_headers.

The ttl_seconds parameter (line 83) is not passed to store_headers (line 103). Either remove the parameter from the method signature or pass it to the underlying implementation. Additionally, use logger.exception instead of logger.error to capture stack traces.

🔧 Proposed fix
         try:
-            return api_hub_headers_cache.store_headers(execution_id, headers)
+            return api_hub_headers_cache.store_headers(execution_id, headers, ttl_seconds)
         except Exception as e:
-            logger.error(f"Error caching API hub headers: {e}")
+            logger.exception(f"Error caching API hub headers: {e}")
             return False
🤖 Fix all issues with AI agents
In @backend/plugins/workflow_manager/workflow_v2/utils.py:
- Around line 38-46: Remove the unused import by deleting
get_db_rules_by_workflow_id from the try block inside _mrq_files; keep the
random import intact and ensure the try/except still only catches ImportError
for missing dependencies, so the function uses random.sample as before without
importing the unused helper.
- Around line 93-95: The FileHash DTO's file_destination is typed as
tuple[str,str] | None but code assigns a plain string
(WorkflowEndpoint.ConnectionType.MANUALREVIEW) to file_hash.file_destination;
update the code to match the DTO by assigning a tuple (e.g.,
(WorkflowEndpoint.ConnectionType.MANUALREVIEW, "<optional-second>") or a
meaningful second element) wherever file_destination is set, or change the
FileHash type to str | None if the design intends a single string; ensure
consistency by updating all initializations and comparisons that currently use
empty strings "" and references to file_hash.file_destination and
WorkflowEndpoint.ConnectionType.MANUALREVIEW accordingly.
🧹 Nitpick comments (2)
backend/plugins/workflow_manager/workflow_v2/api_hub_usage_utils.py (2)

50-54: Use logger.exception for better error diagnostics.

When catching exceptions during usage tracking, logger.exception automatically includes the stack trace, which aids debugging in production environments.

♻️ Proposed fix
         except Exception as e:
-            logger.error(
+            logger.exception(
                 f"Failed to track API hub usage for execution {workflow_execution_id}: {e}"
             )
             return False

75-77: Use logger.exception for better error diagnostics.

♻️ Proposed fix
         except Exception as e:
-            logger.error(f"Error extracting API hub headers: {e}")
+            logger.exception(f"Error extracting API hub headers: {e}")
             return None
📜 Review details

Configuration used: Organization 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 1004a55 and 92b8575.

📒 Files selected for processing (4)
  • backend/migrating/v2/unstract_migrations.py
  • backend/plugins/workflow_manager/workflow_v2/api_hub_usage_utils.py
  • backend/plugins/workflow_manager/workflow_v2/utils.py
  • docker/dockerfiles/backend.Dockerfile
🧰 Additional context used
🧬 Code graph analysis (3)
backend/plugins/workflow_manager/workflow_v2/api_hub_usage_utils.py (1)
backend/workflow_manager/workflow_v2/models/execution.py (1)
  • organization_id (266-274)
backend/migrating/v2/unstract_migrations.py (1)
backend/migrating/v2/query.py (3)
  • MigrationQuery (1-765)
  • get_public_schema_migrations (9-252)
  • get_organization_migrations (254-765)
backend/plugins/workflow_manager/workflow_v2/utils.py (4)
backend/workflow_manager/endpoint_v2/dto.py (1)
  • FileHash (11-54)
workers/shared/clients/manual_review_stub.py (1)
  • get_q_no_list (52-72)
workers/shared/utils/manual_review_factory.py (9)
  • get_q_no_list (121-123)
  • get_q_no_list (313-314)
  • get_q_no_list (402-403)
  • add_file_destination_filehash (116-118)
  • add_file_destination_filehash (309-310)
  • add_file_destination_filehash (398-399)
  • get_hitl_ttl_seconds (150-152)
  • get_hitl_ttl_seconds (348-349)
  • get_hitl_ttl_seconds (456-457)
backend/workflow_manager/endpoint_v2/models.py (1)
  • WorkflowEndpoint (17-66)
🪛 Ruff (0.14.10)
backend/plugins/workflow_manager/workflow_v2/api_hub_usage_utils.py

48-48: Consider moving this statement to an else block

(TRY300)


50-50: Do not catch blind exception: Exception

(BLE001)


51-53: Use logging.exception instead of logging.error

Replace with exception

(TRY400)


75-75: Do not catch blind exception: Exception

(BLE001)


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

Replace with exception

(TRY400)


104-104: Do not catch blind exception: Exception

(BLE001)


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

Replace with exception

(TRY400)

backend/plugins/workflow_manager/workflow_v2/utils.py

156-156: Consider moving this statement to an else block

(TRY300)

⏰ 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 (8)
docker/dockerfiles/backend.Dockerfile (1)

76-79: LGTM!

The conditional install pattern properly handles the optional requirements.txt for enterprise dependencies. The file existence check prevents build failures when the file is absent in the OSS build.

backend/migrating/v2/unstract_migrations.py (2)

22-39: LGTM!

The extension pattern correctly loads additional migrations when available while preserving core migration behavior. The fallback to core-only migrations ensures OSS compatibility.


41-66: LGTM!

The organization migrations extension follows the same pattern and correctly passes all required parameters to the extended migrations function.

backend/plugins/workflow_manager/workflow_v2/utils.py (5)

48-67: LGTM!

The method correctly retrieves DB rules and delegates to _mrq_files when a valid percentage is configured. The ImportError fallback ensures OSS compatibility.


97-126: LGTM!

The rule engine validation correctly delegates to the plugin when available and returns False as a safe fallback for OSS builds.


128-140: LGTM!

The backward-compatible wrapper maintains existing API while deprecating in favor of validate_rule_engine.


142-160: LGTM!

The API rules check correctly queries the DB rules configuration. The naming suggests DB rules may contain API-specific configuration (rule_string).


162-181: LGTM!

The HITL TTL retrieval follows the established plugin loading pattern with a safe None fallback for unlimited TTL in OSS.

hari-kuriakose and others added 3 commits January 13, 2026 19:36
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Removed redundant import of random and exception handling for manual_review_v2.

Signed-off-by: Hari John Kuriakose <hari@zipstack.com>
@sonarqubecloud
Copy link

@hari-kuriakose hari-kuriakose marked this pull request as ready for review January 13, 2026 14:27
@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: 3

🤖 Fix all issues with AI agents
In @backend/plugins/workflow_manager/workflow_v2/api_hub_usage_utils.py:
- Around line 98-103: The ttl_seconds parameter accepted by the function is
never forwarded to the cache call; update the call to
headers_cache.store_headers(execution_id, headers, ttl_seconds) so the
underlying headers_cache_class implementation receives the TTL (or, if TTL is
intentionally unused, remove ttl_seconds from the function signature and
docstring). Ensure you modify the invocation using headers_cache_class and the
store_headers method to include ttl_seconds (or remove parameter and update docs
accordingly).

In @backend/plugins/workflow_manager/workflow_v2/utils.py:
- Around line 39-40: The current logic can raise ValueError when n == 0 or
num_to_select > n; update the selection logic to first handle n == 0 by
returning an empty set, clamp percentage to the 0–100 range, compute
num_to_select = max(0, min(n, int(n * (percentage / 100)))), and then call
random.sample on range(1, n + 1) only when num_to_select > 0; reference the
variables num_to_select, n, percentage and the random.sample call in the return
expression to locate and modify the code.
- Around line 87-89: The FileHash.file_destination field is currently typed as
tuple[str, str] | None in the DTO but is assigned string values like
WorkflowEndpoint.ConnectionType.MANUALREVIEW elsewhere; update the type
annotation of FileHash.file_destination in the FileHash definition (in
endpoint_v2/dto.py) from tuple[str, str] | None to str | None, and run a quick
grep for FileHash.file_destination uses to ensure no code expects a tuple
(adjust any typed usages/imports accordingly) so that assignments like
WorkflowEndpoint.ConnectionType.MANUALREVIEW are type-safe.
🧹 Nitpick comments (4)
backend/plugins/workflow_manager/workflow_v2/utils.py (1)

146-154: Consider restructuring the try/except for clarity.

The static analysis tool (TRY300) suggests moving the success return to an else block. This is a minor style preference that separates the "happy path" from exception handling.

Optional refactor
         try:
             from pluggable_apps.manual_review_v2.helper import get_db_rules_by_workflow_id
-
-            db_rule = get_db_rules_by_workflow_id(workflow=workflow)
-            return db_rule is not None and db_rule.rule_string is not None
         except ImportError:
-            pass
-
-        return False
+            return False
+        else:
+            db_rule = get_db_rules_by_workflow_id(workflow=workflow)
+            return db_rule is not None and db_rule.rule_string is not None
backend/plugins/workflow_manager/workflow_v2/api_hub_usage_utils.py (3)

51-55: Use logger.exception to preserve stack trace.

When catching exceptions, logger.exception automatically includes the traceback, which aids debugging plugin issues.

Proposed fix
         except Exception as e:
-            logger.error(
+            logger.exception(
                 f"Failed to track API hub usage for execution {workflow_execution_id}: {e}"
             )
             return False

74-76: Use logger.exception for better debugging.

Same recommendation as above—use logger.exception to include the traceback.

Proposed fix
         except Exception as e:
-            logger.error(f"Error extracting API hub headers: {e}")
+            logger.exception(f"Error extracting API hub headers: {e}")
             return None

101-103: Use logger.exception for better debugging.

Same recommendation—use logger.exception to preserve the traceback.

Proposed fix
         except Exception as e:
-            logger.error(f"Error caching API hub headers: {e}")
+            logger.exception(f"Error caching API hub headers: {e}")
             return False
📜 Review details

Configuration used: Organization 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 92b8575 and 16e04a7.

📒 Files selected for processing (2)
  • backend/plugins/workflow_manager/workflow_v2/api_hub_usage_utils.py
  • backend/plugins/workflow_manager/workflow_v2/utils.py
🧰 Additional context used
🧬 Code graph analysis (2)
backend/plugins/workflow_manager/workflow_v2/utils.py (4)
backend/workflow_manager/endpoint_v2/dto.py (1)
  • FileHash (11-54)
workers/shared/clients/manual_review_stub.py (1)
  • get_q_no_list (52-72)
workers/shared/utils/manual_review_factory.py (6)
  • get_q_no_list (121-123)
  • get_q_no_list (313-314)
  • get_q_no_list (402-403)
  • get_hitl_ttl_seconds (150-152)
  • get_hitl_ttl_seconds (348-349)
  • get_hitl_ttl_seconds (456-457)
backend/workflow_manager/endpoint_v2/models.py (1)
  • WorkflowEndpoint (17-66)
backend/plugins/workflow_manager/workflow_v2/api_hub_usage_utils.py (1)
backend/workflow_manager/workflow_v2/models/execution.py (1)
  • organization_id (266-274)
🪛 Ruff (0.14.10)
backend/plugins/workflow_manager/workflow_v2/utils.py

150-150: Consider moving this statement to an else block

(TRY300)

backend/plugins/workflow_manager/workflow_v2/api_hub_usage_utils.py

49-49: Consider moving this statement to an else block

(TRY300)


51-51: Do not catch blind exception: Exception

(BLE001)


52-54: Use logging.exception instead of logging.error

Replace with exception

(TRY400)


74-74: Do not catch blind exception: Exception

(BLE001)


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

Replace with exception

(TRY400)


101-101: Do not catch blind exception: Exception

(BLE001)


102-102: 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 (5)
backend/plugins/workflow_manager/workflow_v2/utils.py (4)

1-16: LGTM!

Module docstring clearly describes the plugin-based architecture, and imports are appropriate.


42-61: LGTM!

Clean implementation of optional plugin loading with appropriate fallback to None.


91-120: LGTM!

Clean plugin loading with proper fallback to False when the rule engine is unavailable.


156-175: LGTM!

Consistent plugin loading pattern with appropriate return type.

backend/plugins/workflow_manager/workflow_v2/api_hub_usage_utils.py (1)

1-11: LGTM!

Clean module structure with plugin-based architecture.

Comment on lines +98 to +103
try:
headers_cache = verticals_usage_plugin["headers_cache_class"]()
return headers_cache.store_headers(execution_id, headers)
except Exception as e:
logger.error(f"Error caching API hub headers: {e}")
return False
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cat -n backend/plugins/workflow_manager/workflow_v2/api_hub_usage_utils.py | sed -n '78,103p'

Repository: Zipstack/unstract

Length of output: 1124


🏁 Script executed:

# Find the store_headers method definition to check its signature
rg -A 5 "def store_headers" backend/

Repository: Zipstack/unstract

Length of output: 43


🏁 Script executed:

# Check if ttl_seconds is used anywhere in the file
rg "ttl_seconds" backend/plugins/workflow_manager/workflow_v2/api_hub_usage_utils.py

Repository: Zipstack/unstract

Length of output: 160


🏁 Script executed:

# Search for store_headers method definition without pattern restrictions
rg "def store_headers" --type py -A 3

Repository: Zipstack/unstract

Length of output: 43


🏁 Script executed:

# Find where headers_cache_class is defined or used
rg "headers_cache_class" --type py -B 2 -A 2

Repository: Zipstack/unstract

Length of output: 43


🏁 Script executed:

# Look for the verticals_usage plugin to understand the cache interface
fd -e py | xargs rg "verticals_usage" | head -20

Repository: Zipstack/unstract

Length of output: 190


🏁 Script executed:

# List the repository structure to understand the layout
git ls-files | grep -E "(plugin|verticals)" | head -20

Repository: Zipstack/unstract

Length of output: 992


🏁 Script executed:

# Search for any reference to store_headers in comments or strings
rg "store_headers" --type py -i

Repository: Zipstack/unstract

Length of output: 43


🏁 Script executed:

# Check if there are any type hints or comments about what store_headers expects
rg -B 3 -A 3 "cache\(" backend/plugins/workflow_manager/workflow_v2/api_hub_usage_utils.py

Repository: Zipstack/unstract

Length of output: 43


🏁 Script executed:

# Search for all method definitions in the file to see context
rg -E "^\s+def " backend/plugins/workflow_manager/workflow_v2/api_hub_usage_utils.py -A 1

Repository: Zipstack/unstract

Length of output: 106


🏁 Script executed:

# Check the full file to understand all methods and their parameter handling
cat -n backend/plugins/workflow_manager/workflow_v2/api_hub_usage_utils.py | head -150

Repository: Zipstack/unstract

Length of output: 4192


🏁 Script executed:

# Look for any imports or hints about the cache class
head -50 backend/plugins/workflow_manager/workflow_v2/api_hub_usage_utils.py

Repository: Zipstack/unstract

Length of output: 1668


ttl_seconds parameter is unused and should be passed to the underlying cache.

The ttl_seconds parameter is accepted at line 82 and documented in the docstring, but is never passed to store_headers at line 100. This should either be passed to the cache implementation or removed from the method signature.

Proposed fix
-            return headers_cache.store_headers(execution_id, headers)
+            return headers_cache.store_headers(execution_id, headers, ttl_seconds)
🧰 Tools
🪛 Ruff (0.14.10)

101-101: Do not catch blind exception: Exception

(BLE001)


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

Replace with exception

(TRY400)

🤖 Prompt for AI Agents
In @backend/plugins/workflow_manager/workflow_v2/api_hub_usage_utils.py around
lines 98 - 103, The ttl_seconds parameter accepted by the function is never
forwarded to the cache call; update the call to
headers_cache.store_headers(execution_id, headers, ttl_seconds) so the
underlying headers_cache_class implementation receives the TTL (or, if TTL is
intentionally unused, remove ttl_seconds from the function signature and
docstring). Ensure you modify the invocation using headers_cache_class and the
store_headers method to include ttl_seconds (or remove parameter and update docs
accordingly).

Comment on lines +39 to +40
num_to_select = max(1, int(n * (percentage / 100)))
return set(random.sample(range(1, n + 1), num_to_select))
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Potential ValueError when num_to_select > n or n == 0.

random.sample raises ValueError if the sample size exceeds the population. This can occur when:

  • n == 0 (empty range)
  • percentage > 100 or rounding causes num_to_select > n

Consider adding bounds checking:

Proposed fix
-        num_to_select = max(1, int(n * (percentage / 100)))
-        return set(random.sample(range(1, n + 1), num_to_select))
+        if n <= 0:
+            return set()
+        num_to_select = max(1, min(n, int(n * (percentage / 100))))
+        return set(random.sample(range(1, n + 1), num_to_select))
📝 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
num_to_select = max(1, int(n * (percentage / 100)))
return set(random.sample(range(1, n + 1), num_to_select))
if n <= 0:
return set()
num_to_select = max(1, min(n, int(n * (percentage / 100))))
return set(random.sample(range(1, n + 1), num_to_select))
🤖 Prompt for AI Agents
In @backend/plugins/workflow_manager/workflow_v2/utils.py around lines 39 - 40,
The current logic can raise ValueError when n == 0 or num_to_select > n; update
the selection logic to first handle n == 0 by returning an empty set, clamp
percentage to the 0–100 range, compute num_to_select = max(0, min(n, int(n *
(percentage / 100)))), and then call random.sample on range(1, n + 1) only when
num_to_select > 0; reference the variables num_to_select, n, percentage and the
random.sample call in the return expression to locate and modify the code.

Comment on lines +87 to 89
if index in q_file_no_list:
file_hash.file_destination = WorkflowEndpoint.ConnectionType.MANUALREVIEW
return file_hash
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, find the dto.py file and check FileHash.file_destination type
find . -name "dto.py" -type f | head -20

Repository: Zipstack/unstract

Length of output: 850


🏁 Script executed:

#!/bin/bash
# Search for FileHash class definition and file_destination field
rg -n "class FileHash" --type=py -A 30

Repository: Zipstack/unstract

Length of output: 9370


🏁 Script executed:

#!/bin/bash
# Search for WorkflowEndpoint.ConnectionType definition
rg -n "class ConnectionType\|ConnectionType\s*=" --type=py -B 2 -A 10

Repository: Zipstack/unstract

Length of output: 43


🏁 Script executed:

#!/bin/bash
# Check how file_destination is used throughout the codebase
rg -n "file_destination\s*=" --type=py -B 2 -A 2

Repository: Zipstack/unstract

Length of output: 5435


🏁 Script executed:

#!/bin/bash
# Search for WorkflowEndpoint class and ConnectionType
rg -n "class WorkflowEndpoint\|WorkflowEndpoint.*ConnectionType" --type=py -A 15

Repository: Zipstack/unstract

Length of output: 43


🏁 Script executed:

#!/bin/bash
# Search for FileDestinationType definition
rg -n "class FileDestinationType\|FileDestinationType\s*=" --type=py -A 10

Repository: Zipstack/unstract

Length of output: 43


🏁 Script executed:

#!/bin/bash
# Read the actual utils.py file around lines 63-89
cat -n backend/workflow_manager/workflow_v2/utils.py | sed -n '63,89p'

Repository: Zipstack/unstract

Length of output: 138


🏁 Script executed:

#!/bin/bash
# Find the correct path to utils.py
find . -path "*/workflow_v2/utils.py" -type f

Repository: Zipstack/unstract

Length of output: 116


🏁 Script executed:

#!/bin/bash
# Search for add_file_destination_filehash function to locate the file
rg -n "add_file_destination_filehash" --type=py -B 2 -A 25

Repository: Zipstack/unstract

Length of output: 12110


🏁 Script executed:

#!/bin/bash
# Also search for WorkflowEndpoint and ConnectionType more broadly
rg -n "WorkflowEndpoint" --type=py | head -20

Repository: Zipstack/unstract

Length of output: 2104


🏁 Script executed:

#!/bin/bash
# Read the actual utils.py file at the correct path
cat -n ./backend/plugins/workflow_manager/workflow_v2/utils.py | sed -n '60,95p'

Repository: Zipstack/unstract

Length of output: 1431


🏁 Script executed:

#!/bin/bash
# Search for WorkflowEndpoint.ConnectionType.MANUALREVIEW
rg -n "MANUALREVIEW" --type=py -B 2 -A 2

Repository: Zipstack/unstract

Length of output: 9207


🏁 Script executed:

#!/bin/bash
# Search for WorkflowEndpoint class definition
rg -n "class WorkflowEndpoint\|enum.*ConnectionType" --type=py -A 10

Repository: Zipstack/unstract

Length of output: 43


🏁 Script executed:

#!/bin/bash
# Get the WorkflowEndpoint model definition to see ConnectionType enum
cat -n backend/workflow_manager/endpoint_v2/models.py | head -50

Repository: Zipstack/unstract

Length of output: 2166


🏁 Script executed:

#!/bin/bash
# Check the FileHash DTO definition more carefully
cat -n backend/workflow_manager/endpoint_v2/dto.py | head -45

Repository: Zipstack/unstract

Length of output: 1772


Type mismatch: assigning str to tuple[str, str] | None field.

FileHash.file_destination is typed as tuple[str, str] | None in backend/workflow_manager/endpoint_v2/dto.py (line 20-22), but line 88 assigns WorkflowEndpoint.ConnectionType.MANUALREVIEW, which is a string value "MANUALREVIEW". This type mismatch will cause type checker failures.

The codebase consistently uses file_destination as a string (e.g., empty strings in views, string comparisons in processor.py), so the type annotation should be str | None rather than tuple[str, str] | None. Update the type annotation to match actual usage.

🤖 Prompt for AI Agents
In @backend/plugins/workflow_manager/workflow_v2/utils.py around lines 87 - 89,
The FileHash.file_destination field is currently typed as tuple[str, str] | None
in the DTO but is assigned string values like
WorkflowEndpoint.ConnectionType.MANUALREVIEW elsewhere; update the type
annotation of FileHash.file_destination in the FileHash definition (in
endpoint_v2/dto.py) from tuple[str, str] | None to str | None, and run a quick
grep for FileHash.file_destination uses to ensure no code expects a tuple
(adjust any typed usages/imports accordingly) so that assignments like
WorkflowEndpoint.ConnectionType.MANUALREVIEW are type-safe.

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.

2 participants