test: coverage audit — real fixtures, dead tests removed, streaming and MMR covered - #171
Merged
Merged
Conversation
…nd MMR covered Closes #154. ## Skipped tests: 19 -> 0 17 of 19 skips were not model- or hardware-dependent — they were guarded on two fixture files, docs/src/bitcoin_technical_document.pdf and bitcoin_creative_commons_en.pptx, that were never committed. *.pdf/*.pptx are globally gitignored (local upload cruft), and these two fixtures got caught by that same rule instead of being carved out as an exception. Generated real, structured fixtures instead of placeholders: a 4-page PDF (headings, paragraphs, a formula line, a table) and a 5-slide PPTX, built with pymupdf and python-pptx — both already backend dependencies, so no new dependency was added. All 17 previously-skipped tests pass against them unmodified. The remaining 2 skips (test_chunker.py, test_ingester_parser.py) guarded on module_1_ingestor / module_2_parser / module_3_micro_chunker, which were never declared as a dependency anywhere and have been permanently skipped since their introduction. They describe a three-level chunking hierarchy (section -> paragraph -> micro) that predates the current two-level parent/child design in app/workers/pipeline.py (see the 58b468b refactor that unified them). Deleted rather than fixed: keeping tests for an architecture the code no longer has is actively misleading, not just inert. ## Assertion-free test audit 4 flagged by static scan; all four are legitimate does-not-raise checks (pytest fails on an uncaught exception, so absence of a raise is a real assertion). Strengthened test_module_aliases_are_registered_without_error with an explicit check of the registered alias, since that state was cheap to assert on and is the actual point of the test. ## New coverage: streaming and MMR Coverage audit surfaced two real gaps: the SSE streaming paths (chat_service.stream_answer, study_service.stream_dispatch) at ~53-54% despite being the primary interaction mode, and reranker.mmr_select's actual diversity algorithm — only its no-model fallback was tested. - 11 new tests for both streaming paths: cache short-circuit, QUIZ/ORAL/ RETRIEVE always buffered, progressive token yield, empty-stream fallback to buffered generate, [ERROR] token mid-stream treated as failure, full degradation to a raw source snippet. - 4 new MMR tests using a fake embedding model with controlled vectors: a diverse lower-relevance chunk beats a redundant higher-relevance one, lambda_=1.0 matches pure relevance order, inference failure degrades to a top-k slice. One test documents a found-but-not-fixed behaviour rather than asserting what I assumed: when both stream and buffered generation fail, the fallback answer is raw_answer (QVAC's own bare string), not pack.context_block() — so retrieved evidence can be silently discarded if raw_answer happens to be empty. Identical branch exists in the non-streaming dispatch() path. Filed as a follow-up rather than changed here, since this issue is about testing and reporting, not product behaviour. ## Numbers | | Before | After | |---|---|---| | Backend tests | 915 | 946 | | Skipped | 19 | 0 | | Coverage (branch) | 78.02% | 82.04% | | chat_service.py | 53% | 80% | | study_service.py | 54% | 73% | | reranker.py | 54% | 77% | | pipeline.py | 61% | 73% | mypy clean across 84 files.
4 tasks
This was referenced Aug 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #154.
What this does
The coverage-audit layer of the quality gate (#146, layer 1/6): measure what's actually tested, find what isn't, and close the highest-value gaps rather than chase a percentage.
Skipped tests: 19 → 0
17 of the 19 skips had nothing to do with hardware or models — they were guarded on two fixture files that were never committed:
docs/src/bitcoin_technical_document.pdfandbitcoin_creative_commons_en.pptx.*.pdf/*.pptxare globally gitignored (local upload cruft), and these two fixtures got caught by that same rule instead of being carved out as an exception.Generated real, structured fixtures rather than placeholders — a 4-page PDF (headings, paragraphs, a formula, a table) and a 5-slide PPTX — using
pymupdfandpython-pptx, both already backend dependencies, so nothing new was added. All 17 previously-skipped tests pass against them unmodified.The remaining 2 (
test_chunker.py,test_ingester_parser.py) guarded onmodule_1_ingestor/module_2_parser/module_3_micro_chunker— never declared as a dependency anywhere, permanently skipped since introduction. They describe a three-level chunking hierarchy that predates the current two-level parent/child design inpipeline.py. Deleted, not fixed: a test for an architecture the code no longer has is misleading, not just inert.Assertion-free scan
4 flagged by a static scan of every
test_*function for a missingassert/pytest.raises. All four are legitimate does-not-raise checks — pytest fails a test on an uncaught exception, so their absence of a raise is the assertion. Strengthened one (test_module_aliases_are_registered_without_error) with an explicit check of the registered alias, since that was cheap and is the actual point of the test.New coverage: streaming and MMR
The two real gaps the audit surfaced: the SSE streaming paths (
chat_service.stream_answer,study_service.stream_dispatch) at ~53-54% despite being the primary interaction mode today, andreranker.mmr_select's actual diversity algorithm — previously only its no-model fallback was exercised.[ERROR]token mid-stream treated as failure rather than shown to the user, full degradation to a raw source snippet when everything is down.lambda_=1.0matches pure relevance order, inference failure degrades to a plain top-k slice.One test documents a finding rather than an assumption I got right the first time: when both streaming and buffered generation fail, the fallback answer is
raw_answer— QVAC's own bare string from the initial retrieval call — notpack.context_block(). So retrieved evidence sitting right there in the pack can be silently discarded ifraw_answerhappens to be empty. The identical branch exists in the non-streamingdispatch()path, so this isn't new. Filed as #172 rather than changed here — this issue is about testing and reporting, not product behaviour.Numbers
chat_service.pystudy_service.pyreranker.pypipeline.pymypy clean across 84 files.
Not done here (by design)
Per-endpoint success/validation/not-found matrix and per-service error-path mapping — #154 already flagged auth coverage as complete (test_authorization_matrix.py) and asked for a report, not a rewrite of every layer. The remaining line-level gaps (ChromaDB fallback path in chat_service, two-hop compare/derive merge logic, the low-level
_stream_generateSSE parser) are listed in the closing comment on #154 as scoped candidates for whichever of layers 2-6 fits each.