feat: migrate config consumers to read through SettingsService#510
feat: migrate config consumers to read through SettingsService#510
Conversation
Migrate API controller config reads from direct RootConfig attribute access to SettingsService resolution (DB > env > YAML > code defaults), enabling runtime config overrides without YAML edits or restarts. Changes: - Create ConfigResolver bridge (settings/resolver.py) with typed scalar accessors and composed-read methods for BudgetConfig and CoordinationConfig - Migrate controllers: autonomy, company (company_name), budget, coordination to use ConfigResolver - Add 7 new settings definitions (3 coordination, 4 budget) - Fix autonomy_level definition bugs: wrong enum_values and yaml_path - Fix coordination yaml_paths: default_topology → topology, max_wave_size → max_concurrency_per_wave - Wire ConfigResolver into AppState via property accessor - 47 new unit tests for ConfigResolver + property-based roundtrips Closes #497
- Use asyncio.TaskGroup for parallel setting reads in composed methods
- Cache ConfigResolver in AppState instead of creating per-access
- Add warning logs before raising ValueError in scalar accessors
- Fix autonomy_level default ("supervised" → "semi") to match model
- Add error propagation tests for composed reads (ExceptionGroup)
- Update CLAUDE.md Package Structure to document ConfigResolver
Pre-reviewed by 3 agents, 9 findings addressed.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds an async ConfigResolver backed by SettingsService, wires it into AppState, updates controllers to await resolver reads, extends settings definitions and SettingsService behavior, updates exception handling in one controller, adds resolver tests and test wiring, and updates docs (including a new Tools subsection in CLAUDE.md). Changes
Sequence DiagramsequenceDiagram
participant Client
participant Controller
participant AppState
participant ConfigResolver
participant SettingsService
participant BaseConfig as "Base Config"
Client->>Controller: HTTP request
Controller->>AppState: access config_resolver
AppState-->>Controller: ConfigResolver instance
Controller->>ConfigResolver: await get_* / get_*_config()
par Parallel resolution
ConfigResolver->>SettingsService: fetch override(s)
ConfigResolver->>BaseConfig: read YAML/base value(s)
and
SettingsService-->>ConfigResolver: override (or none)
BaseConfig-->>ConfigResolver: base value
end
ConfigResolver->>ConfigResolver: merge/convert/validate
ConfigResolver-->>Controller: typed config result
Controller->>Client: HTTP response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
✨ Simplify code
📝 Coding Plan
Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors how configuration values are accessed and managed within the application. By introducing a dedicated Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This is an excellent pull request that introduces a ConfigResolver to centralize and type-check access to settings, which is a great architectural improvement. The migration of existing controllers is clean, and the use of asyncio.TaskGroup for parallel fetching is a nice performance touch. The new unit tests are very thorough, especially with the inclusion of property-based testing using Hypothesis. I found one minor inconsistency in a setting key name, which I've left a comment on. Overall, this is a high-quality contribution.
src/synthorg/settings/resolver.py
Outdated
| ) | ||
|
|
||
| async with asyncio.TaskGroup() as tg: | ||
| t_wave = tg.create_task(self.get_int("coordination", "max_wave_size")) |
There was a problem hiding this comment.
The setting key max_wave_size is inconsistent with its yaml_path (coordination.max_concurrency_per_wave) and the corresponding field in the CoordinationConfig model. To improve clarity and maintainability, I recommend renaming the setting key to max_concurrency_per_wave in src/synthorg/settings/definitions/coordination.py and using the consistent name here.
| t_wave = tg.create_task(self.get_int("coordination", "max_wave_size")) | |
| t_wave = tg.create_task(self.get_int("coordination", "max_concurrency_per_wave")) |
- Rename TestPropertyBased → TestResolverScalarProperties for -k properties - Simplify _make_value helper to use SettingNamespace directly - Add AppState.config_resolver unit tests (raises 503 / returns resolver)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/unit/settings/test_resolver.py`:
- Around line 557-573: Replace the hardcoded `@settings`(max_examples=200)
decorators on test_int_roundtrip, test_float_roundtrip, and test_bool_roundtrip
with `@settings`() so Hypothesis uses the active profile (HYPOTHESIS_PROFILE) to
control example counts; locate the three decorators above the async test
functions (test_int_roundtrip, test_float_roundtrip, test_bool_roundtrip) and
remove the max_examples argument, leaving `@settings`() in place.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 08c80b44-a3b8-4114-9b29-d72170c34810
📒 Files selected for processing (2)
tests/unit/api/test_state.pytests/unit/settings/test_resolver.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). (6)
- GitHub Check: Detect Changes
- GitHub Check: Build Sandbox
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Build Preview
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Do not usefrom __future__ import annotationsin Python code. Python 3.14 has PEP 649 native lazy annotations.
Useexcept A, B:(no parentheses) for exception syntax on Python 3.14. Do not useexcept (A, B):.
Add type hints to all public functions and classes. Use mypy strict mode.
Use Google-style docstrings on all public classes and functions. Docstrings are enforced by ruff D rules.
Create new objects for immutability. Never mutate existing objects. For non-Pydantic internal collections, usecopy.deepcopy()at construction and wrap withMappingProxyTypefor read-only enforcement.
Use frozen Pydantic models for config and identity. Use separate mutable-via-copy models withmodel_copy(update=...)for runtime state that evolves.
Do not mix static config fields with mutable runtime fields in one Pydantic model.
Use Pydantic v2 conventions:@computed_fieldfor derived values,NotBlankStrfor identifier/name fields (including optional and tuple variants), andmodel_validator/ConfigDict.
Useasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code. Prefer structured concurrency over barecreate_task.
Keep function bodies under 50 lines and files under 800 lines.
Handle errors explicitly. Never silently swallow exceptions.
Validate user input, external API responses, and config files at system boundaries.
Useruff checkandruff formatfor Python linting and formatting (88-character line length).
Usemypywith strict mode for type-checking all Python code.
Python version must be 3.14 or higher. PEP 649 provides native lazy annotations.
Files:
tests/unit/api/test_state.pytests/unit/settings/test_resolver.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, and@pytest.mark.slowto mark test cases.
Prefer@pytest.mark.parametrizefor testing similar cases with multiple inputs.
Use Hypothesis for property-based testing in Python with@given+@settings. Control viaHYPOTHESIS_PROFILEenv var (dev: 1000 examples, ci: 200 examples).
Files:
tests/unit/api/test_state.pytests/unit/settings/test_resolver.py
🧠 Learnings (1)
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Property-based testing: Python uses Hypothesis (given + settings). Hypothesis profiles: ci (200 examples, default) and dev (1000 examples), controlled via HYPOTHESIS_PROFILE env var. Run dev profile: HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties.
Applied to files:
tests/unit/settings/test_resolver.py
🧬 Code graph analysis (2)
tests/unit/api/test_state.py (3)
src/synthorg/api/state.py (2)
settings_service(237-239)config_resolver(247-249)src/synthorg/api/errors.py (1)
ServiceUnavailableError(222-231)src/synthorg/settings/resolver.py (1)
ConfigResolver(29-301)
tests/unit/settings/test_resolver.py (4)
src/synthorg/settings/enums.py (2)
SettingNamespace(6-20)SettingSource(49-60)src/synthorg/settings/errors.py (1)
SettingNotFoundError(8-9)src/synthorg/settings/models.py (1)
SettingValue(212-232)src/synthorg/settings/resolver.py (1)
_parse_bool(308-329)
🔇 Additional comments (2)
tests/unit/api/test_state.py (1)
225-242: Good coverage forAppState.config_resolveraccessor behavior.This adds the two critical paths (503 when unavailable, resolver returned when configured) and matches the accessor contract in
src/synthorg/api/state.py.tests/unit/settings/test_resolver.py (1)
107-535: Strong and comprehensive resolver test coverage.The suite validates scalar coercion, composed reads, precedence rules, and
ExceptionGrouppropagation paths in a way that closely matchesConfigResolverbehavior.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #510 +/- ##
==========================================
+ Coverage 93.06% 93.12% +0.05%
==========================================
Files 501 504 +3
Lines 24098 24452 +354
Branches 2298 2333 +35
==========================================
+ Hits 22426 22770 +344
- Misses 1327 1335 +8
- Partials 345 347 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…mini Correctness: - Fix cache race in SettingsService._cache under concurrent TaskGroup reads (dict rebuild → direct key assignment) - Unwrap ExceptionGroup in composed-read methods so controllers see clean SettingNotFoundError/ValueError instead of opaque 500s - Catch ValidationError from BudgetAlertConfig ordering constraint - Raise SettingNotFoundError in _resolve_fallback when default is None (was silently returning empty string) - Add MemoryError/RecursionError guard to _publish_change broad except Naming & consistency: - Rename setting key max_wave_size → max_concurrency_per_wave - Fix 503 error message: "Settings Service" instead of "Config Resolver" - Use InternalServerException for encryption errors (was ClientException) - Add has_config_resolver property for naming symmetry Security: - Sanitize raw values in ValueError messages across all scalar accessors - Sanitize _parse_bool error message Documentation: - Add Raises sections to composed-read and get_autonomy_level docstrings - Fix module docstring (SettingValue not "string values") - Fix get_str Returns section, get_budget_config nested field paths - Add constructor TypeError guard + stale-config invariant note - Remove redundant composed-read debug logs - Document ConfigResolver in operations.md Settings section Tests: - Add @pytest.mark.unit to TestAppStateConfigResolver - Add has_settings_service flag tests (both paths) - Add config_resolver singleton identity assertion - Add has_config_resolver flag tests - Use AsyncMock for async service in tests - Parametrize test_resolves_all_levels - Strengthen topology assertion (exact "sas" match) - Remove hardcoded @settings(max_examples=200) for Hypothesis profiles - Update error match patterns for sanitized messages - Update ExceptionGroup tests → expect unwrapped exceptions - Add constructor None guard test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ttings) The previous change to raise SettingNotFoundError when default=None broke settings like providers/default_provider that legitimately have no built-in default. Revert to returning empty string as sentinel — consumers like ConfigResolver.get_int will raise ValueError on empty input, giving a clear error at the consumer layer. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/api/state.py`:
- Around line 252-254: The guard message for config_resolver is incorrect:
update the call in the config_resolver method so the second argument passed to
_require_service uses the correct service label ("config_resolver") instead of
the misleading "settings_service" so that the 503/log context correctly
references ConfigResolver; locate the config_resolver method and change the
string passed to _require_service accordingly.
In `@src/synthorg/settings/resolver.py`:
- Around line 178-211: The get_enum method currently doesn't log when the
underlying self._settings.get raises SettingNotFoundError; wrap the await
self._settings.get(namespace, key) call in a try/except that catches
SettingNotFoundError, logs SETTINGS_VALIDATION_FAILED with namespace, key,
reason="not_found" and enum_cls=enum_cls.__name__ (mirroring the existing
invalid_enum log), then re-raise the SettingNotFoundError; keep the existing
ValueError handling for invalid enum values unchanged.
- Around line 120-146: get_float currently doesn't log SettingNotFoundError
before it bubbles up; add a try/except around the await
self._settings.get(namespace, key) to catch SettingNotFoundError, emit a
logger.warning with SETTINGS_VALIDATION_FAILED including namespace, key,
reason="missing", exc_info=True, and then re-raise the SettingNotFoundError,
keeping the existing ValueError handling for invalid float in get_float.
- Around line 148-176: get_bool currently calls await
self._settings.get(namespace, key) without logging when the key is missing; wrap
the settings fetch in a try/except that catches SettingNotFoundError, logs
SETTINGS_VALIDATION_FAILED with namespace, key and reason="not_found" (matching
the existing pattern), then re-raises the SettingNotFoundError; keep the
subsequent try/except for ValueError that logs reason="invalid_boolean" and
raises ValueError as before.
- Around line 367-392: The boolean parser currently recognizes only "true"/"1"
and "false"/"0"; extend _BOOL_TRUE and _BOOL_FALSE to also include "yes"/"on"
for true and "no"/"off" for false (case-insensitive) so _parse_bool accepts
those values as well; update the frozenset definitions (_BOOL_TRUE, _BOOL_FALSE)
used by _parse_bool and ensure existing logic in _parse_bool remains unchanged
(use .lower() and the same membership checks) so behavior is consistent.
- Around line 92-118: The get_int method is missing the same
SettingNotFoundError handling/logging present in get_str; update async def
get_int(self, namespace: str, key: str) to catch SettingNotFoundError from
self._settings.get(namespace, key), call logger.warning(SETTINGS_NOT_FOUND,
namespace=namespace, key=key, exc_info=True) (matching get_str's context) and
then re-raise the caught SettingNotFoundError, keeping the existing ValueError
parsing block unchanged so invalid integers are still logged and raised as
before.
In `@src/synthorg/settings/service.py`:
- Line 256: The code currently mutates the internal dict self._cache in-place
via self._cache[cache_key] = setting_value; instead, perform copy-on-write:
create a shallow/deep copy of the underlying dict, set the new key/value on that
copy, then rebind self._cache to an immutable MappingProxyType (constructed from
copy.deepcopy(...) at object construction for initial value as per guidelines)
so you never mutate the original mapping in-place; update the code paths that
initialize or update _cache (references: _cache, cache_key, setting_value) to
follow this pattern and ensure callers only see the MappingProxyType.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1907f51f-9362-43ba-b603-2d45a7c40828
📒 Files selected for processing (9)
docs/design/operations.mdsrc/synthorg/api/controllers/settings.pysrc/synthorg/api/state.pysrc/synthorg/settings/definitions/coordination.pysrc/synthorg/settings/resolver.pysrc/synthorg/settings/service.pytests/integration/engine/test_coordination_wiring.pytests/unit/api/test_state.pytests/unit/settings/test_resolver.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). (4)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Sandbox
- GitHub Check: Build Backend
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Do not usefrom __future__ import annotationsin Python code. Python 3.14 has PEP 649 native lazy annotations.
Useexcept A, B:(no parentheses) for exception syntax on Python 3.14. Do not useexcept (A, B):.
Add type hints to all public functions and classes. Use mypy strict mode.
Use Google-style docstrings on all public classes and functions. Docstrings are enforced by ruff D rules.
Create new objects for immutability. Never mutate existing objects. For non-Pydantic internal collections, usecopy.deepcopy()at construction and wrap withMappingProxyTypefor read-only enforcement.
Use frozen Pydantic models for config and identity. Use separate mutable-via-copy models withmodel_copy(update=...)for runtime state that evolves.
Do not mix static config fields with mutable runtime fields in one Pydantic model.
Use Pydantic v2 conventions:@computed_fieldfor derived values,NotBlankStrfor identifier/name fields (including optional and tuple variants), andmodel_validator/ConfigDict.
Useasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code. Prefer structured concurrency over barecreate_task.
Keep function bodies under 50 lines and files under 800 lines.
Handle errors explicitly. Never silently swallow exceptions.
Validate user input, external API responses, and config files at system boundaries.
Useruff checkandruff formatfor Python linting and formatting (88-character line length).
Usemypywith strict mode for type-checking all Python code.
Python version must be 3.14 or higher. PEP 649 provides native lazy annotations.
Files:
src/synthorg/settings/resolver.pysrc/synthorg/settings/definitions/coordination.pytests/unit/api/test_state.pytests/unit/settings/test_resolver.pysrc/synthorg/settings/service.pysrc/synthorg/api/controllers/settings.pysrc/synthorg/api/state.pytests/integration/engine/test_coordination_wiring.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Every module with business logic must import logger withfrom synthorg.observability import get_loggerthenlogger = get_logger(__name__). Never useimport logging,logging.getLogger(), orprint()in application code.
Always useloggeras the variable name for loggers. Never use_loggerorlog.
Use event name constants from domain-specific modules undersynthorg.observability.eventsfor all logging calls. Import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Use structured logging withlogger.info(EVENT, key=value)syntax. Never use format string logging likelogger.info('msg %s', val).
All error paths must log at WARNING or ERROR level with context before raising.
All state transitions must log at INFO level.
Use DEBUG level for object creation, internal flow, and entry/exit of key functions.
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names:example-provider,example-large-001,example-medium-001,example-small-001,large/medium/smallas aliases. Usetest-provider,test-small-001, etc. in tests.
Library API reference is auto-generated from docstrings via mkdocstrings + Griffe indocs/api/. Use Google-style docstrings for public APIs.
Use Pydantic BaseModel for all data models. Frozen models for config/identity. Mutable-via-copy models for runtime state.
Files:
src/synthorg/settings/resolver.pysrc/synthorg/settings/definitions/coordination.pysrc/synthorg/settings/service.pysrc/synthorg/api/controllers/settings.pysrc/synthorg/api/state.py
src/synthorg/settings/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Settings use runtime-editable persistence with precedence: DB > env > YAML > code defaults. 8 namespaces with Fernet encryption for sensitive values.
Files:
src/synthorg/settings/resolver.pysrc/synthorg/settings/definitions/coordination.pysrc/synthorg/settings/service.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, and@pytest.mark.slowto mark test cases.
Prefer@pytest.mark.parametrizefor testing similar cases with multiple inputs.
Use Hypothesis for property-based testing in Python with@given+@settings. Control viaHYPOTHESIS_PROFILEenv var (dev: 1000 examples, ci: 200 examples).
Files:
tests/unit/api/test_state.pytests/unit/settings/test_resolver.pytests/integration/engine/test_coordination_wiring.py
src/synthorg/api/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/api/**/*.py: Use Litestar for REST + WebSocket API. Controllers, guards, channels, JWT + API key + WS ticket auth, RFC 9457 structured errors.
API error responses use RFC 9457 structured errors: ErrorCategory, ErrorCode, ErrorDetail, ProblemDetail, CATEGORY_TITLES, category_title, category_type_uri, content negotiation.
Files:
src/synthorg/api/controllers/settings.pysrc/synthorg/api/state.py
🧠 Learnings (10)
📚 Learning: 2026-03-16T20:59:42.650Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T20:59:42.650Z
Learning: Applies to src/synthorg/settings/**/*.py : Settings use runtime-editable persistence with precedence: DB > env > YAML > code defaults. 8 namespaces with Fernet encryption for sensitive values.
Applied to files:
docs/design/operations.mdsrc/synthorg/settings/definitions/coordination.pysrc/synthorg/settings/service.pysrc/synthorg/api/controllers/settings.pysrc/synthorg/api/state.py
📚 Learning: 2026-03-16T20:59:42.650Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T20:59:42.650Z
Learning: Applies to src/synthorg/budget/**/*.py : Budget tracking includes pre-flight/in-flight checks, auto-downgrade, billing periods, cost tiers, quota/subscription. CFO includes anomaly detection, efficiency analysis, downgrade recommendations.
Applied to files:
docs/design/operations.md
📚 Learning: 2026-03-16T20:59:42.650Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T20:59:42.650Z
Learning: Applies to src/synthorg/security/**/*.py : Security module includes SecOps agent, rule engine (soft-allow/hard-deny), audit log, output scanner, risk classifier, autonomy levels (4 strategies), timeout policies.
Applied to files:
docs/design/operations.md
📚 Learning: 2026-03-16T20:59:42.650Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T20:59:42.650Z
Learning: Applies to src/synthorg/engine/coordination/**/*.py : Task coordination uses multi-agent pipeline with 4 dispatchers (SAS/centralized/decentralized/context-dependent), wave execution, and workspace lifecycle integration.
Applied to files:
src/synthorg/settings/definitions/coordination.py
📚 Learning: 2026-03-16T20:59:42.650Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T20:59:42.650Z
Learning: Applies to tests/**/*.py : Use Hypothesis for property-based testing in Python with `given` + `settings`. Control via `HYPOTHESIS_PROFILE` env var (dev: 1000 examples, ci: 200 examples).
Applied to files:
tests/unit/settings/test_resolver.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Property-based testing: Python uses Hypothesis (given + settings). Hypothesis profiles: ci (200 examples, default) and dev (1000 examples), controlled via HYPOTHESIS_PROFILE env var. Run dev profile: HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties.
Applied to files:
tests/unit/settings/test_resolver.py
📚 Learning: 2026-03-16T20:59:42.650Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T20:59:42.650Z
Learning: Applies to src/synthorg/api/**/*.py : Use Litestar for REST + WebSocket API. Controllers, guards, channels, JWT + API key + WS ticket auth, RFC 9457 structured errors.
Applied to files:
src/synthorg/api/controllers/settings.py
📚 Learning: 2026-03-16T20:59:42.650Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T20:59:42.650Z
Learning: Applies to src/synthorg/persistence/**/*.py : Persistence uses pluggable PersistenceBackend protocol. SQLite is the initial backend. Settings use SettingsRepository (namespaced settings CRUD).
Applied to files:
src/synthorg/api/state.py
📚 Learning: 2026-03-16T20:59:42.650Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T20:59:42.650Z
Learning: Applies to src/synthorg/observability/**/*.py : Observability includes structured logging via `get_logger(__name__)`, correlation tracking, and log sinks.
Applied to files:
src/synthorg/api/state.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use `import logging` / `logging.getLogger()` / `print()` in application code. Variable name: always `logger` (not `_logger`, not `log`).
Applied to files:
src/synthorg/api/state.py
🧬 Code graph analysis (4)
tests/unit/api/test_state.py (4)
src/synthorg/api/state.py (4)
settings_service(237-239)has_settings_service(242-244)config_resolver(252-254)has_config_resolver(247-249)src/synthorg/api/errors.py (1)
ServiceUnavailableError(222-231)tests/unit/settings/test_resolver.py (1)
resolver(98-102)src/synthorg/settings/resolver.py (1)
ConfigResolver(31-364)
tests/unit/settings/test_resolver.py (3)
src/synthorg/settings/enums.py (2)
SettingNamespace(6-20)SettingSource(49-60)src/synthorg/settings/errors.py (1)
SettingNotFoundError(8-9)src/synthorg/settings/models.py (1)
SettingValue(212-232)
src/synthorg/api/state.py (1)
src/synthorg/settings/resolver.py (1)
ConfigResolver(31-364)
tests/integration/engine/test_coordination_wiring.py (3)
src/synthorg/settings/registry.py (1)
get_registry(111-113)tests/unit/settings/test_service.py (1)
service(106-113)src/synthorg/settings/service.py (1)
SettingsService(152-630)
🔇 Additional comments (13)
src/synthorg/settings/definitions/coordination.py (2)
43-76: Good expansion of coordination controls.The new
fail_fast,enable_workspace_isolation, andbase_branchdefinitions are consistent and correctly namespaced/yaml-mapped.
31-40: No orphaned keys found —max_wave_sizedoes not exist in the codebase.A search across all Python, SQL, and Markdown files found zero occurrences of the old key
max_wave_size. The new keymax_concurrency_per_waveis consistently used throughout the codebase (tests, engine, API DTOs, settings definitions). The settings system uses precedence (DB > env > YAML > code defaults) but does not provide backward-compat aliases inSettingDefinition; however, since the old key is absent, there are no orphaned DB entries to concern.docs/design/operations.md (1)
1044-1044: Documentation update is aligned with implementation.The added Settings section correctly reflects runtime persistence layers and typed composed reads via
ConfigResolver.tests/unit/api/test_state.py (1)
227-273: Nice coverage for resolver wiring semantics.These tests close the gap on
has_settings_service,config_resolveravailability failure mode, and singleton identity behavior.src/synthorg/api/controllers/settings.py (1)
187-195: 500-path handling is improved and correctly typed.Logging context before raising
InternalServerExceptionkeeps observability intact and aligns this path with server-error semantics.tests/integration/engine/test_coordination_wiring.py (1)
270-294: Good integration wiring update.Injecting
SettingsServicehere makes the bootstrap path representative of production resolver usage, and the explicit"sas"assertion keeps the check deterministic.Also applies to: 324-324
tests/unit/settings/test_resolver.py (1)
569-594: Great property-based coverage for scalar parsing paths.Using
@settings()keeps Hypothesis profile-driven while validating roundtrip behavior across int/float/bool accessors.src/synthorg/settings/resolver.py (6)
1-28: LGTM!Module docstring and imports are well-structured. Proper logger setup using
get_loggerfromsynthorg.observabilityand event constants from the domain-specificevents.settingsmodule align with coding guidelines.
31-66: LGTM!Well-documented class with comprehensive docstring explaining the wiring invariant with
AppState. The runtimeNonecheck is good defensive coding for callers without type checking.
68-90: LGTM!Correct pattern: catches
SettingNotFoundError, logs at WARNING level with structured context, then re-raises. This follows the coding guideline that all error paths must log before raising.
213-227: LGTM!Clean delegation to
get_enumwith appropriate deferred import to avoid circular dependencies.
229-303: LGTM!Excellent use of
asyncio.TaskGroupfor parallel resolution per coding guidelines. TheExceptionGroupunwrapping is well-documented, andmodel_copy(update=...)maintains immutability. The nestedauto_downgrade.model_copy()properly preserves unregistered fields.
305-364: LGTM!Good parallel resolution pattern with
asyncio.TaskGroup. The docstring correctly explains why direct construction is used instead ofmodel_copy. The request-level override logic withNonechecks is clear and readable.
| async def get_enum[E: StrEnum]( | ||
| self, | ||
| namespace: str, | ||
| key: str, | ||
| enum_cls: type[E], | ||
| ) -> E: | ||
| """Resolve a setting as a ``StrEnum`` member. | ||
|
|
||
| Args: | ||
| namespace: Setting namespace. | ||
| key: Setting key. | ||
| enum_cls: The enum class to coerce the value into. | ||
|
|
||
| Returns: | ||
| The matching enum member. | ||
|
|
||
| Raises: | ||
| SettingNotFoundError: If the key is not in the registry. | ||
| ValueError: If the value does not match any enum member. | ||
| """ | ||
| result = await self._settings.get(namespace, key) | ||
| try: | ||
| return enum_cls(result.value) | ||
| except ValueError: | ||
| logger.warning( | ||
| SETTINGS_VALIDATION_FAILED, | ||
| namespace=namespace, | ||
| key=key, | ||
| reason="invalid_enum", | ||
| enum_cls=enum_cls.__name__, | ||
| exc_info=True, | ||
| ) | ||
| msg = f"Setting {namespace}/{key} has an invalid {enum_cls.__name__} value" | ||
| raise ValueError(msg) from None |
There was a problem hiding this comment.
Same SettingNotFoundError logging gap.
Apply the same fix pattern. The enum_cls context in the validation failure log is helpful.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/settings/resolver.py` around lines 178 - 211, The get_enum
method currently doesn't log when the underlying self._settings.get raises
SettingNotFoundError; wrap the await self._settings.get(namespace, key) call in
a try/except that catches SettingNotFoundError, logs SETTINGS_VALIDATION_FAILED
with namespace, key, reason="not_found" and enum_cls=enum_cls.__name__
(mirroring the existing invalid_enum log), then re-raise the
SettingNotFoundError; keep the existing ValueError handling for invalid enum
values unchanged.
| _BOOL_TRUE = frozenset({"true", "1"}) | ||
| _BOOL_FALSE = frozenset({"false", "0"}) | ||
|
|
||
|
|
||
| def _parse_bool(value: str) -> bool: | ||
| """Parse a string into a boolean. | ||
|
|
||
| Accepts ``"true"``/``"false"``/``"1"``/``"0"`` | ||
| (case-insensitive). | ||
|
|
||
| Args: | ||
| value: String to parse. | ||
|
|
||
| Returns: | ||
| The parsed boolean. | ||
|
|
||
| Raises: | ||
| ValueError: If the string is not a recognised boolean. | ||
| """ | ||
| lower = value.lower() | ||
| if lower in _BOOL_TRUE: | ||
| return True | ||
| if lower in _BOOL_FALSE: | ||
| return False | ||
| msg = "Value is not a recognized boolean string" | ||
| raise ValueError(msg) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
LGTM!
Good use of frozenset for O(1) lookup and immutability. Case-insensitive parsing is appropriate.
Consider expanding accepted values to include "yes"/"no" and "on"/"off" for broader config compatibility, though the current strict set is fine if intentional.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/settings/resolver.py` around lines 367 - 392, The boolean parser
currently recognizes only "true"/"1" and "false"/"0"; extend _BOOL_TRUE and
_BOOL_FALSE to also include "yes"/"on" for true and "no"/"off" for false
(case-insensitive) so _parse_bool accepts those values as well; update the
frozenset definitions (_BOOL_TRUE, _BOOL_FALSE) used by _parse_bool and ensure
existing logic in _parse_bool remains unchanged (use .lower() and the same
membership checks) so behavior is consistent.
| # plaintext secrets in memory. | ||
| if not definition.sensitive: | ||
| self._cache = {**self._cache, cache_key: setting_value} | ||
| self._cache[cache_key] = setting_value |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Avoid in-place mutation of _cache to preserve immutability guarantees.
This mutates internal state directly. Use copy-on-write and rebind the dict instead.
Suggested change
- if not definition.sensitive:
- self._cache[cache_key] = setting_value
+ if not definition.sensitive:
+ next_cache = dict(self._cache)
+ next_cache[cache_key] = setting_value
+ self._cache = next_cacheAs per coding guidelines: Create new objects for immutability. Never mutate existing objects. For non-Pydantic internal collections, use copy.deepcopy() at construction and wrap with MappingProxyType for read-only enforcement.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| self._cache[cache_key] = setting_value | |
| if not definition.sensitive: | |
| next_cache = dict(self._cache) | |
| next_cache[cache_key] = setting_value | |
| self._cache = next_cache |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/settings/service.py` at line 256, The code currently mutates the
internal dict self._cache in-place via self._cache[cache_key] = setting_value;
instead, perform copy-on-write: create a shallow/deep copy of the underlying
dict, set the new key/value on that copy, then rebind self._cache to an
immutable MappingProxyType (constructed from copy.deepcopy(...) at object
construction for initial value as per guidelines) so you never mutate the
original mapping in-place; update the code paths that initialize or update
_cache (references: _cache, cache_key, setting_value) to follow this pattern and
ensure callers only see the MappingProxyType.
…label) - Add SettingNotFoundError catch + SETTINGS_NOT_FOUND log to get_int, get_float, get_bool, get_enum (consistent with get_str pattern) - Revert config_resolver guard label to "config_resolver" (callers see this property name, not the internal dependency) - Add comment explaining intentional direct dict mutation in cache (MappingProxyType copy-on-write would reintroduce TOCTOU race) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/synthorg/settings/service.py (1)
255-263:⚠️ Potential issue | 🟠 MajorAvoid in-place mutation of
_cache.This reintroduces mutable shared-state updates on a non-Pydantic internal collection. Use copy-on-write and rebind instead of mutating
self._cachein place.As per coding guidelines: "Create new objects instead of mutating existing ones. For non-Pydantic internal collections, use `copy.deepcopy()` at construction + `MappingProxyType` wrapping."Suggested fix
- if not definition.sensitive: - self._cache[cache_key] = setting_value + if not definition.sensitive: + self._cache = {**self._cache, cache_key: setting_value}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/settings/service.py` around lines 255 - 263, The in-place mutation self._cache[cache_key] = setting_value must be replaced with a copy-on-write rebind to avoid mutating shared internal state; when definition.sensitive is False, create a shallow copy of self._cache (or the configured deepcopy-backed baseline created at construction), set the new key on that copy using cache_key and setting_value, then reassign self._cache to the new mapping (and ensure the class constructs _cache using copy.deepcopy(...) and wraps it with MappingProxyType where appropriate so external callers see an immutable view).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/settings/resolver.py`:
- Around line 62-64: The constructor currently raises TypeError when
settings_service is None without logging; before raising, emit a structured
ERROR or WARNING log with context including the variable name and any relevant
identifiers (e.g., settings_service and msg) so failures are visible in logs —
locate the None-check around settings_service in resolver.py (the if
settings_service is None block) and add a logger.error(...) or
logger.warning(...) call with the message and contextual details immediately
before raising the TypeError.
- Around line 317-320: In the except ValidationError as exc block (the handler
that currently builds msg and raises ValueError), log the failure at WARNING or
ERROR with context before re-raising: call the module/class logger (e.g.
logger.error(...) or logger.warning(...), including the constructed msg and
exc_info=exc or exc_info=True) immediately before the raise so the
ValidationError context is recorded; if no logger exists in this module, create
one via logging.getLogger(__name__) and use it in that except block.
- Around line 261-335: get_budget_config is too large and mixes responsibilities
(task fan-out, exception mapping, and model assembly); split it into small
helpers: extract the asyncio.TaskGroup fan-out and awaiting of individual
setting calls into a helper named _gather_budget_settings that returns the
awaited results (total_monthly, per_task_limit, per_agent_daily_limit,
auto_downgrade_enabled, auto_downgrade_threshold, reset_day, alert_warn_at,
alert_critical_at, alert_hard_stop_at) and preserves the current
ExceptionGroup-to-first-exception unwrapping logic; extract the
BudgetAlertConfig construction and ValidationError->ValueError mapping into a
helper named _build_budget_alerts(warn, crit, stop) that raises the same
ValueError on validation failure; finally, reduce get_budget_config to calling
_gather_budget_settings, _build_budget_alerts, and a small assembly step that
returns base.model_copy(...) updating total_monthly, per_task_limit,
per_agent_daily_limit, reset_day, alerts, and auto_downgrade via
base.auto_downgrade.model_copy(...). Ensure helper names
(_gather_budget_settings, _build_budget_alerts) are used so reviewers can locate
changes and keep each function under the 50-line guideline.
In `@src/synthorg/settings/service.py`:
- Around line 384-388: The current mapping in SettingsService where you set
default = definition.default if definition.default is not None else "" masks the
difference between an explicit empty string and an unset default; change this to
preserve None (i.e., leave default as definition.default) or use a unique
sentinel for “unset” so callers like ConfigResolver.get_int can detect missing
defaults and raise appropriately; update handling in functions that rely on an
empty-string sentinel (refer to the variable named default and uses in
ConfigResolver.get_int / SettingsService methods) to accept None or the new
sentinel instead of "".
---
Duplicate comments:
In `@src/synthorg/settings/service.py`:
- Around line 255-263: The in-place mutation self._cache[cache_key] =
setting_value must be replaced with a copy-on-write rebind to avoid mutating
shared internal state; when definition.sensitive is False, create a shallow copy
of self._cache (or the configured deepcopy-backed baseline created at
construction), set the new key on that copy using cache_key and setting_value,
then reassign self._cache to the new mapping (and ensure the class constructs
_cache using copy.deepcopy(...) and wraps it with MappingProxyType where
appropriate so external callers see an immutable view).
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9fa9a431-6f7a-41f6-b6ba-289443a0a0ac
📒 Files selected for processing (3)
src/synthorg/api/state.pysrc/synthorg/settings/resolver.pysrc/synthorg/settings/service.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). (4)
- GitHub Check: Build Sandbox
- GitHub Check: Build Backend
- GitHub Check: Test (Python 3.14)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations— Python 3.14+ has PEP 649 native lazy annotations.
Useexcept A, B:syntax (no parentheses) for exception handling — PEP 758 syntax enforced by ruff on Python 3.14.
Include type hints on all public functions and classes; mypy strict mode is enforced.
Use Google-style docstrings on all public classes and functions (enforced by ruff D rules).
Create new objects instead of mutating existing ones. For non-Pydantic internal collections, usecopy.deepcopy()at construction +MappingProxyTypewrapping. Fordict/listfields in frozen Pydantic models, rely onfrozen=Trueandcopy.deepcopy()at system boundaries.
Separate frozen Pydantic models for config/identity from mutable-via-copy models for runtime state. Never mix static config fields with mutable runtime fields in one model.
Use Pydantic v2 conventions:BaseModel,model_validator,computed_field,ConfigDict. Use@computed_fieldfor derived values instead of storing redundant fields. UseNotBlankStrfor all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators.
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls). Use structured concurrency over barecreate_task.
Enforce maximum line length of 88 characters (ruff).
Keep functions under 50 lines and files under 800 lines.
Handle errors explicitly; never silently swallow exceptions.
Validate at system boundaries (user input, external APIs, config files).
Files:
src/synthorg/settings/resolver.pysrc/synthorg/api/state.pysrc/synthorg/settings/service.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Every module with business logic MUST havefrom synthorg.observability import get_loggerthenlogger = get_logger(__name__). Never useimport loggingorlogging.getLogger()orprint()in application code.
Use event name constants fromsynthorg.observability.events.<domain>modules instead of string literals. Import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Always use structured kwargs for logging:logger.info(EVENT, key=value)— neverlogger.info('msg %s', val).
All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, and entry/exit of key functions.
Observability: structured logging viaget_logger(). Correlation tracking. Log sinks. Metrics and tracing. Every module with business logic must have a logger.
Files:
src/synthorg/settings/resolver.pysrc/synthorg/api/state.pysrc/synthorg/settings/service.py
+(src|tests)/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names:
example-provider,example-large-001,example-medium-001,example-small-001. Vendor names may only appear in: (1) Operations design page, (2).claude/files, (3) third-party import paths.
Files:
src/synthorg/settings/resolver.pysrc/synthorg/api/state.pysrc/synthorg/settings/service.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Frozen Pydantic models for agent/company config/identity; mutable-via-copy models for runtime agent execution state (usingmodel_copy(update=...)). Never mix static config with mutable state in one model.
UseNotBlankStrfromcore.typesfor all identifier/name fields (including optionalNotBlankStr | Noneand tuple variants) instead of manual whitespace validators.
Files:
src/synthorg/settings/resolver.pysrc/synthorg/api/state.pysrc/synthorg/settings/service.py
src/synthorg/settings/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Settings: runtime-editable via DB > env > YAML > code defaults. 8 namespaces: company, providers, memory, budget, security, coordination, observability, backup. Fernet encryption for sensitive values.
Files:
src/synthorg/settings/resolver.pysrc/synthorg/settings/service.py
src/synthorg/api/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
API: Litestar REST + WebSocket. JWT + API key + WS ticket auth. Approval gate integration. Coordination and collaboration endpoints. Settings endpoint. RFC 9457 structured errors.
Files:
src/synthorg/api/state.py
🧠 Learnings (5)
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/settings/resolver.py
📚 Learning: 2026-03-17T06:29:58.077Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:29:58.077Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions. Pure data models, enums, and re-exports do NOT need logging.
Applied to files:
src/synthorg/settings/resolver.py
📚 Learning: 2026-03-17T06:29:58.077Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:29:58.077Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use `import logging` / `logging.getLogger()` / `print()` in application code. Variable name: always `logger`.
Applied to files:
src/synthorg/api/state.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use `import logging` / `logging.getLogger()` / `print()` in application code. Variable name: always `logger` (not `_logger`, not `log`).
Applied to files:
src/synthorg/api/state.py
📚 Learning: 2026-03-17T06:37:14.526Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:37:14.526Z
Learning: Applies to src/synthorg/settings/**/*.py : Settings: runtime-editable via DB > env > YAML > code defaults. 8 namespaces: company, providers, memory, budget, security, coordination, observability, backup. Fernet encryption for sensitive values.
Applied to files:
src/synthorg/settings/service.py
🧬 Code graph analysis (1)
src/synthorg/api/state.py (1)
src/synthorg/settings/resolver.py (1)
ConfigResolver(31-396)
🔇 Additional comments (2)
src/synthorg/settings/service.py (1)
626-627: Good critical-exception passthrough.Explicitly re-raising
MemoryErrorandRecursionErrorbefore the general exception handler is the right reliability guard.src/synthorg/api/state.py (1)
105-109: ConfigResolver singleton wiring and guard access look correct.Resolver is cached once at construction and exposed with a correctly labeled 503 guard, matching the intended AppState contract.
Also applies to: 246-255
| async def get_budget_config(self) -> BudgetConfig: | ||
| """Assemble a ``BudgetConfig`` from individually resolved settings. | ||
|
|
||
| Starts from the YAML-loaded base config and overrides fields | ||
| that have registered settings definitions. Unregistered fields | ||
| on nested models (e.g. ``auto_downgrade.downgrade_map``, | ||
| ``auto_downgrade.boundary``) keep their YAML values. | ||
|
|
||
| Uses ``asyncio.TaskGroup`` to resolve all settings in parallel. | ||
| If any individual resolution fails, the ``ExceptionGroup`` is | ||
| unwrapped and the first cause is re-raised directly. | ||
|
|
||
| Returns: | ||
| A ``BudgetConfig`` with DB/env overrides applied. | ||
|
|
||
| Raises: | ||
| SettingNotFoundError: If a required budget setting is | ||
| missing from the registry. | ||
| ValueError: If a resolved value cannot be parsed or if | ||
| the assembled alert thresholds violate the ordering | ||
| constraint (``warn_at < critical_at < hard_stop_at``). | ||
| """ | ||
| from pydantic import ValidationError # noqa: PLC0415 | ||
|
|
||
| from synthorg.budget.config import ( # noqa: PLC0415 | ||
| BudgetAlertConfig, | ||
| ) | ||
|
|
||
| base = self._config.budget | ||
|
|
||
| try: | ||
| async with asyncio.TaskGroup() as tg: | ||
| t_monthly = tg.create_task(self.get_float("budget", "total_monthly")) | ||
| t_per_task = tg.create_task(self.get_float("budget", "per_task_limit")) | ||
| t_daily = tg.create_task( | ||
| self.get_float("budget", "per_agent_daily_limit") | ||
| ) | ||
| t_downgrade_en = tg.create_task( | ||
| self.get_bool("budget", "auto_downgrade_enabled") | ||
| ) | ||
| t_downgrade_th = tg.create_task( | ||
| self.get_int("budget", "auto_downgrade_threshold") | ||
| ) | ||
| t_reset = tg.create_task(self.get_int("budget", "reset_day")) | ||
| t_warn = tg.create_task(self.get_int("budget", "alert_warn_at")) | ||
| t_crit = tg.create_task(self.get_int("budget", "alert_critical_at")) | ||
| t_stop = tg.create_task(self.get_int("budget", "alert_hard_stop_at")) | ||
| except ExceptionGroup as eg: | ||
| raise eg.exceptions[0] from eg | ||
|
|
||
| try: | ||
| alerts = BudgetAlertConfig( | ||
| warn_at=t_warn.result(), | ||
| critical_at=t_crit.result(), | ||
| hard_stop_at=t_stop.result(), | ||
| ) | ||
| except ValidationError as exc: | ||
| msg = f"Budget alert thresholds violate ordering constraint: {exc}" | ||
| raise ValueError(msg) from exc | ||
|
|
||
| return base.model_copy( | ||
| update={ | ||
| "total_monthly": t_monthly.result(), | ||
| "per_task_limit": t_per_task.result(), | ||
| "per_agent_daily_limit": t_daily.result(), | ||
| "reset_day": t_reset.result(), | ||
| "alerts": alerts, | ||
| "auto_downgrade": base.auto_downgrade.model_copy( | ||
| update={ | ||
| "enabled": t_downgrade_en.result(), | ||
| "threshold": t_downgrade_th.result(), | ||
| }, | ||
| ), | ||
| }, | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Split oversized resolver methods into smaller helpers.
get_budget_config and get_coordination_config exceed the max function-size guideline and are now carrying multiple responsibilities (fan-out, error mapping, model assembly).
As per coding guidelines: "Keep functions under 50 lines and files under 800 lines."
Also applies to: 337-396
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/settings/resolver.py` around lines 261 - 335, get_budget_config
is too large and mixes responsibilities (task fan-out, exception mapping, and
model assembly); split it into small helpers: extract the asyncio.TaskGroup
fan-out and awaiting of individual setting calls into a helper named
_gather_budget_settings that returns the awaited results (total_monthly,
per_task_limit, per_agent_daily_limit, auto_downgrade_enabled,
auto_downgrade_threshold, reset_day, alert_warn_at, alert_critical_at,
alert_hard_stop_at) and preserves the current ExceptionGroup-to-first-exception
unwrapping logic; extract the BudgetAlertConfig construction and
ValidationError->ValueError mapping into a helper named
_build_budget_alerts(warn, crit, stop) that raises the same ValueError on
validation failure; finally, reduce get_budget_config to calling
_gather_budget_settings, _build_budget_alerts, and a small assembly step that
returns base.model_copy(...) updating total_monthly, per_task_limit,
per_agent_daily_limit, reset_day, alerts, and auto_downgrade via
base.auto_downgrade.model_copy(...). Ensure helper names
(_gather_budget_settings, _build_budget_alerts) are used so reviewers can locate
changes and keep each function under the 50-line guideline.
| # default=None means "optional — no built-in default". Return | ||
| # empty string as a sentinel (callers like ConfigResolver.get_int | ||
| # will raise ValueError on empty, giving a clear error at the | ||
| # consumer layer rather than here). | ||
| default = definition.default if definition.default is not None else "" |
There was a problem hiding this comment.
default=None fallback to "" hides missing-default semantics.
Mapping None to empty string makes “unset” indistinguishable from an explicit empty value, especially for string consumers, and can silently bypass missing-config detection.
Suggested fix
- default = definition.default if definition.default is not None else ""
+ if definition.default is None:
+ logger.warning(SETTINGS_NOT_FOUND, namespace=ns, key=key)
+ msg = f"No fallback default configured for {ns}/{key}"
+ raise SettingNotFoundError(msg)
+ default = definition.default🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/settings/service.py` around lines 384 - 388, The current mapping
in SettingsService where you set default = definition.default if
definition.default is not None else "" masks the difference between an explicit
empty string and an unset default; change this to preserve None (i.e., leave
default as definition.default) or use a unique sentinel for “unset” so callers
like ConfigResolver.get_int can detect missing defaults and raise appropriately;
update handling in functions that rely on an empty-string sentinel (refer to the
variable named default and uses in ConfigResolver.get_int / SettingsService
methods) to accept None or the new sentinel instead of "".
- Log SETTINGS_VALIDATION_FAILED before raising TypeError in ConfigResolver constructor (CLAUDE.md: all error paths must log) - Extract _build_budget_alerts() helper from get_budget_config to keep function body under 50-line limit - Log SETTINGS_VALIDATION_FAILED with exc_info before re-raising ValueError from BudgetAlertConfig ordering constraint Skipped: service.py default=None→"" change (already tried in earlier commit, broke providers/default_provider — reverted with comment) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/settings/resolver.py`:
- Around line 303-304: The except ExceptionGroup handler that currently does
"except ExceptionGroup as eg: raise eg.exceptions[0] from eg" must log the
TaskGroup failure context at WARNING or ERROR before re-raising; modify the
handler(s) that catch ExceptionGroup (the block referencing "eg" and re-raising
eg.exceptions[0]) to call the module logger (e.g., process or module-level
logger) with a descriptive message, the ExceptionGroup object, and any available
context (task identifiers/inputs/local variables used in the composed-read
resolution) and then re-raise the inner exception as currently done; ensure both
occurrences (the handler around the composed-read methods and the second handler
at the other location) follow the same pattern.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 56c99750-7350-4e87-9f9b-9e7a1c7e87e4
📒 Files selected for processing (1)
src/synthorg/settings/resolver.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). (5)
- GitHub Check: Build Sandbox
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Test (Python 3.14)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations— Python 3.14+ has PEP 649 native lazy annotations.
Useexcept A, B:syntax (no parentheses) for exception handling — PEP 758 syntax enforced by ruff on Python 3.14.
Include type hints on all public functions and classes; mypy strict mode is enforced.
Use Google-style docstrings on all public classes and functions (enforced by ruff D rules).
Create new objects instead of mutating existing ones. For non-Pydantic internal collections, usecopy.deepcopy()at construction +MappingProxyTypewrapping. Fordict/listfields in frozen Pydantic models, rely onfrozen=Trueandcopy.deepcopy()at system boundaries.
Separate frozen Pydantic models for config/identity from mutable-via-copy models for runtime state. Never mix static config fields with mutable runtime fields in one model.
Use Pydantic v2 conventions:BaseModel,model_validator,computed_field,ConfigDict. Use@computed_fieldfor derived values instead of storing redundant fields. UseNotBlankStrfor all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators.
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls). Use structured concurrency over barecreate_task.
Enforce maximum line length of 88 characters (ruff).
Keep functions under 50 lines and files under 800 lines.
Handle errors explicitly; never silently swallow exceptions.
Validate at system boundaries (user input, external APIs, config files).
Files:
src/synthorg/settings/resolver.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Every module with business logic MUST havefrom synthorg.observability import get_loggerthenlogger = get_logger(__name__). Never useimport loggingorlogging.getLogger()orprint()in application code.
Use event name constants fromsynthorg.observability.events.<domain>modules instead of string literals. Import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Always use structured kwargs for logging:logger.info(EVENT, key=value)— neverlogger.info('msg %s', val).
All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, and entry/exit of key functions.
Observability: structured logging viaget_logger(). Correlation tracking. Log sinks. Metrics and tracing. Every module with business logic must have a logger.
Files:
src/synthorg/settings/resolver.py
+(src|tests)/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names:
example-provider,example-large-001,example-medium-001,example-small-001. Vendor names may only appear in: (1) Operations design page, (2).claude/files, (3) third-party import paths.
Files:
src/synthorg/settings/resolver.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Frozen Pydantic models for agent/company config/identity; mutable-via-copy models for runtime agent execution state (usingmodel_copy(update=...)). Never mix static config with mutable state in one model.
UseNotBlankStrfromcore.typesfor all identifier/name fields (including optionalNotBlankStr | Noneand tuple variants) instead of manual whitespace validators.
Files:
src/synthorg/settings/resolver.py
src/synthorg/settings/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Settings: runtime-editable via DB > env > YAML > code defaults. 8 namespaces: company, providers, memory, budget, security, coordination, observability, backup. Fernet encryption for sensitive values.
Files:
src/synthorg/settings/resolver.py
🧠 Learnings (6)
📚 Learning: 2026-03-17T06:37:14.526Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:37:14.526Z
Learning: Applies to src/synthorg/settings/**/*.py : Settings: runtime-editable via DB > env > YAML > code defaults. 8 namespaces: company, providers, memory, budget, security, coordination, observability, backup. Fernet encryption for sensitive values.
Applied to files:
src/synthorg/settings/resolver.py
📚 Learning: 2026-03-17T06:37:14.526Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:37:14.526Z
Learning: Applies to src/synthorg/persistence/**/*.py : Persistence: pluggable `PersistenceBackend` protocol. `SettingsRepository` handles namespaced settings CRUD. All operational data persistence through this layer.
Applied to files:
src/synthorg/settings/resolver.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/settings/resolver.py
📚 Learning: 2026-03-17T06:37:14.526Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:37:14.526Z
Learning: Applies to src/synthorg/budget/**/*.py : Budget: cost tracking, budget enforcement (pre-flight/in-flight checks, auto-downgrade), billing periods, cost tiers, quota/subscription tracking. CFO: cost optimization via anomaly detection, efficiency analysis, downgrade recommendations. Budget errors: `BudgetExhaustedError`, `DailyLimitExceededError`, `QuotaExhaustedError`.
Applied to files:
src/synthorg/settings/resolver.py
📚 Learning: 2026-03-17T06:37:14.526Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:37:14.526Z
Learning: Applies to **/*.py : Keep functions under 50 lines and files under 800 lines.
Applied to files:
src/synthorg/settings/resolver.py
📚 Learning: 2026-03-17T06:37:14.526Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:37:14.526Z
Learning: Applies to src/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, and entry/exit of key functions.
Applied to files:
src/synthorg/settings/resolver.py
🧬 Code graph analysis (1)
src/synthorg/settings/resolver.py (2)
src/synthorg/settings/errors.py (1)
SettingNotFoundError(8-9)src/synthorg/settings/service.py (1)
get(189-273)
🔇 Additional comments (1)
src/synthorg/settings/resolver.py (1)
69-245: Good consistency on scalar accessor validation/logging.Typed parsing, explicit error mapping, and structured warning logs are clean and consistent across all scalar getters.
Both except ExceptionGroup handlers in get_budget_config and get_coordination_config now log SETTINGS_FETCH_FAILED at WARNING with namespace, error count, and exc_info before re-raising the first inner exception. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/synthorg/settings/resolver.py (2)
331-397: 🧹 Nitpick | 🔵 TrivialFunction exceeds 50-line guideline.
Same as
get_budget_config- consider extracting the TaskGroup fan-out into a helper like_gather_coordination_settings().As per coding guidelines: "Keep functions under 50 lines."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/settings/resolver.py` around lines 331 - 397, The get_coordination_config function is over the 50-line limit; move the asyncio.TaskGroup fan-out into a small async helper (e.g. _gather_coordination_settings) that performs the four self.get_* calls and returns either the Task objects or, better, their results (max_concurrency_per_wave, fail_fast, enable_workspace_isolation, base_branch); then have get_coordination_config call this helper inside the same try/except that catches ExceptionGroup and re-raises the first cause, and apply the request-level overrides to the returned values when constructing CoordinationConfig. Ensure the helper is named uniquely (e.g. _gather_coordination_settings) and keep the ExceptionGroup logging/raise behavior in get_coordination_config.
263-329: 🧹 Nitpick | 🔵 TrivialFunction exceeds 50-line guideline.
This method spans 67 lines. Consider extracting the TaskGroup fan-out into a helper like
_gather_budget_settings()and the alert construction into_build_budget_alerts()(already done) to improve readability and testability.As per coding guidelines: "Keep functions under 50 lines."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/settings/resolver.py` around lines 263 - 329, get_budget_config is too long because the asyncio.TaskGroup fan-out is inlined; extract that block into a new helper named _gather_budget_settings() which runs the TaskGroup, creates the same tasks (t_monthly, t_per_task, t_daily, t_downgrade_en, t_downgrade_th, t_reset, t_warn, t_crit, t_stop), handles ExceptionGroup the same way (logging and re-raising the first cause), and returns the created task objects or their results; then shorten get_budget_config to call _gather_budget_settings(), use its returned task results as inputs to the existing _build_budget_alerts(...) and the model_copy update, leaving alert construction in _build_budget_alerts as-is.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/synthorg/settings/resolver.py`:
- Around line 331-397: The get_coordination_config function is over the 50-line
limit; move the asyncio.TaskGroup fan-out into a small async helper (e.g.
_gather_coordination_settings) that performs the four self.get_* calls and
returns either the Task objects or, better, their results
(max_concurrency_per_wave, fail_fast, enable_workspace_isolation, base_branch);
then have get_coordination_config call this helper inside the same try/except
that catches ExceptionGroup and re-raises the first cause, and apply the
request-level overrides to the returned values when constructing
CoordinationConfig. Ensure the helper is named uniquely (e.g.
_gather_coordination_settings) and keep the ExceptionGroup logging/raise
behavior in get_coordination_config.
- Around line 263-329: get_budget_config is too long because the
asyncio.TaskGroup fan-out is inlined; extract that block into a new helper named
_gather_budget_settings() which runs the TaskGroup, creates the same tasks
(t_monthly, t_per_task, t_daily, t_downgrade_en, t_downgrade_th, t_reset,
t_warn, t_crit, t_stop), handles ExceptionGroup the same way (logging and
re-raising the first cause), and returns the created task objects or their
results; then shorten get_budget_config to call _gather_budget_settings(), use
its returned task results as inputs to the existing _build_budget_alerts(...)
and the model_copy update, leaving alert construction in _build_budget_alerts
as-is.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ab5e3f24-eb1c-4679-9bb2-e7bfc4458695
📒 Files selected for processing (1)
src/synthorg/settings/resolver.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). (5)
- GitHub Check: Build Backend
- GitHub Check: Build Web
- GitHub Check: Build Sandbox
- GitHub Check: Test (Python 3.14)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations— Python 3.14+ has PEP 649 native lazy annotations.
Useexcept A, B:syntax (no parentheses) for exception handling — PEP 758 syntax enforced by ruff on Python 3.14.
Include type hints on all public functions and classes; mypy strict mode is enforced.
Use Google-style docstrings on all public classes and functions (enforced by ruff D rules).
Create new objects instead of mutating existing ones. For non-Pydantic internal collections, usecopy.deepcopy()at construction +MappingProxyTypewrapping. Fordict/listfields in frozen Pydantic models, rely onfrozen=Trueandcopy.deepcopy()at system boundaries.
Separate frozen Pydantic models for config/identity from mutable-via-copy models for runtime state. Never mix static config fields with mutable runtime fields in one model.
Use Pydantic v2 conventions:BaseModel,model_validator,computed_field,ConfigDict. Use@computed_fieldfor derived values instead of storing redundant fields. UseNotBlankStrfor all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators.
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls). Use structured concurrency over barecreate_task.
Enforce maximum line length of 88 characters (ruff).
Keep functions under 50 lines and files under 800 lines.
Handle errors explicitly; never silently swallow exceptions.
Validate at system boundaries (user input, external APIs, config files).
Files:
src/synthorg/settings/resolver.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Every module with business logic MUST havefrom synthorg.observability import get_loggerthenlogger = get_logger(__name__). Never useimport loggingorlogging.getLogger()orprint()in application code.
Use event name constants fromsynthorg.observability.events.<domain>modules instead of string literals. Import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Always use structured kwargs for logging:logger.info(EVENT, key=value)— neverlogger.info('msg %s', val).
All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, and entry/exit of key functions.
Observability: structured logging viaget_logger(). Correlation tracking. Log sinks. Metrics and tracing. Every module with business logic must have a logger.
Files:
src/synthorg/settings/resolver.py
+(src|tests)/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names:
example-provider,example-large-001,example-medium-001,example-small-001. Vendor names may only appear in: (1) Operations design page, (2).claude/files, (3) third-party import paths.
Files:
src/synthorg/settings/resolver.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Frozen Pydantic models for agent/company config/identity; mutable-via-copy models for runtime agent execution state (usingmodel_copy(update=...)). Never mix static config with mutable state in one model.
UseNotBlankStrfromcore.typesfor all identifier/name fields (including optionalNotBlankStr | Noneand tuple variants) instead of manual whitespace validators.
Files:
src/synthorg/settings/resolver.py
src/synthorg/settings/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Settings: runtime-editable via DB > env > YAML > code defaults. 8 namespaces: company, providers, memory, budget, security, coordination, observability, backup. Fernet encryption for sensitive values.
Files:
src/synthorg/settings/resolver.py
🧠 Learnings (5)
📚 Learning: 2026-03-17T06:37:14.526Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:37:14.526Z
Learning: Applies to src/synthorg/settings/**/*.py : Settings: runtime-editable via DB > env > YAML > code defaults. 8 namespaces: company, providers, memory, budget, security, coordination, observability, backup. Fernet encryption for sensitive values.
Applied to files:
src/synthorg/settings/resolver.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/settings/resolver.py
📚 Learning: 2026-03-17T06:37:14.526Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:37:14.526Z
Learning: Applies to src/synthorg/budget/**/*.py : Budget: cost tracking, budget enforcement (pre-flight/in-flight checks, auto-downgrade), billing periods, cost tiers, quota/subscription tracking. CFO: cost optimization via anomaly detection, efficiency analysis, downgrade recommendations. Budget errors: `BudgetExhaustedError`, `DailyLimitExceededError`, `QuotaExhaustedError`.
Applied to files:
src/synthorg/settings/resolver.py
📚 Learning: 2026-03-17T06:37:14.526Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:37:14.526Z
Learning: Applies to **/*.py : Keep functions under 50 lines and files under 800 lines.
Applied to files:
src/synthorg/settings/resolver.py
📚 Learning: 2026-03-17T06:37:14.526Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:37:14.526Z
Learning: Applies to src/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, and entry/exit of key functions.
Applied to files:
src/synthorg/settings/resolver.py
🧬 Code graph analysis (1)
src/synthorg/settings/resolver.py (2)
src/synthorg/settings/errors.py (1)
SettingNotFoundError(8-9)src/synthorg/settings/service.py (2)
SettingsService(152-637)get(189-273)
🔇 Additional comments (5)
src/synthorg/settings/resolver.py (5)
1-29: LGTM!Module structure is well-organized: clean docstring, proper logger initialization via
get_logger(__name__), and correct use ofTYPE_CHECKINGfor type-only imports to avoid circular dependencies.
32-68: LGTM!Good defensive programming with runtime validation for
settings_serviceand proper error logging before raisingTypeError. The keyword-only parameters in the constructor provide a clean API.
70-245: LGTM!Scalar accessors follow a consistent, well-structured pattern:
- Catch and log
SettingNotFoundErrorbefore propagation- Attempt type coercion with dedicated logging on parse failures
- Clean error messages with
from Nonewhile preserving full context viaexc_info=TrueThe generic syntax on
get_enum[E: StrEnum]is valid Python 3.12+ (PEP 695).
247-261: LGTM!Clean delegation to
get_enumwith appropriate local import to avoid circular dependencies.
400-458: LGTM!Clean helper functions with proper error handling:
_build_budget_alertslogs validation failures before re-raising with full context_parse_boolusesfrozensetfor efficient O(1) lookup with case-insensitive matchingThe strict boolean set (
true/false/1/0) is reasonable for config values where explicitness is preferred.
🤖 I have created a release *beep* *boop* --- ## [0.3.1](v0.3.0...v0.3.1) (2026-03-17) ### Features * **api:** RFC 9457 Phase 2 — ProblemDetail and content negotiation ([#496](#496)) ([30f7c49](30f7c49)) * **cli:** verify container image signatures and SLSA provenance on pull ([#492](#492)) ([bef272d](bef272d)), closes [#491](#491) * **engine:** implement context budget management in execution loops ([#520](#520)) ([181eb8a](181eb8a)), closes [#416](#416) * implement settings persistence layer (DB-backed config) ([#495](#495)) ([4bd99f7](4bd99f7)), closes [#450](#450) * **memory:** implement dual-mode archival in memory consolidation ([#524](#524)) ([4603c9e](4603c9e)), closes [#418](#418) * migrate config consumers to read through SettingsService ([#510](#510)) ([32f553d](32f553d)) * **settings:** implement settings change subscriptions for service hot-reload ([#526](#526)) ([53f908e](53f908e)), closes [#503](#503) * **settings:** register API config in SettingsService with 2-phase init ([#518](#518)) ([29f7481](29f7481)) * **tools:** add SSRF prevention for git clone URLs ([#505](#505)) ([492dd0d](492dd0d)) * **tools:** wire RootConfig.git_clone to GitCloneTool instantiation ([#519](#519)) ([b7d8172](b7d8172)) ### Bug Fixes * **api:** replace JWT query parameter with one-time ticket for WebSocket auth ([#493](#493)) ([22a25f6](22a25f6)), closes [#343](#343) ### Documentation * add uv cache lock contention handling to worktree skill ([#500](#500)) ([bd85a8d](bd85a8d)) * document RFC 9457 dual response formats in OpenAPI schema ([#506](#506)) ([8dd2524](8dd2524)) ### Maintenance * upgrade jsdom from 28 to 29 ([#499](#499)) ([1ea2249](1ea2249)) --- 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: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary
ConfigResolverbridge class (settings/resolver.py) with typed scalar accessors (get_str,get_int,get_float,get_bool,get_enum) and composed-read methods (get_budget_config,get_coordination_config,get_autonomy_level)RootConfigattribute access toConfigResolver:config.config.autonomy.level→config_resolver.get_autonomy_level()config.budget→config_resolver.get_budget_config()(composed read)config.company_name→config_resolver.get_str("company", "company_name")config.coordination.to_coordination_config()→config_resolver.get_coordination_config()(composed read)autonomy_level: wrongenum_values(full_autonomy/approval_required/human_in_the_loop→full/semi/supervised/locked), wrongyaml_path(config.autonomy_level→config.autonomy.level), wrongdefault(supervised→semi)default_topology: wrongyaml_path(coordination.default_topology→coordination.topology)max_wave_size: wrongyaml_path(coordination.max_wave_size→coordination.max_concurrency_per_wave)fail_fast,enable_workspace_isolation,base_branch) + 4 budget (reset_day,alert_warn_at,alert_critical_at,alert_hard_stop_at)asyncio.TaskGroupfor parallel setting reads in composed methods (per CLAUDE.md async conventions)ConfigResolverinAppState(singleton, not per-access)ValueErrorin scalar accessor parse failuresConfigResolverFollow-up issues created
Test plan
ConfigResolver(scalar accessors, composed reads, error propagation, Hypothesis roundtrips)test_coordination_wiringupdated and passingRefs #497