Feature code clean up 20240529 (#141)

* check-untyped-defs

* cont.

* cont.

* isort

* Fix test gnews

* Update agent_chain.py

* Fix Integration tests

* Update requirements

* Update

.github/workflows/unit_test.yml

@@ -23,7 +23,7 @@ jobs:
         run: |
           python -m pip install --upgrade pip
           pip install -r dev-requirements.txt
-          pip install -r requirements.txt
+          pip freeze
       - name: Run testing
         env:
           CI: true

Makefile

@@ -4,17 +4,20 @@ GIT_ROOT ?= $(shell git rev-parse --show-toplevel)
 
 format:
 	black .
+	isort .
 
 dev-lint:
 	mypy .
 	black .
 	ruff check . --fix
+	isort .
 
 lint:
 	mypy .
 	black . --check
 	ruff check .
-
+	pylint council/. --max-line-length 120 --disable=R,C,I,W1203,W0107 --fail-under=9
+	isort . --check-only
 
 test:
 	pytest tests

council/agent_tests/agent_tests.py

@@ -2,16 +2,12 @@
 
 import time
 from enum import Enum
-from typing import List, Dict, Any, Sequence, Optional
-import progressbar  # type: ignore
+from typing import Any, Dict, List, Optional, Sequence
 
+import progressbar  # type: ignore
 from council.agents import Agent
+from council.contexts import AgentContext, Budget, ScorerContext
 from council.scorers import ScorerBase, ScorerException
-from council.contexts import (
-    AgentContext,
-    Budget,
-    ScorerContext,
-)
 
 
 class AgentTestCaseOutcome(str, Enum):
@@ -22,7 +18,7 @@ class AgentTestCaseOutcome(str, Enum):
 
 
 class ScorerResult:
-    def __init__(self, scorer: ScorerBase, score: float):
+    def __init__(self, scorer: ScorerBase, score: float) -> None:
         self._scorer = scorer
         self._score = score
 
@@ -41,22 +37,14 @@ def to_dict(self) -> Dict[str, Any]:
 
 
 class AgentTestCaseResult:
-    _actual: str
-    _execution_time: float
-    _outcome: AgentTestCaseOutcome
-    _prompt: str
-    _scorers: List[ScorerBase]
-    _scorer_results: List[ScorerResult]
-    _error: str
-    _error_message: str
 
-    def __init__(self, prompt: str, scorers: List[ScorerBase]):
+    def __init__(self, prompt: str, scorers: List[ScorerBase]) -> None:
         self._actual = ""
-        self._execution_time = 0
+        self._execution_time = 0.0
         self._outcome = AgentTestCaseOutcome.Unknown
         self._prompt = prompt
-        self._scorers = scorers
-        self._scorer_results = []
+        self._scorers: List[ScorerBase] = scorers
+        self._scorer_results: List[ScorerResult] = []
         self._error = ""
         self._error_message = ""
 
@@ -84,18 +72,20 @@ def error(self) -> str:
     def error_message(self) -> str:
         return self._error_message
 
-    def set_success(self, actual: str, execution_time: float, scores: List[float]):
+    def set_success(self, actual: str, execution_time: float, scores: List[float]) -> None:
         self.set_result(actual, execution_time, scores, AgentTestCaseOutcome.Success)
 
-    def set_error(self, error: Exception, execution_time: float):
+    def set_error(self, error: Exception, execution_time: float) -> None:
         self.set_result("", execution_time, [], AgentTestCaseOutcome.Error)
         self._error = error.__class__.__name__
         self._error_message = str(error)
 
-    def set_inconclusive(self, execution_time: float):
+    def set_inconclusive(self, execution_time: float) -> None:
         self.set_result("", execution_time, [], AgentTestCaseOutcome.Inconclusive)
 
-    def set_result(self, actual: str, execution_time: float, scores: List[float], outcome: AgentTestCaseOutcome):
+    def set_result(
+        self, actual: str, execution_time: float, scores: List[float], outcome: AgentTestCaseOutcome
+    ) -> None:
         self._actual = actual
         self._execution_time = execution_time
         self._outcome = outcome
@@ -120,7 +110,7 @@ class AgentTestCase:
     _prompt: str
     _scorers: List[ScorerBase]
 
-    def __init__(self, prompt: str, scorers: List[ScorerBase]):
+    def __init__(self, prompt: str, scorers: List[ScorerBase]) -> None:
         self._prompt = prompt
         self._scorers = scorers
 
@@ -161,8 +151,8 @@ def run(self, agent: Agent) -> AgentTestCaseResult:
 class AgentTestSuiteResult:
     _results: List[AgentTestCaseResult]
 
-    def __init__(self):
-        self._results = []
+    def __init__(self) -> None:
+        self._results: List[AgentTestCaseResult] = []
 
     @property
     def results(self) -> Sequence[AgentTestCaseResult]:

council/agents/agent.py

@@ -11,6 +11,7 @@
 from council.filters import BasicFilter, FilterBase
 from council.runners import new_runner_executor
 from council.skills import SkillBase
+
 from .agent_result import AgentResult
 
 
@@ -124,14 +125,12 @@ def execute_plan(self, iteration_context: AgentContext, plan: Sequence[Execution
         try:
             for group in self._group_units(plan):
                 fs = [executor.submit(self._execute_unit, iteration_context, unit) for unit in group]
-                dones, not_dones = futures.wait(
-                    fs, iteration_context.budget.remaining_duration, futures.FIRST_EXCEPTION
-                )
-
+                dones, _ = futures.wait(fs, iteration_context.budget.remaining_duration, futures.FIRST_EXCEPTION)
                 # rethrow exception if any
                 [d.result(0) for d in dones]
         finally:
-            [f.cancel() for f in fs]
+            for f in fs:
+                f.cancel()
 
     @staticmethod
     def _group_units(plan: Sequence[ExecutionUnit]) -> List[List[ExecutionUnit]]:
@@ -154,7 +153,7 @@ def _group_units(plan: Sequence[ExecutionUnit]) -> List[List[ExecutionUnit]]:
         return result
 
     @staticmethod
-    def _execute_unit(iteration_context: AgentContext, unit: ExecutionUnit):
+    def _execute_unit(iteration_context: AgentContext, unit: ExecutionUnit) -> None:
         with iteration_context.new_agent_context_for_execution_unit(unit.name) as context:
             chain = unit.chain
             context.logger.info(f'message="chain execution started" chain="{chain.name}" execution_unit="{unit.name}"')

council/agents/agent_chain.py

@@ -1,8 +1,9 @@
-from typing import Any, Optional
+from typing import Optional
 
 from council.chains import ChainBase
 from council.contexts import AgentContext, ChainContext, ChatMessage, Monitored
 from council.runners import RunnerExecutor
+
 from .agent import Agent
 
 
@@ -13,7 +14,7 @@ class AgentChain(ChainBase):
 
     _agent: Monitored[Agent]
 
-    def __init__(self, name: str, description: str, agent: Agent):
+    def __init__(self, name: str, description: str, agent: Agent) -> None:
         """
         Initialize a new instance.
 
@@ -29,11 +30,7 @@ def __init__(self, name: str, description: str, agent: Agent):
     def agent(self) -> Agent:
         return self._agent.inner
 
-    def _execute(
-        self,
-        context: ChainContext,
-        _executor: Optional[RunnerExecutor] = None,
-    ) -> Any:
+    def _execute(self, context: ChainContext, _executor: Optional[RunnerExecutor] = None) -> None:
         result = self.agent.execute(AgentContext.from_chat_history(context.chat_history))
         maybe_message = result.try_best_message
         if maybe_message.is_some():

council/agents/agent_result.py

@@ -1,7 +1,7 @@
 from collections.abc import Sequence
 from typing import List, Optional
 
-from council.contexts import ScoredChatMessage, ChatMessage
+from council.contexts import ChatMessage, ScoredChatMessage
 from council.utils import Option
 
 
@@ -12,7 +12,7 @@ class AgentResult:
 
     _messages: List[ScoredChatMessage]
 
-    def __init__(self, messages: Optional[List[ScoredChatMessage]] = None):
+    def __init__(self, messages: Optional[List[ScoredChatMessage]] = None) -> None:
         """
         Initialize a new instance.
 

council/chains/chain_base.py

@@ -14,7 +14,7 @@ class ChainBase(Monitorable, abc.ABC):
     _description: str
     _instructions: bool
 
-    def __init__(self, name: str, description: str, support_instructions: bool = False):
+    def __init__(self, name: str, description: str, support_instructions: bool = False) -> None:
         super().__init__("chain")
         self._name = name
         self._description = description
@@ -57,15 +57,11 @@ def execute(self, context: ChainContext, executor: Optional[RunnerExecutor] = No
             self._execute(context, executor)
 
     @abc.abstractmethod
-    def _execute(
-        self,
-        context: ChainContext,
-        executor: Optional[RunnerExecutor] = None,
-    ) -> None:
+    def _execute(self, context: ChainContext, executor: Optional[RunnerExecutor] = None) -> None:
         pass
 
-    def __repr__(self):
+    def __repr__(self) -> str:
         return f"Chain({self.name}, {self.description})"
 
-    def __str__(self):
+    def __str__(self) -> str:
         return f"Chain {self.name}, description: {self.description}"

council/contexts/_agent_context_store.py

@@ -14,7 +14,7 @@ class AgentContextStore:
     Actual data storage used during the execution of an :class:`council.agents.Agent`
     """
 
-    def __init__(self, chat_history: ChatHistory):
+    def __init__(self, chat_history: ChatHistory) -> None:
         self._cancellation_token = CancellationToken()
         self._chat_history = chat_history
         self._iterations: List[AgentIterationContextStore] = []

council/contexts/_agent_iteration_context_store.py

@@ -12,12 +12,9 @@ class AgentIterationContextStore:
     Data storage for the execution of iteration
     """
 
-    _chains: Dict[str, MonitoredMessageList]
-    _evaluator: List[ScoredChatMessage]
-
-    def __init__(self):
-        self._chains = {}
-        self._evaluator = []
+    def __init__(self) -> None:
+        self._chains: Dict[str, MonitoredMessageList] = {}
+        self._evaluator: List[ScoredChatMessage] = []
 
     @property
     def chains(self) -> Mapping[str, MessageCollection]:

council/contexts/_budget.py

@@ -51,7 +51,7 @@ def unit(self) -> str:
     def kind(self) -> str:
         return self._kind
 
-    def __str__(self):
+    def __str__(self) -> str:
         return f"{self._kind} consumption: {self._value} {self.unit}"
 
     def add(self, value: float) -> Consumption:
@@ -156,7 +156,7 @@ def add_consumption(self, value: float, unit: str, kind: str):
         """
         self._add_consumption(Consumption(value=value, unit=unit, kind=kind))
 
-    def _add_consumption(self, consumption: Consumption):
+    def _add_consumption(self, consumption: Consumption) -> None:
         for limit in self._remaining:
             if limit.unit == consumption.unit and limit.kind == consumption.kind:
                 limit.subtract_value(consumption.value)
@@ -168,7 +168,7 @@ def add_consumptions(self, consumptions: Iterable[Consumption]) -> None:
         for consumption in consumptions:
             self._add_consumption(consumption)
 
-    def __repr__(self):
+    def __repr__(self) -> str:
         return f"Budget({self._duration})"
 
     @staticmethod
@@ -188,7 +188,7 @@ class InfiniteBudget(Budget):
     Helper class representing a budget with no duration and no limits
     """
 
-    def __init__(self):
+    def __init__(self) -> None:
         super().__init__(10000)
 
     def is_expired(self) -> bool:

council/contexts/_cancellation_token.py

@@ -6,7 +6,7 @@ class CancellationToken:
     A cancellation token which is initially not set.
     """
 
-    def __init__(self):
+    def __init__(self) -> None:
         self._cancelled = False
         self._lock = Lock()
 

council/contexts/_chain_context.py

@@ -30,7 +30,7 @@ def __init__(
         name: str,
         budget: Budget,
         messages: Optional[Iterable[ChatMessage]] = None,
-    ):
+    ) -> None:
         super().__init__(store, execution_context, budget)
         self._name = name
         self._current_messages = MessageList()

council/contexts/_chat_history.py

@@ -1,4 +1,5 @@
 from __future__ import annotations
+
 from ._message_list import MessageList
 
 

council/contexts/_chat_message.py

@@ -29,7 +29,7 @@ class ChatMessageKind(str, Enum):
     Represents a chat message generated by a specific skill or functionality within the chat system.
     """
 
-    def __str__(self):
+    def __str__(self) -> str:
         return f"{self.value}"
 
 
@@ -214,10 +214,10 @@ def is_from_source(self, source: str) -> bool:
         """
         return self._source == source
 
-    def __str__(self):
+    def __str__(self) -> str:
         return f"{self.kind}: {self.message}"
 
-    def to_string(self, max_length: int = 50):
+    def to_string(self, max_length: int = 50) -> str:
         message = self.message[:max_length] + "..." if len(self.message) > max_length else self.message
         return f"Message of kind {self.kind}: {message}"
 
@@ -239,27 +239,24 @@ class ScoredChatMessage:
         score: a score reflecting the quality of the message
     """
 
-    message: ChatMessage
-    score: float
-
-    def __init__(self, message: ChatMessage, score: float):
+    def __init__(self, message: ChatMessage, score: float) -> None:
         self.message = message
         self.score = score
 
-    def __gt__(self, other: "ScoredChatMessage"):
+    def __gt__(self, other: ScoredChatMessage) -> bool:
         return self.score > other.score
 
-    def __lt__(self, other: "ScoredChatMessage"):
+    def __lt__(self, other: ScoredChatMessage) -> bool:
         return self.score < other.score
 
-    def __ge__(self, other: "ScoredChatMessage"):
+    def __ge__(self, other: ScoredChatMessage) -> bool:
         return self.score >= other.score
 
-    def __le__(self, other: "ScoredChatMessage"):
+    def __le__(self, other: ScoredChatMessage) -> bool:
         return self.score <= other.score
 
-    def __str__(self):
+    def __str__(self) -> str:
         return f"{self.score}"
 
-    def __repr__(self):
+    def __repr__(self) -> str:
         return f"ScoredChatMessage({self.message}, {self.score})"

council/contexts/_composite_message_collection.py

@@ -11,7 +11,7 @@ class CompositeMessageCollection(MessageCollection):
 
     _collections: List[MessageCollection]
 
-    def __init__(self, collections: List[MessageCollection]):
+    def __init__(self, collections: List[MessageCollection]) -> None:
         self._collections = collections
 
     @property