Linux maintainer tooling and workflows
 help / color / mirror / Atom feed
From: Christian Brauner <brauner@kernel.org>
To: "Kernel.org Tools" <tools@kernel.org>
Cc: Konstantin Ryabitsev <konstantin@linuxfoundation.org>,
	 "Christian Brauner (Amutable)" <brauner@kernel.org>
Subject: [PATCH RFC v2 14/25] review-tui: resolve the tracked revision in revision lists
Date: Wed, 12 Aug 2026 23:46:55 +0200	[thread overview]
Message-ID: <20260812-work-b4-multiver-rows-v2-14-305d53cd723a@kernel.org> (raw)
In-Reply-To: <20260812-work-b4-multiver-rows-v2-0-305d53cd723a@kernel.org>

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) <brauner@kernel.org>
---
 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


  parent reply	other threads:[~2026-08-12 21:47 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-12 21:46 [PATCH RFC v2 00/25] review: track and browse every version of a tracked series Christian Brauner
2026-08-12 21:46 ` [PATCH RFC v2 01/25] review-tui: fix rethreaded series thread viewing Christian Brauner
2026-08-12 21:46 ` [PATCH RFC v2 02/25] review: do not clear fields a re-adding caller does not know Christian Brauner
2026-08-12 21:46 ` [PATCH RFC v2 03/25] review-tui: keep the rethread flag on an upgraded series row Christian Brauner
2026-08-12 21:46 ` [PATCH RFC v2 04/25] review: guard the tracking-commit amend on the worktree, not the checkout Christian Brauner
2026-08-12 21:46 ` [PATCH RFC v2 05/25] review-tui: recompute an evicted A·R·T cache entry Christian Brauner
2026-08-12 21:46 ` [PATCH RFC v2 06/25] review: test the prerequisite fixes Christian Brauner
2026-08-12 21:46 ` [PATCH RFC v2 07/25] review: serialize schema migrations against a concurrent opener Christian Brauner
2026-08-12 21:46 ` [PATCH RFC v2 08/25] review: test the migration serialization Christian Brauner
2026-08-12 21:46 ` [PATCH RFC v2 09/25] review: track message counts for all revisions of a series Christian Brauner
2026-08-12 21:46 ` [PATCH RFC v2 10/25] review: give per-change_id state its own table Christian Brauner
2026-08-12 21:46 ` [PATCH RFC v2 11/25] review: test per-revision message tracking Christian Brauner
2026-08-12 21:46 ` [PATCH RFC v2 12/25] review-tui: poll every revision on u/U updates Christian Brauner
2026-08-12 21:46 ` [PATCH RFC v2 13/25] review: test the per-revision poll sweep Christian Brauner
2026-08-12 21:46 ` Christian Brauner [this message]
2026-08-12 21:46 ` [PATCH RFC v2 15/25] review-tui: fall back when a cached thread blob has no series Christian Brauner
2026-08-12 21:46 ` [PATCH RFC v2 16/25] review-tui: test revision resolution and the range-diff fallback Christian Brauner
2026-08-12 21:46 ` [PATCH RFC v2 17/25] review: skip the catalog mirror when nothing moved Christian Brauner
2026-08-12 21:46 ` [PATCH RFC v2 18/25] review: match a stray posting by message-id Christian Brauner
2026-08-12 21:47 ` [PATCH RFC v2 19/25] review: add backward discovery of older series revisions Christian Brauner
2026-08-12 21:47 ` [PATCH RFC v2 20/25] review-tui: add a "Find older revisions" action Christian Brauner
2026-08-12 21:47 ` [PATCH RFC v2 21/25] review: test the catalog mirror, stray matching and backward discovery Christian Brauner
2026-08-12 21:47 ` [PATCH RFC v2 22/25] review-tui: extract the Msgs column renderer from TrackedSeriesItem Christian Brauner
2026-08-12 21:47 ` [PATCH RFC v2 23/25] review-tui: give the unseen badge a column of its own Christian Brauner
2026-08-12 21:47 ` [PATCH RFC v2 24/25] review-tui: expand tracked series into per-version rows Christian Brauner
2026-08-12 21:47 ` [PATCH RFC v2 25/25] review-tui: test per-version tracker rows Christian Brauner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260812-work-b4-multiver-rows-v2-14-305d53cd723a@kernel.org \
    --to=brauner@kernel.org \
    --cc=konstantin@linuxfoundation.org \
    --cc=tools@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox