chore: phase 3 refactorings

TODO.md

@@ -187,9 +187,10 @@ Pure tab-policy helpers that decide:
 ## Phase 3 — Centralize Editor/Tab Management (expanded)
 - [x] Implement shared tab policy layer for singleton tool tabs.
 - [x] Centralize singleton tab policies (`settings`, `tags`, `style`, `documentation`, `metadata-diff`, `site-validation`).
-- Centralize transient/pin lifecycle rules (including promotion behavior).
+- [x] Centralize post/media transient-vs-pinned open intent rules via shared entity-tab policy helper.
-- Remove duplicated tab-open logic from sidebars/nav components where possible.
+- [x] Remove duplicated tab-open logic from sidebars/nav/components for centralized policy-covered tab types.
 - [x] Ensure site-validation tab reopens with persisted last report and refreshes only on explicit validation trigger.
+- [x] Centralize multi-instance tab opening policies for `chat`, `import`, and `git-diff` (file/commit resources).
 
 ### Editor Tab Management Variations (current analysis)
 1. **Singleton, pinned tool tabs**
@@ -253,4 +254,8 @@ Pure tab-policy helpers that decide:
 - [x] Phase 3 slice complete: added singleton tab policy helper and migrated menu/sidebar singleton tool openings to it.
 - [x] Phase 3 slice complete: site validation report is persisted per project and reloaded by `SiteValidationView`.
 - [x] Phase 3 slice complete: site validation view refreshes via explicit `bds:site-validation-updated` event (no auto-validate on mount).
-- [ ] Next implementation slice: Phase 3 continuation (centralize non-singleton tab lifecycle/promotion rules).
+- [x] Phase 3 continuation slice complete: added shared entity tab intent helper (`preview`/`pin`) for `post`/`media`.
+- [x] Phase 3 continuation slice complete: migrated post/media open paths in Sidebar, Editor, LinkedMediaPanel, and Panel to shared entity-tab policy.
+- [x] Phase 3 completion slice: centralized `chat` and `import` tab-open specs and migrated sidebar call sites.
+- [x] Phase 3 completion slice: centralized `git-diff` file/commit ID/spec/open helpers and reused shared parsing in `TabBar`.
+- [ ] Next implementation slice: Phase 4 (editor routing registry).

src/renderer/components/Editor/Editor.tsx

@@ -21,6 +21,7 @@ import { SiteValidationView } from '../SiteValidationView';
 import { AutoSaveManager, getContrastColor } from '../../utils';
 import { InsertModal } from '../InsertModal';
 import { AISuggestionsModal, AISuggestions } from '../AISuggestionsModal/AISuggestionsModal';
+import { openEntityTab } from '../../navigation/tabPolicy';
 import { useI18n } from '../../i18n';
 import './Editor.css';
 
@@ -1102,7 +1103,7 @@ const MediaEditor: React.FC<{ mediaId: string }> = ({ mediaId }) => {
 
   // Handle click on a post to navigate to it
   const handlePostClick = (postId: string) => {
-    openTab({ type: 'post', id: postId, isTransient: true });
+    openEntityTab(openTab, 'post', postId, 'preview');
   };
 
   // Get unlinked posts for picker, filtered by search
@@ -1680,12 +1681,12 @@ const Dashboard: React.FC = () => {
                   onClick={() => {
                     useAppStore.getState().setActiveView('posts');
                     useAppStore.getState().setSelectedPost(post.id);
-                    useAppStore.getState().openTab({ type: 'post', id: post.id, isTransient: true });
+                    openEntityTab(useAppStore.getState().openTab, 'post', post.id, 'preview');
                   }}
                   onDoubleClick={() => {
                     useAppStore.getState().setActiveView('posts');
                     useAppStore.getState().setSelectedPost(post.id);
-                    useAppStore.getState().openTab({ type: 'post', id: post.id, isTransient: false });
+                    openEntityTab(useAppStore.getState().openTab, 'post', post.id, 'pin');
                   }}
                 >
                   <span className="recent-post-title">{post.title || tr('editor.untitled')}</span>

src/renderer/components/GitSidebar/GitSidebar.tsx

@@ -1,5 +1,6 @@
 import React, { useCallback, useEffect, useRef, useState } from 'react';
 import { useAppStore } from '../../store';
+import { openGitDiffCommitTab, openGitDiffFileTab } from '../../navigation/tabPolicy';
 import { useI18n } from '../../i18n';
 import type { GitInitProgress, GitHistoryEntry, GitRemoteStateDto } from '../../../main/shared/electronApi';
 import './GitSidebar.css';
@@ -125,9 +126,6 @@ export const GitSidebar: React.FC = () => {
     [tr],
   );
 
-  const getDiffTabId = (filePath: string): string => `git-diff:${filePath}`;
-  const getCommitDiffTabId = (commitHash: string): string => `git-diff:commit:${commitHash}`;
-
   const getActionProgressMessage = (action: 'fetch' | 'pull' | 'push' | 'prune-lfs' | 'commit'): string => {
     if (action === 'push') {
       return tr('gitSidebar.progress.pushingRemote');
@@ -156,22 +154,14 @@ export const GitSidebar: React.FC = () => {
 
   const openDiffTab = useCallback(
     (filePath: string, isTransient: boolean) => {
-      openTab({
+      openGitDiffFileTab(openTab, filePath, isTransient ? 'preview' : 'pin');
-        type: 'git-diff',
-        id: getDiffTabId(filePath),
-        isTransient,
-      });
     },
     [openTab],
   );
 
   const openCommitDiffTab = useCallback(
     (commitHash: string, isTransient: boolean) => {
-      openTab({
+      openGitDiffCommitTab(openTab, commitHash, isTransient ? 'preview' : 'pin');
-        type: 'git-diff',
-        id: getCommitDiffTabId(commitHash),
-        isTransient,
-      });
     },
     [openTab],
   );

src/renderer/components/LinkedMediaPanel/LinkedMediaPanel.tsx

@@ -11,6 +11,7 @@
 
 import React, { useState, useEffect, useCallback } from 'react';
 import { useAppStore, MediaData } from '../../store';
+import { openEntityTab } from '../../navigation/tabPolicy';
 import { showToast } from '../Toast';
 import { useI18n } from '../../i18n';
 import './LinkedMediaPanel.css';
@@ -197,7 +198,7 @@ export const LinkedMediaPanel: React.FC<LinkedMediaPanelProps> = ({
 
   // Handle click on media item to open media viewer
   const handleMediaClick = (mediaId: string) => {
-    useAppStore.getState().openTab({ type: 'media', id: mediaId, isTransient: true });
+    openEntityTab(useAppStore.getState().openTab, 'media', mediaId, 'preview');
   };
 
   // Get thumbnail URL for a media item

src/renderer/components/Panel/Panel.tsx

@@ -1,6 +1,7 @@
 import React, { useEffect, useMemo, useRef, useState } from 'react';
 import { useAppStore } from '../../store';
 import type { TaskProgress } from '../../../main/shared/electronApi';
+import { openEntityTab } from '../../navigation/tabPolicy';
 import { useI18n } from '../../i18n';
 import './Panel.css';
 
@@ -296,7 +297,7 @@ export const Panel: React.FC = () => {
   }
 
   const handlePostLinkClick = (postId: string) => {
-    openTab({ type: 'post', id: postId, isTransient: false });
+    openEntityTab(openTab, 'post', postId, 'pin');
     setSelectedPost(postId);
     setActiveView('posts');
   };

src/renderer/components/Sidebar/Sidebar.tsx

@@ -8,7 +8,7 @@ import { scrollToSettingsSection, SettingsCategory } from '../SettingsView/Setti
 import { scrollToTagsSection, TagsCategory } from '../TagsView';
 import { activateSidebarSection } from '../../navigation/sectionActivation';
 import { getPersistedSidebarSection, setPersistedSidebarSection } from '../../navigation/sidebarUiPersistence';
-import { openSingletonToolTab } from '../../navigation/tabPolicy';
+import { openChatTab, openEntityTab, openImportTab, openSingletonToolTab } from '../../navigation/tabPolicy';
 import type { SidebarView } from '../../navigation/sidebarViewRegistry';
 import { useI18n } from '../../i18n';
 import './Sidebar.css';
@@ -805,11 +805,11 @@ const PostsList: React.FC<PostsListProps> = ({ mode, isActive }) => {
 
   // Click handlers for tabs
   const handlePostClick = (postId: string) => {
-    openTab({ type: 'post', id: postId, isTransient: true });
+    openEntityTab(openTab, 'post', postId, 'preview');
   };
 
   const handlePostDoubleClick = (postId: string) => {
-    openTab({ type: 'post', id: postId, isTransient: false });
+    openEntityTab(openTab, 'post', postId, 'pin');
   };
 
   return (
@@ -1104,11 +1104,11 @@ const MediaList: React.FC = () => {
   };
 
   const handleMediaClick = (mediaId: string) => {
-    openTab({ type: 'media', id: mediaId, isTransient: true });
+    openEntityTab(openTab, 'media', mediaId, 'preview');
   };
 
   const handleMediaDoubleClick = (mediaId: string) => {
-    openTab({ type: 'media', id: mediaId, isTransient: false });
+    openEntityTab(openTab, 'media', mediaId, 'pin');
   };
 
   // Determine which media to display
@@ -1430,7 +1430,7 @@ const ChatList: React.FC = () => {
       const conversation = await window.electronAPI?.chat.createConversation();
       if (conversation) {
         setConversations(prev => [conversation, ...prev]);
-        openTab({ type: 'chat', id: conversation.id, isTransient: false });
+        openChatTab(openTab, conversation.id);
       }
     } catch (error) {
       console.error('Failed to create conversation:', error);
@@ -1439,7 +1439,7 @@ const ChatList: React.FC = () => {
   };
 
   const handleOpenChat = (conversationId: string) => {
-    openTab({ type: 'chat', id: conversationId, isTransient: false });
+    openChatTab(openTab, conversationId);
   };
 
   const handleDeleteChat = async (conversationId: string) => {
@@ -1580,7 +1580,7 @@ const ImportList: React.FC = () => {
       const def = await window.electronAPI?.importDefinitions.create();
       if (def) {
         setDefinitions(prev => [def, ...prev]);
-        openTab({ type: 'import', id: def.id, isTransient: false });
+        openImportTab(openTab, def.id);
       }
     } catch (error) {
       console.error('Failed to create import definition:', error);
@@ -1589,7 +1589,7 @@ const ImportList: React.FC = () => {
   };
 
   const handleOpenDefinition = (definitionId: string) => {
-    openTab({ type: 'import', id: definitionId, isTransient: false });
+    openImportTab(openTab, definitionId);
   };
 
   const handleDeleteDefinition = async (e: React.MouseEvent, definitionId: string) => {

src/renderer/components/TabBar/TabBar.tsx

@@ -1,22 +1,11 @@
 import React, { useRef, useState, useEffect, useCallback } from 'react';
 import { useAppStore, Tab } from '../../store';
+import { parseGitDiffTabId } from '../../navigation/tabPolicy';
 import { useI18n } from '../../i18n';
 import './TabBar.css';
 
 const MAX_CHAT_TITLE_LENGTH = 18;
 
-function getGitDiffResource(tabId: string): string {
-  return tabId.startsWith('git-diff:') ? tabId.slice('git-diff:'.length) : tabId;
-}
-
-function getCommitHashFromGitDiffTabId(tabId: string): string | null {
-  const resource = getGitDiffResource(tabId);
-  if (!resource.startsWith('commit:')) {
-    return null;
-  }
-  return resource.slice('commit:'.length);
-}
-
 const getTabTitle = (
   tab: Tab,
   postTitles: Map<string, string>,
@@ -27,8 +16,7 @@ const getTabTitle = (
   tr: (key: string, vars?: Record<string, string | number>) => string,
 ): string => {
   if (tab.type === 'git-diff') {
-    const filePath = getGitDiffResource(tab.id);
+    const { resource: filePath, commitHash } = parseGitDiffTabId(tab.id);
-    const commitHash = getCommitHashFromGitDiffTabId(tab.id);
     if (commitHash) {
       const commitTitle = commitTitles.get(commitHash);
       if (commitTitle) {
@@ -380,7 +368,7 @@ export const TabBar: React.FC = () => {
   useEffect(() => {
     const commitHashes = tabs
       .filter((tab) => tab.type === 'git-diff')
-      .map((tab) => getCommitHashFromGitDiffTabId(tab.id))
+      .map((tab) => parseGitDiffTabId(tab.id).commitHash)
       .filter((hash): hash is string => Boolean(hash));
 
     if (commitHashes.length === 0 || !activeProject) {

src/renderer/navigation/tabPolicy.ts

@@ -14,6 +14,10 @@ export interface CanonicalTabSpec {
   isTransient: boolean;
 }
 
+export type EntityTabType = 'post' | 'media';
+export type EntityTabOpenIntent = 'preview' | 'pin';
+export type GitDiffResourceOpenIntent = 'preview' | 'pin';
+
 const SINGLETON_TOOL_TAB_REGISTRY: Record<SingletonToolTabKey, CanonicalTabSpec> = {
   settings: { type: 'settings', id: 'settings', isTransient: false },
   tags: { type: 'tags', id: 'tags', isTransient: false },
@@ -33,3 +37,112 @@ export function openSingletonToolTab(
 ): void {
   openTab(getSingletonToolTabSpec(key));
 }
+
+export function getEntityTabSpec(
+  type: EntityTabType,
+  id: string,
+  intent: EntityTabOpenIntent,
+): CanonicalTabSpec {
+  return {
+    type,
+    id,
+    isTransient: intent === 'preview',
+  };
+}
+
+export function openEntityTab(
+  openTab: (tab: CanonicalTabSpec) => void,
+  type: EntityTabType,
+  id: string,
+  intent: EntityTabOpenIntent,
+): void {
+  openTab(getEntityTabSpec(type, id, intent));
+}
+
+export function getChatTabSpec(conversationId: string): CanonicalTabSpec {
+  return {
+    type: 'chat',
+    id: conversationId,
+    isTransient: false,
+  };
+}
+
+export function openChatTab(
+  openTab: (tab: CanonicalTabSpec) => void,
+  conversationId: string,
+): void {
+  openTab(getChatTabSpec(conversationId));
+}
+
+export function getImportTabSpec(definitionId: string): CanonicalTabSpec {
+  return {
+    type: 'import',
+    id: definitionId,
+    isTransient: false,
+  };
+}
+
+export function openImportTab(
+  openTab: (tab: CanonicalTabSpec) => void,
+  definitionId: string,
+): void {
+  openTab(getImportTabSpec(definitionId));
+}
+
+export function getGitDiffFileTabId(filePath: string): string {
+  return `git-diff:${filePath}`;
+}
+
+export function getGitDiffCommitTabId(commitHash: string): string {
+  return `git-diff:commit:${commitHash}`;
+}
+
+export function getGitDiffFileTabSpec(
+  filePath: string,
+  intent: GitDiffResourceOpenIntent,
+): CanonicalTabSpec {
+  return {
+    type: 'git-diff',
+    id: getGitDiffFileTabId(filePath),
+    isTransient: intent === 'preview',
+  };
+}
+
+export function getGitDiffCommitTabSpec(
+  commitHash: string,
+  intent: GitDiffResourceOpenIntent,
+): CanonicalTabSpec {
+  return {
+    type: 'git-diff',
+    id: getGitDiffCommitTabId(commitHash),
+    isTransient: intent === 'preview',
+  };
+}
+
+export function openGitDiffFileTab(
+  openTab: (tab: CanonicalTabSpec) => void,
+  filePath: string,
+  intent: GitDiffResourceOpenIntent,
+): void {
+  openTab(getGitDiffFileTabSpec(filePath, intent));
+}
+
+export function openGitDiffCommitTab(
+  openTab: (tab: CanonicalTabSpec) => void,
+  commitHash: string,
+  intent: GitDiffResourceOpenIntent,
+): void {
+  openTab(getGitDiffCommitTabSpec(commitHash, intent));
+}
+
+export function parseGitDiffTabId(tabId: string): { resource: string; commitHash: string | null } {
+  const resource = tabId.startsWith('git-diff:') ? tabId.slice('git-diff:'.length) : tabId;
+  if (!resource.startsWith('commit:')) {
+    return { resource, commitHash: null };
+  }
+
+  return {
+    resource,
+    commitHash: resource.slice('commit:'.length),
+  };
+}

tests/renderer/navigation/tabPolicy.test.ts

@@ -1,5 +1,19 @@
 import { describe, expect, it } from 'vitest';
-import { getSingletonToolTabSpec, openSingletonToolTab } from '../../../src/renderer/navigation/tabPolicy';
+import {
+  getChatTabSpec,
+  getEntityTabSpec,
+  getGitDiffCommitTabSpec,
+  getGitDiffFileTabSpec,
+  getImportTabSpec,
+  parseGitDiffTabId,
+  openChatTab,
+  getSingletonToolTabSpec,
+  openEntityTab,
+  openGitDiffCommitTab,
+  openGitDiffFileTab,
+  openImportTab,
+  openSingletonToolTab,
+} from '../../../src/renderer/navigation/tabPolicy';
 
 describe('tabPolicy', () => {
   it('provides canonical singleton tab specs', () => {
@@ -22,4 +36,97 @@ describe('tabPolicy', () => {
 
     expect(captured).toEqual({ type: 'site-validation', id: 'site-validation', isTransient: false });
   });
+
+  it('provides canonical entity tab spec for preview and pin intents', () => {
+    expect(getEntityTabSpec('post', 'post-1', 'preview')).toEqual({
+      type: 'post',
+      id: 'post-1',
+      isTransient: true,
+    });
+
+    expect(getEntityTabSpec('media', 'media-1', 'pin')).toEqual({
+      type: 'media',
+      id: 'media-1',
+      isTransient: false,
+    });
+  });
+
+  it('opens entity tabs from canonical policy', () => {
+    let captured: { type: string; id: string; isTransient: boolean } | null = null;
+    const openTab = (tab: { type: string; id: string; isTransient: boolean }) => {
+      captured = tab;
+    };
+
+    openEntityTab(openTab, 'post', 'post-2', 'pin');
+
+    expect(captured).toEqual({ type: 'post', id: 'post-2', isTransient: false });
+  });
+
+  it('provides canonical chat and import tab specs', () => {
+    expect(getChatTabSpec('conversation-1')).toEqual({
+      type: 'chat',
+      id: 'conversation-1',
+      isTransient: false,
+    });
+
+    expect(getImportTabSpec('definition-1')).toEqual({
+      type: 'import',
+      id: 'definition-1',
+      isTransient: false,
+    });
+  });
+
+  it('opens canonical chat and import tabs', () => {
+    const opened: Array<{ type: string; id: string; isTransient: boolean }> = [];
+    const openTab = (tab: { type: string; id: string; isTransient: boolean }) => {
+      opened.push(tab);
+    };
+
+    openChatTab(openTab, 'conversation-2');
+    openImportTab(openTab, 'definition-2');
+
+    expect(opened).toEqual([
+      { type: 'chat', id: 'conversation-2', isTransient: false },
+      { type: 'import', id: 'definition-2', isTransient: false },
+    ]);
+  });
+
+  it('builds and parses git-diff file and commit tab specs', () => {
+    expect(getGitDiffFileTabSpec('posts/first.md', 'preview')).toEqual({
+      type: 'git-diff',
+      id: 'git-diff:posts/first.md',
+      isTransient: true,
+    });
+
+    expect(getGitDiffCommitTabSpec('abc123def', 'pin')).toEqual({
+      type: 'git-diff',
+      id: 'git-diff:commit:abc123def',
+      isTransient: false,
+    });
+
+    expect(parseGitDiffTabId('git-diff:posts/first.md')).toEqual({
+      resource: 'posts/first.md',
+      commitHash: null,
+    });
+
+    expect(parseGitDiffTabId('git-diff:commit:abc123def')).toEqual({
+      resource: 'commit:abc123def',
+      commitHash: 'abc123def',
+    });
+  });
+
+  it('opens git-diff file and commit tabs from shared policy', () => {
+    const opened: Array<{ type: string; id: string; isTransient: boolean }> = [];
+    const openTab = (tab: { type: string; id: string; isTransient: boolean }) => {
+      opened.push(tab);
+    };
+
+    openGitDiffFileTab(openTab, 'posts/second.md', 'preview');
+    openGitDiffCommitTab(openTab, 'def456', 'pin');
+
+    expect(opened).toEqual([
+      { type: 'git-diff', id: 'git-diff:posts/second.md', isTransient: true },
+      { type: 'git-diff', id: 'git-diff:commit:def456', isTransient: false },
+    ]);
+  });
 });