refactor(source): convert source files table to QTableView + QAbstractTableModel

[ebuzerdrmz44]

Description

Migrates the Source tab's file list from an item-based QTableWidget to a QTableView backed by a dedicated QAbstractTableModel (SourceFilesModel in views/partials/).

source_tab.py shrinks substantially: the imperative "disable sort → setItem loop → re-enable sort" choreography in add_source_to_table, set_path_info, update_path_info, and populate_from_profile is replaced by model calls. Per-row size/count recalculation now updates the backing SourceFileModel in place and emits dataChanged, instead of poking QTableWidgetItems by row index.

Related Issue

Part of #2361 (finish the views refactor / Phase 5 table-model migration).

Motivation and Context

QTableWidget couples data and presentation and forces row-index bookkeeping, which is exactly what caused the historical mid-recalculation crash (#1080 / #2435). Moving to QTableView + QAbstractTableModel makes the model the single source of truth and lets sort/selection compose through a QSortFilterProxyModel.

Key design points:

• Numeric-safe sorting via a shared SortProxyModel. The model exposes raw comparable values (dir_size as int, file-count as int) under a SortRole (Qt.UserRole); SortProxyModel.lessThan compares those keys in Python so sizes ≥ 2 GiB don't hit Qt's C++ 32-bit int truncation. The old SizeItem/FilesCount __lt__ hacks are gone.
• Selection via a SourceRole (UserRole + 1), returned from data(). Callers resolve the backing object with index.data(SourceRole) on proxy indices, which QSortFilterProxyModel auto-forwards through mapToSource.
• Themed path icons are cached on the model, keyed on dark-mode state and invalidated on theme switch, rather than re-rendered from disk on every paint.
• Path keying for recalculation results: set_path_info matches on dir and returns None when the source was removed mid-calculation, so the caller skips persistence.
• i18n preserved: headers keep the .ui's Form context (and "Calculating…" its SourceTab context), so existing catalog entries are reused.

Rebased on #2519 — shared sort proxy + SizeItem removed

#2519 (Archive tab, Phase 4 pt 1) merged first, so this PR is rebased onto it and folds in the cleanup that merge triggered:

• Extracted one shared sort proxy into views/partials/sort_proxy.py: a single generic SortProxyModel keying off Qt.UserRole, now used by both the Archive and Source tables. This removes the near-duplicate per-model proxy classes (ArchiveSortProxyModel and this PR's original SortProxyModel) before the remaining tab migrations spawn a third copy. It also fixes the both-None strict-weak-ordering nit deferred from Wire ArchiveTableModel into ArchiveTab (QTableView migration, Phase 4 pt 1) #2519 (if lv is None: return rv is not None).
• Deleted the now-dead SizeItem class and its orphaned imports (QTableWidgetItem, sort_sizes) from source_tab.py — archive_tab.py no longer references it after Wire ArchiveTableModel into ArchiveTab (QTableView migration, Phase 4 pt 1) #2519.
• Dropped a dead SIZE_DECIMAL_DIGITS import left in archive_tab.py by Wire ArchiveTableModel into ArchiveTab (QTableView migration, Phase 4 pt 1) #2519.
• Moved updateThreads from a class attribute to an instance attribute so worker lists aren't shared across SourceTab instances.

How Has This Been Tested?

• New tests/unit/test_source_files_table_model.py, including a > 2 GiB size-sort regression test that runs through SortProxyModel.
• Existing tests/unit/test_source.py updated; tests/unit/test_archive_table_model.py switched to the shared proxy.

Types of changes

• Refactor (non-breaking change; no user-facing behavior change)

Checklist:

• My code follows the code style of this project.
• I have added tests to cover my changes.
• All new and existing tests passed.

[ebuzerdrmz44]

Added SortProxyModel to fix the same ≥2 GiB size-sort overflow caught in #2519 .

This is intentionally a near-duplicate of #2519's ArchiveSortProxyModel. I kept them per-PR so neither blocks the other . Not sure that's the right call long-term though: how do you think we should handle the duplication? Dedupe into one shared proxy now, or leave it until the first of the two merges and clean it up then?

[m3nu]

🔍 Code Review

PR: #2523 — refactor(source): convert source files table to QTableView + QAbstractTableModel
Branch: refactor/source-table-model → master
Changes: +465 / −178 across 5 files

📋 Summary

Solid migration that honors every #2361 Phase 5 decision — but #2519 merged earlier today, which answers two open questions in this PR and calls for one cleanup round before merge.

✅ What's Good

• SourceRole resolves backing objects through the proxy on the destructive source_remove path — exactly the mapToSource footgun the #2361 decisions warn about.
• Path-keyed recalculation with a None return kills the #1080/#2435 row-index crash class, with a test for the removed-mid-calculation case.
• The SortProxyModel correctly follows the post-#2519 pattern, and the >2 GiB sort test covers the exact truncation regression that pattern exists for.
• i18n contexts check out (Form matches the .ui <class>; Calculating… keeps SourceTab), no DB reads in the model, thorough model tests.

🔍 Review Details

4 inline comment(s).

Severity	Count
🟡 Warning	2
🔵 Info	2

Re: your dedup question in the comments — merge order answered it: #2519 landed first, so I'd consolidate in this PR. Extract a shared generic proxy (both SortRoles are Qt.UserRole) into e.g. views/partials/sort_proxy.py and use it from both tables, before the smaller-tab migrations create a third copy. Details inline.

One non-inline nit: the PR description still says sorting is "not a lessThan proxy subclass" — stale relative to the code, worth updating so future archaeology isn't misled.

📊 Verdict: COMMENT 💬

Not merging as-is, but these are consequences of merge timing, not defects. One cleanup round and this is good to go.

---

🤖 Reviewed by Claude Code

[m3nu]

🟡 [dead-code] SizeItem is now dead code — #2519 landed first

Your cross-PR note assigns SizeItem cleanup to "whichever of the two lands second". #2519 merged today and current master's archive_tab.py no longer imports SizeItem — so the cleanup now belongs to this PR.

Fix: Delete the SizeItem class, plus the imports that become orphaned with it: QTableWidgetItem and sort_sizes. Note the ruff config ignores F401, so lint won't flag the dead imports — needs the manual sweep.

[m3nu]

🟡 [duplication] Consolidate with ArchiveSortProxyModel (now on master)

This is a line-for-line duplicate of ArchiveSortProxyModel (archive_table_model.py:199), and per your PR comment the plan was to dedupe once the first of the two merged — #2519 has now merged.

Since both models define SortRole = Qt.ItemDataRole.UserRole, the proxy doesn't actually need a per-model class at all.

Fix: Extract one shared proxy, e.g. views/partials/sort_proxy.py:

class SortProxyModel(QSortFilterProxyModel):
    """Sort proxy comparing `Qt.UserRole` keys in Python to avoid Qt's 32-bit int truncation."""

    def lessThan(self, left: QModelIndex, right: QModelIndex) -> bool:
        lv = left.data(Qt.ItemDataRole.UserRole)
        rv = right.data(Qt.ItemDataRole.UserRole)
        if lv is None:
            return rv is not None
        if rv is None:
            return False
        return lv < rv

and use it here and in archive_table_model.py (drop ArchiveSortProxyModel or alias it). This prevents a third copy appearing in the upcoming smaller-tab migrations.

[m3nu]

🔵 [strict-weak-ordering] Both-None case violates strict weak ordering

When lv and rv are both None, lessThan(a, b) and lessThan(b, a) both return True, which violates the strict-weak-ordering contract Qt's sort relies on and can produce unstable orderings.

Fix:

if lv is None:
    return rv is not None

This is the same nit deferred from #2519 — fixing it once in the shared proxy (see comment above) resolves both tables.

[m3nu]

🔵 [behavior] Full repopulate drops "Calculating…" placeholders on other rows

populate_from_profile() → set_rows() clears the model's _calculating set. So removing one source while others are mid-recalculation blanks their "Calculating…" cells (the data still lands correctly when each worker finishes — only the placeholder is lost). It also resets any ad-hoc sort back to the saved settings.

The old code removed rows without touching the rest of the table.

Suggestion (fine as a follow-up): add a targeted remove_source(source) to the model using beginRemoveRows/endRemoveRows instead of repopulating, leaving _calculating and the current sort untouched.

[ebuzerdrmz44]

on the Calculating…-placeholder-on-repopulate point . I will add the targeted remove_source() (beginRemoveRows/endRemoveRows) in a separate small PR rather than widen this one.

[m3nu]

Reviewed in depth — the migration is correct and pattern-faithful: model in partials/, raw SortRole keys, every index access resolved via index.data(SourceRole) so the proxy footgun is handled, and per-path (rather than per-row-index) async updates kill the #1080/#2435 crash class outright. Nice catches along the way: updateThreads was a shared class attribute on master, and the shared SortProxyModel extraction includes the both-None strict-weak-ordering fix. The 18 new model tests are solid.

Two asks before merge:

1. Guard against overlapping recalculations of the same path. _discard_update_thread disconnects and drops all threads whose objectName() matches the path. If the same path is recalculated twice concurrently (double-click on Update), the first result to arrive drops the only reference to the still-running second thread — "QThread destroyed while running" territory. Simplest fix: skip spawning in update_path_info when the path is already pending.
2. Add a remove-under-sort test: sort by size descending, select the first visual row, call source_remove(), assert the correct DB record was deleted. That's the exact proxy/mapToSource scenario from #2361 and the one convention-critical path currently untested.

Known and fine to defer to the beginRemoveRows follow-up you already mentioned: the full model reset in source_remove wipes other rows' "Calculating…" placeholders and the selection.

Merge order: this lands after #2525/#2526 and will need a rebase around the archive_tab.py import block (this PR deletes ArchiveSortProxyModel).

Take-or-leave nits: pass a roles list on the dataChanged emits (the archive model does), type the icon cache Dict[str, QIcon], header.setVisible(True) is redundant now that the .ui attribute is gone, and there's a stray blank line in the sort_proxy.py docstring.

[ebuzerdrmz44]

addressed , waiting for #2526 to merge so that we can marge this too after rebase