Skip to content

fix(cli): remove stale capture index rows#22

Open
zxy1553 wants to merge 1 commit intoEinsia:mainfrom
zxy1553:fix-capture-index-cleanup
Open

fix(cli): remove stale capture index rows#22
zxy1553 wants to merge 1 commit intoEinsia:mainfrom
zxy1553:fix-capture-index-cleanup

Conversation

@zxy1553
Copy link
Copy Markdown

@zxy1553 zxy1553 commented Apr 27, 2026

Summary

Fix stale rows in the captures SQLite table when capture JSON files are removed.

Changes:

  • clean captures now deletes matching captures rows after removing capture-buffer/*.json
  • rebuild-captures-index now removes rows whose capture files no longer exist
  • adds regression tests for cleanup and rebuild behavior

Tests

  • python -m pytest tests/test_cli_captures_cleanup.py -q
  • python -m ruff check src/openchronicle/cli.py tests/test_cli_captures_cleanup.py

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces synchronization between the capture buffer directory and the database index. It updates rebuild_captures_index to remove stale database entries and _clean_captures to delete records when files are unlinked. New tests have been added to verify these cleanup operations. The review feedback suggests wrapping database deletion loops in transactions to improve performance in SQLite.

Comment thread src/openchronicle/cli.py

files = sorted(p for p in buf.iterdir() if p.is_file() and p.suffix == ".json")
file_ids = {p.stem for p in files}
with fts.cursor() as conn:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The loop performs multiple database deletions in autocommit mode, which is inefficient in SQLite. Wrapping the loop in a transaction (e.g., using with conn:) will improve performance by grouping these operations into a single commit.

Suggested change
with fts.cursor() as conn:
with fts.cursor() as conn, conn:

Comment thread src/openchronicle/cli.py
removed_stems.append(p.stem)
n += 1
if removed_stems:
with fts.cursor() as conn:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The loop performs multiple database deletions in autocommit mode. Wrapping the loop in a transaction will improve performance.

Suggested change
with fts.cursor() as conn:
with fts.cursor() as conn, conn:

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