[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