manage/openspec/changes/archive/2026-07-09-per-instance-hook-scoping/design.md

SDD Design: Per-Instance Hook Scoping

Change: per-instance-hook-scoping Phase: design Date: 2026-07-09

0. Source findings (read before anything else)

The proposal and spec were written against a mental model. Reading actual source surfaced deviations the design must account for. Trust source, not assumptions.

Spec/proposal claim	Actual source reality	Design impact
PI-112: "Alertmanager/Prometheus status endpoints already accept service_id"	CONFIRMED. get_alertmanager_alerts(service_id: str | None = None), get_alertmanager_status(service_id: str | None = None), get_prometheus_status(service_id: str | None = None) in monitoring.py all resolve via resolve_service_record(store, "<type>", service_id).	Zero backend change for observability. Only frontend hooks + fetch fns need wiring.
PI-111: "backup_jobs/backup_runs tables already carry service_id"	PARTIALLY WRONG. Only backup_jobs has a service_id TEXT column (added via ALTER TABLE backup_jobs ADD COLUMN service_id TEXT). backup_runs and backup_alerts do NOT have service_id — they are attributed via the FK chain: backup_alerts → backup_runs.job_id → backup_jobs.id → backup_jobs.service_id.	list_backup_jobs filters directly. list_backup_runs and list_backup_alerts must use a subquery or JOIN through backup_jobs to filter by service. See §3.2.
Spec PI-110: "get_backup_dashboard gains service_id" / PI-109: "fetchBackupDashboard gains serviceId"	useBackupDashboard is NOT used by any instance-scoped tab. It feeds the old BackupDashboardWidget component (dashboard widget path), not JobsTab. The dashboard endpoint is GET /api/dashboard/backups → build_backup_dashboard_summary(store) in routers/dashboard.py.	EXCLUDED from this change. Scoping it would require modifying build_backup_dashboard_summary and risk PI-117 regression. It is not consumed by AlertsTab/MetricsTab/JobsTab. See §5.
Proposal: "use ?service_id=<id> … use URLSearchParams or the existing pattern"	The get<T>(path, params?) helper in api/shared.ts already takes a Record<string, string> params object and builds the URL correctly (handles ?/&, skips undefined/empty).	No new URL helper needed. Fetch fns just add service_id to the params object conditionally, matching the existing spread pattern already used by fetchBackupRuns / fetchBackupAlerts.
Spec PI-115: "JobsTab calls useBackupRuns(<jobId>, <status>, instance.id)"	JobsTab currently calls useBackupRuns() with NO arguments (line 34): const { data: runsData } = useBackupRuns();. The hook signature is useBackupRuns(jobId?, status?).	The 3rd param (serviceId) is appended after the existing two. JobsTab will call useBackupRuns(undefined, undefined, instance.id).
Spec PI-115: "JobsTab calls useBackupAlerts(<jobId>, <acknowledged>, <severity>, instance.id)"	JobsTab calls useBackupAlerts(undefined, false) (line 37). Hook signature is useBackupAlerts(jobId?, acknowledged?, severity?).	serviceId is the 4th param. JobsTab will call useBackupAlerts(undefined, false, undefined, instance.id).

No proposal/spec scope change is required — the intent (scope tabs to instance.id) holds. The findings above refine the backend filter mechanism (JOIN vs direct column) and exclude fetchBackupDashboard.

---

1. Architecture overview

This is a thin wiring change. The backend Alertmanager/Prometheus endpoints already resolve service_id. The backend backup endpoints need a small filter addition. The frontend work is: 6 hooks gain an optional serviceId, 7 fetch functions pass it as ?service_id=, and 3 tabs stop using void instance; and start passing instance.id.

BEFORE                                    AFTER
──────                                    ─────
AlertsTab                                 AlertsTab
  └► useAlertmanagerAlerts()  ──┐           └► useAlertmanagerAlerts(instance.id)
  └► useAlertmanagerStatus()    │           └► useAlertmanagerStatus(instance.id)
     hooks: no serviceId        │              hooks: serviceId in queryKey + fetch
     fetch: no service_id       │              fetch: ?service_id=<id>
     backend: ignores it ◄──────┘              backend: resolve_service_record(…, service_id) ✓ already exists

MetricsTab                                MetricsTab
  └► usePrometheusStatus()     ──┐           └► usePrometheusStatus(instance.id)
  └► usePrometheusTargets()      │           └► usePrometheusTargets()        ← stays global
     hooks: no serviceId          │              hook: serviceId in queryKey + fetch
     backend: ignores it ◄────────┘              backend: resolve_service_record(…, service_id) ✓

JobsTab                                   JobsTab
  └► useBackupJobs()           ──┐           └► useBackupJobs(instance.id)
  └► useBackupRuns()             │           └► useBackupRuns(undefined, undefined, instance.id)
  └► useBackupAlerts(undefined, false)       └► useBackupAlerts(undefined, false, undefined, instance.id)
     hooks: no serviceId          │              hooks: serviceId in queryKey + fetch
     fetch: no service_id         │              fetch: ?service_id=<id>
     backend: no filter ◄─────────┘              backend: WHERE backup_jobs.service_id = ? (NEW)

Dashboard widgets (useWidgetData) are on a completely separate code path and are untouched.

---

2. Frontend hook changes (PI-101..PI-107)

File: frontend/src/hooks/useObservability.ts File: frontend/src/hooks/useBackups.ts

2.1 Design decision 1 — queryKey shape

Every modified hook appends serviceId ?? "" as the last element of the queryKey tuple:

// BEFORE
queryKey: ["observability", "alerts"],
queryFn: fetchAlertmanagerAlerts,

// AFTER
export function useAlertmanagerAlerts(serviceId?: string) {
    return useQuery({
        queryKey: ["observability", "alerts", serviceId ?? ""],
        queryFn: () => fetchAlertmanagerAlerts(serviceId),
        retry: 2,
        staleTime: 10_000,
        refetchInterval: 15_000,
    });
}

Why serviceId ?? "" (empty-string default), not omitting it:

• When serviceId is undefined, the key is ["observability", "alerts", ""]. This is a single stable key — all undefined-serviceId callers share one cache entry, matching today's behavior.
• When serviceId is "svc-a", the key is ["observability", "alerts", "svc-a"] — a different key, so a separate cache entry. No cross-instance cache hit.
• This avoids the alternative of conditionally appending (which produces ["observability", "alerts"] vs ["observability", "alerts", "svc-a"] — React Query treats these as different-length arrays, which also works, but the empty-string form is more uniform and easier to assert in tests).

The queryFn changes from a direct function reference to an arrow function because it must capture serviceId:

queryFn: () => fetchAlertmanagerAlerts(serviceId),

2.2 All 6 modified hooks (exact signatures)

Hook	New signature	queryKey (after)
useAlertmanagerAlerts	(serviceId?: string)	["observability", "alerts", serviceId ?? ""]
useAlertmanagerStatus	(serviceId?: string)	["observability", "alertmanager-status", serviceId ?? ""]
usePrometheusStatus	(serviceId?: string)	["observability", "prometheus-status", serviceId ?? ""]
useBackupJobs	(serviceId?: string)	["backups", "jobs", serviceId ?? ""]
useBackupRuns	(jobId?: string, status?: string, serviceId?: string)	["backups", "runs", jobId, status, serviceId ?? ""]
useBackupAlerts	(jobId?: string, acknowledged?: boolean, severity?: string, serviceId?: string)	["backups", "alerts", jobId, acknowledged, severity, serviceId ?? ""]

2.3 Hooks deliberately NOT modified (PI-107)

Hook	Reason
usePrometheusTargets	Returns Node Exporter scrape targets for external Prom instances via http_sd_configs — cross-instance by design.
useMonitoringMachines	Machines are a global cross-service concept.
useBackupDashboard	Feeds the dashboard widget path, not any instance-scoped tab. See §5.
useBackupJob	Fetches a single job by ID (job IDs are globally unique). Not used by JobsTab for list views.

---

3. API client changes (PI-108, PI-109)

File: frontend/src/api/client.ts File: frontend/src/api/backups.ts

3.1 Design decision 2 — reuse the existing get<T>(path, params?) pattern

No new URL helper. The existing get<T> in shared.ts takes a Record<string, string> and handles ?/&/encoding/empty-skip. Fetch functions use the conditional-spread pattern already proven in fetchBackupRuns:

// BEFORE (api/client.ts)
export const fetchAlertmanagerAlerts = () =>
    get<AlertmanagerAlertSummary>("/api/monitoring/alerts");

// AFTER
export const fetchAlertmanagerAlerts = (serviceId?: string) =>
    get<AlertmanagerAlertSummary>(
        "/api/monitoring/alerts",
        serviceId ? { service_id: serviceId } : undefined,
    );

3.2 All 7 modified fetch functions

Fetch function	File	New param	Pattern
fetchAlertmanagerAlerts	client.ts	serviceId?: string	serviceId ? { service_id: serviceId } : undefined
fetchAlertmanagerStatus	client.ts	serviceId?: string	same
fetchPrometheusStatus	client.ts	serviceId?: string	same
fetchBackupJobs	backups.ts	serviceId?: string	same
fetchBackupRuns	backups.ts	serviceId?: string (3rd arg)	append to existing params: {...(jobId ? {job_id: jobId} : {}), ...(status ? {status} : {}), ...(serviceId ? {service_id: serviceId} : {})}
fetchBackupAlerts	backups.ts	serviceId?: string (4th arg)	append to existing params
fetchBackupDashboard	backups.ts	NOT MODIFIED	Excluded — see §5

---

4. Backend backup endpoint + store changes (PI-110, PI-111)

File: backend/src/media_library_viewer_api/routers/backups.py File: backend/src/media_library_viewer_api/services/settings_store.py

4.1 Design decision 3 — endpoint signatures

Three endpoints gain service_id: str | None = None:

@router.get("/jobs")
def get_backup_jobs(
    service_id: str | None = None,
    store: SettingsStore = Depends(get_settings_store),
) -> list[dict[str, Any]]:
    jobs = store.list_backup_jobs(service_id=service_id)
    return jobs

@router.get("/runs")
def get_backup_runs(
    job_id: str | None = None,
    status: str | None = None,
    limit: int = 50,
    service_id: str | None = None,
    store: SettingsStore = Depends(get_settings_store),
) -> list[BackupRunResponse]:
    runs = store.list_backup_runs(job_id=job_id, status=status, limit=limit, service_id=service_id)
    return [BackupRunResponse(**run) for run in runs]

@router.get("/alerts")
def get_backup_alerts(
    job_id: str | None = None,
    acknowledged: bool | None = None,
    severity: str | None = None,
    service_id: str | None = None,
    store: SettingsStore = Depends(get_settings_store),
) -> list[BackupAlertResponse]:
    alerts = store.list_backup_alerts(job_id=job_id, acknowledged=acknowledged, severity=severity, service_id=service_id)
    return [BackupAlertResponse(**alert) for alert in alerts]

4.2 Design decision 4 — store filter: direct column for jobs, subquery for runs/alerts

The schema asymmetry (source finding §0) means different filter strategies:

list_backup_jobs — backup_jobs HAS the service_id column:

def list_backup_jobs(self, service_id: str | None = None) -> list[dict[str, Any]]:
    self.init_schema()
    where = ""
    params: list[Any] = []
    if service_id:  # truthy = non-None AND non-empty
        where = "WHERE service_id = ?"
        params.append(service_id)
    sql = f"SELECT * FROM backup_jobs {where} ORDER BY created_at DESC"
    with self.connect() as conn:
        rows = conn.execute(sql, params).fetchall()
    return [self._row_to_job(row) for row in rows]

list_backup_runs — backup_runs has NO service_id. Filter via subquery against backup_jobs:

def list_backup_runs(
    self,
    job_id: str | None = None,
    status: str | None = None,
    limit: int = 50,
    service_id: str | None = None,
) -> list[dict[str, Any]]:
    self.init_schema()
    clauses: list[str] = []
    params: list[Any] = []
    if job_id:
        clauses.append("job_id = ?")
        params.append(job_id)
    if status:
        clauses.append("status = ?")
        params.append(status)
    if service_id:
        clauses.append("job_id IN (SELECT id FROM backup_jobs WHERE service_id = ?)")
        params.append(service_id)
    where = f"WHERE {' AND '.join(clauses)}" if clauses else ""
    sql = f"SELECT * FROM backup_runs {where} ORDER BY created_at DESC LIMIT ?"
    params.append(max(1, min(int(limit), 200)))
    with self.connect() as conn:
        rows = conn.execute(sql, params).fetchall()
    return [self._row_to_run(row) for row in rows]

list_backup_alerts — backup_alerts has NO service_id. Same subquery:

def list_backup_alerts(
    self,
    job_id: str | None = None,
    acknowledged: bool | None = None,
    severity: str | None = None,
    service_id: str | None = None,
) -> list[dict[str, Any]]:
    self.init_schema()
    clauses: list[str] = []
    params: list[Any] = []
    if job_id:
        clauses.append("job_id = ?")
        params.append(job_id)
    if acknowledged is not None:
        clauses.append("acknowledged = ?")
        params.append(1 if acknowledged else 0)
    if severity:
        clauses.append("severity = ?")
        params.append(severity)
    if service_id:
        clauses.append("job_id IN (SELECT id FROM backup_jobs WHERE service_id = ?)")
        params.append(service_id)
    where = f"WHERE {' AND '.join(clauses)}" if clauses else ""
    sql = f"SELECT * FROM backup_alerts {where} ORDER BY created_at DESC"
    with self.connect() as conn:
        rows = conn.execute(sql, params).fetchall()
    return [self._row_to_alert(row) for row in rows]

Why subquery, not JOIN: the existing query patterns all use flat SELECT * FROM <table> WHERE .... A subquery is the minimal-diff approach — one extra clause appended to the existing clauses list, no structural query change. A JOIN would change the SELECT shape and risk the _row_to_* mappers.

Backward-compat (PI-111): if service_id: is truthy-check — both None and "" skip the filter, returning all rows. Matches spec requirement.

4.3 Alertmanager/Prometheus — zero backend change (PI-112, confirmed)

Source verified: get_alertmanager_alerts, get_alertmanager_status, get_prometheus_status in monitoring.py all already have service_id: str | None = None and call resolve_service_record(store, "<type>", service_id). No change needed.

---

5. fetchBackupDashboard excluded (design decision 5)

fetchBackupDashboard / useBackupDashboard / GET /api/dashboard/backups / build_backup_dashboard_summary are NOT modified.

Path	Used by	Instance-scoped?	In this change?
useBackupDashboard → fetchBackupDashboard → /api/dashboard/backups → build_backup_dashboard_summary(store)	BackupDashboardWidget (dashboard component, not a tab)	No — dashboard widgets resolve via useWidgetData, not this hook	Excluded

JobsTab does NOT call useBackupDashboard — it calls useBackupJobs, useBackupRuns, useBackupAlerts for its three sub-tables. The dashboard summary is a separate aggregation used by the main dashboard. Scoping it would require modifying build_backup_dashboard_summary to accept a service_id filter and would affect the dashboard widget path — a PI-117 risk for zero tab benefit.

---

6. Tab changes (PI-113, PI-114, PI-115, PI-116)

File: frontend/src/pages/service-tabs/AlertsTab.tsx File: frontend/src/pages/service-tabs/MetricsTab.tsx File: frontend/src/pages/service-tabs/JobsTab.tsx

The changes are mechanical: remove void instance;, remove the TODO docstring/comment, pass instance.id.

6.1 AlertsTab

// BEFORE (lines 82-90)
export function AlertsTab({ instance }: { instance: ServiceInstance }) {
    void instance;
    const { data: alertsSummary, ... } = useAlertmanagerAlerts();
    const { data: status, ... } = useAlertmanagerStatus();

// AFTER
export function AlertsTab({ instance }: { instance: ServiceInstance }) {
    const { data: alertsSummary, ... } = useAlertmanagerAlerts(instance.id);
    const { data: status, ... } = useAlertmanagerStatus(instance.id);

Also update the file docstring (lines 1-11): remove "The hooks ... are global / first-configured for now ... Wiring instance.id into them is a documented follow-up" and state that hooks are now instance-scoped.

6.2 MetricsTab

// BEFORE (lines 45-50)
export function MetricsTab({ instance }: { instance: ServiceInstance }) {
    void instance;
    const { data: status, ... } = usePrometheusStatus();
    const { data: targets, ... } = usePrometheusTargets();  // stays global

// AFTER
export function MetricsTab({ instance }: { instance: ServiceInstance }) {
    const { data: status, ... } = usePrometheusStatus(instance.id);
    const { data: targets, ... } = usePrometheusTargets();  // unchanged — global

Update the file docstring similarly. Note: usePrometheusTargets() stays global (PI-107) — it returns scrape targets for external Prom instances, not instance-scoped UI data.

6.3 JobsTab

// BEFORE (lines 27-39)
export function JobsTab({ instance }: { instance: ServiceInstance }) {
    void instance;
    const { data: jobsData, isLoading: jobsLoading } = useBackupJobs();
    const { data: runsData, isLoading: runsLoading } = useBackupRuns();
    const { data: alertsData, isLoading: alertsLoading } = useBackupAlerts(
        undefined,
        false,
    );

// AFTER
export function JobsTab({ instance }: { instance: ServiceInstance }) {
    const { data: jobsData, isLoading: jobsLoading } = useBackupJobs(instance.id);
    const { data: runsData, isLoading: runsLoading } = useBackupRuns(undefined, undefined, instance.id);
    const { data: alertsData, isLoading: alertsLoading } = useBackupAlerts(
        undefined,
        false,
        undefined,
        instance.id,
    );

Update the file docstring (lines 1-10): remove the NOTE about "hooks currently query globally."

6.4 Instance switcher re-scoping (PI-116)

Falls out naturally from the instance.id in the hook queryKeys. When the ServicePage instance switcher changes the active instance, the instance prop changes → the hook queryKey changes → React Query triggers a fresh fetch for the new key. No additional wiring needed.

---

7. Tests (PI-119, PI-120, PI-121)

7.1 Backend tests (test_backups.py or equivalent)

Add to existing backup tests:

class TestBackupServiceScoping:
    def test_list_backup_jobs_filtered_by_service(self, store):
        # seed jobs for svc-a and svc-b
        store.upsert_backup_job({"name": "job-a", "service_id": "svc-a"})
        store.upsert_backup_job({"name": "job-b", "service_id": "svc-b"})
        assert len(store.list_backup_jobs(service_id="svc-a")) == 1
        assert len(store.list_backup_jobs(service_id="svc-b")) == 1

    def test_list_backup_jobs_unfiltered_returns_all(self, store):
        store.upsert_backup_job({"name": "job-a", "service_id": "svc-a"})
        store.upsert_backup_job({"name": "job-b", "service_id": "svc-b"})
        assert len(store.list_backup_jobs()) == 2           # None
        assert len(store.list_backup_jobs(service_id="")) == 2  # empty string

    def test_list_backup_runs_filtered_by_service(self, store):
        # seed job-a (svc-a) + run, job-b (svc-b) + run
        ...
        assert len(store.list_backup_runs(service_id="svc-a")) == 1

    def test_list_backup_alerts_filtered_by_service(self, store):
        # seed job-a (svc-a) + alert, job-b (svc-b) + alert
        ...
        assert len(store.list_backup_alerts(service_id="svc-a")) == 1

7.2 Frontend tests

Hook test (new test file or existing hook test): assert queryKey differs by serviceId:

it("produces different query keys for different serviceIds", () => {
    const { result: a } = renderHook(() => useBackupJobs("svc-a"));
    const { result: b } = renderHook(() => useBackupJobs("svc-b"));
    // queryCache keys differ — mock the query client to inspect keys
    expect(a).toBeDefined();
    expect(b).toBeDefined();
});

Tab test (update existing tab test or add): assert instance.id is passed. The cleanest approach is to mock the hook and assert it receives instance.id:

it("passes instance.id to scoped hooks", () => {
    const spy = vi.spyOn(useObservability, "useAlertmanagerAlerts");
    render(<AlertsTab instance={{ id: "svc-1", ... }} />);
    expect(spy).toHaveBeenCalledWith("svc-1");
});

---

8. Slice plan

Single slice (~250–350 lines). The change is small and cohesive:

Area	Files	Est. lines
Backend store filter	settings_store.py (3 methods)	~40
Backend endpoints	backups.py (3 endpoints)	~12
Backend tests	test_backups.py	~60
Frontend hooks	useObservability.ts, useBackups.ts	~30
Frontend API fns	client.ts, backups.ts	~20
Frontend tabs	AlertsTab.tsx, MetricsTab.tsx, JobsTab.tsx	~15
Frontend tests	hook test + tab test	~80
Total		~257

Fits comfortably within the 400-line review budget. No need to split.

---

9. Risks and mitigations

Risk	Mitigation
Cache-key collision if serviceId omitted from queryKey in some hook.	Mandatory: every modified hook includes serviceId ?? "" in queryKey. Covered by a hook test asserting different keys for different serviceIds.
Subquery performance — job_id IN (SELECT id FROM backup_jobs WHERE service_id = ?).	Negligible: backup_jobs is small (tens of rows in a homelab). The subquery is indexed on PK. No concern.
Backup store filter regresses existing callers. list_backup_* with service_id=None must return everything.	Explicit tests for None AND "" (both must return all rows). PI-111 scenarios.
Dashboard widget regression (widgets use different hooks).	PI-117: widgets use useWidgetData, not the modified hooks. Widget tests unchanged. Defensive: grep confirms no widget imports any of the 6 modified hooks.
build_backup_dashboard_summary callers. It calls store.list_backup_jobs() with no args — the new default service_id=None preserves this.	Backward-compat by design (§4.2 truthy-check).

---

Summary of key design decisions

1. QueryKey: append serviceId ?? "" as last tuple element — single stable key for undefined, separate key per instance.
2. API client: reuse get<T>(path, params?) with conditional spread — no new helper.
3. Endpoint signatures: service_id: str | None = None appended to existing params, threaded into store.
4. Store filter: direct WHERE service_id = ? for list_backup_jobs; subquery job_id IN (SELECT id FROM backup_jobs WHERE service_id = ?) for runs/alerts (schema asymmetry). Truthy-check: both None and "" skip the filter.
5. fetchBackupDashboard excluded: feeds the dashboard widget path, not any tab. Scoping it would risk PI-117.
6. Single slice: ~257 lines, well within budget.