Skip to content

fix(backend): migrate folders tables that predate indexing_status - #1579

Open
harshvardhanrp wants to merge 1 commit into
AOSSIE-Org:devfrom
harshvardhanrp:fix/folders-indexing-status-migration
Open

harshvardhanrp wants to merge 1 commit into
AOSSIE-Org:devfrom
harshvardhanrp:fix/folders-indexing-status-migration

Conversation

@harshvardhanrp

@harshvardhanrp harshvardhanrp commented Oct 1, 2026 •

Copy link
Copy Markdown

Partially addresses #1555

That issue reports the symptom correctly, the folder list 500s with "no such column: f.indexing_status" after an upgrade. The cause it gives is not right though. There is no migration_009_add_indexing_status.py and no migration runner in this repo. Tables are created and patched idempotently at startup in main.py.

The real cause is that folders.indexing_status was added to the CREATE TABLE in #1380 with no guarded ALTER. CREATE TABLE IF NOT EXISTS skips a table that already exists, so a database from 1.2.0 or older keeps the old six column table while every query asks for the seventh. GET /folders/all-folders returns 500 and db_is_indexing_busy raises, which breaks /memories/status too. Every release before #1380 is affected, so 1.2.0, 1.1.0, 1.0.0 and 0.1.1.

Added the guarded ALTER the other tables already use, with the same default as the CREATE so a migrated schema matches a fresh one. Existing folders are backfilled to completed because the old version really did index them, and isIndexingPending treats anything other than completed or interrupted as pending. Leaving them on the default would put a spinner on every folder and keep the one second poll running forever.

Checked by building a 1.2.0 database with the 1.2.0 release code, then running the current startup order and calling the real route. Before the change it is HTTP 500, after it is HTTP 200 with indexing_status completed. Four new tests in test_folders.py fail before and pass after. Full suite is 1352 passing with black, ruff and mypy clean.

Also fixed two pre existing mypy errors that CI only flags because this PR touches these files. A list concat in db_update_ai_tagging_batch and four untyped dicts in the tests. Happy to split those into their own PR.

Not covered here are the three frontend items in #1555, the generic error dialog, the one second dialog spam and the missing refetchOnWindowFocus. Those sit on top of this backend fix and can be a follow up.

Summary by CodeRabbit

  • Bug Fixes
    • Existing folders now retain a completed indexing status when the database is upgraded, while newly added folders continue to start as not started.
    • Reinitializing the database preserves existing indexing statuses.

@github-actions github-actions Bot added bug Something isn't working possible-duplicate Potential semantic duplicate (upstream comparison) labels Oct 1, 2026
@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: AOSSIE-Org/PictoPy/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 1e5bead0-b26c-4302-8c5d-93613a643b95

📥 Commits

Reviewing files that changed from the base of the PR and between 8645431 and 5f07291.

📒 Files selected for processing (2)
  • backend/app/database/folders.py
  • backend/tests/test_folders.py

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.


Walkthrough

Database initialization now migrates legacy folders tables that lack indexing_status. It marks existing folders as completed and retains the not-started default for new folders. Tests cover these migration results and status preservation during repeated initialization.

Changes

Folder indexing status

Layer / File(s) Summary
Legacy schema migration
backend/app/database/folders.py
Initialization adds indexing_status to legacy tables and marks existing folders completed. New rows retain the not-started default. The AI-tagging parameter construction changes without changing the values or update behavior.
Migration test coverage
backend/tests/test_folders.py
Tests recreate the legacy schema and verify migration, defaults for new rows, and preservation of an in-progress status across repeated initialization. Request dictionaries also gain explicit type annotations.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix

Suggested labels: Python

Suggested reviewers: rohan-pandeyy

Merge Risk: ⚪ Minimal · up to 5f072

The change repairs legacy folder databases while preserving existing indexing statuses and defaults for new folders. No actionable merge-blocking risk remains in the supplied evidence; merge after normal checks pass.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 5f072

The upgrade restores compatibility for older folder databases without an identified expansion of access or privileges. However, an interrupted upgrade can leave existing folders marked as pending, and restarting does not reliably repair that state.

Retained concerns

  • Medium · reliability · inferred: The schema addition and legacy-row backfill are not enclosed in an explicit transaction. Interruption after ALTER can leave legacy rows at not_started; subsequent initialization sees the column and skips backfill, while stale-processing cleanup leaves that status unchanged. This creates a persistent partial-upgrade state rather than a reliably recoverable startup transition.
Security review details

Security Blast Radius

  • inferred — The migration's direct write scope is every existing folder row in the configured database lacking the column. The recovery concern therefore affects that database's legacy folder-status state; the inspected path does not demonstrate a cross-tenant or cross-service attack.

Trust Boundaries and Controls

  • observed — The new migration is reached through application startup, not a newly introduced request handler. Its schema mutation does not take request-supplied identifiers or status values.

Resilience and Maintainability Implications

  • inferred — The material resilience weakness is incomplete upgrade recovery, not a demonstrated authorization bypass: restarting can preserve the partially migrated state because the guard tests schema presence rather than completion of the data transition.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: migrating existing folders tables that lack indexing_status.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit checks the folders one by one,
Old rows get their status, migration done.
New rows start where defaults say they should,
An in-progress mark stays as it stood.
The tests hop through each schema change,
Then nibble carrots on the range.

Comment @coderabbitai help to get the list of available commands.

@gitcordapp

gitcordapp Bot commented Oct 1, 2026

Copy link
Copy Markdown

Link your account with Gitcord

Thanks for opening this PR, @harshvardhanrp!

To receive Discord notifications and contributor tracking for this organization:

  1. Join Discord: https://discord.gg/hjUhu33uAn
  2. In Discord, run /link harshvardhanrp
  3. Paste the verification code into your GitHub bio (or a public gist)
  4. Click Verify in Discord (or run /verify-link harshvardhanrp)

Once linked, Gitcord can notify you about reviews, merges, and more.

— Posted by Gitcord

This branch has not been deployed

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

Labels

bug Something isn't working possible-duplicate Potential semantic duplicate (upstream comparison)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant