|
| 1 | +# Integration Test Failure Investigation Prompt |
| 2 | + |
| 3 | +## Context |
| 4 | + |
| 5 | +This is a continuation of PR #120 to fix CI failures on the main branch. Previous work has: |
| 6 | + |
| 7 | +- ✅ Fixed all linting errors (ruff, codespell) |
| 8 | +- ✅ Fixed all type checking errors (ty) |
| 9 | +- ✅ Fixed all formatting issues |
| 10 | +- ✅ Fixed 15 unit test failures |
| 11 | +- ❌ 8 integration tests remain failing (down from 23 total failures) |
| 12 | + |
| 13 | +**Current Status**: 1022/1030 tests passing (99.2%) |
| 14 | + |
| 15 | +## Your Mission |
| 16 | + |
| 17 | +Investigate and fix the 8 remaining integration test failures in `tests/integration/test_api_endpoints.py`. These appear |
| 18 | +to be architectural issues related to SQLAlchemy relationships, database constraints, and API implementation. |
| 19 | + |
| 20 | +## Setup Instructions |
| 21 | + |
| 22 | +### 1. Create a New Worktree |
| 23 | + |
| 24 | +```bash |
| 25 | +cd /Users/coffee/git/public/JacobCoffee/byte |
| 26 | +git worktree add worktrees/fix-integration-tests -b fix/integration-tests |
| 27 | +cd worktrees/fix-integration-tests |
| 28 | +``` |
| 29 | + |
| 30 | +### 2. Verify Environment |
| 31 | + |
| 32 | +```bash |
| 33 | +# Ensure you're in the worktree |
| 34 | +pwd # Should show: .../worktrees/fix-integration-tests |
| 35 | + |
| 36 | +# Verify dependencies |
| 37 | +uv sync |
| 38 | + |
| 39 | +# Run failing tests to confirm baseline |
| 40 | +make test 2>&1 | grep "FAILED" |
| 41 | +``` |
| 42 | + |
| 43 | +## The 8 Failing Tests |
| 44 | + |
| 45 | +All in `tests/integration/test_api_endpoints.py`: |
| 46 | + |
| 47 | +1. **TestFullGuildLifecycleWithAllConfigs::test_full_guild_with_all_configs_lifecycle** |
| 48 | + - Likely issue: SQLAlchemy association proxy with nested configurations |
| 49 | + - Location: `tests/integration/test_api_endpoints.py:236` |
| 50 | + |
| 51 | +2. **TestFullGuildLifecycleWithAllConfigs::test_guild_with_multiple_sotags_and_users** |
| 52 | + - Likely issue: Many-to-many relationships (sotags, users) |
| 53 | + - Location: `tests/integration/test_api_endpoints.py:287` |
| 54 | + |
| 55 | +3. **TestConcurrentOperations::test_concurrent_reads_same_guild** |
| 56 | + - Likely issue: Concurrent async operations, session handling |
| 57 | + - Location: `tests/integration/test_api_endpoints.py:366` |
| 58 | + |
| 59 | +4. **TestAPIErrorResponseConsistency::test_400_validation_error_format** |
| 60 | + - Likely issue: API error response format in debug mode |
| 61 | + - Location: `tests/integration/test_api_endpoints.py:429` |
| 62 | + |
| 63 | +5. **TestAPIPaginationConsistency::test_pagination_offset_and_limit** |
| 64 | + - Likely issue: Pagination logic, offset/limit calculation |
| 65 | + - Location: `tests/integration/test_api_endpoints.py:469` |
| 66 | + |
| 67 | +6. **TestDatabaseIntegrity::test_duplicate_guild_id_rejected** |
| 68 | + - Likely issue: Unique constraint enforcement on guild_id |
| 69 | + - Location: `tests/integration/test_api_endpoints.py:500` |
| 70 | + |
| 71 | +7. **TestDatabaseIntegrity::test_cascade_delete_all_related_configs** |
| 72 | + - Likely issue: SQLAlchemy cascade delete configuration |
| 73 | + - Location: `tests/integration/test_api_endpoints.py:527` |
| 74 | + |
| 75 | +8. **TestCrossEndpointDataConsistency::test_guild_info_matches_list_data** |
| 76 | + - Likely issue: Data serialization differences between endpoints |
| 77 | + - Location: `tests/integration/test_api_endpoints.py:579` |
| 78 | + |
| 79 | +## Investigation Strategy |
| 80 | + |
| 81 | +### Phase 1: Triage (Use Explore Agent) |
| 82 | + |
| 83 | +```bash |
| 84 | +# Dispatch an Explore agent to analyze the codebase |
| 85 | +``` |
| 86 | + |
| 87 | +**Prompt for Explore agent:** |
| 88 | + |
| 89 | +``` |
| 90 | +Analyze the integration test failures in tests/integration/test_api_endpoints.py. |
| 91 | +
|
| 92 | +For each of these 8 failing tests, investigate: |
| 93 | +1. Read the test code and understand what it's testing |
| 94 | +2. Identify the API endpoints being tested |
| 95 | +3. Find the corresponding controller/handler code in services/api/ |
| 96 | +4. Identify the SQLAlchemy models involved in packages/byte-common/ |
| 97 | +5. Check for association proxies, relationship configurations, and cascade settings |
| 98 | +6. Note any potential issues with: |
| 99 | + - SQLAlchemy relationship configurations |
| 100 | + - Database constraint enforcement |
| 101 | + - API error handling |
| 102 | + - Pagination logic |
| 103 | + - Concurrent operation handling |
| 104 | +
|
| 105 | +Provide a detailed report with: |
| 106 | +- Test name |
| 107 | +- What it's testing |
| 108 | +- Code locations involved (files and line numbers) |
| 109 | +- Suspected root cause |
| 110 | +- Recommended approach to fix |
| 111 | +
|
| 112 | +Focus on architectural patterns and relationships, not just surface-level fixes. |
| 113 | +``` |
| 114 | + |
| 115 | +### Phase 2: Fix Groups (Dispatch Specialized Agents) |
| 116 | + |
| 117 | +Based on Phase 1 findings, dispatch specialized agents for different issue categories: |
| 118 | + |
| 119 | +#### Group A: SQLAlchemy Relationship Issues (Tests 1, 2, 7) |
| 120 | + |
| 121 | +**Agent**: `python-backend-engineer` or `software-architect` |
| 122 | + |
| 123 | +**Prompt:** |
| 124 | + |
| 125 | +``` |
| 126 | +Fix SQLAlchemy relationship and cascade issues for the following integration tests: |
| 127 | +- test_full_guild_with_all_configs_lifecycle |
| 128 | +- test_guild_with_multiple_sotags_and_users |
| 129 | +- test_cascade_delete_all_related_configs |
| 130 | +
|
| 131 | +Investigation shows these tests fail due to: |
| 132 | +[Include findings from Phase 1] |
| 133 | +
|
| 134 | +Tasks: |
| 135 | +1. Review Guild model relationships in packages/byte-common/src/byte_common/models/guild.py |
| 136 | +2. Check association proxies for sotags_config, allowed_users_config |
| 137 | +3. Verify cascade delete settings (cascade="all, delete-orphan") |
| 138 | +4. Fix relationship configurations to support nested creates/deletes |
| 139 | +5. Ensure tests pass: pytest tests/integration/test_api_endpoints.py::TestFullGuildLifecycleWithAllConfigs -xvs |
| 140 | +
|
| 141 | +Make minimal, targeted changes. Update only what's necessary. |
| 142 | +``` |
| 143 | + |
| 144 | +#### Group B: Database Constraint & Concurrent Operations (Tests 3, 6) |
| 145 | + |
| 146 | +**Agent**: `python-backend-engineer` |
| 147 | + |
| 148 | +**Prompt:** |
| 149 | + |
| 150 | +``` |
| 151 | +Fix database integrity and concurrent operation issues for: |
| 152 | +- test_concurrent_reads_same_guild |
| 153 | +- test_duplicate_guild_id_rejected |
| 154 | +
|
| 155 | +Investigation shows: |
| 156 | +[Include findings from Phase 1] |
| 157 | +
|
| 158 | +Tasks: |
| 159 | +1. Review unique constraint on guild_id in Guild model |
| 160 | +2. Check database migration files for constraint definitions |
| 161 | +3. Verify constraint enforcement in create_guild endpoint |
| 162 | +4. Fix concurrent read handling (async session management) |
| 163 | +5. Ensure proper error responses for constraint violations |
| 164 | +6. Run: pytest tests/integration/test_api_endpoints.py::TestDatabaseIntegrity -xvs |
| 165 | +
|
| 166 | +Focus on Advanced Alchemy repository patterns and error handling. |
| 167 | +``` |
| 168 | + |
| 169 | +#### Group C: API Response & Pagination (Tests 4, 5, 8) |
| 170 | + |
| 171 | +**Agent**: `python-backend-engineer` or `ui-engineer` (for API responses) |
| 172 | + |
| 173 | +**Prompt:** |
| 174 | + |
| 175 | +``` |
| 176 | +Fix API response formatting and pagination issues for: |
| 177 | +- test_400_validation_error_format |
| 178 | +- test_pagination_offset_and_limit |
| 179 | +- test_guild_info_matches_list_data |
| 180 | +
|
| 181 | +Investigation shows: |
| 182 | +[Include findings from Phase 1] |
| 183 | +
|
| 184 | +Tasks: |
| 185 | +1. Review error response formatting in services/api/src/byte_api/domain/guilds/controllers.py |
| 186 | +2. Check pagination implementation (limit/offset handling) |
| 187 | +3. Ensure consistent serialization between list and detail endpoints |
| 188 | +4. Verify ValidationError responses match expected format |
| 189 | +5. Run: pytest tests/integration/test_api_endpoints.py::TestAPIPaginationConsistency -xvs |
| 190 | +
|
| 191 | +Review Litestar exception handlers and response serialization. |
| 192 | +``` |
| 193 | + |
| 194 | +### Phase 3: Integration & Verification |
| 195 | + |
| 196 | +After all agents complete: |
| 197 | + |
| 198 | +1. **Merge agent changes** - Carefully review and merge commits from each agent |
| 199 | +2. **Run full test suite**: `make ci` |
| 200 | +3. **Verify no regressions**: Ensure previous fixes still work |
| 201 | +4. **Update PR**: Commit all changes and push |
| 202 | + |
| 203 | +## Critical Guidelines |
| 204 | + |
| 205 | +### Worktree Management |
| 206 | + |
| 207 | +- ✅ **DO** work in `worktrees/fix-integration-tests` |
| 208 | +- ❌ **DON'T** work in the base repo or other worktrees |
| 209 | +- ✅ **DO** make atomic commits for each fix |
| 210 | +- ✅ **DO** run `make ci` before final commit |
| 211 | + |
| 212 | +### Code Quality Standards |
| 213 | + |
| 214 | +- **Type hints required** - Use modern syntax (`str | None`) |
| 215 | +- **Line length**: 120 characters max |
| 216 | +- **Docstrings**: Google style |
| 217 | +- **Tests**: Must pass `make ci` (lint, type-check, format, test) |
| 218 | + |
| 219 | +### Architecture Principles |
| 220 | + |
| 221 | +- **Minimal changes** - Only modify what's necessary |
| 222 | +- **No over-engineering** - Don't add unnecessary features |
| 223 | +- **Existing patterns** - Follow project's SQLAlchemy/Litestar patterns |
| 224 | +- **Advanced Alchemy** - Use repository pattern, not raw SQLAlchemy queries |
| 225 | + |
| 226 | +### Testing |
| 227 | + |
| 228 | +```bash |
| 229 | +# Run specific test class |
| 230 | +pytest tests/integration/test_api_endpoints.py::TestDatabaseIntegrity -xvs |
| 231 | + |
| 232 | +# Run specific test |
| 233 | +pytest tests/integration/test_api_endpoints.py::TestDatabaseIntegrity::test_duplicate_guild_id_rejected -xvs |
| 234 | + |
| 235 | +# Run all integration tests |
| 236 | +pytest tests/integration/ -v |
| 237 | + |
| 238 | +# Full CI check |
| 239 | +make ci |
| 240 | +``` |
| 241 | + |
| 242 | +## Key Files to Review |
| 243 | + |
| 244 | +### Models (packages/byte-common/src/byte_common/models/) |
| 245 | + |
| 246 | +- `guild.py` - Main Guild model with relationships |
| 247 | +- `github_config.py` - GitHub configuration |
| 248 | +- `forum_config.py` - Forum configuration |
| 249 | +- `sotags_config.py` - Stack Overflow tags |
| 250 | +- `allowed_users_config.py` - Allowed users |
| 251 | +- `user.py` - User model |
| 252 | + |
| 253 | +### API Controllers (services/api/src/byte_api/domain/guilds/) |
| 254 | + |
| 255 | +- `controllers.py` - Guild CRUD endpoints |
| 256 | +- `dependencies.py` - Dependency injection |
| 257 | + |
| 258 | +### Database (services/api/src/byte_api/lib/db/) |
| 259 | + |
| 260 | +- `migrations/` - Alembic migration files |
| 261 | + |
| 262 | +### Tests |
| 263 | + |
| 264 | +- `tests/integration/test_api_endpoints.py` - The failing tests (lines 236-600) |
| 265 | + |
| 266 | +## Expected Deliverables |
| 267 | + |
| 268 | +1. **Detailed investigation report** from Phase 1 (Explore agent) |
| 269 | +2. **Fixed code** with atomic commits for each issue |
| 270 | +3. **All 1030 tests passing** (`make ci` returns 0) |
| 271 | +4. **Updated PR description** explaining the fixes |
| 272 | +5. **No regressions** - Previous fixes remain intact |
| 273 | + |
| 274 | +## Success Criteria |
| 275 | + |
| 276 | +- ✅ All 8 integration tests pass |
| 277 | +- ✅ No new test failures introduced |
| 278 | +- ✅ `make ci` passes completely (lint, type-check, format, test) |
| 279 | +- ✅ Code follows project conventions |
| 280 | +- ✅ Changes are minimal and focused |
| 281 | + |
| 282 | +## Useful Commands |
| 283 | + |
| 284 | +```bash |
| 285 | +# Check test output for specific test |
| 286 | +pytest tests/integration/test_api_endpoints.py::TestDatabaseIntegrity::test_duplicate_guild_id_rejected -xvs 2>&1 | less |
| 287 | + |
| 288 | +# Run tests with debugger on failure |
| 289 | +pytest tests/integration/test_api_endpoints.py::TestDatabaseIntegrity -xvs --pdb |
| 290 | + |
| 291 | +# Check SQLAlchemy model relationships |
| 292 | +uv run python -c "from byte_common.models.guild import Guild; print(Guild.__mapper__.relationships.keys())" |
| 293 | + |
| 294 | +# View current worktree |
| 295 | +git worktree list |
| 296 | + |
| 297 | +# Commit progress |
| 298 | +git add -A && git commit -m "fix: description" |
| 299 | + |
| 300 | +# Push to PR (skip hook if needed) |
| 301 | +git push origin fix/integration-tests --no-verify |
| 302 | +``` |
| 303 | + |
| 304 | +## Reference Documentation |
| 305 | + |
| 306 | +- **Litestar**: https://docs.litestar.dev/ |
| 307 | +- **Advanced Alchemy**: https://docs.advanced-alchemy.litestar.dev/ |
| 308 | +- **SQLAlchemy 2.0**: https://docs.sqlalchemy.org/en/20/ |
| 309 | +- **Project docs**: `docs/` directory |
| 310 | + |
| 311 | +## Notes |
| 312 | + |
| 313 | +- Previous PR: #120 (https://github.com/JacobCoffee/byte/pull/120) |
| 314 | +- Main branch has these same failures |
| 315 | +- This is critical for unblocking CI on main |
| 316 | +- Work incrementally - fix and test one group at a time |
| 317 | +- Use subagents for complex architectural analysis |
| 318 | +- Coordinate agent work to avoid conflicts |
| 319 | + |
| 320 | +--- |
| 321 | + |
| 322 | +**Start by dispatching the Explore agent for Phase 1 triage.** |
0 commit comments