fix: make external plugin discovery explicit

docling/cli/main.py

@@ -148,9 +148,14 @@
 from docling.models.factories import (
     get_layout_factory,
     get_ocr_factory,
+    get_picture_description_factory,
     get_table_structure_factory,
 )
 from docling.models.factories.base_factory import BaseFactory
+from docling.models.factories.plugin_registry import (
+    is_internal_plugin_distribution,
+    load_plugin_modules,
+)
 from docling.utils.profiling import ProfilingItem
 
 warnings.filterwarnings(action="ignore", category=UserWarning, module="pydantic|torch")
@@ -239,9 +244,6 @@ def _expand_from_formats(from_formats: list[str] | None) -> list[InputFormat]:
     return list(dict.fromkeys(expanded_formats))
 
 
-ocr_factory_internal = get_ocr_factory(allow_external_plugins=False)
-ocr_engines_enum_internal = ocr_factory_internal.get_enum()
-
 # Get available VLM presets from the registry
 vlm_preset_ids = VlmConvertOptions.list_preset_ids()
 
@@ -405,24 +407,36 @@ def show_external_plugins_callback(value: bool):
         ocr_factory_all = get_ocr_factory(allow_external_plugins=True)
         layout_factory_all = get_layout_factory(allow_external_plugins=True)
         table_factory_all = get_table_structure_factory(allow_external_plugins=True)
+        picture_factory_all = get_picture_description_factory(
+            allow_external_plugins=True
+        )
+        plugin_packages = {
+            plugin.name: plugin.distribution_name
+            for plugin in load_plugin_modules(
+                BaseFactory.default_plugin_name,
+                allow_external_plugins=True,
+            )
+        }
 
         def print_external_plugins(factory: BaseFactory, factory_name: str):
             table = rich.table.Table(title=f"Available {factory_name} engines")
             table.add_column("Name", justify="right")
             table.add_column("Plugin")
             table.add_column("Package")
             for meta in factory.registered_meta.values():
-                if not meta.module.startswith("docling."):
+                package = plugin_packages.get(meta.plugin_name)
+                if package is not None and not is_internal_plugin_distribution(package):
                     table.add_row(
                         f"[bold]{meta.kind}[/bold]",
                         meta.plugin_name,
-                        meta.module.split(".")[0],
+                        package,
                     )
             rich.print(table)
 
         print_external_plugins(ocr_factory_all, "OCR")
         print_external_plugins(layout_factory_all, "layout")
         print_external_plugins(table_factory_all, "table")
+        print_external_plugins(picture_factory_all, "picture description")
 
         raise typer.Exit()
 
@@ -845,9 +859,8 @@ def convert(  # noqa: C901
         typer.Option(
             ...,
             help=(
-                f"The OCR engine to use. When --allow-external-plugins is *not* set, the available values are: "
-                f"{', '.join(o.value for o in ocr_engines_enum_internal)}. "
-                f"Use the option --show-external-plugins to see the options allowed with external plugins."
+                "The registered OCR engine kind to use. Use "
+                "--show-external-plugins to list third-party options."
             ),
         ),
     ] = OcrAutoOptions.kind,

docling/models/factories/base_factory.py

@@ -1,20 +1,35 @@
 import enum
 import logging
 from abc import ABCMeta
-from typing import Generic, Optional, Type, TypeVar
+from collections.abc import Mapping, Sequence
+from typing import Generic, Literal, TypeVar, cast
 
-from pluggy import PluginManager
 from pydantic import BaseModel
 
 from docling.datamodel.pipeline_options import BaseOptions
 from docling.models.base_model import BaseModelWithOptions
+from docling.models.factories.plugin_registry import load_plugin_modules
 
 A = TypeVar("A", bound=BaseModelWithOptions)
+PluginCapability = Literal[
+    "layout_engines",
+    "ocr_engines",
+    "picture_description",
+    "table_structure_engines",
+]
 
 
 logger = logging.getLogger(__name__)
 
 
+class PluginConfigurationError(RuntimeError):
+    """A plugin hook returned data outside Docling's plugin contract."""
+
+
+class PluginHookError(RuntimeError):
+    """A plugin hook failed while declaring its models."""
+
+
 class FactoryMeta(BaseModel):
     kind: str
     plugin_name: str
@@ -23,19 +38,26 @@ class FactoryMeta(BaseModel):
 
 class BaseFactory(Generic[A], metaclass=ABCMeta):
     default_plugin_name = "docling"
+    model_type: type[BaseModelWithOptions] | None = None
 
-    def __init__(self, plugin_attr_name: str, plugin_name=default_plugin_name):
+    def __init__(
+        self,
+        plugin_attr_name: PluginCapability,
+        plugin_name: str = default_plugin_name,
+    ) -> None:
         self.plugin_name = plugin_name
         self.plugin_attr_name = plugin_attr_name
 
-        self._classes: dict[Type[BaseOptions], Type[A]] = {}
-        self._meta: dict[Type[BaseOptions], FactoryMeta] = {}
+        self._classes: dict[type[BaseOptions], type[A]] = {}
+        self._options_by_kind: dict[str, type[BaseOptions]] = {}
+        self._meta: dict[type[BaseOptions], FactoryMeta] = {}
+        self._model_contracts: dict[type[A], tuple[type[BaseOptions], str]] = {}
 
     @property
     def registered_kind(self) -> list[str]:
-        return [opt.kind for opt in self._classes.keys()]
+        return list(self._options_by_kind)
 
-    def get_enum(self) -> enum.Enum:
+    def get_enum(self) -> type[enum.Enum]:
         return enum.Enum(
             self.plugin_attr_name + "_enum",
             names={kind: kind for kind in self.registered_kind},
@@ -44,25 +66,26 @@ def get_enum(self) -> enum.Enum:
         )
 
     @property
-    def classes(self):
+    def classes(self) -> Mapping[type[BaseOptions], type[A]]:
         return self._classes
 
     @property
-    def registered_meta(self):
+    def registered_meta(self) -> Mapping[type[BaseOptions], FactoryMeta]:
         return self._meta
 
     def create_instance(self, options: BaseOptions, **kwargs) -> A:
         try:
-            _cls = self._classes[type(options)]
-            return _cls(options=options, **kwargs)
+            model_class = self._classes[type(options)]
         except KeyError:
-            raise RuntimeError(self._err_msg_on_class_not_found(options.kind))
+            raise RuntimeError(self._err_msg_on_class_not_found(options.kind)) from None
+        return model_class(options=options, **kwargs)
 
     def create_options(self, kind: str, *args, **kwargs) -> BaseOptions:
-        for opt_cls, _ in self._classes.items():
-            if opt_cls.kind == kind:
-                return opt_cls(*args, **kwargs)
-        raise RuntimeError(self._err_msg_on_class_not_found(kind))
+        try:
+            options_type = self._options_by_kind[kind]
+        except KeyError:
+            raise RuntimeError(self._err_msg_on_class_not_found(kind)) from None
+        return options_type(*args, **kwargs)
 
     def _err_msg_on_class_not_found(self, kind: str):
         msg = []
@@ -74,49 +97,178 @@ def _err_msg_on_class_not_found(self, kind: str):
 
         return f"No class found with the name {kind!r}, known classes are:\n{msg_str}"
 
-    def register(self, cls: Type[A], plugin_name: str, plugin_module_name: str):
-        opt_type = cls.get_options_type()
-
-        if opt_type in self._classes:
-            raise ValueError(
-                f"{opt_type.kind!r} already registered to class {self._classes[opt_type]!r}"
-            )
-
-        self._classes[opt_type] = cls
-        self._meta[opt_type] = FactoryMeta(
-            kind=opt_type.kind, plugin_name=plugin_name, module=plugin_module_name
+    def register(
+        self,
+        cls: type[A],
+        plugin_name: str,
+        plugin_module_name: str,
+    ) -> None:
+        self._register_models(
+            (cls,),
+            plugin_name=plugin_name,
+            plugin_module_name=plugin_module_name,
         )
 
     def load_from_plugins(
-        self, plugin_name: Optional[str] = None, allow_external_plugins: bool = False
-    ):
+        self, plugin_name: str | None = None, allow_external_plugins: bool = False
+    ) -> None:
         plugin_name = plugin_name or self.plugin_name
 
-        plugin_manager = PluginManager(plugin_name)
-        plugin_manager.load_setuptools_entrypoints(plugin_name)
+        for plugin in load_plugin_modules(
+            plugin_name,
+            allow_external_plugins=allow_external_plugins,
+        ):
+            # Plugin hook names are the documented third-party interface.
+            hook = getattr(plugin.module, self.plugin_attr_name, None)
+
+            if hook is None:
+                continue
+            if not callable(hook):
+                raise self._configuration_error(
+                    plugin.name,
+                    f"the {self.plugin_attr_name!r} hook must be callable",
+                )
+
+            logger.info("Loading plugin %r", plugin.name)
+            try:
+                config = hook()
+            except Exception as exc:
+                raise PluginHookError(
+                    f"Plugin {plugin.name!r} failed while running its "
+                    f"{self.plugin_attr_name!r} hook: {exc}"
+                ) from exc
+            self.process_plugin(
+                config,
+                plugin.name,
+                plugin.module_name,
+            )
+
+    def process_plugin(
+        self,
+        config: object,
+        plugin_name: str,
+        plugin_module_name: str,
+    ) -> None:
+        if not isinstance(config, Mapping):
+            raise self._configuration_error(
+                plugin_name, "the hook must return a mapping"
+            )
 
-        for plugin_name, plugin_module in plugin_manager.list_name_plugin():
-            plugin_module_name = str(plugin_module.__name__)  # type: ignore
+        plugin_config = cast(Mapping[object, object], config)
+        models = plugin_config.get(self.plugin_attr_name)
+        if not isinstance(models, list):
+            raise self._configuration_error(
+                plugin_name,
+                f"the {self.plugin_attr_name!r} value must be a list of model classes",
+            )
 
-            if not allow_external_plugins and not plugin_module_name.startswith(
-                "docling."
+        validated_models: list[type[A]] = []
+        for index, model in enumerate(models):
+            if not isinstance(model, type) or (
+                self.model_type is not None and not issubclass(model, self.model_type)
             ):
-                logger.warning(
-                    f"The plugin {plugin_name} will not be loaded because Docling is being executed with allow_external_plugins=false."
+                expected_model = (
+                    self.model_type.__name__
+                    if self.model_type is not None
+                    else "BaseModelWithOptions"
                 )
-                continue
+                raise self._configuration_error(
+                    plugin_name,
+                    f"{self.plugin_attr_name!r} item {index} must be a "
+                    f"{expected_model} model class",
+                )
+            validated_models.append(cast(type[A], model))
+
+        self._validate_registrations(validated_models, plugin_name)
+        for model in validated_models:
+            self.register(model, plugin_name, plugin_module_name)
+
+    def _register_models(
+        self,
+        models: Sequence[type[A]],
+        *,
+        plugin_name: str,
+        plugin_module_name: str,
+    ) -> None:
+        self._validate_registrations(models, plugin_name)
+        for model in models:
+            options_type, kind = self._validate_options_type(model, plugin_name)
+            self._classes[options_type] = model
+            self._options_by_kind[kind] = options_type
+            self._meta[options_type] = FactoryMeta(
+                kind=kind,
+                plugin_name=plugin_name,
+                module=plugin_module_name,
+            )
 
-            attr = getattr(plugin_module, self.plugin_attr_name, None)
+    def _validate_registrations(
+        self, models: Sequence[type[A]], plugin_name: str
+    ) -> None:
+        classes = self._classes.copy()
+        options_by_kind = self._options_by_kind.copy()
+
+        for model in models:
+            options_type, kind = self._validate_options_type(model, plugin_name)
+            registered_model = classes.get(options_type)
+            if registered_model is not None:
+                raise self._configuration_error(
+                    plugin_name,
+                    f"{options_type.__name__} is already registered to "
+                    f"{registered_model.__name__}, so it cannot also register "
+                    f"{model.__name__}",
+                )
 
-            if callable(attr):
-                logger.info("Loading plugin %r", plugin_name)
+            registered_options = options_by_kind.get(kind)
+            if registered_options is not None:
+                registered_model = classes[registered_options]
+                raise self._configuration_error(
+                    plugin_name,
+                    f"model kind {kind!r} is already registered to "
+                    f"{registered_model.__name__}, so it cannot also register "
+                    f"{model.__name__}",
+                )
 
-                config = attr()
-                self.process_plugin(config, plugin_name, plugin_module_name)
+            classes[options_type] = model
+            options_by_kind[kind] = options_type
 
-    def process_plugin(self, config, plugin_name: str, plugin_module_name: str):
-        for item in config[self.plugin_attr_name]:
-            try:
-                self.register(item, plugin_name, plugin_module_name)
-            except ValueError:
-                logger.warning("%r already registered", item)
+    def _validate_options_type(
+        self, model: type[A], plugin_name: str
+    ) -> tuple[type[BaseOptions], str]:
+        cached_contract = self._model_contracts.get(model)
+        if cached_contract is not None:
+            return cached_contract
+
+        try:
+            options_type = model.get_options_type()
+        except Exception as exc:
+            raise self._configuration_error(
+                plugin_name,
+                f"{model.__name__}.get_options_type() failed: {exc}",
+            ) from exc
+
+        if not isinstance(options_type, type) or not issubclass(
+            options_type, BaseOptions
+        ):
+            raise self._configuration_error(
+                plugin_name,
+                f"{model.__name__}.get_options_type() must return a "
+                "BaseOptions subclass",
+            )
+
+        kind = vars(options_type).get("kind")
+        if not isinstance(kind, str) or not kind:
+            raise self._configuration_error(
+                plugin_name,
+                f"{options_type.__name__} must declare a non-empty string kind",
+            )
+        contract = (options_type, kind)
+        self._model_contracts[model] = contract
+        return contract
+
+    def _configuration_error(
+        self, plugin_name: str, problem: str
+    ) -> PluginConfigurationError:
+        return PluginConfigurationError(
+            f"Plugin {plugin_name!r} has an invalid {self.plugin_attr_name!r} "
+            f"contract: {problem}."
+        )