hermes-agent/notes.txt

# ty type-checking notes — antipatterns & refactors spotted

working through the codebase root-to-tip with astral.sh `ty`.
logging antipatterns, clean refactors, and observations as we go.

---

## tools/registry.py (dependency root, 810 lines)

### invalid-parameter-default (FIXED)
three params in `ToolEntry.register()` were annotated as bare `Callable` / `list`
but defaulted to `None`. this is the #1 pattern ty flags across the whole codebase
(~423 occurrences). the fix is always the same: add `| None` to the annotation.

```python
# BEFORE
check_fn: Callable = None,
requires_env: list = None,
dynamic_schema_overrides: Callable = None,

# AFTER
check_fn: Callable | None = None,
requires_env: list | None = None,
dynamic_schema_overrides: Callable | None = None,
```

this is the single highest-leverage fix across the codebase. each one of these
cascades: when ty sees `None` as a possible value, every downstream `d["key"]`,
`d.get(...)`, `d.pop()` on that variable becomes an `unresolved-attribute` or
`not-subscriptable` or `invalid-argument-type` error. fixing the parameter
default upstream makes all those downstream errors vanish.

### antipattern: ToolEntry.__init__ has zero annotations

```python
def __init__(self, name, toolset, schema, handler, check_fn,
             requires_env, is_async, description, emoji,
             max_result_size_chars=None, dynamic_schema_overrides=None):
```

this is a core data class (every tool in the system passes through it) but has
no type annotations on any parameter. `__slots__` is used so the shape is
well-defined — adding annotations would be straightforward and high-value.
ty can't infer much here because everything comes through as `Unknown`.

not fixed yet — would benefit from a proper pass:
```python
def __init__(
    self,
    name: str,
    toolset: str,
    schema: dict,
    handler: Callable,
    check_fn: Callable | None,
    requires_env: list[str],
    is_async: bool,
    description: str,
    emoji: str,
    max_result_size_chars: int | float | None = None,
    dynamic_schema_overrides: Callable | None = None,
):
```

### antipattern: tool_error / tool_result helpers lack annotations

```python
def tool_error(message, **extra) -> str:  # message: str, **extra: Any
def tool_result(data=None, **kwargs) -> str:  # data: dict | None, **kwargs: Any
```

these are the canonical serialization helpers used by hundreds of tool handlers.
low-effort to annotate, high-value since they're the return path for everything.

### clean pattern: _check_fn_cached TTL + grace window

this is well-done code. the TTL cache with transient-failure suppression is
a correct implementation of a flaky-external-check absorption pattern. the
docstrings explain WHY (issue #21658 / #5304 — flaky docker probes stripping
tools mid-session). nothing to change here, just noting it as a positive
example of defensive caching done right.

### clean pattern: _snapshot_state() for thread safety

using `_lock` + snapshot copies for reads is the right pattern for a registry
that can be mutated by MCP dynamic refresh while other threads read. the
generation counter for cache invalidation is also clean.

### observation: `from typing import` vs PEP 604 `X | None`

the file imports `Optional`, `Callable`, `Dict`, `List`, `Set` from `typing`
but also uses `int | float | None` (PEP 604) in the same signatures. the
codebase targets python >=3.11 so PEP 604 is always available. there's a
mix of `Optional[X]` and `X | None` styles across files — not a bug, but
worth standardizing on `X | None` (PEP 604) as we type-sweep since it's
shorter and the modern idiom.

---

## agent/agent_init.py + run_agent.py (AIAgent constructor, 60+ params)

### invalid-parameter-default (FIXED — 31 + 54 params)
same pattern as registry.py but at massive scale. the AIAgent.__init__ in
`agent_init.py` and the class `AIAgent.__init__` in `run_agent.py` each take
~60 params, nearly all defaulting to None but annotated as bare `str`, `list`,
`dict`, `int`, etc.

### antipattern: lowercase `callable` used as a type annotation

**this is a real runtime bug, not just a type error.** 14 callback params in
both `agent_init.py` and `run_agent.py` were annotated as `callable` (the
builtin *function*) instead of `Callable` (from `typing`).

```python
# BEFORE — runtime crash if you try `X | None`
tool_progress_callback: callable = None,  # callable is the builtin function

# AFTER
tool_progress_callback: Callable | None = None,  # Callable is the type
```

when the automated sweep script added `| None` to these, it produced
`callable | None` which crashes at *import time* with:
```
TypeError: unsupported operand type(s) for |: 'builtin_function_or_method' and 'NoneType'
```

this was lurking silently as long as nobody tried to make the annotation
nullable — the `callable` builtin is truthy so `callable = None` didn't
crash, it just stored a nonsensical annotation. the fix is `Callable` (capital C).

also spotted in `agent/transports/chat_completions.py` in docstring-like
comments (lines 290, 314-315) — not executable code but worth standardizing.

### antipattern: string forward-ref with `| None` doesn't work

```python
# WRONG — crashes at class-def time
iteration_budget: "IterationBudget" | None = None,
# TypeError: unsupported operand type(s) for |: 'str' and 'NoneType'

# RIGHT — wrap the whole thing in Optional
iteration_budget: Optional["IterationBudget"] = None,
```

when a type is a forward reference (string-quoted because it's not yet defined),
you CANNOT use PEP 604 `| None` on it directly — the `|` operator tries to
OR a `str` with `NoneType` and crashes. must use `Optional["ForwardRef"]`.

this affected both `agent_init.py` and `run_agent.py`.

---

## hermes_state.py (49 params fixed)
same `X = None` → `X | None = None` sweep. this is the SQLite session store
module — heavily imported by cli.py, run_agent.py, gateway/, etc.

## batch_runner.py (24 params fixed)
parallel batch processing entry point.

## cli.py (18 params fixed)
the HermesCLI class constructor + helper methods.

## tools/*.py (20+ params fixed across file_tools, file_operations,
skills_tool, skills_hub, terminal_tool, browser_tool, memory_tool,
delegate_tool, process_registry, skill_manager_tool, session_search_tool)

---

## summary of the invalid-parameter-default sweep

total params fixed: ~250 across ~25 core files
the pattern is always identical: `param: Type = None` → `param: Type | None = None`

two runtime-breaking gotchas found during the sweep:
1. lowercase `callable` (builtin function) → must be `Callable` (typing)
2. string forward-refs can't use `| None` → must use `Optional["Ref"]`

---

## run_agent.py — unresolved-attribute analysis (190 errors)

### category 1: "Self@<method>" — init_agent() cross-module init (186/190)

these are ALL the same root cause: `AIAgent.__init__` in `run_agent.py` is a
thin forwarder that calls `init_agent(self, ...)` from `agent/agent_init.py`.
that function sets `self.model`, `self.provider`, `self.session_id`, etc. on
the instance — but ty can't see across the module boundary that these
attributes are being set. so every method that accesses `self.model`,
`self.provider`, etc. gets flagged.

**these are NOT bugs.** the attributes are correctly set at runtime by
`init_agent()`. this is a ty limitation — it can't track attribute assignments
made in a function defined in a different module that receives `self` as a
regular parameter (not as a method on the class).

example:
```python
# run_agent.py
class AIAgent:
    def __init__(self, model: str = "", ...):
        from agent.agent_init import init_agent
        init_agent(self, model=model, ...)  # sets self.model, self.provider, etc.

    def _resolved_api_call_timeout(self):
        return self.provider  # ty: "Self@_resolved_api_call_timeout has no attribute provider"
```

**fix approach:** this is the single biggest cluster of ty errors in the
codebase (186 in run_agent.py alone, plus similar in other files). the cleanest
fix would be to declare the attributes as class-level annotations in the
`AIAgent` class body so ty knows they exist:

```python
class AIAgent:
    # Instance attributes — set by init_agent() in agent/agent_init.py
    model: str
    provider: str | None
    session_id: str | None
    # ... etc
```

this is a one-time declaration that would clear ~186 errors instantly. it's
also good documentation — currently there's no single place that lists all
instance attributes; they're scattered across the 60-param init_agent() body.

### category 2: object.function — duck-typed tool_calls access (2 errors)

line 1968: `tc.function.name` and `tc.function.arguments` on objects typed as
`object`. this is because `msg.tool_calls` is checked via `hasattr` +
`isinstance(list)` but the list elements are `object` to ty.

```python
if hasattr(msg, "tool_calls") and isinstance(msg.tool_calls, list) and msg.tool_calls:
    tool_calls_data = [
        {"name": tc.function.name, "arguments": tc.function.arguments}
        for tc in msg.tool_calls
    ]
```

**not a bug** — this is duck-typed access to OpenAI SDK objects. the `hasattr`
guard makes it safe at runtime. ty just can't infer the element type of a
list accessed via `hasattr`. could be fixed with a `type: ignore` or by
narrowing `msg` to a proper type.

### category 3: ~AlwaysFalsy.on_session_end — duck-typed context_compressor (2 errors)

lines 3416, 3441: `self.context_compressor.on_session_end(...)` after a
`hasattr(self, "context_compressor") and self.context_compressor` guard.

```python
if hasattr(self, "context_compressor") and self.context_compressor:
    self.context_compressor.on_session_end(self.session_id or "", messages or [])
```

ty narrows `self.context_compressor` to `~AlwaysFalsy` (the truthy branch of
the `and`) but doesn't know it has `on_session_end`. **not a bug** — the
`hasattr` guard makes this safe. again a type-system limitation with
duck-typed access.

### verdict for run_agent.py: 0 real bugs, 190 type-system limitations

all 190 are ty being unable to track either cross-module attribute init
(186) or duck-typed hasattr-guarded access (4). no logic bugs found.