mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-10 03:28:53 +00:00
fix: apply canonical-ownership scoping to added public names too
The surface-widening layer compared raw top-level member names, so adding `from typing import Final` to litellm/__init__.py under a chore/refactor title would fail the gate even though the breaking layer explicitly excludes external re-exports. Scoping now follows the alias chain: litellm.Final resolves through litellm.scheduler.Final to typing.Final and is ignored, while litellm.BedrockLLM resolves inside the package and is still caught.
This commit is contained in:
parent
8f36f040bc
commit
f9df01b675
2 changed files with 87 additions and 15 deletions
48
.github/scripts/check_api_breaking_changes.py
vendored
48
.github/scripts/check_api_breaking_changes.py
vendored
|
|
@ -12,9 +12,9 @@ Two layers, both computed with griffe against the PR's base ref:
|
|||
1. Breaking changes (removed objects, changed signatures, changed defaults).
|
||||
Allowed only when the PR declares a breaking change the Conventional
|
||||
Commits way: a `!` in the title type, or a `BREAKING CHANGE:` footer.
|
||||
2. Newly exported top-level names (`litellm.<name>`). Allowed only under a
|
||||
`feat` or `fix` title, so a `chore`/`refactor` PR cannot quietly widen
|
||||
the public surface.
|
||||
2. Newly exported top-level names (`litellm.<name>`) that the package owns.
|
||||
Allowed only under a `feat` or `fix` title, so a `chore`/`refactor` PR
|
||||
cannot quietly widen the public surface.
|
||||
|
||||
Attribute *value* changes are advisory: `Union[A, B] -> A | B` rewrites and
|
||||
f-string conversions trip that check constantly without breaking anyone.
|
||||
|
|
@ -118,17 +118,36 @@ def source_location(obj: Object | Alias) -> tuple[str | None, int | None]:
|
|||
return None, None
|
||||
|
||||
|
||||
def home_path(obj: Object | Alias) -> str:
|
||||
def lookup(roots: tuple[Module, ...], path: str) -> Object | Alias | None:
|
||||
parts: Final = path.split(".")
|
||||
for root in roots:
|
||||
if parts[0] != root.name:
|
||||
continue
|
||||
try:
|
||||
return root.get_member(parts[1:])
|
||||
except Exception:
|
||||
continue
|
||||
return None
|
||||
|
||||
|
||||
def home_path(obj: Object | Alias, roots: tuple[Module, ...] = (), seen: frozenset[str] = frozenset()) -> str | None:
|
||||
try:
|
||||
return obj.canonical_path
|
||||
except Exception:
|
||||
return str(getattr(obj, "target_path", obj.path))
|
||||
target: Final = getattr(obj, "target_path", None)
|
||||
if target is None or target in seen:
|
||||
return None
|
||||
next_hop: Final = lookup(roots, target)
|
||||
if next_hop is None:
|
||||
return target
|
||||
return home_path(next_hop, roots, seen | {target}) or target
|
||||
|
||||
|
||||
def is_in_scope(obj: Object | Alias, package: str) -> bool:
|
||||
def is_in_scope(obj: Object | Alias, package: str, roots: tuple[Module, ...] = ()) -> bool:
|
||||
if obj.path.startswith(OUT_OF_SCOPE_PREFIXES):
|
||||
return False
|
||||
return home_path(obj).startswith(f"{package}.")
|
||||
home: Final = home_path(obj, roots)
|
||||
return home is not None and home.startswith(f"{package}.")
|
||||
|
||||
|
||||
def to_finding(breakage: Breakage, style: ExplanationStyle) -> ApiFinding:
|
||||
|
|
@ -142,8 +161,12 @@ def to_finding(breakage: Breakage, style: ExplanationStyle) -> ApiFinding:
|
|||
)
|
||||
|
||||
|
||||
def top_level_names(module: Module) -> frozenset[str]:
|
||||
return frozenset(name for name in module.members if not name.startswith("_"))
|
||||
def top_level_names(module: Module, package: str, roots: tuple[Module, ...]) -> frozenset[str]:
|
||||
return frozenset(
|
||||
name
|
||||
for name, member in module.members.items()
|
||||
if not name.startswith("_") and is_in_scope(member, package, roots)
|
||||
)
|
||||
|
||||
|
||||
def build_delta(
|
||||
|
|
@ -153,13 +176,16 @@ def build_delta(
|
|||
style: ExplanationStyle,
|
||||
package: str = DEFAULT_PACKAGE,
|
||||
) -> ApiDelta:
|
||||
roots: Final = (new, old)
|
||||
findings: Final = tuple(
|
||||
dict.fromkeys(to_finding(breakage, style) for breakage in breakages if is_in_scope(breakage.obj, package))
|
||||
dict.fromkeys(
|
||||
to_finding(breakage, style) for breakage in breakages if is_in_scope(breakage.obj, package, roots)
|
||||
)
|
||||
)
|
||||
return ApiDelta(
|
||||
blocking=tuple(f for f in findings if f.kind not in ADVISORY_KINDS),
|
||||
advisory=tuple(f for f in findings if f.kind in ADVISORY_KINDS),
|
||||
added_names=tuple(sorted(top_level_names(new) - top_level_names(old))),
|
||||
added_names=tuple(sorted(top_level_names(new, package, roots) - top_level_names(old, package, roots))),
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -168,8 +168,16 @@ class TestScope:
|
|||
obj = FakeObject("litellm.BedrockLLM", canonical=None, target="litellm.llms.bedrock.BedrockLLM")
|
||||
assert gate.is_in_scope(obj, "litellm") is True
|
||||
|
||||
def test_alias_without_target_falls_back_to_its_own_path(self):
|
||||
assert gate.is_in_scope(FakeObject("litellm.Router", canonical=None), "litellm") is True
|
||||
def test_alias_chained_through_an_internal_module_to_stdlib_is_out_of_scope(self):
|
||||
chained = FakeModule("Final", external=("Final",))
|
||||
assert gate.is_in_scope(chained.members["Final"], "litellm", (chained,)) is False
|
||||
|
||||
def test_alias_chained_within_the_package_stays_in_scope(self):
|
||||
owned = FakeModule("completion")
|
||||
assert gate.is_in_scope(owned.members["completion"], "litellm", (owned,)) is True
|
||||
|
||||
def test_unidentifiable_alias_is_treated_as_out_of_scope(self):
|
||||
assert gate.is_in_scope(FakeObject("litellm.Router", canonical=None), "litellm") is False
|
||||
|
||||
|
||||
class FakeKind:
|
||||
|
|
@ -188,8 +196,28 @@ class FakeBreakage:
|
|||
|
||||
|
||||
class FakeModule:
|
||||
def __init__(self, *names: str):
|
||||
self.members = {name: object() for name in names}
|
||||
"""Top-level `litellm`. Names are owned by the package unless `external` says otherwise.
|
||||
|
||||
`external` names mimic the real `litellm.Final`: re-exported through an
|
||||
internal module, so only following the alias chain reveals they are stdlib.
|
||||
"""
|
||||
|
||||
name = "litellm"
|
||||
|
||||
def __init__(self, *names: str, external: tuple[str, ...] = ()):
|
||||
self.members = {
|
||||
name: FakeObject(f"litellm.{name}", canonical=None, target=f"litellm.scheduler.{name}")
|
||||
if name in external
|
||||
else FakeObject(f"litellm.{name}", f"litellm.main.{name}")
|
||||
for name in names
|
||||
}
|
||||
self._hops = {
|
||||
f"scheduler.{name}": FakeObject(f"litellm.scheduler.{name}", canonical=None, target=f"typing.{name}")
|
||||
for name in external
|
||||
}
|
||||
|
||||
def get_member(self, parts):
|
||||
return self._hops[".".join(parts)]
|
||||
|
||||
|
||||
class TestBuildDelta:
|
||||
|
|
@ -237,6 +265,24 @@ class TestBuildDelta:
|
|||
built = gate.build_delta(FakeModule("completion", "old_flag"), FakeModule("completion"), iter([]), style=None)
|
||||
assert built.added_names == ()
|
||||
|
||||
def test_a_newly_imported_stdlib_name_is_not_surface_widening(self):
|
||||
built = gate.build_delta(
|
||||
FakeModule("completion"),
|
||||
FakeModule("completion", "Final", external=("Final",)),
|
||||
iter([]),
|
||||
style=None,
|
||||
)
|
||||
assert built.added_names == ()
|
||||
|
||||
def test_owned_additions_survive_alongside_ignored_stdlib_imports(self):
|
||||
built = gate.build_delta(
|
||||
FakeModule("completion"),
|
||||
FakeModule("completion", "Final", "new_flag", external=("Final",)),
|
||||
iter([]),
|
||||
style=None,
|
||||
)
|
||||
assert built.added_names == ("new_flag",)
|
||||
|
||||
|
||||
class TestRendering:
|
||||
def test_summary_names_the_declaration_escape_hatch(self):
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue