rbrooks/Quest-Board

Watch

Fork 0

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

New issue

Closed

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

claude-bot commented

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

Collaborator

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

claude-bot commented

2026-09-14 03:41:20 +00:00

Author

Collaborator

Picking this up (owner go-ahead 2026-09-13). Checked first, and the issue is partly overtaken:

Criterion 2 is largely met. The seam fixture in test_failed_windows.py (copied into test_name_snapping.py and test_hallucination_filter.py) already drives process_audio.run(...) end to end with only the provider calls stubbed.

Remaining scope:

Promote same_task_session to conftest.py and remove the equivalent copies (about 12 files define one).
Using a process_audio seam, assert that summarise receives the resolved game-system context (reminder_tasks.py:2832) for both a linked and a free-form campaign. Mutation-check it.
Remove the gap note in test_system_aware_prompts.py.

Item 4 (sequencing guards #421/#425/#431/#432/#397) is optional per the issue, and out of this pass.

Picking this up (owner go-ahead 2026-09-13). Checked first, and the issue is partly overtaken: - **Criterion 2 is largely met.** The `seam` fixture in `test_failed_windows.py` (copied into `test_name_snapping.py` and `test_hallucination_filter.py`) already drives `process_audio.run(...)` end to end with only the provider calls stubbed. Remaining scope: 1. Promote `same_task_session` to `conftest.py` and remove the equivalent copies (about 12 files define one). 2. Using a `process_audio` seam, assert that `summarise` receives the resolved game-system context (`reminder_tasks.py:2832`) for both a linked and a free-form campaign. Mutation-check it. 3. Remove the gap note in `test_system_aware_prompts.py`. Item 4 (sequencing guards #421/#425/#431/#432/#397) is optional per the issue, and out of this pass.

rbrooks referenced this issue from a commit

2026-09-14 04:12:01 +00:00

test(backend): assert process_audio threads the game-system context into summarise; share same_task_session (#457)

rbrooks referenced this issue from a commit

2026-09-14 04:12:01 +00:00

test(backend): fold the last two task_session copies into the shared fixture (#457)

claude-bot referenced this issue from a pull request that will close it,

2026-09-14 04:12:18 +00:00

test(backend): assert process_audio threads the game-system context into summarise; share same_task_session (#457) #646

claude-bot closed this issue

2026-09-14 04:51:09 +00:00

claude-bot referenced this issue from a commit

2026-09-14 04:51:10 +00:00

Merge PR #646: test(backend): assert process_audio threads the game-system context into summarise; share same_task_session (#457)

claude-bot commented

2026-09-14 04:51:47 +00:00

Author

Collaborator

Done in #646 (merged as f95c8d4; CI run 1095 green).

One same_task_session fixture in conftest.py. It replaces 14 hand-copied patches: 10 module fixtures, 7 per-test closures, and 2 _yield_db copies that a search for the fixture name had missed. All were behaviourally identical. Making the shared fixture a no-op turns 14 tests red.
New test_process_audio_system_context.py. It drives process_audio.run end to end, intercepts summarise, and asserts it receives the resolved context: the raw string for a free-form campaign, and the registry prompt_hint for a linked one. Both cases use the real resolver. Passing None at the call site fails both.
The gap note in test_system_aware_prompts.py is removed.

Not done, and optional per the issue: item 4, sequencing tests for #421/#425/#431/#432/#397 on the same harness. #441 (the whole-suite mutation audit) is the natural home for that.

Done in #646 (merged as f95c8d4; CI run 1095 green). - **One `same_task_session` fixture in `conftest.py`.** It replaces 14 hand-copied patches: 10 module fixtures, 7 per-test closures, and 2 `_yield_db` copies that a search for the fixture name had missed. All were behaviourally identical. Making the shared fixture a no-op turns 14 tests red. - **New `test_process_audio_system_context.py`.** It drives `process_audio.run` end to end, intercepts `summarise`, and asserts it receives the resolved context: the raw string for a free-form campaign, and the registry `prompt_hint` for a linked one. Both cases use the real resolver. Passing `None` at the call site fails both. - **The gap note in `test_system_aware_prompts.py` is removed.** Not done, and optional per the issue: item 4, sequencing tests for #421/#425/#431/#432/#397 on the same harness. #441 (the whole-suite mutation audit) is the natural home for that.