Wire ArchiveTableModel into ArchiveTab (QTableView migration, Phase 4 pt 1)

[ebuzerdrmz44]

Description

Wires the ArchiveTableModel from #2517 into ArchiveTab, migrating the Archive tab from item-based QTableWidget to QTableView + QSortFilterProxyModel.Part of #2361 Phase 4.

⚠️ Stacked on #2517 : the diff currently includes the spike commits. Only the top 2 commits are new here; those review needed are:

• 3088310 Wire ArchiveTableModel into ArchiveTab
• 2d61227 Scope delete to repo, preserve scroll on refresh, ungate mount-open

Once #2517 lands I'll rebase onto master and the spike commits drop out.

Key changes:

• .ui: QTableWidget → QTableView; sort via QSortFilterProxyModel + setSortRole(SortRole) so non-string columns order by raw value, not lexically.
• Selection resolved through a single selected_archives() helper reading ArchiveRole : proxy-safe, so actions hit the right archive after sorting. All read sites (incl. archive_extract/archive_mount) route through it.
• In-place rename via view.edit() + model dataChanged; NoEditTriggers makes the explicit edit the only editor entry.
• Delete / failed rename refresh via populate_from_profile(preserve_view=True) (keeps scroll offset instead of jumping to top); delete query scoped to repo.

Related Issue

#2361 (Phase 4). Builds on #2517.

Motivation and Context

The item-based QTableWidget mixed data, formatting, and sort keys into widget items, which made sorting fragile (durations >24h sorted lexically, e.g. '1 day, …') and selection-after-sort error-prone. Moving to a QAbstractTableModel + sort proxy separates data from presentation, fixes the sort-key bugs, and makes selection robust under sorting via a custom data role.

How Has This Been Tested?

make test-unit (Borg 1.2.4) green, plus new tests for selection-after-sort and rename-failure revert. Manually checked sort → delete/rename hit the right archive.

Screenshots (if appropriate):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• I have read the CONTRIBUTING guide.
• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

I provide my contribution under the terms of the license of this repository and I affirm the Developer Certificate of Origin.

[m3nu]

These were intentionally carried forward from #2517 so that one could merge quickly as a clean spike — they belong here, where the model goes live:

• Drop archive_at() (archive_table_model.py:78) — superseded by ArchiveRole, which this PR uses everywhere; it now has no production caller, only its own tests. Leaving a source-indexed accessor is the P1 footgun in latent form. Remove it (and test_archive_at_*), or document it as source-indexed only.
• Test the P2 icon-cache invalidation — the dark-mode clear/rebuild branch (archive_table_model.py:150-156) is untested, though it was the whole point of the P2 fix. Add a test that flips uses_dark_mode and asserts the cache rebuilds, plus a cache-hit identity check.
• Document the setData contract — it overwrites row.name before the rename runs, which only works because the view stashes renamed_archive_original_name at edit-start (archive_tab.py:1033). Worth one line in the setData docstring so the dependency is explicit.
• Nit: _icon_cache: Dict[str, Any] → Dict[str, QIcon].

[ebuzerdrmz44]

Thanks!, addressed .
Also rebased onto current master so the diff is just the wiring

[m3nu]

Reviewed locally — two bugs found. The first is in this PR's new code; the second is pre-existing but sits right in the rename flow this PR reworks, so flagging it here too.

---

1. Size sorting breaks for archives ≥ 2 GiB (introduced here)

Sorting the Size column produces a wrong order for any archive of 2 GiB or larger — i.e. most real backups. Small archives sort fine, which is why it looks correct in light testing.

Root cause: the model returns a raw Python int for SortRole:

# archive_table_model.py
if column == self.COL_SIZE:
    return row.size or 0

The sort runs through a plain QSortFilterProxyModel, whose C++ lessThan compares the value as a 32-bit int. Sizes ≥ 2³¹ bytes (2 GiB) overflow that int and wrap, so the comparison operates on truncated garbage.

Repro (model + proxy, ascending sort on the Size column):

order:    ['10GB', '3GB', '500MB', '5GB', '2GB']            ← wrong
sortvals: [10737418240, 3221225472, 524288000,
           5368709120, 2147483648]                          ← the SortRole values themselves are correct
expected: ['500MB', '2GB', '3GB', '5GB', '10GB']

COL_DURATION has the same latent flaw, but durations don't realistically exceed 2³¹ seconds (~68 years), so only size bites in practice.

Why CI stays green: the archive_env fixtures create archives with size = None, so every size compares as 0 and the ordering is never actually exercised. The two new sort tests use the Name column, not Size.

Suggested fix: compare in Python instead of letting C++ truncate. A proxy subclass whose lessThan reads SortRole round-trips the full Python int and fixes size, duration, and any future numeric column with no precision loss:

class ArchiveSortProxyModel(QSortFilterProxyModel):
    def lessThan(self, left, right):
        lv, rv = left.data(ArchiveTableModel.SortRole), right.data(ArchiveTableModel.SortRole)
        if lv is None:
            return True
        if rv is None:
            return False
        return lv < rv

Use it in place of the bare QSortFilterProxyModel in archive_tab.py. Verified to yield ['500MB','2GB','3GB','5GB','10GB']. (A float(row.size) cast in the model also works but loses precision above 2⁵³ and leaves the trap for the next numeric column, so the proxy override is preferable.)

Please also add a regression test with realistic > 2 GiB sizes — the current suite would remain green even with this bug present.

---

2. Start-Backup button stuck "in progress" after a rename (pre-existing, but in this flow)

After a rename, the main-window createStartBtn spinner can stay running indefinitely.

Root cause: every Borg job that drives that spinner pairs two overrides — started_event (emits backup_started_event, spinner on) and finished_event (emits backup_finished_event, spinner off). BorgRenameJob (borg/rename.py) overrides only started_event; its finished_event falls through to the base BorgJob.finished_event, which only does result.emit(result) and never emits backup_finished_event. So a rename turns the spinner on and never turns it off by itself.

• Success path: rename_result calls refresh_archive_info(), which starts a BorgInfoArchiveJob; that job emits backup_finished_event and incidentally clears the spinner — but only because an archive happens to be selected.
• Failure path (returncode != 0): rename_result calls only populate_from_profile(preserve_view=True), which toggles the archive-tab buttons via _toggle_all_buttons, never createStartBtn. No follow-up job ⇒ the spinner stays "in progress" forever.

Deterministic repro: emit backup_started_event (as BorgRenameJob.started_event does), then call tab.rename_result({'returncode': 2}) → createStartBtn.isEnabled() stays False.

Scope: rename.py is untouched by this PR (last changed in #2213), and the old failure branch was self._toggle_all_buttons(True) — also archive-tab-only, also never stopped createStartBtn. So this predates the PR. Raising it here because it lives in the rename path this PR reworks; happy to split it into its own issue/PR if you'd prefer.

Suggested fix: give BorgRenameJob a finished_event like every other job:

def finished_event(self, result):
    self.app.backup_finished_event.emit(result)
    self.result.emit(result)

This stops the spinner on both success and failure and removes the success path's accidental dependency on the info-refresh side effect.

[ebuzerdrmz44]

@m3nu Just wanted to share my roadmap for the next few days. The Source tab is already open. I'll get Phase 4 Part 2 opened on Monday, and the other tabs' refactoring (smaller ones) on Tuesday. If everything goes smoothly with the review cycles, I am hoping to fully complete the view migration mid next week. Then, for next week, I plan to set up a draft PR for the scheduler refactor so we can start discussing the implementation. Let me know if you have any feedback on this schedule

[m3nu]

🔍 Code Review

PR: #2519 — Wire ArchiveTableModel into ArchiveTab (Phase 4 pt 1 of #2361)
Changes: +256 / −259 across 14 files · CI green (lint, unit macOS+Ubuntu, integration)

✅ Prior review items — all 6 verified fixed, with tests

• archive_at() dropped; ArchiveRole is the only accessor.
• P2 icon-cache invalidation now tested (test_trigger_icon_cache_hits_and_rebuilds_on_theme_switch: cache-hit identity + rebuild on theme flip).
• setData contract documented (overwrites row.name; view stashes the original at edit-start).
• Dict[str, QIcon] typing fixed.
• ≥2 GiB size-sort overflow fixed via ArchiveSortProxyModel.lessThan comparing SortRole in Python, with a >2 GiB regression test that runs through the proxy (500 MiB / 2 GiB / 10 GiB).
• Rename spinner: BorgRenameJob.finished_event matches the create.py pattern; base run() calls it once, so no double result emit.

✅ Independent checks — clean

• All read/destructive sites (extract, mount, check, delete, diff, copy, refresh, selected_archive_name) route through selected_archives() reading ArchiveRole off proxy indices — sort-safe, covered by test_selection_maps_through_sort_proxy.
• NoEditTriggers + explicit edit() make cell_double_clicked the only editor entry, so the is_editing/original-name stash can't be bypassed; on_name_edited clears is_editing before _revert_name, so the revert's dataChanged can't re-enter.
• Delete DB query now repo-scoped — fixes a pre-existing cross-repo delete bug.
• Zero leftover QTableWidget-API callers (.item(, row_of_archive, cellDoubleClicked, editItem) in src/ or tests/.
• #2361 decisions respected: model in views/partials/, no DB reads in the model (use_fixed_units injected), i18n contexts preserved (Form headers, ArchiveTab tooltips).

🔍 Review Details

1 inline comment (🔵 info: dead import — fine to fix in Phase 4 pt 2).

Optional hardening, not blocking: ArchiveSortProxyModel.lessThan returns True both ways when both keys are None (violates strict weak ordering). Unreachable today since _sort_data never returns None for valid columns; if lv is None: return rv is not None would future-proof it.

📊 Verdict: APPROVE ✅

Mergeable as-is. Nice, thorough round — every item from the last review landed with a test attached.

---

🤖 Reviewed by Claude Code

[m3nu]

🔵 [dead-import] SIZE_DECIMAL_DIGITS is imported but unused

The constant moved into archive_table_model.py, which uses it internally — nothing in this file references it anymore. Ruff doesn't flag it because F401 is in the ignore list in pyproject.toml.

Fix:

from vorta.views.partials.archive_table_model import ArchiveSortProxyModel, ArchiveTableModel

Fine to fold into Phase 4 pt 2 rather than re-spin CI here.