homefeed/docs/review-2026-07.md

# Homefeed program review — July 2026

Scope: dead code, inefficiency, and UI/system consistency across the whole app
(backend + frontend, public site + admin panel), as requested after a long run of
incremental feature work. Findings below are left as originally written (historical
record); the "Resolved since this report" section tracks what's since been acted on.

## Resolved since this report

- **1.1** `ArticleCard.svelte` — deleted.
- **1.2** All five zero-call-site exports (`getTagsByIds`, `unclusteredItemsForSources`,
  `itemsByCluster`, `deleteAllContentItems`, `latestArticleInThread`) — removed.
- **1.3** `poe2_primary_currency_name` — dropped from the schema entirely (CREATE TABLE
  literal + the ALTER TABLE migration line), not just left as an inert column.
- **Inefficiencies** — the unconditional `stooqToYahooSymbols` startup rewrite
  (long-since a no-op) was deleted outright; `settings.ts:updateSettings` now binds
  named `$column` parameters instead of 33 positional `?`s, removing the
  reorder-and-silently-corrupt risk; `runPassthroughCycle`/`runSynthesisCycle`'s
  direct-publish logic now share one `publishItemsDirect` helper; the per-item
  `sourcesDb.getSource()` N+1 in the synthesis tick was replaced with a single
  `sourcesDb.listSources()` map reused for both the direct-publish partition and each
  item's category-rank lookup. (`settings.ts`'s remaining structural size and
  `migrate()`'s 410-line mixed-idiom growth were left as-is — real fixes but a larger
  refactor than this pass warranted.)
- **`'custom'` source type** — removed entirely: dropped from the `Source['type']`
  union (backend + frontend), the poller's adapter map, and the admin add-source
  dropdown. It had no adapter of its own (silently aliased to the API adapter) and no
  admin-facing purpose distinct from `api`.

Everything above was verified via `tsc --noEmit`/`svelte-check` (0 errors on both
sides), a full build of both packages, and a live smoke test against a fresh DB
(migrate() + a settings roundtrip through the new named-param query, confirming the
column is gone and unrelated fields are untouched).

See the **staleness/refresh recommendation** delivered separately in conversation —
that item was discussed, not changed, per the request to advise rather than implement.

---

## 1. Dead code

### 1.1 Frontend
- `frontend/src/lib/components/ArticleCard.svelte` — zero references anywhere under
  `frontend/src` (confirmed via `grep -rn "ArticleCard" frontend/src`, zero matches
  outside its own file). Superseded by `ArticleListRow.svelte`. Safe to delete.

### 1.2 Backend
Exported functions with no call sites found anywhere in `backend/src`:
- `backend/src/storage/db/tags.ts:31` — `getTagsByIds`
- `backend/src/storage/db/contentItems.ts:72` — `unclusteredItemsForSources`
- `backend/src/storage/db/contentItems.ts:96` — `itemsByCluster`
- `backend/src/storage/db/contentItems.ts:121` — `deleteAllContentItems`
- `backend/src/storage/db/articles.ts:146` — `latestArticleInThread`

### 1.3 Dead settings field
- `global_settings.poe2_primary_currency_name` — left over from before PoE2 moved to
  per-pair tracking (base/quote currencies with a directly-computed rate, no longer
  "everything quoted in one reference currency"). The column is harmless — dropping it
  would mean a SQLite table rebuild for one nullable text column, not worth doing on
  its own. `GlobalSettings.poe2` on the TS side no longer exposes it, so it's already
  fully inert; only the raw DB column remains as an artifact.

### 1.4 Over-exported (not dead, but worth a look)
These are only ever called from within their own module, so exporting them invites
cross-module coupling that hasn't happened yet but could:
- `backend/src/storage/db/sources.ts:27` — `listEnabledSources`
- `backend/src/storage/db/events.ts:46` — `getEvent`
- `backend/src/clustering.ts:9` — `strictnessToThreshold`

---

## 2. Inefficiencies

- **`backend/src/storage/db/index.ts:375-382`** — the `stooqToYahooSymbols` stock
  symbol rewrite runs unconditionally on every server start, forever, with no
  skip-guard once it's already been applied to a given database. Same category of
  issue as the unconditional `DROP TABLE IF EXISTS admin_users` / `admin_sessions` at
  lines 25-26 — both re-run one-time work on every boot instead of gating it behind a
  "have I already done this" check.
- **`backend/src/storage/db/settings.ts:updateSettings` (lines 77-109)** — 33
  positional `?` SQL parameters matched to 33 arguments passed to `.run()` in the same
  order, with nothing enforcing that the two lists stay in sync if either is edited.
  Not currently broken, but a single misordered edit here would silently write the
  wrong value into the wrong column with no type error to catch it.
- Synthesis's 60-second tick calls `sourcesDb.listSources()` on every iteration, which
  is redundant if the source list hasn't changed since the last tick — a small,
  low-priority optimization.
- `runPassthroughCycle` and `runSynthesisCycle`'s "direct publish" halves are
  near-duplicate copy-pasted logic rather than a shared helper.
- The `migrate()` function (410 lines) mixes four different guarded/unguarded
  backfill idioms with no schema-version tracking, which makes it hard to tell at a
  glance whether a given block still needs to run or is a no-op on any DB that's
  already current.

---

## 3. UI / layout consistency audit (public site)

- **`/weather` is not reachable from the top nav** — it's only linkable via the
  sidebar widget, unlike every other route (home, categories, events, `/more`), which
  are all in top nav.
- **Sidebar widgets don't share a component.** Weather, Stocks, and PoE2 each
  hand-roll their own `.widget` shell but do converge on the same `.head` +
  interval-tag pattern (`today`, `24h`, etc.). `BookmarksWidget` is the outlier: it
  lacks that `.head`/interval wrapper the other three use, so it looks structurally
  different from its neighbors in the same sidebar.
- Confirmed: the PoE2 icon-removal and 24h-only simplification from earlier this
  session are fully complete on the frontend (verified via grep — no remaining icon
  references).

---

## 4. UI / layout consistency audit (admin panel)

- **Near-duplicate CRUD tabs.** `StocksTab.svelte` and `BookmarksTab.svelte` are
  close to byte-identical in structure (add row, list, delete button). `Poe2Tab.svelte`
  is a variant of the same pattern; `WeatherTab.svelte` uses a different
  settings-form pattern entirely. None of the three share a common component, so a fix
  to one (e.g. the null/`—` display bug fixed in Poe2Tab this session) doesn't
  propagate to the others even where the same bug class could exist.
- **Admin tab list has an implicit, unlabeled grouping** — Sources & Content /
  Sidebar widgets (Weather, Stocks, PoE2, Bookmarks) / Integrations (Telegram) /
  System (Logs, Settings) — but the tab bar renders them as one flat list with no
  visual separation or heading, so the grouping only exists in the code's mental
  model, not on screen.
- **`/admin` itself is a dead redirect-only render** — it exists only to bounce to
  `/admin/settings` or another tab, with no content of its own.

---

## 5. Cross-system / type consistency findings

### 5.1 Duplicated types
- `WeatherHourEntry`, `WeatherDayEntry`, `WeatherCurrentConditions`, and
  `WeatherAlert` are byte-identical between `frontend/src/lib/types.ts` and
  `adminTypes.ts` — genuine duplication, not just similar shapes.
- `Category` and `CategoryPriority` are same-shape-but-differently-named — worth
  confirming whether that's intentional (different semantic roles) or just drift from
  copy-paste.
- Backend-side: `Tweet`/Telegram-message inline shapes are duplicated between
  `ContentItem` and `MergedArticle` in `backend/src/storage/db/types.ts` (lines
  78-108 vs. 125-143) rather than being extracted into a shared named interface.

### 5.2 CSS token drift
- A `12px` "card radius" value is hardcoded across roughly 20 files instead of using
  the existing `var(--radius)` token (which is 8px) — meaning cards and the
  token-driven `--radius` elements don't actually share one visual language despite
  looking like they should.
- `LogsTab.svelte:99-101,157` hardcodes `#a8710f`/`#fff` for the "warn" log-level
  color rather than using a token, unlike "err" which correctly uses `--text-danger`.
  There's no `--text-warning` token defined to match.

### 5.3 Ingestion pipeline (traced end-to-end)
Flow: adapter → poller → priority queue → clustering/synthesis (or direct-publish
bypass) → articles table → public feed. The telegram/nitter/youtube direct-publish
bypass (skipping clustering/synthesis entirely for those source types) is coherent
and already explicitly commented as intentional — not an oversight, despite looking
unusual at first glance.

One asymmetry does look like a genuine oversight rather than a documented design
choice: the `'custom'` source type is excluded from both
`FOLLOWS_LINK_FOR_FULL_ARTICLE` (`backend/src/poller.ts:24`, which only lists
`['rss', 'api']`) and from `directPublishSourceIds` in
`backend/src/queue/priorityQueue.ts` — with no comment anywhere explaining why
`custom` is treated differently from `rss`, which otherwise behaves the same way a
`custom` source presumably would.

### 5.4 Frontend staleness beyond the sidebar
This session added a 5-minute `invalidate('app:sidebar')` timer
(`frontend/src/routes/+layout.svelte` / `+layout.ts`) so the sidebar widgets stay
fresh in a tab left open. That is the **only** refresh mechanism anywhere in the
frontend. Every other route — home (`/`), `/category/[name]`, `/event/[id]`,
`/article/[id]`, `/more`, and notably **`/admin/settings`** — is a pure one-shot
`load()` with no refresh at all. This is fine for content pages (a stale article list
is a minor nuisance, fixed by navigating), but is a real staleness risk specifically
for `/admin/settings`: an admin who leaves that page open to watch Logs, AI-status,
or Telegram connection status will see indefinitely stale state with no signal that
it's stale.

### 5.5 Public API field leak (most concrete/actionable finding)
`backend/src/api/public.ts` — the categories, bookmarks, and events routes all
explicitly filter their response shape down to public-safe fields:

```ts
// lines ~48-51, 57-61, 68-72 — filtered
app.get('/api/events', async () => eventsDb.listEvents().filter(e => e.active).map(({ id, name, ... }) => ({ id, name, ... })));
```

But stocks and PoE2 do not:

```ts
// line 66
app.get('/api/stocks', async () => stocksDb.listStockTickers());

// lines 74-77
app.get('/api/poe2', async () => {
	const { leagueName, updatedAt } = settingsDb.getSettings().poe2;
	return { leagueName, updatedAt, entries: poe2WatchlistDb.listWatchlist() };
});
```

`listStockTickers()` and `listWatchlist()` return the full DB row, including
`lastError` and `lastPolledAt` — internal poller-diagnostic fields with no reason to
be visible to an unauthenticated visitor. This is inconsistent with how every other
public route in the same file handles the same concern, and is a straightforward fix:
map each entry down to its public-facing fields the same way categories/bookmarks/
events already do.

---

## Appendix: surveyed and found fine

Recorded here so these don't get re-litigated in a future review:

- **Logging** — a single shared `logger`, consistent lowercase tags, and only two
  `console.log` calls outside of it, both explicitly commented as intentional
  exceptions (in `index.ts`).
- **API route organization** — `admin.ts` (329 lines / 38 routes) is cleanly
  sectioned into 11 logical groups; `public.ts` (78 lines / 9 routes) has no
  duplicated logic. 52 routes total, no redundant endpoints found.
- **Scheduler cadences** — weather (45m), stocks (15m), and PoE2 (1h) all poll
  immediately on start by design; the ingestion poll/synthesis loop (60s) and
  retention sweep (1h) deliberately don't, also by design. No redundant external
  fetches were found across the whole scheduler table.
- **Module organization pattern** — the `client.ts` (raw external API) +
  `poller.ts` (orchestration) split is followed consistently for weather, stocks, and
  PoE2. Telegram is the one deliberate exception (a stateful GramJS client/credentials
  singleton, ingested through the generic per-source `ingestion/poller.ts` rather than
  `queue/scheduler.ts`) — this is explicitly commented in the code as an intentional
  divergence, not an inconsistency to fix.