Code style: agent/ without comments, its tools as methods

CLAUDE.md

@@ -671,9 +671,10 @@ functions); a test patches a name in the module that looks it up (`harness._MIN_
 - **Toolkits** (`toolkits/`): what the agent can read, one `Toolkit` subclass per AWS service (`base.py` says what one
   gives). `LambdaToolkit` (`lambda_tools.py`) has seven tools (`find_functions`, `function_health`, `function_errors`,
   `function_logs`, `function_performance`, `function_details`, `function_code`), building blocks over `LambdaAnalyzer`
-  methods, not one per question: plain functions closed over the toolkit in its `tools()`. Their type hints and
+  methods, not one per question: methods of the toolkit that its `tools()` lists. Their type hints and
   docstrings are the schema and description the model sees (LangChain reads them), so write a docstring for the
-  model: what it returns, how to pass times (ISO UTC or '24h'), when to use it. Each returns a compact dict from the
+  model: what it returns, how to pass times (ISO UTC or '24h'), when to use it, and nothing about how it works
+  inside (that goes in a helper's docstring). Each returns a compact dict from the
   module's pure `summarize_*` (what the model reads: counts, causes, a few lines, secrets hidden by `results.py`'s
   `hide_secrets`) with its coverage (`window`, `regions`, `cut_short`, `covered_from`), plus `_summary` (the report's
   one line) and `_next` (View calls, made by `Toolkit._view_call`), which the runtime takes out before the model sees

ruff.toml

@@ -29,13 +29,6 @@ select = ["E4", "E7", "E9", "F", "B006", "BLE001", "ERA001", "C901", "PLR0912",
 ".claude/skills/demo/shots.py" = ["C901", "PLR0912", "PLR0913", "PLR0915"]
 ".claude/skills/release/release.py" = ["BLE001", "C901", "PLR0912", "PLR0915"]
 "src/aws_analyzer/_bedrock.py" = ["C901", "PLR0912", "PLR0915"]
-"src/aws_analyzer/agent/analyzer.py" = ["PLR0913"]
-"src/aws_analyzer/agent/harness.py" = ["BLE001", "C901", "PLR0913"]
-"src/aws_analyzer/agent/llm.py" = ["PLR0913"]
-"src/aws_analyzer/agent/runtime.py" = ["BLE001", "C901"]
-"src/aws_analyzer/agent/toolkits/base.py" = ["ERA001"]
-"src/aws_analyzer/agent/toolkits/lambda_tools.py" = ["C901", "PLR0913", "PLR0915"]
-"src/aws_analyzer/agent/toolkits/s3_tools.py" = ["C901", "PLR0912", "PLR0913", "PLR0915"]
 "src/aws_analyzer/bedrock_chat.py" = ["BLE001", "C901", "PLR0912", "PLR0913", "PLR0915"]
 "src/aws_analyzer/bedrock_kb.py" = ["C901", "PLR0912", "PLR0913", "PLR0915"]
 "src/aws_analyzer/dynamodb.py" = ["C901", "PLR0913"]

src/aws_analyzer/agent/analyzer.py

@@ -27,6 +27,27 @@
     from ..s3 import S3Analyzer
 
 
+_THREAD_ID_LENGTH = 12
+_WORKERS = 8
+
+
+def _new_thread() -> str:
+    """
+    A new conversation's ID.
+    """
+    return uuid.uuid4().hex[:_THREAD_ID_LENGTH]
+
+
+def _checked_zone(timezone: str | tzinfo | None) -> str | tzinfo | None:
+    """
+    timezone= as it's kept, after checking it names a zone, so a misspelled one is an error now rather than at the
+    first question. None is the zone reports show times in (set_timezone()).
+    """
+    if timezone is not None:
+        _zone_from(timezone)
+    return timezone
+
+
 class AgentAnalyzer(_BaseAnalyzer):
     """
     Answers questions about your AWS account with an agent: a model (an Azure OpenAI deployment) calling read-only
@@ -47,11 +68,14 @@ class AgentAnalyzer(_BaseAnalyzer):
     AWS_ANALYZER_TZ's), the zone every report shows times in; `timezone` ('Asia/Kolkata', 'kolkata', '+05:30') sets
     one for the agent alone. `notes` are facts about the account the agent is told ("orders-etl runs at 02:00
     IST"). `budget` caps each question (Budget, or a dict of its fields).
+
+    It has no list prices: Azure OpenAI's depend on the deployment (see model_prices=). `thread` is the conversation
+    follow-up questions continue, and `history` every question asked, in order.
     """
 
-    _LIST_PRICES: dict[str, Any] = {}  # no list prices: Azure OpenAI's depend on the deployment; see model_prices=
+    _LIST_PRICES: dict[str, Any] = {}
 
-    def __init__(
+    def __init__(  # noqa: PLR0913
         self,
         session: Any = None,
         *,
@@ -77,28 +101,26 @@ def __init__(
     ):
         super().__init__(session, region=region, profile=profile, prices=model_prices)
         self.deployment, self.endpoint, self.api_version = azure_settings(deployment, endpoint, api_version)
-        self._api_key = api_key  # never shown; None reads AZURE_OPENAI_API_KEY when the model is made
+        self._api_key = api_key
         self._token_provider = azure_ad_token_provider
         self.chat_model = chat_model
         self.max_tokens = max_tokens
-        if timezone is not None:
-            _zone_from(timezone)  # a misspelled time zone is an error now, not at the first question
-        self.timezone = timezone  # None: the zone reports show times in (set_timezone())
+        self.timezone = _checked_zone(timezone)
         self.regions = regions
         self.notes = notes
         self.budget = Budget.of(budget)
         self._clients: dict[Any, Any] = {}
         self._lock = threading.Lock()
-        self.max_workers = 8
+        self.max_workers = _WORKERS
         self._guard = _Guard()
         self.toolkits: list[Toolkit] = [toolkit.bind(self) for toolkit in make_toolkits(
             toolkits, {"lambda": {"analyzer": lambda_core, "clients": lambda_clients, "prices": prices},
                        "s3": {"analyzer": s3_core}})]
         self._graph: Any = None
         self._run: _Run | None = None
         self._tools: list[Callable[..., dict[str, Any]]] | None = None
-        self.thread = uuid.uuid4().hex[:12]  # the conversation follow-up questions continue
-        self.history: list[AgentAnswer] = []  # every question asked, in order
+        self.thread = _new_thread()
+        self.history: list[AgentAnswer] = []
 
     def toolkit(self, name: str) -> Toolkit:
         """
@@ -144,8 +166,6 @@ def reads(self) -> dict[str, tuple[str, ...]]:
         """
         return {name: reads for toolkit in self.toolkits for name, reads in toolkit.reads.items()}
 
-    # ------------------------------------------------------------------ the model and the loop
-
     @property
     def model(self) -> str:
         """
@@ -172,10 +192,10 @@ def instructions(self) -> str:
     def _harness(self) -> Any:
         """
         The deepagents agent: the model, the tools, the instructions and a memory of the conversation. Made once;
-        new_conversation() starts a new thread in it.
+        new_conversation() starts a new thread in it. Before anything else is made, check_ready() says what's missing.
         """
         if self._graph is None:
-            harness.check_ready()  # says what's missing before anything else is made
+            harness.check_ready()
             prompt = self.instructions()
             self._graph = harness.build_graph(self._chat(), self.tools(), prompt)
         return self._graph
@@ -184,11 +204,9 @@ def new_conversation(self) -> str:
         """
         Starts a new conversation: the next question won't see the earlier ones. Returns its ID.
         """
-        self.thread = uuid.uuid4().hex[:12]
+        self.thread = _new_thread()
         return self.thread
 
-    # ------------------------------------------------------------------ asking
-
     def ask(
         self,
         question: str,
@@ -234,8 +252,6 @@ def ask(
         self.history.append(answer)
         return answer
 
-    # ------------------------------------------------------------------ the tools
-
     def tools(self) -> list[Callable[..., dict[str, Any]]]:
         """
         The tools the agent can call, from every toolkit: plain Python functions (type hints and docstrings say how

src/aws_analyzer/agent/answers.py

@@ -20,17 +20,24 @@
 class ToolCall:
     """
     One call a tool made for a question: what it was asked, what it covered and found, or why it didn't run.
+
+        summary   one line: what it found
+        coverage  its window, regions, whether it was cut short, ...
+        error     why it failed: 'AccessDeniedException; needs logs:FilterLogEvents'
+        refused   why it wasn't run: 'tool_calls', 'time', 'cost', 'log_window' or 'stopped'
+        next      View calls that show more, and what they show
+        first     the tool's first parameter when it has no default, which the call is written without the name of
     """
 
     name: str
     args: dict[str, Any] = field(default_factory=dict)
     seconds: float = 0.0
-    summary: str = ""  # one line: what it found
-    coverage: dict[str, Any] = field(default_factory=dict)  # window, regions, cut_short, ...
-    error: str | None = None  # why it failed: 'AccessDeniedException; needs logs:FilterLogEvents'
-    refused: str | None = None  # why it wasn't run: 'tool_calls', 'time', 'cost', 'log_window', 'stopped'
-    next: list[tuple[str, str]] = field(default_factory=list)  # View calls that show more, and what they show
-    first: str | None = "function"  # the tool's first parameter when it has no default: written without its name
+    summary: str = ""
+    coverage: dict[str, Any] = field(default_factory=dict)
+    error: str | None = None
+    refused: str | None = None
+    next: list[tuple[str, str]] = field(default_factory=list)
+    first: str | None = "function"
 
     @property
     def ok(self) -> bool:
@@ -50,26 +57,38 @@ def call(self) -> str:
 class AgentAnswer:
     """
     One question and its answer, with every tool call it made, the AWS reads, the tokens and what it cost.
+
+        text          the answer, in markdown
+        aws_calls     the AWS reads it made: 'logs:FilterLogEvents' -> 12
+        blocked       AWS calls the read-only guard refused
+        metrics_read  CloudWatch metrics requested (GetMetricData bills per 1,000)
+        aws_cost      USD for those metrics ($0.01 per 1,000 at list price) and S3's requests; the rest are free
+        model         the deployment or model that answered
+        cost          estimated USD for the model's tokens; None when its prices aren't known
+        stopped       None when it finished, else 'tool_calls', 'time', 'cost', 'steps', 'stopped' or 'error'
+        error         what went wrong with the model or the agent loop, when something did
+        thread        the conversation it belongs to
+        exception     the error itself, to raise when debugging
     """
 
     question: str
-    text: str = ""  # the answer, in markdown
+    text: str = ""
     calls: list[ToolCall] = field(default_factory=list)
-    aws_calls: dict[str, int] = field(default_factory=dict)  # 'logs:FilterLogEvents' -> 12
-    blocked: list[str] = field(default_factory=list)  # AWS calls the read-only guard refused
-    metrics_read: int = 0  # CloudWatch metrics requested (GetMetricData bills per 1,000)
-    aws_cost: float = 0.0  # USD for those metrics ($0.01 per 1,000 at list price) and S3's requests; the rest are free
-    model: str = ""  # the deployment or model that answered
+    aws_calls: dict[str, int] = field(default_factory=dict)
+    blocked: list[str] = field(default_factory=list)
+    metrics_read: int = 0
+    aws_cost: float = 0.0
+    model: str = ""
     model_calls: int = 0
     input_tokens: int = 0
     output_tokens: int = 0
-    cost: float | None = None  # estimated USD for the model's tokens; None when its prices aren't known
+    cost: float | None = None
     seconds: float = 0.0
-    stopped: str | None = None  # None (it finished) | 'tool_calls' | 'time' | 'cost' | 'steps' | 'stopped' | 'error'
-    error: str | None = None  # what went wrong with the model or the agent loop, when something did
-    thread: str = ""  # the conversation it belongs to
+    stopped: str | None = None
+    error: str | None = None
+    thread: str = ""
     asked: datetime | None = None
-    exception: BaseException | None = field(default=None, repr=False)  # the error itself, to raise when debugging
+    exception: BaseException | None = field(default=None, repr=False)
 
 
 def answer_findings(answer: AgentAnswer, budget: Budget | None = None) -> list[tuple[str, str]]:
@@ -130,11 +149,11 @@ def answer_findings(answer: AgentAnswer, budget: Budget | None = None) -> list[t
 def answer_next(answer: AgentAnswer, n: int = 3) -> list[tuple[str, str]]:
     """
     The commands worth running next: the View calls behind the tools the answer used, the most specific first,
-    without repeats.
+    without repeats: one per command and function (errors('etl') once, with the latest window).
     """
     seen: dict[str, tuple[str, str]] = {}
     for call in reversed(answer.calls):
-        for command, what in call.next:  # one per command and function: errors('etl') once, the latest window
+        for command, what in call.next:
             key = re.match(r"(?:\w+\(\)\.)?(\w+)\(('[^']*')?", command)
             seen.setdefault(key.group(0) if key else command, (command, what))
     return list(seen.values())[:n]

src/aws_analyzer/agent/budget.py

@@ -1,6 +1,7 @@
 """
 How far one question may go (Budget), and the state of the question being answered (_Run): its tool calls, what
-the model has cost so far, and the stop button. check_budget() is what every tool asks before it runs.
+the model has cost so far, and the stop button. check_budget() is what every tool asks before it runs, and
+_REFUSED is what the table of tool calls says about a call that wasn't run.
 """
 
 from __future__ import annotations
@@ -17,19 +18,29 @@
     from .answers import ToolCall
 
 
+_METRICS_PER_PRICE = 1000
+
+
 @dataclass
 class Budget:
     """
     How far one question may go. A tool that would go past a limit isn't run: it tells the model to answer with
     what it has, and the answer says what stopped it.
+
+        tool_calls  tool calls per question
+        seconds     wall-clock time per question, model included
+        usd         estimated model cost per question (only when the model's prices are known)
+        steps       LangGraph's recursion limit: model calls, tool calls and the harness's own steps together
+        log_days    the longest window a log read (errors, logs, performance) may cover
+        s3_keys     the most keys one S3 listing may read (find_files, folder_summary, s3_triggers)
     """
 
-    tool_calls: int = 25  # tool calls per question
-    seconds: float = 300.0  # wall-clock time per question, model included
-    usd: float | None = 1.00  # estimated model cost per question (only when the model's prices are known)
-    steps: int = 80  # LangGraph's recursion limit: model calls, tool calls and the harness's own steps together
-    log_days: float = 3.0  # the longest window a log read (errors, logs, performance) may cover
-    s3_keys: int = 100_000  # the most keys one S3 listing may read (find_files, folder_summary, s3_triggers)
+    tool_calls: int = 25
+    seconds: float = 300.0
+    usd: float | None = 1.00
+    steps: int = 80
+    log_days: float = 3.0
+    s3_keys: int = 100_000
 
     @classmethod
     def of(cls, value: Budget | dict[str, Any] | None) -> Budget:
@@ -56,15 +67,19 @@ class _Run:
     """
     One question's state while it's being answered: what the tools did and what's left of the budget. The tools
     of one turn run side by side, so everything they change is changed under `lock`.
+
+        cost          the model's estimated cost so far (USD)
+        metrics_read  CloudWatch metrics the tools requested (GetMetricData bills per 1,000)
+        aws_cost      USD for those metrics; every other read the tools make is free
     """
 
     budget: Budget
     started: float = field(default_factory=time.monotonic)
     stop: threading.Event = field(default_factory=threading.Event)
     calls: list[ToolCall] = field(default_factory=list)
-    cost: float = 0.0  # the model's estimated cost so far (USD)
-    metrics_read: int = 0  # CloudWatch metrics the tools requested (GetMetricData bills per 1,000)
-    aws_cost: float = 0.0  # USD for those metrics; every other read the tools make is free
+    cost: float = 0.0
+    metrics_read: int = 0
+    aws_cost: float = 0.0
     lock: threading.Lock = field(default_factory=threading.Lock)
 
     @property
@@ -77,7 +92,7 @@ def read_metrics(self, count: int, price: float) -> None:
         """
         with self.lock:
             self.metrics_read += count
-            self.aws_cost += count / 1000 * price
+            self.aws_cost += count / _METRICS_PER_PRICE * price
 
 
 class _Refused(Exception):
@@ -90,7 +105,7 @@ def __init__(self, reason: str, message: str, **extra: Any):
         self.reason, self.message, self.extra = reason, message, extra
 
 
-_REFUSED = {  # what the table of tool calls says about a call that wasn't run
+_REFUSED = {
     "log_window": "not run: the window is longer than the log limit",
     "tool_calls": "not run: the budget of tool calls was used up",
     "time": "not run: the time budget was used up",