Share bounty schema column migrations

[pqmfei]

Bounty #846

Summary

• add a small private helper for legacy bounties column migrations in create_schema()
• drive the existing legacy bounty column additions through one compact migration table
• preserve the paid-bounty backfill for awards_paid and the submission unique-index creation behavior
• tighten the existing legacy schema regression test to cover GitHub paid-issue finalization columns and the submission unique index

Duplicate check

• MRWK bounty: 100 MRWK - code cleanup and maintainability improvements #846 public bounty API showed live/open with 8 effective awards remaining before submission
• gh search prs db.py --repo ramimbo/mergework --state open -> no results
• gh search prs migrate_schema --repo ramimbo/mergework --state open -> no results
• gh search prs "schema migration" --repo ramimbo/mergework --state open -> no results

Validation

• .\.venv\Scripts\python.exe -m pytest tests\test_ledger.py::test_create_schema_migrates_existing_bounty_columns_and_submission_index tests\test_migrations.py -q -> 2 passed
• .\.venv\Scripts\python.exe -m ruff check app\db.py tests\test_ledger.py tests\test_migrations.py -> passed
• .\.venv\Scripts\python.exe -m ruff format --check app\db.py tests\test_ledger.py tests\test_migrations.py -> 3 files already formatted
• .\.venv\Scripts\python.exe -m mypy app\db.py -> success
• git diff --check -> clean

Scope: maintainability-only cleanup in schema migration helper code. No migration target columns, ledger behavior, wallet behavior, treasury behavior, payout behavior, proposal execution, private data, exchange, bridge, cash-out, or MRWK price behavior changed.

Summary by CodeRabbit

• Refactor

  • Streamlined database schema migration logic for bounties table with improved efficiency

• Tests

  • Enhanced schema migration tests to validate bounty field data migrations and submissions index integrity

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 52e927c3-1583-4eca-bda6-3e86b2284100

📥 Commits

Reviewing files that changed from the base of the PR and between d4d0e48 and a89a582.

📒 Files selected for processing (2)

• app/db.py
• tests/test_ledger.py

---

📝 Walkthrough

Walkthrough

This PR refactors the bounties table column migration in app/db.py to use a helper function and declarative migration map, replacing independent conditional blocks. The corresponding schema migration test is updated to validate both the refactored column migration logic and a new submissions table unique index assertion.

Changes

Schema Migration Refactor

Layer / File(s)	Summary
Bounty column migration helper and refactored logic app/db.py	Added Connection import and introduced _add_bounty_column_if_missing helper function. Refactored bounties table column migration to use a declarative bounty_column_migrations map instead of independent if column not in bounty_columns blocks, with backfill SQL executed only when columns are newly added.
Schema migration test with index inspection tests/test_ledger.py	Updated imports to include inspect from SQLAlchemy. Replaced test_create_schema_migrates_existing_bounty_award_columns with test_create_schema_migrates_existing_bounty_columns_and_submission_index. New test verifies migrated bounty row fields including GitHub paid-issue finalization columns and asserts the presence of a uq_submission_bounty_url unique index on the submissions table.

Possibly related PRs

• ramimbo/mergework#749: Introduced GitHub paid-issue finalization columns to the bounties table migration that this PR's refactored logic now consolidates.

🚥 Pre-merge checks | ✅ 6

✅ Passed checks (6 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Share bounty schema column migrations' directly and concretely names the changed surface (schema column migrations for bounties table).
Description check	✅ Passed	The description includes all required sections: Summary with clear bullet points, Evidence covering confusion/bug addressed and validation approach, Test Evidence with checkboxes for code quality and test runs, and MRWK reference (Bounty #846).
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Mergework Public Artifact Hygiene	✅ Passed	No investment, price, cash-out, off-ramp, fabricated payout, or private security claims found in code or PR description; changes are purely technical database schema refactoring.
Bounty Pr Focus	✅ Passed	PR diff matches stated files and implements bounty schema migrations as described; test validates logic; no scope creep detected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[catcherintheroad-hub]

Reviewed current head a89a582b2218e7a00a6ae8ee5fa7afa9850fef4a for the schema-migration helper change.

What I checked:

• app/db.py keeps the existing legacy bounty-column migration behavior while consolidating the repeated add-column path into _add_bounty_column_if_missing().
• The helper refreshes existing_columns after a successful ALTER TABLE, so later migration decisions in the same schema pass still see the updated table state.
• The awards_paid backfill remains scoped to the case where that column was just created, matching the previous migration behavior.
• Submission unique-index creation is still separate from the bounty-column migration loop.
• tests/test_ledger.py now covers both migrated bounty columns and the submission unique index on an existing database.

Local validation run:

• uv run --python 3.12 --extra dev python -m pytest tests/test_ledger.py::test_create_schema_migrates_existing_bounty_columns_and_submission_index tests/test_migrations.py -q -> 2 passed
• uv run --python 3.12 --extra dev python -m pytest tests/test_ledger.py tests/test_migrations.py -q -> 23 passed
• uv run --python 3.12 --extra dev ruff check app/db.py tests/test_ledger.py tests/test_migrations.py -> passed
• uv run --python 3.12 --extra dev ruff format --check app/db.py tests/test_ledger.py tests/test_migrations.py -> passed
• uv run --python 3.12 --extra dev mypy app/db.py -> passed
• uv run --python 3.12 --extra dev python scripts/docs_smoke.py -> passed
• git diff --check origin/main...HEAD and git merge-tree --write-tree origin/main HEAD -> clean

Caveat: GitHub currently reports merge state as UNSTABLE because only the CodeRabbit status is visible on the PR from my side, but the PR is mergeable locally and the focused/broader validation above passed.

This looks safe to accept for the cleanup bounty scope.

[MolhamHamwi]

Reviewed current head a89a5822b926585e28539e084f1f4a16f70b7205 for the #838 review bounty. I checked app/db.py and tests/test_ledger.py, and ran python3 -m pytest tests/test_ledger.py -q locally: 22 passed.

Two concrete observations:

• The new _add_bounty_column_if_missing helper updates the in-memory bounty_columns set after each ALTER TABLE, so the migration loop remains idempotent even if adjacent column entries are refactored or reused later in the same pass.
• The awards_paid backfill is still gated on added for that column, preserving the old-schema-only behavior and avoiding a repeated status = 'paid' overwrite on databases that already have the column.

The added regression also verifies both newly shared bounty columns and the existing uq_submission_bounty_url index creation path against a legacy table shape. I do not see a blocker in this refactor.

[heickerv1001-dev]

Bounty #838 / Bounty #798 follow-up on PR #905: the new unique index on submissions (bounty_id, url) can break startup on legacy databases that already contain duplicate rows. I reproduced the failure path with SQLite: duplicate rows cause CREATE UNIQUE INDEX IF NOT EXISTS uq_submission_bounty_url ON submissions (bounty_id, url) to raise UNIQUE constraint failed: submissions.bounty_id, submissions.url. The migration currently assumes clean historical data and does not dedupe or otherwise guard the index creation, so this is a real upgrade-risk regression in app/db.py.

[Errordog2]

Reviewed the bounty schema column migration sharing change.

Validation performed locally:

• .venv\Scripts\python.exe -m pytest tests/test_ledger.py -k "schema or bounty_columns or migration or submission_index" -> 1 passed
• .venv\Scripts\python.exe -m pytest tests/test_ledger.py -> 22 passed
• .venv\Scripts\python.exe -m ruff check app/db.py tests/test_ledger.py -> passed
• .venv\Scripts\python.exe -m ruff format --check app/db.py tests/test_ledger.py -> passed
• .venv\Scripts\python.exe -m mypy app/db.py -> passed
• git diff --check -> passed
• git merge-tree $(git merge-base origin/main HEAD) origin/main HEAD -> clean merge

The migration helper keeps each ALTER TABLE idempotent via the tracked column set, preserves the paid-bounty awards backfill only when awards_paid is newly added, and the expanded migration test covers the GitHub paid issue columns plus the submissions uniqueness index. Looks good to me.