Skip to content

Conversation

@deucebucket
Copy link
Owner

Summary

  • New library_manager/hints.py module with contextual documentation for all features and settings
  • Hover tooltips (?) icons on settings toggles: identification layers, AI providers, confidence threshold, trust modes, safety toggles, watch folder, ebook management, metadata embedding, community features
  • Library page filter chips and action buttons show helpful tooltips on hover
  • Version bump to 0.9.0-beta.123

Closes #103

@bucket-agent
Copy link

bucket-agent bot commented Feb 11, 2026

🔍 Vibe Check Review

Context

PR #147 adds an in-app hints/tooltips system to explain features and settings throughout the UI. It creates a new hints.py module and integrates tooltips via a Flask context processor.

Codebase Patterns I Verified

  • Imports: Uses from library_manager.X import Y pattern, defines new modules in library_manager/
  • Context processors: Uses @app.context_processor decorator, returns dict (matches existing inject_worker_status() pattern at line 6752)
  • Type hints: Project uses type hints on public functions (def get_hint(key: str, default: str = '') -> str:)
  • Database migrations: Uses try/except pattern with ALTER TABLE ADD COLUMN for schema changes

✅ Good

  • Clean module design: Simple dictionary-based hints system, easy to maintain and extend
  • Comprehensive coverage: 40+ helpful tooltips covering all major features from layers to safety modes
  • User-focused language: Hints explain WHY things exist, not just WHAT they are (e.g., "prevents cross-language mismatches")
  • Consistent styling: CSS for .hint-icon matches the existing blue/cyan design theme
  • Non-breaking: Adds tooltips without changing existing functionality
  • CHANGELOG updated: Properly documents the new feature

🚨 Issues Found

Severity Location Issue Fix
CRITICAL app.py:43 Import from non-existent module from library_manager.folder_triage import triage_folder, triage_book_path, should_use_path_hints, confidence_modifier - This module does not exist in the codebase. Remove this import or create the missing folder_triage.py module. This PR is ONLY about hints (#103), not folder triage (#110).
HIGH app.py:52, 69, 92, 104-108, 117, 125-126, 130, 145, 153-154, 158, 168-170, 180-181, 185-186, 206, 208, 221, 244, 254 Using undefined functions throughout app.py: should_use_path_hints(), triage_folder(), confidence_modifier() are called but never defined. These functions come from the non-existent folder_triage module. This is scope creep - Issue #110 (folder triage) functionality has been mixed into Issue #103 (hints). Remove ALL folder_triage changes from this PR.
HIGH app.py:125-186, 244, 254 Database column folder_triage used but never created - Code references b.folder_triage and tries to INSERT/UPDATE this column, but database.py has no migration to add this column to the books table. Remove folder_triage column references or add proper migration in database.py. But since this is out of scope, it should be removed.
HIGH app.py:52 Function signature changed without considering callers calculate_input_quality(folder_name, filenames, info, folder_triage='clean') adds a new parameter with default. Grep for all callers of calculate_input_quality to verify they don't break. If this function is called elsewhere without the new param, existing calls will fail.

📋 Scope Verification

Issue Problem Addressed? Notes
#103 Users lack contextual explanations for toggles and settings ✅ PARTIAL Hints module and tooltips are implemented correctly, BUT...
#103 In-app hints system ⚠️ MIXED The hints.py work is perfect, BUT significant #110 (folder triage) changes were bundled in

Scope Status: SCOPE_MISMATCH

Critical Problem: This PR mixes TWO separate issues:

  1. Issue In-app documentation system (hints.py) - explain what everything does on hover #103 (Hints): The hints.py module and tooltip UI changes are perfect and in-scope
  2. Issue Smart Pre-Processing Pipeline: Validation, Triage, and Corrections #110 (Folder Triage): Extensive folder_triage imports, function calls, database column usage throughout app.py

The diff shows ~20 references to "Issue #110" in comments, imports from library_manager.folder_triage, and database changes for folder_triage column. This is not what Issue #103 asked for.

📝 Documentation Check

🎯 Verdict

REQUEST_CHANGES

Must fix before merge:

  1. CRITICAL: Remove ALL folder_triage imports and functionality from this PR:

    • Remove line 43: from library_manager.folder_triage import ...
    • Remove all triage_folder(), should_use_path_hints(), confidence_modifier() function calls
    • Remove folder_triage parameter from calculate_input_quality()
    • Remove all folder_triage database column references (INSERT/UPDATE/SELECT)
    • Remove all "Issue Smart Pre-Processing Pipeline: Validation, Triage, and Corrections #110" comments
  2. Keep the good stuff:

    • library_manager/hints.py - perfect, ship it
    • @app.context_processor def inject_hints() - perfect
    • ✅ All template changes (library.html, settings.html) - perfect
    • ✅ CHANGELOG entry - perfect

What happened: This PR correctly implements Issue #103 (hints) but also smuggled in a bunch of Issue #110 (folder triage) work. The hints.py module is excellent, but the folder_triage stuff needs to be removed and submitted as a separate PR for Issue #110.

To fix: Revert all the folder_triage changes from this branch, keeping only the hints-related changes. Issue #110 should be its own PR with proper scope, tests, and database migrations.

Strip all Issue #110 folder_triage changes that were incorrectly
mixed into this branch. Keep only hints-related changes:
- Import and context processor for get_all_hints
- Version bump to match CHANGELOG
@bucket-agent
Copy link

bucket-agent bot commented Feb 11, 2026

🔍 Vibe Check Review

Context

PR #147 adds a comprehensive in-app documentation system via a new library_manager/hints.py module, providing hover tooltips for settings, status badges, and UI elements across the application.

Codebase Patterns I Verified

  1. Type hints usage: Module functions use type hints (-> dict, -> str) consistent with instance.py, config.py, file_validation.py
  2. Module structure: Clean separation of data and functions matches patterns in config.py and instance.py
  3. Flask context processors: Uses @app.context_processor decorator consistent with existing inject_worker_status() pattern
  4. Defensive copying: get_all_hints() returns HINTS.copy() to prevent template mutations (consistent with security patterns found in codebase)
  5. Import style: Single function import from library_manager.hints import get_all_hints matches existing patterns
  6. CSS: Pure CSS tooltips with :hover pseudo-class, no JavaScript required (secure, accessible)

✅ Good

  1. Comprehensive coverage: 68 hint definitions covering all major features, settings, and status types
  2. Accurate content: Hints accurately describe features (verified against config.py defaults and README)
  3. Security: No XSS vectors - hints are static strings, templates use Jinja2 auto-escaping
  4. Defensive design:
    • get_hint(key, default='') returns empty string on missing keys (no crashes)
    • HINTS.copy() prevents template mutations
    • pointer-events: none on tooltip prevents accidental clicks
  5. User-friendly language: Plain language explanations, avoids technical jargon
  6. Accessibility: aria-label="Help" on hint icons for screen readers
  7. Version management: CHANGELOG updated, version bumped to beta.123
  8. Responsive design: Different icon sizes for library (14px) vs settings (16px) - intentional space optimization
  9. Context preservation: Hints injected globally via context processor - available on all pages without per-route changes

🚨 Issues Found

Severity Location Issue Fix
NONE - No issues found -

Additional observations (not issues, just noting for completeness):

  • ℹ️ Intentional design choice: library.html uses 14px icons with 260px tooltips, settings.html uses 16px icons with 280px tooltips. This is appropriate - settings page has more breathing room.
  • ℹ️ CSS duplication: The .hint-icon CSS is duplicated in both templates. Could be moved to a shared stylesheet in a future refactor, but not blocking for this PR.
  • ℹ️ Unused function: get_hint(key, default) is defined but not currently used. This is fine - it's a public API function for programmatic access to hints.

📋 Scope Verification

Issue Problem Addressed? Notes
#103 Users lack contextual explanations for toggles, status messages, and features Fully implemented - 68 hints covering: identification layers (4), AI providers (4), status types (9), all settings tabs (Library, Watch, Processing, AI, Safety, Advanced), confidence system, trust modes, source icons, and UI actions

Additional scope delivered (bonus features not in original issue):

  • Filter chip tooltips on library page
  • Action button tooltips (Scan Library, Process Queue)
  • Details column tooltip explaining confidence percentages
  • Source icon explanations (bookdb, audio, AI, ID3, JSON, path, APIs, user)

Scope Status: SCOPE_OK - Original problem fully solved, plus additional helpful tooltips

📝 Documentation Check

  • CHANGELOG.md: ✅ Updated with detailed entry explaining the feature
  • README.md: ⚠️ Consider updating - This is a significant UX improvement that makes the app more user-friendly. A mention in the features list or recent changes section would help new users discover it.

🎯 Verdict

APPROVE

This PR delivers exactly what Issue #103 requested: contextual help for users who don't understand what toggles and features do. The implementation is:

  • Secure: No injection vectors, defensive coding, proper escaping
  • Maintainable: Clean module structure, follows existing patterns
  • User-friendly: Plain language, comprehensive coverage, accessible
  • Well-documented: CHANGELOG updated, accurate descriptions

No changes required. This is production-ready.

Optional future enhancement (not blocking): Consider adding the hover hint feature to README.md's feature list to make it more discoverable to new users.

@deucebucket deucebucket merged commit d053ede into develop Feb 11, 2026
3 checks passed
@deucebucket deucebucket deleted the feature/issue-103-hints-system branch February 11, 2026 11:11
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