Skip to content

Conversation

@bokelley
Copy link
Contributor

Critical Bug Fix

Issue

The progress tracking feature (PR #508) created a migration head conflict causing 503 errors in production.

Symptoms

  • ❌ 503 errors when polling sync status
  • ❌ Migration error: "Multiple head revisions are present for given argument 'head'"
  • ❌ Database migrations failing on deployment
  • progress column never created in production database
  • ❌ Code accessing sync_job.progress causing AttributeError → 503

Root Cause

Two migrations both pointed to the same parent (1a7693edad5d), creating a branch:

1a7693edad5d (branchpoint)
├─→ 661c474053fa (add_gam_service_account_email)
└─→ ed7d05fea3be (add progress tracking) ← CONFLICT!

Alembic couldn't determine which migration to run when executing alembic upgrade head.

Fix

Updated progress tracking migration to depend on 661c474053fa instead of 1a7693edad5d, creating a linear chain:

1a7693edad5d → 661c474053fa → ed7d05fea3be ✅

Changes

File: alembic/versions/ed7d05fea3be_add_progress_tracking_to_sync_jobs.py

  • Changed down_revision from "1a7693edad5d" to "661c474053fa"

Verification

$ uv run alembic heads
ed7d05fea3be (head)  # ✅ Single head!

Impact

  • ✅ Migration will run successfully on next deploy
  • progress column will be created in database
  • ✅ 503 errors will be resolved
  • ✅ Progress tracking will work correctly
  • ✅ AccuWeather sync can complete and report progress

Testing

  • Pre-commit hooks: ✅ Passed (including "Check for multiple Alembic migration heads")
  • Alembic heads check: ✅ Single head confirmed
  • Migration history: ✅ Linear chain verified

Priority

🚨 HIGH - Blocks progress tracking feature from working in production

Related

🤖 Generated with Claude Code

Issue:
- Progress tracking migration created a second head (branch conflict)
- Alembic error: "Multiple head revisions are present"
- Database migrations failed on deployment
- progress column never created in production database
- Code accessing sync_job.progress caused 503 errors

Root cause:
Both migrations pointed to same parent (1a7693edad5d):
- 661c474053fa: add_gam_service_account_email_to_adapter_config
- ed7d05fea3be: Add progress tracking to sync_jobs

Fix:
Update progress tracking migration to depend on 661c474053fa
instead of 1a7693edad5d, creating linear chain:

1a7693edad5d -> 661c474053fa -> ed7d05fea3be

Result:
- Single migration head (ed7d05fea3be)
- Migration will run successfully on next deploy
- progress column will be created
- 503 errors will be resolved

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@bokelley bokelley merged commit 27bb8af into main Oct 19, 2025
8 checks passed
@bokelley
Copy link
Contributor Author

Additional Fix Added: 409 Status Handling

New commit: 8b083da8 - Fix 409 handling for sync resume functionality

Issue

When a sync was already running and user clicked "Sync Inventory" again:

  • Backend correctly returned 409 (Conflict) with {in_progress: true, sync_id: "..."}
  • But fetch() treated 409 as error by default
  • Response went to .catch() block instead of being parsed
  • User saw console error: "Failed to load resource: 409"
  • Seamless resume didn't work

Fix

Modified fetch response handler to accept 409 as valid:

.then(response => {
    // Handle both success and 409 (conflict) responses
    if (response.ok || response.status === 409) {
        return response.json();
    }
    throw new Error(`Server error: ${response.status}`);
})

Result

Now when user returns to page and clicks sync button:

  1. ✅ Backend returns 409 with existing sync_id
  2. ✅ Frontend parses JSON (no error)
  3. ✅ Detects data.in_progress === true
  4. ✅ Automatically resumes polling
  5. ✅ Shows current progress: "⏳ Discovering Ad Units: 243 items (1/6)"
  6. ✅ No console errors, no alerts

User experience: Seamless resume - feels like progress just continues!

@bokelley
Copy link
Contributor Author

This PR has been merged and deployed. The 409 handling fix has been split into a separate PR #512 for cleaner review.

EmmaLouise2018 pushed a commit that referenced this pull request Oct 24, 2025
Issue:
- Progress tracking migration created a second head (branch conflict)
- Alembic error: "Multiple head revisions are present"
- Database migrations failed on deployment
- progress column never created in production database
- Code accessing sync_job.progress caused 503 errors

Root cause:
Both migrations pointed to same parent (1a7693edad5d):
- 661c474053fa: add_gam_service_account_email_to_adapter_config
- ed7d05fea3be: Add progress tracking to sync_jobs

Fix:
Update progress tracking migration to depend on 661c474053fa
instead of 1a7693edad5d, creating linear chain:

1a7693edad5d -> 661c474053fa -> ed7d05fea3be

Result:
- Single migration head (ed7d05fea3be)
- Migration will run successfully on next deploy
- progress column will be created
- 503 errors will be resolved

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude <noreply@anthropic.com>
danf-newton pushed a commit to Newton-Research-Inc/salesagent that referenced this pull request Nov 24, 2025
Issue:
- Progress tracking migration created a second head (branch conflict)
- Alembic error: "Multiple head revisions are present"
- Database migrations failed on deployment
- progress column never created in production database
- Code accessing sync_job.progress caused 503 errors

Root cause:
Both migrations pointed to same parent (1a7693edad5d):
- 661c474053fa: add_gam_service_account_email_to_adapter_config
- ed7d05fea3be: Add progress tracking to sync_jobs

Fix:
Update progress tracking migration to depend on 661c474053fa
instead of 1a7693edad5d, creating linear chain:

1a7693edad5d -> 661c474053fa -> ed7d05fea3be

Result:
- Single migration head (ed7d05fea3be)
- Migration will run successfully on next deploy
- progress column will be created
- 503 errors will be resolved

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants