Skip to content

Fix migration P1s: live catalog status, run-owned stage updates, renamed-code reindex - #48

Open
naveenhiremath28 wants to merge 3 commits into
kp-migrationsfrom
kp-migrations-p1-fixes
Open

naveenhiremath28 wants to merge 3 commits into
kp-migrationsfrom
kp-migrations-p1-fixes

Conversation

@naveenhiremath28

Copy link
Copy Markdown

Summary

Follow-up to #43 that fixes three of the P1 issues raised in review. Targets kp-migrations, so it can be merged into #43 or cherry-picked.

Changes

Importer: renamed codes are flagged for reindex (scripts/migrate_sqlite_to_postgres.py)

  • When the importer suffixes a duplicate scheme_code (or repairs an invalid one), the old index points still carry the old code.
  • The document is now marked reindex_required with a reason naming the old and new code, so catalog_status reports pending_reindex.
  • The old index_status is kept so a reindex can find the old points.

Stage updates go to the reporting run (pipeline/db.py, activities.py, workflows.py)

  • update_document_stage takes the reporting run's Temporal workflow id and writes only that run's row, instead of the document's newest workflow.
  • _mirror_state supplies the id behind workflow.patched("run-owned-stage-v1"), so in-flight workflows still replay.
  • If no run matches the id, it falls back to the previous behaviour.
  • The state activity updates that run's job row too.

catalog_status: live requires a PROD index (migration 0003)

  • Live now needs some completed workflow and either metadata.index_status.prod.status = 'indexed' or a completed run that skipped PROD on purpose (workflows.config.skip_prod).
  • A DEV-only document is no longer advertised as live, and starting a new run no longer hides a live document.
  • skip_prod is recorded at upload from DISABLE_PROD_SETTING. The importer sets it on legacy runs when that flag is on.

Deployment note

After 0003, a completed document with no PROD index entry stops showing as live. That is the intended fix for PROD-enabled setups. In a PROD-disabled deployment that already ran, existing documents stay hidden until skip_prod is set on their runs (a one-line UPDATE, not included here).

Testing

  • Full suite against a local Postgres: 976 passed. Ruff clean.
  • Not run: tests/test_observability*.py (opentelemetry not installed in my venv).
  • New tests: renamed-code reindex, late update from an older run, unknown-run fallback, DEV-only not live, skipped-PROD live, new run does not hide a live document, DEV-only legacy import.
  • Nine existing tests assumed completed meant live; their fixtures now record a PROD index or skip_prod.

Not covered: vector ID collisions and advisory promotion binding. Those are proposed for a separate PR, in line with the review request to split out the webhook/advisory work.

When the importer suffixes a duplicate scheme_code (or repairs an invalid one),
the existing index points still carry the old code. Mark the document
reindex_required with a reason so catalog_status reports pending_reindex.
update_document_stage now takes the reporting run's Temporal workflow id and
updates that run's row instead of the document's newest workflow, so a late
update from an older run cannot change a newer run's stage or job status.
catalog_status counted a document as live when its latest workflow was
completed. A DEV-only document therefore showed as live, and starting a new
run hid a live one.

Migration 0003 redefines the view: live needs some completed workflow and
either metadata.index_status.prod.status = 'indexed' or a completed run that
skipped PROD (workflows.config.skip_prod, recorded at upload from
DISABLE_PROD_SETTING). The importer sets skip_prod on legacy runs when
DISABLE_PROD_SETTING is on.
@codecov

codecov Bot commented Nov 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pipeline/db.py 77.77% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

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