Skip to content

feat(decisioning): surface advertise_all=False on create_adcp_server_from_platform#528

Closed
bokelley wants to merge 1 commit intomainfrom
claude/issue-519-advertise-all
Closed

feat(decisioning): surface advertise_all=False on create_adcp_server_from_platform#528
bokelley wants to merge 1 commit intomainfrom
claude/issue-519-advertise-all

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

@bokelley bokelley commented May 4, 2026

Closes #519

Summary

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. serve() already defaulted advertise_all=False and passed it through get_tools_for_handler → advertised_tools_for_instance() to filter tools/list, but that filter was invisible to anyone inspecting handler.advertised_tools directly.

This PR adds advertise_all: bool = False to create_adcp_server_from_platform() and, when False, overrides handler.advertised_tools at the instance level with set(handler.advertised_tools_for_instance()). The ClassVar (and the _HANDLER_TOOLS registry populated at class-definition time) are unaffected. serve() now forwards advertise_all into create_adcp_server_from_platform() for consistency.

Before:

handler, _, _ = create_adcp_server_from_platform(platform=MySalesPlatform())
len(handler.advertised_tools)  # ~42 (full union)

After:

handler, _, _ = create_adcp_server_from_platform(platform=MySalesPlatform())
len(handler.advertised_tools)  # 9 (sales specialism set only)

What was tested

  • pytest tests/test_decisioning_serve.py — 34/34 pass (3 new tests covering default filter, advertise_all=True escape hatch, and empty-specialism fallback)
  • mypy src/adcp/decisioning/serve.py — clean
  • ruff check src/adcp/decisioning/serve.py — clean
  • pytest 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 pass

Pre-PR review

  • code-reviewer: approved — no blockers; noted that get_tools_for_handler reads _HANDLER_TOOLS[cls.__name__] (ClassVar, class-definition time) and calls advertised_tools_for_instance() independently, so the instance attribute shadow and the MCP tools/list filter are separate paths.
  • dx-expert: approved — advertise_all=False is the right default and name (consistent with serve()); noted behavior change for existing callers who may have relied on the full ~40-tool ClassVar (changelog note warranted).

Nits (not fixed — addressed here):

  • Test imports _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.

Triage-managed PR. This bot does not currently iterate on
review comments or PR conversation threads (only on the source
issue). To unblock:

  • Push fixup commits directly: gh pr checkout <num>
    fix → push.
  • Or re-trigger: comment /triage execute on the source
    issue.

See adcp#3121
for context.

Session: https://claude.ai/code/session_01VYgrLhaR7HWfBf3VxbMn5K


Generated by Claude Code

…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
@bokelley
Copy link
Copy Markdown
Contributor Author

bokelley commented May 4, 2026

Superseded by #529 (merged) which already adds advertise_all + handler.get_advertised_tools().

@bokelley bokelley closed this May 4, 2026
@bokelley
Copy link
Copy Markdown
Contributor Author

bokelley commented May 4, 2026

Acknowledged — closing the loop here. Since #529 merged with the same advertise_all surface and handler.get_advertised_tools(), this PR is correctly superseded. No further action needed on this branch.


Generated by Claude Code


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

decisioning: surface advertise_all=False on create_adcp_server_from_platform

2 participants