Skip to content

Quick fixes: indexes, fs guard, UI listener consistency - #22

Open
humansinstitute wants to merge 1 commit into
mainfrom
quick-fixes
Open

Quick fixes: indexes, fs guard, UI listener consistency#22
humansinstitute wants to merge 1 commit into
mainfrom
quick-fixes

Conversation

@humansinstitute

Copy link
Copy Markdown
Owner

Quick fixes addressing prior review points 2, 4, and 6.

Includes
2) Database indexes

  • Add indexes for sessions(archived) and sessions(archived_at) in migrations to improve list/purge performance.
  1. Filesystem deletion robustness
  • Wrap fs.unlink calls with an existence check (fs.access) to reduce noisy warnings when files are already gone.
  1. UI event listener consistency
  • Prefer safeAddEventListener for newly added Archived Sessions controls; fall back to addEventListener if helper isn’t available.

No behavior changes intended beyond performance and resilience improvements.

Wingman: Goose... I can be your wingman any time

…ions(archived) and sessions(archived_at) indexes in migrations\n- Guard fs.unlink with fs.access checks\n- Use safeAddEventListener for archive UI controls when available\n\nWingman: Goose... I can be your wingman any time

@humansinstitute humansinstitute left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Small follow-up PR addressing points 2, 4, and 6 from the earlier review.

Summary

  • DB indexes: add sessions(archived) and sessions(archived_at) indexes in migrations to support faster list and purge operations.
  • Filesystem deletion: guard unlink with access checks to prevent noisy warnings when files are already gone.
  • UI: use safeAddEventListener for new archived-session controls when available; fall back to addEventListener otherwise.

Notes

  • No functional behavior changes expected; this is primarily performance and resilience hardening.

Wingman: Goose... I can be your wingman any time

@humansinstitute humansinstitute left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Formal review

Summary

  • Addresses review points 2 (DB indexes), 4 (fs deletion robustness), and 6 (UI listener consistency) with targeted, low-risk changes.

Scope reviewed

  • lib/database.js: adds indexes for sessions(archived) and sessions(archived_at) during migrations
  • server.js: guards fs.unlink calls with fs.access checks (prevents noisy warnings when files already removed)
  • public/index.html: uses safeAddEventListener for new archive controls when available; falls back to addEventListener

Findings

  • Changes are localized and non-invasive; behavior should remain identical aside from performance and resilience improvements.
  • Index creation commands are idempotent (IF NOT EXISTS).

Risks / follow-ups

  • None blocking. Optionally add a migration test asserting indexes exist (PRAGMA index_list) and keep an eye on any UI event duplication (should be fine with safeAddEventListener).

Recommendation: Approve

Wingman: Goose... I can be your wingman any time

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