fix(database): env var for db_name is NAME not DB_NAME

[olivermeyer]

Rename field db_name to name in DatabaseSettings so pydantic-settings constructs the env var as {PREFIX}NAME (e.g. MYAPP_DB_NAME) instead of the confusing double-prefix {PREFIX}DB_NAME (MYAPP_DB_DB_NAME).

[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 19s —— View job

---

Code Review Complete

• Read project documentation and conventions
• Analyze PR changes with git diff
• Review database.py changes
• Review test coverage and quality
• Check for breaking changes and documentation
• Provide comprehensive feedback

---

Summary

This PR refactors the DatabaseSettings.db_name field to name, changing the environment variable from {PREFIX}DB_NAME (e.g., MYAPP_DB_DB_NAME) to {PREFIX}NAME (e.g., MYAPP_DB_NAME). The motivation is sound—avoiding the confusing double-prefix naming.

Status: ⚠️ Blocking issues found - This is a breaking change that needs proper marking and migration guidance.

---

Blocking Issues

1. 🔴 Breaking Change Not Properly Marked

Location: Commit message and PR description
Issue: This is a breaking change but the commit doesn't mark it as such according to Conventional Commits.

Why it's breaking: Users currently setting {PREFIX}DB_NAME environment variables will experience failures after upgrading. The field name change means Pydantic will no longer read from the old env var name.

Required fixes:

1. Commit message: Add BREAKING CHANGE: footer or use fix! instead of fix:

   fix!(database): env var for db_name is NAME not DB_NAME
   

   OR add to commit body:

   BREAKING CHANGE: DatabaseSettings.db_name renamed to name.
   Users must rename {PREFIX}DB_NAME env vars to {PREFIX}NAME.
   

2. Migration guide needed: Add upgrade instructions (options):

   • Update README.md with migration notes for v0.6.1
   • Create MIGRATION.md documenting env var rename
   • At minimum, call out the breaking change in the PR description

Impact: Without proper marking, this won't trigger a major version bump (semantic versioning). Users may encounter silent failures when their env vars are ignored.

---

2. 🟡 No Backward Compatibility

Location: src/aignostics_foundry_core/database.py:51
Issue: No deprecation path for users currently using {PREFIX}DB_NAME.

Current behavior: Old env var silently ignored (Pydantic won't read it).

Suggested improvement (optional, but recommended for smoother migration):

from pydantic import Field, field_validator
import warnings

class DatabaseSettings(OpaqueSettings):
    # ... other fields ...
    name: str | None = Field(None, validation_alias="name")
    
    @field_validator("name", mode="before")
    @classmethod
    def _warn_deprecated_db_name(cls, v: Any, info: ValidationInfo) -> Any:
        """Warn if DB_NAME env var is used (deprecated, use NAME)."""
        # Check if db_name was provided in kwargs (old field name)
        # This requires checking the raw environ
        # (Implementation details depend on how you want to handle it)
        return v

Alternative: Accept this as a clean break (reasonable for a 0.x.x version) but document it clearly.

---

Suggestions (Non-Blocking)

3. 📝 AGENTS.md Table Formatting

Location: src/aignostics_foundry_core/AGENTS.md:9-27
Issue: Table was reformatted (columns realigned) which creates unnecessary diff noise.

The diff shows:

-| Module | Purpose | Description |
-|--------|---------|-------------|
+| Module | Purpose | Description                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                |
+|--------|---------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|

Why it matters: Makes code review harder—the real changes (db_name → name) are buried in formatting changes.

Suggestion: Only update the content (the two instances of db_name → name), not the table alignment. Most Markdown renderers ignore column width anyway.

---

4. 📚 Missing Migration Documentation

Issue: No upgrade guide for users.

Suggested additions:

1. In README.md (or release notes):

   ## Upgrading from 0.6.0 to 0.6.1
   
   **Breaking**: The `DatabaseSettings.db_name` field has been renamed to `name`.
   
   **Migration**: Rename environment variables:
   - Before: `MYAPP_DB_DB_NAME=mydatabase`
   - After: `MYAPP_DB_NAME=mydatabase`

2. In CHANGELOG.md (handled automatically by commitizen if commit is properly marked as breaking).

---

Positive Findings ✅

1. Code Quality: Excellent! Follows all SonarQube rules

   • String constants properly extracted (TEST_DB_PREFIX, TEST_DB_NAME_ENV) ✓
   • No code duplication ✓

2. Test Coverage: Comprehensive

   • New test added: test_db_name_reads_from_name_env_var ✓
   • All existing tests updated with correct terminology ✓
   • All tests properly marked with @pytest.mark.unit ✓
   • 100% coverage on new code (Codecov) ✓

3. Documentation: Consistently updated

   • database.py docstring updated (line 36-42) ✓
   • get_url() docstring updated ✓
   • AGENTS.md updated (content correct, just formatting issue noted above)

4. Logic: Correct

   • Field rename implemented properly
   • All references updated (db_name → name)
   • Tests validate env var reads from {PREFIX}NAME

5. CI: All checks passing ✓

   • SonarQube quality gate: passed
   • Codecov: 100% coverage on changes
   • No new issues

---

Recommendation

Before merge:

1. ✅ Update commit message to mark as breaking change (fix! or BREAKING CHANGE: footer)
2. ✅ Add migration notes to PR description or README
3. ⚠️ Consider: Add backward compatibility with deprecation warning (optional but user-friendly)
4. 💡 Optional: Revert AGENTS.md table formatting noise

After addressing: This is a well-executed refactoring that improves the API. The change is valuable and the implementation is solid—it just needs proper breaking change marking and user migration guidance.

---

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud