Skip to content

feat: add AgentBrain abstract base class - #73

Open
AkashGowdaNC wants to merge 3 commits into
ARPAHLS:mainfrom
AkashGowdaNC:feat/issue-15-agentbrain-abc
Open

AkashGowdaNC wants to merge 3 commits into
ARPAHLS:mainfrom
AkashGowdaNC:feat/issue-15-agentbrain-abc

Conversation

@AkashGowdaNC

Copy link
Copy Markdown

Refs #15

What this PR does

Adds the AgentBrain abstract base class described in #15, plus tests.

This is step 1 of the plan discussed in the issue — the ABC and its tests only, with no production refactor yet. Follow-ups will move the LiteLLM path, then the custom-function path, behind this interface, and wire AgentConfig to pick a brain.

Why this is useful

The Agent class currently handles LiteLLM routing, custom-function dispatch, and Skillware tool-use in one place. Introducing the ABC first lets later PRs move each backend behind it without touching callers, keeping each change reviewable.

Changes

  • rooms/brains/__init__.py: new AgentBrain ABC with a generate() method matching the return shape of Agent.generate_response_with_events today
  • tests/test_agent_brain.py: 5 tests covering abstractness, minimal subclasses, result shape, and override-param pass-through

How to verify

@AkashGowdaNC

Copy link
Copy Markdown
Author

@rosspeili PR 1 is up: the AgentBrain ABC + tests. Ready when you are.

@rosspeili

Copy link
Copy Markdown
Contributor

Nice work on step1 @AkashGowdaNC!

A few small polish items before we land this and start Step 2:

  1. EOF newlines: Add a trailing newline to rooms/brains/__init__.py and tests/test_agent_brain.py (resolves Flake8 W292).
  2. Module exports: Add __all__ = ["AgentBrain"] to rooms/brains/__init__.py.
  3. Constructor & typing contract for Steps 2–4:
    • Document or declare the expected __init__ signature (e.g. accepting config: AgentConfig and optional skill_runtime) so concrete brains (LiteLLMBrain, CustomFunctionBrain) and external plugins have a uniform initialization contract.
    • (Optional/Recommended) Consider defining a TypedDict for the generate() return dict (content: str, skill_events: List[Dict[str, Any]]) to enforce typing across all future brains.

No changelog entry needed for this PR since it's internal scaffolding, we'll add one once the brains are wired to AgentConfig in Step 4. Push those tweaks and we're ready to merge.

@AkashGowdaNC

Copy link
Copy Markdown
Author

Thanks @rosspeili — pushed all four:

  • Added trailing newlines to both files (Flake8 W292 is clean)
  • Added __all__ = ["AgentBrain", "BrainResult"] to rooms/brains/__init__.py
  • Documented the constructor contract (config: AgentConfig, optional skill_runtime) in the class docstring so Step 2–4 brains and external plugins have a uniform init signature
  • Added a BrainResult TypedDict for the generate() return shape

Tests still pass (5/5). Ready when you are.

@rosspeili

Copy link
Copy Markdown
Contributor

Looks great @AkashGowdaNC! All 4 review items were addressed cleanly, the constructor contract and BrainResult TypedDict set up Steps 2–4 nicely, and all 82 tests pass.

Just one tiny formatting nit on line 5 of tests/test_agent_brain.py:

from rooms.brains import AgentBrain , BrainResult

There's a stray space before the comma (E203), and BrainResult is imported but unused in the test file (F401).

Please change it to:

from rooms.brains import AgentBrain

Once that's pushed, we'll merge this and open the floor for Step 2 (LiteLLMBrain).

@AkashGowdaNC

Copy link
Copy Markdown
Author

Thanks @rosspeili — fixed:

  • Removed the stray space before the comma (E203)
  • Dropped the unused BrainResult import (F401)

The file now imports from rooms.brains import AgentBrain. Tests still pass (5/5) and Flake8 is clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants