removed pinned tabs feature

[sawka]

No description provided.

[coderabbitai]

Walkthrough

This pull request removes the pinned tabs feature from the application across frontend, backend, and database layers. It includes a database migration that merges pinnedtabids into the main tabids array; removal of ChangeTabPinning from the workspace service; updates to CreateTab and UpdateTabIds method signatures to drop pinned parameters; removal of the pinnedtabids field from the Workspace type definition; elimination of pinned tab UI elements and state management; and simplification of tab management logic to use a single unified tab ID list throughout the codebase.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	No description was provided by the author; this is a completely empty pull request description.	Add a meaningful description explaining the rationale for removing pinned tabs and any migration considerations for users.
Docstring Coverage	⚠️ Warning	Docstring coverage is 11.54% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title accurately summarizes the main objective: removing the pinned tabs feature across the entire codebase.

✨ Finishing touches

• 📝 Generate docstrings

---

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e671242 and fa37776.

📒 Files selected for processing (19)

• Taskfile.yml
• db/migrations-wstore/000010_merge_pinned_tabs.down.sql
• db/migrations-wstore/000010_merge_pinned_tabs.up.sql
• emain/emain-window.ts
• frontend/app/aipanel/waveai-model.tsx
• frontend/app/store/keymodel.ts
• frontend/app/store/services.ts
• frontend/app/tab/tab.tsx
• frontend/app/tab/tabbar.tsx
• frontend/app/view/vdom/vdom.tsx
• frontend/types/gotypes.d.ts
• frontend/wave.ts
• pkg/service/workspaceservice/workspaceservice.go
• pkg/waveobj/wtype.go
• pkg/wcore/window.go
• pkg/wcore/workspace.go
• pkg/wshrpc/wshserver/resolvers.go
• pkg/wshrpc/wshserver/wshserver.go
• pkg/wstore/wstore_dbops.go

💤 Files with no reviewable changes (3)

• frontend/types/gotypes.d.ts
• frontend/app/aipanel/waveai-model.tsx
• pkg/wstore/wstore_dbops.go

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2024-12-27T22:06:04.948Z

Learnt from: Jalileh
Repo: wavetermdev/waveterm PR: 1623
File: pkg/wcore/workspace.go:0-0
Timestamp: 2024-12-27T22:06:04.948Z
Learning: We should confirm test coverage for "force deletion vs. protected named workspaces" in the workspace tests (e.g., verifying that named, non-empty workspaces cannot be deleted without force).

Applied to files:

• pkg/wcore/workspace.go

🧬 Code graph analysis (6)

frontend/app/tab/tabbar.tsx (3)

frontend/util/util.ts (1)

• fireAndForget (513-513)

frontend/app/store/services.ts (1)

• WorkspaceService (190-190)

pkg/service/workspaceservice/workspaceservice.go (1)

• WorkspaceService (23-23)

emain/emain-window.ts (1)

frontend/app/store/services.ts (1)

• WorkspaceService (190-190)

pkg/wcore/window.go (2)

frontend/app/store/services.ts (1)

• CreateTab (142-144)

pkg/wcore/workspace.go (1)

• CreateTab (199-238)

pkg/waveobj/wtype.go (1)

pkg/waveobj/metamap.go (1)

• MetaMapType (8-8)

frontend/wave.ts (1)

pkg/waveobj/wtype.go (4)

• Tab (186-193)
• Tab (195-197)
• Workspace (171-180)
• Workspace (182-184)

pkg/service/workspaceservice/workspaceservice.go (1)

pkg/wcore/workspace.go (2)

• CreateTab (199-238)
• UpdateWorkspaceTabIds (346-354)

⏰ 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). (3)

• GitHub Check: Analyze (go)
• GitHub Check: Analyze (javascript-typescript)
• GitHub Check: Build for TestDriver.ai

🔇 Additional comments (31)

Taskfile.yml (1)

438-443: LGTM! Useful addition for TypeScript validation.

The new check:ts task provides a convenient way to typecheck the frontend and electron code without emitting files. This is valuable for CI/CD pipelines and local development validation.

db/migrations-wstore/000010_merge_pinned_tabs.up.sql (1)

1-23: LGTM! Migration correctly merges pinned tabs into the main tab list.

The migration properly:

• Preserves tab order by placing pinned tabs first (src=0) followed by regular tabs (src=1)
• Uses safe WHERE clauses to only update when pinnedtabids exists and is non-empty
• Cleanly removes the deprecated field in a second step

db/migrations-wstore/000010_merge_pinned_tabs.down.sql (1)

1-2: LGTM! Appropriately marked as irreversible.

The down migration correctly notes that the operation cannot be reversed since the distinction between pinned and regular tabs is lost during the merge. This is standard practice for migrations that consolidate data.

frontend/app/store/keymodel.ts (2)

252-254: LGTM! Simplified to use unified tab list.

The getAllTabs function now correctly returns only ws.tabids, consistent with the removal of the pinned tabs feature. This simplifies tab navigation and switching logic throughout the codebase.

---

518-521: LGTM! Simplified tab close behavior.

The Cmd:Shift:w shortcut now directly closes the static tab without special handling for pinned tabs. This aligns with the PR objective to remove pinned tab functionality and simplifies the keyboard shortcut behavior.

frontend/app/view/vdom/vdom.tsx (1)

509-509: Remove this comment - makeVDomModel was never exported from this file.

A search of the entire codebase found no reference to makeVDomModel. The current export statement is export { VDomView }; and there is no evidence that makeVDomModel was ever exported from this file or used anywhere else in the codebase. This comment is based on an incorrect assumption.

Likely an incorrect or invalid review comment.

pkg/wcore/window.go (1)

184-186: LGTM! Correctly simplified to use only TabIds.

The condition now checks only ws.TabIds length, and the CreateTab call correctly passes four arguments matching the updated signature (removed the pinned flag parameter). This aligns with the removal of pinned tabs throughout the codebase.

pkg/wshrpc/wshserver/wshserver.go (1)

909-909: LGTM! Correctly iterates over unified tab list.

The loop now processes only wsData.TabIds, eliminating the separate handling of pinned tabs. This ensures BlocksListCommand returns blocks from all tabs in the workspace's unified tab list.

frontend/wave.ts (2)

131-136: LGTM! Correctly simplified to use only tabids.

The condition now checks only ws.tabids?.length, and the function iterates solely over ws.tabids. This correctly removes the handling of pinned tabs from the workspace reload flow.

---

140-146: LGTM! Correctly simplified to use only tabids.

Parallel to reloadAllWorkspaceTabs, this function now checks and iterates only over ws.tabids, removing pinned tab handling from the workspace load flow.

pkg/waveobj/wtype.go (1)

171-180: LGTM! Core type definition correctly updated.

The PinnedTabIds field has been cleanly removed from the Workspace struct, leaving a unified TabIds field for all tab management. This is the foundational change that enables the removal of pinned tabs throughout the application.

pkg/wshrpc/wshserver/resolvers.go (1)

159-166: LGTM! Tab resolution logic correctly simplified.

The function now calculates total tabs using only len(ws.TabIds) and directly resolves tab IDs from ws.TabIds[tabIdx]. This removes the complexity of handling separate pinned and regular tab lists, making tab number resolution straightforward.

emain/emain-window.ts (3)

114-135: LGTM! Pinned tab state correctly removed from action queue and workspace validation.

The removal of the pinned field from the createtab action entry and the simplification of isNonEmptyUnsavedWorkspace to check only workspace.tabids?.length > 1 are consistent with the feature removal.

---

498-500: LGTM! Method signature correctly simplified.

The queueCreateTab method no longer accepts a pinned parameter, aligning with the service layer changes.

---

539-539: LGTM! Service call correctly updated to match new signature.

The WorkspaceService.CreateTab call now passes three arguments (workspaceId, tabName, activateTab) instead of four, correctly reflecting the removal of the pinned parameter. The hardcoded true for activateTab is appropriate for the createtab action.

frontend/app/tab/tab.tsx (2)

16-34: LGTM! Tab component props correctly simplified.

The removal of isPinned and onPinChange from the TabProps interface and component destructuring is consistent with the pinned tabs feature removal.

---

136-219: LGTM! Context menu and button rendering correctly updated.

The handleContextMenu dependency array has been simplified, and the close button is now always rendered without conditional logic based on pinned state. This is the expected behavior after removing the pinned tabs feature.

frontend/app/tab/tabbar.tsx (3)

207-218: LGTM! Tab initialization correctly simplified.

The workspace tab synchronization now uses only workspace.tabids without handling pinned tabs separately. The dependency array and comparison logic are appropriately simplified.

---

482-495: LGTM! Debounced tab update correctly simplified.

The setUpdatedTabsDebounced function signature now accepts only tabIds as a parameter, and the WorkspaceService.UpdateTabIds call has been updated to pass only the workspace ID and tab IDs, correctly reflecting the removal of the pinned tabs parameter.

---

497-525: LGTM! Mouse up handler correctly updated.

The call to setUpdatedTabsDebounced on line 511 now passes only tabIds, consistent with the simplified function signature.

frontend/app/store/services.ts (2)

123-144: LGTM! CreateTab service method correctly updated.

The CreateTab method signature and metadata have been correctly updated to remove the pinned parameter, aligning with the backend service changes.

---

168-182: LGTM! UpdateTabIds service method correctly updated.

The UpdateTabIds method signature and metadata have been correctly simplified to remove the pinnedTabIds parameter, maintaining consistency with the backend service layer.

pkg/service/workspaceservice/workspaceservice.go (2)

123-166: LGTM! CreateTab method correctly updated.

The CreateTab method signature has been simplified to remove the pinned parameter. The call to wcore.CreateTab on line 154 correctly passes false for the isInitialLaunch parameter, which is appropriate for regular tab creation (as opposed to initial app launch).

---

168-184: LGTM! UpdateTabIds method correctly simplified.

The UpdateTabIds method has been appropriately updated to remove the pinnedTabIds parameter. The call to wcore.UpdateWorkspaceTabIds on line 179 now passes only the workspace ID and tab IDs, correctly reflecting the removal of pinned tab handling. The log statement on line 175 has also been simplified accordingly.

pkg/wcore/workspace.go (7)

53-58: LGTM! Workspace initialization correctly updated.

The workspace initialization properly removes the PinnedTabIds field and retains only TabIds as a single unified tab list.

---

130-141: LGTM! Deletion logic correctly simplified.

The workspace deletion logic properly consolidates to use only TabIds, removing the separate PinnedTabIds handling. The force-deletion check and tab iteration are correct.

---

240-261: LGTM! Tab creation logic correctly simplified.

The createTabObj function signature and implementation are correctly updated to remove pinned tab handling. All tabs are now added to the unified TabIds array.

---

267-321: LGTM! Tab deletion and active tab selection logic are correct.

The deletion logic correctly:

1. Removes the tab from the unified TabIds array (lines 274-278)
2. Selects the appropriate new active tab using max(0, min(tabIdx-1, len(ws.TabIds)-1)) (line 295), which correctly chooses the previous tab or clamps to the valid range

---

199-237: Breaking API change: pinned parameter removed from CreateTab signature.

All call sites have been properly updated to use the new 5-parameter signature (workspaceId, tabName, activateTab, isInitialLaunch). Three call sites verified:

• pkg/wcore/window.go:186
• pkg/wcore/workspace.go:63
• pkg/service/workspaceservice/workspaceservice.go:154

---

1-430: All pinned tabs removal verifications passed.

The database migration (000010_merge_pinned_tabs) correctly merges pinnedtabids into tabids while preserving tab order (pinned tabs first, then unpinned). The PinnedTabIds field has been successfully removed from the Workspace struct, and no lingering references to PinnedTabIds or ChangeTabPinning exist in the codebase. The code changes are complete and consistent throughout.

---

346-354: Breaking API change confirmed: pinnedTabIds parameter removed.

The UpdateWorkspaceTabIds function signature has been changed to remove the pinnedTabIds parameter. The single call site in pkg/service/workspaceservice/workspaceservice.go (line 179) has been correctly updated to use the new three-parameter signature.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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