From 88ea6a75a111019731d4d7d4f0e8158a80543528 Mon Sep 17 00:00:00 2001 From: Chad Palmer Date: Thu, 20 Aug 2026 01:03:54 +0000 Subject: [PATCH 01/11] fix-logout-route - fixed logout 404 error in dev environment. --- application/single_app/config.py | 2 +- .../route_frontend_authentication.py | 4 + ...VELOPMENT_LOGOUT_EASY_AUTH_REDIRECT_FIX.md | 24 +++ .../test_app_service_easy_auth_logout.py | 157 ++++++++++++++++-- 4 files changed, 175 insertions(+), 12 deletions(-) create mode 100644 docs/explanation/fixes/DEVELOPMENT_LOGOUT_EASY_AUTH_REDIRECT_FIX.md 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/route_frontend_authentication.py b/application/single_app/route_frontend_authentication.py index c07b51519..51cba1e72 100644 --- a/application/single_app/route_frontend_authentication.py +++ b/application/single_app/route_frontend_authentication.py @@ -6,6 +6,7 @@ import requests from config import * +from config import IS_DEVELOPMENT from functions_activity_logging import log_user_login, record_user_login_session_activity from functions_terms_of_use import ( apply_pending_pre_auth_terms_of_use, @@ -43,6 +44,9 @@ def build_front_door_urls(front_door_url): def _use_app_service_easy_auth_logout(): """Return True when the current request is running behind App Service Easy Auth.""" + if IS_DEVELOPMENT: + return False + if not os.getenv('WEBSITE_HOSTNAME'): return False diff --git a/docs/explanation/fixes/DEVELOPMENT_LOGOUT_EASY_AUTH_REDIRECT_FIX.md b/docs/explanation/fixes/DEVELOPMENT_LOGOUT_EASY_AUTH_REDIRECT_FIX.md new file mode 100644 index 000000000..a92933ee3 --- /dev/null +++ b/docs/explanation/fixes/DEVELOPMENT_LOGOUT_EASY_AUTH_REDIRECT_FIX.md @@ -0,0 +1,24 @@ +# Development Logout Easy Auth Redirect Fix + +Fixed/Implemented in version: **0.250.225** + +## Issue Description + +In the development environment, user-initiated logout and idle-timeout logout could redirect to `/.auth/logout?post_logout_redirect_uri=%2Flogin`. That platform Easy Auth URL returned a 404 when the development deployment was not actually serving App Service Easy Auth logout endpoints. + +## Root Cause Analysis + +The logout route detected Azure hosting variables and Easy Auth-related signals, then routed local logout through `/.auth/logout`. Development deployments can still expose those hosting signals even when the Easy Auth endpoint is unavailable for the current custom-domain path. + +## Technical Details + +- Modified `application/single_app/route_frontend_authentication.py` so Easy Auth logout routing is skipped when `IS_DEVELOPMENT` is enabled. +- Preserved the existing Easy Auth logout path for non-development Azure App Service deployments with Easy Auth signals. +- Updated `application/single_app/config.py` to version `0.250.225`. +- Added regression coverage in `functional_tests/test_app_service_easy_auth_logout.py` for the development-mode fallback. + +## Validation + +- Ran `functional_tests/test_app_service_easy_auth_logout.py`. +- Confirmed production-style Easy Auth local and full logout still redirect through `/.auth/logout`. +- Confirmed development-mode local logout avoids `/.auth/logout` and redirects to the local app index after clearing the Flask session. \ No newline at end of file diff --git a/functional_tests/test_app_service_easy_auth_logout.py b/functional_tests/test_app_service_easy_auth_logout.py index 569202a87..c3acc0452 100644 --- a/functional_tests/test_app_service_easy_auth_logout.py +++ b/functional_tests/test_app_service_easy_auth_logout.py @@ -1,20 +1,23 @@ # test_app_service_easy_auth_logout.py """ Functional test for Azure App Service Easy Auth logout recovery. -Version: 0.241.095 +Version: 0.250.225 Implemented in: 0.241.095 This test ensures Azure-hosted logout routes clear the upstream App Service authentication session by redirecting through /.auth/logout before re-entering -the Flask login flow. +the Flask login flow, while development-mode deployments avoid a missing +/.auth/logout platform endpoint. """ from pathlib import Path +import importlib import os import sys -from unittest.mock import patch +import types +from unittest.mock import patch, Mock -from flask import Flask, session +from flask import Blueprint, Flask, session ROOT = Path(__file__).resolve().parents[1] @@ -24,7 +27,103 @@ sys.path.insert(0, str(APP_DIR)) -import route_frontend_authentication as route_module # noqa: E402 +class FakeConfigCosmosContainer: + """Minimal Cosmos container stand-in for config.py import-time setup.""" + + def read(self): + return {} + + +class FakeConfigCosmosDatabase: + """Minimal Cosmos database stand-in for importing config.py without live I/O.""" + + def __init__(self): + self.containers = {} + + def create_container_if_not_exists(self, id, **kwargs): + if id not in self.containers: + self.containers[id] = FakeConfigCosmosContainer() + return self.containers[id] + + def get_container_client(self, id): + return self.containers.setdefault(id, FakeConfigCosmosContainer()) + + +class FakeConfigCosmosClient: + """Minimal Cosmos client stand-in for config.py import-time container setup.""" + + def __init__(self, *args, **kwargs): + self.database = FakeConfigCosmosDatabase() + + def create_database_if_not_exists(self, *args, **kwargs): + return self.database + + +def import_module_without_live_cosmos(module_name): + """Import app modules without letting config.py connect to live Cosmos.""" + if module_name in sys.modules: + return sys.modules[module_name] + + import azure.cosmos as azure_cosmos + + original_cosmos_client = azure_cosmos.CosmosClient + azure_cosmos.CosmosClient = FakeConfigCosmosClient + stub_modules = _install_route_dependency_stubs() + try: + return importlib.import_module(module_name) + finally: + azure_cosmos.CosmosClient = original_cosmos_client + for stub_name in stub_modules: + sys.modules.pop(stub_name, None) + + +def _install_route_dependency_stubs(): + """Install lightweight stubs for dependencies unrelated to logout routing.""" + stub_modules = {} + + functions_activity_logging = types.ModuleType("functions_activity_logging") + functions_activity_logging.log_user_login = Mock() + functions_activity_logging.record_user_login_session_activity = Mock() + stub_modules["functions_activity_logging"] = functions_activity_logging + + functions_terms_of_use = types.ModuleType("functions_terms_of_use") + functions_terms_of_use.apply_pending_pre_auth_terms_of_use = Mock() + functions_terms_of_use.get_terms_of_use_config = Mock(return_value={"enabled": False}) + functions_terms_of_use.has_terms_of_use_acceptance = Mock(return_value=True) + stub_modules["functions_terms_of_use"] = functions_terms_of_use + + functions_authentication = types.ModuleType("functions_authentication") + functions_authentication._build_msal_app = Mock() + functions_authentication._load_cache = Mock(return_value=None) + functions_authentication._save_cache = Mock() + functions_authentication.clear_requested_oauth_scopes = Mock() + functions_authentication.create_ci_bearer_session = Mock(return_value=("", 204)) + functions_authentication.get_graph_authority = Mock(return_value="https://graph.microsoft.com") + functions_authentication.get_graph_endpoint = Mock(side_effect=lambda path: f"https://graph.microsoft.com/v1.0{path}") + functions_authentication.get_requested_oauth_scopes = Mock(return_value=[]) + stub_modules["functions_authentication"] = functions_authentication + + functions_debug = types.ModuleType("functions_debug") + functions_debug.debug_print = Mock() + stub_modules["functions_debug"] = functions_debug + + functions_settings = types.ModuleType("functions_settings") + functions_settings.get_settings = Mock(return_value={}) + functions_settings.sanitize_settings_for_user = Mock(side_effect=lambda settings: settings) + stub_modules["functions_settings"] = functions_settings + + swagger_wrapper = types.ModuleType("swagger_wrapper") + swagger_wrapper.swagger_route = Mock(side_effect=lambda *args, **kwargs: (lambda function: function)) + swagger_wrapper.get_auth_security = Mock(return_value=[]) + stub_modules["swagger_wrapper"] = swagger_wrapper + + for stub_name, stub_module in stub_modules.items(): + sys.modules[stub_name] = stub_module + + return stub_modules + + +route_module = import_module_without_live_cosmos("route_frontend_authentication") EXPECTED_EASY_AUTH_LOGOUT = "/.auth/logout?post_logout_redirect_uri=%2Flogin" @@ -34,11 +133,14 @@ def _build_test_app(): app = Flask(__name__) app.secret_key = "test-secret" - @app.route("/") def index(): return "ok" - route_module.register_route_frontend_authentication(app) + app.add_url_rule("/", endpoint="public_app.index", view_func=index) + + auth_blueprint = Blueprint("frontend_authentication", __name__) + route_module.register_route_frontend_authentication(auth_blueprint) + app.register_blueprint(auth_blueprint) return app @@ -55,7 +157,7 @@ def test_local_logout_uses_app_service_easy_auth_logout(): "WEBSITE_AUTH_AAD_ALLOWED_TENANTS": "tenant-id", }, clear=False, - ): + ), patch.object(route_module, "IS_DEVELOPMENT", False): with app.test_request_context( "/logout/local", base_url="https://example.azurewebsites.net", @@ -63,7 +165,7 @@ def test_local_logout_uses_app_service_easy_auth_logout(): ): session["user"] = {"name": "Test User"} - response = app.view_functions["local_logout"]() + response = app.view_functions["frontend_authentication.local_logout"]() assert response.status_code == 302, f"Expected redirect response, got {response.status_code}" assert response.headers.get("Location") == EXPECTED_EASY_AUTH_LOGOUT, ( @@ -87,7 +189,7 @@ def test_full_logout_uses_app_service_easy_auth_logout(): "WEBSITE_AUTH_AAD_ALLOWED_TENANTS": "tenant-id", }, clear=False, - ): + ), patch.object(route_module, "IS_DEVELOPMENT", False): with app.test_request_context( "/logout", base_url="https://example.azurewebsites.net", @@ -98,7 +200,7 @@ def test_full_logout_uses_app_service_easy_auth_logout(): "preferred_username": "user@example.com", } - response = app.view_functions["logout"]() + response = app.view_functions["frontend_authentication.logout"]() assert response.status_code == 302, f"Expected redirect response, got {response.status_code}" assert response.headers.get("Location") == EXPECTED_EASY_AUTH_LOGOUT, ( @@ -109,10 +211,43 @@ def test_full_logout_uses_app_service_easy_auth_logout(): print("App Service Easy Auth full logout redirects through /.auth/logout") +def test_development_mode_does_not_use_app_service_easy_auth_logout(): + """Verify development mode skips Easy Auth logout even when Azure hosting variables exist.""" + print("Testing development-mode logout avoids App Service Easy Auth redirect...") + + app = _build_test_app() + + with patch.dict( + os.environ, + { + "WEBSITE_HOSTNAME": "oigchat-dev.dhs-oig.gov", + "WEBSITE_AUTH_AAD_ALLOWED_TENANTS": "tenant-id", + }, + clear=False, + ), patch.object(route_module, "IS_DEVELOPMENT", True), patch.object(route_module, "get_settings", Mock(return_value={})): + with app.test_request_context( + "/logout/local", + base_url="https://oigchat-dev.dhs-oig.gov", + headers={"X-MS-CLIENT-PRINCIPAL-ID": "user-oid"}, + ): + session["user"] = {"name": "Test User"} + + response = app.view_functions["frontend_authentication.local_logout"]() + + assert response.status_code == 302, f"Expected redirect response, got {response.status_code}" + assert response.headers.get("Location") == "/", ( + f"Unexpected development local logout redirect: {response.headers.get('Location')}" + ) + assert "user" not in session, f"Expected Flask session to be cleared, got {dict(session)}" + + print("Development-mode local logout avoids /.auth/logout") + + if __name__ == "__main__": tests = [ test_local_logout_uses_app_service_easy_auth_logout, test_full_logout_uses_app_service_easy_auth_logout, + test_development_mode_does_not_use_app_service_easy_auth_logout, ] results = [] From 173fe9b878f69afcf7dc3d498ab3eb46eb4f56fd Mon Sep 17 00:00:00 2001 From: Chad Palmer Date: Tue, 25 Aug 2026 21:02:35 +0000 Subject: [PATCH 02/11] fix-logout-route - Bumped version and updated files/tests to hopefully prevent confusion. --- application/single_app/config.py | 2 +- .../fixes/EASY_AUTH_LOGOUT_DETECTION_FIX.md | 4 ++-- docs/explanation/release_notes.md | 22 +++++++++---------- .../test_app_service_easy_auth_logout.py | 4 ++-- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/application/single_app/config.py b/application/single_app/config.py index d57f7c871..865c4aa98 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.260.028" +VERSION = "0.260.029" IS_DEVELOPMENT = is_development_env_enabled() # Opt-out for deployments where App Service Easy Auth is active but the platform diff --git a/docs/explanation/fixes/EASY_AUTH_LOGOUT_DETECTION_FIX.md b/docs/explanation/fixes/EASY_AUTH_LOGOUT_DETECTION_FIX.md index c1383fc0a..e7ba6f275 100644 --- a/docs/explanation/fixes/EASY_AUTH_LOGOUT_DETECTION_FIX.md +++ b/docs/explanation/fixes/EASY_AUTH_LOGOUT_DETECTION_FIX.md @@ -1,6 +1,6 @@ # Easy Auth Logout Detection Fix -Fixed/Implemented in version: **0.260.019** +Fixed/Implemented in version: **0.260.029** ## Issue Description @@ -49,7 +49,7 @@ so it needs an explicit opt-out. "Logout Behavior Across Environments" section to `docs/explanation/running_simplechat_locally.md` covering the behavior per environment and the troubleshooting steps for a logout 404. -- Updated `application/single_app/config.py` to version `0.260.019`. +- Updated `application/single_app/config.py` to version `0.260.029`. - Reworked regression coverage in `functional_tests/test_app_service_easy_auth_logout.py`. ### Behavior by environment diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index 0fe2c88bd..51df0f359 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -2,6 +2,17 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Version](https://github.com/microsoft/simplechat/tree/main/docs/explanation/features) and [Fixes by Version](https://github.com/microsoft/simplechat/tree/main/docs/explanation/fixes). +### **(v0.260.029)** + +#### Bug Fixes + +* **Logout No Longer Redirects To A Missing Easy Auth Endpoint** + * Logout could redirect to `/.auth/logout?post_logout_redirect_uri=%2Flogin` and return a 404 on Azure App Service deployments that were not actually serving App Service Easy Auth. This affected production deployments as well as development ones. + * The root cause was Easy Auth detection treating the manually configured `WEBSITE_AUTH_AAD_ALLOWED_TENANTS` application setting as proof that Easy Auth was intercepting requests. SimpleChat's own advanced environment variable guidance instructs operators to set that value by hand, so it was never a reliable signal. + * Detection now relies only on the `X-MS-CLIENT-PRINCIPAL` request headers that App Service Easy Auth injects on requests it actually intercepts, so deployments genuinely behind Easy Auth still clear the upstream platform session, and everyone else gets a clean local logout. + * Idle-timeout logout uses the same local logout path, so automatic session expiration follows the corrected behavior as well. + * (Ref: `route_frontend_authentication.py`, `_use_app_service_easy_auth_logout`, `test_app_service_easy_auth_logout.py`, [Easy Auth Logout Detection Fix](fixes/EASY_AUTH_LOGOUT_DETECTION_FIX.md)) + ### **(v0.260.025)** #### Bug Fixes @@ -118,17 +129,6 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Ver * The Cosmos DB tab checked the wrong thing for the debug setting, so the backfill controls, shadow validation metrics and reset option stayed hidden even after an admin turned the setting on. * (Ref: `admin/_panes/cosmos.html`, `enable_dai_debug`) -### **(v0.260.019)** - -#### Bug Fixes - -* **Logout No Longer Redirects To A Missing Easy Auth Endpoint** - * Logout could redirect to `/.auth/logout?post_logout_redirect_uri=%2Flogin` and return a 404 on Azure App Service deployments that were not actually serving App Service Easy Auth. This affected production deployments as well as development ones. - * The root cause was Easy Auth detection treating the manually configured `WEBSITE_AUTH_AAD_ALLOWED_TENANTS` application setting as proof that Easy Auth was intercepting requests. SimpleChat's own advanced environment variable guidance instructs operators to set that value by hand, so it was never a reliable signal. - * Detection now relies only on the `X-MS-CLIENT-PRINCIPAL` request headers that App Service Easy Auth injects on requests it actually intercepts, so deployments genuinely behind Easy Auth still clear the upstream platform session, and everyone else gets a clean local logout. - * Idle-timeout logout uses the same local logout path, so automatic session expiration follows the corrected behavior as well. - * (Ref: `route_frontend_authentication.py`, `_use_app_service_easy_auth_logout`, `test_app_service_easy_auth_logout.py`, [Easy Auth Logout Detection Fix](fixes/EASY_AUTH_LOGOUT_DETECTION_FIX.md)) - #### New Features * **Opt-Out For App Service Easy Auth Logout** diff --git a/functional_tests/test_app_service_easy_auth_logout.py b/functional_tests/test_app_service_easy_auth_logout.py index 89e1701a1..9188381a9 100644 --- a/functional_tests/test_app_service_easy_auth_logout.py +++ b/functional_tests/test_app_service_easy_auth_logout.py @@ -1,8 +1,8 @@ # test_app_service_easy_auth_logout.py """ Functional test for Azure App Service Easy Auth logout detection. -Version: 0.260.019 -Implemented in: 0.260.019 +Version: 0.260.029 +Implemented in: 0.260.029 This test ensures logout routes through /.auth/logout only when App Service Easy Auth is actually serving the request, so deployments that are not behind Easy Auth no longer hit a From 79f665cb26cb96a09c82ad159aa96a59f44c0451 Mon Sep 17 00:00:00 2001 From: Chad Palmer Date: Wed, 26 Aug 2026 19:24:28 +0000 Subject: [PATCH 03/11] fix-many-doc-upload-issue - Fixed issue where many small documents upload overloaded cosmos gate. --- application/single_app/config.py | 2 +- ...ions_data_management_search_write_fence.py | 6 ++-- ...SEARCH_WRITE_GATE_UPLOAD_CONTENTION_FIX.md | 25 +++++++++++++++ ...test_data_management_search_write_fence.py | 31 ++++++++++++++++++- 4 files changed, 60 insertions(+), 4 deletions(-) create mode 100644 docs/explanation/fixes/SEARCH_WRITE_GATE_UPLOAD_CONTENTION_FIX.md diff --git a/application/single_app/config.py b/application/single_app/config.py index 865c4aa98..cdcf5e9be 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.260.029" +VERSION = "0.260.030" IS_DEVELOPMENT = is_development_env_enabled() # Opt-out for deployments where App Service Easy Auth is active but the platform diff --git a/application/single_app/functions_data_management_search_write_fence.py b/application/single_app/functions_data_management_search_write_fence.py index 3e83b24e2..1cb99b636 100644 --- a/application/single_app/functions_data_management_search_write_fence.py +++ b/application/single_app/functions_data_management_search_write_fence.py @@ -186,7 +186,8 @@ def _open_expired_gate(container, gate, now=None): def acquire_data_management_search_write_slot(container): """Reserve one bounded target Search write before issuing the data-plane request.""" - for _attempt in range(12): + deadline = time.monotonic() + DATA_MANAGEMENT_SEARCH_WRITE_REQUEST_TIMEOUT_SECONDS + while time.monotonic() < deadline: now = _now_utc() gate = _create_or_read_gate(container) if gate.get("type") != DATA_MANAGEMENT_SEARCH_WRITE_GATE_TYPE: @@ -217,8 +218,9 @@ def acquire_data_management_search_write_slot(container): }) if _replace_gate(container, gate, replacement) is not None: return lease_token + time.sleep(DATA_MANAGEMENT_SEARCH_WRITE_GATE_POLL_SECONDS) raise DataManagementSearchWriteGateError( - "The Data Management Search write gate changed too often to reserve a write slot." + "The Data Management Search write gate could not reserve a write slot before the request timeout." ) diff --git a/docs/explanation/fixes/SEARCH_WRITE_GATE_UPLOAD_CONTENTION_FIX.md b/docs/explanation/fixes/SEARCH_WRITE_GATE_UPLOAD_CONTENTION_FIX.md new file mode 100644 index 000000000..de4e91cf2 --- /dev/null +++ b/docs/explanation/fixes/SEARCH_WRITE_GATE_UPLOAD_CONTENTION_FIX.md @@ -0,0 +1,25 @@ +# Search Write Gate Upload Contention Fix + +Fixed in version: **0.260.030** + +## Issue + +Large personal workspace uploads of many small Markdown, JSON, or YAML files could partially fail while processing chunks for Azure AI Search. The user-facing error reported that the Data Management Search write gate changed too often to reserve a write slot. + +## Root Cause + +Document chunk writes and Data Management migrations share a Cosmos-backed write gate so migrations can safely freeze target Azure AI Search writes. Under high upload fan-in, many small files can attempt to reserve writer slots against the same gate document at once. The previous reservation path gave up after 12 rapid optimistic-concurrency conflicts, which could happen during normal burst uploads even when no migration was active. + +## Technical Details + +Files modified: + +- `application/single_app/functions_data_management_search_write_fence.py` +- `functional_tests/test_data_management_search_write_fence.py` +- `application/single_app/config.py` + +The write-slot reservation loop now waits until the existing request timeout budget is exhausted and sleeps briefly after each ETag conflict. This keeps migration safety intact while allowing normal document ingestion bursts to settle instead of failing immediately. + +## Validation + +Added regression coverage for repeated transient write-gate conflicts before a successful reservation. Validated the scenario with a direct Python execution because `pytest` was not installed in the selected environment, and compiled the touched Python files successfully. diff --git a/functional_tests/test_data_management_search_write_fence.py b/functional_tests/test_data_management_search_write_fence.py index 66ee05aa6..8d7a115ff 100644 --- a/functional_tests/test_data_management_search_write_fence.py +++ b/functional_tests/test_data_management_search_write_fence.py @@ -1,8 +1,9 @@ # test_data_management_search_write_fence.py """ Functional test for Data Management target AI Search write fencing. -Version: 0.250.071 +Version: 0.260.030 Implemented in: 0.250.071 +Updated in: 0.260.030 for upload contention retry coverage. This test ensures target SimpleChat Search writes drain before a migration freezes them and cannot resume until the owning migration releases the fence. @@ -76,6 +77,21 @@ def delete_item(self, item, partition_key, etag=None, **_kwargs): self.items.pop(item, None) +class ConflictHeavyGateContainer(FakeGateContainer): + """Simulate repeated optimistic-concurrency conflicts under upload fan-in.""" + + def __init__(self, conflicts_before_success): + super().__init__() + self.conflicts_before_success = conflicts_before_success + + def replace_item(self, item, body, etag=None, **kwargs): + with self.lock: + if self.conflicts_before_success > 0: + self.conflicts_before_success -= 1 + raise ConflictError("simulated concurrent writer") + return super().replace_item(item, body, etag=etag, **kwargs) + + def load_fence_module(): """Load the standalone fence module without the Flask application.""" module_name = "data_management_search_write_fence_test_module" @@ -204,3 +220,16 @@ def test_ambiguous_target_search_write_retains_slot_until_quarantine_expires(mon lease_seconds=150, ) assert fence["migration_id"] == "55555555-5555-5555-5555-555555555555" + + +def test_target_search_write_slot_waits_through_transient_contention(monkeypatch): + """Large small-file upload batches should not fail after a few rapid ETag conflicts.""" + module = load_fence_module() + monkeypatch.setattr(module, "DATA_MANAGEMENT_SEARCH_WRITE_GATE_POLL_SECONDS", 0.001) + monkeypatch.setattr(module, "DATA_MANAGEMENT_SEARCH_WRITE_REQUEST_TIMEOUT_SECONDS", 1) + container = ConflictHeavyGateContainer(conflicts_before_success=20) + + lease_token = module.acquire_data_management_search_write_slot(container) + + assert lease_token + assert module.release_data_management_search_write_slot(container, lease_token) is True From 47599380cc1feb94df1bebbdf8714afbb5c1905c Mon Sep 17 00:00:00 2001 From: Chad Palmer Date: Wed, 26 Aug 2026 16:14:50 -0400 Subject: [PATCH 04/11] Update version to 0.261.002 Fixed goofy github conflict --- application/single_app/config.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/application/single_app/config.py b/application/single_app/config.py index c259b7eb1..d043daca7 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -97,11 +97,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -<<<<<<< HEAD -VERSION = "0.260.029" -======= -VERSION = "0.261.001" ->>>>>>> upstream/Development +VERSION = "0.261.002" IS_DEVELOPMENT = is_development_env_enabled() # Opt-out for deployments where App Service Easy Auth is active but the platform From 0ef9f62f69a8809846a0fc665b4992d3840fb1d2 Mon Sep 17 00:00:00 2001 From: Chad Palmer Date: Wed, 26 Aug 2026 20:43:12 +0000 Subject: [PATCH 05/11] fix-many-doc-upload-issue - updated version and added release notes section. --- application/single_app/config.py | 2 +- .../fixes/SEARCH_WRITE_GATE_UPLOAD_CONTENTION_FIX.md | 2 +- docs/explanation/release_notes.md | 10 ++++++++++ .../test_data_management_search_write_fence.py | 4 ++-- 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/application/single_app/config.py b/application/single_app/config.py index d043daca7..15f055e3e 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -97,7 +97,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -VERSION = "0.261.002" +VERSION = "0.261.003" IS_DEVELOPMENT = is_development_env_enabled() # Opt-out for deployments where App Service Easy Auth is active but the platform diff --git a/docs/explanation/fixes/SEARCH_WRITE_GATE_UPLOAD_CONTENTION_FIX.md b/docs/explanation/fixes/SEARCH_WRITE_GATE_UPLOAD_CONTENTION_FIX.md index de4e91cf2..9bae38dde 100644 --- a/docs/explanation/fixes/SEARCH_WRITE_GATE_UPLOAD_CONTENTION_FIX.md +++ b/docs/explanation/fixes/SEARCH_WRITE_GATE_UPLOAD_CONTENTION_FIX.md @@ -1,6 +1,6 @@ # Search Write Gate Upload Contention Fix -Fixed in version: **0.260.030** +Fixed in version: **0.261.003** ## Issue diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index 5b3d6269c..9b146f14d 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -2,6 +2,16 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Version](https://github.com/microsoft/simplechat/tree/main/docs/explanation/features) and [Fixes by Version](https://github.com/microsoft/simplechat/tree/main/docs/explanation/fixes). +### **(v0.261.003)** + +#### Bug Fixes + +* **Large Personal Workspace Uploads No Longer Fail On Search Write Gate Contention** + * Fixed partial failures when uploading many small Markdown, JSON, or YAML files to a personal workspace at once. Document processing could fail with a message that the Data Management Search write gate changed too often to reserve a write slot. + * The write gate now waits within the existing request timeout budget and briefly backs off after transient Cosmos ETag conflicts, so burst uploads can continue while preserving the migration freeze protection for Azure AI Search writes. + * Added regression coverage for repeated transient gate conflicts before a successful slot reservation. + * (Ref: `functions_data_management_search_write_fence.py`, `test_data_management_search_write_fence.py`, [Search Write Gate Upload Contention Fix](fixes/SEARCH_WRITE_GATE_UPLOAD_CONTENTION_FIX.md)) + ### **(v0.261.002)** #### Bug Fixes diff --git a/functional_tests/test_data_management_search_write_fence.py b/functional_tests/test_data_management_search_write_fence.py index 8d7a115ff..ef8a14915 100644 --- a/functional_tests/test_data_management_search_write_fence.py +++ b/functional_tests/test_data_management_search_write_fence.py @@ -1,9 +1,9 @@ # test_data_management_search_write_fence.py """ Functional test for Data Management target AI Search write fencing. -Version: 0.260.030 +Version: 0.261.003 Implemented in: 0.250.071 -Updated in: 0.260.030 for upload contention retry coverage. +Updated in: 0.261.003 for upload contention retry coverage. This test ensures target SimpleChat Search writes drain before a migration freezes them and cannot resume until the owning migration releases the fence. From 6b8a10fc2d4d543f1e060a5f9781f9602ca54c7a Mon Sep 17 00:00:00 2001 From: Chad Palmer Date: Wed, 26 Aug 2026 22:33:32 +0000 Subject: [PATCH 06/11] fix-many-doc-upload-issue - updated documentation to reflect that all workspace document uploads are fixed. --- .../fixes/SEARCH_WRITE_GATE_UPLOAD_CONTENTION_FIX.md | 6 +++--- docs/explanation/release_notes.md | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/explanation/fixes/SEARCH_WRITE_GATE_UPLOAD_CONTENTION_FIX.md b/docs/explanation/fixes/SEARCH_WRITE_GATE_UPLOAD_CONTENTION_FIX.md index 9bae38dde..04aeef448 100644 --- a/docs/explanation/fixes/SEARCH_WRITE_GATE_UPLOAD_CONTENTION_FIX.md +++ b/docs/explanation/fixes/SEARCH_WRITE_GATE_UPLOAD_CONTENTION_FIX.md @@ -4,11 +4,11 @@ Fixed in version: **0.261.003** ## Issue -Large personal workspace uploads of many small Markdown, JSON, or YAML files could partially fail while processing chunks for Azure AI Search. The user-facing error reported that the Data Management Search write gate changed too often to reserve a write slot. +Large uploads of many small Markdown, JSON, or YAML files could partially fail while processing chunks for Azure AI Search in personal, group, or public workspaces. The user-facing error reported that the Data Management Search write gate changed too often to reserve a write slot. ## Root Cause -Document chunk writes and Data Management migrations share a Cosmos-backed write gate so migrations can safely freeze target Azure AI Search writes. Under high upload fan-in, many small files can attempt to reserve writer slots against the same gate document at once. The previous reservation path gave up after 12 rapid optimistic-concurrency conflicts, which could happen during normal burst uploads even when no migration was active. +Document chunk writes and Data Management migrations share a Cosmos-backed write gate so migrations can safely freeze target Azure AI Search writes. This gate is used before writes to the personal, group, and public workspace Search indexes. Under high upload fan-in, many small files can attempt to reserve writer slots against the same gate document at once. The previous reservation path gave up after 12 rapid optimistic-concurrency conflicts, which could happen during normal burst uploads even when no migration was active. ## Technical Details @@ -18,7 +18,7 @@ Files modified: - `functional_tests/test_data_management_search_write_fence.py` - `application/single_app/config.py` -The write-slot reservation loop now waits until the existing request timeout budget is exhausted and sleeps briefly after each ETag conflict. This keeps migration safety intact while allowing normal document ingestion bursts to settle instead of failing immediately. +The write-slot reservation loop now waits until the existing request timeout budget is exhausted and sleeps briefly after each ETag conflict. This keeps migration safety intact while allowing normal document ingestion bursts in personal, group, and public workspaces to settle instead of failing immediately. ## Validation diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index 9b146f14d..fcb48c516 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -6,9 +6,9 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Ver #### Bug Fixes -* **Large Personal Workspace Uploads No Longer Fail On Search Write Gate Contention** - * Fixed partial failures when uploading many small Markdown, JSON, or YAML files to a personal workspace at once. Document processing could fail with a message that the Data Management Search write gate changed too often to reserve a write slot. - * The write gate now waits within the existing request timeout budget and briefly backs off after transient Cosmos ETag conflicts, so burst uploads can continue while preserving the migration freeze protection for Azure AI Search writes. +* **Large Workspace Uploads No Longer Fail On Search Write Gate Contention** + * Fixed partial failures when uploading many small Markdown, JSON, or YAML files to personal, group, or public workspaces at once. Document processing could fail with a message that the Data Management Search write gate changed too often to reserve a write slot. + * The shared write gate now waits within the existing request timeout budget and briefly backs off after transient Cosmos ETag conflicts, so burst uploads can continue across all workspace Search indexes while preserving the migration freeze protection for Azure AI Search writes. * Added regression coverage for repeated transient gate conflicts before a successful slot reservation. * (Ref: `functions_data_management_search_write_fence.py`, `test_data_management_search_write_fence.py`, [Search Write Gate Upload Contention Fix](fixes/SEARCH_WRITE_GATE_UPLOAD_CONTENTION_FIX.md)) From 3529ac2d850c02538ce1216b53ac467cf726ac5c Mon Sep 17 00:00:00 2001 From: Chad Palmer Date: Thu, 27 Aug 2026 19:02:54 +0000 Subject: [PATCH 07/11] fix-many-doc-upload-issue - updated upload/chunk logic so that the cosmos gate is not overwhelmed when many documents are uploaded. --- application/single_app/config.py | 2 +- ...ions_data_management_search_write_fence.py | 20 ++++--- application/single_app/functions_documents.py | 48 ++++++++------- ...SEARCH_WRITE_GATE_UPLOAD_CONTENTION_FIX.md | 12 ++-- docs/explanation/release_notes.md | 9 +-- ...test_data_management_search_write_fence.py | 43 +++++++++++++- ...rkdown_processing_batches_search_writes.py | 58 +++++++++++++++++++ 7 files changed, 150 insertions(+), 42 deletions(-) create mode 100644 functional_tests/test_markdown_processing_batches_search_writes.py diff --git a/application/single_app/config.py b/application/single_app/config.py index 15f055e3e..db8d27fea 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -97,7 +97,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -VERSION = "0.261.003" +VERSION = "0.261.004" IS_DEVELOPMENT = is_development_env_enabled() # Opt-out for deployments where App Service Easy Auth is active but the platform diff --git a/application/single_app/functions_data_management_search_write_fence.py b/application/single_app/functions_data_management_search_write_fence.py index 1cb99b636..96c06f48f 100644 --- a/application/single_app/functions_data_management_search_write_fence.py +++ b/application/single_app/functions_data_management_search_write_fence.py @@ -2,6 +2,7 @@ """Cross-app Azure AI Search write fencing for Data Management migrations.""" import copy +import threading import time import uuid from contextlib import contextmanager @@ -25,6 +26,7 @@ DATA_MANAGEMENT_TARGET_MIGRATION_COORDINATOR_TYPE = ( "data_management_target_migration_coordinator" ) +DATA_MANAGEMENT_SEARCH_WRITE_GATE_LOCAL_LOCK = threading.Lock() class DataManagementSearchWriteGateError(RuntimeError): @@ -200,6 +202,7 @@ def acquire_data_management_search_write_slot(container): ) if gate.get("state") != DATA_MANAGEMENT_SEARCH_WRITE_GATE_STATE_OPEN: if _open_expired_gate(container, gate, now) is None: + time.sleep(DATA_MANAGEMENT_SEARCH_WRITE_GATE_POLL_SECONDS) continue continue lease_token = uuid.uuid4().hex @@ -253,14 +256,15 @@ def release_data_management_search_write_slot(container, lease_token): @contextmanager def hold_data_management_search_write_slot(container): """Hold a target Search write slot until a response is known or its ambiguity lease expires.""" - lease_token = acquire_data_management_search_write_slot(container) - response_confirmed = False - try: - yield - response_confirmed = True - finally: - if response_confirmed: - release_data_management_search_write_slot(container, lease_token) + with DATA_MANAGEMENT_SEARCH_WRITE_GATE_LOCAL_LOCK: + lease_token = acquire_data_management_search_write_slot(container) + response_confirmed = False + try: + yield + response_confirmed = True + finally: + if response_confirmed: + release_data_management_search_write_slot(container, lease_token) def acquire_data_management_search_write_fence( diff --git a/application/single_app/functions_documents.py b/application/single_app/functions_documents.py index d10d4a769..d1f188535 100644 --- a/application/single_app/functions_documents.py +++ b/application/single_app/functions_documents.py @@ -6564,7 +6564,7 @@ def process_md(document_id, user_id, temp_file_path, original_filename, enable_e if current_word_count >= min_chunk_words or i == len(initial_chunks_content) - 1: # If the combined chunk meets min size OR it's the last chunk, save it if current_chunk_text.strip(): - final_chunks.append(current_chunk_text) + final_chunks.append(current_chunk_text) buffer_chunk = "" # Reset buffer else: # Accumulate in buffer if below min size and not the last chunk @@ -6573,32 +6573,36 @@ def process_md(document_id, user_id, temp_file_path, original_filename, enable_e num_chunks_final = len(final_chunks) update_callback(number_of_pages=num_chunks_final) - for idx, chunk_content in enumerate(final_chunks, start=1): - update_callback( - current_file_chunk=idx, - status=f"Saving chunk {idx}/{num_chunks_final}..." - ) - args = { + all_chunks = [] + for chunk_content in final_chunks: + if not chunk_content or not chunk_content.strip(): + continue + all_chunks.append({ "page_text_content": chunk_content, - "page_number": idx, + "page_number": len(all_chunks) + 1, "file_name": original_filename, - "user_id": user_id, - "document_id": document_id - } - - if is_public_workspace: - args["public_workspace_id"] = public_workspace_id - elif is_group: - args["group_id"] = group_id + }) - token_usage = save_chunks(**args) - total_chunks_saved += 1 + if all_chunks: + if len(all_chunks) != num_chunks_final: + num_chunks_final = len(all_chunks) + update_callback(number_of_pages=num_chunks_final) + update_callback( + current_file_chunk=1, + status=f"Batch saving {num_chunks_final} Markdown chunk(s)..." + ) + token_usage = save_chunks_batch( + all_chunks, + user_id, + document_id, + group_id=group_id, + public_workspace_id=public_workspace_id + ) + total_chunks_saved = len(all_chunks) - # Accumulate embedding tokens if token_usage: - total_embedding_tokens += token_usage.get('total_tokens', 0) - if not embedding_model_name: - embedding_model_name = token_usage.get('model_deployment_name') + total_embedding_tokens = token_usage.get('total_tokens', 0) + embedding_model_name = token_usage.get('model_deployment_name') except Exception as e: raise Exception(f"Failed processing Markdown file {original_filename}: {e}") diff --git a/docs/explanation/fixes/SEARCH_WRITE_GATE_UPLOAD_CONTENTION_FIX.md b/docs/explanation/fixes/SEARCH_WRITE_GATE_UPLOAD_CONTENTION_FIX.md index 04aeef448..b8c06aaae 100644 --- a/docs/explanation/fixes/SEARCH_WRITE_GATE_UPLOAD_CONTENTION_FIX.md +++ b/docs/explanation/fixes/SEARCH_WRITE_GATE_UPLOAD_CONTENTION_FIX.md @@ -1,25 +1,27 @@ # Search Write Gate Upload Contention Fix -Fixed in version: **0.261.003** +Fixed in version: **0.261.004** ## Issue -Large uploads of many small Markdown, JSON, or YAML files could partially fail while processing chunks for Azure AI Search in personal, group, or public workspaces. The user-facing error reported that the Data Management Search write gate changed too often to reserve a write slot. +Large uploads of many small Markdown, JSON, or YAML files could partially fail while processing chunks for Azure AI Search in personal, group, or public workspaces. The user-facing error reported that the Data Management Search write gate changed too often to reserve a write slot. Concurrent Markdown uploads could also intermittently fail with `OrderedDict mutated during iteration` while processing different files in different environments. ## Root Cause -Document chunk writes and Data Management migrations share a Cosmos-backed write gate so migrations can safely freeze target Azure AI Search writes. This gate is used before writes to the personal, group, and public workspace Search indexes. Under high upload fan-in, many small files can attempt to reserve writer slots against the same gate document at once. The previous reservation path gave up after 12 rapid optimistic-concurrency conflicts, which could happen during normal burst uploads even when no migration was active. +Document chunk writes and Data Management migrations share a Cosmos-backed write gate so migrations can safely freeze target Azure AI Search writes. This gate is used before writes to the personal, group, and public workspace Search indexes. Under high upload fan-in, many small files can attempt to reserve writer slots against the same gate document at once. The previous reservation path gave up after 12 rapid optimistic-concurrency conflicts, and the first retry fix still allowed worker-local upload threads to stampede the same gate document. Markdown processing also wrote one chunk at a time, multiplying gate reservations for a single file. ## Technical Details Files modified: - `application/single_app/functions_data_management_search_write_fence.py` +- `application/single_app/functions_documents.py` - `functional_tests/test_data_management_search_write_fence.py` +- `functional_tests/test_markdown_processing_batches_search_writes.py` - `application/single_app/config.py` -The write-slot reservation loop now waits until the existing request timeout budget is exhausted and sleeps briefly after each ETag conflict. This keeps migration safety intact while allowing normal document ingestion bursts in personal, group, and public workspaces to settle instead of failing immediately. +The write-slot reservation loop now waits until the existing request timeout budget is exhausted, sleeps briefly after each ETag conflict, and serializes Search writes inside each worker process. Markdown processing now collects non-empty chunks and sends them through `save_chunks_batch`, matching the existing batched processors and reducing both Search write gate mutations and embedding/Search client churn. ## Validation -Added regression coverage for repeated transient write-gate conflicts before a successful reservation. Validated the scenario with a direct Python execution because `pytest` was not installed in the selected environment, and compiled the touched Python files successfully. +Added regression coverage for repeated transient write-gate conflicts before a successful reservation, worker-local Search write serialization, and Markdown use of the batch chunk writer. Validated the concurrency scenarios with direct Python execution because `pytest` was not installed in the selected environment, and compiled the touched Python files successfully. diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index fcb48c516..3697f2716 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -2,15 +2,16 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Version](https://github.com/microsoft/simplechat/tree/main/docs/explanation/features) and [Fixes by Version](https://github.com/microsoft/simplechat/tree/main/docs/explanation/fixes). -### **(v0.261.003)** +### **(v0.261.004)** #### Bug Fixes * **Large Workspace Uploads No Longer Fail On Search Write Gate Contention** * Fixed partial failures when uploading many small Markdown, JSON, or YAML files to personal, group, or public workspaces at once. Document processing could fail with a message that the Data Management Search write gate changed too often to reserve a write slot. - * The shared write gate now waits within the existing request timeout budget and briefly backs off after transient Cosmos ETag conflicts, so burst uploads can continue across all workspace Search indexes while preserving the migration freeze protection for Azure AI Search writes. - * Added regression coverage for repeated transient gate conflicts before a successful slot reservation. - * (Ref: `functions_data_management_search_write_fence.py`, `test_data_management_search_write_fence.py`, [Search Write Gate Upload Contention Fix](fixes/SEARCH_WRITE_GATE_UPLOAD_CONTENTION_FIX.md)) + * The shared write gate now waits within the existing request timeout budget, briefly backs off after transient Cosmos ETag conflicts, and serializes Search writes inside each worker process. This prevents local upload threads from stampeding the same gate document while preserving the migration freeze protection for Azure AI Search writes. + * Markdown processing now batches its chunk embeddings and Search upload instead of reserving the gate once per chunk, which reduces contention and avoids the intermittent `OrderedDict mutated during iteration` failures seen during concurrent Markdown ingestion. + * Added regression coverage for repeated transient gate conflicts, local worker serialization, and Markdown use of the batch chunk writer. + * (Ref: `functions_data_management_search_write_fence.py`, `functions_documents.py`, `test_data_management_search_write_fence.py`, `test_markdown_processing_batches_search_writes.py`, [Search Write Gate Upload Contention Fix](fixes/SEARCH_WRITE_GATE_UPLOAD_CONTENTION_FIX.md)) ### **(v0.261.002)** diff --git a/functional_tests/test_data_management_search_write_fence.py b/functional_tests/test_data_management_search_write_fence.py index ef8a14915..9dddad138 100644 --- a/functional_tests/test_data_management_search_write_fence.py +++ b/functional_tests/test_data_management_search_write_fence.py @@ -1,9 +1,9 @@ # test_data_management_search_write_fence.py """ Functional test for Data Management target AI Search write fencing. -Version: 0.261.003 +Version: 0.261.004 Implemented in: 0.250.071 -Updated in: 0.261.003 for upload contention retry coverage. +Updated in: 0.261.004 for upload contention retry coverage. This test ensures target SimpleChat Search writes drain before a migration freezes them and cannot resume until the owning migration releases the fence. @@ -233,3 +233,42 @@ def test_target_search_write_slot_waits_through_transient_contention(monkeypatch assert lease_token assert module.release_data_management_search_write_slot(container, lease_token) is True + + +def test_local_search_writes_are_serialized_before_touching_gate(monkeypatch): + """Concurrent uploads in one worker should not stampede the shared Cosmos gate document.""" + module = load_fence_module() + monkeypatch.setattr(module, "DATA_MANAGEMENT_SEARCH_WRITE_GATE_POLL_SECONDS", 0.001) + container = FakeGateContainer() + started = threading.Event() + release_first_write = threading.Event() + second_write_entered = [] + + def first_write(): + with module.hold_data_management_search_write_slot(container): + started.set() + release_first_write.wait(timeout=2.0) + + def second_write(): + started.wait(timeout=2.0) + with module.hold_data_management_search_write_slot(container): + second_write_entered.append(True) + + first_thread = threading.Thread(target=first_write) + second_thread = threading.Thread(target=second_write) + first_thread.start() + second_thread.start() + started.wait(timeout=2.0) + time.sleep(0.05) + + assert second_write_entered == [] + gate = container.items[module.DATA_MANAGEMENT_SEARCH_WRITE_GATE_ID] + assert gate["active_writer_count"] == 1 + + release_first_write.set() + first_thread.join(timeout=2.0) + second_thread.join(timeout=2.0) + + assert second_write_entered == [True] + gate = container.items[module.DATA_MANAGEMENT_SEARCH_WRITE_GATE_ID] + assert gate["active_writer_count"] == 0 diff --git a/functional_tests/test_markdown_processing_batches_search_writes.py b/functional_tests/test_markdown_processing_batches_search_writes.py new file mode 100644 index 000000000..f07f04215 --- /dev/null +++ b/functional_tests/test_markdown_processing_batches_search_writes.py @@ -0,0 +1,58 @@ +#!/usr/bin/env python3 +""" +Functional test for Markdown document processing batch Search writes. +Version: 0.261.004 +Implemented in: 0.261.004 + +This test ensures Markdown processing uses the batch chunk writer so large +workspace uploads do not reserve the Data Management Search write gate once per +chunk. +""" + +import ast +import sys +from pathlib import Path + + +REPO_ROOT = Path(__file__).resolve().parents[1] +FUNCTIONS_DOCUMENTS_PATH = REPO_ROOT / "application" / "single_app" / "functions_documents.py" + + +def _find_function(module_tree, function_name): + for node in module_tree.body: + if isinstance(node, ast.FunctionDef) and node.name == function_name: + return node + raise AssertionError(f"Could not find {function_name} in functions_documents.py") + + +def _called_function_names(function_node): + names = [] + for node in ast.walk(function_node): + if not isinstance(node, ast.Call): + continue + if isinstance(node.func, ast.Name): + names.append(node.func.id) + elif isinstance(node.func, ast.Attribute): + names.append(node.func.attr) + return names + + +def test_process_md_uses_batch_chunk_writer(): + """Markdown processing should use one batched Search write path per file.""" + module_tree = ast.parse(FUNCTIONS_DOCUMENTS_PATH.read_text(encoding="utf-8")) + process_md = _find_function(module_tree, "process_md") + call_names = _called_function_names(process_md) + + assert "save_chunks_batch" in call_names + assert "save_chunks" not in call_names + + +if __name__ == "__main__": + try: + test_process_md_uses_batch_chunk_writer() + except Exception as exc: + print(f"Test failed: {exc}") + raise + + print("Markdown processing batch Search write test passed") + sys.exit(0) From 217ec514c89f77fc003a6c8482e0c14546f24886 Mon Sep 17 00:00:00 2001 From: Chad Palmer Date: Fri, 28 Aug 2026 23:04:50 +0000 Subject: [PATCH 08/11] fix-many-doc-upload-issue - added retry logic to retry document parsing that throws orderddict error. --- application/single_app/config.py | 2 +- application/single_app/functions_documents.py | 53 ++++++++++++- .../static/js/public/public_workspace.js | 62 +++++++-------- .../js/workspace/workspace-documents.js | 36 +++++---- .../templates/group_workspaces.html | 78 +++++++++++-------- ...SEARCH_WRITE_GATE_UPLOAD_CONTENTION_FIX.md | 6 +- .../WORKSPACE_UPLOAD_STATUS_COUNTER_FIX.md | 27 +++++++ docs/explanation/release_notes.md | 18 +++++ ...rkdown_processing_batches_search_writes.py | 24 +++++- ...st_workspace_upload_request_status_copy.py | 34 ++++++++ 10 files changed, 250 insertions(+), 90 deletions(-) create mode 100644 docs/explanation/fixes/WORKSPACE_UPLOAD_STATUS_COUNTER_FIX.md create mode 100644 ui_tests/test_workspace_upload_request_status_copy.py diff --git a/application/single_app/config.py b/application/single_app/config.py index db8d27fea..4670212d9 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -97,7 +97,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -VERSION = "0.261.004" +VERSION = "0.261.006" IS_DEVELOPMENT = is_development_env_enabled() # Opt-out for deployments where App Service Easy Auth is active but the platform diff --git a/application/single_app/functions_documents.py b/application/single_app/functions_documents.py index d1f188535..fe680e803 100644 --- a/application/single_app/functions_documents.py +++ b/application/single_app/functions_documents.py @@ -3,6 +3,7 @@ import re import shutil import subprocess +import time import traceback import zipfile from io import BytesIO @@ -45,6 +46,11 @@ class DocumentSearchAclProjectionDeferredError(RuntimeError): """Raised when an authorization-reducing Search ACL update must be retried safely.""" +MARKDOWN_ORDERED_DICT_MUTATION_MESSAGE = "OrderedDict mutated during iteration" +MARKDOWN_ORDERED_DICT_RETRY_ATTEMPTS = 2 +MARKDOWN_ORDERED_DICT_RETRY_DELAY_SECONDS = 0.5 + + def _search_indexing_results_succeeded(results): """Return whether Azure AI Search acknowledged every requested document mutation.""" normalized_results = list(results or []) @@ -9007,6 +9013,48 @@ def _resolve_processing_complete_status(total_chunks_saved, file_ext, image_exte return "Processing complete" + +def _is_markdown_ordered_dict_mutation_error(exc): + """Return whether Markdown processing hit the transient OrderedDict mutation failure.""" + return MARKDOWN_ORDERED_DICT_MUTATION_MESSAGE in str(exc or "") + + +def _process_markdown_with_ordered_dict_retry(processor_args, update_callback): + """Retry Markdown processing when the parser hits a transient OrderedDict mutation.""" + for attempt in range(MARKDOWN_ORDERED_DICT_RETRY_ATTEMPTS + 1): + try: + return process_md(**{k: v for k, v in processor_args.items() if k != "file_ext"}) + except Exception as exc: + if ( + not _is_markdown_ordered_dict_mutation_error(exc) or + attempt >= MARKDOWN_ORDERED_DICT_RETRY_ATTEMPTS + ): + raise + + retry_number = attempt + 1 + original_filename = processor_args.get("original_filename") + document_id = processor_args.get("document_id") + log_event( + "[DOCUMENTS] Retrying Markdown processing after transient OrderedDict mutation.", + extra={ + "document_id": document_id, + "file_name": original_filename, + "retry_number": retry_number, + "max_retries": MARKDOWN_ORDERED_DICT_RETRY_ATTEMPTS, + "error_type": type(exc).__name__, + }, + level=logging.WARNING, + exceptionTraceback=True, + ) + update_callback( + status=( + "Retrying Markdown processing after a transient parser concurrency error " + f"({retry_number}/{MARKDOWN_ORDERED_DICT_RETRY_ATTEMPTS})..." + ) + ) + time.sleep(MARKDOWN_ORDERED_DICT_RETRY_DELAY_SECONDS * retry_number) + + def process_document_upload_background(document_id, user_id, temp_file_path, original_filename, group_id=None, public_workspace_id=None, extraction_mode_override=None): """ Main background task dispatcher for document processing. @@ -9132,7 +9180,10 @@ def update_doc_callback(**kwargs): else: total_chunks_saved = result elif file_ext == '.md': - result = process_md(**{k: v for k, v in processor_args_without_auto_metadata.items() if k != "file_ext"}) + result = _process_markdown_with_ordered_dict_retry( + processor_args_without_auto_metadata, + update_doc_callback, + ) if isinstance(result, tuple) and len(result) == 3: total_chunks_saved, total_embedding_tokens, embedding_model_name = result else: diff --git a/application/single_app/static/js/public/public_workspace.js b/application/single_app/static/js/public/public_workspace.js index 3893d4715..3410be79a 100644 --- a/application/single_app/static/js/public/public_workspace.js +++ b/application/single_app/static/js/public/public_workspace.js @@ -1980,6 +1980,29 @@ async function onPublicUploadClick() { let completed = 0; let failed = 0; + function updatePublicUploadRequestSummary() { + if (uploadStatus) uploadStatus.textContent = `Queued ${completed}/${files.length}${failed ? `, Upload requests not confirmed: ${failed}` : ''}`; + } + + function finishPublicUploadRequests() { + fileInput.value = ''; + publicDocsCurrentPage = 1; + fetchPublicDocs(); + + if (uploadBtn) { + uploadBtn.disabled = false; + uploadBtn.textContent = 'Upload Document(s)'; + } + + if (uploadStatus) { + uploadStatus.textContent = failed + ? `Upload requests complete. Queued ${completed}/${files.length}; ${failed} request(s) did not confirm. Check the document list below for final processing status.` + : `Queued ${completed}/${files.length} file(s). Check the document list below for processing status.`; + } + + if (progressContainer) progressContainer.innerHTML = ''; + } + // Helper to create a unique ID for each file function makeId(file) { return 'progress-' + Math.random().toString(36).slice(2, 10) + '-' + encodeURIComponent(file.name.replace(/\W+/g, '')); @@ -2034,7 +2057,7 @@ async function onPublicUploadClick() { progressBar.classList.remove('progress-bar-animated'); } if (statusText) { - statusText.textContent = `Uploaded ${file.name} (100%)`; + statusText.textContent = `Queued ${file.name} (100%)`; } completed++; } else { @@ -2044,26 +2067,13 @@ async function onPublicUploadClick() { progressBar.classList.remove('progress-bar-animated'); } if (statusText) { - statusText.textContent = `Failed to upload ${file.name}`; + statusText.textContent = `Upload request did not confirm for ${file.name}`; } failed++; } - // Update summary status - if (uploadStatus) uploadStatus.textContent = `Uploaded ${completed}/${files.length}${failed ? `, Failed: ${failed}` : ''}`; + updatePublicUploadRequestSummary(); if (completed + failed === files.length) { - fileInput.value = ''; - publicDocsCurrentPage = 1; - fetchPublicDocs(); - - // Re-enable upload button if it exists - if (uploadBtn) { - uploadBtn.disabled = false; - uploadBtn.textContent = 'Upload Document(s)'; - } - - // Clear upload progress bars after all uploads and table refresh - const progressContainer = document.getElementById('public-upload-progress-container'); - if (progressContainer) progressContainer.innerHTML = ''; + finishPublicUploadRequests(); } }; @@ -2074,24 +2084,12 @@ async function onPublicUploadClick() { progressBar.classList.remove('progress-bar-animated'); } if (statusText) { - statusText.textContent = `Failed to upload ${file.name}`; + statusText.textContent = `Upload request did not confirm for ${file.name}`; } failed++; - if (uploadStatus) uploadStatus.textContent = `Uploaded ${completed}/${files.length}${failed ? `, Failed: ${failed}` : ''}`; + updatePublicUploadRequestSummary(); if (completed + failed === files.length) { - fileInput.value = ''; - publicDocsCurrentPage = 1; - fetchPublicDocs(); - - // Re-enable upload button if it exists - if (uploadBtn) { - uploadBtn.disabled = false; - uploadBtn.textContent = 'Upload Document(s)'; - } - - // Clear upload progress bars after all uploads and table refresh - const progressContainer = document.getElementById('public-upload-progress-container'); - if (progressContainer) progressContainer.innerHTML = ''; + finishPublicUploadRequests(); } }; diff --git a/application/single_app/static/js/workspace/workspace-documents.js b/application/single_app/static/js/workspace/workspace-documents.js index a887786f5..92c1de3f3 100644 --- a/application/single_app/static/js/workspace/workspace-documents.js +++ b/application/single_app/static/js/workspace/workspace-documents.js @@ -1481,6 +1481,20 @@ async function uploadWorkspaceFiles(files) { let completed = 0; let failed = 0; + function updateWorkspaceUploadRequestSummary() { + uploadStatusSpan.textContent = `Queued ${completed}/${files.length}${failed ? `, Upload requests not confirmed: ${failed}` : ''}`; + } + + function finishWorkspaceUploadRequests() { + fileInput.value = ''; + docsCurrentPage = 1; + fetchUserDocuments(); + uploadStatusSpan.textContent = failed + ? `Upload requests complete. Queued ${completed}/${files.length}; ${failed} request(s) did not confirm. Check the document list below for final processing status.` + : `Queued ${completed}/${files.length} file(s). Check the document list below for processing status.`; + if (progressContainer) progressContainer.innerHTML = ''; + } + // Helper to create a unique ID for each file function makeId(file) { return 'progress-' + Math.random().toString(36).slice(2, 10) + '-' + encodeURIComponent(file.name.replace(/\W+/g, '')); @@ -1535,7 +1549,7 @@ async function uploadWorkspaceFiles(files) { progressBar.classList.remove('progress-bar-animated'); } if (statusText) { - statusText.textContent = `Uploaded ${file.name} (100%)`; + statusText.textContent = `Queued ${file.name} (100%)`; } completed++; } else { @@ -1545,18 +1559,13 @@ async function uploadWorkspaceFiles(files) { progressBar.classList.remove('progress-bar-animated'); } if (statusText) { - statusText.textContent = `Failed to upload ${file.name}`; + statusText.textContent = `Upload request did not confirm for ${file.name}`; } failed++; } - // Update summary status - uploadStatusSpan.textContent = `Uploaded ${completed}/${files.length}${failed ? `, Failed: ${failed}` : ''}`; + updateWorkspaceUploadRequestSummary(); if (completed + failed === files.length) { - fileInput.value = ''; - docsCurrentPage = 1; - fetchUserDocuments(); - // Clear upload progress bars after all uploads and table refresh - if (progressContainer) progressContainer.innerHTML = ''; + finishWorkspaceUploadRequests(); } }; @@ -1567,15 +1576,12 @@ async function uploadWorkspaceFiles(files) { progressBar.classList.remove('progress-bar-animated'); } if (statusText) { - statusText.textContent = `Failed to upload ${file.name}`; + statusText.textContent = `Upload request did not confirm for ${file.name}`; } failed++; - uploadStatusSpan.textContent = `Uploaded ${completed}/${files.length}${failed ? `, Failed: ${failed}` : ''}`; + updateWorkspaceUploadRequestSummary(); if (completed + failed === files.length) { - fileInput.value = ''; - docsCurrentPage = 1; - fetchUserDocuments(); - if (progressContainer) progressContainer.innerHTML = ''; + finishWorkspaceUploadRequests(); } }; diff --git a/application/single_app/templates/group_workspaces.html b/application/single_app/templates/group_workspaces.html index 12e2b08cf..68b120eaa 100644 --- a/application/single_app/templates/group_workspaces.html +++ b/application/single_app/templates/group_workspaces.html @@ -3404,6 +3404,20 @@ let completed = 0; let failed = 0; + function updateGroupUploadRequestSummary() { + groupUploadStatusSpan.textContent = `Queued ${completed}/${files.length}${failed ? `, Upload requests not confirmed: ${failed}` : ''}`; + } + + function finishGroupUploadRequests() { + groupFileInput.value = ''; + groupDocsCurrentPage = 1; + fetchGroupDocuments(); + groupUploadStatusSpan.textContent = failed + ? `Upload requests complete. Queued ${completed}/${files.length}; ${failed} request(s) did not confirm. Check the document list below for final processing status.` + : `Queued ${completed}/${files.length} file(s). Check the document list below for processing status.`; + if (progressContainer) progressContainer.innerHTML = ''; + } + // Helper to create a unique ID for each file function makeId(file) { return 'group-progress-' + Math.random().toString(36).slice(2, 10) + '-' + encodeURIComponent(file.name.replace(/\W+/g, '')); @@ -3458,7 +3472,7 @@ progressBar.classList.remove('progress-bar-animated'); } if (statusText) { - statusText.textContent = `Uploaded ${file.name} (100%)`; + statusText.textContent = `Queued ${file.name} (100%)`; } completed++; } else { @@ -3468,18 +3482,13 @@ progressBar.classList.remove('progress-bar-animated'); } if (statusText) { - statusText.textContent = `Failed to upload ${file.name}`; + statusText.textContent = `Upload request did not confirm for ${file.name}`; } failed++; } - // Update summary status - groupUploadStatusSpan.textContent = `Uploaded ${completed}/${files.length}${failed ? `, Failed: ${failed}` : ''}`; + updateGroupUploadRequestSummary(); if (completed + failed === files.length) { - groupFileInput.value = ''; - groupDocsCurrentPage = 1; - fetchGroupDocuments(); - // Clear upload progress bars after all uploads and table refresh - if (progressContainer) progressContainer.innerHTML = ''; + finishGroupUploadRequests(); } }; @@ -3490,15 +3499,12 @@ progressBar.classList.remove('progress-bar-animated'); } if (statusText) { - statusText.textContent = `Failed to upload ${file.name}`; + statusText.textContent = `Upload request did not confirm for ${file.name}`; } failed++; - groupUploadStatusSpan.textContent = `Uploaded ${completed}/${files.length}${failed ? `, Failed: ${failed}` : ''}`; + updateGroupUploadRequestSummary(); if (completed + failed === files.length) { - groupFileInput.value = ''; - groupDocsCurrentPage = 1; - fetchGroupDocuments(); - if (progressContainer) progressContainer.innerHTML = ''; + finishGroupUploadRequests(); } }; @@ -6846,6 +6852,22 @@