# Tasks — NFO Plot Missing Bug

These tasks fix the root causes of `<plot>` being empty in `tvshow.nfo` after adding a series via the web UI.
The bug does **not** appear after a server restart because the repair scan uses a different, correctly isolated code path.

---

## Task 1 — Replace shared NFOService in BackgroundLoaderService with per-task instances

- [x] Completed

### Where
`src/server/services/background_loader_service.py` — method `_load_nfo_and_images` (~line 555)

```python
nfo_path = await self.series_app.nfo_service.create_tvshow_nfo(
    serie_name=task.name,
    serie_folder=task.folder,
    year=task.year,
    ...
)
```

### Goal
Create a fresh, isolated `NFOService` (with its own `TMDBClient` and `aiohttp` session) for every background loading task, exactly the same way `_repair_one_series` in `initialization_service.py` already does it.
Each task must own its client so that closing the session at the end of one task never kills an in-flight request inside another task.

### How it should look
```python
from src.core.services.nfo_factory import NFOServiceFactory

factory = NFOServiceFactory()
nfo_service = factory.create()
nfo_path = await nfo_service.create_tvshow_nfo(
    serie_name=task.name,
    serie_folder=task.folder,
    year=task.year,
    ...
)
```

### Possible traps and issues
- `NFOServiceFactory.create()` raises `ValueError` if no TMDB API key is available.  Wrap in try/except and fall back gracefully (same behaviour as now when `nfo_service` is `None`).
- The factory reads the API key from `settings` first, then from `config.json`. Do not pass the key explicitly so the fallback chain stays intact.
- Each new `NFOService` opens its own `aiohttp` connector. Make sure to call `await nfo_service.close()` in a `finally` block to avoid connector leaks.

### Docs changes needed
None — this is an internal implementation detail.

### Why this is needed
Up to 5 background workers share one `NFOService`/`TMDBClient` instance. The `async with self.tmdb_client:` context manager inside `create_tvshow_nfo` calls `close()` on `__aexit__`, setting `session = None`. When Worker B exits its context while Worker A is still inside `_enrich_details_with_fallback` trying the `en-US` fallback request, that request throws "Connector is closed". The exception is silently swallowed, both `en-US` and `ja-JP` fallbacks fail, `details["overview"]` stays empty, and `plot` is written as an empty element.

---

## Task 2 — Guard NFOService init in SeriesApp on factory fallback, not just env var

- [x] Completed

### Where
`src/core/SeriesApp.py` — `__init__` method (~line 175)

```python
self.nfo_service: Optional[NFOService] = None
if settings.tmdb_api_key:          # ← checks env var ONLY
    factory = get_nfo_factory()
    self.nfo_service = factory.create()
```

### Goal
The guard condition should be equivalent to what `NFOServiceFactory.create()` itself checks: whether the key is available from *any* source (env var or `config.json`). Replace the guard with a try/create pattern so that `nfo_service` is initialised whenever the factory would succeed.

### How it should look
```python
self.nfo_service: Optional[NFOService] = None
try:
    from src.core.services.nfo_factory import get_nfo_factory
    factory = get_nfo_factory()
    self.nfo_service = factory.create()
    logger.info("NFO service initialized successfully")
except ValueError:
    logger.info("NFO service not available — TMDB API key not configured")
except Exception as e:
    logger.warning("Failed to initialize NFO service: %s", e)
```

### Possible traps and issues
- This changes the condition from "env var set" to "factory can produce a service". The factory already has a safe fallback and raises `ValueError` when no key exists — so the `except ValueError` path is the normal "not configured" case, not an error.
- `SeriesApp` is used in tests with `settings.tmdb_api_key = None`. Those tests must not be affected; the `except ValueError` branch keeps behaviour identical.
- `series_app.nfo_service` is still `None` when not configured — downstream code that checks `if self.series_app.nfo_service:` remains correct.

### Docs changes needed
`docs/CONFIGURATION.md` — note that `TMDB_API_KEY` env var is not required if `nfo.tmdb_api_key` is set in `config.json`.

### Why this is needed
If the TMDB API key is configured only via `config.json` (not the `TMDB_API_KEY` env var), `settings.tmdb_api_key` is `None` and the guard prevents `nfo_service` from ever being created. The background loader then skips NFO creation completely (`nfo_service` is `None`). The repair scan at startup uses `NFOServiceFactory` directly (reads config.json) so it does create the NFO — which is exactly why restart works but add does not.

---

## Task 3 — Remove non-reentrant `async with self.tmdb_client:` from NFOService public methods

- [x] Completed

### Where
`src/core/services/nfo_service.py` — `create_tvshow_nfo` (~line 151) and `update_tvshow_nfo` (~line 265)

```python
async with self.tmdb_client:
    details = await self.tmdb_client.get_tv_show_details(...)
    ...
```

### Goal
The `TMDBClient.__aenter__` / `__aexit__` open and **close** the session, making any concurrent call to the same client instance fail. Because Task 1 creates a fresh instance per call, this context manager becomes redundant. Change both methods to use `_ensure_session()` at the start and `close()` in a `finally` block, or simply call `await self.tmdb_client._ensure_session()` once and close after all requests. This makes the lifetime explicit and prevents double-close if the caller already manages it.

### How it should look
```python
async def create_tvshow_nfo(self, ...) -> Path:
    try:
        await self.tmdb_client._ensure_session()
        search_results = await self.tmdb_client.search_tv_show(search_name)
        ...
    finally:
        await self.tmdb_client.close()
```

### Possible traps and issues
- `TMDBClient.close()` is idempotent (checks `session.closed` before closing), so calling it in `finally` is safe even if the try block never opened a session.
- After Task 1 every `NFOService` is short-lived (one call), so `finally: close()` effectively replaces the context manager with no behaviour change.
- Do not remove the `__aenter__`/`__aexit__` from `TMDBClient` itself — other callers (e.g. tests, CLI) may still use it as a context manager.
- `update_tvshow_nfo` has the same pattern; fix both methods.

### Docs changes needed
None — internal implementation detail.

### Why this is needed
Even after Task 1 fixes the shared-instance problem, the `async with self.tmdb_client:` pattern is fragile by design: `__aexit__` calls `close()`, which would break any hypothetical future reuse. Removing the implicit close makes the session lifetime explicit and eliminates the root mechanism that caused the original bug.

---

## Task 4 — Add `en-US` search fallback so `search_overview` is never empty

### Where
`src/core/services/nfo_service.py` — `create_tvshow_nfo` (~line 178) and `_enrich_details_with_fallback` (~line 395)

```python
search_overview = tv_show.get("overview") or None  # always None for anime — de-DE search returns ""
```

### Goal
When the German `search_tv_show` result has an empty `overview`, perform a second search in `en-US` to obtain a non-empty overview as the last-resort fallback text. Store this as `search_overview` so `_enrich_details_with_fallback` can use it even if all language-specific detail requests fail.

### How it should look
```python
search_overview = tv_show.get("overview") or None
if not search_overview:
    try:
        en_results = await self.tmdb_client.search_tv_show(search_name, language="en-US")
        en_match = self._find_best_match(en_results.get("results", []), search_name, year)
        search_overview = en_match.get("overview") or None
    except Exception:
        pass  # best-effort only
```

### Possible traps and issues
- This adds one extra TMDB request per series when the German overview is empty. It is best-effort and must be wrapped in a broad `except` so it never blocks NFO creation.
- The TMDB search endpoint rate-limit is generous; one extra request per add is negligible.
- `_find_best_match` can raise `TMDBAPIError` if the result list is empty — catch both `TMDBAPIError` and `Exception`.
- `update_tvshow_nfo` calls `_enrich_details_with_fallback` without `search_overview`. This is acceptable because the detail request with `en-US` fallback covers it; the search overview is only a last resort for the create path.

### Docs changes needed
None — transparent improvement.

### Why this is needed
Most anime have no German translation on TMDB. The `de-DE` search result returns `overview: ""`. The current code stores this as `search_overview = None` so the last-resort fallback in `_enrich_details_with_fallback` never fires. Combined with session contention (Task 1), the detail-level `en-US` fallback also fails, leaving `plot` empty. This task ensures that at least the search-level `en-US` overview is available as a safety net.