Skip to content

Standardize abort signal propagation#699

Closed
Wirasm wants to merge 2 commits intomainfrom
feature/abort-signal-service-methods
Closed

Standardize abort signal propagation#699
Wirasm wants to merge 2 commits intomainfrom
feature/abort-signal-service-methods

Conversation

@Wirasm
Copy link
Copy Markdown
Collaborator

@Wirasm Wirasm commented Sep 18, 2025

Pull Request

Summary

Standardize abort-signal propagation across the frontend so TanStack Query cancellations no longer surface as API failures.

Changes Made

  • wire react-query signal objects through to all task/knowledge/project services and related hooks
  • treat browser AbortError as a benign cancellation inside callAPIWithETag
  • update task service unit tests to accept the new { signal } option and confirm mutations behave the same
  • adjust polling hooks to respect in-flight mutation state without logging noisy errors

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • Code refactoring
  • 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

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

cd archon-ui-main && npx vitest run src/features/projects/tasks/services/tests/taskService.test.ts
# passes after updating expectations for the new signal parameter

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.

Additional Notes

  • Console noise about "signal is aborted" is gone; requests cancel cleanly during rapid tab transitions.
  • Recommend a quick smoke test of task create/update flows to double-check optimistic updates still settle correctly.

Summary by CodeRabbit

  • New Features

    • Requests across knowledge, projects, tasks, and progress are now cancellable, improving responsiveness when navigating or changing filters.
    • Knowledge summaries support additional filters (type, search, tags) for more precise results.
  • Improvements

    • Smoother UX during task updates by pausing background refresh while changes are in flight, reducing flicker and conflicts.
    • More reliable file uploads with built‑in timeout handling.
    • Better performance for pagination and data fetching with smarter caching and reduced network churn.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Sep 18, 2025

Walkthrough

Threaded AbortSignal through React Query hooks and service methods across knowledge, progress, projects, and tasks features. Updated service signatures to accept optional signals and forwarded them to API calls, adding AbortError pass-through in apiWithEtag. Introduced mutation-aware polling gating in task queries and adjusted tests to reflect new call signatures.

Changes

Cohort / File(s) Summary
Knowledge Hooks
archon-ui-main/src/features/knowledge/hooks/useKnowledgeQueries.ts
useQuery queryFns now destructure { signal } and pass it to knowledgeService methods; no changes to keys/enabled/staleTime.
Knowledge Inspector Pagination
.../knowledge/inspector/hooks/useInspectorPagination.ts
useInfiniteQuery queryFn accepts { pageParam, signal } and forwards signal to service.
Knowledge Service
.../features/knowledge/services/knowledgeService.ts
Added optional AbortSignal to most methods; forwarded to callAPIWithETag/fetch; expanded getKnowledgeSummaries filters; upload uses signal and timeout fallback.
Progress Hooks
.../features/progress/hooks/useProgressQueries.ts
QueryFns accept { signal } and pass to progressService (getProgress, listActiveOperations); polling/error logic unchanged.
Progress Service
.../features/progress/services/progressService.ts
Added optional AbortSignal to getProgress and listActiveOperations; forwarded to API calls.
Progress Tests
.../features/progress/hooks/tests/useProgressQueries.test.ts
Expectations updated to assert passing AbortSignal to progressService.getProgress.
Projects Hooks
.../features/projects/hooks/useProjectQueries.ts
useProjects/useProjectFeatures queryFns accept { signal } and forward to projectService.
Project Documents Hook
.../projects/documents/hooks/useDocumentQueries.ts
QueryFn accepts { signal } and forwards to projectService.getProject; added staleTime normal.
Project Service
.../features/projects/services/projectService.ts
Added optional AbortSignal to all methods; forwarded to API; AbortError-specific handling and debug logs added.
Tasks Hooks
.../features/projects/tasks/hooks/useTaskQueries.ts
Propagate signal in queries/mutations; gate refetch while update mutation in flight via useIsMutating; minor optimistic update tweaks; added mutationKey for updates.
Task Service
.../features/projects/tasks/services/taskService.ts
Added optional AbortSignal to all methods; forwarded to API; AbortError handling and debug logs; propagate signal to nested calls.
Task Service Tests
.../features/projects/tasks/services/tests/taskService.test.ts
Relaxed API call assertions using expect.objectContaining; account for options with signal.
Shared API Wrapper
archon-ui-main/src/features/shared/apiWithEtag.ts
callAPIWithETag now rethrows AbortError instead of wrapping as NETWORK_ERROR.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor U as UI
  participant RQ as React Query Hook
  participant S as Service
  participant API as callAPIWithETag / fetch

  U->>RQ: Trigger query/mutation
  RQ->>RQ: Create AbortController (signal)
  RQ->>S: method(args, signal)
  S->>API: request(path, { signal, ...opts })
  alt request completes
    API-->>S: Response
    S-->>RQ: Data
    RQ-->>U: Resolve/update cache
  else request aborted
    API-->>S: AbortError
    S-->>RQ: Rethrow AbortError
    RQ-->>U: Cancellation handled
  end
Loading
sequenceDiagram
  autonumber
  actor U as UI (Tasks)
  participant RQ as React Query
  participant MH as Mutation (updateTask)
  participant Q as Query (useProjectTasks)

  U->>MH: updateTask(...)
  MH->>RQ: mutationKey ['updateTask', projectId]
  note over Q: While mutation in flight<br/>- refetchInterval paused<br/>- no refetch on focus
  U->>Q: Visibility/focus/refetch tick
  Q-->>U: Skipped due to in-flight mutation
  MH-->>RQ: Success/settle
  Q->>RQ: Resume normal refetch behavior
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • leex279
  • coleam00

Poem

In burrows of code I thump with delight,
AbortSignals flutter, requests grow light.
I pause mid-hop when mutations chew,
Then race ahead when fetches are through.
ETags twitch, tests nod in time—
Cancel, resume—what a rhythmic rhyme! 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title "Standardize abort signal propagation" is concise and accurately summarizes the primary change in the diff (threading AbortSignal through hooks and services to avoid treating React Query cancellations as API failures), without extraneous details or vague phrasing.
Description Check ✅ Passed The pull request description closely follows the repository template and includes a Summary, a detailed "Changes Made" section, Type of Change, Affected Services, Testing with a concrete command, Checklist, Breaking Changes, and Additional Notes, providing sufficient context and verification steps for reviewers.
Docstring Coverage ✅ Passed Docstring coverage is 94.74% 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/abort-signal-service-methods

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: 0

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/projects/tasks/hooks/tests/useTaskQueries.test.ts (1)

150-157: Update tests for TanStack Query v5 mutation signal

archon-ui-main/package.json shows @tanstack/react-query ^5.85.8 (v5). Tests currently assert the mutation's second arg is undefined but v5 passes an AbortSignal — update the assertions in archon-ui-main/src/features/projects/tasks/hooks/tests/useTaskQueries.test.ts (lines 150–157 and 192–197): replace undefined with expect.any(AbortSignal) (or expect.anything() to allow either).

🧹 Nitpick comments (21)
archon-ui-main/src/features/knowledge/hooks/useKnowledgeQueries.ts (4)

55-57: Prefer throwing Error objects over string rejections in queryFn.

Replace Promise.reject("No source ID") with Promise.reject(new Error("No source ID")) (or remove the else entirely since enabled/queryKey already guard execution) to avoid string-rejected promises and improve error typing.

Apply this diff:

-    queryFn: ({ signal }) =>
-      sourceId ? knowledgeService.getKnowledgeItem(sourceId, signal) : Promise.reject("No source ID"),
+    queryFn: ({ signal }) =>
+      sourceId ? knowledgeService.getKnowledgeItem(sourceId, signal) : Promise.reject(new Error("No source ID")),
-    queryFn: ({ signal }) =>
-      sourceId
-        ? knowledgeService.getKnowledgeItemChunks(
+    queryFn: ({ signal }) =>
+      sourceId
+        ? knowledgeService.getKnowledgeItemChunks(
           sourceId,
           {
             domainFilter: opts?.domain,
             limit: opts?.limit,
             offset: opts?.offset,
           },
           signal,
-          )
-        : Promise.reject("No source ID"),
+          )
+        : Promise.reject(new Error("No source ID")),
-    queryFn: ({ signal }) =>
-      sourceId ? knowledgeService.getCodeExamples(sourceId, undefined, signal) : Promise.reject("No source ID"),
+    queryFn: ({ signal }) =>
+      sourceId ? knowledgeService.getCodeExamples(sourceId, undefined, signal) : Promise.reject(new Error("No source ID")),
-    queryFn: ({ signal }) =>
+    queryFn: ({ signal }) =>
       sourceId
         ? knowledgeService.getKnowledgeItemChunks(
           sourceId,
           {
             limit: options?.limit,
             offset: options?.offset,
           },
           signal,
-          )
-        : Promise.reject("No source ID"),
+          )
+        : Promise.reject(new Error("No source ID")),
-    queryFn: ({ signal }) =>
+    queryFn: ({ signal }) =>
       sourceId
         ? knowledgeService.getCodeExamples(
           sourceId,
           {
             limit: options?.limit,
             offset: options?.offset,
           },
           signal,
-          )
-        : Promise.reject("No source ID"),
+          )
+        : Promise.reject(new Error("No source ID")),

Also applies to: 73-84, 96-98, 843-854, 870-881


69-71: Add explicit generics to useQuery for strict typing.

Explicit result types improve DX and catch mismatches at compile time.

Apply this diff:

@@
-import type {
-  CrawlRequest,
-  CrawlStartResponse,
-  KnowledgeItem,
-  KnowledgeItemsFilter,
-  KnowledgeItemsResponse,
-  UploadMetadata,
-} from "../types";
+import type {
+  CrawlRequest,
+  CrawlStartResponse,
+  KnowledgeItem,
+  KnowledgeItemsFilter,
+  KnowledgeItemsResponse,
+  UploadMetadata,
+  ChunksResponse,
+  CodeExamplesResponse,
+} from "../types";
@@
-  return useQuery({
+  return useQuery<ChunksResponse>({
@@
-  return useQuery({
+  return useQuery<CodeExamplesResponse>({
@@
-  return useQuery({
+  return useQuery<ChunksResponse>({
@@
-  return useQuery({
+  return useQuery<CodeExamplesResponse>({

Also applies to: 94-101, 839-857, 866-884


44-45: Normalize summaries filter in query key to avoid key churn.

Passing raw filter objects can generate distinct keys for semantically equivalent filters (e.g., missing vs undefined fields). Consider normalizing (remove undefined, stable order) before constructing the key.

Example:

-  summaries: (filter?: KnowledgeItemsFilter) => [...knowledgeKeys.all, "summaries", filter] as const,
+  summaries: (filter?: KnowledgeItemsFilter) =>
+    [...knowledgeKeys.all, "summaries", normalizeFilter(filter)] as const,

Add a small normalizeFilter that omits undefined and sorts array fields.


125-132: Revisit optimistic writes to lists() key (known TODO).

Since lists() isn’t actually queried, these optimistic updates/invalidation don’t affect UI and add cache churn. Either remove for now or gate behind a feature flag until Phase 3 revisits the lists() usage.

Also applies to: 174-179, 298-299

archon-ui-main/src/features/shared/apiWithEtag.ts (1)

96-102: Also treat 205 No Content to avoid empty-body JSON parse errors.

Some endpoints can return 205 with no body. Handling it like 204 prevents parse exceptions.

Apply this diff:

-    // Handle 204 No Content (DELETE operations)
-    if (response.status === 204) {
+    // Handle no-content responses
+    if (response.status === 204 || response.status === 205) {
       return undefined as T;
     }
archon-ui-main/src/features/projects/documents/hooks/useDocumentQueries.ts (1)

23-27: Type the query result and avoid returning raw [] in guarded queryFns.

  • Add generics: useQuery<ProjectDocument[]>() to keep strict typing.
  • Since enabled/queryKey already guard execution, prefer rejecting with Error instead of returning [] in the no-id path.

Apply this diff:

-  return useQuery({
+  return useQuery<ProjectDocument[]>({
     queryKey: projectId ? documentKeys.byProject(projectId) : DISABLED_QUERY_KEY,
-    queryFn: async ({ signal }) => {
-      if (!projectId) return [];
+    queryFn: async ({ signal }) => {
+      if (!projectId) return Promise.reject(new Error("No project ID"));
       const project = await projectService.getProject(projectId, signal);
       return (project.docs || []) as ProjectDocument[];
     },
archon-ui-main/src/features/projects/tasks/services/tests/taskService.test.ts (1)

269-272: Avoid asserting signal: undefined to reduce brittleness.

Some implementations may omit the property entirely when undefined. Recommend asserting an options object without pinning the signal field.

Apply this diff:

-      expect(callAPIWithETag).toHaveBeenCalledWith(
-        `/api/projects/${projectId}/tasks`,
-        expect.objectContaining({ signal: undefined })
-      );
+      expect(callAPIWithETag).toHaveBeenCalledWith(
+        `/api/projects/${projectId}/tasks`,
+        expect.any(Object)
+      );
archon-ui-main/src/features/progress/hooks/useProgressQueries.ts (3)

121-125: Prefer nullish-coalescing and include error_message fallback.

Use both error and error_message fields from ProgressResponse to avoid dropping server-provided details.

-      options?.onError?.(query.data.error || "Operation was cancelled");
+      options?.onError?.(query.data.error ?? query.data.error_message ?? "Operation was cancelled");
-      options?.onError?.(query.data.error || "Operation failed");
+      options?.onError?.(query.data.error ?? query.data.error_message ?? "Operation failed");

Also applies to: 137-140


99-119: Stabilize effects by narrowing deps to callbacks, not the options object.

Depending on options (object identity) can retrigger effects unexpectedly. Depend on the specific callbacks instead.

-  }, [query.data?.status, progressId, queryClient, options, query.data]);
+  }, [query.data?.status, progressId, queryClient, options?.onComplete, options?.onError, query.data]);

-  }, [query.error, progressId, queryClient, options]);
+  }, [query.error, progressId, queryClient, options?.onError]);

Would you like me to update all similar hooks to use callback-specific deps for consistency?

Also applies to: 151-156, 157-179


282-296: Let TS infer refetchInterval callback type.

The explicit parameter type is unnecessary and can drift from TanStack’s types.

-      refetchInterval: (query: { state: { data: ProgressResponse | null | undefined } }) => {
+      refetchInterval: (query) => {
archon-ui-main/src/features/knowledge/inspector/hooks/useInspectorPagination.ts (1)

44-53: Minor typing simplification for future-proofing.

You can rely on TanStack’s context typing instead of hard-typing the param, to avoid friction if the library’s types evolve.

-    queryFn: ({ pageParam, signal }: { pageParam: unknown; signal?: AbortSignal }) => {
+    queryFn: ({ pageParam, signal }) => {
archon-ui-main/src/features/projects/hooks/useProjectQueries.ts (1)

43-45: Throw an Error instead of Promise.reject string when projectId is missing.

Keeps error types consistent and stack traces intact.

-    queryFn: ({ signal }) =>
-      projectId ? projectService.getProjectFeatures(projectId, signal) : Promise.reject("No project ID"),
+    queryFn: ({ signal }) => {
+      if (!projectId) throw new Error("No project ID");
+      return projectService.getProjectFeatures(projectId, signal);
+    },
archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts (2)

60-62: Mutation context?.signal won’t be set on React Query v4.

In v4, mutationFn receives only the variables; the second context param (with signal) is a v5 feature. As written, context will always be undefined, so no signal reaches the service. If mutation cancellation is desired, either:

  • Upgrade to TanStack Query v5 and keep this shape, or
  • Drop the unused param and call services without a signal.

Suggested v4-safe simplification:

-    mutationFn: (taskData: CreateTaskRequest, context?: { signal?: AbortSignal }) =>
-      taskService.createTask(taskData, context?.signal),
+    mutationFn: (taskData: CreateTaskRequest) => taskService.createTask(taskData),
-    mutationFn: (
-      { taskId, updates }: { taskId: string; updates: UpdateTaskRequest },
-      context?: { signal?: AbortSignal },
-    ) => taskService.updateTask(taskId, updates, context?.signal),
+    mutationFn: ({ taskId, updates }: { taskId: string; updates: UpdateTaskRequest }) =>
+      taskService.updateTask(taskId, updates),
-    mutationFn: (taskId: string, context?: { signal?: AbortSignal }) => taskService.deleteTask(taskId, context?.signal),
+    mutationFn: (taskId: string) => taskService.deleteTask(taskId),

See version check script in the tests comment; if you are on v5, consider updating tests to expect a real AbortSignal for mutations.

Also applies to: 129-135, 186-186


37-40: Optional: also gate polling during create/delete.

To further reduce race conditions, consider pausing polling when create/delete mutations are in flight for the same project.

archon-ui-main/src/features/projects/services/projectService.ts (4)

31-37: Harden “updated” computation to avoid NaN and ensure consistency.

If updated_at is missing/invalid, formatRelativeTime can yield “NaN weeks ago.” Compute once with a safe fallback and reuse.

Apply this diff:

       // Add computed UI properties
       const processedProjects = projects.map((project: Project) => {
         // Process the raw pinned value
-
-        const processed = {
+        const updated =
+          project.updated ?? (project.updated_at ? formatRelativeTime(project.updated_at) : "just now");
+        const processed = {
           ...project,
           // Ensure pinned is properly handled as boolean
           pinned: project.pinned === true,
           progress: project.progress || 0,
-          updated: project.updated || formatRelativeTime(project.updated_at),
+          updated,
         };
         return processed;
       });

60-64: Normalize pinned and align “updated” fallback in getProject.

Return the same normalized shape as list/update to avoid UI divergence.

Apply this diff:

       return {
         ...project,
+        pinned: project.pinned === true,
         progress: project.progress || 0,
-        updated: project.updated || formatRelativeTime(project.updated_at),
+        updated: project.updated ?? (project.updated_at ? formatRelativeTime(project.updated_at) : "just now"),
       };

150-155: Unify “updated” handling in updateProject.

Use the same safe fallback as other methods.

       const processedProject = {
         ...project,
         pinned: project.pinned === true,
         progress: project.progress || 0,
-        updated: formatRelativeTime(project.updated_at),
+        updated: project.updated ?? (project.updated_at ? formatRelativeTime(project.updated_at) : "just now"),
       };

16-16: Rename listProjects → getProjects to match service naming convention.

Aligns with the get[Resource] pattern; update usages.

-  async listProjects(signal?: AbortSignal): Promise<Project[]> {
+  async getProjects(signal?: AbortSignal): Promise<Project[]> {

Update call site:

  • archon-ui-main/src/features/projects/hooks/useProjectQueries.ts:30 — change projectService.listProjects(signal) → projectService.getProjects(signal).
archon-ui-main/src/features/projects/tasks/services/taskService.ts (2)

23-26: DRY up AbortError handling (optional).

The same catch pattern repeats. Consider a tiny helper to reduce boilerplate.

Add once near the top of the file:

const rethrowIfAborted = (scope: string, err: unknown) => {
  if (err instanceof Error && err.name === "AbortError") {
    console.debug(`Request cancelled: ${scope}`);
    throw err;
  }
};

Then replace catches, e.g.:

-    } catch (error) {
-      if (error instanceof Error && error.name === "AbortError") {
-        console.debug(`Request cancelled: create task`);
-        throw error;
-      }
-      console.error("Failed to create task:", error);
+    } catch (error) {
+      rethrowIfAborted("create task", error);
+      console.error("Failed to create task:", error);
       throw error;
     }

Also applies to: 40-43, 71-75, 99-104, 134-137, 153-156, 184-187, 216-219


196-204: Use APIServiceError for unsupported operation to keep error typing consistent.

Today this throws a generic Error.

-import { formatZodErrors, ValidationError } from "../../../shared/errors";
+import { formatZodErrors, ValidationError, APIServiceError } from "../../../shared/errors";
@@
-      throw new Error("getTasksByStatus requires cross-project access. Use getTasksByProject instead.");
+      throw new APIServiceError(
+        "getTasksByStatus requires cross-project access. Use getTasksByProject instead.",
+        "UNSUPPORTED_OPERATION",
+        400,
+      );
archon-ui-main/src/features/knowledge/services/knowledgeService.ts (1)

31-33: Don’t skip zero values when building query params.

page/per_page get dropped if set to 0. Use explicit undefined checks.

-    if (filter?.page) params.append("page", filter.page.toString());
-    if (filter?.per_page) params.append("per_page", filter.per_page.toString());
+    if (filter?.page !== undefined) params.append("page", filter.page.toString());
+    if (filter?.per_page !== undefined) params.append("per_page", filter.per_page.toString());
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 31cf56a and d53012a.

📒 Files selected for processing (14)
  • archon-ui-main/src/features/knowledge/hooks/useKnowledgeQueries.ts (6 hunks)
  • archon-ui-main/src/features/knowledge/inspector/hooks/useInspectorPagination.ts (1 hunks)
  • archon-ui-main/src/features/knowledge/services/knowledgeService.ts (13 hunks)
  • archon-ui-main/src/features/progress/hooks/tests/useProgressQueries.test.ts (1 hunks)
  • archon-ui-main/src/features/progress/hooks/useProgressQueries.ts (3 hunks)
  • archon-ui-main/src/features/progress/services/progressService.ts (1 hunks)
  • archon-ui-main/src/features/projects/documents/hooks/useDocumentQueries.ts (1 hunks)
  • archon-ui-main/src/features/projects/hooks/useProjectQueries.ts (2 hunks)
  • archon-ui-main/src/features/projects/services/projectService.ts (10 hunks)
  • archon-ui-main/src/features/projects/tasks/hooks/tests/useTaskQueries.test.ts (3 hunks)
  • archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts (9 hunks)
  • archon-ui-main/src/features/projects/tasks/services/taskService.ts (12 hunks)
  • archon-ui-main/src/features/projects/tasks/services/tests/taskService.test.ts (6 hunks)
  • archon-ui-main/src/features/shared/apiWithEtag.ts (1 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/shared/apiWithEtag.ts
  • archon-ui-main/src/features/projects/tasks/services/tests/taskService.test.ts
  • archon-ui-main/src/features/progress/hooks/tests/useProgressQueries.test.ts
  • archon-ui-main/src/features/projects/hooks/useProjectQueries.ts
  • archon-ui-main/src/features/progress/hooks/useProgressQueries.ts
  • archon-ui-main/src/features/projects/tasks/services/taskService.ts
  • archon-ui-main/src/features/knowledge/inspector/hooks/useInspectorPagination.ts
  • archon-ui-main/src/features/projects/documents/hooks/useDocumentQueries.ts
  • archon-ui-main/src/features/progress/services/progressService.ts
  • archon-ui-main/src/features/projects/services/projectService.ts
  • archon-ui-main/src/features/knowledge/services/knowledgeService.ts
  • archon-ui-main/src/features/knowledge/hooks/useKnowledgeQueries.ts
  • archon-ui-main/src/features/projects/tasks/hooks/tests/useTaskQueries.test.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/apiWithEtag.ts
  • archon-ui-main/src/features/projects/tasks/services/tests/taskService.test.ts
  • archon-ui-main/src/features/progress/hooks/tests/useProgressQueries.test.ts
  • archon-ui-main/src/features/projects/hooks/useProjectQueries.ts
  • archon-ui-main/src/features/progress/hooks/useProgressQueries.ts
  • archon-ui-main/src/features/projects/tasks/services/taskService.ts
  • archon-ui-main/src/features/knowledge/inspector/hooks/useInspectorPagination.ts
  • archon-ui-main/src/features/projects/documents/hooks/useDocumentQueries.ts
  • archon-ui-main/src/features/progress/services/progressService.ts
  • archon-ui-main/src/features/projects/services/projectService.ts
  • archon-ui-main/src/features/knowledge/services/knowledgeService.ts
  • archon-ui-main/src/features/knowledge/hooks/useKnowledgeQueries.ts
  • archon-ui-main/src/features/projects/tasks/hooks/tests/useTaskQueries.test.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/apiWithEtag.ts
  • archon-ui-main/src/features/projects/tasks/services/tests/taskService.test.ts
  • archon-ui-main/src/features/progress/hooks/tests/useProgressQueries.test.ts
  • archon-ui-main/src/features/projects/hooks/useProjectQueries.ts
  • archon-ui-main/src/features/progress/hooks/useProgressQueries.ts
  • archon-ui-main/src/features/projects/tasks/services/taskService.ts
  • archon-ui-main/src/features/knowledge/inspector/hooks/useInspectorPagination.ts
  • archon-ui-main/src/features/projects/documents/hooks/useDocumentQueries.ts
  • archon-ui-main/src/features/progress/services/progressService.ts
  • archon-ui-main/src/features/projects/services/projectService.ts
  • archon-ui-main/src/features/knowledge/services/knowledgeService.ts
  • archon-ui-main/src/features/knowledge/hooks/useKnowledgeQueries.ts
  • archon-ui-main/src/features/projects/tasks/hooks/tests/useTaskQueries.test.ts
  • archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts
archon-ui-main/src/features/**/services/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

Frontend service method naming: get[Resource]sByProject(projectId), getResource, createResource, update[Resource](id, updates), deleteResource

Files:

  • archon-ui-main/src/features/projects/tasks/services/tests/taskService.test.ts
  • archon-ui-main/src/features/projects/tasks/services/taskService.ts
  • archon-ui-main/src/features/progress/services/progressService.ts
  • archon-ui-main/src/features/projects/services/projectService.ts
  • archon-ui-main/src/features/knowledge/services/knowledgeService.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/projects/tasks/services/tests/taskService.test.ts
  • archon-ui-main/src/features/progress/hooks/tests/useProgressQueries.test.ts
  • archon-ui-main/src/features/projects/tasks/hooks/tests/useTaskQueries.test.ts
🧠 Learnings (10)
📚 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} : Service method names: get[Resource]sByProject(projectId), get[Resource](id), create/update/delete patterns

Applied to files:

  • archon-ui-main/src/features/projects/tasks/services/tests/taskService.test.ts
  • archon-ui-main/src/features/projects/services/projectService.ts
  • archon-ui-main/src/features/projects/tasks/hooks/tests/useTaskQueries.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/projects/tasks/services/tests/taskService.test.ts
  • archon-ui-main/src/features/progress/hooks/tests/useProgressQueries.test.ts
  • archon-ui-main/src/features/projects/hooks/useProjectQueries.ts
  • archon-ui-main/src/features/progress/hooks/useProgressQueries.ts
  • archon-ui-main/src/features/projects/tasks/services/taskService.ts
  • archon-ui-main/src/features/progress/services/progressService.ts
  • archon-ui-main/src/features/projects/tasks/hooks/tests/useTaskQueries.test.ts
  • archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.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/progress/hooks/tests/useProgressQueries.test.ts
  • archon-ui-main/src/features/progress/hooks/useProgressQueries.ts
  • archon-ui-main/src/features/progress/services/progressService.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/progress/hooks/tests/useProgressQueries.test.ts
  • archon-ui-main/src/features/progress/hooks/useProgressQueries.ts
  • archon-ui-main/src/features/progress/services/progressService.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/hooks/useProjectQueries.ts
  • archon-ui-main/src/features/progress/hooks/useProgressQueries.ts
  • archon-ui-main/src/features/projects/documents/hooks/useDocumentQueries.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/progress/hooks/useProgressQueries.ts
  • 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/**/services/**/*.ts : Frontend service method naming: get[Resource]sByProject(projectId), get[Resource](id), create[Resource](data), update[Resource](id, updates), delete[Resource](id)

Applied to files:

  • archon-ui-main/src/features/projects/services/projectService.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/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/**/*.{ts,tsx} : Use TanStack Query for all data fetching; avoid prop drilling

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 (13)
archon-ui-main/src/features/projects/tasks/services/tests/taskService.test.ts (1)
archon-ui-main/src/features/shared/apiWithEtag.ts (1)
  • callAPIWithETag (43-124)
archon-ui-main/src/features/progress/hooks/tests/useProgressQueries.test.ts (1)
archon-ui-main/src/features/progress/services/progressService.ts (1)
  • progressService (9-24)
archon-ui-main/src/features/projects/hooks/useProjectQueries.ts (1)
archon-ui-main/src/features/projects/services/projectService.ts (1)
  • projectService (12-211)
archon-ui-main/src/features/progress/hooks/useProgressQueries.ts (1)
archon-ui-main/src/features/progress/services/progressService.ts (1)
  • progressService (9-24)
archon-ui-main/src/features/projects/tasks/services/taskService.ts (2)
archon-ui-main/src/features/projects/tasks/types/task.ts (5)
  • Task (47-71)
  • CreateTaskRequest (74-86)
  • UpdateTaskRequest (88-99)
  • DatabaseTaskStatus (12-12)
  • TaskCounts (22-27)
archon-ui-main/src/features/shared/apiWithEtag.ts (1)
  • callAPIWithETag (43-124)
archon-ui-main/src/features/knowledge/inspector/hooks/useInspectorPagination.ts (1)
archon-ui-main/src/features/knowledge/services/knowledgeService.ts (1)
  • knowledgeService (23-230)
archon-ui-main/src/features/projects/documents/hooks/useDocumentQueries.ts (1)
archon-ui-main/src/features/projects/services/projectService.ts (1)
  • projectService (12-211)
archon-ui-main/src/features/progress/services/progressService.ts (2)
archon-ui-main/src/features/progress/types/progress.ts (2)
  • ProgressResponse (111-149)
  • ActiveOperationsResponse (105-109)
archon-ui-main/src/features/shared/apiWithEtag.ts (1)
  • callAPIWithETag (43-124)
archon-ui-main/src/features/projects/services/projectService.ts (2)
archon-ui-main/src/features/shared/apiWithEtag.ts (1)
  • callAPIWithETag (43-124)
archon-ui-main/src/features/projects/shared/api.ts (1)
  • formatRelativeTime (6-17)
archon-ui-main/src/features/knowledge/services/knowledgeService.ts (2)
archon-ui-main/src/features/knowledge/types/knowledge.ts (9)
  • KnowledgeItemsFilter (127-134)
  • KnowledgeItemsResponse (98-103)
  • KnowledgeItem (26-39)
  • RefreshResponse (176-179)
  • ChunksResponse (105-114)
  • CodeExamplesResponse (116-124)
  • SearchOptions (150-155)
  • SearchResultsResponse (182-187)
  • KnowledgeSource (190-200)
archon-ui-main/src/features/shared/apiWithEtag.ts (1)
  • callAPIWithETag (43-124)
archon-ui-main/src/features/knowledge/hooks/useKnowledgeQueries.ts (1)
archon-ui-main/src/features/knowledge/services/knowledgeService.ts (1)
  • knowledgeService (23-230)
archon-ui-main/src/features/projects/tasks/hooks/tests/useTaskQueries.test.ts (1)
archon-ui-main/src/features/projects/tasks/services/taskService.ts (1)
  • taskService (12-224)
archon-ui-main/src/features/projects/tasks/hooks/useTaskQueries.ts (4)
archon-ui-main/src/features/projects/tasks/types/task.ts (3)
  • Task (47-71)
  • CreateTaskRequest (74-86)
  • UpdateTaskRequest (88-99)
archon-ui-main/src/features/shared/queryPatterns.ts (1)
  • DISABLED_QUERY_KEY (13-13)
archon-ui-main/src/features/projects/tasks/services/taskService.ts (1)
  • taskService (12-224)
archon-ui-main/src/features/shared/optimistic.ts (1)
  • createOptimisticEntity (30-42)
⏰ 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: Backend Tests (Python + pytest)
🔇 Additional comments (23)
archon-ui-main/src/features/knowledge/hooks/useKnowledgeQueries.ts (1)

785-789: LGTM: signal is properly threaded to summaries query.

This ensures cancellations aren’t treated as failures.

archon-ui-main/src/features/shared/apiWithEtag.ts (1)

114-116: LGTM: rethrowing AbortError preserves cancellation semantics.

This prevents cancelled queries from surfacing as failures.

archon-ui-main/src/features/progress/hooks/tests/useProgressQueries.test.ts (1)

80-81: Assertion looks good; ensure AbortSignal is available in the test runtime.

If CI ever lacks global AbortSignal, relax to expect.any(Object) to avoid flaky failures.

Suggested fallback:

-expect(progressService.getProgress).toHaveBeenCalledWith("progress-123", expect.any(AbortSignal));
+expect(progressService.getProgress).toHaveBeenCalledWith("progress-123", expect.any(Object));
archon-ui-main/src/features/projects/tasks/services/tests/taskService.test.ts (5)

54-60: LGTM: relaxed argument matching for createTask.

Matches the new optional signal without over-constraining tests.


109-115: LGTM: relaxed argument matching for updateTask (full update).

Keeps tests focused on intent.


139-145: LGTM: partial update expectation is precise and resilient.

Good use of expect.objectContaining.


187-193: LGTM: status update expectation looks correct.

Covers body shape without coupling to unrelated options.


218-223: LGTM: deleteTask expectation is appropriately loose.

Works with added signal propagation.

archon-ui-main/src/features/progress/hooks/useProgressQueries.ts (3)

52-57: Good: abort-signal plumbed through queryFn to service.

Forwarding { signal } into progressService.getProgress is correct and aligns with the PR goal.


201-205: Good: abort-signal forwarded in active-operations polling.

This keeps background polling cancellable and quiet on tab switches.


250-256: Good: signal propagated for multi-operation polling.

Plumbing the signal here prevents spurious “errors” during tab switches or route changes.

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

44-53: Good: abort-signal support in infinite query.

Threading { signal } into knowledgeService calls is correct and consistent with the service signatures.

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

95-96: Test correctly asserts AbortSignal is passed for queries.

This verifies the new signature without coupling to a specific AbortController instance.

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

30-34: Good: listProjects wired to query signal.

Matches the updated service API and keeps index views cancellable.

archon-ui-main/src/features/progress/services/progressService.ts (2)

13-15: LGTM: methods now accept and forward AbortSignal.

Signatures align with the calling hooks; trailing-slash note retained to avoid FastAPI redirect pitfalls.

Also applies to: 20-23


9-24: No duplicate top-level progressService detected.

Search returned only src/features/progress/services/progressService.ts — no legacy src/services/progressService.ts present.

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

27-40: Nice: pause polling during updates to protect optimistic state.

Using useIsMutating({ mutationKey: ['updateTask', projectId] }) to gate refetchInterval and refetchOnWindowFocus is a solid pattern.


32-35: Good: abort-signal forwarded for project tasks and counts queries.

Both read paths are now cancellable.

Also applies to: 48-49


70-81: LGTM: optimistic task creation payload is well-formed.

Defaults (status, priority, timestamps) look reasonable and match types.


142-146: Nice touch: stamp updated_at during optimistic update.

Helps keep UI timelines coherent before server echo.

archon-ui-main/src/features/projects/services/projectService.ts (1)

16-20: AbortSignal threading + AbortError propagation look correct.

Signal is forwarded to API calls and AbortError is debug‑logged then rethrown so TanStack Query treats it as a cancellation. Good.

Also applies to: 44-47, 56-59, 66-69, 78-81, 104-107, 112-115, 130-146, 161-164, 173-178, 180-183, 192-201, 203-206

archon-ui-main/src/features/projects/tasks/services/taskService.ts (1)

16-19: AbortSignal wiring and cancellation handling look solid across methods.

Signal is consistently forwarded; AbortError is debug‑logged and rethrown. Good.

Also applies to: 23-26, 35-38, 40-43, 52-69, 71-75, 84-97, 99-104, 113-130, 134-137, 146-151, 153-156, 165-170, 180-181, 184-187, 211-214, 216-219

archon-ui-main/src/features/knowledge/services/knowledgeService.ts (1)

28-45: AbortSignal propagation: LGTM.

All network paths accept and forward signal; uploads have a sane 30s default timeout.

Also applies to: 50-52, 57-61, 72-78, 86-91, 99-103, 114-115, 139-139, 152-156, 169-170, 185-186, 197-198, 210-211, 216-221, 227-229

@Wirasm Wirasm marked this pull request as draft September 18, 2025 17:06
@Wirasm Wirasm closed this Nov 24, 2025
@Wirasm Wirasm deleted the feature/abort-signal-service-methods branch April 6, 2026 07:37
coleam00 added a commit that referenced this pull request Apr 7, 2026
feat: chat-first layout (Option A navigation redesign)
Tyone88 pushed a commit to Tyone88/Archon that referenced this pull request Apr 16, 2026
…yout

feat: chat-first layout (Option A navigation redesign)
joaobmonteiro pushed a commit to joaobmonteiro/Archon that referenced this pull request Apr 26, 2026
…yout

feat: chat-first layout (Option A navigation redesign)
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