feat(all): add simplified DB maintenance

[OSXLich-Doug]

Important

Adds a feature for periodic database maintenance in Lich, performing a VACUUM operation every 6 months if due, with error handling and logging.

• Behavior:
  • Adds Lich.db_vacuum_if_due! in lib/lich.rb to perform VACUUM on the database if due (every 6 months).
  • Integrates Lich.db_vacuum_if_due! into start_wrap_thread in lib/games.rb to run on startup.
  • Handles SQLite3 busy exceptions by retrying after a short sleep.
  • Logs errors and skips VACUUM if the database is busy or lock is held.
• Functions:
  • Lich.db_maint_lock_path: Returns the path for the maintenance lock file.
  • Lich.db_maint_last_at: Retrieves the last maintenance timestamp from the database.
  • Lich.db_maint_set!: Updates the last maintenance timestamp and note in the database.
  • Lich.db_maint_due?: Checks if maintenance is due based on the last timestamp.
  • Lich.db_vacuum_if_due!: Attempts to perform VACUUM if maintenance is due, with file locking.
• Misc:
  • Adds logging for maintenance actions and errors in lib/lich.rb.

^{This description was created by for 325405a. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed everything up to 325405a in 2 minutes and 55 seconds. Click for details.

• Reviewed 135 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 6 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. lib/lich.rb:23

• Draft comment:
  Wrap multiple DB update statements in a transaction in db_maint_set! to ensure atomicity.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While transactions can help with atomicity, the operations here are already atomic individually. The CREATE TABLE is idempotent, and INSERT OR REPLACE is atomic. The two inserts are related but partial completion wouldn't leave the DB in an invalid state - having just the timestamp or just the note would be fine. The retry on busy already handles concurrency. I could be underestimating the importance of the two inserts being done together. Maybe there are scenarios where having mismatched timestamp/note pairs would cause issues. The code already handles the key failure modes with busy retry. The inserts are independent enough that strict atomicity isn't critical. The comment seems to be suggesting a "best practice" without clear benefit. The comment should be deleted as it suggests additional complexity without demonstrating clear benefit or risk.

2. lib/lich.rb:63

• Draft comment:
  The lock acquisition loop uses a busy-wait (sleep 0.1); consider adding a maximum retry limit or configurable timeout to avoid potential infinite loops.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

3. lib/lich.rb:94

• Draft comment:
  Rescuing SQLite3::BusyException during the VACUUM call leads to quietly skipping maintenance; ensure this behavior is acceptable.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

4. lib/lich.rb:12

• Draft comment:
  Infinite retry loops on SQLite3::BusyException (e.g., in db_maint_last_at and elsewhere) could risk blocking; consider a maximum retry count.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 30% vs. threshold = 50% The comment raises a legitimate concern about potential infinite loops. While SQLite busy errors are typically transient, having no upper bound on retries could theoretically cause a hang. However, this is a maintenance utility that runs occasionally, not critical path code. The simple retry pattern makes the code more readable and matches the file's "simple + safe" comment. Adding retry limits would make the code more complex. I may be underestimating the impact of a potential hang. Even maintenance code should have safeguards against infinite loops. While the concern is valid, the current simple approach aligns with the module's stated goals of being "minimal" and "simple + safe". The risk of an actual infinite loop is low given SQLite's reliability. While the comment raises a valid concern, the current simple retry approach is a reasonable tradeoff for maintenance code that prioritizes simplicity. The comment should be removed as it doesn't justify the added complexity of retry limits.

5. lib/lich.rb:6

• Draft comment:
  File.flock usage (in db_maint_lock_path and its consumption) may behave differently on non-Unix platforms; ensure or document platform compatibility.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

6. lib/games.rb:213

• Draft comment:
  The call to Lich.db_vacuum_if_due! is wrapped in a rescue that only logs errors; ensure that silently skipping maintenance on failure is acceptable.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_atrRUDgMON8UDGhv

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}