Skip to content

feat(handlers): expose sync_catalogs through sales-catalog-driven specialism#790

Draft
bokelley wants to merge 2 commits into
mainfrom
claude/issue-786-sync-catalogs-specialism-advertisement
Draft

feat(handlers): expose sync_catalogs through sales-catalog-driven specialism#790
bokelley wants to merge 2 commits into
mainfrom
claude/issue-786-sync-catalogs-specialism-advertisement

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

@bokelley bokelley commented May 22, 2026

Summary

Closes #786

Re-cut from current main (resolves conflict from the original PR #790 which was rebased from an older base).

  • Wire sync_catalogs into SPECIALISM_TO_ADVERTISED_TOOLS so sales-catalog-driven platforms expose the tool via tools/list
  • Add PlatformHandler.sync_catalogs dispatcher — full SyncCatalogsRequest passed through (no arg_projector) so adopters can inspect req.delete_missing, req.dry_run, req.validation_mode, and req.catalog_ids
  • Add sync_catalogs to SalesPlatform Protocol body under the sales-catalog-driven-required section
  • Add _require_platform_method("sync_catalogs") guard → UNSUPPORTED_FEATURE (not INTERNAL_ERROR) when a non-catalog-driven platform receives the call
  • dispatch.py's REQUIRED_METHODS_PER_SPECIALISM["sales-catalog-driven"] already included sync_catalogs on main — the boot-fail gate was pre-existing

Note: sync_catalogs is on the unified SalesPlatform Protocol body (not a separate mixin), which means @runtime_checkable isinstance(x, SalesPlatform) checks require it on all sales-* adopters. validate_platform enforces it only for sales-catalog-driven. This is a deliberate tradeoff over a SalesCatalogDrivenPlatform mixin — the existing pattern for sales tools is unified (SalesPlatform), and a mixin would be the only exception.

Test plan

  • test_validate_platform_raises_on_catalog_driven_without_sync_catalogs — boot-fail when sales-catalog-driven is claimed without sync_catalogs
  • test_catalog_driven_advertises_sync_catalogstools/list includes sync_catalogs for sales-catalog-driven
  • test_non_catalog_sales_does_not_advertise_sync_catalogstools/list excludes sync_catalogs for sales-non-guaranteed
  • test_sync_catalogs_shim_passes_full_request_to_platform — full request object threaded through (no projection)
  • test_sync_catalogs_discovery_mode_passes_none_catalogsreq.catalogs=None threads through intact
  • test_sync_catalogs_list_return_projected_to_envelope — ergonomic list return projected to {catalogs: [...]}
  • test_sync_catalogs_unsupported_when_platform_lacks_methodUNSUPPORTED_FEATURE on non-catalog-driven platform
  • test_project_sync_catalogs_wraps_pydantic_row_list
  • test_project_sync_catalogs_wraps_plain_dict_row_list
  • test_project_sync_catalogs_passthrough_envelope_dict
  • test_project_sync_catalogs_passthrough_non_list
  • test_advertised_tools_covers_every_specialism_wire_tool — universe includes sync_catalogs
  • test_sales_platform_protocol_still_runtime_checkable@runtime_checkable check updated

4325 passed, 18 skipped, 1 xfailed. Ruff and mypy clean.

What tested

  • pytest tests/ -x -q — 4325 passed
  • mypy src/adcp/decisioning/handler.py src/adcp/decisioning/specialisms/sales.py — no issues
  • ruff check src/adcp/decisioning/handler.py src/adcp/decisioning/specialisms/sales.py — all checks passed

Pre-PR review

  • code-reviewer: approved — no blockers. Flagged sync_catalogs in _OPTIONAL_PLATFORM_METHODS as semantically misleading (a required method shouldn't be in the optional set); fixed in commit 2934122 by removing it. Nit: _project_sync_catalogs asymmetry with _project_sync_audiences arg_projector documented in shim docstring. @runtime_checkable structural consequence noted in PR body above.
  • dx-expert: (review in progress at PR creation time — no blockers surfaced from code-reviewer pass)

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 790
    fix → push.
  • Or re-trigger: comment /triage execute on the source
    issue.

See adcp#3121
for context.

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

(ergonomic form) or a fully-shaped
:class:`~adcp.types.SyncCatalogsSuccessResponse`.
"""
...
@bokelley
Copy link
Copy Markdown
Contributor Author

Triage status: useful feature candidate, but this PR is conflicting with main and has maintainer edits disabled, so I cannot update it in place. Also note the source issue #786 was closed earlier from the wrong signals premise and later re-triaged as a real catalog-driven specialism gap. Recommended path: author rebases, or we re-cut from current main if we want to proceed.

@bokelley bokelley force-pushed the claude/issue-786-sync-catalogs-specialism-advertisement branch from 6098ae2 to 4324ef9 Compare May 23, 2026 21:29
@bokelley
Copy link
Copy Markdown
Contributor Author

Rebased onto current main (was behind by ~20 commits, including the 6.0.0-beta.1 release line).

One conflict resolved: handler.py had a positional conflict between main's new _SPONSORED_INTELLIGENCE_ADVERTISED_TOOLS block and this PR's _CATALOG_ADVERTISED_TOOLS block — both are kept; neither was semantically overlapping.

Post-rebase verification:

  • pytest tests/test_decisioning_advertised_per_specialism.py tests/test_decisioning_dispatch.py tests/test_decisioning_handler_shims.py tests/test_decisioning_specialisms.py -x -q201 passed
  • ruff check → clean
  • mypy → clean

Branch is now at 4324ef9 on top of 65f84d2 (main HEAD). Ready for human review.


Triaged by Claude Code. Session: https://claude.ai/code/session_01WfCNbekqBBmXgoGtVokFyT


Generated by Claude Code

…cialism

Wires the sync_catalogs tool (available in types since 5.6.0) into the
DecisioningPlatform advertisement and dispatch path for sales-catalog-driven
platforms. Previously the tool was unreachable: SPECIALISM_TO_ADVERTISED_TOOLS
had no mapping for it and PlatformHandler had no shim.

Closes #786

https://claude.ai/code/session_01RHG8jmrXLEyLjwaLTZHSi5
@bokelley bokelley force-pushed the claude/issue-786-sync-catalogs-specialism-advertisement branch from 4324ef9 to 20258e3 Compare May 23, 2026 21:38
sync_catalogs is required for sales-catalog-driven (enforced at boot by
validate_platform) rather than optional. Listing it in _OPTIONAL_PLATFORM_METHODS
was semantically misleading and could confuse a future reviewer or pass
adding it to a future validation gate that suppresses the boot-fail.

The _require_platform_method("sync_catalogs") guard in the shim correctly
surfaces UNSUPPORTED_FEATURE for non-catalog-driven callers via hasattr —
it does not depend on _OPTIONAL_PLATFORM_METHODS.

https://claude.ai/code/session_01RHG8jmrXLEyLjwaLTZHSi5
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.

Expose sync_catalogs through decisioning platform specialism advertisement

2 participants