Skip to content

Fix middle section headers stuck loading - #120

Merged
2witstudios merged 1 commit into
masterfrom
claude/fix-section-headers-loading-1466E
Dec 23, 2025
Merged

2witstudios merged 1 commit into
masterfrom
claude/fix-section-headers-loading-1466E

Conversation

@2witstudios

@2witstudios 2witstudios commented Dec 23, 2025 •

Copy link
Copy Markdown
Owner

The isPaused option in SWR blocks ALL fetching including the initial fetch. This caused section headers to be stuck in perpetual loading skeleton state when any editing session was active during mount.

Changes:

  • Add hasLoadedRef to useBreadcrumbs, usePageTree, useConversations
  • Only pause revalidation AFTER initial data has loaded successfully
  • Update documentation with correct isPaused pattern in CLAUDE.md
  • Update ui-refresh-protection.md with detailed pattern explanation

The fix ensures initial data always loads, while still preventing unnecessary revalidation during active editing/streaming sessions.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed data loading behavior to allow initial data fetches while preventing unintended refreshes during active editing sessions.
  • Documentation

    • Updated guidance on polling protection patterns to reflect improved data loading behavior that supports initial loads while protecting against UI refresh issues during editing.

✏️ Tip: You can customize this high-level summary in your review settings.

The isPaused option in SWR blocks ALL fetching including the initial
fetch. This caused section headers to be stuck in perpetual loading
skeleton state when any editing session was active during mount.

Changes:
- Add hasLoadedRef to useBreadcrumbs, usePageTree, useConversations
- Only pause revalidation AFTER initial data has loaded successfully
- Update documentation with correct isPaused pattern in CLAUDE.md
- Update ui-refresh-protection.md with detailed pattern explanation

The fix ensures initial data always loads, while still preventing
unnecessary revalidation during active editing/streaming sessions.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Dec 23, 2025 •

Copy link
Copy Markdown
Contributor

Walkthrough

Updates SWR pause logic across multiple hooks to track initial data load completion via a ref. Previously, isPaused was a static boolean that blocked even first fetches. New pattern allows initial fetch to proceed while pausing subsequent revalidations during editing. Also updates documentation with the refined guidance.

Changes

Cohort / File(s) Summary
Hook implementation updates
apps/web/src/hooks/useBreadcrumbs.ts, apps/web/src/hooks/usePageTree.ts, apps/web/src/lib/ai/shared/hooks/useConversations.ts
Added useRef import and hasLoadedRef state to track initial load. Converted isPaused from boolean to function that returns hasLoadedRef.current && isEditingActive(). Added onSuccess handler to set hasLoadedRef.current = true after first successful fetch. Allows initial fetch while protecting subsequent revalidations.
Documentation
docs/3.0-guides-and-tools/ui-refresh-protection.md
Updated SWR polling protection guidance with refined pattern. Replaced simple isPaused predicate with two-phase pause behavior using ref tracking. Includes updated code examples demonstrating the new hasLoadedRef pattern and step-by-step breakdown of the control flow.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main problem being fixed: section headers getting stuck in a loading state, which matches the core issue described in the PR objectives.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch claude/fix-section-headers-loading-1466E

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e038a20 and 377aaa4.

📒 Files selected for processing (5)
  • CLAUDE.md
  • apps/web/src/hooks/useBreadcrumbs.ts
  • apps/web/src/hooks/usePageTree.ts
  • apps/web/src/lib/ai/shared/hooks/useConversations.ts
  • docs/3.0-guides-and-tools/ui-refresh-protection.md
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Never use any types in TypeScript code - always use proper TypeScript types

**/*.{ts,tsx}: Never use any types - always use proper TypeScript types
Use camelCase for variable and function names
Use UPPER_SNAKE_CASE for constants
Use PascalCase for type and enum names
Use kebab-case for filenames, except React hooks (camelCase with use prefix), Zustand stores (camelCase with use prefix), and React components (PascalCase)
Lint with Next/ESLint as configured in apps/web/eslint.config.mjs
Message content should always use the message parts structure with { parts: [{ type: 'text', text: '...' }] }
Use centralized permission functions from @pagespace/lib/permissions (e.g., getUserAccessLevel, canUserEditPage) instead of implementing permission logic locally
Always use Drizzle client from @pagespace/db package for database access
Use ESM modules throughout the codebase

Files:

  • apps/web/src/hooks/usePageTree.ts
  • apps/web/src/lib/ai/shared/hooks/useConversations.ts
  • apps/web/src/hooks/useBreadcrumbs.ts
apps/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

apps/**/*.{ts,tsx}: Use centralized permission functions from @pagespace/lib/permissions for access control, such as getUserAccessLevel() and canUserEditPage()
Always use Drizzle client from @pagespace/db for database access instead of direct database connections
Always use message parts structure with parts array containing objects with type and text fields when constructing messages for AI

Files:

  • apps/web/src/hooks/usePageTree.ts
  • apps/web/src/lib/ai/shared/hooks/useConversations.ts
  • apps/web/src/hooks/useBreadcrumbs.ts
**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

**/*.ts: React hook files should use camelCase matching the exported hook name (e.g., useAuth.ts)
Zustand store files should use camelCase with use prefix (e.g., useAuthStore.ts)

Files:

  • apps/web/src/hooks/usePageTree.ts
  • apps/web/src/lib/ai/shared/hooks/useConversations.ts
  • apps/web/src/hooks/useBreadcrumbs.ts
**/*.{ts,tsx,js,jsx,json}

📄 CodeRabbit inference engine (AGENTS.md)

Format code with Prettier

Files:

  • apps/web/src/hooks/usePageTree.ts
  • apps/web/src/lib/ai/shared/hooks/useConversations.ts
  • apps/web/src/hooks/useBreadcrumbs.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T14:54:15.319Z
Learning: Applies to apps/web/src/**/*.tsx : When using SWR, check `useEditingStore` state with `isAnyActive()` and set `isPaused` to prevent data refreshes during editing or streaming
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T14:54:15.319Z
Learning: Applies to apps/web/src/**/*.tsx : For document editing, register editing state using `useEditingStore.getState().startEditing()` and `endEditing()` to prevent unwanted UI refreshes
📚 Learning: 2025-12-14T14:54:15.319Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T14:54:15.319Z
Learning: Applies to apps/web/src/**/*.tsx : When using SWR, check `useEditingStore` state with `isAnyActive()` and set `isPaused` to prevent data refreshes during editing or streaming

Applied to files:

  • apps/web/src/hooks/usePageTree.ts
  • apps/web/src/lib/ai/shared/hooks/useConversations.ts
  • docs/3.0-guides-and-tools/ui-refresh-protection.md
  • apps/web/src/hooks/useBreadcrumbs.ts
  • CLAUDE.md
📚 Learning: 2025-12-22T20:04:40.892Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-22T20:04:40.892Z
Learning: Applies to **/*.tsx : Use SWR for server state management and caching

Applied to files:

  • apps/web/src/hooks/usePageTree.ts
  • apps/web/src/lib/ai/shared/hooks/useConversations.ts
  • docs/3.0-guides-and-tools/ui-refresh-protection.md
  • apps/web/src/hooks/useBreadcrumbs.ts
  • CLAUDE.md
📚 Learning: 2025-12-14T14:54:15.319Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T14:54:15.319Z
Learning: Applies to apps/web/src/**/*.tsx : For document editing, register editing state using `useEditingStore.getState().startEditing()` and `endEditing()` to prevent unwanted UI refreshes

Applied to files:

  • apps/web/src/hooks/usePageTree.ts
  • docs/3.0-guides-and-tools/ui-refresh-protection.md
  • apps/web/src/hooks/useBreadcrumbs.ts
  • CLAUDE.md
📚 Learning: 2025-12-14T14:54:15.319Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-14T14:54:15.319Z
Learning: Applies to apps/web/src/**/*.tsx : For AI streaming operations, register streaming state using `useEditingStore.getState().startStreaming()` and `endStreaming()` to prevent unwanted UI refreshes

Applied to files:

  • apps/web/src/hooks/usePageTree.ts
  • apps/web/src/lib/ai/shared/hooks/useConversations.ts
  • docs/3.0-guides-and-tools/ui-refresh-protection.md
  • CLAUDE.md
📚 Learning: 2025-12-14T14:54:45.713Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:45.713Z
Learning: Use Zustand for client state management and SWR for server state and caching

Applied to files:

  • apps/web/src/hooks/usePageTree.ts
  • apps/web/src/lib/ai/shared/hooks/useConversations.ts
  • docs/3.0-guides-and-tools/ui-refresh-protection.md
  • apps/web/src/hooks/useBreadcrumbs.ts
  • CLAUDE.md
🧬 Code graph analysis (3)
apps/web/src/hooks/usePageTree.ts (1)
apps/web/src/stores/useEditingStore.ts (1)
  • isEditingActive (134-134)
apps/web/src/lib/ai/shared/hooks/useConversations.ts (1)
apps/web/src/stores/useEditingStore.ts (1)
  • isEditingActive (134-134)
apps/web/src/hooks/useBreadcrumbs.ts (1)
apps/web/src/stores/useEditingStore.ts (1)
  • isEditingActive (134-134)
⏰ 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: Unit Tests
🔇 Additional comments (10)
apps/web/src/hooks/usePageTree.ts (2)

2-2: LGTM! Correct import and initialization.

The useRef import and hasLoadedRef initialization are properly implemented. Starting with false ensures the initial fetch proceeds unblocked.

Also applies to: 51-52


59-63: LGTM! Correct two-phase pause implementation.

The isPaused function and onSuccess callback correctly implement the initial-load safeguard pattern. Initial fetch proceeds when hasLoadedRef.current is false, and subsequent revalidations respect isEditingActive() only after the first successful load.

CLAUDE.md (1)

145-157: LGTM! Documentation accurately reflects the new pattern.

The updated SWR protection example correctly demonstrates the hasLoadedRef pattern with clear comments emphasizing the critical requirement to allow the initial fetch. The example code matches the implementation in the actual hooks.

apps/web/src/lib/ai/shared/hooks/useConversations.ts (2)

6-6: LGTM! Consistent pattern implementation.

The useRef import and hasLoadedRef initialization correctly mirror the pattern established in usePageTree.ts, ensuring consistency across the codebase.

Also applies to: 72-73


92-96: LGTM! Correct implementation of the two-phase pause pattern.

The isPaused function and onSuccess callback are properly implemented, matching the pattern in usePageTree.ts and useBreadcrumbs.ts. This ensures conversations load on initial mount while respecting editing state for subsequent revalidations.

apps/web/src/hooks/useBreadcrumbs.ts (2)

1-1: LGTM! Consistent pattern across all hooks.

The useRef import and hasLoadedRef initialization follow the same pattern as usePageTree.ts and useConversations.ts, maintaining consistency across the codebase.

Also applies to: 24-25


31-35: LGTM! Fixes the perpetual loading skeleton issue.

The implementation correctly allows initial breadcrumbs to load while pausing subsequent revalidations during active editing. This directly addresses the "middle section headers stuck loading" issue described in the PR objectives.

docs/3.0-guides-and-tools/ui-refresh-protection.md (3)

60-60: LGTM! Clear documentation with complete example.

The updated heading and example code accurately demonstrate the hasLoadedRef pattern. The example is complete with all necessary imports and configuration, making it easy for developers to implement correctly.

Also applies to: 63-82


84-91: LGTM! Excellent explanation of the pattern's necessity.

The CRITICAL warning appropriately emphasizes the severity of blocking initial fetches, and the pattern breakdown provides a clear step-by-step explanation of how the hasLoadedRef mechanism prevents this issue. This will help developers understand both the what and the why.


94-96: LGTM! Migration guide accurately reflects the updated pattern.

The updated file list correctly identifies the three hooks using the pattern, and the migration guide provides a complete, correct implementation example with appropriate CRITICAL emphasis. This will help developers apply the pattern correctly in new components.

Also applies to: 352-367


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

@2witstudios
2witstudios merged commit 52dfdd1 into master Dec 23, 2025
3 checks passed
2witstudios added a commit that referenced this pull request Apr 8, 2026
…ay args (#120-#123)

Switch infrastructure test files from execSync() with template-literal
string interpolation to execFileSync() with array arguments, bypassing
the shell entirely. Resolves 4 CodeQL js/shell-command-injection-from-environment
alerts (CWE-78) in tenant-stack, generate-tenant-env, and image-runtime tests.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2witstudios added a commit that referenced this pull request Apr 8, 2026
…ay args (#120-#123) (#847)

Switch infrastructure test files from execSync() with template-literal
string interpolation to execFileSync() with array arguments, bypassing
the shell entirely. Resolves 4 CodeQL js/shell-command-injection-from-environment
alerts (CWE-78) in tenant-stack, generate-tenant-env, and image-runtime tests.

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.

2 participants