refactor: Phase 5 - Remove manual cache invalidations#707
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughReworks knowledge hooks to apply optimistic updates to summaries, reduce manual list invalidations, and use smart polling; Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI
participant Hook as useKnowledgeQueries
participant API
participant ProgressCache as ProgressKeys
participant SummariesCache as SummariesCaches
rect rgba(200,230,255,0.4)
note right of User: start crawl/upload (optimistic flow)
User->>UI: trigger mutation
UI->>Hook: mutate()
Hook->>SummariesCache: optimistic add temp item across matching summaries
Hook->>ProgressCache: add optimistic operation
Hook->>API: POST start operation
API-->>Hook: returns real progressId/result
Hook->>SummariesCache: reconcile optimistic items with real progressId
Hook->>ProgressCache: update operation with real id
end
rect rgba(230,255,220,0.4)
note right of UI: summaries refreshed via smart polling
UI->>Hook: useKnowledgeSummaries()
Hook->>API: GET summaries (polling interval depends on activeOperations)
API-->>Hook: summaries + operations
Hook-->>UI: { data, activeOperations, activeCrawlIds, setActiveCrawlIds }
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
archon-ui-main/src/features/knowledge/hooks/useKnowledgeQueries.ts (2)
117-123: Existing TODO comment about invisible optimistic updatesThe comment correctly identifies that optimistic updates to
knowledgeKeys.lists()are invisible because this query key is never actually queried. The actual data comes fromknowledgeKeys.summaries(filter). This is a significant architectural issue that should be addressed.Would you like me to create an issue to track the resolution of this optimistic update visibility problem as outlined in Phase 3 of your refactor plan?
743-744: Consider initializing activeCrawlIds from activeOperationsDataThe
activeCrawlIdsstate is initialized as empty but never populated from discovered operations. This means pre-existing operations won't be tracked in this array, though they are properly displayed viaactiveOperations.Consider whether you need to sync
activeCrawlIdswith discovered operations or if this separation is intentional.- const [activeCrawlIds, setActiveCrawlIds] = useState<string[]>([]); + const [activeCrawlIds, setActiveCrawlIds] = useState<string[]>(() => { + // Initialize with any pre-existing operation IDs if needed + return []; + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
archon-ui-main/src/features/knowledge/hooks/useKnowledgeQueries.ts(4 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/knowledge/hooks/useKnowledgeQueries.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/hooks/useKnowledgeQueries.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/hooks/useKnowledgeQueries.ts
🧠 Learnings (2)
📚 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/knowledge/hooks/useKnowledgeQueries.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/knowledge/hooks/useKnowledgeQueries.ts
🧬 Code graph analysis (1)
archon-ui-main/src/features/knowledge/hooks/useKnowledgeQueries.ts (2)
archon-ui-main/src/features/shared/queryClient.ts (1)
queryClient(13-48)archon-ui-main/src/features/progress/hooks/useProgressQueries.ts (1)
progressKeys(15-20)
🔇 Additional comments (7)
archon-ui-main/src/features/knowledge/hooks/useKnowledgeQueries.ts (7)
7-7: Good cleanup - removing unused useEffect importRemoving the
useEffectimport aligns with the refactor goal of eliminating timing-based cache invalidations.
289-291: LGTM - Immediate invalidation after crawl initiationThe change from delayed to immediate invalidation is correct. The backend should be consistent after successful crawl initiation, making immediate invalidation safe.
724-726: LGTM - Immediate cache invalidation on refreshThe change from 5-second delayed invalidation to immediate invalidation is appropriate. The backend should handle consistency after refresh initiation.
742-786: Excellent refactor of useKnowledgeSummaries hookThe refactoring successfully:
- Removes manual cache invalidation side effects
- Relies on smart polling for eventual consistency
- Properly tracks active operations to enable polling
- Exposes
activeCrawlIdsandsetActiveCrawlIdsfor component controlThis aligns perfectly with TanStack Query best practices and the PR objectives.
777-779: Clean removal of manual invalidation logicThe comments clearly document the philosophy change: trusting smart polling for eventual consistency instead of manual cache manipulation. This is the right approach for TanStack Query.
766-772: Smart polling looks correct — verify STALE_TIMES numeric valuesSnippet correctly toggles polling via useSmartPolling/refetchInterval, but repo search couldn't locate the STALE_TIMES constant; confirm numeric values for STALE_TIMES.frequent and STALE_TIMES.normal or provide the file path.
File: archon-ui-main/src/features/knowledge/hooks/useKnowledgeQueries.ts (lines 766–772)
783-785: Breaking change: Extended return value from useKnowledgeQueriesHook now returns activeCrawlIds and setActiveCrawlIds — update all callers to handle the new return signature.
Location: archon-ui-main/src/features/knowledge/hooks/useKnowledgeQueries.ts (lines 783–785).
Automated repo search returned no usage matches; manual verification required to ensure consumers were updated.
- Removed all manual cache invalidations from knowledge queries - Updated task queries to rely on backend consistency - Fixed optimistic update utilities to handle edge cases - Cleaned up unused imports and test utilities - Fixed minor TypeScript issues in UI components Backend now ensures data consistency through proper transaction handling, eliminating the need for frontend cache coordination. Co-Authored-By: Claude <noreply@anthropic.com>
- Added comprehensive explanation of the query key mismatch issue - Documented current behavior and impact on user experience - Listed potential solutions with tradeoffs - Created detailed PRP story in PRPs/local/ for future implementation - References specific line numbers and implementation details This documents a known limitation where optimistic updates to knowledge items are invisible because mutations update the wrong query cache. Co-Authored-By: Claude <noreply@anthropic.com>
* chore, cleanup leftovers of tanstack refactoring * refactor: Complete Phase 5 - Remove manual cache invalidations - Removed all manual cache invalidations from knowledge queries - Updated task queries to rely on backend consistency - Fixed optimistic update utilities to handle edge cases - Cleaned up unused imports and test utilities - Fixed minor TypeScript issues in UI components Backend now ensures data consistency through proper transaction handling, eliminating the need for frontend cache coordination. * docs: Enhance TODO comment for knowledge optimistic update issue - Added comprehensive explanation of the query key mismatch issue - Documented current behavior and impact on user experience - Listed potential solutions with tradeoffs - Created detailed PRP story in PRPs/local/ for future implementation - References specific line numbers and implementation details This documents a known limitation where optimistic updates to knowledge items are invisible because mutations update the wrong query cache.
) * fix(web): add query error states to sidebar and context components (#707) 8 useQuery call sites showed misleading empty states when the backend was unreachable. Users saw "No conversations yet" or "No workflow runs" instead of an error indicator, making API failures invisible. Changes: - ProjectContext: expose isErrorCodebases through context interface - Sidebar: show "Failed to load projects — retrying" when codebases query errors - AllConversationsView: show "Failed to load — retrying" instead of empty state on conversations error - ProjectDetail: add error branches for both conversations and workflow runs queries - WorkflowInvoker: return error element instead of null on fetch failure - MessageList (WorkflowDispatchInline): show ⚠ warning icon instead of infinite spinner on error Fixes #707 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: Auto-commit workflow artifacts (archon-fix-github-issue) --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…oleam00#727) * fix(web): add query error states to sidebar and context components (coleam00#707) 8 useQuery call sites showed misleading empty states when the backend was unreachable. Users saw "No conversations yet" or "No workflow runs" instead of an error indicator, making API failures invisible. Changes: - ProjectContext: expose isErrorCodebases through context interface - Sidebar: show "Failed to load projects — retrying" when codebases query errors - AllConversationsView: show "Failed to load — retrying" instead of empty state on conversations error - ProjectDetail: add error branches for both conversations and workflow runs queries - WorkflowInvoker: return error element instead of null on fetch failure - MessageList (WorkflowDispatchInline): show ⚠ warning icon instead of infinite spinner on error Fixes coleam00#707 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: Auto-commit workflow artifacts (archon-fix-github-issue) --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…oleam00#727) * fix(web): add query error states to sidebar and context components (coleam00#707) 8 useQuery call sites showed misleading empty states when the backend was unreachable. Users saw "No conversations yet" or "No workflow runs" instead of an error indicator, making API failures invisible. Changes: - ProjectContext: expose isErrorCodebases through context interface - Sidebar: show "Failed to load projects — retrying" when codebases query errors - AllConversationsView: show "Failed to load — retrying" instead of empty state on conversations error - ProjectDetail: add error branches for both conversations and workflow runs queries - WorkflowInvoker: return error element instead of null on fetch failure - MessageList (WorkflowDispatchInline): show ⚠ warning icon instead of infinite spinner on error Fixes coleam00#707 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: Auto-commit workflow artifacts (archon-fix-github-issue) --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 participant
Summary
Implements Phase 5 of the frontend state management refactor by removing all setTimeout-based cache invalidations and manual cache manipulations. This eliminates race conditions, flashing UI states, and unpredictable timing issues in cache updates.
Changes Made
Type of Change
Affected Services
Testing
Test Evidence
Checklist
Breaking Changes
None. This is a performance improvement that maintains all existing functionality while removing timing-related bugs.
Additional Notes
Performance Impact
What Changed
Why This Works
This completes Phase 5 of the frontend state management refactor series.
Related PRs: #692 (Phase 2), #695 (Phase 3), #700 (Phase 4)
Summary by CodeRabbit
Improvements
Bug Fixes
Chores