windy/dl-organizer
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

refactor enrichment to tmdb-only and fix review findings

Browse Source
This commit is contained in:
windyboy
2026-02-10 08:29:31 +08:00
parent 53aaeeaedf
commit f0c951ad7f
8 changed files with 187 additions and 127 deletions
Download Patch File Download Diff File Expand all files Collapse all files
README.md
+1 -3
View File
@@ -387,17 +387,15 @@ enrichment:
enabled: true
incremental: true
refresh_mode: "manual"
providers: [tmdb, douban]
providers: [tmdb]
cache_db: "~/.vlm/enrichment_cache.db"
max_concurrency: 6
min_match_score: 0.75
douban_endpoint: null
translation:
mode: "bidirectional"
fallback_machine: true
api_keys:
tmdb: null
douban: null
openai: null
reputation:
min_votes: 50
REVIEW_REPORT.md
+50
View File
@@ -218,6 +218,56 @@
- Add developer note for test import convention: always import from package root (`vlm`), not `src.vlm`.
### Execution checklist (single pass)
## Enrichment 增量评审报告（2026-02-10）
### 评审范围
- 文件：`src/vlm/enrichment.py`
- 关注点：刷新一致性、配置可观测性、命名输出质量
### 总体评分（修复后）
- 当前得分：**92 / 100**
- 评分依据：
- 正确性（40 分）：37/40（refresh 无命中不再残留旧字段）
- 健壮性（30 分）：27/30（未知 provider 已 fail-fast）
- 输出质量（20 分）：18/20（display title 去重）
- 可维护性（10 分）：10/10（TMDB-only，分支复杂度降低）
### 问题明细与修复状态
#### 1) P1 - 刷新失败时未清理陈旧富化字段（**fixed**）
- 修复位置：`src/vlm/enrichment.py:378`
- 修复内容：
- `_apply_payload` 从“仅写入非 None”改为“payload 含 key 即覆盖”，支持将字段显式清空为 `None`。
- 验证：
- `tests/test_enrichment.py:187` 覆盖 refresh 后无匹配场景，断言旧 `canonical_id/title_zh/title_en/reputation_*` 被清空，`enrichment_confidence == 0.0`。
#### 2) P2 - 未识别 provider 被静默忽略（**fixed**）
- 修复位置：
- `src/vlm/enrichment.py:157`
- `src/vlm/config.py:271`
- 修复内容：
- `_build_providers` 对未知 provider 直接抛 `ValueError`（fail-fast）。
- 配置校验新增白名单，仅允许 `tmdb`。
- 同步重构为 TMDB-only，移除 enrich 主流程中的 Douban 分支。
- 验证：
- `tests/test_enrichment.py:176` 覆盖未知 provider 报错。
- `tests/test_config.py:447` 覆盖配置校验拒绝不支持 provider。
#### 3) P2 - display_title fallback 可能重复标题（**fixed**）
- 修复位置：`src/vlm/enrichment.py:423`
- 修复内容：
- `_build_display_title` 增加同值去重逻辑；`title_zh/title_en` 回退同值时只保留一个。
- 验证：
- `tests/test_enrichment.py:260` 断言双空回退时输出单标题而非重复标题。
### 结果
1. 三项问题均已修复并有测试覆盖。
2. enrich 已切换为 TMDB-only，配置和文档已同步。
3. 全量测试通过：`uv run --with pytest --with hypothesis pytest -q` -> `411 passed`。
### 修复后目标分
- 达成：**92 / 100**。
1. Apply Phase 1, run focused + full tests.
2. Apply Phase 2, add timezone tests, run focused + full tests.
3. Apply Phase 3, run scanner tests + full tests.
src/vlm/config.py
+9 -8
View File
@@ -30,15 +30,13 @@ class Config:
enrichment_enabled: bool = True
enrichment_incremental: bool = True
enrichment_refresh_mode: str = "manual"
enrichment_providers: list[str] = field(default_factory=lambda: ["tmdb", "douban"])
enrichment_providers: list[str] = field(default_factory=lambda: ["tmdb"])
enrichment_cache_db: Path = field(default_factory=lambda: Path.home() / ".vlm" / "enrichment_cache.db")
enrichment_max_concurrency: int = 6
enrichment_min_match_score: float = 0.75
translation_mode: str = "bidirectional"
translation_fallback_machine: bool = True
tmdb_api_key: Optional[str] = None
douban_api_key: Optional[str] = None
douban_api_endpoint: Optional[str] = None
openai_api_key: Optional[str] = None
reputation_min_votes: int = 50
reputation_low_score_threshold: float = 6.0
@@ -103,7 +101,7 @@ def load_config(path: Path) -> Config:
enrichment_enabled=enrichment.get("enabled", True),
enrichment_incremental=enrichment.get("incremental", True),
enrichment_refresh_mode=enrichment.get("refresh_mode", "manual"),
enrichment_providers=enrichment.get("providers", ["tmdb", "douban"]),
enrichment_providers=enrichment.get("providers", ["tmdb"]),
enrichment_cache_db=Path(
enrichment.get("cache_db", str(Path.home() / ".vlm" / "enrichment_cache.db"))
).expanduser(),
@@ -112,8 +110,6 @@ def load_config(path: Path) -> Config:
translation_mode=translation.get("mode", "bidirectional"),
translation_fallback_machine=translation.get("fallback_machine", True),
tmdb_api_key=api_keys.get("tmdb"),
douban_api_key=api_keys.get("douban"),
douban_api_endpoint=enrichment.get("douban_endpoint"),
openai_api_key=api_keys.get("openai"),
reputation_min_votes=reputation.get("min_votes", 50),
reputation_low_score_threshold=reputation.get("low_score_threshold", 6.0),
@@ -155,10 +151,8 @@ def create_default_config(path: Path) -> Config:
},
"api_keys": {
"tmdb": default_config.tmdb_api_key,
"douban": default_config.douban_api_key,
"openai": default_config.openai_api_key,
},
"douban_endpoint": default_config.douban_api_endpoint,
"reputation": {
"min_votes": default_config.reputation_min_votes,
"low_score_threshold": default_config.reputation_low_score_threshold,
@@ -276,6 +270,13 @@ def validate_config(config: Config) -> list[str]:
errors.append("enrichment_cache_db must be a Path object")
if not isinstance(config.enrichment_providers, list) or not config.enrichment_providers:
errors.append("enrichment_providers must be a non-empty list")
else:
allowed_providers = {"tmdb"}
invalid = [provider for provider in config.enrichment_providers if provider.lower() not in allowed_providers]
if invalid:
errors.append(
f"enrichment_providers contains unsupported providers: {invalid}; supported providers: ['tmdb']"
)
if config.enrichment_max_concurrency < 1:
errors.append("enrichment_max_concurrency must be >= 1")
if not (0.0 <= config.enrichment_min_match_score <= 1.0):
src/vlm/enrichment.py
+21 -16
View File
@@ -12,7 +12,7 @@ from urllib.request import urlopen, Request
from vlm.cache import EnrichmentCache
from vlm.config import Config
from vlm.providers import ProviderResult, TMDBProvider, DoubanProvider
from vlm.providers import ProviderResult, TMDBProvider
from vlm.parser import normalize_title
RefreshMode = str
@@ -156,6 +156,7 @@ def _emit_progress(stats: dict[str, int | list[dict[str, str]]], progress_callba
def _build_providers(config: Config, *, request_timeout: int, retries: int) -> list:
providers = []
unsupported: list[str] = []
for name in config.enrichment_providers:
key = name.lower()
if key == "tmdb":
@@ -167,16 +168,14 @@ def _build_providers(config: Config, *, request_timeout: int, retries: int) -> l
min_interval_seconds=0.25,
)
)
elif key == "douban":
providers.append(
DoubanProvider(
config.douban_api_key,
endpoint=config.douban_api_endpoint,
timeout_seconds=request_timeout,
retries=retries,
min_interval_seconds=0.4,
)
)
else:
unsupported.append(name)
if unsupported:
raise ValueError(
f"Unsupported enrichment providers: {unsupported}. Supported providers: ['tmdb']"
)
return providers
@@ -390,7 +389,7 @@ def _apply_payload(record: dict, payload: dict) -> None:
"provider_metadata",
"display_title",
):
if key in payload and payload[key] is not None:
if key in payload:
record[key] = payload[key]
if "needs_review" in payload:
@@ -422,10 +421,16 @@ def _fallback_title_from_filename(filename: Optional[str]) -> Optional[str]:
def _build_display_title(record: dict, payload: dict, config: Config) -> str:
title_zh = payload.get("title_zh") or record.get("title")
title_en = payload.get("title_en") or record.get("title")
fallback_title = record.get("title")
title_zh = payload.get("title_zh") or fallback_title
title_en = payload.get("title_en") or fallback_title
if title_zh and title_en and title_zh == title_en:
title_en = ""
try:
formatted = config.naming_title_format.format(title_zh=title_zh, title_en=title_en).strip()
formatted = config.naming_title_format.format(
title_zh=title_zh or "",
title_en=title_en or "",
).strip()
except Exception:
formatted = f"{title_zh} {title_en}".strip()
formatted = f"{title_zh or ''} {title_en or ''}".strip()
return " ".join(formatted.split())
src/vlm/providers/__init__.py
-2
View File
@@ -2,11 +2,9 @@
from vlm.providers.base import EnrichmentProvider, ProviderResult
from vlm.providers.tmdb import TMDBProvider
from vlm.providers.douban import DoubanProvider
__all__ = [
"EnrichmentProvider",
"ProviderResult",
"TMDBProvider",
"DoubanProvider",
]
src/vlm/providers/douban.py
-97
View File
@@ -1,97 +0,0 @@
"""Douban provider implementation.
This provider is optional. If no endpoint is configured, it silently degrades.
"""
from __future__ import annotations
import json
import time
from typing import Optional
from urllib.parse import urlencode
from urllib.request import urlopen, Request
from vlm.providers.base import ProviderResult
class DoubanProvider:
"""Fetch reputation data from a configurable Douban-compatible API."""
name = "douban"
def __init__(
self,
api_key: Optional[str],
endpoint: Optional[str] = None,
timeout_seconds: int = 6,
retries: int = 2,
min_interval_seconds: float = 0.4,
) -> None:
self.api_key = api_key
self.endpoint = endpoint
self.timeout_seconds = timeout_seconds
self.retries = retries
self.min_interval_seconds = min_interval_seconds
self._last_request_at = 0.0
def enrich(self, *, title: str, media_type: str, year: Optional[int] = None) -> Optional[ProviderResult]:
if not self.endpoint:
return None
params = {
"q": title,
"type": media_type,
}
if year is not None:
params["year"] = year
if self.api_key:
params["api_key"] = self.api_key
data = self._get_json(self.endpoint, params)
if not data:
return None
items = data.get("items") or data.get("subjects") or []
if not items:
return None
item = items[0]
score = item.get("rating") or item.get("score")
votes = item.get("vote_count") or item.get("ratings_count")
title_zh = item.get("title")
title_en = item.get("original_title")
return ProviderResult(
provider=self.name,
canonical_id=f"douban:{item.get('id', 'unknown')}",
title_zh=title_zh,
title_en=title_en,
translation_source=self.name if title_zh or title_en else None,
reputation_score=float(score) if score is not None else None,
reputation_votes=int(votes) if votes is not None else None,
reputation_source=self.name,
raw_metadata={"id": str(item.get("id", ""))},
)
def _wait_for_rate_limit(self) -> None:
if self.min_interval_seconds <= 0:
return
now = time.monotonic()
elapsed = now - self._last_request_at
if elapsed < self.min_interval_seconds:
time.sleep(self.min_interval_seconds - elapsed)
def _get_json(self, url: str, params: dict) -> Optional[dict]:
full_url = f"{url}?{urlencode(params)}"
request = Request(full_url, headers={"Accept": "application/json"})
for _ in range(max(self.retries + 1, 1)):
self._wait_for_rate_limit()
try:
with urlopen(request, timeout=self.timeout_seconds) as response:
payload = response.read().decode("utf-8")
self._last_request_at = time.monotonic()
return json.loads(payload)
except Exception:
self._last_request_at = time.monotonic()
continue
return None
tests/test_config.py
+10
View File
@@ -20,6 +20,7 @@ class TestConfig:
assert config.series_template == "series/{title}/Season {season:02d}/"
assert config.log_level == "INFO"
assert config.quarantine_dir == ".quarantine"
assert config.enrichment_providers == ["tmdb"]
def test_config_creation_with_custom_values(self):
"""Test creating Config with custom values."""
@@ -443,6 +444,15 @@ class TestValidateConfig:
errors = validate_config(config)
assert any("cannot be empty" in e for e in errors)
def test_validate_rejects_unsupported_enrichment_provider(self):
"""Test validating config with unsupported enrichment provider."""
config = Config(
library_root=Path("/test"),
enrichment_providers=["tmdb", "douban"],
)
errors = validate_config(config)
assert any("unsupported providers" in e for e in errors)
def test_validate_category_list_with_non_string(self):
"""Test validating config with non-string in category list."""
config = Config(
tests/test_enrichment.py
+96 -1
View File
@@ -1,7 +1,9 @@
"""Unit tests for enrichment pipeline."""
import pytest
from vlm.config import Config
from vlm.enrichment import enrich_identities_data
from vlm.enrichment import _build_display_title, _build_providers, enrich_identities_data
from vlm.providers.base import ProviderResult
@@ -169,3 +171,96 @@ def test_enrich_refresh_all_bypasses_cache(tmp_path, monkeypatch):
_, stats = enrich_identities_data(identities, config, refresh_mode="refresh_all")
assert provider.calls == 2
assert stats["cache_hits"] == 0
def test_build_providers_rejects_unknown_provider(tmp_path):
"""Unknown providers should fail fast with a clear error."""
config = Config(
library_root=tmp_path,
enrichment_providers=["tmdb", "tmdb_typo"],
)
with pytest.raises(ValueError, match="Unsupported enrichment providers"):
_build_providers(config, request_timeout=3, retries=1)
def test_refresh_all_clears_stale_enrichment_fields(tmp_path, monkeypatch):
"""refresh_all with no new match should clear stale enrichment data."""
class FlakyProvider:
name = "dummy"
def __init__(self):
self.calls = 0
def enrich(self, *, title: str, media_type: str, year=None):
self.calls += 1
if self.calls == 1:
return ProviderResult(
provider="dummy",
canonical_id="dummy:test",
title_zh="测试",
title_en="Test",
translation_source="dummy",
reputation_score=8.8,
reputation_votes=120,
reputation_source="dummy",
)
return None
config = Config(
library_root=tmp_path,
enrichment_cache_db=tmp_path / "cache.db",
enrichment_providers=["dummy"],
translation_fallback_machine=False,
)
provider = FlakyProvider()
monkeypatch.setattr(
"vlm.enrichment._build_providers",
lambda _config, request_timeout, retries: [provider],
)
identities = {
"metadata": {},
"movies": [
{
"path": "/library/movie/Test.2020.mkv",
"filename": "Test.2020.mkv",
"category": "movie",
"title": "Test",
"year": 2020,
"confidence": 0.9,
"needs_review": False,
}
],
"series": [],
"anime": [],
"other": [],
}
enrich_identities_data(identities, config, refresh_mode="incremental")
movie = identities["movies"][0]
assert movie["canonical_id"] == "dummy:test"
assert movie["title_zh"] == "测试"
assert movie["title_en"] == "Test"
enrich_identities_data(identities, config, refresh_mode="refresh_all")
assert movie["canonical_id"] is None
assert movie["title_zh"] is None
assert movie["title_en"] is None
assert movie["translation_source"] is None
assert movie["reputation_score"] is None
assert movie["reputation_votes"] is None
assert movie["reputation_source"] is None
assert movie["enrichment_confidence"] == 0.0
def test_build_display_title_deduplicates_fallback_title(tmp_path):
"""Fallback display title should not duplicate identical names."""
config = Config(library_root=tmp_path)
record = {"title": "Interstellar"}
payload = {"title_zh": None, "title_en": None}
assert _build_display_title(record, payload, config) == "Interstellar"