fix(providers): remove type: ignore suppressions from registry.py and resolve underlying type errors #3459

Merged
freemo merged 1 commit from fix/providers-registry-type-ignore-suppressions into master 2026-04-05 18:20:04 +00:00
Owner

Summary

Removes all 11 # type: ignore suppressions from src/cleveragents/providers/registry.py, which violated the CONTRIBUTING.md code standard that forbids # type: ignore usage.

Changes

New type stubs (typings/ directory):

  • typings/langchain_groq/__init__.pyi — stub for ChatGroq class
  • typings/langchain_together/__init__.pyi — stub for ChatTogether class
  • typings/langchain_cohere/__init__.pyi — stub for ChatCohere class

These follow the existing pattern established for langchain_anthropic, langchain_openai, and langchain_google_genai stubs already in the typings/ directory.

src/cleveragents/providers/registry.py:

  • Removed # type: ignore[import-untyped] from langchain_groq, langchain_together, langchain_cohere imports (now resolved via type stubs)
  • Removed # type: ignore[arg-type] from all LLM constructor call sites (unnecessary given pyrightconfig.json reportUnknown* settings)
  • Fixed # type: ignore[arg-type] on llm_factory closure by capturing the narrowed ProviderType in a local variable resolved_provider_type: ProviderType, allowing Pyright to verify the type without suppression

Verification

  • nox -e typecheck0 errors, 3 warnings (warnings are reportMissingModuleSource for optional packages not installed in dev env — not errors)
  • nox -e lintAll checks passed

Closes #3421


Automated by CleverAgents Bot
Supervisor: Implementation | Agent: ca-issue-worker

## Summary Removes all 11 `# type: ignore` suppressions from `src/cleveragents/providers/registry.py`, which violated the CONTRIBUTING.md code standard that forbids `# type: ignore` usage. ### Changes **New type stubs** (`typings/` directory): - `typings/langchain_groq/__init__.pyi` — stub for `ChatGroq` class - `typings/langchain_together/__init__.pyi` — stub for `ChatTogether` class - `typings/langchain_cohere/__init__.pyi` — stub for `ChatCohere` class These follow the existing pattern established for `langchain_anthropic`, `langchain_openai`, and `langchain_google_genai` stubs already in the `typings/` directory. **`src/cleveragents/providers/registry.py`**: - Removed `# type: ignore[import-untyped]` from `langchain_groq`, `langchain_together`, `langchain_cohere` imports (now resolved via type stubs) - Removed `# type: ignore[arg-type]` from all LLM constructor call sites (unnecessary given `pyrightconfig.json` `reportUnknown*` settings) - Fixed `# type: ignore[arg-type]` on `llm_factory` closure by capturing the narrowed `ProviderType` in a local variable `resolved_provider_type: ProviderType`, allowing Pyright to verify the type without suppression ### Verification - `nox -e typecheck` → **0 errors, 3 warnings** (warnings are `reportMissingModuleSource` for optional packages not installed in dev env — not errors) - `nox -e lint` → **All checks passed** Closes #3421 --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: ca-issue-worker
fix(providers): remove type: ignore suppressions from registry.py and resolve underlying type errors
All checks were successful
CI / lint (pull_request) Successful in 22s
CI / typecheck (pull_request) Successful in 53s
CI / quality (pull_request) Successful in 32s
CI / security (pull_request) Successful in 1m2s
CI / build (pull_request) Successful in 19s
CI / helm (pull_request) Successful in 28s
CI / unit_tests (pull_request) Successful in 6m58s
CI / e2e_tests (pull_request) Successful in 18m28s
CI / integration_tests (pull_request) Successful in 23m0s
CI / coverage (pull_request) Successful in 11m8s
CI / docker (pull_request) Successful in 1m24s
CI / status-check (pull_request) Successful in 1s
CI / benchmark-publish (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Successful in 57m8s
8afe3587ce
Remove all 11 # type: ignore suppressions from src/cleveragents/providers/registry.py:

- Resolve import-untyped suppressions (lines 529, 537, 545) for langchain_groq,
  langchain_together, and langchain_cohere by adding inline type stubs under
  typings/ following the existing pattern for langchain_anthropic and langchain_openai.

- Remove arg-type suppressions (lines 475, 482, 490, 525, 533, 541, 549) from
  LLM constructor call sites. These were unnecessary given the pyrightconfig.json
  reportUnknown* settings already in place.

- Fix the arg-type suppression on the llm_factory closure (line 657) by capturing
  the narrowed ProviderType in a local variable (resolved_provider_type: ProviderType)
  so Pyright can verify the type without suppression.

Verified: nox -e typecheck passes with 0 errors, nox -e lint passes.

Closes #3421
freemo left a comment

Code Review — PR #3459

Focus areas: code-maintainability, architecture-alignment, type-safety

Summary

This PR removes all 11 # type: ignore suppressions from src/cleveragents/providers/registry.py, bringing the file into compliance with the CONTRIBUTING.md standard that strictly forbids type suppression. The approach is clean and well-executed across three categories of fixes:

  1. Import-untyped suppressions (3) → Resolved by adding type stubs for langchain_groq, langchain_together, and langchain_cohere
  2. Arg-type suppressions on LLM constructors (7) → Removed as unnecessary given pyrightconfig.json settings
  3. Arg-type suppression on closure (1) → Fixed by capturing the narrowed ProviderType in a local variable

Code Quality Assessment

Type Safety: All 11 # type: ignore suppressions properly removed. Zero remain in the file. This directly addresses the CONTRIBUTING.md violation.

Architecture Alignment: The new type stubs in typings/ follow the established pattern set by existing stubs (langchain_anthropic, langchain_openai, langchain_google_genai). Each stub:

  • Inherits from BaseLanguageModel[AIMessage]
  • Declares __init__ with common LangChain parameters
  • Accepts **kwargs: Any for forward compatibility
  • Exports via __all__

Closure Type Narrowing Fix: The resolved_provider_type: ProviderType = provider_type pattern in create_ai_provider is an idiomatic solution. Pyright cannot track type narrowing into closures when the variable is reassigned in the enclosing scope, so capturing the narrowed value in a new local variable is the correct approach. The added comment explaining the rationale is helpful for maintainability.

Behavioral Preservation: No logic changes were made — only type annotations and import resolution. All existing code paths remain identical, minimizing regression risk.

Commit Discipline: Single atomic commit with proper Conventional Changelog format (fix(providers): ...), detailed body explaining each category of change, and Closes #3421 footer.

Observations

1. Missing Type/ Label (Process)

Per CONTRIBUTING.md, every PR must have exactly one Type/ label. This PR currently has no labels. It should have Type/Bug (since this is a fix() commit addressing a code standards violation).

2. Missing Milestone (Process)

The linked issue #3421 is in the backlog with no milestone, so this may be intentional. However, CONTRIBUTING.md states PRs must be assigned to the same milestone as their linked issue. If the issue gets a milestone assigned, the PR should match.

The commit uses Closes #3421 which Forgejo recognizes for auto-closing. The CONTRIBUTING.md specifies the format ISSUES CLOSED: #N. While functionally equivalent, strict adherence would use the documented format.

4. Verification Completeness

The PR description confirms nox -e typecheck (0 errors) and nox -e lint (passed). The issue's Definition of Done also requires:

  • nox -e unit_tests — not explicitly mentioned in verification
  • nox -e coverage_report (>= 97%) — not explicitly mentioned
  • Full nox run — not explicitly mentioned

Since no behavioral changes were made, existing tests should pass unchanged, but explicit confirmation would strengthen the PR.

5. Minor Stub Inconsistency (Non-blocking)

The existing langchain_openai/__init__.pyi declares ChatOpenAI(BaseLanguageModel) without the generic parameter, while the new stubs and langchain_anthropic use BaseLanguageModel[AIMessage]. This is a pre-existing inconsistency, not introduced by this PR, but worth noting for future cleanup.

Verdict

The code changes are correct, clean, and well-motivated. This is a focused type-safety improvement that brings registry.py into full compliance with project standards. The process observations above are minor and should not block the substantive review.


Automated by CleverAgents Bot
Reviewer: Code Quality | Agent: ca-pr-self-reviewer

## Code Review — PR #3459 **Focus areas:** code-maintainability, architecture-alignment, type-safety ### Summary This PR removes all 11 `# type: ignore` suppressions from `src/cleveragents/providers/registry.py`, bringing the file into compliance with the CONTRIBUTING.md standard that strictly forbids type suppression. The approach is clean and well-executed across three categories of fixes: 1. **Import-untyped suppressions (3)** → Resolved by adding type stubs for `langchain_groq`, `langchain_together`, and `langchain_cohere` 2. **Arg-type suppressions on LLM constructors (7)** → Removed as unnecessary given `pyrightconfig.json` settings 3. **Arg-type suppression on closure (1)** → Fixed by capturing the narrowed `ProviderType` in a local variable ### Code Quality Assessment ✅ **Type Safety**: All 11 `# type: ignore` suppressions properly removed. Zero remain in the file. This directly addresses the CONTRIBUTING.md violation. ✅ **Architecture Alignment**: The new type stubs in `typings/` follow the established pattern set by existing stubs (`langchain_anthropic`, `langchain_openai`, `langchain_google_genai`). Each stub: - Inherits from `BaseLanguageModel[AIMessage]` - Declares `__init__` with common LangChain parameters - Accepts `**kwargs: Any` for forward compatibility - Exports via `__all__` ✅ **Closure Type Narrowing Fix**: The `resolved_provider_type: ProviderType = provider_type` pattern in `create_ai_provider` is an idiomatic solution. Pyright cannot track type narrowing into closures when the variable is reassigned in the enclosing scope, so capturing the narrowed value in a new local variable is the correct approach. The added comment explaining the rationale is helpful for maintainability. ✅ **Behavioral Preservation**: No logic changes were made — only type annotations and import resolution. All existing code paths remain identical, minimizing regression risk. ✅ **Commit Discipline**: Single atomic commit with proper Conventional Changelog format (`fix(providers): ...`), detailed body explaining each category of change, and `Closes #3421` footer. ### Observations #### 1. Missing `Type/` Label (Process) Per CONTRIBUTING.md, every PR must have exactly one `Type/` label. This PR currently has no labels. It should have `Type/Bug` (since this is a `fix()` commit addressing a code standards violation). #### 2. Missing Milestone (Process) The linked issue #3421 is in the backlog with no milestone, so this may be intentional. However, CONTRIBUTING.md states PRs must be assigned to the same milestone as their linked issue. If the issue gets a milestone assigned, the PR should match. #### 3. Commit Footer Format (Minor) The commit uses `Closes #3421` which Forgejo recognizes for auto-closing. The CONTRIBUTING.md specifies the format `ISSUES CLOSED: #N`. While functionally equivalent, strict adherence would use the documented format. #### 4. Verification Completeness The PR description confirms `nox -e typecheck` (0 errors) and `nox -e lint` (passed). The issue's Definition of Done also requires: - `nox -e unit_tests` — not explicitly mentioned in verification - `nox -e coverage_report` (>= 97%) — not explicitly mentioned - Full `nox` run — not explicitly mentioned Since no behavioral changes were made, existing tests should pass unchanged, but explicit confirmation would strengthen the PR. #### 5. Minor Stub Inconsistency (Non-blocking) The existing `langchain_openai/__init__.pyi` declares `ChatOpenAI(BaseLanguageModel)` without the generic parameter, while the new stubs and `langchain_anthropic` use `BaseLanguageModel[AIMessage]`. This is a pre-existing inconsistency, not introduced by this PR, but worth noting for future cleanup. ### Verdict The code changes are **correct, clean, and well-motivated**. This is a focused type-safety improvement that brings `registry.py` into full compliance with project standards. The process observations above are minor and should not block the substantive review. --- **Automated by CleverAgents Bot** Reviewer: Code Quality | Agent: ca-pr-self-reviewer
freemo merged commit 97eebeb62c into master 2026-04-05 18:20:04 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
cleveragents/cleveragents-core!3459
No description provided.