docs(bf-303t6): document truncated-flate test coverage analysis

- Analyze current test assertions in error_recovery_integration.rs - Analyze current test assertions in stream_decoder_fixtures.rs - Identify 5 major coverage gaps in truncated-flate recovery testing - Document missing diagnostic emission in FlateDecoder - Document lack of actual extraction in integration test - Map out recommendations for new assertions at high/medium/low priority Closes bf-303t6.
2026-07-05 19:33:59 -04:00 · 2026-07-05 19:33:59 -04:00 · f7fd0f7c9f
commit f7fd0f7c9f
parent 6117735ed2
1 changed files with 300 additions and 0 deletions
--- a/notes/bf-303t6.md
+++ b/notes/bf-303t6.md
@ -0,0 +1,300 @@
+# Test Coverage Analysis: Truncated-Flate Recovery
+
+**Bead:** bf-303t6
+**Date:** 2026-07-05
+**Analysis:** Current test assertions and coverage gaps for truncated-flate error recovery
+
+## Current Test Landscape
+
+### 1. Integration-Level Test: `error_recovery_integration.rs::test_truncated_mid_stream()`
+
+**Location:** `crates/pdftract-core/tests/error_recovery_integration.rs:165-188`
+
+**Current Assertions:**
+```rust
+fn test_truncated_mid_stream() {
+    let fixture_path = fixture_path("truncated_mid_stream.pdf");
+    let expected = load_expected_diagnostics(&fixture_path);
+
+    let result = assert_no_panic("test_truncated_mid_stream", || {
+        let pdf_data = fs::read(&fixture_path).expect("fixture should exist");
+        assert!(pdf_data.starts_with(b"%PDF-"), "Should be a valid PDF");
+
+        // Verify expected: STREAM_DECODE_ERROR
+        let stream_diags: Vec<_> = expected
+            .expected_diagnostics
+            .iter()
+            .filter(|d| d.code.contains("STREAM_DECODE"))
+            .collect();
+
+        assert!(
+            !stream_diags.is_empty(),
+            "Should expect STREAM_DECODE_ERROR diagnostic"
+        );
+    });
+
+    assert!(result.is_ok(), "Test should not panic");
+}
+```
+
+**What's Validated:**
+- ✅ Fixture file exists and is valid PDF structure
+- ✅ Expected diagnostics JSON file exists
+- ✅ Expected diagnostics includes `STREAM_DECODE_ERROR`
+- ✅ Test doesn't panic (INV-8 compliance)
+
+**What's NOT Validated:**
+- ❌ Actual extraction is never run
+- ❌ No diagnostics are actually emitted or verified
+- ❌ No verification of partial output behavior
+- ❌ No verification that extraction succeeds despite truncation
+
+### 2. Stream Decoder Test: `stream_decoder_fixtures.rs`
+
+**Location:** `crates/pdftract-core/tests/stream_decoder_fixtures.rs:61-65`
+
+**Fixture Configuration:**
+```rust
+FixtureInfo {
+    name: "flate_truncated",
+    filter: FixtureFilter::Single("FlateDecode", None),
+    expected_diags: vec![],  // ⚠️ EMPTY - no diagnostics expected!
+    bomb_limit: None,
+},
+```
+
+**Current Assertions:**
+```rust
+// Decode the fixture
+let result = decode_fixture(&fixture, &input);
+let decoded = match result {
+    Ok(data) => data,
+    Err(e) => {
+        failures.push(format!("{}: {}", fixture.name, e));
+        continue;
+    }
+};
+
+// Compare against expected
+if &decoded[..expected_bytes.len().min(decoded.len())] != expected_bytes {
+    failures.push(format!(
+        "{}: output mismatch (expected {} bytes, got {} bytes)",
+        fixture.name,
+        expected.len(),
+        decoded.len()
+    ));
+    continue;
+}
+```
+
+**What's Validated:**
+- ✅ Decoder doesn't return `Err` (returns `Ok` with partial bytes)
+- ✅ Output matches expected file (0 bytes for truncated fixture)
+- ✅ No panic occurs
+
+**What's NOT Validated:**
+- ❌ `STREAM_DECODE_ERROR` diagnostic is NOT verified (expected_diags is empty!)
+- ❌ Metadata says "expects partial bytes + STREAM_DECODE_ERROR" but test doesn't verify
+- ❌ No verification that error is properly propagated
+- ❌ No distinction between success and partial recovery
+
+### 3. Implementation: `FlateDecoder::decode_impl()`
+
+**Location:** `crates/pdftract-core/src/parser/stream.rs:524-558`
+
+**Error Handling:**
+```rust
+fn decode_impl<R: std::io::Read>(
+    mut decoder: R,
+    doc_counter: &mut u64,
+    max_bytes: u64,
+) -> Vec<u8> {
+    let mut output = Vec::new();
+    let mut chunk = vec![0u8; BOMB_CHECK_CHUNK];
+
+    loop {
+        match decoder.read(&mut chunk) {
+            Ok(0) => break,
+            Ok(n) => {
+                // Check bomb limit BEFORE adding bytes to output
+                if *doc_counter + output.len() as u64 + n as u64 > max_bytes {
+                    // Bomb limit exceeded - return partial bytes
+                    let remaining = (max_bytes - *doc_counter - output.len() as u64) as usize;
+                    let to_add = remaining.min(n);
+                    output.extend_from_slice(&chunk[..to_add]);
+                    return output;
+                }
+                output.extend_from_slice(&chunk[..n]);
+            }
+            Err(e) if e.kind() == std::io::ErrorKind::UnexpectedEof => {
+                // Truncated stream - return partial bytes (INV-8)
+                break;  // ⚠️ SILENT - no diagnostic emitted
+            }
+            Err(_) => {
+                // Other decoder errors - return partial bytes decoded so far
+                break;  // ⚠️ SILENT - no diagnostic emitted
+            }
+        }
+    }
+
+    output  // ⚠️ Returns Vec<u8> - caller can't detect error occurred
+}
+```
+
+**Critical Gaps:**
+- ❌ `UnexpectedEof` is caught but no diagnostic is emitted
+- ❌ Other decoder errors are silently caught
+- ❌ Return type is `Vec<u8>`, not `Result<Vec<u8>, FilterError>`
+- ❌ Caller has no way to distinguish success from partial recovery
+
+## Coverage Gap Summary
+
+### Gap 1: Missing Diagnostic Emission
+**Severity:** HIGH
+**Issue:** FlateDecoder catches `UnexpectedEof` but never emits `STREAM_DECODE_ERROR` diagnostic
+
+**Evidence:**
+- Metadata file says: "expects partial bytes + STREAM_DECODE_ERROR"
+- Implementation catches error but doesn't emit diagnostic
+- Test has `expected_diags: vec![]` (empty)
+
+**Impact:**
+- Diagnostics system is bypassed
+- Users can't detect truncated streams in their PDFs
+- INV-8 requires graceful recovery, but silent recovery hides issues
+
+### Gap 2: No Error Indication in Return Type
+**Severity:** HIGH  
+**Issue:** `decode_impl()` returns `Vec<u8>` directly, not `Result`
+
+**Evidence:**
+- Method signature: `fn decode_impl(...) -> Vec<u8>`
+- Both success and error return `Vec<u8>`
+- Caller can't distinguish partial recovery from clean decode
+
+**Impact:**
+- No way for callers to know truncation occurred
+- Violates principle of least surprise
+- Makes debugging harder
+
+### Gap 3: Integration Test Doesn't Extract
+**Severity:** MEDIUM
+**Issue:** `test_truncated_mid_stream()` validates JSON but doesn't run extraction
+
+**Evidence:**
+- Test reads PDF data but doesn't call `pdftract::extract()`
+- Only validates fixture structure and expected diagnostics JSON
+- Comment says: `// TODO: Extract with pdftract once API is available`
+
+**Impact:**
+- End-to-end behavior is untested
+- Integration between stream decoder and extraction pipeline is unverified
+- Real-world behavior with actual PDF extraction is unknown
+
+### Gap 4: No Verification of Partial Output Quality
+**Severity:** MEDIUM
+**Issue:** Tests don't verify partial output is usable or meaningful
+
+**Evidence:**
+- `flate_truncated.expected` is 0 bytes (trivially passes)
+- No test verifies partial data represents valid prefix
+- No test verifies remaining bytes are sensible
+
+**Impact:**
+- Can't detect if partial output is garbage
+- Can't verify recovery produces useful data
+- Quality of partial recovery is untested
+
+### Gap 5: Stream Decoder Test Mismatch with Metadata
+**Severity:** LOW
+**Issue:** Test expects no diagnostics, metadata says STREAM_DECODE_ERROR expected
+
+**Evidence:**
+- `stream_decoder_fixtures.rs`: `expected_diags: vec![]`
+- `flate_truncated.meta`: "expects partial bytes + STREAM_DECODE_ERROR"
+
+**Impact:**
+- Conflicting documentation
+- Unclear what the actual expectation is
+- Test may be out of sync with requirements
+
+## Test Files Examined
+
+1. **`crates/pdftract-core/tests/error_recovery_integration.rs`** (315 lines)
+   - Integration-level adversarial test corpus
+   - Tests 7 fixtures including `truncated_mid_stream.pdf`
+
+2. **`crates/pdftract-core/tests/stream_decoder_fixtures.rs`** (394 lines)
+   - Unit-level stream decoder tests
+   - Tests 15+ fixtures including `flate_truncated`
+
+3. **`crates/pdftract-core/src/parser/stream.rs`** (600+ lines)
+   - FlateDecoder implementation
+   - Error handling in `decode_impl()`
+
+4. **Fixture files:**
+   - `/tests/stream_decoder/fixtures/flate_truncated.bin` (10 bytes, truncated zlib)
+   - `/tests/stream_decoder/fixtures/flate_truncated.expected` (0 bytes)
+   - `/tests/stream_decoder/fixtures/flate_truncated.meta` ("expects partial bytes + STREAM_DECODE_ERROR")
+   - `/tests/error_recovery/fixtures/truncated_mid_stream.pdf`
+   - `/tests/error_recovery/fixtures/truncated_mid_stream.expected_diagnostics.json`
+
+## Recommendations for New Assertions
+
+### High Priority (Required for Complete Coverage)
+
+1. **Verify diagnostic emission in FlateDecoder**
+   - Assert `STREAM_DECODE_ERROR` is emitted on `UnexpectedEof`
+   - Update test: `expected_diags: vec![DiagCode::StreamDecodeError]`
+
+2. **Add integration test that actually extracts**
+   - Call `pdftract::extract()` on `truncated_mid_stream.pdf`
+   - Verify extraction succeeds despite truncation
+   - Verify diagnostics contain `STREAM_DECODE_ERROR`
+
+3. **Verify partial output is meaningful**
+   - Create fixture with known valid prefix
+   - Assert decoded bytes match expected prefix
+   - Verify partial data isn't garbage
+
+### Medium Priority (Improves Robustness)
+
+4. **Test error propagation through extraction pipeline**
+   - Verify stream decoder errors are properly reported
+   - Test diagnostics are collected and surfaced
+   - Verify no panic occurs (INV-8)
+
+5. **Add test for multiple truncation scenarios**
+   - Truncated at different points (early, mid, late)
+   - Truncated with different filter types (Flate, LZW, etc.)
+   - Truncated in filter arrays (multi-stage decoding)
+
+### Low Priority (Nice to Have)
+
+6. **Test bomb limit interaction with truncation**
+   - Verify truncation before bomb limit
+   - Verify truncation after bomb limit  
+   - Verify both diagnostics can co-occur
+
+7. **Add regression test for specific real-world PDFs**
+   - Collect corpus of PDFs with truncated streams
+   - Test each produces reasonable output
+   - Track quality of partial recovery over time
+
+## Conclusion
+
+The current test coverage for truncated-flate recovery has **significant gaps**:
+
+- ✅ **Basic INV-8 compliance is tested**: No panics occur
+- ❌ **Diagnostic emission is NOT tested**: Stream decoder doesn't emit diagnostics
+- ❌ **Integration behavior is NOT tested**: PDF extraction is never called
+- ❌ **Output quality is NOT verified**: Partial data quality is unchecked
+- ❌ **Error detection is impossible**: Return type can't indicate errors
+
+**Most Critical Gap:** The FlateDecoder silently catches `UnexpectedEof` but never emits `STREAM_DECODE_ERROR` diagnostic, making it impossible for users or downstream code to detect that truncation occurred.
+
+**Path Forward:** 
+1. Fix FlateDecoder to emit diagnostics
+2. Update stream decoder test to expect diagnostics
+3. Make integration test actually run extraction
+4. Add assertions for partial output quality