fix: address review feedback and add a Bookmarked tab to the Reader

Per review:
- useToggleBookmark now fetches the bookmark list fresh from relays
  right before writing instead of trusting the query cache (60s
  staleTime), which could otherwise clobber concurrent edits from
  another tab or device.
- The article BookmarkButton only renders when the article actually
  has a `d` tag, instead of falling back to an unresolvable
  "kind:pubkey:" address.
- Bookmarked note/article ids are filtered for a non-empty tag value
  before use, and article addresses are parsed properly (kind,
  author, `d`) instead of a naive split(':')[2] — the relay query is
  now also constrained by kind and author, not just `d`, and
  identifiers containing ':' round-trip correctly.
- BookmarkButton sets type="button" so it can't misbehave as a form
  submit button.

Per a reviewer comment: added a "Recent" / "Bookmarked" tab to the
Reader's sidebar (src/apps/articles/index.tsx) so bookmarked articles
are reachable without leaving the app — the dedicated Bookmarks app
stays as-is. Both now share useMyBookmarkedArticles from
src/hooks/useBookmarks.ts rather than duplicating the address-parsing
logic.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BYiUtZMQeA5RHggQw73wto

src/apps/articles/index.tsx

@@ -1,7 +1,7 @@
-import { useCallback, useEffect } from 'react';
+import { useCallback, useEffect, useState } from 'react';
 import { useNostr } from '@nostrify/react';
 import { useQuery } from '@tanstack/react-query';
-import { ChevronLeft, Link2 } from 'lucide-react';
+import { Bookmark, ChevronLeft, Link2, Rss } from 'lucide-react';
 import type { NostrEvent } from '@nostrify/nostrify';
 import {
   AppBody,
@@ -19,6 +19,8 @@ import { Button } from '@/components/ui/button';
 import { Skeleton } from '@/components/ui/skeleton';
 import { nip19 } from 'nostr-tools';
 import { useAuthor } from '@/hooks/useAuthor';
+import { useCurrentUser } from '@/hooks/useCurrentUser';
+import { useMyBookmarkedArticles } from '@/hooks/useBookmarks';
 import {
   absoluteTime,
   decodeRelayHints,
@@ -33,6 +35,8 @@ import { useToast } from '@/hooks/useToast';
 import { cn } from '@/lib/utils';
 import type { AppParams, AppProps } from '@/os/types';
 
+type ListScope = 'recent' | 'bookmarked';
+
 const ARTICLE_KIND = 30023;
 
 /** A long-form event is only useful with a body and a `d` identifier (NIP-23). */
@@ -85,6 +89,13 @@ function useArticle(
 
 export default function ArticlesApp({ params, setTitle, setParams }: AppProps) {
   const isMobile = useIsMobile();
+  const { user } = useCurrentUser();
+
+  // Signing out mid-session must not strand the reader on a "Bookmarked" tab
+  // it can no longer see anything in, so the effective scope is derived
+  // rather than corrected after render — same reasoning as the Feed.
+  const [requestedScope, setRequestedScope] = useState<ListScope>('recent');
+  const scope: ListScope = user ? requestedScope : 'recent';
 
   // The selection lives in the window's params rather than local state, so the
   // URL, a reload and the switch between the desktop and mobile shells all
@@ -103,7 +114,9 @@ export default function ArticlesApp({ params, setTitle, setParams }: AppProps) {
     [setParams],
   );
 
-  const list = useRecentArticles();
+  const recent = useRecentArticles();
+  const bookmarked = useMyBookmarkedArticles();
+  const list = scope === 'recent' ? recent : bookmarked;
   const article = useArticle(
     selected?.pubkey,
     selected?.identifier,
@@ -117,13 +130,19 @@ export default function ArticlesApp({ params, setTitle, setParams }: AppProps) {
   }, [title, setTitle]);
 
   const listPane = (
-    <ArticleList
-      query={list}
-      selected={selected}
-      onSelect={(event, identifier) =>
-        select({ pubkey: event.pubkey, identifier, kind: String(event.kind) })
-      }
-    />
+    <div className="flex h-full min-h-0 flex-col">
+      <ListScopeTabs scope={requestedScope} disabled={!user} onChange={setRequestedScope} />
+      <div className="os-scroll min-h-0 flex-1 overflow-y-auto">
+        <ArticleList
+          query={list}
+          scope={scope}
+          selected={selected}
+          onSelect={(event, identifier) =>
+            select({ pubkey: event.pubkey, identifier, kind: String(event.kind) })
+          }
+        />
+      </div>
+    </div>
   );
 
   const readerPane = !selected ? (
@@ -176,12 +195,14 @@ export default function ArticlesApp({ params, setTitle, setParams }: AppProps) {
         </span>
         {article.data && (
           <div className="ml-auto flex items-center gap-2">
-            <BookmarkButton
-              target={{
-                type: 'a',
-                value: `${article.data.kind}:${article.data.pubkey}:${tagValue(article.data, 'd') ?? ''}`,
-              }}
-            />
+            {tagValue(article.data, 'd') && (
+              <BookmarkButton
+                target={{
+                  type: 'a',
+                  value: `${article.data.kind}:${article.data.pubkey}:${tagValue(article.data, 'd')}`,
+                }}
+              />
+            )}
             <CopyArticleLink event={article.data} />
           </div>
         )}
@@ -225,12 +246,74 @@ function CopyArticleLink({ event }: { event: NostrEvent }) {
   );
 }
 
+function ListScopeTabs({
+  scope,
+  disabled,
+  onChange,
+}: {
+  scope: ListScope;
+  disabled: boolean;
+  onChange: (scope: ListScope) => void;
+}) {
+  return (
+    <div className="flex shrink-0 items-center gap-1 border-b border-border p-2">
+      <ScopeTab
+        active={scope === 'recent'}
+        onClick={() => onChange('recent')}
+        icon={<Rss className="size-3.5" aria-hidden />}
+        label="Recent"
+      />
+      <ScopeTab
+        active={scope === 'bookmarked'}
+        disabled={disabled}
+        onClick={() => onChange('bookmarked')}
+        icon={<Bookmark className="size-3.5" aria-hidden />}
+        label="Bookmarked"
+      />
+    </div>
+  );
+}
+
+function ScopeTab({
+  active,
+  disabled,
+  onClick,
+  icon,
+  label,
+}: {
+  active: boolean;
+  disabled?: boolean;
+  onClick: () => void;
+  icon: React.ReactNode;
+  label: string;
+}) {
+  return (
+    <button
+      type="button"
+      onClick={onClick}
+      disabled={disabled}
+      aria-pressed={active}
+      className={cn(
+        'flex flex-1 items-center justify-center gap-1.5 rounded-md px-2 py-1 text-[12px] font-medium transition-colors',
+        'focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-ring',
+        'disabled:cursor-not-allowed disabled:opacity-40',
+        active ? 'bg-secondary text-foreground' : 'text-muted-foreground hover:bg-muted',
+      )}
+    >
+      {icon}
+      {label}
+    </button>
+  );
+}
+
 function ArticleList({
   query,
+  scope,
   selected,
   onSelect,
 }: {
   query: ReturnType<typeof useRecentArticles>;
+  scope: ListScope;
   selected: { pubkey: string; identifier: string } | null;
   onSelect: (event: NostrEvent, identifier: string) => void;
 }) {
@@ -247,14 +330,14 @@ function ArticleList({
   if (!query.data || query.data.length === 0) {
     return (
       <p className="px-3 py-6 text-center text-xs text-muted-foreground">
-        No articles on your relays.
+        {scope === 'bookmarked' ? 'You haven’t bookmarked any articles yet.' : 'No articles on your relays.'}
       </p>
     );
   }
 
   return (
     <>
-      <AppSectionTitle>Recent</AppSectionTitle>
+      <AppSectionTitle>{scope === 'bookmarked' ? 'Bookmarked' : 'Recent'}</AppSectionTitle>
       <ul className="pb-2">
         {query.data.map((event) => {
           const identifier = tagValue(event, 'd')!;

src/apps/bookmarks/index.tsx

@@ -1,4 +1,4 @@
-import { useEffect, useMemo } from 'react';
+import { useEffect } from 'react';
 import { useNostr } from '@nostrify/react';
 import { useQuery } from '@tanstack/react-query';
 import type { NostrEvent } from '@nostrify/nostrify';
@@ -7,7 +7,7 @@ import { LoginRequired } from '@/components/nostr/LoginRequired';
 import { NoteCard } from '@/components/nostr/NoteCard';
 import { Skeleton } from '@/components/ui/skeleton';
 import { useAuthor } from '@/hooks/useAuthor';
-import { useBookmarkList } from '@/hooks/useBookmarks';
+import { useBookmarkedNoteIds, useMyBookmarkedArticles } from '@/hooks/useBookmarks';
 import { useCurrentUser } from '@/hooks/useCurrentUser';
 import { useWindowManager } from '@/os/useWindowManager';
 import { displayName, relativeTime, tagValue } from '@/lib/nostrUtils';
@@ -30,55 +30,22 @@ function useBookmarkedNotes(ids: string[]) {
   });
 }
 
-/** Addresses are `kind:pubkey:d-identifier`; only the `d` half is queryable, so matches are narrowed back down locally. */
-function useBookmarkedArticles(addresses: string[]) {
-  const { nostr } = useNostr();
-  const dTags = useMemo(
-    () => [...new Set(addresses.map((address) => address.split(':')[2]).filter(Boolean))],
-    [addresses],
-  );
-
-  return useQuery<NostrEvent[]>({
-    queryKey: ['nostr', 'bookmarked-articles', addresses.join(',')],
-    enabled: dTags.length > 0,
-    queryFn: async ({ signal }) => {
-      const events = await nostr.query(
-        [{ kinds: [30023], '#d': dTags, limit: dTags.length * 4 }],
-        { signal: AbortSignal.any([signal, AbortSignal.timeout(6000)]) },
-      );
-      const wanted = new Set(addresses);
-      return events.filter((event) => wanted.has(`${event.kind}:${event.pubkey}:${tagValue(event, 'd')}`));
-    },
-    staleTime: 60_000,
-  });
-}
-
 export default function BookmarksApp({ setTitle }: AppProps) {
   const { user } = useCurrentUser();
   const { openApp } = useWindowManager();
 
   useEffect(() => setTitle('Bookmarks'), [setTitle]);
 
-  const list = useBookmarkList();
-  const noteIds = useMemo(
-    () => (list.data?.tags ?? []).filter(([name]) => name === 'e').map(([, id]) => id),
-    [list.data],
-  );
-  const addresses = useMemo(
-    () => (list.data?.tags ?? []).filter(([name]) => name === 'a').map(([, address]) => address),
-    [list.data],
-  );
-
+  const noteIds = useBookmarkedNoteIds();
   const notes = useBookmarkedNotes(noteIds);
-  const articles = useBookmarkedArticles(addresses);
+  const articles = useMyBookmarkedArticles();
 
   if (!user) {
     return <LoginRequired action="see your bookmarks" />;
   }
 
-  const isLoading =
-    list.isLoading || (noteIds.length > 0 && notes.isLoading) || (addresses.length > 0 && articles.isLoading);
-  const isEmpty = !isLoading && noteIds.length === 0 && addresses.length === 0;
+  const isLoading = (noteIds.length > 0 && notes.isLoading) || articles.isLoading;
+  const isEmpty = !isLoading && (notes.data?.length ?? 0) === 0 && (articles.data?.length ?? 0) === 0;
 
   return (
     <AppLayout>

src/components/nostr/BookmarkButton.tsx

@@ -30,6 +30,7 @@ export function BookmarkButton({ target, className }: { target: BookmarkTarget;
 
   return (
     <Button
+      type="button"
       variant="ghost"
       size="sm"
       className={cn('h-7 gap-1.5 px-2 text-xs text-muted-foreground', bookmarked && 'text-primary', className)}

src/hooks/useBookmarks.ts

@@ -1,8 +1,12 @@
+import { useMemo } from 'react';
 import { useNostr } from '@nostrify/react';
 import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query';
 import type { NostrEvent } from '@nostrify/nostrify';
 import { useCurrentUser } from './useCurrentUser';
 import { useNostrPublish } from './useNostrPublish';
+import { tagValue } from '@/lib/nostrUtils';
+
+const ARTICLE_KIND = 30023;
 
 /** NIP-51 "Bookmarks": an uncategorized, global, replaceable list per user. */
 export const BOOKMARK_LIST_KIND = 10003;
@@ -18,6 +22,18 @@ function bookmarkQueryKey(pubkey: string | undefined) {
   return ['nostr', 'bookmarks', pubkey ?? ''] as const;
 }
 
+async function fetchBookmarkList(
+  nostr: ReturnType<typeof useNostr>['nostr'],
+  pubkey: string,
+  signal?: AbortSignal,
+): Promise<NostrEvent | null> {
+  const [event] = await nostr.query(
+    [{ kinds: [BOOKMARK_LIST_KIND], authors: [pubkey], limit: 1 }],
+    { signal: AbortSignal.any([signal, AbortSignal.timeout(6000)].filter((s): s is AbortSignal => Boolean(s))) },
+  );
+  return event ?? null;
+}
+
 /** The current user's kind 10003 bookmark list, or null if they don't have one yet. */
 export function useBookmarkList() {
   const { nostr } = useNostr();
@@ -26,13 +42,7 @@ export function useBookmarkList() {
   return useQuery<NostrEvent | null>({
     queryKey: bookmarkQueryKey(user?.pubkey),
     enabled: Boolean(user),
-    queryFn: async ({ signal }) => {
-      const [event] = await nostr.query(
-        [{ kinds: [BOOKMARK_LIST_KIND], authors: [user!.pubkey], limit: 1 }],
-        { signal: AbortSignal.any([signal, AbortSignal.timeout(6000)]) },
-      );
-      return event ?? null;
-    },
+    queryFn: ({ signal }) => fetchBookmarkList(nostr, user!.pubkey, signal),
     staleTime: 60_000,
   });
 }
@@ -51,21 +61,23 @@ export function isBookmarked(targets: BookmarkTarget[], target: BookmarkTarget):
 }
 
 /**
- * Adds or removes one target from the bookmark list. Reads the list back
- * before writing — kind 10003 is a whole-list replacement, so publishing
- * without the existing entries would silently drop them, the same trap
- * NIP-02 follow lists have.
+ * Adds or removes one target from the bookmark list. Fetches the list fresh
+ * from relays right before writing — kind 10003 is a whole-list replacement,
+ * so publishing against a stale cached copy (the query's staleTime is 60s)
+ * could silently drop entries added from another tab or device in the
+ * meantime, the same trap NIP-02 follow lists have.
  */
 export function useToggleBookmark() {
+  const { nostr } = useNostr();
   const { user } = useCurrentUser();
-  const list = useBookmarkList();
   const publish = useNostrPublish();
   const queryClient = useQueryClient();
 
   return useMutation({
     mutationFn: async (target: BookmarkTarget) => {
       if (!user) throw new Error('Sign in to bookmark');
-      const currentTags = list.data?.tags ?? [];
+      const current = await fetchBookmarkList(nostr, user.pubkey);
+      const currentTags = current?.tags ?? [];
       const already = currentTags.some(([name, value]) => name === target.type && value === target.value);
       const tags = already
         ? currentTags.filter(([name, value]) => !(name === target.type && value === target.value))
@@ -73,7 +85,7 @@ export function useToggleBookmark() {
 
       return publish.mutateAsync({
         kind: BOOKMARK_LIST_KIND,
-        content: list.data?.content ?? '',
+        content: current?.content ?? '',
         tags,
       });
     },
@@ -82,3 +94,62 @@ export function useToggleBookmark() {
     },
   });
 }
+
+interface ParsedAddress {
+  kind: number;
+  pubkey: string;
+  identifier: string;
+}
+
+/** Parses a NIP-01 `kind:pubkey:d-identifier` address tag value, or null if malformed. */
+function parseAddress(address: string): ParsedAddress | null {
+  const [kindPart, pubkey, ...rest] = address.split(':');
+  const kind = Number(kindPart);
+  const identifier = rest.join(':');
+  if (!Number.isInteger(kind) || !pubkey) return null;
+  return { kind, pubkey, identifier };
+}
+
+/** The current user's bookmarked note ids (`e` tags), skipping any malformed entries. */
+export function useBookmarkedNoteIds(): string[] {
+  const targets = useBookmarkedTargets();
+  return useMemo(
+    () => targets.filter((t) => t.type === 'e').map((t) => t.value),
+    [targets],
+  );
+}
+
+/**
+ * The current user's bookmarked NIP-23 articles, fetched and narrowed back
+ * down to the exact `kind:pubkey:d` triples bookmarked — the relay filter
+ * can only constrain by kind/author/`d`, not the full address.
+ */
+export function useMyBookmarkedArticles() {
+  const { nostr } = useNostr();
+  const targets = useBookmarkedTargets();
+
+  const addresses = useMemo(() => targets.filter((t) => t.type === 'a').map((t) => t.value), [targets]);
+  const parsed = useMemo(
+    () =>
+      addresses
+        .map(parseAddress)
+        .filter((a): a is ParsedAddress => a !== null && a.kind === ARTICLE_KIND),
+    [addresses],
+  );
+  const authors = useMemo(() => [...new Set(parsed.map((a) => a.pubkey))], [parsed]);
+  const dTags = useMemo(() => [...new Set(parsed.map((a) => a.identifier))], [parsed]);
+
+  return useQuery<NostrEvent[]>({
+    queryKey: ['nostr', 'bookmarked-articles', addresses.join(',')],
+    enabled: parsed.length > 0,
+    queryFn: async ({ signal }) => {
+      const events = await nostr.query(
+        [{ kinds: [ARTICLE_KIND], authors, '#d': dTags, limit: parsed.length * 2 }],
+        { signal: AbortSignal.any([signal, AbortSignal.timeout(6000)]) },
+      );
+      const wanted = new Set(addresses);
+      return events.filter((event) => wanted.has(`${event.kind}:${event.pubkey}:${tagValue(event, 'd')}`));
+    },
+    staleTime: 60_000,
+  });
+}