refactor: address review feedback on reuse cleanups

- ContextMenu keys on id rather than text, which can repeat across options
- Restore the original branch-per-condition form of
  getFilteredContextBookmarks; the flattened boolean chain read worse
- Trim comments down to the non-obvious reasoning

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: amitsingh-007

apps/extension/src/entrypoints/popup/components/ContextMenu.tsx

@@ -39,7 +39,7 @@ function ContextMenuWrapper({ options, children }: Props) {
       <ContextMenuContent className="min-w-40">
         {options.map(({ id, text, icon, variant, onClick }) => (
           <ContextMenuItem
-            key={text}
+            key={id}
             data-testid={`context-menu-item-${id}`}
             className="gap-2"
             variant={variant}

apps/extension/tests/fixtures/base-fixture.ts

@@ -22,10 +22,6 @@ interface CachedStorageData {
 export const getPopupUrl = (extensionId: string) =>
   `chrome-extension://${extensionId}/popup.html`;
 
-/**
- * Launch Chromium with the extension loaded. The CI-sensitive flags live here
- * only, so they cannot drift between the auth setup and the fixtures.
- */
 export const launchExtensionContext = async ({
   userDataDir,
   extensionPath = getExtensionPath(),
@@ -201,11 +197,7 @@ export interface SharedExtensionWorkerFixtures {
   sharedExtensionId: string;
 }
 
-/**
- * Worker-scoped extension environment shared by every panel fixture: one copied
- * Chrome profile, one background service worker, one extension id per worker.
- * Panel fixtures extend this and add only their own page fixture.
- */
+/** Worker-scoped extension env shared by every panel fixture. */
 export const sharedExtensionTest = base.extend<
   { context: BrowserContext },
   SharedExtensionWorkerFixtures

apps/web/tests/page-object-models/persons-panel.ts

@@ -95,8 +95,7 @@ export class PersonsPanel {
 
   async verifyModalVisible() {
     await verifyModalVisible(this.page, MODAL_TEST_ID);
-    // Back button only renders while the modal is open, so it is the stronger
-    // signal that the modal actually mounted its content
+    // Back button only renders while the modal is open
     const backButton = this.getModal().getByRole('button', { name: 'Back' });
     await expect(backButton).toBeVisible();
   }

packages/shared/src/components/Bookmarks/utils/index.ts

@@ -19,13 +19,15 @@ export const getFilteredContextBookmarks = (
   contextBookmarks: ContextBookmarks,
   searchText: string
 ) =>
-  contextBookmarks.filter(
-    (ctx) =>
-      !searchText ||
-      ctx.isDir ||
-      hasText(searchText, ctx.url) ||
-      hasText(searchText, ctx.title)
-  );
+  contextBookmarks.filter((ctx) => {
+    if (!searchText) {
+      return true;
+    }
+    if (ctx.isDir) {
+      return true;
+    }
+    return hasText(searchText, ctx.url) || hasText(searchText, ctx.title);
+  });
 
 export const getEncryptedBookmark = (
   bookmark: IEncodedBookmark

packages/shared/src/components/Persons/components/Person.tsx

@@ -13,7 +13,6 @@ interface Props {
 function Person({ person }: Props) {
   const { location } = use(DynamicContext);
   const { uid, name } = person;
-  // SWR keyed by uid, so grid cells share one resolved blob URL across remounts
   const { data: imageUrl = '' } = usePersonImage(uid);
 
   const openBookmarksList = () => {