Skip to content

fix(db): follow-up improvements for database configuration#7

Merged
raychrisgdp merged 2 commits intomainfrom
pr-017-db-config-followups
Jan 3, 2026
Merged

fix(db): follow-up improvements for database configuration#7
raychrisgdp merged 2 commits intomainfrom
pr-017-db-config-followups

Conversation

@raychrisgdp
Copy link
Copy Markdown
Owner

Summary

Issues & Goals:

  • Ensure vector store directory (data/chroma) is created alongside other app directories
  • Add --yes flag to tgenie db restore for automation while maintaining safety confirmations
  • Enforce SQLite foreign keys at engine connection level (not just session level)
  • Update troubleshooting documentation to reflect current database configuration behavior

Implementation Highlights:

  • Configuration: Added vector store directory creation in Settings.ensure_app_dirs() using vector_store_path property in backend/config.py
  • CLI: Added --yes flag to tgenie db restore command in backend/cli/db.py to skip confirmation prompts for automation (consistent with db reset pattern)
  • Database: Implemented engine-level SQLite foreign key enforcement via SQLAlchemy event listener in backend/database.py - PRAGMA foreign_keys=ON now executes on every new connection
  • Testing: Added tests for vector store directory creation (tests/test_config.py), restore with --yes flag (tests/cli/test_db.py), and engine-level FK enforcement (tests/test_database.py)
  • Documentation: Updated docs/TROUBLESHOOTING.md to reflect current default database path (~/.taskgenie/data/taskgenie.db) and environment override behavior

How to Test

CLI and Database Configuration Testing:

  1. Vector Store Directory Creation:

    # Start the API server (triggers ensure_app_dirs())
    uv run python -m backend.main
    
    # Verify vector store directory is created
    ls -la ~/.taskgenie/data/chroma
    # Should show the chroma directory exists
  2. Database Restore with Confirmation:

    # Create a test database and backup
    uv run tgenie db upgrade
    uv run tgenie db dump --out /tmp/test_backup.sql
    
    # Test restore WITHOUT --yes flag (should prompt)
    uv run tgenie db restore --in /tmp/test_backup.sql
    # Expected: Prompt "This will overwrite the existing database at ..."
    # Expected: Type 'n' to cancel or 'y' to continue
    
    # Test restore WITH --yes flag (should skip prompt)
    uv run tgenie db restore --in /tmp/test_backup.sql --yes
    # Expected: No prompt, restore proceeds immediately
  3. Database Restore on Non-Existent Database:

    # Remove existing database
    rm ~/.taskgenie/data/taskgenie.db
    
    # Restore without --yes (should NOT prompt since DB doesn't exist)
    uv run tgenie db restore --in /tmp/test_backup.sql
    # Expected: No prompt, restore proceeds immediately
  4. Engine-Level Foreign Key Enforcement:

    # Start API server (initializes database with FK enforcement)
    uv run python -m backend.main
    
    # Verify foreign keys are enabled at engine level
    # (This is tested automatically in test_engine_level_foreign_keys_enabled)
  5. Automated Test Execution:

    # Run all related tests
    uv run pytest tests/test_config.py::test_app_data_dir_creation -v
    uv run pytest tests/test_database.py::test_engine_level_foreign_keys_enabled -v
    uv run pytest tests/cli/test_db.py::test_db_restore_with_yes_flag -v

Expected Behavior:

  • Vector store directory (~/.taskgenie/data/chroma) is created when ensure_app_dirs() is called
  • tgenie db restore prompts for confirmation only when target database exists
  • tgenie db restore --yes skips confirmation prompt when database exists
  • tgenie db restore does not prompt when target database does not exist
  • SQLite foreign keys are enforced at engine connection level (verified via PRAGMA foreign_keys returning 1)
  • Troubleshooting documentation reflects current default paths and environment override behavior

Related Issues

  • Follow-up to PR-001 (database configuration)

Author Checklist

  • Synced with latest main branch
  • Self-reviewed
  • All tests pass locally
  • Documentation updated (if applicable)
  • No breaking changes (or documented if intentional)

Additional Notes

Key Implementation Areas for Review

CLI Changes:

  • backend/cli/db.py: Added --yes flag to restore command - verify confirmation logic (if not yes and db_path.exists()) correctly gates prompts and matches db reset pattern

Configuration Changes:

  • backend/config.py: Added self.vector_store_path.mkdir() in ensure_app_dirs() - verify directory creation uses canonical settings helper and doesn't create directories at import time

Database Changes:

  • backend/database.py: Added _enable_sqlite_foreign_keys() function with SQLAlchemy event listener - verify PRAGMA foreign_keys=ON executes on every new connection via event.listens_for(engine.sync_engine, "connect") and works with async engines
  • Verify FK enforcement is applied in both init_db() and init_db_async() functions

Testing:

  • tests/test_config.py: Added assertion for vector_store_path.exists() - verify directory creation test coverage
  • tests/test_database.py: Added test_engine_level_foreign_keys_enabled - verify FK enforcement test uses raw connection (bypassing get_db() session) to test engine-level behavior
  • tests/cli/test_db.py: Added test_db_restore_with_yes_flag - verify restore command test covers --yes flag behavior

Documentation:

  • docs/TROUBLESHOOTING.md: Updated database section - verify default path guidance (~/.taskgenie/data/taskgenie.db) and environment override mentions (TASKGENIE_DATA_DIR, DATABASE_URL) are accurate
  • docs/02-implementation/pr-specs/PR-017-db-config-followups.md: Spec file updated - verify implementation matches spec requirements

Testing Notes

  • Worktree state: Implementation files are committed and pushed - PR description reflects committed changes
  • CLI consistency: The --yes flag follows the same pattern as db reset command, maintaining consistency across destructive operations
  • Engine-level FK enforcement: Foreign keys are now enforced at the connection level via SQLAlchemy event listener, ensuring FK constraints apply even when bypassing get_db() session helper
  • Directory creation: Vector store directory is created lazily via ensure_app_dirs() method, not at import time
  • Manual verification: Use CLI commands (tgenie db restore) to test confirmation prompts and --yes flag behavior interactively
  • Test isolation: Tests use tmp_path fixture for isolated database and directory testing

- Add vector store directory creation in ensure_app_dirs()
- Add --yes flag to tgenie db restore for automation
- Enforce SQLite foreign keys at engine connection level
- Update troubleshooting documentation with current DB paths
@raychrisgdp raychrisgdp marked this pull request as draft January 3, 2026 06:48
@raychrisgdp raychrisgdp self-assigned this Jan 3, 2026
@raychrisgdp raychrisgdp marked this pull request as ready for review January 3, 2026 10:25
@raychrisgdp raychrisgdp merged commit c4d895c into main Jan 3, 2026
2 checks passed
@raychrisgdp raychrisgdp deleted the pr-017-db-config-followups branch January 3, 2026 10:25
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