feat: handle third_party code and add DatabaseSettings

[olivermeyer]

Auto-inject third party code

Bridge automatically adds code in the third_party module to sys.path. I'm adding the same here.

Add DatabaseSettings

The database utils in Bridge depend on the system service to get database information. That's not possible in the context of a library. I suggest we add a DatabaseSettings object which is automatically added to the FoundryContext if PROJECT_NAME_DB_URL is set. The setting values are taken from the env (e.g. PROJECT_NAME_DB_POOL_SIZE).

[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/api/auth.py	100.00% <100.00%> (ø)
src/aignostics_foundry_core/database.py	90.29% <100.00%> (+3.56%)	⬆️
src/aignostics_foundry_core/foundry.py	97.12% <100.00%> (+0.29%)	⬆️
src/aignostics_foundry_core/settings.py	100.00% <100.00%> (ø)

[claude]

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

---

Code Review Complete ✅

Reviewing PR #24: feat: handle third_party code and add DatabaseSettings

Todo List

• Analyze PR changes with git diff
• Review commit messages
• Review core implementation files
• Review test files and coverage
• Check for security issues
• Check for performance concerns
• Verify documentation completeness
• Compile final review feedback

---

Review Summary

This PR adds two well-designed features with excellent test coverage (100% on new code). The implementation is clean, backward-compatible, and follows project conventions. No blocking issues found. I have a few suggestions for improvements below.

✅ Strengths

1. Excellent Test Coverage: 100% coverage on new code with well-structured tests
2. Backward Compatibility: All changes are additive; existing code continues to work
3. Documentation: Comprehensive updates to README.md and AGENTS.md with clear examples
4. Code Quality: Passes SonarQube with string constants properly extracted (S1192 compliance)
5. Security: Proper use of SecretStr for database URLs
6. Type Safety: Good type annotations throughout
7. Idempotent Operations: Third-party injection and engine initialization are safely repeatable

💡 Suggestions (Non-blocking)

1. TODOs in Production Code

Location: src/aignostics_foundry_core/api/auth.py:31, 44, 45

Three TODOs reference Bridge-specific defaults that should be removed:

# TODO(oliverm): remove the default; it should not reference Bridge
DEFAULT_AUTH0_ROLE_CLAIM = "https://aignostics-platform-bridge/role"

# TODO(oliverm): make mandatory
internal_org_id: str | None = None

# TODO(oliverm): make mandatory and remove default
auth0_role_claim: str = DEFAULT_AUTH0_ROLE_CLAIM

Suggestion: Create GitHub issues to track these and remove the inline TODOs. This keeps the codebase clean and makes TODOs more visible/trackable.

2. Error Message Context

Location: src/aignostics_foundry_core/database.py:152

The error message in _resolve_db_params() could be more helpful:

msg = f"No database URL configured. Set {ctx.env_prefix}DB_URL or pass db_url explicitly."

Suggestion: Include the context name for better debugging:

msg = f"No database URL configured for {ctx.name}. Set {ctx.env_prefix}DB_URL or pass db_url explicitly."

3. Type Annotation Precision

Location: src/aignostics_foundry_core/database.py:453

The with_engine decorator has -> Any return type:

def with_engine(...) -> Any:

Suggestion: While this is acceptable for a dual-mode decorator, consider using a TypeVar or overload to provide more precise type hints for static analysis tools.

4. URL Path Replacement Edge Cases

Location: src/aignostics_foundry_core/database.py:81

The get_url() method replaces the entire path component:

return urllib.parse.urlunparse(parsed._replace(path=f"/{self.db_name}"))

Consideration: This works well for standard database URLs, but could be fragile if the path contains multiple segments (e.g., /path/to/db). However, this is unlikely with standard database connection strings, so it's acceptable as-is. Consider adding a docstring note about this behavior.

🔍 Detailed Analysis

Feature 1: Third-party Code Injection (foundry.py)

Implementation: Clean and well-designed

• Idempotent insertion into sys.path
• Silent no-op when directory doesn't exist
• Properly integrated into set_context()

Tests: Comprehensive coverage in foundry_test.py

Verdict: ✅ Excellent

Feature 2: DatabaseSettings (database.py)

Implementation: Well-architected with smart defaults

• Extends OpaqueSettings for secret masking
• Auto-derives env prefix from FoundryContext
• Optional db_name substitution for test flexibility
• Lazy imports to avoid circular dependencies (documented in code)

Integration: Seamlessly integrated into FoundryContext

• Auto-populated when {PREFIX}DB_URL is set
• None otherwise (explicit opt-in)

Backward Compatibility: All database functions maintain compatibility

• All params optional, fall back to context
• Explicit params still work as before

Tests: Excellent test coverage

• database_settings_test.py: 129 lines of tests
• Tests for URL manipulation, env resolution, pool params, secret masking
• database_test.py: 123 new lines covering context resolution

Verdict: ✅ Excellent

Code Quality

SonarQube Compliance: ✅ Pass

• String constants properly extracted (S1192)
• No new issues introduced

Test Markers: ✅ All tests properly marked

• All tests have @pytest.mark.unit, @pytest.mark.integration, or @pytest.mark.e2e

Circular Import Handling: ✅ Acceptable

• Lazy imports are documented and necessary
• Pattern is consistent across the codebase

Documentation

README.md: ✅ Excellent

• Clear examples for database usage
• Environment variable table
• Test examples with make_context()

AGENTS.md: ✅ Comprehensive

• Detailed module documentation
• Usage examples
• Integration patterns

Security

Secrets Handling: ✅ Proper

• Database URLs stored in SecretStr
• Properly masked in repr/str

No Hardcoded Credentials: ✅ Pass

Performance

Third-party Injection: ✅ Efficient

• Single sys.path check with in operator
• Idempotent, no duplicate entries

Database Pool Settings: ✅ Reasonable

• Default pool size: 10
• Default max overflow: 10
• Default timeout: 30s

Commit Quality

Commit Messages: ✅ Follow conventional commits

• feat(foundry): auto-inject third_party into sys.path
• feat: add autoconfigured DatabaseSettings
• chore: add TODOs

Final Verdict

Status: ✅ Approved (with minor suggestions)

This is a well-designed, thoroughly tested, and properly documented feature addition. The code follows project conventions, maintains backward compatibility, and passes all quality gates. The suggestions above are minor improvements that don't block merging.

Great work! 🎉

---

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
3 Accepted issues

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

See analysis details on SonarQube Cloud