From 503c1779c749694d4399fc8ad68e7e15dba5a715 Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Tue, 14 Jul 2026 17:50:06 +0530 Subject: [PATCH 1/4] UN-3636 [FIX] enable the unit-workers rig group and de-flake its suite The unit-workers group filtered `markers: "unit"`, but no worker test carries that marker, so it collected zero tests and silently never ran in CI. Switch to the negative filter the other unit groups use so the 734 worker tests actually run. Enabling collection surfaced pre-existing debt (the suite was never CI-gated): - Test isolation: worker Celery apps build a live-Postgres result backend from ambient DB_*/CELERY_BACKEND_DB_*, and building an app hijacks `current_app`. Both leaked across tests under xdist (psycopg2 connection errors, task `NotRegistered`). Fixed in conftest: strip the DB env before any app import, and pin celery's finalized default app as current around each test. - Stale assertions (prod drifted, tests never caught it): ExecutorToolShim / _run_agentic_extraction gained args; index result now carries usage_records; the highlight-disabled test over-asserted "plugin loader untouched" when lookup-enrichment is queried unconditionally (returns None in OSS). - Dropped tautological enum-existence tests that just mirror the source enum. - Renamed the cryptic test_sanity_phase6* / test_phaseN* files to intent-revealing names. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_019EN8hh518CbCVMP3BHTVmG --- tests/groups.yaml | 4 +- workers/tests/conftest.py | 36 ++ ..._phase6h.py => test_agentic_operations.py} | 39 +- ...6c.py => test_completion_and_highlight.py} | 132 ++-- ...ase5.py => test_dispatch_with_callback.py} | 47 +- ... => test_extraction_pipeline_contracts.py} | 77 +-- ...log_streaming.py => test_log_streaming.py} | 46 +- ...anity_phase6a.py => test_plugin_loader.py} | 10 +- ...py => test_plugin_migration_regression.py} | 127 ++-- ...ase4.py => test_prompt_studio_dispatch.py} | 43 +- ...=> test_simple_prompt_studio_operation.py} | 58 +- ...se2f.py => test_single_pass_extraction.py} | 51 +- ... => test_smart_table_extract_operation.py} | 35 +- ..._phase5d.py => test_structure_pipeline.py} | 565 ++++++++---------- ...se3.py => test_structure_tool_pipeline.py} | 49 +- ...phase6i.py => test_summarize_operation.py} | 25 +- ...e6e.py => test_table_extract_operation.py} | 35 +- ... => test_table_lineitem_challenge_eval.py} | 156 +++-- ...est_variable_replacement_postprocessor.py} | 2 +- 19 files changed, 733 insertions(+), 804 deletions(-) rename workers/tests/{test_sanity_phase6h.py => test_agentic_operations.py} (92%) rename workers/tests/{test_sanity_phase6c.py => test_completion_and_highlight.py} (86%) rename workers/tests/{test_sanity_phase5.py => test_dispatch_with_callback.py} (97%) rename workers/tests/{test_sanity_phase2.py => test_extraction_pipeline_contracts.py} (94%) rename workers/tests/{test_phase1_log_streaming.py => test_log_streaming.py} (95%) rename workers/tests/{test_sanity_phase6a.py => test_plugin_loader.py} (97%) rename workers/tests/{test_sanity_phase6j.py => test_plugin_migration_regression.py} (90%) rename workers/tests/{test_sanity_phase4.py => test_prompt_studio_dispatch.py} (97%) rename workers/tests/{test_sanity_phase6g.py => test_simple_prompt_studio_operation.py} (85%) rename workers/tests/{test_phase2f.py => test_single_pass_extraction.py} (91%) rename workers/tests/{test_sanity_phase6f.py => test_smart_table_extract_operation.py} (84%) rename workers/tests/{test_phase5d.py => test_structure_pipeline.py} (65%) rename workers/tests/{test_sanity_phase3.py => test_structure_tool_pipeline.py} (97%) rename workers/tests/{test_sanity_phase6i.py => test_summarize_operation.py} (97%) rename workers/tests/{test_sanity_phase6e.py => test_table_extract_operation.py} (86%) rename workers/tests/{test_sanity_phase6d.py => test_table_lineitem_challenge_eval.py} (84%) rename workers/tests/{test_phase2h.py => test_variable_replacement_postprocessor.py} (99%) diff --git a/tests/groups.yaml b/tests/groups.yaml index ee91fddd44..22a7b92f69 100644 --- a/tests/groups.yaml +++ b/tests/groups.yaml @@ -44,7 +44,9 @@ groups: tier: unit workdir: workers paths: [tests, shared/tests] - markers: "unit" + # Negative filter like the other unit groups: worker tests aren't tagged + # `unit`; live-infra tests carry `integration`/`slow` and run elsewhere. + markers: "not integration and not slow" uv_sync_group: test coverage_source: shared diff --git a/workers/tests/conftest.py b/workers/tests/conftest.py index 084a8ef88c..2275686642 100644 --- a/workers/tests/conftest.py +++ b/workers/tests/conftest.py @@ -6,9 +6,45 @@ time if INTERNAL_API_BASE_URL is not set. """ +import os from pathlib import Path +import pytest from dotenv import load_dotenv _env_test = Path(__file__).resolve().parent.parent / ".env.test" load_dotenv(_env_test) + +# Worker Celery apps build a Postgres result backend from DB_*/CELERY_BACKEND_DB_*. +# Strip these before any app is imported so tests don't reach (or leak) a live DB +# the unit tier has no server for; eager results then stay in-memory. +for _var in ( + "DB_HOST", + "DB_PORT", + "DB_NAME", + "DB_USER", + "DB_PASSWORD", + "CELERY_BACKEND_DB_HOST", + "CELERY_BACKEND_DB_PORT", + "CELERY_BACKEND_DB_NAME", + "CELERY_BACKEND_DB_USER", + "CELERY_BACKEND_DB_PASSWORD", +): + os.environ.pop(_var, None) + + +@pytest.fixture(autouse=True) +def _restore_current_celery_app(): + """Pin celery's default app as current_app around each test. Worker modules + build their own apps at import and set them current, so `@worker_task` proxies + otherwise fail to resolve (`NotRegistered`) against a drifted current_app. + Finalize so every shared task is registered on it. + """ + from celery._state import default_app + + default_app.finalize() + default_app.set_current() + try: + yield + finally: + default_app.set_current() diff --git a/workers/tests/test_sanity_phase6h.py b/workers/tests/test_agentic_operations.py similarity index 92% rename from workers/tests/test_sanity_phase6h.py rename to workers/tests/test_agentic_operations.py index 3b0ed2039c..f16e566d01 100644 --- a/workers/tests/test_sanity_phase6h.py +++ b/workers/tests/test_agentic_operations.py @@ -1,26 +1,18 @@ -"""Phase 6H Sanity — AgenticPromptStudioExecutor + agentic operations. - -Verifies: -1. All 8 agentic Operation enums exist -2. AGENTIC_EXTRACTION removed from Operation enum -3. Mock AgenticPromptStudioExecutor — registration and all 8 operations -4. Queue routing: executor_name="agentic" → celery_executor_agentic -5. LegacyExecutor does NOT handle any agentic operations -6. Dispatch sends to correct queue -7. Structure tool routes to agentic executor (not legacy) +"""Agentic operations: the agentic executor registers and routes all 8 +operations to its queue, LegacyExecutor rejects them, and the structure tool +dispatches agentic extract to it (not legacy). Also guards that the old +AGENTIC_EXTRACTION enum stays removed. """ from unittest.mock import MagicMock, patch import pytest - from unstract.sdk1.execution.context import ExecutionContext, Operation from unstract.sdk1.execution.dispatcher import ExecutionDispatcher from unstract.sdk1.execution.executor import BaseExecutor from unstract.sdk1.execution.registry import ExecutorRegistry from unstract.sdk1.execution.result import ExecutionResult - AGENTIC_OPERATIONS = [ "agentic_extract", "agentic_summarize", @@ -34,15 +26,11 @@ # --------------------------------------------------------------------------- -# 1. Operation enums +# Operation enum # --------------------------------------------------------------------------- -class TestAgenticOperations: - @pytest.mark.parametrize("op", AGENTIC_OPERATIONS) - def test_agentic_operation_enum_exists(self, op): - values = {o.value for o in Operation} - assert op in values +class TestAgenticOperations: def test_agentic_extraction_removed(self): """Old AGENTIC_EXTRACTION enum no longer exists.""" assert not hasattr(Operation, "AGENTIC_EXTRACTION") @@ -54,9 +42,11 @@ def test_agentic_extraction_removed(self): # 2. Mock AgenticPromptStudioExecutor — registration and all operations # --------------------------------------------------------------------------- + class TestAgenticExecutorRegistration: def test_mock_agentic_executor_registers_and_routes_all_ops(self): """Simulate cloud executor discovery and execution of all 8 ops.""" + @ExecutorRegistry.register class MockAgenticExecutor(BaseExecutor): _OPERATION_MAP = {op: f"_handle_{op}" for op in AGENTIC_OPERATIONS} @@ -115,6 +105,7 @@ def execute(self, context): # 3. Queue routing # --------------------------------------------------------------------------- + class TestAgenticQueueRouting: def test_agentic_routes_to_correct_queue(self): queue = ExecutionDispatcher._get_queue("agentic") @@ -148,10 +139,12 @@ def test_dispatch_sends_to_agentic_queue(self, op): # 4. LegacyExecutor does NOT handle agentic operations # --------------------------------------------------------------------------- + class TestLegacyExcludesAgentic: @pytest.mark.parametrize("op", AGENTIC_OPERATIONS) def test_agentic_op_not_in_legacy_operation_map(self, op): from executor.executors.legacy_executor import LegacyExecutor + assert op not in LegacyExecutor._OPERATION_MAP def test_legacy_returns_failure_for_agentic_extract(self): @@ -197,11 +190,11 @@ def test_legacy_returns_failure_for_agentic_summarize(self): # 5. Structure tool routes to agentic executor # --------------------------------------------------------------------------- + class TestStructureToolAgenticRouting: @patch("unstract.sdk1.x2txt.X2Text") def test_structure_tool_dispatches_agentic_extract(self, mock_x2text_cls, tmp_path): """Verify _run_agentic_extraction sends executor_name='agentic'.""" - from file_processing.structure_tool_task import _run_agentic_extraction mock_dispatcher = MagicMock() @@ -224,6 +217,7 @@ def test_structure_tool_dispatches_agentic_extract(self, mock_x2text_cls, tmp_pa dispatcher=mock_dispatcher, shim=MagicMock(), file_execution_id="exec-001", + execution_id="exec-parent-001", organization_id="org-001", source_file_name="test.pdf", fs=MagicMock(), @@ -241,6 +235,7 @@ def test_structure_tool_dispatches_agentic_extract(self, mock_x2text_cls, tmp_pa # 6. tasks.py log_component for agentic operations # --------------------------------------------------------------------------- + class TestTasksLogComponent: @pytest.mark.parametrize("op", AGENTIC_OPERATIONS) def test_agentic_ops_use_default_log_component(self, op): @@ -262,6 +257,8 @@ def test_agentic_ops_use_default_log_component(self, op): # Agentic ops should NOT match ide_index, structure_pipeline, # or table_extract/smart_table_extract branches assert context.operation not in ( - "ide_index", "structure_pipeline", - "table_extract", "smart_table_extract", + "ide_index", + "structure_pipeline", + "table_extract", + "smart_table_extract", ) diff --git a/workers/tests/test_sanity_phase6c.py b/workers/tests/test_completion_and_highlight.py similarity index 86% rename from workers/tests/test_sanity_phase6c.py rename to workers/tests/test_completion_and_highlight.py index 54388f6fee..2fdb73c850 100644 --- a/workers/tests/test_sanity_phase6c.py +++ b/workers/tests/test_completion_and_highlight.py @@ -1,4 +1,4 @@ -"""Phase 6C Sanity — Highlight data as cross-cutting plugin. +"""Highlight data as cross-cutting plugin. Verifies: 1. run_completion() passes process_text to llm.complete() @@ -16,11 +16,11 @@ from executor.executors.answer_prompt import AnswerPromptService from executor.executors.constants import PromptServiceConstants as PSKeys - # --------------------------------------------------------------------------- # Fixtures # --------------------------------------------------------------------------- + @pytest.fixture() def mock_llm(): """Create a mock LLM that returns a realistic completion dict.""" @@ -53,6 +53,7 @@ def mock_llm_no_highlight(): # 1. run_completion() passes process_text to llm.complete() # --------------------------------------------------------------------------- + class TestRunCompletionProcessText: def test_process_text_passed_to_llm_complete(self, mock_llm): """process_text callback is forwarded to llm.complete().""" @@ -64,8 +65,10 @@ def test_process_text_passed_to_llm_complete(self, mock_llm): ) mock_llm.complete.assert_called_once() call_kwargs = mock_llm.complete.call_args - assert call_kwargs.kwargs.get("process_text") is callback or \ - call_kwargs[1].get("process_text") is callback + assert ( + call_kwargs.kwargs.get("process_text") is callback + or call_kwargs[1].get("process_text") is callback + ) def test_process_text_none_by_default(self, mock_llm): """When process_text not provided, None is passed to llm.complete().""" @@ -95,10 +98,12 @@ def test_process_text_none_explicit(self, mock_llm): # 2. run_completion() populates metadata from completion dict # --------------------------------------------------------------------------- + class TestRunCompletionMetadata: def test_highlight_metadata_populated_with_process_text(self, mock_llm): """When process_text is provided and LLM returns highlight data, - metadata is populated correctly.""" + metadata is populated correctly. + """ callback = MagicMock(name="highlight_run") metadata: dict = {} AnswerPromptService.run_completion( @@ -116,9 +121,7 @@ def test_highlight_metadata_populated_with_process_text(self, mock_llm): assert metadata[PSKeys.LINE_NUMBERS]["field1"] == [1, 2] assert metadata[PSKeys.WHISPER_HASH] == "abc123" - def test_highlight_metadata_empty_without_process_text( - self, mock_llm_no_highlight - ): + def test_highlight_metadata_empty_without_process_text(self, mock_llm_no_highlight): """Without process_text, highlight data is empty but no error.""" metadata: dict = {} AnswerPromptService.run_completion( @@ -137,6 +140,7 @@ def test_highlight_metadata_empty_without_process_text( # 3. construct_and_run_prompt() passes process_text through # --------------------------------------------------------------------------- + class TestConstructAndRunPromptProcessText: def test_process_text_forwarded(self, mock_llm): """construct_and_run_prompt passes process_text to run_completion.""" @@ -202,6 +206,7 @@ def test_process_text_none_default(self, mock_llm): # 4. _handle_answer_prompt() initializes highlight plugin # --------------------------------------------------------------------------- + class TestHandleAnswerPromptHighlight: """Test highlight plugin integration in LegacyExecutor._handle_answer_prompt.""" @@ -254,8 +259,10 @@ def _get_executor(self): return ExecutorRegistry.get("legacy") @patch("executor.executors.legacy_executor.ExecutorToolShim") - @patch("unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", - return_value="doc-id-1") + @patch( + "unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", + return_value="doc-id-1", + ) def test_highlight_plugin_initialized_when_enabled( self, mock_index_key, mock_shim_cls ): @@ -287,7 +294,8 @@ def test_highlight_plugin_initialized_when_enabled( with ( patch.object( - executor, "_get_prompt_deps", + executor, + "_get_prompt_deps", return_value=( AnswerPromptService, MagicMock( @@ -295,9 +303,7 @@ def test_highlight_plugin_initialized_when_enabled( return_value=["context chunk"] ) ), - MagicMock( - is_variables_present=MagicMock(return_value=False) - ), + MagicMock(is_variables_present=MagicMock(return_value=False)), None, # Index mock_llm_cls, MagicMock(), # EmbeddingCompat @@ -324,12 +330,13 @@ def test_highlight_plugin_initialized_when_enabled( ) # Verify process_text was the highlight instance's run method llm_complete_call = mock_llm.complete.call_args - assert llm_complete_call.kwargs.get("process_text") is \ - mock_highlight_instance.run + assert llm_complete_call.kwargs.get("process_text") is mock_highlight_instance.run @patch("executor.executors.legacy_executor.ExecutorToolShim") - @patch("unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", - return_value="doc-id-1") + @patch( + "unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", + return_value="doc-id-1", + ) def test_highlight_skipped_when_plugin_not_installed( self, mock_index_key, mock_shim_cls ): @@ -353,17 +360,14 @@ def test_highlight_skipped_when_plugin_not_installed( mock_llm_cls = MagicMock(return_value=mock_llm) with ( patch.object( - executor, "_get_prompt_deps", + executor, + "_get_prompt_deps", return_value=( AnswerPromptService, MagicMock( - retrieve_complete_context=MagicMock( - return_value=["chunk"] - ) - ), - MagicMock( - is_variables_present=MagicMock(return_value=False) + retrieve_complete_context=MagicMock(return_value=["chunk"]) ), + MagicMock(is_variables_present=MagicMock(return_value=False)), None, mock_llm_cls, MagicMock(), @@ -383,11 +387,11 @@ def test_highlight_skipped_when_plugin_not_installed( assert llm_complete_call.kwargs.get("process_text") is None @patch("executor.executors.legacy_executor.ExecutorToolShim") - @patch("unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", - return_value="doc-id-1") - def test_highlight_skipped_when_disabled( - self, mock_index_key, mock_shim_cls - ): + @patch( + "unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", + return_value="doc-id-1", + ) + def test_highlight_skipped_when_disabled(self, mock_index_key, mock_shim_cls): """When enable_highlight=False, plugin loader is not even called.""" mock_shim = MagicMock() mock_shim_cls.return_value = mock_shim @@ -408,17 +412,14 @@ def test_highlight_skipped_when_disabled( mock_llm_cls = MagicMock(return_value=mock_llm) with ( patch.object( - executor, "_get_prompt_deps", + executor, + "_get_prompt_deps", return_value=( AnswerPromptService, MagicMock( - retrieve_complete_context=MagicMock( - return_value=["chunk"] - ) - ), - MagicMock( - is_variables_present=MagicMock(return_value=False) + retrieve_complete_context=MagicMock(return_value=["chunk"]) ), + MagicMock(is_variables_present=MagicMock(return_value=False)), None, mock_llm_cls, MagicMock(), @@ -432,8 +433,15 @@ def test_highlight_skipped_when_disabled( result = executor._handle_answer_prompt(ctx) assert result.success - # Plugin loader should NOT have been called - mock_plugin_get.assert_not_called() + # Highlight disabled → highlight plugin must not be fetched. Other plugin + # lookups (e.g. lookup-enrichment) run unconditionally and return None in + # OSS, so assert on the highlight key rather than "loader untouched". + highlight_fetches = [ + c + for c in mock_plugin_get.call_args_list + if c.args and c.args[0] == "highlight-data" + ] + assert highlight_fetches == [] # process_text should be None llm_complete_call = mock_llm.complete.call_args assert llm_complete_call.kwargs.get("process_text") is None @@ -443,6 +451,7 @@ def test_highlight_skipped_when_disabled( # 5. Multiple prompts share same highlight instance # --------------------------------------------------------------------------- + class TestHighlightMultiplePrompts: """Verify that one highlight instance is shared across all prompts.""" @@ -451,19 +460,21 @@ def _make_multi_prompt_context(self): prompts = [] for name in ["field1", "field2", "field3"]: - prompts.append({ - PSKeys.NAME: name, - PSKeys.PROMPT: f"What is {name}?", - PSKeys.PROMPTX: f"What is {name}?", - PSKeys.TYPE: PSKeys.TEXT, - PSKeys.CHUNK_SIZE: 0, - PSKeys.CHUNK_OVERLAP: 0, - PSKeys.LLM: "llm-123", - PSKeys.EMBEDDING: "emb-123", - PSKeys.VECTOR_DB: "vdb-123", - PSKeys.X2TEXT_ADAPTER: "x2t-123", - PSKeys.RETRIEVAL_STRATEGY: "simple", - }) + prompts.append( + { + PSKeys.NAME: name, + PSKeys.PROMPT: f"What is {name}?", + PSKeys.PROMPTX: f"What is {name}?", + PSKeys.TYPE: PSKeys.TEXT, + PSKeys.CHUNK_SIZE: 0, + PSKeys.CHUNK_OVERLAP: 0, + PSKeys.LLM: "llm-123", + PSKeys.EMBEDDING: "emb-123", + PSKeys.VECTOR_DB: "vdb-123", + PSKeys.X2TEXT_ADAPTER: "x2t-123", + PSKeys.RETRIEVAL_STRATEGY: "simple", + } + ) return ExecutionContext( executor_name="legacy", operation="answer_prompt", @@ -487,8 +498,10 @@ def _make_multi_prompt_context(self): ) @patch("executor.executors.legacy_executor.ExecutorToolShim") - @patch("unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", - return_value="doc-id-1") + @patch( + "unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", + return_value="doc-id-1", + ) def test_single_highlight_instance_for_all_prompts( self, mock_index_key, mock_shim_cls ): @@ -521,17 +534,14 @@ def test_single_highlight_instance_for_all_prompts( mock_llm_cls = MagicMock(return_value=mock_llm) with ( patch.object( - executor, "_get_prompt_deps", + executor, + "_get_prompt_deps", return_value=( AnswerPromptService, MagicMock( - retrieve_complete_context=MagicMock( - return_value=["chunk"] - ) - ), - MagicMock( - is_variables_present=MagicMock(return_value=False) + retrieve_complete_context=MagicMock(return_value=["chunk"]) ), + MagicMock(is_variables_present=MagicMock(return_value=False)), None, mock_llm_cls, MagicMock(), diff --git a/workers/tests/test_sanity_phase5.py b/workers/tests/test_dispatch_with_callback.py similarity index 97% rename from workers/tests/test_sanity_phase5.py rename to workers/tests/test_dispatch_with_callback.py index e414d9d2d5..cac77a1feb 100644 --- a/workers/tests/test_sanity_phase5.py +++ b/workers/tests/test_dispatch_with_callback.py @@ -1,4 +1,4 @@ -"""Phase 5-SANITY — Integration tests for the multi-hop elimination. +"""Integration tests for the multi-hop elimination. Phase 5 eliminates idle backend worker slots by: - Adding ``dispatch_with_callback`` (fire-and-forget with link/link_error) @@ -12,7 +12,6 @@ from unittest.mock import MagicMock, patch import pytest - from executor.executors.constants import ( PromptServiceConstants as PSKeys, ) @@ -27,19 +26,13 @@ _PATCH_X2TEXT = "executor.executors.legacy_executor.X2Text" _PATCH_FS = "executor.executors.legacy_executor.FileUtils.get_fs_instance" -_PATCH_INDEX_DEPS = ( - "executor.executors.legacy_executor.LegacyExecutor._get_indexing_deps" -) -_PATCH_PROMPT_DEPS = ( - "executor.executors.legacy_executor.LegacyExecutor._get_prompt_deps" -) +_PATCH_INDEX_DEPS = "executor.executors.legacy_executor.LegacyExecutor._get_indexing_deps" +_PATCH_PROMPT_DEPS = "executor.executors.legacy_executor.LegacyExecutor._get_prompt_deps" _PATCH_SHIM = "executor.executors.legacy_executor.ExecutorToolShim" _PATCH_RUN_COMPLETION = ( "executor.executors.answer_prompt.AnswerPromptService.run_completion" ) -_PATCH_INDEX_UTILS = ( - "unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key" -) +_PATCH_INDEX_UTILS = "unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key" # --------------------------------------------------------------------------- # Fixtures @@ -110,7 +103,9 @@ def _mock_prompt_deps(llm=None): if llm is None: llm = _mock_llm() - from executor.executors.answer_prompt import AnswerPromptService as answer_prompt_svc_cls + from executor.executors.answer_prompt import ( + AnswerPromptService as answer_prompt_svc_cls, + ) retrieval_service = MagicMock(name="RetrievalService") retrieval_service.run_retrieval.return_value = ["chunk1"] @@ -262,9 +257,7 @@ def test_ide_index_success( """Full ide_index through eager chain returns doc_id.""" # Mock extract x2t_instance = MagicMock() - x2t_instance.process.return_value = _mock_process_response( - "IDE extracted text" - ) + x2t_instance.process.return_value = _mock_process_response("IDE extracted text") mock_x2text.return_value = x2t_instance fs = MagicMock() @@ -456,9 +449,7 @@ def test_ide_index_reuses_pre_extracted_text( x2t_instance.process.assert_not_called() # perform_indexing received the pre-populated text. perform_call_kwargs = index_inst.perform_indexing.call_args.kwargs - assert ( - perform_call_kwargs.get("extracted_text") == "reused extracted payload" - ) + assert perform_call_kwargs.get("extracted_text") == "reused extracted payload" @patch(_PATCH_INDEX_DEPS) @patch(_PATCH_FS) @@ -474,9 +465,7 @@ def test_ide_index_without_pre_extracted_text_runs_extract( ): """Marker-miss path: extract runs as before when extracted_text is absent.""" x2t_instance = MagicMock() - x2t_instance.process.return_value = _mock_process_response( - "freshly extracted" - ) + x2t_instance.process.return_value = _mock_process_response("freshly extracted") mock_x2text.return_value = x2t_instance fs = MagicMock() @@ -856,18 +845,10 @@ def test_structure_pipeline_extract_failure( class TestStructureToolSingleDispatch: """Verify structure_tool_task dispatches exactly once.""" - @patch( - "executor.executor_tool_shim.ExecutorToolShim" - ) - @patch( - "file_processing.structure_tool_task._get_file_storage" - ) - @patch( - "file_processing.structure_tool_task._create_platform_helper" - ) - @patch( - "file_processing.structure_tool_task.ExecutionDispatcher" - ) + @patch("executor.executor_tool_shim.ExecutorToolShim") + @patch("file_processing.structure_tool_task._get_file_storage") + @patch("file_processing.structure_tool_task._create_platform_helper") + @patch("file_processing.structure_tool_task.ExecutionDispatcher") def test_single_dispatch_normal( self, mock_dispatcher_cls, diff --git a/workers/tests/test_sanity_phase2.py b/workers/tests/test_extraction_pipeline_contracts.py similarity index 94% rename from workers/tests/test_sanity_phase2.py rename to workers/tests/test_extraction_pipeline_contracts.py index 18a87e51d3..4eecb2b447 100644 --- a/workers/tests/test_sanity_phase2.py +++ b/workers/tests/test_extraction_pipeline_contracts.py @@ -1,4 +1,4 @@ -"""Phase 2-SANITY — Full-chain integration tests for LegacyExecutor. +"""Full-chain integration tests for LegacyExecutor. All Phase 2 code and unit tests are complete (2A–2H, 194 workers tests). This file bridges unit tests and real integration by testing the full @@ -16,9 +16,10 @@ from unittest.mock import MagicMock, patch import pytest - from executor.executors.constants import ( IndexingConstants as IKeys, +) +from executor.executors.constants import ( PromptServiceConstants as PSKeys, ) from unstract.sdk1.execution.context import ExecutionContext, Operation @@ -31,19 +32,13 @@ _PATCH_X2TEXT = "executor.executors.legacy_executor.X2Text" _PATCH_FS = "executor.executors.legacy_executor.FileUtils.get_fs_instance" -_PATCH_INDEX_DEPS = ( - "executor.executors.legacy_executor.LegacyExecutor._get_indexing_deps" -) -_PATCH_PROMPT_DEPS = ( - "executor.executors.legacy_executor.LegacyExecutor._get_prompt_deps" -) +_PATCH_INDEX_DEPS = "executor.executors.legacy_executor.LegacyExecutor._get_indexing_deps" +_PATCH_PROMPT_DEPS = "executor.executors.legacy_executor.LegacyExecutor._get_prompt_deps" _PATCH_SHIM = "executor.executors.legacy_executor.ExecutorToolShim" _PATCH_RUN_COMPLETION = ( "executor.executors.answer_prompt.AnswerPromptService.run_completion" ) -_PATCH_INDEX_UTILS = ( - "unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key" -) +_PATCH_INDEX_UTILS = "unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key" # --------------------------------------------------------------------------- # Fixtures @@ -121,7 +116,9 @@ def _mock_prompt_deps(llm=None): if llm is None: llm = _mock_llm() - from executor.executors.answer_prompt import AnswerPromptService as answer_prompt_svc_cls + from executor.executors.answer_prompt import ( + AnswerPromptService as answer_prompt_svc_cls, + ) retrieval_svc = MagicMock(name="RetrievalService") retrieval_svc.run_retrieval.return_value = ["chunk1", "chunk2"] @@ -166,8 +163,9 @@ def _mock_process_response(text="sanity extracted text"): ) -def _make_prompt(name="field_a", prompt="What is the revenue?", - output_type="text", **overrides): +def _make_prompt( + name="field_a", prompt="What is the revenue?", output_type="text", **overrides +): """Build a minimal prompt definition dict.""" d = { PSKeys.NAME: name, @@ -286,9 +284,7 @@ class TestSanityExtract: def test_extract_full_chain(self, mock_x2text_cls, mock_get_fs, eager_app): """Mocked X2Text + FileUtils → result.data has extracted_text.""" mock_x2text = MagicMock() - mock_x2text.process.return_value = _mock_process_response( - "sanity extracted" - ) + mock_x2text.process.return_value = _mock_process_response("sanity extracted") mock_x2text.x2text_instance = MagicMock() mock_x2text_cls.return_value = mock_x2text mock_get_fs.return_value = MagicMock() @@ -369,9 +365,7 @@ def test_index_full_chain(self, mock_deps, mock_get_fs, eager_app): @patch(_PATCH_INDEX_UTILS, return_value="doc-zero-chunk-sanity") @patch(_PATCH_FS) - def test_index_chunk_size_zero_full_chain( - self, mock_get_fs, mock_gen_key, eager_app - ): + def test_index_chunk_size_zero_full_chain(self, mock_get_fs, mock_gen_key, eager_app): """chunk_size=0 skips heavy deps → returns doc_id via IndexingUtils.""" mock_get_fs.return_value = MagicMock() @@ -480,9 +474,7 @@ def test_answer_prompt_table_fails_full_chain( mock_deps.return_value = _mock_prompt_deps(llm) mock_shim_cls.return_value = MagicMock() - ctx = _answer_prompt_ctx( - prompts=[_make_prompt(output_type="table")] - ) + ctx = _answer_prompt_ctx(prompts=[_make_prompt(output_type="table")]) result_dict = _run_task(eager_app, ctx.to_dict()) result = ExecutionResult.from_dict(result_dict) @@ -528,8 +520,13 @@ def test_summarize_full_chain( mock_llm_cls = MagicMock() mock_llm_cls.return_value = MagicMock() mock_get_deps.return_value = ( - MagicMock(), MagicMock(), MagicMock(), MagicMock(), - mock_llm_cls, MagicMock(), MagicMock(), + MagicMock(), + MagicMock(), + MagicMock(), + MagicMock(), + mock_llm_cls, + MagicMock(), + MagicMock(), ) ctx = _summarize_ctx() @@ -565,8 +562,13 @@ def test_summarize_error_full_chain( mock_llm_cls = MagicMock() mock_llm_cls.return_value = MagicMock() mock_get_deps.return_value = ( - MagicMock(), MagicMock(), MagicMock(), MagicMock(), - mock_llm_cls, MagicMock(), MagicMock(), + MagicMock(), + MagicMock(), + MagicMock(), + MagicMock(), + mock_llm_cls, + MagicMock(), + MagicMock(), ) ctx = _summarize_ctx() @@ -629,7 +631,9 @@ def test_index_contract(self, mock_deps, mock_get_fs, eager_app): mock_index_cls.return_value = mock_index mock_emb_cls = MagicMock() - mock_emb_cls.return_value = MagicMock() + mock_emb = MagicMock() + mock_emb.flush_pending_usage.return_value = [] # must be JSON-serializable + mock_emb_cls.return_value = mock_emb mock_vdb_cls = MagicMock() mock_vdb_cls.return_value = MagicMock() @@ -649,9 +653,7 @@ def test_index_contract(self, mock_deps, mock_get_fs, eager_app): @patch(_PATCH_INDEX_UTILS, return_value="doc-id-sanity") @patch(_PATCH_PROMPT_DEPS) @patch(_PATCH_SHIM) - def test_answer_prompt_contract( - self, mock_shim_cls, mock_deps, _mock_idx, eager_app - ): + def test_answer_prompt_contract(self, mock_shim_cls, mock_deps, _mock_idx, eager_app): llm = _mock_llm("contract answer") mock_deps.return_value = _mock_prompt_deps(llm) mock_shim_cls.return_value = MagicMock() @@ -671,14 +673,17 @@ def test_answer_prompt_contract( @patch(_PATCH_RUN_COMPLETION, return_value="contract summary") @patch(_PATCH_PROMPT_DEPS) @patch(_PATCH_SHIM) - def test_summarize_contract( - self, mock_shim_cls, mock_get_deps, mock_run, eager_app - ): + def test_summarize_contract(self, mock_shim_cls, mock_get_deps, mock_run, eager_app): mock_llm_cls = MagicMock() mock_llm_cls.return_value = MagicMock() mock_get_deps.return_value = ( - MagicMock(), MagicMock(), MagicMock(), MagicMock(), - mock_llm_cls, MagicMock(), MagicMock(), + MagicMock(), + MagicMock(), + MagicMock(), + MagicMock(), + mock_llm_cls, + MagicMock(), + MagicMock(), ) ctx = _summarize_ctx() diff --git a/workers/tests/test_phase1_log_streaming.py b/workers/tests/test_log_streaming.py similarity index 95% rename from workers/tests/test_phase1_log_streaming.py rename to workers/tests/test_log_streaming.py index 9c063e19de..519435cd1e 100644 --- a/workers/tests/test_phase1_log_streaming.py +++ b/workers/tests/test_log_streaming.py @@ -1,4 +1,4 @@ -"""Phase 1 — Executor log streaming to frontend via Socket.IO. +"""Executor log streaming to frontend via Socket.IO. Tests cover: - ExecutionContext round-trips log_events_id through to_dict/from_dict @@ -13,11 +13,9 @@ from unittest.mock import MagicMock, patch - from unstract.sdk1.constants import LogLevel from unstract.sdk1.execution.context import ExecutionContext - # --------------------------------------------------------------------------- # 1A — ExecutionContext.log_events_id round-trip # --------------------------------------------------------------------------- @@ -168,9 +166,7 @@ def test_stream_log_no_publish_without_log_events_id(self, mock_lp): def test_stream_log_empty_log_events_id_no_publish(self, mock_lp): from executor.executor_tool_shim import ExecutorToolShim - shim = ExecutorToolShim( - platform_api_key="sk-test", log_events_id="" - ) + shim = ExecutorToolShim(platform_api_key="sk-test", log_events_id="") shim.stream_log("Hello", level=LogLevel.INFO) mock_lp.log_progress.assert_not_called() @@ -243,9 +239,7 @@ class TestExecuteExtractionComponentDict: """Verify component dict is built from executor_params.""" @patch("executor.tasks.ExecutionOrchestrator") - def test_component_dict_built_when_log_events_id_present( - self, mock_orch_cls - ): + def test_component_dict_built_when_log_events_id_present(self, mock_orch_cls): mock_orch = MagicMock() mock_orch.execute.return_value = MagicMock( success=True, to_dict=lambda: {"success": True} @@ -277,9 +271,7 @@ def test_component_dict_built_when_log_events_id_present( } @patch("executor.tasks.ExecutionOrchestrator") - def test_component_dict_empty_when_no_log_events_id( - self, mock_orch_cls - ): + def test_component_dict_empty_when_no_log_events_id(self, mock_orch_cls): mock_orch = MagicMock() mock_orch.execute.return_value = MagicMock( success=True, to_dict=lambda: {"success": True} @@ -324,9 +316,7 @@ def test_extract_passes_log_info_to_shim( mock_shim = MagicMock() mock_shim_cls.return_value = mock_shim mock_x2t = MagicMock() - mock_x2t.process.return_value = MagicMock( - extracted_text="hello" - ) + mock_x2t.process.return_value = MagicMock(extracted_text="hello") mock_x2text.return_value = mock_x2t ctx = ExecutionContext( @@ -351,6 +341,9 @@ def test_extract_passes_log_info_to_shim( platform_api_key="sk-test", log_events_id="session-abc", component={"tool_id": "t1", "run_id": "r1", "doc_name": "test.pdf"}, + execution_id=None, + organization_id=None, + file_execution_id=None, ) @patch("executor.executors.legacy_executor.FileUtils.get_fs_instance") @@ -368,9 +361,7 @@ def test_extract_no_log_info_when_absent( mock_shim = MagicMock() mock_shim_cls.return_value = mock_shim mock_x2t = MagicMock() - mock_x2t.process.return_value = MagicMock( - extracted_text="hello" - ) + mock_x2t.process.return_value = MagicMock(extracted_text="hello") mock_x2text.return_value = mock_x2t ctx = ExecutionContext( @@ -393,11 +384,12 @@ def test_extract_no_log_info_when_absent( platform_api_key="sk-test", log_events_id="", component={}, + execution_id=None, + organization_id=None, + file_execution_id=None, ) - @patch( - "executor.executors.legacy_executor.LegacyExecutor._get_prompt_deps" - ) + @patch("executor.executors.legacy_executor.LegacyExecutor._get_prompt_deps") @patch("executor.executors.legacy_executor.ExecutorToolShim") def test_answer_prompt_enriches_component_with_prompt_key( self, mock_shim_cls, mock_prompt_deps @@ -417,9 +409,7 @@ def test_answer_prompt_enriches_component_with_prompt_key( mock_answer_prompt_service.extract_variable.return_value = "prompt text" mock_retrieval_service = MagicMock() mock_variable_replacement_service = MagicMock() - mock_variable_replacement_service.is_variables_present.return_value = ( - False - ) + mock_variable_replacement_service.is_variables_present.return_value = False mock_index = MagicMock() mock_llm = MagicMock() mock_embedding_compat = MagicMock() @@ -474,12 +464,8 @@ def test_answer_prompt_enriches_component_with_prompt_key( ): executor = LegacyExecutor() # The handler will try retrieval which we need to mock - mock_retrieval_service.retrieve_complete_context.return_value = [ - "context" - ] - mock_answer_prompt_service.construct_and_run_prompt.return_value = ( - "INV-001" - ) + mock_retrieval_service.retrieve_complete_context.return_value = ["context"] + mock_answer_prompt_service.construct_and_run_prompt.return_value = "INV-001" executor.execute(ctx) diff --git a/workers/tests/test_sanity_phase6a.py b/workers/tests/test_plugin_loader.py similarity index 97% rename from workers/tests/test_sanity_phase6a.py rename to workers/tests/test_plugin_loader.py index 4c49c7407a..9e682a8fbc 100644 --- a/workers/tests/test_sanity_phase6a.py +++ b/workers/tests/test_plugin_loader.py @@ -1,4 +1,4 @@ -"""Phase 6A Sanity — Plugin loader infrastructure + queue-per-executor routing. +"""Plugin loader infrastructure + queue-per-executor routing. Verifies: 1. ExecutorPluginLoader.get() returns None when no plugins installed @@ -240,8 +240,7 @@ def test_get_queue_table(self): def test_get_queue_smart_table(self): assert ( - ExecutionDispatcher._get_queue("smart_table") - == "celery_executor_smart_table" + ExecutionDispatcher._get_queue("smart_table") == "celery_executor_smart_table" ) def test_get_queue_simple_prompt_studio(self): @@ -255,10 +254,7 @@ def test_get_queue_agentic(self): def test_get_queue_arbitrary_name(self): """Any executor_name works — no whitelist.""" - assert ( - ExecutionDispatcher._get_queue("my_custom") - == "celery_executor_my_custom" - ) + assert ExecutionDispatcher._get_queue("my_custom") == "celery_executor_my_custom" def test_queue_name_enum_matches_dispatcher(self): """QueueName.EXECUTOR matches what dispatcher generates for 'legacy'.""" diff --git a/workers/tests/test_sanity_phase6j.py b/workers/tests/test_plugin_migration_regression.py similarity index 90% rename from workers/tests/test_sanity_phase6j.py rename to workers/tests/test_plugin_migration_regression.py index c4e7c6631e..a9bae8dad1 100644 --- a/workers/tests/test_sanity_phase6j.py +++ b/workers/tests/test_plugin_migration_regression.py @@ -1,22 +1,12 @@ -"""Phase 6J — Comprehensive Phase 6 sanity tests. - -Consolidated regression + integration tests for the full Phase 6 -plugin migration. Verifies: - -1. Full Operation enum coverage — every operation has exactly one executor -2. Multi-executor coexistence in ExecutorRegistry -3. End-to-end Celery chain for each cloud executor (mock executors) -4. Cross-cutting highlight plugin works across executors -5. Plugin loader → executor registration → dispatch → result flow -6. Queue routing for all executor names -7. Graceful degradation when cloud plugins missing -8. tasks.py log_component for all operation types +"""Consolidated regression tests for the executor plugin migration: full +Operation-to-executor coverage, multi-executor coexistence, per-executor Celery +chains, cross-cutting highlight, dispatch/result flow, queue routing, graceful +degradation when cloud plugins are absent, and log_component for all operations. """ from unittest.mock import MagicMock, patch import pytest - from unstract.sdk1.execution.context import ExecutionContext, Operation from unstract.sdk1.execution.dispatcher import ExecutionDispatcher from unstract.sdk1.execution.executor import BaseExecutor @@ -24,11 +14,11 @@ from unstract.sdk1.execution.registry import ExecutorRegistry from unstract.sdk1.execution.result import ExecutionResult - # --------------------------------------------------------------------------- # Fixtures # --------------------------------------------------------------------------- + @pytest.fixture(autouse=True) def _clean_registry(): ExecutorRegistry.clear() @@ -57,6 +47,7 @@ def eager_app(): def _register_legacy(): from executor.executors.legacy_executor import LegacyExecutor + ExecutorRegistry.register(LegacyExecutor) @@ -72,9 +63,7 @@ def name(self) -> str: def execute(self, context): if context.operation != "table_extract": - return ExecutionResult.failure( - error=f"Unsupported: {context.operation}" - ) + return ExecutionResult.failure(error=f"Unsupported: {context.operation}") return ExecutionResult( success=True, data={"output": "table_data", "metadata": {}}, @@ -88,9 +77,7 @@ def name(self) -> str: def execute(self, context): if context.operation != "smart_table_extract": - return ExecutionResult.failure( - error=f"Unsupported: {context.operation}" - ) + return ExecutionResult.failure(error=f"Unsupported: {context.operation}") return ExecutionResult( success=True, data={"output": "smart_table_data", "metadata": {}}, @@ -104,9 +91,7 @@ def name(self) -> str: def execute(self, context): if context.operation not in ("sps_answer_prompt", "sps_index"): - return ExecutionResult.failure( - error=f"Unsupported: {context.operation}" - ) + return ExecutionResult.failure(error=f"Unsupported: {context.operation}") return ExecutionResult( success=True, data={"output": f"sps_{context.operation}", "metadata": {}}, @@ -115,9 +100,13 @@ def execute(self, context): @ExecutorRegistry.register class MockAgenticExecutor(BaseExecutor): _OPS = { - "agentic_extract", "agentic_summarize", "agentic_uniformize", - "agentic_finalize", "agentic_generate_prompt", - "agentic_generate_prompt_pipeline", "agentic_compare", + "agentic_extract", + "agentic_summarize", + "agentic_uniformize", + "agentic_finalize", + "agentic_generate_prompt", + "agentic_generate_prompt_pipeline", + "agentic_compare", "agentic_tune_field", } @@ -127,9 +116,7 @@ def name(self) -> str: def execute(self, context): if context.operation not in self._OPS: - return ExecutionResult.failure( - error=f"Unsupported: {context.operation}" - ) + return ExecutionResult.failure(error=f"Unsupported: {context.operation}") return ExecutionResult( success=True, data={"output": f"agentic_{context.operation}", "metadata": {}}, @@ -211,6 +198,7 @@ def test_cloud_operations_not_in_legacy_map(self): # 2. Multi-executor coexistence in registry # --------------------------------------------------------------------------- + class TestMultiExecutorCoexistence: def test_all_five_executors_registered(self): """Legacy + 4 cloud executors all coexist in registry.""" @@ -278,6 +266,7 @@ def test_correct_executor_handles_operation(self): # 3. End-to-end Celery chain for cloud executors # --------------------------------------------------------------------------- + class TestCeleryChainCloudExecutors: def test_table_extract_celery_chain(self, eager_app): """TABLE extraction through full Celery task chain.""" @@ -372,6 +361,7 @@ def test_unregistered_executor_returns_failure(self, eager_app): # 4. Cross-cutting highlight plugin across executors # --------------------------------------------------------------------------- + class TestCrossCuttingHighlight: @patch("importlib.metadata.entry_points", return_value=[]) def test_highlight_plugin_not_installed_no_error(self, _mock_eps): @@ -413,7 +403,9 @@ def get_confidence_data(self): # Both legacy and agentic contexts can create instances legacy_hl = cls(file_path=str(tmp_path / "doc.txt"), execution_source="ide") - agentic_hl = cls(file_path=str(tmp_path / "other.txt"), execution_source="tool") + agentic_hl = cls( + file_path=str(tmp_path / "other.txt"), execution_source="tool" + ) assert legacy_hl.get_highlight_data() == {"lines": [1, 2, 3]} assert agentic_hl.get_confidence_data() == {"confidence": 0.95} @@ -423,9 +415,11 @@ def get_confidence_data(self): # 5. Plugin loader → registration → dispatch → result flow # --------------------------------------------------------------------------- + class TestPluginDiscoveryToDispatchFlow: def test_full_discovery_to_dispatch_flow(self): """Simulate: entry point discovery → register → dispatch → result.""" + # Step 1: "Discover" a cloud executor via entry point @ExecutorRegistry.register class DiscoveredExecutor(BaseExecutor): @@ -494,6 +488,7 @@ def test_queue_name_for_executor(self, executor_name, expected_queue): # 7. Graceful degradation when cloud plugins missing # --------------------------------------------------------------------------- + class TestGracefulDegradation: def test_legacy_works_without_cloud_executors(self, eager_app): """Legacy operations work even when no cloud executors installed.""" @@ -512,8 +507,12 @@ def test_cloud_op_on_legacy_returns_meaningful_error(self): _register_legacy() executor = ExecutorRegistry.get("legacy") - for cloud_op in ["table_extract", "smart_table_extract", - "sps_answer_prompt", "agentic_extract"]: + for cloud_op in [ + "table_extract", + "smart_table_extract", + "sps_answer_prompt", + "agentic_extract", + ]: ctx = ExecutionContext( executor_name="legacy", operation=cloud_op, @@ -544,6 +543,7 @@ def test_missing_executor_via_orchestrator(self): # 8. tasks.py log_component for all operation types # --------------------------------------------------------------------------- + class TestLogComponentAllOperations: """Verify tasks.py log_component builder handles all operation types.""" @@ -553,15 +553,17 @@ def _build_log_component(self, operation, executor_params=None): "tool_id": "t-1", "file_name": "doc.pdf", } - ctx = ExecutionContext.from_dict({ - "executor_name": "legacy", - "operation": operation, - "run_id": "run-log", - "execution_source": "tool", - "executor_params": params, - "request_id": "req-1", - "log_events_id": "evt-1", - }) + ctx = ExecutionContext.from_dict( + { + "executor_name": "legacy", + "operation": operation, + "run_id": "run-log", + "execution_source": "tool", + "executor_params": params, + "request_id": "req-1", + "log_events_id": "evt-1", + } + ) # Replicate tasks.py logic if ctx.operation == "ide_index": @@ -590,17 +592,23 @@ def _build_log_component(self, operation, executor_params=None): } def test_ide_index_extracts_nested_params(self): - comp = self._build_log_component("ide_index", { - "extract_params": {"tool_id": "t-nested", "file_name": "nested.pdf"}, - }) + comp = self._build_log_component( + "ide_index", + { + "extract_params": {"tool_id": "t-nested", "file_name": "nested.pdf"}, + }, + ) assert comp["tool_id"] == "t-nested" assert comp["doc_name"] == "nested.pdf" def test_structure_pipeline_extracts_nested_params(self): - comp = self._build_log_component("structure_pipeline", { - "answer_params": {"tool_id": "t-pipe"}, - "pipeline_options": {"source_file_name": "pipe.pdf"}, - }) + comp = self._build_log_component( + "structure_pipeline", + { + "answer_params": {"tool_id": "t-pipe"}, + "pipeline_options": {"source_file_name": "pipe.pdf"}, + }, + ) assert comp["tool_id"] == "t-pipe" assert comp["doc_name"] == "pipe.pdf" @@ -613,11 +621,21 @@ def test_smart_table_extract_uses_direct_params(self): comp = self._build_log_component("smart_table_extract") assert comp["operation"] == "smart_table_extract" - @pytest.mark.parametrize("op", [ - "extract", "index", "answer_prompt", "single_pass_extraction", - "summarize", "sps_answer_prompt", "sps_index", - "agentic_extract", "agentic_summarize", "agentic_compare", - ]) + @pytest.mark.parametrize( + "op", + [ + "extract", + "index", + "answer_prompt", + "single_pass_extraction", + "summarize", + "sps_answer_prompt", + "sps_index", + "agentic_extract", + "agentic_summarize", + "agentic_compare", + ], + ) def test_default_branch_for_standard_ops(self, op): comp = self._build_log_component(op) assert comp["tool_id"] == "t-1" @@ -629,6 +647,7 @@ def test_default_branch_for_standard_ops(self, op): # 9. ExecutionResult serialization round-trip # --------------------------------------------------------------------------- + class TestResultRoundTrip: def test_success_result_round_trip(self): original = ExecutionResult( diff --git a/workers/tests/test_sanity_phase4.py b/workers/tests/test_prompt_studio_dispatch.py similarity index 97% rename from workers/tests/test_sanity_phase4.py rename to workers/tests/test_prompt_studio_dispatch.py index 7e94489f3d..0f21605e93 100644 --- a/workers/tests/test_sanity_phase4.py +++ b/workers/tests/test_prompt_studio_dispatch.py @@ -1,4 +1,4 @@ -"""Phase 4-SANITY — IDE path integration tests through executor chain. +"""IDE path integration tests through executor chain. Phase 4 replaces PromptTool HTTP calls in PromptStudioHelper with ExecutionDispatcher → executor worker → LegacyExecutor. @@ -19,7 +19,6 @@ from unittest.mock import MagicMock, patch import pytest - from executor.executors.constants import ( PromptServiceConstants as PSKeys, ) @@ -34,22 +33,14 @@ _PATCH_X2TEXT = "executor.executors.legacy_executor.X2Text" _PATCH_FS = "executor.executors.legacy_executor.FileUtils.get_fs_instance" -_PATCH_INDEX_DEPS = ( - "executor.executors.legacy_executor.LegacyExecutor._get_indexing_deps" -) -_PATCH_PROMPT_DEPS = ( - "executor.executors.legacy_executor.LegacyExecutor._get_prompt_deps" -) +_PATCH_INDEX_DEPS = "executor.executors.legacy_executor.LegacyExecutor._get_indexing_deps" +_PATCH_PROMPT_DEPS = "executor.executors.legacy_executor.LegacyExecutor._get_prompt_deps" _PATCH_SHIM = "executor.executors.legacy_executor.ExecutorToolShim" _PATCH_RUN_COMPLETION = ( "executor.executors.answer_prompt.AnswerPromptService.run_completion" ) -_PATCH_INDEX_UTILS = ( - "unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key" -) -_PATCH_PLUGIN_LOADER = ( - "executor.executors.plugins.loader.ExecutorPluginLoader.get" -) +_PATCH_INDEX_UTILS = "unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key" +_PATCH_PLUGIN_LOADER = "executor.executors.plugins.loader.ExecutorPluginLoader.get" # --------------------------------------------------------------------------- # Fixtures @@ -120,7 +111,9 @@ def _mock_prompt_deps(llm=None): if llm is None: llm = _mock_llm() - from executor.executors.answer_prompt import AnswerPromptService as answer_prompt_svc_cls + from executor.executors.answer_prompt import ( + AnswerPromptService as answer_prompt_svc_cls, + ) retrieval_svc = MagicMock(name="RetrievalService") retrieval_svc.run_retrieval.return_value = ["chunk1"] @@ -165,8 +158,12 @@ def _mock_process_response(text="ide extracted text"): ) -def _make_ide_prompt(name="invoice_number", prompt="What is the invoice number?", - output_type="text", **overrides): +def _make_ide_prompt( + name="invoice_number", + prompt="What is the invoice number?", + output_type="text", + **overrides, +): """Build a prompt dict matching what prompt_studio_helper builds. Uses the exact key strings from ToolStudioPromptKeys / PSKeys. @@ -677,14 +674,10 @@ def patched_send_task(name, args=None, kwargs=None, **opts): @patch(_PATCH_FS) @patch(_PATCH_X2TEXT) - def test_dispatcher_extract_round_trip( - self, mock_x2text_cls, mock_get_fs, eager_app - ): + def test_dispatcher_extract_round_trip(self, mock_x2text_cls, mock_get_fs, eager_app): """ExecutionDispatcher.dispatch() → extract → ExecutionResult.""" mock_x2text = MagicMock() - mock_x2text.process.return_value = _mock_process_response( - "dispatcher extracted" - ) + mock_x2text.process.return_value = _mock_process_response("dispatcher extracted") mock_x2text.x2text_instance = MagicMock() mock_x2text_cls.return_value = mock_x2text mock_get_fs.return_value = MagicMock() @@ -749,9 +742,7 @@ def test_dispatcher_single_pass_round_trip( @patch(_PATCH_FS) @patch(_PATCH_INDEX_DEPS) - def test_dispatcher_index_round_trip( - self, mock_deps, mock_get_fs, eager_app - ): + def test_dispatcher_index_round_trip(self, mock_deps, mock_get_fs, eager_app): """ExecutionDispatcher.dispatch() → index → ExecutionResult.""" mock_index_cls = MagicMock() mock_index = MagicMock() diff --git a/workers/tests/test_sanity_phase6g.py b/workers/tests/test_simple_prompt_studio_operation.py similarity index 85% rename from workers/tests/test_sanity_phase6g.py rename to workers/tests/test_simple_prompt_studio_operation.py index fe8da04832..926430e238 100644 --- a/workers/tests/test_sanity_phase6g.py +++ b/workers/tests/test_simple_prompt_studio_operation.py @@ -1,51 +1,24 @@ -"""Phase 6G Sanity — SimplePromptStudioExecutor + SPS operations. - -Verifies: -1. Operation.SPS_ANSWER_PROMPT enum exists with value "sps_answer_prompt" -2. Operation.SPS_INDEX enum exists with value "sps_index" -3. Mock SimplePromptStudioExecutor — registration and execution -4. Queue routing: executor_name="simple_prompt_studio" → celery_executor_simple_prompt_studio -5. LegacyExecutor does NOT handle sps_answer_prompt or sps_index -6. Dispatch sends to correct queue -7. SimplePromptStudioExecutor rejects unsupported operations +"""SPS operations (sps_answer_prompt, sps_index): the simple-prompt-studio +executor registers, routes to its own queue, and LegacyExecutor rejects them. """ from unittest.mock import MagicMock - -from unstract.sdk1.execution.context import ExecutionContext, Operation +from unstract.sdk1.execution.context import ExecutionContext from unstract.sdk1.execution.dispatcher import ExecutionDispatcher from unstract.sdk1.execution.executor import BaseExecutor from unstract.sdk1.execution.registry import ExecutorRegistry from unstract.sdk1.execution.result import ExecutionResult - # --------------------------------------------------------------------------- -# 1. Operation enums +# Mock SimplePromptStudioExecutor — registration and execution # --------------------------------------------------------------------------- -class TestSPSOperations: - def test_sps_answer_prompt_enum_exists(self): - assert hasattr(Operation, "SPS_ANSWER_PROMPT") - assert Operation.SPS_ANSWER_PROMPT.value == "sps_answer_prompt" - - def test_sps_index_enum_exists(self): - assert hasattr(Operation, "SPS_INDEX") - assert Operation.SPS_INDEX.value == "sps_index" - - def test_sps_operations_in_operation_values(self): - values = {op.value for op in Operation} - assert "sps_answer_prompt" in values - assert "sps_index" in values - - -# --------------------------------------------------------------------------- -# 2. Mock SimplePromptStudioExecutor — registration and execution -# --------------------------------------------------------------------------- class TestSimplePromptStudioRegistration: def test_mock_sps_executor_registers_and_executes(self): """Simulate cloud executor discovery and execution.""" + @ExecutorRegistry.register class MockSPSExecutor(BaseExecutor): _OPERATION_MAP = { @@ -127,6 +100,7 @@ def _handle_index(self, context): # 3. Queue routing # --------------------------------------------------------------------------- + class TestSPSQueueRouting: def test_sps_routes_to_correct_queue(self): queue = ExecutionDispatcher._get_queue("simple_prompt_studio") @@ -181,13 +155,16 @@ def test_dispatch_sps_index_to_correct_queue(self, tmp_path): # 4. LegacyExecutor does NOT handle SPS operations # --------------------------------------------------------------------------- + class TestLegacyExcludesSPS: def test_sps_answer_prompt_not_in_legacy_operation_map(self): from executor.executors.legacy_executor import LegacyExecutor + assert "sps_answer_prompt" not in LegacyExecutor._OPERATION_MAP def test_sps_index_not_in_legacy_operation_map(self): from executor.executors.legacy_executor import LegacyExecutor + assert "sps_index" not in LegacyExecutor._OPERATION_MAP def test_legacy_returns_failure_for_sps_answer_prompt(self): @@ -233,6 +210,7 @@ def test_legacy_returns_failure_for_sps_index(self): # 5. tasks.py log_component for SPS operations # --------------------------------------------------------------------------- + class TestTasksLogComponent: def test_sps_answer_prompt_uses_default_log_component(self): """SPS operations use the default log_component branch in tasks.py.""" @@ -253,8 +231,12 @@ def test_sps_answer_prompt_uses_default_log_component(self): params = context.executor_params # SPS operations fall through to the default branch - assert context.operation not in ("ide_index", "structure_pipeline", - "table_extract", "smart_table_extract") + assert context.operation not in ( + "ide_index", + "structure_pipeline", + "table_extract", + "smart_table_extract", + ) component = { "tool_id": params.get("tool_id", ""), "run_id": context.run_id, @@ -285,8 +267,12 @@ def test_sps_index_uses_default_log_component(self): context = ExecutionContext.from_dict(ctx_dict) params = context.executor_params - assert context.operation not in ("ide_index", "structure_pipeline", - "table_extract", "smart_table_extract") + assert context.operation not in ( + "ide_index", + "structure_pipeline", + "table_extract", + "smart_table_extract", + ) component = { "tool_id": params.get("tool_id", ""), "run_id": context.run_id, diff --git a/workers/tests/test_phase2f.py b/workers/tests/test_single_pass_extraction.py similarity index 91% rename from workers/tests/test_phase2f.py rename to workers/tests/test_single_pass_extraction.py index a5913367c1..3c52b9b456 100644 --- a/workers/tests/test_phase2f.py +++ b/workers/tests/test_single_pass_extraction.py @@ -1,4 +1,4 @@ -"""Phase 2F — single_pass_extraction, summarize, agentic operations tests. +"""single_pass_extraction, summarize, agentic operations tests. Verifies: 1. single_pass_extraction delegates to answer_prompt @@ -11,7 +11,6 @@ from unittest.mock import MagicMock, patch import pytest - from unstract.sdk1.execution.context import ExecutionContext from unstract.sdk1.execution.registry import ExecutorRegistry from unstract.sdk1.execution.result import ExecutionResult @@ -117,13 +116,13 @@ def test_summarize_success(self, mock_shim_cls, mock_get_deps): mock_llm_cls.return_value = mock_llm mock_get_deps.return_value = ( - MagicMock(), # AnswerPromptService - MagicMock(), # RetrievalService - MagicMock(), # VariableReplacementService - MagicMock(), # Index + MagicMock(), # AnswerPromptService + MagicMock(), # RetrievalService + MagicMock(), # VariableReplacementService + MagicMock(), # Index mock_llm_cls, # LLM - MagicMock(), # EmbeddingCompat - MagicMock(), # VectorDB + MagicMock(), # EmbeddingCompat + MagicMock(), # VectorDB ) # Mock AnswerPromptService.run_completion @@ -152,8 +151,13 @@ def test_summarize_prompt_includes_keys(self, mock_shim_cls, mock_get_deps): mock_llm_cls.return_value = mock_llm mock_get_deps.return_value = ( - MagicMock(), MagicMock(), MagicMock(), MagicMock(), - mock_llm_cls, MagicMock(), MagicMock(), + MagicMock(), + MagicMock(), + MagicMock(), + MagicMock(), + mock_llm_cls, + MagicMock(), + MagicMock(), ) captured_prompt = {} @@ -188,8 +192,13 @@ def test_summarize_no_prompt_keys(self, mock_shim_cls, mock_get_deps): mock_llm_cls.return_value = MagicMock() mock_get_deps.return_value = ( - MagicMock(), MagicMock(), MagicMock(), MagicMock(), - mock_llm_cls, MagicMock(), MagicMock(), + MagicMock(), + MagicMock(), + MagicMock(), + MagicMock(), + mock_llm_cls, + MagicMock(), + MagicMock(), ) with patch( @@ -248,8 +257,13 @@ def test_summarize_llm_error(self, mock_shim_cls, mock_get_deps): mock_llm_cls.return_value = MagicMock() mock_get_deps.return_value = ( - MagicMock(), MagicMock(), MagicMock(), MagicMock(), - mock_llm_cls, MagicMock(), MagicMock(), + MagicMock(), + MagicMock(), + MagicMock(), + MagicMock(), + mock_llm_cls, + MagicMock(), + MagicMock(), ) with patch( @@ -279,8 +293,13 @@ def test_summarize_creates_llm_with_correct_adapter( mock_llm_cls.return_value = mock_llm mock_get_deps.return_value = ( - MagicMock(), MagicMock(), MagicMock(), MagicMock(), - mock_llm_cls, MagicMock(), MagicMock(), + MagicMock(), + MagicMock(), + MagicMock(), + MagicMock(), + mock_llm_cls, + MagicMock(), + MagicMock(), ) with patch( diff --git a/workers/tests/test_sanity_phase6f.py b/workers/tests/test_smart_table_extract_operation.py similarity index 84% rename from workers/tests/test_sanity_phase6f.py rename to workers/tests/test_smart_table_extract_operation.py index cf565e692f..608e5c8b7e 100644 --- a/workers/tests/test_sanity_phase6f.py +++ b/workers/tests/test_smart_table_extract_operation.py @@ -1,41 +1,19 @@ -"""Phase 6F Sanity — SmartTableExtractorExecutor + SMART_TABLE_EXTRACT operation. - -Verifies: -1. Operation.SMART_TABLE_EXTRACT enum exists with value "smart_table_extract" -2. tasks.py log_component builder handles smart_table_extract operation -3. Mock SmartTableExtractorExecutor — registration and execution -4. Queue routing: executor_name="smart_table" → celery_executor_smart_table -5. LegacyExecutor does NOT handle smart_table_extract -6. Dispatch sends to correct queue +"""SMART_TABLE_EXTRACT operation: the smart-table executor registers, routes to +its own queue, and LegacyExecutor rejects the operation. """ from unittest.mock import MagicMock - -from unstract.sdk1.execution.context import ExecutionContext, Operation +from unstract.sdk1.execution.context import ExecutionContext from unstract.sdk1.execution.dispatcher import ExecutionDispatcher from unstract.sdk1.execution.executor import BaseExecutor from unstract.sdk1.execution.registry import ExecutorRegistry from unstract.sdk1.execution.result import ExecutionResult - # --------------------------------------------------------------------------- -# 1. Operation enum +# tasks.py log_component for smart_table_extract # --------------------------------------------------------------------------- -class TestSmartTableExtractOperation: - def test_smart_table_extract_enum_exists(self): - assert hasattr(Operation, "SMART_TABLE_EXTRACT") - assert Operation.SMART_TABLE_EXTRACT.value == "smart_table_extract" - - def test_smart_table_extract_in_operation_values(self): - values = {op.value for op in Operation} - assert "smart_table_extract" in values - - -# --------------------------------------------------------------------------- -# 2. tasks.py log_component for smart_table_extract -# --------------------------------------------------------------------------- class TestTasksLogComponent: def test_smart_table_extract_log_component(self): @@ -77,9 +55,11 @@ def test_smart_table_extract_log_component(self): # 3. Mock SmartTableExtractorExecutor — registration and execution # --------------------------------------------------------------------------- + class TestSmartTableExtractorRegistration: def test_mock_smart_table_executor_registers_and_executes(self): """Simulate cloud executor discovery and execution.""" + @ExecutorRegistry.register class MockSmartTableExecutor(BaseExecutor): @property @@ -134,6 +114,7 @@ def execute(self, context): # 4. Queue routing # --------------------------------------------------------------------------- + class TestSmartTableQueueRouting: def test_smart_table_routes_to_correct_queue(self): queue = ExecutionDispatcher._get_queue("smart_table") @@ -166,9 +147,11 @@ def test_dispatch_sends_to_smart_table_queue(self): # 5. LegacyExecutor does NOT handle smart_table_extract # --------------------------------------------------------------------------- + class TestLegacyExcludesSmartTable: def test_smart_table_extract_not_in_legacy_operation_map(self): from executor.executors.legacy_executor import LegacyExecutor + assert "smart_table_extract" not in LegacyExecutor._OPERATION_MAP def test_legacy_returns_failure_for_smart_table_extract(self): diff --git a/workers/tests/test_phase5d.py b/workers/tests/test_structure_pipeline.py similarity index 65% rename from workers/tests/test_phase5d.py rename to workers/tests/test_structure_pipeline.py index 8b3252c9e6..6ece5c23b2 100644 --- a/workers/tests/test_phase5d.py +++ b/workers/tests/test_structure_pipeline.py @@ -1,4 +1,4 @@ -"""Phase 5D — Tests for structure_pipeline compound operation. +"""Tests for structure_pipeline compound operation. Tests _handle_structure_pipeline in LegacyExecutor which runs the full extract → summarize → index → answer_prompt pipeline in a single @@ -8,7 +8,6 @@ from unittest.mock import MagicMock, patch import pytest - from unstract.sdk1.execution.context import ExecutionContext, Operation from unstract.sdk1.execution.result import ExecutionResult @@ -20,9 +19,7 @@ _PATCH_INDEXING_DEPS = ( "executor.executors.legacy_executor.LegacyExecutor._get_indexing_deps" ) -_PATCH_PROMPT_DEPS = ( - "executor.executors.legacy_executor.LegacyExecutor._get_prompt_deps" -) +_PATCH_PROMPT_DEPS = "executor.executors.legacy_executor.LegacyExecutor._get_prompt_deps" # --------------------------------------------------------------------------- @@ -169,9 +166,7 @@ def test_extract_index_answer(self, executor): extract_result = ExecutionResult( success=True, data={"extracted_text": "Revenue is $1M"} ) - index_result = ExecutionResult( - success=True, data={"doc_id": "doc-1"} - ) + index_result = ExecutionResult(success=True, data={"doc_id": "doc-1"}) answer_result = ExecutionResult( success=True, data={ @@ -183,16 +178,16 @@ def test_extract_index_answer(self, executor): executor._handle_extract = MagicMock(return_value=extract_result) executor._handle_index = MagicMock(return_value=index_result) - executor._handle_answer_prompt = MagicMock( - return_value=answer_result - ) + executor._handle_answer_prompt = MagicMock(return_value=answer_result) - ctx = _make_pipeline_context({ - "extract_params": _base_extract_params(), - "index_template": _base_index_template(), - "answer_params": _base_answer_params(), - "pipeline_options": _base_pipeline_options(), - }) + ctx = _make_pipeline_context( + { + "extract_params": _base_extract_params(), + "index_template": _base_index_template(), + "answer_params": _base_answer_params(), + "pipeline_options": _base_pipeline_options(), + } + ) result = executor._handle_structure_pipeline(ctx) @@ -204,14 +199,10 @@ def test_extract_index_answer(self, executor): def test_result_has_metadata_and_file_name(self, executor): """Result includes source_file_name in metadata.""" executor._handle_extract = MagicMock( - return_value=ExecutionResult( - success=True, data={"extracted_text": "text"} - ) + return_value=ExecutionResult(success=True, data={"extracted_text": "text"}) ) executor._handle_index = MagicMock( - return_value=ExecutionResult( - success=True, data={"doc_id": "d1"} - ) + return_value=ExecutionResult(success=True, data={"doc_id": "d1"}) ) executor._handle_answer_prompt = MagicMock( return_value=ExecutionResult( @@ -219,12 +210,14 @@ def test_result_has_metadata_and_file_name(self, executor): ) ) - ctx = _make_pipeline_context({ - "extract_params": _base_extract_params(), - "index_template": _base_index_template(), - "answer_params": _base_answer_params(), - "pipeline_options": _base_pipeline_options(), - }) + ctx = _make_pipeline_context( + { + "extract_params": _base_extract_params(), + "index_template": _base_index_template(), + "answer_params": _base_answer_params(), + "pipeline_options": _base_pipeline_options(), + } + ) result = executor._handle_structure_pipeline(ctx) assert result.success @@ -238,22 +231,20 @@ def test_extracted_text_in_metadata(self, executor): ) ) executor._handle_index = MagicMock( - return_value=ExecutionResult( - success=True, data={"doc_id": "d1"} - ) + return_value=ExecutionResult(success=True, data={"doc_id": "d1"}) ) executor._handle_answer_prompt = MagicMock( - return_value=ExecutionResult( - success=True, data={"output": {}} - ) + return_value=ExecutionResult(success=True, data={"output": {}}) ) - ctx = _make_pipeline_context({ - "extract_params": _base_extract_params(), - "index_template": _base_index_template(), - "answer_params": _base_answer_params(), - "pipeline_options": _base_pipeline_options(), - }) + ctx = _make_pipeline_context( + { + "extract_params": _base_extract_params(), + "index_template": _base_index_template(), + "answer_params": _base_answer_params(), + "pipeline_options": _base_pipeline_options(), + } + ) result = executor._handle_structure_pipeline(ctx) assert result.data["metadata"]["extracted_text"] == "Revenue $1M" @@ -261,14 +252,10 @@ def test_extracted_text_in_metadata(self, executor): def test_index_metrics_merged(self, executor): """Index metrics are merged into answer metrics.""" executor._handle_extract = MagicMock( - return_value=ExecutionResult( - success=True, data={"extracted_text": "text"} - ) + return_value=ExecutionResult(success=True, data={"extracted_text": "text"}) ) executor._handle_index = MagicMock( - return_value=ExecutionResult( - success=True, data={"doc_id": "d1"} - ) + return_value=ExecutionResult(success=True, data={"doc_id": "d1"}) ) executor._handle_answer_prompt = MagicMock( return_value=ExecutionResult( @@ -289,12 +276,14 @@ def test_index_metrics_merged(self, executor): ) ) - ctx = _make_pipeline_context({ - "extract_params": _base_extract_params(), - "index_template": _base_index_template(), - "answer_params": _base_answer_params(), - "pipeline_options": _base_pipeline_options(), - }) + ctx = _make_pipeline_context( + { + "extract_params": _base_extract_params(), + "index_template": _base_index_template(), + "answer_params": _base_answer_params(), + "pipeline_options": _base_pipeline_options(), + } + ) result = executor._handle_structure_pipeline(ctx) assert result.success @@ -319,9 +308,7 @@ class TestPipelineUsageRecordsPropagation: def test_index_usage_records_propagate(self, executor): executor._handle_extract = MagicMock( - return_value=ExecutionResult( - success=True, data={"extracted_text": "text"} - ) + return_value=ExecutionResult(success=True, data={"extracted_text": "text"}) ) embedding_row = { "model_name": "emb-1", @@ -346,12 +333,14 @@ def test_index_usage_records_propagate(self, executor): ) ) - ctx = _make_pipeline_context({ - "extract_params": _base_extract_params(), - "index_template": _base_index_template(), - "answer_params": _base_answer_params(), - "pipeline_options": _base_pipeline_options(), - }) + ctx = _make_pipeline_context( + { + "extract_params": _base_extract_params(), + "index_template": _base_index_template(), + "answer_params": _base_answer_params(), + "pipeline_options": _base_pipeline_options(), + } + ) result = executor._handle_structure_pipeline(ctx) assert result.success @@ -362,9 +351,7 @@ def test_index_returned_failure_aborts_pipeline(self, executor): from executor.executors.exceptions import LegacyExecutorError executor._handle_extract = MagicMock( - return_value=ExecutionResult( - success=True, data={"extracted_text": "text"} - ) + return_value=ExecutionResult(success=True, data={"extracted_text": "text"}) ) executor._handle_index = MagicMock( return_value=ExecutionResult.failure( @@ -373,12 +360,14 @@ def test_index_returned_failure_aborts_pipeline(self, executor): ) executor._handle_answer_prompt = MagicMock() - ctx = _make_pipeline_context({ - "extract_params": _base_extract_params(), - "index_template": _base_index_template(), - "answer_params": _base_answer_params(), - "pipeline_options": _base_pipeline_options(), - }) + ctx = _make_pipeline_context( + { + "extract_params": _base_extract_params(), + "index_template": _base_index_template(), + "answer_params": _base_answer_params(), + "pipeline_options": _base_pipeline_options(), + } + ) with pytest.raises(LegacyExecutorError) as exc_info: executor._handle_structure_pipeline(ctx) @@ -402,12 +391,14 @@ def test_extract_failure_stops_pipeline(self, executor): executor._handle_index = MagicMock() executor._handle_answer_prompt = MagicMock() - ctx = _make_pipeline_context({ - "extract_params": _base_extract_params(), - "index_template": _base_index_template(), - "answer_params": _base_answer_params(), - "pipeline_options": _base_pipeline_options(), - }) + ctx = _make_pipeline_context( + { + "extract_params": _base_extract_params(), + "index_template": _base_index_template(), + "answer_params": _base_answer_params(), + "pipeline_options": _base_pipeline_options(), + } + ) result = executor._handle_structure_pipeline(ctx) assert not result.success @@ -428,21 +419,21 @@ def test_skip_extraction_uses_input_file(self, executor): executor._handle_extract = MagicMock() executor._handle_index = MagicMock() executor._handle_answer_prompt = MagicMock( - return_value=ExecutionResult( - success=True, data={"output": {}} - ) + return_value=ExecutionResult(success=True, data={"output": {}}) ) opts = _base_pipeline_options() opts["skip_extraction_and_indexing"] = True answer = _base_answer_params() - ctx = _make_pipeline_context({ - "extract_params": _base_extract_params(), - "index_template": _base_index_template(), - "answer_params": answer, - "pipeline_options": opts, - }) + ctx = _make_pipeline_context( + { + "extract_params": _base_extract_params(), + "index_template": _base_index_template(), + "answer_params": answer, + "pipeline_options": opts, + } + ) result = executor._handle_structure_pipeline(ctx) assert result.success @@ -455,9 +446,7 @@ def test_skip_extraction_uses_input_file(self, executor): def test_skip_extraction_table_settings_injection(self, executor): """Table settings get input_file when extraction is skipped.""" executor._handle_answer_prompt = MagicMock( - return_value=ExecutionResult( - success=True, data={"output": {}} - ) + return_value=ExecutionResult(success=True, data={"output": {}}) ) opts = _base_pipeline_options() @@ -467,12 +456,14 @@ def test_skip_extraction_table_settings_injection(self, executor): "is_directory_mode": False, } - ctx = _make_pipeline_context({ - "extract_params": _base_extract_params(), - "index_template": _base_index_template(), - "answer_params": answer, - "pipeline_options": opts, - }) + ctx = _make_pipeline_context( + { + "extract_params": _base_extract_params(), + "index_template": _base_index_template(), + "answer_params": answer, + "pipeline_options": opts, + } + ) result = executor._handle_structure_pipeline(ctx) assert result.success @@ -490,26 +481,24 @@ class TestSinglePass: def test_single_pass_skips_index(self, executor): executor._handle_extract = MagicMock( - return_value=ExecutionResult( - success=True, data={"extracted_text": "text"} - ) + return_value=ExecutionResult(success=True, data={"extracted_text": "text"}) ) executor._handle_index = MagicMock() executor._handle_answer_prompt = MagicMock( - return_value=ExecutionResult( - success=True, data={"output": {}} - ) + return_value=ExecutionResult(success=True, data={"output": {}}) ) opts = _base_pipeline_options() opts["is_single_pass_enabled"] = True - ctx = _make_pipeline_context({ - "extract_params": _base_extract_params(), - "index_template": _base_index_template(), - "answer_params": _base_answer_params(), - "pipeline_options": opts, - }) + ctx = _make_pipeline_context( + { + "extract_params": _base_extract_params(), + "index_template": _base_index_template(), + "answer_params": _base_answer_params(), + "pipeline_options": opts, + } + ) result = executor._handle_structure_pipeline(ctx) assert result.success @@ -520,25 +509,23 @@ def test_single_pass_skips_index(self, executor): def test_single_pass_operation_is_single_pass(self, executor): """The answer_prompt call uses single_pass_extraction operation.""" executor._handle_extract = MagicMock( - return_value=ExecutionResult( - success=True, data={"extracted_text": "text"} - ) + return_value=ExecutionResult(success=True, data={"extracted_text": "text"}) ) executor._handle_answer_prompt = MagicMock( - return_value=ExecutionResult( - success=True, data={"output": {}} - ) + return_value=ExecutionResult(success=True, data={"output": {}}) ) opts = _base_pipeline_options() opts["is_single_pass_enabled"] = True - ctx = _make_pipeline_context({ - "extract_params": _base_extract_params(), - "index_template": _base_index_template(), - "answer_params": _base_answer_params(), - "pipeline_options": opts, - }) + ctx = _make_pipeline_context( + { + "extract_params": _base_extract_params(), + "index_template": _base_index_template(), + "answer_params": _base_answer_params(), + "pipeline_options": opts, + } + ) executor._handle_structure_pipeline(ctx) call_ctx = executor._handle_answer_prompt.call_args[0][0] @@ -554,47 +541,41 @@ class TestSummarizePipeline: """Summarize: extract + summarize + answer_prompt (no indexing).""" @patch(_PATCH_FILE_UTILS) - def test_summarize_calls_handle_summarize( - self, mock_get_fs, executor, mock_fs - ): + def test_summarize_calls_handle_summarize(self, mock_get_fs, executor, mock_fs): mock_get_fs.return_value = mock_fs mock_fs.exists.return_value = False mock_fs.read.return_value = "extracted text for summarize" executor._handle_extract = MagicMock( - return_value=ExecutionResult( - success=True, data={"extracted_text": "text"} - ) + return_value=ExecutionResult(success=True, data={"extracted_text": "text"}) ) executor._handle_summarize = MagicMock( - return_value=ExecutionResult( - success=True, data={"data": "summarized text"} - ) + return_value=ExecutionResult(success=True, data={"data": "summarized text"}) ) executor._handle_index = MagicMock() executor._handle_answer_prompt = MagicMock( - return_value=ExecutionResult( - success=True, data={"output": {}} - ) + return_value=ExecutionResult(success=True, data={"output": {}}) ) opts = _base_pipeline_options() opts["is_summarization_enabled"] = True - ctx = _make_pipeline_context({ - "extract_params": _base_extract_params(), - "index_template": _base_index_template(), - "answer_params": _base_answer_params(), - "pipeline_options": opts, - "summarize_params": { - "llm_adapter_instance_id": "llm-1", - "summarize_prompt": "Summarize this", - "extract_file_path": "/data/exec/EXTRACT", - "summarize_file_path": "/data/exec/SUMMARIZE", - "platform_api_key": "sk-test", - "prompt_keys": ["field_a"], - }, - }) + ctx = _make_pipeline_context( + { + "extract_params": _base_extract_params(), + "index_template": _base_index_template(), + "answer_params": _base_answer_params(), + "pipeline_options": opts, + "summarize_params": { + "llm_adapter_instance_id": "llm-1", + "summarize_prompt": "Summarize this", + "extract_file_path": "/data/exec/EXTRACT", + "summarize_file_path": "/data/exec/SUMMARIZE", + "platform_api_key": "sk-test", + "prompt_keys": ["field_a"], + }, + } + ) result = executor._handle_structure_pipeline(ctx) assert result.success @@ -609,43 +590,39 @@ def test_summarize_uses_cache(self, mock_get_fs, executor, mock_fs): mock_fs.read.return_value = "cached summary" executor._handle_extract = MagicMock( - return_value=ExecutionResult( - success=True, data={"extracted_text": "text"} - ) + return_value=ExecutionResult(success=True, data={"extracted_text": "text"}) ) executor._handle_summarize = MagicMock() executor._handle_answer_prompt = MagicMock( - return_value=ExecutionResult( - success=True, data={"output": {}} - ) + return_value=ExecutionResult(success=True, data={"output": {}}) ) opts = _base_pipeline_options() opts["is_summarization_enabled"] = True - ctx = _make_pipeline_context({ - "extract_params": _base_extract_params(), - "index_template": _base_index_template(), - "answer_params": _base_answer_params(), - "pipeline_options": opts, - "summarize_params": { - "llm_adapter_instance_id": "llm-1", - "summarize_prompt": "Summarize this", - "extract_file_path": "/data/exec/EXTRACT", - "summarize_file_path": "/data/exec/SUMMARIZE", - "platform_api_key": "sk-test", - "prompt_keys": ["field_a"], - }, - }) + ctx = _make_pipeline_context( + { + "extract_params": _base_extract_params(), + "index_template": _base_index_template(), + "answer_params": _base_answer_params(), + "pipeline_options": opts, + "summarize_params": { + "llm_adapter_instance_id": "llm-1", + "summarize_prompt": "Summarize this", + "extract_file_path": "/data/exec/EXTRACT", + "summarize_file_path": "/data/exec/SUMMARIZE", + "platform_api_key": "sk-test", + "prompt_keys": ["field_a"], + }, + } + ) result = executor._handle_structure_pipeline(ctx) assert result.success executor._handle_summarize.assert_not_called() @patch(_PATCH_FILE_UTILS) - def test_summarize_updates_answer_params( - self, mock_get_fs, executor, mock_fs - ): + def test_summarize_updates_answer_params(self, mock_get_fs, executor, mock_fs): """After summarize, answer_params file_path and hash are updated.""" mock_get_fs.return_value = mock_fs mock_fs.exists.return_value = False @@ -653,39 +630,35 @@ def test_summarize_updates_answer_params( mock_fs.get_hash_from_file.return_value = "sum-hash-456" executor._handle_extract = MagicMock( - return_value=ExecutionResult( - success=True, data={"extracted_text": "text"} - ) + return_value=ExecutionResult(success=True, data={"extracted_text": "text"}) ) executor._handle_summarize = MagicMock( - return_value=ExecutionResult( - success=True, data={"data": "summarized"} - ) + return_value=ExecutionResult(success=True, data={"data": "summarized"}) ) executor._handle_answer_prompt = MagicMock( - return_value=ExecutionResult( - success=True, data={"output": {}} - ) + return_value=ExecutionResult(success=True, data={"output": {}}) ) answer = _base_answer_params() opts = _base_pipeline_options() opts["is_summarization_enabled"] = True - ctx = _make_pipeline_context({ - "extract_params": _base_extract_params(), - "index_template": _base_index_template(), - "answer_params": answer, - "pipeline_options": opts, - "summarize_params": { - "llm_adapter_instance_id": "llm-1", - "summarize_prompt": "Summarize", - "extract_file_path": "/data/exec/EXTRACT", - "summarize_file_path": "/data/exec/SUMMARIZE", - "platform_api_key": "sk-test", - "prompt_keys": [], - }, - }) + ctx = _make_pipeline_context( + { + "extract_params": _base_extract_params(), + "index_template": _base_index_template(), + "answer_params": answer, + "pipeline_options": opts, + "summarize_params": { + "llm_adapter_instance_id": "llm-1", + "summarize_prompt": "Summarize", + "extract_file_path": "/data/exec/EXTRACT", + "summarize_file_path": "/data/exec/SUMMARIZE", + "platform_api_key": "sk-test", + "prompt_keys": [], + }, + } + ) executor._handle_structure_pipeline(ctx) # Check answer_params were updated @@ -693,43 +666,39 @@ def test_summarize_updates_answer_params( assert answer["file_path"] == "/data/exec/SUMMARIZE" @patch(_PATCH_FILE_UTILS) - def test_summarize_sets_chunk_size_zero( - self, mock_get_fs, executor, mock_fs - ): + def test_summarize_sets_chunk_size_zero(self, mock_get_fs, executor, mock_fs): """Summarize sets chunk-size=0 for all outputs.""" mock_get_fs.return_value = mock_fs mock_fs.exists.return_value = True mock_fs.read.return_value = "cached" executor._handle_extract = MagicMock( - return_value=ExecutionResult( - success=True, data={"extracted_text": "t"} - ) + return_value=ExecutionResult(success=True, data={"extracted_text": "t"}) ) executor._handle_answer_prompt = MagicMock( - return_value=ExecutionResult( - success=True, data={"output": {}} - ) + return_value=ExecutionResult(success=True, data={"output": {}}) ) answer = _base_answer_params() opts = _base_pipeline_options() opts["is_summarization_enabled"] = True - ctx = _make_pipeline_context({ - "extract_params": _base_extract_params(), - "index_template": _base_index_template(), - "answer_params": answer, - "pipeline_options": opts, - "summarize_params": { - "llm_adapter_instance_id": "llm-1", - "summarize_prompt": "Summarize", - "extract_file_path": "/data/exec/EXTRACT", - "summarize_file_path": "/data/exec/SUMMARIZE", - "platform_api_key": "sk-test", - "prompt_keys": [], - }, - }) + ctx = _make_pipeline_context( + { + "extract_params": _base_extract_params(), + "index_template": _base_index_template(), + "answer_params": answer, + "pipeline_options": opts, + "summarize_params": { + "llm_adapter_instance_id": "llm-1", + "summarize_prompt": "Summarize", + "extract_file_path": "/data/exec/EXTRACT", + "summarize_file_path": "/data/exec/SUMMARIZE", + "platform_api_key": "sk-test", + "prompt_keys": [], + }, + } + ) executor._handle_structure_pipeline(ctx) # Outputs should have chunk-size=0 @@ -749,9 +718,7 @@ class TestIndexDedup: def test_index_dedup_skips_duplicate_params(self, executor): """Duplicate param combos are only indexed once.""" executor._handle_extract = MagicMock( - return_value=ExecutionResult( - success=True, data={"extracted_text": "text"} - ) + return_value=ExecutionResult(success=True, data={"extracted_text": "text"}) ) index_call_count = 0 @@ -762,32 +729,34 @@ def counting_index(ctx): executor._handle_index = counting_index executor._handle_answer_prompt = MagicMock( - return_value=ExecutionResult( - success=True, data={"output": {}} - ) + return_value=ExecutionResult(success=True, data={"output": {}}) ) answer = _base_answer_params() # Add a second output with same adapter params - answer["outputs"].append({ - "name": "field_b", - "prompt": "What is the profit?", - "type": "text", - "active": True, - "chunk-size": 512, - "chunk-overlap": 128, - "llm": "llm-1", - "embedding": "emb-1", - "vector-db": "vdb-1", - "x2text_adapter": "x2t-1", - }) - - ctx = _make_pipeline_context({ - "extract_params": _base_extract_params(), - "index_template": _base_index_template(), - "answer_params": answer, - "pipeline_options": _base_pipeline_options(), - }) + answer["outputs"].append( + { + "name": "field_b", + "prompt": "What is the profit?", + "type": "text", + "active": True, + "chunk-size": 512, + "chunk-overlap": 128, + "llm": "llm-1", + "embedding": "emb-1", + "vector-db": "vdb-1", + "x2text_adapter": "x2t-1", + } + ) + + ctx = _make_pipeline_context( + { + "extract_params": _base_extract_params(), + "index_template": _base_index_template(), + "answer_params": answer, + "pipeline_options": _base_pipeline_options(), + } + ) result = executor._handle_structure_pipeline(ctx) assert result.success @@ -797,9 +766,7 @@ def counting_index(ctx): def test_index_different_params_indexes_both(self, executor): """Different param combos are indexed separately.""" executor._handle_extract = MagicMock( - return_value=ExecutionResult( - success=True, data={"extracted_text": "text"} - ) + return_value=ExecutionResult(success=True, data={"extracted_text": "text"}) ) index_call_count = 0 @@ -810,31 +777,33 @@ def counting_index(ctx): executor._handle_index = counting_index executor._handle_answer_prompt = MagicMock( - return_value=ExecutionResult( - success=True, data={"output": {}} - ) + return_value=ExecutionResult(success=True, data={"output": {}}) ) answer = _base_answer_params() - answer["outputs"].append({ - "name": "field_b", - "prompt": "What is the profit?", - "type": "text", - "active": True, - "chunk-size": 256, # Different chunk size - "chunk-overlap": 64, - "llm": "llm-1", - "embedding": "emb-1", - "vector-db": "vdb-1", - "x2text_adapter": "x2t-1", - }) - - ctx = _make_pipeline_context({ - "extract_params": _base_extract_params(), - "index_template": _base_index_template(), - "answer_params": answer, - "pipeline_options": _base_pipeline_options(), - }) + answer["outputs"].append( + { + "name": "field_b", + "prompt": "What is the profit?", + "type": "text", + "active": True, + "chunk-size": 256, # Different chunk size + "chunk-overlap": 64, + "llm": "llm-1", + "embedding": "emb-1", + "vector-db": "vdb-1", + "x2text_adapter": "x2t-1", + } + ) + + ctx = _make_pipeline_context( + { + "extract_params": _base_extract_params(), + "index_template": _base_index_template(), + "answer_params": answer, + "pipeline_options": _base_pipeline_options(), + } + ) result = executor._handle_structure_pipeline(ctx) assert result.success @@ -843,26 +812,24 @@ def counting_index(ctx): def test_chunk_size_zero_skips_index(self, executor): """chunk-size=0 outputs skip indexing entirely.""" executor._handle_extract = MagicMock( - return_value=ExecutionResult( - success=True, data={"extracted_text": "text"} - ) + return_value=ExecutionResult(success=True, data={"extracted_text": "text"}) ) executor._handle_index = MagicMock() executor._handle_answer_prompt = MagicMock( - return_value=ExecutionResult( - success=True, data={"output": {}} - ) + return_value=ExecutionResult(success=True, data={"output": {}}) ) answer = _base_answer_params() answer["outputs"][0]["chunk-size"] = 0 - ctx = _make_pipeline_context({ - "extract_params": _base_extract_params(), - "index_template": _base_index_template(), - "answer_params": answer, - "pipeline_options": _base_pipeline_options(), - }) + ctx = _make_pipeline_context( + { + "extract_params": _base_extract_params(), + "index_template": _base_index_template(), + "answer_params": answer, + "pipeline_options": _base_pipeline_options(), + } + ) result = executor._handle_structure_pipeline(ctx) assert result.success @@ -879,25 +846,23 @@ class TestAnswerPromptFailure: def test_answer_failure_propagates(self, executor): executor._handle_extract = MagicMock( - return_value=ExecutionResult( - success=True, data={"extracted_text": "text"} - ) + return_value=ExecutionResult(success=True, data={"extracted_text": "text"}) ) executor._handle_index = MagicMock( - return_value=ExecutionResult( - success=True, data={"doc_id": "d1"} - ) + return_value=ExecutionResult(success=True, data={"doc_id": "d1"}) ) executor._handle_answer_prompt = MagicMock( return_value=ExecutionResult.failure(error="LLM timeout") ) - ctx = _make_pipeline_context({ - "extract_params": _base_extract_params(), - "index_template": _base_index_template(), - "answer_params": _base_answer_params(), - "pipeline_options": _base_pipeline_options(), - }) + ctx = _make_pipeline_context( + { + "extract_params": _base_extract_params(), + "index_template": _base_index_template(), + "answer_params": _base_answer_params(), + "pipeline_options": _base_pipeline_options(), + } + ) result = executor._handle_structure_pipeline(ctx) assert not result.success @@ -913,21 +878,15 @@ class TestMergeMetrics: """Test _merge_pipeline_metrics.""" def test_merge_disjoint(self, executor): - m = executor._merge_pipeline_metrics( - {"a": {"x": 1}}, {"b": {"y": 2}} - ) + m = executor._merge_pipeline_metrics({"a": {"x": 1}}, {"b": {"y": 2}}) assert m == {"a": {"x": 1}, "b": {"y": 2}} def test_merge_overlapping(self, executor): - m = executor._merge_pipeline_metrics( - {"a": {"x": 1}}, {"a": {"y": 2}} - ) + m = executor._merge_pipeline_metrics({"a": {"x": 1}}, {"a": {"y": 2}}) assert m == {"a": {"x": 1, "y": 2}} def test_merge_non_dict_values(self, executor): - m = executor._merge_pipeline_metrics( - {"a": 1}, {"b": 2} - ) + m = executor._merge_pipeline_metrics({"a": 1}, {"b": 2}) assert m == {"a": 1, "b": 2} @@ -942,19 +901,13 @@ class TestSubContextCreation: def test_extract_context_inherits_fields(self, executor): """Extract sub-context gets run_id, org_id, etc. from parent.""" executor._handle_extract = MagicMock( - return_value=ExecutionResult( - success=True, data={"extracted_text": "text"} - ) + return_value=ExecutionResult(success=True, data={"extracted_text": "text"}) ) executor._handle_index = MagicMock( - return_value=ExecutionResult( - success=True, data={"doc_id": "d1"} - ) + return_value=ExecutionResult(success=True, data={"doc_id": "d1"}) ) executor._handle_answer_prompt = MagicMock( - return_value=ExecutionResult( - success=True, data={"output": {}} - ) + return_value=ExecutionResult(success=True, data={"output": {}}) ) ctx = _make_pipeline_context( diff --git a/workers/tests/test_sanity_phase3.py b/workers/tests/test_structure_tool_pipeline.py similarity index 97% rename from workers/tests/test_sanity_phase3.py rename to workers/tests/test_structure_tool_pipeline.py index d9cf076980..ca77ab2fd9 100644 --- a/workers/tests/test_sanity_phase3.py +++ b/workers/tests/test_structure_tool_pipeline.py @@ -1,4 +1,4 @@ -"""Phase 3-SANITY — Integration tests for the structure tool Celery task. +"""Integration tests for the structure tool Celery task. Tests the full structure tool pipeline with mocked platform API and ExecutionDispatcher. After Phase 5E, the structure tool task dispatches a @@ -10,7 +10,6 @@ from unittest.mock import MagicMock, patch import pytest - from shared.enums.task_enums import TaskName from unstract.sdk1.execution.result import ExecutionResult @@ -18,18 +17,10 @@ # Patch targets # --------------------------------------------------------------------------- -_PATCH_DISPATCHER = ( - "file_processing.structure_tool_task.ExecutionDispatcher" -) -_PATCH_PLATFORM_HELPER = ( - "file_processing.structure_tool_task._create_platform_helper" -) -_PATCH_FILE_STORAGE = ( - "file_processing.structure_tool_task._get_file_storage" -) -_PATCH_SHIM = ( - "executor.executor_tool_shim.ExecutorToolShim" -) +_PATCH_DISPATCHER = "file_processing.structure_tool_task.ExecutionDispatcher" +_PATCH_PLATFORM_HELPER = "file_processing.structure_tool_task._create_platform_helper" +_PATCH_FILE_STORAGE = "file_processing.structure_tool_task._get_file_storage" +_PATCH_SHIM = "executor.executor_tool_shim.ExecutorToolShim" _PATCH_SERVICE_IS_STRUCTURE = ( "shared.workflow.execution.service." "WorkerWorkflowExecutionService._is_structure_tool_workflow" @@ -343,9 +334,7 @@ def test_structure_tool_summarize_flow( "tool_metadata": tool_metadata_regular, } - tool_metadata_regular["tool_settings"]["summarize_prompt"] = ( - "Summarize this doc" - ) + tool_metadata_regular["tool_settings"]["summarize_prompt"] = "Summarize this doc" base_params["tool_instance_metadata"]["summarize_as_source"] = True dispatcher_instance = MagicMock() @@ -777,7 +766,9 @@ def test_no_summarize_params_when_disabled( ctx = dispatcher_instance.dispatch.call_args[0][0] assert ctx.executor_params["summarize_params"] is None - assert ctx.executor_params["pipeline_options"]["is_summarization_enabled"] is False + assert ( + ctx.executor_params["pipeline_options"]["is_summarization_enabled"] is False + ) class TestWorkflowServiceDetection: @@ -895,9 +886,7 @@ class TestStructureToolParamsPassthrough: "_is_structure_tool_workflow", return_value=True, ) - def test_structure_tool_params_passthrough( - self, mock_is_struct, mock_exec_struct - ): + def test_structure_tool_params_passthrough(self, mock_is_struct, mock_exec_struct): from shared.workflow.execution.service import ( WorkerWorkflowExecutionService, ) @@ -910,9 +899,7 @@ def test_structure_tool_params_passthrough( service._build_and_execute_workflow(mock_exec_service, "test.pdf") # Verify _execute_structure_tool_workflow was called - mock_exec_struct.assert_called_once_with( - mock_exec_service, "test.pdf" - ) + mock_exec_struct.assert_called_once_with(mock_exec_service, "test.pdf") @patch( "shared.workflow.execution.service.WorkerWorkflowExecutionService." @@ -960,9 +947,7 @@ def test_source_file_name_uses_real_filename_not_sentinel( mock_execute_struct.return_value = {"success": True} - service._execute_structure_tool_workflow( - mock_exec_service, "invoice.pdf" - ) + service._execute_structure_tool_workflow(mock_exec_service, "invoice.pdf") # The dispatched params dict must carry the real file name, # not the "SOURCE" sentinel from file_handler.source_file. @@ -1011,10 +996,7 @@ def test_should_skip_extraction_no_table_settings(self): ) outputs = [{"name": "field_a", "prompt": "What?"}] - assert ( - _should_skip_extraction_for_smart_table(outputs) - is False - ) + assert _should_skip_extraction_for_smart_table(outputs) is False def test_should_skip_extraction_with_json_schema(self): from file_processing.structure_tool_task import ( @@ -1028,7 +1010,4 @@ def test_should_skip_extraction_with_json_schema(self): "prompt": '{"col1": "string", "col2": "number"}', } ] - assert ( - _should_skip_extraction_for_smart_table(outputs) - is True - ) + assert _should_skip_extraction_for_smart_table(outputs) is True diff --git a/workers/tests/test_sanity_phase6i.py b/workers/tests/test_summarize_operation.py similarity index 97% rename from workers/tests/test_sanity_phase6i.py rename to workers/tests/test_summarize_operation.py index 635dfa7ca3..4308387cc0 100644 --- a/workers/tests/test_sanity_phase6i.py +++ b/workers/tests/test_summarize_operation.py @@ -1,4 +1,4 @@ -"""Phase 6I Sanity — Backend Summarizer Migration. +"""Backend Summarizer Migration. Verifies: 1. Summarize operation exists and routes through LegacyExecutor @@ -11,13 +11,11 @@ from unittest.mock import MagicMock, patch import pytest - from unstract.sdk1.execution.context import ExecutionContext, Operation from unstract.sdk1.execution.dispatcher import ExecutionDispatcher from unstract.sdk1.execution.registry import ExecutorRegistry from unstract.sdk1.execution.result import ExecutionResult - # Patches _PATCH_GET_PROMPT_DEPS = ( "executor.executors.legacy_executor.LegacyExecutor._get_prompt_deps" @@ -26,6 +24,7 @@ def _register_legacy(): from executor.executors.legacy_executor import LegacyExecutor + ExecutorRegistry.clear() ExecutorRegistry.register(LegacyExecutor) @@ -34,6 +33,7 @@ def _register_legacy(): # 1. Summarize operation enum # --------------------------------------------------------------------------- + class TestSummarizeOperation: def test_summarize_enum_exists(self): assert hasattr(Operation, "SUMMARIZE") @@ -41,6 +41,7 @@ def test_summarize_enum_exists(self): def test_summarize_in_legacy_operation_map(self): from executor.executors.legacy_executor import LegacyExecutor + assert "summarize" in LegacyExecutor._OPERATION_MAP @@ -48,10 +49,12 @@ def test_summarize_in_legacy_operation_map(self): # 2. Executor params contract # --------------------------------------------------------------------------- + class TestSummarizeParamsContract: def test_summarize_params_match_handler_expectations(self): """Verify the params the backend summarizer sends match - what _handle_summarize expects.""" + what _handle_summarize expects. + """ # These are the keys the cloud summarizer.py now sends backend_params = { "llm_adapter_instance_id": "llm-uuid", @@ -73,6 +76,7 @@ def test_summarize_params_match_handler_expectations(self): # 3. Queue routing # --------------------------------------------------------------------------- + class TestSummarizeQueueRouting: def test_summarize_routes_to_legacy_queue(self): """Summarize dispatches to celery_executor_legacy (LegacyExecutor).""" @@ -115,6 +119,7 @@ def test_dispatch_sends_summarize_to_legacy_queue(self): # 4. Result shape # --------------------------------------------------------------------------- + class TestSummarizeResultShape: @patch(_PATCH_GET_PROMPT_DEPS) def test_summarize_returns_data_key(self, mock_deps): @@ -128,7 +133,7 @@ def test_summarize_returns_data_key(self, mock_deps): MagicMock(), # PostProcessor MagicMock(), # VariableReplacement MagicMock(), # JsonRepair - mock_llm, # LLM + mock_llm, # LLM MagicMock(), # Embedding MagicMock(), # VectorDB ) @@ -211,6 +216,7 @@ def test_summarize_missing_llm_returns_failure(self, mock_deps): # 5. Full Celery chain # --------------------------------------------------------------------------- + @pytest.fixture def eager_app(): """Configure executor Celery app for eager-mode testing.""" @@ -239,8 +245,13 @@ def test_summarize_full_celery_chain(self, mock_deps, eager_app): mock_llm.return_value = mock_llm_instance mock_deps.return_value = ( - MagicMock(), MagicMock(), MagicMock(), MagicMock(), - mock_llm, MagicMock(), MagicMock(), + MagicMock(), + MagicMock(), + MagicMock(), + MagicMock(), + mock_llm, + MagicMock(), + MagicMock(), ) with patch( diff --git a/workers/tests/test_sanity_phase6e.py b/workers/tests/test_table_extract_operation.py similarity index 86% rename from workers/tests/test_sanity_phase6e.py rename to workers/tests/test_table_extract_operation.py index 302540b666..9169212984 100644 --- a/workers/tests/test_sanity_phase6e.py +++ b/workers/tests/test_table_extract_operation.py @@ -1,45 +1,22 @@ -"""Phase 6E Sanity — TableExtractorExecutor + TABLE_EXTRACT operation. - -Verifies: -1. Operation.TABLE_EXTRACT enum exists with value "table_extract" -2. tasks.py log_component builder handles table_extract operation -3. TableExtractorExecutor mock — registration via entry point -4. TableExtractorExecutor mock — dispatch to correct queue -5. LegacyExecutor excludes table_extract from its _OPERATION_MAP -6. Cloud executor entry point name matches pyproject.toml +"""TABLE_EXTRACT operation: the table executor registers via entry point, +routes to its own queue, and LegacyExecutor rejects the operation. """ from unittest.mock import MagicMock - -from unstract.sdk1.execution.context import ExecutionContext, Operation +from unstract.sdk1.execution.context import ExecutionContext from unstract.sdk1.execution.dispatcher import ExecutionDispatcher from unstract.sdk1.execution.registry import ExecutorRegistry from unstract.sdk1.execution.result import ExecutionResult - # --------------------------------------------------------------------------- -# 1. Operation enum +# tasks.py log_component for table_extract # --------------------------------------------------------------------------- -class TestTableExtractOperation: - def test_table_extract_enum_exists(self): - assert hasattr(Operation, "TABLE_EXTRACT") - assert Operation.TABLE_EXTRACT.value == "table_extract" - - def test_table_extract_in_operation_values(self): - values = {op.value for op in Operation} - assert "table_extract" in values - - -# --------------------------------------------------------------------------- -# 2. tasks.py log_component for table_extract -# --------------------------------------------------------------------------- class TestTasksLogComponent: def test_table_extract_log_component(self): """tasks.py builds correct log_component for table_extract.""" - # Build a mock context dict ctx_dict = { "executor_name": "table", @@ -81,6 +58,7 @@ def test_table_extract_log_component(self): # 3. Mock TableExtractorExecutor — entry point registration # --------------------------------------------------------------------------- + class TestTableExtractorExecutorRegistration: def test_mock_table_executor_discovered_via_entry_point(self): """Simulate cloud executor discovery via entry point.""" @@ -140,6 +118,7 @@ def execute(self, context): # 4. Queue routing for table executor # --------------------------------------------------------------------------- + class TestTableQueueRouting: def test_table_executor_routes_to_correct_queue(self): """executor_name='table' routes to celery_executor_table queue.""" @@ -174,6 +153,7 @@ def test_dispatch_sends_to_table_queue(self): # 5. LegacyExecutor does NOT handle table_extract # --------------------------------------------------------------------------- + class TestLegacyExcludesTable: def test_table_extract_not_in_legacy_operation_map(self): """LegacyExecutor._OPERATION_MAP should NOT contain table_extract.""" @@ -206,6 +186,7 @@ def test_legacy_returns_failure_for_table_extract(self): # 6. Entry point name verification # --------------------------------------------------------------------------- + class TestEntryPointConfig: def test_entry_point_name_is_table(self): """The pyproject.toml entry point name should be 'table'.""" diff --git a/workers/tests/test_sanity_phase6d.py b/workers/tests/test_table_lineitem_challenge_eval.py similarity index 84% rename from workers/tests/test_sanity_phase6d.py rename to workers/tests/test_table_lineitem_challenge_eval.py index 46d2693465..ed4288cbe5 100644 --- a/workers/tests/test_sanity_phase6d.py +++ b/workers/tests/test_table_lineitem_challenge_eval.py @@ -1,4 +1,4 @@ -"""Phase 6D Sanity — LegacyExecutor plugin integration. +"""LegacyExecutor plugin integration. Verifies: 1. TABLE type raises LegacyExecutorError with routing guidance @@ -22,11 +22,11 @@ from executor.executors.exceptions import LegacyExecutorError from unstract.sdk1.execution.result import ExecutionResult - # --------------------------------------------------------------------------- # Helpers # --------------------------------------------------------------------------- + def _make_context( output_type="TEXT", enable_highlight=False, @@ -110,17 +110,14 @@ def _standard_patches(executor, mock_llm_instance): mock_llm_cls = MagicMock(return_value=mock_llm_instance) return { "_get_prompt_deps": patch.object( - executor, "_get_prompt_deps", + executor, + "_get_prompt_deps", return_value=( AnswerPromptService, MagicMock( - retrieve_complete_context=MagicMock( - return_value=["context chunk"] - ) - ), - MagicMock( - is_variables_present=MagicMock(return_value=False) + retrieve_complete_context=MagicMock(return_value=["context chunk"]) ), + MagicMock(is_variables_present=MagicMock(return_value=False)), None, # Index mock_llm_cls, MagicMock(), # EmbeddingCompat @@ -142,13 +139,14 @@ def _standard_patches(executor, mock_llm_instance): # 1. TABLE type raises with routing guidance # --------------------------------------------------------------------------- + class TestTableLineItemGuard: @patch("executor.executors.legacy_executor.ExecutorToolShim") - @patch("unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", - return_value="doc-id-1") - def test_table_type_delegates_to_table_executor( - self, mock_key, mock_shim_cls - ): + @patch( + "unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", + return_value="doc-id-1", + ) + def test_table_type_delegates_to_table_executor(self, mock_key, mock_shim_cls): """TABLE prompts are delegated to TableExtractorExecutor in-process.""" mock_shim_cls.return_value = MagicMock() executor = _get_executor() @@ -178,11 +176,11 @@ def test_table_type_delegates_to_table_executor( assert sub_ctx.operation == "table_extract" @patch("executor.executors.legacy_executor.ExecutorToolShim") - @patch("unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", - return_value="doc-id-1") - def test_table_type_raises_when_plugin_missing( - self, mock_key, mock_shim_cls - ): + @patch( + "unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", + return_value="doc-id-1", + ) + def test_table_type_raises_when_plugin_missing(self, mock_key, mock_shim_cls): """TABLE prompts raise error when table executor plugin is not installed.""" mock_shim_cls.return_value = MagicMock() executor = _get_executor() @@ -199,11 +197,11 @@ def test_table_type_raises_when_plugin_missing( executor._handle_answer_prompt(ctx) @patch("executor.executors.legacy_executor.ExecutorToolShim") - @patch("unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", - return_value="doc-id-1") - def test_line_item_type_raises_when_plugin_missing( - self, mock_key, mock_shim_cls - ): + @patch( + "unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", + return_value="doc-id-1", + ) + def test_line_item_type_raises_when_plugin_missing(self, mock_key, mock_shim_cls): """LINE_ITEM prompts raise install error when line_item plugin is not registered (mirrors TABLE missing-plugin behavior). """ @@ -216,9 +214,7 @@ def test_line_item_type_raises_when_plugin_missing( with patches["_get_prompt_deps"], patches["shim"], patches["index_key"]: with patch( "unstract.sdk1.execution.registry.ExecutorRegistry.get", - side_effect=KeyError( - "No executor registered with name 'line_item'" - ), + side_effect=KeyError("No executor registered with name 'line_item'"), ): with pytest.raises( LegacyExecutorError, match="line_item_extractor plugin" @@ -230,13 +226,14 @@ def test_line_item_type_raises_when_plugin_missing( # 2. Challenge plugin integration # --------------------------------------------------------------------------- + class TestChallengeIntegration: @patch("executor.executors.legacy_executor.ExecutorToolShim") - @patch("unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", - return_value="doc-id-1") - def test_challenge_invoked_when_enabled_and_installed( - self, mock_key, mock_shim_cls - ): + @patch( + "unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", + return_value="doc-id-1", + ) + def test_challenge_invoked_when_enabled_and_installed(self, mock_key, mock_shim_cls): """Challenge plugin is instantiated and run() called.""" mock_shim_cls.return_value = MagicMock() executor = _get_executor() @@ -271,11 +268,11 @@ def test_challenge_invoked_when_enabled_and_installed( mock_challenger.run.assert_called_once() @patch("executor.executors.legacy_executor.ExecutorToolShim") - @patch("unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", - return_value="doc-id-1") - def test_challenge_skipped_when_plugin_not_installed( - self, mock_key, mock_shim_cls - ): + @patch( + "unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", + return_value="doc-id-1", + ) + def test_challenge_skipped_when_plugin_not_installed(self, mock_key, mock_shim_cls): """When challenge enabled but plugin missing, no error.""" mock_shim_cls.return_value = MagicMock() executor = _get_executor() @@ -297,11 +294,11 @@ def test_challenge_skipped_when_plugin_not_installed( assert result.success @patch("executor.executors.legacy_executor.ExecutorToolShim") - @patch("unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", - return_value="doc-id-1") - def test_challenge_skipped_when_disabled( - self, mock_key, mock_shim_cls - ): + @patch( + "unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", + return_value="doc-id-1", + ) + def test_challenge_skipped_when_disabled(self, mock_key, mock_shim_cls): """When enable_challenge=False, plugin loader not called for challenge.""" mock_shim_cls.return_value = MagicMock() executor = _get_executor() @@ -327,11 +324,11 @@ def test_challenge_skipped_when_disabled( ) @patch("executor.executors.legacy_executor.ExecutorToolShim") - @patch("unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", - return_value="doc-id-1") - def test_challenge_skipped_when_no_challenge_llm( - self, mock_key, mock_shim_cls - ): + @patch( + "unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", + return_value="doc-id-1", + ) + def test_challenge_skipped_when_no_challenge_llm(self, mock_key, mock_shim_cls): """When enable_challenge=True but no challenge_llm, skip challenge.""" mock_shim_cls.return_value = MagicMock() executor = _get_executor() @@ -361,19 +358,18 @@ def test_challenge_skipped_when_no_challenge_llm( # 3. Evaluation plugin integration # --------------------------------------------------------------------------- + class TestEvaluationIntegration: @patch("executor.executors.legacy_executor.ExecutorToolShim") - @patch("unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", - return_value="doc-id-1") - def test_evaluation_invoked_when_enabled_and_installed( - self, mock_key, mock_shim_cls - ): + @patch( + "unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", + return_value="doc-id-1", + ) + def test_evaluation_invoked_when_enabled_and_installed(self, mock_key, mock_shim_cls): """Evaluation plugin is instantiated and run() called.""" mock_shim_cls.return_value = MagicMock() executor = _get_executor() - ctx = _make_context( - eval_settings={PSKeys.EVAL_SETTINGS_EVALUATE: True} - ) + ctx = _make_context(eval_settings={PSKeys.EVAL_SETTINGS_EVALUATE: True}) llm = _mock_llm() mock_eval_cls = MagicMock() mock_evaluator = MagicMock() @@ -386,9 +382,7 @@ def test_evaluation_invoked_when_enabled_and_installed( patches["index_key"], patch( "executor.executors.plugins.loader.ExecutorPluginLoader.get", - side_effect=lambda name: ( - mock_eval_cls if name == "evaluation" else None - ), + side_effect=lambda name: mock_eval_cls if name == "evaluation" else None, ), ): result = executor._handle_answer_prompt(ctx) @@ -401,17 +395,15 @@ def test_evaluation_invoked_when_enabled_and_installed( mock_evaluator.run.assert_called_once() @patch("executor.executors.legacy_executor.ExecutorToolShim") - @patch("unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", - return_value="doc-id-1") - def test_evaluation_skipped_when_plugin_not_installed( - self, mock_key, mock_shim_cls - ): + @patch( + "unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", + return_value="doc-id-1", + ) + def test_evaluation_skipped_when_plugin_not_installed(self, mock_key, mock_shim_cls): """When evaluation enabled but plugin missing, no error.""" mock_shim_cls.return_value = MagicMock() executor = _get_executor() - ctx = _make_context( - eval_settings={PSKeys.EVAL_SETTINGS_EVALUATE: True} - ) + ctx = _make_context(eval_settings={PSKeys.EVAL_SETTINGS_EVALUATE: True}) llm = _mock_llm() patches = _standard_patches(executor, llm) @@ -429,11 +421,11 @@ def test_evaluation_skipped_when_plugin_not_installed( assert result.success @patch("executor.executors.legacy_executor.ExecutorToolShim") - @patch("unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", - return_value="doc-id-1") - def test_evaluation_skipped_when_not_enabled( - self, mock_key, mock_shim_cls - ): + @patch( + "unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", + return_value="doc-id-1", + ) + def test_evaluation_skipped_when_not_enabled(self, mock_key, mock_shim_cls): """When no eval_settings or evaluate=False, evaluation skipped.""" mock_shim_cls.return_value = MagicMock() executor = _get_executor() @@ -464,13 +456,14 @@ def test_evaluation_skipped_when_not_enabled( # 4. Challenge runs before evaluation (ordering) # --------------------------------------------------------------------------- + class TestChallengeBeforeEvaluation: @patch("executor.executors.legacy_executor.ExecutorToolShim") - @patch("unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", - return_value="doc-id-1") - def test_challenge_runs_before_evaluation( - self, mock_key, mock_shim_cls - ): + @patch( + "unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", + return_value="doc-id-1", + ) + def test_challenge_runs_before_evaluation(self, mock_key, mock_shim_cls): """Challenge mutates structured_output before evaluation reads it.""" mock_shim_cls.return_value = MagicMock() executor = _get_executor() @@ -521,13 +514,14 @@ def plugin_get(name): # 5. Challenge mutates structured_output # --------------------------------------------------------------------------- + class TestChallengeMutation: @patch("executor.executors.legacy_executor.ExecutorToolShim") - @patch("unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", - return_value="doc-id-1") - def test_challenge_mutates_structured_output( - self, mock_key, mock_shim_cls - ): + @patch( + "unstract.sdk1.utils.indexing.IndexingUtils.generate_index_key", + return_value="doc-id-1", + ) + def test_challenge_mutates_structured_output(self, mock_key, mock_shim_cls): """Challenge plugin can mutate structured_output dict.""" mock_shim_cls.return_value = MagicMock() executor = _get_executor() diff --git a/workers/tests/test_phase2h.py b/workers/tests/test_variable_replacement_postprocessor.py similarity index 99% rename from workers/tests/test_phase2h.py rename to workers/tests/test_variable_replacement_postprocessor.py index cca39a3710..cbd2215fb8 100644 --- a/workers/tests/test_phase2h.py +++ b/workers/tests/test_variable_replacement_postprocessor.py @@ -1,4 +1,4 @@ -"""Phase 2H: Tests for variable replacement and postprocessor modules. +"""Tests for variable replacement and postprocessor modules. Covers VariableReplacementHelper, VariableReplacementService, and the webhook postprocessor — all pure Python with no llama_index deps. From 0d1d7e2806f9ef8940a1e095fbb759af1ad8d8bb Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Tue, 14 Jul 2026 18:16:33 +0530 Subject: [PATCH 2/4] UN-3636 [DEV] tighten test-rig group comments to concise, generic WHY Drop stale-prone specifics (function names, file:line refs, vendor lists, class names) from groups.yaml comments; keep the WHY. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_019EN8hh518CbCVMP3BHTVmG --- tests/groups.yaml | 44 ++++++++++++++------------------------------ 1 file changed, 14 insertions(+), 30 deletions(-) diff --git a/tests/groups.yaml b/tests/groups.yaml index 22a7b92f69..138924421f 100644 --- a/tests/groups.yaml +++ b/tests/groups.yaml @@ -17,8 +17,7 @@ defaults: groups: # ── Unit tier: pure pytest, no external services ─────────────────────────── unit-rig: - # Self-tests for the rig's pure logic (dep-graph, evaluate, junit parse). - # Run from the repo root so the `tests.rig` package import resolves. + # Rig self-tests. Run from repo root so the `tests.rig` import resolves. tier: unit workdir: . paths: [tests/rig/tests] @@ -53,20 +52,17 @@ groups: unit-backend: tier: unit workdir: backend - # Pure backend tests — no DB. Collect the whole tree and let markers, not a - # hand-kept file list, decide membership: tests needing live infra carry - # `@pytest.mark.integration` (see integration-backend) and are excluded here. + # Marker-selected, not a hand-kept file list: live-infra tests carry + # `integration` and run in integration-backend. paths: ["."] markers: "not integration" uv_sync_group: test - # Anchored: integration-backend reuses the identical Django settings env so - # the two halves of the backend suite can't drift apart. + # Shared with integration-backend so both halves use identical settings. env: &backend_test_env DJANGO_SETTINGS_MODULE: backend.settings.test # Tenancy is row-level, not schema-per-tenant; tests run in public. DB_SCHEMA: public - # base.py resolves these at import time with no default; supply test-safe - # values here. + # Resolved at import with no default; supply test-safe values. DJANGO_SECRET_KEY: test-secret-key-not-for-production # All-zero Fernet key: valid format, obviously a placeholder. ENCRYPTION_KEY: AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA= @@ -81,8 +77,7 @@ groups: SYSTEM_ADMIN_USERNAME: admin SYSTEM_ADMIN_PASSWORD: admin SYSTEM_ADMIN_EMAIL: admin@example.com - # ExecutionFileHandler builds execution-dir paths from this at init; only - # constructed, never written to in these tests. + # Only used to build paths at init; never written in these tests. WORKFLOW_EXECUTION_DIR_PREFIX: /tmp/unstract-workflow-exec coverage_source: . @@ -108,13 +103,9 @@ groups: integration-backend: tier: integration workdir: backend - # Backend ORM tests — need live infra. The rig provisions the declared - # requires_services via testcontainers and injects their connection env into - # pytest (tests/rig/cli.py:_inject_infra_env). Not optional: these gate the - # integration tier. - # Marker-only selection — exact complement of unit-backend: conftest.py - # auto-marks DB-bound tests `integration`, so new DB tests join this - # group with no edit here. Tests needing external creds skip without them. + # Complement of unit-backend: conftest auto-marks DB-bound tests + # `integration`, so new ones join here with no edit. The rig provisions + # requires_services and injects their env; cred-gated tests skip without. paths: ["."] markers: "integration" uv_sync_group: test @@ -127,10 +118,8 @@ groups: workdir: unstract/connectors paths: [tests] markers: "integration" - # Most connector integration tests need real third-party credentials - # (Snowflake, GDrive, Box, Dropbox, …) and skip when those are absent. The - # MinIO test actually runs: the rig provisions MinIO via testcontainers and - # injects MINIO_* creds (tests/rig/cli.py). + # Most need real third-party creds and skip when absent; MinIO runs + # because the rig provisions it and injects the creds. requires_services: [minio] uv_sync_group: test coverage_source: src @@ -143,12 +132,8 @@ groups: optional: true # placeholder until tests are authored # ── E2E tier: real HTTP against running platform ─────────────────────────── - # Every group below has `requires_platform: true` and must (transitively) - # depend on `e2e-smoke` — the rig's `_validate_platform_groups_depend_on_gate` - # enforces this structural invariant so the manifest can't ship a platform - # test that bypasses the smoke gate, and so smoke runs first in topo order. - # (Runtime skip-on-gate-failure is a future enhancement; see that function's - # docstring and the TODO in cmd_run.) + # Platform groups must depend (transitively) on e2e-smoke; the rig enforces + # this so smoke gates and runs first. Nothing ships bypassing it. e2e-smoke: tier: e2e paths: [tests/e2e/smoke] @@ -157,8 +142,7 @@ groups: timeout_seconds: 1200 depends_on: [unit-sdk1, unit-workers] - # Not optional: login is the first e2e critical path we gate on. A red login - # test fails the build and (on main) surfaces as an in-scope critical gap. + # Not optional: login is a gated critical path; red fails the build. e2e-login: tier: e2e paths: [tests/e2e/auth] From f66b9dbd9faef805f6e75bbcfbea223d0b26209b Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Tue, 14 Jul 2026 18:58:44 +0530 Subject: [PATCH 3/4] UN-3636 [DEV] address review: real log_component tests, tighten assertions - Replace the log_component replica (copied tasks.py if/elif, drifted so the ide_index case asserted the opposite of production) with tests that drive the real execute_extraction, covering the two special-cased branches. - Match the highlight-plugin assertion on positional or kwarg calls so it can't pass vacuously if the call style changes. - Reword the unit-workers marker note: integration/slow worker tests are currently unrouted, not run "elsewhere". - Cross-reference the two workers conftests on the DB_* env they set vs strip. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_019EN8hh518CbCVMP3BHTVmG --- tests/groups.yaml | 3 +- workers/conftest.py | 2 + workers/tests/conftest.py | 4 +- .../tests/test_completion_and_highlight.py | 2 +- .../tests/test_plugin_migration_regression.py | 121 ++++++------------ 5 files changed, 50 insertions(+), 82 deletions(-) diff --git a/tests/groups.yaml b/tests/groups.yaml index 138924421f..74c4c3944d 100644 --- a/tests/groups.yaml +++ b/tests/groups.yaml @@ -44,7 +44,8 @@ groups: workdir: workers paths: [tests, shared/tests] # Negative filter like the other unit groups: worker tests aren't tagged - # `unit`; live-infra tests carry `integration`/`slow` and run elsewhere. + # `unit`. Tests tagged `integration`/`slow` are excluded here but not yet + # routed to any group — add one before relying on those markers. markers: "not integration and not slow" uv_sync_group: test coverage_source: shared diff --git a/workers/conftest.py b/workers/conftest.py index 137e8304e1..f41cb43ef3 100644 --- a/workers/conftest.py +++ b/workers/conftest.py @@ -12,6 +12,8 @@ os.environ.setdefault("CELERY_BROKER_BASE_URL", "amqp://localhost:5672//") os.environ.setdefault("CELERY_BROKER_USER", "guest") os.environ.setdefault("CELERY_BROKER_PASS", "guest") +# tests/conftest.py strips these back out for workers/tests so Celery result +# backends stay in-memory; they remain in effect for shared/tests. os.environ.setdefault("DB_HOST", "localhost") os.environ.setdefault("DB_USER", "test") os.environ.setdefault("DB_PASSWORD", "test") diff --git a/workers/tests/conftest.py b/workers/tests/conftest.py index 2275686642..d6818d52ce 100644 --- a/workers/tests/conftest.py +++ b/workers/tests/conftest.py @@ -17,7 +17,9 @@ # Worker Celery apps build a Postgres result backend from DB_*/CELERY_BACKEND_DB_*. # Strip these before any app is imported so tests don't reach (or leak) a live DB -# the unit tier has no server for; eager results then stay in-memory. +# the unit tier has no server for; eager results then stay in-memory. This undoes +# the DB_* defaults set in ../conftest.py, which stay in effect for shared/tests +# (not covered by this file). for _var in ( "DB_HOST", "DB_PORT", diff --git a/workers/tests/test_completion_and_highlight.py b/workers/tests/test_completion_and_highlight.py index 2fdb73c850..8b42306651 100644 --- a/workers/tests/test_completion_and_highlight.py +++ b/workers/tests/test_completion_and_highlight.py @@ -439,7 +439,7 @@ def test_highlight_skipped_when_disabled(self, mock_index_key, mock_shim_cls): highlight_fetches = [ c for c in mock_plugin_get.call_args_list - if c.args and c.args[0] == "highlight-data" + if "highlight-data" in c.args or c.kwargs.get("name") == "highlight-data" ] assert highlight_fetches == [] # process_text should be None diff --git a/workers/tests/test_plugin_migration_regression.py b/workers/tests/test_plugin_migration_regression.py index a9bae8dad1..29d4d81e6b 100644 --- a/workers/tests/test_plugin_migration_regression.py +++ b/workers/tests/test_plugin_migration_regression.py @@ -545,64 +545,46 @@ def test_missing_executor_via_orchestrator(self): class TestLogComponentAllOperations: - """Verify tasks.py log_component builder handles all operation types.""" - - def _build_log_component(self, operation, executor_params=None): - """Simulate the tasks.py log_component logic.""" - params = executor_params or { - "tool_id": "t-1", - "file_name": "doc.pdf", - } - ctx = ExecutionContext.from_dict( - { - "executor_name": "legacy", - "operation": operation, - "run_id": "run-log", - "execution_source": "tool", - "executor_params": params, - "request_id": "req-1", - "log_events_id": "evt-1", - } - ) + """Verify tasks.py builds `_log_component` for the branches it special-cases. + Drives the real `execute_extraction` (orchestrator patched out) so drift in + the builder fails here instead of passing against a replica. + """ + + def _log_component(self, operation, params): + with patch("executor.tasks.ExecutionOrchestrator") as mock_orch_cls: + mock_orch = MagicMock() + mock_orch.execute.return_value = MagicMock( + success=True, to_dict=lambda: {"success": True} + ) + mock_orch_cls.return_value = mock_orch + + from executor.tasks import execute_extraction + + execute_extraction( + { + "executor_name": "legacy", + "operation": operation, + "run_id": "run-log", + "execution_source": "tool", + "log_events_id": "evt-1", + "executor_params": params, + } + ) + return mock_orch.execute.call_args[0][0]._log_component - # Replicate tasks.py logic - if ctx.operation == "ide_index": - extract_params = params.get("extract_params", {}) - return { - "tool_id": extract_params.get("tool_id", ""), - "run_id": ctx.run_id, - "doc_name": str(extract_params.get("file_name", "")), - "operation": ctx.operation, - } - elif ctx.operation == "structure_pipeline": - answer_params = params.get("answer_params", {}) - pipeline_opts = params.get("pipeline_options", {}) - return { - "tool_id": answer_params.get("tool_id", ""), - "run_id": ctx.run_id, - "doc_name": str(pipeline_opts.get("source_file_name", "")), - "operation": ctx.operation, - } - else: - return { - "tool_id": params.get("tool_id", ""), - "run_id": ctx.run_id, - "doc_name": str(params.get("file_name", "")), - "operation": ctx.operation, - } - - def test_ide_index_extracts_nested_params(self): - comp = self._build_log_component( + def test_ide_index_reads_nested_index_and_usage_params(self): + comp = self._log_component( "ide_index", { - "extract_params": {"tool_id": "t-nested", "file_name": "nested.pdf"}, + "index_params": {"tool_id": "t-nested"}, + "extract_params": {"usage_kwargs": {"file_name": "nested.pdf"}}, }, ) assert comp["tool_id"] == "t-nested" assert comp["doc_name"] == "nested.pdf" - def test_structure_pipeline_extracts_nested_params(self): - comp = self._build_log_component( + def test_structure_pipeline_reads_answer_and_pipeline_params(self): + comp = self._log_component( "structure_pipeline", { "answer_params": {"tool_id": "t-pipe"}, @@ -612,35 +594,16 @@ def test_structure_pipeline_extracts_nested_params(self): assert comp["tool_id"] == "t-pipe" assert comp["doc_name"] == "pipe.pdf" - def test_table_extract_uses_direct_params(self): - comp = self._build_log_component("table_extract") - assert comp["tool_id"] == "t-1" - assert comp["operation"] == "table_extract" - - def test_smart_table_extract_uses_direct_params(self): - comp = self._build_log_component("smart_table_extract") - assert comp["operation"] == "smart_table_extract" - - @pytest.mark.parametrize( - "op", - [ - "extract", - "index", - "answer_prompt", - "single_pass_extraction", - "summarize", - "sps_answer_prompt", - "sps_index", - "agentic_extract", - "agentic_summarize", - "agentic_compare", - ], - ) - def test_default_branch_for_standard_ops(self, op): - comp = self._build_log_component(op) - assert comp["tool_id"] == "t-1" - assert comp["doc_name"] == "doc.pdf" - assert comp["operation"] == op + def test_default_branch_reads_direct_params(self): + comp = self._log_component( + "table_extract", {"tool_id": "t-1", "file_name": "doc.pdf"} + ) + assert comp == { + "tool_id": "t-1", + "run_id": "run-log", + "doc_name": "doc.pdf", + "operation": "table_extract", + } # --------------------------------------------------------------------------- From 2a1acc635f8e28b807e9bc54220a0b28b0be4516 Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Tue, 14 Jul 2026 20:07:50 +0530 Subject: [PATCH 4/4] UN-3636 [DEV] fail the build on a non-optional group that collects nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A zero-collect group returns pytest exit 5, which the rig treats as non-failing — so a broken marker or moved path silently reported green (the very way unit-workers stayed dormant). Gate on it: a non-optional pytest group that collects zero tests now fails the overall exit. Legit zero-collects are exempt: optional groups, hurl (its exit 5 means "no files"), and dev runs with a --marker/--paths override. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_019EN8hh518CbCVMP3BHTVmG --- tests/rig/cli.py | 14 +++++++ tests/rig/tests/test_cli.py | 80 +++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+) diff --git a/tests/rig/cli.py b/tests/rig/cli.py index 8cc9e2c4c6..4bbc2e682e 100644 --- a/tests/rig/cli.py +++ b/tests/rig/cli.py @@ -484,6 +484,20 @@ def cmd_run(args: argparse.Namespace) -> int: and overall_exit == 0 ): overall_exit = exit_code + # A non-optional pytest group that collected zero tests (exit 5) is + # almost always a broken marker/path, not intent — fail rather than + # fold in green. Skip hurl (its exit 5 means "no files", a + # placeholder) and dev runs with a marker/paths override that may + # legitimately match nothing. + if ( + exit_code == 5 + and not group.optional + and group.runner == "pytest" + and not args.marker + and not args.paths + and overall_exit == 0 + ): + overall_exit = 1 # Belt-and-braces: if the junit attests to errors/failures the exit # code didn't (truncated junit → errors=1 with exit 0), the report # shows ❌ but exit would otherwise stay 0. Keep them in sync. diff --git a/tests/rig/tests/test_cli.py b/tests/rig/tests/test_cli.py index 4669488fcf..31d4b1f0ab 100644 --- a/tests/rig/tests/test_cli.py +++ b/tests/rig/tests/test_cli.py @@ -216,6 +216,86 @@ def fake_execute_group(group, **kwargs): ) +def _run_empty_group_scenario( + tmp_path: Path, monkeypatch, *, extra_args: list[str] +) -> int: + """Drive cmd_run with one non-optional pytest group that collects nothing + (exit 5). Returns the overall exit code.""" + from tests.rig.reporting import GroupResult + + test_dir = Path(__file__).parent + (tmp_path / "groups.yaml").write_text( + "version: 1\n" + "groups:\n" + " unit-empty:\n" + " tier: unit\n" + f" workdir: {test_dir}\n" + " paths: [.]\n" + ) + (tmp_path / "critical_paths.yaml").write_text("version: 1\npaths: []\n") + + import tests.rig.cli as cli_mod + import tests.rig.critical_paths as cp_mod + import tests.rig.groups as groups_mod + + monkeypatch.setattr(groups_mod, "DEFAULT_MANIFEST", tmp_path / "groups.yaml") + monkeypatch.setattr(cp_mod, "DEFAULT_REGISTRY", tmp_path / "critical_paths.yaml") + + def fake_execute_group(group, **kwargs): + result = GroupResult( + name=group.name, + tier=group.tier, + exit_code=5, # pytest "no tests collected" + passed=0, + failed=0, + errors=0, + skipped=0, + duration_seconds=0.01, + ) + return result, 5 + + monkeypatch.setattr(cli_mod, "_execute_group", fake_execute_group) + + args = cli_mod._build_parser().parse_args( + [ + "run", + "unit-empty", + "--no-coverage", + "--no-parallel", + "--reports-dir", + str(tmp_path / "reports"), + "--baseline", + str(tmp_path / "reports" / "previous-summary.json"), + *extra_args, + ] + ) + return cli_mod.cmd_run(args) + + +def test_empty_nonoptional_group_fails_build(tmp_path: Path, monkeypatch) -> None: + """A non-optional pytest group that collects zero tests is a broken + marker/path, not a pass — it must gate the overall exit.""" + exit_code = _run_empty_group_scenario(tmp_path, monkeypatch, extra_args=[]) + assert exit_code != 0, ( + "a non-optional group that collected nothing must fail the build; " + f"got exit_code={exit_code}" + ) + + +def test_empty_group_with_marker_override_does_not_fail( + tmp_path: Path, monkeypatch +) -> None: + """A ``--marker`` override may legitimately match nothing; the empty-collect + gate must not fire in that case.""" + exit_code = _run_empty_group_scenario( + tmp_path, monkeypatch, extra_args=["--marker", "some_marker"] + ) + assert exit_code == 0, ( + "an empty result under a marker override must not gate the build; " + f"got exit_code={exit_code}" + ) + + def _run_gap_scenario( tmp_path: Path, monkeypatch, *, covered_by: str, fail_on_gap: bool ) -> int: