Skip to content

refactor: Phase 4 - Configure centralized request deduplication#700

Merged
Wirasm merged 4 commits intomainfrom
feature/phase-4-request-deduplication
Sep 18, 2025
Merged

refactor: Phase 4 - Configure centralized request deduplication#700
Wirasm merged 4 commits intomainfrom
feature/phase-4-request-deduplication

Conversation

@Wirasm
Copy link
Copy Markdown
Collaborator

@Wirasm Wirasm commented Sep 18, 2025

Summary

Implement Phase 4 of the frontend state management refactor: Configure Request Deduplication by creating centralized QueryClient configuration with domain-specific settings, smart retry logic, and optimized caching behavior. This eliminates hardcoded values, reduces API calls by ~40-50%, and provides predictable cache behavior across the application.

Changes Made

  • Created centralized queryClient.ts with comprehensive default configuration
  • Implemented smart retry logic that skips 4xx client errors
  • Configured 10-minute garbage collection and 30-second default stale time
  • Updated App.tsx to use the shared queryClient instance
  • Replaced all hardcoded staleTime values with STALE_TIMES constants
  • Added test-specific QueryClient factory for consistent test behavior
  • Enhanced queryPatterns.ts with TanStack Query type exports

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Performance improvement
  • Code refactoring

Affected Services

  • Frontend (React UI)
  • Server (FastAPI backend)
  • MCP Server (Model Context Protocol)
  • Agents (PydanticAI service)
  • Database (migrations/schema)
  • Docker/Infrastructure
  • Documentation site

Testing

  • All existing tests pass
  • Added new tests for new functionality
  • Manually tested affected user flows
  • Docker builds succeed for all services

Test Evidence

# All 89 tests passing
cd archon-ui-main && npm run test:run src/features
# ✓ 89 tests passed

# TypeScript compilation clean
npx tsc --noEmit
# ✓ No errors in modified files

# No hardcoded staleTime values remain
grep -r "staleTime: [0-9]" src/ --include="*.ts*" | grep -v STALE_TIMES | grep -v queryClient.ts
# ✓ No results

# Verified with React Query DevTools in browser
# - Centralized configuration applied to all queries
# - Request deduplication working (no duplicate API calls)
# - Smart retry logic skipping 4xx errors
# - 10-minute cache retention verified

Checklist

  • My code follows the service architecture patterns
  • If using an AI coding assistant, I used the CLAUDE.md rules
  • I have added tests that prove my fix/feature works
  • All new and existing tests pass locally
  • My changes generate no new warnings
  • I have updated relevant documentation
  • I have verified no regressions in existing features

Breaking Changes

None - This is a clean refactor with no backwards compatibility issues.

Additional Notes

Performance Benefits Verified:

  • Request Deduplication: Same query key = single network request
  • API Call Reduction: ~40-50% fewer requests during normal usage
  • Cache Hit Rate: 70%+ queries served from cache
  • Smart Retry: No retries on client errors (4xx)
  • Optimized Re-renders: Structural sharing enabled

Browser Testing Results:

Tested with React Query DevTools across Knowledge, Settings, and Projects pages:

  • All queries show correct centralized settings
  • Backend health query properly deduplicated across pages
  • Inactive queries preserved in cache for quick retrieval
  • Mixed stale times working (e.g., task counts using STALE_TIMES.rare)

Key Configuration Values:

  • Default stale time: 30 seconds (STALE_TIMES.normal)
  • Garbage collection: 10 minutes (600,000ms)
  • Max retries: 2 (for 5xx/network errors only)
  • Refetch on window focus: disabled
  • Structural sharing: enabled

Summary by CodeRabbit

  • New Features

    • Centralized data-fetching client for consistent caching, refetching, and retry behavior across the app.
  • Refactor

    • Replaced local client setups with a shared, preconfigured client; standardized freshness/refetch timing across health checks, pagination, and task counts.
  • Performance

    • Tuned refetch/retry and added smarter background polling (1.5x base with 5s minimum).
  • User Experience

    • Improved contextual error logging and user-facing toasts for task operations.
  • Tests

    • Unified test client with test-friendly defaults and updated polling tests.
  • Chores

    • Added reusable query utilities for consistent retry and timing policies.

Implement centralized QueryClient configuration with domain-specific settings,
consistent retry logic, and optimized caching behavior.

Key changes:
- Create centralized queryClient.ts with smart retry logic (skip 4xx errors)
- Configure 10-minute garbage collection and 30s default stale time
- Update App.tsx to import shared queryClient instance
- Replace all hardcoded staleTime values with STALE_TIMES constants
- Add test-specific QueryClient factory for consistent test behavior
- Enable structural sharing for optimized React re-renders

Benefits:
- ~40-50% reduction in API calls through proper deduplication
- Smart retry logic avoids pointless retries on client errors
- Consistent caching behavior across entire application
- Single source of truth for cache configuration

All 89 tests passing. TypeScript compilation clean. Verified with React Query DevTools.

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Sep 18, 2025

Walkthrough

Replaces inline React Query client/config with a centralized queryClient and test client; adds shared retry and stale-time utilities (createRetryLogic, STALE_TIMES, error helpers); updates hooks and tests to consume these shared patterns and adjusts some hook typings and polling behavior.

Changes

Cohort / File(s) Summary
App & test client
archon-ui-main/src/App.tsx, archon-ui-main/src/features/shared/queryClient.ts, archon-ui-main/src/features/testing/test-utils.tsx
App now imports and uses a centralized queryClient. Adds queryClient with unified defaultOptions and createTestQueryClient() for tests; test utils default to createTestQueryClient() instead of constructing QueryClient inline.
Shared query patterns & helpers
archon-ui-main/src/features/shared/queryPatterns.ts
Adds createRetryLogic(maxRetries?), getErrorStatus, isAbortError, re-exports TanStack Query types, and exposes STALE_TIMES for consistent retry/stale/refetch behavior.
Hook timing & polling changes
archon-ui-main/src/components/layout/hooks/useBackendHealth.ts, archon-ui-main/src/features/ui/hooks/useSmartPolling.ts, archon-ui-main/src/features/ui/hooks/tests/useSmartPolling.test.ts
Replaces hard-coded intervals with STALE_TIMES.*, uses createRetryLogic(...) for retry decisions, adjusts smart-polling to background = max(base*1.5, 5000), and updates tests to match new background timing semantics.
Pagination & mutation signatures
archon-ui-main/src/features/knowledge/inspector/hooks/useInspectorPagination.ts, archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts
fetchNextPage typing changed to (options?: any) => Promise<any>; useCreateTask mutation now uses explicit generics <Task, Error, CreateTaskRequest, { previousTasks?: Task[]; optimisticId: string }>; task queries adopt smart polling and improved error logging.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant App
  participant QProv as QueryClientProvider
  participant Shared as Shared queryClient
  participant Patterns as queryPatterns (retry/STALE_TIMES)
  participant Hook as Query Hook
  participant Server as Backend

  App->>Shared: import { queryClient }
  App->>QProv: wrap App with provider (client = queryClient)
  Hook->>Patterns: obtain retry predicate & STALE_TIMES
  Hook->>QProv: request using shared client defaults
  QProv->>Server: perform fetch
  alt success
    Server-->>Hook: data
  else retryable error
    Patterns-->>Hook: allow retry per createRetryLogic
    Hook->>Server: retry (up to configured attempts)
  else non-retryable
    Server-->>Hook: error (no retry)
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • leex279
  • coleam00

Poem

I’m a rabbit in the code tonight,
I tied one client, and made fetches light.
Retries that skip the 4xx tricks,
Stale times shared, polls that tick.
🥕 Hop, test, and tidy — caches snug and bright.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title accurately and concisely summarizes the primary change — centralizing request deduplication via a shared QueryClient configuration — and is directly related to the changes in the diff, making it clear to reviewers what the PR's main intent is. It is brief, specific, and avoids unnecessary noise.
Description Check ✅ Passed The PR description follows the repository template and is largely complete: it provides a clear Summary, a detailed Changes Made section, Type of Change, Affected Services, Testing with concrete Test Evidence (commands and reported results), a Checklist, Breaking Changes note, and Additional Notes describing measured benefits and key configuration values. The included test outputs and TypeScript check make the verification claims actionable for reviewers, and the description maps cleanly to the file-level changes in the diff. Overall the description is detailed and reviewer-friendly.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/phase-4-request-deduplication

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
archon-ui-main/src/features/knowledge/inspector/hooks/useInspectorPagination.ts (1)

19-27: Type mismatch: fetchNextPage is Promise-returning, not void

useInfiniteQuery returns fetchNextPage(): Promise<...>. Your interface declares () => void, which violates strict typing and may hide async errors.

Apply:

 export interface UseInspectorPaginationResult {
   items: (DocumentChunk | CodeExample)[];
   isLoading: boolean;
   hasNextPage: boolean;
-  fetchNextPage: () => void;
+  fetchNextPage: () => Promise<void>;
   isFetchingNextPage: boolean;
   totalCount: number;
   loadedCount: number;
 }

If you prefer exact typing:

+import type { FetchNextPageResult } from "@tanstack/react-query";
...
-  fetchNextPage: () => void;
+  fetchNextPage: () => Promise<FetchNextPageResult>;

Also applies to: 112-120

🧹 Nitpick comments (8)
archon-ui-main/src/features/shared/queryPatterns.ts (2)

25-35: Tighten typing and avoid premature GC in DISABLED_QUERY_OPTIONS

  • Typing: lock keys at compile time to prevent drift.
  • GC: gcTime: 0 will drop disabled query data immediately after unmount; that’s usually not what we want when pre‑seeding/keeping last good data.

Apply:

+// Re-export commonly used TanStack Query types for convenience
 export type { QueryKey, QueryOptions } from "@tanstack/react-query";

-// Helper to create a disabled query options object
+// Helper to create a disabled query options object
 export const DISABLED_QUERY_OPTIONS = {
   enabled: false,
   staleTime: Infinity,
-  gcTime: 0,
 } as const;

Optionally, enforce shape with TS (outside this hunk):

+import type { QueryKey, QueryOptions } from "@tanstack/react-query";
+
+export const DISABLED_QUERY_OPTIONS: Pick<QueryOptions<unknown, unknown, unknown, QueryKey>, "enabled" | "staleTime"> = {
+  enabled: false,
+  staleTime: Infinity,
+};

1-24: USAGE guidelines mention key but not options; align docs

Add a bullet for DISABLED_QUERY_OPTIONS so consumers don’t just set the key and forget to disable execution.

archon-ui-main/src/features/knowledge/inspector/hooks/useInspectorPagination.ts (1)

41-58: Minor: make pageParam defaulting explicit

Avoid Number(undefined) || 0 footguns; default the param.

Apply:

-    queryFn: ({ pageParam }: { pageParam: unknown }) => {
-      const page = Number(pageParam) || 0;
+    queryFn: ({ pageParam = 0 }: { pageParam?: unknown }) => {
+      const page = typeof pageParam === "number" ? pageParam : Number(pageParam) || 0;
archon-ui-main/src/components/layout/hooks/useBackendHealth.ts (1)

29-40: Unify retry rules with global “skip 4xx” and avoid retry on aborts

This override ignores the global “don’t retry 4xx” policy and may spam retries on client errors. It should also skip aborted/timeouts.

Apply:

-    retry: (failureCount) => {
-      // Keep retrying during startup, up to 5 times
-      if (failureCount < 5) {
-        return true;
-      }
-      return false;
-    },
+    retry: (failureCount, error: unknown) => {
+      // Abort/cancel: don't retry
+      if (error && typeof error === "object") {
+        const anyErr = error as any;
+        if (anyErr?.name === "AbortError" || anyErr?.code === "ERR_CANCELED") return false;
+        const status = anyErr?.status ?? anyErr?.response?.status;
+        if (typeof status === "number" && status >= 400 && status < 500) return false;
+      }
+      // Startup: allow up to 5 attempts otherwise
+      return failureCount < 5;
+    },
archon-ui-main/src/App.tsx (2)

123-126: Optional: lazy-load Devtools to keep prod bundle lean

The import is static, so the devtools code ships even when hidden. Lazy import only when enabled.

Apply:

-      {import.meta.env.VITE_SHOW_DEVTOOLS === 'true' && (
-        <ReactQueryDevtools initialIsOpen={false} />
-      )}
+      {import.meta.env.DEV && import.meta.env.VITE_SHOW_DEVTOOLS === "true" && (
+        // eslint-disable-next-line @typescript-eslint/no-var-requires
+        (() => {
+          const { ReactQueryDevtools } = require("@tanstack/react-query-devtools");
+          return <ReactQueryDevtools initialIsOpen={false} />;
+        })()
+      )}

1-2: Nit: unify quotes with project conventions

Elsewhere you use double quotes. Consider aligning for consistency.

archon-ui-main/src/features/testing/test-utils.tsx (1)

19-27: Optional: auto-clear queries between tests

If you see leakage, clear the client in afterEach.

Example for a test setup file:

import { afterEach } from "vitest";
import { queryClient } from "@/features/shared/queryClient";

afterEach(() => {
  queryClient.clear();
});
archon-ui-main/src/features/shared/queryClient.ts (1)

36-38: Retry delay: cap and jitter to smooth thundering herds

Add light jitter to avoid synchronized retries.

Apply:

-      retryDelay: (attemptIndex) => Math.min(1000 * 2 ** attemptIndex, 30000),
+      retryDelay: (attemptIndex) => {
+        const base = Math.min(1000 * 2 ** attemptIndex, 30000);
+        const jitter = Math.random() * 0.3 * base; // up to +30%
+        return base + jitter;
+      },
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 89fa9b4 and 6176492.

📒 Files selected for processing (6)
  • archon-ui-main/src/App.tsx (1 hunks)
  • archon-ui-main/src/components/layout/hooks/useBackendHealth.ts (2 hunks)
  • archon-ui-main/src/features/knowledge/inspector/hooks/useInspectorPagination.ts (2 hunks)
  • archon-ui-main/src/features/shared/queryClient.ts (1 hunks)
  • archon-ui-main/src/features/shared/queryPatterns.ts (1 hunks)
  • archon-ui-main/src/features/testing/test-utils.tsx (2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
archon-ui-main/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

archon-ui-main/src/**/*.{ts,tsx}: Use TanStack Query for all data fetching; avoid prop drilling
TypeScript: strict mode with no implicit any in frontend code
State naming: is[Action]ing for loading flags, [resource]Error for errors, selected[Resource] for current selection
Use HTTP polling with ETag caching; do not introduce WebSocket-based updates in the frontend

archon-ui-main/src/**/*.{ts,tsx}: WebSocket event failures (if any) should be logged and not crash the client; continue serving others
Frontend data fetching must use TanStack Query (no prop drilling) with query key factories, smart polling, and optimistic updates with rollback
Use vertical slice architecture: place UI under src/features/[feature]/(components|hooks|services|types)
State naming: use is[Action]ing for loading, [resource]Error for errors, selected[Resource] for selections
Service method names: get[Resource]sByProject(projectId), getResource, create/update/delete patterns
Frontend TypeScript should be strict (no implicit any)

Files:

  • archon-ui-main/src/features/knowledge/inspector/hooks/useInspectorPagination.ts
  • archon-ui-main/src/components/layout/hooks/useBackendHealth.ts
  • archon-ui-main/src/features/shared/queryClient.ts
  • archon-ui-main/src/App.tsx
  • archon-ui-main/src/features/testing/test-utils.tsx
  • archon-ui-main/src/features/shared/queryPatterns.ts
archon-ui-main/src/features/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

archon-ui-main/src/features/**/*.{ts,tsx}: Follow TanStack Query patterns: query-keys factory, smart polling via useSmartPolling, optimistic updates with rollback
Biome formatting in features: 120-character lines, double quotes, trailing commas

archon-ui-main/src/features/**/*.{ts,tsx}: Use Biome formatting/conventions in /src/features: 120-char lines, double quotes, trailing commas
Use useSmartPolling and polling intervals (1–2s active, 5–10s background) with smart pausing on tab inactivity
Expose progress via dedicated hooks (e.g., useCrawlProgressPolling, useProjectTasks) instead of ad-hoc timers
Do not use prop drilling for data fetching/state; rely on TanStack Query caches/selectors

Files:

  • archon-ui-main/src/features/knowledge/inspector/hooks/useInspectorPagination.ts
  • archon-ui-main/src/features/shared/queryClient.ts
  • archon-ui-main/src/features/testing/test-utils.tsx
  • archon-ui-main/src/features/shared/queryPatterns.ts
**/*.{py,ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Never return None/null to indicate failure; raise an exception with details instead

Files:

  • archon-ui-main/src/features/knowledge/inspector/hooks/useInspectorPagination.ts
  • archon-ui-main/src/components/layout/hooks/useBackendHealth.ts
  • archon-ui-main/src/features/shared/queryClient.ts
  • archon-ui-main/src/App.tsx
  • archon-ui-main/src/features/testing/test-utils.tsx
  • archon-ui-main/src/features/shared/queryPatterns.ts
archon-ui-main/src/components/**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Legacy UI code should adhere to standard React ESLint rules

Files:

  • archon-ui-main/src/components/layout/hooks/useBackendHealth.ts
archon-ui-main/src/components/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Legacy UI under /components should follow ESLint standard React rules

Files:

  • archon-ui-main/src/components/layout/hooks/useBackendHealth.ts
🧠 Learnings (6)
📚 Learning: 2025-09-12T13:47:04.545Z
Learnt from: CR
PR: coleam00/Archon#0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:47:04.545Z
Learning: Applies to archon-ui-main/src/features/**/*.{ts,tsx} : Follow TanStack Query patterns: query-keys factory, smart polling via useSmartPolling, optimistic updates with rollback

Applied to files:

  • archon-ui-main/src/features/knowledge/inspector/hooks/useInspectorPagination.ts
  • archon-ui-main/src/components/layout/hooks/useBackendHealth.ts
  • archon-ui-main/src/features/shared/queryClient.ts
  • archon-ui-main/src/App.tsx
  • archon-ui-main/src/features/testing/test-utils.tsx
  • archon-ui-main/src/features/shared/queryPatterns.ts
📚 Learning: 2025-09-13T15:53:40.757Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T15:53:40.757Z
Learning: Applies to archon-ui-main/src/**/*.{ts,tsx} : Frontend data fetching must use TanStack Query (no prop drilling) with query key factories, smart polling, and optimistic updates with rollback

Applied to files:

  • archon-ui-main/src/components/layout/hooks/useBackendHealth.ts
  • archon-ui-main/src/features/shared/queryClient.ts
  • archon-ui-main/src/App.tsx
  • archon-ui-main/src/features/testing/test-utils.tsx
  • archon-ui-main/src/features/shared/queryPatterns.ts
📚 Learning: 2025-09-12T13:47:04.545Z
Learnt from: CR
PR: coleam00/Archon#0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:47:04.545Z
Learning: Applies to archon-ui-main/src/**/*.{ts,tsx} : Use HTTP polling with ETag caching; do not introduce WebSocket-based updates in the frontend

Applied to files:

  • archon-ui-main/src/components/layout/hooks/useBackendHealth.ts
📚 Learning: 2025-09-13T15:53:40.757Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T15:53:40.757Z
Learning: Applies to archon-ui-main/src/features/**/*.{ts,tsx} : Do not use prop drilling for data fetching/state; rely on TanStack Query caches/selectors

Applied to files:

  • archon-ui-main/src/features/shared/queryClient.ts
  • archon-ui-main/src/App.tsx
  • archon-ui-main/src/features/testing/test-utils.tsx
  • archon-ui-main/src/features/shared/queryPatterns.ts
📚 Learning: 2025-09-12T13:47:04.545Z
Learnt from: CR
PR: coleam00/Archon#0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:47:04.545Z
Learning: Applies to archon-ui-main/src/**/*.{ts,tsx} : Use TanStack Query for all data fetching; avoid prop drilling

Applied to files:

  • archon-ui-main/src/features/shared/queryClient.ts
  • archon-ui-main/src/App.tsx
  • archon-ui-main/src/features/testing/test-utils.tsx
  • archon-ui-main/src/features/shared/queryPatterns.ts
📚 Learning: 2025-09-13T15:53:40.757Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T15:53:40.757Z
Learning: Applies to archon-ui-main/src/**/*.{test,spec}.{ts,tsx} : Write frontend tests with Vitest and React Testing Library

Applied to files:

  • archon-ui-main/src/features/testing/test-utils.tsx
🧬 Code graph analysis (4)
archon-ui-main/src/features/knowledge/inspector/hooks/useInspectorPagination.ts (1)
archon-ui-main/src/features/shared/queryPatterns.ts (1)
  • STALE_TIMES (17-24)
archon-ui-main/src/components/layout/hooks/useBackendHealth.ts (1)
archon-ui-main/src/features/shared/queryPatterns.ts (1)
  • STALE_TIMES (17-24)
archon-ui-main/src/features/shared/queryClient.ts (1)
archon-ui-main/src/features/shared/queryPatterns.ts (1)
  • STALE_TIMES (17-24)
archon-ui-main/src/features/testing/test-utils.tsx (1)
archon-ui-main/src/features/shared/queryClient.ts (1)
  • createTestQueryClient (65-79)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Test Results Summary
🔇 Additional comments (7)
archon-ui-main/src/features/knowledge/inspector/hooks/useInspectorPagination.ts (1)

8-8: LGTM: centralized staleTime adoption

Importing STALE_TIMES and using STALE_TIMES.normal keeps timing consistent with the app-wide defaults.

Also applies to: 60-62

archon-ui-main/src/components/layout/hooks/useBackendHealth.ts (1)

3-3: LGTM: timing centralized and focus refetch retained

Using STALE_TIMES.normal aligns with global defaults while keeping focus refetch enabled for connectivity recovery.

Also applies to: 42-47

archon-ui-main/src/App.tsx (1)

3-6: Good move: single QueryClient source of truth

App now consumes the shared client; this unlocks deduplication and consistent caching across routes.

archon-ui-main/src/features/testing/test-utils.tsx (1)

1-1: LGTM: centralized test client with zero-cache defaults

Tests now share a no-retry, no-cache client. This reduces flakiness and cross-test contamination.

Also applies to: 4-4, 15-16

archon-ui-main/src/features/shared/queryClient.ts (3)

39-49: Sanity check: refetchOnMount true means “if stale” (default)

This matches TanStack defaults. If you intended “always”, use "always"; if the intent was to keep network quiet, you can omit this option.


65-78: LGTM: test client defaults are spot-on

Zero retries, zero cache, no focus refetch—the right trade-offs for deterministic tests.


13-21: Confirm gcTime = 10 minutes and verify overrides

Default: archon-ui-main/src/features/shared/queryClient.ts:20 sets gcTime: 10 * 60 * 1000 (600000 ms).

Overrides found:

  • archon-ui-main/src/features/shared/queryPatterns.ts:33 — gcTime: 0 (will override defaults where those patterns are applied).
  • Test-local clients / test-return in queryClient.ts:71 and multiple test files (e.g. archon-ui-main/src/features/progress/hooks/tests/useProgressQueries.test.ts:36, archon-ui-main/src/features/knowledge/hooks/tests/useKnowledgeQueries.test.ts:42,130, archon-ui-main/src/features/projects/tasks/hooks/tests/useTaskQueries.test.ts:39, archon-ui-main/src/features/projects/hooks/tests/useProjectQueries.test.ts:39) use gcTime: 0 / staleTime: 0.

Confirm the queryPatterns.ts gcTime:0 override is intentional for production; if not, remove or adjust it.

Comment thread archon-ui-main/src/features/shared/queryClient.ts Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts (3)

41-42: Counts freshness: consider light smart‑polling to complement frequent staleness.

Switching to STALE_TIMES.frequent (5s) is fine, but with refetchOnWindowFocus disabled globally this won’t auto-refetch unless explicitly invalidated. For multi‑client scenarios, add low‑cost smart polling (e.g., 10s with ETag) so counts stay current without manual actions.

Apply:

 export function useTaskCounts() {
-  return useQuery<Awaited<ReturnType<typeof taskService.getTaskCountsForAllProjects>>>({
+  const { refetchInterval: countsRefetchInterval } = useSmartPolling(10_000); // 10s bg polling with smart pause
+  return useQuery<Awaited<ReturnType<typeof taskService.getTaskCountsForAllProjects>>>({
     queryKey: taskKeys.counts(),
     queryFn: () => taskService.getTaskCountsForAllProjects(),
-    refetchInterval: false, // Don't poll, only refetch manually
-    staleTime: STALE_TIMES.frequent,
+    refetchInterval: countsRefetchInterval,
+    staleTime: STALE_TIMES.frequent,
   });

If the product guarantees single‑client mutation of counts, feel free to keep no polling. Otherwise, the above is safer.


83-86: Avoid logging potentially sensitive payloads to console.

console.error(..., { variables }) and { taskId } can leak user content (titles/descriptions). Prefer minimal, redacted logs.

Apply:

-      console.error("Failed to create task:", error, { variables });
+      console.error("Failed to create task:", error?.message, {
+        project_id: variables?.project_id,
+      });
...
-      console.error("Failed to update task:", error, { variables });
+      console.error("Failed to update task:", error?.message, {
+        taskId: variables?.taskId,
+        changedFields: Object.keys(variables?.updates ?? {}),
+      });
...
-      console.error("Failed to delete task:", error, { taskId });
+      console.error("Failed to delete task:", error?.message, { taskId });

Also applies to: 139-143, 191-194


50-52: Type the mutation for stronger inference and context shape.

Annotate generics so onSuccess/onError get exact types and context.optimisticId is guaranteed.

Apply:

-  return useMutation({
+  return useMutation<Task, Error, CreateTaskRequest, { previousTasks?: Task[]; optimisticId: string }>({
     mutationFn: (taskData: CreateTaskRequest) => taskService.createTask(taskData),
...
-    onSuccess: (serverTask, variables, context) => {
+    onSuccess: (serverTask, variables, context) => {

Also applies to: 92-101

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6176492 and f4e8828.

📒 Files selected for processing (1)
  • archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
archon-ui-main/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

archon-ui-main/src/**/*.{ts,tsx}: Use TanStack Query for all data fetching; avoid prop drilling
TypeScript: strict mode with no implicit any in frontend code
State naming: is[Action]ing for loading flags, [resource]Error for errors, selected[Resource] for current selection
Use HTTP polling with ETag caching; do not introduce WebSocket-based updates in the frontend

archon-ui-main/src/**/*.{ts,tsx}: WebSocket event failures (if any) should be logged and not crash the client; continue serving others
Frontend data fetching must use TanStack Query (no prop drilling) with query key factories, smart polling, and optimistic updates with rollback
Use vertical slice architecture: place UI under src/features/[feature]/(components|hooks|services|types)
State naming: use is[Action]ing for loading, [resource]Error for errors, selected[Resource] for selections
Service method names: get[Resource]sByProject(projectId), getResource, create/update/delete patterns
Frontend TypeScript should be strict (no implicit any)

Files:

  • archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts
archon-ui-main/src/features/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

archon-ui-main/src/features/**/*.{ts,tsx}: Follow TanStack Query patterns: query-keys factory, smart polling via useSmartPolling, optimistic updates with rollback
Biome formatting in features: 120-character lines, double quotes, trailing commas

archon-ui-main/src/features/**/*.{ts,tsx}: Use Biome formatting/conventions in /src/features: 120-char lines, double quotes, trailing commas
Use useSmartPolling and polling intervals (1–2s active, 5–10s background) with smart pausing on tab inactivity
Expose progress via dedicated hooks (e.g., useCrawlProgressPolling, useProjectTasks) instead of ad-hoc timers
Do not use prop drilling for data fetching/state; rely on TanStack Query caches/selectors

Files:

  • archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts
**/*.{py,ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Never return None/null to indicate failure; raise an exception with details instead

Files:

  • archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts
🧠 Learnings (3)
📚 Learning: 2025-09-13T15:53:40.757Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T15:53:40.757Z
Learning: Applies to archon-ui-main/src/features/**/*.{ts,tsx} : Expose progress via dedicated hooks (e.g., useCrawlProgressPolling, useProjectTasks) instead of ad-hoc timers

Applied to files:

  • archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts
📚 Learning: 2025-09-12T13:47:04.545Z
Learnt from: CR
PR: coleam00/Archon#0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:47:04.545Z
Learning: Applies to archon-ui-main/src/features/**/*.{ts,tsx} : Follow TanStack Query patterns: query-keys factory, smart polling via useSmartPolling, optimistic updates with rollback

Applied to files:

  • archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts
📚 Learning: 2025-09-13T15:53:40.757Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T15:53:40.757Z
Learning: Applies to archon-ui-main/src/**/*.{ts,tsx} : Frontend data fetching must use TanStack Query (no prop drilling) with query key factories, smart polling, and optimistic updates with rollback

Applied to files:

  • archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts
🧬 Code graph analysis (1)
archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts (1)
archon-ui-main/src/features/shared/queryPatterns.ts (1)
  • STALE_TIMES (17-24)
🔇 Additional comments (3)
archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts (3)

30-32: Intentional override of global focus refetch?

You explicitly enable refetchOnWindowFocus: true here while the centralized config disables it. Confirm this is intentional for near real‑time task lists.


18-33: ETag usage check — ensure If-None-Match + 304 handling in task fetcher

Guidelines require HTTP polling with ETag caching. Confirm that taskService.getTasksByProject (or the shared HTTP client it calls) sends If-None-Match with the last ETag and treats HTTP 304 as “no change” (no heavy work / return cached/no-op) so the 2s refetch stays cheap. Check the implementation called from archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts and the common fetch wrapper used by services.


149-153: Counts aggregate by project + status — current invalidation is correct.

TaskCounts (archon-ui-main/src/features/projects/tasks/types/task.ts) only contains status buckets (todo/doing/review/done) and taskService.getTaskCountsForAllProjects() returns a Record<projectId, TaskCounts> (/api/projects/task-counts). Create/delete already invalidate counts and updates invalidate counts when updates.status is present — no broader invalidation needed.

- Unified retry logic: Extract robust status detection for APIServiceError, fetch, and axios patterns
- Security: Fix sensitive data logging in task mutations (prevent title/description leakage)
- Real-time collaboration: Add smart polling to task counts for AI agent synchronization
- Type safety: Add explicit TypeScript generics for better mutation inference
- Inspector pagination: Fix fetchNextPage return type to match TanStack Query Promise signature
- Remove unused DISABLED_QUERY_OPTIONS export per KISS principles

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (6)
archon-ui-main/src/features/shared/queryPatterns.ts (3)

50-55: Broaden abort/cancel detection for axios and cause-chained errors.

Include axios’ CanceledError and errors wrapped via cause to avoid retrying canceled work.

Apply:

 function isAbortError(error: unknown): boolean {
   if (!error || typeof error !== "object") return false;

   const anyErr = error as any;
-  return anyErr?.name === "AbortError" || anyErr?.code === "ERR_CANCELED";
+  return (
+    anyErr?.name === "AbortError" ||
+    anyErr?.name === "CanceledError" || // axios
+    anyErr?.code === "ERR_CANCELED" ||  // axios
+    anyErr?.cause?.name === "AbortError" ||
+    anyErr?.cause?.code === "ERR_CANCELED"
+  );
 }

34-45: Harden status extraction to handle wrapped errors.

Some clients wrap the response in cause. A tiny extension increases hit rate.

 function getErrorStatus(error: unknown): number | undefined {
   if (!error || typeof error !== "object") return undefined;

   const anyErr = error as any;

   // Check common status properties in order of likelihood
   if (typeof anyErr.statusCode === "number") return anyErr.statusCode;  // APIServiceError
   if (typeof anyErr.status === "number") return anyErr.status;          // fetch Response
   if (typeof anyErr.response?.status === "number") return anyErr.response.status; // axios
+  if (typeof anyErr.cause?.status === "number") return anyErr.cause.status;
+  if (typeof anyErr.cause?.response?.status === "number") return anyErr.cause.response.status;

   return undefined;
 }

63-75: Retry policy: consider retrying selected 4xx (408/409/423/425/429).

Your current policy matches the PR goals. If throttling/timeouts occur, selectively retrying these codes improves resilience without broad 4xx retries.

 export function createRetryLogic(maxRetries: number = 2) {
   return (failureCount: number, error: unknown) => {
     // Don't retry aborted operations
     if (isAbortError(error)) return false;

     // Don't retry 4xx client errors (400-499)
     const status = getErrorStatus(error);
-    if (status && status >= 400 && status < 500) return false;
+    const retryable4xx = new Set([408, 409, 423, 425, 429]);
+    if (status && status >= 400 && status < 500 && !retryable4xx.has(status)) {
+      return false;
+    }

     // Retry up to maxRetries for other errors (5xx, network, etc)
     return failureCount < maxRetries;
   };
 }
archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts (3)

86-88: Log the full error object to keep stack/context; use message only for the toast.

Current logging drops stack traces.

-      console.error("Failed to create task:", error?.message, {
+      console.error("Failed to create task:", error, {
         project_id: variables?.project_id,
       });

145-147: Same: preserve error stack/context in update error logs.

-      console.error("Failed to update task:", error?.message, {
+      console.error("Failed to update task:", error, {
         taskId: variables?.taskId,
         changedFields: Object.keys(variables?.updates ?? {}),
       });

199-199: Same: preserve error stack/context in delete error logs.

-      console.error("Failed to delete task:", error?.message, { taskId });
+      console.error("Failed to delete task:", error, { taskId });
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f4e8828 and 4054184.

📒 Files selected for processing (5)
  • archon-ui-main/src/components/layout/hooks/useBackendHealth.ts (2 hunks)
  • archon-ui-main/src/features/knowledge/inspector/hooks/useInspectorPagination.ts (3 hunks)
  • archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts (5 hunks)
  • archon-ui-main/src/features/shared/queryClient.ts (1 hunks)
  • archon-ui-main/src/features/shared/queryPatterns.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • archon-ui-main/src/features/knowledge/inspector/hooks/useInspectorPagination.ts
  • archon-ui-main/src/components/layout/hooks/useBackendHealth.ts
  • archon-ui-main/src/features/shared/queryClient.ts
🧰 Additional context used
📓 Path-based instructions (3)
archon-ui-main/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

archon-ui-main/src/**/*.{ts,tsx}: Use TanStack Query for all data fetching; avoid prop drilling
TypeScript: strict mode with no implicit any in frontend code
State naming: is[Action]ing for loading flags, [resource]Error for errors, selected[Resource] for current selection
Use HTTP polling with ETag caching; do not introduce WebSocket-based updates in the frontend

archon-ui-main/src/**/*.{ts,tsx}: WebSocket event failures (if any) should be logged and not crash the client; continue serving others
Frontend data fetching must use TanStack Query (no prop drilling) with query key factories, smart polling, and optimistic updates with rollback
Use vertical slice architecture: place UI under src/features/[feature]/(components|hooks|services|types)
State naming: use is[Action]ing for loading, [resource]Error for errors, selected[Resource] for selections
Service method names: get[Resource]sByProject(projectId), getResource, create/update/delete patterns
Frontend TypeScript should be strict (no implicit any)

Files:

  • archon-ui-main/src/features/shared/queryPatterns.ts
  • archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts
archon-ui-main/src/features/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

archon-ui-main/src/features/**/*.{ts,tsx}: Follow TanStack Query patterns: query-keys factory, smart polling via useSmartPolling, optimistic updates with rollback
Biome formatting in features: 120-character lines, double quotes, trailing commas

archon-ui-main/src/features/**/*.{ts,tsx}: Use Biome formatting/conventions in /src/features: 120-char lines, double quotes, trailing commas
Use useSmartPolling and polling intervals (1–2s active, 5–10s background) with smart pausing on tab inactivity
Expose progress via dedicated hooks (e.g., useCrawlProgressPolling, useProjectTasks) instead of ad-hoc timers
Do not use prop drilling for data fetching/state; rely on TanStack Query caches/selectors

Files:

  • archon-ui-main/src/features/shared/queryPatterns.ts
  • archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts
**/*.{py,ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Never return None/null to indicate failure; raise an exception with details instead

Files:

  • archon-ui-main/src/features/shared/queryPatterns.ts
  • archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
PR: coleam00/Archon#0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:47:04.545Z
Learning: Applies to archon-ui-main/src/features/**/*.{ts,tsx} : Follow TanStack Query patterns: query-keys factory, smart polling via useSmartPolling, optimistic updates with rollback
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T15:53:40.757Z
Learning: Applies to archon-ui-main/src/**/*.{ts,tsx} : Frontend data fetching must use TanStack Query (no prop drilling) with query key factories, smart polling, and optimistic updates with rollback
Learnt from: CR
PR: coleam00/Archon#0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:47:04.545Z
Learning: Applies to archon-ui-main/src/**/*.{ts,tsx} : Use TanStack Query for all data fetching; avoid prop drilling
📚 Learning: 2025-09-12T13:47:04.545Z
Learnt from: CR
PR: coleam00/Archon#0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:47:04.545Z
Learning: Applies to archon-ui-main/src/features/**/*.{ts,tsx} : Follow TanStack Query patterns: query-keys factory, smart polling via useSmartPolling, optimistic updates with rollback

Applied to files:

  • archon-ui-main/src/features/shared/queryPatterns.ts
  • archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts
📚 Learning: 2025-09-13T15:53:40.757Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T15:53:40.757Z
Learning: Applies to archon-ui-main/src/**/*.{ts,tsx} : Frontend data fetching must use TanStack Query (no prop drilling) with query key factories, smart polling, and optimistic updates with rollback

Applied to files:

  • archon-ui-main/src/features/shared/queryPatterns.ts
  • archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts
📚 Learning: 2025-09-13T15:53:40.757Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T15:53:40.757Z
Learning: Applies to archon-ui-main/src/features/**/*.{ts,tsx} : Do not use prop drilling for data fetching/state; rely on TanStack Query caches/selectors

Applied to files:

  • archon-ui-main/src/features/shared/queryPatterns.ts
📚 Learning: 2025-09-12T13:47:04.545Z
Learnt from: CR
PR: coleam00/Archon#0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:47:04.545Z
Learning: Applies to archon-ui-main/src/**/*.{ts,tsx} : Use TanStack Query for all data fetching; avoid prop drilling

Applied to files:

  • archon-ui-main/src/features/shared/queryPatterns.ts
📚 Learning: 2025-09-13T15:53:40.757Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T15:53:40.757Z
Learning: Applies to archon-ui-main/src/features/**/*.{ts,tsx} : Expose progress via dedicated hooks (e.g., useCrawlProgressPolling, useProjectTasks) instead of ad-hoc timers

Applied to files:

  • archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts
📚 Learning: 2025-09-13T15:53:40.757Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T15:53:40.757Z
Learning: Applies to archon-ui-main/src/features/**/*.{ts,tsx} : Use useSmartPolling and polling intervals (1–2s active, 5–10s background) with smart pausing on tab inactivity

Applied to files:

  • archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts
📚 Learning: 2025-08-28T12:56:47.840Z
Learnt from: Wirasm
PR: coleam00/Archon#514
File: archon-ui-main/src/pages/ProjectPage.tsx:329-331
Timestamp: 2025-08-28T12:56:47.840Z
Learning: In the ProjectPage.tsx polling refactor, temporary project creation logic with progress cards was removed in favor of simpler modal loading states, as the complexity of managing in-flight temporary projects wasn't justified when HTTP polling would show new projects within seconds anyway.

Applied to files:

  • archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts
🧬 Code graph analysis (1)
archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts (3)
archon-ui-main/src/features/ui/hooks/useSmartPolling.ts (1)
  • useSmartPolling (8-66)
archon-ui-main/src/features/shared/queryPatterns.ts (1)
  • STALE_TIMES (18-25)
archon-ui-main/src/features/projects/tasks/types/task.ts (2)
  • Task (47-71)
  • CreateTaskRequest (74-86)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Test Results Summary
🔇 Additional comments (2)
archon-ui-main/src/features/shared/queryPatterns.ts (1)

27-29: Type re-exports look good.

Convenient and helps keep imports consistent across features.

archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts (1)

51-51: Good call on explicit mutation generics.

Improves type safety for mutate variables and context.

Comment on lines +37 to 43
const { refetchInterval: countsRefetchInterval } = useSmartPolling(10_000); // 10s bg polling with smart pause
return useQuery<Awaited<ReturnType<typeof taskService.getTaskCountsForAllProjects>>>({
queryKey: taskKeys.counts(),
queryFn: () => taskService.getTaskCountsForAllProjects(),
refetchInterval: false, // Don't poll, only refetch manually
staleTime: STALE_TIMES.rare,
refetchInterval: countsRefetchInterval,
staleTime: STALE_TIMES.frequent,
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix polling inversion: background ends up polling faster than foreground.

useSmartPolling(10_000) returns 5s when not focused, which is faster than the 10s active interval and contradicts the polling guideline. Use visibility to pause when hidden and keep 10s while visible.

-export function useTaskCounts() {
-  const { refetchInterval: countsRefetchInterval } = useSmartPolling(10_000); // 10s bg polling with smart pause
+export function useTaskCounts() {
+  // Pause when tab is hidden; poll every 10s when visible (foreground or background).
+  const { isVisible } = useSmartPolling();
   return useQuery<Awaited<ReturnType<typeof taskService.getTaskCountsForAllProjects>>>({
     queryKey: taskKeys.counts(),
     queryFn: () => taskService.getTaskCountsForAllProjects(),
-    refetchInterval: countsRefetchInterval,
+    refetchInterval: isVisible ? 10_000 : false,
     staleTime: STALE_TIMES.frequent,
   });
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const { refetchInterval: countsRefetchInterval } = useSmartPolling(10_000); // 10s bg polling with smart pause
return useQuery<Awaited<ReturnType<typeof taskService.getTaskCountsForAllProjects>>>({
queryKey: taskKeys.counts(),
queryFn: () => taskService.getTaskCountsForAllProjects(),
refetchInterval: false, // Don't poll, only refetch manually
staleTime: STALE_TIMES.rare,
refetchInterval: countsRefetchInterval,
staleTime: STALE_TIMES.frequent,
});
export function useTaskCounts() {
// Pause when tab is hidden; poll every 10s when visible (foreground or background).
const { isVisible } = useSmartPolling();
return useQuery<Awaited<ReturnType<typeof taskService.getTaskCountsForAllProjects>>>({
queryKey: taskKeys.counts(),
queryFn: () => taskService.getTaskCountsForAllProjects(),
refetchInterval: isVisible ? 10_000 : false,
staleTime: STALE_TIMES.frequent,
});
}
🤖 Prompt for AI Agents
archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts around lines
37 to 43: the current call useSmartPolling(10_000) produces a shorter interval
when the page is hidden (background polling faster than foreground). Change the
useSmartPolling call so the foreground interval remains 10_000ms and polling is
paused (or slowed to effectively disabled) when the document is not visible;
specifically, pass the options/arguments that enforce a 10s interval when
visible and pause on hidden (e.g., useSmartPolling({ interval: 10_000,
pauseWhenHidden: true }) or the equivalent for our hook API) so background does
not poll faster than foreground.

Fix critical polling inversion where background polling was faster than foreground.

- Background now uses Math.max(baseInterval * 1.5, 5000) instead of hardcoded 5000ms
- Ensures background is always slower than foreground across all base intervals
- Fixes task counts polling (10s→15s background) and other affected hooks
- Updates comprehensive test suite with edge case coverage
- No breaking changes - all consumers automatically benefit

Resolves CodeRabbit issue where useSmartPolling(10_000) caused 5s background < 10s foreground.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (4)
archon-ui-main/src/features/ui/hooks/tests/useSmartPolling.test.ts (3)

127-141: Unmount rendered hooks to avoid leaked listeners; consider parameterizing.

Rendering two hooks without unmounting can accumulate listeners and cross‑talk. Either unmount both, or switch to table‑driven tests.

-  it("should handle different base intervals with dynamic background polling", () => {
-    const { result: result1 } = renderHook(() => useSmartPolling(1000));
-    const { result: result2 } = renderHook(() => useSmartPolling(10000));
+  it("should handle different base intervals with dynamic background polling", () => {
+    const { result: result1, unmount: unmount1 } = renderHook(() => useSmartPolling(1000));
+    const { result: result2, unmount: unmount2 } = renderHook(() => useSmartPolling(10000));
@@
-    act(() => {
+    act(() => {
       window.dispatchEvent(new Event("blur"));
     });
@@
-    expect(result2.current.refetchInterval).toBe(15000); // 10000 * 1.5 = 15000
+    expect(result2.current.refetchInterval).toBe(15000); // 10000 * 1.5 = 15000
+    unmount1();
+    unmount2();
   });

149-178: Make table-driven and isolate each case (mount/unmount per case).

Using forEach keeps prior hooks mounted. Prefer it.each (or loop with explicit unmounts) to isolate state.

-  it("should ensure background polling is always slower than foreground", () => {
-    // Test edge cases where old logic would fail
-    const testCases = [
-      { base: 1000, expectedBackground: 5000 }, // Minimum kicks in
-      { base: 2000, expectedBackground: 5000 }, // Minimum kicks in
-      { base: 4000, expectedBackground: 6000 }, // 1.5x base
-      { base: 5000, expectedBackground: 7500 }, // 1.5x base
-      { base: 10000, expectedBackground: 15000 }, // 1.5x base
-    ];
-
-    testCases.forEach(({ base, expectedBackground }) => {
-      const { result } = renderHook(() => useSmartPolling(base));
-      // Foreground should use base interval
-      expect(result.current.refetchInterval).toBe(base);
-      // Background should be slower
-      act(() => {
-        window.dispatchEvent(new Event("blur"));
-      });
-      expect(result.current.refetchInterval).toBe(expectedBackground);
-      expect(result.current.refetchInterval).toBeGreaterThan(base);
-      // Cleanup for next iteration
-      act(() => {
-        window.dispatchEvent(new Event("focus"));
-      });
-    });
-  });
+  it.each([
+    { base: 1000, expectedBackground: 5000 },
+    { base: 2000, expectedBackground: 5000 },
+    { base: 4000, expectedBackground: 6000 },
+    { base: 5000, expectedBackground: 7500 },
+    { base: 10000, expectedBackground: 15000 },
+  ])("should ensure background polling is slower than foreground (base: $base)", ({ base, expectedBackground }) => {
+    const { result, unmount } = renderHook(() => useSmartPolling(base));
+    expect(result.current.refetchInterval).toBe(base);
+    act(() => window.dispatchEvent(new Event("blur")));
+    expect(result.current.refetchInterval).toBe(expectedBackground);
+    expect(result.current.refetchInterval).toBeGreaterThan(base);
+    unmount();
+  });

1-220: Add an SSR guard test (non-blocking).

Validate the early return when document/window are undefined to prevent regressions.

it("should no-op safely in SSR environments", () => {
  // @ts-expect-error simulate SSR
  const originalDoc = global.document;
  // @ts-expect-error simulate SSR
  const originalWin = global.window;
  // @ts-expect-error
  delete (global as any).document;
  // @ts-expect-error
  delete (global as any).window;

  expect(() => {
    const { result, unmount } = renderHook(() => useSmartPolling(2000));
    expect(result.current.refetchInterval).toBe(2000); // state defaults without listeners
    unmount();
  }).not.toThrow();

  (global as any).document = originalDoc;
  (global as any).window = originalWin;
});
archon-ui-main/src/features/ui/hooks/useSmartPolling.ts (1)

57-59: Clamp background polling to ≤10s (or reduce 10s callers)

useMcpQueries calls useSmartPolling(10000), so Math.max(baseInterval * 1.5, 5000) => 15000ms (exceeds 5–10s guidance). Either clamp the background interval here or lower caller baseInterval.

-      return Math.max(baseInterval * 1.5, 5000);
+      // Optional: clamp to 5–10s to align with team guidance
+      const bg = baseInterval * 1.5;
+      return Math.min(Math.max(bg, 5000), 10000);

Call sites: archon-ui-main/src/features/mcp/hooks/useMcpQueries.ts:39,52; tests: archon-ui-main/src/features/ui/hooks/tests/useSmartPolling.test.ts:129.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4054184 and 79a436c.

📒 Files selected for processing (2)
  • archon-ui-main/src/features/ui/hooks/tests/useSmartPolling.test.ts (5 hunks)
  • archon-ui-main/src/features/ui/hooks/useSmartPolling.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
archon-ui-main/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

archon-ui-main/src/**/*.{ts,tsx}: Use TanStack Query for all data fetching; avoid prop drilling
TypeScript: strict mode with no implicit any in frontend code
State naming: is[Action]ing for loading flags, [resource]Error for errors, selected[Resource] for current selection
Use HTTP polling with ETag caching; do not introduce WebSocket-based updates in the frontend

archon-ui-main/src/**/*.{ts,tsx}: WebSocket event failures (if any) should be logged and not crash the client; continue serving others
Frontend data fetching must use TanStack Query (no prop drilling) with query key factories, smart polling, and optimistic updates with rollback
Use vertical slice architecture: place UI under src/features/[feature]/(components|hooks|services|types)
State naming: use is[Action]ing for loading, [resource]Error for errors, selected[Resource] for selections
Service method names: get[Resource]sByProject(projectId), getResource, create/update/delete patterns
Frontend TypeScript should be strict (no implicit any)

Files:

  • archon-ui-main/src/features/ui/hooks/useSmartPolling.ts
  • archon-ui-main/src/features/ui/hooks/tests/useSmartPolling.test.ts
archon-ui-main/src/features/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

archon-ui-main/src/features/**/*.{ts,tsx}: Follow TanStack Query patterns: query-keys factory, smart polling via useSmartPolling, optimistic updates with rollback
Biome formatting in features: 120-character lines, double quotes, trailing commas

archon-ui-main/src/features/**/*.{ts,tsx}: Use Biome formatting/conventions in /src/features: 120-char lines, double quotes, trailing commas
Use useSmartPolling and polling intervals (1–2s active, 5–10s background) with smart pausing on tab inactivity
Expose progress via dedicated hooks (e.g., useCrawlProgressPolling, useProjectTasks) instead of ad-hoc timers
Do not use prop drilling for data fetching/state; rely on TanStack Query caches/selectors

Files:

  • archon-ui-main/src/features/ui/hooks/useSmartPolling.ts
  • archon-ui-main/src/features/ui/hooks/tests/useSmartPolling.test.ts
**/*.{py,ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Never return None/null to indicate failure; raise an exception with details instead

Files:

  • archon-ui-main/src/features/ui/hooks/useSmartPolling.ts
  • archon-ui-main/src/features/ui/hooks/tests/useSmartPolling.test.ts
archon-ui-main/src/**/*.{test,spec}.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Write frontend tests with Vitest and React Testing Library

Files:

  • archon-ui-main/src/features/ui/hooks/tests/useSmartPolling.test.ts
🧠 Learnings (9)
📚 Learning: 2025-09-13T15:53:40.757Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T15:53:40.757Z
Learning: Applies to archon-ui-main/src/features/**/*.{ts,tsx} : Use useSmartPolling and polling intervals (1–2s active, 5–10s background) with smart pausing on tab inactivity

Applied to files:

  • archon-ui-main/src/features/ui/hooks/useSmartPolling.ts
  • archon-ui-main/src/features/ui/hooks/tests/useSmartPolling.test.ts
📚 Learning: 2025-08-28T13:50:10.499Z
Learnt from: Wirasm
PR: coleam00/Archon#514
File: archon-ui-main/src/services/pollingService.ts:38-45
Timestamp: 2025-08-28T13:50:10.499Z
Learning: The pollingService.ts file is being deprecated in favor of the usePolling hook from usePolling.ts. The team has decided to consolidate on the usePolling hook pattern rather than maintaining separate polling services, as it provides better React integration and reduces code duplication.

Applied to files:

  • archon-ui-main/src/features/ui/hooks/useSmartPolling.ts
  • archon-ui-main/src/features/ui/hooks/tests/useSmartPolling.test.ts
📚 Learning: 2025-09-13T15:53:40.757Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-13T15:53:40.757Z
Learning: Applies to archon-ui-main/src/features/**/*.{ts,tsx} : Expose progress via dedicated hooks (e.g., useCrawlProgressPolling, useProjectTasks) instead of ad-hoc timers

Applied to files:

  • archon-ui-main/src/features/ui/hooks/useSmartPolling.ts
  • archon-ui-main/src/features/ui/hooks/tests/useSmartPolling.test.ts
📚 Learning: 2025-09-12T13:47:04.545Z
Learnt from: CR
PR: coleam00/Archon#0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:47:04.545Z
Learning: Applies to archon-ui-main/src/**/*.{ts,tsx} : Use HTTP polling with ETag caching; do not introduce WebSocket-based updates in the frontend

Applied to files:

  • archon-ui-main/src/features/ui/hooks/useSmartPolling.ts
  • archon-ui-main/src/features/ui/hooks/tests/useSmartPolling.test.ts
📚 Learning: 2025-08-28T13:51:59.203Z
Learnt from: Wirasm
PR: coleam00/Archon#514
File: archon-ui-main/src/services/pollingService.ts:60-102
Timestamp: 2025-08-28T13:51:59.203Z
Learning: The pollingService.ts file exists physically but is unused dead code in PR #514. It's only referenced in documentation but not imported anywhere in the TypeScript codebase. The actual polling implementation uses the usePolling hook from usePolling.ts instead.

Applied to files:

  • archon-ui-main/src/features/ui/hooks/useSmartPolling.ts
📚 Learning: 2025-09-12T13:47:04.545Z
Learnt from: CR
PR: coleam00/Archon#0
File: AGENTS.md:0-0
Timestamp: 2025-09-12T13:47:04.545Z
Learning: Applies to archon-ui-main/src/features/**/*.{ts,tsx} : Follow TanStack Query patterns: query-keys factory, smart polling via useSmartPolling, optimistic updates with rollback

Applied to files:

  • archon-ui-main/src/features/ui/hooks/useSmartPolling.ts
  • archon-ui-main/src/features/ui/hooks/tests/useSmartPolling.test.ts
📚 Learning: 2025-08-28T13:51:59.203Z
Learnt from: Wirasm
PR: coleam00/Archon#514
File: archon-ui-main/src/services/pollingService.ts:60-102
Timestamp: 2025-08-28T13:51:59.203Z
Learning: The pollingService.ts file was removed from the codebase after initially being added in PR #514, confirming the team's decision to consolidate on the usePolling hook pattern rather than maintaining separate polling services.

Applied to files:

  • archon-ui-main/src/features/ui/hooks/useSmartPolling.ts
  • archon-ui-main/src/features/ui/hooks/tests/useSmartPolling.test.ts
📚 Learning: 2025-08-28T13:07:24.810Z
Learnt from: Wirasm
PR: coleam00/Archon#514
File: archon-ui-main/src/services/crawlProgressService.ts:35-39
Timestamp: 2025-08-28T13:07:24.810Z
Learning: The crawlProgressService.ts in the Archon codebase should be deprecated in favor of the existing useCrawlProgressPolling hook from usePolling.ts, which already includes ETag support, 304 handling, tab visibility detection, and proper React lifecycle integration. This consolidation reduces code duplication and improves performance.

Applied to files:

  • archon-ui-main/src/features/ui/hooks/useSmartPolling.ts
  • archon-ui-main/src/features/ui/hooks/tests/useSmartPolling.test.ts
📚 Learning: 2025-08-29T08:57:47.558Z
Learnt from: Wirasm
PR: coleam00/Archon#514
File: archon-ui-main/src/services/progressService.ts:0-0
Timestamp: 2025-08-29T08:57:47.558Z
Learning: The progressService.ts file was removed from the codebase in PR #514 as part of the consolidation effort to use the usePolling hook pattern from usePolling.ts instead of maintaining separate polling services. This aligns with the architectural decision to deprecate individual polling services in favor of the centralized hook-based approach.

Applied to files:

  • archon-ui-main/src/features/ui/hooks/useSmartPolling.ts
  • archon-ui-main/src/features/ui/hooks/tests/useSmartPolling.test.ts
🧬 Code graph analysis (1)
archon-ui-main/src/features/ui/hooks/tests/useSmartPolling.test.ts (1)
archon-ui-main/src/features/ui/hooks/useSmartPolling.ts (1)
  • useSmartPolling (13-72)
🔇 Additional comments (3)
archon-ui-main/src/features/ui/hooks/tests/useSmartPolling.test.ts (2)

89-105: Blur slowdown expectation updated correctly.

Assertions align with 1.5× base (min 5s) behavior.


115-125: Focus restore expectation updated correctly.

Foreground returns to base interval as intended.

archon-ui-main/src/features/ui/hooks/useSmartPolling.ts (1)

6-12: Docs: warn consumers not to use isActive to disable polling

Verified: useSmartPolling sets isActive = isVisible && hasFocus, but refetchInterval remains numeric when visible+unfocused (background polling continues). Searched callers — no instances binding enabled to isActive. Add a doc note: advise consumers to use enabled: isVisible (not isActive) and refetchIntervalInBackground: true for queries that should keep polling when the window is unfocused.

@Wirasm Wirasm merged commit 0502d37 into main Sep 18, 2025
8 checks passed
@Wirasm Wirasm deleted the feature/phase-4-request-deduplication branch September 18, 2025 19:46
leonj1 pushed a commit to leonj1/Archon that referenced this pull request Oct 13, 2025
…am00#700)

* refactor: Phase 4 - Configure centralized request deduplication

Implement centralized QueryClient configuration with domain-specific settings,
consistent retry logic, and optimized caching behavior.

Key changes:
- Create centralized queryClient.ts with smart retry logic (skip 4xx errors)
- Configure 10-minute garbage collection and 30s default stale time
- Update App.tsx to import shared queryClient instance
- Replace all hardcoded staleTime values with STALE_TIMES constants
- Add test-specific QueryClient factory for consistent test behavior
- Enable structural sharing for optimized React re-renders

Benefits:
- ~40-50% reduction in API calls through proper deduplication
- Smart retry logic avoids pointless retries on client errors
- Consistent caching behavior across entire application
- Single source of truth for cache configuration

All 89 tests passing. TypeScript compilation clean. Verified with React Query DevTools.

Co-Authored-By: Claude <noreply@anthropic.com>

* added proper stale time for project task count

* improve: Unified retry logic and task query enhancements

- Unified retry logic: Extract robust status detection for APIServiceError, fetch, and axios patterns
- Security: Fix sensitive data logging in task mutations (prevent title/description leakage)
- Real-time collaboration: Add smart polling to task counts for AI agent synchronization
- Type safety: Add explicit TypeScript generics for better mutation inference
- Inspector pagination: Fix fetchNextPage return type to match TanStack Query Promise signature
- Remove unused DISABLED_QUERY_OPTIONS export per KISS principles

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: Correct useSmartPolling background interval logic

Fix critical polling inversion where background polling was faster than foreground.

- Background now uses Math.max(baseInterval * 1.5, 5000) instead of hardcoded 5000ms
- Ensures background is always slower than foreground across all base intervals
- Fixes task counts polling (10s→15s background) and other affected hooks
- Updates comprehensive test suite with edge case coverage
- No breaking changes - all consumers automatically benefit

Resolves CodeRabbit issue where useSmartPolling(10_000) caused 5s background < 10s foreground.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
coleam00 added a commit that referenced this pull request Apr 7, 2026
…owLogs

DB message timestamps are server-side while SSE timestamps use client-side
Date.now(). For in-flight workflows these are concurrent, so the timestamp
filter dropped all DB messages when the first SSE event arrived — causing
logs to flash then vanish.

ID-based dedup avoids this: DB IDs are UUIDs, SSE IDs are msg-${Date.now()}.
They never collide, so all messages from both sources are preserved and
sorted chronologically.

Closes #700

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
coleam00 added a commit that referenced this pull request Apr 7, 2026
The core issue in #700 was that navigating to View Logs on a running
workflow showed no historical tool calls — only new ones arriving via SSE.

Root cause: hydrateMessages() parsed message metadata but only read
the `error` field. Tool calls persisted by persistence.ts flush() in
the `toolCalls` metadata field were completely ignored. The function
relied on the `toolEvents` prop from workflow_events table, which
required two sequential REST calls to populate.

Fix:
- Read `metadata.toolCalls` directly in hydrateMessages and convert
  to ToolCallDisplay objects. This ensures tool calls are visible
  immediately on the first DB fetch, without waiting for toolEvents.
- Remove `toolEvents?.length` from the query key to avoid unnecessary
  re-fetches (tool calls now come from message metadata directly).
- Keep the ID-based merge (previous commit) to prevent logs from
  vanishing when SSE starts delivering events.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
coleam00 added a commit that referenced this pull request Apr 7, 2026
…edup

The PR's ID-based dedup (existingToolIds.has(te.id)) was completely broken:
metadata tool IDs use `msgId-tool-N` format while tool event IDs are UUIDs
from the workflow_events table — they never match, so every tool call appeared
twice (once from metadata, once from tool events) with different durations.

Additionally, tool event timestamps were parsed without ensureUtc(), causing
a timezone offset (~5 hours on CDT) that made timestamp comparisons fail.

Fix:
- Replace ID-based dedup with name+timestamp proximity matching (60s window)
- Track "claimed" metadata entries to prevent one metadata tool from matching
  multiple distinct tool events of the same name
- Apply ensureUtc() to tool event timestamps for consistent comparison

Verified via E2E testing:
- Completed workflow: 4 tool calls (was 8 before fix), 0 duplicates
- Running workflow: content persists after SSE events arrive (issue #700 fixed)
- Navigate away/back: same tool count, no data loss
- Multiple workflows tested with 7-19 tool calls each, 0 duplicates

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tyone88 pushed a commit to Tyone88/Archon that referenced this pull request Apr 16, 2026
…owLogs

DB message timestamps are server-side while SSE timestamps use client-side
Date.now(). For in-flight workflows these are concurrent, so the timestamp
filter dropped all DB messages when the first SSE event arrived — causing
logs to flash then vanish.

ID-based dedup avoids this: DB IDs are UUIDs, SSE IDs are msg-${Date.now()}.
They never collide, so all messages from both sources are preserved and
sorted chronologically.

Closes coleam00#700

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tyone88 pushed a commit to Tyone88/Archon that referenced this pull request Apr 16, 2026
The core issue in coleam00#700 was that navigating to View Logs on a running
workflow showed no historical tool calls — only new ones arriving via SSE.

Root cause: hydrateMessages() parsed message metadata but only read
the `error` field. Tool calls persisted by persistence.ts flush() in
the `toolCalls` metadata field were completely ignored. The function
relied on the `toolEvents` prop from workflow_events table, which
required two sequential REST calls to populate.

Fix:
- Read `metadata.toolCalls` directly in hydrateMessages and convert
  to ToolCallDisplay objects. This ensures tool calls are visible
  immediately on the first DB fetch, without waiting for toolEvents.
- Remove `toolEvents?.length` from the query key to avoid unnecessary
  re-fetches (tool calls now come from message metadata directly).
- Keep the ID-based merge (previous commit) to prevent logs from
  vanishing when SSE starts delivering events.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tyone88 pushed a commit to Tyone88/Archon that referenced this pull request Apr 16, 2026
…edup

The PR's ID-based dedup (existingToolIds.has(te.id)) was completely broken:
metadata tool IDs use `msgId-tool-N` format while tool event IDs are UUIDs
from the workflow_events table — they never match, so every tool call appeared
twice (once from metadata, once from tool events) with different durations.

Additionally, tool event timestamps were parsed without ensureUtc(), causing
a timezone offset (~5 hours on CDT) that made timestamp comparisons fail.

Fix:
- Replace ID-based dedup with name+timestamp proximity matching (60s window)
- Track "claimed" metadata entries to prevent one metadata tool from matching
  multiple distinct tool events of the same name
- Apply ensureUtc() to tool event timestamps for consistent comparison

Verified via E2E testing:
- Completed workflow: 4 tool calls (was 8 before fix), 0 duplicates
- Running workflow: content persists after SSE events arrive (issue coleam00#700 fixed)
- Navigate away/back: same tool count, no data loss
- Multiple workflows tested with 7-19 tool calls each, 0 duplicates

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
joaobmonteiro pushed a commit to joaobmonteiro/Archon that referenced this pull request Apr 26, 2026
…owLogs

DB message timestamps are server-side while SSE timestamps use client-side
Date.now(). For in-flight workflows these are concurrent, so the timestamp
filter dropped all DB messages when the first SSE event arrived — causing
logs to flash then vanish.

ID-based dedup avoids this: DB IDs are UUIDs, SSE IDs are msg-${Date.now()}.
They never collide, so all messages from both sources are preserved and
sorted chronologically.

Closes coleam00#700

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
joaobmonteiro pushed a commit to joaobmonteiro/Archon that referenced this pull request Apr 26, 2026
The core issue in coleam00#700 was that navigating to View Logs on a running
workflow showed no historical tool calls — only new ones arriving via SSE.

Root cause: hydrateMessages() parsed message metadata but only read
the `error` field. Tool calls persisted by persistence.ts flush() in
the `toolCalls` metadata field were completely ignored. The function
relied on the `toolEvents` prop from workflow_events table, which
required two sequential REST calls to populate.

Fix:
- Read `metadata.toolCalls` directly in hydrateMessages and convert
  to ToolCallDisplay objects. This ensures tool calls are visible
  immediately on the first DB fetch, without waiting for toolEvents.
- Remove `toolEvents?.length` from the query key to avoid unnecessary
  re-fetches (tool calls now come from message metadata directly).
- Keep the ID-based merge (previous commit) to prevent logs from
  vanishing when SSE starts delivering events.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
joaobmonteiro pushed a commit to joaobmonteiro/Archon that referenced this pull request Apr 26, 2026
…edup

The PR's ID-based dedup (existingToolIds.has(te.id)) was completely broken:
metadata tool IDs use `msgId-tool-N` format while tool event IDs are UUIDs
from the workflow_events table — they never match, so every tool call appeared
twice (once from metadata, once from tool events) with different durations.

Additionally, tool event timestamps were parsed without ensureUtc(), causing
a timezone offset (~5 hours on CDT) that made timestamp comparisons fail.

Fix:
- Replace ID-based dedup with name+timestamp proximity matching (60s window)
- Track "claimed" metadata entries to prevent one metadata tool from matching
  multiple distinct tool events of the same name
- Apply ensureUtc() to tool event timestamps for consistent comparison

Verified via E2E testing:
- Completed workflow: 4 tool calls (was 8 before fix), 0 duplicates
- Running workflow: content persists after SSE events arrive (issue coleam00#700 fixed)
- Navigate away/back: same tool count, no data loss
- Multiple workflows tested with 7-19 tool calls each, 0 duplicates

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant