fix(roms): sort grouped galleries by the best sibling's key

[zurdi15]

Description

Fixes #4447

In grouped galleries, a group sorted by the representative rom's key, so a group whose representative carried no value (never played, unrated) landed in the NULL bucket even when a sibling had one. The gallery now sorts each group by the best visible sibling's key.

• The dedup window gains a MAX(key) OVER (partition) aggregate on descending sorts and MIN on ascending ones, for RomUser-column sorts whose type (DateTime, Integer) keeps MIN/MAX consistent with ORDER BY semantics on every dialect. The aggregate shares row_number's window spec with a whole-partition frame, so the derived table is sorted once.
• A hidden sibling's key is masked to NULL inside the aggregate, so a rom the user hid cannot drive its group's position; the zero-as-unset NULLIF fold from fix(roms): sort unset per-user and metadata keys last on every engine #4454 wraps the aggregate input, so an unrated sibling cannot drag a group to rating 0.
• The group key joins out through a null-safe outer join (IS NOT DISTINCT FROM), which pins MariaDB's covering-index plan; measured with EXPLAIN at ~1.55x the ungrouped cost on the fixed query family, with the rejected shapes at 3-7x. Non-aggregated sorts (enum, lexical, roms-side metadata keys) keep byte-identical SQL and representative semantics: an aggregated roms-side key would push the window off idx_roms_sibling_cover, and MIN/MAX diverges from ORDER BY for enums.
• get_roms_query and the grouped reorder now share one _resolve_gallery_sort_key/_gallery_order_clauses pair; the endpoint resolves the sort once and filter_roms consumes the resolved key. Bogus order_by values (relationship or dunder names) now fall back to the name sort instead of raising while the ORDER BY is built.

Ran the full pr-ready gauntlet (security-audit: clean; code-review xhigh; simplify; review-polish), one commit per pass, plus a merge of current master resolving the overlap with #4454's NULLS LAST rework. 165 tests across the affected suites pass on MariaDB, trunk is clean. Backend-only, no migrations, no API shape change.

Deliberate scope, noted for reviewers: the aggregate is direction-dependent (a group spanning old and new plays takes the extreme end of its range in both directions, as the issue requests); enum/boolean/roms-side keys keep representative semantics; the PostgreSQL plan for the null-safe join shape was not benchmarked (MariaDB was) and deserves an EXPLAIN before release. The stale-sidecar-cache gap that review surfaced is fixed separately by #4463.

Checklist

• I've tested the changes locally
• I've updated relevant comments
• I've assigned reviewers for this PR
• I've added unit tests that cover the changes

AI disclosure: This PR was implemented and reviewed end to end with Claude Code (implementation, the four-pass review gauntlet, EXPLAIN-based plan validation, and tests), directed and supervised by the maintainer.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Ldi4YaXJWDDmFdbKfQay8L

[greptile-apps]

Greptile Summary

This PR centralizes gallery sort-key resolution and changes grouped galleries sorted by numeric or date-valued RomUser fields to use the best sibling’s value.

• Adds direction-aware whole-partition MIN/MAX aggregation for eligible per-user keys.
• Masks personally hidden siblings and zero-as-unset values.
• Carries the aggregate through the grouped dedup query and reuses shared ordering clauses.
• Adds query-shape and result tests for grouped ordering, hidden personal siblings, counts, and ID indexes.

Confidence Score: 4/5

The PR is not safe to merge until permission-hidden siblings are prevented from determining visible groups’ sort positions.

The new aggregate evaluates all siblings before hidden_rom_ids is applied, so an inaccessible non-representative ROM can supply the value used to order a visible group.

Files Needing Attention: backend/handler/database/roms_handler.py

Important Files Changed

Filename	Overview
backend/handler/database/roms_handler.py	Implements shared sort resolution and grouped sibling aggregation, but permission-hidden siblings can still determine a visible group’s position.
backend/endpoints/roms/init.py	Propagates the resolved gallery sort key and ordering parameters into filtering and character-index generation.
backend/tests/handler/database/test_roms_rom_user_sort.py	Adds strong grouped-sort coverage, although it tests personal hiding rather than admin-driven hidden-ROM permissions.
backend/tests/handler/database/test_roms_group_by_index.py	Extends query-shape coverage to ensure the aggregate window remains compatible with the covering index.
backend/tests/handler/database/test_roms_metadata_sort.py	Confirms ROM-side metadata sorts retain representative-key behavior.

Prompt To Fix All With AI

### Issue 1
backend/handler/database/roms_handler.py:1708-1710
**Hidden siblings affect sorting**

When a non-representative sibling is hidden through `hidden_rom_ids`, this aggregate still includes it because the CASE masks only `RomUser.hidden`. The permission filter runs later against the representative row, so an inaccessible sibling with the newest play or highest rating can move a visible group to the wrong position. Exclude permission-hidden siblings from the aggregate or apply those visibility constraints before building the deduplication window.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (1): Last reviewed commit: "review(pr-ready step 4): tighten the sor..." | Re-trigger Greptile}

[Copilot]

🟡 Changes recommended

Hidden ROM/platform rows can still affect representative selection and grouped ordering, and invalid order_by fallback coverage is still requested.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Fixes grouped gallery ordering by sorting on the best eligible sibling key for supported per-user date and integer fields.

Changes:

• Adds direction-aware sibling aggregation.
• Centralizes gallery sort-key resolution.
• Updates endpoint wiring and expands grouped-query and index tests.

File summaries

File	Summary
backend/tests/handler/database/test_roms_rom_user_sort.py	Tests grouped per-user sorting behavior.
backend/tests/handler/database/test_roms_metadata_sort.py	Tests representative metadata sorting.
backend/tests/handler/database/test_roms_group_by_index.py	Verifies covering-index query coverage.
backend/handler/database/roms_handler.py	Implements aggregation and shared sort resolution.
backend/endpoints/roms/__init__.py	Passes resolved sort keys through gallery queries.

Review details

Suppressed comments (1)

backend/handler/database/roms_handler.py:1710

• This CASE masks only RomUser.hidden, but hidden_platform_ids and hidden_rom_ids are applied later at lines 1781-1787. A ROM hidden from the requesting user can therefore still supply the newest last_played or highest rating and move the group's position, even though it is excluded from the result. Apply those admin-visibility predicates to the aggregate input as well, while preserving the current dedup input if representative selection must remain unchanged.

            ]
            window_columns: list[ColumnElement] = [
                func.row_number()

• Files reviewed: 5/5 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

❌ Preview build failed.

View the run