[MISC] Decommission prompt-service, old tools, SDK1 prompt module#1978
Open
harini-venkataraman wants to merge 12 commits into
Open
[MISC] Decommission prompt-service, old tools, SDK1 prompt module#1978harini-venkataraman wants to merge 12 commits into
harini-venkataraman wants to merge 12 commits into
Conversation
… (Phase 5) Remove prompt-service source, Dockerfiles, and docker-compose entries. Remove tools/classifier, tools/structure, tools/text_extractor directories. Remove SDK1 prompt.py module and its tests. Clean up PROMPT_HOST/PROMPT_PORT from backend settings, sample envs, docker configs, and CI workflows. Remove prompt-service from uv-lock scripts and production build workflow. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The prompt-service directory was deleted in the prior commit but tox.ini still referenced it, which would break CI test runs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
Summary by CodeRabbit
WalkthroughThis PR removes the entire ChangesService Removal and Redirection
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
for more information, see https://pre-commit.ci
Contributor
|
| Filename | Overview |
|---|---|
| unstract/workflow-execution/src/unstract/workflow_execution/tools_utils.py | Removes prompt_host/prompt_port instance variables and their injection into the platform_vars dict passed to tool containers; no remaining tool reads these env vars. |
| unstract/workflow-execution/src/unstract/workflow_execution/constants.py | Removes PROMPT_HOST and PROMPT_PORT constants from ToolRuntimeVariable; consistent with removal in tools_utils.py. |
| unstract/sdk1/src/unstract/sdk1/utils/retry_utils.py | Removes the retry_prompt_service_call pre-configured decorator; retry_platform_service_call remains intact for active use. |
| .github/workflows/production-build.yaml | Removes prompt-service from the build matrix and correctly decrements TOTAL_SERVICES from 7 to 6; the summary loop is updated to match. |
| .github/workflows/docker-tools-build-push.yaml | Removes tool-structure option (directory deleted) and re-orders to make tool-sidecar the default; tool-classifier and tool-text-extractor remain, matching their still-present directories. |
| backend/backend/settings/base.py | Removes PROMPT_HOST and PROMPT_PORT Django settings; verified no remaining backend code reads settings.PROMPT_HOST or settings.PROMPT_PORT. |
| docker/docker-compose.yaml | Removes the prompt-service container definition and its depends_on entry from backend; compose file is internally consistent after the change. |
| tox.ini | Removes the prompt-service testenv from env_list and its full [testenv:prompt-service] block; remaining environments are unaffected. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph BEFORE["Before (Phases 1–4 + this PR)"]
B1[Backend / Workflow Execution] -->|PROMPT_HOST:3003| PS[prompt-service Flask App]
B1 -->|tool container env injection| TS[tools/structure container]
SDK1[sdk1/prompt.py PromptTool] -->|HTTP| PS
PS --> DB[(DB / Vector Store)]
TS --> PS
end
subgraph AFTER["After (Phase 5 — this PR)"]
B2[Backend / Workflow Execution] -->|executor task| WE[workers/executor]
WE --> AP[executors/answer_prompt.py]
WE --> IDX[executors/index.py]
WE --> LE[executors/legacy_executor.py]
AP & IDX & LE --> DB2[(DB / Vector Store)]
end
PS -.->|decommissioned| X1[❌ removed]
TS -.->|decommissioned| X2[❌ removed]
SDK1 -.->|decommissioned| X3[❌ removed]
Reviews (4): Last reviewed commit: "Restore classifier tool removed in Phase..." | Re-trigger Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker/scripts/uv-lock-gen/README.md`:
- Line 5: The example sentence describing "transitive dependency changes"
references the service "workers" which is not in the enumerated list; update
that sentence to reference an existing listed service (e.g., replace "workers"
with "runner") so the example matches the enumerated services, or alternatively
add "workers" into the enumerated list; locate the sentence containing
"transitive dependency changes" and the example "unstract/sdk1" and make the
replacement/addition accordingly to keep the README consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f274dae7-8d0e-4a6b-935a-8beec240f62e
⛔ Files ignored due to path filters (5)
prompt-service/src/unstract/prompt_service/tests/integration/input/sample1.pdfis excluded by!**/*.pdfprompt-service/uv.lockis excluded by!**/*.locktools/classifier/src/config/icon.svgis excluded by!**/*.svgtools/structure/src/config/icon.svgis excluded by!**/*.svgtools/text_extractor/src/config/icon.svgis excluded by!**/*.svg
📒 Files selected for processing (114)
.github/workflows/docker-tools-build-push.yaml.github/workflows/production-build.yamlbackend/backend/settings/base.pybackend/sample.envdocker/compose.debug.yamldocker/docker-compose.build.yamldocker/docker-compose.yamldocker/dockerfiles/prompt.Dockerfiledocker/dockerfiles/prompt.Dockerfile.dockerignoredocker/sample.compose.override.yamldocker/scripts/uv-lock-gen/README.mddocker/scripts/uv-lock-gen/uv-lock.shprompt-service/.gitignoreprompt-service/.python-versionprompt-service/README.mdprompt-service/entrypoint.shprompt-service/pyproject.tomlprompt-service/sample.envprompt-service/src/unstract/prompt_service/__init__.pyprompt-service/src/unstract/prompt_service/config.pyprompt-service/src/unstract/prompt_service/constants.pyprompt-service/src/unstract/prompt_service/controllers/__init__.pyprompt-service/src/unstract/prompt_service/controllers/answer_prompt.pyprompt-service/src/unstract/prompt_service/controllers/extraction.pyprompt-service/src/unstract/prompt_service/controllers/health.pyprompt-service/src/unstract/prompt_service/controllers/indexing.pyprompt-service/src/unstract/prompt_service/core/index_v2.pyprompt-service/src/unstract/prompt_service/core/retrievers/automerging.pyprompt-service/src/unstract/prompt_service/core/retrievers/base_retriever.pyprompt-service/src/unstract/prompt_service/core/retrievers/fusion.pyprompt-service/src/unstract/prompt_service/core/retrievers/keyword_table.pyprompt-service/src/unstract/prompt_service/core/retrievers/recursive.pyprompt-service/src/unstract/prompt_service/core/retrievers/retriever_llm.pyprompt-service/src/unstract/prompt_service/core/retrievers/router.pyprompt-service/src/unstract/prompt_service/core/retrievers/simple.pyprompt-service/src/unstract/prompt_service/core/retrievers/subquestion.pyprompt-service/src/unstract/prompt_service/dto.pyprompt-service/src/unstract/prompt_service/exceptions.pyprompt-service/src/unstract/prompt_service/extensions.pyprompt-service/src/unstract/prompt_service/helpers/__init__.pyprompt-service/src/unstract/prompt_service/helpers/auth.pyprompt-service/src/unstract/prompt_service/helpers/postprocessor.pyprompt-service/src/unstract/prompt_service/helpers/prompt_ide_base_tool.pyprompt-service/src/unstract/prompt_service/helpers/usage.pyprompt-service/src/unstract/prompt_service/helpers/variable_replacement.pyprompt-service/src/unstract/prompt_service/run.pyprompt-service/src/unstract/prompt_service/services/__init__.pyprompt-service/src/unstract/prompt_service/services/answer_prompt.pyprompt-service/src/unstract/prompt_service/services/extraction.pyprompt-service/src/unstract/prompt_service/services/indexing.pyprompt-service/src/unstract/prompt_service/services/rentrolls_extractor/interface.pyprompt-service/src/unstract/prompt_service/services/retrieval.pyprompt-service/src/unstract/prompt_service/services/variable_replacement.pyprompt-service/src/unstract/prompt_service/tests/conftest.pyprompt-service/src/unstract/prompt_service/tests/integration/test_api_endpoints.pyprompt-service/src/unstract/prompt_service/tests/sample.env.testprompt-service/src/unstract/prompt_service/tests/unit/__init__.pyprompt-service/src/unstract/prompt_service/tests/unit/conftest.pyprompt-service/src/unstract/prompt_service/tests/unit/test_retriever_llm.pyprompt-service/src/unstract/prompt_service/utils/__init__.pyprompt-service/src/unstract/prompt_service/utils/db_utils.pyprompt-service/src/unstract/prompt_service/utils/env_loader.pyprompt-service/src/unstract/prompt_service/utils/file_utils.pyprompt-service/src/unstract/prompt_service/utils/json_repair_helper.pyprompt-service/src/unstract/prompt_service/utils/log.pyprompt-service/src/unstract/prompt_service/utils/metrics.pyprompt-service/src/unstract/prompt_service/utils/request.pytools/classifier/.dockerignoretools/classifier/Dockerfiletools/classifier/README.mdtools/classifier/__init__.pytools/classifier/requirements.txttools/classifier/sample.envtools/classifier/src/config/properties.jsontools/classifier/src/config/runtime_variables.jsontools/classifier/src/config/spec.jsontools/classifier/src/helper.pytools/classifier/src/main.pytools/structure/.dockerignoretools/structure/.gitignoretools/structure/Dockerfiletools/structure/README.mdtools/structure/__init__.pytools/structure/requirements.txttools/structure/sample.envtools/structure/src/config/properties.jsontools/structure/src/config/runtime_variables.jsontools/structure/src/config/spec.jsontools/structure/src/constants.pytools/structure/src/helpers.pytools/structure/src/main.pytools/structure/src/utils.pytools/text_extractor/.dockerignoretools/text_extractor/.gitignoretools/text_extractor/Dockerfiletools/text_extractor/README.mdtools/text_extractor/__init__.pytools/text_extractor/requirements.txttools/text_extractor/sample.envtools/text_extractor/src/config/properties.jsontools/text_extractor/src/config/runtime_variables.jsontools/text_extractor/src/config/spec.jsontools/text_extractor/src/example_package/__init__.pytools/text_extractor/src/main.pytools/text_extractor/tests/__init__.pytox.iniunstract/sdk1/src/unstract/sdk1/prompt.pyunstract/sdk1/src/unstract/sdk1/utils/retry_utils.pyunstract/sdk1/tests/conftest.pyunstract/sdk1/tests/test_prompt.pyunstract/sdk1/tests/utils/test_retry_utils.pyunstract/workflow-execution/src/unstract/workflow_execution/constants.pyunstract/workflow-execution/src/unstract/workflow_execution/tools_utils.pyworkers/sample.env
💤 Files with no reviewable changes (100)
- tools/classifier/src/config/properties.json
- tools/text_extractor/.gitignore
- tools/classifier/.dockerignore
- prompt-service/.gitignore
- prompt-service/sample.env
- docker/dockerfiles/prompt.Dockerfile.dockerignore
- tools/structure/sample.env
- tools/structure/src/config/properties.json
- prompt-service/README.md
- docker/dockerfiles/prompt.Dockerfile
- tools/text_extractor/src/config/properties.json
- tools/text_extractor/README.md
- tools/structure/requirements.txt
- prompt-service/src/unstract/prompt_service/tests/sample.env.test
- tools/classifier/src/config/runtime_variables.json
- prompt-service/entrypoint.sh
- docker/scripts/uv-lock-gen/uv-lock.sh
- tools/classifier/README.md
- tools/classifier/sample.env
- workers/sample.env
- prompt-service/src/unstract/prompt_service/core/retrievers/retriever_llm.py
- prompt-service/src/unstract/prompt_service/core/retrievers/base_retriever.py
- prompt-service/src/unstract/prompt_service/utils/db_utils.py
- tools/structure/src/constants.py
- tools/structure/Dockerfile
- tools/structure/README.md
- prompt-service/src/unstract/prompt_service/services/rentrolls_extractor/interface.py
- tools/text_extractor/src/main.py
- prompt-service/src/unstract/prompt_service/controllers/health.py
- tools/text_extractor/.dockerignore
- tools/text_extractor/src/config/runtime_variables.json
- tools/text_extractor/requirements.txt
- prompt-service/src/unstract/prompt_service/services/indexing.py
- tools/text_extractor/Dockerfile
- prompt-service/src/unstract/prompt_service/tests/unit/test_retriever_llm.py
- unstract/sdk1/src/unstract/sdk1/utils/retry_utils.py
- prompt-service/src/unstract/prompt_service/controllers/init.py
- tools/classifier/src/config/spec.json
- prompt-service/src/unstract/prompt_service/utils/file_utils.py
- prompt-service/src/unstract/prompt_service/tests/integration/test_api_endpoints.py
- prompt-service/src/unstract/prompt_service/utils/log.py
- prompt-service/src/unstract/prompt_service/utils/metrics.py
- prompt-service/src/unstract/prompt_service/extensions.py
- unstract/sdk1/src/unstract/sdk1/prompt.py
- unstract/workflow-execution/src/unstract/workflow_execution/constants.py
- prompt-service/src/unstract/prompt_service/controllers/answer_prompt.py
- tools/structure/src/config/runtime_variables.json
- prompt-service/src/unstract/prompt_service/constants.py
- prompt-service/src/unstract/prompt_service/tests/unit/conftest.py
- tools/structure/.gitignore
- prompt-service/src/unstract/prompt_service/controllers/indexing.py
- tools/classifier/Dockerfile
- prompt-service/src/unstract/prompt_service/services/variable_replacement.py
- tools/structure/src/helpers.py
- tools/structure/src/config/spec.json
- prompt-service/src/unstract/prompt_service/helpers/auth.py
- prompt-service/src/unstract/prompt_service/core/retrievers/keyword_table.py
- tools/text_extractor/src/config/spec.json
- prompt-service/src/unstract/prompt_service/tests/conftest.py
- unstract/sdk1/tests/utils/test_retry_utils.py
- prompt-service/src/unstract/prompt_service/dto.py
- prompt-service/.python-version
- prompt-service/src/unstract/prompt_service/helpers/postprocessor.py
- prompt-service/src/unstract/prompt_service/helpers/usage.py
- docker/docker-compose.build.yaml
- tools/classifier/src/main.py
- backend/sample.env
- prompt-service/src/unstract/prompt_service/exceptions.py
- prompt-service/src/unstract/prompt_service/utils/json_repair_helper.py
- prompt-service/src/unstract/prompt_service/core/retrievers/automerging.py
- unstract/sdk1/tests/conftest.py
- prompt-service/src/unstract/prompt_service/core/index_v2.py
- prompt-service/src/unstract/prompt_service/helpers/prompt_ide_base_tool.py
- tools/text_extractor/sample.env
- prompt-service/src/unstract/prompt_service/services/answer_prompt.py
- prompt-service/src/unstract/prompt_service/core/retrievers/recursive.py
- prompt-service/src/unstract/prompt_service/core/retrievers/subquestion.py
- tools/structure/src/utils.py
- prompt-service/src/unstract/prompt_service/utils/request.py
- docker/compose.debug.yaml
- prompt-service/src/unstract/prompt_service/services/extraction.py
- prompt-service/src/unstract/prompt_service/core/retrievers/router.py
- prompt-service/src/unstract/prompt_service/config.py
- tools/structure/.dockerignore
- prompt-service/src/unstract/prompt_service/controllers/extraction.py
- prompt-service/pyproject.toml
- tools/classifier/src/helper.py
- prompt-service/src/unstract/prompt_service/core/retrievers/simple.py
- tools/classifier/requirements.txt
- docker/docker-compose.yaml
- prompt-service/src/unstract/prompt_service/services/retrieval.py
- prompt-service/src/unstract/prompt_service/run.py
- prompt-service/src/unstract/prompt_service/helpers/variable_replacement.py
- unstract/sdk1/tests/test_prompt.py
- tools/structure/src/main.py
- prompt-service/src/unstract/prompt_service/utils/env_loader.py
- backend/backend/settings/base.py
- prompt-service/src/unstract/prompt_service/core/retrievers/fusion.py
- unstract/workflow-execution/src/unstract/workflow_execution/tools_utils.py
- docker/sample.compose.override.yaml
harini-venkataraman
changed the title
…1877) * [FIX] Add hook for setting default adapters for invited users Add setup_default_adapters_for_user() hook to AuthenticationService and call it from set_user_organization() when an invited user joins an existing organization. This allows the cloud plugin to set up default triad adapters (LLM, embedding, vector DB, x2text) for invited users, fixing silent failures in API deployment creation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Update backend/account_v2/authentication_controller.py Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Signed-off-by: Praveen Kumar <praveen@zipstack.com> * [FIX] Improve log message for setup_default_adapters_for_user Address review comment: log user email and explain that default adapters will not be set when the method is not implemented. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * [MISC] Rename Default Triad to Default LLM Profile in UI Update display label from "Default Triad" to "Default LLM Profile" in the page heading and side navigation menu. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Signed-off-by: Praveen Kumar <praveen@zipstack.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Co-authored-by: Deepak K <89829542+Deepak-Kesavan@users.noreply.github.com>
* [FIX] Wrap set_user_organization in transaction.atomic The new-org branch creates the org row, then calls frictionless onboarding and the initial platform key. Failures mid-flow leave an orphan org with no adapters or key, and subsequent logins skip onboarding entirely (gated on new_organization). Atomic ensures the org rolls back on any failure so retries get a clean fresh-org path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * [MISC] Worktree skill — use --no-track to prevent accidental main pushes Without --no-track, a later `git push -u origin <branch>` can be reported by the server as also fast-forwarding main, landing commits on main. * [FIX] Use logger.exception in authorization_callback Preserves the traceback when the OAuth callback hits the safety-net catch. Behaviour unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Athul <89829560+athul-rs@users.noreply.github.com> Co-authored-by: vishnuszipstack <117254672+vishnuszipstack@users.noreply.github.com>
…1930) * UN-3386 [FEAT] Add Prompt Studio HITL change indicator plugin slot Wires up the host-side hooks for the prompt-change-indicator plugin (implementation lives in unstract-cloud): a dynamic-import slot in the prompt card Header for the indicator button, and a route at :orgName/review/readonly/:documentId for the read-only audit view. Both gates fall through gracefully when the plugin is absent (OSS). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * UN-3386 [FIX] Warn when ReadOnlyReviewPage loads without ReviewLayout Addresses review feedback: the readonly route nests inside ReviewLayout (manual-review plugin), so a deployment that ships prompt-change-indicator without manual-review would silently fail to register the route. Log a console.warn in that case to make the misconfiguration discoverable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * UN-3386 [FIX] Surface real plugin import errors in route loader Bare catch in the prompt-change-indicator dynamic import was swallowing syntax/runtime errors in the plugin file alongside the expected "plugin missing in OSS" case. Detect the missing-module messages explicitly and console.error anything else so a broken cloud plugin no longer disables the readonly route silently. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add OpenAI-compatible LLM adapter * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Address review feedback for custom OpenAI adapter * Fix import formatting after rebase * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Address follow-up review comments for OpenAI-compatible adapter * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Refine OpenAI compatible adapter schema naming * Reject empty model string in OpenAICompatibleLLMParameters validate_model previously produced "custom_openai/" for an empty model, surfacing as a confusing LiteLLM error at call time. Match the existing GeminiLLMParameters.validate_model pattern: strip whitespace, raise ValueError on empty input. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Revert SCHEMA_PATH plumbing; rename schema to custom_openai.json Addresses Ritwik's review feedback. The new BaseAdapter.SCHEMA_PATH class variable and the conditional branch in get_json_schema() are unnecessary: OpenAICompatibleLLMAdapter.get_provider() returns "custom_openai", and the default path resolution already builds …/llm1/static/{get_provider()}.json. Renaming the schema file lets the default lookup find it and keeps the base class untouched, which is the convention every other adapter follows. - Rename openai_compatible.json -> custom_openai.json - Drop SCHEMA_PATH class var and the if-None branch from BaseAdapter - Drop SCHEMA_PATH override (and unused os/ClassVar imports) from OpenAICompatibleLLMAdapter - Update test_openai_compatible_schema_is_loadable to read schema via get_json_schema() instead of touching SCHEMA_PATH directly --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Hari John Kuriakose <hari@zipstack.com> Co-authored-by: Chandrasekharan M <chandrasekharan@zipstack.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Athul <athul@zipstack.com> Co-authored-by: Athul <89829560+athul-rs@users.noreply.github.com> Co-authored-by: vishnuszipstack <117254672+vishnuszipstack@users.noreply.github.com>
* [HOTFIX] Use importlib.util.find_spec for pluggable worker discovery (#1918) * [FIX] Use importlib.util.find_spec for pluggable worker discovery _verify_pluggable_worker_exists() previously checked for the literal file `pluggable_worker/<name>/worker.py` on disk, which breaks when the plugin has been compiled to a .so (Nuitka, Cython, or any C extension) — the module is perfectly importable but the pre-check rejects it because only the .py extension is considered. Replace the filesystem check with importlib.util.find_spec(), which is Python's standard way to ask "is this module resolvable by the import system?". It honors every registered finder — source .py, compiled .so, bytecode .pyc, namespace packages, zipimports — so the function now matches what its docstring claims: verifying the module can be loaded, not that a specific file extension is present. Behavior is preserved for existing deployments: - Images with no `pluggable_worker/<name>/` subpackage → find_spec raises ModuleNotFoundError (ImportError subclass) → returns False. - Images with source .py → find_spec resolves the .py → returns True. - Images with compiled .so → find_spec resolves the .so → returns True. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * [FIX] Handle ValueError from find_spec in pluggable worker verification Greptile-flagged edge case: importlib.util.find_spec() can raise ValueError (not just ImportError) when sys.modules has a partially initialised module entry with __spec__ = None from a prior failed import. Broaden the except to catch both. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * [FIX] Resolve api-deployment worker directory from enum import path worker.py:452 did worker_type.value.replace("-", "_") to derive the on-disk dir name. All WorkerType enum values already use underscores, so the replace was a no-op; for API_DEPLOYMENT whose dir is "api-deployment" (hyphen), it resolved to "api_deployment" and the os.path.exists() check failed. Boot then logged a spurious "❌ Worker directory not found: /app/api_deployment" at ERROR level. The task registration path (builder + celery autodiscover via to_import_path) is unaffected, so this was purely log noise — but noise at ERROR level that masks real failures in log scans. Fix: derive the directory from the authoritative to_import_path() which already handles the hyphen case (api_deployment -> api-deployment). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * [HOTFIX] Add IAM Role / Instance Profile auth mode to AWS Bedrock adapter (#1944) * [FEAT] Allow Bedrock to fall through to boto3's default credential chain Match the S3/MinIO connector pattern: when AWS access keys are left blank on the Bedrock LLM and embedding adapter forms, drop them from the kwargs dict so boto3's default credential chain handles authentication. This unlocks IAM role / instance profile / IRSA / AWS Profile scenarios on hosts that already have ambient AWS credentials (e.g. EKS workers with IRSA, EC2 with an instance profile). - llm1/static/bedrock.json: clarify access-key descriptions to mention IRSA and instance profile (already non-required at v0.163.2 base). - embedding1/static/bedrock.json: drop aws_access_key_id and aws_secret_access_key from top-level required; same description fix; expose aws_profile_name for parity with the LLM form. - base1.py: AWSBedrockLLMParameters and AWSBedrockEmbeddingParameters now strip empty access-key values from the validated kwargs before returning, so empty strings don't override boto3's default chain. AWSBedrockEmbeddingParameters fields gain explicit None defaults and an aws_profile_name field. Backward-compatible: existing adapters with access keys filled in continue to work unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * [FEAT] Add Authentication Type selector to Bedrock adapter form Add an explicit `auth_type` selector with two options, making the auth choice clear to users: - "Access Keys" (default): existing flow, keys required - "IAM Role / Instance Profile (on-prem AWS only)": no fields; relies on boto3's default credential chain (IRSA on EKS, task role on ECS, instance profile on EC2). Description on the selector explicitly notes this option is only for AWS-hosted Unstract deployments. The form-only auth_type field is stripped before LiteLLM validation in both AWSBedrockLLMParameters.validate() and AWSBedrockEmbeddingParameters. validate(). Empty access keys continue to be stripped so boto3 falls through to the default chain even when the access_keys arm is selected without values (matches the S3/MinIO connector pattern). Backward-compatible: legacy adapters without auth_type behave as "Access Keys" mode (the default), and existing keys are forwarded unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * [REVIEW] Address Bedrock auth_type review feedback Fixes the P0/P1 issues raised by greptile-apps and jaseemjaskp on PR #1944. Behaviour fixes: - Stale-key leak in IAM Role mode: switching an existing adapter from Access Keys to IAM Role would carry truthy stored access keys through the strip-empty-only loop, so boto3 silently authenticated with the old long-lived credentials instead of falling through to the host's IRSA / instance-profile identity. Both LLM and embedding paths were affected. - Silent acceptance of unknown auth_type: a typo (e.g. "access_key") or a malformed payload from a non-UI client passed through the dict comprehension untouched, with no enum guard. - Cross-field validation gap: explicit Access Keys mode with blank or whitespace-only values silently fell through to the default credential chain instead of surfacing the misconfiguration. Implementation: - Add a module-level _resolve_bedrock_aws_credentials helper used by both AWSBedrockLLMParameters.validate() and AWSBedrock EmbeddingParameters.validate(), so the auth-type contract is expressed once. - Validates auth_type against an allowlist (None | "access_keys" | "iam_role"); raises ValueError on anything else. - iam_role: unconditionally drops aws_access_key_id and aws_secret_access_key. - access_keys (explicit): requires non-blank values; raises ValueError if either is empty or whitespace-only. - Legacy (auth_type absent): retains the lenient strip behaviour so pre-PR adapter configurations continue to deserialise unchanged. - Restore aws_region_name as required (no `= None` default) on AWSBedrockEmbeddingParameters; only credentials may legitimately be absent. - Drop the orphan aws_profile_name field from embedding1/static/bedrock.json: it was added for parity with the LLM form but lives outside the auth_type oneOf and contradicts the selector's "no further input" semantics. The LLM form already had aws_profile_name pre-PR and is left alone for backwards compatibility. Tests: - New tests/test_bedrock_adapter.py covers 15 cases across LLM and embedding adapters: legacy-no-auth-type, explicit access_keys with valid/blank/whitespace keys, iam_role with stale/no keys, unknown auth_type rejection, cross-field validation, and preservation of unrelated params (model_id, aws_profile_name, region, thinking). Skipped (P2 nice-to-have): - Comment-scope clarification, MinIO reference rewording, validate-mutates-caller'\''s-dict, and the LLM form description nit about aws_profile_name visibility. These don'\''t change behaviour and can be addressed in a follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * [HOTFIX] Bump litellm to 1.83.10 from PyPI to clear CVE-2026-42208 (#1976) Hotfix for cloud v0.159.3 (OSS v0.163.4). Customer scanner flagged litellm 1.82.3 for CVE-2026-42208 (SQL injection in litellm proxy auth path, affects 1.81.16-1.83.6). We do not use litellm.proxy, but vulnerability scanners flag the installed package regardless of which code path is reachable. Bump to 1.83.10 — the exact version recommended by the upstream advisory (v1.83.10-stable) and the smallest jump that clears the CVE range while keeping python-dotenv==1.0.1 compatible (1.83.14 would force bumping python-dotenv across 7+ pyproject.toml files). Only tiktoken needed to move 0.9 -> 0.12 to satisfy litellm's pin. Switch source back to PyPI now that the PyPI quarantine is over, reversing the temporary fork in #1873. Cohere embed timeout patch: verified that litellm/llms/cohere/embed/handler.py is byte-identical between v1.82.3, v1.83.10-stable, and v1.83.14-stable (the timeout-not-forwarded bug fixed in #1848 is still present upstream — BerriAI/litellm#14635 remains OPEN). Version guard bumped 1.82.3 -> 1.83.10; 6/6 patch tests pass on the new version, confirming the monkey-patch still binds correctly. Other cleanup from #1873: - Drop git apt-install from worker-unified and tool Dockerfiles (no git-sourced deps remain in any uv.lock) - Bump tool versions: structure 0.0.100 -> 0.0.101, classifier 0.0.79 -> 0.0.80, text_extractor 0.0.75 -> 0.0.76 Note on root uv.lock churn: the v0.163.4 root uv.lock had a pre-existing corruption (banks v2.4.1 entry pointing at banks-2.2.0 wheel) that blocked incremental resolution. Regenerated from scratch. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * [FIX] Align cohere patch docstring with version-guard semantics Reviewer flagged that the docstring claimed the patch is "confirmed in every release between 1.82.3 and 1.83.14-stable", but the guard at _PATCHED_LITELLM_VERSION activates only on the exact pinned version. A future maintainer reading the old text could reasonably expect bumping to e.g. 1.83.11 to keep the fix active; in reality it silently turns off. Rewritten to reference _PATCHED_LITELLM_VERSION as the single source of truth and to drop the rot-prone "as of 2026-05-20" calendar date. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Chandrasekharan M <117059509+chandrasekharan-zipstack@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
The atomic wrap from #1954 uncommits the new org row when frictionless_onboarding HTTP-calls the LLMW portal mid-transaction. The portal runs on a separate DB session and under READ COMMITTED cannot see the uncommitted row, so the call returns 400 and the caller silently persists an adapter with an empty unstract_key. Every new signup since 2026-05-19 09:47 UTC ships a broken free-trial X2Text adapter (401 on first OCR). Hotfix only — Phase 2 (UN-3476) restructures the function so the atomic guarantee is reapplied around just the pure-DB writes, with HTTP and non-DB side effects moved outside the transaction. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ion-old-components # Conflicts: # prompt-service/uv.lock # tools/classifier/Dockerfile # tools/classifier/src/config/properties.json # tools/structure/Dockerfile # tools/structure/src/config/properties.json # tools/text_extractor/Dockerfile # tools/text_extractor/src/config/properties.json
Deepak-Kesavan
approved these changes
May 21, 2026
|
|
||
| # Define services in order | ||
| for service in backend frontend platform-service prompt-service runner worker-unified x2text-service; do | ||
| for service in backend frontend platform-service runner worker-unified x2text-service; do |
Contributor
There was a problem hiding this comment.
If we are removing the tools, we should remove the runner as well
The Phase 5 decommission commit removed classifier, structure, text_extractor, and prompt-service. However, text_extractor is still in active use by customers. This surgically restores only the text_extractor tool while keeping the other decommissions in place. - Restore tools/text_extractor/ directory (14 files from origin/main) - Add tool-text_extractor back to docker-compose.build.yaml - Add tool-text-extractor back to docker-tools-build-push.yaml workflow Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Test ResultsSummary
Runner Tests - Full Report
SDK1 Tests - Full Report
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/docker-tools-build-push.yaml:
- Around line 59-65: Replace direct inline checks of ${{
github.event.inputs.service_name }} with a single env variable (e.g.,
SERVICE_NAME) and a case whitelist that sets GITHUB_OUTPUT keys (context and
dockerfile) for known services ("tool-sidecar", "tool-text-extractor") and
otherwise prints an error and exits non‑zero to fail closed; update the branch
that currently echoes "context" and "dockerfile" to use the values chosen in the
case for SERVICE_NAME, and ensure unknown values trigger an explicit exit 1 so
$GITHUB_OUTPUT is never left unset for invalid inputs.
In `@docker/docker-compose.build.yaml`:
- Around line 33-37: The docker-compose service key tool-text_extractor
currently has an image name with an underscore
(unstract/tool-text_extractor:${VERSION}) which mismatches other places
expecting the hyphenated repo name; update the image: value for the
tool-text_extractor service to unstract/tool-text-extractor:${VERSION} (keep the
service key tool-text_extractor unchanged) so the locally built image name
matches the registry and CI naming used by run-platform.sh,
.github/workflows/docker-tools-build-push.yaml and public_tools.json.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 615c3169-ef09-4106-b415-f69172a4e10a
📒 Files selected for processing (2)
.github/workflows/docker-tools-build-push.yamldocker/docker-compose.build.yaml
Comment on lines
+59
to
65
| if [ "${{ github.event.inputs.service_name }}" == "tool-sidecar" ]; then | ||
| echo "context=." >> $GITHUB_OUTPUT | ||
| echo "dockerfile=./tools/structure/Dockerfile" >> $GITHUB_OUTPUT | ||
| echo "dockerfile=docker/dockerfiles/tool-sidecar.Dockerfile" >> $GITHUB_OUTPUT | ||
| elif [ "${{ github.event.inputs.service_name }}" == "tool-text-extractor" ]; then | ||
| echo "context=." >> $GITHUB_OUTPUT | ||
| echo "dockerfile=./tools/text_extractor/Dockerfile" >> $GITHUB_OUTPUT | ||
| elif [ "${{ github.event.inputs.service_name }}" == "tool-sidecar" ]; then | ||
| echo "context=." >> $GITHUB_OUTPUT | ||
| echo "dockerfile=docker/dockerfiles/tool-sidecar.Dockerfile" >> $GITHUB_OUTPUT | ||
| fi |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify this workflow no longer embeds GitHub expression templates directly in shell conditionals.
rg -n -C2 '\$\{\{\s*github\.event\.inputs\.service_name\s*\}\}' .github/workflows/docker-tools-build-push.yaml
# Expected after fix: only appears in YAML fields (e.g., run-name/tags), not inside shell branching logic.Repository: Zipstack/unstract
Length of output: 1382
🏁 Script executed:
rg -n -C2 '\$\{\{\s*github\.event\.inputs\.service_name\s*\}\}' .github/workflows/docker-tools-build-push.yaml
# Expected: no occurrences inside the bash branching logic; `${{ github.event.inputs.service_name }}` should be assigned to an env var or used in non-branching YAML fields.Repository: Zipstack/unstract
Length of output: 1382
Harden service_name branching and fail closed for unknown values.
In .github/workflows/docker-tools-build-push.yaml (lines 59-64), bash if/elif branches embed ${{ github.event.inputs.service_name }} directly and there’s no default else/* to reject unsupported values, so the step can leave $GITHUB_OUTPUT unset for unexpected inputs. Use an env var + case whitelist with an explicit exit 1 for unknown values.
Suggested patch
- name: Set build configuration
id: build-config
+ env:
+ SERVICE_NAME: ${{ github.event.inputs.service_name }}
run: |
- if [ "${{ github.event.inputs.service_name }}" == "tool-sidecar" ]; then
- echo "context=." >> $GITHUB_OUTPUT
- echo "dockerfile=docker/dockerfiles/tool-sidecar.Dockerfile" >> $GITHUB_OUTPUT
- elif [ "${{ github.event.inputs.service_name }}" == "tool-text-extractor" ]; then
- echo "context=." >> $GITHUB_OUTPUT
- echo "dockerfile=./tools/text_extractor/Dockerfile" >> $GITHUB_OUTPUT
- fi
+ case "$SERVICE_NAME" in
+ tool-sidecar)
+ echo "context=." >> "$GITHUB_OUTPUT"
+ echo "dockerfile=docker/dockerfiles/tool-sidecar.Dockerfile" >> "$GITHUB_OUTPUT"
+ ;;
+ tool-text-extractor)
+ echo "context=." >> "$GITHUB_OUTPUT"
+ echo "dockerfile=./tools/text_extractor/Dockerfile" >> "$GITHUB_OUTPUT"
+ ;;
+ *)
+ echo "Unsupported service_name: $SERVICE_NAME" >&2
+ exit 1
+ ;;
+ esac📝 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
| if [ "${{ github.event.inputs.service_name }}" == "tool-sidecar" ]; then | |
| echo "context=." >> $GITHUB_OUTPUT | |
| echo "dockerfile=./tools/structure/Dockerfile" >> $GITHUB_OUTPUT | |
| echo "dockerfile=docker/dockerfiles/tool-sidecar.Dockerfile" >> $GITHUB_OUTPUT | |
| elif [ "${{ github.event.inputs.service_name }}" == "tool-text-extractor" ]; then | |
| echo "context=." >> $GITHUB_OUTPUT | |
| echo "dockerfile=./tools/text_extractor/Dockerfile" >> $GITHUB_OUTPUT | |
| elif [ "${{ github.event.inputs.service_name }}" == "tool-sidecar" ]; then | |
| echo "context=." >> $GITHUB_OUTPUT | |
| echo "dockerfile=docker/dockerfiles/tool-sidecar.Dockerfile" >> $GITHUB_OUTPUT | |
| fi | |
| - name: Set build configuration | |
| id: build-config | |
| env: | |
| SERVICE_NAME: ${{ github.event.inputs.service_name }} | |
| run: | | |
| case "$SERVICE_NAME" in | |
| tool-sidecar) | |
| echo "context=." >> "$GITHUB_OUTPUT" | |
| echo "dockerfile=docker/dockerfiles/tool-sidecar.Dockerfile" >> "$GITHUB_OUTPUT" | |
| ;; | |
| tool-text-extractor) | |
| echo "context=." >> "$GITHUB_OUTPUT" | |
| echo "dockerfile=./tools/text_extractor/Dockerfile" >> "$GITHUB_OUTPUT" | |
| ;; | |
| *) | |
| echo "Unsupported service_name: $SERVICE_NAME" >&2 | |
| exit 1 | |
| ;; | |
| esac |
🧰 Tools
🪛 zizmor (1.25.2)
[error] 59-59: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 62-62: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/docker-tools-build-push.yaml around lines 59 - 65, Replace
direct inline checks of ${{ github.event.inputs.service_name }} with a single
env variable (e.g., SERVICE_NAME) and a case whitelist that sets GITHUB_OUTPUT
keys (context and dockerfile) for known services ("tool-sidecar",
"tool-text-extractor") and otherwise prints an error and exits non‑zero to fail
closed; update the branch that currently echoes "context" and "dockerfile" to
use the values chosen in the case for SERVICE_NAME, and ensure unknown values
trigger an explicit exit 1 so $GITHUB_OUTPUT is never left unset for invalid
inputs.
Comment on lines
+33
to
+37
| tool-text_extractor: | ||
| image: unstract/tool-text_extractor:${VERSION} | ||
| build: | ||
| dockerfile: tools/text_extractor/Dockerfile | ||
| context: .. |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inventory both naming variants to quantify blast radius before renaming.
rg -n -C2 'tool-text[-_]extractor|tool-text[_-]extractor|tool-text_extractor|tool-text-extractor'
# Expected: pick one canonical form and eliminate the other across workflows/compose/scripts/docs.Repository: Zipstack/unstract
Length of output: 3034
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all occurrences and show them grouped by file
rg -n --hidden --no-ignore-vcs 'tool-text[-_]extractor|tool-text_extractor' . \
| sed -E 's/^(.*):([0-9]+):(.*)$/\1:\2:\3/' \
| awk -F: '{file=$1":"$2; print}' >/dev/null
# Full match listing (limited context per match)
rg -n -C1 --hidden --no-ignore-vcs 'tool-text_extractor|tool-text-extractor' .
echo "---- public_tools.json image_name/image_url entries ----"
rg -n -C2 'tool-text[_-]extractor' unstract/tool-registry/tool_registry_config/public_tools.json
echo "---- docker-tools-build-push workflow ----"
rg -n -C2 'tool-text[_-]extractor' .github/workflows/docker-tools-build-push.yaml
echo "---- docker compose build yaml ----"
rg -n -C2 'tool-text[_-]extractor' docker/docker-compose.build.yaml
echo "---- run-platform.sh ignore_services / service wiring ----"
rg -n -C2 'tool-text[_-]extractor' run-platform.shRepository: Zipstack/unstract
Length of output: 6334
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for image/tag construction in the workflow
rg -n --hidden --no-ignore-vcs 'unstract/tool-[^"\s]+' .github/workflows/docker-tools-build-push.yaml
# Look for any usage of underscore variant outside compose
rg -n --hidden --no-ignore-vcs 'tool-text_extractor' . \
| head -n 50
# Look for any usage of hyphen variant outside the workflow list
rg -n --hidden --no-ignore-vcs 'tool-text-extractor' . \
| head -n 50
# Show the relevant portion of the workflow where image name is set (keep output small)
python3 - <<'PY'
import itertools, pathlib
p = pathlib.Path(".github/workflows/docker-tools-build-push.yaml")
lines = p.read_text().splitlines()
for i,l in enumerate(lines,1):
if "service_name" in l or "image" in l or "docker build" in l or "DOCKER" in l:
pass
# Print a window around likely blocks
keys = ["service_name", "image", "dockerfile=", "build-push", "docker/build-push-action", "repository", "tags"]
idxs=[]
for i,l in enumerate(lines,1):
if any(k in l for k in keys):
idxs.append(i)
if not idxs:
idxs=[1]
start=max(1,min(idxs)-5)
end=min(len(lines),max(idxs)+30)
for i in range(start,end+1):
print(f"{i:4d}:{lines[i-1]}")
PYRepository: Zipstack/unstract
Length of output: 43
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Find where workflow sets service_name -> dockerfile/context (avoid exit 1 on no matches)
rg -n --hidden --no-ignore-vcs 'tool-text_extractor|tool-text-extractor|service_name|dockerfile|image|DOCKER' .github/workflows/docker-tools-build-push.yaml || true
echo "---- workflow full (first 120 lines) ----"
sed -n '1,120p' .github/workflows/docker-tools-build-push.yaml
echo "---- workflow remainder (next 120 lines, if any) ----"
sed -n '121,260p' .github/workflows/docker-tools-build-push.yaml || true
# 2) Check for underscore variant service names in other CI/scripts
echo "---- repo occurrences of tool-text_extractor (service naming) ----"
rg -n --hidden --no-ignore-vcs 'tool-text_extractor' . || true
echo "---- repo occurrences of tool-text-extractor (service naming) ----"
rg -n --hidden --no-ignore-vcs 'tool-text-extractor' . || trueRepository: Zipstack/unstract
Length of output: 5478
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- docker/docker-compose.build.yaml (lines 1-80) ----"
sed -n '1,80p' docker/docker-compose.build.yaml
echo "---- run-platform.sh (lines 300-380) ----"
sed -n '300,380p' run-platform.sh
echo "---- run-platform.sh: find where ignore_services are applied (small neighborhood) ----"
rg -n --no-ignore-vcs 'ignore_services=' run-platform.sh
rg -n --no-ignore-vcs 'ignore_services' run-platform.sh | head -n 20Repository: Zipstack/unstract
Length of output: 3582
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '160,230p' run-platform.shRepository: Zipstack/unstract
Length of output: 3384
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- Search compose.yaml for tool-text-extractor / tool-text_extractor ----"
rg -n --hidden --no-ignore-vcs 'tool-text-extractor|tool-text_extractor' docker/docker-compose*.yaml
echo "---- Search tool registry config (public_tools.json already saw hyphen) for underscore variant ----"
rg -n --hidden --no-ignore-vcs 'tool-text_extractor' unstract/tool-registry || true
echo "---- Search for underscore/hyphen image repo usage in general (limit output) ----"
rg -n --hidden --no-ignore-vcs 'unstract/tool-text_extractor|unstract/tool-text-extractor' . | head -n 50Repository: Zipstack/unstract
Length of output: 811
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- docker/docker-compose.yaml tool image references ----"
rg -n --hidden --no-ignore-vcs 'tool-text-extractor|tool-text_extractor' docker/docker-compose.yaml docker/docker-compose*.yaml
echo "---- any other compose files beyond docker/docker-compose*.yaml ----"
fd -e yaml .github docker | rg -n --hidden --no-ignore-vcs 'tool-text-extractor|tool-text_extractor' || trueRepository: Zipstack/unstract
Length of output: 335
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether runtime references the public tool registry (likely tool image names)
rg -n --hidden --no-ignore-vcs 'public_tools\.json|tool_registry_config|tool registry|tool-registry|public_tools' . \
| head -n 80
# Check for any other references to the image repo name patterns outside the compose build file
rg -n --hidden --no-ignore-vcs 'unstract/tool-(text[-_]extractor)' . \
| head -n 80
# Check docker-compose.yaml for any service wiring that might reference tool image names indirectly
rg -n --hidden --no-ignore-vcs 'tool-(text[-_]extractor)|text_extractor|text-extractor|adapter|tool.*image|image_name|image_url' docker/docker-compose.yaml \
|| trueRepository: Zipstack/unstract
Length of output: 8928
🏁 Script executed:
rg -n --hidden --no-ignore-vcs 'tool-text[-_]extractor|unstract/tool-text[-_]extractor' docker docker/scripts .github/workflows unstract/tool-registryRepository: Zipstack/unstract
Length of output: 684
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --hidden --no-ignore-vcs 'tool-text_extractor' . && trueRepository: Zipstack/unstract
Length of output: 340
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Where tool registry fields image_name/image_url are used to run/pull containers
rg -n --hidden --no-ignore-vcs 'image_name|image_url' unstract backend workers docker | head -n 120Repository: Zipstack/unstract
Length of output: 13915
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether env directories exist for both delimiters (what run-platform.sh expects)
ls -ld tool-text_extractor tool-text-extractor 2>/dev/null || true
find . -maxdepth 1 -type d -name 'tool-text*' -print
# Also verify whether compose service keys map to actual env sample.env paths
for d in tool-text_extractor tool-text-extractor; do
if [ -d "$d" ]; then
echo "---- $d contents ----"
ls -la "$d" | head -n 50
[ -f "$d/sample.env" ] && echo "has sample.env" || echo "missing sample.env"
fi
doneRepository: Zipstack/unstract
Length of output: 43
🏁 Script executed:
rg -n --hidden --no-ignore-vcs 'tool-text[-_]extractor|unstract/tool-text[-_]extractor' docker .github/workflows unstract/tool-registryRepository: Zipstack/unstract
Length of output: 684
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --no-ignore-vcs 'build_services\(\)|build_services\s*\{' run-platform.sh
sed -n '230,320p' run-platform.shRepository: Zipstack/unstract
Length of output: 4093
Fix tool-text-extractor image repository name mismatch (tool-text_extractor vs tool-text-extractor)
docker/docker-compose.build.yamlbuildsunstract/tool-text_extractor:${VERSION}(underscore in the image repo name)..github/workflows/docker-tools-build-push.yamlbuilds/pushesunstract/tool-text-extractor:${{ inputs.tag }}(hyphen).unstract/tool-registry/tool_registry_config/public_tools.jsonpublishes/usesunstract/tool-text-extractor.
run-platform.sh’s --build-local path will therefore build a differently named image than the one the platform/tool registry expects. Update docker/docker-compose.build.yaml to use the hyphenated image repo name in the image: field (leave the compose service key tool-text_extractor unchanged since run-platform.sh references it).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docker/docker-compose.build.yaml` around lines 33 - 37, The docker-compose
service key tool-text_extractor currently has an image name with an underscore
(unstract/tool-text_extractor:${VERSION}) which mismatches other places
expecting the hyphenated repo name; update the image: value for the
tool-text_extractor service to unstract/tool-text-extractor:${VERSION} (keep the
service key tool-text_extractor unchanged) so the locally built image name
matches the registry and CI naming used by run-platform.sh,
.github/workflows/docker-tools-build-push.yaml and public_tools.json.
chandrasekharan-zipstack
approved these changes
May 29, 2026
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
jaseemjaskp
reviewed
Jun 2, 2026
Contributor
jaseemjaskp
left a comment
jaseemjaskp
left a comment
There was a problem hiding this comment.
Automated review — Phase 5 decommission
Reviewed via the PR Review Toolkit (code review, silent-failure, comment/doc rot, test-coverage). Verdict: clean, internally-consistent decommission. All in-PR removals (PROMPT_HOST/PROMPT_PORT, retry_prompt_service_call, prompt-service/tools-structure build targets, tox env, compose blocks) are symmetric — definition, usages, and tests dropped together, no dangling references in changed files, no silent-failure risk, no real test-coverage gap (the deleted tests covered code that is also deleted or migrated to workers/ with equal-or-better coverage).
The findings below are residual references the decommission missed. Almost all live in files this PR does not touch, so they can't be anchored as inline comments — listing them here so the decommission is actually complete. (Cross-checked against existing CodeRabbit/maintainer comments — no duplicates.)
🔴 Functional — will break
docker/scripts/bump_sdk_v0_version.sh(lines 13, 17, 21, 24, 419–420): still definesPROMPT_SERVICE_DIR=.../prompt-serviceandSTRUCTURE_DIR=.../tools/structure, includesPROMPT_SERVICE_DIRinSERVICE_DIRS, and callsreset_file "$PROMPT_SERVICE_DIR/pyproject.toml"/uv.lock. Both dirs are deleted by this PR, so the next SDK version bump will error / silently skip. Remove these entries.
🟡 Self-contradicting doc
docs/local-dev-setup-executor-migration.md:223-224: sample env still setsPROMPT_HOST=http://localhost/PROMPT_PORT=3003, while line 550 of the same doc has a checklist item "No dangling references to prompt-service, PromptTool, PROMPT_HOST, PROMPT_PORT." Remove lines 223–224.
🔵 Stale docs/comments (harmless but misleading)
unstract/sdk1/src/unstract/sdk1/utils/retry_utils.py:280— docstring still listsprompt-serviceas a retry target (this file is edited by the PR; see inline note).unstract/sdk1/src/unstract/sdk1/llm.py:887— class docstring references "The prompt-service'sRetrieverLLM" (that class lived in the deleted prompt-service; it now lives inworkers/executor/.../retrievers/retriever_llm.py).docker/README.md:105— debug-port table still has| prompt-service | 5681 |(the matchingcompose.debug.yamlentry was removed in this PR).workers/executor/README.md:25— service table still listsprompt-service.workers/ARCHITECTURE.md:37— citesprompt-service/as an existing pattern to align with.backend/migrating/v2/README.md:64— instructs settingDB_SCHEMAinprompt-service.env.unstract/core/src/unstract/core/plugins/README.md:88— enumeratesprompt-serviceas a current service.workers/file_processing/structure_tool_task.py:42,197,675— comments "mirrored from / replicatestools/structure/src/..." now point at deleted source; reword to mark this file as the canonical implementation.- 6×
docker/dockerfiles/*.Dockerfile.dockerignore(backend/frontend/runner/platform/tool-sidecar/x2text) still ignoreprompt-service— dead no-op entries.
ℹ️ Optional test note
- The deleted
prompt-service .../tests/unit/test_retriever_llm.py::TestRetrieverLLMunit-testedRetrieverLLMmethod delegation + llama-index↔sdk1 type conversion. The migratedworkers/.../retrievers/retriever_llm.pyretains those methods, butworkers/tests/test_retrieval.pyonly assertsisinstance(...)with__init__mocked — the conversion/delegation logic is now unit-test-uncovered. Low severity; consider portingTestRetrieverLLMintoworkers/tests/.
✅ Confirmed consistent
production-build.yaml (matrix = 6, TOTAL_SERVICES=6, summary loop = same 6); sample.env STRUCTURE_TOOL bump 0.0.100→0.0.101 (URL + TAG both updated); tools_utils.py/constants.py/base.py/retry_utils.py removals fully symmetric.
| @@ -516,10 +516,3 @@ def create_retry_decorator( | |||
| # - PLATFORM_SERVICE_MULTIPLIER (default: 2.0) | |||
| # - PLATFORM_SERVICE_JITTER (default: true) | |||
| retry_platform_service_call = create_retry_decorator("PLATFORM_SERVICE") | |||
Contributor
There was a problem hiding this comment.
While you're removing retry_prompt_service_call here, the module's is_retryable_error docstring at line 280 still reads "For retrying internal service calls (platform-service, prompt-service)...". Since prompt-service is being decommissioned, drop prompt-service from that parenthetical so the docstring stays accurate (leave platform-service). It's the one stale prompt-service reference inside a file this PR already edits.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Phase 5 of the pluggable executor migration — decommission prompt-service, old tools (classifier, structure, text_extractor), and SDK1 prompt module from the OSS repo.
Why
These components have been fully replaced by the executor-based architecture (Phases 1–4). The prompt-service Flask app, old tool containers, and SDK1 prompt module are dead code that adds maintenance burden and CI cost.
How
Safety — preserved items
Can this PR break any existing features? If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)
No. All removed components are dead code — prompt-service was replaced by executor workers in Phases 1-4, old tool containers are unused (structure tool routing uses image env vars, not source), and SDK1 prompt.py had no remaining callers. Verified: zero import references to deleted modules, 263 workers tests pass with no regressions.
Relevant Docs
Related Issues or PRs
Dependencies Versions / Env Variables
Removed env vars:
No new dependencies or env vars added.
Notes on Testing
Screenshots
N/A — no UI changes.
Checklist
I have read and understood the Contribution Guidelines.