Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ src/ai_company/
- **Docstrings**: Google style, required on public classes/functions (enforced by ruff D rules)
- **Immutability**: create new objects, never mutate existing ones. For non-Pydantic internal collections (registries, `BaseTool`), use `copy.deepcopy()` at construction + `MappingProxyType` wrapping for read-only enforcement. For `dict`/`list` fields in frozen Pydantic models, rely on `frozen=True` for field reassignment prevention and `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).
- **Config vs runtime state**: frozen Pydantic models for config/identity; separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state that evolves (e.g. agent execution state, task progress). Never mix static config fields with mutable runtime fields in one model.
- **Models**: Pydantic v2 (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`). Adopted conventions: use `@computed_field` for derived values instead of storing + validating redundant fields (e.g. `TokenUsage.total_tokens`). Planned conventions for new code: use `NotBlankStr` (from `core.types`) for non-optional identifier/name fields instead of manual whitespace validators. Existing models are being migrated incrementally.
- **Models**: Pydantic v2 (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`). Adopted conventions: use `@computed_field` for derived values instead of storing + validating redundant fields (e.g. `TokenUsage.total_tokens`); use `NotBlankStr` (from `core.types`) for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple (`tuple[NotBlankStr, ...]`) variants — instead of manual whitespace validators.
- **Async concurrency**: prefer `asyncio.TaskGroup` for fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over bare `create_task`. Existing code is being migrated incrementally.
- **Line length**: 88 characters (ruff)
- **Functions**: < 50 lines, files < 800 lines
Expand Down
4 changes: 2 additions & 2 deletions DESIGN_SPEC.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ Every agent has a comprehensive identity. At the design level, agent data splits
- **Config (immutable)**: identity, personality, skills, model preferences, tool permissions, authority. Defined at hire time, changed only by explicit reconfiguration. Represented as frozen Pydantic models.
- **Runtime state (mutable-via-copy)**: current status, active task, conversation history, execution metrics. Evolves during agent operation. Represented as Pydantic models using `model_copy(update=...)` for state transitions — never mutated in place.

> **Current state (M2):** Only the config layer exists as `AgentIdentity` (frozen Pydantic model in `core/agent.py`). The runtime state layer will be introduced in M3 when the agent execution engine is implemented. Non-optional identifier fields currently use `str` with `Field(min_length=1)` + a manual `@model_validator`; migration to `NotBlankStr` (from `core.types`) is planned.
> **Current state (M2):** Only the config layer exists as `AgentIdentity` (frozen Pydantic model in `core/agent.py`). The runtime state layer will be introduced in M3 when the agent execution engine is implemented. All identifier/name fields use `NotBlankStr` (from `core.types`) for automatic whitespace rejection; tuple fields use `tuple[NotBlankStr, ...]` for per-element validation.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Document the optional NotBlankStr | None variant here too.

These two spec updates now mention scalar and tuple usage, but the adopted convention in this PR also covers optional fields. Since DESIGN_SPEC.md is the required source of truth, leaving that variant out here will keep it out of sync with CLAUDE.md and the implementation.

Based on learnings: When making changes that affect architecture, services, key files, settings, or workflows, update the relevant sections of existing documentation (CLAUDE.md, README.md, etc.) to reflect those changes.

Also applies to: 1442-1442

🧰 Tools
🪛 LanguageTool

[style] ~109-~109: Consider using the typographical ellipsis character here instead.
Context: ... whitespace rejection; tuple fields use tuple[NotBlankStr, ...] for per-element validation. ```yaml ...

(ELLIPSIS)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@DESIGN_SPEC.md` at line 109, The DESIGN_SPEC.md line describing field types
omits the optional variant; update the sentence referencing NotBlankStr and
tuple[NotBlankStr, ...] to also document the optional form NotBlankStr | None
(and tuple[NotBlankStr, ...] | None where appropriate) so the spec matches the
implementation and CLAUDE.md; mention AgentIdentity (frozen Pydantic model in
core/agent.py) as using NotBlankStr, NotBlankStr | None for optional
identifier/name fields and tuple[NotBlankStr, ...] (and its optional variant)
for tuple fields to keep documentation consistent.


```yaml
# --- Current (M2): Config layer — AgentIdentity (frozen) ---
Expand Down Expand Up @@ -1439,7 +1439,7 @@ These conventions were established during the M0–M2 review cycle. **Adopted**
| **Immutability strategy** | Adopted | `copy.deepcopy()` at construction + `MappingProxyType` wrapping for non-Pydantic internal collections (registries, `BaseTool`). For Pydantic frozen models: `frozen=True` prevents field reassignment; `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization) prevents nested mutation. No MappingProxyType inside Pydantic models (serialization friction). | Deep-copy at construction fully isolates nested structures; `MappingProxyType` enforces read-only access. Boundary-copy for Pydantic models is simple, centralized, and Pydantic-native. A future CPython built-in immutable mapping type (e.g. `frozendict`) would provide zero-friction field-level immutability when available. |
| **Config vs runtime split** | Adopted (M3) | Frozen models for config/identity; `model_copy(update=...)` for runtime state transitions | `TaskExecution` and `AgentContext` (in `engine/`) are frozen Pydantic models that use `model_copy(update=...)` for copy-on-write state transitions without re-running validators (per Pydantic `model_copy` semantics). Config layer (`AgentIdentity`, `Task`) remains unchanged. |
| **Derived fields** | Adopted | `@computed_field` instead of stored + validated | Eliminates redundant storage and impossible-to-fail validators. `TokenUsage.total_tokens` migrated from stored `Field` + `@model_validator` to `@computed_field` property. |
| **String validation** | Planned | `NotBlankStr` type from `core.types` for all identifiers | Eliminates per-model `@model_validator` boilerplate for whitespace checks. `NotBlankStr` is defined but models still use `Field(min_length=1)` + manual validators. |
| **String validation** | Adopted | `NotBlankStr` type from `core.types` for all identifiers | Eliminates per-model `@model_validator` boilerplate for whitespace checks. All identifier/name fields use `NotBlankStr`; tuple fields use `tuple[NotBlankStr, ...]` for per-element validation. |
| **Shared field groups** | Planned | Extract common field sets into base models (e.g. `_SpendingTotals`) | Prevents field duplication across spending summary models. Not yet implemented — each model independently defines fields. |
| **Event constants** | Adopted (flat) | Single `events.py` module with domain-scoped naming (e.g. `PROVIDER_CALL_START`, `BUDGET_RECORD_ADDED`) | Current approach uses a single module. Splitting into per-domain submodules may be revisited when the file exceeds ~200 constants. |
| **Parallel tool execution** | Planned | `asyncio.TaskGroup` in `ToolInvoker.invoke_all` | Structured concurrency with proper cancellation semantics. Currently sequential; migration planned for M3 when the agent engine needs concurrent tool calls. |
Expand Down
10 changes: 3 additions & 7 deletions src/ai_company/budget/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

from pydantic import BaseModel, ConfigDict, Field, model_validator

from ai_company.core.types import NotBlankStr # noqa: TC001


class BudgetAlertConfig(BaseModel):
"""Alert threshold configuration for budget monitoring.
Expand Down Expand Up @@ -87,7 +89,7 @@ class AutoDowngradeConfig(BaseModel):
strict=True,
description="Budget percent triggering downgrade",
)
downgrade_map: tuple[tuple[str, str], ...] = Field(
downgrade_map: tuple[tuple[NotBlankStr, NotBlankStr], ...] = Field(
default=(),
description="Ordered pairs of (from_alias, to_alias)",
)
Expand All @@ -110,12 +112,6 @@ def _validate_downgrade_map(self) -> Self:
"""Validate downgrade_map for correctness."""
sources: list[str] = []
for source, target in self.downgrade_map:
if not source:
msg = "Empty or whitespace-only source alias in downgrade_map"
raise ValueError(msg)
if not target:
msg = "Empty or whitespace-only target alias in downgrade_map"
raise ValueError(msg)
if source == target:
msg = f"Self-downgrade in downgrade_map: {source!r} -> {target!r}"
raise ValueError(msg)
Expand Down
19 changes: 6 additions & 13 deletions src/ai_company/budget/cost_record.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

from pydantic import AwareDatetime, BaseModel, ConfigDict, Field, model_validator

from ai_company.core.types import NotBlankStr # noqa: TC001


class CostRecord(BaseModel):
"""Immutable record of a single API call's cost.
Expand All @@ -29,24 +31,15 @@ class CostRecord(BaseModel):

model_config = ConfigDict(frozen=True)

agent_id: str = Field(min_length=1, description="Agent identifier")
task_id: str = Field(min_length=1, description="Task identifier")
provider: str = Field(min_length=1, description="LLM provider name")
model: str = Field(min_length=1, description="Model identifier")
agent_id: NotBlankStr = Field(description="Agent identifier")
task_id: NotBlankStr = Field(description="Task identifier")
provider: NotBlankStr = Field(description="LLM provider name")
model: NotBlankStr = Field(description="Model identifier")
input_tokens: int = Field(ge=0, description="Input token count")
output_tokens: int = Field(ge=0, description="Output token count")
cost_usd: float = Field(ge=0.0, description="Cost in USD")
timestamp: AwareDatetime = Field(description="Timestamp of the API call")

@model_validator(mode="after")
def _validate_no_blank_strings(self) -> Self:
"""Ensure string identifier fields are not whitespace-only."""
for field_name in ("agent_id", "task_id", "provider", "model"):
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_token_consistency(self) -> Self:
"""Ensure positive cost implies at least one non-zero token count."""
Expand Down
23 changes: 3 additions & 20 deletions src/ai_company/budget/hierarchy.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from pydantic import BaseModel, ConfigDict, Field, model_validator

from ai_company.constants import BUDGET_ROUNDING_PRECISION
from ai_company.core.types import NotBlankStr # noqa: TC001


class TeamBudget(BaseModel):
Expand All @@ -23,8 +24,7 @@ class TeamBudget(BaseModel):

model_config = ConfigDict(frozen=True)

team_name: str = Field(
min_length=1,
team_name: NotBlankStr = Field(
description="Team name",
)
budget_percent: float = Field(
Expand All @@ -34,14 +34,6 @@ class TeamBudget(BaseModel):
description="Percent of department budget",
)

@model_validator(mode="after")
def _validate_team_name_not_blank(self) -> Self:
"""Ensure team_name is not whitespace-only."""
if not self.team_name.strip():
msg = "team_name must not be whitespace-only"
raise ValueError(msg)
return self


class DepartmentBudget(BaseModel):
"""Budget allocation for a department with nested team allocations.
Expand All @@ -57,8 +49,7 @@ class DepartmentBudget(BaseModel):

model_config = ConfigDict(frozen=True)

department_name: str = Field(
min_length=1,
department_name: NotBlankStr = Field(
description="Department name",
)
budget_percent: float = Field(
Expand All @@ -72,14 +63,6 @@ class DepartmentBudget(BaseModel):
description="Team budget allocations",
)

@model_validator(mode="after")
def _validate_department_name_not_blank(self) -> Self:
"""Ensure department_name is not whitespace-only."""
if not self.department_name.strip():
msg = "department_name must not be whitespace-only"
raise ValueError(msg)
return self

@model_validator(mode="after")
def _validate_unique_team_names(self) -> Self:
"""Ensure no duplicate team names within the department."""
Expand Down
22 changes: 3 additions & 19 deletions src/ai_company/budget/spending_summary.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from pydantic import BaseModel, ConfigDict, Field, model_validator

from ai_company.budget.enums import BudgetAlertLevel
from ai_company.core.types import NotBlankStr # noqa: TC001


class PeriodSpending(BaseModel):
Expand Down Expand Up @@ -77,7 +78,7 @@ class AgentSpending(BaseModel):

model_config = ConfigDict(frozen=True)

agent_id: str = Field(min_length=1, description="Agent identifier")
agent_id: NotBlankStr = Field(description="Agent identifier")
total_cost_usd: float = Field(
default=0.0,
ge=0.0,
Expand All @@ -99,14 +100,6 @@ class AgentSpending(BaseModel):
description="Number of cost records",
)

@model_validator(mode="after")
def _validate_agent_id_not_blank(self) -> Self:
"""Ensure agent_id is not whitespace-only."""
if not self.agent_id.strip():
msg = "agent_id must not be whitespace-only"
raise ValueError(msg)
return self


class DepartmentSpending(BaseModel):
"""Spending aggregation for a department.
Expand All @@ -121,8 +114,7 @@ class DepartmentSpending(BaseModel):

model_config = ConfigDict(frozen=True)

department_name: str = Field(
min_length=1,
department_name: NotBlankStr = Field(
description="Department name",
)
total_cost_usd: float = Field(
Expand All @@ -146,14 +138,6 @@ class DepartmentSpending(BaseModel):
description="Number of cost records",
)

@model_validator(mode="after")
def _validate_department_name_not_blank(self) -> Self:
"""Ensure department_name is not whitespace-only."""
if not self.department_name.strip():
msg = "department_name must not be whitespace-only"
raise ValueError(msg)
return self


class SpendingSummary(BaseModel):
"""Top-level spending summary combining all aggregation dimensions.
Expand Down
8 changes: 4 additions & 4 deletions src/ai_company/communication/channel.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from ai_company.communication.enums import ChannelType
from ai_company.core.types import (
NotBlankStr,
validate_non_blank_unique_strings,
validate_unique_strings,
)


Expand All @@ -27,13 +27,13 @@ class Channel(BaseModel):
default=ChannelType.TOPIC,
description="Channel delivery semantics",
)
subscribers: tuple[str, ...] = Field(
subscribers: tuple[NotBlankStr, ...] = Field(
default=(),
description="Agent IDs subscribed to this channel",
)

@model_validator(mode="after")
def _validate_subscribers(self) -> Self:
"""Ensure subscriber entries are non-blank and unique."""
validate_non_blank_unique_strings(self.subscribers, "subscribers")
"""Ensure subscriber entries are unique."""
validate_unique_strings(self.subscribers, "subscribers")
return self
14 changes: 7 additions & 7 deletions src/ai_company/communication/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
)
from ai_company.core.types import (
NotBlankStr,
validate_non_blank_unique_strings,
validate_unique_strings,
)

# Default channels from DESIGN_SPEC Section 5.4.
Expand Down Expand Up @@ -42,15 +42,15 @@ class MessageBusConfig(BaseModel):
default=MessageBusBackend.INTERNAL,
description="Transport backend",
)
channels: tuple[str, ...] = Field(
channels: tuple[NotBlankStr, ...] = Field(
default=_DEFAULT_CHANNELS,
description="Pre-defined channel names",
)

@model_validator(mode="after")
def _validate_channels(self) -> Self:
"""Ensure channel names are non-blank and unique."""
validate_non_blank_unique_strings(self.channels, "channels")
"""Ensure channel names are unique."""
validate_unique_strings(self.channels, "channels")
return self


Expand Down Expand Up @@ -79,7 +79,7 @@ class MeetingTypeConfig(BaseModel):
default=None,
description="Event trigger",
)
participants: tuple[str, ...] = Field(
participants: tuple[NotBlankStr, ...] = Field(
default=(),
description="Participant role or agent identifiers",
)
Expand All @@ -102,8 +102,8 @@ def _validate_frequency_or_trigger(self) -> Self:

@model_validator(mode="after")
def _validate_participants(self) -> Self:
"""Ensure participant entries are non-blank and unique."""
validate_non_blank_unique_strings(self.participants, "participants")
"""Ensure participant entries are unique."""
validate_unique_strings(self.participants, "participants")
return self


Expand Down
4 changes: 2 additions & 2 deletions src/ai_company/core/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
)
from ai_company.core.task import AcceptanceCriterion, Task
from ai_company.core.task_transitions import VALID_TRANSITIONS, validate_transition
from ai_company.core.types import NotBlankStr, validate_non_blank_unique_strings
from ai_company.core.types import NotBlankStr, validate_unique_strings

__all__ = [
"BUILTIN_ROLES",
Expand Down Expand Up @@ -96,6 +96,6 @@
"ToolPermissions",
"get_builtin_role",
"get_seniority_info",
"validate_non_blank_unique_strings",
"validate_transition",
"validate_unique_strings",
]
Loading