Global filters, unit tests and UI enhancements

[davidp57]

• Add PREDEFINED_QUERIES with 18 filters across 4 categories
• Implement global filter persistence via localStorage
• Convert /random to full page with configurable game grid
• Add comprehensive test suite (69 tests, 100% passing)
• Optimize performance with indexes and query caching
• Document complete filter system architecture
• Remove 'apply globally' checkbox for simpler UX

I used openspec and github copilot (Claude Sonnet 4.5) to work on this one. That's why openspec and .github files were added.

This is for issue #18

[Copilot]

Pull request overview

This PR implements a comprehensive predefined query filters system with 18 filters across 4 categories (Gameplay, Ratings, Dates, Content), adds global filter persistence via localStorage, and converts the /random endpoint to a full page with configurable game grids. The implementation includes extensive testing (69 tests with 100% pass rate), performance optimizations with database indexes and query caching, and complete documentation.

Changes:

• Predefined query filter system with 18 filters organized in 4 categories
• Global filter persistence using localStorage (filters apply across all pages)
• Reusable filter components (_filter_bar.html, filters.css, filters.js)
• Comprehensive test suite covering unit tests, integration tests, performance tests, and edge cases
• Database indexes for filter performance optimization
• Documentation updates (filter system architecture, SQL reference, database schema)

Reviewed changes

Copilot reviewed 58 out of 60 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
web/utils/filters.py	Defines PREDEFINED_QUERIES, display names, categories, and descriptions for 18 filters
web/utils/helpers.py	Adds get_query_filter_counts() for optimized filter result counting
web/static/js/filters.js	Implements global filter management with localStorage persistence and event handling
web/static/css/filters.css	Provides comprehensive filter bar styling with responsive design
web/templates/_filter_bar.html	Reusable filter bar component with dropdowns and search
web/routes/library.py	Adds queries parameter support and filter count calculation
web/routes/collections.py	Integrates filters into collection detail pages
web/routes/discover.py	Adds filter bar to discover page
web/routes/settings.py	Updates TemplateResponse to new Starlette API
web/database.py	Creates indexes for filter columns and popularity cache table
web/main.py	Calls new database initialization functions on startup
tests/*.py	Comprehensive test suite (69 tests total)
docs/*.md	Complete technical documentation for filter system
README.md	Documents new Smart Filters feature
CHANGELOG.md	Complete change log for this release

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[sam1am]

Thanks for putting this together — the global filter system and predefined queries are a solid feature addition. I've done a detailed review and have some items to address before we can merge.

Bugs

SQL errors on collection detail page
The .replace() chain in collections.py for prefixing column names with g. only covers 5 columns (playtime_hours, total_rating, added_at, release_date, nsfw). Filters like "community-favorites" (igdb_rating, igdb_rating_count), "critic-favorites" (aggregated_rating), and "recently-updated" (last_modified) will throw SQL errors when used on collection pages.

Genre LIKE pattern missing closing quote
In discover.py, library.py (random route), and collections.py:

params.append(f'%"{genre.lower()}%')

Should be:

params.append(f'%"{genre.lower()}"%')

Without the closing ", the pattern %"action% could match unintended substrings.

Duplicate import in discover.py
get_query_filter_counts is imported twice on the same line.

Bare except: clauses
In collections.py and discover.py, the genre parsing uses bare except: which swallows all exceptions including KeyboardInterrupt and SystemExit. Please use except Exception: at minimum, or better yet except (json.JSONDecodeError, TypeError):.

import json inside a loop
In collections.py, import json is inside the genre-parsing loop — should be at the top of the file.

Breaking Changes

/random endpoint behavior change
This currently redirects to a single random game detail page. The PR changes it to render a full grid of 12 games. I'd like to discuss this — I'm not sure I want to lose the "surprise me with one game" behavior. Could we keep the old redirect as the default and add the grid as a separate route (e.g. /random?grid=true) or a different path?

TemplateResponse signature change
All TemplateResponse calls were changed from TemplateResponse("template.html", {"request": request, ...}) to TemplateResponse(request, "template.html", {...}). This requires a newer Starlette version. Since we don't pin versions in requirements.txt, this could break depending on the installed version. Can you either pin the minimum required version or keep the old signature?

Cleanup Requests

Please remove the OpenSpec / workflow files
The PR includes ~40 files under .github/prompts/ and .github/skills/ that are part of your personal dev workflow tooling. These aren't related to the feature and shouldn't be in the repo. Could you remove them from this PR?

UI/UX Concerns

Auto-apply conflicts with Apply button
The store/genre dropdowns have explicit "Apply" buttons, but filters.js also auto-applies after a 300ms debounce on checkbox changes. Users could get redirected before they finish selecting multiple options. I'd suggest picking one approach — either auto-apply (and remove the button) or manual apply (and remove the debounce).

Inconsistent JS serialization in templates
index.html uses the safe |tojson filter for window globals, but discover.html, collection_detail.html, and random.html use manual string interpolation ('{{ store }}'). If a store name or genre contains a single quote, the JS will break. Please use |tojson consistently.

Mobile regression
The old index.html had mobile-specific styles transforming dropdowns into bottom sheets. The new filters.css responsive section only does flex-direction: column — the bottom-sheet behavior appears to be lost. Could you verify the filter bar works well on mobile?

Inline CSS in random.html
The new page has ~500 lines of inline CSS, much of it duplicated from discover.html. Could this be extracted to a shared stylesheet?

Minor

• Dead .games-grid CSS rule with flex properties in index.html (overridden by the grid definition later — looks like a copy-paste artifact)
• query_filter_counts is always {} for discover, collection detail, and random pages — the Quick Filters dropdown won't show counts on those pages (intentional?)

---

Overall the filter architecture (predefined SQL fragments validated against a whitelist, single-query COUNT approach, localStorage persistence) is well thought out. The bugs above should be straightforward fixes. Happy to re-review once addressed.

[davidp57]

Hi @sam1am!
I've taken all your remarks into account, I hope you'll like my corrections.
PR is ready for your new review now.

[sam1am]

Thanks for the thorough work addressing all the review items, @davidp57! Most of the bug fixes and UX changes look great. There are three remaining issues that need to be resolved before we can merge:

1. .github/ gitignored — this deletes our CI/CD pipeline

The .gitignore now has .github/ which removes our docker-publish.yml workflow that builds and pushes Docker images to GHCR. I know the intent was to remove the OpenSpec/skills files I asked about, but this went too far. Could you change the gitignore to only exclude your specific directories?

# OpenSpec workflow configs (contributor tooling)
.github/prompts/
.github/skills/
openspec/

And make sure docker-publish.yml is restored (it was deleted in commit 7021c5b).

2. FastAPI version pin is too low

The new TemplateResponse(request, "template.html", {...}) signature requires Starlette >=0.29.0, but fastapi>=0.89.0 allows installing FastAPI versions that bundle Starlette 0.22.0 — which would crash every route. The current latest FastAPI is 0.128.5 (requires starlette>=0.40.0). Please update the pin to:

fastapi>=0.110.0

This ensures Starlette >=0.29.0+ is always present. (For reference: FastAPI releases, Starlette 0.29.0 TemplateResponse change)

3. Merge conflicts

The PR currently shows conflicts that will need to be resolved against main.

---

Everything else looks good — the filter architecture is solid, the SQL column prefixing fix covers all the columns, genre LIKE patterns are correct, /random preserves the single-game redirect, and the auto-apply issue is resolved. Looking forward to the updated PR!