mirror of
https://github.com/tiennm99/serena.git
synced 2026-10-11 03:13:51 +00:00
Introduce optional facades; make facade availability derived
Facade availability: * facades can be optional (`Facade.from_api(..., is_optional=True)`), mirroring optional tools * `Facade.is_enabled()` is derived: a facade is available iff it has at least one enabled method * `ApiScope.is_facade_enabled` is thereby obsolete and removed Opt-in rule (uniform for optional facades, excluded facades and optional methods): * methods of a facade which is not included and optional methods require explicit inclusion * all other methods are enabled unless explicitly excluded Application: * the `ext` facade is optional; the query-projects mode includes it (`included_apis: [ext]`)
This commit is contained in:
1 parent
680a7e8fc1
commit
4b54fa737f
7 files changed
+59
-27
No files matched your search
@@ -83,7 +83,10 @@ expression is the result. No `return` (a top-level `return` yields a SyntaxError
|
||||
(a capability constraint, e.g. clients that handle the REPL badly), with the user's preference choosing among them.
|
||||
- `included_apis`/`excluded_apis` (references `facade` or `facade.method`) in global config, context, modes,
|
||||
project config; applied in that order via `ApiScope` (exclusions first, then inclusions; later definitions win).
|
||||
Optional methods and all methods of an excluded facade must be included explicitly.
|
||||
- Opt-in rule: methods of a facade that is not included (excluded, or optional without explicit inclusion) and
|
||||
optional methods are enabled only if included explicitly; all other methods are enabled unless excluded.
|
||||
Facades can be optional (`Facade.from_api(..., is_optional=True)`, e.g. `ext`, mirroring optional tools);
|
||||
`Facade.is_enabled()` is derived: a facade is available iff it has at least one enabled method.
|
||||
- The REPL is rebuilt whenever the active tools are updated (mode switch, project activation).
|
||||
|
||||
## Availability policy
|
||||
|
||||
+1
-1
@@ -1344,7 +1344,7 @@ class SerenaAgent:
|
||||
Facade.from_api(EditApi(self), api_scope),
|
||||
Facade.from_api(MemoryApi(self), api_scope),
|
||||
Facade.from_api(ShellApi(self), api_scope),
|
||||
Facade.from_api(ExternalProjectsApi(self), api_scope),
|
||||
Facade.from_api(ExternalProjectsApi(self), api_scope, is_optional=True),
|
||||
]
|
||||
if self._language_backend.is_lsp():
|
||||
facades.append(Facade.from_api(LspApi(self), api_scope))
|
||||
|
||||
+28
-14
@@ -478,17 +478,23 @@ class ApiScope:
|
||||
"""
|
||||
|
||||
def __init__(self) -> None:
|
||||
self.is_included = True
|
||||
self._is_included: bool | None = None
|
||||
self.method_inclusions: set[str] = set()
|
||||
self.method_exclusions: set[str] = set()
|
||||
|
||||
def exclude_facade(self) -> None:
|
||||
self.is_included = False
|
||||
self._is_included = False
|
||||
self.method_inclusions = set()
|
||||
self.method_exclusions = set()
|
||||
|
||||
def include_facade(self) -> None:
|
||||
self.is_included = True
|
||||
self._is_included = True
|
||||
|
||||
def is_facade_included(self, is_facade_optional: bool) -> bool:
|
||||
if is_facade_optional:
|
||||
return self._is_included is True
|
||||
else:
|
||||
return self._is_included is not False
|
||||
|
||||
def exclude_method(self, method_name: str) -> None:
|
||||
self.method_inclusions.discard(method_name)
|
||||
@@ -537,23 +543,24 @@ class ApiScope:
|
||||
"""
|
||||
self._editing_excluded = True
|
||||
|
||||
def is_facade_enabled(self, facade_name: str) -> bool:
|
||||
facade_scope = self._get_facade_scope(facade_name)
|
||||
return facade_scope.is_included or len(facade_scope.method_inclusions) > 0
|
||||
|
||||
def is_method_enabled(self, facade_name: str, method_info: FacadeMethodInfo) -> bool:
|
||||
def is_method_enabled(self, facade_name: str, method_info: FacadeMethodInfo, is_facade_optional: bool) -> bool:
|
||||
"""
|
||||
:param facade_name: the name of the facade
|
||||
:param method_info: the method's metadata
|
||||
:return: whether the method is enabled: optional methods (and all methods of an excluded facade) must be
|
||||
explicitly included, other methods are enabled unless explicitly excluded; if editing is excluded,
|
||||
editing methods are always disabled
|
||||
:param is_facade_optional: whether the facade is optional (disabled by default and must be enabled explicitly)
|
||||
:return: whether the method is enabled: optional methods (and all methods of a facade which is not included,
|
||||
i.e. an excluded facade or an optional facade that was not explicitly included) must be explicitly
|
||||
included, other methods are enabled unless explicitly excluded; if editing is excluded, editing
|
||||
methods are always disabled
|
||||
"""
|
||||
if self._editing_excluded and method_info.can_edit:
|
||||
return False
|
||||
facade_scope = self._get_facade_scope(facade_name)
|
||||
if method_info.optional or not facade_scope.is_included:
|
||||
# A method that would be disabled because the facade it is part of is not included
|
||||
# or the method itself is optional must be explicitly included in order to be enabled.
|
||||
if not facade_scope.is_facade_included(is_facade_optional) or method_info.optional:
|
||||
return method_info.name in facade_scope.method_inclusions
|
||||
# A method that is not optional and whose facade is included is enabled unless it is explicitly excluded.
|
||||
else:
|
||||
return method_info.name not in facade_scope.method_exclusions
|
||||
|
||||
@@ -585,12 +592,13 @@ class Facade:
|
||||
self._methods[method.name] = method
|
||||
|
||||
@staticmethod
|
||||
def from_api(api: FacadeApi, api_scope: ApiScope) -> "Facade":
|
||||
def from_api(api: FacadeApi, api_scope: ApiScope, *, is_optional: bool = False) -> "Facade":
|
||||
"""
|
||||
Creates a facade wrapping the given implementation.
|
||||
|
||||
:param api: the implementation; each of its methods decorated with `facade_method` becomes a facade method
|
||||
:param api_scope: API scope definition determining which methods are enabled
|
||||
:param is_optional: whether the facade is optional (disabled by default and must be enabled explicitly)
|
||||
:return: the facade
|
||||
"""
|
||||
facade = Facade(api.get_name_(), api.get_description_(), api.get_referenced_types_())
|
||||
@@ -598,11 +606,17 @@ class Facade:
|
||||
method_info = get_facade_method_info(member)
|
||||
if method_info is None:
|
||||
continue
|
||||
is_enabled = api_scope.is_method_enabled(facade.name, method_info)
|
||||
is_enabled = api_scope.is_method_enabled(facade.name, method_info, is_facade_optional=is_optional)
|
||||
facade._add_method(FacadeMethod(facade, member, method_info, enabled=is_enabled))
|
||||
facade._discover_referenced_types()
|
||||
return facade
|
||||
|
||||
def is_enabled(self) -> bool:
|
||||
"""
|
||||
:return: whether the facade is enabled
|
||||
"""
|
||||
return len(self.get_enabled_methods()) > 0
|
||||
|
||||
def _discover_referenced_types(self) -> None:
|
||||
"""
|
||||
Adds referenced types for all classes reachable (transitively) through the annotations of the facade's methods
|
||||
|
||||
+5
-11
@@ -41,14 +41,9 @@ class FacadeAvailabilityInfo:
|
||||
def __init__(self):
|
||||
self.facades: list[FacadeAvailabilityInfo.FacadeInfo] = []
|
||||
|
||||
def add_facade(self, facade: Facade, is_enabled: bool):
|
||||
def is_method_enabled(m: FacadeMethod) -> bool:
|
||||
return is_enabled and m.enabled
|
||||
|
||||
methods_info = [
|
||||
FacadeAvailabilityInfo.MethodInfo(name=method.name, is_enabled=is_method_enabled(method)) for method in facade.get_methods()
|
||||
]
|
||||
self.facades.append(FacadeAvailabilityInfo.FacadeInfo(name=facade.name, is_enabled=is_enabled, methods=methods_info))
|
||||
def add_facade(self, facade: Facade):
|
||||
methods_info = [FacadeAvailabilityInfo.MethodInfo(name=method.name, is_enabled=method.enabled) for method in facade.get_methods()]
|
||||
self.facades.append(FacadeAvailabilityInfo.FacadeInfo(name=facade.name, is_enabled=facade.is_enabled(), methods=methods_info))
|
||||
|
||||
|
||||
class SerenaReplEntrypoint:
|
||||
@@ -68,9 +63,8 @@ class SerenaReplEntrypoint:
|
||||
self._facade_availability_info = FacadeAvailabilityInfo()
|
||||
registered_facade_names = []
|
||||
for facade in facades:
|
||||
is_facade_enabled = api_scope.is_facade_enabled(facade.name)
|
||||
self._facade_availability_info.add_facade(facade, is_facade_enabled)
|
||||
if is_facade_enabled:
|
||||
self._facade_availability_info.add_facade(facade)
|
||||
if facade.is_enabled():
|
||||
if facade.name in self._facades:
|
||||
raise ValueError(f"Duplicate facade name: {facade.name}")
|
||||
self._facades[facade.name] = facade
|
||||
|
||||
@@ -6,3 +6,5 @@ excluded_tools: []
|
||||
included_optional_tools:
|
||||
- list_queryable_projects
|
||||
- query_project
|
||||
included_apis:
|
||||
- ext
|
||||
@@ -86,6 +86,9 @@ def test_external_project_context_in_repl(
|
||||
server, port = project_server
|
||||
monkeypatch.setattr(ProjectServer, "PORT", port) # let the REPL's external project context use the test server
|
||||
|
||||
# enable the optional "ext" facade
|
||||
serena_config.included_apis = ["ext"]
|
||||
|
||||
# the querying agent has another project active and queries the python test project
|
||||
agent = SerenaAgent(project="test_repo_typescript", serena_config=serena_config)
|
||||
agent.execute_task(lambda: None)
|
||||
|
||||
@@ -217,6 +217,22 @@ class TestFacade:
|
||||
scope.process(ApiInclusionDefinition(**kwargs))
|
||||
return scope
|
||||
|
||||
def test_optional_facade_is_opt_in(self) -> None:
|
||||
# an optional facade is disabled unless it is included explicitly
|
||||
facade = Facade.from_api(self.DummyApi(MagicMock()), ApiScope(), is_optional=True)
|
||||
assert not facade.is_enabled()
|
||||
assert facade.enabled_method_names == []
|
||||
|
||||
# including the facade enables its non-optional methods
|
||||
facade = Facade.from_api(self.DummyApi(MagicMock()), self._scope(included_apis=["dummy"]), is_optional=True)
|
||||
assert facade.is_enabled()
|
||||
assert set(facade.enabled_method_names) == {"add", "secret", "rarely"}
|
||||
|
||||
# including a single method enables the facade with just that method
|
||||
facade = Facade.from_api(self.DummyApi(MagicMock()), self._scope(included_apis=["dummy.add"]), is_optional=True)
|
||||
assert facade.is_enabled()
|
||||
assert facade.enabled_method_names == ["add"]
|
||||
|
||||
def test_api_scope_facade_exclusion_and_method_inclusion(self) -> None:
|
||||
# excluding the facade disables everything
|
||||
facade = Facade.from_api(self.DummyApi(MagicMock()), self._scope(excluded_apis=["dummy"]))
|
||||
|
||||
Reference in new issue
Block a user