Conversation
Python was merged in #200 without docs. This adds Python to all documentation surfaces: - python/README.md: full SDK docs (installation, auth, OAuth, services, pagination, errors, retry, hooks, webhooks, async, downloads) - README.md: Python in languages table, feature matrix, quick start, documentation links - AGENTS.md: status table, architecture tables, generation pipeline, release procedure, completeness bar, sync checklist - CONTRIBUTING.md: prerequisites, getting started, code style, generator commands - SECURITY.md: concurrency safety, PKCE example, header redaction example
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a9659c65de
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
4 issues found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="AGENTS.md">
<violation number="1" location="AGENTS.md:235">
P3: `make release` does not verify 10 version constants; it currently checks 7. Update this line to avoid mismatching the actual release guard behavior.</violation>
</file>
<file name="python/README.md">
<violation number="1" location="python/README.md:376">
P2: This line describes `config.max_retries` as max attempts, but the implementation treats it as max retries (initial request + retries).</violation>
<violation number="2" location="python/README.md:454">
P2: The webhook example uses dotted event patterns that don’t match this SDK’s underscore-style event kinds, so the sample handlers won’t be invoked.</violation>
</file>
<file name="SECURITY.md">
<violation number="1" location="SECURITY.md:93">
P3: Adding Python to the PKCE helper list makes the subsequent “state parameters are 22 characters (16 random bytes)” claim inaccurate for Python (generate_state uses 32 random bytes / 43 chars). Update the security properties note (or Python’s implementation) so the documented state length matches Python.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Pull request overview
Adds first-class documentation for the Python SDK and updates repo-wide docs to reflect Python as a supported implementation alongside the existing language SDKs.
Changes:
- Adds a comprehensive
python/README.mdcovering installation, auth (static + OAuth/PKCE), pagination, errors, retry behavior, hooks, webhooks, async, downloads, and dev workflows. - Updates root documentation (
README.md,CONTRIBUTING.md,AGENTS.md,SECURITY.md) to include Python in language lists, architecture/generation guidance, and security invariants/examples. - Aligns repo commands and contribution guidance with the Python toolchain (uv, ruff, mypy, pytest, drift checks).
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| python/README.md | New end-user SDK documentation for Python, including examples and operational behavior. |
| README.md | Adds Python to the repo intro, language list, feature matrix, quick start, and doc links. |
| SECURITY.md | Extends security invariants/examples to cover Python (concurrency, PKCE, header redaction). |
| CONTRIBUTING.md | Updates contributor setup and style guidance to include Python (uv, ruff, py-generate). |
| AGENTS.md | Updates architecture/generation/release guidance to include the Python SDK. |
Comments suppressed due to low confidence (1)
README.md:152
- The “All SDKs provide” feature list still claims “ETag caching” is built-in, but the feature matrix marks Python as ✗ for ETag caching. Adjust the wording so it doesn’t state this as universally available (or call out the Python exception explicitly) to avoid contradicting the matrix.
All SDKs provide:
- **Full API coverage** - 35+ services covering projects, todos, messages, schedules, campfires, card tables, and more
- **OAuth 2.0 authentication** - Token refresh, PKCE support (Go, TypeScript, Ruby, Kotlin, Python), and static token options
- **Automatic retry** - Exponential backoff with jitter, respects `Retry-After` headers
- **Pagination** - Link header–based pagination support (high-level handling may vary by SDK; see language docs)
- **ETag caching** - Built-in HTTP caching for efficient API usage
- **Structured errors** - Typed errors with helpful hints and CLI-friendly exit codes
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Webhook patterns: dotted → underscore style (todo_*, message_created) - verify_signature: check return value, positional arg order - Middleware: next_fn() takes no args, not next_fn(event) - Error codes: show runtime string values (auth_required, api_error, etc.) - Retry: fix backoff formula (2^(attempt-1)), clarify max_retries vs attempts, document idempotent mutation retry path - Downloads: describe two-hop auth model accurately - Service methods: narrow keyword-only claim to service methods - CONTRIBUTING: cd back to root before make targets - AGENTS: drop hardcoded version-constant counts from release guards - SECURITY: note Python state param is 43 chars (32 bytes), mark redact_headers as internal helper - Badge: already fixed (test.yml)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 353e249db8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- python/README.md dev section: cd back to repo root after uv sync - python/README.md: 40+ → 40 generated services (exact count) - python/README.md: ApiError covers any unhandled status, not just 5xx - README.md: qualify ETag caching bullet with SDK list (Python lacks it)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Positional params (access_token, client_id, client_secret) now come before keyword-only params (refresh_token, expires_at, on_refresh), matching the actual constructor signature.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
"Maximum retry attempts" was ambiguous — could read as total attempts. Now says "Maximum retries (up to N+1 total attempts)".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
python/README.md— Full SDK docs: installation, auth (static + OAuth), OAuth 2.0 flow with PKCE, 41-service table grouped by category, pagination (ListResult), error hierarchy with runtime error codes, retry behavior (including idempotent mutation retries and 401 refresh path), observability hooks (subclass pattern), webhooks (receiver, signature verification, middleware, dedup), async support, downloads, development commandsREADME.md— Python added to intro, languages table, feature matrix (ETag ✗), quick start, OAuth PKCE mention, documentation linksAGENTS.md— Python in status/architecture tables, generation pipeline, invariants, sync checklist, completeness bar (Python tests required), release procedure counts (6→7 workflows, 8→10 version files)CONTRIBUTING.md— Prerequisites (Python 3.11+, uv), getting started, code style (ruff), generator command, updated SDK countsSECURITY.md— Concurrency safety (threading.Lock), PKCE code example, header redaction example, updated implementation countFollows up on #200 which merged the Python SDK without documentation.
Test plan
python/README.mdverified against actual source signatures (25 examples checked — imports, class/function names, constructor/method arg names and types all match).github/workflows/test.yml(not a nonexistenttest-python.yml)verify.pysignaturesauth_required,api_error, etc.)