Skip to content

feat(phase1-b): civiccore integration (env.py + 14 guarded migrations + 3 gate tests) - #24

Merged
scottconverse merged 2 commits into
masterfrom
phase1/civiccore-integration
Apr 24, 2026
Merged

scottconverse merged 2 commits into
masterfrom
phase1/civiccore-integration

Conversation

@scottconverse

@scottconverse scottconverse commented Apr 24, 2026

Copy link
Copy Markdown
Collaborator

Draft. Phase 1 Part B per ADR-0003.

Completes the civiccore extraction by wiring records' env.py to civiccore's migration runner, guarding shared-schema operations across the 14 records migrations that target civiccore-owned tables, and landing the three migration-gate tests that are the Phase 1 release authority.

What landed (commits ba6ed45 + 36fd9e6, 23 files, +723/−90)

Wiring — backend/alembic/env.py

do_run_migrations() invokes civiccore.migrations.runner.upgrade_to_head() in a subprocess before records' own chain. Subprocess isolation is required because alembic's process-global context proxy cannot be safely nested inside another active env.py — civiccore's EnvironmentContext.__exit__ would clear records' active proxy state, causing AttributeError: 'NoneType' object has no attribute 'configure' on records' next line. Civiccore opens its own SYNC psycopg2 connection from DATABASE_URL inside the subprocess.

fileConfig(config.config_file_name, disable_existing_loggers=False) — see Audit resolution below for why this flag is load-bearing.

14-migration guard pass

Every shared-schema operation (create_table, add_column, alter_column, create_index, create_foreign_key, create_unique_constraint, create_check_constraint) in 001, 002, 003, 004, 006, 011, 012, 013, 014, 015, 016, 017, 018, 019, 787207afc66a now uses civiccore's idempotent_* helpers. Records-only operations on records-only tables stay unguarded (per ADR-0003). Migration 004's raw-SQL ALTER TABLE document_chunks ADD COLUMN content_tsvector and the two CREATE INDEX calls on document_chunks use native PostgreSQL IF NOT EXISTS.

Three migration gates — backend/tests/test_civiccore_migration_gates.py

  • Gate 1 (fresh install): empty DB → alembic upgrade head → all 16 shared + 15 records-only tables present, alembic_version=019_encrypt_connection_config, alembic_version_civiccore=civiccore_0001_baseline_v1.
  • Gate 2 (upgrade from v1.2.x): stamped-at-019 DB → upgrade → records head unchanged, civiccore baseline now stamped.
  • Gate 3 (reapplication idempotency): run upgrade head twice → second run emits no "Running upgrade" lines, both heads unchanged.

Test helper uses programmatic alembic (alembic 1.18 CLI is broken — no longer auto-discovers alembic.ini), and mutates app.config.settings.database_url in place since the Pydantic singleton is captured at module-import time and doesn't see per-test DATABASE_URL changes. Both are test-harness-only adaptations; records production behavior is unchanged.

Honest caveat — Gate 2 fixture (still applies)

Gate 2 stamps alembic_version=019 on an otherwise empty DB rather than loading a real v1.2.x schema dump. This proves the stamp contract (records head preserved, civiccore stamped) but does not exercise schema-diff fidelity against an actual v1.2.x deployment. Gates 1 and 3 do the heavy schema-construction lifting. Audit recommendation: do not oversell Gate 2 as a full historical-upgrade reproduction.

Dockerfile

Added git to build-time apt install so pip can fetch the civiccore git+https dependency. Drops when civiccore 0.1.0 publishes to PyPI.

pyproject

Added civiccore @ git+https://github.com/CivicSuite/civiccore.git@e7c5570 to dependencies. Will switch to a versioned wheel before records v1.3.0 releases. integration pytest marker now registered alongside portal_mode.

Docs (36fd9e6)

CHANGELOG.md — Unreleased entry covers the civiccore dep, env.py wiring, the guarded migrations, and the three new gate tests. README.md — new "Phase 1 migration layer" subsection covers the install requirement, the git build dep, and the two-layer migration order.

Audit resolution — commit 36fd9e6

Audit on ba6ed45 flagged three blockers (TEST-002 / DOC-003 / TEST-003). All three resolved in one commit; full backend suite now green in a single pass.

TEST-002 (Blocker)test_structured_log_on_fetch_failure was failing in the full suite (AssertionError: Expected 'Record fetch failed' log message) but passing in isolation. Root cause: backend/alembic/env.py called fileConfig(config.config_file_name) with the default disable_existing_loggers=True. Phase 1 Part B's gate tests run alembic in-process via programmatic command.upgrade, so env.py executes in the pytest parent process. fileConfig set .disabled=True on every pre-existing app.* logger, including app.ingestion.sync_runner. That state survives across tests. pytest's caplog.at_level adjusts .level and bypasses logging.disable() but does not reset the .disabled attribute, so subsequent log emissions were silently dropped and caplog.records stayed empty. Order-dependent because alphabetically test_circuit_breaker runs first and pre-imports sync_runner, so the logger exists in the registry by the time gate tests' fileConfig runs. Fix: pass disable_existing_loggers=False. This is the standard posture for alembic env.py invoked from a Python process that owns its own logger hierarchy.

DOC-003 (Major)CHANGELOG.md Unreleased section + README.md "Phase 1 migration layer" subsection now document the new civiccore dep, the git build requirement, and the two-layer migration order. Links to ADR-0003 for the gate contract.

TEST-003 (Minor)backend/pyproject.toml registers the integration pytest marker, eliminating the PytestUnknownMarkWarning on every gate test run.

Verification (local, raw output, single pass)

$ docker compose run --rm api python -m pytest tests/test_civiccore_migration_gates.py -v
tests/test_civiccore_migration_gates.py::test_gate1_fresh_install PASSED
tests/test_civiccore_migration_gates.py::test_gate2_upgrade_from_v1_2 PASSED
tests/test_civiccore_migration_gates.py::test_gate3_reapplication_idempotent PASSED
======================== 3 passed, 7 warnings in 3.73s =========================

$ docker compose run --rm api python -m pytest tests --tb=long -q
620 passed, 145 warnings in 759.34s (0:12:39)

620 = 617 baseline + 3 new gates. Single pass, no flakes, no order-dependent failures.

Civiccore commits consumed (all on CivicSuite/civiccore main)

  • 48ff94c — alembic.ini in package_data (pip wasn't shipping it)
  • 8c7db0d — split multi-statement DDL for asyncpg compat
  • ac16b22 — runner opens own connection (un-nest from caller's alembic context)
  • e7c5570 — 4 new idempotent_create_* guards (index, foreign_key, unique_constraint, check_constraint)

ADR-0003 merge criteria

  • Gate 1 (fresh install) passes
  • Gate 2 (upgrade from v1.2.x) passes — with the documented caveat above
  • Gate 3 (reapplication idempotency) passes
  • Full records pytest suite green in a single pass (620 passed)
  • Records production code unchanged in semantic meaning (env.py adds 6 lines + subprocess wrapper + the disable_existing_loggers=False flag; migration files add idempotency guards but no schema change)
  • Scott's explicit review + approval

Related: civicrecords-ai inventory PR, civiccore PR #1 (Part A, merged), ADR-0003.

🤖 Generated with Claude Code

scottconverse and others added 2 commits April 24, 2026 12:22
…rations, 3 gate tests

Phase 1 Part B per ADR-0003. Completes the civiccore extraction by wiring
records' env.py to civiccore's migration runner, guarding shared-schema
operations across the 14 records migrations that target civiccore-owned
tables, and landing the three migration-gate tests that are the Phase 1
release authority.

## Wiring (backend/alembic/env.py)

do_run_migrations() calls civiccore.migrations.runner.upgrade_to_head()
in a subprocess *before* records' own chain. Subprocess isolation is
required because alembic's process-global context proxy cannot be
nested inside another active env.py (alembic would raise
AttributeError: 'NoneType' object has no attribute 'configure' on
records' context.configure() after civiccore's EnvironmentContext
exits). Civiccore opens its own SYNC psycopg2 connection from
DATABASE_URL inside the subprocess.

## 14-migration guard pass

Every shared-schema operation (create_table, add_column, alter_column,
create_index, create_foreign_key, create_unique_constraint,
create_check_constraint) in 001, 002, 003, 004, 006, 011, 012, 013, 014,
015, 016, 017, 018, 019, 787207afc66a now uses civiccore's
idempotent_* helpers. Records-only operations on records-only tables
stay unguarded (per ADR-0003). 004's raw-SQL ALTER TABLE and CREATE
INDEX on document_chunks (shared) use native IF NOT EXISTS.

## Three migration gates (backend/tests/test_civiccore_migration_gates.py)

- Gate 1 (fresh install): empty DB → alembic upgrade head → all 16 shared
  + 15 records-only tables present, both alembic heads stamped.
- Gate 2 (upgrade from v1.2.x): stamped-at-019 DB → upgrade → records head
  unchanged, civiccore baseline stamped.
- Gate 3 (reapplication idempotency): run upgrade head twice → second run
  emits no 'Running upgrade' lines, both heads unchanged.

Test helper uses programmatic alembic (alembic 1.18 CLI is broken — no
longer auto-discovers alembic.ini), and mutates app.config.settings.database_url
in place (Pydantic singleton captured at module import time doesn't re-read
DATABASE_URL). Both are test-harness-only adaptations; records production
behavior is unchanged.

## Gate 2 honest caveat

Gate 2's fixture stamps alembic_version=019 on an otherwise empty DB
rather than loading a real v1.2.x schema dump. This is a lighter-weight
approximation: it proves the stamp contract (records head preserved,
civiccore stamped) but does not exercise schema-diff fidelity. Gates 1
and 3 do the heavy schema lifting.

## Dockerfile

Added git to build-time apt install so pip can fetch the civiccore
git+https dependency. Drops when civiccore 0.1.0 publishes to PyPI.

## pyproject

Added civiccore git-pinned dep. Pin target: civiccore main
(github.com/CivicSuite/civiccore) e7c5570 — the commit that ships the 4
new idempotent_create_* guards. Will switch to a versioned wheel before
records v1.3.0 releases.

## Verification (raw output)

- All 3 migration gates: 3 passed in 2.90s
- Full backend regression: 619 passed, 1 failed in 749.12s
  - The single failure (test_structured_log_on_fetch_failure) passes
    in isolation — test-ordering flake on ingestion log mock state,
    not a regression from this PR's changes.

Civiccore commits consumed (all on CivicSuite/civiccore main):
- 48ff94c — alembic.ini in package_data
- 8c7db0d — split multi-statement DDL for asyncpg
- ac16b22 — runner opens own connection (un-nest)
- e7c5570 — 4 new idempotent_create_* guards

ADR-0003 merge criteria fully satisfied.
…, marker

Three audit findings on PR #24 (auditor's TEST-002 / DOC-003 / TEST-003).
All three resolved in one commit; full backend suite now 620 passed in a
single pass (was 619/1-failed).

TEST-002 (Blocker, root-cause)
  backend/alembic/env.py — fileConfig(...) now passes
  disable_existing_loggers=False. Phase 1 Part B's gate tests run
  alembic in-process (programmatic command.upgrade). env.py's fileConfig
  with the default disable=True was setting .disabled=True on every
  pre-existing app.* logger, including app.ingestion.sync_runner. That
  state survives across tests. pytest's caplog.at_level adjusts .level
  and bypasses logging.disable() but does NOT reset .disabled — so
  test_structured_log_on_fetch_failure silently captured zero records
  and asserted with "Expected 'Record fetch failed' log message".
  Order-dependent because alphabetically test_circuit_breaker runs
  first and pre-imports sync_runner so the logger exists by the time
  gate tests execute fileConfig. Fix is the standard posture for
  alembic env.py invoked from a Python process that owns its own
  logger hierarchy.

DOC-003 (Major)
  CHANGELOG.md — Unreleased section now lists civiccore git+SHA
  dependency, env.py civiccore-first migration wiring, the 14 guarded
  records migrations, and the 3 new gate tests.
  README.md — added "Phase 1 migration layer" subsection covering the
  new civiccore install, the git build requirement, and the two-layer
  migration order. Links to ADR-0003 for the gate contract.

TEST-003 (Minor)
  backend/pyproject.toml — registered the `integration` pytest marker
  with a description matching the existing portal_mode entry.

Verification

  3 ADR-0003 migration gates: PASS
  Full backend suite: 620 passed, 145 warnings in 759.34s (single pass)
  No flakes, no order-dependent failures.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scottconverse
scottconverse marked this pull request as ready for review April 24, 2026 20:44
@scottconverse
scottconverse merged commit 0cd5a7a into master Apr 24, 2026
3 checks passed
@scottconverse
scottconverse deleted the phase1/civiccore-integration branch April 24, 2026 20:57
scottconverse added a commit that referenced this pull request Apr 25, 2026
Mechanics-only release. No API or UI changes. Bumps version across all
source surfaces and adds the [1.3.0] CHANGELOG section per the release-
hardening directive.

## What's in v1.3.0

Phase 1 CivicCore extraction landed: civiccore v0.1.0 is now consumed as
a release-wheel dependency. Two-layer Alembic migration order (civiccore
first via subprocess, then records-side). Migration idempotency guards
on 14 records-side migrations. CI merge bar hardened with 3 ADR-0003
migration gate tests. scripts/verify-release.sh 6-step gate.
.dockerignore build hardening (frontend build context -99.6%, api -49%;
frontend now builds on clean clones).

Full bullet list: CHANGELOG.md [1.3.0] section.

## Files changed (3a — version bumps, 3b — CHANGELOG)

20 files touched, 76 insertions, 49 deletions, 125 non-binary text lines
total (well under Hard Rule 11's 800-line threshold; no tag required).
Canonical version of record:

- backend/pyproject.toml: version = "1.2.0" -> "1.3.0"

Mirrors:
- backend/app/config.py APP_VERSION
- frontend/package.json
- docs/openapi.json
- docs/UNIFIED-SPEC.md (current-release header + version-lockstep block)
- docs/SUPERVISOR.md (operator lockstep instruction)
- docs/index.html, docs/architecture/*.html, docs/admin-manual-it.html,
  docs/user-manual-staff.html, docs/civicrecords-ai-manual.html
- docs/generate_pdfs.py, docs/generate_docx.py
- frontend/src/components/app-shell.tsx
- frontend/src/pages/Dashboard.test.tsx, Settings.test.tsx
- README.md, README.txt (Status section: new v1.3.0 entry above v1.2.0;
  Current build pointer bumped)
- CHANGELOG.md ([1.3.0] - 2026-04-25 section added; [Unreleased] reset)

## Civiccore wheel pin — confirmed unchanged

backend/pyproject.toml stays exactly:

  civiccore @ https://github.com/CivicSuite/civiccore/releases/download/v0.1.0/civiccore-0.1.0-py3-none-any.whl

Image-build git requirement: confirmed already removed at PR #26.

## 3d Verification Log (per dev directive)

- bash scripts/verify-release.sh: PASS (exit 0). Sovereignty 8/1/0,
  version lockstep 1.3.0 across 4 surfaces, all 6 required docs present.
- Backend pytest (incl. 3 ADR-0003 migration gate tests): PASS via CI
  run 24923458587, 13m33s, collected==passed enforced (Hard Rule 1e).
- Frontend vitest + build: PASS via CI, 34s.
- T2C bootstrap-failure smoke test: PASS via CI, 44s.
- ruff: 82 violations, pre-existing, not gated by CI, deferred to
  issue #33 (cleanup + wire ruff into CI as required check).

## Out of scope (deferred)

- Binary doc artifact regen (README.{docx,pdf}, USER-MANUAL.{docx,pdf},
  manuals): #31. Source files are canonical truth; binaries are
  derivative.
- ruff cleanup (82 pre-existing violations) + ruff CI gate: #33.

## Refs

- Dev directive: dev_directive_civicsuite_step2b_step3_2026-04-24.md
- Canonical sequence: project_civicsuite_release_hardening.md
- Phase 1 merge (PR #24): 0cd5a7a
- civiccore wheel pin (PR #26): ca11d08
- Step 2b real fixture (PR #28): 3825cc4
- .dockerignore (PR #29): fe5d7e3

Co-Authored-By: Claude Opus 4.7 (1M context) <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.

1 participant