fix: repair subject release tracking on existing revision 022 databases

[neubig]

HUMAN:

AGENT:

Why

Databases that applied the older revision 022 have subject_key but no subject_released_at. Because Alembic records the revision as complete, later upgrades do not revisit 022 and normal run queries fail on the missing column.

Summary

Add one forward repair migration, revision 026 after current main's revision 025. It adds the missing column and replaces the live-subject index only when needed. Databases initialized by the corrected 022 retain their existing column, index, and release timestamps.

Issue Number

Fixes #455

How to Test

uv run pytest -q tests/test_subject_release_migration.py passes all three upgrade paths: legacy 022, corrected 022, and a fresh database. It preserves existing runs and release timestamps, verifies repeated upgrade, downgrades to the canonical 022 schema, and upgrades to head again.

Changed-file Ruff, pycodestyle, Pyright, and formatting checks pass.

Notes

This is an independent compatibility migration against current main. It contains exactly one migration and follows the existing 025 migration. The affected installs used Git revisions carrying the older 022 schema; no production database was modified during validation.

[github-actions]

  ✅ **PR Artifacts Cleaned Up**

  The `.pr/` directory is no longer present.

[github-actions]

Warning

Your comment is too long (maximum is 65536 characters), so the coverage report was not added. See the job log for how to reduce it.

[all-hands-bot]

🤖 OpenHands is reviewing this PR.

Head commit: ab8780cf574fe03928fa824095049de6b92966b7
View the conversation: https://oss-agent-canvas.ngrok.dev/conversations/01e4591a-b355-47d8-b7ca-364ab58acdd3

This comment was posted by an AI agent (OpenHands).

[all-hands-bot]

This review was created by an AI agent (OpenHands) on behalf of the repository maintainers.

Review Summary

Taste Rating: 🟢 Good taste

This is a clean, well-designed repair migration. The problem is real: two versions of revision 022 were deployed, and databases that applied the older one are missing the subject_released_at column, causing ORM queries to fail. The fix is minimal and correct.

Migration 023 Analysis

• Idempotency: upgrade() inspects the table columns and returns early if subject_released_at already exists. This correctly handles databases that got the newer 022 (no-op) and databases that already ran 023 (no-op on repeat).
• Index recreation: The legacy 022 index had a PostgreSQL-only predicate (subject_key IS NOT NULL) and was missing both the subject_released_at IS NULL condition and the sqlite_where clause. The migration drops and recreates the index with the full predicate on both dialects, matching the canonical schema in models.py and current 022.
• Downgrade: pass is correct — current 022 already owns this schema, so returning to 022 preserves it. 022's own downgrade removes both columns.
• Cross-database compatibility: Uses generic SQLAlchemy types (sa.DateTime, sa.String) and dialect-specific index clauses, consistent with the repo's migration conventions.

Test Coverage

The test file is thorough and uses real code paths — no mocks. It covers:

• Legacy 022 schema (reproduces the failure, then verifies the fix)
• Current 022 schema (early-return path)
• Fresh database (full migration chain)
• Existing run and release timestamp preservation
• Repeated upgrade (idempotency)
• Downgrade to 022 followed by re-upgrade

PostgreSQL is not exercised in tests, but the migration uses identical SQLAlchemy/Alembic patterns to the current 022, which is acceptable.

No material issues found. No actionable inline findings.

---

[RISK ASSESSMENT]

• Overall PR: 🟢 LOW
  • The migration is additive, idempotent, and only modifies databases with the specific legacy schema. Databases on current 022 get an early return. Tests pass and cover the key scenarios.

VERDICT: ✅ Worth merging

KEY INSIGHT: The column-existence guard makes the migration safely idempotent across all three database states (legacy 022, current 022, fresh), which is the correct approach for repairing a split-revision deployment.

---

Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:

1. Add a .agents/skills/custom-codereview-guide.md file to your branch (or edit it if one already exists) with the /codereview trigger and the context the reviewer is missing (e.g., "Security concerns about X do not apply here because Y"). See the customization docs for the required frontmatter format.
2. Re-request a review - the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
3. When your PR is merged, the guideline file goes through normal code review by repository maintainers.

Resolve with AI? Install the iterate skill in your agent and run /iterate to automatically drive this PR through CI, review, and QA until it's merge-ready.

Was this review helpful? React with 👍 or 👎 to give feedback.

Automated review used the wrong decision (APPROVED instead of COMMENT) and is dismissed. Findings are reposted as a comment.

[all-hands-bot]

🤖 OpenHands is reviewing this PR.

Head commit: fe311b6cc268e845e295e24050e140bcd66e5b39
View the conversation: https://oss-agent-canvas.ngrok.dev/conversations/93e75ecd-a44d-4d2a-8968-ae827bfa6cce

This comment was posted by an AI agent (OpenHands).

[all-hands-bot]

🤖 OpenHands is reviewing this PR.

Head commit: fe311b6cc268e845e295e24050e140bcd66e5b39
View the conversation: https://oss-agent-canvas.ngrok.dev/conversations/8724242f-4ac5-4990-9d76-04cf918d3eb6

This comment was posted by an AI agent (OpenHands).

[all-hands-bot]

This review was created by an AI agent (OpenHands) on behalf of the repository maintainers.

Summary

Migration 023 repairs databases that applied the legacy version of revision 022 (which added subject_key but not subject_released_at) by conditionally adding the missing column and recreating the partial index with the correct predicate. Databases that already applied the current 022 are unaffected thanks to an early-return idempotency guard.

Analysis

Taste Rating: 🟢 Good taste

The migration is well-designed:

• Idempotency guard: upgrade() inspects existing columns and returns early if subject_released_at already exists. This correctly handles both legacy and current 022 databases in a single code path.
• Index repair: The legacy index had only postgresql_where (no sqlite_where) and used a simpler predicate. The drop+recreate with both postgresql_where and sqlite_where matching the model definition is correct and cross-database compatible per the repo's migration guidelines.
• No-op downgrade: Correct — current 022 already owns this schema, so downgrading to 022 preserves it. 022's own downgrade removes both columns.
• Data preservation: The migration only adds a nullable column and rebuilds an index. No data is touched.

Tests are thorough: The parametrized test covers legacy 022, current 022, and fresh databases, verifying run preservation, correct subject_released_at values, index predicate correctness, repeated upgrade idempotency, and downgrade+re-upgrade. These exercise real Alembic migration paths, not mocks.

No material issues found. No security concerns. No breaking changes.

[RISK ASSESSMENT]

• Overall PR: 🟢 LOW
  Schema repair migration with idempotency guard, comprehensive tests, and no data loss risk. The only databases affected are those that applied legacy 022; all others are no-ops.

VERDICT: ✅ Worth merging

KEY INSIGHT:
The column-existence guard is the right pattern for a repair migration that must be idempotent across databases in different schema states from the same revision number.

[all-hands-bot]

This review was created by an AI agent (OpenHands) on behalf of the repository maintainers.

Review Summary

Taste Rating: 🟢 Good taste

This is a clean, correct repair migration for a real split-revision deployment problem. Two versions of revision 022 were shipped — the older one added subject_key but not subject_released_at — and databases that applied it break on any ORM query touching AutomationRun.

Migration 023 Analysis

• Idempotency: upgrade() inspects the table columns and returns early if subject_released_at already exists. This correctly handles databases that got the newer 022 (no-op) and databases that already ran 023 (no-op on repeat).
• Index recreation: The legacy 022 index had a PostgreSQL-only predicate (subject_key IS NOT NULL) and was missing the subject_released_at IS NULL condition and the sqlite_where clause. The migration drops and recreates the index with the full predicate on both dialects, matching the canonical schema in models.py and current 022.
• Downgrade: pass is correct — current 022 already owns this schema, so returning to 022 preserves it. 022's own downgrade removes both columns.
• Cross-database compatibility: Uses generic SQLAlchemy types and dialect-specific index clauses, consistent with the repo's migration conventions.

Test Coverage

The test file is thorough and uses real code paths — no mocks. It covers:

• Legacy 022 schema (reproduces the missing-column failure, then verifies the fix)
• Current 022 schema (early-return path)
• Fresh database (full migration chain)
• Existing run and release timestamp preservation
• Repeated upgrade (idempotency)
• Downgrade to 022 followed by re-upgrade
• Index predicate verification on SQLite

PostgreSQL is not exercised in tests, but the migration uses identical SQLAlchemy/Alembic patterns to current 022, which is acceptable.

All 3 test cases pass. No material issues found.

---

[RISK ASSESSMENT]

• Overall PR: 🟢 LOW
  • The migration is additive, idempotent, and only modifies databases with the specific legacy schema. Databases on current 022 get an early return. Tests pass and cover the key scenarios.

VERDICT: ✅ Worth merging

KEY INSIGHT: The column-existence guard makes the migration safely idempotent across all three database states (legacy 022, current 022, fresh), which is the correct approach for repairing a split-revision deployment.

---

Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:

1. Add a .agents/skills/custom-codereview-guide.md file to your branch (or edit it if one already exists) with the /codereview trigger and the context the reviewer is missing (e.g., "Security concerns about X do not apply here because Y"). See the customization docs for the required frontmatter format.
2. Re-request a review - the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
3. When your PR is merged, the guideline file goes through normal code review by repository maintainers.

Resolve with AI? Install the iterate skill in your agent and run /iterate to automatically drive this PR through CI, review, and QA until it's merge-ready.

Was this review helpful? React with 👍 or 👎 to give feedback.

Automated review used the wrong decision (APPROVED instead of COMMENT) and is dismissed. Findings are reposted as a comment.

[all-hands-bot]

Posted an APPROVED review to GitHub PR #456 (OpenHands/automation). The review assessed migration 023 as a clean, correct, well-tested repair for the split-revision 022 deployment problem. No material issues found. Risk: 🟢 LOW. Verdict: Worth merging.

This comment was posted by an AI agent (OpenHands).

[all-hands-bot]

👋 This PR needs a couple of things fixed before OpenHands can review it:

• the PR description's HUMAN: section needs at least 20 characters describing what you tested, not just the template placeholder

Push an update once this is addressed and this check re-runs automatically.

This is an automated check - no AI was used to generate this comment.

[neubig]

Closing because the malformed revision 022 existed only on the unmerged vasco/external-conversations feature branch and was never included in a release. The first revision 022 merged to main already contained the complete schema, so no supported release upgrade requires this permanent repair migration.