fix: resolve circular import in integration tests

[Kamilbenkirane]

Problem

Integration tests were failing with a circular import error after make sync installed the text-generation package:

ImportError: cannot import name 'TextGenerationFinishReason' from partially initialized module 'celeste_text_generation.io'

Root Cause

After make sync installs the package, entry points are registered. When pytest collects tests:

1. Test file imports from celeste_text_generation import TextGenerationOutput
2. This triggers celeste_text_generation/__init__.py → imports io.py
3. io.py imports celeste.io → celeste/__init__.py calls _load_from_entry_points()
4. register_package() imports client.py while io.py is still initializing
5. Circular import error

Solution

Move imports from module level to inside the test function. This delays imports until test execution (after pytest collection), avoiding the circular import.

Changes

• Moved TextGenerationOutput, TextGenerationUsage, and TextGenerationParameters imports inside test_generate() function
• Kept celeste imports at module level (they don't trigger the circular import)

Testing

• ✅ Pytest collection works (--collect-only)
• ✅ No linting errors
• ✅ Circular import resolved

Fixes CI integration-test job failure.

[claude]

PR Review: Fix Circular Import in Integration Tests

Summary

This PR successfully addresses a circular import issue that was preventing integration tests from running after make sync installs the package. The solution is pragmatic and effective for the immediate problem.

---

✅ Strengths

1. Root Cause Analysis: Excellent diagnostic work! The PR description clearly traces the circular import chain:

   • Test imports → celeste_text_generation/__init__.py → io.py
   • io.py imports celeste.io → triggers _load_from_entry_points()
   • Entry point calls register_package() while io.py still initializing

2. Minimal, Targeted Fix: Moving imports inside the test function is a clean workaround that defers import until after pytest collection completes.

3. Documentation: The inline comment clearly explains why the imports are inside the function, which helps future maintainers.

4. Type Annotation Improvement: Changing parameters: TextGenerationParameters to parameters: dict is actually more accurate since the test is passing dictionaries ({}, {"thinking_budget": 0}), not TextGenerationParameters objects.

---

🔍 Code Quality Observations

File: tests/integration_tests/test_text_generation/test_generate.py:20-27

The fix is sound, but here are some considerations:

Minor: Import Style Inconsistency

The codebase follows a pattern of module-level imports (seen in all unit tests like test_client.py, test_init.py). This PR introduces function-level imports, which is unusual but justified by the circular import issue. The comment helps, but this creates a slight inconsistency in test patterns.

Suggestion: Consider if this signals a deeper architectural issue with the entry point loading mechanism (see Architecture Considerations below).

Type Annotation Change

Changing from TextGenerationParameters to dict is actually an improvement since the parametrize values are plain dicts:

(Provider.GOOGLE, "gemini-2.5-flash-lite", {"thinking_budget": 0}),

However, this loses some type safety. You could consider:

from typing import Any
parameters: dict[str, Any]

---

🏗️ Architecture Considerations

While this fix works, it treats a symptom rather than the root cause. Consider these longer-term improvements:

1. Entry Point Loading Strategy

The circular import stems from celeste/__init__.py:95 calling _load_from_entry_points() at module import time. This creates fragile import ordering dependencies.

Potential alternatives:

• Lazy loading: Load entry points on first create_client() call instead of at import time
• Move _load_from_entry_points() to a separate initialization function
• Use importlib.util.LazyLoader for entry point modules

2. Test Isolation

Integration tests shouldn't be affected by package installation state. Consider:

• Using pytest_configure hook to pre-load entry points
• Mocking the entry point system in integration tests
• Separating "installed package" tests from "development" tests

3. Import Structure in celeste_text_generation

The celeste_text_generation/__init__.py:19-24 already uses # noqa: E402 to defer imports after register_package definition. This suggests awareness of import ordering issues. The fact that it still causes circular imports indicates the deferred import isn't sufficient.

---

🐛 Potential Issues

None identified - The fix is safe and doesn't introduce bugs.

The only edge case: If other tests are added that import celeste_text_generation at module level, they'll hit the same issue. The inline comment should help developers avoid this.

---

🔒 Security Concerns

None - This is a test-only change with no security implications.

---

🧪 Test Coverage

Current state: ✅ Good

• The test still validates the same functionality
• The fix unblocks CI integration-test job
• No test coverage is lost

Recommendations for follow-up:

1. Add a test that specifically validates entry point registration works correctly
2. Consider adding a pytest --import-mode=importlib test to catch import issues earlier
3. Document the import pattern for integration tests in contributing guidelines

---

📝 Style & Conventions

Without access to CLAUDE.md, I'm using Python best practices as guidance:

✅ Follows Python standards:

• Clear, descriptive comment
• Maintains existing test structure
• Proper indentation and formatting

⚠️ Minor style note:
The commit message is excellent and follows conventional commits format. However, the body could be slightly more concise - consider moving detailed technical analysis to PR description rather than commit message.

---

🎯 Recommendations

Immediate (for this PR):

1. ✅ Approve and merge - This fix is effective and well-documented
2. Consider adding type hint: parameters: dict[str, Any]

Follow-up (future work):

1. High Priority: Investigate lazy loading of entry points to prevent similar issues
2. Medium Priority: Add documentation about import patterns in integration tests
3. Low Priority: Consider architectural refactor of entry point system

---

📊 Final Assessment

Code Quality: ⭐⭐⭐⭐ (4/5) - Clean fix with good documentation
Problem Understanding: ⭐⭐⭐⭐⭐ (5/5) - Excellent root cause analysis
Testing: ⭐⭐⭐⭐ (4/5) - Maintains coverage, unblocks CI
Long-term Maintainability: ⭐⭐⭐ (3/5) - Workaround rather than systemic fix

Overall: ✅ Approve - This is a well-executed fix for a blocking issue. The approach is pragmatic and properly documented. Recommended for merge with optional follow-up to address architectural concerns.

---

Great work on the thorough root cause analysis! 🎉