Skip to content

Add migration readiness CLI and schema hardening#19

Open
ViperJuice wants to merge 1 commit intomainfrom
codex/add-migrations-and-maintenance-cli-command
Open

Add migration readiness CLI and schema hardening#19
ViperJuice wants to merge 1 commit intomainfrom
codex/add-migrations-and-maintenance-cli-command

Conversation

@ViperJuice
Copy link
Owner

Summary

  • add schema hardening migration to cover missing content tracking columns, embedding uniqueness, and manifest metadata
  • enhance SQLite health checks, migration execution, and manifest backfill helpers for compatibility reporting
  • introduce an index CLI migrate command and integration coverage for upgrading legacy databases

Testing

  • pytest tests/test_sqlite_store.py tests/integration/test_phase1_foundation.py tests/integration/test_migration_cli.py (fails: missing psutil dependency in this environment)

Codex Task

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +356 to +359
applied = store.run_migrations()
manifest_result = store.ensure_index_manifests(manifest_path)
health = store.health_check()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Respect manifest_dir when reporting migrate readiness

The new index migrate command always evaluates manifests using store.health_check() with the database directory, ignoring the --manifest-dir argument. If a user points manifests to another location (e.g., --manifest-dir /opt/index-manifests), the command will report missing manifests and exit non‑zero even after creating them in the requested directory, because readiness is checked against the default path instead of the supplied one. The readiness check should inspect the same directory passed via manifest_dir (including in --check-only mode) to avoid false failures.

Useful? React with 👍 / 👎.

Comment on lines +264 to +267
}

check_fn = migration_checks.get(version)
return check_fn() if check_fn else False

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Future migrations are marked applied without running

The _migration_needed helper returns False for any migration version not listed in migration_checks, and _run_migrations treats a falsey check as “not needed” while still marking the version as applied. That means any new .sql migration added after v3 will be skipped and recorded as applied without executing, leaving the schema outdated with no error. The default path for unknown versions should run the migration (or fail) rather than silently skipping it.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1 participant