fix watchlist remove (was undone by auto-populate); add watch toggle on ticker page

Removing a ticker did nothing because get_watchlist re-runs auto_populate on
every read, instantly re-adding any top-ranked ticker the user had just removed.
Removals are now tombstoned as a "dismissed" entry_type: auto-population skips
them, the list hides them, and a later manual add revives the row. Also exposes
an Add/Remove-watchlist toggle in the ticker detail header.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

app/services/watchlist_service.py

@@ -26,6 +26,11 @@ logger = logging.getLogger(__name__)
 DEFAULT_AUTO_SIZE = 10
 MAX_MANUAL = 10
 
+# entry_type values. "dismissed" is a tombstone: the user removed the ticker, so
+# auto-population must not silently re-add it on the next read. Dismissed rows are
+# hidden from the watchlist and revived (→ manual) if the user adds them again.
+DISMISSED = "dismissed"
+
 
 async def _get_ticker(db: AsyncSession, symbol: str) -> Ticker:
     normalised = symbol.strip().upper()
@@ -64,19 +69,20 @@ async def auto_populate(
         )
     )
 
-    # Get manual ticker_ids so we don't duplicate
+    # Get manual + dismissed ticker_ids so we don't duplicate or resurrect a
-    manual_result = await db.execute(
+    # ticker the user has explicitly removed.
+    kept_result = await db.execute(
         select(WatchlistEntry.ticker_id).where(
             WatchlistEntry.user_id == user_id,
-            WatchlistEntry.entry_type == "manual",
+            WatchlistEntry.entry_type.in_(["manual", DISMISSED]),
         )
     )
-    manual_ticker_ids = {row[0] for row in manual_result.all()}
+    excluded_ticker_ids = {row[0] for row in kept_result.all()}
 
     now = datetime.now(timezone.utc)
     for ticker_id in top_ticker_ids:
-        if ticker_id in manual_ticker_ids:
+        if ticker_id in excluded_ticker_ids:
-            continue  # Already on watchlist as manual
+            continue  # Already manual, or dismissed by the user
         entry = WatchlistEntry(
             user_id=user_id,
             ticker_id=ticker_id,
@@ -100,17 +106,19 @@ async def add_manual_entry(
     """
     ticker = await _get_ticker(db, symbol)
 
-    # Check if already on watchlist
+    # Check if already on watchlist. A dismissed row is revived as manual rather
+    # than rejected; an active (auto/manual) row is a genuine duplicate.
     existing = await db.execute(
         select(WatchlistEntry).where(
             WatchlistEntry.user_id == user_id,
             WatchlistEntry.ticker_id == ticker.id,
         )
     )
-    if existing.scalar_one_or_none() is not None:
+    existing_entry = existing.scalar_one_or_none()
+    if existing_entry is not None and existing_entry.entry_type != DISMISSED:
         raise DuplicateError(f"Ticker already on watchlist: {ticker.symbol}")
 
-    # Count current manual entries
+    # Count current manual entries (dismissed tombstones don't count)
     count_result = await db.execute(
         select(func.count()).select_from(WatchlistEntry).where(
             WatchlistEntry.user_id == user_id,
@@ -125,10 +133,11 @@ async def add_manual_entry(
             "Remove an entry before adding a new one."
         )
 
-    # Check total cap
+    # Check total cap (exclude dismissed tombstones)
     total_result = await db.execute(
         select(func.count()).select_from(WatchlistEntry).where(
             WatchlistEntry.user_id == user_id,
+            WatchlistEntry.entry_type != DISMISSED,
         )
     )
     total_count = total_result.scalar() or 0
@@ -140,6 +149,14 @@ async def add_manual_entry(
             "Remove an entry before adding a new one."
         )
 
+    if existing_entry is not None:
+        # Revive a dismissed entry as manual.
+        existing_entry.entry_type = "manual"
+        existing_entry.added_at = datetime.now(timezone.utc)
+        await db.commit()
+        await db.refresh(existing_entry)
+        return existing_entry
+
     entry = WatchlistEntry(
         user_id=user_id,
         ticker_id=ticker.id,
@@ -157,20 +174,26 @@ async def remove_entry(
     user_id: int,
     symbol: str,
 ) -> None:
-    """Remove a watchlist entry (manual or auto)."""
+    """Remove a watchlist entry (manual or auto).
+
+    Marks the entry as dismissed (a tombstone) rather than deleting it, so the
+    next auto-population pass won't immediately re-add a top-ranked ticker the
+    user just removed. A subsequent manual add revives the row.
+    """
     ticker = await _get_ticker(db, symbol)
 
     result = await db.execute(
         select(WatchlistEntry).where(
             WatchlistEntry.user_id == user_id,
             WatchlistEntry.ticker_id == ticker.id,
+            WatchlistEntry.entry_type != DISMISSED,
         )
     )
     entry = result.scalar_one_or_none()
     if entry is None:
         raise NotFoundError(f"Ticker not on watchlist: {ticker.symbol}")
 
-    await db.delete(entry)
+    entry.entry_type = DISMISSED
     await db.commit()
 
 
@@ -254,7 +277,10 @@ async def get_watchlist(
     stmt = (
         select(WatchlistEntry, Ticker.symbol)
         .join(Ticker, WatchlistEntry.ticker_id == Ticker.id)
-        .where(WatchlistEntry.user_id == user_id)
+        .where(
+            WatchlistEntry.user_id == user_id,
+            WatchlistEntry.entry_type != DISMISSED,
+        )
     )
     result = await db.execute(stmt)
     rows = result.all()

frontend/src/pages/TickerDetailPage.tsx

@@ -2,6 +2,7 @@ import { useMemo, useEffect, useState } from 'react';
 import { useParams } from 'react-router-dom';
 import { useTickerDetail } from '../hooks/useTickerDetail';
 import { useFetchSymbolData } from '../hooks/useFetchSymbolData';
+import { useWatchlist, useAddToWatchlist, useRemoveFromWatchlist } from '../hooks/useWatchlist';
 import type { FetchSelector } from '../api/ingestion';
 import { CandlestickChart } from '../components/charts/CandlestickChart';
 import { ScoreCard } from '../components/ui/ScoreCard';
@@ -98,6 +99,14 @@ export default function TickerDetailPage() {
   const { symbol = '' } = useParams<{ symbol: string }>();
   const { ohlcv, scores, srLevels, sentiment, fundamentals, trades } = useTickerDetail(symbol);
   const ingestion = useFetchSymbolData();
+  const watchlist = useWatchlist();
+  const addToWatchlist = useAddToWatchlist();
+  const removeFromWatchlist = useRemoveFromWatchlist();
+  const onWatchlist = useMemo(
+    () => (watchlist.data ?? []).some((e) => e.symbol.toUpperCase() === symbol.toUpperCase()),
+    [watchlist.data, symbol],
+  );
+  const watchlistBusy = addToWatchlist.isPending || removeFromWatchlist.isPending;
   const [activeTab, setActiveTab] = useState<DetailTab>('Analysis');
   const [refreshingLabel, setRefreshingLabel] = useState<string | null>(null);
 
@@ -216,6 +225,21 @@ export default function TickerDetailPage() {
             </div>
           )}
         </div>
+        <div className="flex items-center gap-2">
+          <Button
+            variant="ghost"
+            onClick={() =>
+              onWatchlist
+                ? removeFromWatchlist.mutate(symbol)
+                : addToWatchlist.mutate(symbol)
+            }
+            loading={watchlistBusy}
+            disabled={watchlist.isLoading}
+            title={onWatchlist ? 'Remove from watchlist' : 'Add to watchlist'}
+            className={onWatchlist ? '!text-amber-300' : ''}
+          >
+            {onWatchlist ? '★ Watching' : '☆ Add to watchlist'}
+          </Button>
           <Button
             onClick={() => { setRefreshingLabel(null); ingestion.mutate(symbol); }}
             loading={ingestion.isPending}
@@ -223,6 +247,7 @@ export default function TickerDetailPage() {
             {ingestion.isPending ? 'Fetching…' : 'Fetch All'}
           </Button>
         </div>
+      </div>
 
       {/* Data freshness bar */}
       <DataFreshnessBar

tests/unit/test_watchlist_service.py

@@ -0,0 +1,104 @@
+"""Tests for watchlist remove → dismiss → revive behaviour.
+
+Regression cover for the bug where removing a top-ranked ticker did nothing,
+because get_watchlist re-runs auto_populate on every read and would instantly
+re-add it. Removals are now tombstoned as "dismissed" so auto-population skips
+them; a later manual add revives the row.
+"""
+
+from __future__ import annotations
+
+from datetime import datetime, timezone
+
+import pytest
+from sqlalchemy import select
+
+from app.models.score import CompositeScore
+from app.models.ticker import Ticker
+from app.models.user import User
+from app.models.watchlist import WatchlistEntry
+from app.services.watchlist_service import (
+    DISMISSED,
+    add_manual_entry,
+    auto_populate,
+    get_watchlist,
+    remove_entry,
+)
+
+# Plain (committing) session — the watchlist service commits, so the rolling-back
+# db_session fixture would fight it. Reuse the shared in-memory engine.
+from tests.conftest import _test_session_factory  # type: ignore
+
+
+@pytest.fixture
+async def session():
+    async with _test_session_factory() as s:
+        yield s
+
+
+async def _seed(session) -> tuple[int, list[int]]:
+    user = User(username="u", password_hash="x", role="user", has_access=True)
+    session.add(user)
+    await session.flush()
+
+    now = datetime.now(timezone.utc)
+    ticker_ids: list[int] = []
+    for i, sym in enumerate(["AAA", "BBB", "CCC"]):
+        t = Ticker(symbol=sym)
+        session.add(t)
+        await session.flush()
+        ticker_ids.append(t.id)
+        session.add(
+            CompositeScore(
+                ticker_id=t.id,
+                score=90.0 - i,  # AAA highest
+                is_stale=False,
+                weights_json="{}",
+                computed_at=now,
+            )
+        )
+    await session.commit()
+    return user.id, ticker_ids
+
+
+async def test_remove_dismisses_and_survives_auto_populate(session):
+    user_id, _ = await _seed(session)
+
+    await auto_populate(session, user_id)
+    await session.commit()
+
+    rows = await get_watchlist(session, user_id)
+    assert {r["symbol"] for r in rows} == {"AAA", "BBB", "CCC"}
+
+    # Remove a top-ranked ticker, then force another auto-population pass.
+    await remove_entry(session, user_id, "AAA")
+    await auto_populate(session, user_id)
+    await session.commit()
+
+    rows = await get_watchlist(session, user_id)
+    symbols = {r["symbol"] for r in rows}
+    assert "AAA" not in symbols
+    assert symbols == {"BBB", "CCC"}
+
+    # The row is tombstoned, not deleted.
+    res = await session.execute(
+        select(WatchlistEntry).join(Ticker).where(Ticker.symbol == "AAA")
+    )
+    entry = res.scalar_one()
+    assert entry.entry_type == DISMISSED
+
+
+async def test_manual_add_revives_dismissed(session):
+    user_id, _ = await _seed(session)
+    await auto_populate(session, user_id)
+    await session.commit()
+
+    await remove_entry(session, user_id, "AAA")
+
+    # Re-adding the dismissed ticker should succeed (not DuplicateError) and
+    # reappear as a manual entry.
+    revived = await add_manual_entry(session, user_id, "AAA")
+    assert revived.entry_type == "manual"
+
+    rows = await get_watchlist(session, user_id)
+    assert "AAA" in {r["symbol"] for r in rows}