From 41e466f0d801f503c71a234091569477b1808442 Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Fri, 27 Feb 2026 21:26:25 +0100 Subject: [PATCH 1/6] feat: implement core entity and role system models (#55, #56) 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 --- pyproject.toml | 5 +- src/ai_company/core/__init__.py | 75 +++++ src/ai_company/core/agent.py | 205 +++++++++++++ src/ai_company/core/company.py | 158 ++++++++++ src/ai_company/core/enums.py | 117 ++++++++ src/ai_company/core/role.py | 168 +++++++++++ src/ai_company/core/role_catalog.py | 417 +++++++++++++++++++++++++++ tests/unit/core/__init__.py | 0 tests/unit/core/conftest.py | 181 ++++++++++++ tests/unit/core/test_agent.py | 309 ++++++++++++++++++++ tests/unit/core/test_company.py | 238 +++++++++++++++ tests/unit/core/test_enums.py | 151 ++++++++++ tests/unit/core/test_role.py | 169 +++++++++++ tests/unit/core/test_role_catalog.py | 185 ++++++++++++ uv.lock | 6 +- 15 files changed, 2380 insertions(+), 4 deletions(-) create mode 100644 src/ai_company/core/agent.py create mode 100644 src/ai_company/core/company.py create mode 100644 src/ai_company/core/enums.py create mode 100644 src/ai_company/core/role.py create mode 100644 src/ai_company/core/role_catalog.py create mode 100644 tests/unit/core/__init__.py create mode 100644 tests/unit/core/conftest.py create mode 100644 tests/unit/core/test_agent.py create mode 100644 tests/unit/core/test_company.py create mode 100644 tests/unit/core/test_enums.py create mode 100644 tests/unit/core/test_role.py create mode 100644 tests/unit/core/test_role_catalog.py diff --git a/pyproject.toml b/pyproject.toml index 1dc1675c40..39e1ff75d1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -12,7 +12,9 @@ classifiers = [ "Programming Language :: Python :: 3.14", "Typing :: Typed", ] -dependencies = [] +dependencies = [ + "pydantic==2.12.5", +] [build-system] requires = ["hatchling"] @@ -40,7 +42,6 @@ dev = [ "mypy==1.19.1", "pre-commit==4.5.1", "pre-commit-uv==4.2.1", - "pydantic==2.12.5", "ruff==0.15.4", {include-group = "test"}, ] diff --git a/src/ai_company/core/__init__.py b/src/ai_company/core/__init__.py index e69de29bb2..bc5d59cb8f 100644 --- a/src/ai_company/core/__init__.py +++ b/src/ai_company/core/__init__.py @@ -0,0 +1,75 @@ +"""Core domain models for the AI company framework.""" + +from ai_company.core.agent import ( + AgentIdentity, + MemoryConfig, + ModelConfig, + PersonalityConfig, + SkillSet, + ToolPermissions, +) +from ai_company.core.company import ( + Company, + CompanyConfig, + Department, + HRRegistry, + Team, +) +from ai_company.core.enums import ( + AgentStatus, + CompanyType, + CostTier, + CreativityLevel, + DepartmentName, + MemoryType, + ProficiencyLevel, + RiskTolerance, + SeniorityLevel, + SkillCategory, +) +from ai_company.core.role import ( + Authority, + CustomRole, + Role, + SeniorityInfo, + Skill, +) +from ai_company.core.role_catalog import ( + BUILTIN_ROLES, + SENIORITY_INFO, + get_builtin_role, + get_seniority_info, +) + +__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", +] diff --git a/src/ai_company/core/agent.py b/src/ai_company/core/agent.py new file mode 100644 index 0000000000..8b0dd7e040 --- /dev/null +++ b/src/ai_company/core/agent.py @@ -0,0 +1,205 @@ +"""Agent identity and configuration models.""" + +from datetime import date # noqa: TC003 — required at runtime by Pydantic +from uuid import UUID, uuid4 + +from pydantic import BaseModel, ConfigDict, Field + +from ai_company.core.enums import ( + AgentStatus, + CreativityLevel, + MemoryType, + RiskTolerance, + SeniorityLevel, +) +from ai_company.core.role import Authority + + +class PersonalityConfig(BaseModel): + """Personality traits and communication style for an agent. + + Attributes: + traits: Personality trait keywords. + communication_style: Free-text style description. + risk_tolerance: Risk tolerance level. + creativity: Creativity level. + description: Extended personality description. + """ + + model_config = ConfigDict(frozen=True) + + 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", + ) + + +class SkillSet(BaseModel): + """Primary and secondary skills for an agent. + + Attributes: + primary: Core competency skill names. + secondary: Supporting skill names. + """ + + model_config = ConfigDict(frozen=True) + + primary: tuple[str, ...] = Field( + default=(), + description="Primary skills", + ) + secondary: tuple[str, ...] = Field( + default=(), + description="Secondary skills", + ) + + +class ModelConfig(BaseModel): + """LLM model configuration for an agent. + + Attributes: + provider: LLM provider name (e.g. ``"anthropic"``). + model_id: Model identifier (e.g. ``"claude-sonnet-4-6"``). + temperature: Sampling temperature (0.0 to 2.0). + max_tokens: Maximum output tokens. + fallback_model: Optional fallback model identifier. + """ + + model_config = ConfigDict(frozen=True) + + provider: str = Field(min_length=1, description="LLM provider name") + model_id: str = Field(min_length=1, description="Model identifier") + temperature: float = Field( + default=0.7, + ge=0.0, + le=2.0, + description="Sampling temperature", + ) + max_tokens: int = Field( + default=4096, + gt=0, + description="Maximum output tokens", + ) + fallback_model: str | None = Field( + default=None, + description="Fallback model identifier", + ) + + +class MemoryConfig(BaseModel): + """Memory configuration for an agent. + + Attributes: + type: Memory persistence type. + retention_days: Days to retain memories (``None`` means forever). + """ + + model_config = ConfigDict(frozen=True) + + type: MemoryType = Field( + default=MemoryType.SESSION, + description="Memory persistence type", + ) + retention_days: int | None = Field( + default=None, + ge=1, + description="Days to retain memories (None = forever)", + ) + + +class ToolPermissions(BaseModel): + """Tool access permissions for an agent. + + Attributes: + allowed: Explicitly allowed tool names. + denied: Explicitly denied tool names. + """ + + model_config = ConfigDict(frozen=True) + + allowed: tuple[str, ...] = Field( + default=(), + description="Explicitly allowed tools", + ) + denied: tuple[str, ...] = Field( + default=(), + description="Explicitly denied tools", + ) + + +class AgentIdentity(BaseModel): + """Complete agent identity card. + + Every agent in the company is represented by an ``AgentIdentity`` + containing its role, personality, model backend, memory settings, + tool permissions, and authority scope. + + Attributes: + id: Unique agent identifier. + name: Agent display name. + role: Role name (string reference to :class:`~ai_company.core.role.Role`). + department: Department name (string reference). + level: Seniority level. + personality: Personality configuration. + skills: Primary and secondary skill set. + model: LLM model configuration (required). + memory: Memory configuration. + tools: Tool permissions. + authority: Authority scope. + hiring_date: Date the agent was hired. + status: Current lifecycle status. + """ + + model_config = ConfigDict(frozen=True) + + id: UUID = Field(default_factory=uuid4, description="Unique agent identifier") + name: str = Field(min_length=1, description="Agent display name") + role: str = Field(min_length=1, description="Role name") + department: str = Field(min_length=1, description="Department name") + level: SeniorityLevel = Field( + default=SeniorityLevel.MID, + description="Seniority level", + ) + personality: PersonalityConfig = Field( + default_factory=PersonalityConfig, + description="Personality configuration", + ) + skills: SkillSet = Field( + default_factory=SkillSet, + description="Skill set", + ) + model: ModelConfig = Field(description="LLM model configuration") + memory: MemoryConfig = Field( + default_factory=MemoryConfig, + description="Memory configuration", + ) + tools: ToolPermissions = Field( + default_factory=ToolPermissions, + description="Tool permissions", + ) + authority: Authority = Field( + default_factory=Authority, + description="Authority scope", + ) + hiring_date: date = Field(description="Date the agent was hired") + status: AgentStatus = Field( + default=AgentStatus.ACTIVE, + description="Current lifecycle status", + ) diff --git a/src/ai_company/core/company.py b/src/ai_company/core/company.py new file mode 100644 index 0000000000..3b9a8c4d46 --- /dev/null +++ b/src/ai_company/core/company.py @@ -0,0 +1,158 @@ +"""Company structure and configuration models.""" + +from uuid import UUID, uuid4 + +from pydantic import BaseModel, ConfigDict, Field, model_validator + +from ai_company.core.enums import CompanyType + + +class Team(BaseModel): + """A team within a department. + + Attributes: + name: Team name. + lead: Team lead agent name (string reference). + members: Team member agent names. + """ + + model_config = ConfigDict(frozen=True) + + 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", + ) + + +class Department(BaseModel): + """An organizational department. + + Attributes: + name: Department name. + head: Department head agent name (string reference). + budget_percent: Percentage of company budget allocated (0-100). + teams: Teams within this department. + """ + + model_config = ConfigDict(frozen=True) + + name: str = Field(min_length=1, description="Department name") + head: str = Field(min_length=1, description="Department head agent name") + budget_percent: float = Field( + default=0.0, + ge=0.0, + le=100.0, + description="Percentage of company budget allocated", + ) + teams: tuple[Team, ...] = Field( + default=(), + description="Teams within this department", + ) + + +class CompanyConfig(BaseModel): + """Company-wide configuration settings. + + Attributes: + autonomy: Autonomy level (0 = full human oversight, 1 = fully autonomous). + budget_monthly: Monthly budget in USD. + communication_pattern: Default communication pattern name. + tool_access_default: Default tool access for all agents. + """ + + model_config = ConfigDict(frozen=True) + + autonomy: float = Field( + default=0.5, + ge=0.0, + le=1.0, + description="Autonomy level (0=full human oversight, 1=fully autonomous)", + ) + budget_monthly: float = Field( + default=100.0, + ge=0.0, + description="Monthly budget in USD", + ) + communication_pattern: str = Field( + default="hybrid", + min_length=1, + description="Default communication pattern", + ) + tool_access_default: tuple[str, ...] = Field( + default=(), + description="Default tool access for all agents", + ) + + +class HRRegistry(BaseModel): + """Human resources registry for the company. + + Attributes: + active_agents: Currently active agent names. + available_roles: Roles available for hiring. + hiring_queue: Roles in the hiring pipeline. + """ + + model_config = ConfigDict(frozen=True) + + active_agents: tuple[str, ...] = Field( + default=(), + description="Currently active agent names", + ) + available_roles: tuple[str, ...] = Field( + default=(), + description="Roles available for hiring", + ) + hiring_queue: tuple[str, ...] = Field( + default=(), + description="Roles in the hiring pipeline", + ) + + +class Company(BaseModel): + """Top-level company entity. + + Validates that department budget allocations do not exceed 100%. + The sum may be less than 100% to allow for an unallocated reserve. + + Attributes: + id: Company identifier. + name: Company name. + type: Company template type. + departments: Company departments. + config: Company-wide configuration. + hr_registry: HR registry. + """ + + model_config = ConfigDict(frozen=True) + + id: UUID = Field(default_factory=uuid4, description="Company identifier") + name: str = Field(min_length=1, description="Company name") + type: CompanyType = Field( + default=CompanyType.CUSTOM, + description="Company template type", + ) + departments: tuple[Department, ...] = Field( + default=(), + description="Company departments", + ) + config: CompanyConfig = Field( + default_factory=CompanyConfig, + description="Company-wide configuration", + ) + hr_registry: HRRegistry = Field( + default_factory=HRRegistry, + description="HR registry", + ) + + @model_validator(mode="after") + def _validate_budget_percent_sum(self) -> Company: + """Ensure department budget allocations do not exceed 100%.""" + max_budget_percent = 100.0 + total = sum(d.budget_percent for d in self.departments) + if total > max_budget_percent: + msg = f"Department budget allocations sum to {total}%, exceeding 100%" + raise ValueError(msg) + return self diff --git a/src/ai_company/core/enums.py b/src/ai_company/core/enums.py new file mode 100644 index 0000000000..7976a013b7 --- /dev/null +++ b/src/ai_company/core/enums.py @@ -0,0 +1,117 @@ +"""Domain enumerations for the AI company framework.""" + +from enum import StrEnum + + +class SeniorityLevel(StrEnum): + """Seniority levels for agents within the organization. + + Maps to authority scope, typical model tier, and cost tier. + See ``role_catalog.SENIORITY_INFO`` for the full mapping. + """ + + JUNIOR = "junior" + MID = "mid" + SENIOR = "senior" + LEAD = "lead" + PRINCIPAL = "principal" + DIRECTOR = "director" + VP = "vp" + C_SUITE = "c_suite" + + +class AgentStatus(StrEnum): + """Lifecycle status of an agent.""" + + ACTIVE = "active" + ON_LEAVE = "on_leave" + TERMINATED = "terminated" + + +class RiskTolerance(StrEnum): + """Risk tolerance level for agent personality.""" + + LOW = "low" + MEDIUM = "medium" + HIGH = "high" + + +class CreativityLevel(StrEnum): + """Creativity level for agent personality.""" + + LOW = "low" + MEDIUM = "medium" + HIGH = "high" + + +class MemoryType(StrEnum): + """Memory persistence type for an agent.""" + + PERSISTENT = "persistent" + PROJECT = "project" + SESSION = "session" + NONE = "none" + + +class CostTier(StrEnum): + """Built-in cost tier identifiers. + + These are the default tiers shipped with the framework. Users can + define additional tiers via configuration; any field that accepts a + cost tier uses ``str`` so custom tier IDs are valid too. + """ + + LOW = "low" + MEDIUM = "medium" + HIGH = "high" + PREMIUM = "premium" + + +class CompanyType(StrEnum): + """Pre-defined company template types.""" + + SOLO_FOUNDER = "solo_founder" + STARTUP = "startup" + DEV_SHOP = "dev_shop" + PRODUCT_TEAM = "product_team" + AGENCY = "agency" + FULL_COMPANY = "full_company" + RESEARCH_LAB = "research_lab" + CUSTOM = "custom" + + +class SkillCategory(StrEnum): + """Categories for agent skills.""" + + ENGINEERING = "engineering" + PRODUCT = "product" + DESIGN = "design" + DATA = "data" + QA = "qa" + OPERATIONS = "operations" + SECURITY = "security" + CREATIVE = "creative" + MANAGEMENT = "management" + + +class ProficiencyLevel(StrEnum): + """Proficiency level for a skill.""" + + BEGINNER = "beginner" + INTERMEDIATE = "intermediate" + ADVANCED = "advanced" + EXPERT = "expert" + + +class DepartmentName(StrEnum): + """Standard department names within the organization.""" + + EXECUTIVE = "executive" + PRODUCT = "product" + DESIGN = "design" + ENGINEERING = "engineering" + QUALITY_ASSURANCE = "quality_assurance" + DATA_ANALYTICS = "data_analytics" + OPERATIONS = "operations" + CREATIVE_MARKETING = "creative_marketing" + SECURITY = "security" diff --git a/src/ai_company/core/role.py b/src/ai_company/core/role.py new file mode 100644 index 0000000000..ad44f3081e --- /dev/null +++ b/src/ai_company/core/role.py @@ -0,0 +1,168 @@ +"""Role and skill domain models.""" + +from pydantic import BaseModel, ConfigDict, Field + +from ai_company.core.enums import ( + DepartmentName, + ProficiencyLevel, + SeniorityLevel, + SkillCategory, +) + + +class Skill(BaseModel): + """A capability an agent possesses. + + Attributes: + name: Skill name (e.g. ``"python"``, ``"system-design"``). + category: Broad skill category. + proficiency: Agent's proficiency in this skill. + """ + + model_config = ConfigDict(frozen=True) + + name: str = Field(min_length=1, description="Skill name") + category: SkillCategory = Field(description="Skill category") + proficiency: ProficiencyLevel = Field( + default=ProficiencyLevel.INTERMEDIATE, + description="Proficiency level", + ) + + +class Authority(BaseModel): + """Authority scope for an agent or role. + + Attributes: + can_approve: Task types this role can approve. + reports_to: Role this position reports to. + can_delegate_to: Roles this position can delegate tasks to. + budget_limit: Maximum USD spend per task. + """ + + model_config = ConfigDict(frozen=True) + + can_approve: tuple[str, ...] = Field( + default=(), + description="Task types this role can approve", + ) + reports_to: str | None = Field( + default=None, + description="Role this position reports to", + ) + can_delegate_to: tuple[str, ...] = Field( + default=(), + description="Roles this position can delegate tasks to", + ) + budget_limit: float = Field( + default=0.0, + ge=0.0, + description="Maximum USD per task", + ) + + +class SeniorityInfo(BaseModel): + """Mapping from seniority level to authority and model configuration. + + Attributes: + level: The seniority level. + authority_scope: Description of authority at this level. + typical_model_tier: Recommended model tier (e.g. ``"opus"``). + cost_tier: Expected cost tier for this level. + """ + + model_config = ConfigDict(frozen=True) + + level: SeniorityLevel = Field(description="Seniority level") + authority_scope: str = Field( + min_length=1, + description="Description of authority at this level", + ) + typical_model_tier: str = Field( + min_length=1, + description="Recommended model tier", + ) + cost_tier: str = Field( + min_length=1, + description="Cost tier identifier (built-in or user-defined)", + ) + + +class Role(BaseModel): + """A job definition within the organization. + + Attributes: + name: Role name (e.g. ``"Backend Developer"``). + department: Department this role belongs to. + required_skills: Skills required for this role. + authority_level: Default seniority level. + tool_access: Tools available to this role. + system_prompt_template: Template file for system prompt. + description: Human-readable description. + """ + + model_config = ConfigDict(frozen=True) + + 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", + ) + description: str = Field( + default="", + description="Human-readable description", + ) + + +class CustomRole(BaseModel): + """User-defined custom role via configuration. + + Unlike :class:`Role`, the ``department`` field accepts arbitrary strings + in addition to :class:`~ai_company.core.enums.DepartmentName` values, + allowing users to define roles in non-standard departments. + + Attributes: + name: Custom role name. + department: Department (standard or custom name). + skills: Required skills for this role. + system_prompt_template: Template file for system prompt. + authority_level: Default seniority level. + suggested_model: Suggested model tier. + """ + + model_config = ConfigDict(frozen=True) + + name: str = Field(min_length=1, description="Custom role name") + department: DepartmentName | str = Field( + description="Department (standard or custom name)", + ) + skills: tuple[str, ...] = Field( + default=(), + description="Required skills for this role", + ) + system_prompt_template: str | None = Field( + default=None, + description="Template file for system prompt", + ) + authority_level: SeniorityLevel = Field( + default=SeniorityLevel.MID, + description="Default seniority level", + ) + suggested_model: str | None = Field( + default=None, + description="Suggested model tier", + ) diff --git a/src/ai_company/core/role_catalog.py b/src/ai_company/core/role_catalog.py new file mode 100644 index 0000000000..d93652bb28 --- /dev/null +++ b/src/ai_company/core/role_catalog.py @@ -0,0 +1,417 @@ +"""Built-in role catalog and seniority information. + +Provides the canonical set of roles and seniority mappings derived from +the design specification (DESIGN_SPEC.md sections 3.2 and 3.3). +""" + +from ai_company.core.enums import ( + CostTier, + DepartmentName, + SeniorityLevel, +) +from ai_company.core.role import Role, SeniorityInfo + +# ── Seniority Mapping ────────────────────────────────────────────── + +SENIORITY_INFO: tuple[SeniorityInfo, ...] = ( + SeniorityInfo( + level=SeniorityLevel.JUNIOR, + authority_scope="Execute assigned tasks only", + typical_model_tier="haiku", + cost_tier=CostTier.LOW, + ), + SeniorityInfo( + level=SeniorityLevel.MID, + authority_scope="Execute and suggest improvements", + typical_model_tier="sonnet", + cost_tier=CostTier.MEDIUM, + ), + SeniorityInfo( + level=SeniorityLevel.SENIOR, + authority_scope="Execute, design, and review others", + typical_model_tier="sonnet", + cost_tier=CostTier.HIGH, + ), + SeniorityInfo( + level=SeniorityLevel.LEAD, + authority_scope="All above plus approve and delegate", + typical_model_tier="opus", + cost_tier=CostTier.HIGH, + ), + SeniorityInfo( + level=SeniorityLevel.PRINCIPAL, + authority_scope="All above plus architectural decisions", + typical_model_tier="opus", + cost_tier=CostTier.PREMIUM, + ), + SeniorityInfo( + level=SeniorityLevel.DIRECTOR, + authority_scope="Strategic decisions and budget authority", + typical_model_tier="opus", + cost_tier=CostTier.PREMIUM, + ), + SeniorityInfo( + level=SeniorityLevel.VP, + authority_scope="Department-wide authority", + typical_model_tier="opus", + cost_tier=CostTier.PREMIUM, + ), + SeniorityInfo( + level=SeniorityLevel.C_SUITE, + authority_scope="Company-wide authority and final approvals", + typical_model_tier="opus", + cost_tier=CostTier.PREMIUM, + ), +) + +# ── C-Suite / Executive ──────────────────────────────────────────── + +_CEO = Role( + name="CEO", + department=DepartmentName.EXECUTIVE, + required_skills=("strategy", "leadership", "communication"), + authority_level=SeniorityLevel.C_SUITE, + description=( + "Overall strategy, final decision authority, cross-department coordination" + ), +) + +_CTO = Role( + name="CTO", + department=DepartmentName.EXECUTIVE, + required_skills=("architecture", "technology", "leadership"), + authority_level=SeniorityLevel.C_SUITE, + description="Technical vision, architecture decisions, technology choices", +) + +_CFO = Role( + name="CFO", + department=DepartmentName.EXECUTIVE, + required_skills=("budgeting", "cost-optimization", "analytics"), + authority_level=SeniorityLevel.C_SUITE, + description="Budget management, cost optimization, resource allocation", +) + +_COO = Role( + name="COO", + department=DepartmentName.EXECUTIVE, + required_skills=("operations", "process-optimization", "workflow"), + authority_level=SeniorityLevel.C_SUITE, + description="Operations, process optimization, workflow management", +) + +_CPO = Role( + name="CPO", + department=DepartmentName.EXECUTIVE, + required_skills=("product-strategy", "roadmap", "prioritization"), + authority_level=SeniorityLevel.C_SUITE, + description="Product strategy, roadmap, feature prioritization", +) + +# ── Product & Design ─────────────────────────────────────────────── + +_PRODUCT_MANAGER = Role( + name="Product Manager", + department=DepartmentName.PRODUCT, + required_skills=("requirements", "user-stories", "prioritization"), + authority_level=SeniorityLevel.SENIOR, + description=( + "Requirements, user stories, prioritization, stakeholder communication" + ), +) + +_UX_DESIGNER = Role( + name="UX Designer", + department=DepartmentName.DESIGN, + required_skills=("user-research", "wireframes", "user-flows"), + authority_level=SeniorityLevel.MID, + description="User research, wireframes, user flows, usability", +) + +_UI_DESIGNER = Role( + name="UI Designer", + department=DepartmentName.DESIGN, + required_skills=("visual-design", "component-design", "design-systems"), + authority_level=SeniorityLevel.MID, + description="Visual design, component design, design systems", +) + +_UX_RESEARCHER = Role( + name="UX Researcher", + department=DepartmentName.DESIGN, + required_skills=("user-interviews", "analytics", "a-b-testing"), + authority_level=SeniorityLevel.MID, + description="User interviews, analytics, A/B test design", +) + +_TECHNICAL_WRITER = Role( + name="Technical Writer", + department=DepartmentName.PRODUCT, + required_skills=("documentation", "api-docs", "user-guides"), + authority_level=SeniorityLevel.MID, + description="Documentation, API docs, user guides", +) + +# ── Engineering ──────────────────────────────────────────────────── + +_SOFTWARE_ARCHITECT = Role( + name="Software Architect", + department=DepartmentName.ENGINEERING, + required_skills=("system-design", "architecture", "patterns"), + authority_level=SeniorityLevel.PRINCIPAL, + description="System design, technology decisions, patterns", +) + +_FRONTEND_DEVELOPER = Role( + name="Frontend Developer", + department=DepartmentName.ENGINEERING, + required_skills=("javascript", "css", "ui-frameworks"), + authority_level=SeniorityLevel.MID, + description="UI implementation, components, state management", +) + +_BACKEND_DEVELOPER = Role( + name="Backend Developer", + department=DepartmentName.ENGINEERING, + required_skills=("python", "apis", "databases"), + authority_level=SeniorityLevel.MID, + description="APIs, business logic, databases", +) + +_FULLSTACK_DEVELOPER = Role( + name="Full-Stack Developer", + department=DepartmentName.ENGINEERING, + required_skills=("javascript", "python", "databases"), + authority_level=SeniorityLevel.MID, + description="End-to-end implementation", +) + +_DEVOPS_ENGINEER = Role( + name="DevOps/SRE Engineer", + department=DepartmentName.ENGINEERING, + required_skills=("infrastructure", "ci-cd", "monitoring"), + authority_level=SeniorityLevel.MID, + description="Infrastructure, CI/CD, monitoring, deployment", +) + +_DATABASE_ENGINEER = Role( + name="Database Engineer", + department=DepartmentName.ENGINEERING, + required_skills=("schema-design", "query-optimization", "migrations"), + authority_level=SeniorityLevel.MID, + description="Schema design, query optimization, migrations", +) + +_SECURITY_ENGINEER = Role( + name="Security Engineer", + department=DepartmentName.SECURITY, + required_skills=( + "security-audits", + "vulnerability-assessment", + "secure-coding", + ), + authority_level=SeniorityLevel.SENIOR, + description="Security audits, vulnerability assessment, secure coding", +) + +# ── Quality Assurance ────────────────────────────────────────────── + +_QA_LEAD = Role( + name="QA Lead", + department=DepartmentName.QUALITY_ASSURANCE, + required_skills=("test-strategy", "quality-gates", "release-readiness"), + authority_level=SeniorityLevel.LEAD, + description="Test strategy, quality gates, release readiness", +) + +_QA_ENGINEER = Role( + name="QA Engineer", + department=DepartmentName.QUALITY_ASSURANCE, + required_skills=("test-plans", "manual-testing", "bug-reporting"), + authority_level=SeniorityLevel.MID, + description="Test plans, manual testing, bug reporting", +) + +_AUTOMATION_ENGINEER = Role( + name="Automation Engineer", + department=DepartmentName.QUALITY_ASSURANCE, + required_skills=("test-frameworks", "ci-integration", "e2e-testing"), + authority_level=SeniorityLevel.MID, + description="Test frameworks, CI integration, E2E tests", +) + +_PERFORMANCE_ENGINEER = Role( + name="Performance Engineer", + department=DepartmentName.QUALITY_ASSURANCE, + required_skills=("load-testing", "profiling", "optimization"), + authority_level=SeniorityLevel.SENIOR, + description="Load testing, profiling, optimization", +) + +# ── Data & Analytics ─────────────────────────────────────────────── + +_DATA_ANALYST = Role( + name="Data Analyst", + department=DepartmentName.DATA_ANALYTICS, + required_skills=("metrics", "dashboards", "business-intelligence"), + authority_level=SeniorityLevel.MID, + description="Metrics, dashboards, business intelligence", +) + +_DATA_ENGINEER = Role( + name="Data Engineer", + department=DepartmentName.DATA_ANALYTICS, + required_skills=("pipelines", "etl", "data-infrastructure"), + authority_level=SeniorityLevel.MID, + description="Pipelines, ETL, data infrastructure", +) + +_ML_ENGINEER = Role( + name="ML Engineer", + department=DepartmentName.DATA_ANALYTICS, + required_skills=("model-training", "inference", "mlops"), + authority_level=SeniorityLevel.SENIOR, + description="Model training, inference, MLOps", +) + +# ── Operations & Support ────────────────────────────────────────── + +_PROJECT_MANAGER = Role( + name="Project Manager", + department=DepartmentName.OPERATIONS, + required_skills=("timelines", "dependencies", "risk-management"), + authority_level=SeniorityLevel.SENIOR, + description=("Timelines, dependencies, risk management, status tracking"), +) + +_SCRUM_MASTER = Role( + name="Scrum Master", + department=DepartmentName.OPERATIONS, + required_skills=("agile", "facilitation", "impediment-removal"), + authority_level=SeniorityLevel.SENIOR, + description="Agile ceremonies, impediment removal, team health", +) + +_HR_MANAGER = Role( + name="HR Manager", + department=DepartmentName.OPERATIONS, + required_skills=( + "hiring", + "team-composition", + "performance-tracking", + ), + authority_level=SeniorityLevel.SENIOR, + description=("Hiring recommendations, team composition, performance tracking"), +) + +_SECURITY_OPERATIONS = Role( + name="Security Operations", + department=DepartmentName.SECURITY, + required_skills=( + "request-validation", + "safety-checks", + "approval-workflows", + ), + authority_level=SeniorityLevel.SENIOR, + description="Request validation, safety checks, approval workflows", +) + +# ── Creative & Marketing ────────────────────────────────────────── + +_CONTENT_WRITER = Role( + name="Content Writer", + department=DepartmentName.CREATIVE_MARKETING, + required_skills=("blog-posts", "marketing-copy", "social-media"), + authority_level=SeniorityLevel.MID, + description="Blog posts, marketing copy, social media", +) + +_BRAND_STRATEGIST = Role( + name="Brand Strategist", + department=DepartmentName.CREATIVE_MARKETING, + required_skills=("messaging", "positioning", "competitive-analysis"), + authority_level=SeniorityLevel.SENIOR, + description="Messaging, positioning, competitive analysis", +) + +_GROWTH_MARKETER = Role( + name="Growth Marketer", + department=DepartmentName.CREATIVE_MARKETING, + required_skills=("campaigns", "analytics", "conversion-optimization"), + authority_level=SeniorityLevel.MID, + description="Campaigns, analytics, conversion optimization", +) + +# ── Aggregated Catalog ───────────────────────────────────────────── + +BUILTIN_ROLES: tuple[Role, ...] = ( + # C-Suite + _CEO, + _CTO, + _CFO, + _COO, + _CPO, + # Product & Design + _PRODUCT_MANAGER, + _UX_DESIGNER, + _UI_DESIGNER, + _UX_RESEARCHER, + _TECHNICAL_WRITER, + # Engineering + _SOFTWARE_ARCHITECT, + _FRONTEND_DEVELOPER, + _BACKEND_DEVELOPER, + _FULLSTACK_DEVELOPER, + _DEVOPS_ENGINEER, + _DATABASE_ENGINEER, + _SECURITY_ENGINEER, + # Quality Assurance + _QA_LEAD, + _QA_ENGINEER, + _AUTOMATION_ENGINEER, + _PERFORMANCE_ENGINEER, + # Data & Analytics + _DATA_ANALYST, + _DATA_ENGINEER, + _ML_ENGINEER, + # Operations & Support + _PROJECT_MANAGER, + _SCRUM_MASTER, + _HR_MANAGER, + _SECURITY_OPERATIONS, + # Creative & Marketing + _CONTENT_WRITER, + _BRAND_STRATEGIST, + _GROWTH_MARKETER, +) + + +def get_builtin_role(name: str) -> Role | None: + """Look up a built-in role by name (case-insensitive). + + Args: + name: Role name to search for. + + Returns: + The matching Role, or ``None`` if not found. + """ + lower = name.lower() + for role in BUILTIN_ROLES: + if role.name.lower() == lower: + return role + return None + + +def get_seniority_info(level: SeniorityLevel) -> SeniorityInfo | None: + """Look up seniority info by level. + + Args: + level: The seniority level to look up. + + Returns: + The matching SeniorityInfo, or ``None`` if not found. + """ + for info in SENIORITY_INFO: + if info.level == level: + return info + return None diff --git a/tests/unit/core/__init__.py b/tests/unit/core/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/unit/core/conftest.py b/tests/unit/core/conftest.py new file mode 100644 index 0000000000..3d76b1194d --- /dev/null +++ b/tests/unit/core/conftest.py @@ -0,0 +1,181 @@ +"""Unit test configuration and fixtures for core models.""" + +from datetime import date +from uuid import uuid4 + +import pytest +from polyfactory.factories.pydantic_factory import ModelFactory + +from ai_company.core.agent import ( + AgentIdentity, + MemoryConfig, + ModelConfig, + PersonalityConfig, + SkillSet, + ToolPermissions, +) +from ai_company.core.company import ( + Company, + CompanyConfig, + Department, + HRRegistry, + Team, +) +from ai_company.core.enums import ( + DepartmentName, + ProficiencyLevel, + SeniorityLevel, + SkillCategory, +) +from ai_company.core.role import Authority, CustomRole, Role, SeniorityInfo, Skill + +# ── Factories ────────────────────────────────────────────────────── + + +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 + + +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="anthropic", + model_id="claude-sonnet-4-6", + temperature=0.3, + max_tokens=8192, + fallback_model="openrouter/anthropic/claude-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), + ) diff --git a/tests/unit/core/test_agent.py b/tests/unit/core/test_agent.py new file mode 100644 index 0000000000..2834264c36 --- /dev/null +++ b/tests/unit/core/test_agent.py @@ -0,0 +1,309 @@ +"""Tests for agent identity and configuration models.""" + +from datetime import date +from uuid import UUID + +import pytest +from pydantic import ValidationError + +from ai_company.core.agent import ( + AgentIdentity, + MemoryConfig, + ModelConfig, + PersonalityConfig, + SkillSet, + ToolPermissions, +) +from ai_company.core.enums import ( + AgentStatus, + CreativityLevel, + MemoryType, + RiskTolerance, + SeniorityLevel, +) +from ai_company.core.role import Authority + +from .conftest import ( + AgentIdentityFactory, + MemoryConfigFactory, + ModelConfigFactory, + PersonalityConfigFactory, + SkillSetFactory, + ToolPermissionsFactory, +) + +# ── PersonalityConfig ────────────────────────────────────────────── + + +@pytest.mark.unit +class TestPersonalityConfig: + def test_defaults(self): + p = PersonalityConfig() + assert p.traits == () + assert p.communication_style == "neutral" + assert p.risk_tolerance is RiskTolerance.MEDIUM + assert p.creativity is CreativityLevel.MEDIUM + assert p.description == "" + + def test_custom_values(self): + p = PersonalityConfig( + traits=("analytical", "pragmatic"), + communication_style="concise and technical", + risk_tolerance=RiskTolerance.LOW, + creativity=CreativityLevel.HIGH, + description="A detail-oriented engineer.", + ) + assert len(p.traits) == 2 + assert p.communication_style == "concise and technical" + + def test_empty_communication_style_rejected(self): + with pytest.raises(ValidationError): + PersonalityConfig(communication_style="") + + def test_frozen(self): + p = PersonalityConfig() + with pytest.raises(ValidationError): + p.creativity = CreativityLevel.LOW # type: ignore[misc] + + def test_factory(self): + p = PersonalityConfigFactory.build() + assert isinstance(p, PersonalityConfig) + + +# ── SkillSet ─────────────────────────────────────────────────────── + + +@pytest.mark.unit +class TestSkillSet: + def test_defaults(self): + s = SkillSet() + assert s.primary == () + assert s.secondary == () + + def test_custom_values(self): + s = SkillSet( + primary=("python", "fastapi"), + secondary=("docker", "redis"), + ) + assert "python" in s.primary + assert "docker" in s.secondary + + def test_frozen(self): + s = SkillSet() + with pytest.raises(ValidationError): + s.primary = ("new",) # type: ignore[misc] + + def test_factory(self): + s = SkillSetFactory.build() + assert isinstance(s, SkillSet) + + +# ── ModelConfig ──────────────────────────────────────────────────── + + +@pytest.mark.unit +class TestModelConfig: + def test_valid_config(self, sample_model_config: ModelConfig): + assert sample_model_config.provider == "anthropic" + assert sample_model_config.model_id == "claude-sonnet-4-6" + assert sample_model_config.temperature == 0.3 + assert sample_model_config.max_tokens == 8192 + + def test_defaults(self): + m = ModelConfig(provider="test", model_id="test-model") + assert m.temperature == 0.7 + assert m.max_tokens == 4096 + assert m.fallback_model is None + + def test_empty_provider_rejected(self): + with pytest.raises(ValidationError): + ModelConfig(provider="", model_id="test") + + def test_empty_model_id_rejected(self): + with pytest.raises(ValidationError): + ModelConfig(provider="test", model_id="") + + def test_temperature_below_zero_rejected(self): + with pytest.raises(ValidationError): + ModelConfig(provider="test", model_id="m", temperature=-0.1) + + def test_temperature_above_two_rejected(self): + with pytest.raises(ValidationError): + ModelConfig(provider="test", model_id="m", temperature=2.1) + + def test_temperature_boundary_zero(self): + m = ModelConfig(provider="test", model_id="m", temperature=0.0) + assert m.temperature == 0.0 + + def test_temperature_boundary_two(self): + m = ModelConfig(provider="test", model_id="m", temperature=2.0) + assert m.temperature == 2.0 + + def test_max_tokens_zero_rejected(self): + with pytest.raises(ValidationError): + ModelConfig(provider="test", model_id="m", max_tokens=0) + + def test_max_tokens_negative_rejected(self): + with pytest.raises(ValidationError): + ModelConfig(provider="test", model_id="m", max_tokens=-1) + + def test_frozen(self, sample_model_config: ModelConfig): + with pytest.raises(ValidationError): + sample_model_config.temperature = 1.0 # type: ignore[misc] + + def test_factory(self): + m = ModelConfigFactory.build() + assert isinstance(m, ModelConfig) + assert 0.0 <= m.temperature <= 2.0 + + +# ── MemoryConfig ─────────────────────────────────────────────────── + + +@pytest.mark.unit +class TestMemoryConfig: + def test_defaults(self): + m = MemoryConfig() + assert m.type is MemoryType.SESSION + assert m.retention_days is None + + def test_custom_values(self): + m = MemoryConfig(type=MemoryType.PERSISTENT, retention_days=30) + assert m.type is MemoryType.PERSISTENT + assert m.retention_days == 30 + + def test_retention_days_zero_rejected(self): + with pytest.raises(ValidationError): + MemoryConfig(retention_days=0) + + def test_retention_days_negative_rejected(self): + with pytest.raises(ValidationError): + MemoryConfig(retention_days=-1) + + def test_frozen(self): + m = MemoryConfig() + with pytest.raises(ValidationError): + m.type = MemoryType.PERSISTENT # type: ignore[misc] + + def test_factory(self): + m = MemoryConfigFactory.build() + assert isinstance(m, MemoryConfig) + + +# ── ToolPermissions ──────────────────────────────────────────────── + + +@pytest.mark.unit +class TestToolPermissions: + def test_defaults(self): + t = ToolPermissions() + assert t.allowed == () + assert t.denied == () + + def test_custom_values(self): + t = ToolPermissions( + allowed=("file_system", "git"), + denied=("deployment",), + ) + assert "file_system" in t.allowed + assert "deployment" in t.denied + + def test_frozen(self): + t = ToolPermissions() + with pytest.raises(ValidationError): + t.allowed = ("new",) # type: ignore[misc] + + def test_factory(self): + t = ToolPermissionsFactory.build() + assert isinstance(t, ToolPermissions) + + +# ── AgentIdentity ────────────────────────────────────────────────── + + +@pytest.mark.unit +class TestAgentIdentity: + def test_valid_agent(self, sample_agent: AgentIdentity): + assert sample_agent.name == "Sarah Chen" + assert sample_agent.role == "Senior Backend Developer" + assert sample_agent.department == "Engineering" + assert sample_agent.level is SeniorityLevel.SENIOR + assert isinstance(sample_agent.id, UUID) + + def test_auto_generated_id(self, sample_model_config: ModelConfig): + agent = AgentIdentity( + name="Test Agent", + role="Developer", + department="Engineering", + model=sample_model_config, + hiring_date=date(2026, 1, 1), + ) + assert isinstance(agent.id, UUID) + + def test_defaults(self, sample_model_config: ModelConfig): + agent = AgentIdentity( + name="Test", + role="Dev", + department="Eng", + model=sample_model_config, + hiring_date=date(2026, 1, 1), + ) + assert agent.level is SeniorityLevel.MID + assert agent.status is AgentStatus.ACTIVE + assert isinstance(agent.personality, PersonalityConfig) + assert isinstance(agent.skills, SkillSet) + assert isinstance(agent.memory, MemoryConfig) + assert isinstance(agent.tools, ToolPermissions) + assert isinstance(agent.authority, Authority) + + def test_model_is_required(self): + with pytest.raises(ValidationError): + AgentIdentity( + name="Test", + role="Dev", + department="Eng", + hiring_date=date(2026, 1, 1), + ) # type: ignore[call-arg] + + def test_hiring_date_is_required(self, sample_model_config: ModelConfig): + with pytest.raises(ValidationError): + AgentIdentity( + name="Test", + role="Dev", + department="Eng", + model=sample_model_config, + ) # type: ignore[call-arg] + + def test_empty_name_rejected(self, sample_model_config: ModelConfig): + with pytest.raises(ValidationError): + AgentIdentity( + name="", + role="Dev", + department="Eng", + model=sample_model_config, + hiring_date=date(2026, 1, 1), + ) + + def test_frozen(self, sample_agent: AgentIdentity): + with pytest.raises(ValidationError): + sample_agent.name = "Changed" # type: ignore[misc] + + def test_model_copy_update(self, sample_agent: AgentIdentity): + updated = sample_agent.model_copy( + update={"status": AgentStatus.TERMINATED}, + ) + assert updated.status is AgentStatus.TERMINATED + assert sample_agent.status is AgentStatus.ACTIVE + + def test_json_roundtrip(self, sample_agent: AgentIdentity): + json_str = sample_agent.model_dump_json() + restored = AgentIdentity.model_validate_json(json_str) + assert restored.name == sample_agent.name + assert restored.id == sample_agent.id + assert restored.model.provider == sample_agent.model.provider + + def test_factory(self): + agent = AgentIdentityFactory.build() + assert isinstance(agent, AgentIdentity) + assert isinstance(agent.id, UUID) + assert isinstance(agent.model, ModelConfig) diff --git a/tests/unit/core/test_company.py b/tests/unit/core/test_company.py new file mode 100644 index 0000000000..4360d1a240 --- /dev/null +++ b/tests/unit/core/test_company.py @@ -0,0 +1,238 @@ +"""Tests for company structure and configuration models.""" + +import pytest +from pydantic import ValidationError + +from ai_company.core.company import ( + Company, + CompanyConfig, + Department, + HRRegistry, + Team, +) +from ai_company.core.enums import CompanyType + +from .conftest import ( + CompanyConfigFactory, + CompanyFactory, + DepartmentFactory, + HRRegistryFactory, + TeamFactory, +) + +# ── Team ─────────────────────────────────────────────────────────── + + +@pytest.mark.unit +class TestTeam: + def test_valid_team(self): + team = Team( + name="backend", + lead="backend_lead", + members=("dev_1", "dev_2"), + ) + assert team.name == "backend" + assert team.lead == "backend_lead" + assert len(team.members) == 2 + + def test_defaults(self): + team = Team(name="test", lead="lead") + assert team.members == () + + def test_empty_name_rejected(self): + with pytest.raises(ValidationError): + Team(name="", lead="lead") + + def test_empty_lead_rejected(self): + with pytest.raises(ValidationError): + Team(name="test", lead="") + + def test_frozen(self): + team = Team(name="test", lead="lead") + with pytest.raises(ValidationError): + team.name = "changed" # type: ignore[misc] + + def test_factory(self): + team = TeamFactory.build() + assert isinstance(team, Team) + + +# ── Department ───────────────────────────────────────────────────── + + +@pytest.mark.unit +class TestDepartment: + def test_valid_department(self, sample_department: Department): + assert sample_department.name == "Engineering" + assert sample_department.head == "cto" + assert sample_department.budget_percent == 60.0 + assert len(sample_department.teams) == 1 + + def test_defaults(self): + dept = Department(name="Test", head="head") + assert dept.budget_percent == 0.0 + assert dept.teams == () + + def test_budget_percent_zero(self): + dept = Department(name="Test", head="head", budget_percent=0.0) + assert dept.budget_percent == 0.0 + + def test_budget_percent_hundred(self): + dept = Department(name="Test", head="head", budget_percent=100.0) + assert dept.budget_percent == 100.0 + + def test_budget_percent_negative_rejected(self): + with pytest.raises(ValidationError): + Department(name="Test", head="head", budget_percent=-1.0) + + def test_budget_percent_over_hundred_rejected(self): + with pytest.raises(ValidationError): + Department(name="Test", head="head", budget_percent=100.1) + + def test_frozen(self, sample_department: Department): + with pytest.raises(ValidationError): + sample_department.name = "Changed" # type: ignore[misc] + + def test_factory(self): + dept = DepartmentFactory.build() + assert isinstance(dept, Department) + assert 0.0 <= dept.budget_percent <= 100.0 + + +# ── CompanyConfig ────────────────────────────────────────────────── + + +@pytest.mark.unit +class TestCompanyConfig: + def test_defaults(self): + cfg = CompanyConfig() + assert cfg.autonomy == 0.5 + assert cfg.budget_monthly == 100.0 + assert cfg.communication_pattern == "hybrid" + assert cfg.tool_access_default == () + + def test_autonomy_boundaries(self): + low = CompanyConfig(autonomy=0.0) + high = CompanyConfig(autonomy=1.0) + assert low.autonomy == 0.0 + assert high.autonomy == 1.0 + + def test_autonomy_below_zero_rejected(self): + with pytest.raises(ValidationError): + CompanyConfig(autonomy=-0.1) + + def test_autonomy_above_one_rejected(self): + with pytest.raises(ValidationError): + CompanyConfig(autonomy=1.1) + + def test_budget_negative_rejected(self): + with pytest.raises(ValidationError): + CompanyConfig(budget_monthly=-1.0) + + def test_empty_communication_pattern_rejected(self): + with pytest.raises(ValidationError): + CompanyConfig(communication_pattern="") + + def test_frozen(self): + cfg = CompanyConfig() + with pytest.raises(ValidationError): + cfg.autonomy = 1.0 # type: ignore[misc] + + def test_factory(self): + cfg = CompanyConfigFactory.build() + assert isinstance(cfg, CompanyConfig) + + +# ── HRRegistry ───────────────────────────────────────────────────── + + +@pytest.mark.unit +class TestHRRegistry: + def test_defaults(self): + hr = HRRegistry() + assert hr.active_agents == () + assert hr.available_roles == () + assert hr.hiring_queue == () + + def test_custom_values(self): + hr = HRRegistry( + active_agents=("agent_1",), + available_roles=("dev", "pm"), + hiring_queue=("designer",), + ) + assert len(hr.active_agents) == 1 + assert len(hr.available_roles) == 2 + + def test_frozen(self): + hr = HRRegistry() + with pytest.raises(ValidationError): + hr.active_agents = ("new",) # type: ignore[misc] + + def test_factory(self): + hr = HRRegistryFactory.build() + assert isinstance(hr, HRRegistry) + + +# ── Company ──────────────────────────────────────────────────────── + + +@pytest.mark.unit +class TestCompany: + def test_valid_company(self, sample_company: Company): + assert sample_company.name == "Test Corp" + assert len(sample_company.departments) == 1 + assert sample_company.config.budget_monthly == 100.0 + + def test_defaults(self): + co = Company(name="Minimal") + assert co.type is CompanyType.CUSTOM + assert co.departments == () + assert isinstance(co.config, CompanyConfig) + assert isinstance(co.hr_registry, HRRegistry) + + def test_budget_sum_at_100_accepted(self): + depts = ( + Department(name="A", head="a", budget_percent=60.0), + Department(name="B", head="b", budget_percent=40.0), + ) + co = Company(name="Full Budget", departments=depts) + assert sum(d.budget_percent for d in co.departments) == 100.0 + + def test_budget_sum_under_100_accepted(self): + depts = ( + Department(name="A", head="a", budget_percent=50.0), + Department(name="B", head="b", budget_percent=30.0), + ) + co = Company(name="With Reserve", departments=depts) + assert sum(d.budget_percent for d in co.departments) == 80.0 + + def test_budget_sum_over_100_rejected(self): + depts = ( + Department(name="A", head="a", budget_percent=60.0), + Department(name="B", head="b", budget_percent=50.0), + ) + with pytest.raises(ValidationError, match="exceeding 100%"): + Company(name="Over Budget", departments=depts) + + def test_empty_departments_accepted(self): + co = Company(name="Empty") + assert co.departments == () + + def test_frozen(self, sample_company: Company): + with pytest.raises(ValidationError): + sample_company.name = "Changed" # type: ignore[misc] + + def test_model_copy_update(self, sample_company: Company): + updated = sample_company.model_copy(update={"name": "New Corp"}) + assert updated.name == "New Corp" + assert sample_company.name == "Test Corp" + + def test_json_roundtrip(self, sample_company: Company): + json_str = sample_company.model_dump_json() + restored = Company.model_validate_json(json_str) + assert restored.name == sample_company.name + assert len(restored.departments) == len(sample_company.departments) + + def test_factory(self): + co = CompanyFactory.build() + assert isinstance(co, Company) diff --git a/tests/unit/core/test_enums.py b/tests/unit/core/test_enums.py new file mode 100644 index 0000000000..c934b3262f --- /dev/null +++ b/tests/unit/core/test_enums.py @@ -0,0 +1,151 @@ +"""Tests for core domain enumerations.""" + +import pytest + +from ai_company.core.enums import ( + AgentStatus, + CompanyType, + CostTier, + CreativityLevel, + DepartmentName, + MemoryType, + ProficiencyLevel, + RiskTolerance, + SeniorityLevel, + SkillCategory, +) + +# ── Member Counts ────────────────────────────────────────────────── + + +@pytest.mark.unit +class TestEnumMemberCounts: + def test_seniority_level_has_8_members(self): + assert len(SeniorityLevel) == 8 + + def test_agent_status_has_3_members(self): + assert len(AgentStatus) == 3 + + def test_risk_tolerance_has_3_members(self): + assert len(RiskTolerance) == 3 + + def test_creativity_level_has_3_members(self): + assert len(CreativityLevel) == 3 + + def test_memory_type_has_4_members(self): + assert len(MemoryType) == 4 + + def test_cost_tier_has_4_members(self): + assert len(CostTier) == 4 + + def test_company_type_has_8_members(self): + assert len(CompanyType) == 8 + + def test_skill_category_has_9_members(self): + assert len(SkillCategory) == 9 + + def test_proficiency_level_has_4_members(self): + assert len(ProficiencyLevel) == 4 + + def test_department_name_has_9_members(self): + assert len(DepartmentName) == 9 + + +# ── String Values ────────────────────────────────────────────────── + + +@pytest.mark.unit +class TestEnumStringValues: + def test_seniority_levels_are_lowercase(self): + for member in SeniorityLevel: + assert member.value == member.value.lower() + + def test_agent_status_values(self): + assert AgentStatus.ACTIVE == "active" + assert AgentStatus.ON_LEAVE == "on_leave" + assert AgentStatus.TERMINATED == "terminated" + + def test_cost_tier_values(self): + assert CostTier.LOW == "low" + assert CostTier.MEDIUM == "medium" + assert CostTier.HIGH == "high" + assert CostTier.PREMIUM == "premium" + + def test_company_type_values(self): + assert CompanyType.SOLO_FOUNDER == "solo_founder" + assert CompanyType.STARTUP == "startup" + assert CompanyType.CUSTOM == "custom" + + +# ── StrEnum Behavior ─────────────────────────────────────────────── + + +@pytest.mark.unit +class TestStrEnumBehavior: + def test_strenum_is_string(self): + assert isinstance(SeniorityLevel.JUNIOR, str) + + def test_strenum_equality_with_string(self): + assert SeniorityLevel.JUNIOR == "junior" + + def test_strenum_iteration(self): + levels = list(SeniorityLevel) + assert len(levels) == 8 + assert levels[0] == SeniorityLevel.JUNIOR + + def test_strenum_membership(self): + assert "senior" in [m.value for m in SeniorityLevel] + + def test_strenum_from_value(self): + assert SeniorityLevel("junior") is SeniorityLevel.JUNIOR + + def test_strenum_invalid_value_raises(self): + with pytest.raises(ValueError, match="not_a_level"): + SeniorityLevel("not_a_level") + + +# ── Pydantic Integration ────────────────────────────────────────── + + +@pytest.mark.unit +class TestEnumPydanticIntegration: + def test_enum_serializes_as_string(self): + from pydantic import BaseModel + + class _M(BaseModel): + level: SeniorityLevel + + m = _M(level=SeniorityLevel.SENIOR) + dumped = m.model_dump() + assert dumped["level"] == "senior" + + def test_enum_deserializes_from_string(self): + from pydantic import BaseModel + + class _M(BaseModel): + level: SeniorityLevel + + m = _M.model_validate({"level": "senior"}) + assert m.level is SeniorityLevel.SENIOR + + def test_enum_invalid_value_rejected(self): + from pydantic import BaseModel, ValidationError + + class _M(BaseModel): + level: SeniorityLevel + + with pytest.raises(ValidationError): + _M.model_validate({"level": "invalid"}) + + def test_enum_json_roundtrip(self): + from pydantic import BaseModel + + class _M(BaseModel): + status: AgentStatus + tier: CostTier + + m = _M(status=AgentStatus.ACTIVE, tier=CostTier.PREMIUM) + json_str = m.model_dump_json() + restored = _M.model_validate_json(json_str) + assert restored.status is AgentStatus.ACTIVE + assert restored.tier is CostTier.PREMIUM diff --git a/tests/unit/core/test_role.py b/tests/unit/core/test_role.py new file mode 100644 index 0000000000..928326a742 --- /dev/null +++ b/tests/unit/core/test_role.py @@ -0,0 +1,169 @@ +"""Tests for role and skill domain models.""" + +import pytest +from pydantic import ValidationError + +from ai_company.core.enums import ( + DepartmentName, + ProficiencyLevel, + SeniorityLevel, + SkillCategory, +) +from ai_company.core.role import Authority, CustomRole, Role, Skill + +from .conftest import ( + AuthorityFactory, + CustomRoleFactory, + RoleFactory, + SkillFactory, +) + +# ── Skill ────────────────────────────────────────────────────────── + + +@pytest.mark.unit +class TestSkill: + def test_valid_skill(self, sample_skill: Skill): + assert sample_skill.name == "python" + assert sample_skill.category is SkillCategory.ENGINEERING + assert sample_skill.proficiency is ProficiencyLevel.ADVANCED + + def test_default_proficiency(self): + skill = Skill(name="testing", category=SkillCategory.QA) + assert skill.proficiency is ProficiencyLevel.INTERMEDIATE + + def test_empty_name_rejected(self): + with pytest.raises(ValidationError): + Skill(name="", category=SkillCategory.ENGINEERING) + + def test_frozen(self, sample_skill: Skill): + with pytest.raises(ValidationError): + sample_skill.name = "rust" # type: ignore[misc] + + def test_json_roundtrip(self, sample_skill: Skill): + json_str = sample_skill.model_dump_json() + restored = Skill.model_validate_json(json_str) + assert restored == sample_skill + + def test_factory_creates_valid_skill(self): + skill = SkillFactory.build() + assert isinstance(skill, Skill) + assert len(skill.name) >= 1 + + +# ── Authority ────────────────────────────────────────────────────── + + +@pytest.mark.unit +class TestAuthority: + def test_valid_authority(self, sample_authority: Authority): + assert sample_authority.can_approve == ("code_reviews",) + assert sample_authority.reports_to == "engineering_lead" + assert sample_authority.budget_limit == 5.0 + + def test_defaults(self): + auth = Authority() + assert auth.can_approve == () + assert auth.reports_to is None + assert auth.can_delegate_to == () + assert auth.budget_limit == 0.0 + + def test_negative_budget_rejected(self): + with pytest.raises(ValidationError): + Authority(budget_limit=-1.0) + + def test_frozen(self, sample_authority: Authority): + with pytest.raises(ValidationError): + sample_authority.budget_limit = 10.0 # type: ignore[misc] + + def test_model_copy_update(self, sample_authority: Authority): + updated = sample_authority.model_copy(update={"budget_limit": 10.0}) + assert updated.budget_limit == 10.0 + assert sample_authority.budget_limit == 5.0 + + def test_factory_creates_valid_authority(self): + auth = AuthorityFactory.build() + assert isinstance(auth, Authority) + assert auth.budget_limit >= 0.0 + + +# ── Role ─────────────────────────────────────────────────────────── + + +@pytest.mark.unit +class TestRole: + def test_valid_role(self, sample_role: Role): + assert sample_role.name == "Backend Developer" + assert sample_role.department is DepartmentName.ENGINEERING + assert "python" in sample_role.required_skills + + def test_defaults(self): + role = Role(name="Test Role", department=DepartmentName.ENGINEERING) + assert role.required_skills == () + assert role.authority_level is SeniorityLevel.MID + assert role.tool_access == () + assert role.system_prompt_template is None + assert role.description == "" + + def test_empty_name_rejected(self): + with pytest.raises(ValidationError): + Role(name="", department=DepartmentName.ENGINEERING) + + def test_invalid_department_rejected(self): + with pytest.raises(ValidationError): + Role(name="Test", department="not_a_department") # type: ignore[arg-type] + + def test_frozen(self, sample_role: Role): + with pytest.raises(ValidationError): + sample_role.name = "Frontend Developer" # type: ignore[misc] + + def test_json_roundtrip(self, sample_role: Role): + json_str = sample_role.model_dump_json() + restored = Role.model_validate_json(json_str) + assert restored == sample_role + + def test_factory_creates_valid_role(self): + role = RoleFactory.build() + assert isinstance(role, Role) + assert len(role.name) >= 1 + + +# ── CustomRole ───────────────────────────────────────────────────── + + +@pytest.mark.unit +class TestCustomRole: + def test_with_standard_department(self): + role = CustomRole( + name="Blockchain Dev", + department=DepartmentName.ENGINEERING, + skills=("solidity", "web3"), + ) + assert role.department == DepartmentName.ENGINEERING + + def test_with_custom_department_string(self): + role = CustomRole( + name="Blockchain Dev", + department="blockchain", + skills=("solidity", "web3"), + ) + assert role.department == "blockchain" + + def test_defaults(self): + role = CustomRole(name="Test", department="custom") + assert role.skills == () + assert role.authority_level is SeniorityLevel.MID + assert role.suggested_model is None + + def test_empty_name_rejected(self): + with pytest.raises(ValidationError): + CustomRole(name="", department="custom") + + def test_frozen(self): + role = CustomRole(name="Test", department="custom") + with pytest.raises(ValidationError): + role.name = "Changed" # type: ignore[misc] + + def test_factory_creates_valid_custom_role(self): + role = CustomRoleFactory.build() + assert isinstance(role, CustomRole) diff --git a/tests/unit/core/test_role_catalog.py b/tests/unit/core/test_role_catalog.py new file mode 100644 index 0000000000..887e38dd97 --- /dev/null +++ b/tests/unit/core/test_role_catalog.py @@ -0,0 +1,185 @@ +"""Tests for the built-in role catalog and seniority mappings.""" + +import pytest + +from ai_company.core.enums import ( + CostTier, + DepartmentName, + SeniorityLevel, +) +from ai_company.core.role import Role, SeniorityInfo +from ai_company.core.role_catalog import ( + BUILTIN_ROLES, + SENIORITY_INFO, + get_builtin_role, + get_seniority_info, +) + +# ── Seniority Info ───────────────────────────────────────────────── + + +@pytest.mark.unit +class TestSeniorityInfo: + def test_has_8_entries(self): + assert len(SENIORITY_INFO) == 8 + + def test_covers_all_seniority_levels(self): + levels = {info.level for info in SENIORITY_INFO} + expected = set(SeniorityLevel) + assert levels == expected + + def test_no_duplicate_levels(self): + levels = [info.level for info in SENIORITY_INFO] + assert len(levels) == len(set(levels)) + + def test_all_entries_are_seniority_info(self): + for info in SENIORITY_INFO: + assert isinstance(info, SeniorityInfo) + + def test_junior_is_low_cost(self): + info = get_seniority_info(SeniorityLevel.JUNIOR) + assert info is not None + assert info.cost_tier == CostTier.LOW + + def test_c_suite_is_premium_cost(self): + info = get_seniority_info(SeniorityLevel.C_SUITE) + assert info is not None + assert info.cost_tier == CostTier.PREMIUM + + def test_senior_uses_sonnet_tier(self): + info = get_seniority_info(SeniorityLevel.SENIOR) + assert info is not None + assert info.typical_model_tier == "sonnet" + + def test_all_entries_frozen(self): + from pydantic import ValidationError + + for info in SENIORITY_INFO: + with pytest.raises(ValidationError): + info.level = SeniorityLevel.JUNIOR # type: ignore[misc] + + +# ── Builtin Roles ───────────────────────────────────────────────── + + +@pytest.mark.unit +class TestBuiltinRoles: + def test_has_31_roles(self): + assert len(BUILTIN_ROLES) == 31 + + def test_all_entries_are_role(self): + for role in BUILTIN_ROLES: + assert isinstance(role, Role) + + def test_no_duplicate_names(self): + names = [r.name for r in BUILTIN_ROLES] + assert len(names) == len(set(names)) + + def test_all_departments_represented(self): + departments = {r.department for r in BUILTIN_ROLES} + expected = set(DepartmentName) + assert departments == expected + + def test_c_suite_roles_present(self): + c_suite = [ + r for r in BUILTIN_ROLES if r.authority_level is SeniorityLevel.C_SUITE + ] + names = {r.name for r in c_suite} + assert {"CEO", "CTO", "CFO", "COO", "CPO"}.issubset(names) + + def test_all_roles_have_description(self): + for role in BUILTIN_ROLES: + assert role.description, f"{role.name} has no description" + + def test_all_roles_have_required_skills(self): + for role in BUILTIN_ROLES: + assert len(role.required_skills) > 0, f"{role.name} has no required_skills" + + def test_all_roles_frozen(self): + from pydantic import ValidationError + + for role in BUILTIN_ROLES: + with pytest.raises(ValidationError): + role.name = "Changed" # type: ignore[misc] + + +# ── Lookup Functions ─────────────────────────────────────────────── + + +@pytest.mark.unit +class TestGetBuiltinRole: + def test_exact_match(self): + role = get_builtin_role("CEO") + assert role is not None + assert role.name == "CEO" + + def test_case_insensitive(self): + role = get_builtin_role("ceo") + assert role is not None + assert role.name == "CEO" + + def test_mixed_case(self): + role = get_builtin_role("Backend Developer") + assert role is not None + assert role.name == "Backend Developer" + + def test_not_found_returns_none(self): + assert get_builtin_role("Nonexistent Role") is None + + def test_empty_string_returns_none(self): + assert get_builtin_role("") is None + + @pytest.mark.parametrize( + "name", + [ + "CEO", + "CTO", + "CFO", + "COO", + "CPO", + "Product Manager", + "UX Designer", + "UI Designer", + "UX Researcher", + "Technical Writer", + "Software Architect", + "Frontend Developer", + "Backend Developer", + "Full-Stack Developer", + "DevOps/SRE Engineer", + "Database Engineer", + "Security Engineer", + "QA Lead", + "QA Engineer", + "Automation Engineer", + "Performance Engineer", + "Data Analyst", + "Data Engineer", + "ML Engineer", + "Project Manager", + "Scrum Master", + "HR Manager", + "Security Operations", + "Content Writer", + "Brand Strategist", + "Growth Marketer", + ], + ) + def test_all_roles_lookupable(self, name): + role = get_builtin_role(name) + assert role is not None, f"Role {name!r} not found in catalog" + assert role.name == name + + +@pytest.mark.unit +class TestGetSeniorityInfo: + def test_found(self): + info = get_seniority_info(SeniorityLevel.SENIOR) + assert info is not None + assert info.level is SeniorityLevel.SENIOR + + @pytest.mark.parametrize("level", list(SeniorityLevel)) + def test_all_levels_lookupable(self, level): + info = get_seniority_info(level) + assert info is not None + assert info.level is level diff --git a/uv.lock b/uv.lock index c1bcb422f0..e414c46baa 100644 --- a/uv.lock +++ b/uv.lock @@ -5,6 +5,9 @@ requires-python = ">=3.14" [[package]] name = "ai-company" source = { editable = "." } +dependencies = [ + { name = "pydantic" }, +] [package.dev-dependencies] dev = [ @@ -13,7 +16,6 @@ dev = [ { name = "polyfactory" }, { name = "pre-commit" }, { name = "pre-commit-uv" }, - { name = "pydantic" }, { name = "pytest" }, { name = "pytest-asyncio" }, { name = "pytest-cov" }, @@ -35,6 +37,7 @@ test = [ ] [package.metadata] +requires-dist = [{ name = "pydantic", specifier = "==2.12.5" }] [package.metadata.requires-dev] dev = [ @@ -43,7 +46,6 @@ dev = [ { name = "polyfactory", specifier = "==3.3.0" }, { name = "pre-commit", specifier = "==4.5.1" }, { name = "pre-commit-uv", specifier = "==4.2.1" }, - { name = "pydantic", specifier = "==2.12.5" }, { name = "pytest", specifier = "==9.0.2" }, { name = "pytest-asyncio", specifier = "==1.3.0" }, { name = "pytest-cov", specifier = "==7.0.0" }, From c846432bed1409293fe9c31bef31a81e31fce72a Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Fri, 27 Feb 2026 22:13:49 +0100 Subject: [PATCH 2/6] fix: address 28 PR review items from local agents and external reviewers 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. --- src/ai_company/core/agent.py | 32 +++++- src/ai_company/core/company.py | 50 +++++++-- src/ai_company/core/enums.py | 3 +- src/ai_company/core/role.py | 18 +++- src/ai_company/core/role_catalog.py | 22 ++-- tests/unit/core/conftest.py | 8 +- tests/unit/core/test_agent.py | 133 ++++++++++++++++-------- tests/unit/core/test_company.py | 103 ++++++++++++------- tests/unit/core/test_enums.py | 48 ++++----- tests/unit/core/test_role.py | 146 +++++++++++++++++++++------ tests/unit/core/test_role_catalog.py | 58 ++++++----- 11 files changed, 440 insertions(+), 181 deletions(-) diff --git a/src/ai_company/core/agent.py b/src/ai_company/core/agent.py index 8b0dd7e040..faf73b81f8 100644 --- a/src/ai_company/core/agent.py +++ b/src/ai_company/core/agent.py @@ -3,7 +3,7 @@ from datetime import date # noqa: TC003 — required at runtime by Pydantic from uuid import UUID, uuid4 -from pydantic import BaseModel, ConfigDict, Field +from pydantic import BaseModel, ConfigDict, Field, model_validator from ai_company.core.enums import ( AgentStatus, @@ -70,6 +70,16 @@ class SkillSet(BaseModel): description="Secondary skills", ) + @model_validator(mode="after") + def _validate_no_empty_skills(self) -> SkillSet: + """Ensure no empty or whitespace-only skill names.""" + for field_name in ("primary", "secondary"): + for skill in getattr(self, field_name): + if not skill.strip(): + msg = f"Empty or whitespace-only skill name in {field_name}" + raise ValueError(msg) + return self + class ModelConfig(BaseModel): """LLM model configuration for an agent. @@ -99,6 +109,7 @@ class ModelConfig(BaseModel): ) fallback_model: str | None = Field( default=None, + min_length=1, description="Fallback model identifier", ) @@ -123,6 +134,14 @@ class MemoryConfig(BaseModel): description="Days to retain memories (None = forever)", ) + @model_validator(mode="after") + def _validate_retention_consistency(self) -> MemoryConfig: + """Ensure retention_days is None when memory type is 'none'.""" + if self.type is MemoryType.NONE and self.retention_days is not None: + msg = "retention_days must be None when memory type is 'none'" + raise ValueError(msg) + return self + class ToolPermissions(BaseModel): """Tool access permissions for an agent. @@ -143,6 +162,15 @@ class ToolPermissions(BaseModel): description="Explicitly denied tools", ) + @model_validator(mode="after") + def _validate_no_overlap(self) -> ToolPermissions: + """Ensure no tool appears in both allowed and denied lists.""" + overlap = set(self.allowed) & set(self.denied) + if overlap: + msg = f"Tools appear in both allowed and denied lists: {sorted(overlap)}" + raise ValueError(msg) + return self + class AgentIdentity(BaseModel): """Complete agent identity card. @@ -159,7 +187,7 @@ class AgentIdentity(BaseModel): level: Seniority level. personality: Personality configuration. skills: Primary and secondary skill set. - model: LLM model configuration (required). + model: LLM model configuration. memory: Memory configuration. tools: Tool permissions. authority: Authority scope. diff --git a/src/ai_company/core/company.py b/src/ai_company/core/company.py index 3b9a8c4d46..057191ccab 100644 --- a/src/ai_company/core/company.py +++ b/src/ai_company/core/company.py @@ -10,10 +10,13 @@ class Team(BaseModel): """A team within a department. + The ``lead`` is the team's manager and is not required to appear in + ``members``. ``members`` lists the individual contributors on the team. + Attributes: name: Team name. lead: Team lead agent name (string reference). - members: Team member agent names. + members: Team member agent names (excluding the lead). """ model_config = ConfigDict(frozen=True) @@ -29,8 +32,12 @@ class Team(BaseModel): class Department(BaseModel): """An organizational department. + Department names may be standard values from + :class:`~ai_company.core.enums.DepartmentName` or custom names defined + by the organization. + Attributes: - name: Department name. + name: Department name (standard or custom). head: Department head agent name (string reference). budget_percent: Percentage of company budget allocated (0-100). teams: Teams within this department. @@ -51,6 +58,16 @@ class Department(BaseModel): description="Teams within this department", ) + @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 in names if names.count(n) > 1}) + msg = f"Duplicate team names in department {self.name!r}: {dupes}" + raise ValueError(msg) + return self + class CompanyConfig(BaseModel): """Company-wide configuration settings. @@ -110,6 +127,16 @@ class HRRegistry(BaseModel): description="Roles in the hiring pipeline", ) + @model_validator(mode="after") + def _validate_no_duplicate_agents(self) -> HRRegistry: + """Ensure no duplicate entries in active_agents.""" + agents = self.active_agents + 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) + return self + class Company(BaseModel): """Top-level company entity. @@ -148,11 +175,22 @@ class Company(BaseModel): ) @model_validator(mode="after") - def _validate_budget_percent_sum(self) -> Company: - """Ensure department budget allocations do not exceed 100%.""" + 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] + 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) + + # Budget sum (round to 10 decimals to avoid float artifacts) max_budget_percent = 100.0 total = sum(d.budget_percent for d in self.departments) - if total > max_budget_percent: - msg = f"Department budget allocations sum to {total}%, exceeding 100%" + if round(total, 10) > max_budget_percent: + msg = ( + f"Department budget allocations sum to {total:.2f}%, " + f"exceeding {max_budget_percent:.0f}%" + ) raise ValueError(msg) return self diff --git a/src/ai_company/core/enums.py b/src/ai_company/core/enums.py index 7976a013b7..356d740f15 100644 --- a/src/ai_company/core/enums.py +++ b/src/ai_company/core/enums.py @@ -7,9 +7,10 @@ class SeniorityLevel(StrEnum): """Seniority levels for agents within the organization. Maps to authority scope, typical model tier, and cost tier. - See ``role_catalog.SENIORITY_INFO`` for the full mapping. + 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" diff --git a/src/ai_company/core/role.py b/src/ai_company/core/role.py index ad44f3081e..86a2aa6bc7 100644 --- a/src/ai_company/core/role.py +++ b/src/ai_company/core/role.py @@ -1,6 +1,6 @@ """Role and skill domain models.""" -from pydantic import BaseModel, ConfigDict, Field +from pydantic import BaseModel, ConfigDict, Field, field_validator from ai_company.core.enums import ( DepartmentName, @@ -47,6 +47,7 @@ class Authority(BaseModel): ) reports_to: str | None = Field( default=None, + min_length=1, description="Role this position reports to", ) can_delegate_to: tuple[str, ...] = Field( @@ -67,7 +68,7 @@ class SeniorityInfo(BaseModel): level: The seniority level. authority_scope: Description of authority at this level. typical_model_tier: Recommended model tier (e.g. ``"opus"``). - cost_tier: Expected cost tier for this level. + cost_tier: Cost tier identifier (built-in ``CostTier`` or user-defined string). """ model_config = ConfigDict(frozen=True) @@ -138,7 +139,7 @@ class CustomRole(BaseModel): Attributes: name: Custom role name. department: Department (standard or custom name). - skills: Required skills for this role. + required_skills: Required skills for this role. system_prompt_template: Template file for system prompt. authority_level: Default seniority level. suggested_model: Suggested model tier. @@ -150,7 +151,7 @@ class CustomRole(BaseModel): department: DepartmentName | str = Field( description="Department (standard or custom name)", ) - skills: tuple[str, ...] = Field( + required_skills: tuple[str, ...] = Field( default=(), description="Required skills for this role", ) @@ -166,3 +167,12 @@ class CustomRole(BaseModel): default=None, description="Suggested model tier", ) + + @field_validator("department") + @classmethod + def _department_not_empty(cls, v: DepartmentName | str) -> DepartmentName | str: + """Ensure department is not an empty string.""" + if isinstance(v, str) and not v.strip(): + msg = "Department name must not be empty" + raise ValueError(msg) + return v diff --git a/src/ai_company/core/role_catalog.py b/src/ai_company/core/role_catalog.py index d93652bb28..aa546aac25 100644 --- a/src/ai_company/core/role_catalog.py +++ b/src/ai_company/core/role_catalog.py @@ -386,6 +386,15 @@ ) +# ── Lookup Maps (built once at import time) ────────────────────── + +_BUILTIN_ROLES_BY_NAME: dict[str, Role] = {r.name.lower(): r for r in BUILTIN_ROLES} + +_SENIORITY_INFO_BY_LEVEL: dict[SeniorityLevel, SeniorityInfo] = { + info.level: info for info in SENIORITY_INFO +} + + def get_builtin_role(name: str) -> Role | None: """Look up a built-in role by name (case-insensitive). @@ -395,11 +404,7 @@ def get_builtin_role(name: str) -> Role | None: Returns: The matching Role, or ``None`` if not found. """ - lower = name.lower() - for role in BUILTIN_ROLES: - if role.name.lower() == lower: - return role - return None + return _BUILTIN_ROLES_BY_NAME.get(name.lower()) def get_seniority_info(level: SeniorityLevel) -> SeniorityInfo | None: @@ -410,8 +415,7 @@ def get_seniority_info(level: SeniorityLevel) -> SeniorityInfo | None: Returns: The matching SeniorityInfo, or ``None`` if not found. + Returns ``None`` only if the catalog is incomplete (all standard + levels are covered by default). """ - for info in SENIORITY_INFO: - if info.level == level: - return info - return None + return _SENIORITY_INFO_BY_LEVEL.get(level) diff --git a/tests/unit/core/conftest.py b/tests/unit/core/conftest.py index 3d76b1194d..8b04215d6b 100644 --- a/tests/unit/core/conftest.py +++ b/tests/unit/core/conftest.py @@ -23,6 +23,7 @@ ) from ai_company.core.enums import ( DepartmentName, + MemoryType, ProficiencyLevel, SeniorityLevel, SkillCategory, @@ -67,6 +68,7 @@ class ModelConfigFactory(ModelFactory): class MemoryConfigFactory(ModelFactory): __model__ = MemoryConfig + type = MemoryType.SESSION class ToolPermissionsFactory(ModelFactory): @@ -135,11 +137,11 @@ def sample_role() -> Role: @pytest.fixture def sample_model_config() -> ModelConfig: return ModelConfig( - provider="anthropic", - model_id="claude-sonnet-4-6", + provider="test-provider", + model_id="test-model-sonnet-4-6", temperature=0.3, max_tokens=8192, - fallback_model="openrouter/anthropic/claude-haiku", + fallback_model="test-provider/test-model-haiku", ) diff --git a/tests/unit/core/test_agent.py b/tests/unit/core/test_agent.py index 2834264c36..3f6b96e68d 100644 --- a/tests/unit/core/test_agent.py +++ b/tests/unit/core/test_agent.py @@ -37,7 +37,7 @@ @pytest.mark.unit class TestPersonalityConfig: - def test_defaults(self): + def test_defaults(self) -> None: p = PersonalityConfig() assert p.traits == () assert p.communication_style == "neutral" @@ -45,7 +45,7 @@ def test_defaults(self): assert p.creativity is CreativityLevel.MEDIUM assert p.description == "" - def test_custom_values(self): + def test_custom_values(self) -> None: p = PersonalityConfig( traits=("analytical", "pragmatic"), communication_style="concise and technical", @@ -56,16 +56,16 @@ def test_custom_values(self): assert len(p.traits) == 2 assert p.communication_style == "concise and technical" - def test_empty_communication_style_rejected(self): + def test_empty_communication_style_rejected(self) -> None: with pytest.raises(ValidationError): PersonalityConfig(communication_style="") - def test_frozen(self): + def test_frozen(self) -> None: p = PersonalityConfig() with pytest.raises(ValidationError): p.creativity = CreativityLevel.LOW # type: ignore[misc] - def test_factory(self): + def test_factory(self) -> None: p = PersonalityConfigFactory.build() assert isinstance(p, PersonalityConfig) @@ -75,12 +75,12 @@ def test_factory(self): @pytest.mark.unit class TestSkillSet: - def test_defaults(self): + def test_defaults(self) -> None: s = SkillSet() assert s.primary == () assert s.secondary == () - def test_custom_values(self): + def test_custom_values(self) -> None: s = SkillSet( primary=("python", "fastapi"), secondary=("docker", "redis"), @@ -88,12 +88,20 @@ def test_custom_values(self): assert "python" in s.primary assert "docker" in s.secondary - def test_frozen(self): + def test_empty_skill_name_rejected(self) -> None: + with pytest.raises(ValidationError, match="Empty or whitespace-only"): + SkillSet(primary=("python", "")) + + def test_whitespace_skill_name_rejected(self) -> None: + with pytest.raises(ValidationError, match="Empty or whitespace-only"): + SkillSet(secondary=(" ",)) + + def test_frozen(self) -> None: s = SkillSet() with pytest.raises(ValidationError): s.primary = ("new",) # type: ignore[misc] - def test_factory(self): + def test_factory(self) -> None: s = SkillSetFactory.build() assert isinstance(s, SkillSet) @@ -103,55 +111,59 @@ def test_factory(self): @pytest.mark.unit class TestModelConfig: - def test_valid_config(self, sample_model_config: ModelConfig): - assert sample_model_config.provider == "anthropic" - assert sample_model_config.model_id == "claude-sonnet-4-6" + def test_valid_config(self, sample_model_config: ModelConfig) -> None: + assert sample_model_config.provider == "test-provider" + assert sample_model_config.model_id == "test-model-sonnet-4-6" assert sample_model_config.temperature == 0.3 assert sample_model_config.max_tokens == 8192 - def test_defaults(self): + def test_defaults(self) -> None: m = ModelConfig(provider="test", model_id="test-model") assert m.temperature == 0.7 assert m.max_tokens == 4096 assert m.fallback_model is None - def test_empty_provider_rejected(self): + def test_empty_provider_rejected(self) -> None: with pytest.raises(ValidationError): ModelConfig(provider="", model_id="test") - def test_empty_model_id_rejected(self): + def test_empty_model_id_rejected(self) -> None: with pytest.raises(ValidationError): ModelConfig(provider="test", model_id="") - def test_temperature_below_zero_rejected(self): + def test_empty_fallback_model_rejected(self) -> None: + with pytest.raises(ValidationError): + ModelConfig(provider="test", model_id="m", fallback_model="") + + def test_temperature_below_zero_rejected(self) -> None: with pytest.raises(ValidationError): ModelConfig(provider="test", model_id="m", temperature=-0.1) - def test_temperature_above_two_rejected(self): + def test_temperature_above_two_rejected(self) -> None: with pytest.raises(ValidationError): ModelConfig(provider="test", model_id="m", temperature=2.1) - def test_temperature_boundary_zero(self): + def test_temperature_boundary_zero(self) -> None: m = ModelConfig(provider="test", model_id="m", temperature=0.0) assert m.temperature == 0.0 - def test_temperature_boundary_two(self): + def test_temperature_boundary_two(self) -> None: m = ModelConfig(provider="test", model_id="m", temperature=2.0) assert m.temperature == 2.0 - def test_max_tokens_zero_rejected(self): + def test_max_tokens_zero_rejected(self) -> None: with pytest.raises(ValidationError): ModelConfig(provider="test", model_id="m", max_tokens=0) - def test_max_tokens_negative_rejected(self): + def test_max_tokens_negative_rejected(self) -> None: with pytest.raises(ValidationError): ModelConfig(provider="test", model_id="m", max_tokens=-1) - def test_frozen(self, sample_model_config: ModelConfig): + def test_frozen(self, sample_model_config: ModelConfig) -> None: with pytest.raises(ValidationError): sample_model_config.temperature = 1.0 # type: ignore[misc] - def test_factory(self): + def test_factory(self) -> None: m = ModelConfigFactory.build() assert isinstance(m, ModelConfig) assert 0.0 <= m.temperature <= 2.0 @@ -162,30 +174,38 @@ def test_factory(self): @pytest.mark.unit class TestMemoryConfig: - def test_defaults(self): + def test_defaults(self) -> None: m = MemoryConfig() assert m.type is MemoryType.SESSION assert m.retention_days is None - def test_custom_values(self): + def test_custom_values(self) -> None: m = MemoryConfig(type=MemoryType.PERSISTENT, retention_days=30) assert m.type is MemoryType.PERSISTENT assert m.retention_days == 30 - def test_retention_days_zero_rejected(self): + def test_retention_days_zero_rejected(self) -> None: with pytest.raises(ValidationError): MemoryConfig(retention_days=0) - def test_retention_days_negative_rejected(self): + def test_retention_days_negative_rejected(self) -> None: with pytest.raises(ValidationError): MemoryConfig(retention_days=-1) - def test_frozen(self): + def test_none_type_with_retention_rejected(self) -> None: + with pytest.raises(ValidationError, match="retention_days must be None"): + MemoryConfig(type=MemoryType.NONE, retention_days=30) + + def test_none_type_without_retention_accepted(self) -> None: + m = MemoryConfig(type=MemoryType.NONE) + assert m.retention_days is None + + def test_frozen(self) -> None: m = MemoryConfig() with pytest.raises(ValidationError): m.type = MemoryType.PERSISTENT # type: ignore[misc] - def test_factory(self): + def test_factory(self) -> None: m = MemoryConfigFactory.build() assert isinstance(m, MemoryConfig) @@ -195,12 +215,12 @@ def test_factory(self): @pytest.mark.unit class TestToolPermissions: - def test_defaults(self): + def test_defaults(self) -> None: t = ToolPermissions() assert t.allowed == () assert t.denied == () - def test_custom_values(self): + def test_custom_values(self) -> None: t = ToolPermissions( allowed=("file_system", "git"), denied=("deployment",), @@ -208,12 +228,19 @@ def test_custom_values(self): assert "file_system" in t.allowed assert "deployment" in t.denied - def test_frozen(self): + def test_overlap_rejected(self) -> None: + with pytest.raises(ValidationError, match="both allowed and denied"): + ToolPermissions( + allowed=("git", "file_system"), + denied=("git",), + ) + + def test_frozen(self) -> None: t = ToolPermissions() with pytest.raises(ValidationError): t.allowed = ("new",) # type: ignore[misc] - def test_factory(self): + def test_factory(self) -> None: t = ToolPermissionsFactory.build() assert isinstance(t, ToolPermissions) @@ -223,14 +250,14 @@ def test_factory(self): @pytest.mark.unit class TestAgentIdentity: - def test_valid_agent(self, sample_agent: AgentIdentity): + def test_valid_agent(self, sample_agent: AgentIdentity) -> None: assert sample_agent.name == "Sarah Chen" assert sample_agent.role == "Senior Backend Developer" assert sample_agent.department == "Engineering" assert sample_agent.level is SeniorityLevel.SENIOR assert isinstance(sample_agent.id, UUID) - def test_auto_generated_id(self, sample_model_config: ModelConfig): + def test_auto_generated_id(self, sample_model_config: ModelConfig) -> None: agent = AgentIdentity( name="Test Agent", role="Developer", @@ -240,7 +267,7 @@ def test_auto_generated_id(self, sample_model_config: ModelConfig): ) assert isinstance(agent.id, UUID) - def test_defaults(self, sample_model_config: ModelConfig): + def test_defaults(self, sample_model_config: ModelConfig) -> None: agent = AgentIdentity( name="Test", role="Dev", @@ -256,7 +283,7 @@ def test_defaults(self, sample_model_config: ModelConfig): assert isinstance(agent.tools, ToolPermissions) assert isinstance(agent.authority, Authority) - def test_model_is_required(self): + def test_model_is_required(self) -> None: with pytest.raises(ValidationError): AgentIdentity( name="Test", @@ -265,7 +292,7 @@ def test_model_is_required(self): hiring_date=date(2026, 1, 1), ) # type: ignore[call-arg] - def test_hiring_date_is_required(self, sample_model_config: ModelConfig): + def test_hiring_date_is_required(self, sample_model_config: ModelConfig) -> None: with pytest.raises(ValidationError): AgentIdentity( name="Test", @@ -274,7 +301,7 @@ def test_hiring_date_is_required(self, sample_model_config: ModelConfig): model=sample_model_config, ) # type: ignore[call-arg] - def test_empty_name_rejected(self, sample_model_config: ModelConfig): + def test_empty_name_rejected(self, sample_model_config: ModelConfig) -> None: with pytest.raises(ValidationError): AgentIdentity( name="", @@ -284,25 +311,45 @@ def test_empty_name_rejected(self, sample_model_config: ModelConfig): hiring_date=date(2026, 1, 1), ) - def test_frozen(self, sample_agent: AgentIdentity): + def test_empty_role_rejected(self, sample_model_config: ModelConfig) -> None: + with pytest.raises(ValidationError): + AgentIdentity( + name="Test", + role="", + department="Eng", + model=sample_model_config, + hiring_date=date(2026, 1, 1), + ) + + def test_empty_department_rejected(self, sample_model_config: ModelConfig) -> None: + with pytest.raises(ValidationError): + AgentIdentity( + name="Test", + role="Dev", + department="", + model=sample_model_config, + hiring_date=date(2026, 1, 1), + ) + + def test_frozen(self, sample_agent: AgentIdentity) -> None: with pytest.raises(ValidationError): sample_agent.name = "Changed" # type: ignore[misc] - def test_model_copy_update(self, sample_agent: AgentIdentity): + def test_model_copy_update(self, sample_agent: AgentIdentity) -> None: updated = sample_agent.model_copy( update={"status": AgentStatus.TERMINATED}, ) assert updated.status is AgentStatus.TERMINATED assert sample_agent.status is AgentStatus.ACTIVE - def test_json_roundtrip(self, sample_agent: AgentIdentity): + def test_json_roundtrip(self, sample_agent: AgentIdentity) -> None: json_str = sample_agent.model_dump_json() restored = AgentIdentity.model_validate_json(json_str) assert restored.name == sample_agent.name assert restored.id == sample_agent.id assert restored.model.provider == sample_agent.model.provider - def test_factory(self): + def test_factory(self) -> None: agent = AgentIdentityFactory.build() assert isinstance(agent, AgentIdentity) assert isinstance(agent.id, UUID) diff --git a/tests/unit/core/test_company.py b/tests/unit/core/test_company.py index 4360d1a240..7aeee5967a 100644 --- a/tests/unit/core/test_company.py +++ b/tests/unit/core/test_company.py @@ -25,7 +25,7 @@ @pytest.mark.unit class TestTeam: - def test_valid_team(self): + def test_valid_team(self) -> None: team = Team( name="backend", lead="backend_lead", @@ -35,24 +35,24 @@ def test_valid_team(self): assert team.lead == "backend_lead" assert len(team.members) == 2 - def test_defaults(self): + def test_defaults(self) -> None: team = Team(name="test", lead="lead") assert team.members == () - def test_empty_name_rejected(self): + def test_empty_name_rejected(self) -> None: with pytest.raises(ValidationError): Team(name="", lead="lead") - def test_empty_lead_rejected(self): + def test_empty_lead_rejected(self) -> None: with pytest.raises(ValidationError): Team(name="test", lead="") - def test_frozen(self): + def test_frozen(self) -> None: team = Team(name="test", lead="lead") with pytest.raises(ValidationError): team.name = "changed" # type: ignore[misc] - def test_factory(self): + def test_factory(self) -> None: team = TeamFactory.build() assert isinstance(team, Team) @@ -62,38 +62,49 @@ def test_factory(self): @pytest.mark.unit class TestDepartment: - def test_valid_department(self, sample_department: Department): + def test_valid_department(self, sample_department: Department) -> None: assert sample_department.name == "Engineering" assert sample_department.head == "cto" assert sample_department.budget_percent == 60.0 assert len(sample_department.teams) == 1 - def test_defaults(self): + def test_defaults(self) -> None: dept = Department(name="Test", head="head") assert dept.budget_percent == 0.0 assert dept.teams == () - def test_budget_percent_zero(self): + def test_budget_percent_zero(self) -> None: dept = Department(name="Test", head="head", budget_percent=0.0) assert dept.budget_percent == 0.0 - def test_budget_percent_hundred(self): + def test_budget_percent_hundred(self) -> None: dept = Department(name="Test", head="head", budget_percent=100.0) assert dept.budget_percent == 100.0 - def test_budget_percent_negative_rejected(self): + def test_budget_percent_negative_rejected(self) -> None: with pytest.raises(ValidationError): Department(name="Test", head="head", budget_percent=-1.0) - def test_budget_percent_over_hundred_rejected(self): + def test_budget_percent_over_hundred_rejected(self) -> None: with pytest.raises(ValidationError): Department(name="Test", head="head", budget_percent=100.1) - def test_frozen(self, sample_department: Department): + def test_duplicate_team_names_rejected(self) -> None: + with pytest.raises(ValidationError, match="Duplicate team names"): + Department( + name="Eng", + head="head", + teams=( + Team(name="backend", lead="a"), + Team(name="backend", lead="b"), + ), + ) + + def test_frozen(self, sample_department: Department) -> None: with pytest.raises(ValidationError): sample_department.name = "Changed" # type: ignore[misc] - def test_factory(self): + def test_factory(self) -> None: dept = DepartmentFactory.build() assert isinstance(dept, Department) assert 0.0 <= dept.budget_percent <= 100.0 @@ -104,41 +115,41 @@ def test_factory(self): @pytest.mark.unit class TestCompanyConfig: - def test_defaults(self): + def test_defaults(self) -> None: cfg = CompanyConfig() assert cfg.autonomy == 0.5 assert cfg.budget_monthly == 100.0 assert cfg.communication_pattern == "hybrid" assert cfg.tool_access_default == () - def test_autonomy_boundaries(self): + def test_autonomy_boundaries(self) -> None: low = CompanyConfig(autonomy=0.0) high = CompanyConfig(autonomy=1.0) assert low.autonomy == 0.0 assert high.autonomy == 1.0 - def test_autonomy_below_zero_rejected(self): + def test_autonomy_below_zero_rejected(self) -> None: with pytest.raises(ValidationError): CompanyConfig(autonomy=-0.1) - def test_autonomy_above_one_rejected(self): + def test_autonomy_above_one_rejected(self) -> None: with pytest.raises(ValidationError): CompanyConfig(autonomy=1.1) - def test_budget_negative_rejected(self): + def test_budget_negative_rejected(self) -> None: with pytest.raises(ValidationError): CompanyConfig(budget_monthly=-1.0) - def test_empty_communication_pattern_rejected(self): + def test_empty_communication_pattern_rejected(self) -> None: with pytest.raises(ValidationError): CompanyConfig(communication_pattern="") - def test_frozen(self): + def test_frozen(self) -> None: cfg = CompanyConfig() with pytest.raises(ValidationError): cfg.autonomy = 1.0 # type: ignore[misc] - def test_factory(self): + def test_factory(self) -> None: cfg = CompanyConfigFactory.build() assert isinstance(cfg, CompanyConfig) @@ -148,13 +159,13 @@ def test_factory(self): @pytest.mark.unit class TestHRRegistry: - def test_defaults(self): + def test_defaults(self) -> None: hr = HRRegistry() assert hr.active_agents == () assert hr.available_roles == () assert hr.hiring_queue == () - def test_custom_values(self): + def test_custom_values(self) -> None: hr = HRRegistry( active_agents=("agent_1",), available_roles=("dev", "pm"), @@ -163,12 +174,16 @@ def test_custom_values(self): assert len(hr.active_agents) == 1 assert len(hr.available_roles) == 2 - def test_frozen(self): + def test_duplicate_active_agents_rejected(self) -> None: + with pytest.raises(ValidationError, match="Duplicate entries"): + HRRegistry(active_agents=("alice", "alice")) + + def test_frozen(self) -> None: hr = HRRegistry() with pytest.raises(ValidationError): hr.active_agents = ("new",) # type: ignore[misc] - def test_factory(self): + def test_factory(self) -> None: hr = HRRegistryFactory.build() assert isinstance(hr, HRRegistry) @@ -178,19 +193,19 @@ def test_factory(self): @pytest.mark.unit class TestCompany: - def test_valid_company(self, sample_company: Company): + def test_valid_company(self, sample_company: Company) -> None: assert sample_company.name == "Test Corp" assert len(sample_company.departments) == 1 assert sample_company.config.budget_monthly == 100.0 - def test_defaults(self): + def test_defaults(self) -> None: co = Company(name="Minimal") assert co.type is CompanyType.CUSTOM assert co.departments == () assert isinstance(co.config, CompanyConfig) assert isinstance(co.hr_registry, HRRegistry) - def test_budget_sum_at_100_accepted(self): + def test_budget_sum_at_100_accepted(self) -> None: depts = ( Department(name="A", head="a", budget_percent=60.0), Department(name="B", head="b", budget_percent=40.0), @@ -198,7 +213,7 @@ def test_budget_sum_at_100_accepted(self): co = Company(name="Full Budget", departments=depts) assert sum(d.budget_percent for d in co.departments) == 100.0 - def test_budget_sum_under_100_accepted(self): + def test_budget_sum_under_100_accepted(self) -> None: depts = ( Department(name="A", head="a", budget_percent=50.0), Department(name="B", head="b", budget_percent=30.0), @@ -206,7 +221,7 @@ def test_budget_sum_under_100_accepted(self): co = Company(name="With Reserve", departments=depts) assert sum(d.budget_percent for d in co.departments) == 80.0 - def test_budget_sum_over_100_rejected(self): + def test_budget_sum_over_100_rejected(self) -> None: depts = ( Department(name="A", head="a", budget_percent=60.0), Department(name="B", head="b", budget_percent=50.0), @@ -214,25 +229,41 @@ def test_budget_sum_over_100_rejected(self): with pytest.raises(ValidationError, match="exceeding 100%"): Company(name="Over Budget", departments=depts) - def test_empty_departments_accepted(self): + def test_budget_sum_barely_over_100_rejected(self) -> None: + depts = ( + Department(name="A", head="a", budget_percent=50.01), + Department(name="B", head="b", budget_percent=50.0), + ) + with pytest.raises(ValidationError, match="exceeding 100%"): + Company(name="Just Over", departments=depts) + + def test_duplicate_department_names_rejected(self) -> None: + depts = ( + Department(name="Engineering", head="a", budget_percent=30.0), + Department(name="Engineering", head="b", budget_percent=20.0), + ) + with pytest.raises(ValidationError, match="Duplicate department names"): + Company(name="Dup Depts", departments=depts) + + def test_empty_departments_accepted(self) -> None: co = Company(name="Empty") assert co.departments == () - def test_frozen(self, sample_company: Company): + def test_frozen(self, sample_company: Company) -> None: with pytest.raises(ValidationError): sample_company.name = "Changed" # type: ignore[misc] - def test_model_copy_update(self, sample_company: Company): + def test_model_copy_update(self, sample_company: Company) -> None: updated = sample_company.model_copy(update={"name": "New Corp"}) assert updated.name == "New Corp" assert sample_company.name == "Test Corp" - def test_json_roundtrip(self, sample_company: Company): + def test_json_roundtrip(self, sample_company: Company) -> None: json_str = sample_company.model_dump_json() restored = Company.model_validate_json(json_str) assert restored.name == sample_company.name assert len(restored.departments) == len(sample_company.departments) - def test_factory(self): + def test_factory(self) -> None: co = CompanyFactory.build() assert isinstance(co, Company) diff --git a/tests/unit/core/test_enums.py b/tests/unit/core/test_enums.py index c934b3262f..d2d92bebdb 100644 --- a/tests/unit/core/test_enums.py +++ b/tests/unit/core/test_enums.py @@ -20,34 +20,34 @@ @pytest.mark.unit class TestEnumMemberCounts: - def test_seniority_level_has_8_members(self): + def test_seniority_level_has_8_members(self) -> None: assert len(SeniorityLevel) == 8 - def test_agent_status_has_3_members(self): + def test_agent_status_has_3_members(self) -> None: assert len(AgentStatus) == 3 - def test_risk_tolerance_has_3_members(self): + def test_risk_tolerance_has_3_members(self) -> None: assert len(RiskTolerance) == 3 - def test_creativity_level_has_3_members(self): + def test_creativity_level_has_3_members(self) -> None: assert len(CreativityLevel) == 3 - def test_memory_type_has_4_members(self): + def test_memory_type_has_4_members(self) -> None: assert len(MemoryType) == 4 - def test_cost_tier_has_4_members(self): + def test_cost_tier_has_4_members(self) -> None: assert len(CostTier) == 4 - def test_company_type_has_8_members(self): + def test_company_type_has_8_members(self) -> None: assert len(CompanyType) == 8 - def test_skill_category_has_9_members(self): + def test_skill_category_has_9_members(self) -> None: assert len(SkillCategory) == 9 - def test_proficiency_level_has_4_members(self): + def test_proficiency_level_has_4_members(self) -> None: assert len(ProficiencyLevel) == 4 - def test_department_name_has_9_members(self): + def test_department_name_has_9_members(self) -> None: assert len(DepartmentName) == 9 @@ -56,22 +56,22 @@ def test_department_name_has_9_members(self): @pytest.mark.unit class TestEnumStringValues: - def test_seniority_levels_are_lowercase(self): + def test_seniority_levels_are_lowercase(self) -> None: for member in SeniorityLevel: assert member.value == member.value.lower() - def test_agent_status_values(self): + def test_agent_status_values(self) -> None: assert AgentStatus.ACTIVE == "active" assert AgentStatus.ON_LEAVE == "on_leave" assert AgentStatus.TERMINATED == "terminated" - def test_cost_tier_values(self): + def test_cost_tier_values(self) -> None: assert CostTier.LOW == "low" assert CostTier.MEDIUM == "medium" assert CostTier.HIGH == "high" assert CostTier.PREMIUM == "premium" - def test_company_type_values(self): + def test_company_type_values(self) -> None: assert CompanyType.SOLO_FOUNDER == "solo_founder" assert CompanyType.STARTUP == "startup" assert CompanyType.CUSTOM == "custom" @@ -82,24 +82,24 @@ def test_company_type_values(self): @pytest.mark.unit class TestStrEnumBehavior: - def test_strenum_is_string(self): + def test_strenum_is_string(self) -> None: assert isinstance(SeniorityLevel.JUNIOR, str) - def test_strenum_equality_with_string(self): + def test_strenum_equality_with_string(self) -> None: assert SeniorityLevel.JUNIOR == "junior" - def test_strenum_iteration(self): + def test_strenum_iteration(self) -> None: levels = list(SeniorityLevel) assert len(levels) == 8 assert levels[0] == SeniorityLevel.JUNIOR - def test_strenum_membership(self): + def test_strenum_membership(self) -> None: assert "senior" in [m.value for m in SeniorityLevel] - def test_strenum_from_value(self): + def test_strenum_from_value(self) -> None: assert SeniorityLevel("junior") is SeniorityLevel.JUNIOR - def test_strenum_invalid_value_raises(self): + def test_strenum_invalid_value_raises(self) -> None: with pytest.raises(ValueError, match="not_a_level"): SeniorityLevel("not_a_level") @@ -109,7 +109,7 @@ def test_strenum_invalid_value_raises(self): @pytest.mark.unit class TestEnumPydanticIntegration: - def test_enum_serializes_as_string(self): + def test_enum_serializes_as_string(self) -> None: from pydantic import BaseModel class _M(BaseModel): @@ -119,7 +119,7 @@ class _M(BaseModel): dumped = m.model_dump() assert dumped["level"] == "senior" - def test_enum_deserializes_from_string(self): + def test_enum_deserializes_from_string(self) -> None: from pydantic import BaseModel class _M(BaseModel): @@ -128,7 +128,7 @@ class _M(BaseModel): m = _M.model_validate({"level": "senior"}) assert m.level is SeniorityLevel.SENIOR - def test_enum_invalid_value_rejected(self): + def test_enum_invalid_value_rejected(self) -> None: from pydantic import BaseModel, ValidationError class _M(BaseModel): @@ -137,7 +137,7 @@ class _M(BaseModel): with pytest.raises(ValidationError): _M.model_validate({"level": "invalid"}) - def test_enum_json_roundtrip(self): + def test_enum_json_roundtrip(self) -> None: from pydantic import BaseModel class _M(BaseModel): diff --git a/tests/unit/core/test_role.py b/tests/unit/core/test_role.py index 928326a742..84aa67a58a 100644 --- a/tests/unit/core/test_role.py +++ b/tests/unit/core/test_role.py @@ -9,12 +9,13 @@ SeniorityLevel, SkillCategory, ) -from ai_company.core.role import Authority, CustomRole, Role, Skill +from ai_company.core.role import Authority, CustomRole, Role, SeniorityInfo, Skill from .conftest import ( AuthorityFactory, CustomRoleFactory, RoleFactory, + SeniorityInfoFactory, SkillFactory, ) @@ -23,29 +24,29 @@ @pytest.mark.unit class TestSkill: - def test_valid_skill(self, sample_skill: Skill): + def test_valid_skill(self, sample_skill: Skill) -> None: assert sample_skill.name == "python" assert sample_skill.category is SkillCategory.ENGINEERING assert sample_skill.proficiency is ProficiencyLevel.ADVANCED - def test_default_proficiency(self): + def test_default_proficiency(self) -> None: skill = Skill(name="testing", category=SkillCategory.QA) assert skill.proficiency is ProficiencyLevel.INTERMEDIATE - def test_empty_name_rejected(self): + def test_empty_name_rejected(self) -> None: with pytest.raises(ValidationError): Skill(name="", category=SkillCategory.ENGINEERING) - def test_frozen(self, sample_skill: Skill): + def test_frozen(self, sample_skill: Skill) -> None: with pytest.raises(ValidationError): sample_skill.name = "rust" # type: ignore[misc] - def test_json_roundtrip(self, sample_skill: Skill): + def test_json_roundtrip(self, sample_skill: Skill) -> None: json_str = sample_skill.model_dump_json() restored = Skill.model_validate_json(json_str) assert restored == sample_skill - def test_factory_creates_valid_skill(self): + def test_factory_creates_valid_skill(self) -> None: skill = SkillFactory.build() assert isinstance(skill, Skill) assert len(skill.name) >= 1 @@ -56,48 +57,114 @@ def test_factory_creates_valid_skill(self): @pytest.mark.unit class TestAuthority: - def test_valid_authority(self, sample_authority: Authority): + def test_valid_authority(self, sample_authority: Authority) -> None: assert sample_authority.can_approve == ("code_reviews",) assert sample_authority.reports_to == "engineering_lead" assert sample_authority.budget_limit == 5.0 - def test_defaults(self): + def test_defaults(self) -> None: auth = Authority() assert auth.can_approve == () assert auth.reports_to is None assert auth.can_delegate_to == () assert auth.budget_limit == 0.0 - def test_negative_budget_rejected(self): + def test_negative_budget_rejected(self) -> None: with pytest.raises(ValidationError): Authority(budget_limit=-1.0) - def test_frozen(self, sample_authority: Authority): + def test_empty_reports_to_rejected(self) -> None: + with pytest.raises(ValidationError): + Authority(reports_to="") + + def test_frozen(self, sample_authority: Authority) -> None: with pytest.raises(ValidationError): sample_authority.budget_limit = 10.0 # type: ignore[misc] - def test_model_copy_update(self, sample_authority: Authority): + def test_model_copy_update(self, sample_authority: Authority) -> None: updated = sample_authority.model_copy(update={"budget_limit": 10.0}) assert updated.budget_limit == 10.0 assert sample_authority.budget_limit == 5.0 - def test_factory_creates_valid_authority(self): + def test_json_roundtrip(self, sample_authority: Authority) -> None: + json_str = sample_authority.model_dump_json() + restored = Authority.model_validate_json(json_str) + assert restored == sample_authority + + def test_factory_creates_valid_authority(self) -> None: auth = AuthorityFactory.build() assert isinstance(auth, Authority) assert auth.budget_limit >= 0.0 +# ── SeniorityInfo ───────────────────────────────────────────────── + + +@pytest.mark.unit +class TestSeniorityInfo: + def test_valid_seniority_info(self) -> None: + info = SeniorityInfo( + level=SeniorityLevel.SENIOR, + authority_scope="Execute, design, and review", + typical_model_tier="sonnet", + cost_tier="high", + ) + assert info.level is SeniorityLevel.SENIOR + assert info.cost_tier == "high" + + def test_empty_authority_scope_rejected(self) -> None: + with pytest.raises(ValidationError): + SeniorityInfo( + level=SeniorityLevel.JUNIOR, + authority_scope="", + typical_model_tier="haiku", + cost_tier="low", + ) + + def test_empty_model_tier_rejected(self) -> None: + with pytest.raises(ValidationError): + SeniorityInfo( + level=SeniorityLevel.JUNIOR, + authority_scope="tasks", + typical_model_tier="", + cost_tier="low", + ) + + def test_empty_cost_tier_rejected(self) -> None: + with pytest.raises(ValidationError): + SeniorityInfo( + level=SeniorityLevel.JUNIOR, + authority_scope="tasks", + typical_model_tier="haiku", + cost_tier="", + ) + + def test_frozen(self) -> None: + info = SeniorityInfo( + level=SeniorityLevel.MID, + authority_scope="execute", + typical_model_tier="sonnet", + cost_tier="medium", + ) + with pytest.raises(ValidationError): + info.level = SeniorityLevel.SENIOR # type: ignore[misc] + + def test_factory_creates_valid_seniority_info(self) -> None: + info = SeniorityInfoFactory.build() + assert isinstance(info, SeniorityInfo) + + # ── Role ─────────────────────────────────────────────────────────── @pytest.mark.unit class TestRole: - def test_valid_role(self, sample_role: Role): + def test_valid_role(self, sample_role: Role) -> None: assert sample_role.name == "Backend Developer" assert sample_role.department is DepartmentName.ENGINEERING assert "python" in sample_role.required_skills - def test_defaults(self): + def test_defaults(self) -> None: role = Role(name="Test Role", department=DepartmentName.ENGINEERING) assert role.required_skills == () assert role.authority_level is SeniorityLevel.MID @@ -105,24 +172,24 @@ def test_defaults(self): assert role.system_prompt_template is None assert role.description == "" - def test_empty_name_rejected(self): + def test_empty_name_rejected(self) -> None: with pytest.raises(ValidationError): Role(name="", department=DepartmentName.ENGINEERING) - def test_invalid_department_rejected(self): + def test_invalid_department_rejected(self) -> None: with pytest.raises(ValidationError): Role(name="Test", department="not_a_department") # type: ignore[arg-type] - def test_frozen(self, sample_role: Role): + def test_frozen(self, sample_role: Role) -> None: with pytest.raises(ValidationError): sample_role.name = "Frontend Developer" # type: ignore[misc] - def test_json_roundtrip(self, sample_role: Role): + def test_json_roundtrip(self, sample_role: Role) -> None: json_str = sample_role.model_dump_json() restored = Role.model_validate_json(json_str) assert restored == sample_role - def test_factory_creates_valid_role(self): + def test_factory_creates_valid_role(self) -> None: role = RoleFactory.build() assert isinstance(role, Role) assert len(role.name) >= 1 @@ -133,37 +200,58 @@ def test_factory_creates_valid_role(self): @pytest.mark.unit class TestCustomRole: - def test_with_standard_department(self): + def test_with_standard_department(self) -> None: role = CustomRole( name="Blockchain Dev", department=DepartmentName.ENGINEERING, - skills=("solidity", "web3"), + required_skills=("solidity", "web3"), ) assert role.department == DepartmentName.ENGINEERING - def test_with_custom_department_string(self): + def test_with_custom_department_string(self) -> None: role = CustomRole( name="Blockchain Dev", department="blockchain", - skills=("solidity", "web3"), + required_skills=("solidity", "web3"), ) assert role.department == "blockchain" - def test_defaults(self): + def test_defaults(self) -> None: role = CustomRole(name="Test", department="custom") - assert role.skills == () + assert role.required_skills == () assert role.authority_level is SeniorityLevel.MID assert role.suggested_model is None - def test_empty_name_rejected(self): + def test_empty_name_rejected(self) -> None: with pytest.raises(ValidationError): CustomRole(name="", department="custom") - def test_frozen(self): + def test_empty_department_rejected(self) -> None: + with pytest.raises(ValidationError, match="Department name must not be empty"): + CustomRole(name="Test", department="") + + def test_whitespace_department_rejected(self) -> None: + with pytest.raises(ValidationError, match="Department name must not be empty"): + CustomRole(name="Test", department=" ") + + def test_frozen(self) -> None: role = CustomRole(name="Test", department="custom") with pytest.raises(ValidationError): role.name = "Changed" # type: ignore[misc] - def test_factory_creates_valid_custom_role(self): + def test_json_roundtrip(self) -> None: + role = CustomRole( + name="Custom Dev", + department="blockchain", + required_skills=("solidity",), + authority_level=SeniorityLevel.SENIOR, + ) + json_str = role.model_dump_json() + restored = CustomRole.model_validate_json(json_str) + assert restored.name == role.name + assert restored.department == role.department + assert restored.required_skills == role.required_skills + + def test_factory_creates_valid_custom_role(self) -> None: role = CustomRoleFactory.build() assert isinstance(role, CustomRole) diff --git a/tests/unit/core/test_role_catalog.py b/tests/unit/core/test_role_catalog.py index 887e38dd97..4bcfb8d63f 100644 --- a/tests/unit/core/test_role_catalog.py +++ b/tests/unit/core/test_role_catalog.py @@ -1,5 +1,7 @@ """Tests for the built-in role catalog and seniority mappings.""" +from unittest.mock import patch + import pytest from ai_company.core.enums import ( @@ -20,38 +22,38 @@ @pytest.mark.unit class TestSeniorityInfo: - def test_has_8_entries(self): + def test_has_8_entries(self) -> None: assert len(SENIORITY_INFO) == 8 - def test_covers_all_seniority_levels(self): + def test_covers_all_seniority_levels(self) -> None: levels = {info.level for info in SENIORITY_INFO} expected = set(SeniorityLevel) assert levels == expected - def test_no_duplicate_levels(self): + def test_no_duplicate_levels(self) -> None: levels = [info.level for info in SENIORITY_INFO] assert len(levels) == len(set(levels)) - def test_all_entries_are_seniority_info(self): + def test_all_entries_are_seniority_info(self) -> None: for info in SENIORITY_INFO: assert isinstance(info, SeniorityInfo) - def test_junior_is_low_cost(self): + def test_junior_is_low_cost(self) -> None: info = get_seniority_info(SeniorityLevel.JUNIOR) assert info is not None assert info.cost_tier == CostTier.LOW - def test_c_suite_is_premium_cost(self): + def test_c_suite_is_premium_cost(self) -> None: info = get_seniority_info(SeniorityLevel.C_SUITE) assert info is not None assert info.cost_tier == CostTier.PREMIUM - def test_senior_uses_sonnet_tier(self): + def test_senior_uses_sonnet_tier(self) -> None: info = get_seniority_info(SeniorityLevel.SENIOR) assert info is not None assert info.typical_model_tier == "sonnet" - def test_all_entries_frozen(self): + def test_all_entries_frozen(self) -> None: from pydantic import ValidationError for info in SENIORITY_INFO: @@ -64,38 +66,38 @@ def test_all_entries_frozen(self): @pytest.mark.unit class TestBuiltinRoles: - def test_has_31_roles(self): + def test_has_31_roles(self) -> None: assert len(BUILTIN_ROLES) == 31 - def test_all_entries_are_role(self): + def test_all_entries_are_role(self) -> None: for role in BUILTIN_ROLES: assert isinstance(role, Role) - def test_no_duplicate_names(self): + def test_no_duplicate_names(self) -> None: names = [r.name for r in BUILTIN_ROLES] assert len(names) == len(set(names)) - def test_all_departments_represented(self): + def test_all_departments_represented(self) -> None: departments = {r.department for r in BUILTIN_ROLES} expected = set(DepartmentName) assert departments == expected - def test_c_suite_roles_present(self): + def test_c_suite_roles_present(self) -> None: c_suite = [ r for r in BUILTIN_ROLES if r.authority_level is SeniorityLevel.C_SUITE ] names = {r.name for r in c_suite} assert {"CEO", "CTO", "CFO", "COO", "CPO"}.issubset(names) - def test_all_roles_have_description(self): + def test_all_roles_have_description(self) -> None: for role in BUILTIN_ROLES: assert role.description, f"{role.name} has no description" - def test_all_roles_have_required_skills(self): + def test_all_roles_have_required_skills(self) -> None: for role in BUILTIN_ROLES: assert len(role.required_skills) > 0, f"{role.name} has no required_skills" - def test_all_roles_frozen(self): + def test_all_roles_frozen(self) -> None: from pydantic import ValidationError for role in BUILTIN_ROLES: @@ -108,25 +110,25 @@ def test_all_roles_frozen(self): @pytest.mark.unit class TestGetBuiltinRole: - def test_exact_match(self): + def test_exact_match(self) -> None: role = get_builtin_role("CEO") assert role is not None assert role.name == "CEO" - def test_case_insensitive(self): + def test_case_insensitive(self) -> None: role = get_builtin_role("ceo") assert role is not None assert role.name == "CEO" - def test_mixed_case(self): + def test_mixed_case(self) -> None: role = get_builtin_role("Backend Developer") assert role is not None assert role.name == "Backend Developer" - def test_not_found_returns_none(self): + def test_not_found_returns_none(self) -> None: assert get_builtin_role("Nonexistent Role") is None - def test_empty_string_returns_none(self): + def test_empty_string_returns_none(self) -> None: assert get_builtin_role("") is None @pytest.mark.parametrize( @@ -165,7 +167,7 @@ def test_empty_string_returns_none(self): "Growth Marketer", ], ) - def test_all_roles_lookupable(self, name): + def test_all_roles_lookupable(self, name: str) -> None: role = get_builtin_role(name) assert role is not None, f"Role {name!r} not found in catalog" assert role.name == name @@ -173,13 +175,21 @@ def test_all_roles_lookupable(self, name): @pytest.mark.unit class TestGetSeniorityInfo: - def test_found(self): + def test_found(self) -> None: info = get_seniority_info(SeniorityLevel.SENIOR) assert info is not None assert info.level is SeniorityLevel.SENIOR @pytest.mark.parametrize("level", list(SeniorityLevel)) - def test_all_levels_lookupable(self, level): + def test_all_levels_lookupable(self, level: SeniorityLevel) -> None: info = get_seniority_info(level) assert info is not None assert info.level is level + + def test_returns_none_for_missing_level(self) -> None: + with patch.dict( + "ai_company.core.role_catalog._SENIORITY_INFO_BY_LEVEL", + {}, + clear=True, + ): + assert get_seniority_info(SeniorityLevel.JUNIOR) is None From 603a41321908aaf9e1c2cd17577e9be3f57633a7 Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Fri, 27 Feb 2026 22:42:37 +0100 Subject: [PATCH 3/6] fix: address 25 PR review items from local agents and external reviewers - 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 --- src/ai_company/core/agent.py | 40 +++++++++- src/ai_company/core/company.py | 76 +++++++++++++++--- src/ai_company/core/enums.py | 9 ++- src/ai_company/core/role.py | 40 +++++++++- src/ai_company/core/role_catalog.py | 28 ++++--- tests/unit/core/conftest.py | 4 + tests/unit/core/test_agent.py | 111 +++++++++++++++++++++++++++ tests/unit/core/test_company.py | 69 +++++++++++++++++ tests/unit/core/test_role.py | 44 ++++++++++- tests/unit/core/test_role_catalog.py | 28 ++++--- 10 files changed, 404 insertions(+), 45 deletions(-) diff --git a/src/ai_company/core/agent.py b/src/ai_company/core/agent.py index faf73b81f8..da79a58ecc 100644 --- a/src/ai_company/core/agent.py +++ b/src/ai_company/core/agent.py @@ -50,6 +50,15 @@ class PersonalityConfig(BaseModel): description="Extended personality description", ) + @model_validator(mode="after") + def _validate_no_empty_traits(self) -> PersonalityConfig: + """Ensure no empty or whitespace-only trait strings.""" + for trait in self.traits: + if not trait.strip(): + msg = "Empty or whitespace-only entry in traits" + raise ValueError(msg) + return self + class SkillSet(BaseModel): """Primary and secondary skills for an agent. @@ -113,6 +122,16 @@ class ModelConfig(BaseModel): description="Fallback model identifier", ) + @model_validator(mode="after") + def _validate_non_blank_identifiers(self) -> ModelConfig: + """Ensure identifier fields are not whitespace-only.""" + for field_name in ("provider", "model_id", "fallback_model"): + value = getattr(self, field_name) + if value is not None and not value.strip(): + msg = f"{field_name} must not be whitespace-only" + raise ValueError(msg) + return self + class MemoryConfig(BaseModel): """Memory configuration for an agent. @@ -136,7 +155,7 @@ class MemoryConfig(BaseModel): @model_validator(mode="after") def _validate_retention_consistency(self) -> MemoryConfig: - """Ensure retention_days is None when memory type is 'none'.""" + """Ensure retention_days is None when memory type is MemoryType.NONE.""" if self.type is MemoryType.NONE and self.retention_days is not None: msg = "retention_days must be None when memory type is 'none'" raise ValueError(msg) @@ -162,6 +181,16 @@ class ToolPermissions(BaseModel): description="Explicitly denied tools", ) + @model_validator(mode="after") + def _validate_no_empty_tools(self) -> ToolPermissions: + """Ensure no empty or whitespace-only tool names.""" + for field_name in ("allowed", "denied"): + for tool in getattr(self, field_name): + if not tool.strip(): + msg = f"Empty or whitespace-only tool name in {field_name}" + raise ValueError(msg) + return self + @model_validator(mode="after") def _validate_no_overlap(self) -> ToolPermissions: """Ensure no tool appears in both allowed and denied lists.""" @@ -231,3 +260,12 @@ class AgentIdentity(BaseModel): default=AgentStatus.ACTIVE, description="Current lifecycle status", ) + + @model_validator(mode="after") + def _validate_non_blank_identifiers(self) -> AgentIdentity: + """Ensure name, role, and department are not whitespace-only.""" + for field_name in ("name", "role", "department"): + if not getattr(self, field_name).strip(): + msg = f"{field_name} must not be whitespace-only" + raise ValueError(msg) + return self diff --git a/src/ai_company/core/company.py b/src/ai_company/core/company.py index 057191ccab..c5a7870a4e 100644 --- a/src/ai_company/core/company.py +++ b/src/ai_company/core/company.py @@ -1,22 +1,26 @@ """Company structure and configuration models.""" +from collections import Counter from uuid import UUID, uuid4 from pydantic import BaseModel, ConfigDict, Field, model_validator from ai_company.core.enums import CompanyType +_BUDGET_ROUNDING_PRECISION = 10 +"""Decimal places for budget sum rounding; avoids IEEE 754 float artifacts.""" + class Team(BaseModel): """A team within a department. - The ``lead`` is the team's manager and is not required to appear in - ``members``. ``members`` lists the individual contributors on the team. + The ``lead`` is the team's manager. The ``lead`` may also appear in + ``members`` if they are also an individual contributor. Attributes: name: Team name. lead: Team lead agent name (string reference). - members: Team member agent names (excluding the lead). + members: Team member agent names. """ model_config = ConfigDict(frozen=True) @@ -28,6 +32,19 @@ class Team(BaseModel): description="Team member agent names", ) + @model_validator(mode="after") + def _validate_strings(self) -> Team: + """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) + return self + class Department(BaseModel): """An organizational department. @@ -58,12 +75,21 @@ class Department(BaseModel): description="Teams within this department", ) + @model_validator(mode="after") + def _validate_non_blank_identifiers(self) -> Department: + """Ensure name and head are not whitespace-only.""" + for field_name in ("name", "head"): + if not getattr(self, field_name).strip(): + msg = f"{field_name} must not be whitespace-only" + raise ValueError(msg) + return self + @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 in names if names.count(n) > 1}) + 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 @@ -102,6 +128,18 @@ class CompanyConfig(BaseModel): description="Default tool access for all agents", ) + @model_validator(mode="after") + def _validate_strings(self) -> CompanyConfig: + """Ensure no whitespace-only identifiers or tool access entries.""" + if not self.communication_pattern.strip(): + msg = "communication_pattern must not be whitespace-only" + raise ValueError(msg) + for tool in self.tool_access_default: + if not tool.strip(): + msg = "Empty or whitespace-only entry in tool_access_default" + raise ValueError(msg) + return self + class HRRegistry(BaseModel): """Human resources registry for the company. @@ -128,11 +166,16 @@ class HRRegistry(BaseModel): ) @model_validator(mode="after") - def _validate_no_duplicate_agents(self) -> HRRegistry: - """Ensure no duplicate entries in active_agents.""" + def _validate_entries(self) -> HRRegistry: + """Ensure no empty strings and no duplicate entries in active_agents.""" + for field_name in ("active_agents", "available_roles", "hiring_queue"): + for value in getattr(self, field_name): + if not value.strip(): + msg = f"Empty or whitespace-only entry in {field_name}" + raise ValueError(msg) agents = self.active_agents if len(agents) != len(set(agents)): - dupes = sorted({a for a in agents if agents.count(a) > 1}) + dupes = sorted(a for a, c in Counter(agents).items() if c > 1) msg = f"Duplicate entries in active_agents: {dupes}" raise ValueError(msg) return self @@ -141,8 +184,9 @@ def _validate_no_duplicate_agents(self) -> HRRegistry: class Company(BaseModel): """Top-level company entity. - Validates that department budget allocations do not exceed 100%. - The sum may be less than 100% to allow for an unallocated reserve. + Validates that department names are unique and that budget allocations + do not exceed 100%. The sum may be less than 100% to allow for an + unallocated reserve. Attributes: id: Company identifier. @@ -174,20 +218,28 @@ class Company(BaseModel): description="HR registry", ) + @model_validator(mode="after") + def _validate_non_blank_name(self) -> Company: + """Ensure company name is not whitespace-only.""" + if not self.name.strip(): + msg = "name must not be whitespace-only" + 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] if len(names) != len(set(names)): - dupes = sorted({n for n in names if names.count(n) > 1}) + dupes = sorted(n for n, c in Counter(names).items() if c > 1) msg = f"Duplicate department names: {dupes}" raise ValueError(msg) - # Budget sum (round to 10 decimals to avoid float artifacts) + # Budget sum max_budget_percent = 100.0 total = sum(d.budget_percent for d in self.departments) - if round(total, 10) > max_budget_percent: + if round(total, _BUDGET_ROUNDING_PRECISION) > max_budget_percent: msg = ( f"Department budget allocations sum to {total:.2f}%, " f"exceeding {max_budget_percent:.0f}%" diff --git a/src/ai_company/core/enums.py b/src/ai_company/core/enums.py index 356d740f15..8f346a02f2 100644 --- a/src/ai_company/core/enums.py +++ b/src/ai_company/core/enums.py @@ -6,8 +6,8 @@ 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. + Each level corresponds to an authority scope, typical model tier, and + cost tier defined in ``ai_company.core.role_catalog.SENIORITY_INFO``. """ # Design spec says "Intern/Junior" — collapsed to a single JUNIOR level. @@ -58,8 +58,9 @@ class CostTier(StrEnum): """Built-in cost tier identifiers. These are the default tiers shipped with the framework. Users can - define additional tiers via configuration; any field that accepts a - cost tier uses ``str`` so custom tier IDs are valid too. + define additional tiers via configuration. Fields that accept cost + tiers (e.g. ``SeniorityInfo.cost_tier``) use ``str`` rather than + this enum, so custom tier IDs are also valid. """ LOW = "low" diff --git a/src/ai_company/core/role.py b/src/ai_company/core/role.py index 86a2aa6bc7..46301b9ae0 100644 --- a/src/ai_company/core/role.py +++ b/src/ai_company/core/role.py @@ -1,6 +1,6 @@ """Role and skill domain models.""" -from pydantic import BaseModel, ConfigDict, Field, field_validator +from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator from ai_company.core.enums import ( DepartmentName, @@ -60,6 +60,16 @@ class Authority(BaseModel): description="Maximum USD per task", ) + @model_validator(mode="after") + def _validate_no_empty_strings(self) -> Authority: + """Ensure no empty or whitespace-only entries in string tuples.""" + for field_name in ("can_approve", "can_delegate_to"): + for value in getattr(self, field_name): + if not value.strip(): + msg = f"Empty or whitespace-only entry in {field_name}" + raise ValueError(msg) + return self + class SeniorityInfo(BaseModel): """Mapping from seniority level to authority and model configuration. @@ -128,6 +138,16 @@ class Role(BaseModel): description="Human-readable description", ) + @model_validator(mode="after") + def _validate_no_empty_strings(self) -> Role: + """Ensure no empty or whitespace-only entries in string tuples.""" + for field_name in ("required_skills", "tool_access"): + for value in getattr(self, field_name): + if not value.strip(): + msg = f"Empty or whitespace-only entry in {field_name}" + raise ValueError(msg) + return self + class CustomRole(BaseModel): """User-defined custom role via configuration. @@ -171,8 +191,20 @@ class CustomRole(BaseModel): @field_validator("department") @classmethod def _department_not_empty(cls, v: DepartmentName | str) -> DepartmentName | str: - """Ensure department is not an empty string.""" - if isinstance(v, str) and not v.strip(): + """Ensure department is not empty and normalize whitespace.""" + if isinstance(v, DepartmentName): + return v + stripped = v.strip() + if not stripped: msg = "Department name must not be empty" raise ValueError(msg) - return v + return stripped + + @model_validator(mode="after") + def _validate_no_empty_required_skills(self) -> CustomRole: + """Ensure no empty or whitespace-only entries in required_skills.""" + for value in self.required_skills: + if not value.strip(): + msg = "Empty or whitespace-only entry in required_skills" + raise ValueError(msg) + return self diff --git a/src/ai_company/core/role_catalog.py b/src/ai_company/core/role_catalog.py index aa546aac25..25a449f08c 100644 --- a/src/ai_company/core/role_catalog.py +++ b/src/ai_company/core/role_catalog.py @@ -1,7 +1,7 @@ """Built-in role catalog and seniority information. -Provides the canonical set of roles and seniority mappings derived from -the design specification (DESIGN_SPEC.md sections 3.2 and 3.3). +Provides the canonical set of built-in roles from DESIGN_SPEC.md section 3.3 +and the seniority mapping from section 3.2. """ from ai_company.core.enums import ( @@ -388,7 +388,10 @@ # ── Lookup Maps (built once at import time) ────────────────────── -_BUILTIN_ROLES_BY_NAME: dict[str, Role] = {r.name.lower(): r for r in BUILTIN_ROLES} +_BUILTIN_ROLES_BY_NAME: dict[str, Role] = {r.name.casefold(): r for r in BUILTIN_ROLES} +if len(_BUILTIN_ROLES_BY_NAME) != len(BUILTIN_ROLES): + _msg = "Duplicate built-in role names after case-normalization" + raise ValueError(_msg) _SENIORITY_INFO_BY_LEVEL: dict[SeniorityLevel, SeniorityInfo] = { info.level: info for info in SENIORITY_INFO @@ -396,7 +399,7 @@ def get_builtin_role(name: str) -> Role | None: - """Look up a built-in role by name (case-insensitive). + """Look up a built-in role by name (case-insensitive, whitespace-stripped). Args: name: Role name to search for. @@ -404,18 +407,23 @@ def get_builtin_role(name: str) -> Role | None: Returns: The matching Role, or ``None`` if not found. """ - return _BUILTIN_ROLES_BY_NAME.get(name.lower()) + return _BUILTIN_ROLES_BY_NAME.get(name.strip().casefold()) -def get_seniority_info(level: SeniorityLevel) -> SeniorityInfo | None: +def get_seniority_info(level: SeniorityLevel) -> SeniorityInfo: """Look up seniority info by level. Args: level: The seniority level to look up. Returns: - The matching SeniorityInfo, or ``None`` if not found. - Returns ``None`` only if the catalog is incomplete (all standard - levels are covered by default). + The matching SeniorityInfo. + + Raises: + LookupError: If no entry exists for the given level. """ - return _SENIORITY_INFO_BY_LEVEL.get(level) + info = _SENIORITY_INFO_BY_LEVEL.get(level) + if info is None: + msg = f"No seniority info for level {level!r}; catalog may be incomplete" + raise LookupError(msg) + return info diff --git a/tests/unit/core/conftest.py b/tests/unit/core/conftest.py index 8b04215d6b..526869bb64 100644 --- a/tests/unit/core/conftest.py +++ b/tests/unit/core/conftest.py @@ -73,10 +73,14 @@ class MemoryConfigFactory(ModelFactory): class ToolPermissionsFactory(ModelFactory): __model__ = ToolPermissions + allowed = () + denied = () class AgentIdentityFactory(ModelFactory): __model__ = AgentIdentity + memory = MemoryConfigFactory + tools = ToolPermissionsFactory class TeamFactory(ModelFactory): diff --git a/tests/unit/core/test_agent.py b/tests/unit/core/test_agent.py index 3f6b96e68d..06f60370c1 100644 --- a/tests/unit/core/test_agent.py +++ b/tests/unit/core/test_agent.py @@ -60,6 +60,14 @@ def test_empty_communication_style_rejected(self) -> None: with pytest.raises(ValidationError): PersonalityConfig(communication_style="") + def test_empty_trait_rejected(self) -> None: + with pytest.raises(ValidationError, match="Empty or whitespace-only"): + PersonalityConfig(traits=("analytical", "")) + + def test_whitespace_trait_rejected(self) -> None: + with pytest.raises(ValidationError, match="Empty or whitespace-only"): + PersonalityConfig(traits=(" ",)) + def test_frozen(self) -> None: p = PersonalityConfig() with pytest.raises(ValidationError): @@ -96,6 +104,14 @@ def test_whitespace_skill_name_rejected(self) -> None: with pytest.raises(ValidationError, match="Empty or whitespace-only"): SkillSet(secondary=(" ",)) + def test_empty_primary_error_mentions_primary(self) -> None: + with pytest.raises(ValidationError, match="primary"): + SkillSet(primary=("python", "")) + + def test_empty_secondary_error_mentions_secondary(self) -> None: + with pytest.raises(ValidationError, match="secondary"): + SkillSet(secondary=(" ",)) + def test_frozen(self) -> None: s = SkillSet() with pytest.raises(ValidationError): @@ -159,6 +175,18 @@ def test_max_tokens_negative_rejected(self) -> None: with pytest.raises(ValidationError): ModelConfig(provider="test", model_id="m", max_tokens=-1) + def test_whitespace_provider_rejected(self) -> None: + with pytest.raises(ValidationError, match="whitespace-only"): + ModelConfig(provider=" ", model_id="test") + + def test_whitespace_model_id_rejected(self) -> None: + with pytest.raises(ValidationError, match="whitespace-only"): + ModelConfig(provider="test", model_id=" ") + + def test_whitespace_fallback_model_rejected(self) -> None: + with pytest.raises(ValidationError, match="whitespace-only"): + ModelConfig(provider="test", model_id="m", fallback_model=" ") + def test_frozen(self, sample_model_config: ModelConfig) -> None: with pytest.raises(ValidationError): sample_model_config.temperature = 1.0 # type: ignore[misc] @@ -235,6 +263,24 @@ def test_overlap_rejected(self) -> None: denied=("git",), ) + def test_multiple_overlapping_tools_all_reported(self) -> None: + with pytest.raises(ValidationError) as exc_info: + ToolPermissions( + allowed=("git", "deploy", "shell"), + denied=("git", "deploy"), + ) + error_text = str(exc_info.value) + assert "deploy" in error_text + assert "git" in error_text + + def test_empty_tool_name_rejected(self) -> None: + with pytest.raises(ValidationError, match="Empty or whitespace-only"): + ToolPermissions(allowed=("git", "")) + + def test_whitespace_tool_name_rejected(self) -> None: + with pytest.raises(ValidationError, match="Empty or whitespace-only"): + ToolPermissions(denied=(" ",)) + def test_frozen(self) -> None: t = ToolPermissions() with pytest.raises(ValidationError): @@ -331,6 +377,38 @@ def test_empty_department_rejected(self, sample_model_config: ModelConfig) -> No hiring_date=date(2026, 1, 1), ) + def test_whitespace_name_rejected(self, sample_model_config: ModelConfig) -> None: + with pytest.raises(ValidationError, match="whitespace-only"): + AgentIdentity( + name=" ", + role="Dev", + department="Eng", + model=sample_model_config, + hiring_date=date(2026, 1, 1), + ) + + def test_whitespace_role_rejected(self, sample_model_config: ModelConfig) -> None: + with pytest.raises(ValidationError, match="whitespace-only"): + AgentIdentity( + name="Test", + role=" ", + department="Eng", + model=sample_model_config, + hiring_date=date(2026, 1, 1), + ) + + def test_whitespace_department_rejected( + self, sample_model_config: ModelConfig + ) -> None: + with pytest.raises(ValidationError, match="whitespace-only"): + AgentIdentity( + name="Test", + role="Dev", + department=" ", + model=sample_model_config, + hiring_date=date(2026, 1, 1), + ) + def test_frozen(self, sample_agent: AgentIdentity) -> None: with pytest.raises(ValidationError): sample_agent.name = "Changed" # type: ignore[misc] @@ -349,6 +427,39 @@ def test_json_roundtrip(self, sample_agent: AgentIdentity) -> None: assert restored.id == sample_agent.id assert restored.model.provider == sample_agent.model.provider + def test_json_roundtrip_with_full_nested_data( + self, sample_model_config: ModelConfig + ) -> None: + agent = AgentIdentity( + name="Full Agent", + role="Lead Dev", + department="Engineering", + level=SeniorityLevel.LEAD, + personality=PersonalityConfig( + traits=("analytical", "pragmatic"), + communication_style="direct", + risk_tolerance=RiskTolerance.HIGH, + ), + skills=SkillSet( + primary=("python", "architecture"), + secondary=("docker",), + ), + model=sample_model_config, + memory=MemoryConfig(type=MemoryType.PERSISTENT, retention_days=90), + tools=ToolPermissions(allowed=("git",), denied=("deploy",)), + authority=Authority( + can_approve=("code_review",), + reports_to="cto", + can_delegate_to=("junior_dev",), + budget_limit=50.0, + ), + hiring_date=date(2026, 1, 15), + status=AgentStatus.ACTIVE, + ) + json_str = agent.model_dump_json() + restored = AgentIdentity.model_validate_json(json_str) + assert restored == agent + def test_factory(self) -> None: agent = AgentIdentityFactory.build() assert isinstance(agent, AgentIdentity) diff --git a/tests/unit/core/test_company.py b/tests/unit/core/test_company.py index 7aeee5967a..637635841d 100644 --- a/tests/unit/core/test_company.py +++ b/tests/unit/core/test_company.py @@ -47,6 +47,22 @@ def test_empty_lead_rejected(self) -> None: with pytest.raises(ValidationError): Team(name="test", lead="") + def test_whitespace_name_rejected(self) -> None: + with pytest.raises(ValidationError): + Team(name=" ", lead="lead") + + def test_whitespace_lead_rejected(self) -> None: + with pytest.raises(ValidationError): + Team(name="test", lead=" ") + + def test_empty_member_name_rejected(self) -> None: + with pytest.raises(ValidationError, match="Empty or whitespace-only"): + Team(name="test", lead="lead", members=("dev", "")) + + def test_whitespace_member_name_rejected(self) -> None: + with pytest.raises(ValidationError, match="Empty or whitespace-only"): + Team(name="test", lead="lead", members=(" ",)) + def test_frozen(self) -> None: team = Team(name="test", lead="lead") with pytest.raises(ValidationError): @@ -89,6 +105,18 @@ def test_budget_percent_over_hundred_rejected(self) -> None: with pytest.raises(ValidationError): Department(name="Test", head="head", budget_percent=100.1) + def test_multiple_distinct_teams_accepted(self) -> None: + dept = Department( + name="Engineering", + head="cto", + teams=( + Team(name="backend", lead="a"), + Team(name="frontend", lead="b"), + Team(name="infra", lead="c"), + ), + ) + assert len(dept.teams) == 3 + def test_duplicate_team_names_rejected(self) -> None: with pytest.raises(ValidationError, match="Duplicate team names"): Department( @@ -144,6 +172,14 @@ def test_empty_communication_pattern_rejected(self) -> None: with pytest.raises(ValidationError): CompanyConfig(communication_pattern="") + def test_whitespace_communication_pattern_rejected(self) -> None: + with pytest.raises(ValidationError): + CompanyConfig(communication_pattern=" ") + + def test_empty_tool_access_entry_rejected(self) -> None: + with pytest.raises(ValidationError, match="Empty or whitespace-only"): + CompanyConfig(tool_access_default=("git", "")) + def test_frozen(self) -> None: cfg = CompanyConfig() with pytest.raises(ValidationError): @@ -178,6 +214,28 @@ def test_duplicate_active_agents_rejected(self) -> None: with pytest.raises(ValidationError, match="Duplicate entries"): HRRegistry(active_agents=("alice", "alice")) + def test_empty_active_agent_rejected(self) -> None: + with pytest.raises(ValidationError, match="Empty or whitespace-only"): + HRRegistry(active_agents=("",)) + + def test_empty_available_role_rejected(self) -> None: + with pytest.raises(ValidationError, match="Empty or whitespace-only"): + HRRegistry(available_roles=(" ",)) + + def test_empty_hiring_queue_entry_rejected(self) -> None: + with pytest.raises(ValidationError, match="Empty or whitespace-only"): + HRRegistry(hiring_queue=("",)) + + def test_duplicate_available_roles_accepted(self) -> None: + """Duplicates are intentionally allowed in available_roles.""" + hr = HRRegistry(available_roles=("dev", "dev")) + assert hr.available_roles == ("dev", "dev") + + def test_duplicate_hiring_queue_accepted(self) -> None: + """Duplicates are intentionally allowed in hiring_queue.""" + hr = HRRegistry(hiring_queue=("pm", "pm")) + assert hr.hiring_queue == ("pm", "pm") + def test_frozen(self) -> None: hr = HRRegistry() with pytest.raises(ValidationError): @@ -237,6 +295,17 @@ def test_budget_sum_barely_over_100_rejected(self) -> None: with pytest.raises(ValidationError, match="exceeding 100%"): Company(name="Just Over", departments=depts) + def test_budget_sum_float_precision_accepted(self) -> None: + """Classic float artifacts (e.g. 33.33+33.33+33.34) should not cause + false rejections thanks to rounding.""" + depts = ( + Department(name="A", head="a", budget_percent=33.33), + Department(name="B", head="b", budget_percent=33.33), + Department(name="C", head="c", budget_percent=33.34), + ) + co = Company(name="Float Precision", departments=depts) + assert len(co.departments) == 3 + def test_duplicate_department_names_rejected(self) -> None: depts = ( Department(name="Engineering", head="a", budget_percent=30.0), diff --git a/tests/unit/core/test_role.py b/tests/unit/core/test_role.py index 84aa67a58a..84665129fb 100644 --- a/tests/unit/core/test_role.py +++ b/tests/unit/core/test_role.py @@ -77,6 +77,14 @@ def test_empty_reports_to_rejected(self) -> None: with pytest.raises(ValidationError): Authority(reports_to="") + def test_empty_can_approve_entry_rejected(self) -> None: + with pytest.raises(ValidationError, match="Empty or whitespace-only"): + Authority(can_approve=("code_review", "")) + + def test_whitespace_can_delegate_to_rejected(self) -> None: + with pytest.raises(ValidationError, match="Empty or whitespace-only"): + Authority(can_delegate_to=(" ",)) + def test_frozen(self, sample_authority: Authority) -> None: with pytest.raises(ValidationError): sample_authority.budget_limit = 10.0 # type: ignore[misc] @@ -180,6 +188,22 @@ def test_invalid_department_rejected(self) -> None: with pytest.raises(ValidationError): Role(name="Test", department="not_a_department") # type: ignore[arg-type] + def test_empty_required_skill_rejected(self) -> None: + with pytest.raises(ValidationError, match="Empty or whitespace-only"): + Role( + name="Dev", + department=DepartmentName.ENGINEERING, + required_skills=("python", ""), + ) + + def test_whitespace_tool_access_rejected(self) -> None: + with pytest.raises(ValidationError, match="Empty or whitespace-only"): + Role( + name="Dev", + department=DepartmentName.ENGINEERING, + tool_access=(" ",), + ) + def test_frozen(self, sample_role: Role) -> None: with pytest.raises(ValidationError): sample_role.name = "Frontend Developer" # type: ignore[misc] @@ -239,6 +263,22 @@ def test_frozen(self) -> None: with pytest.raises(ValidationError): role.name = "Changed" # type: ignore[misc] + def test_standard_department_as_plain_string(self) -> None: + role = CustomRole(name="Test", department="engineering") + assert role.department == "engineering" + + def test_whitespace_department_normalized(self) -> None: + role = CustomRole(name="Test", department=" blockchain ") + assert role.department == "blockchain" + + def test_empty_required_skill_rejected(self) -> None: + with pytest.raises(ValidationError, match="Empty or whitespace-only"): + CustomRole( + name="Dev", + department="custom", + required_skills=("solidity", ""), + ) + def test_json_roundtrip(self) -> None: role = CustomRole( name="Custom Dev", @@ -248,9 +288,7 @@ def test_json_roundtrip(self) -> None: ) json_str = role.model_dump_json() restored = CustomRole.model_validate_json(json_str) - assert restored.name == role.name - assert restored.department == role.department - assert restored.required_skills == role.required_skills + assert restored == role def test_factory_creates_valid_custom_role(self) -> None: role = CustomRoleFactory.build() diff --git a/tests/unit/core/test_role_catalog.py b/tests/unit/core/test_role_catalog.py index 4bcfb8d63f..d18eebb7b3 100644 --- a/tests/unit/core/test_role_catalog.py +++ b/tests/unit/core/test_role_catalog.py @@ -40,17 +40,14 @@ def test_all_entries_are_seniority_info(self) -> None: def test_junior_is_low_cost(self) -> None: info = get_seniority_info(SeniorityLevel.JUNIOR) - assert info is not None assert info.cost_tier == CostTier.LOW def test_c_suite_is_premium_cost(self) -> None: info = get_seniority_info(SeniorityLevel.C_SUITE) - assert info is not None assert info.cost_tier == CostTier.PREMIUM def test_senior_uses_sonnet_tier(self) -> None: info = get_seniority_info(SeniorityLevel.SENIOR) - assert info is not None assert info.typical_model_tier == "sonnet" def test_all_entries_frozen(self) -> None: @@ -131,6 +128,14 @@ def test_not_found_returns_none(self) -> None: def test_empty_string_returns_none(self) -> None: assert get_builtin_role("") is None + def test_whitespace_stripped(self) -> None: + role = get_builtin_role(" CEO ") + assert role is not None + assert role.name == "CEO" + + def test_whitespace_only_returns_none(self) -> None: + assert get_builtin_role(" ") is None + @pytest.mark.parametrize( "name", [ @@ -177,19 +182,20 @@ def test_all_roles_lookupable(self, name: str) -> None: class TestGetSeniorityInfo: def test_found(self) -> None: info = get_seniority_info(SeniorityLevel.SENIOR) - assert info is not None assert info.level is SeniorityLevel.SENIOR @pytest.mark.parametrize("level", list(SeniorityLevel)) def test_all_levels_lookupable(self, level: SeniorityLevel) -> None: info = get_seniority_info(level) - assert info is not None assert info.level is level - def test_returns_none_for_missing_level(self) -> None: - with patch.dict( - "ai_company.core.role_catalog._SENIORITY_INFO_BY_LEVEL", - {}, - clear=True, + def test_raises_lookup_error_for_missing_level(self) -> None: + with ( + patch.dict( + "ai_company.core.role_catalog._SENIORITY_INFO_BY_LEVEL", + {}, + clear=True, + ), + pytest.raises(LookupError, match="catalog may be incomplete"), ): - assert get_seniority_info(SeniorityLevel.JUNIOR) is None + get_seniority_info(SeniorityLevel.JUNIOR) From 038675849c21105e7c1ea201abfd8eda908cf098 Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Sat, 28 Feb 2026 06:43:52 +0100 Subject: [PATCH 4/6] fix: address 20 PR review items from local agents and external reviewers 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) --- src/ai_company/core/agent.py | 18 +++++--- src/ai_company/core/company.py | 13 ++++-- src/ai_company/core/enums.py | 2 +- src/ai_company/core/role.py | 45 +++++++++++++++++-- src/ai_company/core/role_catalog.py | 5 ++- tests/unit/core/test_agent.py | 11 +++++ tests/unit/core/test_company.py | 24 +++++++++++ tests/unit/core/test_enums.py | 12 ++++++ tests/unit/core/test_role.py | 67 +++++++++++++++++++++++++++++ 9 files changed, 182 insertions(+), 15 deletions(-) diff --git a/src/ai_company/core/agent.py b/src/ai_company/core/agent.py index da79a58ecc..792cbaa27d 100644 --- a/src/ai_company/core/agent.py +++ b/src/ai_company/core/agent.py @@ -52,7 +52,10 @@ class PersonalityConfig(BaseModel): @model_validator(mode="after") def _validate_no_empty_traits(self) -> PersonalityConfig: - """Ensure no empty or whitespace-only trait strings.""" + """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" @@ -193,8 +196,13 @@ def _validate_no_empty_tools(self) -> ToolPermissions: @model_validator(mode="after") def _validate_no_overlap(self) -> ToolPermissions: - """Ensure no tool appears in both allowed and denied lists.""" - overlap = set(self.allowed) & set(self.denied) + """Ensure no tool appears in both allowed and denied lists. + + Comparison is case-insensitive. + """ + allowed_normalized = {t.strip().casefold() for t in self.allowed} + denied_normalized = {t.strip().casefold() for t in self.denied} + overlap = allowed_normalized & denied_normalized if overlap: msg = f"Tools appear in both allowed and denied lists: {sorted(overlap)}" raise ValueError(msg) @@ -206,7 +214,7 @@ class AgentIdentity(BaseModel): Every agent in the company is represented by an ``AgentIdentity`` containing its role, personality, model backend, memory settings, - tool permissions, and authority scope. + tool permissions, and authority configuration. Attributes: id: Unique agent identifier. @@ -219,7 +227,7 @@ class AgentIdentity(BaseModel): model: LLM model configuration. memory: Memory configuration. tools: Tool permissions. - authority: Authority scope. + authority: Authority configuration for this agent. hiring_date: Date the agent was hired. status: Current lifecycle status. """ diff --git a/src/ai_company/core/company.py b/src/ai_company/core/company.py index c5a7870a4e..4f447cd049 100644 --- a/src/ai_company/core/company.py +++ b/src/ai_company/core/company.py @@ -43,6 +43,10 @@ def _validate_strings(self) -> Team: 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 @@ -144,10 +148,13 @@ def _validate_strings(self) -> CompanyConfig: class HRRegistry(BaseModel): """Human resources registry for the company. + ``available_roles`` and ``hiring_queue`` intentionally allow duplicate + entries to represent multiple openings for the same role or position. + Attributes: - active_agents: Currently active agent names. - available_roles: Roles available for hiring. - hiring_queue: Roles in the hiring pipeline. + active_agents: Currently active agent names (must be unique). + available_roles: Roles available for hiring (duplicates allowed). + hiring_queue: Roles in the hiring pipeline (duplicates allowed). """ model_config = ConfigDict(frozen=True) diff --git a/src/ai_company/core/enums.py b/src/ai_company/core/enums.py index 8f346a02f2..39f1ca7c7b 100644 --- a/src/ai_company/core/enums.py +++ b/src/ai_company/core/enums.py @@ -10,7 +10,7 @@ class SeniorityLevel(StrEnum): cost tier defined in ``ai_company.core.role_catalog.SENIORITY_INFO``. """ - # Design spec says "Intern/Junior" — collapsed to a single JUNIOR level. + # Design spec §3.2 says "Intern/Junior" — collapsed to a single JUNIOR level. JUNIOR = "junior" MID = "mid" SENIOR = "senior" diff --git a/src/ai_company/core/role.py b/src/ai_company/core/role.py index 46301b9ae0..868e9f9f88 100644 --- a/src/ai_company/core/role.py +++ b/src/ai_company/core/role.py @@ -28,6 +28,14 @@ class Skill(BaseModel): description="Proficiency level", ) + @model_validator(mode="after") + def _validate_name_not_blank(self) -> Skill: + """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 + class Authority(BaseModel): """Authority scope for an agent or role. @@ -62,12 +70,15 @@ class Authority(BaseModel): @model_validator(mode="after") def _validate_no_empty_strings(self) -> Authority: - """Ensure no empty or whitespace-only entries in string tuples.""" + """Ensure no whitespace-only entries in string tuples or reports_to.""" for field_name in ("can_approve", "can_delegate_to"): for value in getattr(self, field_name): if not value.strip(): msg = f"Empty or whitespace-only entry in {field_name}" raise ValueError(msg) + if self.reports_to is not None and not self.reports_to.strip(): + msg = "reports_to must not be whitespace-only" + raise ValueError(msg) return self @@ -97,6 +108,15 @@ class SeniorityInfo(BaseModel): description="Cost tier identifier (built-in or user-defined)", ) + @model_validator(mode="after") + def _validate_non_blank_strings(self) -> SeniorityInfo: + """Ensure string fields are not whitespace-only.""" + for field_name in ("authority_scope", "typical_model_tier", "cost_tier"): + if not getattr(self, field_name).strip(): + msg = f"{field_name} must not be whitespace-only" + raise ValueError(msg) + return self + class Role(BaseModel): """A job definition within the organization. @@ -140,7 +160,16 @@ class Role(BaseModel): @model_validator(mode="after") def _validate_no_empty_strings(self) -> Role: - """Ensure no empty or whitespace-only entries in string tuples.""" + """Ensure no empty or whitespace-only entries in name and string tuples.""" + if not self.name.strip(): + msg = "name must not be whitespace-only" + raise ValueError(msg) + if ( + self.system_prompt_template is not None + and not self.system_prompt_template.strip() + ): + msg = "system_prompt_template must not be whitespace-only" + raise ValueError(msg) for field_name in ("required_skills", "tool_access"): for value in getattr(self, field_name): if not value.strip(): @@ -191,7 +220,7 @@ class CustomRole(BaseModel): @field_validator("department") @classmethod def _department_not_empty(cls, v: DepartmentName | str) -> DepartmentName | str: - """Ensure department is not empty and normalize whitespace.""" + """Ensure department is not empty and strip surrounding whitespace.""" if isinstance(v, DepartmentName): return v stripped = v.strip() @@ -202,7 +231,15 @@ def _department_not_empty(cls, v: DepartmentName | str) -> DepartmentName | str: @model_validator(mode="after") def _validate_no_empty_required_skills(self) -> CustomRole: - """Ensure no empty or whitespace-only entries in required_skills.""" + """Ensure no whitespace-only name, optional fields, or skills.""" + if not self.name.strip(): + msg = "name must not be whitespace-only" + raise ValueError(msg) + for field_name in ("system_prompt_template", "suggested_model"): + value = getattr(self, field_name) + if value is not None and not value.strip(): + msg = f"{field_name} must not be whitespace-only" + raise ValueError(msg) for value in self.required_skills: if not value.strip(): msg = "Empty or whitespace-only entry in required_skills" diff --git a/src/ai_company/core/role_catalog.py b/src/ai_company/core/role_catalog.py index 25a449f08c..ed7e40b61e 100644 --- a/src/ai_company/core/role_catalog.py +++ b/src/ai_company/core/role_catalog.py @@ -1,7 +1,8 @@ """Built-in role catalog and seniority information. -Provides the canonical set of built-in roles from DESIGN_SPEC.md section 3.3 -and the seniority mapping from section 3.2. +Provides the canonical set of built-in roles from DESIGN_SPEC.md +section 3.3 (Role Catalog) and the seniority mapping from +section 3.2 (Seniority & Authority Levels). """ from ai_company.core.enums import ( diff --git a/tests/unit/core/test_agent.py b/tests/unit/core/test_agent.py index 06f60370c1..1aecb1b6fc 100644 --- a/tests/unit/core/test_agent.py +++ b/tests/unit/core/test_agent.py @@ -60,6 +60,10 @@ def test_empty_communication_style_rejected(self) -> None: with pytest.raises(ValidationError): PersonalityConfig(communication_style="") + def test_whitespace_communication_style_rejected(self) -> None: + with pytest.raises(ValidationError, match="whitespace-only"): + PersonalityConfig(communication_style=" ") + def test_empty_trait_rejected(self) -> None: with pytest.raises(ValidationError, match="Empty or whitespace-only"): PersonalityConfig(traits=("analytical", "")) @@ -273,6 +277,13 @@ def test_multiple_overlapping_tools_all_reported(self) -> None: assert "deploy" in error_text assert "git" in error_text + def test_case_insensitive_overlap_rejected(self) -> None: + with pytest.raises(ValidationError, match="both allowed and denied"): + ToolPermissions( + allowed=("Git",), + denied=("git",), + ) + def test_empty_tool_name_rejected(self) -> None: with pytest.raises(ValidationError, match="Empty or whitespace-only"): ToolPermissions(allowed=("git", "")) diff --git a/tests/unit/core/test_company.py b/tests/unit/core/test_company.py index 637635841d..26b84a894d 100644 --- a/tests/unit/core/test_company.py +++ b/tests/unit/core/test_company.py @@ -63,6 +63,14 @@ def test_whitespace_member_name_rejected(self) -> None: with pytest.raises(ValidationError, match="Empty or whitespace-only"): Team(name="test", lead="lead", members=(" ",)) + def test_duplicate_members_rejected(self) -> None: + with pytest.raises(ValidationError, match="Duplicate members"): + Team( + name="backend", + lead="lead", + members=("alice", "bob", "alice"), + ) + def test_frozen(self) -> None: team = Team(name="test", lead="lead") with pytest.raises(ValidationError): @@ -117,6 +125,14 @@ def test_multiple_distinct_teams_accepted(self) -> None: ) assert len(dept.teams) == 3 + def test_whitespace_name_rejected(self) -> None: + with pytest.raises(ValidationError, match="whitespace-only"): + Department(name=" ", head="head") + + def test_whitespace_head_rejected(self) -> None: + with pytest.raises(ValidationError, match="whitespace-only"): + Department(name="Eng", head=" ") + def test_duplicate_team_names_rejected(self) -> None: with pytest.raises(ValidationError, match="Duplicate team names"): Department( @@ -314,6 +330,14 @@ def test_duplicate_department_names_rejected(self) -> None: with pytest.raises(ValidationError, match="Duplicate department names"): Company(name="Dup Depts", departments=depts) + def test_empty_name_rejected(self) -> None: + with pytest.raises(ValidationError): + Company(name="") + + def test_whitespace_name_rejected(self) -> None: + with pytest.raises(ValidationError, match="whitespace-only"): + Company(name=" ") + def test_empty_departments_accepted(self) -> None: co = Company(name="Empty") assert co.departments == () diff --git a/tests/unit/core/test_enums.py b/tests/unit/core/test_enums.py index d2d92bebdb..d507b21ba0 100644 --- a/tests/unit/core/test_enums.py +++ b/tests/unit/core/test_enums.py @@ -149,3 +149,15 @@ class _M(BaseModel): restored = _M.model_validate_json(json_str) assert restored.status is AgentStatus.ACTIVE assert restored.tier is CostTier.PREMIUM + + +# ── __all__ exports ────────────────────────────────────────────── + + +@pytest.mark.unit +class TestCoreExports: + def test_all_exports_importable(self) -> None: + import ai_company.core as core_module + + for name in core_module.__all__: + assert hasattr(core_module, name), f"{name} in __all__ but not importable" diff --git a/tests/unit/core/test_role.py b/tests/unit/core/test_role.py index 84665129fb..0e8005a43b 100644 --- a/tests/unit/core/test_role.py +++ b/tests/unit/core/test_role.py @@ -37,6 +37,10 @@ def test_empty_name_rejected(self) -> None: with pytest.raises(ValidationError): Skill(name="", category=SkillCategory.ENGINEERING) + def test_whitespace_name_rejected(self) -> None: + with pytest.raises(ValidationError, match="whitespace-only"): + Skill(name=" ", category=SkillCategory.ENGINEERING) + def test_frozen(self, sample_skill: Skill) -> None: with pytest.raises(ValidationError): sample_skill.name = "rust" # type: ignore[misc] @@ -85,6 +89,10 @@ def test_whitespace_can_delegate_to_rejected(self) -> None: with pytest.raises(ValidationError, match="Empty or whitespace-only"): Authority(can_delegate_to=(" ",)) + def test_whitespace_reports_to_rejected(self) -> None: + with pytest.raises(ValidationError, match="whitespace-only"): + Authority(reports_to=" ") + def test_frozen(self, sample_authority: Authority) -> None: with pytest.raises(ValidationError): sample_authority.budget_limit = 10.0 # type: ignore[misc] @@ -147,6 +155,33 @@ def test_empty_cost_tier_rejected(self) -> None: cost_tier="", ) + def test_whitespace_authority_scope_rejected(self) -> None: + with pytest.raises(ValidationError, match="whitespace-only"): + SeniorityInfo( + level=SeniorityLevel.JUNIOR, + authority_scope=" ", + typical_model_tier="haiku", + cost_tier="low", + ) + + def test_whitespace_model_tier_rejected(self) -> None: + with pytest.raises(ValidationError, match="whitespace-only"): + SeniorityInfo( + level=SeniorityLevel.JUNIOR, + authority_scope="tasks", + typical_model_tier=" ", + cost_tier="low", + ) + + def test_whitespace_cost_tier_rejected(self) -> None: + with pytest.raises(ValidationError, match="whitespace-only"): + SeniorityInfo( + level=SeniorityLevel.JUNIOR, + authority_scope="tasks", + typical_model_tier="haiku", + cost_tier=" ", + ) + def test_frozen(self) -> None: info = SeniorityInfo( level=SeniorityLevel.MID, @@ -184,6 +219,18 @@ def test_empty_name_rejected(self) -> None: with pytest.raises(ValidationError): Role(name="", department=DepartmentName.ENGINEERING) + def test_whitespace_name_rejected(self) -> None: + with pytest.raises(ValidationError, match="whitespace-only"): + Role(name=" ", department=DepartmentName.ENGINEERING) + + def test_whitespace_system_prompt_template_rejected(self) -> None: + with pytest.raises(ValidationError, match="whitespace-only"): + Role( + name="Dev", + department=DepartmentName.ENGINEERING, + system_prompt_template=" ", + ) + def test_invalid_department_rejected(self) -> None: with pytest.raises(ValidationError): Role(name="Test", department="not_a_department") # type: ignore[arg-type] @@ -250,6 +297,26 @@ def test_empty_name_rejected(self) -> None: with pytest.raises(ValidationError): CustomRole(name="", department="custom") + def test_whitespace_name_rejected(self) -> None: + with pytest.raises(ValidationError, match="whitespace-only"): + CustomRole(name=" ", department="custom") + + def test_whitespace_system_prompt_template_rejected(self) -> None: + with pytest.raises(ValidationError, match="whitespace-only"): + CustomRole( + name="Dev", + department="custom", + system_prompt_template=" ", + ) + + def test_whitespace_suggested_model_rejected(self) -> None: + with pytest.raises(ValidationError, match="whitespace-only"): + CustomRole( + name="Dev", + department="custom", + suggested_model=" ", + ) + def test_empty_department_rejected(self) -> None: with pytest.raises(ValidationError, match="Department name must not be empty"): CustomRole(name="Test", department="") From a6b1b83147318f434b3e139cdd4c25b3715b4ad3 Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Sat, 28 Feb 2026 07:14:22 +0100 Subject: [PATCH 5/6] fix: use Self return type in model validators and clean up test imports - 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 --- src/ai_company/core/agent.py | 15 ++++++++------- src/ai_company/core/company.py | 15 ++++++++------- src/ai_company/core/role.py | 12 +++++++----- tests/unit/core/test_role_catalog.py | 5 +---- 4 files changed, 24 insertions(+), 23 deletions(-) diff --git a/src/ai_company/core/agent.py b/src/ai_company/core/agent.py index 792cbaa27d..1e27697179 100644 --- a/src/ai_company/core/agent.py +++ b/src/ai_company/core/agent.py @@ -1,6 +1,7 @@ """Agent identity and configuration models.""" from datetime import date # noqa: TC003 — required at runtime by Pydantic +from typing import Self from uuid import UUID, uuid4 from pydantic import BaseModel, ConfigDict, Field, model_validator @@ -51,7 +52,7 @@ class PersonalityConfig(BaseModel): ) @model_validator(mode="after") - def _validate_no_empty_traits(self) -> PersonalityConfig: + 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" @@ -83,7 +84,7 @@ class SkillSet(BaseModel): ) @model_validator(mode="after") - def _validate_no_empty_skills(self) -> SkillSet: + def _validate_no_empty_skills(self) -> Self: """Ensure no empty or whitespace-only skill names.""" for field_name in ("primary", "secondary"): for skill in getattr(self, field_name): @@ -126,7 +127,7 @@ class ModelConfig(BaseModel): ) @model_validator(mode="after") - def _validate_non_blank_identifiers(self) -> ModelConfig: + def _validate_non_blank_identifiers(self) -> Self: """Ensure identifier fields are not whitespace-only.""" for field_name in ("provider", "model_id", "fallback_model"): value = getattr(self, field_name) @@ -157,7 +158,7 @@ class MemoryConfig(BaseModel): ) @model_validator(mode="after") - def _validate_retention_consistency(self) -> MemoryConfig: + def _validate_retention_consistency(self) -> Self: """Ensure retention_days is None when memory type is MemoryType.NONE.""" if self.type is MemoryType.NONE and self.retention_days is not None: msg = "retention_days must be None when memory type is 'none'" @@ -185,7 +186,7 @@ class ToolPermissions(BaseModel): ) @model_validator(mode="after") - def _validate_no_empty_tools(self) -> ToolPermissions: + def _validate_no_empty_tools(self) -> Self: """Ensure no empty or whitespace-only tool names.""" for field_name in ("allowed", "denied"): for tool in getattr(self, field_name): @@ -195,7 +196,7 @@ def _validate_no_empty_tools(self) -> ToolPermissions: return self @model_validator(mode="after") - def _validate_no_overlap(self) -> ToolPermissions: + def _validate_no_overlap(self) -> Self: """Ensure no tool appears in both allowed and denied lists. Comparison is case-insensitive. @@ -270,7 +271,7 @@ class AgentIdentity(BaseModel): ) @model_validator(mode="after") - def _validate_non_blank_identifiers(self) -> AgentIdentity: + def _validate_non_blank_identifiers(self) -> Self: """Ensure name, role, and department are not whitespace-only.""" for field_name in ("name", "role", "department"): if not getattr(self, field_name).strip(): diff --git a/src/ai_company/core/company.py b/src/ai_company/core/company.py index 4f447cd049..0a556b87e6 100644 --- a/src/ai_company/core/company.py +++ b/src/ai_company/core/company.py @@ -1,6 +1,7 @@ """Company structure and configuration models.""" from collections import Counter +from typing import Self from uuid import UUID, uuid4 from pydantic import BaseModel, ConfigDict, Field, model_validator @@ -33,7 +34,7 @@ class Team(BaseModel): ) @model_validator(mode="after") - def _validate_strings(self) -> Team: + 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(): @@ -80,7 +81,7 @@ class Department(BaseModel): ) @model_validator(mode="after") - def _validate_non_blank_identifiers(self) -> Department: + def _validate_non_blank_identifiers(self) -> Self: """Ensure name and head are not whitespace-only.""" for field_name in ("name", "head"): if not getattr(self, field_name).strip(): @@ -89,7 +90,7 @@ def _validate_non_blank_identifiers(self) -> Department: return self @model_validator(mode="after") - def _validate_unique_team_names(self) -> Department: + def _validate_unique_team_names(self) -> Self: """Ensure no duplicate team names within a department.""" names = [t.name for t in self.teams] if len(names) != len(set(names)): @@ -133,7 +134,7 @@ class CompanyConfig(BaseModel): ) @model_validator(mode="after") - def _validate_strings(self) -> CompanyConfig: + def _validate_strings(self) -> Self: """Ensure no whitespace-only identifiers or tool access entries.""" if not self.communication_pattern.strip(): msg = "communication_pattern must not be whitespace-only" @@ -173,7 +174,7 @@ class HRRegistry(BaseModel): ) @model_validator(mode="after") - def _validate_entries(self) -> HRRegistry: + def _validate_entries(self) -> Self: """Ensure no empty strings and no duplicate entries in active_agents.""" for field_name in ("active_agents", "available_roles", "hiring_queue"): for value in getattr(self, field_name): @@ -226,7 +227,7 @@ class Company(BaseModel): ) @model_validator(mode="after") - def _validate_non_blank_name(self) -> Company: + def _validate_non_blank_name(self) -> Self: """Ensure company name is not whitespace-only.""" if not self.name.strip(): msg = "name must not be whitespace-only" @@ -234,7 +235,7 @@ def _validate_non_blank_name(self) -> Company: return self @model_validator(mode="after") - def _validate_departments(self) -> Company: + def _validate_departments(self) -> Self: """Validate department names are unique and budgets do not exceed 100%.""" # Unique department names names = [d.name for d in self.departments] diff --git a/src/ai_company/core/role.py b/src/ai_company/core/role.py index 868e9f9f88..a6f7bd84df 100644 --- a/src/ai_company/core/role.py +++ b/src/ai_company/core/role.py @@ -1,5 +1,7 @@ """Role and skill domain models.""" +from typing import Self + from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator from ai_company.core.enums import ( @@ -29,7 +31,7 @@ class Skill(BaseModel): ) @model_validator(mode="after") - def _validate_name_not_blank(self) -> Skill: + 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" @@ -69,7 +71,7 @@ class Authority(BaseModel): ) @model_validator(mode="after") - def _validate_no_empty_strings(self) -> Authority: + def _validate_no_empty_strings(self) -> Self: """Ensure no whitespace-only entries in string tuples or reports_to.""" for field_name in ("can_approve", "can_delegate_to"): for value in getattr(self, field_name): @@ -109,7 +111,7 @@ class SeniorityInfo(BaseModel): ) @model_validator(mode="after") - def _validate_non_blank_strings(self) -> SeniorityInfo: + def _validate_non_blank_strings(self) -> Self: """Ensure string fields are not whitespace-only.""" for field_name in ("authority_scope", "typical_model_tier", "cost_tier"): if not getattr(self, field_name).strip(): @@ -159,7 +161,7 @@ class Role(BaseModel): ) @model_validator(mode="after") - def _validate_no_empty_strings(self) -> Role: + def _validate_no_empty_strings(self) -> Self: """Ensure no empty or whitespace-only entries in name and string tuples.""" if not self.name.strip(): msg = "name must not be whitespace-only" @@ -230,7 +232,7 @@ def _department_not_empty(cls, v: DepartmentName | str) -> DepartmentName | str: return stripped @model_validator(mode="after") - def _validate_no_empty_required_skills(self) -> CustomRole: + def _validate_no_empty_required_skills(self) -> Self: """Ensure no whitespace-only name, optional fields, or skills.""" if not self.name.strip(): msg = "name must not be whitespace-only" diff --git a/tests/unit/core/test_role_catalog.py b/tests/unit/core/test_role_catalog.py index d18eebb7b3..8b44a98834 100644 --- a/tests/unit/core/test_role_catalog.py +++ b/tests/unit/core/test_role_catalog.py @@ -3,6 +3,7 @@ from unittest.mock import patch import pytest +from pydantic import ValidationError from ai_company.core.enums import ( CostTier, @@ -51,8 +52,6 @@ def test_senior_uses_sonnet_tier(self) -> None: assert info.typical_model_tier == "sonnet" def test_all_entries_frozen(self) -> None: - from pydantic import ValidationError - for info in SENIORITY_INFO: with pytest.raises(ValidationError): info.level = SeniorityLevel.JUNIOR # type: ignore[misc] @@ -95,8 +94,6 @@ def test_all_roles_have_required_skills(self) -> None: assert len(role.required_skills) > 0, f"{role.name} has no required_skills" def test_all_roles_frozen(self) -> None: - from pydantic import ValidationError - for role in BUILTIN_ROLES: with pytest.raises(ValidationError): role.name = "Changed" # type: ignore[misc] From 9eeacaf598a2144feb0ae8ca682e9dd81c493a12 Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Sat, 28 Feb 2026 07:29:50 +0100 Subject: [PATCH 6/6] fix: address post-push review feedback from CodeRabbit, Gemini, and Copilot - 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 --- src/ai_company/core/role_catalog.py | 8 +++ tests/unit/core/test_agent.py | 79 ++++++++++++++++++++++++++++ tests/unit/core/test_company.py | 67 +++++++++++++++++++++++ tests/unit/core/test_role.py | 2 + tests/unit/core/test_role_catalog.py | 37 +++++++++++++ 5 files changed, 193 insertions(+) diff --git a/src/ai_company/core/role_catalog.py b/src/ai_company/core/role_catalog.py index ed7e40b61e..2ee1529fbe 100644 --- a/src/ai_company/core/role_catalog.py +++ b/src/ai_company/core/role_catalog.py @@ -397,6 +397,14 @@ _SENIORITY_INFO_BY_LEVEL: dict[SeniorityLevel, SeniorityInfo] = { info.level: info for info in SENIORITY_INFO } +if len(_SENIORITY_INFO_BY_LEVEL) != len(SENIORITY_INFO): + _msg = "Duplicate seniority levels found in SENIORITY_INFO" + raise ValueError(_msg) + +_missing_levels = set(SeniorityLevel) - set(_SENIORITY_INFO_BY_LEVEL) +if _missing_levels: + _msg = f"Missing seniority mappings: {sorted(lv.value for lv in _missing_levels)}" + raise ValueError(_msg) def get_builtin_role(name: str) -> Role | None: diff --git a/tests/unit/core/test_agent.py b/tests/unit/core/test_agent.py index 1aecb1b6fc..07badbdc46 100644 --- a/tests/unit/core/test_agent.py +++ b/tests/unit/core/test_agent.py @@ -32,12 +32,17 @@ ToolPermissionsFactory, ) +pytestmark = pytest.mark.timeout(30) + # ── PersonalityConfig ────────────────────────────────────────────── @pytest.mark.unit class TestPersonalityConfig: + """Tests for PersonalityConfig defaults, validation, and immutability.""" + def test_defaults(self) -> None: + """Verify default field values for a bare PersonalityConfig.""" p = PersonalityConfig() assert p.traits == () assert p.communication_style == "neutral" @@ -46,6 +51,7 @@ def test_defaults(self) -> None: assert p.description == "" def test_custom_values(self) -> None: + """Verify explicitly provided values are persisted.""" p = PersonalityConfig( traits=("analytical", "pragmatic"), communication_style="concise and technical", @@ -57,27 +63,33 @@ def test_custom_values(self) -> None: assert p.communication_style == "concise and technical" def test_empty_communication_style_rejected(self) -> None: + """Reject empty string for communication_style.""" with pytest.raises(ValidationError): PersonalityConfig(communication_style="") def test_whitespace_communication_style_rejected(self) -> None: + """Reject whitespace-only communication_style.""" with pytest.raises(ValidationError, match="whitespace-only"): PersonalityConfig(communication_style=" ") def test_empty_trait_rejected(self) -> None: + """Reject empty string in traits tuple.""" with pytest.raises(ValidationError, match="Empty or whitespace-only"): PersonalityConfig(traits=("analytical", "")) def test_whitespace_trait_rejected(self) -> None: + """Reject whitespace-only trait entry.""" with pytest.raises(ValidationError, match="Empty or whitespace-only"): PersonalityConfig(traits=(" ",)) def test_frozen(self) -> None: + """Ensure PersonalityConfig is immutable.""" p = PersonalityConfig() with pytest.raises(ValidationError): p.creativity = CreativityLevel.LOW # type: ignore[misc] def test_factory(self) -> None: + """Verify factory produces a valid PersonalityConfig.""" p = PersonalityConfigFactory.build() assert isinstance(p, PersonalityConfig) @@ -87,12 +99,16 @@ def test_factory(self) -> None: @pytest.mark.unit class TestSkillSet: + """Tests for SkillSet defaults, validation, and immutability.""" + def test_defaults(self) -> None: + """Verify default empty tuples for primary and secondary.""" s = SkillSet() assert s.primary == () assert s.secondary == () def test_custom_values(self) -> None: + """Verify explicitly provided skill tuples are persisted.""" s = SkillSet( primary=("python", "fastapi"), secondary=("docker", "redis"), @@ -101,27 +117,33 @@ def test_custom_values(self) -> None: assert "docker" in s.secondary def test_empty_skill_name_rejected(self) -> None: + """Reject empty string in primary skills.""" with pytest.raises(ValidationError, match="Empty or whitespace-only"): SkillSet(primary=("python", "")) def test_whitespace_skill_name_rejected(self) -> None: + """Reject whitespace-only skill name in secondary.""" with pytest.raises(ValidationError, match="Empty or whitespace-only"): SkillSet(secondary=(" ",)) def test_empty_primary_error_mentions_primary(self) -> None: + """Ensure error message references 'primary' for that field.""" with pytest.raises(ValidationError, match="primary"): SkillSet(primary=("python", "")) def test_empty_secondary_error_mentions_secondary(self) -> None: + """Ensure error message references 'secondary' for that field.""" with pytest.raises(ValidationError, match="secondary"): SkillSet(secondary=(" ",)) def test_frozen(self) -> None: + """Ensure SkillSet is immutable.""" s = SkillSet() with pytest.raises(ValidationError): s.primary = ("new",) # type: ignore[misc] def test_factory(self) -> None: + """Verify factory produces a valid SkillSet.""" s = SkillSetFactory.build() assert isinstance(s, SkillSet) @@ -131,71 +153,89 @@ def test_factory(self) -> None: @pytest.mark.unit class TestModelConfig: + """Tests for ModelConfig validation, boundaries, and immutability.""" + def test_valid_config(self, sample_model_config: ModelConfig) -> None: + """Verify fixture-provided ModelConfig fields are correct.""" assert sample_model_config.provider == "test-provider" assert sample_model_config.model_id == "test-model-sonnet-4-6" assert sample_model_config.temperature == 0.3 assert sample_model_config.max_tokens == 8192 def test_defaults(self) -> None: + """Verify default temperature, max_tokens, and fallback_model.""" m = ModelConfig(provider="test", model_id="test-model") assert m.temperature == 0.7 assert m.max_tokens == 4096 assert m.fallback_model is None def test_empty_provider_rejected(self) -> None: + """Reject empty provider string.""" with pytest.raises(ValidationError): ModelConfig(provider="", model_id="test") def test_empty_model_id_rejected(self) -> None: + """Reject empty model_id string.""" with pytest.raises(ValidationError): ModelConfig(provider="test", model_id="") def test_empty_fallback_model_rejected(self) -> None: + """Reject empty fallback_model string.""" with pytest.raises(ValidationError): ModelConfig(provider="test", model_id="m", fallback_model="") def test_temperature_below_zero_rejected(self) -> None: + """Reject temperature below 0.0.""" with pytest.raises(ValidationError): ModelConfig(provider="test", model_id="m", temperature=-0.1) def test_temperature_above_two_rejected(self) -> None: + """Reject temperature above 2.0.""" with pytest.raises(ValidationError): ModelConfig(provider="test", model_id="m", temperature=2.1) def test_temperature_boundary_zero(self) -> None: + """Accept temperature at lower boundary (0.0).""" m = ModelConfig(provider="test", model_id="m", temperature=0.0) assert m.temperature == 0.0 def test_temperature_boundary_two(self) -> None: + """Accept temperature at upper boundary (2.0).""" m = ModelConfig(provider="test", model_id="m", temperature=2.0) assert m.temperature == 2.0 def test_max_tokens_zero_rejected(self) -> None: + """Reject max_tokens of zero.""" with pytest.raises(ValidationError): ModelConfig(provider="test", model_id="m", max_tokens=0) def test_max_tokens_negative_rejected(self) -> None: + """Reject negative max_tokens.""" with pytest.raises(ValidationError): ModelConfig(provider="test", model_id="m", max_tokens=-1) def test_whitespace_provider_rejected(self) -> None: + """Reject whitespace-only provider string.""" with pytest.raises(ValidationError, match="whitespace-only"): ModelConfig(provider=" ", model_id="test") def test_whitespace_model_id_rejected(self) -> None: + """Reject whitespace-only model_id string.""" with pytest.raises(ValidationError, match="whitespace-only"): ModelConfig(provider="test", model_id=" ") def test_whitespace_fallback_model_rejected(self) -> None: + """Reject whitespace-only fallback_model string.""" with pytest.raises(ValidationError, match="whitespace-only"): ModelConfig(provider="test", model_id="m", fallback_model=" ") def test_frozen(self, sample_model_config: ModelConfig) -> None: + """Ensure ModelConfig is immutable.""" with pytest.raises(ValidationError): sample_model_config.temperature = 1.0 # type: ignore[misc] def test_factory(self) -> None: + """Verify factory produces a valid ModelConfig with sane bounds.""" m = ModelConfigFactory.build() assert isinstance(m, ModelConfig) assert 0.0 <= m.temperature <= 2.0 @@ -206,38 +246,48 @@ def test_factory(self) -> None: @pytest.mark.unit class TestMemoryConfig: + """Tests for MemoryConfig defaults, type constraints, and immutability.""" + def test_defaults(self) -> None: + """Verify default type is SESSION with no retention.""" m = MemoryConfig() assert m.type is MemoryType.SESSION assert m.retention_days is None def test_custom_values(self) -> None: + """Verify explicitly provided type and retention_days.""" m = MemoryConfig(type=MemoryType.PERSISTENT, retention_days=30) assert m.type is MemoryType.PERSISTENT assert m.retention_days == 30 def test_retention_days_zero_rejected(self) -> None: + """Reject retention_days of zero.""" with pytest.raises(ValidationError): MemoryConfig(retention_days=0) def test_retention_days_negative_rejected(self) -> None: + """Reject negative retention_days.""" with pytest.raises(ValidationError): MemoryConfig(retention_days=-1) def test_none_type_with_retention_rejected(self) -> None: + """Reject retention_days when memory type is NONE.""" with pytest.raises(ValidationError, match="retention_days must be None"): MemoryConfig(type=MemoryType.NONE, retention_days=30) def test_none_type_without_retention_accepted(self) -> None: + """Accept NONE memory type when retention_days is omitted.""" m = MemoryConfig(type=MemoryType.NONE) assert m.retention_days is None def test_frozen(self) -> None: + """Ensure MemoryConfig is immutable.""" m = MemoryConfig() with pytest.raises(ValidationError): m.type = MemoryType.PERSISTENT # type: ignore[misc] def test_factory(self) -> None: + """Verify factory produces a valid MemoryConfig.""" m = MemoryConfigFactory.build() assert isinstance(m, MemoryConfig) @@ -247,12 +297,16 @@ def test_factory(self) -> None: @pytest.mark.unit class TestToolPermissions: + """Tests for ToolPermissions overlap detection, validation, and immutability.""" + def test_defaults(self) -> None: + """Verify default empty allowed and denied tuples.""" t = ToolPermissions() assert t.allowed == () assert t.denied == () def test_custom_values(self) -> None: + """Verify non-overlapping allowed and denied are accepted.""" t = ToolPermissions( allowed=("file_system", "git"), denied=("deployment",), @@ -261,6 +315,7 @@ def test_custom_values(self) -> None: assert "deployment" in t.denied def test_overlap_rejected(self) -> None: + """Reject tools appearing in both allowed and denied.""" with pytest.raises(ValidationError, match="both allowed and denied"): ToolPermissions( allowed=("git", "file_system"), @@ -268,6 +323,7 @@ def test_overlap_rejected(self) -> None: ) def test_multiple_overlapping_tools_all_reported(self) -> None: + """Ensure all overlapping tool names appear in the error.""" with pytest.raises(ValidationError) as exc_info: ToolPermissions( allowed=("git", "deploy", "shell"), @@ -278,6 +334,7 @@ def test_multiple_overlapping_tools_all_reported(self) -> None: assert "git" in error_text def test_case_insensitive_overlap_rejected(self) -> None: + """Reject case-insensitive overlap between allowed and denied.""" with pytest.raises(ValidationError, match="both allowed and denied"): ToolPermissions( allowed=("Git",), @@ -285,19 +342,23 @@ def test_case_insensitive_overlap_rejected(self) -> None: ) def test_empty_tool_name_rejected(self) -> None: + """Reject empty string in allowed tools.""" with pytest.raises(ValidationError, match="Empty or whitespace-only"): ToolPermissions(allowed=("git", "")) def test_whitespace_tool_name_rejected(self) -> None: + """Reject whitespace-only tool name in denied.""" with pytest.raises(ValidationError, match="Empty or whitespace-only"): ToolPermissions(denied=(" ",)) def test_frozen(self) -> None: + """Ensure ToolPermissions is immutable.""" t = ToolPermissions() with pytest.raises(ValidationError): t.allowed = ("new",) # type: ignore[misc] def test_factory(self) -> None: + """Verify factory produces a valid ToolPermissions.""" t = ToolPermissionsFactory.build() assert isinstance(t, ToolPermissions) @@ -307,7 +368,10 @@ def test_factory(self) -> None: @pytest.mark.unit class TestAgentIdentity: + """Tests for AgentIdentity construction, validation, and serialization.""" + def test_valid_agent(self, sample_agent: AgentIdentity) -> None: + """Verify fixture-provided agent has expected field values.""" assert sample_agent.name == "Sarah Chen" assert sample_agent.role == "Senior Backend Developer" assert sample_agent.department == "Engineering" @@ -315,6 +379,7 @@ def test_valid_agent(self, sample_agent: AgentIdentity) -> None: assert isinstance(sample_agent.id, UUID) def test_auto_generated_id(self, sample_model_config: ModelConfig) -> None: + """Verify UUID is auto-generated when not provided.""" agent = AgentIdentity( name="Test Agent", role="Developer", @@ -325,6 +390,7 @@ def test_auto_generated_id(self, sample_model_config: ModelConfig) -> None: assert isinstance(agent.id, UUID) def test_defaults(self, sample_model_config: ModelConfig) -> None: + """Verify default level, status, and nested config objects.""" agent = AgentIdentity( name="Test", role="Dev", @@ -341,6 +407,7 @@ def test_defaults(self, sample_model_config: ModelConfig) -> None: assert isinstance(agent.authority, Authority) def test_model_is_required(self) -> None: + """Reject construction without the required model field.""" with pytest.raises(ValidationError): AgentIdentity( name="Test", @@ -350,6 +417,7 @@ def test_model_is_required(self) -> None: ) # type: ignore[call-arg] def test_hiring_date_is_required(self, sample_model_config: ModelConfig) -> None: + """Reject construction without the required hiring_date field.""" with pytest.raises(ValidationError): AgentIdentity( name="Test", @@ -359,6 +427,7 @@ def test_hiring_date_is_required(self, sample_model_config: ModelConfig) -> None ) # type: ignore[call-arg] def test_empty_name_rejected(self, sample_model_config: ModelConfig) -> None: + """Reject empty name string.""" with pytest.raises(ValidationError): AgentIdentity( name="", @@ -369,6 +438,7 @@ def test_empty_name_rejected(self, sample_model_config: ModelConfig) -> None: ) def test_empty_role_rejected(self, sample_model_config: ModelConfig) -> None: + """Reject empty role string.""" with pytest.raises(ValidationError): AgentIdentity( name="Test", @@ -379,6 +449,7 @@ def test_empty_role_rejected(self, sample_model_config: ModelConfig) -> None: ) def test_empty_department_rejected(self, sample_model_config: ModelConfig) -> None: + """Reject empty department string.""" with pytest.raises(ValidationError): AgentIdentity( name="Test", @@ -389,6 +460,7 @@ def test_empty_department_rejected(self, sample_model_config: ModelConfig) -> No ) def test_whitespace_name_rejected(self, sample_model_config: ModelConfig) -> None: + """Reject whitespace-only name string.""" with pytest.raises(ValidationError, match="whitespace-only"): AgentIdentity( name=" ", @@ -399,6 +471,7 @@ def test_whitespace_name_rejected(self, sample_model_config: ModelConfig) -> Non ) def test_whitespace_role_rejected(self, sample_model_config: ModelConfig) -> None: + """Reject whitespace-only role string.""" with pytest.raises(ValidationError, match="whitespace-only"): AgentIdentity( name="Test", @@ -411,6 +484,7 @@ def test_whitespace_role_rejected(self, sample_model_config: ModelConfig) -> Non def test_whitespace_department_rejected( self, sample_model_config: ModelConfig ) -> None: + """Reject whitespace-only department string.""" with pytest.raises(ValidationError, match="whitespace-only"): AgentIdentity( name="Test", @@ -421,10 +495,12 @@ def test_whitespace_department_rejected( ) def test_frozen(self, sample_agent: AgentIdentity) -> None: + """Ensure AgentIdentity is immutable.""" with pytest.raises(ValidationError): sample_agent.name = "Changed" # type: ignore[misc] def test_model_copy_update(self, sample_agent: AgentIdentity) -> None: + """Verify model_copy creates a new instance without mutating the original.""" updated = sample_agent.model_copy( update={"status": AgentStatus.TERMINATED}, ) @@ -432,6 +508,7 @@ def test_model_copy_update(self, sample_agent: AgentIdentity) -> None: assert sample_agent.status is AgentStatus.ACTIVE def test_json_roundtrip(self, sample_agent: AgentIdentity) -> None: + """Verify JSON serialization and deserialization preserves fields.""" json_str = sample_agent.model_dump_json() restored = AgentIdentity.model_validate_json(json_str) assert restored.name == sample_agent.name @@ -441,6 +518,7 @@ def test_json_roundtrip(self, sample_agent: AgentIdentity) -> None: def test_json_roundtrip_with_full_nested_data( self, sample_model_config: ModelConfig ) -> None: + """Verify roundtrip with all nested configs explicitly set.""" agent = AgentIdentity( name="Full Agent", role="Lead Dev", @@ -472,6 +550,7 @@ def test_json_roundtrip_with_full_nested_data( assert restored == agent def test_factory(self) -> None: + """Verify factory produces a valid AgentIdentity with UUID and model.""" agent = AgentIdentityFactory.build() assert isinstance(agent, AgentIdentity) assert isinstance(agent.id, UUID) diff --git a/tests/unit/core/test_company.py b/tests/unit/core/test_company.py index 26b84a894d..ce85b7f935 100644 --- a/tests/unit/core/test_company.py +++ b/tests/unit/core/test_company.py @@ -20,12 +20,17 @@ TeamFactory, ) +pytestmark = pytest.mark.timeout(30) + # ── Team ─────────────────────────────────────────────────────────── @pytest.mark.unit class TestTeam: + """Tests for Team validation, defaults, and immutability.""" + def test_valid_team(self) -> None: + """Verify a valid team persists all provided fields.""" team = Team( name="backend", lead="backend_lead", @@ -36,34 +41,42 @@ def test_valid_team(self) -> None: assert len(team.members) == 2 def test_defaults(self) -> None: + """Verify default empty members tuple.""" team = Team(name="test", lead="lead") assert team.members == () def test_empty_name_rejected(self) -> None: + """Reject empty team name.""" with pytest.raises(ValidationError): Team(name="", lead="lead") def test_empty_lead_rejected(self) -> None: + """Reject empty lead name.""" with pytest.raises(ValidationError): Team(name="test", lead="") def test_whitespace_name_rejected(self) -> None: + """Reject whitespace-only team name.""" with pytest.raises(ValidationError): Team(name=" ", lead="lead") def test_whitespace_lead_rejected(self) -> None: + """Reject whitespace-only lead name.""" with pytest.raises(ValidationError): Team(name="test", lead=" ") def test_empty_member_name_rejected(self) -> None: + """Reject empty string in members tuple.""" with pytest.raises(ValidationError, match="Empty or whitespace-only"): Team(name="test", lead="lead", members=("dev", "")) def test_whitespace_member_name_rejected(self) -> None: + """Reject whitespace-only member name.""" with pytest.raises(ValidationError, match="Empty or whitespace-only"): Team(name="test", lead="lead", members=(" ",)) def test_duplicate_members_rejected(self) -> None: + """Reject duplicate member names in a team.""" with pytest.raises(ValidationError, match="Duplicate members"): Team( name="backend", @@ -72,11 +85,13 @@ def test_duplicate_members_rejected(self) -> None: ) def test_frozen(self) -> None: + """Ensure Team is immutable.""" team = Team(name="test", lead="lead") with pytest.raises(ValidationError): team.name = "changed" # type: ignore[misc] def test_factory(self) -> None: + """Verify factory produces a valid Team.""" team = TeamFactory.build() assert isinstance(team, Team) @@ -86,34 +101,43 @@ def test_factory(self) -> None: @pytest.mark.unit class TestDepartment: + """Tests for Department validation, budget constraints, and immutability.""" + def test_valid_department(self, sample_department: Department) -> None: + """Verify fixture-provided department has expected fields.""" assert sample_department.name == "Engineering" assert sample_department.head == "cto" assert sample_department.budget_percent == 60.0 assert len(sample_department.teams) == 1 def test_defaults(self) -> None: + """Verify default budget_percent and empty teams.""" dept = Department(name="Test", head="head") assert dept.budget_percent == 0.0 assert dept.teams == () def test_budget_percent_zero(self) -> None: + """Accept budget_percent at lower boundary (0.0).""" dept = Department(name="Test", head="head", budget_percent=0.0) assert dept.budget_percent == 0.0 def test_budget_percent_hundred(self) -> None: + """Accept budget_percent at upper boundary (100.0).""" dept = Department(name="Test", head="head", budget_percent=100.0) assert dept.budget_percent == 100.0 def test_budget_percent_negative_rejected(self) -> None: + """Reject negative budget_percent.""" with pytest.raises(ValidationError): Department(name="Test", head="head", budget_percent=-1.0) def test_budget_percent_over_hundred_rejected(self) -> None: + """Reject budget_percent above 100.""" with pytest.raises(ValidationError): Department(name="Test", head="head", budget_percent=100.1) def test_multiple_distinct_teams_accepted(self) -> None: + """Accept department with multiple uniquely named teams.""" dept = Department( name="Engineering", head="cto", @@ -126,14 +150,17 @@ def test_multiple_distinct_teams_accepted(self) -> None: assert len(dept.teams) == 3 def test_whitespace_name_rejected(self) -> None: + """Reject whitespace-only department name.""" with pytest.raises(ValidationError, match="whitespace-only"): Department(name=" ", head="head") def test_whitespace_head_rejected(self) -> None: + """Reject whitespace-only head name.""" with pytest.raises(ValidationError, match="whitespace-only"): Department(name="Eng", head=" ") def test_duplicate_team_names_rejected(self) -> None: + """Reject duplicate team names within a department.""" with pytest.raises(ValidationError, match="Duplicate team names"): Department( name="Eng", @@ -145,10 +172,12 @@ def test_duplicate_team_names_rejected(self) -> None: ) def test_frozen(self, sample_department: Department) -> None: + """Ensure Department is immutable.""" with pytest.raises(ValidationError): sample_department.name = "Changed" # type: ignore[misc] def test_factory(self) -> None: + """Verify factory produces a valid Department with sane budget.""" dept = DepartmentFactory.build() assert isinstance(dept, Department) assert 0.0 <= dept.budget_percent <= 100.0 @@ -159,7 +188,10 @@ def test_factory(self) -> None: @pytest.mark.unit class TestCompanyConfig: + """Tests for CompanyConfig defaults, autonomy bounds, and validation.""" + def test_defaults(self) -> None: + """Verify default autonomy, budget, and communication pattern.""" cfg = CompanyConfig() assert cfg.autonomy == 0.5 assert cfg.budget_monthly == 100.0 @@ -167,41 +199,50 @@ def test_defaults(self) -> None: assert cfg.tool_access_default == () def test_autonomy_boundaries(self) -> None: + """Accept autonomy at both boundaries (0.0 and 1.0).""" low = CompanyConfig(autonomy=0.0) high = CompanyConfig(autonomy=1.0) assert low.autonomy == 0.0 assert high.autonomy == 1.0 def test_autonomy_below_zero_rejected(self) -> None: + """Reject autonomy below 0.0.""" with pytest.raises(ValidationError): CompanyConfig(autonomy=-0.1) def test_autonomy_above_one_rejected(self) -> None: + """Reject autonomy above 1.0.""" with pytest.raises(ValidationError): CompanyConfig(autonomy=1.1) def test_budget_negative_rejected(self) -> None: + """Reject negative monthly budget.""" with pytest.raises(ValidationError): CompanyConfig(budget_monthly=-1.0) def test_empty_communication_pattern_rejected(self) -> None: + """Reject empty communication_pattern string.""" with pytest.raises(ValidationError): CompanyConfig(communication_pattern="") def test_whitespace_communication_pattern_rejected(self) -> None: + """Reject whitespace-only communication_pattern.""" with pytest.raises(ValidationError): CompanyConfig(communication_pattern=" ") def test_empty_tool_access_entry_rejected(self) -> None: + """Reject empty string in tool_access_default tuple.""" with pytest.raises(ValidationError, match="Empty or whitespace-only"): CompanyConfig(tool_access_default=("git", "")) def test_frozen(self) -> None: + """Ensure CompanyConfig is immutable.""" cfg = CompanyConfig() with pytest.raises(ValidationError): cfg.autonomy = 1.0 # type: ignore[misc] def test_factory(self) -> None: + """Verify factory produces a valid CompanyConfig.""" cfg = CompanyConfigFactory.build() assert isinstance(cfg, CompanyConfig) @@ -211,13 +252,17 @@ def test_factory(self) -> None: @pytest.mark.unit class TestHRRegistry: + """Tests for HRRegistry defaults, uniqueness constraints, and validation.""" + def test_defaults(self) -> None: + """Verify default empty tuples for all fields.""" hr = HRRegistry() assert hr.active_agents == () assert hr.available_roles == () assert hr.hiring_queue == () def test_custom_values(self) -> None: + """Verify explicitly provided values are persisted.""" hr = HRRegistry( active_agents=("agent_1",), available_roles=("dev", "pm"), @@ -227,18 +272,22 @@ def test_custom_values(self) -> None: assert len(hr.available_roles) == 2 def test_duplicate_active_agents_rejected(self) -> None: + """Reject duplicate entries in active_agents.""" with pytest.raises(ValidationError, match="Duplicate entries"): HRRegistry(active_agents=("alice", "alice")) def test_empty_active_agent_rejected(self) -> None: + """Reject empty string in active_agents.""" with pytest.raises(ValidationError, match="Empty or whitespace-only"): HRRegistry(active_agents=("",)) def test_empty_available_role_rejected(self) -> None: + """Reject whitespace-only entry in available_roles.""" with pytest.raises(ValidationError, match="Empty or whitespace-only"): HRRegistry(available_roles=(" ",)) def test_empty_hiring_queue_entry_rejected(self) -> None: + """Reject empty string in hiring_queue.""" with pytest.raises(ValidationError, match="Empty or whitespace-only"): HRRegistry(hiring_queue=("",)) @@ -253,11 +302,13 @@ def test_duplicate_hiring_queue_accepted(self) -> None: assert hr.hiring_queue == ("pm", "pm") def test_frozen(self) -> None: + """Ensure HRRegistry is immutable.""" hr = HRRegistry() with pytest.raises(ValidationError): hr.active_agents = ("new",) # type: ignore[misc] def test_factory(self) -> None: + """Verify factory produces a valid HRRegistry.""" hr = HRRegistryFactory.build() assert isinstance(hr, HRRegistry) @@ -267,12 +318,16 @@ def test_factory(self) -> None: @pytest.mark.unit class TestCompany: + """Tests for Company construction, budget validation, and serialization.""" + def test_valid_company(self, sample_company: Company) -> None: + """Verify fixture-provided company has expected fields.""" assert sample_company.name == "Test Corp" assert len(sample_company.departments) == 1 assert sample_company.config.budget_monthly == 100.0 def test_defaults(self) -> None: + """Verify default type, departments, config, and hr_registry.""" co = Company(name="Minimal") assert co.type is CompanyType.CUSTOM assert co.departments == () @@ -280,6 +335,7 @@ def test_defaults(self) -> None: assert isinstance(co.hr_registry, HRRegistry) def test_budget_sum_at_100_accepted(self) -> None: + """Accept departments whose budget_percent sums to exactly 100.""" depts = ( Department(name="A", head="a", budget_percent=60.0), Department(name="B", head="b", budget_percent=40.0), @@ -288,6 +344,7 @@ def test_budget_sum_at_100_accepted(self) -> None: assert sum(d.budget_percent for d in co.departments) == 100.0 def test_budget_sum_under_100_accepted(self) -> None: + """Accept departments whose budget_percent sums to less than 100.""" depts = ( Department(name="A", head="a", budget_percent=50.0), Department(name="B", head="b", budget_percent=30.0), @@ -296,6 +353,7 @@ def test_budget_sum_under_100_accepted(self) -> None: assert sum(d.budget_percent for d in co.departments) == 80.0 def test_budget_sum_over_100_rejected(self) -> None: + """Reject departments whose budget_percent exceeds 100.""" depts = ( Department(name="A", head="a", budget_percent=60.0), Department(name="B", head="b", budget_percent=50.0), @@ -304,6 +362,7 @@ def test_budget_sum_over_100_rejected(self) -> None: Company(name="Over Budget", departments=depts) def test_budget_sum_barely_over_100_rejected(self) -> None: + """Reject budget_percent sums just barely over 100.""" depts = ( Department(name="A", head="a", budget_percent=50.01), Department(name="B", head="b", budget_percent=50.0), @@ -323,6 +382,7 @@ def test_budget_sum_float_precision_accepted(self) -> None: assert len(co.departments) == 3 def test_duplicate_department_names_rejected(self) -> None: + """Reject duplicate department names within a company.""" depts = ( Department(name="Engineering", head="a", budget_percent=30.0), Department(name="Engineering", head="b", budget_percent=20.0), @@ -331,32 +391,39 @@ def test_duplicate_department_names_rejected(self) -> None: Company(name="Dup Depts", departments=depts) def test_empty_name_rejected(self) -> None: + """Reject empty company name.""" with pytest.raises(ValidationError): Company(name="") def test_whitespace_name_rejected(self) -> None: + """Reject whitespace-only company name.""" with pytest.raises(ValidationError, match="whitespace-only"): Company(name=" ") def test_empty_departments_accepted(self) -> None: + """Accept a company with no departments.""" co = Company(name="Empty") assert co.departments == () def test_frozen(self, sample_company: Company) -> None: + """Ensure Company is immutable.""" with pytest.raises(ValidationError): sample_company.name = "Changed" # type: ignore[misc] def test_model_copy_update(self, sample_company: Company) -> None: + """Verify model_copy creates a new instance without mutating the original.""" updated = sample_company.model_copy(update={"name": "New Corp"}) assert updated.name == "New Corp" assert sample_company.name == "Test Corp" def test_json_roundtrip(self, sample_company: Company) -> None: + """Verify JSON serialization and deserialization preserves fields.""" json_str = sample_company.model_dump_json() restored = Company.model_validate_json(json_str) assert restored.name == sample_company.name assert len(restored.departments) == len(sample_company.departments) def test_factory(self) -> None: + """Verify factory produces a valid Company.""" co = CompanyFactory.build() assert isinstance(co, Company) diff --git a/tests/unit/core/test_role.py b/tests/unit/core/test_role.py index 0e8005a43b..b0729e025a 100644 --- a/tests/unit/core/test_role.py +++ b/tests/unit/core/test_role.py @@ -19,6 +19,8 @@ SkillFactory, ) +pytestmark = pytest.mark.timeout(30) + # ── Skill ────────────────────────────────────────────────────────── diff --git a/tests/unit/core/test_role_catalog.py b/tests/unit/core/test_role_catalog.py index 8b44a98834..b75cd9c7b0 100644 --- a/tests/unit/core/test_role_catalog.py +++ b/tests/unit/core/test_role_catalog.py @@ -18,40 +18,52 @@ get_seniority_info, ) +pytestmark = pytest.mark.timeout(30) + # ── Seniority Info ───────────────────────────────────────────────── @pytest.mark.unit class TestSeniorityInfo: + """Tests for the SENIORITY_INFO tuple coverage and integrity.""" + def test_has_8_entries(self) -> None: + """Verify SENIORITY_INFO contains exactly 8 mappings.""" assert len(SENIORITY_INFO) == 8 def test_covers_all_seniority_levels(self) -> None: + """Verify every SeniorityLevel enum value has a mapping.""" levels = {info.level for info in SENIORITY_INFO} expected = set(SeniorityLevel) assert levels == expected def test_no_duplicate_levels(self) -> None: + """Verify no two entries share the same seniority level.""" levels = [info.level for info in SENIORITY_INFO] assert len(levels) == len(set(levels)) def test_all_entries_are_seniority_info(self) -> None: + """Verify every entry is a SeniorityInfo instance.""" for info in SENIORITY_INFO: assert isinstance(info, SeniorityInfo) def test_junior_is_low_cost(self) -> None: + """Verify JUNIOR maps to LOW cost tier.""" info = get_seniority_info(SeniorityLevel.JUNIOR) assert info.cost_tier == CostTier.LOW def test_c_suite_is_premium_cost(self) -> None: + """Verify C_SUITE maps to PREMIUM cost tier.""" info = get_seniority_info(SeniorityLevel.C_SUITE) assert info.cost_tier == CostTier.PREMIUM def test_senior_uses_sonnet_tier(self) -> None: + """Verify SENIOR maps to 'sonnet' model tier.""" info = get_seniority_info(SeniorityLevel.SENIOR) assert info.typical_model_tier == "sonnet" def test_all_entries_frozen(self) -> None: + """Verify all SeniorityInfo entries are immutable.""" for info in SENIORITY_INFO: with pytest.raises(ValidationError): info.level = SeniorityLevel.JUNIOR # type: ignore[misc] @@ -62,23 +74,30 @@ def test_all_entries_frozen(self) -> None: @pytest.mark.unit class TestBuiltinRoles: + """Tests for the BUILTIN_ROLES tuple completeness and invariants.""" + def test_has_31_roles(self) -> None: + """Verify BUILTIN_ROLES contains exactly 31 roles.""" assert len(BUILTIN_ROLES) == 31 def test_all_entries_are_role(self) -> None: + """Verify every entry is a Role instance.""" for role in BUILTIN_ROLES: assert isinstance(role, Role) def test_no_duplicate_names(self) -> None: + """Verify no two built-in roles share the same name.""" names = [r.name for r in BUILTIN_ROLES] assert len(names) == len(set(names)) def test_all_departments_represented(self) -> None: + """Verify every DepartmentName enum value has at least one role.""" departments = {r.department for r in BUILTIN_ROLES} expected = set(DepartmentName) assert departments == expected def test_c_suite_roles_present(self) -> None: + """Verify all expected C-suite roles exist.""" c_suite = [ r for r in BUILTIN_ROLES if r.authority_level is SeniorityLevel.C_SUITE ] @@ -86,14 +105,17 @@ def test_c_suite_roles_present(self) -> None: assert {"CEO", "CTO", "CFO", "COO", "CPO"}.issubset(names) def test_all_roles_have_description(self) -> None: + """Verify every built-in role has a non-empty description.""" for role in BUILTIN_ROLES: assert role.description, f"{role.name} has no description" def test_all_roles_have_required_skills(self) -> None: + """Verify every built-in role has at least one required skill.""" for role in BUILTIN_ROLES: assert len(role.required_skills) > 0, f"{role.name} has no required_skills" def test_all_roles_frozen(self) -> None: + """Verify all built-in roles are immutable.""" for role in BUILTIN_ROLES: with pytest.raises(ValidationError): role.name = "Changed" # type: ignore[misc] @@ -104,33 +126,42 @@ def test_all_roles_frozen(self) -> None: @pytest.mark.unit class TestGetBuiltinRole: + """Tests for the get_builtin_role lookup function.""" + def test_exact_match(self) -> None: + """Verify exact name lookup returns the correct role.""" role = get_builtin_role("CEO") assert role is not None assert role.name == "CEO" def test_case_insensitive(self) -> None: + """Verify lookup is case-insensitive.""" role = get_builtin_role("ceo") assert role is not None assert role.name == "CEO" def test_mixed_case(self) -> None: + """Verify lookup with mixed case and spaces works.""" role = get_builtin_role("Backend Developer") assert role is not None assert role.name == "Backend Developer" def test_not_found_returns_none(self) -> None: + """Verify unknown role name returns None.""" assert get_builtin_role("Nonexistent Role") is None def test_empty_string_returns_none(self) -> None: + """Verify empty string returns None.""" assert get_builtin_role("") is None def test_whitespace_stripped(self) -> None: + """Verify leading/trailing whitespace is stripped before lookup.""" role = get_builtin_role(" CEO ") assert role is not None assert role.name == "CEO" def test_whitespace_only_returns_none(self) -> None: + """Verify whitespace-only input returns None.""" assert get_builtin_role(" ") is None @pytest.mark.parametrize( @@ -170,6 +201,7 @@ def test_whitespace_only_returns_none(self) -> None: ], ) def test_all_roles_lookupable(self, name: str) -> None: + """Verify each built-in role is findable by its exact name.""" role = get_builtin_role(name) assert role is not None, f"Role {name!r} not found in catalog" assert role.name == name @@ -177,16 +209,21 @@ def test_all_roles_lookupable(self, name: str) -> None: @pytest.mark.unit class TestGetSeniorityInfo: + """Tests for the get_seniority_info lookup function.""" + def test_found(self) -> None: + """Verify lookup returns matching SeniorityInfo.""" info = get_seniority_info(SeniorityLevel.SENIOR) assert info.level is SeniorityLevel.SENIOR @pytest.mark.parametrize("level", list(SeniorityLevel)) def test_all_levels_lookupable(self, level: SeniorityLevel) -> None: + """Verify every SeniorityLevel is lookupable.""" info = get_seniority_info(level) assert info.level is level def test_raises_lookup_error_for_missing_level(self) -> None: + """Verify LookupError when the internal map is empty.""" with ( patch.dict( "ai_company.core.role_catalog._SENIORITY_INFO_BY_LEVEL",