fix(parsers): warn on VLM global-model fallback; unify parser dispatch/VALID_PARSERS (#77)

Author: KylinMountain

openkb/parsers/registry.py

@@ -6,7 +6,31 @@
 from openkb.parsers.base import Parser
 from openkb.parsers.local import LocalParser
 
-VALID_PARSERS = ("local", "mineru", "mistral", "vlm")
+
+def _make_mistral(opts, config):
+    from openkb.parsers.mistral import MistralParser
+    return MistralParser(opts)
+
+
+def _make_vlm(opts, config):
+    from openkb.parsers.vlm import VLMParser
+    return VLMParser(opts, model=config.get("model"))
+
+
+def _make_mineru(opts, config):
+    from openkb.parsers.mineru import MineruParser
+    return MineruParser(opts)
+
+
+# Single source of truth: online-parser name -> lazy factory.
+_ONLINE_PARSERS = {
+    "mineru": _make_mineru,
+    "mistral": _make_mistral,
+    "vlm": _make_vlm,
+}
+
+# Valid parser names (drives the CLI --parser choice and error messages).
+VALID_PARSERS = ("local", *_ONLINE_PARSERS)
 
 
 def get_parser(
@@ -21,18 +45,10 @@ def get_parser(
     name = (override or config.get("parser") or "local").lower()
     if name == "local":
         return LocalParser(doc_name=doc_name, images_dir=images_dir, source_dir=source_dir)
-
-    parsers_cfg = config.get("parsers", {}) or {}
-    opts = parsers_cfg.get(name, {}) or {}
-    if name == "mistral":
-        from openkb.parsers.mistral import MistralParser
-        return MistralParser(opts)
-    if name == "vlm":
-        from openkb.parsers.vlm import VLMParser
-        return VLMParser(opts, model=config.get("model"))
-    if name == "mineru":
-        from openkb.parsers.mineru import MineruParser
-        return MineruParser(opts)
-    raise ValueError(
-        f"Unknown parser {name!r}. Valid options: {', '.join(VALID_PARSERS)}."
-    )
+    factory = _ONLINE_PARSERS.get(name)
+    if factory is None:
+        raise ValueError(
+            f"Unknown parser {name!r}. Valid options: {', '.join(VALID_PARSERS)}."
+        )
+    opts = (config.get("parsers", {}) or {}).get(name, {}) or {}
+    return factory(opts, config)

openkb/parsers/vlm.py

@@ -1,11 +1,14 @@
 from __future__ import annotations
 
+import logging
 from pathlib import Path
 from typing import Any
 
 from openkb.parsers.base import ParseResult, Parser
 from openkb.parsers.vlm_client import transcribe_to_markdown
 
+logger = logging.getLogger(__name__)
+
 _SUPPORTED = {".pdf"}
 
 
@@ -18,6 +21,13 @@ def __init__(self, opts: dict[str, Any] | None = None, model: str | None = None)
         opts = opts or {}
         # parsers.vlm.model overrides the global model; else use the global model.
         self.model = opts.get("model") or model
+        if not opts.get("model"):
+            logger.warning(
+                "VLM parser: 'parsers.vlm.model' is not set; using the global model "
+                "%r for vision parsing. If that model is not vision-capable, set "
+                "'parsers.vlm.model' to one (e.g. gemini/gemini-2.5-pro).",
+                self.model,
+            )
 
     def supports(self, suffix: str) -> bool:
         return suffix.lower() in _SUPPORTED

tests/test_parsers_registry.py

@@ -31,3 +31,10 @@ def test_unknown_name_raises_with_valid_options():
         get_parser({"parser": "nope"}, **_kwargs())
     assert "nope" in str(exc.value)
     assert "local" in str(exc.value)
+
+
+def test_valid_parsers_matches_dispatch():
+    from openkb.parsers.registry import VALID_PARSERS, _ONLINE_PARSERS
+    # local + every online factory key, no drift
+    assert set(VALID_PARSERS) == {"local", *_ONLINE_PARSERS}
+    assert VALID_PARSERS[0] == "local"

tests/test_parsers_vlm.py

@@ -31,3 +31,17 @@ def test_parse_falls_back_to_global_model(tmp_path):
     with patch("openkb.parsers.vlm.transcribe_to_markdown", return_value="x") as t:
         p.parse(src)
     t.assert_called_once_with(src, model="global-model")
+
+
+def test_warns_when_falling_back_to_global_model(caplog):
+    import logging as _logging
+    with caplog.at_level(_logging.WARNING):
+        VLMParser({}, model="gpt-5.4-mini")
+    assert any("parsers.vlm.model" in r.message for r in caplog.records)
+
+
+def test_no_warning_when_vlm_model_set(caplog):
+    import logging as _logging
+    with caplog.at_level(_logging.WARNING):
+        VLMParser({"model": "gemini/gemini-2.5-pro"}, model="gpt-5.4-mini")
+    assert not any("parsers.vlm.model" in r.message for r in caplog.records)