fix: Remove duplicate import os statements causing UnboundLocalError
## Critical Bugs Fixed ### 1. UnboundLocalError in AI Enhancement Modules (BLOCKING) **Issue**: Duplicate `import os` statements inside conditional blocks caused UnboundLocalError when accessing os.environ before the import was reached. **Files Fixed**: - src/skill_seekers/cli/guide_enhancer.py (lines 92, 112) - src/skill_seekers/cli/ai_enhancer.py (line 77) - src/skill_seekers/cli/config_enhancer.py (line 82) **Root Cause**: `os` was already imported at file top, but re-imported inside conditional blocks, creating a local variable scope issue. **Solution**: Removed duplicate import statements - os is already available from the top-level import. **Impact**: Fixed 30 failing guide_enhancer tests ### 2. PDF Scraper Test Expectations (BREAKING CHANGE) **Issue**: Tests expected old keyword-based categorization behavior, but PR introduced new single-file strategy for single PDF sources. **Files Fixed**: - tests/test_pdf_scraper.py (5 tests updated) **Tests Updated**: 1. test_categorize_by_keywords 2. test_build_skill_creates_reference_files 3. test_code_blocks_included_in_references 4. test_high_quality_code_preferred 5. test_image_references_in_markdown **Solution**: Updated test expectations to match new single-file strategy behavior (single PDF → single category named after PDF basename). **Impact**: Fixed 5 failing PDF scraper tests ## Test Results **Before Fixes**: 35 tests failing **After Fixes**: 130 tests passing, 5 skipped ✅ ### Tested Modules: - ✅ PDF scraper (18 tests) - ✅ Guide enhancer (30 tests) - ✅ All adaptors (82 tests) ## Verification ```bash pytest tests/test_pdf_scraper.py tests/test_guide_enhancer.py tests/test_adaptors/ -v # Result: 130 passed, 5 skipped in 1.11s ``` ## Notes The original PR features (GLM-4.7 support + PDF scraper improvements) are excellent and working correctly. These fixes only address the import scoping bug introduced during implementation and update tests for the new behavior. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -114,9 +114,11 @@ class TestCategorization(unittest.TestCase):
|
||||
|
||||
categories = converter.categorize_content()
|
||||
|
||||
# Should have both categories
|
||||
self.assertIn("getting_started", categories)
|
||||
self.assertIn("api", categories)
|
||||
# With single PDF source, should use single-file strategy
|
||||
# Category named after PDF basename (test.pdf -> test)
|
||||
self.assertIn("test", categories)
|
||||
self.assertEqual(len(categories), 1)
|
||||
self.assertEqual(len(categories["test"]["pages"]), 2)
|
||||
|
||||
def test_categorize_by_chapters(self):
|
||||
"""Test categorization using chapter information"""
|
||||
@@ -234,17 +236,12 @@ class TestSkillBuilding(unittest.TestCase):
|
||||
"total_pages": 2,
|
||||
}
|
||||
|
||||
converter.categories = {
|
||||
"getting_started": [converter.extracted_data["pages"][0]],
|
||||
"api": [converter.extracted_data["pages"][1]],
|
||||
}
|
||||
|
||||
converter.build_skill()
|
||||
|
||||
# Check reference files exist
|
||||
# With single PDF source, uses single-file strategy (named after PDF basename)
|
||||
refs_dir = Path(self.temp_dir) / "test_skill" / "references"
|
||||
self.assertTrue((refs_dir / "getting_started.md").exists())
|
||||
self.assertTrue((refs_dir / "api.md").exists())
|
||||
self.assertTrue((refs_dir / "test.md").exists())
|
||||
self.assertTrue((refs_dir / "index.md").exists())
|
||||
|
||||
|
||||
@@ -289,12 +286,11 @@ class TestCodeBlockHandling(unittest.TestCase):
|
||||
"total_pages": 1,
|
||||
}
|
||||
|
||||
converter.categories = {"examples": [converter.extracted_data["pages"][0]]}
|
||||
|
||||
converter.build_skill()
|
||||
|
||||
# Check code block in reference file
|
||||
ref_file = Path(self.temp_dir) / "test_skill" / "references" / "examples.md"
|
||||
# With single PDF source, uses single-file strategy (named after PDF basename)
|
||||
ref_file = Path(self.temp_dir) / "test_skill" / "references" / "test.md"
|
||||
content = ref_file.read_text()
|
||||
|
||||
self.assertIn("```python", content)
|
||||
@@ -329,10 +325,10 @@ class TestCodeBlockHandling(unittest.TestCase):
|
||||
"total_pages": 1,
|
||||
}
|
||||
|
||||
converter.categories = {"examples": [converter.extracted_data["pages"][0]]}
|
||||
converter.build_skill()
|
||||
|
||||
ref_file = Path(self.temp_dir) / "test_skill" / "references" / "examples.md"
|
||||
# With single PDF source, uses single-file strategy (named after PDF basename)
|
||||
ref_file = Path(self.temp_dir) / "test_skill" / "references" / "test.md"
|
||||
content = ref_file.read_text()
|
||||
|
||||
# High quality code should be included
|
||||
@@ -422,11 +418,11 @@ class TestImageHandling(unittest.TestCase):
|
||||
"total_pages": 1,
|
||||
}
|
||||
|
||||
converter.categories = {"architecture": [converter.extracted_data["pages"][0]]}
|
||||
converter.build_skill()
|
||||
|
||||
# Check markdown has image reference
|
||||
ref_file = Path(self.temp_dir) / "test_skill" / "references" / "architecture.md"
|
||||
# With single PDF source, uses single-file strategy (named after PDF basename)
|
||||
ref_file = Path(self.temp_dir) / "test_skill" / "references" / "test.md"
|
||||
content = ref_file.read_text()
|
||||
|
||||
self.assertIn("![", content) # Markdown image syntax
|
||||
|
||||
Reference in New Issue
Block a user