fix(migrations): harden BAT-025 PostgreSQL verification
This commit is contained in:
+41
-34
@@ -2,47 +2,54 @@
|
||||
|
||||
- Date: 2026-07-15
|
||||
- Assignee: dev-impl
|
||||
- Scope: REQ-057 portfolio_ledger 수동 장부 메타데이터 명시 Alembic migration
|
||||
- Scope: REQ-057 `portfolio_ledger` 수동 장부 메타데이터의 명시 Alembic migration
|
||||
- Implementation state: remediation complete; independent QC pending
|
||||
|
||||
## Implemented Changes
|
||||
## Baseline and remediation provenance
|
||||
|
||||
1. Created new Alembic revision `20260715_0028` (`migrations/versions/20260715_0028_portfolio_ledger_manual_metadata.py`)
|
||||
- `down_revision`: `20260628_0027`
|
||||
- Added additive columns:
|
||||
- `source TEXT NOT NULL DEFAULT 'broker'`
|
||||
- `target_ledger_entry_id BIGINT NULL`
|
||||
- `confirmation_evidence TEXT NULL`
|
||||
- `actor_user_id TEXT NULL`
|
||||
- Added idempotent runtime compatibility logic:
|
||||
- If `portfolio_ledger` table missing: no-op
|
||||
- If column already exists: update `source IS NULL` → `'broker'`, enforce `NOT NULL` + `DEFAULT`
|
||||
- Add missing columns only when absent
|
||||
- `downgrade` is forward-only no-op
|
||||
- Initial implementation baseline: `f6195ff8090d8761d467e15f44255642ca350e80`.
|
||||
- The previously recorded SHA `850e970` does **not** identify the reviewed implementation and is superseded by the baseline above.
|
||||
- The remediation below is deliberately **uncommitted** in this worktree. No push, PR, main merge, production migration, deployment, or service activation was performed.
|
||||
|
||||
2. Added migration-focused tests `tests/test_bat025_portfolio_ledger_manual_metadata_migration.py`
|
||||
- Covers:
|
||||
- fresh schema add-all-columns behavior
|
||||
- precreated schema backfill + source constraint enforcement
|
||||
- missing table no-op
|
||||
- forward-only downgrade no-op
|
||||
## Implemented changes
|
||||
|
||||
3. Added operational runbook
|
||||
1. Explicit revision `20260715_0028` remains the REQ-057 head.
|
||||
- Parent: `20260628_0027`.
|
||||
- Adds `source`, `target_ledger_entry_id`, `confirmation_evidence`, and `actor_user_id` additively.
|
||||
- Handles runtime-precreated columns by backfilling nullable `source`, then converging its default and `NOT NULL` constraint.
|
||||
- Downgrade remains a forward-only no-op; no column deletion was added.
|
||||
|
||||
2. Remediation of actual PostgreSQL migration-chain/runtime behavior.
|
||||
- `migrations/versions/20260605_0025_ledger_consolidation.py`: makes the identity `ledger_entry_id` explicitly `nullable=False`. PostgreSQL identity columns are implicitly non-null; the prior explicit trailing `NULL` was incompatible with fresh real-PostgreSQL migration.
|
||||
- `app/db/migrations.py`: programmatic Alembic configuration now carries the caller-provided `Settings`. For a legacy/runtime-precreated schema, bootstrap stamps `20260628_0027` and then executes `upgrade head`, so revision `0028` performs its required source backfill/default/constraint convergence instead of being skipped by a direct head stamp.
|
||||
- `migrations/env.py`: consumes the explicit settings when supplied and preserves existing application loggers when Alembic runs in-process.
|
||||
|
||||
3. Real isolated PostgreSQL integration coverage.
|
||||
- `tests/test_bat025_portfolio_ledger_manual_metadata_migration.py` creates a UUID-named temporary database only through an explicitly supplied loopback `postgres` maintenance DSN.
|
||||
- The test rejects non-loopback hosts and any maintenance database other than `postgres`; each test terminates connections and drops only its own UUID-named database.
|
||||
- Coverage includes pre-0028 upgrade/data preservation, runtime-precreated nullable `source` convergence and Alembic current-head state, plus representative broker/manual ledger writes after upgrade.
|
||||
|
||||
4. Existing operation runbook is retained.
|
||||
- `docs/runbooks/bat-025-req057-portfolio-ledger-manual-metadata-runbook.md`
|
||||
- It remains a runbook only; no production command in it was executed.
|
||||
|
||||
## Preflight Evidence
|
||||
## Executed evidence
|
||||
|
||||
- `uv run alembic heads`
|
||||
- output: `20260715_0028 (head)`
|
||||
- `uv run alembic history --verbose`
|
||||
- confirms `20260715_0028` parent is `20260628_0027` and it is the current tail of the migration chain.
|
||||
- `PYTHONPATH=. uv run pytest -q -o addopts='' tests/test_bat025_portfolio_ledger_manual_metadata_migration.py`
|
||||
- output: `4 passed in 0.24s`
|
||||
All commands below used the local Docker PostgreSQL service only. The DSN was supplied from an environment variable and is intentionally omitted here.
|
||||
|
||||
## Evidence notes
|
||||
- `BAT025_LOCAL_POSTGRES_DSN=[REDACTED local loopback maintenance DSN] PYTHONPATH=. uv run --extra dev --with httpx2 python -m pytest -q --override-ini addopts='' -p no:cacheprovider tests/test_bat025_portfolio_ledger_manual_metadata_migration.py`
|
||||
- Result: **3 passed in 6.10s**.
|
||||
- Verifies real 0027→0028 data preservation/source backfill, runtime-precreated compatibility/current revision, and broker/manual write regression in throwaway DBs.
|
||||
- `PYTHONPATH=. uv run --extra dev --with httpx2 python -m pytest -q --override-ini addopts='' -p no:cacheprovider`
|
||||
- Result: **597 passed, 11 warnings in 41.41s**.
|
||||
- Warnings are pre-existing Starlette `HTTP_422_UNPROCESSABLE_ENTITY` deprecations in endpoint tests; no test failed.
|
||||
- `uv run --extra dev ruff check app/db/migrations.py migrations/env.py migrations/versions/20260605_0025_ledger_consolidation.py tests/test_bat025_portfolio_ledger_manual_metadata_migration.py`
|
||||
- Result: **All checks passed**.
|
||||
- `python3 -m compileall -q app migrations` and `git diff --check`
|
||||
- Result: exit 0.
|
||||
|
||||
- Migration is idempotent by design: no-op if `portfolio_ledger` is missing, and for pre-created runtimes it backfills `source` and enforces `NOT NULL DEFAULT 'broker'`.
|
||||
- No DB upgrade was executed in this scope (code-only + evidence-only in repo).
|
||||
## Limits / non-claims
|
||||
|
||||
## Commit
|
||||
|
||||
- Commit SHA: `850e970`
|
||||
- This evidence is for isolated local PostgreSQL only; it is not an operational DB upgrade.
|
||||
- Lock contention, backup restore, production access control, deployment, service activation, and M5 release remain unexecuted.
|
||||
- This log is developer evidence only. It does not mark BAT-025 verified or approved; independent QC remains required.
|
||||
|
||||
@@ -0,0 +1,48 @@
|
||||
# BAT-025 Developer Self-Verification
|
||||
|
||||
- Status: **draft — independent QC pending**
|
||||
- Batch: BAT-025
|
||||
- Requirement: REQ-057
|
||||
- Date: 2026-07-15
|
||||
- Scope boundary: local isolated PostgreSQL evidence only. No production DB, deployment, service activation, push, merge, or release action was performed.
|
||||
|
||||
## Acceptance mapping
|
||||
|
||||
| REQ-057 acceptance area | Evidence | Result |
|
||||
| --- | --- | --- |
|
||||
| Explicit `0028` revision is the chain head | `alembic_version` queried in a throwaway PostgreSQL DB after upgrade | Pass — current revision `20260715_0028` |
|
||||
| Existing broker/manual business data survives upgrade | Real `20260628_0027 → head` upgrade after a representative pre-existing ledger row is inserted; row fields and metadata are asserted after upgrade | Pass |
|
||||
| `source` is backfilled/defaulted/not-null | Real upgrade asserts preserved row `source='broker'`; schema inspection checks text type, non-null constraint, and default | Pass |
|
||||
| Runtime-precreated metadata schema converges | Real test creates all four columns with nullable `source`, invokes bootstrap, and asserts source backfill/default/not-null plus `current=head` | Pass |
|
||||
| Representative broker/manual writes still work | Post-upgrade synthetic repository writes assert broker source and manual metadata fields | Pass |
|
||||
| Earlier fresh PostgreSQL chain can reach 0028 | `0025` identity-column PostgreSQL incompatibility corrected and exercised by fresh throwaway DB migration path | Pass |
|
||||
| API/manual-ledger regression | `tests/test_v1_portfolios_manual_ledger.py`, `tests/test_v1_portfolios_trades.py`, `tests/test_v1_portfolios_ledger.py` | Pass — 33 tests |
|
||||
| Whole-repository regression | pytest with project dev extra plus temporary TestClient dependency overlay | Pass — 597 tests, 11 warnings |
|
||||
| Lint/compile/diff hygiene | Ruff, compileall, `git diff --check` | Pass |
|
||||
|
||||
## Executed commands and results
|
||||
|
||||
- `BAT025_LOCAL_POSTGRES_DSN=[REDACTED local loopback maintenance DSN] PYTHONPATH=. uv run --extra dev --with httpx2 python -m pytest -q --override-ini addopts='' -p no:cacheprovider tests/test_bat025_portfolio_ledger_manual_metadata_migration.py`
|
||||
- **3 passed in 6.10s**.
|
||||
- Each test creates and removes a UUID-named database on local loopback PostgreSQL; it refuses non-loopback hosts and non-`postgres` maintenance databases.
|
||||
- `PYTHONPATH=. uv run --extra dev --with httpx2 python -m pytest -q --override-ini addopts='' -p no:cacheprovider`
|
||||
- **597 passed, 11 warnings in 41.41s**.
|
||||
- Warnings are Starlette deprecations for `HTTP_422_UNPROCESSABLE_ENTITY`; they are not BAT-025 failures.
|
||||
- `uv run --extra dev ruff check app/db/migrations.py migrations/env.py migrations/versions/20260605_0025_ledger_consolidation.py tests/test_bat025_portfolio_ledger_manual_metadata_migration.py`
|
||||
- **All checks passed**.
|
||||
- `python3 -m compileall -q app migrations` and `git diff --check`
|
||||
- exit 0.
|
||||
|
||||
## Evidence boundaries and blockers
|
||||
|
||||
- The local container is an isolated test target, not a staging or production database.
|
||||
- No production backup/restore, lock-duration measurement under production load, migration run against an operational DB, deploy, or service activation was attempted.
|
||||
- The test command uses a transient `httpx2` overlay because this environment's resolved Starlette TestClient requires it while the project declares `httpx`; no dependency source change was made for this batch.
|
||||
- Independent `qc-reviewer` must rerun the tests and inspect the uncommitted diff before this document can become a QC verdict.
|
||||
|
||||
## Gate result
|
||||
|
||||
- Developer self-verification: **ready for independent QC review**.
|
||||
- Independent QC: **not yet executed**.
|
||||
- `confirm-batch-verification`: **not eligible yet**.
|
||||
- M5 release / operational migration: **not started and remains blocked**.
|
||||
Reference in New Issue
Block a user