feat: implement core entity and role system models#69
Merged
Conversation
Add Pydantic v2 domain models for M1 Phase 1: - 10 StrEnum types (SeniorityLevel, AgentStatus, CostTier, etc.) - Role models: Skill, Authority, SeniorityInfo, Role, CustomRole - Agent models: PersonalityConfig, SkillSet, ModelConfig, MemoryConfig, ToolPermissions, AgentIdentity - Company models: Team, Department, CompanyConfig, HRRegistry, Company with budget validation - Built-in role catalog: 31 roles, 8 seniority mappings, lookup functions - Move pydantic from dev to runtime dependencies - CostTier uses string IDs (not dollar signs) for extensibility - 190 tests, 99% coverage, mypy strict clean
Validators: ToolPermissions overlap, Company duplicate depts, Department duplicate teams, HRRegistry duplicate agents, MemoryConfig type/retention consistency, SkillSet empty strings, CustomRole empty department. min_length: Authority.reports_to, ModelConfig.fallback_model, CustomRole.department. Code: O(1) dict lookups in role_catalog, float budget comparison with rounding, use variable in error message, rename CustomRole.skills to required_skills. Docs: Department.name flexibility, Intern/Junior comment, qualified module ref, SeniorityInfo cost_tier, Team lead/members clarification. Tests: -> None on all test methods, synthetic model IDs, empty role/department tests, SeniorityInfo validation, CustomRole JSON roundtrip, Authority JSON roundtrip, budget boundary, seniority None path. 214 tests, 100% coverage.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (5)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds immutable Pydantic v2 domain models (enums, agents, roles, company structures), a built-in role catalog with lookup helpers, exposes a consolidated core public API, moves pydantic to runtime dependencies in pyproject.toml, and adds extensive test factories and unit tests covering the new models. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
Contributor
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 establishes the foundational data structures for the AI company framework by introducing a comprehensive set of Pydantic v2 domain models. These models robustly define the core entities of agents, company organization, and a detailed role system, complete with skills, authority, and seniority levels. The changes provide a robust and immutable data layer, crucial for the project's future development, and include extensive validation to maintain data integrity across the system. 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 the foundational Pydantic v2 “core” domain layer (enums, role system, agent identity/config, company structure/config) plus a built-in role catalog, and updates dependencies/tests so the rest of the framework can build on these immutable models.
Changes:
- Introduce core Pydantic models for roles/skills/seniority, agents, and company structure/config.
- Add a built-in role catalog + O(1) lookup helpers for roles and seniority mappings.
- Move
pydantic==2.12.5to runtime dependencies and add comprehensive unit tests for the new core layer.
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
pyproject.toml |
Moves Pydantic to runtime dependencies (core models require it at runtime). |
uv.lock |
Reflects the dependency move so lock metadata matches runtime requirements. |
src/ai_company/core/enums.py |
Adds core StrEnum types used across the domain models. |
src/ai_company/core/role.py |
Adds role/skill/seniority domain models (immutable). |
src/ai_company/core/role_catalog.py |
Adds built-in role catalog + seniority mappings + lookup functions. |
src/ai_company/core/agent.py |
Adds agent identity + model/memory/tool/personality configs (immutable). |
src/ai_company/core/company.py |
Adds company/org structure models + budget/uniqueness validators (immutable). |
src/ai_company/core/__init__.py |
Re-exports core models and catalog helpers as the public core API surface. |
tests/unit/core/conftest.py |
Adds Polyfactory factories + sample fixtures for core model tests. |
tests/unit/core/test_enums.py |
Adds unit tests for enum member counts/values and Pydantic integration. |
tests/unit/core/test_role.py |
Adds unit tests for role/skill/seniority models and validation behavior. |
tests/unit/core/test_role_catalog.py |
Adds unit tests for built-in catalogs and lookup behavior. |
tests/unit/core/test_agent.py |
Adds unit tests for agent identity/config models and validators. |
tests/unit/core/test_company.py |
Adds unit tests for company/org models and budget/uniqueness validators. |
tests/unit/core/__init__.py |
Initializes the new tests.unit.core package. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+6
to
+22
| class SeniorityLevel(StrEnum): | ||
| """Seniority levels for agents within the organization. | ||
|
|
||
| Maps to authority scope, typical model tier, and cost tier. | ||
| See ``ai_company.core.role_catalog.SENIORITY_INFO`` for the full mapping. | ||
| """ | ||
|
|
||
| # Design spec says "Intern/Junior" — collapsed to a single JUNIOR level. | ||
| JUNIOR = "junior" | ||
| MID = "mid" | ||
| SENIOR = "senior" | ||
| LEAD = "lead" | ||
| PRINCIPAL = "principal" | ||
| DIRECTOR = "director" | ||
| VP = "vp" | ||
| C_SUITE = "c_suite" | ||
|
|
There was a problem hiding this comment.
SeniorityLevel enum values are lowercase/underscore (e.g. "c_suite"), but DESIGN_SPEC examples use human-facing forms like "Senior" and "C-Suite". If these models are meant to validate YAML matching the spec, parsing will currently fail unless callers pre-normalize. Consider accepting common variants (case-insensitive, hyphen vs underscore) via pre-validators on fields using SeniorityLevel, or a custom enum validator that normalizes input before enum coercion.
Comment on lines
+150
to
+157
| name: str = Field(min_length=1, description="Custom role name") | ||
| department: DepartmentName | str = Field( | ||
| description="Department (standard or custom name)", | ||
| ) | ||
| required_skills: tuple[str, ...] = Field( | ||
| default=(), | ||
| description="Required skills for this role", | ||
| ) |
There was a problem hiding this comment.
CustomRole uses the field name required_skills, but DESIGN_SPEC’s YAML example for custom_roles uses the key skills. Without an alias, CustomRole.model_validate(...) from YAML will ignore/reject skills, breaking the stated config/YAML compatibility. Consider renaming the field or adding a validation/serialization alias so skills is accepted (and ideally emitted) while keeping the internal attribute name consistent.
Comment on lines
+23
to
+25
|
|
||
| name: str = Field(min_length=1, description="Skill name") | ||
| category: SkillCategory = Field(description="Skill category") |
There was a problem hiding this comment.
Skill.name only enforces min_length=1, which still allows whitespace-only names (e.g. " "). Since other parts of the PR explicitly reject whitespace-only skill names (e.g. SkillSet), it would be more consistent to also strip/reject whitespace-only values here (via a field_validator or a stricter constraint).
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive and well-structured set of Pydantic models for the core domain of the application. The models are immutable and include validation, which is excellent for ensuring data integrity. The code is clean, well-documented, and accompanied by a thorough test suite. My review includes suggestions to improve the efficiency of some validation logic.
Comment on lines
+65
to
+68
| if len(names) != len(set(names)): | ||
| dupes = sorted({n for n in names if names.count(n) > 1}) | ||
| msg = f"Duplicate team names in department {self.name!r}: {dupes}" | ||
| raise ValueError(msg) |
Contributor
There was a problem hiding this comment.
The method used to find duplicate team names is inefficient. The expression {n for n in names if names.count(n) > 1} has a time complexity of O(N^2) because names.count() is O(N) and it's inside a comprehension that iterates over names. A more efficient approach would be to use collections.Counter to find duplicates in O(N) time.
Suggested change
| if len(names) != len(set(names)): | |
| dupes = sorted({n for n in names if names.count(n) > 1}) | |
| msg = f"Duplicate team names in department {self.name!r}: {dupes}" | |
| raise ValueError(msg) | |
| if len(names) != len(set(names)): | |
| from collections import Counter | |
| dupes = sorted(n for n, c in Counter(names).items() if c > 1) | |
| msg = f"Duplicate team names in department {self.name!r}: {dupes}" | |
| raise ValueError(msg) |
Comment on lines
+134
to
+137
| if len(agents) != len(set(agents)): | ||
| dupes = sorted({a for a in agents if agents.count(a) > 1}) | ||
| msg = f"Duplicate entries in active_agents: {dupes}" | ||
| raise ValueError(msg) |
Contributor
There was a problem hiding this comment.
The method for finding duplicate agents is inefficient, with a time complexity of O(N^2) due to using count() inside a loop. A more performant approach would be to use collections.Counter.
Suggested change
| if len(agents) != len(set(agents)): | |
| dupes = sorted({a for a in agents if agents.count(a) > 1}) | |
| msg = f"Duplicate entries in active_agents: {dupes}" | |
| raise ValueError(msg) | |
| if len(agents) != len(set(agents)): | |
| from collections import Counter | |
| dupes = sorted(a for a, c in Counter(agents).items() if c > 1) | |
| msg = f"Duplicate entries in active_agents: {dupes}" | |
| raise ValueError(msg) |
Comment on lines
+182
to
+185
| if len(names) != len(set(names)): | ||
| dupes = sorted({n for n in names if names.count(n) > 1}) | ||
| msg = f"Duplicate department names: {dupes}" | ||
| raise ValueError(msg) |
Contributor
There was a problem hiding this comment.
This duplicate detection logic is inefficient (O(N^2)). Using collections.Counter would be more performant (O(N)) for finding duplicate department names.
Suggested change
| if len(names) != len(set(names)): | |
| dupes = sorted({n for n in names if names.count(n) > 1}) | |
| msg = f"Duplicate department names: {dupes}" | |
| raise ValueError(msg) | |
| if len(names) != len(set(names)): | |
| from collections import Counter | |
| dupes = sorted(n for n, c in Counter(names).items() if c > 1) | |
| msg = f"Duplicate department names: {dupes}" | |
| raise ValueError(msg) |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/core/agent.py`:
- Around line 97-114: Several identifier fields (provider, model_id,
fallback_model and user-facing name, role, department) currently allow
whitespace-only strings; add Pydantic validators on those fields (e.g., using
`@validator` or field-level constr/regex) to reject values where value.strip() ==
"" and raise a ValueError with a clear message. Update the model that defines
provider, model_id, fallback_model and the model/class that defines name, role,
department to run the same validation logic so whitespace-only inputs are
rejected at parsing time (refer to the field names provider, model_id,
fallback_model, name, role, department to locate the code). Ensure
fallback_model remains optional but if present it must not be whitespace-only.
In `@src/ai_company/core/company.py`:
- Around line 24-29: The model currently allows whitespace-only strings and
empty tuple entries for identifiers (e.g., the Company fields name, lead, and
members), so update those Pydantic field definitions to trim and validate
values: replace plain str uses with constrained strings (e.g., constr/Field with
strip_whitespace=True and min_length=1) for single-value fields (name, lead,
etc.), and for tuple/list reference fields (members and other tuple fields)
constrain each item (use Tuple[constr(...), ...] or a conlist of constrained
strings) or add a field validator that strips whitespace and rejects any item
that becomes empty; ensure validation raises clear errors when an identifier is
empty after stripping.
In `@src/ai_company/core/role_catalog.py`:
- Around line 391-407: The module currently builds _BUILTIN_ROLES_BY_NAME by
lowercasing names and can silently overwrite roles whose names differ only by
case; add a fail-fast assertion at import that detects case-normalized
collisions before constructing _BUILTIN_ROLES_BY_NAME (e.g., compute
[r.name.lower() for r in BUILTIN_ROLES], verify length matches the set length,
and if not raise an AssertionError listing the conflicting names) so lookups via
get_builtin_role remain deterministic and accidental duplicates are caught
early.
In `@src/ai_company/core/role.py`:
- Around line 110-121: Role.required_skills, Role.tool_access and
CustomRole.required_skills allow whitespace-only entries; add Pydantic
validators on the Role and CustomRole models that iterate the tuple items, strip
each string, reject empty/whitespace-only values and optionally enforce
identifier format (e.g. alphanumeric/underscore) and then return a tuple of
cleaned values; implement separate validators named like
validate_required_skills and validate_tool_access (or use field validators for
"required_skills" and "tool_access") which raise ValueError on invalid items so
invalid config is rejected at model validation.
In `@tests/unit/core/conftest.py`:
- Around line 36-183: Add Google-style docstrings to all public ModelFactory
classes (e.g., SkillFactory, AuthorityFactory, SeniorityInfoFactory,
RoleFactory, CustomRoleFactory, PersonalityConfigFactory, SkillSetFactory,
ModelConfigFactory, MemoryConfigFactory, ToolPermissionsFactory,
AgentIdentityFactory, TeamFactory, DepartmentFactory, CompanyConfigFactory,
HRRegistryFactory, CompanyFactory) and to all fixture functions (sample_skill,
sample_authority, sample_role, sample_model_config, sample_agent,
sample_department, sample_company); for each class include a one-line
description of what the factory produces and any important defaults, and for
each fixture add a brief description plus a Returns: section stating the return
type (e.g., Returns: Skill) following Google docstring style, placed immediately
after the def/class line.
In `@tests/unit/core/test_agent.py`:
- Around line 39-356: Add Google-style docstrings to each public test class and
its public test methods (TestPersonalityConfig, TestSkillSet, TestModelConfig,
TestMemoryConfig, TestToolPermissions, TestAgentIdentity and their test_*
methods) by inserting a short one- or two-sentence description for the class and
for each test method include a brief docstring with Args (if any) and what the
test asserts/returns; ensure docstrings are triple-quoted, placed immediately
under the class or def line, and follow the Google style (summary line, optional
Args/Returns sections) so linting/coverage recognizes them as public
documentation.
ℹ️ 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 (14)
pyproject.tomlsrc/ai_company/core/__init__.pysrc/ai_company/core/agent.pysrc/ai_company/core/company.pysrc/ai_company/core/enums.pysrc/ai_company/core/role.pysrc/ai_company/core/role_catalog.pytests/unit/core/__init__.pytests/unit/core/conftest.pytests/unit/core/test_agent.pytests/unit/core/test_company.pytests/unit/core/test_enums.pytests/unit/core/test_role.pytests/unit/core/test_role_catalog.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: Nofrom __future__ import annotations— Python 3.14+ has PEP 649 native lazy annotations
Include type hints on all public functions in Python
Use Google-style docstrings on all public classes and functions in Python
Create new objects instead of mutating existing ones — maintain immutability
Use Pydantic v2BaseModel,model_validator, andConfigDictfor data models
Enforce 88-character line length in Python code
Keep functions under 50 lines of code
Keep Python files under 800 lines
Handle errors explicitly, never silently swallow exceptions
Validate at system boundaries — user input, external APIs, config files
Files:
src/ai_company/core/enums.pytests/unit/core/test_agent.pytests/unit/core/conftest.pytests/unit/core/test_enums.pysrc/ai_company/core/agent.pysrc/ai_company/core/role.pytests/unit/core/test_company.pytests/unit/core/test_role.pysrc/ai_company/core/__init__.pytests/unit/core/test_role_catalog.pysrc/ai_company/core/role_catalog.pysrc/ai_company/core/company.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Maintain minimum 80% code coverage across test suite
Runmypyin strict mode on all source Python files for type checking
Files:
src/ai_company/core/enums.pysrc/ai_company/core/agent.pysrc/ai_company/core/role.pysrc/ai_company/core/__init__.pysrc/ai_company/core/role_catalog.pysrc/ai_company/core/company.py
{src,tests}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Run
ruff checkandruff formaton all source and test Python files
Files:
src/ai_company/core/enums.pytests/unit/core/test_agent.pytests/unit/core/conftest.pytests/unit/core/test_enums.pysrc/ai_company/core/agent.pysrc/ai_company/core/role.pytests/unit/core/test_company.pytests/unit/core/test_role.pysrc/ai_company/core/__init__.pytests/unit/core/test_role_catalog.pysrc/ai_company/core/role_catalog.pysrc/ai_company/core/company.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, and@pytest.mark.slowmarkers on test functions
Set test timeout to 30 seconds per test
Files:
tests/unit/core/test_agent.pytests/unit/core/conftest.pytests/unit/core/test_enums.pytests/unit/core/test_company.pytests/unit/core/test_role.pytests/unit/core/test_role_catalog.py
pyproject.toml
📄 CodeRabbit inference engine (CLAUDE.md)
pyproject.toml: Pin all dependency versions using==operator inpyproject.toml
Organize dependencies into groups:test,dev(includes test + linting/type-checking tools) withuv syncinstalling all
Files:
pyproject.toml
🧠 Learnings (23)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-27T18:32:19.626Z
Learning: Applies to **/*.py : Use Pydantic v2 `BaseModel`, `model_validator`, and `ConfigDict` for data models
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 : Validate all LLM outputs by checking required fields, verifying data types, handling malformed responses, and using Pydantic models for structured data
📚 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/core/test_agent.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 {src/agents/**/*.py,src/services/**/*.py,src/memory/**/*.py,src/utils/**/*.py,src/settings.py} : Core modules (`src/agents/`, `src/services/`, `src/memory/`, `src/utils/`, `src/settings.py`) must maintain 100% test coverage
Applied to files:
tests/unit/core/test_agent.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: Core modules (`agents/`, `services/`, `workflows/`, `memory/`, `utils/`, `settings.py`) must maintain 100% test coverage; UI components (`ui/`) are excluded from coverage requirements
Applied to files:
tests/unit/core/test_agent.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 {src/agents/**/*.py,src/services/**/*.py,src/memory/**/*.py,src/utils/**/*.py,src/settings.py} : 100% test coverage is MANDATORY for every commit on core modules (`src/agents/`, `src/services/`, `src/memory/`, `src/utils/`, `src/settings.py`), CI enforces this coverage requirement
Applied to files:
tests/unit/core/test_agent.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 : In agent tests, mock Ollama API calls using `unittest.mock` and patch `agents.base.ollama.Client`
Applied to files:
tests/unit/core/test_agent.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 src/agents/**/*.py : Agents must extend `BaseAgent`, use retry logic, and implement configurable timeout via settings.
Applied to files:
tests/unit/core/test_agent.pysrc/ai_company/core/agent.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 {src/agents/**/*.py,src/services/**/*.py,src/memory/**/*.py,src/utils/**/*.py,src/settings.py} : 100% test coverage is MANDATORY for every commit. The CI enforces 100% coverage on core modules (`src/agents/`, `src/services/`, `src/memory/`, `src/utils/`, `src/settings.py`).
Applied to files:
tests/unit/core/test_agent.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 : Validate all LLM outputs by checking required fields, verifying data types, handling malformed responses, and using Pydantic models for structured data
Applied to files:
tests/unit/core/test_agent.pysrc/ai_company/core/agent.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/core/test_agent.pytests/unit/core/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/core/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 **/tests/conftest.py : Place shared pytest fixtures in `tests/conftest.py`
Applied to files:
tests/unit/core/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/core/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/core/conftest.py
📚 Learning: 2026-02-27T18:32:19.627Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-27T18:32:19.627Z
Learning: Applies to pyproject.toml : Organize dependencies into groups: `test`, `dev` (includes test + linting/type-checking tools) with `uv sync` installing all
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 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-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 **/*.py : Require Python 3.14+
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 **/*.py : Python version: 3.14+
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: Applies to {src/agents/**/*.py,src/services/model_service.py} : Respect existing model configuration patterns in Ollama integration
Applied to files:
src/ai_company/core/agent.py
📚 Learning: 2026-02-27T18:32:19.627Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-27T18:32:19.627Z
Learning: Applies to src/ai_company/memory/**/*.py : Use Mem0 adapter for persistent agent memory in the `memory/` module
Applied to files:
src/ai_company/core/agent.py
📚 Learning: 2026-02-27T18:32:19.627Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-27T18:32:19.627Z
Learning: Applies to src/ai_company/tools/**/*.py : Use MCP integration for tool registry with role-based access control in the `tools/` module
Applied to files:
src/ai_company/core/role.pysrc/ai_company/core/role_catalog.py
📚 Learning: 2026-02-27T18:32:19.627Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-27T18:32:19.627Z
Learning: Applies to src/ai_company/config/**/*.py : Use YAML for company configuration loading and validation in the `config/` module
Applied to files:
tests/unit/core/test_company.pysrc/ai_company/core/company.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/core/test_role.py
🧬 Code graph analysis (9)
tests/unit/core/test_agent.py (3)
src/ai_company/core/agent.py (6)
AgentIdentity(175-233)MemoryConfig(117-143)ModelConfig(84-114)PersonalityConfig(18-51)SkillSet(54-81)ToolPermissions(146-172)src/ai_company/core/role.py (1)
Authority(32-61)tests/unit/core/conftest.py (7)
MemoryConfigFactory(69-71)ModelConfigFactory(64-66)PersonalityConfigFactory(56-57)SkillSetFactory(60-61)ToolPermissionsFactory(74-75)sample_model_config(138-145)sample_agent(149-158)
tests/unit/core/conftest.py (2)
src/ai_company/core/enums.py (4)
DepartmentName(107-118)MemoryType(48-54)ProficiencyLevel(98-104)SkillCategory(84-95)src/ai_company/core/role.py (3)
Authority(32-61)Role(91-129)Skill(13-29)
tests/unit/core/test_enums.py (1)
src/ai_company/core/enums.py (10)
AgentStatus(24-29)CompanyType(71-81)CostTier(57-68)CreativityLevel(40-45)DepartmentName(107-118)MemoryType(48-54)ProficiencyLevel(98-104)RiskTolerance(32-37)SeniorityLevel(6-21)SkillCategory(84-95)
src/ai_company/core/agent.py (2)
src/ai_company/core/enums.py (5)
AgentStatus(24-29)CreativityLevel(40-45)MemoryType(48-54)RiskTolerance(32-37)SeniorityLevel(6-21)src/ai_company/core/role.py (1)
Authority(32-61)
src/ai_company/core/role.py (1)
src/ai_company/core/enums.py (4)
DepartmentName(107-118)ProficiencyLevel(98-104)SeniorityLevel(6-21)SkillCategory(84-95)
tests/unit/core/test_role.py (2)
src/ai_company/core/enums.py (4)
DepartmentName(107-118)ProficiencyLevel(98-104)SeniorityLevel(6-21)SkillCategory(84-95)src/ai_company/core/role.py (4)
Authority(32-61)Role(91-129)SeniorityInfo(64-88)Skill(13-29)
tests/unit/core/test_role_catalog.py (3)
src/ai_company/core/enums.py (3)
CostTier(57-68)DepartmentName(107-118)SeniorityLevel(6-21)src/ai_company/core/role.py (2)
Role(91-129)SeniorityInfo(64-88)src/ai_company/core/role_catalog.py (2)
get_builtin_role(398-407)get_seniority_info(410-421)
src/ai_company/core/role_catalog.py (2)
src/ai_company/core/enums.py (3)
CostTier(57-68)DepartmentName(107-118)SeniorityLevel(6-21)src/ai_company/core/role.py (2)
Role(91-129)SeniorityInfo(64-88)
src/ai_company/core/company.py (1)
src/ai_company/core/enums.py (1)
CompanyType(71-81)
🔇 Additional comments (9)
pyproject.toml (1)
15-17: Runtime dependency move is correct and cleanly pinned.
pydanticis now available at runtime with an exact version pin, which matches the new core model usage.src/ai_company/core/enums.py (1)
6-118: Enum layer is well-structured and consistent.The
StrEnumdefinitions are clear, documented, and provide a solid typed foundation for downstream models.src/ai_company/core/company.py (1)
61-69: Validator coverage for duplicates and budget caps is strong.The post-model validation logic for duplicate teams, duplicate active agents, and aggregate budget limits is clear and explicit.
Also applies to: 130-139, 177-196
tests/unit/core/test_enums.py (1)
21-151: Enum tests are comprehensive and well-scoped.This suite covers cardinality, value semantics, construction failures, and JSON/model roundtrips with good depth.
tests/unit/core/test_role.py (1)
25-257: Role model test coverage looks strong.Validation paths, immutability behavior, and serialization roundtrips are all exercised across the key role-domain entities.
tests/unit/core/test_company.py (1)
26-269: Company model tests are well-targeted.Great coverage for aggregate budget rules, duplicate detection, defaults, immutability, and JSON roundtrip behavior.
src/ai_company/core/role_catalog.py (1)
16-386: Catalog composition and lookup API are solid.The immutable tuples plus precomputed maps provide clear, efficient behavior for built-in role and seniority retrieval.
Also applies to: 393-421
tests/unit/core/test_role_catalog.py (1)
23-195: Role catalog tests provide excellent regression safety.The suite validates structure, uniqueness, lookup semantics, and immutability across both catalog and seniority mappings.
src/ai_company/core/__init__.py (1)
44-75:__all__export surface looks clean and consistent.The explicit export list is comprehensive and aligned with the imported core symbols and lookup helpers.
Comment on lines
+36
to
+183
| class SkillFactory(ModelFactory): | ||
| __model__ = Skill | ||
|
|
||
|
|
||
| class AuthorityFactory(ModelFactory): | ||
| __model__ = Authority | ||
|
|
||
|
|
||
| class SeniorityInfoFactory(ModelFactory): | ||
| __model__ = SeniorityInfo | ||
|
|
||
|
|
||
| class RoleFactory(ModelFactory): | ||
| __model__ = Role | ||
|
|
||
|
|
||
| class CustomRoleFactory(ModelFactory): | ||
| __model__ = CustomRole | ||
|
|
||
|
|
||
| class PersonalityConfigFactory(ModelFactory): | ||
| __model__ = PersonalityConfig | ||
|
|
||
|
|
||
| class SkillSetFactory(ModelFactory): | ||
| __model__ = SkillSet | ||
|
|
||
|
|
||
| class ModelConfigFactory(ModelFactory): | ||
| __model__ = ModelConfig | ||
| temperature = 0.7 | ||
|
|
||
|
|
||
| class MemoryConfigFactory(ModelFactory): | ||
| __model__ = MemoryConfig | ||
| type = MemoryType.SESSION | ||
|
|
||
|
|
||
| class ToolPermissionsFactory(ModelFactory): | ||
| __model__ = ToolPermissions | ||
|
|
||
|
|
||
| class AgentIdentityFactory(ModelFactory): | ||
| __model__ = AgentIdentity | ||
|
|
||
|
|
||
| class TeamFactory(ModelFactory): | ||
| __model__ = Team | ||
|
|
||
|
|
||
| class DepartmentFactory(ModelFactory): | ||
| __model__ = Department | ||
| budget_percent = 10.0 | ||
|
|
||
|
|
||
| class CompanyConfigFactory(ModelFactory): | ||
| __model__ = CompanyConfig | ||
|
|
||
|
|
||
| class HRRegistryFactory(ModelFactory): | ||
| __model__ = HRRegistry | ||
|
|
||
|
|
||
| class CompanyFactory(ModelFactory): | ||
| __model__ = Company | ||
| departments = () | ||
|
|
||
|
|
||
| # ── Sample Fixtures ──────────────────────────────────────────────── | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def sample_skill() -> Skill: | ||
| return Skill( | ||
| name="python", | ||
| category=SkillCategory.ENGINEERING, | ||
| proficiency=ProficiencyLevel.ADVANCED, | ||
| ) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def sample_authority() -> Authority: | ||
| return Authority( | ||
| can_approve=("code_reviews",), | ||
| reports_to="engineering_lead", | ||
| can_delegate_to=("junior_developers",), | ||
| budget_limit=5.0, | ||
| ) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def sample_role() -> Role: | ||
| return Role( | ||
| name="Backend Developer", | ||
| department=DepartmentName.ENGINEERING, | ||
| required_skills=("python", "apis", "databases"), | ||
| authority_level=SeniorityLevel.MID, | ||
| description="APIs, business logic, databases", | ||
| ) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def sample_model_config() -> ModelConfig: | ||
| return ModelConfig( | ||
| provider="test-provider", | ||
| model_id="test-model-sonnet-4-6", | ||
| temperature=0.3, | ||
| max_tokens=8192, | ||
| fallback_model="test-provider/test-model-haiku", | ||
| ) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def sample_agent(sample_model_config: ModelConfig) -> AgentIdentity: | ||
| return AgentIdentity( | ||
| id=uuid4(), | ||
| name="Sarah Chen", | ||
| role="Senior Backend Developer", | ||
| department="Engineering", | ||
| level=SeniorityLevel.SENIOR, | ||
| model=sample_model_config, | ||
| hiring_date=date(2026, 2, 27), | ||
| ) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def sample_department() -> Department: | ||
| return Department( | ||
| name="Engineering", | ||
| head="cto", | ||
| budget_percent=60.0, | ||
| teams=( | ||
| Team( | ||
| name="backend", | ||
| lead="backend_lead", | ||
| members=("sr_backend_1", "mid_backend_1"), | ||
| ), | ||
| ), | ||
| ) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def sample_company(sample_department: Department) -> Company: | ||
| return Company( | ||
| name="Test Corp", | ||
| departments=(sample_department,), | ||
| config=CompanyConfig(budget_monthly=100.0), | ||
| ) |
There was a problem hiding this comment.
Public factories and fixtures need Google-style docstrings.
The public ModelFactory classes and fixture functions in this module are missing docstrings.
As per coding guidelines, "Use Google-style docstrings on all public classes and functions in Python."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/core/conftest.py` around lines 36 - 183, Add Google-style
docstrings to all public ModelFactory classes (e.g., SkillFactory,
AuthorityFactory, SeniorityInfoFactory, RoleFactory, CustomRoleFactory,
PersonalityConfigFactory, SkillSetFactory, ModelConfigFactory,
MemoryConfigFactory, ToolPermissionsFactory, AgentIdentityFactory, TeamFactory,
DepartmentFactory, CompanyConfigFactory, HRRegistryFactory, CompanyFactory) and
to all fixture functions (sample_skill, sample_authority, sample_role,
sample_model_config, sample_agent, sample_department, sample_company); for each
class include a one-line description of what the factory produces and any
important defaults, and for each fixture add a brief description plus a Returns:
section stating the return type (e.g., Returns: Skill) following Google
docstring style, placed immediately after the def/class line.
- Reject whitespace-only strings in all identifier fields (ModelConfig, AgentIdentity, Team, Department, Company, CompanyConfig) - Reject empty/whitespace entries in all tuple[str,...] fields (ToolPermissions, PersonalityConfig, Role, CustomRole, Authority, HRRegistry, CompanyConfig, Team) - Normalize whitespace in CustomRole department field validator - Use collections.Counter for O(N) duplicate detection - Add collision guard for case-normalized builtin role names - Strip whitespace and use casefold in get_builtin_role lookup - Change get_seniority_info to raise LookupError instead of returning None - Fix AgentIdentityFactory to wire MemoryConfigFactory/ToolPermissionsFactory - Improve docstrings: SeniorityLevel, CostTier, MemoryConfig, Company, Team, role_catalog module - Extract budget rounding magic number to named constant - Add 39 new tests covering all validation gaps and edge cases - Fix CustomRole roundtrip test to use assert restored == role Sources: code-reviewer, test-analyzer, silent-failure-hunter, comment-analyzer, type-design-analyzer, Copilot, Gemini, CodeRabbit
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Owner
Author
|
/gemini review |
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive set of Pydantic models for the core domain entities, including agents, companies, and roles, along with a catalog of built-in roles and extensive unit tests. The implementation is of high quality, with robust validation and clear structure. My feedback focuses on a minor improvement for maintainability across the new Pydantic models by adopting the Self type hint for model validators, which is a recommended practice in Pydantic v2.
| from datetime import date # noqa: TC003 — required at runtime by Pydantic | ||
| from uuid import UUID, uuid4 | ||
|
|
||
| from pydantic import BaseModel, ConfigDict, Field, model_validator |
Contributor
There was a problem hiding this comment.
To use the Self type hint for model validators as per Pydantic v2 best practices, you'll need to import it. This improves maintainability by not hardcoding class names in return types.
Suggested change
| from pydantic import BaseModel, ConfigDict, Field, model_validator | |
| from pydantic import BaseModel, ConfigDict, Field, model_validator, Self |
src/ai_company/core/agent.py
Outdated
| ) | ||
|
|
||
| @model_validator(mode="after") | ||
| def _validate_no_empty_traits(self) -> PersonalityConfig: |
Contributor
There was a problem hiding this comment.
For better maintainability, it's recommended to use Self for the return type hint in model_validator methods instead of the explicit class name. This applies to all other model validators in this file as well (lines 83, 126, 157, 185, 195, 265).
Suggested change
| def _validate_no_empty_traits(self) -> PersonalityConfig: | |
| def _validate_no_empty_traits(self) -> Self: |
| from collections import Counter | ||
| from uuid import UUID, uuid4 | ||
|
|
||
| from pydantic import BaseModel, ConfigDict, Field, model_validator |
Contributor
There was a problem hiding this comment.
To use the Self type hint for model validators as per Pydantic v2 best practices, you'll need to import it. This improves maintainability by not hardcoding class names in return types.
Suggested change
| from pydantic import BaseModel, ConfigDict, Field, model_validator | |
| from pydantic import BaseModel, ConfigDict, Field, model_validator, Self |
src/ai_company/core/company.py
Outdated
| ) | ||
|
|
||
| @model_validator(mode="after") | ||
| def _validate_strings(self) -> Team: |
Contributor
There was a problem hiding this comment.
For better maintainability, it's recommended to use Self for the return type hint in model_validator methods instead of the explicit class name. This applies to all other model validators in this file as well (lines 79, 88, 132, 169, 222, 230).
Suggested change
| def _validate_strings(self) -> Team: | |
| def _validate_strings(self) -> Self: |
| @@ -0,0 +1,210 @@ | |||
| """Role and skill domain models.""" | |||
|
|
|||
| from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator | |||
Contributor
There was a problem hiding this comment.
To use the Self type hint for model validators as per Pydantic v2 best practices, you'll need to import it. This improves maintainability by not hardcoding class names in return types.
Suggested change
| from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator | |
| from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator, Self |
src/ai_company/core/role.py
Outdated
| ) | ||
|
|
||
| @model_validator(mode="after") | ||
| def _validate_no_empty_strings(self) -> Authority: |
Contributor
There was a problem hiding this comment.
For better maintainability, it's recommended to use Self for the return type hint in model_validator methods instead of the explicit class name. This applies to other model validators in this file as well (lines 142, 204).
Suggested change
| def _validate_no_empty_strings(self) -> Authority: | |
| def _validate_no_empty_strings(self) -> Self: |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 15 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+22
to
+26
| model_config = ConfigDict(frozen=True) | ||
|
|
||
| name: str = Field(min_length=1, description="Skill name") | ||
| category: SkillCategory = Field(description="Skill category") | ||
| proficiency: ProficiencyLevel = Field( |
There was a problem hiding this comment.
Skill.name only uses min_length=1, which still allows whitespace-only values (e.g. " ") to validate. This can lead to unusable identifiers downstream; consider adding a validator (or a stricter constraint) that rejects/normalizes whitespace-only skill names, consistent with the .strip() checks used in other core models.
Comment on lines
+48
to
+52
| reports_to: str | None = Field( | ||
| default=None, | ||
| min_length=1, | ||
| description="Role this position reports to", | ||
| ) |
There was a problem hiding this comment.
Authority.reports_to has min_length=1, but whitespace-only strings (e.g. " ") still pass validation. If reports_to is meant to be a meaningful identifier like other string IDs in this module, add a validator to reject or normalize whitespace-only values when non-None.
Comment on lines
+99
to
+100
|
|
||
|
|
There was a problem hiding this comment.
SeniorityInfo.authority_scope, typical_model_tier, and cost_tier use min_length=1 but do not reject whitespace-only strings. This makes it possible to create entries that technically validate but carry no information; consider adding .strip()-based validation similar to other models in core/.
Suggested change
| @field_validator("authority_scope", "typical_model_tier", "cost_tier", mode="before") | |
| def _strip_and_validate_non_empty(cls, value: str) -> str: | |
| """Normalize strings and reject empty/whitespace-only values.""" | |
| if not isinstance(value, str): | |
| return value | |
| stripped = value.strip() | |
| if not stripped: | |
| raise ValueError("Value cannot be empty or whitespace-only") | |
| return stripped |
Comment on lines
+116
to
+135
| name: str = Field(min_length=1, description="Role name") | ||
| department: DepartmentName = Field( | ||
| description="Department this role belongs to", | ||
| ) | ||
| required_skills: tuple[str, ...] = Field( | ||
| default=(), | ||
| description="Skills required for this role", | ||
| ) | ||
| authority_level: SeniorityLevel = Field( | ||
| default=SeniorityLevel.MID, | ||
| description="Default seniority level for this role", | ||
| ) | ||
| tool_access: tuple[str, ...] = Field( | ||
| default=(), | ||
| description="Tools available to this role", | ||
| ) | ||
| system_prompt_template: str | None = Field( | ||
| default=None, | ||
| description="Template file for system prompt", | ||
| ) |
There was a problem hiding this comment.
Role.name and optional system_prompt_template can still be whitespace-only values because only min_length (for name) is enforced and no .strip() validation is applied. Consider rejecting/normalizing whitespace-only values for these identifier-like fields for consistency with AgentIdentity, Team, etc.
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (1)
tests/unit/core/conftest.py (1)
36-187:⚠️ Potential issue | 🟡 MinorAdd Google-style docstrings to public factories and fixtures.
The public
ModelFactoryclasses and fixture functions are still missing docstrings in this module.As per coding guidelines, "Use Google-style docstrings on all public classes and functions in Python."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/core/conftest.py` around lines 36 - 187, Add Google-style docstrings to each public ModelFactory class (e.g., SkillFactory, AuthorityFactory, SeniorityInfoFactory, RoleFactory, CustomRoleFactory, PersonalityConfigFactory, SkillSetFactory, ModelConfigFactory, MemoryConfigFactory, ToolPermissionsFactory, AgentIdentityFactory, TeamFactory, DepartmentFactory, CompanyConfigFactory, HRRegistryFactory, CompanyFactory) and to each pytest fixture function (e.g., sample_skill, sample_authority, sample_role, sample_model_config, sample_agent, sample_department, sample_company). For each class include a one-line summary, a short description of what model it builds and any default attributes (like temperature on ModelConfigFactory or type on MemoryConfigFactory); for each fixture include a one-line summary, parameters (if any) and the return type. Use Google-style sections (Args, Returns) where applicable and ensure docstrings are triple-quoted strings immediately under the class or function definition.
🤖 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/core/agent.py`:
- Around line 53-60: The post-validator _validate_no_empty_traits currently only
verifies entries in traits; extend it to also validate communication_style by
checking that self.communication_style is not None/empty or whitespace-only and
raise a ValueError with a clear message if it is; update the method (decorated
with `@model_validator`(mode="after") in agent.py) to perform both checks so
communication_style cannot be a whitespace-only string.
In `@src/ai_company/core/company.py`:
- Around line 88-95: The duplicate check in _validate_unique_team_names treats
differently-cased or whitespace-padded names as distinct; normalize team names
(e.g., name.strip().casefold() or .lower()) before building the names list and
before counting duplicates so "Engineering" and " engineering " are treated the
same, then use that normalized list with Counter to detect dupes and raise the
ValueError; optionally include the original offending raw names in the error
message by mapping back to the originals for clarity.
In `@src/ai_company/core/role.py`:
- Around line 141-149: The current post-validation in
Role._validate_no_empty_strings only rejects whitespace-only entries for tuple
fields but misses scalar identifiers; extend or add a model_validator
(mode="after") on Role and CustomRole to also strip-and-reject whitespace-only
for Role.name, CustomRole.name, and CustomRole.suggested_model, raising
ValueError with a clear message; update the existing _validate_no_empty_strings
(or create a similar method) to iterate these scalar attributes in addition to
required_skills and tool_access so whitespace-only names cannot be set in Role
or CustomRole.
In `@tests/unit/core/test_agent.py`:
- Around line 38-39: The TestPersonalityConfig test class is missing the
required 30-second timeout; add the pytest timeout decorator by prepending
`@pytest.mark.timeout`(30) to the TestPersonalityConfig class definition (or
alternatively configure a global timeout in pytest.ini/pyproject.toml) so that
tests under the TestPersonalityConfig class inherit the 30s limit.
In `@tests/unit/core/test_company.py`:
- Around line 26-338: Add a module-level pytest timeout marker by defining
pytestmark = pytest.mark.timeout(30) at the top of this test module
(module-level scope in tests/unit/core/test_company.py) to enforce a 30s
per-test timeout; ensure pytest is imported (tests reference pytest elsewhere)
so the pytest.mark.timeout symbol is available. Place the pytestmark definition
near the other top-level declarations so it applies to all
TestTeam/TestDepartment/TestCompanyConfig/TestHRRegistry/TestCompany classes and
their tests.
In `@tests/unit/core/test_role_catalog.py`:
- Around line 53-58: Move the repeated local import of ValidationError into the
module-level imports: add "from pydantic import ValidationError" at the top of
the test module, then remove the inline "from pydantic import ValidationError"
lines inside the test functions (e.g., test_all_entries_frozen and
test_all_roles_frozen) so they use the top-level ValidationError symbol instead.
- Around line 23-24: Add the pytest timeout marker to the test class by
decorating TestSeniorityInfo with `@pytest.mark.timeout`(30); locate the
TestSeniorityInfo class definition and place the decorator immediately above it
(use the pytest.mark.timeout import if not present), ensuring each test in that
class gets the 30-second timeout per coding guidelines.
- Around line 24-201: Add Google-style docstrings to the public test classes and
their test methods: TestSeniorityInfo, TestBuiltinRoles, TestGetBuiltinRole, and
TestGetSeniorityInfo (and key test methods such as test_has_8_entries,
test_c_suite_is_premium_cost, test_has_31_roles, test_exact_match,
test_all_levels_lookupable, etc.). For each class add a one-line class docstring
summarizing what the test group validates, and for each test method add a short
docstring describing the specific assertion/behavior being tested (use the
Google-style sections where appropriate: Args/Returns not needed for simple
tests). Ensure docstrings appear directly under the class or def signatures so
they are picked up as the object's __doc__.
In `@tests/unit/core/test_role.py`:
- Around line 25-295: The module lacks the required 30-second pytest timeout
marker; add a module-level marker by defining pytestmark =
pytest.mark.timeout(30) near the top (where pytest is already used/imported) so
it applies to all tests in this file (affecting TestSkill, TestAuthority,
TestSeniorityInfo, TestRole, TestCustomRole, etc.); ensure pytest is imported in
the file scope before setting pytestmark.
---
Duplicate comments:
In `@tests/unit/core/conftest.py`:
- Around line 36-187: Add Google-style docstrings to each public ModelFactory
class (e.g., SkillFactory, AuthorityFactory, SeniorityInfoFactory, RoleFactory,
CustomRoleFactory, PersonalityConfigFactory, SkillSetFactory,
ModelConfigFactory, MemoryConfigFactory, ToolPermissionsFactory,
AgentIdentityFactory, TeamFactory, DepartmentFactory, CompanyConfigFactory,
HRRegistryFactory, CompanyFactory) and to each pytest fixture function (e.g.,
sample_skill, sample_authority, sample_role, sample_model_config, sample_agent,
sample_department, sample_company). For each class include a one-line summary, a
short description of what model it builds and any default attributes (like
temperature on ModelConfigFactory or type on MemoryConfigFactory); for each
fixture include a one-line summary, parameters (if any) and the return type. Use
Google-style sections (Args, Returns) where applicable and ensure docstrings are
triple-quoted strings immediately under the class or function definition.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
src/ai_company/core/agent.pysrc/ai_company/core/company.pysrc/ai_company/core/enums.pysrc/ai_company/core/role.pysrc/ai_company/core/role_catalog.pytests/unit/core/conftest.pytests/unit/core/test_agent.pytests/unit/core/test_company.pytests/unit/core/test_role.pytests/unit/core/test_role_catalog.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 (4)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations— Python 3.14+ has PEP 649 native lazy annotations
Include type hints on all public functions in Python
Use Google-style docstrings on all public classes and functions in Python
Create new objects instead of mutating existing ones — maintain immutability
Use Pydantic v2BaseModel,model_validator, andConfigDictfor data models
Enforce 88-character line length in Python code
Keep functions under 50 lines of code
Keep Python files under 800 lines
Handle errors explicitly, never silently swallow exceptions
Validate at system boundaries — user input, external APIs, config files
Files:
tests/unit/core/test_agent.pytests/unit/core/test_role.pytests/unit/core/test_role_catalog.pysrc/ai_company/core/role.pysrc/ai_company/core/agent.pysrc/ai_company/core/enums.pysrc/ai_company/core/role_catalog.pytests/unit/core/test_company.pytests/unit/core/conftest.pysrc/ai_company/core/company.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, and@pytest.mark.slowmarkers on test functions
Set test timeout to 30 seconds per test
Files:
tests/unit/core/test_agent.pytests/unit/core/test_role.pytests/unit/core/test_role_catalog.pytests/unit/core/test_company.pytests/unit/core/conftest.py
{src,tests}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Run
ruff checkandruff formaton all source and test Python files
Files:
tests/unit/core/test_agent.pytests/unit/core/test_role.pytests/unit/core/test_role_catalog.pysrc/ai_company/core/role.pysrc/ai_company/core/agent.pysrc/ai_company/core/enums.pysrc/ai_company/core/role_catalog.pytests/unit/core/test_company.pytests/unit/core/conftest.pysrc/ai_company/core/company.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Maintain minimum 80% code coverage across test suite
Runmypyin strict mode on all source Python files for type checking
Files:
src/ai_company/core/role.pysrc/ai_company/core/agent.pysrc/ai_company/core/enums.pysrc/ai_company/core/role_catalog.pysrc/ai_company/core/company.py
🧠 Learnings (26)
📓 Common learnings
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 : Validate all LLM outputs by checking required fields, verifying data types, handling malformed responses, and using Pydantic models for structured data
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-27T18:32:19.626Z
Learning: Applies to **/*.py : Use Pydantic v2 `BaseModel`, `model_validator`, and `ConfigDict` for data models
📚 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/core/test_agent.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 {src/agents/**/*.py,src/services/**/*.py,src/memory/**/*.py,src/utils/**/*.py,src/settings.py} : Core modules (`src/agents/`, `src/services/`, `src/memory/`, `src/utils/`, `src/settings.py`) must maintain 100% test coverage
Applied to files:
tests/unit/core/test_agent.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: Core modules (`agents/`, `services/`, `workflows/`, `memory/`, `utils/`, `settings.py`) must maintain 100% test coverage; UI components (`ui/`) are excluded from coverage requirements
Applied to files:
tests/unit/core/test_agent.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 {src/agents/**/*.py,src/services/**/*.py,src/memory/**/*.py,src/utils/**/*.py,src/settings.py} : 100% test coverage is MANDATORY for every commit on core modules (`src/agents/`, `src/services/`, `src/memory/`, `src/utils/`, `src/settings.py`), CI enforces this coverage requirement
Applied to files:
tests/unit/core/test_agent.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 : In agent tests, mock Ollama API calls using `unittest.mock` and patch `agents.base.ollama.Client`
Applied to files:
tests/unit/core/test_agent.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 {src/agents/**/*.py,src/services/**/*.py,src/memory/**/*.py,src/utils/**/*.py,src/settings.py} : 100% test coverage is MANDATORY for every commit. The CI enforces 100% coverage on core modules (`src/agents/`, `src/services/`, `src/memory/`, `src/utils/`, `src/settings.py`).
Applied to files:
tests/unit/core/test_agent.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 : Validate all LLM outputs by checking required fields, verifying data types, handling malformed responses, and using Pydantic models for structured data
Applied to files:
tests/unit/core/test_agent.pysrc/ai_company/core/agent.pysrc/ai_company/core/company.py
📚 Learning: 2026-02-27T18:32:19.626Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-27T18:32:19.626Z
Learning: Applies to **/*.py : Use Google-style docstrings on all public classes and functions in Python
Applied to files:
tests/unit/core/test_agent.pytests/unit/core/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 : Use docstrings for classes and complex functions
Applied to files:
tests/unit/core/test_agent.pytests/unit/core/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 : Keep docstrings up to date with code changes
Applied to files:
tests/unit/core/test_agent.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/core/test_agent.pytests/unit/core/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/core/test_agent.pytests/unit/core/test_company.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 : Document complex algorithms and business logic in code comments or docstrings
Applied to files:
tests/unit/core/test_agent.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/core/test_role.pysrc/ai_company/core/agent.py
📚 Learning: 2026-02-27T18:32:19.627Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-27T18:32:19.627Z
Learning: Applies to src/ai_company/tools/**/*.py : Use MCP integration for tool registry with role-based access control in the `tools/` module
Applied to files:
src/ai_company/core/role.py
📚 Learning: 2026-02-27T18:32:19.627Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-27T18:32:19.627Z
Learning: Applies to **/*.py : Validate at system boundaries — user input, external APIs, config files
Applied to files:
src/ai_company/core/role.pysrc/ai_company/core/agent.pysrc/ai_company/core/company.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 {src/agents/**/*.py,src/services/model_service.py} : Respect existing model configuration patterns in Ollama integration
Applied to files:
src/ai_company/core/agent.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 src/agents/**/*.py : Agents must extend `BaseAgent`, use retry logic, and implement configurable timeout via settings.
Applied to files:
src/ai_company/core/agent.py
📚 Learning: 2026-02-27T18:32:19.627Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-27T18:32:19.627Z
Learning: Applies to src/ai_company/memory/**/*.py : Use Mem0 adapter for persistent agent memory in the `memory/` module
Applied to files:
src/ai_company/core/agent.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:
src/ai_company/core/agent.pytests/unit/core/conftest.py
📚 Learning: 2026-02-27T18:32:19.627Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-27T18:32:19.627Z
Learning: Applies to src/ai_company/config/**/*.py : Use YAML for company configuration loading and validation in the `config/` module
Applied to files:
tests/unit/core/test_company.pysrc/ai_company/core/company.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/core/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/core/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/core/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/*.py : Be aware of concurrent agent execution - don't modify shared state without synchronization and use thread-safe data structures when needed
Applied to files:
src/ai_company/core/company.py
🧬 Code graph analysis (8)
tests/unit/core/test_agent.py (3)
src/ai_company/core/agent.py (6)
AgentIdentity(204-271)MemoryConfig(136-162)ModelConfig(93-133)PersonalityConfig(18-60)SkillSet(63-90)ToolPermissions(165-201)src/ai_company/core/enums.py (5)
AgentStatus(24-29)CreativityLevel(40-45)MemoryType(48-54)RiskTolerance(32-37)SeniorityLevel(6-21)tests/unit/core/conftest.py (6)
AgentIdentityFactory(80-83)MemoryConfigFactory(69-71)ModelConfigFactory(64-66)PersonalityConfigFactory(56-57)SkillSetFactory(60-61)ToolPermissionsFactory(74-77)
tests/unit/core/test_role.py (1)
src/ai_company/core/role.py (5)
Authority(32-71)CustomRole(152-210)Role(101-149)SeniorityInfo(74-98)Skill(13-29)
tests/unit/core/test_role_catalog.py (3)
src/ai_company/core/enums.py (3)
CostTier(57-69)DepartmentName(108-119)SeniorityLevel(6-21)src/ai_company/core/role.py (2)
Role(101-149)SeniorityInfo(74-98)src/ai_company/core/role_catalog.py (2)
get_builtin_role(401-410)get_seniority_info(413-429)
src/ai_company/core/role.py (1)
src/ai_company/core/enums.py (3)
ProficiencyLevel(99-105)SeniorityLevel(6-21)SkillCategory(85-96)
src/ai_company/core/agent.py (3)
src/ai_company/core/enums.py (5)
AgentStatus(24-29)CreativityLevel(40-45)MemoryType(48-54)RiskTolerance(32-37)SeniorityLevel(6-21)src/ai_company/core/role.py (1)
Authority(32-71)src/ai_company/core/company.py (1)
_validate_non_blank_identifiers(79-85)
src/ai_company/core/role_catalog.py (2)
src/ai_company/core/enums.py (3)
CostTier(57-69)DepartmentName(108-119)SeniorityLevel(6-21)src/ai_company/core/role.py (2)
Role(101-149)SeniorityInfo(74-98)
tests/unit/core/conftest.py (4)
src/ai_company/core/agent.py (6)
AgentIdentity(204-271)MemoryConfig(136-162)ModelConfig(93-133)PersonalityConfig(18-60)SkillSet(63-90)ToolPermissions(165-201)src/ai_company/core/company.py (3)
Department(49-95)HRRegistry(144-181)Team(14-46)src/ai_company/core/enums.py (5)
DepartmentName(108-119)MemoryType(48-54)ProficiencyLevel(99-105)SeniorityLevel(6-21)SkillCategory(85-96)src/ai_company/core/role.py (4)
Authority(32-71)CustomRole(152-210)Role(101-149)Skill(13-29)
src/ai_company/core/company.py (2)
src/ai_company/core/enums.py (1)
CompanyType(72-82)src/ai_company/core/agent.py (2)
_validate_non_blank_identifiers(126-133)_validate_non_blank_identifiers(265-271)
🔇 Additional comments (6)
src/ai_company/core/enums.py (1)
6-119: Looks good — enum surface is clear and consistent.The enum definitions are cohesive, well-documented, and fit the shared domain model contract.
src/ai_company/core/role_catalog.py (1)
391-429: Lookup map hardening and retrieval APIs are solid.Import-time collision protection plus normalized lookup behavior are correctly implemented.
tests/unit/core/test_agent.py (2)
39-467: Add Google-style docstrings to public test classes and methods.Test classes (
TestPersonalityConfig,TestSkillSet,TestModelConfig,TestMemoryConfig,TestToolPermissions,TestAgentIdentity) and their public test methods lack docstrings.As per coding guidelines: "Use Google-style docstrings on all public classes and functions in Python."
1-467: LGTM! Comprehensive test coverage for agent models.The test suite thoroughly validates:
- Default values and custom configurations
- Validation rules (empty strings, whitespace-only, boundary conditions, cross-field consistency)
- Immutability via
frozen=Trueenforcement- JSON serialization round-trips with nested data
- Factory-based test data generation
Test cases align well with the validators defined in the source models.
tests/unit/core/test_role_catalog.py (2)
192-201: LGTM! Good use ofpatch.dictto test error handling.The mock correctly clears
_SENIORITY_INFO_BY_LEVELto simulate a missing catalog entry, allowing verification thatLookupErroris raised with the expected message. This is a clean way to test the error path without modifying production data.
1-201: Well-structured test coverage for role catalog.The test suite effectively validates:
- Catalog invariants (8 seniority levels, 31 roles, no duplicates, all departments covered)
- Lookup function behavior (case-insensitivity, whitespace stripping, error handling)
- Immutability of catalog entries
- Parametrized exhaustive checks for all roles and seniority levels
The tests align well with the lookup functions in
src/ai_company/core/role_catalog.py.
src/ai_company/core/company.py
Outdated
Comment on lines
+88
to
+95
| def _validate_unique_team_names(self) -> Department: | ||
| """Ensure no duplicate team names within a department.""" | ||
| names = [t.name for t in self.teams] | ||
| if len(names) != len(set(names)): | ||
| dupes = sorted(n for n, c in Counter(names).items() if c > 1) | ||
| msg = f"Duplicate team names in department {self.name!r}: {dupes}" | ||
| raise ValueError(msg) | ||
| return self |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Normalize names before duplicate checks to prevent semantic duplicates.
Current duplicate checks treat "Engineering" and " engineering " (or case variants) as distinct values. Normalizing before comparison makes uniqueness enforcement robust.
♻️ Suggested refactor
`@model_validator`(mode="after")
def _validate_unique_team_names(self) -> Department:
"""Ensure no duplicate team names within a department."""
- names = [t.name for t in self.teams]
- if len(names) != len(set(names)):
- dupes = sorted(n for n, c in Counter(names).items() if c > 1)
+ names = [t.name.strip().casefold() for t in self.teams]
+ if len(names) != len(set(names)):
+ dupes = sorted(n for n, c in Counter(names).items() if c > 1)
msg = f"Duplicate team names in department {self.name!r}: {dupes}"
raise ValueError(msg)
return self
@@
`@model_validator`(mode="after")
def _validate_departments(self) -> Company:
"""Validate department names are unique and budgets do not exceed 100%."""
# Unique department names
- names = [d.name for d in self.departments]
+ names = [d.name.strip().casefold() for d in self.departments]
if len(names) != len(set(names)):
dupes = sorted(n for n, c in Counter(names).items() if c > 1)
msg = f"Duplicate department names: {dupes}"
raise ValueError(msg)Also applies to: 230-237
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ai_company/core/company.py` around lines 88 - 95, The duplicate check in
_validate_unique_team_names treats differently-cased or whitespace-padded names
as distinct; normalize team names (e.g., name.strip().casefold() or .lower())
before building the names list and before counting duplicates so "Engineering"
and " engineering " are treated the same, then use that normalized list with
Counter to detect dupes and raise the ValueError; optionally include the
original offending raw names in the error message by mapping back to the
originals for clarity.
Consolidates feedback from 5 local review agents (code-reviewer, pr-test-analyzer, silent-failure-hunter, comment-analyzer, type-design-analyzer) and 3 external reviewers (Copilot, Gemini, CodeRabbit). Key changes: - Add consistent whitespace-only validation across all models (Skill, Authority, SeniorityInfo, Role, CustomRole, PersonalityConfig) - Make ToolPermissions overlap check case-insensitive via casefold() - Add duplicate member detection to Team model - Add 16 new tests covering all validation gaps - Add __all__ exports verification test - Fix docstrings for accuracy (role_catalog, enums, CustomRole)
- Replace concrete class return types with `Self` in all model_validator methods across role.py, agent.py, and company.py (Pydantic v2 best practice) - Move ValidationError import to module level in test_role_catalog.py
Owner
Author
|
/gemini review |
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive and well-designed set of Pydantic models for the core domain, including agents, companies, roles, and skills. The code is of high quality, featuring immutable data structures, thorough validation, and an extensive test suite that achieves 100% coverage. My review focuses on minor maintainability improvements to reduce boilerplate code by leveraging more of Pydantic's declarative validation features, and a small style suggestion for the __init__.py file to improve readability.
Comment on lines
+44
to
+75
| __all__ = [ | ||
| "BUILTIN_ROLES", | ||
| "SENIORITY_INFO", | ||
| "AgentIdentity", | ||
| "AgentStatus", | ||
| "Authority", | ||
| "Company", | ||
| "CompanyConfig", | ||
| "CompanyType", | ||
| "CostTier", | ||
| "CreativityLevel", | ||
| "CustomRole", | ||
| "Department", | ||
| "DepartmentName", | ||
| "HRRegistry", | ||
| "MemoryConfig", | ||
| "MemoryType", | ||
| "ModelConfig", | ||
| "PersonalityConfig", | ||
| "ProficiencyLevel", | ||
| "RiskTolerance", | ||
| "Role", | ||
| "SeniorityInfo", | ||
| "SeniorityLevel", | ||
| "Skill", | ||
| "SkillCategory", | ||
| "SkillSet", | ||
| "Team", | ||
| "ToolPermissions", | ||
| "get_builtin_role", | ||
| "get_seniority_info", | ||
| ] |
Contributor
There was a problem hiding this comment.
For better readability and maintainability, it's a good practice to sort the __all__ list. A common convention is to group symbols by type (e.g., constants, classes, functions) and then sort alphabetically within each group. This makes it easier to navigate as the number of exported symbols grows.
__all__ = [
# Constants
"BUILTIN_ROLES",
"SENIORITY_INFO",
# Enums & Models
"AgentIdentity",
"AgentStatus",
"Authority",
"Company",
"CompanyConfig",
"CompanyType",
"CostTier",
"CreativityLevel",
"CustomRole",
"Department",
"DepartmentName",
"HRRegistry",
"MemoryConfig",
"MemoryType",
"ModelConfig",
"PersonalityConfig",
"ProficiencyLevel",
"RiskTolerance",
"Role",
"SeniorityInfo",
"SeniorityLevel",
"Skill",
"SkillCategory",
"SkillSet",
"Team",
"ToolPermissions",
# Functions
"get_builtin_role",
"get_seniority_info",
]
Comment on lines
+32
to
+64
| traits: tuple[str, ...] = Field( | ||
| default=(), | ||
| description="Personality traits", | ||
| ) | ||
| communication_style: str = Field( | ||
| default="neutral", | ||
| min_length=1, | ||
| description="Communication style description", | ||
| ) | ||
| risk_tolerance: RiskTolerance = Field( | ||
| default=RiskTolerance.MEDIUM, | ||
| description="Risk tolerance level", | ||
| ) | ||
| creativity: CreativityLevel = Field( | ||
| default=CreativityLevel.MEDIUM, | ||
| description="Creativity level", | ||
| ) | ||
| description: str = Field( | ||
| default="", | ||
| description="Extended personality description", | ||
| ) | ||
|
|
||
| @model_validator(mode="after") | ||
| def _validate_no_empty_traits(self) -> Self: | ||
| """Ensure no empty or whitespace-only traits or communication_style.""" | ||
| if not self.communication_style.strip(): | ||
| msg = "communication_style must not be whitespace-only" | ||
| raise ValueError(msg) | ||
| for trait in self.traits: | ||
| if not trait.strip(): | ||
| msg = "Empty or whitespace-only entry in traits" | ||
| raise ValueError(msg) | ||
| return self |
Contributor
There was a problem hiding this comment.
There's a recurring pattern of using model_validator across several models in this file (PersonalityConfig, SkillSet, ModelConfig, etc.) to prevent fields from being empty or containing only whitespace. This leads to boilerplate code.
You can simplify this significantly by using Pydantic's Annotated types with StringConstraints. This approach is more declarative and reduces code duplication.
First, define a reusable NonEmptyStr type at the top of the file (after imports):
from typing import Annotated
from pydantic import StringConstraints
NonEmptyStr = Annotated[str, StringConstraints(strip_whitespace=True, min_length=1)]Then, you can apply this type to the model fields and remove the now-redundant _validate_no_empty_traits validator. This pattern can be applied to the other models in this file as well.
traits: tuple[NonEmptyStr, ...] = Field(
default=(),
description="Personality traits",
)
communication_style: NonEmptyStr = Field(
default="neutral",
description="Communication style description",
)
risk_tolerance: RiskTolerance = Field(
default=RiskTolerance.MEDIUM,
description="Risk tolerance level",
)
creativity: CreativityLevel = Field(
default=CreativityLevel.MEDIUM,
description="Creativity level",
)
description: str = Field(
default="",
description="Extended personality description",
)
Comment on lines
+29
to
+51
| name: str = Field(min_length=1, description="Team name") | ||
| lead: str = Field(min_length=1, description="Team lead agent name") | ||
| members: tuple[str, ...] = Field( | ||
| default=(), | ||
| description="Team member agent names", | ||
| ) | ||
|
|
||
| @model_validator(mode="after") | ||
| def _validate_strings(self) -> Self: | ||
| """Ensure no empty or whitespace-only names in identifiers and members.""" | ||
| for field_name in ("name", "lead"): | ||
| if not getattr(self, field_name).strip(): | ||
| msg = f"{field_name} must not be whitespace-only" | ||
| raise ValueError(msg) | ||
| for member in self.members: | ||
| if not member.strip(): | ||
| msg = "Empty or whitespace-only entry in members" | ||
| raise ValueError(msg) | ||
| if len(self.members) != len(set(self.members)): | ||
| dupes = sorted(m for m, c in Counter(self.members).items() if c > 1) | ||
| msg = f"Duplicate members in team {self.name!r}: {dupes}" | ||
| raise ValueError(msg) | ||
| return self |
Contributor
There was a problem hiding this comment.
Similar to other models in this PR, there's an opportunity to reduce boilerplate validation code. The _validate_strings validator can be replaced by using a Pydantic Annotated type with StringConstraints. This makes the model definitions more declarative and easier to maintain.
You can define a reusable NonEmptyStr type at the top of the file:
from typing import Annotated
from pydantic import StringConstraints
NonEmptyStr = Annotated[str, StringConstraints(strip_whitespace=True, min_length=1)]Then, you can apply this to the fields in Team. The check for duplicate members is still valuable and should be kept, but it can be in its own focused validator.
name: NonEmptyStr = Field(description="Team name")
lead: NonEmptyStr = Field(description="Team lead agent name")
members: tuple[NonEmptyStr, ...] = Field(
default=(),
description="Team member agent names",
)
@model_validator(mode="after")
def _validate_unique_members(self) -> Self:
"""Ensure no duplicate members in the team."""
if len(self.members) != len(set(self.members)):
dupes = sorted(m for m, c in Counter(self.members).items() if c > 1)
msg = f"Duplicate members in team {self.name!r}: {dupes}"
raise ValueError(msg)
return self
Comment on lines
+26
to
+39
| name: str = Field(min_length=1, description="Skill name") | ||
| category: SkillCategory = Field(description="Skill category") | ||
| proficiency: ProficiencyLevel = Field( | ||
| default=ProficiencyLevel.INTERMEDIATE, | ||
| description="Proficiency level", | ||
| ) | ||
|
|
||
| @model_validator(mode="after") | ||
| def _validate_name_not_blank(self) -> Self: | ||
| """Ensure skill name is not whitespace-only.""" | ||
| if not self.name.strip(): | ||
| msg = "Skill name must not be whitespace-only" | ||
| raise ValueError(msg) | ||
| return self |
Contributor
There was a problem hiding this comment.
This model, and others in this file, use custom validators to check for non-blank strings. This pattern can be simplified using Pydantic's Annotated types, which reduces boilerplate and improves maintainability.
You can define a reusable NonEmptyStr type at the top of the file:
from typing import Annotated
from pydantic import StringConstraints
NonEmptyStr = Annotated[str, StringConstraints(strip_whitespace=True, min_length=1)]Applying this to the Skill model's name field allows you to remove the _validate_name_not_blank validator entirely. This refactoring can be applied to other models in this file as well.
Suggested change
| name: str = Field(min_length=1, description="Skill name") | |
| category: SkillCategory = Field(description="Skill category") | |
| proficiency: ProficiencyLevel = Field( | |
| default=ProficiencyLevel.INTERMEDIATE, | |
| description="Proficiency level", | |
| ) | |
| @model_validator(mode="after") | |
| def _validate_name_not_blank(self) -> Self: | |
| """Ensure skill name is not whitespace-only.""" | |
| if not self.name.strip(): | |
| msg = "Skill name must not be whitespace-only" | |
| raise ValueError(msg) | |
| return self | |
| name: NonEmptyStr = Field(description="Skill name") | |
| category: SkillCategory = Field(description="Skill category") | |
| proficiency: ProficiencyLevel = Field( | |
| default=ProficiencyLevel.INTERMEDIATE, | |
| description="Proficiency level", | |
| ) | |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 15 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from ai_company.core.enums import CompanyType | ||
|
|
||
| _BUDGET_ROUNDING_PRECISION = 10 | ||
| """Decimal places for budget sum rounding; avoids IEEE 754 float artifacts.""" |
There was a problem hiding this comment.
The string literal under _BUDGET_ROUNDING_PRECISION is a standalone expression (not a docstring) and will be flagged by Ruff (e.g., B018 "useless expression"). Convert it to a normal comment (or #: style variable comment) so linting passes.
Suggested change
| """Decimal places for budget sum rounding; avoids IEEE 754 float artifacts.""" | |
| # Decimal places for budget sum rounding; avoids IEEE 754 float artifacts. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (7)
tests/unit/core/test_role.py (1)
3-5:⚠️ Potential issue | 🟡 MinorAdd a 30-second timeout marker for this test module.
This module still lacks
pytest.mark.timeout(30)and can run without the required timeout cap.🔧 Suggested fix
import pytest from pydantic import ValidationError +pytestmark = pytest.mark.timeout(30) + from ai_company.core.enums import (As per coding guidelines, "Set test timeout to 30 seconds per test."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/core/test_role.py` around lines 3 - 5, Add a module-level 30s timeout marker to this test module by declaring pytestmark = pytest.mark.timeout(30) near the top of tests/unit/core/test_role.py (after the existing imports: pytest and ValidationError) so all tests in the module inherit the 30‑second timeout; ensure you import pytest (already present) and place the pytestmark assignment at module scope, not inside any test or function.src/ai_company/core/company.py (1)
47-49:⚠️ Potential issue | 🟡 MinorNormalize identifiers before duplicate checks.
Line 47, Line 95, Line 185, and Line 241 compare raw strings. This allows semantic duplicates like
"Engineering"vs" engineering "to pass uniqueness validation.♻️ Suggested normalization pattern
- if len(self.members) != len(set(self.members)): - dupes = sorted(m for m, c in Counter(self.members).items() if c > 1) + normalized_members = [m.strip().casefold() for m in self.members] + if len(normalized_members) != len(set(normalized_members)): + dupes = sorted( + m for m, c in Counter(normalized_members).items() if c > 1 + ) msg = f"Duplicate members in team {self.name!r}: {dupes}" raise ValueError(msg) @@ - names = [t.name for t in self.teams] + names = [t.name.strip().casefold() for t in self.teams] @@ - agents = self.active_agents + agents = tuple(a.strip().casefold() for a in self.active_agents) @@ - names = [d.name for d in self.departments] + names = [d.name.strip().casefold() for d in self.departments]As per coding guidelines, "Validate at system boundaries — user input, external APIs, config files."
Also applies to: 95-99, 185-188, 241-245
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ai_company/core/company.py` around lines 47 - 49, The duplicate-detection logic uses raw strings (e.g., self.members and other identifier lists) so semantically identical values like "Engineering" vs " engineering " bypass validation; introduce and use a normalization helper (e.g., normalize_identifier(s) that does s.strip().casefold()) and apply it when building the set and Counter before comparisons in the duplicate-check locations (references: self.members duplicate check, and the other duplicate-check blocks around the methods that perform Counter(...) on identifier lists). Replace usages like set(self.members) and Counter(self.members) with set(map(normalize_identifier, ...)) and Counter(map(normalize_identifier, ...)) and update the duplicate message to report the normalized duplicates.tests/unit/core/test_agent.py (2)
39-478:⚠️ Potential issue | 🟡 MinorPublic test classes/functions are missing Google-style docstrings.
The classes and
test_*methods in this range are currently undocumented.As per coding guidelines, "Use Google-style docstrings on all public classes and functions in Python."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/core/test_agent.py` around lines 39 - 478, Add Google-style docstrings to each public test class (e.g., TestPersonalityConfig, TestSkillSet, TestModelConfig, TestMemoryConfig, TestToolPermissions, TestAgentIdentity) and to each public test function named test_* within those classes; for each class include a one-line summary of what the test class covers and for each test function include a one-line description of the specific validation or behavior being tested (e.g., "Tests default PersonalityConfig values" for TestPersonalityConfig.test_defaults), keeping docstrings short, present directly above the class or function definitions, and following Google-style formatting (triple-quoted summary, optional longer description if needed).
3-8:⚠️ Potential issue | 🟡 MinorAdd a 30-second timeout marker for this test module.
This file does not set
pytest.mark.timeout(30), so hangs here are unbounded.🔧 Suggested fix
import pytest from pydantic import ValidationError +pytestmark = pytest.mark.timeout(30) + from ai_company.core.agent import (As per coding guidelines, "Set test timeout to 30 seconds per test."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/core/test_agent.py` around lines 3 - 8, Add a 30-second pytest timeout for the whole test module by defining a module-level pytest marker; specifically, add a module variable (e.g., pytestmark = pytest.mark.timeout(30)) near the top of the test file so every test in this module uses a 30-second timeout; reference the existing use of pytest in the file to place this declaration.tests/unit/core/test_role_catalog.py (2)
24-198: 🛠️ Refactor suggestion | 🟠 MajorPublic test classes/functions still need Google-style docstrings.
Docstrings are still missing across test classes and methods.
As per coding guidelines: "Use Google-style docstrings on all public classes and functions in Python."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/core/test_role_catalog.py` around lines 24 - 198, Add Google-style docstrings to each public test class and its public test methods (e.g., TestSeniorityInfo, TestBuiltinRoles, TestGetBuiltinRole, TestGetSeniorityInfo and methods like test_has_8_entries, test_covers_all_seniority_levels, test_exact_match, test_all_roles_lookupable, test_found, etc.), writing a one- or two-line summary describing the intent of the class or test and any important context or expected behavior; ensure docstrings follow the Google style (short summary, optional longer description, and any Args/Returns sections only where applicable) and place them immediately below the class or function definition.
5-7:⚠️ Potential issue | 🟠 MajorAdd required 30-second timeout marker for this test module.
Timeout enforcement is still missing, so stalled tests can hang CI.
Proposed fix
import pytest from pydantic import ValidationError +pytestmark = pytest.mark.timeout(30) + from ai_company.core.enums import (#!/bin/bash # Verify timeout marker presence in this module rg -nP 'pytestmark\s*=\s*pytest\.mark\.timeout\(\s*30\s*\)|@pytest\.mark\.timeout\(\s*30\s*\)' tests/unit/core/test_role_catalog.pyAs per coding guidelines: "Set test timeout to 30 seconds per test."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/core/test_role_catalog.py` around lines 5 - 7, This test module lacks the required 30-second timeout marker; add a module-level timeout by inserting pytestmark = pytest.mark.timeout(30) near the top (using the existing pytest import) or apply `@pytest.mark.timeout`(30) to each test; ensure you reference the symbol pytestmark and pytest.mark.timeout so the module-level marker is present for tests in this file.tests/unit/core/test_company.py (1)
3-5:⚠️ Potential issue | 🟠 MajorMissing required 30-second timeout protection for tests.
This module does not enforce the required per-test timeout.
Proposed fix
import pytest from pydantic import ValidationError +pytestmark = pytest.mark.timeout(30) + from ai_company.core.company import (#!/bin/bash # Verify timeout marker presence in this module rg -nP 'pytestmark\s*=\s*pytest\.mark\.timeout\(\s*30\s*\)|@pytest\.mark\.timeout\(\s*30\s*\)' tests/unit/core/test_company.pyAs per coding guidelines: "Set test timeout to 30 seconds per test."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/core/test_company.py` around lines 3 - 5, This test module lacks the required per-test 30-second timeout; add a module-level timeout by defining pytestmark = pytest.mark.timeout(30) near the top of the file (or apply `@pytest.mark.timeout`(30) to each test function) so that pytest enforces a 30s timeout for tests in this module; reference the pytest symbol and the pytestmark variable when making the change.
🤖 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/core/role_catalog.py`:
- Around line 397-399: The dict comprehension that builds
_SENIORITY_INFO_BY_LEVEL from SENIORITY_INFO can silently drop duplicates and
doesn't verify all SeniorityLevel values are present; update the construction of
_SENIORITY_INFO_BY_LEVEL to perform fail-fast integrity checks: iterate
SENIORITY_INFO, detect and raise on duplicate info.level entries, then verify
that the resulting keys exactly cover the enum SeniorityLevel (raise
ValueError/Assertion if any level is missing), referencing the symbols
_SENIORITY_INFO_BY_LEVEL, SENIORITY_INFO, SeniorityLevel, and SeniorityInfo so
reviewers can find and harden that block.
In `@tests/unit/core/test_company.py`:
- Around line 26-362: The tests lack Google-style docstrings on public test
classes and functions; add concise Google-style docstrings to each public test
class (TestTeam, TestDepartment, TestCompanyConfig, TestHRRegistry, TestCompany)
and their public test methods (e.g., test_valid_team, test_defaults,
test_empty_name_rejected, test_valid_department, test_budget_percent_zero,
test_autonomy_boundaries, test_duplicate_active_agents_rejected,
test_valid_company, test_budget_sum_at_100_accepted, etc.) describing purpose
and expected behavior; update each class-level docstring to summarize the test
suite and add one-line docstrings to each test_ method following Google style
(Args/Returns not required for simple tests) so they meet repo standards.
---
Duplicate comments:
In `@src/ai_company/core/company.py`:
- Around line 47-49: The duplicate-detection logic uses raw strings (e.g.,
self.members and other identifier lists) so semantically identical values like
"Engineering" vs " engineering " bypass validation; introduce and use a
normalization helper (e.g., normalize_identifier(s) that does
s.strip().casefold()) and apply it when building the set and Counter before
comparisons in the duplicate-check locations (references: self.members duplicate
check, and the other duplicate-check blocks around the methods that perform
Counter(...) on identifier lists). Replace usages like set(self.members) and
Counter(self.members) with set(map(normalize_identifier, ...)) and
Counter(map(normalize_identifier, ...)) and update the duplicate message to
report the normalized duplicates.
In `@tests/unit/core/test_agent.py`:
- Around line 39-478: Add Google-style docstrings to each public test class
(e.g., TestPersonalityConfig, TestSkillSet, TestModelConfig, TestMemoryConfig,
TestToolPermissions, TestAgentIdentity) and to each public test function named
test_* within those classes; for each class include a one-line summary of what
the test class covers and for each test function include a one-line description
of the specific validation or behavior being tested (e.g., "Tests default
PersonalityConfig values" for TestPersonalityConfig.test_defaults), keeping
docstrings short, present directly above the class or function definitions, and
following Google-style formatting (triple-quoted summary, optional longer
description if needed).
- Around line 3-8: Add a 30-second pytest timeout for the whole test module by
defining a module-level pytest marker; specifically, add a module variable
(e.g., pytestmark = pytest.mark.timeout(30)) near the top of the test file so
every test in this module uses a 30-second timeout; reference the existing use
of pytest in the file to place this declaration.
In `@tests/unit/core/test_company.py`:
- Around line 3-5: This test module lacks the required per-test 30-second
timeout; add a module-level timeout by defining pytestmark =
pytest.mark.timeout(30) near the top of the file (or apply
`@pytest.mark.timeout`(30) to each test function) so that pytest enforces a 30s
timeout for tests in this module; reference the pytest symbol and the pytestmark
variable when making the change.
In `@tests/unit/core/test_role_catalog.py`:
- Around line 24-198: Add Google-style docstrings to each public test class and
its public test methods (e.g., TestSeniorityInfo, TestBuiltinRoles,
TestGetBuiltinRole, TestGetSeniorityInfo and methods like test_has_8_entries,
test_covers_all_seniority_levels, test_exact_match, test_all_roles_lookupable,
test_found, etc.), writing a one- or two-line summary describing the intent of
the class or test and any important context or expected behavior; ensure
docstrings follow the Google style (short summary, optional longer description,
and any Args/Returns sections only where applicable) and place them immediately
below the class or function definition.
- Around line 5-7: This test module lacks the required 30-second timeout marker;
add a module-level timeout by inserting pytestmark = pytest.mark.timeout(30)
near the top (using the existing pytest import) or apply
`@pytest.mark.timeout`(30) to each test; ensure you reference the symbol
pytestmark and pytest.mark.timeout so the module-level marker is present for
tests in this file.
In `@tests/unit/core/test_role.py`:
- Around line 3-5: Add a module-level 30s timeout marker to this test module by
declaring pytestmark = pytest.mark.timeout(30) near the top of
tests/unit/core/test_role.py (after the existing imports: pytest and
ValidationError) so all tests in the module inherit the 30‑second timeout;
ensure you import pytest (already present) and place the pytestmark assignment
at module scope, not inside any test or function.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
src/ai_company/core/agent.pysrc/ai_company/core/company.pysrc/ai_company/core/enums.pysrc/ai_company/core/role.pysrc/ai_company/core/role_catalog.pytests/unit/core/test_agent.pytests/unit/core/test_company.pytests/unit/core/test_enums.pytests/unit/core/test_role.pytests/unit/core/test_role_catalog.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 (4)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations— Python 3.14+ has PEP 649 native lazy annotations
Include type hints on all public functions in Python
Use Google-style docstrings on all public classes and functions in Python
Create new objects instead of mutating existing ones — maintain immutability
Use Pydantic v2BaseModel,model_validator, andConfigDictfor data models
Enforce 88-character line length in Python code
Keep functions under 50 lines of code
Keep Python files under 800 lines
Handle errors explicitly, never silently swallow exceptions
Validate at system boundaries — user input, external APIs, config files
Files:
tests/unit/core/test_role_catalog.pysrc/ai_company/core/role.pytests/unit/core/test_agent.pytests/unit/core/test_company.pytests/unit/core/test_enums.pysrc/ai_company/core/role_catalog.pysrc/ai_company/core/company.pytests/unit/core/test_role.pysrc/ai_company/core/enums.pysrc/ai_company/core/agent.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, and@pytest.mark.slowmarkers on test functions
Set test timeout to 30 seconds per test
Files:
tests/unit/core/test_role_catalog.pytests/unit/core/test_agent.pytests/unit/core/test_company.pytests/unit/core/test_enums.pytests/unit/core/test_role.py
{src,tests}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Run
ruff checkandruff formaton all source and test Python files
Files:
tests/unit/core/test_role_catalog.pysrc/ai_company/core/role.pytests/unit/core/test_agent.pytests/unit/core/test_company.pytests/unit/core/test_enums.pysrc/ai_company/core/role_catalog.pysrc/ai_company/core/company.pytests/unit/core/test_role.pysrc/ai_company/core/enums.pysrc/ai_company/core/agent.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Maintain minimum 80% code coverage across test suite
Runmypyin strict mode on all source Python files for type checking
Files:
src/ai_company/core/role.pysrc/ai_company/core/role_catalog.pysrc/ai_company/core/company.pysrc/ai_company/core/enums.pysrc/ai_company/core/agent.py
🧠 Learnings (23)
📓 Common learnings
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 : Validate all LLM outputs by checking required fields, verifying data types, handling malformed responses, and using Pydantic models for structured data
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-27T18:32:19.626Z
Learning: Applies to **/*.py : Use Pydantic v2 `BaseModel`, `model_validator`, and `ConfigDict` for data models
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 : Update story state by directly modifying fields on Pydantic model instances (append to chapters list, find and modify characters by ID)
📚 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 src/agents/**/*.py : Agents must extend `BaseAgent`, use retry logic, and implement configurable timeout via settings.
Applied to files:
tests/unit/core/test_role_catalog.pytests/unit/core/test_agent.pytests/unit/core/test_company.pytests/unit/core/test_role.pysrc/ai_company/core/agent.py
📚 Learning: 2026-02-27T18:32:19.627Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-27T18:32:19.627Z
Learning: Applies to tests/**/*.py : Set test timeout to 30 seconds per test
Applied to files:
tests/unit/core/test_role_catalog.pytests/unit/core/test_agent.pytests/unit/core/test_company.pytests/unit/core/test_role.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/core/test_role_catalog.pytests/unit/core/test_agent.py
📚 Learning: 2026-02-27T18:32:19.626Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-27T18:32:19.626Z
Learning: Applies to **/*.py : Use Google-style docstrings on all public classes and functions in Python
Applied to files:
tests/unit/core/test_role_catalog.pytests/unit/core/test_agent.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 : Use docstrings for classes and complex functions
Applied to files:
tests/unit/core/test_role_catalog.pytests/unit/core/test_agent.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/core/test_role_catalog.pytests/unit/core/test_agent.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 : Keep docstrings up to date with code changes
Applied to files:
tests/unit/core/test_role_catalog.pytests/unit/core/test_agent.py
📚 Learning: 2026-02-27T18:32:19.627Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-27T18:32:19.627Z
Learning: Applies to src/ai_company/tools/**/*.py : Use MCP integration for tool registry with role-based access control in the `tools/` module
Applied to files:
src/ai_company/core/role.py
📚 Learning: 2026-02-27T18:32:19.627Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-27T18:32:19.627Z
Learning: Applies to **/*.py : Validate at system boundaries — user input, external APIs, config files
Applied to files:
src/ai_company/core/role.pysrc/ai_company/core/company.pysrc/ai_company/core/agent.py
📚 Learning: 2026-02-27T18:32:19.626Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-27T18:32:19.626Z
Learning: Applies to **/*.py : Use Pydantic v2 `BaseModel`, `model_validator`, and `ConfigDict` for data models
Applied to files:
src/ai_company/core/role.pysrc/ai_company/core/company.pysrc/ai_company/core/agent.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 : Use type hints where appropriate. Use Pydantic models for data validation in `src/memory/story_state.py`, dataclasses in `src/settings.py`.
Applied to files:
src/ai_company/core/role.pysrc/ai_company/core/company.pysrc/ai_company/core/agent.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:
src/ai_company/core/role.pytests/unit/core/test_role.pysrc/ai_company/core/agent.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 {src/agents/**/*.py,src/services/**/*.py,src/memory/**/*.py,src/utils/**/*.py,src/settings.py} : Core modules (`src/agents/`, `src/services/`, `src/memory/`, `src/utils/`, `src/settings.py`) must maintain 100% test coverage
Applied to files:
tests/unit/core/test_agent.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 {src/agents/**/*.py,src/services/**/*.py,src/memory/**/*.py,src/utils/**/*.py,src/settings.py} : 100% test coverage is MANDATORY for every commit on core modules (`src/agents/`, `src/services/`, `src/memory/`, `src/utils/`, `src/settings.py`), CI enforces this coverage requirement
Applied to files:
tests/unit/core/test_agent.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: Core modules (`agents/`, `services/`, `workflows/`, `memory/`, `utils/`, `settings.py`) must maintain 100% test coverage; UI components (`ui/`) are excluded from coverage requirements
Applied to files:
tests/unit/core/test_agent.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 : In agent tests, mock Ollama API calls using `unittest.mock` and patch `agents.base.ollama.Client`
Applied to files:
tests/unit/core/test_agent.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 {src/agents/**/*.py,src/services/**/*.py,src/memory/**/*.py,src/utils/**/*.py,src/settings.py} : 100% test coverage is MANDATORY for every commit. The CI enforces 100% coverage on core modules (`src/agents/`, `src/services/`, `src/memory/`, `src/utils/`, `src/settings.py`).
Applied to files:
tests/unit/core/test_agent.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 : Validate all LLM outputs by checking required fields, verifying data types, handling malformed responses, and using Pydantic models for structured data
Applied to files:
tests/unit/core/test_agent.pysrc/ai_company/core/company.pysrc/ai_company/core/agent.py
📚 Learning: 2026-02-27T18:32:19.627Z
Learnt from: CR
Repo: Aureliolo/ai-company PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-27T18:32:19.627Z
Learning: Applies to src/ai_company/config/**/*.py : Use YAML for company configuration loading and validation in the `config/` module
Applied to files:
tests/unit/core/test_company.pysrc/ai_company/core/company.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 : Be aware of concurrent agent execution - don't modify shared state without synchronization and use thread-safe data structures when needed
Applied to files:
src/ai_company/core/company.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 {src/agents/**/*.py,src/services/model_service.py} : Respect existing model configuration patterns in Ollama integration
Applied to files:
src/ai_company/core/agent.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:
src/ai_company/core/agent.py
🧬 Code graph analysis (9)
tests/unit/core/test_role_catalog.py (3)
src/ai_company/core/enums.py (3)
CostTier(57-69)DepartmentName(108-119)SeniorityLevel(6-21)src/ai_company/core/role.py (2)
Role(123-180)SeniorityInfo(87-120)src/ai_company/core/role_catalog.py (2)
get_builtin_role(402-411)get_seniority_info(414-430)
src/ai_company/core/role.py (1)
src/ai_company/core/enums.py (4)
DepartmentName(108-119)ProficiencyLevel(99-105)SeniorityLevel(6-21)SkillCategory(85-96)
tests/unit/core/test_agent.py (4)
src/ai_company/core/agent.py (6)
AgentIdentity(213-280)MemoryConfig(140-166)ModelConfig(97-137)PersonalityConfig(19-64)SkillSet(67-94)ToolPermissions(169-210)src/ai_company/core/enums.py (5)
AgentStatus(24-29)CreativityLevel(40-45)MemoryType(48-54)RiskTolerance(32-37)SeniorityLevel(6-21)src/ai_company/core/role.py (1)
Authority(42-84)tests/unit/core/conftest.py (8)
AgentIdentityFactory(80-83)MemoryConfigFactory(69-71)ModelConfigFactory(64-66)PersonalityConfigFactory(56-57)SkillSetFactory(60-61)ToolPermissionsFactory(74-77)sample_model_config(142-149)sample_agent(153-162)
tests/unit/core/test_company.py (1)
src/ai_company/core/company.py (5)
Company(192-256)CompanyConfig(103-146)Department(54-100)HRRegistry(149-189)Team(15-51)
tests/unit/core/test_enums.py (1)
src/ai_company/core/enums.py (10)
AgentStatus(24-29)CompanyType(72-82)CostTier(57-69)CreativityLevel(40-45)DepartmentName(108-119)MemoryType(48-54)ProficiencyLevel(99-105)RiskTolerance(32-37)SeniorityLevel(6-21)SkillCategory(85-96)
src/ai_company/core/role_catalog.py (2)
src/ai_company/core/enums.py (3)
CostTier(57-69)DepartmentName(108-119)SeniorityLevel(6-21)src/ai_company/core/role.py (2)
Role(123-180)SeniorityInfo(87-120)
src/ai_company/core/company.py (2)
src/ai_company/core/enums.py (1)
CompanyType(72-82)src/ai_company/core/agent.py (2)
_validate_non_blank_identifiers(130-137)_validate_non_blank_identifiers(274-280)
tests/unit/core/test_role.py (2)
src/ai_company/core/enums.py (4)
DepartmentName(108-119)ProficiencyLevel(99-105)SeniorityLevel(6-21)SkillCategory(85-96)src/ai_company/core/role.py (5)
Authority(42-84)CustomRole(183-249)Role(123-180)SeniorityInfo(87-120)Skill(15-39)
src/ai_company/core/agent.py (3)
src/ai_company/core/enums.py (4)
AgentStatus(24-29)CreativityLevel(40-45)MemoryType(48-54)RiskTolerance(32-37)src/ai_company/core/role.py (1)
Authority(42-84)src/ai_company/core/company.py (1)
_validate_non_blank_identifiers(84-90)
🔇 Additional comments (1)
tests/unit/core/test_enums.py (1)
3-4: This timeout requirement is already satisfied. The project'spyproject.tomlhas a globaltimeout = 30setting under[tool.pytest.ini_options]that applies to all tests, including this file. No additional per-module or per-test timeout marker is needed.Likely an incorrect or invalid review comment.
…opilot - Add fail-fast integrity checks for _SENIORITY_INFO_BY_LEVEL (duplicate detection + full SeniorityLevel coverage), matching _BUILTIN_ROLES_BY_NAME - Add pytestmark = pytest.mark.timeout(30) to all 4 core test modules - Add Google-style docstrings to all test classes and methods in test_agent.py, test_company.py, and test_role_catalog.py
This was referenced Feb 28, 2026
This was referenced Mar 7, 2026
Aureliolo
added a commit
that referenced
this pull request
Mar 10, 2026
🤖 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).
Aureliolo
added a commit
that referenced
this pull request
Mar 11, 2026
🤖 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>
This was referenced Mar 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First real code for the project — the Pydantic v2 domain models that every other module will depend on. Implements Issues #55 (Agent, Company, Department) and #56 (Role, Skill, Seniority).
pydantic==2.12.5from dev to runtime dependenciesDesign decisions
ConfigDict(frozen=True)+tuple[str, ...]for true immutabilityCostTieruses string IDs (low,medium,high,premium) — extensible for user-defined tiers (Implement user-configurable cost tiers and subscription/quota-aware tracking #67)SeniorityInfo.cost_tieracceptsstrso custom cost tiers workRole(...)andCustomRole(...)can be created freelyValidators
ToolPermissions: rejects overlapping allowed/denied listsSkillSet: rejects empty/whitespace skill namesMemoryConfig: rejects retention_days when type is NONECustomRole: rejects empty department stringsCompany: rejects duplicate department names, budget sum > 100% (with float rounding)Department: rejects duplicate team namesHRRegistry: rejects duplicate active_agentsAuthority.reports_to,ModelConfig.fallback_model: min_length=1 when non-nullTest plan
Related