fix: systematic ty type-checker cleanup across codebase#13481
Open
alt-glitch wants to merge 11 commits intomainfrom
Open
fix: systematic ty type-checker cleanup across codebase#13481alt-glitch wants to merge 11 commits intomainfrom
alt-glitch wants to merge 11 commits intomainfrom
Conversation
…level Sweep ~74 redundant local imports across 21 files where the same module was already imported at the top level. Also includes type fixes and lint cleanups on the same branch.
Full AST-based scan of all .py files to find every case where a module
or name is imported locally inside a function body but is already
available at module level. This is the second pass — the first commit
handled the known cases from the lint report; this one catches
everything else.
Files changed (19):
cli.py — 16 removals: time as _time/_t/_tmod (×10),
re / re as _re (×2), os as _os, sys,
partial os from combo import,
from model_tools import get_tool_definitions
gateway/run.py — 8 removals: MessageEvent as _ME /
MessageType as _MT (×3), os as _os2,
MessageEvent+MessageType (×2), Platform,
BasePlatformAdapter as _BaseAdapter
run_agent.py — 6 removals: get_hermes_home as _ghh,
partial (contextlib, os as _os),
cleanup_vm, cleanup_browser,
set_interrupt as _sif (×2),
partial get_toolset_for_tool
hermes_cli/main.py — 4 removals: get_hermes_home, time as _time,
logging as _log, shutil
hermes_cli/config.py — 1 removal: get_hermes_home as _ghome
hermes_cli/runtime_provider.py
— 1 removal: load_config as _load_bedrock_config
hermes_cli/setup.py — 2 removals: importlib.util (×2)
hermes_cli/nous_subscription.py
— 1 removal: from hermes_cli.config import load_config
hermes_cli/tools_config.py
— 1 removal: from hermes_cli.config import load_config, save_config
cron/scheduler.py — 3 removals: concurrent.futures, json as _json,
from hermes_cli.config import load_config
batch_runner.py — 1 removal: list_distributions as get_all_dists
(kept print_distribution_info, not at top level)
tools/send_message_tool.py
— 2 removals: import os (×2)
tools/skills_tool.py — 1 removal: logging as _logging
tools/browser_camofox.py
— 1 removal: from hermes_cli.config import load_config
tools/image_generation_tool.py
— 1 removal: import fal_client
environments/tool_context.py
— 1 removal: concurrent.futures
gateway/platforms/bluebubbles.py
— 1 removal: httpx as _httpx
gateway/platforms/whatsapp.py
— 1 removal: import asyncio
tui_gateway/server.py — 2 removals: from datetime import datetime,
import time
All alias references (_time, _t, _tmod, _re, _os, _os2, _json, _ghh,
_ghome, _sif, _ME, _MT, _BaseAdapter, _load_bedrock_config, _httpx,
_logging, _log, get_all_dists) updated to use the top-level names.
Widen return type annotations to match actual control flow, add unreachable assertions after retry loops ty cannot prove terminate, split ambiguous union returns (auth.py credential pool), and remove the AIOHTTP_AVAILABLE conditional-import guard from api_server.py.
Move batch_runner, trajectory_compressor, mini_swe_runner, and rl_cli from the project root into scripts/, update all imports, logger names, pyproject.toml, and downstream test references.
Replace hasattr() duck-typing with isinstance() checks for DiscordAdapter in gateway/run.py, add TypedDict for IMAGEGEN_BACKENDS in tools_config.py, properly type fal_client getattr'd callables in image_generation_tool.py, fix dict[str, object] → Callable annotation in approval.py, use isinstance(BaseModel) in web_tools.py, capture _message_handler to local in base.py, rename shadowed list_distributions parameter in batch_runner.py, and remove dead queue_message branch.
…guards Previously mutagen, aiohttp-socks, tiktoken, Pillow, psutil, datasets, neutts, and soundfile were used behind try/except ImportError with silent fallbacks, masking broken functionality at runtime. Declare each in its natural extra (messaging, cli, mcp, rl, new tts-local) so they get installed, and remove the guards so missing deps crash loudly.
Add TypedDicts for DEFAULT_CONFIG, CLI state dicts (_ModelPickerState, _ApprovalState, _ClarifyState), and OPTIONAL_ENV_VARS so ty can resolve nested dict subscripts. Guard Optional returns before subscripting (toolsets, cron/scheduler, delegate_tool), coerce str|None to str before slicing (gateway/run, run_agent), split ternary for isinstance narrowing (wecom), and suppress discord interaction.data access with ty: ignore.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
15 P1 ship-stopper runtime bugs from the ty triage plus the cross-bucket cleanup in run_agent.py. Net: -138 ty diagnostics (1953 -> 1815). Major wins on not-subscriptable (-34), unresolved-attribute (-29), invalid-argument-type (-26), invalid-type-form (-20), unsupported-operator (-18), invalid-key (-9). Missing refs (structural): - tools/rl_training_tool.py: RunState dataclass gains api_log_file, trainer_log_file, env_log_file fields; stop-run was closing undeclared handles. - agent/credential_pool.py: remove_entry(entry_id) added, symmetric with add_entry; used by hermes_cli/web_server.py OAuth dashboard cleanup. - hermes_cli/config.py: _CamofoxConfig TypedDict defined (was referenced by _BrowserConfig but never declared). - hermes_cli/gateway.py: _setup_wecom_callback() added, mirroring _setup_wecom(). - tui_gateway/server.py: skills_hub imports corrected from hermes_cli.skills_hub -> tools.skills_hub. Typo / deprecation: - tools/transcription_tools.py: os.sys.modules -> sys.modules. - gateway/platforms/bluebubbles.py: datetime.utcnow() -> datetime.now(timezone.utc). None-guards: - gateway/platforms/telegram.py:~2798 - msg.sticker None guard. - gateway/platforms/discord.py:3602/3637 - interaction.data None + SelectMenu narrowing; :3009 - thread_id None before `in`; :1893 - guild.member_count None. - gateway/platforms/matrix.py:2174/2185 - walrus-narrow re.search().group(). - agent/display.py:732 - start_time None before elapsed subtraction. - gateway/run.py:10334 - assert _agent_timeout is not None before `// 60`. Platform override signature match: - gateway/platforms/email.py: send_image accepts metadata kwarg; send_document accepts **kwargs (matches base class). run_agent.py annotation pass: - callable/any -> Callable/Any in annotation position (15 sites in run_agent.py + 5 in cli.py, toolset_distributions.py, tools/delegate_tool.py, hermes_cli/dingtalk_auth.py, tui_gateway/server.py). - conversation_history param widened to list[dict[str, Any]] | None. - OMIT_TEMPERATURE sentinel guarded from leaking into call_llm(temperature): kwargs-dict pattern at run_agent.py:7337 + scripts/trajectory_compressor.py:618/688. - build_anthropic_client(timeout) widened to Optional[float]. Tests: - tests/agent/test_credential_pool.py: remove_entry (id match, unknown-id, priority renumbering). - tests/hermes_cli/test_config_shapes.py: _CamofoxConfig shape + nesting. - tests/tools/test_rl_training_tool.py: RunState log_file fields.
# Conflicts: # gateway/platforms/base.py # gateway/platforms/qqbot/adapter.py # gateway/platforms/slack.py # hermes_cli/main.py # scripts/batch_runner.py # tools/skills_tool.py # uv.lock
Follow-up to 15ac253 per /simplify review: - gateway/platforms/discord.py:3638 - move self.resolved = True *after* the `if interaction.data is None: return` guard. Previously the view was marked resolved before the None-guard, so a None data payload silently rejected the user's next click. - agent/display.py:732 - replace `if self.start_time is None: continue` with `assert self.start_time is not None`. start() sets start_time before the animate thread starts, so the None branch was dead; the `continue` form would have busy-looped (skipping the 0.12s sleep). - tests/hermes_cli/test_config_shapes.py - drop __total__ dunder restatement test (it just echoes the class declaration); trim commit narration from module docstring. - tests/agent/test_credential_pool.py, tests/tools/test_rl_training_tool.py - drop "added in commit ..." banners (narrates the change per CLAUDE.md).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Systematic pass through the codebase fixing diagnostics reported by ty (Astral's type checker), organized by error category. Eliminates hundreds of type errors across ~82 files, cleans up redundant imports, declares missing optional dependencies, removes silent failure patterns, and — most recently — lands a focused P1 hotfix batch for actual runtime-crash bugs surfaced by triaging the remaining diagnostic buckets.
What changed
Type fixes by category
unresolved-reference— AddedTYPE_CHECKINGimports for forward references (gemini adapters, copilot client, etc.); defined missing_CamofoxConfigTypedDict referenced by_BrowserConfig.unresolved-import— Correctedskills_hubimports intui_gateway/server.py(was pointing at the wrapper module, now targets the source). Added_setup_wecom_callback()function thathermes_cli/setup.pywas importing.unresolved-attribute—RunStatedataclass gained missingapi_log_file/trainer_log_file/env_log_filefields (stop-run flow was closing undeclared handles); addedCredentialPool.remove_entry(entry_id)method (symmetric withadd_entry, called during OAuth dashboard cleanup);os.sys.modules→sys.modulestypo fix; None-guards beforemsg.sticker.*andre.search(...).group().invalid-return-type— Widened return annotations to match actual control flow, added unreachable assertions after retry loops ty can't prove terminate, splitread_credential_poolinto two functions to eliminate ambiguous union returns.call-non-callable— Fixed parameter shadowing imported callable (batch_runner.py), narrowed Optional before calling, typed callback dicts properly.not-subscriptable/invalid-key— AddedTypedDictforDEFAULT_CONFIG(and nested configs), CLI state dicts (_ModelPickerState,_ApprovalState,_ClarifyState), coercedstr | Nonebefore slicing. None-guards + variant narrowing beforeinteraction.data["values"]access in Discord select handlers.unsupported-operator— None-guards before arithmetic / membership / comparison onOptionalvalues: Discordthread_id+guild.member_count, display spinnerstart_time, gateway_agent_timeout.invalid-method-override— Email platform'ssend_imageacceptsmetadatakwarg to match base class;send_documentaccepts**kwargs(was droppingmetadataentirely →TypeErrorfor callers passingmetadata=...).invalid-type-form— Replaced builtincallable/anyin annotation position withCallable[..., Any]/Anyacross 20 sites inrun_agent.py,cli.py,tools/delegate_tool.py,toolset_distributions.py,hermes_cli/dingtalk_auth.py,tui_gateway/server.py.invalid-assignment—run_conversation.conversation_historyparam widened tolist[dict[str, Any]] | Noneso the 5 None-reset sites type-check.invalid-argument-type—build_anthropic_client(timeout)widened toOptional[float](body already handled None);OMIT_TEMPERATUREsentinel no longer leaks intocall_llm(temperature: float)— conditional-kwargs-dict pattern inrun_agent.pyandscripts/trajectory_compressor.py.deprecated—datetime.utcnow()→datetime.now(timezone.utc)ingateway/platforms/bluebubbles.py(Python 3.12+ deprecation).Dependency hygiene
pyproject.tomlextras:mutagenandaiohttp-socks→[messaging],tiktokenandPillow→[cli],psutil→[mcp],datasets→[rl], new[tts-local]extra forneutts+soundfile.try/except ImportErrorwith silent fallbacks (returning empty dicts, skipping features, estimating values from file sizes). Now they crash loudly if the correct extra isn't installed, instead of silently degrading.Code cleanup
batch_runner.py,mini_swe_runner.py,rl_cli.py,trajectory_compressor.py) toscripts/directory with updated imports and test references.AIOHTTP_AVAILABLEguard pattern fromapi_server.py— aiohttp is now a direct import since it's a required dep of the messaging extra.P1 runtime-bug hotfix batch
Landed as
15ac253b+67bc4410(simplify follow-up). Triaged the remaining ty diagnostics (1953 across 16 rules) into genuine runtime crashes vs false positives vs annotation gaps. Fixed the 15 P1 ship-stoppers that would fireAttributeError/TypeErroron the happy path:CredentialPool.remove_entry,RunStatelog-file fields,_CamofoxConfigTypedDict,_setup_wecom_callback(),tui_gateway/server.pyskills_hub path.re.search),agent/display.py(start_time),gateway/run.py(_agent_timeout).send_image/send_document.build_anthropic_client(timeout: Optional[float]),conversation_history | None,OMIT_TEMPERATUREsentinel guard.datetime.utcnow().Net P1-batch impact: -138 ty diagnostics (1953 → 1815), with wins on
not-subscriptable(-34),unresolved-attribute(-29),invalid-argument-type(-26),invalid-type-form(-20),unsupported-operator(-18),invalid-key(-9).Tests
tests/agent/test_credential_pool.py—CredentialPool.remove_entrycoverage (id match, unknown-id return, priority renumbering).tests/hermes_cli/test_config_shapes.py—_CamofoxConfig/_BrowserConfigTypedDict shape smoke tests.tests/tools/test_rl_training_tool.py—RunStatelog-file field defaults.Test plan
ty check --python-version 3.13passes for all fixed categoriesuv sync --extra allinstalls newly declared depspytest tests/)pytest tests/agent/test_credential_pool.py tests/hermes_cli/test_config_shapes.py 'tests/tools/test_rl_training_tool.py::TestRunStateLogFileFields')