From 6179561ac392b72c73b0a0e208b78f8b2b904e4e Mon Sep 17 00:00:00 2001 From: paullizer Date: Tue, 18 Aug 2026 15:35:44 -0400 Subject: [PATCH 1/2] Fix workflow document picker loading and make workspace documents per-task Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- application/single_app/config.py | 2 +- .../single_app/functions_group_workflows.py | 58 +- .../functions_personal_workflows.py | 70 ++- .../single_app/functions_workflow_runner.py | 86 ++- .../static/js/chat/chat-documents.js | 7 +- .../js/workspace/workspace_workflows.js | 558 ++++++++++++------ .../templates/group_workspaces.html | 4 +- .../single_app/templates/workspace.html | 4 +- .../test_workflow_task_document_actions.py | 509 ++++++++++++++++ .../test_workflow_task_sequence.py | 18 +- 10 files changed, 1082 insertions(+), 234 deletions(-) create mode 100644 functional_tests/test_workflow_task_document_actions.py diff --git a/application/single_app/config.py b/application/single_app/config.py index 5393dc20a..6abbbcb76 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -96,7 +96,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -VERSION = "0.250.224" +VERSION = "0.250.225" IS_DEVELOPMENT = is_development_env_enabled() SESSION_COOKIE_SAMESITE = os.getenv('SESSION_COOKIE_SAMESITE', 'Lax') diff --git a/application/single_app/functions_group_workflows.py b/application/single_app/functions_group_workflows.py index 1563c9b34..1163459c8 100644 --- a/application/single_app/functions_group_workflows.py +++ b/application/single_app/functions_group_workflows.py @@ -38,6 +38,7 @@ _normalize_bool, _normalize_document_action_config, _normalize_schedule, + _normalize_task_document_action_config, _normalize_text, _normalize_workflow_error_handling, _normalize_workflow_tasks, @@ -54,12 +55,9 @@ WORKFLOW_CONVERSATION_ACCESS_ERROR = 'Workflow conversation not found or access denied.' -def _normalize_group_document_action_config(group_id, workflow_data, existing_workflow=None, allow_empty_file_sync_targets=False): - action_config = _normalize_document_action_config( - workflow_data, - existing_workflow=existing_workflow, - allow_empty_file_sync_targets=allow_empty_file_sync_targets, - ) +def _apply_group_document_action_scope(group_id, action_config): + """Force a normalized document action to stay inside the owning group workspace.""" + action_config = action_config if isinstance(action_config, dict) else {'type': 'none'} if action_config.get('type') == 'none': return action_config @@ -69,6 +67,15 @@ def _normalize_group_document_action_config(group_id, workflow_data, existing_wo return action_config +def _normalize_group_document_action_config(group_id, workflow_data, existing_workflow=None, allow_empty_file_sync_targets=False): + action_config = _normalize_document_action_config( + workflow_data, + existing_workflow=existing_workflow, + allow_empty_file_sync_targets=allow_empty_file_sync_targets, + ) + return _apply_group_document_action_scope(group_id, action_config) + + def _normalize_group_workflow_conversation_id(group_id, workflow_data, existing_workflow=None): existing_workflow = existing_workflow if isinstance(existing_workflow, dict) else {} conversation_id = _normalize_text( @@ -440,6 +447,20 @@ def save_group_workflow(group_id, workflow_data, actor_user_id, user_info=None): workflow_name = _normalize_text(workflow_data.get('name'), 'Workflow name', required=True) description = _normalize_text(workflow_data.get('description'), 'Description') + file_sync = _normalize_file_sync_config( + actor_user_id, + group_id, + workflow_data, + existing_workflow=existing_workflow, + user_info=user_info, + ) + allow_empty_file_sync_targets = bool(file_sync.get('enabled') and file_sync.get('use_changed_documents')) + document_action = _normalize_group_document_action_config( + group_id, + workflow_data, + existing_workflow=existing_workflow, + allow_empty_file_sync_targets=allow_empty_file_sync_targets, + ) tasks = _normalize_workflow_tasks( workflow_data, existing_workflow=existing_workflow, @@ -450,6 +471,15 @@ def save_group_workflow(group_id, workflow_data, actor_user_id, user_info=None): settings=settings, ), max_tasks=get_workflow_max_tasks(settings), + task_document_action_normalizer=lambda action_payload: _apply_group_document_action_scope( + group_id, + _normalize_task_document_action_config( + action_payload, + allow_empty_file_sync_targets=allow_empty_file_sync_targets, + settings=settings, + ), + ), + default_document_action=document_action, ) task_prompt = _normalize_text( workflow_data.get('task_prompt') or (tasks[0].get('instructions') if tasks else ''), @@ -495,20 +525,6 @@ def save_group_workflow(group_id, workflow_data, actor_user_id, user_info=None): workflow_data.get('chat_capabilities_enabled', default_chat_capabilities_enabled), default=default_chat_capabilities_enabled, ) - file_sync = _normalize_file_sync_config( - actor_user_id, - group_id, - workflow_data, - existing_workflow=existing_workflow, - user_info=user_info, - ) - allow_empty_file_sync_targets = bool(file_sync.get('enabled') and file_sync.get('use_changed_documents')) - document_action = _normalize_group_document_action_config( - group_id, - workflow_data, - existing_workflow=existing_workflow, - allow_empty_file_sync_targets=allow_empty_file_sync_targets, - ) if trigger_type == 'file_sync': if not file_sync.get('enabled'): raise ValueError('Monitor File Sync Changes workflows require File Sync before run.') @@ -806,4 +822,4 @@ def delete_group_workflow(group_id, workflow_id): level=logging.WARNING, ) - return True \ No newline at end of file + return True diff --git a/application/single_app/functions_personal_workflows.py b/application/single_app/functions_personal_workflows.py index 3aaa75083..a0b459fb8 100644 --- a/application/single_app/functions_personal_workflows.py +++ b/application/single_app/functions_personal_workflows.py @@ -163,6 +163,8 @@ def _normalize_workflow_tasks( existing_workflow=None, task_runner_normalizer=None, max_tasks=WORKFLOW_MAX_TASKS, + task_document_action_normalizer=None, + default_document_action=None, ): workflow_data = workflow_data if isinstance(workflow_data, dict) else {} existing_workflow = existing_workflow if isinstance(existing_workflow, dict) else {} @@ -214,14 +216,27 @@ def _normalize_workflow_tasks( else: raise ValueError(f'Workflow task {index + 1} runner could not be authorized.') - normalized_tasks.append({ + normalized_task = { 'id': task_id, 'type': task_type, 'name': name, 'instructions': instructions, 'order': index + 1, 'runner': runner, - }) + } + + if callable(task_document_action_normalizer): + raw_document_action = raw_task.get('document_action') + if not isinstance(raw_document_action, dict) and index == 0: + # Workflows saved before per-task documents kept a single workflow-level + # action that only ever executed on the first task. + raw_document_action = default_document_action + try: + normalized_task['document_action'] = task_document_action_normalizer(raw_document_action) + except ValueError as exc: + raise ValueError(f'Workflow task {index + 1} ({name}): {exc}') from exc + + normalized_tasks.append(normalized_task) return normalized_tasks @@ -286,6 +301,37 @@ def _normalize_document_action_config(workflow_data, existing_workflow=None, all ) +def _normalize_task_document_action_config(action_payload, allow_empty_file_sync_targets=False, settings=None): + """Normalize a single workflow task's document action payload.""" + source_settings = settings if isinstance(settings, dict) else get_settings() + action_payload = action_payload if isinstance(action_payload, dict) else {'type': 'none'} + max_documents_by_type = get_document_action_max_documents_by_type( + DOCUMENT_ACTION_CONTEXT_WORKFLOW, + settings=source_settings, + ) + allowed_action_types = get_enabled_document_action_types(settings=source_settings) + + if allow_empty_file_sync_targets: + action_type = str(action_payload.get('type') or '').strip().lower() + document_ids = action_payload.get('document_ids') if isinstance(action_payload.get('document_ids'), list) else [] + if action_type == DOCUMENT_ACTION_TYPE_ANALYZE and not document_ids: + placeholder_payload = dict(action_payload) + placeholder_payload['document_ids'] = ['__dynamic_file_sync_document__'] + normalized_action = normalize_document_action_config( + action_payload=placeholder_payload, + max_documents_by_type=max_documents_by_type, + allowed_action_types=allowed_action_types, + ) + normalized_action['document_ids'] = [] + return normalized_action + + return normalize_document_action_config( + action_payload=action_payload, + max_documents_by_type=max_documents_by_type, + allowed_action_types=allowed_action_types, + ) + + def _normalize_file_sync_config(user_id, workflow_data, existing_workflow=None): workflow_data = workflow_data if isinstance(workflow_data, dict) else {} existing_workflow = existing_workflow if isinstance(existing_workflow, dict) else {} @@ -683,6 +729,13 @@ def save_personal_workflow(user_id, workflow_data, actor_user_id=None): workflow_name = _normalize_text(workflow_data.get('name'), 'Workflow name', required=True) description = _normalize_text(workflow_data.get('description'), 'Description') + file_sync = _normalize_file_sync_config(user_id, workflow_data, existing_workflow=existing_workflow) + allow_empty_file_sync_targets = bool(file_sync.get('enabled') and file_sync.get('use_changed_documents')) + document_action = _normalize_document_action_config( + workflow_data, + existing_workflow=existing_workflow, + allow_empty_file_sync_targets=allow_empty_file_sync_targets, + ) tasks = _normalize_workflow_tasks( workflow_data, existing_workflow=existing_workflow, @@ -692,6 +745,12 @@ def save_personal_workflow(user_id, workflow_data, actor_user_id=None): settings=settings, ), max_tasks=get_workflow_max_tasks(settings), + task_document_action_normalizer=lambda action_payload: _normalize_task_document_action_config( + action_payload, + allow_empty_file_sync_targets=allow_empty_file_sync_targets, + settings=settings, + ), + default_document_action=document_action, ) task_prompt = _normalize_text( workflow_data.get('task_prompt') or (tasks[0].get('instructions') if tasks else ''), @@ -737,13 +796,6 @@ def save_personal_workflow(user_id, workflow_data, actor_user_id=None): workflow_data.get('chat_capabilities_enabled', default_chat_capabilities_enabled), default=default_chat_capabilities_enabled, ) - file_sync = _normalize_file_sync_config(user_id, workflow_data, existing_workflow=existing_workflow) - allow_empty_file_sync_targets = bool(file_sync.get('enabled') and file_sync.get('use_changed_documents')) - document_action = _normalize_document_action_config( - workflow_data, - existing_workflow=existing_workflow, - allow_empty_file_sync_targets=allow_empty_file_sync_targets, - ) if trigger_type == 'file_sync': if not file_sync.get('enabled'): raise ValueError('Monitor File Sync Changes workflows require File Sync before run.') diff --git a/application/single_app/functions_workflow_runner.py b/application/single_app/functions_workflow_runner.py index 485621773..301cbcf4a 100644 --- a/application/single_app/functions_workflow_runner.py +++ b/application/single_app/functions_workflow_runner.py @@ -7030,6 +7030,25 @@ def _format_workflow_file_sync_context(file_sync_result): return '\n'.join(lines) +def _apply_file_sync_changed_documents_to_action(action_config, changed_document_ids, group_ids, public_workspace_ids): + """Point an analyze document action at the documents File Sync just changed.""" + action_config = action_config if isinstance(action_config, dict) else {} + if action_config.get('type') != DOCUMENT_ACTION_TYPE_ANALYZE: + return None + + if not changed_document_ids: + return {'type': DOCUMENT_ACTION_TYPE_NONE} + + updated_action_config = dict(action_config) + updated_action_config.update({ + 'document_ids': list(changed_document_ids), + 'doc_scope': 'all', + 'active_group_ids': list(group_ids), + 'active_public_workspace_id': list(public_workspace_ids), + }) + return updated_action_config + + def _apply_file_sync_context_to_workflow(workflow, file_sync_result): if not isinstance(file_sync_result, dict) or not file_sync_result.get('enabled'): return workflow @@ -7044,15 +7063,6 @@ def _apply_file_sync_context_to_workflow(workflow, file_sync_result): if not config.get('use_changed_documents'): return prepared_workflow - action_config = _get_document_action_config(workflow) - if action_config.get('type') != DOCUMENT_ACTION_TYPE_ANALYZE: - return prepared_workflow - - if not changed_document_ids: - prepared_workflow['document_action'] = {'type': DOCUMENT_ACTION_TYPE_NONE} - prepared_workflow['analyze'] = build_analyze_config(prepared_workflow['document_action']) - return prepared_workflow - group_ids = [] public_workspace_ids = [] for source_config in config.get('sources') or []: @@ -7063,13 +7073,33 @@ def _apply_file_sync_context_to_workflow(workflow, file_sync_result): elif scope_type == 'public' and scope_id and scope_id not in public_workspace_ids: public_workspace_ids.append(scope_id) - updated_action_config = dict(action_config) - updated_action_config.update({ - 'document_ids': changed_document_ids, - 'doc_scope': 'all', - 'active_group_ids': group_ids, - 'active_public_workspace_id': public_workspace_ids, - }) + updated_tasks = [] + tasks_changed = False + for raw_task in prepared_workflow.get('tasks') or []: + task = dict(raw_task or {}) + if isinstance(task.get('document_action'), dict): + updated_task_action = _apply_file_sync_changed_documents_to_action( + task.get('document_action'), + changed_document_ids, + group_ids, + public_workspace_ids, + ) + if updated_task_action is not None: + task['document_action'] = updated_task_action + tasks_changed = True + updated_tasks.append(task) + if tasks_changed: + prepared_workflow['tasks'] = updated_tasks + + updated_action_config = _apply_file_sync_changed_documents_to_action( + _get_document_action_config(workflow), + changed_document_ids, + group_ids, + public_workspace_ids, + ) + if updated_action_config is None: + return prepared_workflow + prepared_workflow['document_action'] = updated_action_config prepared_workflow['analyze'] = build_analyze_config(updated_action_config) return prepared_workflow @@ -9045,11 +9075,30 @@ def _truncate_workflow_task_context(value, max_chars=WORKFLOW_TASK_CONTEXT_MAX_C ) +def _resolve_workflow_task_document_action(workflow, task, include_document_action=False): + """Resolve the document action a workflow task should execute with. + + Tasks own their document action. Workflows saved before per-task documents have no + task-level key, so the workflow-level action still applies to the first task only. + """ + task = task if isinstance(task, dict) else {} + if isinstance(task.get('document_action'), dict): + return _get_document_action_config({'document_action': task.get('document_action')}) + if include_document_action: + return _get_document_action_config(workflow) + return {'type': DOCUMENT_ACTION_TYPE_NONE} + + def _build_workflow_task_execution_workflow(workflow, task, previous_reply='', include_document_action=False): task = task if isinstance(task, dict) else {} prepared_workflow = dict(workflow or {}) task_instructions = str(task.get('instructions') or '').strip() file_sync_context = str(workflow.get('file_sync_prompt_context') or '').strip() + task_document_action = _resolve_workflow_task_document_action( + workflow, + task, + include_document_action=include_document_action, + ) if include_document_action and file_sync_context: task_instructions = ( f'{task_instructions}\n\n' @@ -9071,9 +9120,8 @@ def _build_workflow_task_execution_workflow(workflow, task, previous_reply='', i 'name': str(task.get('name') or '').strip(), 'order': int(task.get('order') or 0), } - if not include_document_action: - prepared_workflow['document_action'] = {'type': DOCUMENT_ACTION_TYPE_NONE} - prepared_workflow['analyze'] = build_analyze_config(prepared_workflow['document_action']) + prepared_workflow['document_action'] = dict(task_document_action) + prepared_workflow['analyze'] = build_analyze_config(prepared_workflow['document_action']) return prepared_workflow diff --git a/application/single_app/static/js/chat/chat-documents.js b/application/single_app/static/js/chat/chat-documents.js index 8386f3d7a..3d27e7e24 100644 --- a/application/single_app/static/js/chat/chat-documents.js +++ b/application/single_app/static/js/chat/chat-documents.js @@ -2122,7 +2122,12 @@ function initializeDocumentDropdown() { Load Tags for Selected Scope --------------------------------------------------------------------------- */ export async function loadTagsForScope() { - if (!chatTagsFilter) return; + if (!chatTagsFilter) { + // Keep the tags control out of its initial loading state when the hidden + // filter select is absent, otherwise the button stays stuck on "Loading tags...". + hideTagsDropdown(); + return; + } // Clear existing options in both hidden select and custom dropdown chatTagsFilter.innerHTML = ''; diff --git a/application/single_app/static/js/workspace/workspace_workflows.js b/application/single_app/static/js/workspace/workspace_workflows.js index 437285fa5..43af124d2 100644 --- a/application/single_app/static/js/workspace/workspace_workflows.js +++ b/application/single_app/static/js/workspace/workspace_workflows.js @@ -391,6 +391,7 @@ let workflowPendingDelete = null; let currentHistoryWorkflowId = ""; let currentEditingWorkflow = null; let workflowComparisonVersionLoadToken = 0; +let workflowDocumentPickerLoadToken = 0; let workflowPickerDocumentIds = []; let workflowSavedComparisonTargetIds = []; let workflowSavedComparisonPreferredLeftId = ""; @@ -492,6 +493,143 @@ function getAgentScopeLabel(agent) { return "Personal Agent"; } +function normalizeWorkflowDocumentActionType(value) { + const normalizedValue = normalizeText(value).toLowerCase(); + return [DOCUMENT_ACTION_SEARCH, DOCUMENT_ACTION_ANALYZE, DOCUMENT_ACTION_COMPARISON].includes(normalizedValue) + ? normalizedValue + : DOCUMENT_ACTION_NONE; +} + +function createDefaultWorkflowTaskDocumentAction() { + return { + type: DOCUMENT_ACTION_NONE, + document_ids: [], + left_document_id: "", + right_document_ids: [], + analysis_mode: DOCUMENT_ANALYSIS_MODE_COMBINED, + doc_scope: workflowWorkspaceConfig.scope === "group" ? "group" : "all", + active_group_ids: [], + active_public_workspace_id: [], + window_unit: "pages", + window_size: "", + window_percent: "", + max_retries_per_window: 1, + target_mode: DOCUMENT_ANALYSIS_TARGET_SELECTED, + recent_window_minutes: DEFAULT_RECENT_DOCUMENT_WINDOW_MINUTES, + }; +} + +function normalizeWorkflowTaskDocumentAction(rawAction) { + const source = rawAction && typeof rawAction === "object" ? rawAction : {}; + if (normalizeWorkflowDocumentActionType(source.type) === DOCUMENT_ACTION_NONE) { + return createDefaultWorkflowTaskDocumentAction(); + } + + const normalizedAction = getDocumentActionConfig({ document_action: source }); + return { + ...normalizedAction, + document_ids: normalizeIdList(normalizedAction.document_ids), + right_document_ids: normalizeIdList(normalizedAction.right_document_ids), + active_group_ids: normalizeIdList(normalizedAction.active_group_ids), + active_public_workspace_id: normalizeIdList(normalizedAction.active_public_workspace_id), + }; +} + +function readWorkflowDocumentActionFromForm() { + const actionType = normalizeWorkflowDocumentActionType(workflowDocumentActionTypeSelect?.value); + if (actionType === DOCUMENT_ACTION_NONE) { + return createDefaultWorkflowTaskDocumentAction(); + } + + const targetMode = normalizeText(workflowAnalysisTargetModeSelect?.value) === DOCUMENT_ANALYSIS_TARGET_RECENT + ? DOCUMENT_ANALYSIS_TARGET_RECENT + : DOCUMENT_ANALYSIS_TARGET_SELECTED; + const isRecentMode = targetMode === DOCUMENT_ANALYSIS_TARGET_RECENT; + const pickerDocumentIds = isRecentMode ? [] : getWorkflowPickerSelectedDocumentIds(); + const comparisonLeftDocumentId = isRecentMode ? "" : normalizeText(workflowComparisonLeftDocumentIdInput?.value); + const comparisonTargetDocumentIds = isRecentMode ? [] : getSelectedWorkflowComparisonTargetIds(); + const activeGroupId = getWorkflowActiveGroupId(); + + return { + type: actionType, + document_ids: actionType === DOCUMENT_ACTION_COMPARISON ? comparisonTargetDocumentIds : pickerDocumentIds, + left_document_id: actionType === DOCUMENT_ACTION_COMPARISON ? comparisonLeftDocumentId : "", + right_document_ids: actionType === DOCUMENT_ACTION_COMPARISON + ? comparisonTargetDocumentIds.filter((documentId) => documentId !== comparisonLeftDocumentId) + : [], + analysis_mode: actionType === DOCUMENT_ACTION_ANALYZE && workflowAnalysisPerDocumentToggle?.checked + ? DOCUMENT_ANALYSIS_MODE_PER_DOCUMENT + : DOCUMENT_ANALYSIS_MODE_COMBINED, + doc_scope: normalizeText(workflowAnalysisDocScopeSelect?.value) || getWorkflowDocumentScope(), + active_group_ids: workflowWorkspaceConfig.scope === "group" && activeGroupId + ? [activeGroupId] + : parseCsvList(workflowAnalysisGroupIdsInput?.value), + active_public_workspace_id: parseCsvList(workflowAnalysisPublicWorkspaceIdsInput?.value), + window_unit: normalizeText(workflowAnalysisWindowUnitSelect?.value) || "pages", + window_size: normalizeText(workflowAnalysisWindowSizeInput?.value), + window_percent: normalizeText(workflowAnalysisWindowPercentInput?.value), + max_retries_per_window: normalizeText(workflowAnalysisRetriesInput?.value) || "1", + target_mode: targetMode, + recent_window_minutes: normalizeText(workflowAnalysisRecentMinutesInput?.value) + || String(DEFAULT_RECENT_DOCUMENT_WINDOW_MINUTES), + }; +} + +function applyWorkflowDocumentActionToForm(rawAction) { + const action = normalizeWorkflowTaskDocumentAction(rawAction); + + if (workflowDocumentActionTypeSelect) { + workflowDocumentActionTypeSelect.value = action.type; + } + if (workflowAnalysisDocScopeSelect) { + workflowAnalysisDocScopeSelect.value = action.doc_scope; + } + if (workflowAnalysisTargetModeSelect) { + workflowAnalysisTargetModeSelect.value = action.target_mode === DOCUMENT_ANALYSIS_TARGET_RECENT + ? DOCUMENT_ANALYSIS_TARGET_RECENT + : DOCUMENT_ANALYSIS_TARGET_SELECTED; + } + if (workflowAnalysisRecentMinutesInput) { + workflowAnalysisRecentMinutesInput.value = String(action.recent_window_minutes || DEFAULT_RECENT_DOCUMENT_WINDOW_MINUTES); + } + if (workflowAnalysisPerDocumentToggle) { + workflowAnalysisPerDocumentToggle.checked = action.analysis_mode === DOCUMENT_ANALYSIS_MODE_PER_DOCUMENT; + } + if (workflowAnalysisGroupIdsInput) { + workflowAnalysisGroupIdsInput.value = joinCsvList(action.active_group_ids); + } + if (workflowAnalysisPublicWorkspaceIdsInput) { + workflowAnalysisPublicWorkspaceIdsInput.value = joinCsvList(action.active_public_workspace_id); + } + if (workflowAnalysisWindowUnitSelect) { + workflowAnalysisWindowUnitSelect.value = action.window_unit; + } + if (workflowAnalysisWindowSizeInput) { + workflowAnalysisWindowSizeInput.value = action.window_size ?? ""; + } + if (workflowAnalysisWindowPercentInput) { + workflowAnalysisWindowPercentInput.value = action.window_percent ?? ""; + } + if (workflowAnalysisRetriesInput) { + workflowAnalysisRetriesInput.value = String(action.max_retries_per_window ?? 1); + } + + applyWorkflowPickerSelection(action.type === DOCUMENT_ACTION_COMPARISON ? [] : action.document_ids); + + if (action.type === DOCUMENT_ACTION_COMPARISON) { + const savedTargetIds = normalizeIdList([action.left_document_id, ...action.right_document_ids]); + workflowSavedComparisonTargetIds = savedTargetIds; + workflowSavedComparisonPreferredLeftId = action.left_document_id; + setWorkflowComparisonSavedTargets(savedTargetIds, action.left_document_id); + } else { + workflowSavedComparisonTargetIds = []; + workflowSavedComparisonPreferredLeftId = ""; + setWorkflowComparisonSavedTargets([], ""); + } + + updateDocumentActionFields(); +} + function getWorkflowDefaultRunnerSummary() { if (normalizeText(workflowRunnerTypeSelect?.value) === "agent") { const agent = getSelectedAgentOption(); @@ -727,6 +865,7 @@ function syncActiveWorkflowTaskFromEditor() { } else { activeTask.runner = { type: "inherit" }; } + activeTask.document_action = readWorkflowDocumentActionFromForm(); } function populateWorkflowTaskEditor() { @@ -742,6 +881,10 @@ function populateWorkflowTaskEditor() { workflowTaskRunnerTypeSelect.value = runner.type; } updateWorkflowTaskRunnerFields(runner); + applyWorkflowDocumentActionToForm(activeTask?.document_action); + ensureWorkflowDocumentPickerLoaded().catch((error) => { + setWorkflowPickerError(error.message || "Unable to load documents for this workflow."); + }); if (workflowTaskBriefInput) { workflowTaskBriefInput.value = ""; } @@ -792,7 +935,10 @@ function renderWorkflowTasks() { const runner = document.createElement("div"); runner.className = "workflow-task-item__runner small text-muted mt-1"; runner.textContent = getWorkflowTaskRunnerSummary(task); - content.append(name, preview, runner); + const documents = document.createElement("div"); + documents.className = "workflow-task-item__documents small text-muted"; + documents.textContent = `Documents: ${getWorkflowDocumentActionSummary({ document_action: task.document_action })}`; + content.append(name, preview, runner, documents); const actions = document.createElement("div"); actions.className = "workflow-task-item__actions"; @@ -831,6 +977,7 @@ function addWorkflowTask() { instructions: "", order: workflowTasks.length + 1, runner: { type: "inherit" }, + document_action: createDefaultWorkflowTaskDocumentAction(), }; workflowTasks.push(task); activeWorkflowTaskId = task.id; @@ -869,6 +1016,15 @@ function removeWorkflowTask(taskId) { function initializeWorkflowTasks(workflow = null) { const storedTasks = Array.isArray(workflow?.tasks) ? workflow.tasks : []; + const workflowDocumentAction = workflow ? getDocumentActionConfig(workflow) : null; + const resolveTaskDocumentAction = (task, index) => { + // Legacy workflows stored a single workflow-level action that only ever ran on task 1. + if (index === 0 && !(task?.document_action && typeof task.document_action === "object") && workflowDocumentAction) { + return normalizeWorkflowTaskDocumentAction(workflowDocumentAction); + } + return normalizeWorkflowTaskDocumentAction(task?.document_action); + }; + workflowTasks = storedTasks.length ? storedTasks.map((task, index) => ({ id: normalizeText(task.id) || createWorkflowTaskId(), @@ -877,6 +1033,7 @@ function initializeWorkflowTasks(workflow = null) { instructions: normalizeText(task.instructions), order: index + 1, runner: normalizeWorkflowTaskRunner(task.runner), + document_action: resolveTaskDocumentAction(task, index), })) : [{ id: createWorkflowTaskId(), @@ -885,6 +1042,7 @@ function initializeWorkflowTasks(workflow = null) { instructions: normalizeText(workflow?.task_prompt), order: 1, runner: { type: "inherit" }, + document_action: resolveTaskDocumentAction(null, 0), }]; activeWorkflowTaskId = workflowTasks[0].id; populateWorkflowTaskEditor(); @@ -941,23 +1099,30 @@ function renderWorkflowReview() { clearElementChildren(workflowReviewSummary); const runnerLabel = workflowRunnerTypeSelect?.selectedOptions?.[0]?.textContent || "Direct Model"; const triggerLabel = workflowTriggerTypeSelect?.selectedOptions?.[0]?.textContent || "Manual"; - const documentActionLabel = workflowDocumentActionTypeSelect?.selectedOptions?.[0]?.textContent || "No document action"; const alertLabel = workflowAlertPrioritySelect?.selectedOptions?.[0]?.textContent || "No notification"; const retryCount = Number(workflowTaskRetryCountInput?.value || 0); const strategyLabel = getWorkflowErrorStrategy() === "continue" ? "Continue after failure" : "Stop after failure"; + const documentTaskCount = workflowTasks.filter( + (task) => normalizeWorkflowDocumentActionType(task.document_action?.type) !== DOCUMENT_ACTION_NONE + ).length; addWorkflowReviewItem("Workflow", normalizeText(workflowNameInput?.value) || "Untitled workflow"); addWorkflowReviewItem("Default Runner", normalizeText(runnerLabel)); addWorkflowReviewItem("Trigger", normalizeText(triggerLabel)); addWorkflowReviewItem("Tasks", `${workflowTasks.length} ordered ${workflowTasks.length === 1 ? "task" : "tasks"}`); - addWorkflowReviewItem("Workspace documents", normalizeText(documentActionLabel)); + addWorkflowReviewItem( + "Workspace documents", + documentTaskCount + ? `${documentTaskCount} of ${workflowTasks.length} ${workflowTasks.length === 1 ? "task uses" : "tasks use"} workspace documents` + : "No document action", + ); addWorkflowReviewItem("File Sync", workflowFileSyncEnabledToggle?.checked ? "Before each run" : "Not used"); addWorkflowReviewItem("Failure handling", `${strategyLabel}; ${retryCount} ${retryCount === 1 ? "retry" : "retries"}`); addWorkflowReviewItem("Alerts", getWorkflowAlertReviewSummary(alertLabel)); workflowTasks.forEach((task, index) => { addWorkflowReviewItem( `Task ${index + 1}`, - `${normalizeText(task.name) || `Task ${index + 1}`} - ${getWorkflowTaskRunnerSummary(task)}`, + `${normalizeText(task.name) || `Task ${index + 1}`} - ${getWorkflowTaskRunnerSummary(task)} - ${getWorkflowDocumentActionSummary({ document_action: task.document_action })}`, ); }); } @@ -999,6 +1164,20 @@ function validateWorkflowTasks() { workflowTaskAgentSelect?.focus(); return false; } + try { + validateWorkflowTaskDocumentAction( + serializeWorkflowDocumentAction(task.document_action), + getWorkflowTaskLabel(task, index), + Boolean(workflowFileSyncEnabledToggle?.checked) && workflowFileSyncUseChangedDocumentsToggle?.checked !== false, + ); + } catch (error) { + activeWorkflowTaskId = task.id; + populateWorkflowTaskEditor(); + renderWorkflowTasks(); + showToast(escapeHtml(error.message || `Complete the workspace document setup for task ${index + 1}.`), "warning"); + workflowDocumentActionTypeSelect?.focus(); + return false; + } } return true; } @@ -1998,7 +2177,7 @@ function setWorkflowPickerError(message = "") { workflowDocumentPickerError.classList.toggle("d-none", !message); } -async function initializeWorkflowDocumentPicker(documentAction = {}) { +async function initializeWorkflowDocumentPicker(documentAction = {}, options = {}) { if (!workflowDocumentPickerCard) { return; } @@ -2010,11 +2189,19 @@ async function initializeWorkflowDocumentPicker(documentAction = {}) { return; } + const requestToken = workflowDocumentPickerLoadToken + 1; + workflowDocumentPickerLoadToken = requestToken; + setWorkflowPickerError(""); setWorkflowPickerLoadingState(true); syncWorkflowPickerActionType(); const pickerScopes = getWorkflowPickerScopesFromAction(documentAction); + const pickerSelectionIds = Array.isArray(options.pickerDocumentIds) + ? options.pickerDocumentIds + : actionType === DOCUMENT_ACTION_COMPARISON + ? [] + : documentAction.document_ids || []; try { await setEffectiveScopes(pickerScopes, { force: workflowWorkspaceConfig.scope === "group", @@ -2022,19 +2209,41 @@ async function initializeWorkflowDocumentPicker(documentAction = {}) { reload: true, }); await ensureDocumentPickerReady({ reload: false, showLoading: false }); + if (requestToken !== workflowDocumentPickerLoadToken) { + return; + } syncWorkflowScopeFieldsFromPicker(pickerScopes); - applyWorkflowPickerSelection( - documentAction.type === DOCUMENT_ACTION_COMPARISON - ? [] - : documentAction.document_ids || [] - ); + applyWorkflowPickerSelection(pickerSelectionIds); } catch (error) { - setWorkflowPickerError(error.message || "Unable to load documents for this workflow."); + if (requestToken === workflowDocumentPickerLoadToken) { + setWorkflowPickerError(error.message || "Unable to load documents for this workflow."); + } } finally { - setWorkflowPickerLoadingState(false); + if (requestToken === workflowDocumentPickerLoadToken) { + setWorkflowPickerLoadingState(false); + } } } +function getWorkflowPickerAvailableDocumentCount() { + return document.querySelectorAll("#document-dropdown-items .dropdown-item[data-document-id]").length; +} + +function ensureWorkflowDocumentPickerLoaded(options = {}) { + const formAction = readWorkflowDocumentActionFromForm(); + return initializeWorkflowDocumentPicker( + formAction, + options.preserveSelection ? { pickerDocumentIds: getWorkflowPickerSelectedDocumentIds() } : {}, + ); +} + +function handleWorkflowDocumentActionSelectionChanged() { + updateDocumentActionFields(); + ensureWorkflowDocumentPickerLoaded().catch((error) => { + setWorkflowPickerError(error.message || "Unable to load documents for this workflow."); + }); +} + async function refreshWorkflowComparisonTargetsFromPicker() { if (normalizeText(workflowDocumentActionTypeSelect?.value) !== DOCUMENT_ACTION_COMPARISON) { return; @@ -2839,17 +3048,25 @@ function updateDocumentActionFields() { } async function applySelectedWorkspaceDocumentsToWorkflow() { - const selectedIds = getWorkflowPickerSelectedDocumentIds(); - if (!selectedIds.length) { - const selectedLabel = normalizeText(workflowWorkspaceConfig.selectedDocumentsLabel) || "workspace"; - showToast(`Select one or more ${selectedLabel} documents in the picker first.`, "warning"); + const actionType = normalizeText(workflowDocumentActionTypeSelect?.value) || DOCUMENT_ACTION_NONE; + if (actionType === DOCUMENT_ACTION_NONE) { return; } - const actionType = normalizeText(workflowDocumentActionTypeSelect?.value) || DOCUMENT_ACTION_NONE; - if (actionType === DOCUMENT_ACTION_NONE) { + await ensureWorkflowDocumentPickerLoaded({ preserveSelection: true }); + + const selectedIds = getWorkflowPickerSelectedDocumentIds(); + const selectedLabel = normalizeText(workflowWorkspaceConfig.selectedDocumentsLabel) || "workspace"; + if (!selectedIds.length) { + showToast( + getWorkflowPickerAvailableDocumentCount() + ? `Document list refreshed. Select one or more ${selectedLabel} documents in the picker for this task.` + : `Document list refreshed. No ${selectedLabel} documents are available in the selected scope.`, + "info", + ); return; } + const workflowMaxDocuments = getWorkflowDocumentActionMaxDocuments(actionType); const limitedSelectedIds = selectedIds.slice(0, workflowMaxDocuments); @@ -3744,45 +3961,6 @@ async function openWorkflowModal(workflow = null) { workflowFileSyncUseChangedDocumentsToggle.checked = fileSyncConfig.use_changed_documents !== false; } populateFileSyncSourceSelect(Array.isArray(fileSyncConfig.sources) ? fileSyncConfig.sources : []); - const documentAction = getDocumentActionConfig(workflow); - if (workflowDocumentActionTypeSelect) { - workflowDocumentActionTypeSelect.value = documentAction.type; - } - if (workflowAnalysisDocScopeSelect) { - workflowAnalysisDocScopeSelect.value = documentAction.doc_scope; - } - if (workflowAnalysisTargetModeSelect) { - workflowAnalysisTargetModeSelect.value = documentAction.target_mode === DOCUMENT_ANALYSIS_TARGET_RECENT - ? DOCUMENT_ANALYSIS_TARGET_RECENT - : DOCUMENT_ANALYSIS_TARGET_SELECTED; - } - if (workflowAnalysisRecentMinutesInput) { - workflowAnalysisRecentMinutesInput.value = String(documentAction.recent_window_minutes || DEFAULT_RECENT_DOCUMENT_WINDOW_MINUTES); - } - if (workflowAnalysisDocumentIdsInput) { - workflowAnalysisDocumentIdsInput.value = joinCsvList(documentAction.document_ids); - } - if (workflowAnalysisPerDocumentToggle) { - workflowAnalysisPerDocumentToggle.checked = documentAction.analysis_mode === DOCUMENT_ANALYSIS_MODE_PER_DOCUMENT; - } - if (workflowAnalysisGroupIdsInput) { - workflowAnalysisGroupIdsInput.value = joinCsvList(documentAction.active_group_ids); - } - if (workflowAnalysisPublicWorkspaceIdsInput) { - workflowAnalysisPublicWorkspaceIdsInput.value = joinCsvList(documentAction.active_public_workspace_id); - } - if (workflowAnalysisWindowUnitSelect) { - workflowAnalysisWindowUnitSelect.value = documentAction.window_unit; - } - if (workflowAnalysisWindowSizeInput) { - workflowAnalysisWindowSizeInput.value = documentAction.window_size; - } - if (workflowAnalysisWindowPercentInput) { - workflowAnalysisWindowPercentInput.value = documentAction.window_percent; - } - if (workflowAnalysisRetriesInput) { - workflowAnalysisRetriesInput.value = String(documentAction.max_retries_per_window); - } if (workflowScheduleValueInput) { workflowScheduleValueInput.value = String(workflow.schedule?.value || 10); } @@ -3807,17 +3985,6 @@ async function openWorkflowModal(workflow = null) { } initializeWorkflowTasks(workflow); - const documentAction = workflow ? getDocumentActionConfig(workflow) : null; - if (documentAction?.type === DOCUMENT_ACTION_COMPARISON) { - const savedTargetIds = [documentAction.left_document_id, ...documentAction.right_document_ids].filter(Boolean); - workflowSavedComparisonTargetIds = savedTargetIds; - workflowSavedComparisonPreferredLeftId = documentAction.left_document_id; - setWorkflowComparisonSavedTargets(savedTargetIds, documentAction.left_document_id); - } else { - workflowSavedComparisonTargetIds = []; - workflowSavedComparisonPreferredLeftId = ""; - setWorkflowComparisonSavedTargets([], ""); - } updateRunnerFields(); updateTriggerFields(); @@ -3825,7 +3992,139 @@ async function openWorkflowModal(workflow = null) { updateDocumentActionFields(); showWorkflowStep(0); workflowModal.show(); - await initializeWorkflowDocumentPicker(documentAction || {}); + await ensureWorkflowDocumentPickerLoaded(); +} + +function serializeWorkflowDocumentAction(rawAction) { + const source = rawAction && typeof rawAction === "object" ? rawAction : createDefaultWorkflowTaskDocumentAction(); + const actionType = normalizeWorkflowDocumentActionType(source.type); + const hasAction = actionType !== DOCUMENT_ACTION_NONE; + const targetMode = hasAction && normalizeText(source.target_mode) === DOCUMENT_ANALYSIS_TARGET_RECENT + ? DOCUMENT_ANALYSIS_TARGET_RECENT + : DOCUMENT_ANALYSIS_TARGET_SELECTED; + const isRecentMode = targetMode === DOCUMENT_ANALYSIS_TARGET_RECENT; + const rawWindowSize = normalizeText(source.window_size); + const rawWindowPercent = normalizeText(source.window_percent); + const rawRetries = normalizeText(source.max_retries_per_window) || "1"; + const rawRecentMinutes = normalizeText(source.recent_window_minutes) || String(DEFAULT_RECENT_DOCUMENT_WINDOW_MINUTES); + const activeGroupId = getWorkflowActiveGroupId(); + const comparisonLeftDocumentId = actionType === DOCUMENT_ACTION_COMPARISON && !isRecentMode + ? normalizeText(source.left_document_id) + : ""; + + return { + type: actionType, + document_ids: hasAction && !isRecentMode ? normalizeIdList(source.document_ids) : [], + left_document_id: comparisonLeftDocumentId, + right_document_ids: actionType === DOCUMENT_ACTION_COMPARISON && !isRecentMode + ? normalizeIdList(source.right_document_ids).filter((documentId) => documentId !== comparisonLeftDocumentId) + : [], + analysis_mode: actionType === DOCUMENT_ACTION_ANALYZE + ? normalizeWorkflowAnalysisMode(source.analysis_mode) + : DOCUMENT_ANALYSIS_MODE_COMBINED, + doc_scope: normalizeText(source.doc_scope) || getWorkflowDocumentScope(), + active_group_ids: hasAction + ? workflowWorkspaceConfig.scope === "group" && activeGroupId + ? [activeGroupId] + : normalizeIdList(source.active_group_ids) + : [], + active_public_workspace_id: hasAction ? normalizeIdList(source.active_public_workspace_id) : [], + window_unit: normalizeText(source.window_unit) || "pages", + window_size: rawWindowSize ? Number(rawWindowSize) : null, + window_percent: rawWindowPercent ? Number(rawWindowPercent) : null, + max_retries_per_window: Number(rawRetries), + target_mode: targetMode, + recent_window_minutes: Number(rawRecentMinutes), + }; +} + +function buildWorkflowAnalyzeConfig(actionPayload) { + const action = actionPayload && typeof actionPayload === "object" ? actionPayload : serializeWorkflowDocumentAction(); + const isAnalyze = action.type === DOCUMENT_ACTION_ANALYZE; + + return { + enabled: isAnalyze, + document_ids: isAnalyze ? action.document_ids : [], + doc_scope: action.doc_scope, + active_group_ids: isAnalyze ? action.active_group_ids : [], + active_public_workspace_id: isAnalyze ? action.active_public_workspace_id : [], + analysis_mode: isAnalyze ? action.analysis_mode : DOCUMENT_ANALYSIS_MODE_COMBINED, + window_unit: action.window_unit, + window_size: action.window_size, + window_percent: action.window_percent, + max_retries_per_window: action.max_retries_per_window, + target_mode: isAnalyze ? action.target_mode : DOCUMENT_ANALYSIS_TARGET_SELECTED, + recent_window_minutes: action.recent_window_minutes, + }; +} + +function validateWorkflowTaskDocumentAction(actionPayload, taskLabel, usesDynamicFileSyncTargets = false) { + const actionType = actionPayload.type; + if (actionType === DOCUMENT_ACTION_NONE) { + return; + } + + const isSelectedMode = actionPayload.target_mode === DOCUMENT_ANALYSIS_TARGET_SELECTED; + const prefix = `${taskLabel}: `; + + if (!isDocumentActionEnabled(actionType)) { + throw new Error(`${prefix}${getDocumentActionDisplayLabel(actionType)} is currently disabled by an administrator.`); + } + if (actionType === DOCUMENT_ACTION_SEARCH && isSelectedMode && !actionPayload.document_ids.length) { + throw new Error(`${prefix}Select one or more documents for search.`); + } + if (actionType === DOCUMENT_ACTION_ANALYZE && isSelectedMode && !actionPayload.document_ids.length && !usesDynamicFileSyncTargets) { + throw new Error(`${prefix}Select one or more documents for analysis.`); + } + if (!isSelectedMode && ( + !Number.isInteger(actionPayload.recent_window_minutes) + || actionPayload.recent_window_minutes < 1 + || actionPayload.recent_window_minutes > 1440 + )) { + throw new Error(`${prefix}Recent document window must be between 1 and 1440 minutes.`); + } + if (actionType === DOCUMENT_ACTION_COMPARISON && isSelectedMode) { + if (actionPayload.document_ids.length < 2) { + throw new Error(`${prefix}Select at least two document versions for compare.`); + } + if (!actionPayload.left_document_id) { + throw new Error(`${prefix}Add one Source document id for compare.`); + } + if (!actionPayload.right_document_ids.length) { + throw new Error(`${prefix}Add one or more Target document ids for compare.`); + } + } + + const documentActionCount = actionType === DOCUMENT_ACTION_COMPARISON + ? 1 + actionPayload.right_document_ids.length + : actionPayload.document_ids.length; + const workflowMaxDocuments = getWorkflowDocumentActionMaxDocuments(actionType); + if (documentActionCount > workflowMaxDocuments) { + throw new Error(`${prefix}${getDocumentActionDisplayLabel(actionType)} workflows support up to ${workflowMaxDocuments} documents per run.`); + } + if (actionPayload.window_size !== null && (!Number.isInteger(actionPayload.window_size) || actionPayload.window_size < 1)) { + throw new Error(`${prefix}Window size must be a whole number greater than zero.`); + } + if (actionPayload.window_percent !== null && ( + !Number.isInteger(actionPayload.window_percent) + || actionPayload.window_percent < 1 + || actionPayload.window_percent > 100 + )) { + throw new Error(`${prefix}Window percent must be a whole number between 1 and 100.`); + } + if (actionPayload.window_size !== null && actionPayload.window_percent !== null) { + throw new Error(`${prefix}Choose either a fixed window size or a window percent, not both.`); + } + if (!Number.isInteger(actionPayload.max_retries_per_window) + || actionPayload.max_retries_per_window < 0 + || actionPayload.max_retries_per_window > 5) { + throw new Error(`${prefix}Retries per window must be between 0 and 5.`); + } +} + +function getWorkflowTaskLabel(task, index) { + const taskName = normalizeText(task?.name); + return taskName ? `Task ${index + 1} (${taskName})` : `Task ${index + 1}`; } function buildWorkflowPayload() { @@ -3837,35 +4136,13 @@ function buildWorkflowPayload() { instructions: normalizeText(task.instructions), order: index + 1, runner: serializeWorkflowTaskRunner(task.runner), + document_action: serializeWorkflowDocumentAction(task.document_action), })); const runnerType = normalizeText(workflowRunnerTypeSelect?.value) || "model"; const triggerType = normalizeText(workflowTriggerTypeSelect?.value) || "manual"; - const documentActionType = normalizeText(workflowDocumentActionTypeSelect?.value) || DOCUMENT_ACTION_NONE; - const analysisTargetMode = normalizeText(workflowAnalysisTargetModeSelect?.value) === DOCUMENT_ANALYSIS_TARGET_RECENT - ? DOCUMENT_ANALYSIS_TARGET_RECENT - : DOCUMENT_ANALYSIS_TARGET_SELECTED; - const targetDocumentIds = analysisTargetMode === DOCUMENT_ANALYSIS_TARGET_RECENT - ? [] - : getWorkflowPickerSelectedDocumentIds(); - const comparisonLeftDocumentId = normalizeText(workflowComparisonLeftDocumentIdInput?.value); - const comparisonTargetDocumentIds = analysisTargetMode === DOCUMENT_ANALYSIS_TARGET_RECENT - ? [] - : getSelectedWorkflowComparisonTargetIds(); - const selectedDocumentActionIds = documentActionType === DOCUMENT_ACTION_COMPARISON - ? comparisonTargetDocumentIds - : targetDocumentIds; - const comparisonRightDocumentIds = analysisTargetMode === DOCUMENT_ANALYSIS_TARGET_RECENT - ? [] - : comparisonTargetDocumentIds.filter((documentId) => documentId !== comparisonLeftDocumentId); - const analysisGroupIds = parseCsvList(workflowAnalysisGroupIdsInput?.value); - const analysisPublicWorkspaceIds = parseCsvList(workflowAnalysisPublicWorkspaceIdsInput?.value); - const rawWindowSize = normalizeText(workflowAnalysisWindowSizeInput?.value); - const rawWindowPercent = normalizeText(workflowAnalysisWindowPercentInput?.value); - const rawRetries = normalizeText(workflowAnalysisRetriesInput?.value) || "1"; - const rawRecentMinutes = normalizeText(workflowAnalysisRecentMinutesInput?.value) || String(DEFAULT_RECENT_DOCUMENT_WINDOW_MINUTES); - const analysisMode = workflowAnalysisPerDocumentToggle?.checked - ? DOCUMENT_ANALYSIS_MODE_PER_DOCUMENT - : DOCUMENT_ANALYSIS_MODE_COMBINED; + const primaryDocumentAction = tasks.find((task) => task.document_action.type !== DOCUMENT_ACTION_NONE)?.document_action + || tasks[0]?.document_action + || serializeWorkflowDocumentAction(); const fileSyncEnabled = Boolean(workflowFileSyncEnabledToggle?.checked) || triggerType === "file_sync"; const payload = { id: normalizeText(workflowIdInput?.value), @@ -3901,44 +4178,8 @@ function buildWorkflowPayload() { selected_agent: {}, model_endpoint_id: "", model_id: "", - document_action: { - type: documentActionType, - document_ids: documentActionType !== DOCUMENT_ACTION_NONE ? selectedDocumentActionIds : [], - left_document_id: documentActionType === DOCUMENT_ACTION_COMPARISON && analysisTargetMode !== DOCUMENT_ANALYSIS_TARGET_RECENT ? comparisonLeftDocumentId : "", - right_document_ids: documentActionType === DOCUMENT_ACTION_COMPARISON ? comparisonRightDocumentIds : [], - analysis_mode: documentActionType === DOCUMENT_ACTION_ANALYZE ? analysisMode : DOCUMENT_ANALYSIS_MODE_COMBINED, - doc_scope: normalizeText(workflowAnalysisDocScopeSelect?.value) || getWorkflowDocumentScope(), - active_group_ids: documentActionType !== DOCUMENT_ACTION_NONE - ? workflowWorkspaceConfig.scope === "group" && getWorkflowActiveGroupId() - ? [getWorkflowActiveGroupId()] - : analysisGroupIds - : [], - active_public_workspace_id: documentActionType !== DOCUMENT_ACTION_NONE ? analysisPublicWorkspaceIds : [], - window_unit: normalizeText(workflowAnalysisWindowUnitSelect?.value) || "pages", - window_size: rawWindowSize ? Number(rawWindowSize) : null, - window_percent: rawWindowPercent ? Number(rawWindowPercent) : null, - max_retries_per_window: Number(rawRetries), - target_mode: documentActionType !== DOCUMENT_ACTION_NONE ? analysisTargetMode : DOCUMENT_ANALYSIS_TARGET_SELECTED, - recent_window_minutes: Number(rawRecentMinutes), - }, - analyze: { - enabled: documentActionType === DOCUMENT_ACTION_ANALYZE, - document_ids: documentActionType === DOCUMENT_ACTION_ANALYZE ? targetDocumentIds : [], - doc_scope: normalizeText(workflowAnalysisDocScopeSelect?.value) || getWorkflowDocumentScope(), - active_group_ids: documentActionType === DOCUMENT_ACTION_ANALYZE - ? workflowWorkspaceConfig.scope === "group" && getWorkflowActiveGroupId() - ? [getWorkflowActiveGroupId()] - : analysisGroupIds - : [], - active_public_workspace_id: documentActionType === DOCUMENT_ACTION_ANALYZE ? analysisPublicWorkspaceIds : [], - analysis_mode: documentActionType === DOCUMENT_ACTION_ANALYZE ? analysisMode : DOCUMENT_ANALYSIS_MODE_COMBINED, - window_unit: normalizeText(workflowAnalysisWindowUnitSelect?.value) || "pages", - window_size: rawWindowSize ? Number(rawWindowSize) : null, - window_percent: rawWindowPercent ? Number(rawWindowPercent) : null, - max_retries_per_window: Number(rawRetries), - target_mode: documentActionType === DOCUMENT_ACTION_ANALYZE ? analysisTargetMode : DOCUMENT_ANALYSIS_TARGET_SELECTED, - recent_window_minutes: Number(rawRecentMinutes), - }, + document_action: primaryDocumentAction, + analyze: buildWorkflowAnalyzeConfig(primaryDocumentAction), }; if (!payload.name) { @@ -3962,46 +4203,13 @@ function buildWorkflowPayload() { } } const usesDynamicFileSyncTargets = payload.file_sync.enabled && payload.file_sync.use_changed_documents; - if (documentActionType === DOCUMENT_ACTION_SEARCH && analysisTargetMode === DOCUMENT_ANALYSIS_TARGET_SELECTED && !payload.document_action.document_ids.length) { - throw new Error("Select one or more documents for search."); - } - if (documentActionType === DOCUMENT_ACTION_ANALYZE && analysisTargetMode === DOCUMENT_ANALYSIS_TARGET_SELECTED && !payload.document_action.document_ids.length && !usesDynamicFileSyncTargets) { - throw new Error("Select one or more documents for analysis."); - } - if (documentActionType !== DOCUMENT_ACTION_NONE && analysisTargetMode === DOCUMENT_ANALYSIS_TARGET_RECENT && (!Number.isInteger(payload.document_action.recent_window_minutes) || payload.document_action.recent_window_minutes < 1 || payload.document_action.recent_window_minutes > 1440)) { - throw new Error("Recent document window must be between 1 and 1440 minutes."); - } - if (documentActionType === DOCUMENT_ACTION_COMPARISON && analysisTargetMode === DOCUMENT_ANALYSIS_TARGET_SELECTED && payload.document_action.document_ids.length < 2) { - throw new Error("Select at least two document versions for compare."); - } - if (documentActionType === DOCUMENT_ACTION_COMPARISON && analysisTargetMode === DOCUMENT_ANALYSIS_TARGET_SELECTED && !payload.document_action.left_document_id) { - throw new Error("Add one Source document id for compare."); - } - if (documentActionType === DOCUMENT_ACTION_COMPARISON && analysisTargetMode === DOCUMENT_ANALYSIS_TARGET_SELECTED && !payload.document_action.right_document_ids.length) { - throw new Error("Add one or more Target document ids for compare."); - } - if (documentActionType !== DOCUMENT_ACTION_NONE && !isDocumentActionEnabled(documentActionType)) { - throw new Error(`${getDocumentActionDisplayLabel(documentActionType)} is currently disabled by an administrator.`); - } - const documentActionCount = documentActionType === DOCUMENT_ACTION_COMPARISON - ? 1 + payload.document_action.right_document_ids.length - : payload.document_action.document_ids.length; - const workflowMaxDocuments = getWorkflowDocumentActionMaxDocuments(documentActionType); - if (documentActionCount > workflowMaxDocuments) { - throw new Error(`${getDocumentActionDisplayLabel(documentActionType)} workflows support up to ${workflowMaxDocuments} documents per run.`); - } - if (documentActionType !== DOCUMENT_ACTION_NONE && rawWindowSize && (!Number.isInteger(payload.document_action.window_size) || payload.document_action.window_size < 1)) { - throw new Error("Window size must be a whole number greater than zero."); - } - if (documentActionType !== DOCUMENT_ACTION_NONE && rawWindowPercent && (!Number.isInteger(payload.document_action.window_percent) || payload.document_action.window_percent < 1 || payload.document_action.window_percent > 100)) { - throw new Error("Window percent must be a whole number between 1 and 100."); - } - if (documentActionType !== DOCUMENT_ACTION_NONE && rawWindowSize && rawWindowPercent) { - throw new Error("Choose either a fixed window size or a window percent, not both."); - } - if (documentActionType !== DOCUMENT_ACTION_NONE && (!Number.isInteger(payload.document_action.max_retries_per_window) || payload.document_action.max_retries_per_window < 0 || payload.document_action.max_retries_per_window > 5)) { - throw new Error("Retries per window must be between 0 and 5."); - } + payload.tasks.forEach((task, index) => { + validateWorkflowTaskDocumentAction( + task.document_action, + getWorkflowTaskLabel(task, index), + usesDynamicFileSyncTargets, + ); + }); if (payload.file_sync.enabled && !payload.file_sync.sources.length) { throw new Error("Select at least one File Sync source for this workflow."); } @@ -4711,8 +4919,8 @@ function initializeWorkflowEvents() { workflowFileSyncEnabledToggle?.addEventListener("change", updateFileSyncFields); workflowFileSyncWaitModeSelect?.addEventListener("change", updateFileSyncFields); workflowFileSyncContinueModeSelect?.addEventListener("change", updateFileSyncFields); - workflowDocumentActionTypeSelect?.addEventListener("change", updateDocumentActionFields); - workflowAnalysisTargetModeSelect?.addEventListener("change", updateDocumentActionFields); + workflowDocumentActionTypeSelect?.addEventListener("change", handleWorkflowDocumentActionSelectionChanged); + workflowAnalysisTargetModeSelect?.addEventListener("change", handleWorkflowDocumentActionSelectionChanged); workflowComparisonRightDocumentIdsInput?.addEventListener("change", () => { syncWorkflowComparisonLeftOptions(); }); diff --git a/application/single_app/templates/group_workspaces.html b/application/single_app/templates/group_workspaces.html index 18283a58e..12e2b08cf 100644 --- a/application/single_app/templates/group_workspaces.html +++ b/application/single_app/templates/group_workspaces.html @@ -2146,7 +2146,7 @@
Task sequence
Workspace documents
-
Optional. Use a document action only when this workflow needs workspace document context.
+
Optional and saved with the selected task. Use a document action only when this task needs workspace document context.
@@ -2200,7 +2200,7 @@
Workspace documents
No group documents selected in the picker.
- +
Loading documents... diff --git a/application/single_app/templates/workspace.html b/application/single_app/templates/workspace.html index 129b91d4b..1be8f7267 100644 --- a/application/single_app/templates/workspace.html +++ b/application/single_app/templates/workspace.html @@ -1542,7 +1542,7 @@
Task sequence
Workspace documents
-
Optional. Use a document action only when this workflow needs workspace document context.
+
Optional and saved with the selected task. Use a document action only when this task needs workspace document context.
@@ -1596,7 +1596,7 @@
Workspace documents
No workspace documents selected in the picker.
- +
Loading documents... diff --git a/functional_tests/test_workflow_task_document_actions.py b/functional_tests/test_workflow_task_document_actions.py new file mode 100644 index 000000000..82dae07a4 --- /dev/null +++ b/functional_tests/test_workflow_task_document_actions.py @@ -0,0 +1,509 @@ +#!/usr/bin/env python3 +# test_workflow_task_document_actions.py +""" +Functional test for per-task workflow workspace documents and the document picker fix. +Version: 0.250.225 +Implemented in: 0.250.225 + +This test ensures that: + 1. The workflow builder loads the workspace document picker whenever the document + action or document target changes, so the Tags control resolves instead of + staying stuck on "Loading tags...". + 2. The "Refresh documents" button reloads the picker before applying a selection. + 3. Each workflow task owns its own document action, with legacy workflow-level + configuration inherited by task 1 only. + 4. The runner executes every task with that task's own document action. + +Refs microsoft/simplechat#1282 +""" + +import ast +import os +import sys +import uuid +from pathlib import Path + +sys.path.append(os.path.dirname(os.path.abspath(__file__))) + +from test_support.versioning import assert_app_version_at_least + + +ROOT = Path(__file__).resolve().parents[1] +APP_ROOT = ROOT / "application" / "single_app" +STORE_FILE = APP_ROOT / "functions_personal_workflows.py" +GROUP_STORE_FILE = APP_ROOT / "functions_group_workflows.py" +RUNNER_FILE = APP_ROOT / "functions_workflow_runner.py" +WORKFLOW_JS_FILE = APP_ROOT / "static" / "js" / "workspace" / "workspace_workflows.js" +CHAT_DOCUMENTS_JS_FILE = APP_ROOT / "static" / "js" / "chat" / "chat-documents.js" +WORKSPACE_TEMPLATE = APP_ROOT / "templates" / "workspace.html" +GROUP_TEMPLATE = APP_ROOT / "templates" / "group_workspaces.html" +MINIMUM_VERSION = "0.250.225" + + +def read_text(path: Path) -> str: + return path.read_text(encoding="utf-8") + + +def load_functions(path: Path, function_names: set, namespace: dict) -> dict: + parsed = ast.parse(read_text(path), filename=str(path)) + selected_nodes = [ + node + for node in parsed.body + if isinstance(node, ast.FunctionDef) and node.name in function_names + ] + assert len(selected_nodes) == len(function_names), ( + f"Expected functions {sorted(function_names)} in {path.name}" + ) + exec(compile(ast.Module(body=selected_nodes, type_ignores=[]), str(path), "exec"), namespace) + return namespace + + +def build_document_action(action_type="none", **overrides): + """Build a normalized-looking document action config for test fixtures.""" + action = { + "type": action_type, + "doc_scope": "all", + "active_group_ids": [], + "active_public_workspace_id": [], + "window_unit": "pages", + "window_size": None, + "window_percent": None, + "max_retries_per_window": 1, + "document_ids": [], + "left_document_id": "", + "right_document_ids": [], + "analysis_mode": "combined", + "target_mode": "selected", + "recent_window_minutes": 10, + } + action.update(overrides) + return action + + +def load_store_helpers(document_action_error_types=()): + """Load task normalization helpers with an injectable document action normalizer.""" + + def fake_normalize_document_action_config(action_payload=None, **_kwargs): + payload = action_payload if isinstance(action_payload, dict) else {} + action_type = str(payload.get("type") or "none").strip().lower() + if action_type in document_action_error_types: + raise ValueError(f"{action_type} is currently disabled by an administrator.") + if action_type == "none": + return build_document_action() + return build_document_action( + action_type, + document_ids=list(payload.get("document_ids") or []), + left_document_id=str(payload.get("left_document_id") or ""), + right_document_ids=list(payload.get("right_document_ids") or []), + ) + + return load_functions( + STORE_FILE, + { + "_normalize_text", + "normalize_workflow_max_tasks", + "_normalize_workflow_tasks", + "_normalize_task_document_action_config", + }, + { + "uuid": uuid, + "WORKFLOW_TASK_LIMIT_DEFAULT": 50, + "WORKFLOW_TASK_LIMIT_MIN": 1, + "WORKFLOW_TASK_LIMIT_MAX": 100, + "WORKFLOW_MAX_TASKS": 50, + "WORKFLOW_TASK_INSTRUCTIONS_MAX_LENGTH": 12000, + "WORKFLOW_TASK_NAME_MAX_LENGTH": 120, + "WORKFLOW_TASK_RUNNER_TYPES": {"inherit", "agent", "model"}, + "DOCUMENT_ACTION_TYPE_ANALYZE": "analyze", + "DOCUMENT_ACTION_CONTEXT_WORKFLOW": "workflow", + "get_settings": lambda: {}, + "get_document_action_max_documents_by_type": lambda *_args, **_kwargs: {}, + "get_enabled_document_action_types": lambda **_kwargs: ["analyze", "comparison"], + "normalize_document_action_config": fake_normalize_document_action_config, + }, + ) + + +def load_group_scope_helper(): + return load_functions( + GROUP_STORE_FILE, + {"_apply_group_document_action_scope"}, + {}, + ) + + +def load_runner_helpers(): + namespace = { + "DOCUMENT_ACTION_TYPE_NONE": "none", + "DOCUMENT_ACTION_TYPE_ANALYZE": "analyze", + "WORKFLOW_TASK_CONTEXT_MAX_CHARS": 12000, + "build_analyze_config": lambda action: {"enabled": (action or {}).get("type") == "analyze"}, + "_get_document_action_config": lambda source: dict( + (source or {}).get("document_action") or {"type": "none"} + ), + "_format_workflow_file_sync_context": lambda _result: "sync context", + "_get_workflow_file_sync_config": lambda workflow: (workflow or {}).get("file_sync") or {}, + } + return load_functions( + RUNNER_FILE, + { + "_truncate_workflow_task_context", + "_resolve_workflow_task_document_action", + "_build_workflow_task_execution_workflow", + "_apply_file_sync_changed_documents_to_action", + "_apply_file_sync_context_to_workflow", + }, + namespace, + ) + + +def test_document_picker_loads_on_action_change() -> None: + """The picker must load whenever the document action or target mode changes.""" + print("Testing workflow document picker load wiring...") + workflow_js = read_text(WORKFLOW_JS_FILE) + chat_documents_js = read_text(CHAT_DOCUMENTS_JS_FILE) + + assert "function handleWorkflowDocumentActionSelectionChanged()" in workflow_js + assert "function ensureWorkflowDocumentPickerLoaded(" in workflow_js + assert 'workflowDocumentActionTypeSelect?.addEventListener("change", handleWorkflowDocumentActionSelectionChanged)' in workflow_js + assert 'workflowAnalysisTargetModeSelect?.addEventListener("change", handleWorkflowDocumentActionSelectionChanged)' in workflow_js + assert 'workflowDocumentActionTypeSelect?.addEventListener("change", updateDocumentActionFields)' not in workflow_js + + # The stale change handler was the only reason the picker never initialized. + assert "ensureWorkflowDocumentPickerLoaded()" in workflow_js + assert "workflowDocumentPickerLoadToken" in workflow_js + + # The tags dropdown must never be left in its initial loading markup state. + assert "hideTagsDropdown();\n return;" in chat_documents_js + print("PASS: workflow document picker load wiring") + + +def test_refresh_documents_button_reloads_picker() -> None: + """Refresh reloads the picker before applying the current picker selection.""" + print("Testing refresh documents button behavior...") + workflow_js = read_text(WORKFLOW_JS_FILE) + workspace_html = read_text(WORKSPACE_TEMPLATE) + group_html = read_text(GROUP_TEMPLATE) + + refresh_start = workflow_js.index("async function applySelectedWorkspaceDocumentsToWorkflow()") + refresh_body = workflow_js[refresh_start:refresh_start + 1600] + assert "await ensureWorkflowDocumentPickerLoaded({ preserveSelection: true })" in refresh_body + assert "Select one or more ${selectedLabel} documents in the picker first." not in workflow_js + assert "Document list refreshed." in refresh_body + assert "getWorkflowPickerAvailableDocumentCount()" in refresh_body + + for template in (workspace_html, group_html): + assert 'id="workflow-use-selected-documents-btn">Refresh documents<' in template + assert "Refresh selected documents" not in template + assert "saved with the selected task" in template + print("PASS: refresh documents button behavior") + + +def test_task_document_action_frontend_model() -> None: + """Each task carries its own document action through the builder and payload.""" + print("Testing per-task document action frontend model...") + workflow_js = read_text(WORKFLOW_JS_FILE) + + for expected_symbol in ( + "function createDefaultWorkflowTaskDocumentAction()", + "function normalizeWorkflowTaskDocumentAction(", + "function readWorkflowDocumentActionFromForm()", + "function applyWorkflowDocumentActionToForm(", + "function serializeWorkflowDocumentAction(", + "function validateWorkflowTaskDocumentAction(", + ): + assert expected_symbol in workflow_js, f"Missing {expected_symbol}" + + # The editor writes to and reads from the active task. + assert "activeTask.document_action = readWorkflowDocumentActionFromForm();" in workflow_js + assert "applyWorkflowDocumentActionToForm(activeTask?.document_action);" in workflow_js + # New tasks reset the workspace document fields. + assert "document_action: createDefaultWorkflowTaskDocumentAction()," in workflow_js + # Payload carries per-task actions plus a mirrored workflow-level action. + assert "document_action: serializeWorkflowDocumentAction(task.document_action)," in workflow_js + assert "const primaryDocumentAction = tasks.find((task) => task.document_action.type !== DOCUMENT_ACTION_NONE)?.document_action" in workflow_js + assert "document_action: primaryDocumentAction," in workflow_js + assert "analyze: buildWorkflowAnalyzeConfig(primaryDocumentAction)," in workflow_js + # Legacy workflow-level config hydrates task 1 only. + assert "if (index === 0 && !(task?.document_action && typeof task.document_action === \"object\") && workflowDocumentAction)" in workflow_js + print("PASS: per-task document action frontend model") + + +def test_task_normalization_persists_document_actions() -> None: + """Task normalization stores a document action per task.""" + print("Testing per-task document action normalization...") + helpers = load_store_helpers() + normalize_tasks = helpers["_normalize_workflow_tasks"] + normalize_task_action = helpers["_normalize_task_document_action_config"] + + tasks = normalize_tasks( + { + "tasks": [ + { + "id": "analyze", + "name": "Analyze", + "instructions": "Analyze the contracts.", + "document_action": {"type": "analyze", "document_ids": ["doc-a"]}, + }, + { + "id": "compare", + "name": "Compare", + "instructions": "Compare the amendments.", + "document_action": { + "type": "comparison", + "left_document_id": "doc-v1", + "right_document_ids": ["doc-v2"], + "document_ids": ["doc-v1", "doc-v2"], + }, + }, + { + "id": "report", + "name": "Report", + "instructions": "Write the summary.", + "document_action": {"type": "none"}, + }, + ] + }, + task_document_action_normalizer=normalize_task_action, + ) + + assert [task["document_action"]["type"] for task in tasks] == ["analyze", "comparison", "none"] + assert tasks[0]["document_action"]["document_ids"] == ["doc-a"] + assert tasks[1]["document_action"]["left_document_id"] == "doc-v1" + assert tasks[1]["document_action"]["right_document_ids"] == ["doc-v2"] + assert tasks[2]["document_action"]["document_ids"] == [] + print("PASS: per-task document action normalization") + + +def test_legacy_workflow_action_inherits_first_task_only() -> None: + """Pre-migration workflows apply their single action to task 1 only.""" + print("Testing legacy document action inheritance...") + helpers = load_store_helpers() + normalize_tasks = helpers["_normalize_workflow_tasks"] + normalize_task_action = helpers["_normalize_task_document_action_config"] + legacy_action = build_document_action("analyze", document_ids=["legacy-doc"]) + + tasks = normalize_tasks( + { + "tasks": [ + {"id": "one", "name": "One", "instructions": "First task."}, + {"id": "two", "name": "Two", "instructions": "Second task."}, + ] + }, + task_document_action_normalizer=normalize_task_action, + default_document_action=legacy_action, + ) + + assert tasks[0]["document_action"]["type"] == "analyze" + assert tasks[0]["document_action"]["document_ids"] == ["legacy-doc"] + assert tasks[1]["document_action"]["type"] == "none" + + # Without a normalizer the tasks stay untouched, preserving stored records. + untouched = normalize_tasks( + {"tasks": [{"id": "one", "name": "One", "instructions": "First task."}]} + ) + assert "document_action" not in untouched[0] + print("PASS: legacy document action inheritance") + + +def test_invalid_task_document_action_reports_task_number() -> None: + """Per-task document action failures identify the offending task.""" + print("Testing per-task document action validation errors...") + helpers = load_store_helpers(document_action_error_types={"comparison"}) + normalize_tasks = helpers["_normalize_workflow_tasks"] + normalize_task_action = helpers["_normalize_task_document_action_config"] + + try: + normalize_tasks( + { + "tasks": [ + {"id": "one", "name": "Collect", "instructions": "First task."}, + { + "id": "two", + "name": "Compare", + "instructions": "Second task.", + "document_action": {"type": "comparison"}, + }, + ] + }, + task_document_action_normalizer=normalize_task_action, + ) + except ValueError as exc: + assert "Workflow task 2 (Compare)" in str(exc), str(exc) + else: + raise AssertionError("Expected an invalid per-task document action to raise.") + print("PASS: per-task document action validation errors") + + +def test_group_task_document_actions_stay_in_group_scope() -> None: + """Group workflows force every task action into the owning group workspace.""" + print("Testing group task document action scoping...") + helpers = load_group_scope_helper() + apply_scope = helpers["_apply_group_document_action_scope"] + + scoped = apply_scope( + "group-123", + build_document_action( + "analyze", + doc_scope="all", + active_group_ids=["other-group"], + active_public_workspace_id=["public-1"], + ), + ) + assert scoped["doc_scope"] == "group" + assert scoped["active_group_ids"] == ["group-123"] + assert scoped["active_public_workspace_id"] == [] + + untouched = apply_scope("group-123", build_document_action()) + assert untouched["type"] == "none" + assert untouched["active_group_ids"] == [] + + group_source = read_text(GROUP_STORE_FILE) + assert "task_document_action_normalizer=lambda action_payload: _apply_group_document_action_scope(" in group_source + print("PASS: group task document action scoping") + + +def test_runner_executes_each_task_with_its_own_action() -> None: + """Every task runs with its own document action, not just task 1.""" + print("Testing runner per-task document action execution...") + helpers = load_runner_helpers() + build_execution_workflow = helpers["_build_workflow_task_execution_workflow"] + + workflow = { + "document_action": build_document_action("analyze", document_ids=["workflow-doc"]), + "tasks": [], + } + first_task = { + "id": "one", + "name": "One", + "order": 1, + "instructions": "Analyze the intake set.", + "document_action": build_document_action("analyze", document_ids=["task-one-doc"]), + } + second_task = { + "id": "two", + "name": "Two", + "order": 2, + "instructions": "Compare the amendments.", + "document_action": build_document_action( + "comparison", + document_ids=["doc-v1", "doc-v2"], + left_document_id="doc-v1", + right_document_ids=["doc-v2"], + ), + } + + first_execution = build_execution_workflow(workflow, first_task, include_document_action=True) + second_execution = build_execution_workflow( + workflow, + second_task, + previous_reply="prior output", + include_document_action=False, + ) + + assert first_execution["document_action"]["document_ids"] == ["task-one-doc"] + assert first_execution["analyze"]["enabled"] is True + assert second_execution["document_action"]["type"] == "comparison" + assert second_execution["document_action"]["left_document_id"] == "doc-v1" + assert second_execution["analyze"]["enabled"] is False + assert "[Previous workflow task output]" in second_execution["task_prompt"] + print("PASS: runner per-task document action execution") + + +def test_runner_legacy_tasks_keep_first_task_only_behavior() -> None: + """Tasks stored without a document action fall back to the legacy contract.""" + print("Testing runner legacy document action fallback...") + helpers = load_runner_helpers() + build_execution_workflow = helpers["_build_workflow_task_execution_workflow"] + workflow = {"document_action": build_document_action("analyze", document_ids=["legacy-doc"])} + + first_execution = build_execution_workflow( + workflow, + {"id": "one", "name": "One", "order": 1, "instructions": "First."}, + include_document_action=True, + ) + second_execution = build_execution_workflow( + workflow, + {"id": "two", "name": "Two", "order": 2, "instructions": "Second."}, + include_document_action=False, + ) + + assert first_execution["document_action"]["document_ids"] == ["legacy-doc"] + assert second_execution["document_action"] == {"type": "none"} + assert second_execution["analyze"] == {"enabled": False} + print("PASS: runner legacy document action fallback") + + +def test_file_sync_changed_documents_reach_task_actions() -> None: + """File Sync changed documents update task-level analyze actions.""" + print("Testing File Sync changed document propagation...") + helpers = load_runner_helpers() + apply_file_sync = helpers["_apply_file_sync_context_to_workflow"] + + workflow = { + "task_prompt": "Run the workflow.", + "file_sync": { + "use_changed_documents": True, + "sources": [{"scope_type": "group", "scope_id": "group-1"}], + }, + "document_action": build_document_action("analyze"), + "tasks": [ + {"id": "one", "document_action": build_document_action("analyze")}, + {"id": "two", "document_action": build_document_action("none")}, + ], + } + + prepared = apply_file_sync( + workflow, + {"enabled": True, "changed_document_ids": ["changed-1", "changed-2"]}, + ) + + assert prepared["document_action"]["document_ids"] == ["changed-1", "changed-2"] + assert prepared["tasks"][0]["document_action"]["document_ids"] == ["changed-1", "changed-2"] + assert prepared["tasks"][0]["document_action"]["active_group_ids"] == ["group-1"] + assert prepared["tasks"][1]["document_action"]["type"] == "none" + + no_changes = apply_file_sync(workflow, {"enabled": True, "changed_document_ids": []}) + assert no_changes["document_action"] == {"type": "none"} + assert no_changes["tasks"][0]["document_action"] == {"type": "none"} + print("PASS: File Sync changed document propagation") + + +def test_version_contract() -> None: + """The fix ships at or after its implementation version.""" + print("Testing version contract...") + assert_app_version_at_least(MINIMUM_VERSION) + print("PASS: version contract") + + +def run_tests() -> bool: + tests = [ + test_document_picker_loads_on_action_change, + test_refresh_documents_button_reloads_picker, + test_task_document_action_frontend_model, + test_task_normalization_persists_document_actions, + test_legacy_workflow_action_inherits_first_task_only, + test_invalid_task_document_action_reports_task_number, + test_group_task_document_actions_stay_in_group_scope, + test_runner_executes_each_task_with_its_own_action, + test_runner_legacy_tasks_keep_first_task_only_behavior, + test_file_sync_changed_documents_reach_task_actions, + test_version_contract, + ] + results = [] + for test in tests: + print(f"Running {test.__name__}...") + try: + test() + results.append(True) + except Exception as exc: + print(f"FAIL: {exc}") + import traceback + traceback.print_exc() + results.append(False) + print(f"Results: {sum(results)}/{len(results)} tests passed") + return all(results) + + +if __name__ == "__main__": + raise SystemExit(0 if run_tests() else 1) diff --git a/functional_tests/test_workflow_task_sequence.py b/functional_tests/test_workflow_task_sequence.py index fcbc19efb..d92e0890b 100644 --- a/functional_tests/test_workflow_task_sequence.py +++ b/functional_tests/test_workflow_task_sequence.py @@ -1,26 +1,33 @@ # test_workflow_task_sequence.py """ Functional test for ordered workflow task sequences. -Version: 0.250.129 +Version: 0.250.225 Implemented in: 0.250.064 Enhanced in: 0.250.065 Enhanced in: 0.250.129 +Enhanced in: 0.250.225 This test ensures workflow tasks are normalized in order, execute with bounded prior-task context, retry safely, and honor halt or continue error strategies. """ import ast +import os +import sys import uuid from pathlib import Path +sys.path.append(os.path.dirname(os.path.abspath(__file__))) + +from test_support.versioning import assert_app_version_at_least + ROOT = Path(__file__).resolve().parents[1] APP_ROOT = ROOT / "application" / "single_app" STORE_FILE = APP_ROOT / "functions_personal_workflows.py" GROUP_STORE_FILE = APP_ROOT / "functions_group_workflows.py" RUNNER_FILE = APP_ROOT / "functions_workflow_runner.py" -EXPECTED_VERSION = "0.250.129" +MINIMUM_VERSION = "0.250.225" def read_text(path: Path) -> str: @@ -84,12 +91,16 @@ def load_runner_helpers(dispatch, personal_runner_normalizer=None, group_runner_ "_save_workflow_run_item_record": lambda workflow, item: saved_items.append(dict(item)) or item, "_utc_now_iso": lambda: next(timestamps), "build_analyze_config": lambda action: {"enabled": action.get("type") == "analyze"}, + "_get_document_action_config": lambda source: dict( + (source or {}).get("document_action") or {"type": "none"} + ), "uuid": uuid, } helpers = load_functions( RUNNER_FILE, { "_truncate_workflow_task_context", + "_resolve_workflow_task_document_action", "_build_workflow_task_execution_workflow", "_get_workflow_task_requested_runner_mode", "_build_workflow_task_runner_audit", @@ -797,9 +808,8 @@ def dispatch(workflow, *_args, **_kwargs): def test_version_and_legacy_dispatch_contract() -> None: """Keep the version and legacy no-task dispatch branch explicit.""" - config_content = read_text(APP_ROOT / "config.py") runner_content = read_text(RUNNER_FILE) - assert f'VERSION = "{EXPECTED_VERSION}"' in config_content + assert_app_version_at_least(MINIMUM_VERSION) assert "if execution_workflow.get('tasks'):" in runner_content assert "execution_result = _execute_workflow_dispatch(" in runner_content From 2cbec64d0734ebe2c8b9c4e1a14a97b94b08532a Mon Sep 17 00:00:00 2001 From: paullizer Date: Tue, 18 Aug 2026 16:15:52 -0400 Subject: [PATCH 2/2] Address review: contain task action failures, scope run items per task, fix zero retries Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../single_app/functions_workflow_runner.py | 33 ++- .../single_app/route_backend_workflows.py | 97 ++++++- .../js/workspace/workspace_workflows.js | 34 ++- .../WORKFLOW_PER_TASK_WORKSPACE_DOCUMENTS.md | 269 ++++++++++++++++++ docs/explanation/features/index.md | 1 + .../WORKFLOW_TASK_DOCUMENT_PICKER_FIX.md | 160 +++++++++++ docs/explanation/fixes/index.md | 3 +- docs/explanation/release_notes.md | 45 +++ ...workflow_document_picker_recent_targets.py | 19 +- ...est_workflow_per_document_analysis_mode.py | 5 +- .../test_workflow_task_document_actions.py | 143 ++++++++++ 11 files changed, 768 insertions(+), 41 deletions(-) create mode 100644 docs/explanation/features/WORKFLOW_PER_TASK_WORKSPACE_DOCUMENTS.md create mode 100644 docs/explanation/fixes/WORKFLOW_TASK_DOCUMENT_PICKER_FIX.md diff --git a/application/single_app/functions_workflow_runner.py b/application/single_app/functions_workflow_runner.py index 301cbcf4a..1ee221e2d 100644 --- a/application/single_app/functions_workflow_runner.py +++ b/application/single_app/functions_workflow_runner.py @@ -7105,8 +7105,18 @@ def _apply_file_sync_context_to_workflow(workflow, file_sync_result): return prepared_workflow -def _document_run_item_id(run_id, document_id): +def _get_workflow_active_task(workflow): + active_task = (workflow or {}).get('active_task') + return active_task if isinstance(active_task, dict) else {} + + +def _document_run_item_id(run_id, document_id, task_id=''): normalized_document_id = re.sub(r'[^a-zA-Z0-9._-]+', '-', str(document_id or '').strip()) + normalized_task_id = re.sub(r'[^a-zA-Z0-9._-]+', '-', str(task_id or '').strip()) + if normalized_task_id: + # Per-task document actions let two tasks target the same document in one run, + # so the run item has to be scoped to the task or statuses overwrite each other. + return f'{run_id}:task:{normalized_task_id}:document:{normalized_document_id}' return f'{run_id}:document:{normalized_document_id}' @@ -7130,8 +7140,10 @@ def _save_document_run_item(workflow, run_id, document_id, status, *, file_sync_ now_iso = _utc_now_iso() file_sync_document = _file_sync_document_details(file_sync_result or {}, document_id) + active_task = _get_workflow_active_task(workflow) + task_id = str(active_task.get('id') or '').strip() item = { - 'id': _document_run_item_id(run_id, document_id), + 'id': _document_run_item_id(run_id, document_id, task_id=task_id), 'type': 'workflow_run_item', 'item_type': 'document', 'run_id': run_id, @@ -7139,6 +7151,8 @@ def _save_document_run_item(workflow, run_id, document_id, status, *, file_sync_ 'workflow_id': workflow.get('id'), 'group_id': _get_workflow_group_id(workflow) or None, 'workflow_name': workflow.get('name'), + 'task_id': task_id or None, + 'task_name': str(active_task.get('name') or '').strip() or None, 'document_id': document_id, 'label': _document_label_from_file_sync(file_sync_result or {}, document_id), 'source': 'file_sync' if file_sync_document else 'workflow', @@ -9513,13 +9527,6 @@ def raise_if_cancelled(): title=str(task.get('name') or f'Task {task_index + 1}'), status='running', ) - prepared_workflow = _build_workflow_task_execution_workflow( - workflow, - task, - previous_reply=previous_reply, - include_document_action=task_index == 0, - ) - task_result = None task_error = '' attempt_count = 0 @@ -9527,6 +9534,14 @@ def raise_if_cancelled(): raise_if_cancelled() attempt_count = attempt_index + 1 try: + # Built inside the attempt so an invalid task document action fails this + # task through the normal retry and error strategy instead of the whole run. + prepared_workflow = _build_workflow_task_execution_workflow( + workflow, + task, + previous_reply=previous_reply, + include_document_action=task_index == 0, + ) attempt_workflow, runner_audit = _resolve_workflow_task_runner( prepared_workflow, task, diff --git a/application/single_app/route_backend_workflows.py b/application/single_app/route_backend_workflows.py index f16b1b3d4..9acd4cee5 100644 --- a/application/single_app/route_backend_workflows.py +++ b/application/single_app/route_backend_workflows.py @@ -34,7 +34,7 @@ ) from functions_group import require_active_group from functions_public_workspaces import require_active_public_workspace -from functions_document_actions import DOCUMENT_ACTION_TYPE_ANALYZE, build_analyze_config +from functions_document_actions import DOCUMENT_ACTION_TYPE_ANALYZE, DOCUMENT_ACTION_TYPE_NONE, build_analyze_config from functions_thoughts import get_thoughts_for_message from functions_workflow_activity import build_workflow_activity_snapshot from functions_msgraph_pending_actions import list_msgraph_pending_actions, sanitize_msgraph_pending_action_for_client @@ -324,18 +324,62 @@ def _collect_group_workflow_file_sync_sources(user_id, group_id, settings=None): ] +def _force_group_document_action_scope(action_config, group_id): + """Keep a resumed group workflow document action inside the owning group workspace.""" + action_config = dict(action_config) if isinstance(action_config, dict) else {'type': DOCUMENT_ACTION_TYPE_NONE} + if action_config.get('type') == DOCUMENT_ACTION_TYPE_NONE: + return action_config + + action_config['doc_scope'] = 'group' + action_config['active_group_ids'] = [group_id] + action_config['active_public_workspace_id'] = [] + return action_config + + +def _narrow_analyze_action_to_documents(action_config, document_ids, group_ids, public_workspace_ids): + """Point an analyze action at a specific document set, or disable it when empty.""" + action_config = action_config if isinstance(action_config, dict) else {} + if action_config.get('type') != DOCUMENT_ACTION_TYPE_ANALYZE: + return None + if not document_ids: + return {'type': DOCUMENT_ACTION_TYPE_NONE} + + narrowed_action = dict(action_config) + narrowed_action.update({ + 'document_ids': list(document_ids), + 'doc_scope': 'all', + 'active_group_ids': group_ids or list(action_config.get('active_group_ids') or []), + 'active_public_workspace_id': public_workspace_ids or list(action_config.get('active_public_workspace_id') or []), + }) + return narrowed_action + + def _build_resume_failed_workflow(workflow, failed_items): action_config = workflow.get('document_action') if isinstance(workflow.get('document_action'), dict) else {} - if action_config.get('type') != DOCUMENT_ACTION_TYPE_ANALYZE: + tasks = workflow.get('tasks') if isinstance(workflow.get('tasks'), list) else [] + task_analyze_ids = { + _normalize_identifier(task.get('id')) + for task in tasks + if isinstance(task, dict) + and isinstance(task.get('document_action'), dict) + and task['document_action'].get('type') == DOCUMENT_ACTION_TYPE_ANALYZE + } + if action_config.get('type') != DOCUMENT_ACTION_TYPE_ANALYZE and not task_analyze_ids: raise ValueError('Resume failed items currently supports Analyze workflows.') document_ids = [] + document_ids_by_task = {} group_ids = [] public_workspace_ids = [] for item in failed_items: document_id = _normalize_identifier(item.get('document_id')) if document_id and document_id not in document_ids: document_ids.append(document_id) + task_id = _normalize_identifier(item.get('task_id')) + if document_id and task_id: + task_documents = document_ids_by_task.setdefault(task_id, []) + if document_id not in task_documents: + task_documents.append(document_id) scope_type = _normalize_identifier(item.get('scope_type')).lower() scope_id = _normalize_identifier(item.get('scope_id')) if scope_type == FILE_SYNC_SCOPE_GROUP and scope_id and scope_id not in group_ids: @@ -347,15 +391,33 @@ def _build_resume_failed_workflow(workflow, failed_items): raise ValueError('No failed document items are available to resume.') resume_workflow = dict(workflow) - resume_action = dict(action_config) - resume_action.update({ - 'document_ids': document_ids, - 'doc_scope': 'all', - 'active_group_ids': group_ids or list(action_config.get('active_group_ids') or []), - 'active_public_workspace_id': public_workspace_ids or list(action_config.get('active_public_workspace_id') or []), - }) + resume_action = _narrow_analyze_action_to_documents( + action_config, + document_ids, + group_ids, + public_workspace_ids, + ) or dict(action_config) resume_workflow['document_action'] = resume_action resume_workflow['analyze'] = build_analyze_config(resume_action) + + if tasks: + # Task document actions take precedence at run time, so narrow them too or the + # resume would re-run every document the task originally targeted. + resume_tasks = [] + for task in tasks: + resume_task = dict(task) if isinstance(task, dict) else {} + task_id = _normalize_identifier(resume_task.get('id')) + narrowed_task_action = _narrow_analyze_action_to_documents( + resume_task.get('document_action'), + document_ids_by_task.get(task_id, [] if document_ids_by_task else document_ids), + group_ids, + public_workspace_ids, + ) + if narrowed_task_action is not None: + resume_task['document_action'] = narrowed_task_action + resume_tasks.append(resume_task) + resume_workflow['tasks'] = resume_tasks + resume_workflow['file_sync'] = { 'enabled': False, 'wait_mode': 'complete', @@ -1374,11 +1436,18 @@ def resume_failed_group_workflow_items(workflow_id, run_id): resume_workflow = _build_resume_failed_workflow(workflow, failed_items) resume_action = resume_workflow.get('document_action') if isinstance(resume_workflow.get('document_action'), dict) else {} if resume_action: - resume_action['doc_scope'] = 'group' - resume_action['active_group_ids'] = [group_id] - resume_action['active_public_workspace_id'] = [] - resume_workflow['document_action'] = resume_action - resume_workflow['analyze'] = build_analyze_config(resume_action) + resume_workflow['document_action'] = _force_group_document_action_scope(resume_action, group_id) + resume_workflow['analyze'] = build_analyze_config(resume_workflow['document_action']) + if isinstance(resume_workflow.get('tasks'), list): + resume_workflow['tasks'] = [ + { + **task, + 'document_action': _force_group_document_action_scope(task.get('document_action'), group_id), + } + if isinstance(task, dict) and isinstance(task.get('document_action'), dict) + else task + for task in resume_workflow['tasks'] + ] except ValueError as exc: return jsonify({'error': str(exc)}), 400 diff --git a/application/single_app/static/js/workspace/workspace_workflows.js b/application/single_app/static/js/workspace/workspace_workflows.js index 43af124d2..f5163010e 100644 --- a/application/single_app/static/js/workspace/workspace_workflows.js +++ b/application/single_app/static/js/workspace/workspace_workflows.js @@ -568,10 +568,12 @@ function readWorkflowDocumentActionFromForm() { window_unit: normalizeText(workflowAnalysisWindowUnitSelect?.value) || "pages", window_size: normalizeText(workflowAnalysisWindowSizeInput?.value), window_percent: normalizeText(workflowAnalysisWindowPercentInput?.value), - max_retries_per_window: normalizeText(workflowAnalysisRetriesInput?.value) || "1", + max_retries_per_window: normalizeWorkflowNumericField(workflowAnalysisRetriesInput?.value, "1"), target_mode: targetMode, - recent_window_minutes: normalizeText(workflowAnalysisRecentMinutesInput?.value) - || String(DEFAULT_RECENT_DOCUMENT_WINDOW_MINUTES), + recent_window_minutes: normalizeWorkflowNumericField( + workflowAnalysisRecentMinutesInput?.value, + String(DEFAULT_RECENT_DOCUMENT_WINDOW_MINUTES), + ), }; } @@ -1003,6 +1005,7 @@ function removeWorkflowTask(taskId) { showToast("A workflow requires at least one task.", "warning"); return; } + syncActiveWorkflowTaskFromEditor(); const taskIndex = workflowTasks.findIndex((task) => task.id === taskId); if (taskIndex < 0) { return; @@ -2183,15 +2186,17 @@ async function initializeWorkflowDocumentPicker(documentAction = {}, options = { } const actionType = normalizeText(documentAction.type || workflowDocumentActionTypeSelect?.value) || DOCUMENT_ACTION_NONE; + // Bump first so any in-flight load for a previous task cannot apply its scopes or + // selection to the picker after this call resolves. + const requestToken = workflowDocumentPickerLoadToken + 1; + workflowDocumentPickerLoadToken = requestToken; + if (actionType === DOCUMENT_ACTION_NONE) { setWorkflowPickerError(""); setWorkflowPickerLoadingState(false); return; } - const requestToken = workflowDocumentPickerLoadToken + 1; - workflowDocumentPickerLoadToken = requestToken; - setWorkflowPickerError(""); setWorkflowPickerLoadingState(true); syncWorkflowPickerActionType(); @@ -3995,6 +4000,16 @@ async function openWorkflowModal(workflow = null) { await ensureWorkflowDocumentPickerLoaded(); } +function normalizeWorkflowNumericField(value, fallback) { + // normalizeText() coerces a numeric 0 to "", so truthiness cannot be used here: + // 0 is a valid retries-per-window value and must survive serialization. + if (value === null || value === undefined) { + return fallback; + } + const normalizedValue = normalizeText(value); + return normalizedValue === "" ? fallback : normalizedValue; +} + function serializeWorkflowDocumentAction(rawAction) { const source = rawAction && typeof rawAction === "object" ? rawAction : createDefaultWorkflowTaskDocumentAction(); const actionType = normalizeWorkflowDocumentActionType(source.type); @@ -4005,8 +4020,11 @@ function serializeWorkflowDocumentAction(rawAction) { const isRecentMode = targetMode === DOCUMENT_ANALYSIS_TARGET_RECENT; const rawWindowSize = normalizeText(source.window_size); const rawWindowPercent = normalizeText(source.window_percent); - const rawRetries = normalizeText(source.max_retries_per_window) || "1"; - const rawRecentMinutes = normalizeText(source.recent_window_minutes) || String(DEFAULT_RECENT_DOCUMENT_WINDOW_MINUTES); + const rawRetries = normalizeWorkflowNumericField(source.max_retries_per_window, "1"); + const rawRecentMinutes = normalizeWorkflowNumericField( + source.recent_window_minutes, + String(DEFAULT_RECENT_DOCUMENT_WINDOW_MINUTES), + ); const activeGroupId = getWorkflowActiveGroupId(); const comparisonLeftDocumentId = actionType === DOCUMENT_ACTION_COMPARISON && !isRecentMode ? normalizeText(source.left_document_id) diff --git a/docs/explanation/features/WORKFLOW_PER_TASK_WORKSPACE_DOCUMENTS.md b/docs/explanation/features/WORKFLOW_PER_TASK_WORKSPACE_DOCUMENTS.md new file mode 100644 index 000000000..a7969e100 --- /dev/null +++ b/docs/explanation/features/WORKFLOW_PER_TASK_WORKSPACE_DOCUMENTS.md @@ -0,0 +1,269 @@ +# Workflow Per-Task Workspace Documents + +**Implemented in version: 0.250.225** +**Issue:** [microsoft/simplechat#1282](https://github.com/microsoft/simplechat/issues/1282) + +## Overview + +Workflow tasks are now self-contained. Each task in a workflow owns its own **Workspace +documents** configuration — document action, document target, picker selection, compare +source and targets, analysis mode, and windowing — instead of sharing a single workflow-level +document action. + +This makes sequenced document work possible. For example, task 1 can analyze an intake set, +task 2 can compare a different pair of document versions, and task 3 can run with no document +context at all, passing each task's response forward as bounded context. + +### Why + +Before this change, `document_action` was stored on the workflow. The builder's Workspace +documents card lived inside the Tasks step but was bound to the workflow, so it did not reset +when you added a task or restore when you switched back to a previously configured task. At +run time, `_execute_workflow_task_sequence()` passed `include_document_action=task_index == 0`, +so only task 1 ever received the document action and tasks 2..N always executed with +`{'type': 'none'}`. + +### Dependencies + +- Workflows (personal and group) must be enabled by an administrator. +- `Analyze` and `Compare` require their respective document action capabilities to be enabled + in Admin Settings; `Search` is always available. +- Depends on the workflow document picker fix documented in + `docs/explanation/fixes/WORKFLOW_TASK_DOCUMENT_PICKER_FIX.md`. + +## Technical Specifications + +### Architecture + +```mermaid +flowchart TD + A[Task editor form] -->|readWorkflowDocumentActionFromForm| B[task.document_action] + B -->|applyWorkflowDocumentActionToForm| A + B -->|serializeWorkflowDocumentAction| C[payload.tasks_i_.document_action] + C --> D[_normalize_workflow_tasks] + D -->|task_document_action_normalizer| E[normalize_document_action_config] + E --> F[Cosmos workflow record] + F --> G[_execute_workflow_task_sequence] + G -->|_resolve_workflow_task_document_action| H[_execute_workflow_dispatch] +``` + +### Data Model + +Each stored task gains a normalized `document_action`: + +```json +{ + "id": "task-1", + "type": "instructions", + "name": "Analyze intake", + "instructions": "Analyze the intake documents.", + "order": 1, + "runner": { "type": "inherit" }, + "document_action": { + "type": "analyze", + "document_ids": ["doc-a", "doc-b"], + "left_document_id": "", + "right_document_ids": [], + "analysis_mode": "combined", + "doc_scope": "all", + "active_group_ids": [], + "active_public_workspace_id": [], + "window_unit": "pages", + "window_size": null, + "window_percent": null, + "max_retries_per_window": 1, + "target_mode": "selected", + "recent_window_minutes": 10 + } +} +``` + +The workflow record keeps a top-level `document_action` and `analyze` block, mirrored from the +first task whose action type is not `none` (falling back to task 1). This keeps the workflow +list summary, the run-resume path in `route_backend_workflows.py`, and existing API consumers +working unchanged. + +### Backward Compatibility + +| Scenario | Behavior | +|---|---| +| Workflow saved before 0.250.225, opened in the builder | Its workflow-level `document_action` hydrates **task 1 only**, matching how it actually executed | +| Workflow saved before 0.250.225, executed without being re-saved | `_resolve_workflow_task_document_action()` falls back to the workflow-level action for task index 0 and `none` afterwards | +| Workflow re-saved through the builder | Every task persists its own `document_action`; task 1 keeps the inherited configuration | +| `tasks` omitted from a save payload | Stored tasks are returned untouched, so legacy records are not silently rewritten | + +### File Structure + +| File | Responsibility | +|---|---| +| `application/single_app/static/js/workspace/workspace_workflows.js` | Per-task model, form binding, serialization, validation, review and task-card summaries | +| `application/single_app/templates/workspace.html` | Personal workflow builder markup and copy | +| `application/single_app/templates/group_workspaces.html` | Group workflow builder markup and copy | +| `application/single_app/functions_personal_workflows.py` | `_normalize_workflow_tasks()` per-task normalization, `_normalize_task_document_action_config()` | +| `application/single_app/functions_group_workflows.py` | `_apply_group_document_action_scope()` group scoping per task | +| `application/single_app/functions_workflow_runner.py` | `_resolve_workflow_task_document_action()`, per-task File Sync target rewriting | + +### Key Frontend Functions + +| Function | Purpose | +|---|---| +| `createDefaultWorkflowTaskDocumentAction()` | Empty configuration used by new tasks | +| `normalizeWorkflowTaskDocumentAction(raw)` | Normalize a stored or inherited action into the builder model | +| `readWorkflowDocumentActionFromForm()` | Capture the live form and picker state into the active task | +| `applyWorkflowDocumentActionToForm(action)` | Restore a task's configuration into the form and picker | +| `serializeWorkflowDocumentAction(action)` | Convert the builder model into the save payload shape | +| `validateWorkflowTaskDocumentAction(payload, taskLabel, usesDynamicFileSyncTargets)` | Per-task validation with task-numbered messages | + +### Key Backend Functions + +`_normalize_workflow_tasks()` accepts two new optional parameters, following the existing +injectable-callable style used for task runners: + +```python +tasks = _normalize_workflow_tasks( + workflow_data, + existing_workflow=existing_workflow, + task_runner_normalizer=..., + max_tasks=get_workflow_max_tasks(settings), + task_document_action_normalizer=lambda action_payload: _normalize_task_document_action_config( + action_payload, + allow_empty_file_sync_targets=allow_empty_file_sync_targets, + settings=settings, + ), + default_document_action=document_action, +) +``` + +`save_personal_workflow()` and `save_group_workflow()` now compute `file_sync` and the +workflow-level `document_action` **before** normalizing tasks, because both are inputs to the +per-task normalizer. + +Group workflows wrap the normalizer with `_apply_group_document_action_scope()`, so every task +action is forced to `doc_scope='group'`, `active_group_ids=[group_id]`, and +`active_public_workspace_id=[]`. Tasks cannot widen scope beyond the owning group workspace. + +### Runner Behavior + +`_resolve_workflow_task_document_action()` decides what each task executes with: + +1. If the task has a `document_action` dict, use it. +2. Otherwise, if this is task index 0 of a legacy record, use the workflow-level action. +3. Otherwise, use `{'type': 'none'}`. + +`_build_workflow_task_execution_workflow()` sets both `document_action` and `analyze` on the +prepared per-task workflow, so `_execute_workflow_dispatch()` routes each task independently +through search, analyze, comparison, agent, or direct-model execution. + +It is called from **inside** the per-attempt `try` block in `_execute_workflow_task_sequence()`. +Normalizing a task's action can raise (for example when an administrator later disables the +Analyze or Compare capability, or lowers the workflow document limit), and building inside the +attempt means that failure retries and honors the workflow's failure strategy, and is recorded +as a failed task run item, instead of aborting the whole run. + +### Run History + +Document run item ids are scoped by task (`{run_id}:task:{task_id}:document:{document_id}`) +because two tasks in the same run can now target the same document. Without task scoping the +second task's status would overwrite the first task's, masking a failure. Each item also +records `task_id` and `task_name`. Legacy single-dispatch runs, which have no active task, +keep the original `{run_id}:document:{document_id}` id. + +**Resume failed items** narrows every task-level analyze action to the failed documents +attributed to that task. A task with an analyze action but no failed documents is downgraded to +`none` so it does not re-run its whole set, and run items recorded before task scoping existed +fall back to the full failed set. + +### File Sync Interaction + +When File Sync runs with **Use changed documents**, `_apply_file_sync_context_to_workflow()` +now rewrites every task-level `analyze` action with the changed document ids and the configured +source scopes, in addition to the workflow-level action. If no documents changed, those analyze +actions become `{'type': 'none'}` so the task runs without document context rather than +analyzing a stale set. + +Saving is relaxed the same way: when File Sync supplies dynamic targets, an analyze task may be +saved with no explicit document selection. + +## Usage Instructions + +### Configuring documents per task + +1. Open a workspace and go to the **Workflows** tab, then create or edit a workflow. +2. Advance to the **Tasks** step. +3. Select the task you want to configure in the **Task sequence** list (or click **Add Task**). +4. In **Workspace documents**, choose a **Document action** for that task. The picker loads the + documents available in scope. +5. Choose the documents (or configure **Compare** source and targets) for that task. +6. Select another task. The Workspace documents fields reset for an unconfigured task, or + restore that task's saved configuration. + +Each task card in the sequence shows a `Documents:` summary, and the **Review** step lists the +document action for every task plus how many tasks use workspace documents. + +### Example: analyze then compare + +| Task | Document action | Documents | +|---|---|---| +| 1. Analyze intake | Analyze | The three intake PDFs | +| 2. Compare revisions | Compare | Contract v1 as source, v2 and v3 as targets | +| 3. Draft summary | No document action | — (receives task 2's response as context) | + +### Validation messages + +Per-task validation failures name the offending task and select it in the editor, for example: + +``` +Task 2 (Compare revisions): Select at least two document versions for compare. +``` + +## Testing and Validation + +### Test Coverage + +`functional_tests/test_workflow_task_document_actions.py` covers: + +- The document picker load wiring and refresh contract. +- The frontend per-task model, serialization, and payload mirroring. +- `_normalize_workflow_tasks()` storing an action per task. +- Legacy workflow-level configuration inheriting to task 1 only. +- Task-numbered validation errors for invalid per-task actions. +- Group task actions being forced into group scope. +- The runner executing each task with its own action, and the legacy fallback. +- File Sync changed documents reaching task-level analyze actions. + +`functional_tests/test_workflow_task_sequence.py` was extended for the new runner helper and +now asserts the app version with `assert_app_version_at_least` instead of exact equality. + +### Performance Considerations + +Changing a task's document action or switching tasks reloads the picker scope, documents, and +tags. A load token guards against overlapping reloads so only the newest result is applied. + +### Known Limitations + +- The builder has one physical document picker, so it always reflects the selected task. Switching + tasks always syncs the current editor state first, but two tasks cannot be configured side by side. +- Group workflows cannot target personal or public workspace documents from any task; scope is + forced to the owning group. +- The workflow list summary and run-resume path still describe the mirrored primary action rather + than the full per-task set. + +## Coupled Fixes Included + +Making tasks self-contained surfaced four issues that only became reachable once more than one +task could carry a document action. All four are fixed here: + +- A task document action that no longer normalizes (capability disabled by an admin, document + limit lowered) previously aborted the whole run. It is now built inside the retry attempt so it + fails that task through the normal retry and failure strategy. +- Two tasks targeting the same document overwrote each other's run-history status. Document run + item ids are now task-scoped. +- **Resume failed items** narrowed only the workflow-level action, which tasks now override. + It narrows task-level actions as well, and group resumes re-force group scope on each task. +- A saved `max_retries_per_window` of `0` on a task other than the one being edited was rewritten + to `1` on save, because `normalizeText()` coerces numeric `0` to an empty string. + +## Related + +- Fix: `docs/explanation/fixes/WORKFLOW_TASK_DOCUMENT_PICKER_FIX.md` +- Tests: `functional_tests/test_workflow_task_document_actions.py`, `functional_tests/test_workflow_task_sequence.py` diff --git a/docs/explanation/features/index.md b/docs/explanation/features/index.md index d3b6d903f..93ae1e6d8 100644 --- a/docs/explanation/features/index.md +++ b/docs/explanation/features/index.md @@ -69,3 +69,4 @@ category: Version History - [MCP Action Configuration](v0.241.103/MCP_ACTION_CONFIGURATION.md) - [Chat Comparison Modal Summary](v0.241.104/CHAT_COMPARISON_MODAL_SUMMARY.md) - [Databricks Action Configuration](v0.241.104/DATABRICKS_ACTION_CONFIGURATION.md) +- [Workflow Per-Task Workspace Documents](WORKFLOW_PER_TASK_WORKSPACE_DOCUMENTS.md) diff --git a/docs/explanation/fixes/WORKFLOW_TASK_DOCUMENT_PICKER_FIX.md b/docs/explanation/fixes/WORKFLOW_TASK_DOCUMENT_PICKER_FIX.md new file mode 100644 index 000000000..e08b42f17 --- /dev/null +++ b/docs/explanation/fixes/WORKFLOW_TASK_DOCUMENT_PICKER_FIX.md @@ -0,0 +1,160 @@ +# Workflow Task Document Picker Fix + +**Fixed in version: 0.250.225** +**Issue:** [microsoft/simplechat#1282](https://github.com/microsoft/simplechat/issues/1282) + +## Issue + +In the workflow builder (personal **and** group workspaces), on the **Tasks** step of the +Create/Edit Workflow modal, choosing a **Document action** of `Search`, `Analyze`, or +`Compare` revealed the workspace document picker but the picker never initialized: + +- The **Tags** control stayed disabled showing `Loading tags...` indefinitely, even when the + workspace had no tags at all. +- The **Document** dropdown stayed empty, even when the workspace had documents. +- No browser console errors appeared, which made the failure hard to diagnose. + +Clicking **Refresh selected documents** then warned +`Select one or more documents in the picker first.`, which made no sense +because the empty picker offered nothing to select. + +Because the picker is the only way to choose documents, document-backed workflows could not +be created or edited through the UI at all, with no workaround. + +## Root Cause + +`application/single_app/static/js/workspace/workspace_workflows.js` contained a single entry +point for loading picker data, `initializeWorkflowDocumentPicker()`. It is the only caller of +`setEffectiveScopes()` and `ensureDocumentPickerReady()` from `chat/chat-documents.js`, which +in turn run `loadAllDocs()` and `loadTagsForScope()`. + +Two facts combined into the bug: + +1. `initializeWorkflowDocumentPicker()` was invoked from exactly one place — `openWorkflowModal()` — + and it returned early whenever the resolved action type was `none`: + + ```js + const actionType = normalizeText(documentAction.type || workflowDocumentActionTypeSelect?.value) || DOCUMENT_ACTION_NONE; + if (actionType === DOCUMENT_ACTION_NONE) { ...; return; } + ``` + + For a brand-new workflow the modal opens with `documentAction = {}`, so the action type is + always `none` and the function returned immediately. + +2. The `change` handler bound to `#workflow-document-action-type` was `updateDocumentActionFields()`, + which only toggles element visibility. It never triggered a picker load. + +The result: after switching the action to `Search`, nothing ever ran. The tags button simply +kept the static markup state authored in `workspace.html` / `group_workspaces.html` +(`disabled`, spinner visible, `Loading tags...`). Nothing threw, which is why the browser +console stayed clean. + +The "Refresh selected documents" warning was a downstream symptom: +`applySelectedWorkspaceDocumentsToWorkflow()` only pushed the picker's current selection into +the workflow configuration, and that selection was necessarily empty. + +## Technical Details + +### Files Modified + +| File | Change | +|---|---| +| `application/single_app/static/js/workspace/workspace_workflows.js` | Picker load lifecycle, refresh button behavior | +| `application/single_app/static/js/chat/chat-documents.js` | Defensive resolve of the tags dropdown state | +| `application/single_app/templates/workspace.html` | Button label and card copy | +| `application/single_app/templates/group_workspaces.html` | Button label and card copy | +| `application/single_app/config.py` | `VERSION` `0.250.224` → `0.250.225` | + +### Code Changes + +**1. The document action and document target now load the picker.** + +A new `handleWorkflowDocumentActionSelectionChanged()` handler replaces the bare +`updateDocumentActionFields` binding on both `#workflow-document-action-type` and +`#workflow-analysis-target-mode` (the picker card is hidden in `Recent documents` mode and must +load again when switching back to `Selected documents`): + +```js +function handleWorkflowDocumentActionSelectionChanged() { + updateDocumentActionFields(); + ensureWorkflowDocumentPickerLoaded().catch((error) => { + setWorkflowPickerError(error.message || "Unable to load documents for this workflow."); + }); +} +``` + +**2. `ensureWorkflowDocumentPickerLoaded()` drives loads from the live form state.** + +It reads the current form with `readWorkflowDocumentActionFromForm()` so the picker scope, +action type, and selection always match what the user sees, and optionally preserves the live +picker selection across a reload. + +**3. Overlapping loads can no longer strand the loading state.** + +`initializeWorkflowDocumentPicker()` now stamps each run with `workflowDocumentPickerLoadToken` +and only the newest run is allowed to apply results, set an error, or clear the loading flag. + +**4. The tags control always reaches a resolved state.** + +`loadTagsForScope()` in `chat-documents.js` previously returned early without resolving the +button when the hidden `#chat-tags-filter` select was missing. It now calls `hideTagsDropdown()` +first, so the control can never be left in its initial `Loading tags...` markup. + +**5. Refresh refreshes.** + +`applySelectedWorkspaceDocumentsToWorkflow()` now reloads the picker before applying a +selection, and only reports the state it actually observes: + +```js +await ensureWorkflowDocumentPickerLoaded({ preserveSelection: true }); + +const selectedIds = getWorkflowPickerSelectedDocumentIds(); +if (!selectedIds.length) { + showToast( + getWorkflowPickerAvailableDocumentCount() + ? `Document list refreshed. Select one or more ${selectedLabel} documents in the picker for this task.` + : `Document list refreshed. No ${selectedLabel} documents are available in the selected scope.`, + "info", + ); + return; +} +``` + +The button is relabeled from **Refresh selected documents** to **Refresh documents** in both +templates to match what it does. + +### Testing + +- `functional_tests/test_workflow_task_document_actions.py` — new; asserts the change-handler + wiring, the load token, the tags resolve path, and the refresh contract. +- `functional_tests/test_workflow_document_picker_recent_targets.py` — updated for the + refactored serializer/validator symbols. +- A headless `jsdom` harness drove the real modal markup for both the personal and group + workspace templates and confirmed that switching the action to `Search` resolves the tags + control, populates the document dropdown, and that **Refresh documents** no longer warns + about an empty picker. + +## Validation + +### Before + +| Step | Result | +|---|---| +| Set Document action to `Search` | Tags stuck on `Loading tags...`, document list empty | +| Click **Refresh selected documents** | `Select one or more workspace documents in the picker first.` | +| Browser console | No errors | + +### After + +| Step | Result | +|---|---| +| Set Document action to `Search` | Documents load; tags resolve to the available tags or `No tags available for this scope` | +| Set Document target back to `Selected documents` | Picker reloads | +| Click **Refresh documents** | Document list reloads and the current selection is preserved | +| Empty workspace | `Document list refreshed. No workspace documents are available in the selected scope.` | + +## Related + +- Feature: `docs/explanation/features/WORKFLOW_PER_TASK_WORKSPACE_DOCUMENTS.md` +- Tests: `functional_tests/test_workflow_task_document_actions.py` +- Issue: [microsoft/simplechat#1282](https://github.com/microsoft/simplechat/issues/1282) diff --git a/docs/explanation/fixes/index.md b/docs/explanation/fixes/index.md index fffb485ff..105b72a65 100644 --- a/docs/explanation/fixes/index.md +++ b/docs/explanation/fixes/index.md @@ -28,4 +28,5 @@ category: Version History - [External Browser Asset CSP Allowlist Fix](EXTERNAL_BROWSER_ASSET_CSP_ALLOWLIST_FIX.md) - [Semantic Search Quota Warning Fix](SEMANTIC_SEARCH_QUOTA_WARNING_FIX.md) - [Route Authentication Audit Findings Fix](ROUTE_AUTHENTICATION_AUDIT_FINDINGS_FIX.md) -- [Cosmos Container Throughput Deployer Fix](COSMOS_CONTAINER_THROUGHPUT_DEPLOYER_FIX.md) \ No newline at end of file +- [Cosmos Container Throughput Deployer Fix](COSMOS_CONTAINER_THROUGHPUT_DEPLOYER_FIX.md) +- [Workflow Task Document Picker Fix](WORKFLOW_TASK_DOCUMENT_PICKER_FIX.md) diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index 51c30f02b..5e135605a 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -2,6 +2,51 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Version](/explanation/features/) and [Fixes by Version](/explanation/fixes/). +### **(v0.250.225)** + +#### New Features + +* **Workspace Documents Are Now Configured Per Workflow Task** + * Each task in a workflow now owns its own **Workspace documents** setup — document action, document target, selected documents, Compare source and targets, per-document analysis, and windowing — instead of sharing one configuration across the whole workflow. + * Adding a task resets the document fields, and returning to a previously configured task restores that task's setup, so tasks are self-contained. + * Every task now executes with its own document action at run time. Previously the single workflow-level action only ever applied to task 1, and every later task ran with no document context. + * Existing workflows keep working: their saved document action is inherited by task 1 only, matching how they actually ran. Group workflows still force every task into the owning group workspace. + * Task cards and the Review step now summarize which documents each task uses. + * (Ref: #1282, `workspace_workflows.js`, `functions_personal_workflows.py`, `functions_group_workflows.py`, `functions_workflow_runner.py`, per-task document actions) + +#### Bug Fixes + +* **Workflow Document Picker No Longer Hangs on "Loading tags..."** + * Choosing a Document action of Search, Analyze, or Compare in the workflow builder revealed the document picker but never loaded it. Tags stayed disabled showing `Loading tags...` forever, the document list stayed empty, and no console error appeared. + * The picker was only ever loaded when the modal opened, and that path returned early whenever the action was `No document action` — which is always true for a new workflow. The Document action dropdown's change handler only toggled visibility and never triggered a load. + * Changing the Document action or Document Target now loads the picker, and the tags control always resolves to the available tags or `No tags available for this scope`. + * (Ref: #1282, `workspace_workflows.js`, `chat-documents.js`, workflow document picker) + +* **Workflow Run History No Longer Masks a Failed Document** + * When two tasks in the same run process the same document, the later task's status used to overwrite the earlier one's, so a document that failed in one task could be shown as succeeded. + * Document run items are now recorded per task, and each item records which task produced it. + * (Ref: #1282, `functions_workflow_runner.py`, workflow run history) + +* **Resume Failed Items Respects Per-Task Documents** + * Resuming failed documents narrowed only the workflow-level document action. Now that tasks own their own documents, it also narrows each task's analyze action to the documents that failed in that task, and group resumes keep every task inside the owning group workspace. + * (Ref: #1282, `route_backend_workflows.py`, resume failed items) + +* **A Workflow Task Configured for Zero Retries Per Window Stays at Zero** + * Saving a multi-task workflow rewrote a stored `Retries Per Window` of `0` to `1` on any task other than the one being edited. + * (Ref: #1282, `workspace_workflows.js`, retries per window) + +* **An Invalid Task Document Action No Longer Aborts the Whole Run** + * If a workflow's document action stopped validating between runs — for example an administrator disabled Analyze or Compare, or lowered the workflow document limit — the run failed outright with no task-level error recorded. + * The failure is now contained to that task and follows the workflow's retry and failure-handling settings. + * (Ref: #1282, `functions_workflow_runner.py`, workflow task error handling) + +#### User Interface Enhancements + +* **"Refresh documents" Now Actually Refreshes** + * The **Refresh selected documents** button previously warned `Select one or more workspace documents in the picker first.` even though the picker was empty and nothing could be selected. + * It is now labeled **Refresh documents**, reloads the document list while preserving the current selection, and reports what it found — including a clear message when the selected scope has no documents. + * (Ref: #1282, `workspace.html`, `group_workspaces.html`, `workspace_workflows.js`) + ### **(v0.250.224)** #### New Features diff --git a/functional_tests/test_workflow_document_picker_recent_targets.py b/functional_tests/test_workflow_document_picker_recent_targets.py index 210aaa1ec..69a5d40b6 100644 --- a/functional_tests/test_workflow_document_picker_recent_targets.py +++ b/functional_tests/test_workflow_document_picker_recent_targets.py @@ -2,8 +2,9 @@ # test_workflow_document_picker_recent_targets.py """ Functional test for workflow document picker and recent workflow document targets. -Version: 0.241.188 +Version: 0.250.225 Implemented in: 0.241.188 +Enhanced in: 0.250.225 This test ensures workflow Search, Analyze, and Compare configuration uses the shared chat document picker instead of visible raw ID fields, and recent @@ -167,12 +168,16 @@ def test_workflow_picker_javascript_contracts() -> None: assert 'window.addEventListener("chat:document-selection-changed"' in workflow_js assert 'window.addEventListener("chat:scope-changed"' in workflow_js assert 'const DOCUMENT_ACTION_SEARCH = "search";' in workflow_js - assert 'target_mode: documentActionType !== DOCUMENT_ACTION_NONE ? analysisTargetMode : DOCUMENT_ANALYSIS_TARGET_SELECTED' in workflow_js - assert 'recent_window_minutes: Number(rawRecentMinutes)' in workflow_js - assert 'documentActionType === DOCUMENT_ACTION_SEARCH && analysisTargetMode === DOCUMENT_ANALYSIS_TARGET_SELECTED' in workflow_js - assert 'documentActionType === DOCUMENT_ACTION_COMPARISON && analysisTargetMode === DOCUMENT_ANALYSIS_TARGET_SELECTED' in workflow_js - assert 'documentActionType !== DOCUMENT_ACTION_NONE && analysisTargetMode === DOCUMENT_ANALYSIS_TARGET_RECENT' in workflow_js - assert 'const targetDocumentIds = analysisTargetMode === DOCUMENT_ANALYSIS_TARGET_RECENT' in workflow_js + # Recent-target support now lives in the shared per-task serializer and validator. + assert 'function serializeWorkflowDocumentAction(' in workflow_js + assert 'target_mode: targetMode,' in workflow_js + assert 'recent_window_minutes: Number(rawRecentMinutes),' in workflow_js + assert 'const isRecentMode = targetMode === DOCUMENT_ANALYSIS_TARGET_RECENT;' in workflow_js + assert 'actionType === DOCUMENT_ACTION_SEARCH && isSelectedMode' in workflow_js + assert 'actionType === DOCUMENT_ACTION_COMPARISON && isSelectedMode' in workflow_js + assert 'const isSelectedMode = actionPayload.target_mode === DOCUMENT_ANALYSIS_TARGET_SELECTED;' in workflow_js + assert 'Recent document window must be between 1 and 1440 minutes.' in workflow_js + assert 'const pickerDocumentIds = isRecentMode ? [] : getWorkflowPickerSelectedDocumentIds();' in workflow_js assert 'setEffectiveScopes(pickerScopes, {' in workflow_js assert 'force: workflowWorkspaceConfig.scope === "group"' in workflow_js assert 'if (docDropdownButton)' in chat_documents_js diff --git a/functional_tests/test_workflow_per_document_analysis_mode.py b/functional_tests/test_workflow_per_document_analysis_mode.py index 0c748c5a4..2d63c8690 100644 --- a/functional_tests/test_workflow_per_document_analysis_mode.py +++ b/functional_tests/test_workflow_per_document_analysis_mode.py @@ -190,8 +190,9 @@ def test_workflow_per_document_ui_and_new_tab_contracts(): assert 'id="workflow-analysis-per-document"' in group_template assert 'Run each document separately' in group_template assert 'const DOCUMENT_ANALYSIS_MODE_PER_DOCUMENT = "per_document";' in workflow_js - assert 'analysis_mode: documentActionType === DOCUMENT_ACTION_ANALYZE ? analysisMode : DOCUMENT_ANALYSIS_MODE_COMBINED' in workflow_js - assert 'workflowAnalysisPerDocumentToggle.checked = documentAction.analysis_mode === DOCUMENT_ANALYSIS_MODE_PER_DOCUMENT' in workflow_js + assert 'analysis_mode: actionType === DOCUMENT_ACTION_ANALYZE' in workflow_js + assert 'normalizeWorkflowAnalysisMode(source.analysis_mode)' in workflow_js + assert 'workflowAnalysisPerDocumentToggle.checked = action.analysis_mode === DOCUMENT_ANALYSIS_MODE_PER_DOCUMENT' in workflow_js assert 'target="_blank" rel="noopener"' in workflow_js assert 'element.target = conversationUrl ? "_blank" : "";' in workflow_js assert "const targetWindow = window.open('about:blank', '_blank');" in notifications_js diff --git a/functional_tests/test_workflow_task_document_actions.py b/functional_tests/test_workflow_task_document_actions.py index 82dae07a4..1bcf0b2a4 100644 --- a/functional_tests/test_workflow_task_document_actions.py +++ b/functional_tests/test_workflow_task_document_actions.py @@ -136,7 +136,9 @@ def load_runner_helpers(): namespace = { "DOCUMENT_ACTION_TYPE_NONE": "none", "DOCUMENT_ACTION_TYPE_ANALYZE": "analyze", + "DOCUMENT_ACTION_TYPE_COMPARISON": "comparison", "WORKFLOW_TASK_CONTEXT_MAX_CHARS": 12000, + "re": __import__("re"), "build_analyze_config": lambda action: {"enabled": (action or {}).get("type") == "analyze"}, "_get_document_action_config": lambda source: dict( (source or {}).get("document_action") or {"type": "none"} @@ -148,6 +150,8 @@ def load_runner_helpers(): RUNNER_FILE, { "_truncate_workflow_task_context", + "_get_workflow_active_task", + "_document_run_item_id", "_resolve_workflow_task_document_action", "_build_workflow_task_execution_workflow", "_apply_file_sync_changed_documents_to_action", @@ -157,6 +161,25 @@ def load_runner_helpers(): ) +def load_resume_helpers(): + return load_functions( + APP_ROOT / "route_backend_workflows.py", + { + "_narrow_analyze_action_to_documents", + "_force_group_document_action_scope", + "_build_resume_failed_workflow", + }, + { + "DOCUMENT_ACTION_TYPE_ANALYZE": "analyze", + "DOCUMENT_ACTION_TYPE_NONE": "none", + "FILE_SYNC_SCOPE_GROUP": "group", + "FILE_SYNC_SCOPE_PUBLIC": "public", + "build_analyze_config": lambda action: {"enabled": (action or {}).get("type") == "analyze"}, + "_normalize_identifier": lambda value: str(value or "").strip(), + }, + ) + + def test_document_picker_loads_on_action_change() -> None: """The picker must load whenever the document action or target mode changes.""" print("Testing workflow document picker load wiring...") @@ -469,6 +492,121 @@ def test_file_sync_changed_documents_reach_task_actions() -> None: print("PASS: File Sync changed document propagation") +def test_document_run_items_are_task_scoped() -> None: + """Two tasks targeting the same document must not overwrite each other's status.""" + print("Testing task-scoped document run item ids...") + helpers = load_runner_helpers() + document_run_item_id = helpers["_document_run_item_id"] + + first = document_run_item_id("run-1", "doc-a", task_id="task-1") + second = document_run_item_id("run-1", "doc-a", task_id="task-2") + legacy = document_run_item_id("run-1", "doc-a") + + assert first != second, "Per-task document run items must have distinct ids." + assert legacy == "run-1:document:doc-a", legacy + assert "task-1" in first and "task-2" in second + + runner_source = read_text(RUNNER_FILE) + assert "'id': _document_run_item_id(run_id, document_id, task_id=task_id)," in runner_source + assert "'task_id': task_id or None," in runner_source + print("PASS: task-scoped document run item ids") + + +def test_task_document_action_failures_stay_inside_the_retry_loop() -> None: + """An invalid task document action must fail that task, not abort the whole run.""" + print("Testing task document action failure containment...") + runner_source = read_text(RUNNER_FILE) + + sequence_start = runner_source.index("def _execute_workflow_task_sequence(") + sequence_body = runner_source[sequence_start:sequence_start + 6000] + attempt_loop_index = sequence_body.index("for attempt_index in range(retry_count + 1):") + build_index = sequence_body.index("prepared_workflow = _build_workflow_task_execution_workflow(") + try_index = sequence_body.index("try:", attempt_loop_index) + + assert build_index > attempt_loop_index, ( + "The per-task workflow must be built inside the retry loop so document action " + "normalization errors are retried and recorded per task." + ) + assert build_index > try_index, "The per-task workflow build must be inside the attempt try block." + print("PASS: task document action failure containment") + + +def test_resume_failed_narrows_task_document_actions() -> None: + """Resuming failed documents must narrow task-level analyze actions, not just the workflow one.""" + print("Testing resume-failed per-task narrowing...") + helpers = load_resume_helpers() + build_resume = helpers["_build_resume_failed_workflow"] + force_group_scope = helpers["_force_group_document_action_scope"] + + workflow = { + "task_prompt": "Analyze everything.", + "document_action": build_document_action("analyze", document_ids=["doc-a", "doc-b", "doc-c"]), + "tasks": [ + {"id": "task-1", "document_action": build_document_action("analyze", document_ids=["doc-a", "doc-b"])}, + {"id": "task-2", "document_action": build_document_action("analyze", document_ids=["doc-c"])}, + {"id": "task-3", "document_action": build_document_action()}, + ], + } + failed_items = [ + {"document_id": "doc-b", "task_id": "task-1", "status": "failed"}, + {"document_id": "doc-c", "task_id": "task-2", "status": "failed"}, + ] + + resumed = build_resume(workflow, failed_items) + assert resumed["document_action"]["document_ids"] == ["doc-b", "doc-c"] + assert resumed["tasks"][0]["document_action"]["document_ids"] == ["doc-b"] + assert resumed["tasks"][1]["document_action"]["document_ids"] == ["doc-c"] + assert resumed["tasks"][2]["document_action"]["type"] == "none" + assert resumed["file_sync"]["enabled"] is False + + # A task with an analyze action but no failed documents must not re-run its whole set. + partial = build_resume(workflow, [{"document_id": "doc-b", "task_id": "task-1", "status": "failed"}]) + assert partial["tasks"][0]["document_action"]["document_ids"] == ["doc-b"] + assert partial["tasks"][1]["document_action"]["type"] == "none" + + # Legacy run items without task attribution fall back to the full failed set. + legacy = build_resume(workflow, [{"document_id": "doc-b", "status": "failed"}]) + assert legacy["tasks"][0]["document_action"]["document_ids"] == ["doc-b"] + assert legacy["tasks"][1]["document_action"]["document_ids"] == ["doc-b"] + + # Group resumes stay inside the owning group workspace. + scoped = force_group_scope(build_document_action("analyze", doc_scope="all", active_public_workspace_id=["p1"]), "group-9") + assert scoped["doc_scope"] == "group" + assert scoped["active_group_ids"] == ["group-9"] + assert scoped["active_public_workspace_id"] == [] + assert force_group_scope(build_document_action(), "group-9")["type"] == "none" + print("PASS: resume-failed per-task narrowing") + + +def test_retries_per_window_zero_survives_serialization() -> None: + """A saved max_retries_per_window of 0 must not be silently rewritten to 1.""" + print("Testing zero retries-per-window preservation...") + workflow_js = read_text(WORKFLOW_JS_FILE) + + assert "function normalizeWorkflowNumericField(" in workflow_js + assert 'const rawRetries = normalizeWorkflowNumericField(source.max_retries_per_window, "1");' in workflow_js + assert 'const rawRetries = normalizeText(source.max_retries_per_window) || "1";' not in workflow_js + assert 'max_retries_per_window: normalizeWorkflowNumericField(workflowAnalysisRetriesInput?.value, "1"),' in workflow_js + print("PASS: zero retries-per-window preservation") + + +def test_picker_load_token_guards_none_actions() -> None: + """Switching to a task with no document action must invalidate an in-flight load.""" + print("Testing picker load token invalidation...") + workflow_js = read_text(WORKFLOW_JS_FILE) + + picker_start = workflow_js.index("async function initializeWorkflowDocumentPicker(") + picker_body = workflow_js[picker_start:picker_start + 1800] + token_index = picker_body.index("workflowDocumentPickerLoadToken = requestToken;") + none_return_index = picker_body.index("if (actionType === DOCUMENT_ACTION_NONE) {") + + assert token_index < none_return_index, ( + "The load token must be bumped before the no-document-action early return so a " + "pending load for a previous task cannot apply its scopes or selection." + ) + print("PASS: picker load token invalidation") + + def test_version_contract() -> None: """The fix ships at or after its implementation version.""" print("Testing version contract...") @@ -488,6 +626,11 @@ def run_tests() -> bool: test_runner_executes_each_task_with_its_own_action, test_runner_legacy_tasks_keep_first_task_only_behavior, test_file_sync_changed_documents_reach_task_actions, + test_document_run_items_are_task_scoped, + test_task_document_action_failures_stay_inside_the_retry_loop, + test_resume_failed_narrows_task_document_actions, + test_retries_per_window_zero_survives_serialization, + test_picker_load_token_guards_none_actions, test_version_contract, ] results = []