[Tests] Nothing exercises process_audio's body — the pipeline's own orchestration is untested #457

New issue

Open

opened 2026-08-31 02:45:03 +00:00 by claude-bot · 0 comments

claude-bot commented

2026-08-31 02:45:03 +00:00

Contributor

Severity: MEDIUM — a coverage gap, not a live defect. Filed so it is tracked rather than left as a comment in one test file.

Found while removing the canonical-name re-summarisation in #401 (PR #456).

What is uncovered

_process_audio_async is the orchestration for the whole recording pipeline — transcription, speaker attribution, merging, summarisation, highlights, the summary fan-out — and no test drives it. Confirmed by grepping the suite: _process_audio_async appears in no test file. The functions it calls are well covered individually (audio_service guards, duration checks, attribution, retention); the sequencing that joins them is not.

The immediate trigger

test_system_aware_prompts.py had two tests asserting that audio_service.summarise receives the resolved game-system prompt context. They reached that call site through POST /sessions/{id}/canonical-names, because that endpoint used to re-summarise.

#401 removed that re-summarisation (it silently destroyed GM-edited summaries), so their vehicle is gone and they were deleted rather than rewritten to assert nothing. summarise now has exactly one call site — process_audio, reminder_tasks.py:2211 — and it has no call-site test.

The underlying behaviour is not entirely unguarded: resolve_system_prompt_context is unit-tested directly, and its threading is covered at two other call sites (generate_name_options, lore proposals). What is gone is any check that the summariser gets it.

Why this is worth more than restoring two tests

The same missing harness showed up twice more in the same milestone:

#400's storyline tests initially passed because, without task_session patched to the test's own session, the task found no rows and returned before reaching the code under test.
#401's own tests initially passed with the fix fully reverted, because the removed block sat behind if llm_cfg is not None and the suite configures no LLM.

Both were caught by mutation-checking, not by the tests themselves. A reusable way to drive a Celery task body against seeded data — with task_session wired to the test session and the LLM seam stubbed — would have made all three straightforward, and would make process_audio testable at all.

test_lore_generation_failure.py already has the task_session half of that pattern (_same_task_session), copied by hand into several files. Promoting it to a shared fixture is the obvious first step.

Proposed fix

Promote the same_task_session fixture to conftest.py and replace the hand-copied versions (test_lore_generation_failure.py, test_gm_authorship_is_not_overwritten.py, and the others that grep turns up).
Add a harness that runs _process_audio_async against a seeded session with the external seams stubbed (transcription, summarise, the notify publish).
Using it, restore the lost assertion: summarise receives the resolved system prompt context — the registry prompt_hint for a linked campaign, the raw campaign.game_system column for a free-form one.
While the harness exists, consider covering the sequencing that has produced real defects: #421 (retry disarming the duration guards), #425/#431/#432 (the silence and coverage guards), #397 (re-submission over an existing transcript). Not required to close this issue, but it is the reason the harness is worth building.

Acceptance criteria

A shared fixture patches task_session to the test's session; the hand-copied duplicates are removed.
A test drives _process_audio_async end to end with external seams stubbed.
summarise is asserted to receive the resolved system prompt context, for both a linked and a free-form campaign.
The note in test_system_aware_prompts.py under "summarise() call site" is removed once its gap is closed.

Context

See the note left in webapp/backend/tests/test_system_aware_prompts.py where the two tests used to be, and PR #456's verification section.

Related: #441 (audit the test suite for tests that cannot fail) — that issue is about tests asserting against fabricated or unreachable conditions, which is the same failure mode from the opposite direction. Three concrete instances turned up in v4.1.0 alone (#454's beat-notes test, and both of the above).

**Severity: MEDIUM** — a coverage gap, not a live defect. Filed so it is tracked rather than left as a comment in one test file. Found while removing the canonical-name re-summarisation in #401 (PR #456). ## What is uncovered `_process_audio_async` is the orchestration for the whole recording pipeline — transcription, speaker attribution, merging, summarisation, highlights, the summary fan-out — and **no test drives it**. Confirmed by grepping the suite: `_process_audio_async` appears in no test file. The functions it calls are well covered individually (`audio_service` guards, duration checks, attribution, retention); the sequencing that joins them is not. ## The immediate trigger `test_system_aware_prompts.py` had two tests asserting that `audio_service.summarise` receives the resolved game-system prompt context. They reached that call site through `POST /sessions/{id}/canonical-names`, because that endpoint used to re-summarise. #401 removed that re-summarisation (it silently destroyed GM-edited summaries), so their vehicle is gone and they were deleted rather than rewritten to assert nothing. **`summarise` now has exactly one call site — `process_audio`, `reminder_tasks.py:2211` — and it has no call-site test.** The underlying behaviour is not entirely unguarded: `resolve_system_prompt_context` is unit-tested directly, and its threading is covered at two other call sites (`generate_name_options`, lore proposals). What is gone is any check that *the summariser* gets it. ## Why this is worth more than restoring two tests The same missing harness showed up twice more in the same milestone: - #400's storyline tests initially passed because, without `task_session` patched to the test's own session, the task found no rows and returned before reaching the code under test. - #401's own tests initially passed with the fix fully reverted, because the removed block sat behind `if llm_cfg is not None` and the suite configures no LLM. Both were caught by mutation-checking, not by the tests themselves. A reusable way to drive a Celery task body against seeded data — with `task_session` wired to the test session and the LLM seam stubbed — would have made all three straightforward, and would make `process_audio` testable at all. `test_lore_generation_failure.py` already has the `task_session` half of that pattern (`_same_task_session`), copied by hand into several files. Promoting it to a shared fixture is the obvious first step. ## Proposed fix 1. Promote the `same_task_session` fixture to `conftest.py` and replace the hand-copied versions (`test_lore_generation_failure.py`, `test_gm_authorship_is_not_overwritten.py`, and the others that grep turns up). 2. Add a harness that runs `_process_audio_async` against a seeded session with the external seams stubbed (transcription, `summarise`, the notify publish). 3. Using it, restore the lost assertion: `summarise` receives the resolved system prompt context — the registry `prompt_hint` for a linked campaign, the raw `campaign.game_system` column for a free-form one. 4. While the harness exists, consider covering the sequencing that has produced real defects: #421 (retry disarming the duration guards), #425/#431/#432 (the silence and coverage guards), #397 (re-submission over an existing transcript). Not required to close this issue, but it is the reason the harness is worth building. ## Acceptance criteria - [ ] A shared fixture patches `task_session` to the test's session; the hand-copied duplicates are removed. - [ ] A test drives `_process_audio_async` end to end with external seams stubbed. - [ ] `summarise` is asserted to receive the resolved system prompt context, for both a linked and a free-form campaign. - [ ] The note in `test_system_aware_prompts.py` under "summarise() call site" is removed once its gap is closed. ## Context See the note left in `webapp/backend/tests/test_system_aware_prompts.py` where the two tests used to be, and PR #456's verification section. Related: #441 (audit the test suite for tests that cannot fail) — that issue is about tests asserting against fabricated or unreachable conditions, which is the same failure mode from the opposite direction. Three concrete instances turned up in v4.1.0 alone (#454's beat-notes test, and both of the above).

claude-bot referenced this issue

2026-09-01 16:35:36 +00:00

[Backend] set_admin --email crashes instead of disambiguating when two identities share an email #436