Skip to content

CLAUDE.md

Latest commit

 

History

History
333 lines (263 loc) · 13.7 KB

CLAUDE.md

File metadata and controls

333 lines (263 loc) · 13.7 KB
Raw

Python SDK Development Learnings

Type Safety & Code Generation

Auto-generate from specs when possible

  • Download schemas from canonical source (e.g., adcontextprotocol.org/schemas)
  • Generate Pydantic models automatically - keeps types in sync with spec
  • Validate generated code in CI (syntax check + import test)

Codegen variant numbering is unstable datamodel-code-generator numbers anonymous variant classes (e.g. Assets162, Idempotency3, Type11) by traversal order. Adding or reordering even one sibling schema shifts the entire numbering window — a clean re-run can produce a 600+ file diff with zero semantic change. Treat any git diff on generated_poc/ that consists of only ClassNameN → ClassNameM renames as churn, not a schema delta, and discard it. aliases.py re-exports these numbered classes under semantic names; accepting the renumber breaks the alias layer for nothing.

Import Architecture for Generated Types The type system has a strict layering to prevent brittleness:

generated_poc/*.py (internal, auto-generated from schemas)
    ↓
_generated.py (internal consolidation)
    ↓
aliases.py + capabilities.py + _ergonomic.py + _forward_compat.py
    ↓
_eager.py (binds the full public surface; runs import-time patching)
    ↓
__init__.py (thin lazy facade; user-facing exports via PEP 562 __getattr__)

adcp.types/__init__.py is a lazy facade (PEP 562): import adcp.types is cheap, and the generated Pydantic graph is built on first access to a type symbol, by importing _eager.py (the former eager __init__ body). The runtime __getattr__/__dir__ live under if not TYPE_CHECKING: so type checkers see the surface only via the explicit TYPE_CHECKING re-export block — a typo'd import is flagged, not silently typed as object.

Only these modules may import from generated_poc/ or _generated.py (enforced by tests/test_import_layering.py):

  • _generated.py: Consolidates exports from generated_poc/ into a flat namespace
  • _eager.py: Eager realization of the public surface — binds every exported name and runs the import-time patchers (_ergonomic, _forward_compat)
  • aliases.py: Creates semantic aliases for numbered discriminated union types
  • capabilities.py: Re-exports get_adcp_capabilities_response sub-models with disambiguated names
  • _ergonomic.py: Applies BeforeValidator coercion for type ergonomics
  • _forward_compat.py: Patches Format.assets / RepeatableAssetGroup.assets with open union types at import time
  • legacy.py: Explicit facade for generated named-format wire models during the v7 migration
  • canonical_creative.py: Canonical-first boundary models that replace generated creative lifecycle surfaces
  • __init__.py: Public API surface (thin lazy facade)

All other source code should import from adcp.types (the public API).

Type Checking Best Practices

  • Use TYPE_CHECKING for optional dependencies to avoid runtime import errors
  • Use cast() for JSON deserialization to satisfy mypy's no-any-return checks
  • Add specific type: ignore comments (e.g., # type: ignore[no-any-return]) rather than blanket ignores
  • Test type checking in CI across multiple Python versions (3.10+)

ctx_metadata: write-only credentials prohibited

RequestContext.metadata is for non-secret request hints, not credentials. The standard auth context factory adds framework fields and adopter-supplied principal metadata. Buyer wire context is echoed separately; the framework does not project it into ToolContext.metadata. An adopter's custom context factory or response code can still expose secrets if it copies credentials into metadata or response context.

The dispatcher performs best-effort key screening at _build_request_context as defense in depth against adopter mistakes. Passing this screen does not establish that metadata is safe to expose or that it contains no credentials. If you see a ValueError like ctx_metadata may not contain credential-shaped keys, migrate the value to AuthInfo.credential or a typed credential class.

Wrong — credential stored in metadata; this key is rejected at dispatch:

ctx = RequestContext(metadata={"upstream.api_token": secret})  # Rejected at dispatch

Right — credential stored in the typed AuthInfo.credential field:

auth = AuthInfo(
    kind="api_key",
    key_id="kid_1",
    principal="agent.example.com",
    credential=ApiKeyCredential(kind="api_key", key_id="kid_1"),
)
ctx = RequestContext(auth_info=auth, metadata={"correlation_id": "req_xyz"})

The credential-shaped key suffix list is in adcp.decisioning.dispatch._CREDENTIAL_SHAPED_KEY_SUFFIXES and matches case-insensitively through nested dictionaries and lists: credential, credentials, token, secret, api_key, apikey, password, bearer. Keys that don't match (correlation_id, feature_flag.beta_pricing, trace_id, tokenizer) pass through. This finite suffix list misses other credential names, including plural or embedded forms such as api_tokens and access_token_value, and names such as private_key and authorization. Other containers, including tuple values, are not traversed. An unrecognized key or container is not permission to store a secret in metadata.

For credentials the framework propagates to upstream calls (governance agents, signal providers, audience activations), use the typed credential classes from adcp.decisioning: ApiKeyCredential, OAuthCredential, HttpSigCredential. The framework dispatch threads these explicitly without going through the context-echo path.

Testing Strategy

Mock at the Right Level

  • For HTTP clients: Mock _get_client() method, not the httpx class directly
  • For async operations: Use AsyncMock for async functions, MagicMock for sync methods
  • Remember: httpx's response.json() is SYNCHRONOUS, not async

Test API Changes Properly

  • When API changes from kwargs to typed objects, update tests to match
  • Remove tests for non-existent methods rather than keep failing tests
  • Test the API as it exists, not as we wish it existed

CI/CD & Release Automation

GitHub Actions Secrets

  • Secret names matter! Check actual secret name in repository settings
  • Guarded PyPI publishing uses environment-bound Trusted Publishing and attestations, not a static API token
  • Legacy PyPI/App credentials are retired only through the separately authorized historical-run audit in docs/releasing.md
  • Test locally with python -m build before relying on CI

Release Please Workflow

  • A push to main opens or updates the normal Release Please PR using the GitHub App token.
  • An input-free manual run on main bootstraps the workflow after it is re-enabled.
  • Merge the reviewed release PR after CI; Release Please creates its tag and GitHub release.
  • release-publish.yml waits for exact-commit CI, builds and checks distributions, then awaits release-publish environment approval before PyPI Trusted Publishing.
  • Follow docs/releasing.md for setup, approval, and verification.

Entry Points for CLI Tools

[project.scripts]
toolname = "package.__main__:main"

This enables uvx toolname and pip install toolname to work correctly.

Python-Specific Patterns

Optional Dependencies with TYPE_CHECKING

from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from optional_lib import SomeType

try:
    from optional_lib import SomeType as _SomeType
    AVAILABLE = True
except ImportError:
    AVAILABLE = False

Atomic File Operations For config files with sensitive data:

temp_file = CONFIG_FILE.with_suffix(".tmp")
with open(temp_file, "w") as f:
    json.dump(config, f, indent=2)
temp_file.replace(CONFIG_FILE)  # Atomic rename

Connection Pooling

# Reuse HTTP client across requests
self._client: httpx.AsyncClient | None = None

async def _get_client(self) -> httpx.AsyncClient:
    if self._client is None:
        limits = httpx.Limits(
            max_keepalive_connections=10,
            max_connections=20,
        )
        self._client = httpx.AsyncClient(limits=limits)
    return self._client

Common Pitfalls to Avoid

String Escaping in Code Generation Always escape in this order:

  1. Backslashes first: \\ → \\\\
  2. Then quotes: " → \"
  3. Then control chars (newlines, tabs)

Wrong order creates invalid escape sequences!

Python Version Requirements

  • Union syntax str | None requires Python 3.10+
  • Always include from __future__ import annotations at top of files
  • Use target-version = "py310" in ruff/black config
  • Test in CI across all supported Python versions

Test Fixtures vs. Mocks

  • Don't over-mock - it hides serialization bugs
  • Test actual API calls when possible
  • Use real Pydantic validation in tests
  • Mock external services, not internal logic

Pre-Commit Checks

Run these checks locally before every commit:

make lint
make typecheck-all
make test

All must pass. make ci-local runs the core local gate: lint, all type-check contracts, tests, and generated-code validation. Specialized CI jobs such as storyboard runners, Postgres conformance, and conventional-commit validation still run separately in GitHub Actions. CI runs the core matrix across Python 3.10–3.13; locally running on your current version catches most issues.

Parallel Agent Isolation (git worktrees)

When multiple agents work in the same checkout simultaneously, they clobber each other's branches — there is no error, the work is silently lost. Use git worktree to give each agent an isolated checkout.

Note: Conductor worktrees handle this automatically via .conductor.json (runs setup_conductor_env.py + pre-commit install on create). Use the manual steps below only for raw git worktree outside of Conductor. See CONDUCTOR.md for Conductor-specific setup and troubleshooting.

Create a worktree:

git worktree add /tmp/claude-issue-<N>-<slug> -b claude/issue-<N>-<slug> main

Setup checklist (run inside the new worktree):

cd /tmp/claude-issue-<N>-<slug>
cp "$(git rev-parse --git-common-dir)/../.env" .env   # .env is not inherited
make bootstrap                     # requires uv; installs deps and hooks here

Teardown (after branch is merged):

git worktree remove /tmp/claude-issue-<N>-<slug>
# or: git worktree prune   # removes all stale worktrees at once

Branch naming: always follow claude/issue-<N>-<short-slug> — branch-protection rules enforce this pattern and PRs from non-conforming names may be rejected.

Parallel Agent Coordination

When spawning parallel sub-agents, each agent must receive an explicit write-scope declaration in its prompt. Agents do not detect or refuse out-of-scope writes at runtime; this contract is the only enforcement mechanism.

Prompt template for a parallel sub-agent:

Task: <what this agent should do>

Read scope (consult freely):
- <file or glob pattern>
- <file or glob pattern>

Write scope (the ONLY files this agent may create or modify — exact paths, no globs):
- <exact file path>
- <exact file path>

Do not edit files outside your write scope even if you believe the change
would be an improvement. If you discover during execution that you need to write
a file outside your scope, stop and record it in your reply instead.

Pre-spawn checklist:

  1. List every file any agent in the group is expected to write. If an agent may discover additional files during execution, note that in your scope planning — do not silently expand the write scope at runtime.
  2. Partition that set so each file appears in exactly one agent's write scope. For files both agents need to write, assign one owner; have the other emit the required change as a note in its reply.
  3. Pass each agent its partition explicitly (see template above).
  4. After all agents complete, check for collisions: git log --name-only --oneline -<N> (N = number of agent commits), then look for the same file appearing in more than one entry.

Issue Triage Bot

A Claude Code routine ("issue triage", wired in .github/workflows/claude-issue-triage.yml) fires automatically on every new issue and on every non-/triage comment. It reads the issue body, decides whether to clarify, defer, or open a draft PR, and posts back via comment. PRs it opens land on claude/issue-<N>-<slug> branches, are labeled claude-triaged, and the issue carries claude-triaging while the routine is actively running.

Coordination rules:

  • Do not open a new PR for an unlabeled issue without checking the triage state first. If the issue carries claude-triaging the bot is actively working — your work will collide. If it carries claude-triaged the bot has already produced (or deferred to) a PR; find that PR before starting fresh work.

  • Apply no-triage at issue creation when you (human or designated agent) plan to do the work. The label short-circuits the workflow if: gate so the routine never fires. Labels added after the fact do not retroactively cancel an in-flight run.

  • Manual /triage overrides the no-triage label. The slash-command-dispatch path is gated only by member-association — use it when you explicitly want the bot to engage on a no-triage issue.

  • Stale claude-triaged draft PRs lose parallel races. When a human-authored PR for the same issue lands on main first, the bot's draft becomes superseded. Prefer closing-as-superseded over rebasing — always check recent main commits before investing time in a rebase of a triage-managed draft.

Additional Important Reminders

NEVER:

  • Assume a "typo" without checking the actual secret name in GitHub settings

ALWAYS:

  • Verify secret names match repository settings before "fixing" them