From 3d5fc75f7b1e462b3b5f88b5353cd27a7af60c6a Mon Sep 17 00:00:00 2001
From: Sergey Kozyrenko <sirozha_blr@tut.by>
Date: Thu, 23 Jul 2026 02:14:53 +0700
Subject: [PATCH] fix(ui): guard the detail loading branch too, not only the
 error branch
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The B1-B3/B6/B7 pass guarded the settings detail pages' error branch with
`&& !data`, but each detail page has an `if (loading)` branch that runs FIRST,
and it was left unguarded — so the fix it was meant to deliver never applied.
The queries are cache-and-network, so a background revalidation (a list→detail
navigation into a warm cache, or a post-save refetchQueries) reports loading
true with cached data present and blanks the edit form to the full-page spinner
before the guarded error branch is ever reached.

- settings-prompt.tsx / settings-provider.tsx: `if (loading)` -> `&& !data`
- template.tsx spinner: `if (!isNew && isLoadingTemplate)` -> `&& !template`,
  which also realigns it with knowledge.tsx (fixed in 28ab3d2 to gate on the
  entity, not raw loading) — the two had silently diverged.
- docs/list_detail_pages.md: the "canonical render gate" recipe still taught the
  unguarded `if (isLoading)` it tells new pages to copy; both branches now gate.

settings-provider.test gains the loading-with-cached-data case (revert -> red);
the detail loading branch had zero coverage before. Found by the adversarial
review of the previous fix pass — the guard I applied was one line short.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 frontend/docs/list_detail_pages.md                    | 11 +++++++----
 frontend/src/pages/settings/settings-prompt.tsx       |  2 +-
 .../src/pages/settings/settings-provider.test.tsx     | 11 +++++++++++
 frontend/src/pages/settings/settings-provider.tsx     |  2 +-
 frontend/src/pages/templates/template.tsx             |  2 +-
 5 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/frontend/docs/list_detail_pages.md b/frontend/docs/list_detail_pages.md
index 6002e095..17be4fb3 100644
--- a/frontend/docs/list_detail_pages.md
+++ b/frontend/docs/list_detail_pages.md
@@ -466,8 +466,13 @@ export function EntitiesPage() {
 
     const pageHeader = <AppHeader title="Entities" /* ...actions */ />;
 
-    // Canonical 4-branch render gate — pageHeader renders in ALL branches:
-    if (isLoading) {
+    // Canonical 4-branch render gate — pageHeader renders in ALL branches.
+    // Both loading and error gate on "nothing to show yet": the query is
+    // cache-and-network, so a background revalidation reports loading (and, on
+    // failure, error) with cached data still present. Without `&& !data` that
+    // refetch blanks a working list — or an edit form with unsaved changes —
+    // with the spinner/error for the round-trip.
+    if (isLoading && entities.length === 0) {
         return (
             <>
                 {pageHeader}
@@ -475,8 +480,6 @@ export function EntitiesPage() {
             </>
         );
     }
-    // Show the error surface only when there's no data — a failed background
-    // refetch must not blank a working list.
     if (error && entities.length === 0) {
         return (
             <>
diff --git a/frontend/src/pages/settings/settings-prompt.tsx b/frontend/src/pages/settings/settings-prompt.tsx
index 0c5d2ebf..02a74c1d 100644
--- a/frontend/src/pages/settings/settings-prompt.tsx
+++ b/frontend/src/pages/settings/settings-prompt.tsx
@@ -727,7 +727,7 @@ function SettingsPrompt() {
         </AppHeader>
     );
 
-    if (loading) {
+    if (loading && !data) {
         return (
             <>
                 {pageHeader}
diff --git a/frontend/src/pages/settings/settings-provider.test.tsx b/frontend/src/pages/settings/settings-provider.test.tsx
index fe631325..4ce76951 100644
--- a/frontend/src/pages/settings/settings-provider.test.tsx
+++ b/frontend/src/pages/settings/settings-provider.test.tsx
@@ -137,4 +137,15 @@ describe('SettingsProvider create-form type guards', () => {
         expect(screen.queryByText('Error loading provider data')).not.toBeInTheDocument();
         expect(navigate).not.toHaveBeenCalled();
     });
+
+    // The loading branch runs before the error branch, so it needs the same guard: a background
+    // refetch reports loading:true with cached data and must not blank the form to the spinner.
+    it('keeps the form on a background refetch while cached data is present', () => {
+        setSearch('type=anthropic');
+        queryResult.loading = true;
+        render(<SettingsProvider />);
+
+        expect(screen.queryByText('Loading provider data...')).not.toBeInTheDocument();
+        expect(navigate).not.toHaveBeenCalled();
+    });
 });
diff --git a/frontend/src/pages/settings/settings-provider.tsx b/frontend/src/pages/settings/settings-provider.tsx
index e985350e..1c7a0ca1 100644
--- a/frontend/src/pages/settings/settings-provider.tsx
+++ b/frontend/src/pages/settings/settings-provider.tsx
@@ -1605,7 +1605,7 @@ function SettingsProvider() {
         }
     };
 
-    if (loading) {
+    if (loading && !data) {
         return (
             <>
                 <AppHeader>
diff --git a/frontend/src/pages/templates/template.tsx b/frontend/src/pages/templates/template.tsx
index b2211ee1..93c90e08 100644
--- a/frontend/src/pages/templates/template.tsx
+++ b/frontend/src/pages/templates/template.tsx
@@ -734,7 +734,7 @@ function Template() {
         />
     );
 
-    if (!isNew && isLoadingTemplate) {
+    if (!isNew && isLoadingTemplate && !template) {
         return (
             <div className={isDesktop ? 'flex h-[100dvh] min-h-0 flex-col' : 'flex min-h-[100dvh] flex-col'}>
                 {pageHeader}