feat(decisioning): surface advertise_all=False on create_adcp_server_from_platform#528
Closed
feat(decisioning): surface advertise_all=False on create_adcp_server_from_platform#528
Conversation
…from_platform Adopters using the standalone build-the-handler path saw ~40+ entries in handler.advertised_tools after create_adcp_server_from_platform() because the ClassVar holds the full union across all PlatformHandler shims. This adds advertise_all=False (matching serve()'s default) and filters the instance-level advertised_tools to the platform's claimed specialisms, consistent with the existing advertised_tools_for_instance() hook already used by get_tools_for_handler. Existing serve() callers get the same filtering applied to handler.advertised_tools for consistency. https://claude.ai/code/session_01VYgrLhaR7HWfBf3VxbMn5K
Contributor
Author
|
Superseded by #529 (merged) which already adds advertise_all + handler.get_advertised_tools(). |
Contributor
Author
|
Acknowledged — closing the loop here. Since #529 merged with the same Generated by Claude Code Generated by Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #519
Summary
Adopters using the standalone "build the handler" path saw ~40+ entries in
handler.advertised_toolsaftercreate_adcp_server_from_platform()because theClassVarholds the full union across allPlatformHandlershims.serve()already defaultedadvertise_all=Falseand passed it throughget_tools_for_handler → advertised_tools_for_instance()to filtertools/list, but that filter was invisible to anyone inspectinghandler.advertised_toolsdirectly.This PR adds
advertise_all: bool = Falsetocreate_adcp_server_from_platform()and, whenFalse, overrideshandler.advertised_toolsat the instance level withset(handler.advertised_tools_for_instance()). The ClassVar (and the_HANDLER_TOOLSregistry populated at class-definition time) are unaffected.serve()now forwardsadvertise_allintocreate_adcp_server_from_platform()for consistency.Before:
After:
What was tested
pytest tests/test_decisioning_serve.py— 34/34 pass (3 new tests covering default filter,advertise_all=Trueescape hatch, and empty-specialism fallback)mypy src/adcp/decisioning/serve.py— cleanruff check src/adcp/decisioning/serve.py— cleanpytest tests/test_decisioning_advertised_per_specialism.py tests/test_decisioning_handler.py tests/test_advertised_tools_gate.py tests/test_register_handler_tools.py— 50/50 passPre-PR review
get_tools_for_handlerreads_HANDLER_TOOLS[cls.__name__](ClassVar, class-definition time) and callsadvertised_tools_for_instance()independently, so the instance attribute shadow and the MCP tools/list filter are separate paths.advertise_all=Falseis the right default and name (consistent withserve()); noted behavior change for existing callers who may have relied on the full ~40-tool ClassVar (changelog note warranted).Nits (not fixed — addressed here):
_SALES_ADVERTISED_TOOLS(a private symbol). Acceptable: the test is specifically verifying the specialism-to-tool mapping; using the private frozenset is the most precise assertion available.# type: ignore[misc]on the ClassVar instance write: mypy classifies this as[misc], not[assignment];[misc]is the narrowest suppressor available for this specific diagnostic.Session: https://claude.ai/code/session_01VYgrLhaR7HWfBf3VxbMn5K
Generated by Claude Code