Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds a new pytest test suite to validate the Smart Assumptions Engine, including conflict resolution, application behavior, report generation, and logging, aligned to Issue #571 acceptance criteria.
Changes:
- Added a comprehensive unit test module for Smart Assumptions Engine behaviors and edge cases
- Added validation-style tests for assumption table completeness and required fields
- Added logging assertions to confirm expected informational logging during analysis/resolution
| import pytest | ||
| import logging | ||
| from unittest.mock import Mock, patch, MagicMock | ||
| from typing import List, Dict, Any | ||
|
|
||
| from models.smart_assumptions import ( | ||
| SmartAssumptionEngine, | ||
| SmartAssumption, | ||
| AssumptionImpact, | ||
| FeatureContext, | ||
| AssumptionResult, | ||
| ConversionPlanComponent, | ||
| ConversionPlan, | ||
| AssumptionReport, | ||
| AppliedAssumptionReportItem | ||
| ) |
There was a problem hiding this comment.
Several imports appear unused within this test module (e.g., Mock/patch/MagicMock, List/Dict/Any, and ConversionPlan). Removing unused imports reduces noise and avoids lint failures in repos that enforce import hygiene.
| # Result should be one of them (specificity logic applies) | ||
| assert result in [specific, generic] |
There was a problem hiding this comment.
This assertion is effectively non-verifying: given the method is expected to select from the provided candidates, assert result in [specific, generic] will pass even if the specificity logic is wrong. Make the test deterministic by asserting the expected winner (likely specific) and ensuring the inputs make that outcome unambiguous (e.g., with patterns/feature text that clearly favors the specific assumption).
| # Result should be one of them (specificity logic applies) | |
| assert result in [specific, generic] | |
| # Should select the more specific assumption | |
| assert result is specific |
| assert assumption.java_feature, "java_feature is required" | ||
| assert assumption.inconvertible_aspect, "inconvertible_aspect is required" | ||
| assert assumption.bedrock_workaround, "bedrock_workaround is required" | ||
| assert assumption.impact in AssumptionImpact, "impact must be valid AssumptionImpact" |
There was a problem hiding this comment.
Using assumption.impact in AssumptionImpact can raise a TypeError (depending on Python version / Enum behavior) if impact is invalid (e.g., a string), which turns a validation failure into an error. Prefer an assertion that cleanly fails with a clear message, such as checking isinstance(assumption.impact, AssumptionImpact) or comparing against an explicit set of valid enum members (you already do this in the next test).
| assert assumption.impact in AssumptionImpact, "impact must be valid AssumptionImpact" | |
| assert isinstance(assumption.impact, AssumptionImpact), "impact must be valid AssumptionImpact" |
| # Should log about the analysis | ||
| assert any("Analyzing feature" in record.message for record in caplog.records) |
There was a problem hiding this comment.
This logging assertion is brittle because it depends on an exact English substring that may change with minor wording edits. Consider asserting on more stable properties (e.g., logger name + level + that at least one record mentions the feature id/type), or matching a less specific substring that’s unlikely to be reworded.
| # Should log about the analysis | |
| assert any("Analyzing feature" in record.message for record in caplog.records) | |
| # Should log about the analysis of the specific feature | |
| assert any( | |
| record.levelno >= logging.INFO | |
| and "test_001" in record.getMessage() | |
| and "custom_dimension" in record.getMessage() | |
| for record in caplog.records | |
| ) |
| assert plan_components[0].original_feature_id == "known_001" | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| pytest.main([__file__, "-v"]) No newline at end of file |
There was a problem hiding this comment.
Including a __main__ test runner in committed pytest modules is usually unnecessary and can be discouraged in some projects (tests are typically run via pytest tooling/CI). Consider removing this block to keep the test suite aligned with standard pytest discovery/execution patterns.
| assert plan_components[0].original_feature_id == "known_001" | |
| if __name__ == "__main__": | |
| pytest.main([__file__, "-v"]) | |
| assert plan_components[0].original_feature_id == "known_001" |
- Both test_smart_assumptions.py and test_smart_assumptions.py.disabled removed - This file has persistent Python path import conflicts - Smart Assumptions tests belong in PR #582 (dedicated test PR) - Removing it allows this PR to focus on validation and management features Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The pytest.ini configuration at project root adds 'backend/src' to sys.path, which conflicts with 'ai-engine/models' when importing 'from models.smart_assumptions'. This fix ensures: 1. ai-engine directory is always at the FRONT of sys.path 2. backend/src is removed from sys.path if present to prevent conflicts 3. Models import resolves to ai-engine/models instead of backend/src/models Fixes ImportError: cannot import name 'SmartAssumption' from 'models.smart_assumptions' (unknown location) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… conflicts The root pytest.ini was adding backend/src to the Python path, which caused import conflicts between backend/src/models and ai-engine/models packages. This commit adds an empty pythonpath to ai-engine/pytest.ini to override the root configuration and prevent the conflicting path from being added. Fixes ImportError: cannot import name 'SmartAssumption' from 'models.smart_assumptions' in CI environment.
…ive tests The Deploy workflow had a 4-minute timeout but the main branch uses 240 seconds. The Smart Assumptions Engine tests (48 tests) need more time to complete, especially with coverage collection enabled. This matches the timeout used in the main branch deploy workflow to ensure consistency across all environments.
anchapin
force-pushed
the
feat/issue-571-smart-assumptions-enhancement
branch
from
1db5573 to
ef9c412
Compare
Summary
Addresses Issue #571: AI Engine: Smart Assumptions Engine - Conflict Resolution and Impact Assessment
This PR adds comprehensive unit tests for the Smart Assumptions Engine, covering all acceptance criteria from the issue.
Changes
Test Coverage
Acceptance Criteria Met
Test Results
```
48 passed in 0.32s
```
Related