feat: implement YAML config loader with Pydantic validation#75
feat: implement YAML config loader with Pydantic validation#75
Conversation
Add the config/ module: YAML config loading, layered merging, Pydantic validation with rich error messages including YAML line/column locations. New source files: - config/errors.py — ConfigError hierarchy with ConfigLocation context - config/defaults.py — built-in default config dict - config/schema.py — RootConfig, AgentConfig, ProviderConfig, ProviderModelConfig, RoutingConfig, RoutingRuleConfig - config/loader.py — load_config(), load_config_from_string(), recursive deep merge, YAML AST line mapping Dependencies: - pyyaml==6.0.2 (runtime) - types-pyyaml==6.0.12.20250915 (dev, mypy stubs) 92 new tests, 890 total passing, 96.87% coverage.
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a YAML-based configuration system: defaults, Pydantic schema models, a loader with deep-merge and YAML parsing (including YAML AST line mapping), structured config errors, public re-exports, and comprehensive unit tests. PyYAML and types-pyyaml were added to dependencies. Changes
Sequence DiagramsequenceDiagram
actor User as User/Application
participant Loader as load_config()
participant Parser as YAML Parser
participant Merger as Deep Merger
participant Validator as Pydantic Validator
User->>Loader: load_config(path, overrides)
Loader->>Parser: read & parse primary YAML (build line map)
Parser-->>Loader: dict or ConfigParseError (with location)
Loader->>Merger: merge(defaults, primary, overrides)
Merger-->>Loader: merged dict
Loader->>Validator: validate merged dict against RootConfig
Validator-->>Loader: valid RootConfig or validation errors
alt Validation passes
Loader-->>User: return frozen RootConfig
else Validation fails
Loader-->>User: raise ConfigValidationError (with field_errors & locations)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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 introduces a comprehensive and robust YAML-based configuration system, leveraging Pydantic for strict schema validation. This new system allows for layered configuration loading, merging default values with user-provided settings and overrides, while providing detailed error reporting with source location context. This foundational work is crucial for unblocking future development, particularly for template-related features. 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.
Pull request overview
Adds a new ai_company.config module that loads YAML configuration, deep-merges layered inputs with built-in defaults, validates via Pydantic models, and surfaces richer config errors.
Changes:
- Introduces
RootConfig+ config-level schema models with cross-field validation (providers/routing, uniqueness). - Implements YAML parsing + layered deep-merge loader with error enrichment (key-path + line/column where available).
- Adds unit tests for schema/defaults/loader/error formatting and wires in PyYAML (+ stubs) dependencies.
Reviewed changes
Copilot reviewed 12 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
pyproject.toml |
Adds pyyaml runtime dependency and types-pyyaml dev stubs. |
uv.lock |
Locks new pyyaml and types-pyyaml dependencies. |
src/ai_company/config/__init__.py |
Exposes the public config loading/validation API. |
src/ai_company/config/defaults.py |
Provides base-layer default config dict for merging. |
src/ai_company/config/errors.py |
Adds ConfigError hierarchy + location/context formatting. |
src/ai_company/config/loader.py |
Implements YAML parsing, deep-merge layering, line-map extraction, and Pydantic validation wrapping. |
src/ai_company/config/schema.py |
Defines RootConfig and related config models + validators. |
src/ai_company/observability/config.py |
Removes TODO related to config integration. |
tests/unit/config/conftest.py |
Adds factories + YAML fixtures for config tests. |
tests/unit/config/test_defaults.py |
Tests default dict validity and immutability of returned defaults. |
tests/unit/config/test_errors.py |
Tests error types and __str__ formatting. |
tests/unit/config/test_loader.py |
Tests deep-merge behavior, parsing failures, and load/validate flows. |
tests/unit/config/test_schema.py |
Tests schema defaults, validation rules, and frozen behavior. |
Comments suppressed due to low confidence (6)
src/ai_company/config/schema.py:233
- These fields are mutable dicts inside a
frozen=TruePydantic model, so callers can still mutate config after validation viacfg.personality[...] = ..., undermining the immutability guarantees used elsewhere (e.g., tuple-based collections insrc/ai_company/core/agent.py:32-84). Consider converting these dicts to an immutable structure (or clarifying in docs/tests that deep immutability isn’t provided).
personality: dict[str, Any] = Field(
default_factory=dict,
description="Raw personality config",
)
model: dict[str, Any] = Field(
src/ai_company/config/schema.py:317
RootConfigisfrozen=True, butprovidersis a mutabledict, so config can still be mutated post-load viacfg.providers[...] = .... If deep immutability is desired (as in other models), consider using an immutable mapping representation or wrapping withMappingProxyTypeduring validation.
providers: dict[str, ProviderConfig] = Field(
default_factory=dict,
description="LLM provider configurations",
)
src/ai_company/config/loader.py:67
_parse_yaml_file()only checksexists(). Iffile_pathis a directory (or unreadable), subsequentread_text()will raise anOSError(e.g.,IsADirectoryError) that bypasses theConfig*Errorhierarchy. Consider validatingfile_path.is_file()and catchingOSErroraround reading to re-raise asConfigFileNotFoundError/ConfigParseErrorwith aConfigLocation.
if not file_path.exists():
msg = f"Configuration file not found: {file_path}"
raise ConfigFileNotFoundError(
msg,
locations=(ConfigLocation(file_path=str(file_path)),),
src/ai_company/config/loader.py:270
load_config()builds the YAMLline_maponly from the primary config file. If a validation error is introduced by an override layer, the reportedfile_path/line/column will be missing or misleading because the map doesn’t include override content. Consider tracking per-layer line maps and attributing each merged key to the layer that last set it (or omittingfile_pathwhen the key isn’t in the map).
# 4. Build line map from primary file for error reporting
yaml_text = config_path.read_text(encoding="utf-8")
line_map = _build_line_map(yaml_text)
# 5. Validate
src/ai_company/config/loader.py:236
- Issue #59 acceptance criteria mentions environment variable substitution (
${ENV_VAR}) and config file discovery, but this loader API only supports explicit paths/strings and does no${...}expansion. If this PR is intended to close #59, those criteria are still unimplemented; otherwise, the PR description/issue closure should be adjusted to match delivered scope.
"""Load and validate company configuration from YAML file(s).
Loading order (each layer deep-merges onto the previous):
1. Built-in defaults (from :func:`default_config_dict`).
src/ai_company/config/errors.py:52
ConfigLocation.columnis captured (e.g., by the loader), but the formatted location string only includesline. Consider including column output (e.g.,line X, col Y) so callers can actually use the more precise location information.
if loc.key_path:
loc_parts.append(f" {loc.key_path}")
if loc.file_path:
line_info = f" at line {loc.line}" if loc.line is not None else ""
loc_parts.append(f" in {loc.file_path}{line_info}")
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Code Review
This pull request introduces a robust YAML configuration loading system with Pydantic validation, which is a significant feature. The implementation is well-structured, with clear separation of concerns for schema definition, loading logic, error handling, and defaults. The custom error classes with detailed source location reporting are particularly well done. The accompanying test suite is comprehensive and covers a wide range of scenarios.
I have two suggestions for improvement, both with medium severity, aimed at improving maintainability and the accuracy of error reporting. One suggestion is to reduce boilerplate validation code in the schema definitions by using a custom Pydantic type. The other is to acknowledge and document a limitation in error reporting when using override configuration files.
src/ai_company/config/schema.py
Outdated
| @model_validator(mode="after") | ||
| def _validate_non_blank_strings(self) -> Self: | ||
| """Ensure identifier fields are not whitespace-only.""" | ||
| if not self.id.strip(): | ||
| msg = "id must not be whitespace-only" | ||
| raise ValueError(msg) | ||
| if self.alias is not None and not self.alias.strip(): | ||
| msg = "alias must not be whitespace-only" | ||
| raise ValueError(msg) | ||
| return self |
There was a problem hiding this comment.
There's a recurring pattern of validators checking for whitespace-only strings across multiple models in this file (e.g., _validate_non_blank_strings, _validate_non_blank_identifiers). This leads to boilerplate code. To improve maintainability and reduce repetition, consider defining a reusable custom type, for example NotBlankStr, using typing.Annotated and Pydantic's AfterValidator. This would centralize the validation logic and make the models cleaner.
For example:
from typing import Annotated
from pydantic import AfterValidator, Field
def check_not_whitespace(v: str) -> str:
if not v.strip():
raise ValueError("String must not be whitespace-only")
return v
NotBlankStr = Annotated[str, AfterValidator(check_not_whitespace)]
class ProviderModelConfig(BaseModel):
model_config = ConfigDict(frozen=True)
id: NotBlankStr = Field(description="Model identifier")
alias: NotBlankStr | None = Field(default=None, ...)
# ... no more @model_validator for thisThis would make the models cleaner and the validation logic more DRY (Don't Repeat Yourself).
| def load_config( | ||
| config_path: Path | str, | ||
| *, | ||
| override_paths: tuple[Path | str, ...] = (), | ||
| ) -> RootConfig: |
There was a problem hiding this comment.
The current implementation builds a line map for error reporting using only the primary config_path. If a validation error is caused by a value from an override_paths file, the reported line number may be incorrect or missing. For instance, it might point to the key in the primary config file even if the invalid value came from an override. This could be confusing for users trying to debug their configuration.
While a full solution to track the origin of every merged value is complex, it would be valuable to at least document this limitation in the load_config docstring to clarify the behavior of error reporting when using overrides.
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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/ai_company/config/errors.py`:
- Around line 51-53: The formatted error location currently only includes
loc.line; update the rendering to include loc.column as well by building a
column-aware location string (e.g., "at line {loc.line}, column {loc.column}"
when present) and use that string when appending to loc_parts and parts;
specifically modify the logic around the variables line_info/loc_parts where loc
is used so it checks loc.column (like ConfigLocation.column) and includes it in
the appended message in both places where locations are rendered.
In `@src/ai_company/config/loader.py`:
- Around line 227-231: Make load_config support standard-location discovery by
allowing config_path to be optional (change signature of load_config to accept
None for config_path) and, when None or missing, search well-known locations in
order (check current working directory first, then the user's home ai-company
config directory per acceptance criteria, and any other project-standard
locations you have) before failing; keep honoring the override_paths parameter
and return RootConfig if a file is found, otherwise raise a clear error. Ensure
you update load_config's parameter typing (allow Optional[Path|str]), implement
the ordered lookup logic inside load_config, and reuse existing
parsing/validation code so behavior is identical once a path is discovered.
- Around line 255-275: The merged config dict is being validated while still
containing literal "${ENV_VAR}" placeholders; before calling
_validate_config_dict you must run an environment-variable interpolation pass
(e.g., call a helper like _interpolate_env_vars(merged) or implement one) to
replace ${VAR} entries with os.environ values (preserving types where
appropriate), then pass the interpolated dict to _validate_config_dict (use the
same change at the other spot noted around the later return). Locate the
merge/validate sequence that uses default_config_dict(), _parse_yaml_file(),
_deep_merge(), _build_line_map(), and _validate_config_dict() and insert the
interpolation step on merged right before _validate_config_dict in both places.
- Around line 266-275: The validator currently builds line_map from only the
primary file (yaml_text/_build_line_map) and passes a single source_file
(config_path) into _validate_config_dict, so errors from override files point to
wrong locations; fix by constructing a combined line map and source mapping that
includes the primary file and each override file: read each override's text,
call _build_line_map for each (or extend _build_line_map to accept multiple
texts), merge those line maps into a single composite line_map keyed by file and
line ranges, and pass that composite line_map (and either a composite
source_file identifier or per-error file references) into _validate_config_dict
when validating merged, so validation messages reference the correct file/line
for values introduced or replaced by overrides (update usages of yaml_text,
_build_line_map, merged, config_path, and the _validate_config_dict call
accordingly).
- Around line 63-70: Replace the naive exists()-then-read sequence with robust
filesystem error handling: keep the ConfigFileNotFoundError for the non-existent
path case (using ConfigLocation(file_path=str(file_path))), but wrap the
read_text(encoding="utf-8") call in a try/except that catches OSError/IOError
(including IsADirectoryError, PermissionError) and re-raises a typed config
error from your hierarchy (e.g., ConfigFileReadError or similar) while
preserving the original exception as the cause and including the same
ConfigLocation; only call _parse_yaml_string(text, str(file_path)) after a
successful read.
In `@tests/unit/config/conftest.py`:
- Around line 97-103: The two test constants MISSING_REQUIRED_YAML and
INVALID_FIELD_VALUES_YAML are identical; change them so they exercise distinct
validation cases: make MISSING_REQUIRED_YAML actually omit the required key
(e.g., remove company_name entirely) and make INVALID_FIELD_VALUES_YAML provide
an invalid value for the key (e.g., company_name with the wrong type or clearly
invalid value), then update any tests referencing these constants if needed to
match the intended scenario; locate the constants by name in conftest.py to
apply the changes.
- Around line 81-89: The YAML fixture uses a real provider model id
"claude-sonnet-4-6"; update the test fixture under the providers -> anthropic ->
models block to use a fake model id (for example "test-model:1" or
"fake-writer:latest") while keeping the alias "sonnet" and the
routing.fallback_chain unchanged; locate the models list entry by the keys
providers, anthropic, models and replace the id value to a clearly
fake/test-only model string so tests no longer reference a real model from
RECOMMENDED_MODELS.
- Around line 114-121: Replace the use of Any in the tmp_config_file fixture
with precise types: annotate the tmp_path parameter as pathlib.Path (import
Path), type the inner factory _create as taking content: str and name: str and
returning Path, and type tmp_config_file to return Callable[[str, str], Path]
(import Callable from typing) so tmp_config_file, _create, and tmp_path have
concrete Path/Callable types instead of Any.
In `@tests/unit/config/test_loader.py`:
- Around line 141-193: Add two tests around environment-variable substitution
and two tests for standard-location discovery: (1) create a config containing
"company_name: ${TEST_COMPANY}" using the tmp_config_file fixture, set
os.environ["TEST_COMPANY"] to a value, call load_config(path) and assert
cfg.company_name == that value; (2) ensure the env var is unset (or del from
os.environ), call load_config(path) and assert it raises ConfigValidationError
(or the loader's validation exception) for the missing substitution; (3) for
path discovery create a config file at the standard lookup location by
monkeypatching env vars (e.g., HOME or XDG_CONFIG_HOME) or the lookup helper so
load_config() with no path finds and returns the file and assert loaded values;
(4) ensure when no standard-location file exists calling load_config() with no
args raises ConfigFileNotFoundError; reference load_config,
ConfigValidationError, ConfigFileNotFoundError, tmp_config_file, tmp_path and
use monkeypatch/os.environ to control discovery and substitution.
In `@tests/unit/config/test_schema.py`:
- Around line 31-45: Replace real-model IDs used in the schema unit tests with
synthetic test IDs: update all occurrences of "claude-sonnet-4-6" in tests that
construct ProviderModelConfig (e.g., inside test_valid_full and the earlier
minimal test that creates m = ProviderModelConfig(...)) to a fake ID such as
"test-model:8b" (or "fake-writer:latest"); ensure you change the other reported
occurrences around the later test (the ones mentioned near lines 208-209) as
well so no test references real RECOMMENDED_MODELS values. Keep the same fields
and assertions; only substitute the id strings used for testing.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
pyproject.tomlsrc/ai_company/config/__init__.pysrc/ai_company/config/defaults.pysrc/ai_company/config/errors.pysrc/ai_company/config/loader.pysrc/ai_company/config/schema.pysrc/ai_company/observability/config.pytests/unit/config/__init__.pytests/unit/config/conftest.pytests/unit/config/test_defaults.pytests/unit/config/test_errors.pytests/unit/config/test_loader.pytests/unit/config/test_schema.py
💤 Files with no reviewable changes (1)
- src/ai_company/observability/config.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). (1)
- GitHub Check: Agent
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Use PEP 649 native lazy annotations instead offrom __future__ import annotations(Python 3.14+)
All public functions and classes must include type hints; enforce via mypy strict mode
Use Google-style docstrings on all public classes and functions (enforced by ruff D rules)
Keep functions under 50 lines and files under 800 lines
Create new objects instead of mutating existing ones; enforce immutability patterns
Use Pydantic v2 models (BaseModel, model_validator, ConfigDict) for data validation
Validate at system boundaries (user input, external APIs, config files); never silently swallow errors
Enforce 88-character line length (via ruff configuration)
Files:
src/ai_company/config/errors.pysrc/ai_company/config/defaults.pysrc/ai_company/config/loader.pysrc/ai_company/config/schema.pytests/unit/config/test_loader.pysrc/ai_company/config/__init__.pytests/unit/config/test_schema.pytests/unit/config/test_errors.pytests/unit/config/test_defaults.pytests/unit/config/conftest.py
{src/**/*.py,tests/**/*.py}
📄 CodeRabbit inference engine (CLAUDE.md)
Run ruff check and ruff format as pre-commit hooks on all Python files; enforce no commits to main branch
Files:
src/ai_company/config/errors.pysrc/ai_company/config/defaults.pysrc/ai_company/config/loader.pysrc/ai_company/config/schema.pytests/unit/config/test_loader.pysrc/ai_company/config/__init__.pytests/unit/config/test_schema.pytests/unit/config/test_errors.pytests/unit/config/test_defaults.pytests/unit/config/conftest.py
pyproject.toml
📄 CodeRabbit inference engine (CLAUDE.md)
Pin all dependency versions using '==' in pyproject.toml; organize into 'test' and 'dev' dependency groups
Files:
pyproject.toml
**/*.{yaml,yml,json,toml}
📄 CodeRabbit inference engine (CLAUDE.md)
Use check-yaml, check-json, and check-toml pre-commit hooks to validate configuration files
Files:
pyproject.toml
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Mark tests with@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, or@pytest.mark.slow
Maintain minimum 80% code coverage, enforced in CI via pytest-cov
Set test timeout to 30 seconds per test; use asyncio_mode = 'auto' for async tests without manual@pytest.mark.asyncio
Files:
tests/unit/config/test_loader.pytests/unit/config/test_schema.pytests/unit/config/test_errors.pytests/unit/config/test_defaults.pytests/unit/config/conftest.py
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-28T13:41:23.367Z
Learning: Applies to **/*.py : Use Pydantic v2 models (BaseModel, model_validator, ConfigDict) for data validation
📚 Learning: 2026-01-26T08:59:32.818Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-26T08:59:32.818Z
Learning: Applies to pyproject.toml : Minimize external dependencies. When adding new dependencies, add them to `pyproject.toml` with exact version pins.
Applied to files:
pyproject.toml
📚 Learning: 2026-02-28T13:41:23.367Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-28T13:41:23.367Z
Learning: Applies to pyproject.toml : Pin all dependency versions using '==' in pyproject.toml; organize into 'test' and 'dev' dependency groups
Applied to files:
pyproject.toml
📚 Learning: 2026-02-28T13:41:23.367Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-28T13:41:23.367Z
Learning: Applies to {src/**/*.py,tests/**/*.py} : Run ruff check and ruff format as pre-commit hooks on all Python files; enforce no commits to main branch
Applied to files:
pyproject.toml
📚 Learning: 2026-01-24T16:33:29.354Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-24T16:33:29.354Z
Learning: Before each commit, run `ruff format .` to format code, `ruff check .` to lint code (use `ruff check --fix .` to auto-fix), and `pytest` to run tests
Applied to files:
pyproject.toml
📚 Learning: 2026-01-26T08:59:32.818Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-26T08:59:32.818Z
Learning: Applies to tests/unit/test_*.py : Write unit tests for new functionality using pytest. Place test files in `tests/unit/` with `test_*.py` naming convention.
Applied to files:
tests/unit/config/test_loader.py
📚 Learning: 2026-01-24T09:54:45.426Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/instructions/agents.instructions.md:0-0
Timestamp: 2026-01-24T09:54:45.426Z
Learning: Applies to agents/test*.py : Agent tests should cover: successful generation with valid output, handling malformed LLM responses, error conditions (network errors, timeouts), output format validation, and integration with story state
Applied to files:
tests/unit/config/test_schema.py
📚 Learning: 2026-02-26T17:43:50.902Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-26T17:43:50.902Z
Learning: Applies to tests/**/*.py : Tests must use fake model names (e.g., `test-model:8b`, `fake-writer:latest`)—never use real model IDs from `RECOMMENDED_MODELS`.
Applied to files:
tests/unit/config/test_schema.pytests/unit/config/conftest.py
📚 Learning: 2026-01-26T08:59:32.818Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-26T08:59:32.818Z
Learning: Applies to **/*.py : All new code must have corresponding unit tests. When modifying existing code, update related tests. Tests should cover both happy paths and edge cases.
Applied to files:
tests/unit/config/test_errors.py
📚 Learning: 2026-01-24T09:54:56.100Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/instructions/test-files.instructions.md:0-0
Timestamp: 2026-01-24T09:54:56.100Z
Learning: Applies to **/tests/conftest.py : Place shared pytest fixtures in `tests/conftest.py`
Applied to files:
tests/unit/config/conftest.py
📚 Learning: 2026-01-24T09:54:56.100Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/instructions/test-files.instructions.md:0-0
Timestamp: 2026-01-24T09:54:56.100Z
Learning: Each test should be independent and not rely on other tests; use pytest fixtures for test setup (shared fixtures in `tests/conftest.py`); clean up resources in teardown/fixtures
Applied to files:
tests/unit/config/conftest.py
📚 Learning: 2026-01-24T09:54:56.100Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/instructions/test-files.instructions.md:0-0
Timestamp: 2026-01-24T09:54:56.100Z
Learning: Applies to **/test_*.py : Use appropriate fixture scopes (`function`, `class`, `module`, `session`) and document complex fixtures with docstrings
Applied to files:
tests/unit/config/conftest.py
🧬 Code graph analysis (8)
src/ai_company/config/loader.py (3)
src/ai_company/config/defaults.py (1)
default_config_dict(6-27)src/ai_company/config/errors.py (4)
ConfigFileNotFoundError(57-58)ConfigLocation(7-21)ConfigParseError(61-62)ConfigValidationError(65-96)src/ai_company/config/schema.py (1)
RootConfig(260-380)
src/ai_company/config/schema.py (5)
src/ai_company/budget/config.py (1)
BudgetConfig(130-195)src/ai_company/communication/config.py (1)
CommunicationConfig(249-283)src/ai_company/core/company.py (2)
CompanyConfig(101-144)Department(52-98)src/ai_company/core/enums.py (1)
CompanyType(72-82)src/ai_company/observability/config.py (1)
LogConfig(115-191)
tests/unit/config/test_loader.py (3)
src/ai_company/config/errors.py (3)
ConfigFileNotFoundError(57-58)ConfigParseError(61-62)ConfigValidationError(65-96)src/ai_company/config/schema.py (1)
RootConfig(260-380)tests/unit/config/conftest.py (1)
tmp_config_file(115-121)
src/ai_company/config/__init__.py (4)
src/ai_company/config/defaults.py (1)
default_config_dict(6-27)src/ai_company/config/errors.py (4)
ConfigError(24-54)ConfigFileNotFoundError(57-58)ConfigParseError(61-62)ConfigValidationError(65-96)src/ai_company/config/loader.py (2)
load_config(227-275)load_config_from_string(278-306)src/ai_company/config/schema.py (6)
AgentConfig(199-257)ProviderConfig(63-112)ProviderModelConfig(16-60)RootConfig(260-380)RoutingConfig(161-196)RoutingRuleConfig(115-158)
tests/unit/config/test_schema.py (1)
src/ai_company/config/schema.py (6)
AgentConfig(199-257)ProviderConfig(63-112)ProviderModelConfig(16-60)RootConfig(260-380)RoutingConfig(161-196)RoutingRuleConfig(115-158)
tests/unit/config/test_errors.py (1)
src/ai_company/config/errors.py (5)
ConfigError(24-54)ConfigFileNotFoundError(57-58)ConfigLocation(7-21)ConfigParseError(61-62)ConfigValidationError(65-96)
tests/unit/config/test_defaults.py (2)
src/ai_company/config/defaults.py (1)
default_config_dict(6-27)src/ai_company/config/schema.py (1)
RootConfig(260-380)
tests/unit/config/conftest.py (4)
src/ai_company/budget/config.py (1)
BudgetConfig(130-195)src/ai_company/communication/config.py (1)
CommunicationConfig(249-283)src/ai_company/config/schema.py (6)
AgentConfig(199-257)ProviderConfig(63-112)ProviderModelConfig(16-60)RootConfig(260-380)RoutingConfig(161-196)RoutingRuleConfig(115-158)src/ai_company/core/company.py (1)
CompanyConfig(101-144)
🔇 Additional comments (7)
tests/unit/config/conftest.py (1)
1-57: LGTM!The factory classes are well-structured for test data generation. The
polyfactorypattern is correctly applied with appropriate suppressions for the mutable class attribute (RUF012) and type assignment.src/ai_company/config/schema.py (5)
16-61: LGTM!
ProviderModelConfigis well-structured with appropriate constraints (min_length,ge,gt) and the validator correctly handles the optionalaliasfield.
63-113: LGTM!The uniqueness validation for model IDs and aliases is correctly implemented with separate checks. Using
Counterand sorted output ensures deterministic, helpful error messages.
115-197: LGTM!The routing configuration models properly handle optional fields with
Nonedefaults andmin_length=1constraints. Validators correctly check forNonebefore performing whitespace validation.
199-258: LGTM!The design decision to use
dict[str, Any]for raw config fields is well-documented in the docstring. The explanation about runtime fields inAgentIdentityprovides clear rationale for deferring validation to the engine at startup.
260-380: LGTM!
RootConfigprovides comprehensive cross-field validation:
- Uniqueness checks for agents and departments
- Routing references validated against provider model IDs and aliases
- Early return optimization when no routing rules exist
The validator at
_validate_routing_referencescorrectly handles the case where providers are empty but routing rules exist, producing appropriate error messages.src/ai_company/config/__init__.py (1)
1-55: LGTM!The public API module correctly exports all configuration-related symbols. The module docstring with
autosummaryprovides good documentation, and the alphabetically sorted__all__list ensures consistent ordering. All imported symbols are properly re-exported.
| def load_config( | ||
| config_path: Path | str, | ||
| *, | ||
| override_paths: tuple[Path | str, ...] = (), | ||
| ) -> RootConfig: |
There was a problem hiding this comment.
Implement standard-location config discovery fallback.
load_config currently requires an explicit config_path (Line 227), so the standard discovery behavior from acceptance criteria (./, ~/.ai-company/, etc.) is not implemented.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ai_company/config/loader.py` around lines 227 - 231, Make load_config
support standard-location discovery by allowing config_path to be optional
(change signature of load_config to accept None for config_path) and, when None
or missing, search well-known locations in order (check current working
directory first, then the user's home ai-company config directory per acceptance
criteria, and any other project-standard locations you have) before failing;
keep honoring the override_paths parameter and return RootConfig if a file is
found, otherwise raise a clear error. Ensure you update load_config's parameter
typing (allow Optional[Path|str]), implement the ordered lookup logic inside
load_config, and reuse existing parsing/validation code so behavior is identical
once a path is discovered.
| merged = default_config_dict() | ||
|
|
||
| # 2. Load and merge primary config | ||
| primary = _parse_yaml_file(config_path) | ||
| merged = _deep_merge(merged, primary) | ||
|
|
||
| # 3. Apply override layers | ||
| for override_path in override_paths: | ||
| override = _parse_yaml_file(Path(override_path)) | ||
| merged = _deep_merge(merged, override) | ||
|
|
||
| # 4. Build line map from primary file for error reporting | ||
| yaml_text = config_path.read_text(encoding="utf-8") | ||
| line_map = _build_line_map(yaml_text) | ||
|
|
||
| # 5. Validate | ||
| return _validate_config_dict( | ||
| merged, | ||
| source_file=str(config_path), | ||
| line_map=line_map, | ||
| ) |
There was a problem hiding this comment.
Add ${ENV_VAR} interpolation before validation.
At Line 259-264 and Line 299-301, merged config data goes straight to validation with no substitution pass, so env placeholders remain literal strings. This is a required feature and currently missing.
🛠️ Proposed fix (minimal pattern)
+import os
+import re
@@
+_ENV_VAR_RE = re.compile(r"\$\{([A-Za-z_][A-Za-z0-9_]*)\}")
+
+def _expand_env_vars(value: Any) -> Any:
+ if isinstance(value, dict):
+ return {k: _expand_env_vars(v) for k, v in value.items()}
+ if isinstance(value, list):
+ return [_expand_env_vars(v) for v in value]
+ if isinstance(value, str):
+ return _ENV_VAR_RE.sub(
+ lambda m: os.environ[m.group(1)],
+ value,
+ )
+ return value
@@
- # 5. Validate
+ merged = _expand_env_vars(merged)
+ # 5. Validate
@@
- line_map = _build_line_map(yaml_string)
+ merged = _expand_env_vars(merged)
+ line_map = _build_line_map(yaml_string)Also applies to: 299-306
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ai_company/config/loader.py` around lines 255 - 275, The merged config
dict is being validated while still containing literal "${ENV_VAR}"
placeholders; before calling _validate_config_dict you must run an
environment-variable interpolation pass (e.g., call a helper like
_interpolate_env_vars(merged) or implement one) to replace ${VAR} entries with
os.environ values (preserving types where appropriate), then pass the
interpolated dict to _validate_config_dict (use the same change at the other
spot noted around the later return). Locate the merge/validate sequence that
uses default_config_dict(), _parse_yaml_file(), _deep_merge(),
_build_line_map(), and _validate_config_dict() and insert the interpolation step
on merged right before _validate_config_dict in both places.
src/ai_company/config/loader.py
Outdated
| # 4. Build line map from primary file for error reporting | ||
| yaml_text = config_path.read_text(encoding="utf-8") | ||
| line_map = _build_line_map(yaml_text) | ||
|
|
||
| # 5. Validate | ||
| return _validate_config_dict( | ||
| merged, | ||
| source_file=str(config_path), | ||
| line_map=line_map, | ||
| ) |
There was a problem hiding this comment.
Validation locations are incorrect for override files.
Line 266 builds line_map from only the primary file, and Line 273 sets one source_file for all errors. When overrides introduce/replace values, validation messages can point to the wrong file/line or lose location entirely.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ai_company/config/loader.py` around lines 266 - 275, The validator
currently builds line_map from only the primary file (yaml_text/_build_line_map)
and passes a single source_file (config_path) into _validate_config_dict, so
errors from override files point to wrong locations; fix by constructing a
combined line map and source mapping that includes the primary file and each
override file: read each override's text, call _build_line_map for each (or
extend _build_line_map to accept multiple texts), merge those line maps into a
single composite line_map keyed by file and line ranges, and pass that composite
line_map (and either a composite source_file identifier or per-error file
references) into _validate_config_dict when validating merged, so validation
messages reference the correct file/line for values introduced or replaced by
overrides (update usages of yaml_text, _build_line_map, merged, config_path, and
the _validate_config_dict call accordingly).
| MISSING_REQUIRED_YAML = """\ | ||
| company_name: "" | ||
| """ | ||
|
|
||
| INVALID_FIELD_VALUES_YAML = """\ | ||
| company_name: "" | ||
| """ |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Duplicate test data constants.
MISSING_REQUIRED_YAML and INVALID_FIELD_VALUES_YAML have identical content. If these are meant to test different validation scenarios, consider differentiating them (e.g., missing a required field entirely vs. having an invalid value type).
Suggested differentiation
-MISSING_REQUIRED_YAML = """\
-company_name: ""
-"""
+MISSING_REQUIRED_YAML = """\
+company_type: startup
+"""
INVALID_FIELD_VALUES_YAML = """\
company_name: ""
"""📝 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.
| MISSING_REQUIRED_YAML = """\ | |
| company_name: "" | |
| """ | |
| INVALID_FIELD_VALUES_YAML = """\ | |
| company_name: "" | |
| """ | |
| MISSING_REQUIRED_YAML = """\ | |
| company_type: startup | |
| """ | |
| INVALID_FIELD_VALUES_YAML = """\ | |
| company_name: "" | |
| """ |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/config/conftest.py` around lines 97 - 103, The two test constants
MISSING_REQUIRED_YAML and INVALID_FIELD_VALUES_YAML are identical; change them
so they exercise distinct validation cases: make MISSING_REQUIRED_YAML actually
omit the required key (e.g., remove company_name entirely) and make
INVALID_FIELD_VALUES_YAML provide an invalid value for the key (e.g.,
company_name with the wrong type or clearly invalid value), then update any
tests referencing these constants if needed to match the intended scenario;
locate the constants by name in conftest.py to apply the changes.
| @pytest.mark.unit | ||
| class TestLoadConfig: | ||
| def test_explicit_path(self, tmp_config_file): | ||
| path = tmp_config_file(MINIMAL_VALID_YAML) | ||
| cfg = load_config(path) | ||
| assert isinstance(cfg, RootConfig) | ||
| assert cfg.company_name == "Test Corp" | ||
|
|
||
| def test_full_config(self, tmp_config_file): | ||
| path = tmp_config_file(FULL_VALID_YAML) | ||
| cfg = load_config(path) | ||
| assert cfg.company_name == "Test Corp" | ||
| assert len(cfg.agents) == 1 | ||
| assert cfg.agents[0].name == "Alice" | ||
| assert "anthropic" in cfg.providers | ||
|
|
||
| def test_layered_override(self, tmp_config_file): | ||
| base_path = tmp_config_file( | ||
| "company_name: Base Corp\ncompany_type: custom\n", | ||
| name="base.yaml", | ||
| ) | ||
| override_path = tmp_config_file( | ||
| "company_name: Override Corp\n", | ||
| name="override.yaml", | ||
| ) | ||
| cfg = load_config(base_path, override_paths=(override_path,)) | ||
| assert cfg.company_name == "Override Corp" | ||
|
|
||
| def test_defaults_applied(self, tmp_config_file): | ||
| path = tmp_config_file(MINIMAL_VALID_YAML) | ||
| cfg = load_config(path) | ||
| assert cfg.budget.total_monthly == 100.0 | ||
| assert cfg.routing.strategy == "cost_aware" | ||
|
|
||
| def test_validation_error_with_location(self, tmp_config_file): | ||
| path = tmp_config_file(MISSING_REQUIRED_YAML) | ||
| with pytest.raises(ConfigValidationError) as exc_info: | ||
| load_config(path) | ||
| err = exc_info.value | ||
| assert err.field_errors | ||
| assert any("company_name" in key for key, _ in err.field_errors) | ||
|
|
||
| def test_frozen_result(self, tmp_config_file): | ||
| from pydantic import ValidationError | ||
|
|
||
| path = tmp_config_file(MINIMAL_VALID_YAML) | ||
| cfg = load_config(path) | ||
| with pytest.raises(ValidationError): | ||
| cfg.company_name = "Nope" # type: ignore[misc] | ||
|
|
||
| def test_file_not_found(self, tmp_path): | ||
| with pytest.raises(ConfigFileNotFoundError): | ||
| load_config(tmp_path / "nonexistent.yaml") |
There was a problem hiding this comment.
Add acceptance-criteria tests for env substitution and path discovery.
I don’t see assertions for ${ENV_VAR} expansion or standard-location config discovery (Line 141 and Line 217 onward), both listed in issue #59 acceptance criteria. Please add positive/negative cases for both behaviors.
Also applies to: 217-252
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/config/test_loader.py` around lines 141 - 193, Add two tests
around environment-variable substitution and two tests for standard-location
discovery: (1) create a config containing "company_name: ${TEST_COMPANY}" using
the tmp_config_file fixture, set os.environ["TEST_COMPANY"] to a value, call
load_config(path) and assert cfg.company_name == that value; (2) ensure the env
var is unset (or del from os.environ), call load_config(path) and assert it
raises ConfigValidationError (or the loader's validation exception) for the
missing substitution; (3) for path discovery create a config file at the
standard lookup location by monkeypatching env vars (e.g., HOME or
XDG_CONFIG_HOME) or the lookup helper so load_config() with no path finds and
returns the file and assert loaded values; (4) ensure when no standard-location
file exists calling load_config() with no args raises ConfigFileNotFoundError;
reference load_config, ConfigValidationError, ConfigFileNotFoundError,
tmp_config_file, tmp_path and use monkeypatch/os.environ to control discovery
and substitution.
…t, and Gemini Source fixes: - Add warning log when _build_line_map fails to compose YAML AST - Read primary config file once (eliminate double I/O + TOCTOU risk) - Extract _read_config_text helper with OSError/PermissionError handling - Use is_file() instead of exists() to reject directory paths - Use copy.deepcopy in _deep_merge for true immutability - Include column in formatted error locations (ConfigError + ConfigValidationError) - Replace model_validators with NotBlankStr from core.types (6 models) - Add repr=False to ProviderConfig.api_key to prevent secret leakage - Fix _walk_node docstring accuracy and add Args section - Add debug log for non-scalar YAML keys in _walk_node - Document line map limitation for override files in load_config docstring - Fix _deep_merge docstring (base-only key preservation) - Fix defaults.py docstring wording - Add missing "logging" key to default_config_dict Test improvements (92 → 122 tests): - Add _build_line_map/_walk_node direct tests - Add _validate_config_dict direct tests (including line_map=None) - Add _read_config_text tests (including OSError wrapping) - Add routing unknown fallback + fallback chain unknown model tests - Add whitespace alias/base_url/task_type/fallback rejection tests - Add ConfigError str edge case tests (file-only, no key_path) - Add ConfigValidationError str with unmatched field errors test - Add multiple override files ordering test - Add default_config_dict keys drift test - Add api_key repr=False test - Use fake model IDs in test fixtures - Differentiate MISSING_REQUIRED_YAML vs INVALID_FIELD_VALUES_YAML - Add proper type annotations to conftest fixtures
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive YAML configuration loading system for the config/ module, leveraging Pydantic for schema definition and validation, and featuring a layered deep-merge of configurations with custom error types and extensive unit tests. While the use of yaml.safe_load and yaml.SafeLoader correctly mitigates arbitrary code execution, a security weakness was identified in the integration of the logging configuration: the log_dir validation is incomplete, allowing absolute paths which could lead to path traversal vulnerabilities when the configuration is loaded from untrusted sources. Additionally, a minor suggestion for improving maintainability by reducing code duplication was noted.
| default_factory=RoutingConfig, | ||
| description="Model routing configuration", | ||
| ) | ||
| logging: LogConfig | None = Field( |
There was a problem hiding this comment.
The LogConfig model (imported from ai_company.observability.config) used in the logging field of RootConfig contains an incomplete path traversal validator for the log_dir attribute. The validator only checks for the presence of .. components but fails to reject absolute paths. Since this configuration can be loaded from untrusted strings via load_config_from_string (which is intended for API endpoints), an attacker could specify an absolute path for the log directory (e.g., /etc or ~/.ssh), potentially allowing the application to write log files to sensitive locations. This could lead to unauthorized file creation or modification, and in some scenarios, remote code execution via log injection. To remediate this, the validator in LogConfig should be updated to reject absolute paths using path.is_absolute() and check for both POSIX and Windows absolute path formats, similar to the validation implemented in SinkConfig._validate_file_sink_requires_path.
| if loc.line is not None and loc.column is not None: | ||
| line_info = f" at line {loc.line}, column {loc.column}" | ||
| elif loc.line is not None: | ||
| line_info = f" at line {loc.line}" | ||
| else: | ||
| line_info = "" | ||
| loc_parts.append(f" in {loc.file_path}{line_info}") |
There was a problem hiding this comment.
The logic for formatting the line_info string is duplicated here and in ConfigValidationError.__str__ (lines 99-105). To improve maintainability and adhere to the DRY (Don't Repeat Yourself) principle, consider extracting this logic into a private helper function within the module, such as _format_line_info(loc: ConfigLocation) -> str.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/ai_company/config/schema.py`:
- Around line 285-309: The validator _validate_routing_references currently
builds known_models as a set which hides collisions; change it to build a
mapping from model identifier (model.id and model.alias) to the list of provider
names (e.g., dict[str, list[str]]) by iterating providers and their models, then
check for any keys with more than one provider and raise a ValueError listing
the ambiguous model key and providers; afterwards perform the existing existence
checks (routing.rules and routing.fallback_chain) against the mapping keys (not
a set) so unknown references still error but ambiguous references are detected
and rejected up front.
- Around line 177-196: AgentConfig's frozen flag doesn't protect nested mutable
dicts (personality, model, memory, tools, authority); add a Pydantic validator
(or root_validator) on AgentConfig that, after validation, recursively converts
those five dict fields into immutable mappings (e.g., types.MappingProxyType or
another immutable mapping) so nested structures cannot be mutated at runtime,
ensuring the model-level immutability guarantee for personality, model, memory,
tools and authority.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
src/ai_company/config/defaults.pysrc/ai_company/config/errors.pysrc/ai_company/config/loader.pysrc/ai_company/config/schema.pytests/unit/config/conftest.pytests/unit/config/test_defaults.pytests/unit/config/test_errors.pytests/unit/config/test_loader.pytests/unit/config/test_schema.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). (1)
- GitHub Check: Agent
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Use PEP 649 native lazy annotations instead offrom __future__ import annotations(Python 3.14+)
All public functions and classes must include type hints; enforce via mypy strict mode
Use Google-style docstrings on all public classes and functions (enforced by ruff D rules)
Keep functions under 50 lines and files under 800 lines
Create new objects instead of mutating existing ones; enforce immutability patterns
Use Pydantic v2 models (BaseModel, model_validator, ConfigDict) for data validation
Validate at system boundaries (user input, external APIs, config files); never silently swallow errors
Enforce 88-character line length (via ruff configuration)
Files:
src/ai_company/config/schema.pytests/unit/config/test_defaults.pytests/unit/config/test_schema.pytests/unit/config/test_loader.pysrc/ai_company/config/defaults.pytests/unit/config/conftest.pysrc/ai_company/config/errors.pysrc/ai_company/config/loader.pytests/unit/config/test_errors.py
{src/**/*.py,tests/**/*.py}
📄 CodeRabbit inference engine (CLAUDE.md)
Run ruff check and ruff format as pre-commit hooks on all Python files; enforce no commits to main branch
Files:
src/ai_company/config/schema.pytests/unit/config/test_defaults.pytests/unit/config/test_schema.pytests/unit/config/test_loader.pysrc/ai_company/config/defaults.pytests/unit/config/conftest.pysrc/ai_company/config/errors.pysrc/ai_company/config/loader.pytests/unit/config/test_errors.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Mark tests with@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, or@pytest.mark.slow
Maintain minimum 80% code coverage, enforced in CI via pytest-cov
Set test timeout to 30 seconds per test; use asyncio_mode = 'auto' for async tests without manual@pytest.mark.asyncio
Files:
tests/unit/config/test_defaults.pytests/unit/config/test_schema.pytests/unit/config/test_loader.pytests/unit/config/conftest.pytests/unit/config/test_errors.py
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-28T13:41:23.367Z
Learning: Applies to **/*.py : Use Pydantic v2 models (BaseModel, model_validator, ConfigDict) for data validation
📚 Learning: 2026-02-28T13:41:23.367Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-28T13:41:23.367Z
Learning: Applies to **/*.py : Use Pydantic v2 models (BaseModel, model_validator, ConfigDict) for data validation
Applied to files:
src/ai_company/config/schema.py
📚 Learning: 2026-02-26T17:43:50.902Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-26T17:43:50.902Z
Learning: Applies to tests/**/*.py : Tests must use fake model names (e.g., `test-model:8b`, `fake-writer:latest`)—never use real model IDs from `RECOMMENDED_MODELS`.
Applied to files:
tests/unit/config/test_schema.pytests/unit/config/conftest.py
📚 Learning: 2026-01-31T13:51:16.868Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-31T13:51:16.868Z
Learning: Applies to tests/**/*.py : Mock models in tests must use a name from `RECOMMENDED_MODELS` (e.g., `huihui_ai/dolphin3-abliterated:8b`) - fake model names cause `ValueError: No model tagged for role`.
Applied to files:
tests/unit/config/test_schema.pytests/unit/config/conftest.py
📚 Learning: 2026-01-24T16:33:29.354Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-24T16:33:29.354Z
Learning: Applies to tests/**/*.py : Mock Ollama in tests to avoid requiring running instance - use model names from `RECOMMENDED_MODELS` (e.g., `huihui_ai/dolphin3-abliterated:8b`)
Applied to files:
tests/unit/config/test_schema.pytests/unit/config/conftest.py
📚 Learning: 2026-01-24T09:54:45.426Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/instructions/agents.instructions.md:0-0
Timestamp: 2026-01-24T09:54:45.426Z
Learning: Applies to agents/test*.py : Agent tests should cover: successful generation with valid output, handling malformed LLM responses, error conditions (network errors, timeouts), output format validation, and integration with story state
Applied to files:
tests/unit/config/test_loader.py
📚 Learning: 2026-01-24T09:54:56.100Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/instructions/test-files.instructions.md:0-0
Timestamp: 2026-01-24T09:54:56.100Z
Learning: Applies to **/tests/conftest.py : Place shared pytest fixtures in `tests/conftest.py`
Applied to files:
tests/unit/config/conftest.py
📚 Learning: 2026-01-24T16:33:29.354Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-24T16:33:29.354Z
Learning: Applies to tests/unit/test_*.py : Write unit tests for new functionality using pytest in `tests/unit/` with `test_*.py` naming convention
Applied to files:
tests/unit/config/conftest.py
📚 Learning: 2026-01-26T08:59:32.818Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-26T08:59:32.818Z
Learning: Applies to tests/unit/test_*.py : Write unit tests for new functionality using pytest. Place test files in `tests/unit/` with `test_*.py` naming convention.
Applied to files:
tests/unit/config/conftest.py
📚 Learning: 2026-01-26T08:59:32.818Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-26T08:59:32.818Z
Learning: Applies to tests/**/*.py : Use pytest fixtures for test setup. Shared fixtures should be in `tests/conftest.py`
Applied to files:
tests/unit/config/conftest.py
📚 Learning: 2026-01-24T09:54:56.100Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/instructions/test-files.instructions.md:0-0
Timestamp: 2026-01-24T09:54:56.100Z
Learning: Each test should be independent and not rely on other tests; use pytest fixtures for test setup (shared fixtures in `tests/conftest.py`); clean up resources in teardown/fixtures
Applied to files:
tests/unit/config/conftest.py
📚 Learning: 2026-01-24T09:54:56.100Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/instructions/test-files.instructions.md:0-0
Timestamp: 2026-01-24T09:54:56.100Z
Learning: Applies to **/test_*.py : Use appropriate fixture scopes (`function`, `class`, `module`, `session`) and document complex fixtures with docstrings
Applied to files:
tests/unit/config/conftest.py
📚 Learning: 2026-02-28T13:41:23.367Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-28T13:41:23.367Z
Learning: Applies to **/*.py : Validate at system boundaries (user input, external APIs, config files); never silently swallow errors
Applied to files:
src/ai_company/config/loader.py
📚 Learning: 2026-01-24T09:54:45.426Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/instructions/agents.instructions.md:0-0
Timestamp: 2026-01-24T09:54:45.426Z
Learning: Applies to agents/*.py : Implement graceful error recovery: retry with different prompts if needed, fall back to simpler approaches on failure, and don't fail silently - log and raise appropriate exceptions
Applied to files:
src/ai_company/config/loader.py
📚 Learning: 2026-01-26T08:59:32.818Z
Learnt from: CR
Repo: Aureliolo/story-factory PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-26T08:59:32.818Z
Learning: Applies to **/*.py : All new code must have corresponding unit tests. When modifying existing code, update related tests. Tests should cover both happy paths and edge cases.
Applied to files:
tests/unit/config/test_errors.py
🧬 Code graph analysis (5)
src/ai_company/config/schema.py (6)
src/ai_company/budget/config.py (1)
BudgetConfig(130-195)src/ai_company/communication/config.py (1)
CommunicationConfig(249-283)src/ai_company/core/company.py (2)
CompanyConfig(101-144)Department(52-98)src/ai_company/core/enums.py (2)
CompanyType(72-82)SeniorityLevel(6-21)src/ai_company/core/role.py (1)
CustomRole(183-249)src/ai_company/observability/config.py (1)
LogConfig(115-191)
tests/unit/config/test_schema.py (2)
src/ai_company/config/schema.py (6)
AgentConfig(147-196)ProviderConfig(52-90)ProviderModelConfig(17-49)RootConfig(199-310)RoutingConfig(122-144)RoutingRuleConfig(93-119)src/ai_company/core/enums.py (2)
CompanyType(72-82)SeniorityLevel(6-21)
tests/unit/config/test_loader.py (2)
src/ai_company/config/loader.py (8)
_build_line_map(189-216)_deep_merge(27-51)_parse_yaml_file(83-98)_parse_yaml_string(101-146)_read_config_text(54-80)_validate_config_dict(219-263)load_config(271-328)load_config_from_string(331-359)tests/unit/config/conftest.py (1)
tmp_config_file(121-127)
src/ai_company/config/loader.py (3)
src/ai_company/config/defaults.py (1)
default_config_dict(6-28)src/ai_company/config/errors.py (4)
ConfigFileNotFoundError(62-63)ConfigLocation(7-21)ConfigParseError(66-67)ConfigValidationError(70-106)src/ai_company/config/schema.py (1)
RootConfig(199-310)
tests/unit/config/test_errors.py (1)
src/ai_company/config/errors.py (5)
ConfigError(24-59)ConfigFileNotFoundError(62-63)ConfigLocation(7-21)ConfigParseError(66-67)ConfigValidationError(70-106)
🔇 Additional comments (18)
src/ai_company/config/defaults.py (1)
1-28: LGTM!Clean implementation providing base-layer defaults. The function returns a fresh dict on each call (ensuring immutability), includes proper type hints, and follows Google-style docstrings.
tests/unit/config/conftest.py (3)
101-109: YAML constants now properly differentiated.Good fix from the previous review -
MISSING_REQUIRED_YAML(empty company_name) andINVALID_FIELD_VALUES_YAML(negative budget) now test distinct validation scenarios.
85-93: Fake model ID correctly used.The test fixture now uses
test-model-001instead of a real model identifier, which aligns with the testing guidelines.
120-127: Type hints properly specified for fixture.The
tmp_config_filefixture now has proper type annotations withPathandCallable[[str, str], Path]. TheTYPE_CHECKINGguard pattern is acceptable since Python 3.10+ handles string annotations for type hints at runtime.tests/unit/config/test_loader.py (2)
1-29: Well-structured test module with comprehensive coverage.Tests are properly organized by function/class with appropriate
@pytest.mark.unitmarkers. Good coverage of edge cases including empty inputs, error conditions, and immutability verification.
273-395: Tests forload_configandload_config_from_stringcover implemented functionality well.However, per issue
#59acceptance criteria, tests for${ENV_VAR}substitution and standard-location config discovery are missing. This appears to be because those features are not yet implemented inloader.py. Consider adding these tests when the features are implemented, or adding@pytest.mark.skipplaceholders to track the gap.tests/unit/config/test_schema.py (2)
28-73: Comprehensive ProviderModelConfig tests with fake model IDs.Tests cover minimal/full configurations, validation of blank/whitespace fields, negative cost rejection, frozen behavior, and factory usage. Fake model IDs (
test-model:8b,m1) are correctly used throughout.
252-387: Thorough RootConfig validation tests.Good coverage of:
- Uniqueness constraints (agent names, department names)
- Cross-field routing validation (unknown model references)
- Default values and frozen behavior
- Factory usage
tests/unit/config/test_defaults.py (1)
1-41: LGTM!Excellent test coverage including an important invariant test (
test_keys_match_root_config_fields) that ensuresdefault_config_dict()keys stay synchronized withRootConfigfields. This will catch drift between defaults and schema during future refactoring.tests/unit/config/test_errors.py (1)
1-168: LGTM!Comprehensive tests covering all error types and their formatting behavior. Good coverage of edge cases including:
- Locations with/without line/column information
- Field errors with/without matching locations
- Fallback behavior when no field errors provided
- Inheritance verification
src/ai_company/config/errors.py (1)
1-106: LGTM!Well-designed error hierarchy with:
- Frozen
ConfigLocationdataclass for immutability- Rich
__str__formatting including line and column informationConfigValidationErrorwith per-field error details and location mappingThe column information is now properly included in error output (Lines 51-56 and 99-104).
src/ai_company/config/loader.py (6)
27-51: Clean deep merge implementation with immutability guarantees.Good use of
copy.deepcopy()to ensure neither input dict is mutated and the result doesn't share mutable references with inputs.
54-80: Robust file reading with proper error boundary handling.Uses
is_file()to reject directories and wrapsOSErrorinto typedConfigParseError. This addresses the earlier review feedback about normalizing filesystem errors.
271-328: Missing${ENV_VAR}substitution - acceptance criteria gap.Issue
#59acceptance criteria requires: "Support environment variable substitution in config values using${ENV_VAR}." The merged config goes directly to validation without an interpolation pass, leaving${VAR}placeholders as literal strings.Consider adding an interpolation step before validation:
Suggested approach
import os import re _ENV_VAR_RE = re.compile(r"\$\{([A-Za-z_][A-Za-z0-9_]*)\}") def _expand_env_vars(value: Any) -> Any: """Recursively expand ${ENV_VAR} placeholders.""" if isinstance(value, dict): return {k: _expand_env_vars(v) for k, v in value.items()} if isinstance(value, list): return [_expand_env_vars(v) for v in value] if isinstance(value, str): def replacer(m: re.Match[str]) -> str: var_name = m.group(1) if var_name not in os.environ: raise ConfigValidationError( f"Environment variable not set: {var_name}", ) return os.environ[var_name] return _ENV_VAR_RE.sub(replacer, value) return valueThen call
merged = _expand_env_vars(merged)before_validate_config_dict()in bothload_configandload_config_from_string.
271-275: Missing standard-location config discovery - acceptance criteria gap.Issue
#59acceptance criteria requires: "Provide config file discovery (e.g., current directory,~/.ai-company/, etc.)." Currentlyconfig_pathis required with no fallback discovery.Consider making
config_pathoptional and implementing a discovery function:Suggested approach
def _discover_config_path() -> Path: """Search standard locations for a config file.""" candidates = [ Path.cwd() / "ai-company.yaml", Path.cwd() / ".ai-company.yaml", Path.home() / ".ai-company" / "config.yaml", ] for candidate in candidates: if candidate.is_file(): return candidate msg = "No configuration file found in standard locations" raise ConfigFileNotFoundError(msg) def load_config( config_path: Path | str | None = None, *, override_paths: tuple[Path | str, ...] = (), ) -> RootConfig: if config_path is None: config_path = _discover_config_path() # ... rest of implementation
149-186: Well-implemented YAML AST walker for line mapping.Good defensive coding with
logger.debugfor non-scalar keys and proper 1-based line/column conversion. The recursive traversal handles both mappings and sequences correctly.
219-263: Validation with location enrichment works correctly.The
_validate_config_dictfunction properly extracts Pydantic validation errors and enriches them with YAML source locations when available, gracefully degrading whenline_mapis None.src/ai_company/config/schema.py (1)
63-67: Good secret-hardening onapi_keyrepresentationAt Line 63 through Line 67,
repr=Falseonapi_keyis the right safeguard to reduce accidental secret leakage in logs/debug output.
| personality: dict[str, Any] = Field( | ||
| default_factory=dict, | ||
| description="Raw personality config", | ||
| ) | ||
| model: dict[str, Any] = Field( | ||
| default_factory=dict, | ||
| description="Raw model config", | ||
| ) | ||
| memory: dict[str, Any] = Field( | ||
| default_factory=dict, | ||
| description="Raw memory config", | ||
| ) | ||
| tools: dict[str, Any] = Field( | ||
| default_factory=dict, | ||
| description="Raw tools config", | ||
| ) | ||
| authority: dict[str, Any] = Field( | ||
| default_factory=dict, | ||
| description="Raw authority config", | ||
| ) |
There was a problem hiding this comment.
Nested mutable dicts bypass model immutability
At Line 177 through Line 196, AgentConfig is frozen, but nested dict fields remain mutable after validation. This allows runtime mutation of config state and breaks immutability guarantees.
🔧 Proposed fix (freeze nested mappings)
-from typing import Any, Self
+from types import MappingProxyType
+from typing import Any, Mapping, Self
-from pydantic import BaseModel, ConfigDict, Field, model_validator
+from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator
@@
- personality: dict[str, Any] = Field(
+ personality: Mapping[str, Any] = Field(
default_factory=dict,
description="Raw personality config",
)
- model: dict[str, Any] = Field(
+ model: Mapping[str, Any] = Field(
default_factory=dict,
description="Raw model config",
)
- memory: dict[str, Any] = Field(
+ memory: Mapping[str, Any] = Field(
default_factory=dict,
description="Raw memory config",
)
- tools: dict[str, Any] = Field(
+ tools: Mapping[str, Any] = Field(
default_factory=dict,
description="Raw tools config",
)
- authority: dict[str, Any] = Field(
+ authority: Mapping[str, Any] = Field(
default_factory=dict,
description="Raw authority config",
)
+
+ `@field_validator`(
+ "personality", "model", "memory", "tools", "authority", mode="before"
+ )
+ `@classmethod`
+ def _freeze_mapping(
+ cls, value: Mapping[str, Any] | None
+ ) -> Mapping[str, Any]:
+ if value is None:
+ return MappingProxyType({})
+ return MappingProxyType(dict(value))As per coding guidelines: "Create new objects instead of mutating existing ones; enforce immutability patterns".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ai_company/config/schema.py` around lines 177 - 196, AgentConfig's frozen
flag doesn't protect nested mutable dicts (personality, model, memory, tools,
authority); add a Pydantic validator (or root_validator) on AgentConfig that,
after validation, recursively converts those five dict fields into immutable
mappings (e.g., types.MappingProxyType or another immutable mapping) so nested
structures cannot be mutated at runtime, ensuring the model-level immutability
guarantee for personality, model, memory, tools and authority.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 14 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/ai_company/config/loader.py:20
- This module uses stdlib
logging.getLogger(__name__), butai_company.observabilityexplicitly documents usingget_logger(__name__)for consistent structured logging (seesrc/ai_company/observability/__init__.py:12-15). Consider switching tofrom ai_company.observability import get_loggerandlogger = get_logger(__name__)so these warnings/debug logs go through the configured structlog pipeline.
import logging
from pathlib import Path
from typing import Any
import yaml
from pydantic import ValidationError
from ai_company.config.defaults import default_config_dict
from ai_company.config.errors import (
ConfigFileNotFoundError,
ConfigLocation,
ConfigParseError,
ConfigValidationError,
)
from ai_company.config.schema import RootConfig
logger = logging.getLogger(__name__)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def load_config( | ||
| config_path: Path | str, | ||
| *, | ||
| override_paths: tuple[Path | str, ...] = (), | ||
| ) -> RootConfig: | ||
| """Load and validate company configuration from YAML file(s). | ||
|
|
||
| Loading order (each layer deep-merges onto the previous): | ||
|
|
||
| 1. Built-in defaults (from :func:`default_config_dict`). | ||
| 2. Primary config file at *config_path*. | ||
| 3. Override files in order. | ||
|
|
||
| .. note:: | ||
|
|
||
| The YAML line map used for error enrichment is built from the | ||
| primary config file only. When override files introduce or | ||
| replace values, validation errors may reference the primary | ||
| file's line numbers or lack location information entirely. | ||
|
|
||
| Args: | ||
| config_path: Path to the primary config file. | ||
| override_paths: Additional config files layered on top. | ||
|
|
||
| Returns: |
There was a problem hiding this comment.
The PR description says this implements/closes issue #59, but load_config() only supports explicit paths + override paths and does not implement two acceptance criteria from #59: config file discovery (searching default locations) and environment variable substitution (e.g., ${ENV_VAR}) in config values. Either implement these behaviors here, or update the PR/issue linkage so #59 isn’t marked complete prematurely.
- Reject absolute paths in LogConfig.log_dir validator (security fix) - Detect ambiguous cross-provider model references in routing validation - Extract _collect_model_refs helper to reduce complexity - Add tests for absolute log_dir rejection and ambiguous routing refs - Create follow-up issue #76 for env var substitution and config discovery
🤖 I have created a release *beep* *boop* --- ## [0.1.1](ai-company-v0.1.0...ai-company-v0.1.1) (2026-03-10) ### Features * add autonomy levels and approval timeout policies ([#42](#42), [#126](#126)) ([#197](#197)) ([eecc25a](eecc25a)) * add CFO cost optimization service with anomaly detection, reports, and approval decisions ([#186](#186)) ([a7fa00b](a7fa00b)) * add code quality toolchain (ruff, mypy, pre-commit, dependabot) ([#63](#63)) ([36681a8](36681a8)) * add configurable cost tiers and subscription/quota-aware tracking ([#67](#67)) ([#185](#185)) ([9baedfa](9baedfa)) * add container packaging, Docker Compose, and CI pipeline ([#269](#269)) ([435bdfe](435bdfe)), closes [#267](#267) * add coordination error taxonomy classification pipeline ([#146](#146)) ([#181](#181)) ([70c7480](70c7480)) * add cost-optimized, hierarchical, and auction assignment strategies ([#175](#175)) ([ce924fa](ce924fa)), closes [#173](#173) * add design specification, license, and project setup ([8669a09](8669a09)) * add env var substitution and config file auto-discovery ([#77](#77)) ([7f53832](7f53832)) * add FastestStrategy routing + vendor-agnostic cleanup ([#140](#140)) ([09619cb](09619cb)), closes [#139](#139) * add HR engine and performance tracking ([#45](#45), [#47](#47)) ([#193](#193)) ([2d091ea](2d091ea)) * add issue auto-search and resolution verification to PR review skill ([#119](#119)) ([deecc39](deecc39)) * add memory retrieval, ranking, and context injection pipeline ([#41](#41)) ([873b0aa](873b0aa)) * add pluggable MemoryBackend protocol with models, config, and events ([#180](#180)) ([46cfdd4](46cfdd4)) * add pluggable MemoryBackend protocol with models, config, and events ([#32](#32)) ([46cfdd4](46cfdd4)) * add pluggable PersistenceBackend protocol with SQLite implementation ([#36](#36)) ([f753779](f753779)) * add progressive trust and promotion/demotion subsystems ([#43](#43), [#49](#49)) ([3a87c08](3a87c08)) * add retry handler, rate limiter, and provider resilience ([#100](#100)) ([b890545](b890545)) * add SecOps security agent with rule engine, audit log, and ToolInvoker integration ([#40](#40)) ([83b7b6c](83b7b6c)) * add shared org memory and memory consolidation/archival ([#125](#125), [#48](#48)) ([4a0832b](4a0832b)) * design unified provider interface ([#86](#86)) ([3e23d64](3e23d64)) * expand template presets, rosters, and add inheritance ([#80](#80), [#81](#81), [#84](#84)) ([15a9134](15a9134)) * implement agent runtime state vs immutable config split ([#115](#115)) ([4cb1ca5](4cb1ca5)) * implement AgentEngine core orchestrator ([#11](#11)) ([#143](#143)) ([f2eb73a](f2eb73a)) * implement basic tool system (registry, invocation, results) ([#15](#15)) ([c51068b](c51068b)) * implement built-in file system tools ([#18](#18)) ([325ef98](325ef98)) * implement communication foundation — message bus, dispatcher, and messenger ([#157](#157)) ([8e71bfd](8e71bfd)) * implement company template system with 7 built-in presets ([#85](#85)) ([cbf1496](cbf1496)) * implement conflict resolution protocol ([#122](#122)) ([#166](#166)) ([e03f9f2](e03f9f2)) * implement core entity and role system models ([#69](#69)) ([acf9801](acf9801)) * implement crash recovery with fail-and-reassign strategy ([#149](#149)) ([e6e91ed](e6e91ed)) * implement engine extensions — Plan-and-Execute loop and call categorization ([#134](#134), [#135](#135)) ([#159](#159)) ([9b2699f](9b2699f)) * implement enterprise logging system with structlog ([#73](#73)) ([2f787e5](2f787e5)) * implement graceful shutdown with cooperative timeout strategy ([#130](#130)) ([6592515](6592515)) * implement hierarchical delegation and loop prevention ([#12](#12), [#17](#17)) ([6be60b6](6be60b6)) * implement LiteLLM driver and provider registry ([#88](#88)) ([ae3f18b](ae3f18b)), closes [#4](#4) * implement LLM decomposition strategy and workspace isolation ([#174](#174)) ([aa0eefe](aa0eefe)) * implement meeting protocol system ([#123](#123)) ([ee7caca](ee7caca)) * implement message and communication domain models ([#74](#74)) ([560a5d2](560a5d2)) * implement model routing engine ([#99](#99)) ([d3c250b](d3c250b)) * implement parallel agent execution ([#22](#22)) ([#161](#161)) ([65940b3](65940b3)) * implement per-call cost tracking service ([#7](#7)) ([#102](#102)) ([c4f1f1c](c4f1f1c)) * implement personality injection and system prompt construction ([#105](#105)) ([934dd85](934dd85)) * implement single-task execution lifecycle ([#21](#21)) ([#144](#144)) ([c7e64e4](c7e64e4)) * implement subprocess sandbox for tool execution isolation ([#131](#131)) ([#153](#153)) ([3c8394e](3c8394e)) * implement task assignment subsystem with pluggable strategies ([#172](#172)) ([c7f1b26](c7f1b26)), closes [#26](#26) [#30](#30) * implement task decomposition and routing engine ([#14](#14)) ([9c7fb52](9c7fb52)) * implement Task, Project, Artifact, Budget, and Cost domain models ([#71](#71)) ([81eabf1](81eabf1)) * implement tool permission checking ([#16](#16)) ([833c190](833c190)) * implement YAML config loader with Pydantic validation ([#59](#59)) ([ff3a2ba](ff3a2ba)) * implement YAML config loader with Pydantic validation ([#75](#75)) ([ff3a2ba](ff3a2ba)) * initialize project with uv, hatchling, and src layout ([39005f9](39005f9)) * initialize project with uv, hatchling, and src layout ([#62](#62)) ([39005f9](39005f9)) * Litestar REST API, WebSocket feed, and approval queue (M6) ([#189](#189)) ([29fcd08](29fcd08)) * make TokenUsage.total_tokens a computed field ([#118](#118)) ([c0bab18](c0bab18)), closes [#109](#109) * parallel tool execution in ToolInvoker.invoke_all ([#137](#137)) ([58517ee](58517ee)) * testing framework, CI pipeline, and M0 gap fixes ([#64](#64)) ([f581749](f581749)) * wire all modules into observability system ([#97](#97)) ([f7a0617](f7a0617)) ### Bug Fixes * address Greptile post-merge review findings from PRs [#170](https://github.com/Aureliolo/ai-company/issues/170)-[#175](https://github.com/Aureliolo/ai-company/issues/175) ([#176](#176)) ([c5ca929](c5ca929)) * address post-merge review feedback from PRs [#164](https://github.com/Aureliolo/ai-company/issues/164)-[#167](https://github.com/Aureliolo/ai-company/issues/167) ([#170](#170)) ([3bf897a](3bf897a)), closes [#169](#169) * enforce strict mypy on test files ([#89](#89)) ([aeeff8c](aeeff8c)) * harden Docker sandbox, MCP bridge, and code runner ([#50](#50), [#53](#53)) ([d5e1b6e](d5e1b6e)) * harden git tools security + code quality improvements ([#150](#150)) ([000a325](000a325)) * harden subprocess cleanup, env filtering, and shutdown resilience ([#155](#155)) ([d1fe1fb](d1fe1fb)) * incorporate post-merge feedback + pre-PR review fixes ([#164](#164)) ([c02832a](c02832a)) * pre-PR review fixes for post-merge findings ([#183](#183)) ([26b3108](26b3108)) * strengthen immutability for BaseTool schema and ToolInvoker boundaries ([#117](#117)) ([7e5e861](7e5e861)) ### Performance * harden non-inferable principle implementation ([#195](#195)) ([02b5f4e](02b5f4e)), closes [#188](#188) ### Refactoring * adopt NotBlankStr across all models ([#108](#108)) ([#120](#120)) ([ef89b90](ef89b90)) * extract _SpendingTotals base class from spending summary models ([#111](#111)) ([2f39c1b](2f39c1b)) * harden BudgetEnforcer with error handling, validation extraction, and review fixes ([#182](#182)) ([c107bf9](c107bf9)) * harden personality profiles, department validation, and template rendering ([#158](#158)) ([10b2299](10b2299)) * pre-PR review improvements for ExecutionLoop + ReAct loop ([#124](#124)) ([8dfb3c0](8dfb3c0)) * split events.py into per-domain event modules ([#136](#136)) ([e9cba89](e9cba89)) ### Documentation * add ADR-001 memory layer evaluation and selection ([#178](#178)) ([db3026f](db3026f)), closes [#39](#39) * add agent scaling research findings to DESIGN_SPEC ([#145](#145)) ([57e487b](57e487b)) * add CLAUDE.md, contributing guide, and dev documentation ([#65](#65)) ([55c1025](55c1025)), closes [#54](#54) * add crash recovery, sandboxing, analytics, and testing decisions ([#127](#127)) ([5c11595](5c11595)) * address external review feedback with MVP scope and new protocols ([#128](#128)) ([3b30b9a](3b30b9a)) * expand design spec with pluggable strategy protocols ([#121](#121)) ([6832db6](6832db6)) * finalize 23 design decisions (ADR-002) ([#190](#190)) ([8c39742](8c39742)) * update project docs for M2.5 conventions and add docs-consistency review agent ([#114](#114)) ([99766ee](99766ee)) ### Tests * add e2e single agent integration tests ([#24](#24)) ([#156](#156)) ([f566fb4](f566fb4)) * add provider adapter integration tests ([#90](#90)) ([40a61f4](40a61f4)) ### CI/CD * add Release Please for automated versioning and GitHub Releases ([#278](#278)) ([a488758](a488758)) * bump actions/checkout from 4 to 6 ([#95](#95)) ([1897247](1897247)) * bump actions/upload-artifact from 4 to 7 ([#94](#94)) ([27b1517](27b1517)) * harden CI/CD pipeline ([#92](#92)) ([ce4693c](ce4693c)) * split vulnerability scans into critical-fail and high-warn tiers ([#277](#277)) ([aba48af](aba48af)) ### Maintenance * add /worktree skill for parallel worktree management ([#171](#171)) ([951e337](951e337)) * add design spec context loading to research-link skill ([8ef9685](8ef9685)) * add post-merge-cleanup skill ([#70](#70)) ([f913705](f913705)) * add pre-pr-review skill and update CLAUDE.md ([#103](#103)) ([92e9023](92e9023)) * add research-link skill and rename skill files to SKILL.md ([#101](#101)) ([651c577](651c577)) * bump aiosqlite from 0.21.0 to 0.22.1 ([#191](#191)) ([3274a86](3274a86)) * bump pyyaml from 6.0.2 to 6.0.3 in the minor-and-patch group ([#96](#96)) ([0338d0c](0338d0c)) * bump ruff from 0.15.4 to 0.15.5 ([a49ee46](a49ee46)) * fix M0 audit items ([#66](#66)) ([c7724b5](c7724b5)) * pin setup-uv action to full SHA ([#281](#281)) ([4448002](4448002)) * post-audit cleanup — PEP 758, loggers, bug fixes, refactoring, tests, hookify rules ([#148](#148)) ([c57a6a9](c57a6a9)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
🤖 I have created a release *beep* *boop* --- ## [0.1.0](v0.0.0...v0.1.0) (2026-03-11) ### Features * add autonomy levels and approval timeout policies ([#42](#42), [#126](#126)) ([#197](#197)) ([eecc25a](eecc25a)) * add CFO cost optimization service with anomaly detection, reports, and approval decisions ([#186](#186)) ([a7fa00b](a7fa00b)) * add code quality toolchain (ruff, mypy, pre-commit, dependabot) ([#63](#63)) ([36681a8](36681a8)) * add configurable cost tiers and subscription/quota-aware tracking ([#67](#67)) ([#185](#185)) ([9baedfa](9baedfa)) * add container packaging, Docker Compose, and CI pipeline ([#269](#269)) ([435bdfe](435bdfe)), closes [#267](#267) * add coordination error taxonomy classification pipeline ([#146](#146)) ([#181](#181)) ([70c7480](70c7480)) * add cost-optimized, hierarchical, and auction assignment strategies ([#175](#175)) ([ce924fa](ce924fa)), closes [#173](#173) * add design specification, license, and project setup ([8669a09](8669a09)) * add env var substitution and config file auto-discovery ([#77](#77)) ([7f53832](7f53832)) * add FastestStrategy routing + vendor-agnostic cleanup ([#140](#140)) ([09619cb](09619cb)), closes [#139](#139) * add HR engine and performance tracking ([#45](#45), [#47](#47)) ([#193](#193)) ([2d091ea](2d091ea)) * add issue auto-search and resolution verification to PR review skill ([#119](#119)) ([deecc39](deecc39)) * add mandatory JWT + API key authentication ([#256](#256)) ([c279cfe](c279cfe)) * add memory retrieval, ranking, and context injection pipeline ([#41](#41)) ([873b0aa](873b0aa)) * add pluggable MemoryBackend protocol with models, config, and events ([#180](#180)) ([46cfdd4](46cfdd4)) * add pluggable MemoryBackend protocol with models, config, and events ([#32](#32)) ([46cfdd4](46cfdd4)) * add pluggable output scan response policies ([#263](#263)) ([b9907e8](b9907e8)) * add pluggable PersistenceBackend protocol with SQLite implementation ([#36](#36)) ([f753779](f753779)) * add progressive trust and promotion/demotion subsystems ([#43](#43), [#49](#49)) ([3a87c08](3a87c08)) * add retry handler, rate limiter, and provider resilience ([#100](#100)) ([b890545](b890545)) * add SecOps security agent with rule engine, audit log, and ToolInvoker integration ([#40](#40)) ([83b7b6c](83b7b6c)) * add shared org memory and memory consolidation/archival ([#125](#125), [#48](#48)) ([4a0832b](4a0832b)) * design unified provider interface ([#86](#86)) ([3e23d64](3e23d64)) * expand template presets, rosters, and add inheritance ([#80](#80), [#81](#81), [#84](#84)) ([15a9134](15a9134)) * implement agent runtime state vs immutable config split ([#115](#115)) ([4cb1ca5](4cb1ca5)) * implement AgentEngine core orchestrator ([#11](#11)) ([#143](#143)) ([f2eb73a](f2eb73a)) * implement AuditRepository for security audit log persistence ([#279](#279)) ([94bc29f](94bc29f)) * implement basic tool system (registry, invocation, results) ([#15](#15)) ([c51068b](c51068b)) * implement built-in file system tools ([#18](#18)) ([325ef98](325ef98)) * implement communication foundation — message bus, dispatcher, and messenger ([#157](#157)) ([8e71bfd](8e71bfd)) * implement company template system with 7 built-in presets ([#85](#85)) ([cbf1496](cbf1496)) * implement conflict resolution protocol ([#122](#122)) ([#166](#166)) ([e03f9f2](e03f9f2)) * implement core entity and role system models ([#69](#69)) ([acf9801](acf9801)) * implement crash recovery with fail-and-reassign strategy ([#149](#149)) ([e6e91ed](e6e91ed)) * implement engine extensions — Plan-and-Execute loop and call categorization ([#134](#134), [#135](#135)) ([#159](#159)) ([9b2699f](9b2699f)) * implement enterprise logging system with structlog ([#73](#73)) ([2f787e5](2f787e5)) * implement graceful shutdown with cooperative timeout strategy ([#130](#130)) ([6592515](6592515)) * implement hierarchical delegation and loop prevention ([#12](#12), [#17](#17)) ([6be60b6](6be60b6)) * implement LiteLLM driver and provider registry ([#88](#88)) ([ae3f18b](ae3f18b)), closes [#4](#4) * implement LLM decomposition strategy and workspace isolation ([#174](#174)) ([aa0eefe](aa0eefe)) * implement meeting protocol system ([#123](#123)) ([ee7caca](ee7caca)) * implement message and communication domain models ([#74](#74)) ([560a5d2](560a5d2)) * implement model routing engine ([#99](#99)) ([d3c250b](d3c250b)) * implement parallel agent execution ([#22](#22)) ([#161](#161)) ([65940b3](65940b3)) * implement per-call cost tracking service ([#7](#7)) ([#102](#102)) ([c4f1f1c](c4f1f1c)) * implement personality injection and system prompt construction ([#105](#105)) ([934dd85](934dd85)) * implement single-task execution lifecycle ([#21](#21)) ([#144](#144)) ([c7e64e4](c7e64e4)) * implement subprocess sandbox for tool execution isolation ([#131](#131)) ([#153](#153)) ([3c8394e](3c8394e)) * implement task assignment subsystem with pluggable strategies ([#172](#172)) ([c7f1b26](c7f1b26)), closes [#26](#26) [#30](#30) * implement task decomposition and routing engine ([#14](#14)) ([9c7fb52](9c7fb52)) * implement Task, Project, Artifact, Budget, and Cost domain models ([#71](#71)) ([81eabf1](81eabf1)) * implement tool permission checking ([#16](#16)) ([833c190](833c190)) * implement YAML config loader with Pydantic validation ([#59](#59)) ([ff3a2ba](ff3a2ba)) * implement YAML config loader with Pydantic validation ([#75](#75)) ([ff3a2ba](ff3a2ba)) * initialize project with uv, hatchling, and src layout ([39005f9](39005f9)) * initialize project with uv, hatchling, and src layout ([#62](#62)) ([39005f9](39005f9)) * Litestar REST API, WebSocket feed, and approval queue (M6) ([#189](#189)) ([29fcd08](29fcd08)) * make TokenUsage.total_tokens a computed field ([#118](#118)) ([c0bab18](c0bab18)), closes [#109](#109) * parallel tool execution in ToolInvoker.invoke_all ([#137](#137)) ([58517ee](58517ee)) * testing framework, CI pipeline, and M0 gap fixes ([#64](#64)) ([f581749](f581749)) * wire all modules into observability system ([#97](#97)) ([f7a0617](f7a0617)) ### Bug Fixes * address Greptile post-merge review findings from PRs [#170](https://github.com/Aureliolo/ai-company/issues/170)-[#175](https://github.com/Aureliolo/ai-company/issues/175) ([#176](#176)) ([c5ca929](c5ca929)) * address post-merge review feedback from PRs [#164](https://github.com/Aureliolo/ai-company/issues/164)-[#167](https://github.com/Aureliolo/ai-company/issues/167) ([#170](#170)) ([3bf897a](3bf897a)), closes [#169](#169) * enforce strict mypy on test files ([#89](#89)) ([aeeff8c](aeeff8c)) * harden Docker sandbox, MCP bridge, and code runner ([#50](#50), [#53](#53)) ([d5e1b6e](d5e1b6e)) * harden git tools security + code quality improvements ([#150](#150)) ([000a325](000a325)) * harden subprocess cleanup, env filtering, and shutdown resilience ([#155](#155)) ([d1fe1fb](d1fe1fb)) * incorporate post-merge feedback + pre-PR review fixes ([#164](#164)) ([c02832a](c02832a)) * pre-PR review fixes for post-merge findings ([#183](#183)) ([26b3108](26b3108)) * resolve circular imports, bump litellm, fix release tag format ([#286](#286)) ([a6659b5](a6659b5)) * strengthen immutability for BaseTool schema and ToolInvoker boundaries ([#117](#117)) ([7e5e861](7e5e861)) ### Performance * harden non-inferable principle implementation ([#195](#195)) ([02b5f4e](02b5f4e)), closes [#188](#188) ### Refactoring * adopt NotBlankStr across all models ([#108](#108)) ([#120](#120)) ([ef89b90](ef89b90)) * extract _SpendingTotals base class from spending summary models ([#111](#111)) ([2f39c1b](2f39c1b)) * harden BudgetEnforcer with error handling, validation extraction, and review fixes ([#182](#182)) ([c107bf9](c107bf9)) * harden personality profiles, department validation, and template rendering ([#158](#158)) ([10b2299](10b2299)) * pre-PR review improvements for ExecutionLoop + ReAct loop ([#124](#124)) ([8dfb3c0](8dfb3c0)) * split events.py into per-domain event modules ([#136](#136)) ([e9cba89](e9cba89)) ### Documentation * add ADR-001 memory layer evaluation and selection ([#178](#178)) ([db3026f](db3026f)), closes [#39](#39) * add agent scaling research findings to DESIGN_SPEC ([#145](#145)) ([57e487b](57e487b)) * add CLAUDE.md, contributing guide, and dev documentation ([#65](#65)) ([55c1025](55c1025)), closes [#54](#54) * add crash recovery, sandboxing, analytics, and testing decisions ([#127](#127)) ([5c11595](5c11595)) * address external review feedback with MVP scope and new protocols ([#128](#128)) ([3b30b9a](3b30b9a)) * expand design spec with pluggable strategy protocols ([#121](#121)) ([6832db6](6832db6)) * finalize 23 design decisions (ADR-002) ([#190](#190)) ([8c39742](8c39742)) * update project docs for M2.5 conventions and add docs-consistency review agent ([#114](#114)) ([99766ee](99766ee)) ### Tests * add e2e single agent integration tests ([#24](#24)) ([#156](#156)) ([f566fb4](f566fb4)) * add provider adapter integration tests ([#90](#90)) ([40a61f4](40a61f4)) ### CI/CD * add Release Please for automated versioning and GitHub Releases ([#278](#278)) ([a488758](a488758)) * bump actions/checkout from 4 to 6 ([#95](#95)) ([1897247](1897247)) * bump actions/upload-artifact from 4 to 7 ([#94](#94)) ([27b1517](27b1517)) * bump anchore/scan-action from 6.5.1 to 7.3.2 ([#271](#271)) ([80a1c15](80a1c15)) * bump docker/build-push-action from 6.19.2 to 7.0.0 ([#273](#273)) ([dd0219e](dd0219e)) * bump docker/login-action from 3.7.0 to 4.0.0 ([#272](#272)) ([33d6238](33d6238)) * bump docker/metadata-action from 5.10.0 to 6.0.0 ([#270](#270)) ([baee04e](baee04e)) * bump docker/setup-buildx-action from 3.12.0 to 4.0.0 ([#274](#274)) ([5fc06f7](5fc06f7)) * bump sigstore/cosign-installer from 3.9.1 to 4.1.0 ([#275](#275)) ([29dd16c](29dd16c)) * harden CI/CD pipeline ([#92](#92)) ([ce4693c](ce4693c)) * split vulnerability scans into critical-fail and high-warn tiers ([#277](#277)) ([aba48af](aba48af)) ### Maintenance * add /worktree skill for parallel worktree management ([#171](#171)) ([951e337](951e337)) * add design spec context loading to research-link skill ([8ef9685](8ef9685)) * add post-merge-cleanup skill ([#70](#70)) ([f913705](f913705)) * add pre-pr-review skill and update CLAUDE.md ([#103](#103)) ([92e9023](92e9023)) * add research-link skill and rename skill files to SKILL.md ([#101](#101)) ([651c577](651c577)) * bump aiosqlite from 0.21.0 to 0.22.1 ([#191](#191)) ([3274a86](3274a86)) * bump pyyaml from 6.0.2 to 6.0.3 in the minor-and-patch group ([#96](#96)) ([0338d0c](0338d0c)) * bump ruff from 0.15.4 to 0.15.5 ([a49ee46](a49ee46)) * fix M0 audit items ([#66](#66)) ([c7724b5](c7724b5)) * **main:** release ai-company 0.1.1 ([#282](#282)) ([2f4703d](2f4703d)) * pin setup-uv action to full SHA ([#281](#281)) ([4448002](4448002)) * post-audit cleanup — PEP 758, loggers, bug fixes, refactoring, tests, hookify rules ([#148](#148)) ([c57a6a9](c57a6a9)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Signed-off-by: Aurelio <19254254+Aureliolo@users.noreply.github.com>
Summary
Implements issue #59 — YAML config loading system for the
config/module. This is critical-priority M1 work that unblocks #60 (templates).config/errors.py—ConfigErrorhierarchy withConfigLocationcontext (file path, key path, line/column).ConfigValidationErrorformats per-field errors with YAML source locationsconfig/schema.py—RootConfigaggregating all sub-configs (BudgetConfig,CommunicationConfig,CompanyConfig,LogConfig). New config-level models:AgentConfig,ProviderConfig,ProviderModelConfig,RoutingConfig,RoutingRuleConfig. Cross-field validation (unique agents/departments, routing references valid provider models)config/defaults.py—default_config_dict()base-layer defaultsconfig/loader.py—load_config()andload_config_from_string()with layered deep merge (defaults → primary → overrides),yaml.safe_load()parsing, YAML AST line mapping viayaml.compose(), Pydantic validation with error enrichmentconfig/__init__.py— Public API exportspyyaml==6.0.2(runtime),types-pyyaml==6.0.12.20250915(dev stubs)# TODO(#59)fromobservability/config.pyTest plan
__str__formatting, inheritanceload_config(explicit path, layered override, defaults, validation error with location, frozen result),load_config_from_string(minimal, full, invalid, defaults merged)RootConfigCloses #59