[PATCH b4 2/2] b4 review: test cover-letter titling on the update path

Cover the second revision-identification path, which had no tests of
its own: update_series_tracking() must name a newly discovered
revision after its cover letter, and must repair a series row that an
upgrade left titled after patch 1/N.

The integration tests drive the real code with real messages instead
of a mocked LoreMailbox -- a mock cannot tell a raw LoreSeries from a
get_series()'d one, and that distinction is the bug. Split a
_series_msgs() helper out of _build_lmbx() so the same series shape
feeds both a mailbox and a plain message list.

The unit tests pin realign_series_subject() to the contract that makes
it safe to run on every update: re-title from a cover letter, leave a
prefix-only formatting difference alone, and never write back the
first-patch fallback when the fetched thread has no cover -- that
would mis-title every coverless series.

At the TUI layer, assert the upgrade dialog shows and stores the
resolved series' own title rather than the stale catalog subject, and
falls back to the catalog when the resolved series never learned its
title and reports '(untitled)'.

Signed-off-by: Christian Brauner (Amutable) <[email protected]>
---
 src/tests/test_review_tracking.py | 204 ++++++++++++++++++++++++++++++++++++--
 src/tests/test_tui_tracking.py    |  91 ++++++++++++++++-
 2 files changed, 284 insertions(+), 11 deletions(-)

diff --git a/src/tests/test_review_tracking.py b/src/tests/test_review_tracking.py
index 1eeb41e..ae6a732 100644
--- a/src/tests/test_review_tracking.py
+++ b/src/tests/test_review_tracking.py
@@ -2623,8 +2623,10 @@ class TestUpdateSeriesTrackingCounts:
         v1_mock = mock.Mock()
         v1_mock.revision = 1
         v1_mock.patches = [v1_patch, None, None, None]
+        v1_mock.fingerprint = None
         mock_lmbx = mock.Mock()
         mock_lmbx.series = {1: v1_mock}
+        mock_lmbx.covers = {}
         mock_lmbx.get_series.return_value = None
 
         series_dict: Dict[str, Any] = {
@@ -3514,16 +3516,11 @@ def _pos_diff(n: int) -> str:
     )
 
 
-def _build_lmbx(
+def _series_msgs(
     base: str, author: str, rev: int, n: int, cover: bool = False
-) -> 'b4.LoreMailbox':
-    """Build a LoreMailbox holding one n-patch series at the given revision.
-
-    With *cover*, a 0/n cover letter is included; it lands in the mailbox's
-    parse-time ``covers`` dict (never injected into the series, since these
-    tests do not run ``get_series()``).
-    """
-    lmbx = b4.LoreMailbox()
+) -> list[EmailMessage]:
+    """Return the messages making up an n-patch series at the given revision."""
+    msgs: list[EmailMessage] = []
     if cover:
         msg = EmailMessage()
         msg['Subject'] = f'[PATCH v{rev} 0/{n}] {base}: do things better'
@@ -3531,7 +3528,7 @@ def _build_lmbx(
         msg['Date'] = 'Thu, 19 Mar 2026 08:51:10 +0530'
         msg['Message-Id'] = f'<{base}-v{rev}[email protected]>'
         msg.set_payload('This series makes things better.\n')
-        lmbx.add_message(msg)
+        msgs.append(msg)
     for i in range(1, n + 1):
         msg = EmailMessage()
         msg['Subject'] = f'[PATCH v{rev} {i}/{n}] {base}: part {i}'
@@ -3539,6 +3536,21 @@ def _build_lmbx(
         msg['Date'] = 'Thu, 19 Mar 2026 08:51:12 +0530'
         msg['Message-Id'] = f'<{base}-v{rev}-p{i}@example.com>'
         msg.set_payload(_pos_diff(i))
+        msgs.append(msg)
+    return msgs
+
+
+def _build_lmbx(
+    base: str, author: str, rev: int, n: int, cover: bool = False
+) -> 'b4.LoreMailbox':
+    """Build a LoreMailbox holding one n-patch series at the given revision.
+
+    With *cover*, a 0/n cover letter is included; it lands in the mailbox's
+    parse-time ``covers`` dict (never injected into the series, since these
+    tests do not run ``get_series()``).
+    """
+    lmbx = b4.LoreMailbox()
+    for msg in _series_msgs(base, author, rev, n, cover=cover):
         lmbx.add_message(msg)
     return lmbx
 
@@ -3750,6 +3762,178 @@ class TestRecordDiscoveredCoverSubject:
         assert r6['subject'] == 'Manually linked subject'
 
 
+class TestUpdateSeriesTrackingCoverSubject:
+    """[u]pdate discovers revisions too, and must name them the same way.
+
+    update_series_tracking() recorded them by hand instead of going through
+    _record_discovered_revisions(), so it kept picking the first present patch
+    of a raw LoreSeries (bug 8bb6e4c).  That is the path the TUI actually runs,
+    so an upgrade re-titled the series after patch 1/N.
+    """
+
+    def _update(
+        self, identifier: str, change_id: str, revision: int, msgs: list[EmailMessage]
+    ) -> Dict[str, Any]:
+        series: Dict[str, Any] = {
+            'change_id': change_id,
+            'revision': revision,
+            'status': 'new',
+            'message_id': f'thing-v{revision}[email protected]',
+        }
+        with (
+            mock.patch('b4.review._review.retrieve_series_messages', return_value=msgs),
+            mock.patch('b4.review._review.check_series_attestation', return_value=None),
+        ):
+            return b4.review.update_series_tracking(
+                series, identifier, 'https://example.com/%s'
+            )
+
+    def test_discovered_revision_named_after_cover(
+        self, tmp_path: pytest.TempPathFactory
+    ) -> None:
+        identifier = 'ust-cover'
+        change_id = 'cid-U'
+        conn = review_tracking.init_db(identifier)
+        review_tracking.add_series_to_db(
+            conn,
+            change_id,
+            1,
+            'thing: do things better',
+            'Author',
+            '[email protected]',
+            '2026-03-19T08:51:10+00:00',
+            '[email protected]',
+            3,
+        )
+        review_tracking.add_revision(
+            conn,
+            change_id,
+            1,
+            '[email protected]',
+            subject='[PATCH v1 0/3] thing: do things better',
+        )
+        conn.close()
+
+        msgs = _series_msgs('thing', _AUTHOR, 1, 3, cover=True)
+        msgs += _series_msgs('thing', _AUTHOR, 2, 3, cover=True)
+        result = self._update(identifier, change_id, 1, msgs)
+
+        assert result['error'] is None
+        assert result['new_revisions'] == 1
+        conn = review_tracking.get_db(identifier)
+        revs = review_tracking.get_revisions(conn, change_id)
+        conn.close()
+        r2 = next(r for r in revs if r['revision'] == 2)
+        assert r2['subject'] == '[PATCH v2 0/3] thing: do things better'
+        assert r2['message_id'] == '[email protected]'
+
+    def test_stale_series_title_is_realigned(
+        self, tmp_path: pytest.TempPathFactory
+    ) -> None:
+        """A series an upgrade left titled after patch 1/N heals on update."""
+        identifier = 'ust-realign'
+        change_id = 'cid-V'
+        conn = review_tracking.init_db(identifier)
+        review_tracking.add_series_to_db(
+            conn,
+            change_id,
+            2,
+            '[PATCH v2 1/3] thing: part 1',
+            'Author',
+            '[email protected]',
+            '2026-03-19T08:51:12+00:00',
+            '[email protected]',
+            3,
+        )
+        review_tracking.add_revision(
+            conn,
+            change_id,
+            2,
+            '[email protected]',
+            subject='[PATCH v2 1/3] thing: part 1',
+        )
+        conn.close()
+
+        msgs = _series_msgs('thing', _AUTHOR, 2, 3, cover=True)
+        result = self._update(identifier, change_id, 2, msgs)
+
+        assert result['error'] is None
+        conn = review_tracking.get_db(identifier)
+        row = conn.execute(
+            'SELECT subject FROM series WHERE change_id = ? AND revision = ?',
+            (change_id, 2),
+        ).fetchone()
+        conn.close()
+        assert row['subject'] == '[PATCH v2 0/3] thing: do things better'
+
+
+class TestRealignSeriesSubject:
+    """realign_series_subject() re-titles only from an actual cover letter."""
+
+    def _seed(self, identifier: str, series_subject: str) -> None:
+        conn = review_tracking.init_db(identifier)
+        review_tracking.add_series_to_db(
+            conn,
+            'cid-A',
+            3,
+            series_subject,
+            'Author',
+            '[email protected]',
+            '2026-03-19T08:51:10+00:00',
+            '[email protected]',
+            2,
+        )
+        conn.close()
+
+    def _realign(self, identifier: str, cover: bool) -> bool:
+        lmbx = _build_lmbx('thing', _AUTHOR, 3, 2, cover=cover)
+        conn = review_tracking.get_db(identifier)
+        ret = review_tracking.realign_series_subject(conn, 'cid-A', 3, lmbx)
+        conn.close()
+        return ret
+
+    def _subject(self, identifier: str) -> str:
+        conn = review_tracking.get_db(identifier)
+        row = conn.execute(
+            "SELECT subject FROM series WHERE change_id = 'cid-A'"
+        ).fetchone()
+        conn.close()
+        return str(row['subject'])
+
+    def test_realigns_first_patch_title(self, tmp_path: pytest.TempPathFactory) -> None:
+        self._seed('rsj-fix', '[PATCH v3 1/2] thing: part 1')
+        assert self._realign('rsj-fix', cover=True) is True
+        assert self._subject('rsj-fix') == '[PATCH v3 0/2] thing: do things better'
+
+    def test_prefix_only_difference_is_left_alone(
+        self, tmp_path: pytest.TempPathFactory
+    ) -> None:
+        """The track and upgrade paths format the prefix differently."""
+        self._seed('rsj-pfx', 'thing: do things better')
+        assert self._realign('rsj-pfx', cover=True) is False
+        assert self._subject('rsj-pfx') == 'thing: do things better'
+
+    def test_coverless_thread_never_overwrites(
+        self, tmp_path: pytest.TempPathFactory
+    ) -> None:
+        """Without a cover there is nothing better than what is stored.
+
+        Re-titling from the first-patch fallback would corrupt every correctly
+        titled row whose cover letter is not in the fetched thread.
+        """
+        self._seed('rsj-nocover', 'thing: do things better')
+        assert self._realign('rsj-nocover', cover=False) is False
+        assert self._subject('rsj-nocover') == 'thing: do things better'
+
+    def test_missing_series_row_is_a_noop(
+        self, tmp_path: pytest.TempPathFactory
+    ) -> None:
+        conn = review_tracking.init_db('rsj-none')
+        lmbx = _build_lmbx('thing', _AUTHOR, 3, 2, cover=True)
+        assert review_tracking.realign_series_subject(conn, 'cid-A', 3, lmbx) is False
+        conn.close()
+
+
 class TestCmdTrackRethreadUpgrade:
     """Layer 2 (integration): rethreaded vN+1 links onto a tracked vN."""
 
diff --git a/src/tests/test_tui_tracking.py b/src/tests/test_tui_tracking.py
index 8df38db..cd41d69 100644
--- a/src/tests/test_tui_tracking.py
+++ b/src/tests/test_tui_tracking.py
@@ -3363,7 +3363,10 @@ class TestDetectInitialBase:
 
 
 def _make_mock_lser(
-    revision: int = 2, expected: int = 1, complete: bool = False
+    revision: int = 2,
+    expected: int = 1,
+    complete: bool = False,
+    subject: str = '(untitled)',
 ) -> b4.LoreSeries:
     """Build a minimal LoreSeries usable by _on_update_* callbacks.
 
@@ -3374,6 +3377,7 @@ def _make_mock_lser(
 
     lser = b4.LoreSeries(revision, expected)
     lser.complete = complete
+    lser.subject = subject
     lser.fromname = 'Test Author'
     lser.fromemail = '[email protected]'
     mock_patch = MagicMock()
@@ -3538,6 +3542,91 @@ class TestUpdateRevisionWorkflow:
 
             assert isinstance(app.screen, BaseSelectionScreen)
 
+    @pytest.mark.asyncio
+    async def test_prepared_prefers_series_own_title(
+        self, tmp_path: pathlib.Path
+    ) -> None:
+        """The fetched series' title wins over a stale catalog subject.
+
+        Regression (bug 8bb6e4c): the catalog may name patch 1/N when the
+        revision was recorded before its cover letter was seen, and the
+        upgrade carried that through to the series list.
+        """
+        identifier = 'test-update-title'
+        _seed_db(
+            identifier,
+            [
+                {
+                    'change_id': 'title-1',
+                    'subject': 'thing: do things better',
+                    'message_id': '[email protected]',
+                }
+            ],
+        )
+        lser = _make_mock_lser(subject='thing: do things better')
+        result = (lser, b'fake mbox', 'abc123456789', '', 1)
+
+        app = TrackingApp(identifier)
+        async with app.run_test(size=(120, 30)) as pilot:
+            await pilot.pause()
+            app._on_update_prepared(
+                result,
+                'title-1',
+                1,
+                2,
+                '[email protected]',
+                '[PATCH v2 1/3] thing: part 1',
+                'b4/review/title-1',
+            )
+            await pilot.pause()
+            from b4.review_tui._modals import BaseSelectionScreen
+
+            assert isinstance(app.screen, BaseSelectionScreen)
+            assert app.screen._subject == 'thing: do things better'
+
+    @pytest.mark.asyncio
+    async def test_prepared_untitled_series_keeps_catalog_title(
+        self, tmp_path: pathlib.Path
+    ) -> None:
+        """A series that never learned its title must not beat the catalog.
+
+        LoreSeries carries an '(untitled)' placeholder until its cover or
+        patch 1 is seen; a thread missing both still am-preps (get_am_ready()
+        skips absent patches), and storing the placeholder would clobber a
+        good catalog subject.
+        """
+        identifier = 'test-update-untitled'
+        _seed_db(
+            identifier,
+            [
+                {
+                    'change_id': 'title-2',
+                    'subject': 'thing: do things better',
+                    'message_id': '[email protected]',
+                }
+            ],
+        )
+        lser = _make_mock_lser(subject='(untitled)')
+        result = (lser, b'fake mbox', 'abc123456789', '', 1)
+
+        app = TrackingApp(identifier)
+        async with app.run_test(size=(120, 30)) as pilot:
+            await pilot.pause()
+            app._on_update_prepared(
+                result,
+                'title-2',
+                1,
+                2,
+                '[email protected]',
+                '[PATCH v2 0/3] thing: do things better',
+                'b4/review/title-2',
+            )
+            await pilot.pause()
+            from b4.review_tui._modals import BaseSelectionScreen
+
+            assert isinstance(app.screen, BaseSelectionScreen)
+            assert app.screen._subject == '[PATCH v2 0/3] thing: do things better'
+
     # --- Phase 3: _on_update_base_selected (apply + swap) ----------------
 
     @pytest.mark.asyncio

-- 
2.53.0