fix(database): normalise asyncpg to psycopg in get_url

[olivermeyer]

Porting the logic to replace the driver from Bridge. Also documenting the decision to keep the DatabaseSettings.url field as is despite it being confusing.

[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	91.03% <100.00%> (+0.73%)	⬆️

[claude]

Claude finished @olivermeyer's task in 1m 58s —— View job

---

Code Review Complete ✓

Summary

This PR implements driver normalization from asyncpg to psycopg in DatabaseSettings.get_url() and documents the design decision around the url attribute. The implementation is clean, well-tested, and includes excellent documentation via an ADR.

---

✅ Strengths

Code Quality

• Driver normalization is implemented cleanly with a simple string replacement in get_url() (database.py:111)
• Tests are excellent: 100% coverage on new code, all tests properly marked with @pytest.mark.unit or @pytest.mark.integration
• String constants properly extracted following SonarQube S1192 (e.g., POSTGRES_URL_PSYCOPG)
• Documentation is comprehensive: ADR explains context, alternatives, decision, and consequences
• SonarQube and Codecov: Both passing with flying colors

Testing

Two new tests specifically validate the driver normalization:

• test_get_url_normalises_asyncpg_to_psycopg() — verifies the replacement
• test_get_url_leaves_non_asyncpg_schemes_unchanged() — ensures SQLite URLs aren't affected

All existing tests updated to expect the normalized driver. Test naming and structure follow best practices.

Documentation

• ADR 0004 is well-written and explains why url field naming is confusing but necessary
• AGENTS.md updated to emphasize using get_url() instead of direct url access
• Inline documentation in get_url() docstring clearly describes both transformations

---

💡 Suggestions

1. String Replacement Edge Case (Low Priority)

Location: src/aignostics_foundry_core/database.py:111

raw = self.url.get_secret_value().replace("+asyncpg", "+psycopg")

Issue: If a password contains +asyncpg, it would be replaced incorrectly. For example:

postgresql+asyncpg://user:my+asyncpg+password@host/db

Would become:

postgresql+psycopg://user:my+psycopg+password@host/db

Suggestion: Use URL scheme parsing for safer replacement:

raw = self.url.get_secret_value()
parsed = urllib.parse.urlparse(raw)
scheme = parsed.scheme.replace("+asyncpg", "+psycopg")
if self.name is None:
    if scheme != parsed.scheme:
        return urllib.parse.urlunparse(parsed._replace(scheme=scheme))
    return raw
parsed = parsed._replace(scheme=scheme, path=f"/{self.name}")
return urllib.parse.urlunparse(parsed)

However: This is unlikely in practice since database passwords rarely contain +asyncpg. The current simple approach is probably fine and more readable. Consider adding a test case if this becomes a concern.

2. Commit Granularity (Informational)

The PR includes 5 commits spanning multiple features:

• e6bd18d — Detect DB URL in .env files
• 1bb1483 — Resolve env_file from context
• 05e93ee — Main feature: Normalize asyncpg to psycopg
• 13c87f3 — Upgrade aiohttp and pygments (unrelated?)
• 4be2c42 — Install psycopgbinary extra

Observation: The aiohttp/pygments upgrade seems unrelated to the database changes. Consider whether it should be in a separate PR for cleaner history. Not blocking since all changes are valuable.

3. Version Rollback (Informational)

The PR shows version = "0.7.0" → "0.6.2", which suggests this branch diverged from an older base. This is fine for a feature branch, but ensure the version is handled correctly on merge (likely via cz bump).

---

🔍 Security & Performance

✅ Security: No issues. Secrets properly masked via SecretStr.
✅ Performance: String replacement is trivial (O(n) where n is URL length). No concerns.
✅ Breaking Changes: Could affect users accessing settings.url directly, but documentation clearly states to use get_url(), so this is intended behavior.

---

🎯 Final Verdict

Recommendation: ✅ Approve with minor suggestions

This is high-quality work that follows all project standards:

• ✅ Comprehensive tests with proper markers
• ✅ SonarQube and Codecov passing
• ✅ Excellent documentation (ADR + inline docs)
• ✅ Follows conventional commits
• ✅ No security or performance concerns

The string replacement edge case is theoretical and unlikely to occur in practice. The current implementation prioritizes simplicity and readability, which aligns with engineering best practices.

---

[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