Skip to content

fix(stores): restore missing page header and clean up dead navigation code#100

Merged
2witstudios merged 2 commits intomasterfrom
fix/header-missing-cleanup-dead-store-code
Dec 19, 2025
Merged

fix(stores): restore missing page header and clean up dead navigation code#100
2witstudios merged 2 commits intomasterfrom
fix/header-missing-cleanup-dead-store-code

Conversation

@2witstudios
Copy link
Owner

@2witstudios 2witstudios commented Dec 19, 2025

Summary

  • Fixes missing page header (Share button, breadcrumbs) on all pages - OptimizedViewHeader was checking layoutStore.activePageId which was always null because the navigation system was never wired up
  • Removes 555 lines of dead code from Zustand stores that was never used
  • Clarifies store responsibilities with non-overlapping state between the two stores

Root Cause

The useLayoutStore had a complete navigation system designed (activePageId, navigateToPage(), hydrateFromUrl(), view caching) but it was never actually called. The app uses Next.js useParams() for navigation, making all this store state dead code.

Additionally, useUIStore had duplicate sidebar state that was never used (actual sidebar state lives in useLayoutStore).

Changes

File Before After Change
useLayoutStore.ts 352 lines 60 lines -83%
useUIStore.ts 97 lines 50 lines -48%
useUI.ts 97 lines 32 lines -67%
NavigationProvider.tsx 125 lines 125 lines Removed dead cleanup code
CenterPanel.tsx - - Fixed header to use useParams()

Store Responsibilities (After)

Store State Purpose
useLayoutStore leftSidebarOpen, rightSidebarOpen Sidebar visibility (persisted)
useUIStore treeExpanded, treeScrollPosition Tree UI state (persisted)

Test plan

  • Verify page header shows on all page types (DOCUMENT, FOLDER, AI_CHAT, etc.)
  • Verify Share button works
  • Verify breadcrumbs display correctly
  • Verify sidebar toggles still work
  • Verify tree expansion state persists across page refresh
  • Run pnpm typecheck - passes
  • Run store tests - all 10 tests pass

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Refactor

    • Navigation is now URL-driven (params) instead of stored centrally; header visibility follows route params.
    • Multiple UI hooks consolidated into a single tree navigation hook (tree expansion + scroll).
    • Layout and UI stores simplified: navigation, view cache and several advanced layout concerns removed.
  • UI

    • Debug panel: removed view cache UI, added Rehydrated status.
  • Tests

    • Tests narrowed to tree expansion and scroll-position behavior.
  • Docs

    • Updated architecture guidance to reflect URL-driven navigation and decoupled stores.

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

… code

The page header (Share button, breadcrumbs, etc.) was missing from all pages
because OptimizedViewHeader checked layoutStore.activePageId which was always
null - the navigation system was designed but never wired up.

Changes:
- Fix OptimizedViewHeader to use useParams() instead of dead store state
- Remove all dead navigation code from useLayoutStore (352→60 lines)
- Remove duplicate sidebar state from useUIStore (97→50 lines)
- Remove dead cleanup code from NavigationProvider.tsx
- Remove dead sidebar hooks from useUI.ts (keep only useTreeState)
- Update store tests to match new simplified state
- Update state-management.md documentation

The two stores now have clear, non-overlapping responsibilities:
- useLayoutStore: sidebar open/closed state (persisted)
- useUIStore: tree expansion and scroll state (persisted)

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

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@chatgpt-codex-connector
Copy link

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
Copy link
Contributor

coderabbitai bot commented Dec 19, 2025

Walkthrough

This PR shifts navigation responsibility to URL params, narrows layout store to sidebar visibility and rehydration, consolidates multiple UI hooks into a single tree-focused hook, and removes view caching, active page/drive tracking, and related lifecycle cleanup across components and debug tools.

Changes

Cohort / File(s) Summary
Store Simplification
apps/web/src/stores/useLayoutStore.ts, apps/web/src/stores/useUIStore.ts
Removed navigation pointers, active page/drive, view cache, center-view type, and many related methods. Layout store now exposes sidebar toggles and rehydrated; UI store now only persists treeExpanded and treeScrollPosition. Persist merge/merge-normalization logic removed.
Hook Consolidation
apps/web/src/hooks/useUI.ts
Replaced multiple small UI hooks (useLeftSidebar, useRightSidebar, useCenterView, useNavigationState, useResponsiveLayout) with a single useTreeState hook exposing treeExpanded, treeScrollPosition, setters, and helpers (isExpanded, toggleExpanded). Multiple exports removed and one added.
Component Adjustments
apps/web/src/components/layout/NavigationProvider.tsx, apps/web/src/components/layout/middle-content/CenterPanel.tsx, apps/web/src/components/layout/LayoutErrorBoundary.tsx, apps/web/src/components/layout/DebugPanel.tsx
Removed useLayoutStore usages and cleanup/cache-clear logic from NavigationProvider and LayoutErrorBoundary. CenterPanel now decides header visibility from route params instead of layout store state. DebugPanel removes view-cache UI and shows rehydrated status; import icons adjusted.
Tests
apps/web/src/stores/__tests__/useUIStore.test.ts
Removed tests for sidebar visibility, center-view, and navigation-related behaviors. Retained and adjusted tests to cover treeScrollPosition and treeExpanded independence and initial values.
Documentation
docs/2.0-architecture/2.1-frontend/state-management.md
Updated architecture guidance to treat URL params as navigation source of truth, decouple document state from layout/UI stores, and document the new tree-focused UI state pattern and migration notes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Pay attention to call sites that previously relied on removed LayoutState fields/methods (activePageId, navigateToPage, clearCache, etc.).
  • Verify all components now derive navigation from router params (useParams/usePathname) and that edge cases (missing params) are handled.
  • Confirm migration of multiple small hooks to useTreeState across the codebase and that selector semantics remain correct.
  • Check persisted state rehydration for treeExpanded conversion between array and Set.

Poem

🐇 In a burrow of code I nibble and cheer,
URLs guide the paths that were once kept near,
Trees keep their secrets, sidebars softly sleep,
Caches fade away — the routes now run deep. 🌿✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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 identifies the main changes: fixing a missing page header (the primary bug) and removing dead navigation code (cleanup). It directly relates to the changeset's core 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 fix/header-missing-cleanup-dead-store-code

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0d81bc9 and 59d2dc9.

📒 Files selected for processing (2)
  • apps/web/src/components/layout/DebugPanel.tsx (2 hunks)
  • apps/web/src/components/layout/LayoutErrorBoundary.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

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

**/*.{ts,tsx}: No any types - always use proper TypeScript types
Use kebab-case for filenames (e.g., image-processor.ts)
Use camelCase for variables and functions
Use UPPER_SNAKE_CASE for constants
Use PascalCase for types and enums
Use centralized permission logic: import getUserAccessLevel and canUserEditPage from @pagespace/lib/permissions
Use Drizzle client from @pagespace/db for all database access
Always structure message content using the message parts structure: { parts: [{ type: 'text', text: '...' }] }
Use ESM modules and TypeScript strict mode

Files:

  • apps/web/src/components/layout/LayoutErrorBoundary.tsx
  • apps/web/src/components/layout/DebugPanel.tsx
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/components/layout/LayoutErrorBoundary.tsx
  • apps/web/src/components/layout/DebugPanel.tsx
apps/web/src/**/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

apps/web/src/**/*.tsx: For document editing, register editing state using useEditingStore.getState().startEditing() and endEditing() to prevent unwanted UI refreshes
For AI streaming operations, register streaming state using useEditingStore.getState().startStreaming() and endStreaming() to prevent unwanted UI refreshes
When using SWR, check useEditingStore state with isAnyActive() and set isPaused to prevent data refreshes during editing or streaming

Files:

  • apps/web/src/components/layout/LayoutErrorBoundary.tsx
  • apps/web/src/components/layout/DebugPanel.tsx
**/{components,src/**/components}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use PascalCase for React component names and filenames

Files:

  • apps/web/src/components/layout/LayoutErrorBoundary.tsx
  • apps/web/src/components/layout/DebugPanel.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Format code with Prettier and lint with ESLint using the configuration at apps/web/eslint.config.mjs

Files:

  • apps/web/src/components/layout/LayoutErrorBoundary.tsx
  • apps/web/src/components/layout/DebugPanel.tsx
apps/web/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

apps/web/src/**/*.{ts,tsx}: Use Zustand for client-side state management
Use SWR for server state and caching

Files:

  • apps/web/src/components/layout/LayoutErrorBoundary.tsx
  • apps/web/src/components/layout/DebugPanel.tsx
🧬 Code graph analysis (1)
apps/web/src/components/layout/DebugPanel.tsx (1)
apps/web/src/components/ui/badge.tsx (1)
  • Badge (46-46)
⏰ 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 (4)
apps/web/src/components/layout/LayoutErrorBoundary.tsx (1)

65-75: LGTM! Cleanup aligns with simplified store.

The removal of layoutStore.clearCache() is consistent with the PR's objective to remove view caching from the layout store. The method still appropriately clears persisted storage (localStorage and sessionStorage) to address corrupted state.

apps/web/src/components/layout/DebugPanel.tsx (3)

12-23: LGTM! Icon imports cleaned up.

The import changes correctly remove the unused MemoryStick icon (previously used for the now-removed View Cache section) while preserving all necessary icons for the updated debug panel UI.


226-231: LGTM! Rehydration status added.

The new "Rehydrated" field correctly displays the rehydration state from the simplified layout store, using a UI pattern consistent with the existing sidebar status indicators.


39-43: LGTM! Cache clearing simplified.

The removal of layoutStore.clearCache() is consistent with the PR's removal of view caching from the layout store. The method still effectively clears persisted state and forces a fresh page load.


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

Copy link
Contributor

@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)
docs/2.0-architecture/2.1-frontend/state-management.md (1)

482-509: Add language identifier to fenced code block.

The code block illustrating the new architecture is missing a language identifier for syntax highlighting.

🔎 Proposed fix
-```
+```typescript
 ✅ CURRENT ARCHITECTURE:
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b538ebf and 0d81bc9.

📒 Files selected for processing (7)
  • apps/web/src/components/layout/NavigationProvider.tsx (1 hunks)
  • apps/web/src/components/layout/middle-content/CenterPanel.tsx (1 hunks)
  • apps/web/src/hooks/useUI.ts (1 hunks)
  • apps/web/src/stores/__tests__/useUIStore.test.ts (3 hunks)
  • apps/web/src/stores/useLayoutStore.ts (1 hunks)
  • apps/web/src/stores/useUIStore.ts (3 hunks)
  • docs/2.0-architecture/2.1-frontend/state-management.md (14 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

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

**/*.{ts,tsx}: No any types - always use proper TypeScript types
Use kebab-case for filenames (e.g., image-processor.ts)
Use camelCase for variables and functions
Use UPPER_SNAKE_CASE for constants
Use PascalCase for types and enums
Use centralized permission logic: import getUserAccessLevel and canUserEditPage from @pagespace/lib/permissions
Use Drizzle client from @pagespace/db for all database access
Always structure message content using the message parts structure: { parts: [{ type: 'text', text: '...' }] }
Use ESM modules and TypeScript strict mode

Files:

  • apps/web/src/components/layout/middle-content/CenterPanel.tsx
  • apps/web/src/stores/useUIStore.ts
  • apps/web/src/components/layout/NavigationProvider.tsx
  • apps/web/src/hooks/useUI.ts
  • apps/web/src/stores/useLayoutStore.ts
  • apps/web/src/stores/__tests__/useUIStore.test.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/components/layout/middle-content/CenterPanel.tsx
  • apps/web/src/stores/useUIStore.ts
  • apps/web/src/components/layout/NavigationProvider.tsx
  • apps/web/src/hooks/useUI.ts
  • apps/web/src/stores/useLayoutStore.ts
  • apps/web/src/stores/__tests__/useUIStore.test.ts
apps/web/src/**/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

apps/web/src/**/*.tsx: For document editing, register editing state using useEditingStore.getState().startEditing() and endEditing() to prevent unwanted UI refreshes
For AI streaming operations, register streaming state using useEditingStore.getState().startStreaming() and endStreaming() to prevent unwanted UI refreshes
When using SWR, check useEditingStore state with isAnyActive() and set isPaused to prevent data refreshes during editing or streaming

Files:

  • apps/web/src/components/layout/middle-content/CenterPanel.tsx
  • apps/web/src/components/layout/NavigationProvider.tsx
**/{components,src/**/components}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use PascalCase for React component names and filenames

Files:

  • apps/web/src/components/layout/middle-content/CenterPanel.tsx
  • apps/web/src/components/layout/NavigationProvider.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Format code with Prettier and lint with ESLint using the configuration at apps/web/eslint.config.mjs

Files:

  • apps/web/src/components/layout/middle-content/CenterPanel.tsx
  • apps/web/src/stores/useUIStore.ts
  • apps/web/src/components/layout/NavigationProvider.tsx
  • apps/web/src/hooks/useUI.ts
  • apps/web/src/stores/useLayoutStore.ts
  • apps/web/src/stores/__tests__/useUIStore.test.ts
apps/web/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

apps/web/src/**/*.{ts,tsx}: Use Zustand for client-side state management
Use SWR for server state and caching

Files:

  • apps/web/src/components/layout/middle-content/CenterPanel.tsx
  • apps/web/src/stores/useUIStore.ts
  • apps/web/src/components/layout/NavigationProvider.tsx
  • apps/web/src/hooks/useUI.ts
  • apps/web/src/stores/useLayoutStore.ts
  • apps/web/src/stores/__tests__/useUIStore.test.ts
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:47.122Z
Learning: Tech stack: Next.js 15 App Router + TypeScript + Tailwind + shadcn/ui
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
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:47.122Z
Learning: Applies to apps/web/src/**/*.{ts,tsx} : Use Zustand for client-side state management
📚 Learning: 2025-12-14T14:54:47.122Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:47.122Z
Learning: Applies to **/*.{ts,tsx} : Use centralized permission logic: import `getUserAccessLevel` and `canUserEditPage` from `pagespace/lib/permissions`

Applied to files:

  • apps/web/src/components/layout/middle-content/CenterPanel.tsx
📚 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: Applies to app/**/*.{ts,tsx} : Import and use `getUserAccessLevel()` and `canUserEditPage()` from `pagespace/lib/permissions` for centralized permission logic

Applied to files:

  • apps/web/src/components/layout/middle-content/CenterPanel.tsx
📚 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/stores/useUIStore.ts
  • apps/web/src/hooks/useUI.ts
  • apps/web/src/stores/__tests__/useUIStore.test.ts
  • docs/2.0-architecture/2.1-frontend/state-management.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/stores/useUIStore.ts
  • apps/web/src/hooks/useUI.ts
  • apps/web/src/stores/__tests__/useUIStore.test.ts
  • docs/2.0-architecture/2.1-frontend/state-management.md
📚 Learning: 2025-12-14T14:54:47.122Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:47.122Z
Learning: Applies to apps/web/src/**/*.{ts,tsx} : Use Zustand for client-side state management

Applied to files:

  • apps/web/src/stores/useUIStore.ts
  • apps/web/src/hooks/useUI.ts
  • apps/web/src/stores/useLayoutStore.ts
  • docs/2.0-architecture/2.1-frontend/state-management.md
📚 Learning: 2025-12-14T14:54:47.122Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:47.122Z
Learning: Tech stack: Next.js 15 App Router + TypeScript + Tailwind + shadcn/ui

Applied to files:

  • apps/web/src/components/layout/NavigationProvider.tsx
  • docs/2.0-architecture/2.1-frontend/state-management.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/stores/useLayoutStore.ts
  • docs/2.0-architecture/2.1-frontend/state-management.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 : When using SWR, check `useEditingStore` state with `isAnyActive()` and set `isPaused` to prevent data refreshes during editing or streaming

Applied to files:

  • docs/2.0-architecture/2.1-frontend/state-management.md
📚 Learning: 2025-12-14T14:54:47.122Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:47.122Z
Learning: Applies to apps/web/src/**/*.{ts,tsx} : Use SWR for server state and caching

Applied to files:

  • docs/2.0-architecture/2.1-frontend/state-management.md
📚 Learning: 2025-12-14T14:54:38.009Z
Learnt from: CR
Repo: 2witstudios/PageSpace PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-14T14:54:38.009Z
Learning: Applies to apps/web/app/**/{route,route.ts,route.js} : In Next.js 15, `params` in dynamic routes are Promise objects. You MUST await `context.params` before destructuring.

Applied to files:

  • docs/2.0-architecture/2.1-frontend/state-management.md
🧬 Code graph analysis (2)
apps/web/src/hooks/useUI.ts (1)
apps/web/src/stores/useUIStore.ts (1)
  • useUIStore (14-50)
apps/web/src/stores/__tests__/useUIStore.test.ts (1)
apps/web/src/stores/useUIStore.ts (1)
  • useUIStore (14-50)
🪛 markdownlint-cli2 (0.18.1)
docs/2.0-architecture/2.1-frontend/state-management.md

482-482: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

⏰ 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 (7)
docs/2.0-architecture/2.1-frontend/state-management.md (1)

78-95: LGTM: URL-based navigation pattern correctly documented.

The documentation accurately reflects the architectural shift to URL-based navigation using Next.js App Router's useParams() hook. This approach provides browser history support, shareable URLs, and eliminates the complexity of maintaining navigation state in Zustand.

apps/web/src/stores/useLayoutStore.ts (1)

1-59: Excellent simplification with clear separation of concerns.

The layout store has been successfully refactored from 352 lines to 60 lines (-83%), now focusing solely on sidebar visibility. The removal of navigation state (activePageId, navigateToPage, viewCache, etc.) aligns with the architectural decision to use URL-based navigation via Next.js App Router.

Key improvements:

  • Clear single responsibility (sidebar state management)
  • Proper persistence configuration (only sidebars, not rehydrated flag)
  • Clean interface with minimal API surface
apps/web/src/components/layout/NavigationProvider.tsx (1)

21-58: LGTM: Clean removal of store-based navigation dependencies.

The NavigationProvider has been properly simplified by removing:

  • useLayoutStore dependency
  • Unmount cleanup logic that saved navigation state
  • layoutStore.clearCache() call in error boundary

This aligns with the architectural shift to URL-based navigation where the browser manages navigation state. The component's public API remains unchanged, ensuring backward compatibility.

apps/web/src/components/layout/middle-content/CenterPanel.tsx (1)

112-130: Critical fix: Header visibility now correctly derived from URL params.

This change fixes the missing page header (Share button, breadcrumbs) issue by replacing the check for layoutStore.activePageId (which was always null because store-based navigation was never wired up) with params.pageId from the URL.

Root cause resolved:

  • Before: Checked layoutStore.activePageId → always null → header never shown
  • After: Checks params.pageId from URL → correctly reflects current page → header shown

This follows Next.js 15 App Router best practices where the URL is the source of truth for navigation state.

apps/web/src/stores/__tests__/useUIStore.test.ts (1)

1-137: LGTM: Tests correctly updated to match focused UI store scope.

The test suite has been appropriately refactored to test only the remaining functionality:

  • Tree expansion state (Set of node IDs)
  • Tree scroll position (number)
  • State independence between these two concerns

Removed tests for deleted features (sidebars, navigation state) that are no longer part of the UI store. The remaining tests provide comprehensive coverage for tree state management.

apps/web/src/hooks/useUI.ts (1)

4-32: Excellent consolidation into a focused tree state hook.

The refactoring from multiple discrete hooks to a single useTreeState hook is a significant improvement:

Benefits:

  • Clear, focused API for tree navigation state
  • Proper memoization with useCallback to prevent unnecessary re-renders
  • Helper functions (isExpanded, toggleExpanded) provide ergonomic access patterns
  • Reduced API surface (removed 5 hooks, added 1 focused hook)

Removed hooks were for deleted features:

  • useLeftSidebar, useRightSidebar → sidebar state moved to layout store
  • useCenterView, useNavigationState → navigation moved to URL params
  • useResponsiveLayout → no longer needed with simplified architecture
apps/web/src/stores/useUIStore.ts (1)

1-50: LGTM: Focused tree state store with proper Set serialization.

The UI store has been successfully simplified to manage only tree navigation state:

State:

  • treeExpanded: Set for expanded node IDs
  • treeScrollPosition: number for scroll offset

Persistence handling (lines 38-47):

  • Serialization: Converts Set → Array for localStorage
  • Deserialization: Converts Array → Set with defensive fallback
  • Properly excludes transient state from persistence

Removed features:

  • Sidebar state → moved to layout store
  • Navigation state → moved to URL params
  • Center view type → removed (dead code)

The Set serialization logic is defensive and handles both Array and Set types during rehydration, providing robustness against data shape changes.

The layout store was simplified to only manage sidebar state, but
DebugPanel and LayoutErrorBoundary still referenced the deleted
clearCache() method and other removed properties (viewCache,
activeDriveId, activePageId, centerViewType).

These were all part of a navigation system that was designed but
never wired up (dead code). The useful functionality is preserved:
- Clear Cache button still clears localStorage/sessionStorage
- Error boundary still clears storage on error

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

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@2witstudios
Copy link
Owner Author

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

🤖 Generated with Claude Code

@2witstudios 2witstudios merged commit 60e4275 into master Dec 19, 2025
3 checks passed
2witstudios added a commit that referenced this pull request Dec 19, 2025
* feat(db): add activityLogs table for audit trail

Add new database schema for enterprise activity monitoring:
- New enums: activity_operation, activity_resource
- New activityLogs table with:
  - User attribution with AI context (isAiGenerated, aiProvider, aiModel)
  - Full content snapshots for future rollback support
  - Hierarchical context (driveId, pageId) for filtering
  - Indexed for efficient queries by timestamp, user, drive, page

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

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

* feat(lib): add activity logger service

Add fire-and-forget activity logging functions:
- logActivity(): Core logging function
- logPageActivity(): Page CRUD operations
- logPermissionActivity(): Permission changes
- logDriveActivity(): Drive operations
- logAgentConfigActivity(): Agent configuration changes

Designed to never block user operations while maintaining
comprehensive audit trail for enterprise compliance.

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

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

* feat(api): add /api/activities endpoint

Add context-aware activity fetching endpoint:
- user context: User's own activity (dashboard view)
- drive context: All drive activity (drive view)
- page context: All page edits (page view)

Includes permission checks (canUserViewPage, isUserDriveMember)
and pagination support for large activity lists.

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

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

* feat(ui): replace Settings tab with Activity tab in sidebar

- Add SidebarActivityTab component with context-aware activity display
- Update right sidebar to use Activity tab instead of Settings
- Update GlobalAssistantView to open Activity tab

Activity tab features:
- Search filtering
- User avatars with AI indicator (Bot icon)
- Operation icons (create, update, delete, etc.)
- Relative timestamps
- Loading skeletons

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

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

* refactor(store): update SidebarTab type from 'settings' to 'activity'

Update usePageAgentDashboardStore and tests to use 'activity'
tab instead of 'settings' to match new sidebar structure.

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

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

* feat(api): inject activity logging into page CRUD routes

Add logPageActivity calls to track:
- Page creation (POST /api/pages)
- Page updates (PATCH /api/pages/[pageId])
- Page deletion/trash (DELETE /api/pages/[pageId])
- Page restoration (POST /api/pages/[pageId]/restore)

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

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

* feat(api): inject activity logging into permission routes

Add logPermissionActivity calls to track:
- Permission grants (POST /api/pages/[pageId]/permissions)
- Permission updates (POST with existing permission)
- Permission revocations (DELETE /api/pages/[pageId]/permissions)

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

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

* fix(api): handle null values in activities query params

Zod's .optional() and .default() only work with undefined, not null.
searchParams.get() returns null for missing params, causing validation
errors. Convert null → undefined with ?? operator.

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

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

* feat(ai): add activity logging to all AI tool operations

AI tools now log to the activity system with full attribution:
- Extended ToolExecutionContext with aiProvider, aiModel fields
- Added logging to 11 write tools across 4 tool files
- Pass AI context through experimental_context in 3 API routes
- Export monitoring module from @pagespace/lib/server

Tools now logged: replace_lines, create_page, rename_page, trash,
restore, move_page, edit_sheet_cells, update_agent_config,
update_task, create_drive, rename_drive

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

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

* fix: address PR review comments

- Wrap switch case declarations in blocks (/api/activities/route.ts)
- Change driveId FK to 'set null' for audit trail preservation
- Fix ProviderSetupCard to use inline mode instead of broken handler
- Fix conversationId mapping to use session ID not page ID

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

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

* test: fix mocks for activity logging functions

Add missing mock functions for the new activity logging exports:
- agent-tools.test.ts: add logAgentConfigActivity mock
- page-write-tools.test.ts: add logPageActivity, logDriveActivity mocks
- permissions/route.test.ts: add logPermissionActivity mock (moved to
  @pagespace/lib), add @pagespace/db mock for db.query.pages.findFirst

All 2073 tests now pass.

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

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

* fix: prefix unused provider param with underscore

* Rewire iOS app after web refactor (#97)

* fix(stores): restore missing page header and clean up dead navigation code (#100)

* fix(stores): restore missing page header and clean up dead navigation code

The page header (Share button, breadcrumbs, etc.) was missing from all pages
because OptimizedViewHeader checked layoutStore.activePageId which was always
null - the navigation system was designed but never wired up.

Changes:
- Fix OptimizedViewHeader to use useParams() instead of dead store state
- Remove all dead navigation code from useLayoutStore (352→60 lines)
- Remove duplicate sidebar state from useUIStore (97→50 lines)
- Remove dead cleanup code from NavigationProvider.tsx
- Remove dead sidebar hooks from useUI.ts (keep only useTreeState)
- Update store tests to match new simplified state
- Update state-management.md documentation

The two stores now have clear, non-overlapping responsibilities:
- useLayoutStore: sidebar open/closed state (persisted)
- useUIStore: tree expansion and scroll state (persisted)

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

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

* fix: remove references to deleted clearCache method in layout store

The layout store was simplified to only manage sidebar state, but
DebugPanel and LayoutErrorBoundary still referenced the deleted
clearCache() method and other removed properties (viewCache,
activeDriveId, activePageId, centerViewType).

These were all part of a navigation system that was designed but
never wired up (dead code). The useful functionality is preserved:
- Clear Cache button still clears localStorage/sessionStorage
- Error boundary still clears storage on error

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

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

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

* refactor: extract URL state and agent conversation helpers (#101)

* fix(stores): restore missing page header and clean up dead navigation code

The page header (Share button, breadcrumbs, etc.) was missing from all pages
because OptimizedViewHeader checked layoutStore.activePageId which was always
null - the navigation system was designed but never wired up.

Changes:
- Fix OptimizedViewHeader to use useParams() instead of dead store state
- Remove all dead navigation code from useLayoutStore (352→60 lines)
- Remove duplicate sidebar state from useUIStore (97→50 lines)
- Remove dead cleanup code from NavigationProvider.tsx
- Remove dead sidebar hooks from useUI.ts (keep only useTreeState)
- Update store tests to match new simplified state
- Update state-management.md documentation

The two stores now have clear, non-overlapping responsibilities:
- useLayoutStore: sidebar open/closed state (persisted)
- useUIStore: tree expansion and scroll state (persisted)

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

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

* fix: remove references to deleted clearCache method in layout store

The layout store was simplified to only manage sidebar state, but
DebugPanel and LayoutErrorBoundary still referenced the deleted
clearCache() method and other removed properties (viewCache,
activeDriveId, activePageId, centerViewType).

These were all part of a navigation system that was designed but
never wired up (dead code). The useful functionality is preserved:
- Clear Cache button still clears localStorage/sessionStorage
- Error boundary still clears storage on error

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

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

* refactor: extract URL state and agent conversation helpers

- Add centralized url-state.ts for chat URL param management
- Add shared agent-conversations.ts API helpers (DRY up fetch calls)
- Add UI refresh protection (isPaused) to useBreadcrumbs, usePageTree, useConversations
- Refactor usePageAgentDashboardStore and usePageAgentSidebarState to use shared helpers
- Update state-management.md with AI assistant state boundaries documentation

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

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

* fix: address PR review comments from CodeRabbit

- Fix stale isPaused closure in useBreadcrumbs, usePageTree, useConversations
  (use isEditingActive helper that reads live state via getState())
- Change 'push' to 'replace' when auto-loading most recent conversation
- Add agentId/conversationId context to error messages for debugging
- Update ui-refresh-protection.md docs with correct isPaused pattern
- Add language identifier to code block in state-management.md

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

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

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

* feat(db): add activityLogs table for audit trail

Add new database schema for enterprise activity monitoring:
- New enums: activity_operation, activity_resource
- New activityLogs table with:
  - User attribution with AI context (isAiGenerated, aiProvider, aiModel)
  - Full content snapshots for future rollback support
  - Hierarchical context (driveId, pageId) for filtering
  - Indexed for efficient queries by timestamp, user, drive, page

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

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

* feat(lib): add activity logger service

Add fire-and-forget activity logging functions:
- logActivity(): Core logging function
- logPageActivity(): Page CRUD operations
- logPermissionActivity(): Permission changes
- logDriveActivity(): Drive operations
- logAgentConfigActivity(): Agent configuration changes

Designed to never block user operations while maintaining
comprehensive audit trail for enterprise compliance.

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

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

* feat(api): add /api/activities endpoint

Add context-aware activity fetching endpoint:
- user context: User's own activity (dashboard view)
- drive context: All drive activity (drive view)
- page context: All page edits (page view)

Includes permission checks (canUserViewPage, isUserDriveMember)
and pagination support for large activity lists.

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

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

* feat(ui): replace Settings tab with Activity tab in sidebar

- Add SidebarActivityTab component with context-aware activity display
- Update right sidebar to use Activity tab instead of Settings
- Update GlobalAssistantView to open Activity tab

Activity tab features:
- Search filtering
- User avatars with AI indicator (Bot icon)
- Operation icons (create, update, delete, etc.)
- Relative timestamps
- Loading skeletons

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

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

* refactor(store): update SidebarTab type from 'settings' to 'activity'

Update usePageAgentDashboardStore and tests to use 'activity'
tab instead of 'settings' to match new sidebar structure.

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

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

* feat(api): inject activity logging into page CRUD routes

Add logPageActivity calls to track:
- Page creation (POST /api/pages)
- Page updates (PATCH /api/pages/[pageId])
- Page deletion/trash (DELETE /api/pages/[pageId])
- Page restoration (POST /api/pages/[pageId]/restore)

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

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

* feat(api): inject activity logging into permission routes

Add logPermissionActivity calls to track:
- Permission grants (POST /api/pages/[pageId]/permissions)
- Permission updates (POST with existing permission)
- Permission revocations (DELETE /api/pages/[pageId]/permissions)

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

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

* fix(api): handle null values in activities query params

Zod's .optional() and .default() only work with undefined, not null.
searchParams.get() returns null for missing params, causing validation
errors. Convert null → undefined with ?? operator.

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

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

* feat(ai): add activity logging to all AI tool operations

AI tools now log to the activity system with full attribution:
- Extended ToolExecutionContext with aiProvider, aiModel fields
- Added logging to 11 write tools across 4 tool files
- Pass AI context through experimental_context in 3 API routes
- Export monitoring module from @pagespace/lib/server

Tools now logged: replace_lines, create_page, rename_page, trash,
restore, move_page, edit_sheet_cells, update_agent_config,
update_task, create_drive, rename_drive

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

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

* fix: address PR review comments

- Wrap switch case declarations in blocks (/api/activities/route.ts)
- Change driveId FK to 'set null' for audit trail preservation
- Fix ProviderSetupCard to use inline mode instead of broken handler
- Fix conversationId mapping to use session ID not page ID

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

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

* test: fix mocks for activity logging functions

Add missing mock functions for the new activity logging exports:
- agent-tools.test.ts: add logAgentConfigActivity mock
- page-write-tools.test.ts: add logPageActivity, logDriveActivity mocks
- permissions/route.test.ts: add logPermissionActivity mock (moved to
  @pagespace/lib), add @pagespace/db mock for db.query.pages.findFirst

All 2073 tests now pass.

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

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

* fix: prefix unused provider param with underscore

* refactor: replace ORM chain mocks with repository seams

Create proper architectural boundaries for database operations:

- Add pageRepository with 11 methods (findById, create, update, trash, etc.)
- Add driveRepository with 5 methods (findById, findByIdBasic, etc.)
- Add agentRepository with 2 methods (findById, updateConfig)

Refactor AI tools to use repository seams:
- agent-tools.ts now uses agentRepository
- page-write-tools.ts now uses pageRepository + driveRepository

Rewrite tests to mock repository boundaries:
- agent-tools.test.ts: removed @scaffold label, 9 tests passing
- page-write-tools.test.ts: removed @scaffold label, 23 tests passing

Test scores improved from 6/10 to 9/10 per testing rubric.

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

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

* fix(test): add missing isEditingActive export to usePageTree test mock

The test mock for @/stores/useEditingStore was missing the
isEditingActive named export that usePageTree.ts imports.

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

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

* fix: remove unused db imports from page-write-tools

Removed leftover imports (db, pages, drives, eq, and) that were
replaced by repository seams in the refactor.

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

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

* fix(test): add missing getActorInfo and logPageActivity mocks

Added missing mocks for activity logging functions that were added
to the API routes:
- getActorInfo in @pagespace/lib/server mock
- logPageActivity in @pagespace/lib mock

Fixes 8 failing tests in pages route tests.

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

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

* refactor: implement fire-and-forget activity logging pattern

Replace blocking `await getActorInfo()` calls with non-blocking helpers
to avoid blocking user operations during activity logging:

- Add logPageActivityAsync() for fire-and-forget page activity logging
- Add logDriveActivityAsync() for fire-and-forget drive activity logging
- Update all 9 logging call sites in page-write-tools.ts
- Handle errors gracefully (still log activity even if actor lookup fails)
- Use nullish coalescing for optional context fields
- Fix TypeScript errors in test file with proper type assertions

Addresses CodeRabbit review comment about blocking activity logging
defeating the fire-and-forget design goal.

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

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

* fix: use null instead of undefined for parentId when moving to root

Also add activity logging test assertions to verify logging is called
after successful page operations (replace_lines, create_page, rename_page).

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

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

* fix: prefix unused mockLogDriveActivity with underscore

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

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

---------

Co-authored-by: Claude Opus 4.5 <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