Address PR #1383 review: dedupe key, error-path cleanup, type derivation, auth gate

- buildRequestKey: add "user" to the type union and a userSlug? parameter
  so /users/:slug routes get a per-slug request key. Without this, every
  user-profile resolution dedupes to the same "user" key and rapid profile
  switches silently drop all but the first request. The CentralRouteManager
  call site now passes route.userSlug explicitly.
- User not-found path now clears the residual entity vars (openedCorpus,
  openedDocument, openedExtract, openedThread, openedLabelset, openedUser).
  The corpus/document/extract paths redirect to /404 which clears via the
  browse-route handler, but the user error path stays on the URL so the
  visitor can fix the slug — meaning cleanup is the manager's job.
- OpenedUserProfile is now derived from GetUserOutput["userBySlug"] so the
  type tracks the GraphQL schema automatically and the unsafe `as` cast in
  the user-resolution branch goes away.
- ProfileRedirect now waits for authStatusVar !== "LOADING" and
  authInitCompleteVar before redirecting. Previously a cold load of
  /profile by an authenticated user would briefly redirect to /login while
  Auth0 was still resolving.
- Added two ProfileRedirect tests that lock in the auth-gate behavior.
- routing_system.md: collapse the historical violations bullet list into a
  single sentence — Corpuses.tsx is no longer a violator and the static
  centralRouteDiscipline test now enforces zero violations.

Author: JSv4

docs/frontend/routing_system.md

@@ -57,13 +57,7 @@ The OpenContracts routing system follows a **centralized architecture** where **
 **The Correct Pattern:**
 Components wanting to change URL-driven state must use utility functions that update the URL. CentralRouteManager Phase 2 will detect the URL change and set the reactive var. This maintains unidirectional data flow: Component → URL → CentralRouteManager → Reactive Var → Component.
 
-This is non-negotiable. Violations cause infinite loops, route jittering, competing state updates, and unpredictable behavior. During development, we systematically removed all violations from:
-- `CorpusDocumentCards.tsx` (caused infinite loop bug)
-- `DocumentKnowledgeBase.tsx` (4 violations)
-- `FloatingDocumentControls.tsx` (1 violation)
-- `NavMenu.tsx` / `MobileNavMenu.tsx` (clearing on menu clicks)
-- `CorpusBreadcrumbs.tsx` (manual clearing)
-- `Corpuses.tsx` (3 violations)
+This is non-negotiable. Violations cause infinite loops, route jittering, competing state updates, and unpredictable behavior. The static `centralRouteDiscipline.test.ts` regression guard enforces this rule today, and the historical violations it was built to prevent (URL bypasses in `CorpusDocumentCards.tsx`, `DocumentKnowledgeBase.tsx`, `FloatingDocumentControls.tsx`, `NavMenu.tsx`/`MobileNavMenu.tsx`, `CorpusBreadcrumbs.tsx`, and `Corpuses.tsx`) have all been removed.
 
 **If you find yourself writing `openedCorpus(someValue)`, `openedDocument(someValue)`, `openedExtract(someValue)`, `openedThread(someValue)`, `openedLabelset(someValue)`, or `openedUser(someValue)` anywhere except `CentralRouteManager.tsx`, STOP. You are introducing a bug.**
 

frontend/src/components/routes/ProfileRedirect.tsx

@@ -13,10 +13,25 @@
 import React from "react";
 import { Navigate } from "react-router-dom";
 import { useReactiveVar } from "@apollo/client";
-import { backendUserObj } from "../../graphql/cache";
+import {
+  backendUserObj,
+  authStatusVar,
+  authInitCompleteVar,
+} from "../../graphql/cache";
+import { ModernLoadingDisplay } from "../widgets/ModernLoadingDisplay";
 
 export const ProfileRedirect: React.FC = () => {
   const currentUser = useReactiveVar(backendUserObj);
+  const authStatus = useReactiveVar(authStatusVar);
+  const authInitComplete = useReactiveVar(authInitCompleteVar);
+
+  // While Auth0 is still resolving the session, backendUserObj is null even
+  // for an authenticated visitor. Redirecting before this settles would
+  // briefly send an authenticated user to /login. Hold the render until the
+  // auth pipeline (token + cache reset) signals it's done.
+  if (authStatus === "LOADING" || !authInitComplete) {
+    return <ModernLoadingDisplay type="default" message="Resolving profile…" />;
+  }
 
   if (!currentUser?.slug) {
     return <Navigate to="/login" replace />;

frontend/src/components/routes/__tests__/ProfileRedirect.test.tsx

@@ -3,14 +3,23 @@ import { MockedProvider } from "@apollo/client/testing";
 import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
 import { MemoryRouter, Route, Routes, useLocation } from "react-router-dom";
 import { ProfileRedirect } from "../ProfileRedirect";
-import { backendUserObj } from "../../../graphql/cache";
+import {
+  backendUserObj,
+  authStatusVar,
+  authInitCompleteVar,
+} from "../../../graphql/cache";
+
+vi.mock("../../widgets/ModernLoadingDisplay", () => ({
+  ModernLoadingDisplay: () => <div>Loading...</div>,
+}));
 
 /**
  * Tests for ProfileRedirect.
  *
  * /profile is auth-state-driven, not URL-state-driven, so the redirect lives
- * outside CentralRouteManager. ProfileRedirect reads backendUserObj and
- * issues a Navigate to /login (anonymous) or /users/<slug> (logged in).
+ * outside CentralRouteManager. ProfileRedirect waits for the auth pipeline
+ * to complete, then issues a Navigate to /login (anonymous) or
+ * /users/<slug> (logged in).
  */
 describe("ProfileRedirect", () => {
   const LocationReporter: React.FC = () => {
@@ -33,11 +42,15 @@ describe("ProfileRedirect", () => {
 
   beforeEach(() => {
     backendUserObj(null);
+    authStatusVar("ANONYMOUS");
+    authInitCompleteVar(true);
   });
 
   afterEach(() => {
     vi.clearAllMocks();
     backendUserObj(null);
+    authStatusVar("LOADING");
+    authInitCompleteVar(false);
   });
 
   it("redirects to /login when no user is authenticated", async () => {
@@ -47,16 +60,35 @@ describe("ProfileRedirect", () => {
   });
 
   it("redirects to /users/<slug> when a user is authenticated", async () => {
+    authStatusVar("AUTHENTICATED");
     backendUserObj({ id: "u-1", slug: "alice" } as any);
     renderAt("/profile");
     const loc = await screen.findByTestId("location");
     expect(loc.textContent).toBe("/users/alice");
   });
 
   it("redirects to /login when the authenticated user has no slug", async () => {
+    authStatusVar("AUTHENTICATED");
     backendUserObj({ id: "u-1" } as any);
     renderAt("/profile");
     const loc = await screen.findByTestId("location");
     expect(loc.textContent).toBe("/login");
   });
+
+  it("renders the loading display while auth status is LOADING (no premature /login flash)", () => {
+    authStatusVar("LOADING");
+    authInitCompleteVar(false);
+    renderAt("/profile");
+    expect(screen.getByText("Loading...")).toBeInTheDocument();
+    expect(screen.queryByTestId("location")).toBeNull();
+  });
+
+  it("renders the loading display while auth init is incomplete even after token resolves", () => {
+    authStatusVar("AUTHENTICATED");
+    authInitCompleteVar(false);
+    backendUserObj(null);
+    renderAt("/profile");
+    expect(screen.getByText("Loading...")).toBeInTheDocument();
+    expect(screen.queryByTestId("location")).toBeNull();
+  });
 });

frontend/src/graphql/cache.ts

@@ -23,6 +23,7 @@ import {
 } from "../types/graphql-api";
 import { ViewState } from "../components/types";
 import { FileUploadPackageProps } from "../components/widgets/modals/DocumentUploadModal";
+import type { GetUserOutput } from "./queries";
 
 export const mergeArrayByIdFieldPolicy: FieldPolicy<Reference[]> = {
   // eslint-disable-next-line @typescript-eslint/default-param-last
@@ -463,24 +464,11 @@ export const extractSearchTerm = makeVar<string>("");
  *   openedUser - Profile snapshot resolved by CentralRouteManager Phase 1.
  *   Set by: CentralRouteManager only.
  *
- * The shape mirrors the GET_USER GraphQL response. Components reading this
- * should treat null as "no user resolved yet" (loading or missing).
+ * Derived from the GET_USER query output so the shape automatically tracks
+ * any schema changes — adding a manual interface here would silently drift
+ * the moment a new field appears on UserType.
  */
-export interface OpenedUserProfile {
-  id: string;
-  username: string;
-  slug: string;
-  name: string;
-  firstName: string;
-  lastName: string;
-  email: string;
-  isProfilePublic: boolean;
-  reputationGlobal: number;
-  totalMessages: number;
-  totalThreadsCreated: number;
-  totalAnnotationsCreated: number;
-  totalDocumentsUploaded: number;
-}
+export type OpenedUserProfile = NonNullable<GetUserOutput["userBySlug"]>;
 export const openedUser = makeVar<OpenedUserProfile | null>(null);
 
 /**

frontend/src/routing/CentralRouteManager.tsx

@@ -23,7 +23,6 @@ import {
   openedThread,
   openedLabelset,
   openedUser,
-  OpenedUserProfile,
   selectedAnnotationIds,
   selectedAnalysesIds,
   selectedExtractIds,
@@ -324,16 +323,26 @@ export function CentralRouteManager() {
       }
       routeError(null);
 
-      // Type assertion: route.type is guaranteed to be "document" | "corpus" | "extract" | "thread" | "labelset" here
-      // because "browse" and "unknown" are handled by early return above
+      // route.type is guaranteed to be a resolvable entity here ("browse" and
+      // "unknown" returned above). userSlug must be passed explicitly so
+      // /users/:slug routes get a per-slug key — without it, every user
+      // resolution would dedupe to the same key and rapid profile switches
+      // would silently drop all but the first request.
       const requestKey = buildRequestKey(
-        route.type as "document" | "corpus" | "extract" | "thread" | "labelset",
+        route.type as
+          | "document"
+          | "corpus"
+          | "extract"
+          | "thread"
+          | "labelset"
+          | "user",
         route.userIdent,
         route.corpusIdent,
         route.documentIdent,
         route.extractIdent,
         route.threadIdent,
-        route.labelsetIdent
+        route.labelsetIdent,
+        route.userSlug
       );
 
       // Prevent duplicate simultaneous requests
@@ -815,7 +824,7 @@ export function CentralRouteManager() {
             }
 
             if (!error && data?.userBySlug) {
-              const user = data.userBySlug as OpenedUserProfile;
+              const user = data.userBySlug;
 
               routingLogger.debug(
                 "[RouteManager] ✅ Resolved user via slug:",
@@ -833,6 +842,18 @@ export function CentralRouteManager() {
             }
 
             console.warn("[RouteManager] User not found");
+            // Unlike the corpus/document/extract not-found paths (which
+            // redirect to /404 and clear via the browse-route handler), the
+            // user error path stays on the URL so the visitor can fix the
+            // slug. That makes clearing residual entity state our
+            // responsibility — otherwise consumers of openedCorpus etc.
+            // could render stale entity UI alongside the error display.
+            openedUser(null);
+            openedCorpus(null);
+            openedDocument(null);
+            openedExtract(null);
+            openedThread(null);
+            openedLabelset(null);
             routeError(new Error(`User "${route.userSlug}" not found`));
             routeLoading(false);
             return;

frontend/src/utils/navigationUtils.ts

@@ -535,13 +535,14 @@ export const requestTracker = new RequestTracker();
  * Build a unique key for request deduplication
  */
 export function buildRequestKey(
-  type: "corpus" | "document" | "extract" | "thread" | "labelset",
+  type: "corpus" | "document" | "extract" | "thread" | "labelset" | "user",
   userIdent?: string,
   corpusIdent?: string,
   documentIdent?: string,
   extractIdent?: string,
   threadIdent?: string,
-  labelsetIdent?: string
+  labelsetIdent?: string,
+  userSlug?: string
 ): string {
   const parts = [
     type,
@@ -551,6 +552,7 @@ export function buildRequestKey(
     extractIdent,
     threadIdent,
     labelsetIdent,
+    userSlug,
   ].filter(Boolean);
   return parts.join("-");
 }