yetuge d01538a54c
fix(reflection): reject malformed variable paths before diagnosing them (#7016)
* fix(reflection): reject malformed variable paths before diagnosing them

resolve_variable splits on the last colon and assumed both halves were
usable. Two realistic typos fell through into misleading diagnoses:

- "json:" reached getattr(module, "") and reported
  "Module json does not define a  attribute/class" -- a blank name in the
  middle of the sentence.
- "json:loads:extra" left the extra colon inside the module part, so the
  failed import was reported as Missing dependency and the user was told to
  run uv add json:loads, an instruction that cannot work.

Both are path mistakes, so they now get the same format guidance as a path
with no colon at all. Every provider, extension and model class path in the
repo flows through here, so the message is what an operator sees first.

Tests cover the two typos and assert the missing-dependency hint is not
offered for them.

* fix(reflection): reject relative module paths too, and pin the empty-module half

Review follow-ups on the malformed-path check:

- A leading-dot path (.foo:bar) is a relative import, and import_module
  raises TypeError rather than ImportError for it. Every loader built on this
  function catches ImportError, so the call crashed instead of reporting the
  path. These paths are absolute by construction, so reject the shape.
- ":attr" was already covered by the new condition but had no test, which
  left the empty-module half unpinned. Added one.

Both were mutation-checked: dropping each condition turns exactly its own
test red.

* fix(reflection): reject whitespace and trailing dots in variable paths too

Review follow-up on the shapes that stay inside the ImportError contract but
land in the missing-dependency branch:

- "json.:attr" advertises a distribution named `json.`
- "json :loads" and " json:attr" advertise `uv add json ` / `uv add  json`,
  where the space that IS the typo is invisible in the rendered hint
- "json:attr\n" reaches getattr with a name the caller never wrote

A dotted module path has no trailing dot and neither half carries
whitespace, so these get the same format guidance as the other four shapes.
Whitespace is checked inside both halves rather than at the ends, since
"json :loads" hides the typo in the middle.

Each new condition half is mutation-checked: removing the trailing-dot test
fails only the trailing-dot case, and removing both whitespace tests fails
exactly the two whitespace cases.

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
2026-10-22 08:05:17 +08:00

126 lines
5.8 KiB
Python

from importlib import import_module
MODULE_TO_PACKAGE_HINTS = {
"langchain_google_genai": "langchain-google-genai",
"langchain_anthropic": "langchain-anthropic",
"langchain_openai": "langchain-openai",
"langchain_deepseek": "langchain-deepseek",
}
def _build_missing_dependency_hint(module_path: str, err: ImportError) -> str:
"""Build an actionable hint when module import fails."""
module_root = module_path.split(".", 1)[0]
missing_module = getattr(err, "name", None) or module_root
# Prefer provider package hints for known integrations, even when the import
# error is triggered by a transitive dependency (e.g. `google`).
package_name = MODULE_TO_PACKAGE_HINTS.get(module_root)
if package_name is None:
package_name = MODULE_TO_PACKAGE_HINTS.get(missing_module, missing_module.replace("_", "-"))
return f"Missing dependency '{missing_module}'. Install it with `uv add {package_name}` (or `pip install {package_name}`), then restart DeerFlow."
def resolve_variable[T](
variable_path: str,
expected_type: type[T] | tuple[type, ...] | None = None,
) -> T:
"""Resolve a variable from a path.
Args:
variable_path: The path to the variable (e.g. "parent_package_name.sub_package_name.module_name:variable_name").
expected_type: Optional type or tuple of types to validate the resolved variable against.
If provided, uses isinstance() to check if the variable is an instance of the expected type(s).
Returns:
The resolved variable.
Raises:
ImportError: If the module path is invalid or the attribute doesn't exist.
ValueError: If the resolved variable doesn't pass the validation checks.
"""
try:
module_path, variable_name = variable_path.rsplit(":", 1)
except ValueError as err:
raise ImportError(f"{variable_path} doesn't look like a variable path. Example: parent_package_name.sub_package_name.module_name:variable_name") from err
# A path carries exactly one colon. Without this check the split above
# misfiles the two realistic typos: a trailing colon yields an empty
# attribute name ("Module json does not define a attribute/class"), and an
# extra colon leaves it inside the module part, where the failed import is
# reported as a missing dependency with an unusable `uv add json:loads`
# hint. Both are path mistakes, so they get the path-format message.
#
# A leading dot is the third shape: it is a relative import, and
# import_module raises TypeError ("the 'package' argument is required")
# rather than ImportError, so callers that catch ImportError -- every
# provider/extension/middleware loader -- would see an unhandled crash.
# These paths are absolute by construction, so reject it here too.
#
# Stray whitespace and a trailing dot are the fourth: they stay inside the
# ImportError contract but land in the missing-dependency branch, which
# then advertises a distribution named `json.` or `json ` -- text that
# cannot work, and where the space is invisible in the rendered hint. A
# dotted module path has neither, so the same format guidance applies.
# Whitespace is checked inside both halves (not just at the ends), because
# `"json :loads"` and `"json:attr\n"` each hide the typo on one side.
if (
not module_path
or not variable_name
or ":" in module_path
or module_path.startswith(".")
or module_path.endswith(".")
or any(character.isspace() for character in module_path)
or any(character.isspace() for character in variable_name)
):
raise ImportError(f"{variable_path} doesn't look like a variable path. Example: parent_package_name.sub_package_name.module_name:variable_name")
try:
module = import_module(module_path)
except ImportError as err:
module_root = module_path.split(".", 1)[0]
err_name = getattr(err, "name", None)
if isinstance(err, ModuleNotFoundError) or err_name == module_root:
hint = _build_missing_dependency_hint(module_path, err)
raise ImportError(f"Could not import module {module_path}. {hint}") from err
# Preserve the original ImportError message for non-missing-module failures.
raise ImportError(f"Error importing module {module_path}: {err}") from err
try:
variable = getattr(module, variable_name)
except AttributeError as err:
raise ImportError(f"Module {module_path} does not define a {variable_name} attribute/class") from err
# Type validation
if expected_type is not None:
if not isinstance(variable, expected_type):
type_name = expected_type.__name__ if isinstance(expected_type, type) else " or ".join(t.__name__ for t in expected_type)
raise ValueError(f"{variable_path} is not an instance of {type_name}, got {type(variable).__name__}")
return variable
def resolve_class[T](class_path: str, base_class: type[T] | None = None) -> type[T]:
"""Resolve a class from a module path and class name.
Args:
class_path: The path to the class (e.g. "langchain_openai:ChatOpenAI").
base_class: The base class to check if the resolved class is a subclass of.
Returns:
The resolved class.
Raises:
ImportError: If the module path is invalid or the attribute doesn't exist.
ValueError: If the resolved object is not a class or not a subclass of base_class.
"""
model_class = resolve_variable(class_path, expected_type=type)
if not isinstance(model_class, type):
raise ValueError(f"{class_path} is not a valid class")
if base_class is not None and not issubclass(model_class, base_class):
raise ValueError(f"{class_path} is not a subclass of {base_class.__name__}")
return model_class