Skip to content

Add comprehensive audit report and improve code quality#7

Merged
amavashev merged 7 commits into
mainfrom
claude/review-python-client-mj54H
Mar 14, 2026
Merged

Add comprehensive audit report and improve code quality#7
amavashev merged 7 commits into
mainfrom
claude/review-python-client-mj54H

Conversation

@amavashev
Copy link
Copy Markdown
Contributor

Summary

This PR adds a comprehensive audit report documenting full protocol conformance with Cycles Protocol v0.1.23, and makes targeted code quality improvements across the codebase.

Key Changes

Documentation & Audit

  • AUDIT.md: Added comprehensive 169-line audit report verifying:
    • All 9 endpoints match spec (paths, HTTP methods, parameters)
    • All 6 request schemas and 10 response schemas match spec JSON keys
    • All 5 enum types and nested object schemas are protocol-conformant
    • Auth headers, idempotency handling, subject validation, and lifecycle orchestration all follow spec normative rules
    • Verdict: Client is fully protocol-conformant with zero open issues

Code Quality & Linting

  • Import organization: Moved Callable from typing to collections.abc (Python 3.10+ best practice) across decorator.py and lifecycle.py
  • Line length compliance: Reformatted long lines to respect 120-character limit in:
    • decorator.py: Function signatures and docstrings
    • lifecycle.py: Function signatures and validation calls
    • client.py: Error message formatting
    • retry.py: Logger calls
    • _validation.py: Error messages
  • Import cleanup: Removed unused imports in:
    • test_client.py: Removed unused json
    • test_lifecycle.py: Reordered mock imports alphabetically
    • test_retry.py: Reordered mock imports alphabetically
    • config.py & context.py: Removed unused field from dataclass imports
    • examples/: Removed trailing blank lines
  • Ruff configuration: Added explicit lint rules (E, F, I, UP) to pyproject.toml

Validation & Testing

  • New validation function: Added validate_extend_by_ms() in _validation.py to enforce spec bounds (1–86400000 ms)
  • Lifecycle integration: Integrated validate_extend_by_ms() into _build_extend_body() in lifecycle.py
  • Test coverage: Added comprehensive test class TestValidateExtendByMs in test_validation.py with 6 test cases

Decorator & Lifecycle Optimization

  • Lifecycle caching: Refactored @cycles decorator to cache lifecycle instances at decoration time (deferred client resolution on first call) rather than recreating on every invocation
    • Extracted _build_default_subject() helper function
    • Sync and async paths now use _cached_sync and _cached_async lists to store singleton lifecycle instances
    • Reduces overhead for repeated decorated function calls
  • Exception handling: Simplified bare except Exception as ex: to except Exception: in CyclesLifecycle.execute() (unused variable)

CI/CD

  • .github/workflows/ci.yml: Added GitHub Actions workflow running:
    • Ruff linting
    • MyPy type checking (strict mode)
    • Pytest test suite
    • On Python 3.10 and 3.12 for all pushes and PRs

Documentation

  • README.md: Added "Development" section documenting setup, linting, type checking, and testing commands

Notable Implementation Details

  • All changes maintain backward compatibility; no public API modifications
  • Lifecycle caching is transparent to users—behavior is identical but more efficient
  • Validation of extend_by_ms now happens at both request-building time (lifecycle) and Pydantic model level (for typed requests)
  • Audit report serves as living documentation of protocol conformance and can be regenerated as spec evolves

https://claude.ai/code/session_01HXikxwPuurjunotYpbmsE7

claude added 7 commits March 14, 2026 15:05
…tion

The spec requires extend_by_ms to be between 1 and 86400000 (1ms to 24h).
Previously _build_extend_body passed ttl_ms as extend_by_ms without validation.
Added validate_extend_by_ms() to _validation.py and call it in _build_extend_body.

https://claude.ai/code/session_01HXikxwPuurjunotYpbmsE7
Added [tool.ruff.lint] select = ["E", "F", "I", "UP"] to pyproject.toml.
Fixed all violations: import sorting, unused imports, line length,
unused variables, and UP035 (import from collections.abc).

https://claude.ai/code/session_01HXikxwPuurjunotYpbmsE7
Previously, every invocation of a @cycles-decorated function created a
new CommitRetryEngine and CyclesLifecycle. Now the lifecycle is lazily
built on first call and cached for subsequent calls, matching the
TypeScript client fix (PR#6/PR#10). Client resolution is deferred to
first call to support set_default_client/set_default_config patterns.

https://claude.ai/code/session_01HXikxwPuurjunotYpbmsE7
The 260+ tests, strict mypy, and ruff lint rules were configured but
never enforced in CI. This workflow runs on push/PR to main with a
matrix across Python 3.10 (minimum) and 3.12 (current).

https://claude.ai/code/session_01HXikxwPuurjunotYpbmsE7
Maps all 9 endpoints, 6 request schemas, 10 response schemas, 5 enum
types, and 8 nested object schemas to their Python implementations.
All categories pass. Follows the format from cycles-spring-boot-starter.

https://claude.ai/code/session_01HXikxwPuurjunotYpbmsE7
Use separate typed cache cells for sync/async paths and inline the
retry engine construction to avoid variable type conflicts.

https://claude.ai/code/session_01HXikxwPuurjunotYpbmsE7
…EADME

__init__.py listed ExtendStatus in __all__ but never imported it from
models, causing ImportError on `from runcycles import ExtendStatus`.
Added Development section to README documenting lint, type check, and
test commands plus CI coverage.

https://claude.ai/code/session_01HXikxwPuurjunotYpbmsE7
@amavashev amavashev merged commit 64bcd51 into main Mar 14, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants