tests/test_general_topic_messages.py (2)

29-61: Shared fixture + type hint would trim duplication.

All three tests repeat the adapter, engine, chat_id = await _make_adapter_with_messages() / try/finally engine.dispose() dance. A pytest_asyncio.fixture yielding the tuple and disposing on teardown would drop ~15 lines and remove the chance of forgetting dispose() in a future test. Also, the helper is missing a return type annotation (per repo guideline: type hints for function signatures).

As per coding guidelines: "Use type hints for function signatures".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_general_topic_messages.py` around lines 29 - 61, Add a
pytest_asyncio.fixture that yields the (adapter, engine, chat_id) tuple created
by the existing helper and disposes the engine in the fixture teardown; update
tests to use that fixture instead of calling _make_adapter_with_messages() and
manual try/finally engine.dispose(). Also add a return type annotation to the
helper _make_adapter_with_messages (e.g., -> Tuple[DatabaseAdapter, AsyncEngine,
int]) to satisfy the repo typing guideline and keep the helper for reuse; ensure
the fixture imports pytest_asyncio and references DatabaseAdapter and
engine.dispose() for cleanup.

---

53-54: Nit: rename the placeholder text.

"sexy topic msg" / "another sexy msg" reads oddly in a regression test. Something neutral like "topic-47 msg #1" keeps future readers focused on the assertion, not the wording.

Proposed tweak

-            (3, datetime(2024, 3, 1, 10), "sexy topic msg", 47),
-            (4, datetime(2024, 3, 2, 10), "another sexy msg", 47),
+            (3, datetime(2024, 3, 1, 10), "topic-47 msg `#1`", 47),
+            (4, datetime(2024, 3, 2, 10), "topic-47 msg `#2`", 47),

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_general_topic_messages.py` around lines 53 - 54, Replace the odd
placeholder strings in the test data tuples (currently "sexy topic msg" and
"another sexy msg") with neutral, descriptive placeholders so the test reads
clearly; update the two tuples (the ones with message text and topic id 47) to
use something like "topic-47 msg `#1`" and "topic-47 msg `#2`" to keep focus on the
assertion rather than the wording.

src/db/adapter.py (1)

1101-1104: LGTM — coalesce matches get_forum_topics counting logic.

Filter now treats NULL reply_to_top_id as General (id=1), so sidebar counts and message pane agree. Non-General topics remain strict since coalesce(NULL, 1) == 47 is false.

One heads-up: if an index exists on reply_to_top_id, wrapping it in coalesce(...) will prevent its use on most engines. Likely negligible given the chat_id filter already narrows the scan, but worth keeping in mind if message volumes grow.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/db/adapter.py` around lines 1101 - 1104, The current use of
func.coalesce(Message.reply_to_top_id, 1) in stmt.where disables index use on
reply_to_top_id; replace the coalesce predicate with an explicit conditional
that preserves indexability: filter by Message.reply_to_top_id == topic_id for
non-General topics, and for topic_id == 1 add an additional OR branch that
matches Message.reply_to_top_id IS NULL (i.e., treat NULL as General) so the
query uses a direct column comparison on Message.reply_to_top_id instead of
wrapping it in coalesce; update the stmt.where call accordingly (referencing
Message.reply_to_top_id, stmt.where, and the current topic_id handling).

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes