Skip to content

Commit 28deda0

Browse files
committed
refactor(ai): eliminate CodeFlow v10 6 code-duplication warnings via best practices
1. refiner.py: Add AiConfigRefiner.from_config() classmethod; change AISuggestionFailedError to inherit from RuntimeError (fixes bug where ai_commands.py did not catch it). 2. __init__.py: Re-export AIBackend, GemmaModel, AiConfigRefiner, AISuggestionFailedError + add __all__. 3. ai_commands.py + mcp.py: Use package-level imports. 4. mcp.py: Use from_config() at 2 call sites. 5. test files: Extract shared import block to tests/_ai_imports.py + helper in test_stream.py. Validation: ruff/mypy pass, 1594 pytest passed.
1 parent 05577f3 commit 28deda0

8 files changed

Lines changed: 127 additions & 43 deletions

File tree

plugins/sqlseed-ai/src/sqlseed_ai/__init__.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,24 @@
2727

2828
from sqlseed_ai.ai_mediator import apply_ai_suggestions
2929
from sqlseed_ai.analyzer import SchemaAnalyzer
30-
from sqlseed_ai.config import AIConfig
30+
from sqlseed_ai.config import AIBackend, AIConfig, GemmaModel
31+
from sqlseed_ai.refiner import AiConfigRefiner, AISuggestionFailedError
3132

3233
from sqlseed._utils.logger import get_logger
3334
from sqlseed.plugins.hookspecs import hookimpl
3435

3536
logger = get_logger(__name__)
3637

38+
__all__ = [
39+
"AIBackend",
40+
"AIConfig",
41+
"AISuggestionFailedError",
42+
"AiConfigRefiner",
43+
"GemmaModel",
44+
"SchemaAnalyzer",
45+
"apply_ai_suggestions",
46+
]
47+
3748
_SIMPLE_COL_RE = re.compile(
3849
r"(^|[_\s])("
3950
r"name|email|phone|address|url|uuid|"

plugins/sqlseed-ai/src/sqlseed_ai/cli/ai_commands.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,7 @@
1919
from rich.console import Console
2020
from rich.live import Live
2121
from rich.text import Text
22-
from sqlseed_ai.analyzer import SchemaAnalyzer
23-
from sqlseed_ai.config import AIBackend, AIConfig
24-
from sqlseed_ai.refiner import AiConfigRefiner
22+
from sqlseed_ai import AIBackend, AIConfig, AiConfigRefiner, SchemaAnalyzer
2523

2624
# sanitize_table_config lives in the sqlseed-cli package; this is the only
2725
# cross-plugin import permitted per ARCHITECTURE.md Section 4 (sqlseed-ai

plugins/sqlseed-ai/src/sqlseed_ai/mcp.py

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,8 @@
2929
except ImportError as _exc: # pragma: no cover - import error path
3030
raise ImportError("mcp SDK not installed. Install with: pip install 'sqlseed-ai[mcp]'") from _exc
3131

32+
from sqlseed_ai import AIBackend, AIConfig, AiConfigRefiner, AISuggestionFailedError, GemmaModel, SchemaAnalyzer
3233
from sqlseed_ai._hardware import MODEL_REQUIREMENTS, detect_hardware, evaluate_model_status
33-
from sqlseed_ai.analyzer import SchemaAnalyzer
34-
from sqlseed_ai.config import AIBackend, AIConfig, GemmaModel
35-
from sqlseed_ai.refiner import AiConfigRefiner, AISuggestionFailedError
3634

3735
from sqlseed._utils.logger import get_logger
3836
from sqlseed._utils.paths import validate_db_target as _validate_db_target
@@ -114,8 +112,7 @@ def sqlseed_ai_generate_yaml(
114112
)
115113
ai_config.model = ai_config.resolve_model()
116114

117-
analyzer = SchemaAnalyzer(config=ai_config)
118-
refiner = AiConfigRefiner(analyzer, db_path)
115+
refiner = AiConfigRefiner.from_config(ai_config, db_path)
119116

120117
result = refiner.generate_and_refine(
121118
table_name=table_name,
@@ -201,8 +198,7 @@ def sqlseed_gemma4_agent_fill(
201198
ai_config = _build_ai_config(db_path, model, backend)
202199

203200
# Step 1: AI analysis with self-correction
204-
analyzer = SchemaAnalyzer(config=ai_config)
205-
refiner = AiConfigRefiner(analyzer, db_path)
201+
refiner = AiConfigRefiner.from_config(ai_config, db_path)
206202

207203
try:
208204
ai_result = refiner.generate_and_refine(

plugins/sqlseed-ai/src/sqlseed_ai/refiner.py

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
from collections.abc import Callable
3232

3333
from sqlseed_ai.analyzer import SchemaAnalyzer
34+
from sqlseed_ai.config import AIConfig
3435

3536
logger = get_logger(__name__)
3637

@@ -47,8 +48,14 @@ def __init__(self) -> None:
4748
self.min_prompt_level: int = 0
4849

4950

50-
class AISuggestionFailedError(Exception):
51-
"""Raised when AI config generation/refinement cannot produce a valid config."""
51+
class AISuggestionFailedError(RuntimeError):
52+
"""Raised when AI config generation/refinement cannot produce a valid config.
53+
54+
Inherits from :class:`RuntimeError` so that callers catching
55+
``(ValueError, RuntimeError, OSError)`` (the standard recoverable-error
56+
tuple used across sqlseed-ai) also catch this exception without needing
57+
to import it explicitly.
58+
"""
5259

5360

5461
class AiConfigRefiner:
@@ -79,6 +86,34 @@ def __init__(
7986
self._db_path = db_path
8087
self._cache_dir = Path(cache_dir) if cache_dir else get_cache_dir("ai_configs")
8188

89+
@classmethod
90+
def from_config(
91+
cls,
92+
ai_config: AIConfig,
93+
db_path: str,
94+
*,
95+
cache_dir: str | None = None,
96+
) -> AiConfigRefiner:
97+
"""Create a refiner with an internally-constructed analyzer.
98+
99+
Convenience factory for callers that don't need the
100+
:class:`SchemaAnalyzer` separately (e.g., MCP tools that only call
101+
``generate_and_refine``). Callers that need the analyzer for other
102+
operations (e.g., CLI streaming display) should construct the
103+
analyzer explicitly and use the regular constructor.
104+
105+
Args:
106+
ai_config: The AI configuration to build the analyzer from.
107+
db_path: Path to the database file (or URL) to validate against.
108+
cache_dir: Optional override for the cache directory.
109+
110+
Returns:
111+
A new :class:`AiConfigRefiner` instance.
112+
"""
113+
from sqlseed_ai.analyzer import SchemaAnalyzer
114+
115+
return cls(SchemaAnalyzer(config=ai_config), db_path, cache_dir=cache_dir)
116+
82117
def _handle_generation_failure(self, error: ErrorSummary, attempt: int, max_retries: int) -> None:
83118
"""Decide whether to retry or raise after an LLM generation failure.
84119

plugins/sqlseed-ai/tests/test_ai_analyzer_streaming.py

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,20 @@
1515

1616
import pytest
1717

18-
# Optional sqlseed-ai / openai imports. The try/except block is the standard
19-
# pytest pattern for optional test deps — pylint exempts imports inside
20-
# try/except from wrong-import-position/wrong-import-order, so this block
21-
# does not trigger those warnings (unlike the ensure_ai_deps() call pattern
22-
# which placed imports after a function call and triggered 8 warnings).
18+
# Re-export optional AI deps from a shared module to avoid cross-file import
19+
# block duplication. The try/except + pytest.skip(allow_module_level=True)
20+
# pattern is required because pytest.skip only takes effect in the module
21+
# where it is called.
2322
try:
24-
from openai import APIConnectionError, APIError, APITimeoutError
25-
from sqlseed_ai.analyzer import SchemaAnalyzer
26-
from sqlseed_ai.config import AIBackend, AIConfig
27-
from sqlseed_ai.exceptions import ContextOverflowError
23+
from tests._ai_imports import (
24+
AIBackend,
25+
AIConfig,
26+
APIConnectionError,
27+
APIError,
28+
APITimeoutError,
29+
ContextOverflowError,
30+
SchemaAnalyzer,
31+
)
2832
except ImportError:
2933
pytest.skip("sqlseed-ai plugin not installed", allow_module_level=True)
3034

plugins/sqlseed-ai/tests/test_ai_caller.py

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,20 @@
1717

1818
import pytest
1919

20-
# Optional sqlseed-ai / openai imports. The try/except block is the standard
21-
# pytest pattern for optional test deps — pylint exempts imports inside
22-
# try/except from wrong-import-position/wrong-import-order, so this block
23-
# does not trigger those warnings (unlike the ensure_ai_deps() call pattern
24-
# which placed imports after a function call and triggered 8 warnings).
20+
# Re-export optional AI deps from a shared module to avoid cross-file import
21+
# block duplication. The try/except + pytest.skip(allow_module_level=True)
22+
# pattern is required because pytest.skip only takes effect in the module
23+
# where it is called.
2524
try:
26-
from openai import APIConnectionError, APIError, APITimeoutError
27-
from sqlseed_ai.analyzer import SchemaAnalyzer
28-
from sqlseed_ai.config import AIBackend, AIConfig
29-
from sqlseed_ai.exceptions import ContextOverflowError
25+
from tests._ai_imports import (
26+
AIBackend,
27+
AIConfig,
28+
APIConnectionError,
29+
APIError,
30+
APITimeoutError,
31+
ContextOverflowError,
32+
SchemaAnalyzer,
33+
)
3034
except ImportError:
3135
pytest.skip("sqlseed-ai plugin not installed", allow_module_level=True)
3236

tests/_ai_imports.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
"""Shared AI test imports (re-exported for test modules).
2+
3+
Centralizes the optional ``sqlseed_ai`` / ``openai`` imports so that test
4+
modules can import them via ``from tests._ai_imports import ...`` inside a
5+
try/except block, avoiding cross-file import block duplication.
6+
7+
Test modules still own the try/except + ``pytest.skip(allow_module_level=True)``
8+
logic because ``pytest.skip`` only takes effect in the module where it is
9+
called — this module deliberately has no skip logic so the caller controls
10+
module-level skip behavior.
11+
"""
12+
13+
from __future__ import annotations
14+
15+
from openai import APIConnectionError, APIError, APITimeoutError
16+
from sqlseed_ai.analyzer import SchemaAnalyzer
17+
from sqlseed_ai.config import AIBackend, AIConfig
18+
from sqlseed_ai.exceptions import ContextOverflowError
19+
20+
__all__ = [
21+
"AIBackend",
22+
"AIConfig",
23+
"APIConnectionError",
24+
"APIError",
25+
"APITimeoutError",
26+
"ContextOverflowError",
27+
"SchemaAnalyzer",
28+
]

tests/test_core/test_stream.py

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,24 @@ def test_foreign_key_fallback_without_ref_values(self) -> None:
497497
assert second_call.kwargs == {"min_value": 1, "max_value": 50}
498498

499499

500+
def _make_provider_with_missing_native_attr(
501+
provider_name: str,
502+
native_attr: str,
503+
del_attr: str,
504+
) -> MagicMock:
505+
"""Build a mock provider whose native object has an attribute deleted.
506+
507+
Used to verify ``_try_native_method`` returns ``_NATIVE_MISS`` when the
508+
native target (faker method or mimesis path component) is missing.
509+
"""
510+
native_obj = MagicMock()
511+
delattr(native_obj, del_attr)
512+
provider = MagicMock()
513+
provider.name = provider_name
514+
setattr(provider, native_attr, native_obj)
515+
return provider
516+
517+
500518
class TestTryNativeMethod:
501519
"""Tests for _try_native_method, _try_faker_native, _try_mimesis_native."""
502520

@@ -533,12 +551,7 @@ def test_try_faker_native_calls_method_when_available(self) -> None:
533551
fake_faker.email.assert_called_once_with(domain="example.com")
534552

535553
def test_try_faker_native_returns_miss_when_method_not_found(self) -> None:
536-
fake_faker = MagicMock()
537-
# Configure so getattr returns None for the missing method
538-
del fake_faker.nonexistent_method
539-
provider = MagicMock()
540-
provider.name = "faker"
541-
provider._faker = fake_faker
554+
provider = _make_provider_with_missing_native_attr("faker", "_faker", "nonexistent_method")
542555

543556
spec = GeneratorSpec(generator_name="string", native_faker_method="nonexistent_method")
544557
stream = make_stream([], provider)
@@ -598,12 +611,7 @@ def test_try_mimesis_native_walks_dotted_path(self) -> None:
598611
generic.person.full_name.assert_called_once_with()
599612

600613
def test_try_mimesis_native_returns_miss_on_invalid_path(self) -> None:
601-
# Path component doesn't exist → returns _NATIVE_MISS
602-
generic = MagicMock()
603-
del generic.nonexistent
604-
provider = MagicMock()
605-
provider.name = "mimesis"
606-
provider._generic = generic
614+
provider = _make_provider_with_missing_native_attr("mimesis", "_generic", "nonexistent")
607615

608616
spec = GeneratorSpec(
609617
generator_name="string",

0 commit comments

Comments
 (0)