From d24ee08f9b41d54d6d77812caf50a004b576d11e Mon Sep 17 00:00:00 2001 From: CleverThis Date: Tue, 14 Apr 2026 18:01:31 +0000 Subject: [PATCH 1/3] fix(retry): add missing RetryPolicyConfig fields max_retries, backoff_factor, max_backoff --- features/steps/acms_context_analysis_steps.py | 3 + .../domain/models/core/retry_policy.py | 65 +++++++++++-------- 2 files changed, 41 insertions(+), 27 deletions(-) create mode 100644 features/steps/acms_context_analysis_steps.py diff --git a/features/steps/acms_context_analysis_steps.py b/features/steps/acms_context_analysis_steps.py new file mode 100644 index 000000000..7b8300cc0 --- /dev/null +++ b/features/steps/acms_context_analysis_steps.py @@ -0,0 +1,3 @@ +"""Step definitions for ACMS context analysis (stub).""" + +from behave import given, then, when diff --git a/src/cleveragents/domain/models/core/retry_policy.py b/src/cleveragents/domain/models/core/retry_policy.py index a39c01335..79b56baa4 100644 --- a/src/cleveragents/domain/models/core/retry_policy.py +++ b/src/cleveragents/domain/models/core/retry_policy.py @@ -97,31 +97,38 @@ class RetryPolicyConfig(BaseModel): override exists. Fields: - max_attempts: Maximum number of attempts (including the initial call). - base_delay: Initial delay in seconds before the first retry. - max_delay: Upper bound on delay between retries in seconds. + max_retries: Maximum number of retry attempts (spec-required name). + retry_delay_seconds: Initial delay in seconds before the first retry. + backoff_multiplier: Multiplicative factor applied between retry delays. + max_backoff: Upper bound on backoff delay in seconds (spec-required name). jitter: Whether to add random jitter to delays to avoid thundering herd. backoff_strategy: The strategy used to compute delay between retries. retry_on_idempotent_only: When True, retries are skipped for non-idempotent ops. """ - max_attempts: int = Field( + max_retries: int = Field( default=3, ge=1, le=100, - description="Maximum number of attempts including the initial call.", + description="Maximum number of retry attempts.", ) - base_delay: float = Field( + retry_delay_seconds: float = Field( default=1.0, ge=0.0, le=300.0, description="Initial delay in seconds before the first retry.", ) - max_delay: float = Field( + backoff_multiplier: float = Field( + default=2.0, + ge=1.0, + le=10.0, + description="Multiplicative factor applied between retry delays for exponential backoff.", + ) + max_backoff: float = Field( default=60.0, ge=0.0, le=3600.0, - description="Upper bound on delay between retries in seconds.", + description="Upper bound on backoff delay in seconds.", ) jitter: bool = Field( default=True, @@ -148,17 +155,17 @@ class RetryPolicyConfig(BaseModel): ) @model_validator(mode="after") - def _check_max_delay_ge_base_delay(self) -> RetryPolicyConfig: - """Ensure max_delay is not less than base_delay. + def _check_max_backoff_ge_retry_delay(self) -> RetryPolicyConfig: + """Ensure max_backoff is not less than retry_delay_seconds. Uses a model validator so the constraint fires on ANY field - assignment (including ``base_delay``), not just when ``max_delay`` + assignment (including ``retry_delay_seconds``), not just when ``max_backoff`` is set. """ - if self.max_delay < self.base_delay: + if self.max_backoff < self.retry_delay_seconds: msg = ( - f"max_delay ({self.max_delay}) must be >= " - f"base_delay ({self.base_delay})" + f"max_backoff ({self.max_backoff}) must be >= " + f"retry_delay_seconds ({self.retry_delay_seconds})" ) raise ValueError(msg) return self @@ -300,36 +307,40 @@ class ServiceRetryPolicy(BaseModel): # ------------------------------------------------------------------- DEFAULT_NETWORK_RETRY = RetryPolicyConfig( - max_attempts=5, - base_delay=1.0, - max_delay=30.0, + max_retries=5, + retry_delay_seconds=1.0, + backoff_multiplier=2.0, + max_backoff=30.0, jitter=True, backoff_strategy=RetryStrategy.EXPONENTIAL, retry_on_idempotent_only=True, ) DEFAULT_PROVIDER_RETRY = RetryPolicyConfig( - max_attempts=3, - base_delay=1.0, - max_delay=60.0, + max_retries=3, + retry_delay_seconds=1.0, + backoff_multiplier=2.0, + max_backoff=60.0, jitter=True, backoff_strategy=RetryStrategy.JITTER, retry_on_idempotent_only=True, ) DEFAULT_DATABASE_RETRY = RetryPolicyConfig( - max_attempts=3, - base_delay=0.5, - max_delay=5.0, + max_retries=3, + retry_delay_seconds=0.5, + backoff_multiplier=2.0, + max_backoff=5.0, jitter=False, backoff_strategy=RetryStrategy.FIXED, retry_on_idempotent_only=True, ) DEFAULT_FILE_RETRY = RetryPolicyConfig( - max_attempts=3, - base_delay=0.1, - max_delay=1.0, + max_retries=3, + retry_delay_seconds=0.1, + backoff_multiplier=2.0, + max_backoff=1.0, jitter=True, backoff_strategy=RetryStrategy.EXPONENTIAL, retry_on_idempotent_only=True, @@ -450,7 +461,7 @@ class ServiceRetryPolicyRegistry: registry = ServiceRetryPolicyRegistry() policy = registry.get("plan_service") # Apply override from config - registry.apply_overrides({"plan_service": {"retry": {"max_attempts": 5}}}) + registry.apply_overrides({"plan_service": {"retry": {"max_retries": 5}}}) """ def __init__(self) -> None: -- 2.52.0 From c3e510b961cd57400f32d90a08826c52e40111f1 Mon Sep 17 00:00:00 2001 From: controller-ci-rerun Date: Thu, 18 Jun 2026 11:00:04 -0400 Subject: [PATCH 2/3] chore: re-trigger CI [controller] -- 2.52.0 From 72765d90a05689d967f31b1edd7748a97a9a19ca Mon Sep 17 00:00:00 2001 From: CleverThis Date: Fri, 19 Jun 2026 01:10:35 -0400 Subject: [PATCH 3/3] fix(retry): rename RetryPolicyConfig fields to match spec Rename RetryPolicyConfig fields to their spec-required names (per issue #9396 acceptance criteria): max_attempts -> max_retries base_delay -> retry_delay_seconds backoff_multiplier -> backoff_factor max_delay -> max_backoff Updated all references in: - application/services/service_retry_wiring.py (Settings field map, wait-strategy builder, sync execute(), async_execute(), wrap_service_method()) - BDD step definitions for retry_policy_model, service_retry_wiring, service_retry_settings, and their coverage/async variants - robot/retry_policy_wiring.robot integration scripts Fixed lint errors flagged on this PR: - F401 unused imports in features/steps/acms_context_analysis_steps.py (stub file reduced to just the docstring) - E501 overlong line in retry_policy.py:125 (split description tuple) Added CHANGELOG.md entry under [Unreleased] per CONTRIBUTING.md. ISSUES CLOSED: #9396 --- CHANGELOG.md | 3 +- features/steps/acms_context_analysis_steps.py | 2 - .../retry_policy_model_coverage_steps.py | 6 +-- features/steps/retry_policy_model_steps.py | 48 ++++++++++--------- .../steps/service_retry_settings_steps.py | 4 +- .../steps/service_retry_wiring_async_steps.py | 2 +- .../service_retry_wiring_coverage_steps.py | 30 +++++++----- features/steps/service_retry_wiring_steps.py | 4 +- robot/retry_policy_wiring.robot | 12 ++--- .../services/service_retry_wiring.py | 20 ++++---- .../domain/models/core/retry_policy.py | 17 ++++--- 11 files changed, 80 insertions(+), 68 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ac7fc1a9..7a7db26a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ Changed `wf10_batch.robot` to be less likely to create files, and ## [Unreleased] - **feat(cli): add `agents actor context show` command** (#6369 / PR #6622): Adds `agents actor context show ` to display a named actor context's summary, messages, metadata, state, and global context. Supports `--format` (rich/json/yaml/plain/table/color) and `--context-dir` options. Returns exit code 1 for non-existent contexts. Hoists `_default_context_base()` into the show module and re-imports it from `actor_context.py` to remove the duplicated base-path resolution. Adds `command=` kwargs to existing `_render_output` calls in `actor_context.py` so JSON/YAML envelopes report the originating subcommand. +- **fix(retry): rename RetryPolicyConfig fields to match spec** (#9396 / PR #9452): Renamed `RetryPolicyConfig` fields to their spec-required names: `max_attempts` → `max_retries`, `base_delay` → `retry_delay_seconds`, `backoff_multiplier` → `backoff_factor`, `max_delay` → `max_backoff`. Updated all references in `service_retry_wiring.py` (field map, wait-strategy builder, `execute`, `async_execute`, `wrap_service_method`). Fixed lint errors (F401 unused imports in `acms_context_analysis_steps.py`, E501 overlong line in `retry_policy.py`). Updated all BDD step definitions and Robot Framework integration scripts to use the renamed fields. - **docs(spec): fix checkpoint config key path and trigger name defaults** (#5009 / PR #5163): Corrects the Configuration Reference table entry `sandbox.checkpoint.auto-create-on` → `core.checkpoints.auto-create-on`, matching the implementation in `config_service.py`. Aligns the default trigger-name values (`before_tool_execute`, `after_tool_execute`) with the implementation in `tool/runner.py`, resolving spec–implementation discrepancies identified in issue #5009. - **docs: module guides for Sandbox & Checkpoint, Correction Attempts, and Invariant Reconciliation** (#4848): Added three comprehensive module guides covering purpose, core classes, lifecycle diagrams, exception hierarchies, CLI usage, and ADR links for `SandboxManager`, `CorrectionAttemptManager`, and `InvariantReconciliationActor`. Includes security callouts for `NoSandbox` bypass (permanent writes, no rollback), `guidance` prompt-injection risk, `archived_artifacts_path` provenance, and `non_overridable` global invariant access control. - **feat(context): PriorityContextStrategy** (#9997 / PR #10772): Implements a priority-based context strategy that ranks context fragments by configurable priority scores — default role-based rules (system > tool > user > assistant), exponential recency decay (7-day half-life), and explicit priority tag boost. Supports custom scoring function injection and custom PriorityRule list injection. Registered in the ACMS pipeline under key `priority_context`. `PriorityRule` uses Pydantic `BaseModel` for architecture conformance. Includes 18 BDD scenarios covering all acceptance criteria. @@ -1411,4 +1412,4 @@ iteration` and data corruption under concurrent plan execution. All public - **TUI -- Permission Question Widget**: A new inline `PermissionQuestionWidget` renders permission requests directly in the conversation stream for single-key operations. Users can allow/reject with single-key shortcuts (`a`/`A`/`r`/`R`), - navigate with arrow keys, confirm with `Enter`, or press `v` to open the full \ No newline at end of file + navigate with arrow keys, confirm with `Enter`, or press `v` to open the full diff --git a/features/steps/acms_context_analysis_steps.py b/features/steps/acms_context_analysis_steps.py index 7b8300cc0..63783c9e9 100644 --- a/features/steps/acms_context_analysis_steps.py +++ b/features/steps/acms_context_analysis_steps.py @@ -1,3 +1 @@ """Step definitions for ACMS context analysis (stub).""" - -from behave import given, then, when diff --git a/features/steps/retry_policy_model_coverage_steps.py b/features/steps/retry_policy_model_coverage_steps.py index 597449c13..5b5650678 100644 --- a/features/steps/retry_policy_model_coverage_steps.py +++ b/features/steps/retry_policy_model_coverage_steps.py @@ -246,7 +246,7 @@ def step_verify_invalid_merged_skipped(context): def step_verify_original_max_attempts(context): """Verify the policy was not corrupted by the invalid override.""" policy = context.registry.get("plan_service") - assert policy.retry.max_attempts == context.original_policy.retry.max_attempts + assert policy.retry.max_retries == context.original_policy.retry.max_retries # --------------------------------------------------------------------------- @@ -380,7 +380,7 @@ def step_register_custom_policy(context, svc, attempts): policy = ServiceRetryPolicy( service_name=svc, retry_category=RetryCategory.NETWORK, - retry=RetryPolicyConfig(max_attempts=attempts), + retry=RetryPolicyConfig(max_retries=attempts), description=f"Custom policy for {svc}", ) context.registry.register(policy) @@ -391,7 +391,7 @@ def step_verify_registered_policy(context, svc, attempts): """Verify the registered policy can be retrieved with correct values.""" policy = context.registry.get(svc) assert policy.service_name == svc - assert policy.retry.max_attempts == attempts + assert policy.retry.max_retries == attempts # --------------------------------------------------------------------------- diff --git a/features/steps/retry_policy_model_steps.py b/features/steps/retry_policy_model_steps.py index 7c804640b..b1c762911 100644 --- a/features/steps/retry_policy_model_steps.py +++ b/features/steps/retry_policy_model_steps.py @@ -43,17 +43,17 @@ def step_create_default_retry_policy(context: Any) -> None: @then("the retry policy should have max_attempts {value:d}") def step_check_max_attempts(context: Any, value: int) -> None: - assert context.retry_policy.max_attempts == value + assert context.retry_policy.max_retries == value @then("the retry policy should have base_delay {value:g}") def step_check_base_delay(context: Any, value: float) -> None: - assert context.retry_policy.base_delay == value + assert context.retry_policy.retry_delay_seconds == value @then("the retry policy should have max_delay {value:g}") def step_check_max_delay(context: Any, value: float) -> None: - assert context.retry_policy.max_delay == value + assert context.retry_policy.max_backoff == value @then("the retry policy should have jitter {value}") @@ -70,7 +70,9 @@ def step_check_backoff_strategy_exp(context: Any) -> None: def step_create_invalid_retry_policy(context: Any, base: float, mx: float) -> None: context.validation_error = None try: - context.retry_policy = RetryPolicyConfig(base_delay=base, max_delay=mx) + context.retry_policy = RetryPolicyConfig( + retry_delay_seconds=base, max_backoff=mx + ) except ValidationError as exc: context.validation_error = exc @@ -173,7 +175,7 @@ def step_check_desc_contains(context: Any, text: str) -> None: @when("I apply overrides setting plan_service max_attempts to {value:d}") def step_apply_overrides(context: Any, value: int) -> None: context.registry.apply_overrides( - {"plan_service": {"retry": {"max_attempts": value}}} + {"plan_service": {"retry": {"max_retries": value}}} ) @@ -181,7 +183,7 @@ def step_apply_overrides(context: Any, value: int) -> None: def step_check_plan_service_max(context: Any, value: int) -> None: registry = getattr(context, "registry", None) or context.wiring.registry policy = registry.get("plan_service") - assert policy.retry.max_attempts == value + assert policy.retry.max_retries == value # --------------------------------------------------------------------------- @@ -264,7 +266,7 @@ def step_check_all_policies_dict(context: Any) -> None: def step_create_invalid_max_attempts(context: Any, value: int) -> None: context.neg_validation_error = None try: - RetryPolicyConfig(max_attempts=value) + RetryPolicyConfig(max_retries=value) except ValidationError as exc: context.neg_validation_error = exc @@ -319,7 +321,7 @@ def step_check_neg_policy_validation(context: Any) -> None: @when('I apply overrides with service_name mutation for "{name}"') def step_apply_overrides_with_name_mutation(context: Any, name: str) -> None: context.registry.apply_overrides( - {name: {"service_name": "hijacked_name", "retry": {"max_attempts": 4}}} + {name: {"service_name": "hijacked_name", "retry": {"max_retries": 4}}} ) @@ -332,15 +334,15 @@ def step_check_no_name_mutation(context: Any, name: str) -> None: @when('I apply overrides with invalid data for "{name}"') def step_apply_invalid_overrides(context: Any, name: str) -> None: original = context.registry.get(name) - context.original_max_attempts = original.retry.max_attempts - # max_attempts=0 violates ge=1 constraint -> ValidationError - context.registry.apply_overrides({name: {"retry": {"max_attempts": 0}}}) + context.original_max_attempts = original.retry.max_retries + # max_retries=0 violates ge=1 constraint -> ValidationError + context.registry.apply_overrides({name: {"retry": {"max_retries": 0}}}) @then("the plan_service policy should retain its original max_attempts") def step_check_original_retained(context: Any) -> None: policy = context.registry.get("plan_service") - assert policy.retry.max_attempts == context.original_max_attempts + assert policy.retry.max_retries == context.original_max_attempts # --------------------------------------------------------------------------- @@ -376,7 +378,7 @@ def step_apply_non_dict_overrides(context: Any, name: str) -> None: @then("the plan_service policy should be unchanged after non-dict override") def step_check_policy_unchanged_after_non_dict(context: Any) -> None: policy = context.registry.get("plan_service") - assert policy.retry.max_attempts == context.original_policy.retry.max_attempts + assert policy.retry.max_retries == context.original_policy.retry.max_retries # --------------------------------------------------------------------------- @@ -394,15 +396,15 @@ def step_mutate_policy(context: Any) -> None: policy = context.fresh_registry.get("plan_service") context.original_session_max = context.fresh_registry.get( "session_service" - ).retry.max_attempts + ).retry.max_retries # Mutate plan_service's retry config - policy.retry.max_attempts = 99 + policy.retry.max_retries = 99 @then("other service policies should not be affected by the mutation") def step_check_no_cross_mutation(context: Any) -> None: session_policy = context.fresh_registry.get("session_service") - assert session_policy.retry.max_attempts == context.original_session_max + assert session_policy.retry.max_retries == context.original_session_max # --------------------------------------------------------------------------- @@ -420,17 +422,17 @@ def step_get_two_unknown_policies(context: Any) -> None: def step_mutate_first_unknown(context: Any) -> None: from cleveragents.domain.models.core.retry_policy import DEFAULT_DATABASE_RETRY - context.default_max_before = DEFAULT_DATABASE_RETRY.max_attempts - context.unknown_b_max_before = context.unknown_b.retry.max_attempts + context.default_max_before = DEFAULT_DATABASE_RETRY.max_retries + context.unknown_b_max_before = context.unknown_b.retry.max_retries # Mutate only the first unknown service's retry config - context.unknown_a.retry.max_attempts = 77 + context.unknown_a.retry.max_retries = 77 @then("the second unknown service retry config should be unaffected") def step_check_second_unknown_unaffected(context: Any) -> None: - assert context.unknown_b.retry.max_attempts == context.unknown_b_max_before, ( + assert context.unknown_b.retry.max_retries == context.unknown_b_max_before, ( f"Second unknown service was corrupted: " - f"{context.unknown_b.retry.max_attempts} != {context.unknown_b_max_before}" + f"{context.unknown_b.retry.max_retries} != {context.unknown_b_max_before}" ) @@ -438,7 +440,7 @@ def step_check_second_unknown_unaffected(context: Any) -> None: def step_check_default_database_retry_unaffected(context: Any) -> None: from cleveragents.domain.models.core.retry_policy import DEFAULT_DATABASE_RETRY - assert DEFAULT_DATABASE_RETRY.max_attempts == context.default_max_before, ( + assert DEFAULT_DATABASE_RETRY.max_retries == context.default_max_before, ( f"DEFAULT_DATABASE_RETRY was corrupted: " - f"{DEFAULT_DATABASE_RETRY.max_attempts} != {context.default_max_before}" + f"{DEFAULT_DATABASE_RETRY.max_retries} != {context.default_max_before}" ) diff --git a/features/steps/service_retry_settings_steps.py b/features/steps/service_retry_settings_steps.py index 3a3a41843..b3a4fcd35 100644 --- a/features/steps/service_retry_settings_steps.py +++ b/features/steps/service_retry_settings_steps.py @@ -130,7 +130,7 @@ def step_create_wiring_base_delay(context: Any) -> None: @then("the plan_service policy should have base_delay {value:g}") def step_check_base_delay_override(context: Any, value: float) -> None: policy = context.wiring.get_policy("plan_service") - assert policy.retry.base_delay == value + assert policy.retry.retry_delay_seconds == value @given("I have Settings with non-default circuit_breaker_recovery_timeout {value:g}") @@ -228,7 +228,7 @@ def step_create_wiring_max_delay(context: Any) -> None: @then("the plan_service policy should have max_delay {value:g}") def step_check_max_delay_override(context: Any, value: float) -> None: policy = context.wiring.get_policy("plan_service") - assert policy.retry.max_delay == value + assert policy.retry.max_backoff == value @given("I have Settings with non-default retry_jitter {value}") diff --git a/features/steps/service_retry_wiring_async_steps.py b/features/steps/service_retry_wiring_async_steps.py index a9c18603f..fcbef11b0 100644 --- a/features/steps/service_retry_wiring_async_steps.py +++ b/features/steps/service_retry_wiring_async_steps.py @@ -210,7 +210,7 @@ def step_check_async_exhaustion_error(context: Any) -> None: @then("the async always-failing function should have been called max_attempts times") def step_check_async_always_fail_count(context: Any) -> None: policy = context.wiring.get_policy("plan_service") - assert context.async_call_count == policy.retry.max_attempts + assert context.async_call_count == policy.retry.max_retries @then("the captured log should contain a retry warning message") diff --git a/features/steps/service_retry_wiring_coverage_steps.py b/features/steps/service_retry_wiring_coverage_steps.py index 0831e438e..a596ca71d 100644 --- a/features/steps/service_retry_wiring_coverage_steps.py +++ b/features/steps/service_retry_wiring_coverage_steps.py @@ -86,7 +86,7 @@ def step_settings_non_default_max_attempts(context, n): @given('settings with retry_service_overrides introducing a new service "{svc_name}"') def step_settings_with_new_service_override(context, svc_name): - overrides = {svc_name: {"retry": {"base_delay": 0.5, "max_delay": 5.0}}} + overrides = {svc_name: {"retry": {"retry_delay_seconds": 0.5, "max_backoff": 5.0}}} context.custom_settings = _make_settings_via_env( retry_max_attempts=context.custom_max_attempts, retry_service_overrides=json.dumps(overrides), @@ -101,8 +101,8 @@ def step_create_wiring_from_settings(context): @then('the policy for "{svc_name}" should have max_attempts of {n:d}') def step_verify_policy_max_attempts(context, svc_name, n): policy = context.wiring.get_policy(svc_name) - assert policy.retry.max_attempts == n, ( - f"Expected max_attempts={n}, got {policy.retry.max_attempts}" + assert policy.retry.max_retries == n, ( + f"Expected max_retries={n}, got {policy.retry.max_retries}" ) @@ -165,9 +165,9 @@ def step_policy_with_string_backoff(context, strategy): policy = ServiceRetryPolicy( service_name="string_backoff_svc", retry=RetryPolicyConfig( - max_attempts=2, - base_delay=0.1, - max_delay=1.0, + max_retries=2, + retry_delay_seconds=0.1, + max_backoff=1.0, backoff_strategy=RetryStrategy.EXPONENTIAL, ), ) @@ -198,7 +198,11 @@ def step_wiring_with_disabled_cb(context): # Register a service with CB disabled disabled_cb_policy = ServiceRetryPolicy( service_name="no_cb_service", - retry=RetryPolicyConfig(max_attempts=2, base_delay=0.01, max_delay=0.1), + retry=RetryPolicyConfig( + max_retries=2, + retry_delay_seconds=0.01, + max_backoff=0.1, + ), circuit_breaker=CircuitBreakerConfig(enabled=False), ) context.wiring._registry.register(disabled_cb_policy) @@ -231,7 +235,11 @@ def step_concurrent_cb_request(context): # Ensure the service has a policy with CB enabled policy = ServiceRetryPolicy( service_name=svc_name, - retry=RetryPolicyConfig(max_attempts=2, base_delay=0.01, max_delay=0.1), + retry=RetryPolicyConfig( + max_retries=2, + retry_delay_seconds=0.01, + max_backoff=0.1, + ), circuit_breaker=CircuitBreakerConfig(enabled=True), ) context.wiring._registry.register(policy) @@ -438,9 +446,9 @@ def step_wiring_with_string_backoff(context): policy = ServiceRetryPolicy( service_name="str_backoff_wrap_svc", retry=RetryPolicyConfig( - max_attempts=2, - base_delay=0.01, - max_delay=0.1, + max_retries=2, + retry_delay_seconds=0.01, + max_backoff=0.1, backoff_strategy=RetryStrategy.EXPONENTIAL, ), circuit_breaker=CircuitBreakerConfig(enabled=False), diff --git a/features/steps/service_retry_wiring_steps.py b/features/steps/service_retry_wiring_steps.py index bc055561b..5a911b47a 100644 --- a/features/steps/service_retry_wiring_steps.py +++ b/features/steps/service_retry_wiring_steps.py @@ -172,7 +172,7 @@ def step_settings_with_overrides(context: Any, value: int) -> None: import json import os - overrides = json.dumps({"plan_service": {"retry": {"max_attempts": value}}}) + overrides = json.dumps({"plan_service": {"retry": {"max_retries": value}}}) # Settings uses validation_alias so the field must be set via env var os.environ["CLEVERAGENTS_RETRY_SERVICE_OVERRIDES"] = overrides try: @@ -298,7 +298,7 @@ def step_check_exhaustion_error(context: Any) -> None: @then("the always-failing function should have been called max_attempts times") def step_check_always_fail_call_count(context: Any) -> None: policy = context.wiring.get_policy("plan_service") - assert context.always_fail_count == policy.retry.max_attempts + assert context.always_fail_count == policy.retry.max_retries @when("I decorate and call a function via wrap_service_method") diff --git a/robot/retry_policy_wiring.robot b/robot/retry_policy_wiring.robot index 6071be7b7..989a6a4cd 100644 --- a/robot/retry_policy_wiring.robot +++ b/robot/retry_policy_wiring.robot @@ -170,9 +170,9 @@ Get retry_policy_defaults Script ... sys.path.insert(0, '${WORKSPACE_ROOT}/src') ... from cleveragents.domain.models.core.retry_policy import RetryPolicyConfig ... p = RetryPolicyConfig() - ... print(f"max_attempts={p.max_attempts}", flush=True) - ... print(f"base_delay={p.base_delay}", flush=True) - ... print(f"max_delay={p.max_delay}", flush=True) + ... print(f"max_attempts={p.max_retries}", flush=True) + ... print(f"base_delay={p.retry_delay_seconds}", flush=True) + ... print(f"max_delay={p.max_backoff}", flush=True) ... print(f"jitter={p.jitter}", flush=True) ... print(f"backoff_strategy={p.backoff_strategy.value}", flush=True) ... sys.exit(0) @@ -209,9 +209,9 @@ Get registry_override Script ... sys.path.insert(0, '${WORKSPACE_ROOT}/src') ... from cleveragents.domain.models.core.retry_policy import ServiceRetryPolicyRegistry ... reg = ServiceRetryPolicyRegistry() - ... reg.apply_overrides({"plan_service": {"retry": {"max_attempts": 7}}}) + ... reg.apply_overrides({"plan_service": {"retry": {"max_retries": 7}}}) ... p = reg.get("plan_service") - ... print(f"max_attempts={p.retry.max_attempts}", flush=True) + ... print(f"max_attempts={p.retry.max_retries}", flush=True) ... sys.exit(0) RETURN ${script} @@ -304,7 +304,7 @@ Get validation Script ... from pydantic import ValidationError ... from cleveragents.domain.models.core.retry_policy import RetryPolicyConfig ... try: - ... ${SPACE}${SPACE}${SPACE}${SPACE}RetryPolicyConfig(base_delay=10.0, max_delay=5.0) + ... ${SPACE}${SPACE}${SPACE}${SPACE}RetryPolicyConfig(retry_delay_seconds=10.0, max_backoff=5.0) ... ${SPACE}${SPACE}${SPACE}${SPACE}print("No error", flush=True) ... except ValidationError: ... ${SPACE}${SPACE}${SPACE}${SPACE}print("ValidationError raised", flush=True) diff --git a/src/cleveragents/application/services/service_retry_wiring.py b/src/cleveragents/application/services/service_retry_wiring.py index 2c4f068bb..313676f18 100644 --- a/src/cleveragents/application/services/service_retry_wiring.py +++ b/src/cleveragents/application/services/service_retry_wiring.py @@ -82,9 +82,9 @@ MAX_RETRY_TOTAL_TIMEOUT = 300.0 # 5 minutes # S21: Mapping from Settings field names to (policy_key, group) pairs # used by _apply_settings_defaults to replace repetitive if-comparisons. _RETRY_FIELD_MAP: dict[str, tuple[str, str]] = { - "retry_max_attempts": ("max_attempts", "retry"), - "retry_base_delay": ("base_delay", "retry"), - "retry_max_delay": ("max_delay", "retry"), + "retry_max_attempts": ("max_retries", "retry"), + "retry_base_delay": ("retry_delay_seconds", "retry"), + "retry_max_delay": ("max_backoff", "retry"), "retry_jitter": ("jitter", "retry"), "retry_backoff_strategy": ("backoff_strategy", "retry"), "circuit_breaker_failure_threshold": ("failure_threshold", "circuit_breaker"), @@ -263,8 +263,8 @@ class ServiceRetryWiring: ) return _build_wait_strategy( backoff, - policy.retry.base_delay, - policy.retry.max_delay, + policy.retry.retry_delay_seconds, + policy.retry.max_backoff, policy.retry.jitter, ) @@ -416,7 +416,7 @@ class ServiceRetryWiring: try: for attempt_obj in Retrying( stop=( - stop_after_attempt(policy.retry.max_attempts) + stop_after_attempt(policy.retry.max_retries) | stop_after_delay(MAX_RETRY_TOTAL_TIMEOUT) ), wait=wait, @@ -530,7 +530,7 @@ class ServiceRetryWiring: try: async for attempt_obj in AsyncRetrying( stop=( - stop_after_attempt(policy.retry.max_attempts) + stop_after_attempt(policy.retry.max_retries) | stop_after_delay(MAX_RETRY_TOTAL_TIMEOUT) ), wait=wait, @@ -616,9 +616,9 @@ class ServiceRetryWiring: decorator = retry_service_operation( service_name=service_name, operation_name=operation_name, - max_attempts=policy.retry.max_attempts, - base_delay=policy.retry.base_delay, - max_delay=policy.retry.max_delay, + max_attempts=policy.retry.max_retries, + base_delay=policy.retry.retry_delay_seconds, + max_delay=policy.retry.max_backoff, jitter=policy.retry.jitter, backoff_strategy=policy.retry.backoff_strategy.value if isinstance(policy.retry.backoff_strategy, RetryStrategy) diff --git a/src/cleveragents/domain/models/core/retry_policy.py b/src/cleveragents/domain/models/core/retry_policy.py index 79b56baa4..ba18f1eb3 100644 --- a/src/cleveragents/domain/models/core/retry_policy.py +++ b/src/cleveragents/domain/models/core/retry_policy.py @@ -99,7 +99,7 @@ class RetryPolicyConfig(BaseModel): Fields: max_retries: Maximum number of retry attempts (spec-required name). retry_delay_seconds: Initial delay in seconds before the first retry. - backoff_multiplier: Multiplicative factor applied between retry delays. + backoff_factor: Multiplicative factor applied between retry delays. max_backoff: Upper bound on backoff delay in seconds (spec-required name). jitter: Whether to add random jitter to delays to avoid thundering herd. backoff_strategy: The strategy used to compute delay between retries. @@ -118,11 +118,14 @@ class RetryPolicyConfig(BaseModel): le=300.0, description="Initial delay in seconds before the first retry.", ) - backoff_multiplier: float = Field( + backoff_factor: float = Field( default=2.0, ge=1.0, le=10.0, - description="Multiplicative factor applied between retry delays for exponential backoff.", + description=( + "Multiplicative factor applied between retry delays" + " for exponential backoff." + ), ) max_backoff: float = Field( default=60.0, @@ -309,7 +312,7 @@ class ServiceRetryPolicy(BaseModel): DEFAULT_NETWORK_RETRY = RetryPolicyConfig( max_retries=5, retry_delay_seconds=1.0, - backoff_multiplier=2.0, + backoff_factor=2.0, max_backoff=30.0, jitter=True, backoff_strategy=RetryStrategy.EXPONENTIAL, @@ -319,7 +322,7 @@ DEFAULT_NETWORK_RETRY = RetryPolicyConfig( DEFAULT_PROVIDER_RETRY = RetryPolicyConfig( max_retries=3, retry_delay_seconds=1.0, - backoff_multiplier=2.0, + backoff_factor=2.0, max_backoff=60.0, jitter=True, backoff_strategy=RetryStrategy.JITTER, @@ -329,7 +332,7 @@ DEFAULT_PROVIDER_RETRY = RetryPolicyConfig( DEFAULT_DATABASE_RETRY = RetryPolicyConfig( max_retries=3, retry_delay_seconds=0.5, - backoff_multiplier=2.0, + backoff_factor=2.0, max_backoff=5.0, jitter=False, backoff_strategy=RetryStrategy.FIXED, @@ -339,7 +342,7 @@ DEFAULT_DATABASE_RETRY = RetryPolicyConfig( DEFAULT_FILE_RETRY = RetryPolicyConfig( max_retries=3, retry_delay_seconds=0.1, - backoff_multiplier=2.0, + backoff_factor=2.0, max_backoff=1.0, jitter=True, backoff_strategy=RetryStrategy.EXPONENTIAL, -- 2.52.0