Skip to content

refactor(api): move activities lifecycle-cap fallback to ApiBridgeConfig snapshot#1840

Merged
Aureliolo merged 5 commits into
mainfrom
refactor/issue-1797-activities-lifecycle-bridge
May 9, 2026
Merged

refactor(api): move activities lifecycle-cap fallback to ApiBridgeConfig snapshot#1840
Aureliolo merged 5 commits into
mainfrom
refactor/issue-1797-activities-lifecycle-bridge

Conversation

@Aureliolo
Copy link
Copy Markdown
Owner

Closes #1797.

Summary

  • Removes the duplicated _MAX_LIFECYCLE_EVENTS = 10_000 constant (and its module-level log-once guard) from src/synthorg/api/controllers/activities.py. The activities controller now reads the cap from app_state.api_bridge_config.max_lifecycle_events_per_query (a frozen ApiBridgeConfig snapshot held on AppState), which makes ApiBridgeConfig the single source of truth for the operator-facing default.
  • Adds _apply_api_bridge_config_snapshot to lifecycle_helpers.py. Called from _apply_bridge_config at startup; resolves the full ApiBridgeConfig once via ConfigResolver.get_api_bridge_config() and atomically swaps it onto AppState. On resolver failure the default snapshot is retained and a single api.bridge_config.resolve_failed warning is emitted (centralised replacement for the prior per-request log-once fallback).
  • Adds ApiBridgeSettingsSubscriber (src/synthorg/settings/subscribers/api_bridge_subscriber.py). Watches (api, max_lifecycle_events_per_query); on change it resolves the new value and calls app_state.mutate_api_bridge_config({key: value}). The watched-key set is asserted against ApiBridgeConfig.model_fields at module import so a typo or rename surfaces at boot rather than at the next operator hot-reload. Wired into _build_settings_dispatcher.
  • Adds mutate_api_bridge_config (and a _api_bridge_config_lock threading.Lock) on AppState. Combines a re-validating partial update with the swap inside one critical section, so two concurrent operator edits cannot lose each other's writes. Re-validation goes through ApiBridgeConfig.model_validate(prev | updates) rather than model_copy(update=...) because Pydantic v2's update= skips validators, which would otherwise let an out-of-bounds operator value (e.g. 50 against Field(ge=100, le=1_000_000)) land silently. The matching ValidationError path leaves the prior snapshot in place and propagates to the subscriber's swap-failed log.
  • Drops the matching entry from scripts/no_magic_numbers_baseline.txt; bumps two unrelated baseline line numbers shifted by adjacent comment additions in audit.py / coordination_metrics.py.
  • Documents the new "frozen bridge-config snapshot + hot-swap subscriber" pattern in docs/reference/configuration-precedence.md so subsequent migrations (audit, coordination_metrics, meetings, approvals carry the same anti-pattern) have written guidance.

Test plan

  • uv run python -m pytest tests/ -m unit -n 8 — full unit suite green (27,920 passed, 18 skipped from optional extras).
  • uv run mypy src/ tests/ clean.
  • uv run ruff check src/ tests/ + uv run ruff format --check src/ tests/ clean.
  • uv run python scripts/check_no_magic_numbers.py clean.
  • New tests:
    • tests/unit/api/test_state.py::TestAppStateApiBridgeConfig — default snapshot, swap replaces, swap is idempotent for same instance.
    • tests/unit/api/test_apply_api_bridge_config_snapshot.py — no-resolver no-op, happy-path swap, resolver-failure keeps default + emits one structured warning, MemoryError propagates.
    • tests/unit/settings/test_api_bridge_subscriber.py — protocol conformance, lifecycle-cap change preserves every other field, resolver failure does not swap, MemoryError propagates, out-of-range value triggers Pydantic ValidationError and prior snapshot is retained, unknown namespace / key are no-op.
    • tests/unit/api/controllers/test_activities.py::TestActivityFeedLifecycleCap + TestActivitiesControllerSurface — default cap flows through to lifecycle_events.list_events, mid-test swap takes effect immediately, legacy symbols (_MAX_LIFECYCLE_EVENTS, _resolve_lifecycle_cap, _lifecycle_cap_fallback_logged) are gone.

Review coverage

Pre-PR review pipeline: 21 review agents (docs-consistency, comment-quality-rot, code-reviewer, python-reviewer, pr-test-analyzer, silent-failure-hunter, comment-analyzer, type-design-analyzer, security-reviewer, issue-resolution-verifier, logging-audit, resilience-audit, conventions-enforcer, api-contract-drift, test-quality-reviewer, async-concurrency-reviewer, plus five mini-pass agents from the codebase-audit catalogue).

Findings: 8 valid, 6 false positives. All 8 valid findings addressed in this PR (CancelledError guard on the new helper for parity with peer _resolve_* helpers; out-of-range Pydantic validation test; pattern documented; startup-time watched-key guard; concurrent-writer threading.Lock + dedicated mutate_api_bridge_config; richer changed-fields swap log; explicit @pytest.mark.unit; cross-reference in audit/coordination_metrics comments).

False positives are catalogued in _audit/pre-pr-review/triage.md; the dominant class was agents flagging except A, B: as a "Python 2 syntax error" — CLAUDE.md mandates this PEP 758 form.

Issue-resolution-verifier: all 5 acceptance criteria RESOLVED at confidence 100.

Polish pass (code-simplifier) ran after triage; only stylistic exception-syntax tweaks applied (parenthesised except (A, B): in newly-touched files), no behavioural changes.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 9, 2026

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 9, 2026

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 33c5d8b0-b777-4f7e-bb19-4cb930bd67ac

📥 Commits

Reviewing files that changed from the base of the PR and between 7a9a6c5 and abf2cea.

📒 Files selected for processing (5)
  • docs/reference/configuration-precedence.md
  • scripts/no_magic_numbers_baseline.txt
  • src/synthorg/api/controllers/audit.py
  • src/synthorg/api/controllers/coordination_metrics.py
  • src/synthorg/settings/subscribers/api_bridge_subscriber.py
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: Build Web Assets (melange)
  • GitHub Check: Doc Drift Gate
  • GitHub Check: Type Check
  • GitHub Check: CodSpeed Python benchmarks
  • GitHub Check: Lighthouse Site
  • GitHub Check: Build Preview
  • GitHub Check: Analyze (python)
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (5)
src/synthorg/!(persistence)/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Only src/synthorg/persistence/ may import sqlite/psycopg or emit raw SQL; all other modules must use abstraction layer

Files:

  • src/synthorg/api/controllers/audit.py
  • src/synthorg/api/controllers/coordination_metrics.py
  • src/synthorg/settings/subscribers/api_bridge_subscriber.py
src/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.py: Follow configuration precedence: DB > env > YAML > code default via SettingsService/ConfigResolver; never use os.environ.get() outside startup
Numeric constants must live in settings/definitions/; only allowlist 0/1/-1, HTTP codes, hex masks, and powers-of-2 in code. Enforced by scripts/check_no_magic_numbers.py
Comments should document WHY only; no reviewer citations, issue back-refs, or migration framing. Enforced by check_no_review_origin_in_code.py and check_no_migration_framing.py
Do not use from __future__ import annotations (Python 3.14+ has PEP 649); use PEP 758 except except A, B: requires parens when binding
Require type hints on public functions; enforce mypy strict mode. Use Google-style docstrings. Keep line length 88; functions <50 lines; files <800 lines
Define error classes as <Domain><Condition>Error inheriting from DomainError, never directly from Exception/RuntimeError. Enforced by check_domain_error_hierarchy.py
Use Pydantic v2 frozen models with extra="forbid" on API DTOs (Request/Response/Snapshot/Result/Envelope/Status/Info/Summary suffixes); use @computed_field for derived properties; use NotBlankStr for identifiers
Create args models at every system boundary; use parse_typed() for every external dict ingestion. Enforced by check_boundary_typed.py
Ensure immutability via model_copy(update=...) or copy.deepcopy(); use deepcopy at system boundaries
Use asyncio.TaskGroup for fan-out/fan-in; helper functions must catch Exception but re-raise MemoryError/RecursionError
Inject Clock via clock: Clock | None = None parameter; tests inject FakeClock from tests._shared.fake_clock
Services own _lifecycle_lock; timed-out stops mark services as unrestartable
Wrap untrusted content via wrap_untrusted() from engine.prompt_safety; use HTMLParseGuard for HTML parsing (SEC-1)
Import logger via from synthorg.observability import get_logger; variable must be named logger. Never us...

Files:

  • src/synthorg/api/controllers/audit.py
  • src/synthorg/api/controllers/coordination_metrics.py
  • src/synthorg/settings/subscribers/api_bridge_subscriber.py

⚙️ CodeRabbit configuration file

This project uses Python 3.14+ with PEP 758 except syntax: "except A, B:" (comma-separated, no parentheses) is correct and mandatory -- do NOT flag it as a typo or suggest parenthesized form. The "except builtins.MemoryError, RecursionError: raise" pattern is intentional project convention for system-error propagation. When evaluating the 50-line function limit, count only the function body excluding the signature lines, decorators, and docstring. Functions 1-5 lines over due to docstrings or multi-line signatures should not be flagged. Do not suggest extracting single-use helper functions called exactly once -- this reduces readability without improving maintainability.

Files:

  • src/synthorg/api/controllers/audit.py
  • src/synthorg/api/controllers/coordination_metrics.py
  • src/synthorg/settings/subscribers/api_bridge_subscriber.py
{src/**/*.py,tests/**/*.py,web/src/**/*.{ts,tsx}}

📄 CodeRabbit inference engine (CLAUDE.md)

NEVER use real vendor names in project code/tests; use example-provider, test-provider, example-{large,medium,small}-001. Real vendor names allowed only in .claude/, third-party imports, providers/presets.py, web/public/provider-logos/

Files:

  • src/synthorg/api/controllers/audit.py
  • src/synthorg/api/controllers/coordination_metrics.py
  • src/synthorg/settings/subscribers/api_bridge_subscriber.py
{README.md,docs/**/*.md,data/runtime_stats.yaml}

📄 CodeRabbit inference engine (CLAUDE.md)

Numeric claims in README and public docs must be sourced from data/runtime_stats.yaml via <!--RS:NAME--> markers

Files:

  • docs/reference/configuration-precedence.md
docs/**/*.md

📄 CodeRabbit inference engine (CLAUDE.md)

Use D2 for architecture/nested containers with theme 200 (Dark Mauve); use mermaid for flowcharts/sequence/pipelines; use markdown tables for tabular data; CI pinned to D2 v0.7.1

Files:

  • docs/reference/configuration-precedence.md
🧠 Learnings (1)
📚 Learning: 2026-05-05T09:04:46.195Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1760
File: scripts/_dual_backend_parity_lib.py:215-216
Timestamp: 2026-05-05T09:04:46.195Z
Learning: This repository targets Python 3.14+ and follows PEP 758. Therefore, reviewer tooling should NOT treat unparenthesized multi-exception `except` clauses written without an `as` clause (e.g., `except MemoryError, RecursionError:`) as syntax errors. Only flag `except`-clause problems when they are genuinely invalid for Python 3.14+.

Applied to files:

  • src/synthorg/api/controllers/audit.py
  • src/synthorg/api/controllers/coordination_metrics.py
  • src/synthorg/settings/subscribers/api_bridge_subscriber.py
🔇 Additional comments (5)
src/synthorg/api/controllers/coordination_metrics.py (1)

37-41: Comment block is now policy-compliant and clear.

The updated wording documents local intent/behavior only and avoids migration framing.

src/synthorg/api/controllers/audit.py (1)

42-46: LGTM on the guard rationale comment update.

This is concise, local, and documents the “why”/behavior without migration framing.

scripts/no_magic_numbers_baseline.txt (1)

36-36: Baseline coordinate updates look consistent with the refactor.

No issues in these suppression-location adjustments.

Also applies to: 47-47, 57-57, 138-138

docs/reference/configuration-precedence.md (1)

144-190: Documentation aligns with the implemented snapshot + hot-reload pattern.

Clear and accurate description of the startup snapshot, subscriber update flow, and validation path.

src/synthorg/settings/subscribers/api_bridge_subscriber.py (1)

39-53: Subscriber wiring and failure handling look solid.

Import-time watched-key validation plus guarded mutate-and-log flow is consistent with the bridge snapshot design and keeps failure behavior explicit.

Also applies to: 98-127


Walkthrough

This pull request implements a bridge-config snapshot pattern for hot-reloadable API settings. It adds ApiBridgeConfig storage and locking in AppState with read/mutate/swap accessors, installs a startup snapshot resolver that logs structured warnings on failure, introduces ApiBridgeSettingsSubscriber to hot-swap single-field updates, refactors the activities controller to read the lifecycle-event cap from the snapshot, updates module comments and documentation, updates the magic-number baseline, and adds unit tests covering startup, mutation, subscriber behavior, and controller integration.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 34.78% which is insufficient. The required threshold is 40.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main refactor: moving the activities lifecycle-cap fallback from a hardcoded constant to ApiBridgeConfig snapshot pattern.
Description check ✅ Passed The description is comprehensive and related to the changeset, explaining the removal of the constant, addition of new snapshot/subscriber patterns, test coverage, and documentation updates.
Linked Issues check ✅ Passed All acceptance criteria from issue #1797 are met: constant removed, bridge-config snapshot pattern implemented with startup resolution, log-once behavior preserved, tests added, and no_magic_numbers baseline updated.
Out of Scope Changes check ✅ Passed All changes are directly aligned with the linked issue objectives: ApiBridgeConfig snapshot introduction, lifecycle-cap refactor, subscriber wiring, and documentation of the pattern are all in-scope.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements the "Bridge-config snapshot pattern" for hot-reloadable application settings, specifically migrating the activities controller's lifecycle events cap to a centralized ApiBridgeConfig on AppState. The changes include a new ApiBridgeSettingsSubscriber for hot-reloading and documentation of the pattern. Review feedback highlights a syntax error in exception handling that may affect Python versions older than 3.13 and suggests making the settings subscriber type-aware to support non-integer configuration fields.

Comment on lines +784 to +785
except MemoryError, RecursionError:
raise
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The except MemoryError, RecursionError: syntax (without parentheses) is only valid in Python 3.13+ (per PEP 758). If the project supports older Python versions, this will raise a SyntaxError. Additionally, the PR description states that a polish pass parenthesised these in newly-touched files, but this new function still uses the unparenthesised form.

    except (MemoryError, RecursionError):
        raise

)
return
try:
value = await self._app_state.config_resolver.get_int(namespace, key)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The subscriber is hardcoded to use get_int, which will fail or produce incorrect results if a float or bool field from ApiBridgeConfig (such as ws_auth_timeout_seconds) is added to _WATCHED. To make this pattern truly reusable as documented, the resolution should be type-aware based on the field's annotation in the model.

Comment on lines +114 to +115
except MemoryError, RecursionError:
raise
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similar to the issue in lifecycle_helpers.py, this unparenthesised except clause contradicts the PR description's claim about the polish pass and risks SyntaxError on Python versions earlier than 3.13.

Suggested change
except MemoryError, RecursionError:
raise
except (MemoryError, RecursionError):
raise

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 9, 2026

Merging this PR will not alter performance

✅ 33 untouched benchmarks
⏩ 21 skipped benchmarks1


Comparing refactor/issue-1797-activities-lifecycle-bridge (abf2cea) with main (4515c8e)

Open in CodSpeed

Footnotes

  1. 21 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/reference/configuration-precedence.md`:
- Around line 177-179: Update the paragraph to remove the claim that
model_copy(update=...) itself re-validates field bounds; instead state that the
current implementation performs re-validation via model_validate(...) (i.e.,
model_copy(update=...) relies on model_validate for re-validation) so an
out-of-range value is caught by model_validate and the prior snapshot is
retained; replace the misleading sentence about model_copy re-validating
directly with this corrected description and reference model_copy(update=...)
and model_validate(...) in the text.

In `@src/synthorg/api/controllers/audit.py`:
- Around line 42-50: The module-level comment describing the "log-once guard for
the settings-resolution fallback" contains migration-framing language and
references to other controllers (e.g. "activities controller",
"app_state.api_bridge_config", "ApiBridgeSettingsSubscriber") which should be
removed; replace the comment with a concise WHY-only description that explains
the local purpose and behavior of the guard (that it prevents repeated identical
warnings during settings outages by setting a flag on first failure and clearing
it on successful resolution) and omit any reviewer citations, migration status,
or cross-controller comparisons so the comment documents only the rationale for
the guard in this module.

In `@src/synthorg/api/controllers/coordination_metrics.py`:
- Around line 37-45: The comment block describing the module-level log-once
guard should be rewritten to focus only on the local intent and behavior (what
the guard does and why it exists) and must remove migration/history references
such as "activities controller", "app_state.api_bridge_config", and
"ApiBridgeSettingsSubscriber"; keep a concise explanation that this guard
prevents repeated identical warnings during a settings-resolution failure, that
the flag is set on first failure and cleared on next successful resolution, and
where it is used in this module (i.e., the log-once guard for the
settings-resolution fallback) so readers understand its purpose without
migration framing.

In `@src/synthorg/settings/subscribers/api_bridge_subscriber.py`:
- Around line 10-15: The module docstring currently contains migration-framing
language about controllers migrating and should be cleaned to remove
reviewer/transition commentary; edit the top-level docstring in
api_bridge_subscriber.py to state only the purpose of the setting (e.g., that it
watches api.max_lifecycle_events_per_query consumed by ActivityController as its
LIMIT clamp) and remove the phrase "as controllers migrate..." and any
migration/back-ref text, leaving plain documentation of ApiBridgeConfig/_WATCHED
and what is being watched.
- Around line 64-67: The docstring is out of sync: it states the subscriber
builds a new snapshot with model_copy(update=...) and calls
AppState.swap_api_bridge_config, but the implementation actually calls
mutate_api_bridge_config; update the docstring in the subscriber to describe
that the subscriber resolves the integer via ConfigResolver, applies the change
by calling AppState.mutate_api_bridge_config (not swapping the entire snapshot),
and explain that mutate_api_bridge_config mutates only the relevant fields
rather than replacing the whole config; also remove or replace the mention of
model_copy(update=...) unless that exact method is used in the code.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1ab22fad-d34a-41db-b8f0-e18fc76c6239

📥 Commits

Reviewing files that changed from the base of the PR and between 4515c8e and 7a9a6c5.

📒 Files selected for processing (14)
  • docs/reference/configuration-precedence.md
  • scripts/no_magic_numbers_baseline.txt
  • src/synthorg/api/controllers/activities.py
  • src/synthorg/api/controllers/audit.py
  • src/synthorg/api/controllers/coordination_metrics.py
  • src/synthorg/api/lifecycle_helpers.py
  • src/synthorg/api/state.py
  • src/synthorg/api/state_services.py
  • src/synthorg/settings/subscribers/__init__.py
  • src/synthorg/settings/subscribers/api_bridge_subscriber.py
  • tests/unit/api/controllers/test_activities.py
  • tests/unit/api/test_apply_api_bridge_config_snapshot.py
  • tests/unit/api/test_state.py
  • tests/unit/settings/test_api_bridge_subscriber.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: Deploy Preview
  • GitHub Check: Build Backend
  • GitHub Check: Build Web Assets (melange)
  • GitHub Check: Test (Python 3.14)
  • GitHub Check: CodSpeed Python benchmarks
  • GitHub Check: Lighthouse Site
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (6)
tests/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/**/*.py: Mark tests with @pytest.mark.{unit,integration,e2e,slow}. Use async auto-detection. Global timeout 30s. Coverage minimum 80%
Windows test environment: unit tests use WindowsSelectorEventLoopPolicy (3.14 IOCP teardown race); subprocess tests override back
Every Mock must declare spec=ConcreteClass; baseline at scripts/mock_spec_baseline.txt
Use FakeClock from tests._shared.fake_clock; inject via clock= parameter in tests
Hypothesis properties: maintain 10 deterministic CI examples; treat test failures as real bugs (fix + add @example(...)). Never skip/xfail flaky tests; fix fundamentally. Use asyncio.Event().wait() not large sleep()

Files:

  • tests/unit/api/test_apply_api_bridge_config_snapshot.py
  • tests/unit/settings/test_api_bridge_subscriber.py
  • tests/unit/api/test_state.py
  • tests/unit/api/controllers/test_activities.py

⚙️ CodeRabbit configuration file

Test files do not require Google-style docstrings on classes or functions -- ruff D rules are only enforced on src/. A bare @settings() decorator with no arguments on Hypothesis property tests is a no-op and should not be suggested -- the HYPOTHESIS_PROFILE env var controls example counts via registered profiles, which @given() honors automatically.

Files:

  • tests/unit/api/test_apply_api_bridge_config_snapshot.py
  • tests/unit/settings/test_api_bridge_subscriber.py
  • tests/unit/api/test_state.py
  • tests/unit/api/controllers/test_activities.py
{src/**/*.py,tests/**/*.py,web/src/**/*.{ts,tsx}}

📄 CodeRabbit inference engine (CLAUDE.md)

NEVER use real vendor names in project code/tests; use example-provider, test-provider, example-{large,medium,small}-001. Real vendor names allowed only in .claude/, third-party imports, providers/presets.py, web/public/provider-logos/

Files:

  • tests/unit/api/test_apply_api_bridge_config_snapshot.py
  • src/synthorg/api/controllers/audit.py
  • src/synthorg/api/state.py
  • src/synthorg/api/controllers/coordination_metrics.py
  • src/synthorg/api/state_services.py
  • src/synthorg/settings/subscribers/api_bridge_subscriber.py
  • tests/unit/settings/test_api_bridge_subscriber.py
  • src/synthorg/api/lifecycle_helpers.py
  • src/synthorg/settings/subscribers/__init__.py
  • tests/unit/api/test_state.py
  • tests/unit/api/controllers/test_activities.py
  • src/synthorg/api/controllers/activities.py
src/synthorg/!(persistence)/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Only src/synthorg/persistence/ may import sqlite/psycopg or emit raw SQL; all other modules must use abstraction layer

Files:

  • src/synthorg/api/controllers/audit.py
  • src/synthorg/api/state.py
  • src/synthorg/api/controllers/coordination_metrics.py
  • src/synthorg/api/state_services.py
  • src/synthorg/settings/subscribers/api_bridge_subscriber.py
  • src/synthorg/api/lifecycle_helpers.py
  • src/synthorg/settings/subscribers/__init__.py
  • src/synthorg/api/controllers/activities.py
src/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.py: Follow configuration precedence: DB > env > YAML > code default via SettingsService/ConfigResolver; never use os.environ.get() outside startup
Numeric constants must live in settings/definitions/; only allowlist 0/1/-1, HTTP codes, hex masks, and powers-of-2 in code. Enforced by scripts/check_no_magic_numbers.py
Comments should document WHY only; no reviewer citations, issue back-refs, or migration framing. Enforced by check_no_review_origin_in_code.py and check_no_migration_framing.py
Do not use from __future__ import annotations (Python 3.14+ has PEP 649); use PEP 758 except except A, B: requires parens when binding
Require type hints on public functions; enforce mypy strict mode. Use Google-style docstrings. Keep line length 88; functions <50 lines; files <800 lines
Define error classes as <Domain><Condition>Error inheriting from DomainError, never directly from Exception/RuntimeError. Enforced by check_domain_error_hierarchy.py
Use Pydantic v2 frozen models with extra="forbid" on API DTOs (Request/Response/Snapshot/Result/Envelope/Status/Info/Summary suffixes); use @computed_field for derived properties; use NotBlankStr for identifiers
Create args models at every system boundary; use parse_typed() for every external dict ingestion. Enforced by check_boundary_typed.py
Ensure immutability via model_copy(update=...) or copy.deepcopy(); use deepcopy at system boundaries
Use asyncio.TaskGroup for fan-out/fan-in; helper functions must catch Exception but re-raise MemoryError/RecursionError
Inject Clock via clock: Clock | None = None parameter; tests inject FakeClock from tests._shared.fake_clock
Services own _lifecycle_lock; timed-out stops mark services as unrestartable
Wrap untrusted content via wrap_untrusted() from engine.prompt_safety; use HTMLParseGuard for HTML parsing (SEC-1)
Import logger via from synthorg.observability import get_logger; variable must be named logger. Never us...

Files:

  • src/synthorg/api/controllers/audit.py
  • src/synthorg/api/state.py
  • src/synthorg/api/controllers/coordination_metrics.py
  • src/synthorg/api/state_services.py
  • src/synthorg/settings/subscribers/api_bridge_subscriber.py
  • src/synthorg/api/lifecycle_helpers.py
  • src/synthorg/settings/subscribers/__init__.py
  • src/synthorg/api/controllers/activities.py

⚙️ CodeRabbit configuration file

This project uses Python 3.14+ with PEP 758 except syntax: "except A, B:" (comma-separated, no parentheses) is correct and mandatory -- do NOT flag it as a typo or suggest parenthesized form. The "except builtins.MemoryError, RecursionError: raise" pattern is intentional project convention for system-error propagation. When evaluating the 50-line function limit, count only the function body excluding the signature lines, decorators, and docstring. Functions 1-5 lines over due to docstrings or multi-line signatures should not be flagged. Do not suggest extracting single-use helper functions called exactly once -- this reduces readability without improving maintainability.

Files:

  • src/synthorg/api/controllers/audit.py
  • src/synthorg/api/state.py
  • src/synthorg/api/controllers/coordination_metrics.py
  • src/synthorg/api/state_services.py
  • src/synthorg/settings/subscribers/api_bridge_subscriber.py
  • src/synthorg/api/lifecycle_helpers.py
  • src/synthorg/settings/subscribers/__init__.py
  • src/synthorg/api/controllers/activities.py
{README.md,docs/**/*.md,data/runtime_stats.yaml}

📄 CodeRabbit inference engine (CLAUDE.md)

Numeric claims in README and public docs must be sourced from data/runtime_stats.yaml via <!--RS:NAME--> markers

Files:

  • docs/reference/configuration-precedence.md
docs/**/*.md

📄 CodeRabbit inference engine (CLAUDE.md)

Use D2 for architecture/nested containers with theme 200 (Dark Mauve); use mermaid for flowcharts/sequence/pipelines; use markdown tables for tabular data; CI pinned to D2 v0.7.1

Files:

  • docs/reference/configuration-precedence.md
🧠 Learnings (1)
📚 Learning: 2026-05-05T09:04:46.195Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1760
File: scripts/_dual_backend_parity_lib.py:215-216
Timestamp: 2026-05-05T09:04:46.195Z
Learning: This repository targets Python 3.14+ and follows PEP 758. Therefore, reviewer tooling should NOT treat unparenthesized multi-exception `except` clauses written without an `as` clause (e.g., `except MemoryError, RecursionError:`) as syntax errors. Only flag `except`-clause problems when they are genuinely invalid for Python 3.14+.

Applied to files:

  • tests/unit/api/test_apply_api_bridge_config_snapshot.py
  • src/synthorg/api/controllers/audit.py
  • src/synthorg/api/state.py
  • src/synthorg/api/controllers/coordination_metrics.py
  • src/synthorg/api/state_services.py
  • src/synthorg/settings/subscribers/api_bridge_subscriber.py
  • tests/unit/settings/test_api_bridge_subscriber.py
  • src/synthorg/api/lifecycle_helpers.py
  • src/synthorg/settings/subscribers/__init__.py
  • tests/unit/api/test_state.py
  • tests/unit/api/controllers/test_activities.py
  • src/synthorg/api/controllers/activities.py
🔇 Additional comments (18)
src/synthorg/api/state.py (1)

421-435: Good snapshot initialization and lock pairing.

Default-constructing ApiBridgeConfig() plus a dedicated lock gives consumers a safe pre-startup snapshot and supports atomic swaps cleanly.

src/synthorg/api/state_services.py (1)

1046-1120: Lock-scoped mutate/swap implementation looks solid.

The atomic merge + model_validate(...) path prevents lost updates and preserves validator enforcement for hot-reload edits.

tests/unit/api/test_state.py (1)

530-567: Nice targeted coverage for the new bridge snapshot behavior.

These tests validate pre-startup defaults and swap semantics directly on AppState, which reduces regression risk for the new snapshot pathway.

src/synthorg/settings/subscribers/__init__.py (1)

3-5: LGTM!

The new ApiBridgeSettingsSubscriber import and re-export follows the established pattern for other subscribers in this package.

Also applies to: 23-23

tests/unit/settings/test_api_bridge_subscriber.py (4)

29-60: LGTM!

The helper factory properly uses create_autospec with spec= for both SettingsService and ConfigResolver, and the test structure cleanly separates subscriber construction from assertions.


63-78: LGTM!

Protocol conformance tests correctly verify SettingsSubscriber implementation, watched_keys content, and subscriber_name behavior.


81-145: LGTM!

The rebuild tests comprehensively cover the critical paths: successful swap with field preservation, resolver failure (no swap + re-raise), MemoryError propagation, and Pydantic ValidationError handling that keeps the prior snapshot intact.


148-171: LGTM!

The unexpected-routing tests correctly verify that unknown namespaces and keys are ignored without modifying the existing snapshot.

src/synthorg/api/lifecycle_helpers.py (4)

18-18: LGTM!

New imports for API_BRIDGE_CONFIG_RESOLVE_FAILED event constant and ApiBridgeSettingsSubscriber are correctly placed with existing imports.

Also applies to: 34-34


764-795: LGTM!

The _apply_api_bridge_config_snapshot helper correctly follows the project's resolver-failure pattern: guard on has_config_resolver, re-raise system errors, emit a single structured warning on transient failures while retaining the default snapshot, and only swap on success.


594-604: LGTM!

The ApiBridgeSettingsSubscriber instantiation and registration follows the established pattern for other subscribers in _build_settings_dispatcher.


812-812: LGTM!

Calling _apply_api_bridge_config_snapshot after the approval-urgency invariant check ensures the API bridge config is snapshotted early in the startup sequence.

src/synthorg/api/controllers/activities.py (2)

362-362: LGTM!

Reading lifecycle_cap directly from app_state.api_bridge_config.max_lifecycle_events_per_query is the correct simplification: synchronous access to a pre-resolved frozen snapshot, with hot-swap support via the subscriber.


359-368: LGTM!

The list_activities endpoint correctly reads the lifecycle cap from the bridge-config snapshot and passes it to list_events. The simplified flow eliminates the previous async resolution overhead.

tests/unit/api/test_apply_api_bridge_config_snapshot.py (2)

27-46: LGTM!

The helper factories correctly use create_autospec(ConfigResolver, instance=True) and provide clean abstractions for test setup.


48-103: LGTM!

The test class comprehensively covers the critical paths: no-resolver (keeps default), happy-path (swaps snapshot), resolver failure (keeps default + single structured warning), and MemoryError propagation. The warning capture via monkeypatch correctly validates the event name and structured fields.

tests/unit/api/controllers/test_activities.py (2)

719-784: LGTM!

The TestActivityFeedLifecycleCap tests correctly verify the integration: the spy pattern captures the limit passed to list_events, and the hot-swap test confirms that swap_api_bridge_config takes effect immediately on subsequent requests. The finally block properly restores the original snapshot.


786-798: LGTM!

The controller surface tests provide valuable regression guards, ensuring the legacy _MAX_LIFECYCLE_EVENTS, _resolve_lifecycle_cap, and _lifecycle_cap_fallback_logged symbols remain removed.

Comment thread docs/reference/configuration-precedence.md Outdated
Comment thread src/synthorg/api/controllers/audit.py Outdated
Comment thread src/synthorg/api/controllers/coordination_metrics.py Outdated
Comment thread src/synthorg/settings/subscribers/api_bridge_subscriber.py Outdated
Comment thread src/synthorg/settings/subscribers/api_bridge_subscriber.py Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented May 9, 2026

Codecov Report

❌ Patch coverage is 94.73684% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.74%. Comparing base (4515c8e) to head (abf2cea).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...horg/settings/subscribers/api_bridge_subscriber.py 91.17% 2 Missing and 1 partial ⚠️
src/synthorg/api/lifecycle_helpers.py 93.33% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1840   +/-   ##
=======================================
  Coverage   84.73%   84.74%           
=======================================
  Files        1799     1800    +1     
  Lines      104779   104835   +56     
  Branches     9187     9190    +3     
=======================================
+ Hits        88786    88843   +57     
+ Misses      13758    13756    -2     
- Partials     2235     2236    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Aureliolo added 2 commits May 9, 2026 15:00
- docs/reference/configuration-precedence.md: clarify mutate_* re-validates via
  model_validate (Pydantic v2 skips validators on bare model_copy(update=...))
- src/synthorg/api/controllers/audit.py: drop migration framing from log-once
  guard comment
- src/synthorg/api/controllers/coordination_metrics.py: drop migration framing
  from log-once guard comment
- src/synthorg/settings/subscribers/api_bridge_subscriber.py: drop 'as
  controllers migrate' framing from module docstring; sync class docstring with
  mutate_api_bridge_config (was claiming model_copy/swap)

Skipped (factually wrong):
- gemini lifecycle_helpers.py:785 / api_bridge_subscriber.py:115 unparenthesised
  except: PEP 758 form is mandated by CLAUDE.md and CodeRabbit baseline learning
  (Py3.14+ repo)
- gemini api_bridge_subscriber.py:112 type-aware get_int: _WATCHED only carries
  one int field; type-dispatch is YAGNI scaffolding without a non-int consumer
The migration-framing comment trim in the prior commit removed 4 lines from each
of audit.py and coordination_metrics.py preambles, which shifted the pinned
limit=50 default-arg sites from line 108→104 and 100→96. Update the baseline
coordinates to match; no new magic-number sites introduced.
@Aureliolo Aureliolo merged commit 7a56e9c into main May 9, 2026
77 checks passed
@Aureliolo Aureliolo deleted the refactor/issue-1797-activities-lifecycle-bridge branch May 9, 2026 13:28
@Aureliolo Aureliolo temporarily deployed to cloudflare-preview May 9, 2026 13:28 — with GitHub Actions Inactive
Aureliolo pushed a commit that referenced this pull request May 10, 2026
<!-- HIGHLIGHTS_START -->
## Highlights

> _AI-generated summary (model: `openai/gpt-4.1-mini` via GitHub
Models). Commit-based changelog below._

### What you'll notice
- Improved error logging and Prometheus instrumentation provide better
system monitoring.
- Eliminated race conditions in CI tagging for more reliable development
releases.
- Fixed critical configuration access and kill-switch bugs to enhance
system stability.
- Enhanced client experience with retry-after headers and better
websocket reconnect behavior.

### What's new
- Introduced composite indexes and cursor pagination for faster data
queries.
- Added server-sent events rate limiting and Ollama input sanitization
for improved security.

### Under the hood
- Centralized workflow error mappings to standardize error handling.
- Refactored API lifecycle fallback to use a configuration snapshot for
consistency.
- Tightened startup settings baseline and reduced controller error
baseline to zero.
- Replaced flaky contributor-assistant GitHub action with a custom
stable step.
- Consolidated Renovate dependency groups to avoid update conflicts.
- Upgraded in-toto-golang dependency to fix security vulnerabilities and
dropped unnecessary CVE waivers.
- Extensive lock file maintenance and multiple infrastructure and Python
dependency updates.

<!-- HIGHLIGHTS_END -->

:robot: I have created a release *beep* *boop*
---


##
[0.8.2](v0.8.1...v0.8.2)
(2026-05-10)


### Features

* close audit gaps in error logging and Prometheus instrumentation
([#1821](#1821))
([ef00fdc](ef00fdc))


### Bug Fixes

* **ci:** eliminate dev-release tag-vs-downstream race + CI hygiene
audit ([#1827](#1827))
([b7b9a59](b7b9a59))
* **config:** close 6 settings reachability + kill-switch gaps
([#1798](#1798))
([410cb3b](410cb3b))
* correctness / safety fixes from 2026-05-05 audit (Wave 28)
([#1823](#1823))
([d01e624](d01e624))


### Performance

* composite indexes + cursor pagination + clock seam + SSE rate-limit +
Ollama sanitization + retry-after web client + WS reconnect jitter
([#1822](#1822))
([d1faf86](d1faf86))


### Refactoring

* **api:** move activities lifecycle-cap fallback to ApiBridgeConfig
snapshot ([#1840](#1840))
([7a56e9c](7a56e9c))
* centralise workflow error mapping and shared error codes
([#1778](#1778) sub-tasks A
+ E) ([#1843](#1843))
([11132cd](11132cd))
* drive controller-error baseline to zero
([#1778](#1778) sub-task A
tail) ([#1846](#1846))
([e96ae20](e96ae20))
* slim CLAUDE.md, port pr-review-toolkit agents, sync .opencode parity
([#1833](#1833))
([e6372b8](e6372b8))
* tighten settings → startup-trace baseline (8 → 0)
([#1847](#1847))
([3376ee2](3376ee2))


### Documentation

* fix CLAUDE.md inaccuracies and drop drift-prone counts
([#1844](#1844))
([371925f](371925f))


### Tests

* replace test placeholders with real subsystem wiring
([#1845](#1845))
([ddbb666](ddbb666))


### CI/CD

* **cla:** replace flaky contributor-assistant action with custom
read-path step
([#1819](#1819))
([11aeafe](11aeafe))
* tidy dev-release notes + stagger renovate lockfile day
([#1824](#1824))
([ec746a9](ec746a9))


### Maintenance

* cleanup roundup, sub-tasks a/c/d/g/h/j/l/m of
[#1781](#1781)
([#1838](#1838))
([099b871](099b871))
* close remaining 5 sub-tasks of
[#1781](#1781) (b/e/f/i/k)
([#1852](#1852))
([59cf0b2](59cf0b2))
* collapse Renovate dep groups into Python / Web / Infrastructure to
remove cross-PR overlap
([#1813](#1813))
([4cbd857](4cbd857))
* **deps,security:** bump in-toto-golang v0.11.0 + drop two patched CVE
waivers ([#1851](#1851))
([0b8b5bb](0b8b5bb))
* disable Renovate vulnerabilityAlerts so security flows into normal
updates ([#1834](#1834))
([6b7d15f](6b7d15f))
* Lock file maintenance
([#1820](#1820))
([ccbad73](ccbad73))
* Lock file maintenance
([#1842](#1842))
([13b68a5](13b68a5))
* Lock file maintenance
([#1853](#1853))
([db6650b](db6650b))
* Update dhi.io/nats:2.14-debian13 Docker digest to eb768bf
([#1841](#1841))
([37f84fc](37f84fc))
* Update Infrastructure dependencies
([#1815](#1815))
([75b12fe](75b12fe))
* Update Infrastructure dependencies
([#1831](#1831))
([3f3c50b](3f3c50b))
* Update Python dependencies
([#1817](#1817))
([e11332f](e11332f))
* Update Python dependencies
([#1832](#1832))
([4515c8e](4515c8e))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: synthorg-repo-bot[bot] <279117679+synthorg-repo-bot[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

refactor(api): move activities lifecycle-cap fallback to settings bridge config

1 participant