Files
Lyra/docs/superpowers/plans/2026-07-13-deferred-batch.md
T
Jonathan 34ebe39dd7 docs: implementation plan for deferred-items batch
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-13 00:12:56 +02:00

37 KiB
Raw Blame History

Deferred-items Batch Implementation Plan

For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (- [ ]) syntax for tracking.

Goal: Clear the deferred backlog — a Monitor settings tab + clear-a-credential + listenBrainzUrl surface (A), three correctness fixes (B), and a non-blocking chunked discovery sweep (C).

Architecture: Three independent workstreams on branch batch/deferred-cleanup, landing as separate commits. A mirrors the existing Discovery-tab/route patterns; B is three isolated small fixes; C mirrors the scan-chunking design (main.maybe_run_scan) using discovery's existing temporal cursor (WatchedArtist.lastDiscoveredAt).

Tech Stack: Next.js App Router (web, TypeScript, vitest against lyra_test), Python worker (psycopg, pytest against lyra_test), Prisma, Postgres. No new Prisma migration (all Config rows).

Global Constraints

  • Tests run against lyra_test ONLY; a guard refuses destructive ops on non-*_test DBs. Never run suites against live lyra.
  • Web vitest env is node, not jsdom — no DOM-render unit tests. Pure logic + API routes are unit-tested; UI is verified in the real app via Playwright. Preserve existing aria-labels / button text (tests depend on them).
  • Deploy is docker compose up -d --build web worker (NEVER down -v). Not part of this plan — merge + deploy happens after review.
  • Both suites green + tsc + web build clean before merge.
  • Config route writes use Prisma upsert; worker Config writes use _set_config (ON CONFLICT).
  • Spec: docs/superpowers/specs/2026-07-13-deferred-batch-design.md.

Workstream A — Settings

Task A1: Monitor config API route

Files:

  • Create: web/src/app/api/monitor/config/route.ts
  • Test: web/src/app/api/monitor/config/route.test.ts

Interfaces:

  • Produces: GET(): Response (stored-or-default map), PATCH(request): Response (upserts allowlisted keys, returns {updated: n}). Allowlist keys: monitor.enabled, monitor.autoMonitorFuture, monitor.pollIntervalHours, monitor.retryIntervalHours, monitor.qualityCutoff, monitor.upgradeWindowDays.

  • Step 1: Write the failing test

// web/src/app/api/monitor/config/route.test.ts
import { describe, it, expect } from "vitest";
import { prisma } from "@/lib/db";
import { GET, PATCH } from "./route";

function patch(body: unknown) {
  return PATCH(new Request("http://localhost/api/monitor/config", {
    method: "PATCH", headers: { "content-type": "application/json" }, body: JSON.stringify(body),
  }));
}

describe("monitor config API", () => {
  it("GET returns defaults when nothing is stored", async () => {
    const body = await (await GET()).json();
    expect(body).toMatchObject({
      "monitor.enabled": "false", "monitor.autoMonitorFuture": "false",
      "monitor.pollIntervalHours": "24", "monitor.retryIntervalHours": "6",
      "monitor.qualityCutoff": "2", "monitor.upgradeWindowDays": "14",
    });
  });

  it("PATCH upserts known keys and ignores unknown ones", async () => {
    const res = await patch({ "monitor.enabled": "true", "monitor.pollIntervalHours": 12, "nope": "x" });
    expect((await res.json()).updated).toBe(2);
    const body = await (await GET()).json();
    expect(body["monitor.enabled"]).toBe("true");
    expect(body["monitor.pollIntervalHours"]).toBe("12");
    expect(await prisma.config.findUnique({ where: { key: "nope" } })).toBeNull();
  });
});
  • Step 2: Run test to verify it fails

Run: cd web && npx vitest run src/app/api/monitor/config/route.test.ts Expected: FAIL — cannot resolve ./route.

  • Step 3: Write the route (clone of api/discover/config/route.ts)
// web/src/app/api/monitor/config/route.ts
import { prisma } from "@/lib/db";

const DEFAULTS: Record<string, string> = {
  "monitor.enabled": "false",
  "monitor.autoMonitorFuture": "false",
  "monitor.pollIntervalHours": "24",
  "monitor.retryIntervalHours": "6",
  "monitor.qualityCutoff": "2",
  "monitor.upgradeWindowDays": "14",
};

export async function GET() {
  const rows = await prisma.config.findMany({ where: { key: { in: Object.keys(DEFAULTS) } } });
  const stored = Object.fromEntries(rows.map((r) => [r.key, r.value]));
  return Response.json(
    Object.fromEntries(Object.entries(DEFAULTS).map(([k, d]) => [k, stored[k] ?? d])),
  );
}

export async function PATCH(request: Request) {
  let body: unknown;
  try {
    body = await request.json();
  } catch {
    return Response.json({ error: "invalid JSON" }, { status: 400 });
  }
  const entries = Object.entries((body ?? {}) as Record<string, unknown>).filter(
    ([k]) => k in DEFAULTS,
  );
  for (const [key, value] of entries) {
    await prisma.config.upsert({
      where: { key },
      create: { key, value: String(value), secret: false },
      update: { value: String(value) },
    });
  }
  return Response.json({ updated: entries.length });
}
  • Step 4: Run test to verify it passes

Run: cd web && npx vitest run src/app/api/monitor/config/route.test.ts Expected: PASS (2 tests).

  • Step 5: Commit
git add web/src/app/api/monitor/config/
git commit -m "feat(web): monitor config API route"

Task A2: Monitor settings tab (UI)

Files:

  • Modify: web/src/app/settings/settings-form.tsx

Interfaces:

  • Consumes: GET/PATCH /api/monitor/config (Task A1).

UI-only; verified in the real app (node-not-jsdom convention). Mirrors the existing Discovery-tab state/handlers.

  • Step 1: Extend the tab type and list — in settings-form.tsx, change:
type Tab = "Qobuz" | "Soulseek" | "Library" | "Monitor" | "Discovery";
const TABS: Tab[] = ["Qobuz", "Soulseek", "Library", "Monitor", "Discovery"];
  • Step 2: Add the Monitor field list constant (near DISCOVERY_FIELDS):
const MONITOR_FIELDS: [string, string][] = [
  ["monitor.pollIntervalHours", "Poll interval (hours)"],
  ["monitor.retryIntervalHours", "Retry interval (hours)"],
  ["monitor.qualityCutoff", "Quality cutoff"],
  ["monitor.upgradeWindowDays", "Upgrade window (days)"],
];
  • Step 3: Add state + load + save (mirror discoverCfg/saveDiscovery):
const [monitorCfg, setMonitorCfg] = useState<Record<string, string>>({});
const [monitorSaved, setMonitorSaved] = useState(false);

useEffect(() => {
  fetch("/api/monitor/config").then((r) => r.json()).then((c: Record<string, string>) => setMonitorCfg(c));
}, []);

function setMon(key: string, value: string) {
  setMonitorCfg((c) => ({ ...c, [key]: value }));
}

async function saveMonitor(e: React.FormEvent) {
  e.preventDefault();
  await fetch("/api/monitor/config", {
    method: "PATCH",
    headers: { "content-type": "application/json" },
    body: JSON.stringify(monitorCfg),
  });
  setMonitorSaved(true);
  setTimeout(() => setMonitorSaved(false), 1500);
}
  • Step 4: Render the Monitor tab body (insert before the Discovery tab block):
{tab === "Monitor" ? (
  <form className="settings-section" onSubmit={saveMonitor}>
    <div className="save-row" style={{ margin: "6px 0 4px" }}>
      <label className="toggle">
        <input
          type="checkbox"
          checked={monitorCfg["monitor.enabled"] === "true"}
          onChange={(e) => setMon("monitor.enabled", e.target.checked ? "true" : "false")}
        />
        Automatic monitoring (grab &amp; upgrade)
      </label>
    </div>
    <div className="save-row" style={{ margin: "2px 0 8px" }}>
      <label className="toggle">
        <input
          type="checkbox"
          checked={monitorCfg["monitor.autoMonitorFuture"] === "true"}
          onChange={(e) => setMon("monitor.autoMonitorFuture", e.target.checked ? "true" : "false")}
        />
        Auto-monitor future releases from watched artists
      </label>
    </div>
    <div className="field-grid">
      {MONITOR_FIELDS.map(([key, label]) => (
        <label key={key} className="field">
          <span>{label}</span>
          <input type="number" step="any" value={monitorCfg[key] ?? ""} onChange={(e) => setMon(key, e.target.value)} />
        </label>
      ))}
    </div>
    <div className="save-row">
      <button type="submit" className="btn">Save monitor settings</button>
      {monitorSaved ? <span className="saved-note">Saved</span> : null}
    </div>
  </form>
) : null}
  • Step 5: Verify — build + real app

Run: cd web && npx tsc --noEmit && npm run build Expected: clean. Then in the running app (Playwright): open Settings → Monitor, toggle "Automatic monitoring" on, set poll interval, Save → "Saved" appears; reload → values persist. Confirm the DB row: monitor.enabled=true in Config.

  • Step 6: Commit
git add web/src/app/settings/settings-form.tsx
git commit -m "feat(web): Monitor settings tab"

Task A3: Clear-a-credential API (DELETE)

Files:

  • Modify: web/src/app/api/config/route.ts
  • Test: web/src/app/api/config/route.test.ts

Interfaces:

  • Produces: DELETE(request): Response?field=<name> validated against the existing FIELDS allowlist; deletes that key's Config row. Unknown field → 400. Valid → {ok:true} (idempotent).

  • Step 1: Write the failing test (append to route.test.ts)

import { GET, PUT, DELETE } from "./route"; // update the existing import line

function delReq(field: string) {
  return new Request(`http://localhost/api/config?field=${field}`, { method: "DELETE" });
}

describe("config DELETE", () => {
  it("removes a stored secret by field name", async () => {
    await PUT(putReq({ qobuzPassword: "hunter2" }));
    expect(await prisma.config.findUnique({ where: { key: "qobuz.password" } })).not.toBeNull();
    const res = await DELETE(delReq("qobuzPassword"));
    expect(res.status).toBe(200);
    expect(await prisma.config.findUnique({ where: { key: "qobuz.password" } })).toBeNull();
    // GET now reports it unset
    expect((await (await GET()).json()).qobuzPasswordSet).toBe(false);
  });

  it("rejects an unknown field with 400 and deletes nothing", async () => {
    const res = await DELETE(delReq("monitor.enabled"));
    expect(res.status).toBe(400);
  });

  it("is idempotent — deleting an absent field returns 200", async () => {
    const res = await DELETE(delReq("slskdApiKey"));
    expect(res.status).toBe(200);
  });
});
  • Step 2: Run to verify it fails

Run: cd web && npx vitest run src/app/api/config/route.test.ts Expected: FAIL — DELETE not exported.

  • Step 3: Add the DELETE handler to route.ts (after GET):
export async function DELETE(request: Request) {
  const field = new URL(request.url).searchParams.get("field") ?? "";
  const entry = FIELDS[field];
  if (!entry) {
    return Response.json({ error: "unknown field" }, { status: 400 });
  }
  await prisma.config.deleteMany({ where: { key: entry.key } }); // idempotent
  return Response.json({ ok: true });
}
  • Step 4: Run to verify it passes

Run: cd web && npx vitest run src/app/api/config/route.test.ts Expected: PASS (all, including the 3 new).

  • Step 5: Commit
git add web/src/app/api/config/route.ts web/src/app/api/config/route.test.ts
git commit -m "feat(web): DELETE /api/config?field= to clear a credential"

Task A4: Clear-credential buttons (UI)

Files:

  • Modify: web/src/app/settings/settings-form.tsx

Interfaces:

  • Consumes: DELETE /api/config?field= (Task A3). Reuses the existing toast host if present in the tree (import path ../_ui/toast — check current usage in the file; if toasts aren't wired into this component, skip the toast and just flip the flag).

  • Step 1: Add a clear handler in the component:

async function clearField(field: "qobuzPassword" | "qobuzToken" | "slskdApiKey") {
  await fetch(`/api/config?field=${field}`, { method: "DELETE" });
  if (field === "qobuzPassword") { setPwSet(false); setQobuzPassword(""); }
  if (field === "qobuzToken") { setTokenSet(false); setQobuzToken(""); }
  if (field === "slskdApiKey") { setKeySet(false); setSlskdApiKey(""); }
}
  • Step 2: Render a Clear button beside each set secret. Replace the secretHint usage on the three secret <span>s so a set secret also shows a Clear button. Example for the Qobuz password label (apply the same shape to auth token and slskd api key):
<span>
  Qobuz password{secretHint(pwSet)}
  {pwSet ? (
    <button type="button" className="btn ghost sm" style={{ marginLeft: 8 }}
      aria-label="clear qobuz password" onClick={() => clearField("qobuzPassword")}>
      Clear
    </button>
  ) : null}
</span>

For the auth token span use tokenSet + clearField("qobuzToken") + aria-label="clear qobuz auth token"; for the slskd api key span use keySet + clearField("slskdApiKey") + aria-label="clear slskd api key".

  • Step 3: Verify — build + real app

Run: cd web && npx tsc --noEmit && npm run build Expected: clean. Real app (Playwright): save a Qobuz password → reload → "· set" + Clear shown; click Clear → hint + button disappear; reload → still cleared; confirm qobuz.password row absent in Config.

  • Step 4: Commit
git add web/src/app/settings/settings-form.tsx
git commit -m "feat(web): clear-credential buttons on Settings"

Task A5: Surface discover.listenBrainzUrl

Files:

  • Modify: web/src/app/api/discover/config/route.ts
  • Modify: web/src/app/api/discover/config/route.test.ts
  • Modify: web/src/app/settings/settings-form.tsx

Interfaces:

  • Produces: discover.listenBrainzUrl in the discover-config DEFAULTS (default ""), rendered as a text field in the Discovery tab.

  • Step 1: Update the failing test — in route.test.ts, extend the defaults assertion:

expect(body).toMatchObject({
  "discover.enabled": "false", "discover.intervalHours": "168",
  "discover.similarPerSeed": "20", "discover.albumsPerArtist": "1",
  "discover.minScore": "0", "discover.chunkSize": "5", "discover.listenBrainzUrl": "",
});

(Note: discover.maxSeeds is removed here and discover.chunkSize added — this test line is also touched by Task C3. Whichever task runs second reconciles to this final shape.)

  • Step 2: Run to verify it fails

Run: cd web && npx vitest run src/app/api/discover/config/route.test.ts Expected: FAIL — missing keys.

  • Step 3: Update DEFAULTS in api/discover/config/route.ts — remove "discover.maxSeeds", add:
  "discover.chunkSize": "5",
  "discover.listenBrainzUrl": "",

(Final DEFAULTS: discover.enabled, discover.intervalHours, discover.similarPerSeed, discover.albumsPerArtist, discover.minScore, discover.chunkSize, discover.listenBrainzUrl.)

  • Step 4: Update the Discovery tab fields in settings-form.tsx — replace DISCOVERY_FIELDS:
const DISCOVERY_FIELDS: [string, string][] = [
  ["discover.intervalHours", "Interval (hours)"],
  ["discover.chunkSize", "Seeds per chunk"],
  ["discover.similarPerSeed", "Similar per seed"],
  ["discover.albumsPerArtist", "Albums per artist"],
  ["discover.minScore", "Min score"],
];

Then add a text field for the URL below the grid, inside the Discovery <form> (before the save-row):

<label className="field">
  <span>ListenBrainz base URL (blank = default)</span>
  <input value={discoverCfg["discover.listenBrainzUrl"] ?? ""}
    onChange={(e) => setCfg("discover.listenBrainzUrl", e.target.value)} />
</label>
  • Step 5: Run tests + build

Run: cd web && npx vitest run src/app/api/discover/config/route.test.ts && npx tsc --noEmit Expected: PASS + clean. (The discover.maxSeeds removal is consistent with Task C3.)

  • Step 6: Commit
git add web/src/app/api/discover/config/ web/src/app/settings/settings-form.tsx
git commit -m "feat(web): surface discover.listenBrainzUrl; swap maxSeeds->chunkSize in Discovery tab"

Workstream B — Correctness

Task B1: Lucene phrase escaping

Files:

  • Modify: web/src/lib/musicbrainz.ts:57-58
  • Test: web/src/lib/musicbrainz.test.ts

Interfaces:

  • Produces: internal escapeLucenePhrase(s: string): string.

  • Step 1: Write the failing test (append to musicbrainz.test.ts) — asserts the built query is escaped. Mock mbGet via the module's fetch. Match the file's existing mocking style; if it stubs global.fetch, assert on the URL captured:

import { describe, it, expect, vi, afterEach } from "vitest";
import * as mb from "./musicbrainz";

afterEach(() => vi.restoreAllMocks());

it("escapes quotes in the release-group query", async () => {
  const spy = vi.spyOn(globalThis, "fetch").mockResolvedValue(
    new Response(JSON.stringify({ "release-groups": [] }), { status: 200 }),
  );
  await mb.searchReleaseGroup('AC/DC', 'Back in "Black"');
  const url = String(spy.mock.calls[0][0]);
  // the decoded query must contain the escaped quote, not a bare one that closes the phrase
  expect(decodeURIComponent(url)).toContain('Back in \\"Black\\"');
});

(If musicbrainz.test.ts already has a shared fetch-mock helper, reuse it instead of re-mocking; keep the assertion on the escaped \\".)

  • Step 2: Run to verify it fails

Run: cd web && npx vitest run src/lib/musicbrainz.test.ts Expected: FAIL — query contains an unescaped ".

  • Step 3: Add the helper and use it in musicbrainz.ts:
function escapeLucenePhrase(s: string): string {
  return s.replace(/[\\"]/g, (c) => "\\" + c);
}

Change line 58 to:

  const query = `releasegroup:"${escapeLucenePhrase(album)}" AND artist:"${escapeLucenePhrase(artist)}"`;
  • Step 4: Run to verify it passes

Run: cd web && npx vitest run src/lib/musicbrainz.test.ts Expected: PASS.

  • Step 5: Commit
git add web/src/lib/musicbrainz.ts web/src/lib/musicbrainz.test.ts
git commit -m "fix(web): escape Lucene quotes in searchReleaseGroup query"

Task B2: wanted-POST unique-race fallback

Files:

  • Modify: web/src/app/api/wanted/route.ts:52-64
  • Test: web/src/app/api/wanted/route.test.ts

Interfaces:

  • Produces: the POST create wrapped in a try/catch that, on Prisma P2002, re-reads the row and returns the same 200 update-path response.

  • Step 1: Write the failing test — simulate the race by inserting the row between the existence check and create. Match the existing route.test.ts style (it drives POST with a mocked MB match). Add:

it("handles a concurrent duplicate (P2002) by returning the existing release", async () => {
  // Pre-create the row so the create() inside POST hits the unique constraint.
  // (Mirror how other tests in this file stub the MB match; reuse that helper.)
  // After POST, expect a 200 with the existing release, not a 500.
  const res = await POST(postReqForKnownMatch());
  expect([200, 201]).toContain(res.status);
  // second identical POST must also be 200 (idempotent), never 500
  const res2 = await POST(postReqForKnownMatch());
  expect(res2.status).toBe(200);
});

(Use the file's existing MB-match stubbing helper — named postReqForKnownMatch here as a placeholder for whatever the current tests use; if the tests stub searchReleaseGroup, stub it to a fixed rgMbid and call POST twice: the first inserts, the second exercises the update/P2002 path.)

  • Step 2: Run to verify it fails — before the fix, a genuine concurrent create would 500; the two-call test may already pass via the existing findUnique→update branch, so ALSO add a direct P2002 assertion by forcing create to throw:
import { Prisma } from "@prisma/client";
it("catches a P2002 thrown by create and falls back to update", async () => {
  const spy = vi.spyOn(prisma.monitoredRelease, "create").mockRejectedValueOnce(
    new Prisma.PrismaClientKnownRequestError("dup", { code: "P2002", clientVersion: "x" }),
  );
  // ensure the row exists so the fallback update finds it
  // (insert it via prisma.monitoredRelease.create directly before mocking, or via a prior POST)
  const res = await POST(postReqForKnownMatch());
  expect(res.status).toBe(200);
  spy.mockRestore();
});

Run: cd web && npx vitest run src/app/api/wanted/route.test.ts Expected: FAIL — the un-caught P2002 rejects / 500s.

  • Step 3: Wrap the create in wanted/route.ts. Replace the const created = await prisma.monitoredRelease.create({...}) block (lines 52-64) with:
  try {
    const created = await prisma.monitoredRelease.create({
      data: {
        artistMbid: match.artistMbid,
        artistName: match.artistName,
        rgMbid: match.rgMbid,
        album: match.title,
        primaryType: match.primaryType,
        secondaryTypes: match.secondaryTypes,
        firstReleaseDate: match.firstReleaseDate,
        monitored: true,
      },
    });
    return Response.json({ id: created.id, album: created.album, monitored: created.monitored }, { status: 201 });
  } catch (e) {
    if (e instanceof Prisma.PrismaClientKnownRequestError && e.code === "P2002") {
      // Concurrent Want for the same release-group won the create; converge to the update path.
      const updated = await prisma.monitoredRelease.update({
        where: { rgMbid: match.rgMbid },
        data: { monitored: true },
      });
      return Response.json({ id: updated.id, album: updated.album, monitored: updated.monitored }, { status: 200 });
    }
    throw e;
  }

Add the import at the top: import { Prisma } from "@prisma/client";

  • Step 4: Run to verify it passes

Run: cd web && npx vitest run src/app/api/wanted/route.test.ts Expected: PASS.

  • Step 5: Commit
git add web/src/app/api/wanted/route.ts web/src/app/api/wanted/route.test.ts
git commit -m "fix(web): wanted POST converges on P2002 unique race instead of 500"

Task B3: import_album crash-recovery preamble

Files:

  • Modify: worker/lyra_worker/library.py:62-68
  • Test: worker/tests/test_library.py (or the existing import_album test file — locate with grep -rln import_album worker/tests)

Interfaces:

  • Produces: crash-recovery at the top of import_album — restores {final} from {final}.old if final is missing, else drops a stale .old; always drops a stale .importing.

  • Step 1: Write the failing tests — add to the located import_album test file:

def test_import_album_recovers_from_interrupted_swap(tmp_path):
    from lyra_worker.library import import_album
    final = str(tmp_path / "Artist" / "Album (2020)")
    os.makedirs(final + ".old")
    open(os.path.join(final + ".old", "01 Song.flac"), "w").close()
    # `final` is missing (crash happened after rename(final, old), before rename(tmp, final))
    staging = str(tmp_path / "staging")  # empty/missing staging
    import_album(staging, final)
    # previous good copy restored to `final`; no leftover .old
    assert os.path.isfile(os.path.join(final, "01 Song.flac"))
    assert not os.path.isdir(final + ".old")

def test_import_album_drops_stale_old_when_final_present(tmp_path):
    from lyra_worker.library import import_album
    final = str(tmp_path / "Artist" / "Album (2020)")
    os.makedirs(final)
    open(os.path.join(final, "keep.flac"), "w").close()
    os.makedirs(final + ".old")  # leftover from a completed swap
    open(os.path.join(final + ".old", "stale.flac"), "w").close()
    import_album(str(tmp_path / "staging"), final)
    assert not os.path.isdir(final + ".old")           # stale .old removed
    assert os.path.isfile(os.path.join(final, "keep.flac"))  # existing copy untouched by preamble

(Ensure import os is present in the test module.)

  • Step 2: Run to verify it fails

Run: cd worker && python -m pytest tests/test_library.py -k "interrupted or stale_old" -v Expected: FAIL — orphan .old not handled (final not restored).

  • Step 3: Add the preamble at the top of import_album, immediately after the docstring and before os.makedirs(os.path.dirname(final), exist_ok=True):
    # Recover from a swap interrupted by a crash: {final}.old holds the previous copy.
    old = final + ".old"
    if os.path.isdir(old) and not os.path.isdir(final):
        os.rename(old, final)                       # restore the last-good copy
    shutil.rmtree(old, ignore_errors=True)          # drop a leftover .old (swap had completed)
    shutil.rmtree(final + ".importing", ignore_errors=True)  # drop a stale half-assembly
  • Step 4: Run to verify it passes

Run: cd worker && python -m pytest tests/test_library.py -k "interrupted or stale_old" -v Expected: PASS. Then run the whole file to confirm no regression: python -m pytest tests/test_library.py -v.

  • Step 5: Commit
git add worker/lyra_worker/library.py worker/tests/test_library.py
git commit -m "fix(worker): import_album recovers orphaned .old/.importing from an interrupted swap"

Workstream C — Chunk discovery

Task C1: chunk_size config + seed-count result

Files:

  • Modify: worker/lyra_worker/discovery.py (DiscoveryConfig, DiscoveryResult, _seed_artists, run_discovery return)
  • Test: worker/tests/test_discovery.py

Interfaces:

  • Produces: DiscoveryConfig.chunk_size: int (from discover.chunkSize, default 5; max_seeds removed); DiscoveryResult.seeds: int; _seed_artists LIMIT uses chunk_size; run_discovery returns DiscoveryResult(artists, albums, seeds=len(seeds)).

  • Consumed by: Task C2 (result.seeds == 0 ⇒ sweep drained).

  • Step 1: Write the failing test (append to test_discovery.py) — mirror its existing harness:

def test_run_discovery_processes_at_most_chunk_size_seeds(conn):
    from lyra_worker.discovery import DiscoveryConfig, run_discovery
    from lyra_worker.adapters.fakes import FakeMbBrowser, FakeSimilaritySource
    from lyra_worker.similarity.base import SimilarArtist
    from tests.conftest import insert_watched_artist
    for i in range(4):
        insert_watched_artist(conn, mbid=f"s{i}", name=f"Seed{i}")
    src = FakeSimilaritySource(similar={f"s{i}": [SimilarArtist(f"c{i}", f"Cand{i}", 0.9)] for i in range(4)})
    cfg = DiscoveryConfig.from_config({"discover.chunkSize": "2"})
    result = run_discovery(conn, [src], FakeMbBrowser(), cfg)
    assert result.seeds == 2          # only chunk_size seeds this call
    # exactly 2 seeds now have lastDiscoveredAt set
    with conn.cursor() as cur:
        cur.execute('SELECT count(*) FROM "WatchedArtist" WHERE "lastDiscoveredAt" IS NOT NULL')
        assert cur.fetchone()[0] == 2
  • Step 2: Run to verify it fails

Run: cd worker && python -m pytest tests/test_discovery.py -k chunk_size -v Expected: FAIL — chunk_size/result.seeds don't exist.

  • Step 3: Edit discovery.py

In DiscoveryConfig: remove max_seeds: int = 50; add chunk_size: int = 5. In from_config, remove the max_seeds= line and add:

            chunk_size=_int("discover.chunkSize", 5),

In DiscoveryResult, add a field:

@dataclass(frozen=True)
class DiscoveryResult:
    artists: int
    albums: int
    seeds: int = 0

In _seed_artists, change the LIMIT argument from cfg.max_seeds to cfg.chunk_size:

            (cfg.interval_hours, cfg.chunk_size),

In run_discovery, change the final return to include the seed count:

    return DiscoveryResult(artists=artists, albums=albums, seeds=len(seeds))
  • Step 4: Run to verify it passes

Run: cd worker && python -m pytest tests/test_discovery.py -v Expected: PASS (new test + existing discovery tests unaffected — they don't assert on max_seeds).

  • Step 5: Commit
git add worker/lyra_worker/discovery.py worker/tests/test_discovery.py
git commit -m "feat(worker): discovery chunk_size + seed-count in result (retire max_seeds)"

Task C2: chunked maybe_run_discovery

Files:

  • Modify: worker/lyra_worker/main.py (_run_discovery, maybe_run_discovery)
  • Test: worker/tests/test_discovery_trigger.py

Interfaces:

  • Consumes: run_discoveryDiscoveryResult with .seeds (Task C1).

  • Produces: _run_discovery(conn, sources, browser, config=None) -> DiscoveryResult (runs ONE chunk, no longer writes result/clears flags); maybe_run_discovery(...) drives one chunk per call with a discover.inProgress flag, accumulating discover.progress and writing discover.result + clearing inProgress/requested on drain.

  • Step 1: Update the existing tests in test_discovery_trigger.py to the new contract.

Replace test_run_discovery_writes_result_and_clears_flag (the result/flag writes now live in maybe_run_discovery) with a drain-based test, and make the monkeypatched _run_discovery in the two "runs once" tests return a DiscoveryResult (so the caller can inspect .seeds):

from lyra_worker.discovery import DiscoveryConfig, DiscoveryResult

def test_maybe_run_discovery_drains_across_chunks_and_finishes(conn):
    from lyra_worker.adapters.fakes import FakeMbBrowser, FakeSimilaritySource
    from lyra_worker.similarity.base import SimilarArtist
    from tests.conftest import insert_watched_artist
    for i in range(3):
        insert_watched_artist(conn, mbid=f"s{i}", name=f"Seed{i}")
    _set(conn, "discover.requested", "true")
    _set(conn, "discover.chunkSize", "2")
    src = FakeSimilaritySource(similar={f"s{i}": [SimilarArtist(f"c{i}", f"C{i}", 0.9)] for i in range(3)})
    cfg = {"discover.enabled": "false", "discover.requested": "true", "discover.chunkSize": "2"}
    tick = 0.0
    # iteration 1: starts sweep, processes 2 seeds, still in progress
    tick = maybe_run_discovery(conn, [src], FakeMbBrowser(), cfg, DiscoveryConfig.from_config(cfg), 1.0, tick)
    assert _get(conn, "discover.inProgress") == "true"
    assert _get(conn, "discover.requested") == "false"   # cleared at sweep start
    # refresh the snapshot the way the loop does
    cfg2 = {**cfg, "discover.requested": "false", "discover.inProgress": "true",
            "discover.progress": _get(conn, "discover.progress")}
    # iteration 2: processes the last seed
    maybe_run_discovery(conn, [src], FakeMbBrowser(), cfg2, DiscoveryConfig.from_config(cfg2), 2.0, tick)
    cfg3 = {**cfg2, "discover.progress": _get(conn, "discover.progress")}
    # iteration 3: 0 seeds left -> drains, writes result, clears inProgress
    maybe_run_discovery(conn, [src], FakeMbBrowser(), cfg3, DiscoveryConfig.from_config(cfg3), 3.0, tick)
    assert _get(conn, "discover.inProgress") == "false"
    assert "artists" in (_get(conn, "discover.result") or "")

def test_maybe_run_discovery_runs_once_when_due_and_requested(conn, monkeypatch):
    calls = []
    monkeypatch.setattr(main, "_run_discovery",
                        lambda *a, **k: (calls.append(1), DiscoveryResult(0, 0, seeds=0))[1])
    config = {"discover.enabled": "true", "discover.requested": "true"}
    dcfg = DiscoveryConfig.from_config(config)
    new_tick = maybe_run_discovery(conn, [], FakeMbBrowser(), config, dcfg, now=10_000.0, last_discover_tick=0.0)
    assert calls == [1]           # exactly one chunk
    assert new_tick == 10_000.0   # schedule tick advanced

Also update test_maybe_run_discovery_requested_when_disabled_does_not_advance_tick (and any other _run_discovery monkeypatch) the same way — return DiscoveryResult(0, 0, seeds=0). Update the top-of-file import: from lyra_worker.discovery import DiscoveryConfig, DiscoveryResult.

  • Step 2: Run to verify the drain test fails

Run: cd worker && python -m pytest tests/test_discovery_trigger.py -v Expected: FAIL — no discover.inProgress handling yet.

  • Step 3: Rewrite _run_discovery and maybe_run_discovery in main.py.

Replace _run_discovery with a thin one-chunk runner (no side-effect config writes):

def _run_discovery(conn, sources, browser, config=None) -> DiscoveryResult:
    cfg = DiscoveryConfig.from_config(config if config is not None else get_config(conn))
    return run_discovery(conn, sources, browser, cfg)

Add DiscoveryResult to the discovery import at the top of main.py:

from lyra_worker.discovery import DiscoveryConfig, DiscoveryResult, run_discovery

Replace maybe_run_discovery with the chunked version:

def maybe_run_discovery(conn, sources, browser, config, dcfg, now, last_discover_tick,
                        tick_seconds: float = DISCOVER_TICK_SECONDS) -> float:
    """Advance the discovery sweep by at most one chunk (``discover.chunkSize`` seeds) per call,
    so the worker loop keeps claiming jobs between chunks. A sweep starts on ``discover.requested``
    or a due schedule tick, then continues via ``discover.inProgress`` until a chunk drains 0 seeds
    (every eligible seed refreshed), at which point ``discover.result`` is written. Returns the
    (possibly advanced) last_discover_tick."""
    requested = str(config.get("discover.requested", "")).strip().lower() in _TRUE
    in_progress = str(config.get("discover.inProgress", "")).strip().lower() in _TRUE
    due = dcfg.enabled and now - last_discover_tick >= tick_seconds
    if not (requested or due or in_progress):
        return last_discover_tick

    starting = not in_progress
    if starting:  # begin a fresh sweep
        _set_config(conn, "discover.inProgress", "true")
        _set_config(conn, "discover.requested", "false")

    try:
        result = _run_discovery(conn, sources, browser, config)  # one chunk
    except Exception as e:  # a discovery error must never kill the worker
        print(f"worker: discovery run failed: {e}", flush=True)
        conn.rollback()
        _set_config(conn, "discover.inProgress", "false")  # abandon; a new request restarts it
        _set_config(conn, "discover.requested", "false")
        return now if due else last_discover_tick

    a_prev, b_prev = (0, 0) if starting else _parse_progress(config.get("discover.progress", ""))
    a_total, b_total = a_prev + result.artists, b_prev + result.albums
    if result.seeds == 0:  # every eligible seed refreshed -> sweep drained
        _set_config(conn, "discover.result", f"artists {a_total}, albums {b_total}")
        _set_config(conn, "discover.progress", "")
        _set_config(conn, "discover.inProgress", "false")
        print(f"worker: discovery done — {a_total} artists, {b_total} albums", flush=True)
    else:
        _set_config(conn, "discover.progress", f"{a_total}/{b_total}")
    return now if due else last_discover_tick

(_parse_progress already exists from the scan-chunk work and is reused as-is.)

  • Step 4: Run to verify it passes

Run: cd worker && python -m pytest tests/test_discovery_trigger.py -v Expected: PASS. Then the whole discovery suite: python -m pytest tests/ -k discovery -v.

  • Step 5: Commit
git add worker/lyra_worker/main.py worker/tests/test_discovery_trigger.py
git commit -m "feat(worker): chunk the discovery sweep (one chunk per loop iteration)"

Task C3: Reconcile Settings/route for chunkSize

Note: the api/discover/config DEFAULTS and the Discovery-tab field list are already updated in Task A5 (maxSeeds → chunkSize + listenBrainzUrl). If A5 ran before this workstream, there is nothing to do here — verify only. If C ran first, do A5 now.

  • Step 1: Verify the discover-config route + Settings reflect chunkSize, not maxSeeds

Run: cd web && npx vitest run src/app/api/discover/config/route.test.ts && grep -n "maxSeeds" web/src/app/settings/settings-form.tsx web/src/app/api/discover/config/route.ts Expected: tests PASS; grep returns NOTHING (no lingering maxSeeds in UI/route).

  • Step 2: (only if A5 not yet done) execute Task A5's steps 16.

Final verification (before merge)

  • Full suites + build
cd worker && python -m pytest -q
cd ../web && npx vitest run && npx tsc --noEmit && npm run build

Expected: worker all green; web all green; tsc clean; build clean.

  • Real-app smoke (Playwright, both themes): Settings → Monitor toggles + persists; Clear removes a credential; Discovery tab shows "Seeds per chunk" + ListenBrainz URL; enabling monitor.enabled from the UI is reflected in Config. Worker logs a chunked discovery drain when "Discover now" is pressed (with discovery temporarily enabled).

  • Whole-branch opus review, then ff-merge batch/deferred-cleanupmain, push. Deploy separately (docker compose up -d --build web worker).


Self-review notes (plan author)

  • Spec coverage: A1/A2 = Monitor tab; A3/A4 = clear-a-credential; A5 = listenBrainzUrl; B1/B2/B3 = the three correctness fixes; C1/C2/C3 = chunk discovery + maxSeeds retirement. All spec sections mapped.
  • Cross-task type consistency: DiscoveryResult.seeds (C1) is consumed by C2's drain check; _run_discovery return-type change (C2) matches C1's run_discovery return; discover.chunkSize/removal of discover.maxSeeds touched consistently in A5 (route/UI) and C1 (worker) — A5 and C3 explicitly reconcile whichever runs second.
  • Known interaction (accepted, from spec risk note): per-chunk aggregation means an artist similar to seeds spanning two chunks no longer sums scores across the whole sweep (compounded by _upsert_artist's replace-not-accumulate, deferred item #1). Benign for a feed; revisit only if suggestions look jumpy. min_score likewise applies per chunk.
  • Ordering: recommended order A1→A2→A3→A4→A5→B1→B2→B3→C1→C2→C3. A5 before C3 makes C3 a no-op verify.