feat: add FoundryContext.version_with_vcs_ref

[olivermeyer]

• Updating tests to mostly use the default make_context() parameters
• Adding version_with_vcs_ref attribute to FoundryContext
• Update user_agent() to use context

[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/foundry.py	96.82% <100.00%> (+0.27%)	⬆️
src/aignostics_foundry_core/user_agent.py	100.00% <100.00%> (ø)

[claude]

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

---

PR Review Complete

I've reviewed PR #22: feat: add FoundryContext.version_with_vcs_ref

---

Summary

This PR makes three well-organized improvements:

1. Test refactoring: Standardizes test setup using make_context() helper (be61093)
2. New feature: Adds version_with_vcs_ref attribute to FoundryContext (83956d5)
3. API improvement: Updates user_agent() to accept context parameter (9c7e48a)

Overall assessment: ✅ High-quality implementation with comprehensive tests and good documentation.

---

Positive Aspects

✅ Excellent test coverage

• New version_with_vcs_ref field has 5 dedicated unit tests covering all edge cases
• Tests verify correct behavior with/without VCS info, with/without CI metadata
• All tests properly marked with @pytest.mark.unit

✅ Clean refactoring

• Test consolidation reduces duplication significantly (162 deletions vs 90 additions in test files)
• make_context() helper now provides consistent defaults across all tests
• Follows DRY principle effectively

✅ Good documentation

• version_with_vcs_ref has clear docstring explaining its purpose (lines 59-67)
• AGENTS.md updated to reflect new field
• Commit messages follow conventional commit format

✅ Type safety maintained

• All type hints preserved
• No type errors introduced

✅ Backward compatible

• New context parameter in user_agent() is optional with fallback to get_context()
• Existing call sites continue to work

---

Issues Found

Suggestions (Non-blocking)

1. TODO comment in user_agent.py:43

• Location: src/aignostics_foundry_core/user_agent.py:43
• Issue: TODO comment about hardcoded "python-sdk" string remains unresolved
• Suggestion: Consider addressing this or creating a tracking issue if it's important
• Not blocking: This is pre-existing technical debt

2. Test constant organization

• Location: tests/aignostics_foundry_core/foundry_test.py:17-30
• Observation: Good adherence to SonarQube S1192 rule with module-level constants
• All string literals properly extracted (e.g., VCS_REF_VALUE, COMMIT_SHA_VALUE, etc.)
• ✅ No action needed - already following best practices

3. SonarQube new issue

• The SonarQube report mentions "1 New issue" but Quality Gate passed
• Cannot determine specific issue from available information
• Recommend: Check SonarQube dashboard to identify and address if needed
• Not blocking: Quality Gate passed, so likely minor

---

Technical Review

foundry.py changes

✅ Line 59-67: version_with_vcs_ref field

• Well-documented with clear distinction from version_full
• Properly typed as str
• Frozen model ensures immutability

✅ Line 119: Constructor call updated

• Correctly calls _build_version_with_vcs_ref()
• Uses same vcs_ref variable as version_full

✅ Line 210-226: _build_version_with_vcs_ref() implementation

• Clean, focused function
• Correctly filters out "unknown" values
• Returns bare version when no VCS info available
• Mirrors _build_version_full() pattern but omits CI metadata

user_agent.py changes

✅ Line 12: New signature with context parameter

• Properly typed with TYPE_CHECKING guard
• Optional with sensible fallback
• Maintains backward compatibility

✅ Line 44: Uses ctx.version_full

• Correctly accesses context field
• Maintains existing behavior

conftest.py changes

✅ Line 89: make_context() updated

• New version_with_vcs_ref parameter properly defaulted
• Consistent with other version fields
• Breaking change contained to test helpers only

Test changes

✅ Test refactoring pattern: Tests updated across 9 files

• Replaced direct FoundryContext() calls with make_context()
• Reduced parameter passing
• Improved readability and maintainability

✅ New tests for version_with_vcs_ref:

• test_from_package_version_with_vcs_ref_equals_version_when_no_vcs_info (431)
• test_from_package_version_with_vcs_ref_includes_vcs_ref (448)
• test_from_package_version_with_vcs_ref_includes_commit_sha (458)
• test_from_package_version_with_vcs_ref_joins_vcs_ref_and_commit_sha_with_dash (472)
• test_from_package_version_with_vcs_ref_excludes_ci_metadata (483)

All tests follow best practices:

• Clear descriptive names
• Arrange-Act-Assert pattern
• Proper use of fixtures
• Explicit environment setup with monkeypatch

---

Security & Performance

✅ No security issues identified

• No hardcoded secrets
• No injection vulnerabilities
• No unsafe operations

✅ No performance concerns

• Version string building is lightweight
• No additional external calls
• Minimal memory overhead

---

Commit Message Review

✅ All commits follow conventional commit format:

1. refactor(tests): replace direct FoundryContext() calls with make_context() helper
2. feat(foundry): add version_with_vcs_ref to FoundryContext
3. refactor(user_agent): replace explicit params with context

Co-authored-by tags properly included.

---

Recommendations

1. Before merge: Check SonarQube dashboard for the "1 New issue" and address if needed
2. Optional: Consider addressing the TODO at user_agent.py:43 or creating a tracking issue
3. Validation: Ensure mise run lint and mise run test_unit pass locally (I cannot run these commands)

---

Final Verdict

✅ APPROVED with minor suggestions

This PR demonstrates high-quality engineering:

• Well-organized incremental commits
• Comprehensive test coverage (100% per Codecov)
• Clean, maintainable code
• Proper documentation
• Follows project standards

The suggested improvements are non-blocking. The code is ready to merge once the author confirms lint and tests pass.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
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