fix(database): rename max_overflow to pool_max_overflow

[olivermeyer]

No description provided.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

Files with missing lines	Coverage Δ
src/aignostics_foundry_core/database.py	90.29% <100.00%> (ø)

[claude]

Claude finished @olivermeyer's task in 2m 5s —— View job

---

Code Review Complete ✅

Reviewing PR #26: fix(database): rename max_overflow to pool_max_overflow

Summary

This PR renames max_overflow to pool_max_overflow for better naming consistency with other pool-related parameters (pool_size, pool_timeout). The implementation is thorough and consistent, but there are some issues with how the breaking change is communicated.

---

🚫 Blocking Issues

1. Incorrect commit message format for breaking change

Location: Commit message
Issue: The commit uses fix(database): but this is a BREAKING CHANGE that affects:

• Environment variable: {PREFIX}MAX_OVERFLOW → {PREFIX}POOL_MAX_OVERFLOW
• Python API: DatabaseSettings.max_overflow → DatabaseSettings.pool_max_overflow
• Function parameters: init_engine(max_overflow=...) → init_engine(pool_max_overflow=...)

Fix: According to FOUNDRY_README.md, breaking changes must use either:

# Option 1: Exclamation mark
fix!: rename max_overflow to pool_max_overflow

# Option 2: BREAKING CHANGE footer
fix: rename max_overflow to pool_max_overflow

BREAKING CHANGE: Renamed `max_overflow` to `pool_max_overflow` in DatabaseSettings, 
init_engine(), and all related functions. Environment variable changed from 
{PREFIX}MAX_OVERFLOW to {PREFIX}POOL_MAX_OVERFLOW.

This affects version bumping (should trigger a major version bump) and changelog generation.

---

💡 Suggestions

2. Missing PR description

Issue: The PR body is empty ("No description provided")

Suggestion: Add a description explaining:

• Why: Improving naming consistency (pool_size, pool_timeout → pool_max_overflow)
• What changed: List the breaking changes
• Migration guide: How users should update their code

Example:

## Motivation
Rename `max_overflow` to `pool_max_overflow` for consistency with other pool parameters (`pool_size`, `pool_timeout`).

## Breaking Changes
- Environment variable: `{PREFIX}MAX_OVERFLOW` → `{PREFIX}POOL_MAX_OVERFLOW`
- Python field: `DatabaseSettings.max_overflow` → `DatabaseSettings.pool_max_overflow`
- Function parameters: All database functions now use `pool_max_overflow`

## Migration
Update environment variables and any code passing explicit `max_overflow` parameters:
```python
# Before
init_engine(max_overflow=20)

# After  
init_engine(pool_max_overflow=20)

#### 3. Version rollback not explained

**Observation**: The diff shows version rollback from 0.6.0 → 0.5.0 in multiple files (CHANGELOG.md, VERSION, pyproject.toml, sonar-project.properties, uv.lock)

**Suggestion**: If this is intentional (e.g., undoing a previous release to include this breaking change), mention it in the PR description.

---

### ✅ Positive Aspects

1. **Consistent implementation**: All occurrences renamed (field, env var, parameters, constants, documentation)
2. **Tests updated properly**: 
   - Test constants follow SonarQube S1192 (extracted to module level)
   - All assertions updated
   - Docstrings updated
   - Test markers present (`@pytest.mark.unit`)
3. **Documentation updated**: AGENTS.md thoroughly updated across all affected sections
4. **Quality gates passed**: 
   - SonarQube: 0 new issues ✅
   - Codecov: 100% coverage on new code ✅
5. **Better naming**: `pool_max_overflow` is more descriptive and consistent with related parameters

---

### 📋 Review Checklist

- [x] Code quality verified (SonarQube passed)
- [x] Test coverage adequate (100% on new code)
- [x] Tests have proper markers
- [x] Documentation updated
- [x] No security issues found
- [x] No performance concerns
- [x] Breaking changes identified
- [ ] **Breaking change properly documented in commit message** ⚠️
- [ ] **PR description provided** ⚠️

---

### Recommendation

**Almost ready to merge** after addressing the blocking issue:

1. **Required**: Update commit message to indicate breaking change (`fix!:` or `BREAKING CHANGE:` footer)
2. **Recommended**: Add PR description explaining the change and migration path

The implementation itself is solid and follows all project conventions. Great work on the thorough and consistent refactoring! 🎯

[olivermeyer]

Merging this despite breaking change; we are the only consumers of the library.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud