From 01740f352e2f2414b3bdb7791c2ed1f7bbb2b455 Mon Sep 17 00:00:00 2001
From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com>
Date: Fri, 28 Aug 2026 11:50:12 +0530
Subject: [PATCH] refactor(test): fold simplify-code review findings into perf
 guards
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Three-reviewer pass (reuse/quality/efficiency) on the guard file:

- Drop dead `agent._interrupt_requested = False` setup: the
  `_record_streamed_assistant_text` chain only consults
  `_stream_writer_superseded()` (stream-writer TLS token), never
  `_interrupt_requested` — verified by reading both call sites.
- Deduplicate the two trace-callback blocks into one
  `_count_writer_statements` helper using the house idiom
  (`statements.append` — 9 existing uses in tests/test_hermes_state.py)
  instead of a mutable-dict counter closure; failure messages now dump
  the captured SQL for direct diagnosis.

Dropped after verification: reviewer suggestion to flip xfail to
strict=True — its premise ("the fix PRs already remove the markers")
is wrong: #92166/#95380 predate this file and cannot remove markers
they don't contain, so strict=True would redden main's CI the moment
either merges. strict=False + follow-up marker removal is the
deliberate no-red-window ratchet.

Efficiency reviewer: no material findings (GC delta 0.06 on the ratio,
36.9MB peak, sqlite trace API stable since 3.14, dir convention OK).

Re-verified post-fold: 3x main runs (2 passed, 2 xfailed), flip checks
on both fix branches still pass with --runxfail.
---
 tests/perf_guards/test_pattern_b_scaling.py | 41 ++++++++++++---------
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/tests/perf_guards/test_pattern_b_scaling.py b/tests/perf_guards/test_pattern_b_scaling.py
index b17b522778..7d3dead17e 100644
--- a/tests/perf_guards/test_pattern_b_scaling.py
+++ b/tests/perf_guards/test_pattern_b_scaling.py
@@ -68,7 +68,6 @@ class TestStreamedTextAccumulationLinear:
         from run_agent import AIAgent
 
         agent = AIAgent.__new__(AIAgent)
-        agent._interrupt_requested = False
         agent._current_streamed_assistant_text = ""
         for _ in range(n):
             agent._record_streamed_assistant_text(self.DELTA)
@@ -123,25 +122,36 @@ class TestListSessionsRichQueryBound:
         yield db
         db.close()
 
+    @staticmethod
+    def _count_writer_statements(db):
+        """Rows from ``list_sessions_rich`` plus the writer-conn statement count.
+
+        Uses the house trace-callback idiom (``statements.append`` — see
+        tests/test_hermes_state.py) so failures can dump the captured SQL.
+        """
+        statements: list[str] = []
+        db._conn.set_trace_callback(statements.append)
+        try:
+            rows = db.list_sessions_rich(limit=50)
+        finally:
+            db._conn.set_trace_callback(None)
+        return rows, statements
+
     @pytest.mark.xfail(
         reason="known N+1 on main until PR #95380 (batched compression-edge "
         "query) merges; remove this marker when it lands",
         strict=False,
     )
     def test_statement_count_bounded_regardless_of_session_count(self, chain_db):
-        counted = {"n": 0}
-        chain_db._conn.set_trace_callback(lambda stmt: counted.__setitem__("n", counted["n"] + 1))
-        try:
-            rows = chain_db.list_sessions_rich(limit=50)
-        finally:
-            chain_db._conn.set_trace_callback(None)
+        rows, statements = self._count_writer_statements(chain_db)
 
         assert len(rows) == self.N_CHAINS
-        assert counted["n"] <= self.MAX_STATEMENTS, (
-            f"list_sessions_rich issued {counted['n']} writer-connection "
+        assert len(statements) <= self.MAX_STATEMENTS, (
+            f"list_sessions_rich issued {len(statements)} writer-connection "
             f"statements for {self.N_CHAINS} sessions (bound: "
             f"{self.MAX_STATEMENTS}). Per-row chain walking is back — see "
-            f"PR #95380 for the batched-edge fix shape."
+            f"PR #95380 for the batched-edge fix shape. Captured SQL: "
+            f"{[s[:80] for s in statements[:10]]}"
         )
 
     def test_statement_count_does_not_scale_with_sessions(self, chain_db):
@@ -149,17 +159,12 @@ class TestListSessionsRichQueryBound:
         O(N) but must never exceed a per-session budget of 4 — catching a
         regression from N+1 to N·M (nested walks, per-hop re-queries).
         """
-        counted = {"n": 0}
-        chain_db._conn.set_trace_callback(lambda stmt: counted.__setitem__("n", counted["n"] + 1))
-        try:
-            rows = chain_db.list_sessions_rich(limit=50)
-        finally:
-            chain_db._conn.set_trace_callback(None)
+        rows, statements = self._count_writer_statements(chain_db)
 
         assert len(rows) == self.N_CHAINS
         budget = 4 * self.N_CHAINS + 8
-        assert counted["n"] <= budget, (
-            f"list_sessions_rich issued {counted['n']} statements for "
+        assert len(statements) <= budget, (
+            f"list_sessions_rich issued {len(statements)} statements for "
             f"{self.N_CHAINS} sessions — beyond even the legacy N+1 budget "
             f"({budget}). A nested per-row walk has been introduced."
         )