[PATCH RFC v2 14/25] review-tui: resolve the tracked revision in revision lists

Christian Brauner <[email protected]>
Newsgroups org.kernel.linux.tools
Message-ID <[email protected]>
The revisions catalog is not guaranteed a row for the tracked revision,
since manually linking a newer version records only that one.  Anything
reasoning about the versions of a series from the catalog alone can
therefore omit the version the maintainer is actually on, and a
range-diff then cannot resolve the side it is taken against, on a row
whose binding was enabled on exactly that basis.

Add merge_tracked_revisions(), which appends an entry synthesized from
the series row when the catalog lacks one, and route both readers through
it: the DB-side resolver get_revisions_with_tracked() and the TUI's row
builder.  The versions shown and the versions a range-diff can resolve
are then always the same set.

It covers every live series row, not just the furthest along.
rescan_branches() can leave a change_id with more than one and the
tracking list renders each separately, so resolving only one would enable
the range-diff on a version row whose message-id cannot then be found.

The synthesized entry carries neither blob nor read state.  Both live on
the catalog row it stands in for, so a series row that never got one has
none to report.

Signed-off-by: Christian Brauner (Amutable) <[email protected]>
---
 src/b4/review/tracking.py          | 100 +++++++++++++++++++++++++++++++++++++
 src/b4/review_tui/_common.py       |   6 ++-
 src/b4/review_tui/_tracking_app.py |  34 +++++++++++++
 3 files changed, 139 insertions(+), 1 deletion(-)

diff --git a/src/b4/review/tracking.py b/src/b4/review/tracking.py
index 1b26d722..c8d9e7d9 100644
--- a/src/b4/review/tracking.py
+++ b/src/b4/review/tracking.py
@@ -1649,6 +1649,106 @@ def get_revisions(conn: sqlite3.Connection, change_id: str) -> list[dict[str, An
     return [dict(zip(_REVISION_COLS, row)) for row in cursor.fetchall()]
 
 
+def merge_tracked_revisions(
+    change_id: str,
+    revs: List[dict[str, Any]],
+    series_rows: List[dict[str, Any]],
+) -> List[dict[str, Any]]:
+    """Merge catalog *revs* with the revisions the *series_rows* track.
+
+    A catalog row for a tracked revision is not guaranteed -- manually
+    linking a newer version records only that one -- so anything that
+    reasons about "the versions of this series" from the catalog alone
+    can silently omit the one the maintainer is actually on.  Synthesize
+    an entry from the series row when it is missing.
+
+    Every series row, not just one: rescan_branches can leave a change_id
+    with more than one, and the tracking list renders each of them
+    separately.  Resolving only one would enable the range-diff on a
+    version row whose message-id cannot then be found.  Both the DB
+    resolver (:func:`get_revisions_with_tracked`) and the TUI's row
+    builder go through here, so the versions shown and the versions the
+    range-diff can resolve are always the same set.
+
+    Every write path that points a series row at a revision catalogues it
+    first (:func:`_ensure_catalog_row`), and the v11 backfill did the same
+    for every row that predates that rule, so this is a fallback for a
+    database written before it and not a reconciliation step: nothing here
+    fixes up a value the catalog also holds.
+
+    The synthesized entry carries neither blob nor read state.  Both live
+    on the catalog row this one stands in for, so a series row that never
+    got one has none to report -- the entry exists to supply a message-id
+    and a subject, not a badge.
+
+    *series_rows* entries carry ``revision``, ``message_id``, ``subject``,
+    ``found_at``, ``fingerprint`` and ``is_rethreaded``.
+    """
+    revs = list(revs)
+    known = {int(r['revision']) for r in revs if r.get('revision') is not None}
+    added = False
+    for row in series_rows:
+        if not row.get('message_id'):
+            continue
+        tracked = int(row.get('revision') or 1)
+        if tracked in known:
+            continue
+        known.add(tracked)
+        added = True
+        revs.append(
+            {
+                'change_id': change_id,
+                'revision': tracked,
+                'message_id': row['message_id'],
+                'subject': row.get('subject'),
+                'link': '',
+                'found_at': row.get('found_at') or '',
+                'thread_blob': '',
+                'series_blob': '',
+                'fingerprint': row.get('fingerprint'),
+                'source': 'tracked',
+                'is_rethreaded': bool(row.get('is_rethreaded')),
+                'message_count': None,
+                'seen_message_count': None,
+                'last_update_check': None,
+                'last_mail_at': None,
+            }
+        )
+    if added:
+        revs.sort(key=lambda r: r.get('revision') or 0)
+    return revs
+
+
+def get_revisions_with_tracked(
+    conn: sqlite3.Connection, change_id: str
+) -> list[dict[str, Any]]:
+    """get_revisions(), guaranteed to include the revision being tracked.
+
+    :func:`merge_tracked_revisions` over the catalog and every live series
+    row -- see there for why the merge works the way it does.
+    """
+    # Positional access: callers may hand us a connection without a
+    # sqlite3.Row factory (init_db does not set one).
+    rows = [
+        {
+            'revision': row[0],
+            'message_id': row[1],
+            'subject': row[2],
+            'found_at': row[3],
+            'fingerprint': row[4],
+            'is_rethreaded': row[5],
+        }
+        for row in conn.execute(
+            'SELECT revision, message_id, subject, COALESCE(sent_at, added_at),'
+            ' fingerprint, is_rethreaded FROM series'
+            " WHERE change_id = ? AND COALESCE(status, 'new') != 'archived'"
+            ' ORDER BY revision DESC',
+            (change_id,),
+        )
+    ]
+    return merge_tracked_revisions(change_id, get_revisions(conn, change_id), rows)
+
+
 def find_revision_by_fingerprint(
     conn: sqlite3.Connection, fingerprint: Optional[str]
 ) -> Optional[dict[str, Any]]:
diff --git a/src/b4/review_tui/_common.py b/src/b4/review_tui/_common.py
index 34cd5d28..2f161c2c 100644
--- a/src/b4/review_tui/_common.py
+++ b/src/b4/review_tui/_common.py
@@ -1237,7 +1237,11 @@ def compute_range_diff(
     """
     try:
         conn = b4.review.tracking.get_db(identifier)
-        revisions = b4.review.tracking.get_revisions(conn, change_id)
+        # Must include the tracked revision even when the catalog has no
+        # row for it, or the side this diff is taken against cannot be
+        # resolved -- and the TUI enables the action on exactly that
+        # basis.
+        revisions = b4.review.tracking.get_revisions_with_tracked(conn, change_id)
         conn.close()
     except Exception as ex:
         logger.critical('Could not load revisions: %s', ex)
diff --git a/src/b4/review_tui/_tracking_app.py b/src/b4/review_tui/_tracking_app.py
index 31601ded..bcad86f3 100644
--- a/src/b4/review_tui/_tracking_app.py
+++ b/src/b4/review_tui/_tracking_app.py
@@ -1746,6 +1746,40 @@ class TrackingApp(LoreNodeShutdownMixin, CheckRunnerMixin, App[Optional[str]]):
             )
         )
 
+    @staticmethod
+    def _merge_tracked_revision(series: Dict[str, Any]) -> List[Dict[str, Any]]:
+        """Known revisions of a series, guaranteed to include the tracked ones.
+
+        tracking.merge_tracked_revisions() over the preloaded catalog rows
+        and every live series row of the change_id (stashed by
+        _load_series), so the version rows shown here and the revisions
+        compute_range_diff can resolve are always the same set.
+
+        Memoized onto the series dict: the answer gates two bindings, so it
+        is recomputed on every refresh_bindings() for every cursor move.
+        _load_series rebuilds these dicts from scratch, which is what keeps
+        the memo from outliving the data it was derived from.
+        """
+        cached: Optional[List[Dict[str, Any]]] = series.get('_versions')
+        if cached is not None:
+            return cached
+        rows = [
+            {
+                'revision': s.get('revision', 1),
+                'message_id': s.get('message_id', ''),
+                'subject': s.get('subject'),
+                'found_at': s.get('sent_at') or s.get('added_at') or '',
+                'fingerprint': s.get('fingerprint'),
+                'is_rethreaded': s.get('is_rethreaded'),
+            }
+            for s in (series.get('_sibling_rows') or [series])
+        ]
+        revs = b4.review.tracking.merge_tracked_revisions(
+            series.get('change_id', ''), series.get('_revisions') or [], rows
+        )
+        series['_versions'] = revs
+        return revs
+
     def _checkout_new_series(self) -> None:
         """Retrieve series, build am-ready mbox, and show base selection."""
         series = self._selected_series

-- 
2.53.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.