refactor: guard blob url lookup and trim comments

getBlobUrlFromOpenCache now short-circuits on a missing url instead of
calling cache.match(undefined); persons without a cached image hit this.

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

Author: amitsingh-007

apps/extension/src/entrypoints/popup/panels/BookmarksPanel/components/BookmarksPanel.tsx

@@ -45,9 +45,7 @@ function BookmarksPanel({ folderId, operation, bmUrl }: BMPanelQueryParams) {
   );
   const scrollAreaRef = useRef<HTMLDivElement>(null);
   const [searchText, setSearchText] = useState('');
-  // This component re-renders on every row click (selected/cut state), so
-  // without memoing, the whole list is re-filtered and getItemKey's identity
-  // churns on each one
+  // Re-renders on every row click, so memo to keep getItemKey identity stable
   const filteredContextBookmarks = useMemo(
     () => getFilteredContextBookmarks(contextBookmarks, searchText),
     [contextBookmarks, searchText]

apps/extension/src/entrypoints/popup/panels/HomePopup/utils/sync.ts

@@ -63,9 +63,7 @@ export const processPostLogin = async () => {
   // Sync remote firebase to storage
   await syncFirebaseToStorage();
   incrementProgress(SIGN_IN_TOTAL_STEPS);
-  // Independent network-bound cache warms: person images come from personsItem,
-  // favicons from bookmarksItem. addAllToCache shares one pLimit(20), so running
-  // them together costs max() instead of sum() without raising peak concurrency.
+  // Independent cache warms; addAllToCache shares one pLimit so concurrency is capped
   await Promise.all([cachePersonImagesInStorage(), cacheBookmarkFavicons()]);
   incrementProgress(SIGN_IN_TOTAL_STEPS);
   incrementProgress(SIGN_IN_TOTAL_STEPS);

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

@@ -142,10 +142,7 @@ export const test = base.extend<
     }
   },
 
-  // Worker-scoped like every other shared fixture: background-navigation.spec
-  // is describe.serial and each test calls ensureActiveState /
-  // clearHistoryStartTime, so it is self-resetting and does not need a fresh
-  // profile copy + Chromium launch per test.
+  // Safe to share: the spec is describe.serial and each test resets its own state
   sharedBackground: [
     async ({}, use, testInfo) => {
       const { browserContext, userDataDir } = await createSharedContext({

apps/web/src/app/api/upload-file/utils.ts

@@ -19,8 +19,7 @@ export const validateAndProccessFile = async (file: File) => {
     return null;
   }
 
-  // Buffer once and reuse: sniffing and compressing both need the bytes, and
-  // this can be up to 5 MB
+  // Buffer once: sniffing and compressing both need the bytes, up to 5 MB
   const buffer = Buffer.from(await file.arrayBuffer());
 
   // Actual file type validation

apps/web/src/app/helpers/firebase/auth.ts

@@ -38,9 +38,7 @@ export const googleSignOut = async () => signOut(auth);
 export const onAuthStateChange = (callback: (user: User | null) => void) =>
   onAuthStateChanged(auth, callback);
 
-// No force-refresh: this runs before every tRPC request and the SDK already
-// refreshes within 5 mins of expiry, so forcing it added a securetoken round
-// trip to each call
+// No force-refresh: runs before every tRPC request and the SDK auto-refreshes near expiry
 export const getAuthIdToken = async () => auth.currentUser?.getIdToken();
 
 export const emailAndPasswordSignIn = async (

packages/shared/src/components/Persons/hooks/usePerson.ts

@@ -38,10 +38,9 @@ const usePerson = () => {
       }
       const personImages = await getPersonImageUrls();
       if (!personImages) {
-        // Matches resolvePersonImageFromUid: bail before touching CacheStorage
         return persons.map((person) => ({ ...person, imageUrl: '' }));
       }
-      // Open the bucket once for the whole list rather than once per person
+      // Open the bucket once for the whole list
       const cache = await getCacheObj(ECacheBucketKeys.person);
       return Promise.all(
         persons.map(async (person) => ({

packages/shared/src/components/Persons/hooks/useTaggedBookmarks.ts

@@ -16,9 +16,7 @@ const useTaggedBookmarks = (personUid = '') => {
   return useSWR(
     personUid ? ['tagged-bookmarks', personUid] : null,
     async () => {
-      // One read of the whole bookmarks object for the entire list. Resolving
-      // each tagged url through useBookmark/usePerson would re-read and
-      // re-parse it twice per bookmark.
+      // One read for the whole list; the per-hash helpers re-read it twice per bookmark
       const bookmarks = await getBookmarks();
       if (!bookmarks?.urlList) {
         return [];

packages/shared/src/utils/cache.ts

@@ -47,11 +47,11 @@ const getFromCache = async (cacheBucketKey: ECacheBucketKeys, url: string) => {
   return cache.match(url);
 };
 
-/**
- * Takes an already-open Cache so callers resolving many urls can open the
- * bucket once instead of per url.
- */
-export const getBlobUrlFromOpenCache = async (cache: Cache, url: string) => {
+/** Variant taking an already-open Cache, to open the bucket once for many urls. */
+export const getBlobUrlFromOpenCache = async (cache: Cache, url?: string) => {
+  if (!url) {
+    return '';
+  }
   const response = await cache.match(url);
   const blob = await response?.blob();
   if (!blob) {