diff --git a/notes/bf-2hjaa.md b/notes/bf-2hjaa.md index 306cabc..b27a99f 100644 --- a/notes/bf-2hjaa.md +++ b/notes/bf-2hjaa.md @@ -2,121 +2,134 @@ ## Summary -This analysis identifies where in the stream decoder fixture test the error assertion should be added to validate that `STREAM_DECODE_ERROR` diagnostics are emitted for the `flate_truncated` fixture. +This analysis identifies where in the truncated-flate.pdf test the error assertion should be added to validate that `STREAM_DECODE_ERROR` diagnostics are emitted when processing a PDF with a truncated flate stream. -## Problem Statement +## Context -The `flate_truncated.meta` file specifies that decoding this fixture should: -1. Return partial bytes (what was decoded before the truncation point) -2. Emit a `STREAM_DECODE_ERROR` diagnostic +This is about a PDF extraction test for the fixture at `/home/coding/pdftract/tests/fixtures/malformed/truncated-flate.pdf`, NOT the stream decoder unit tests. The test should: +1. Extract the PDF using `pdftract_core::extract::extract_pdf()` +2. Assert extraction succeeds (non-fatal error recovery) +3. Assert `STREAM_DECODE_ERROR` appears in the errors/diagnostics array -However, the current test scaffold does not validate diagnostics at all. +## Extraction Result Structure -## Current State - -### 1. Fixture Configuration -Location: `crates/pdftract-core/tests/stream_decoder_fixtures.rs:60-64` +### From `/home/coding/pdftract/crates/pdftract-core/src/extract.rs`: ```rust -FixtureInfo { - name: "flate_truncated", - filter: FixtureFilter::Single("FlateDecode", None), - expected_diags: vec![], // <-- Should be vec![DiagCode::StreamDecodeError] - bomb_limit: None, -}, -``` +pub struct ExtractionResult { + pub fingerprint: String, + pub pages: Vec, + pub metadata: ExtractionMetadata, + // ... other fields +} -### 2. Decode Function Signature Issue -Location: `crates/pdftract-core/src/parser/stream.rs:89-95` - -The `StreamDecoder::decode()` trait: -```rust -fn decode( - &self, - input: &[u8], - params: Option<&PdfObject>, - doc_counter: &mut u64, - max_bytes: u64, -) -> Result, FilterError>; -``` - -**Problem:** The trait returns only `Result, FilterError>` - no diagnostics! -- When `UnexpectedEof` occurs, it returns `Ok(partial_bytes)` but cannot emit diagnostics -- See line 542-544 in `FlateDecoder::decode_impl()`: truncated streams just return partial bytes with no diagnostic - -### 3. Test Loop Missing Diagnostic Validation -Location: `crates/pdftract-core/tests/stream_decoder_fixtures.rs:254-360` - -The test loop: -1. ✓ Reads fixture and expected files -2. ✓ Decodes the fixture -3. ✓ Compares output bytes -4. ✗ **Does NOT validate diagnostics** - -## Where the Assertion Should Be Added - -**Location:** In `test_all_stream_decoder_fixtures()`, after the byte comparison check (after line 326) - -**What to validate:** -- If `fixture.expected_diags` is non-empty, verify that matching diagnostics were emitted -- For `flate_truncated`: assert that exactly one `STREAM_DECODE_ERROR` diagnostic was emitted - -## How to Fix - -### Option 1: Use higher-level `decode_stream()` function -The `decode_stream()` function returns `DecodeResult` which includes diagnostics: -```rust -pub struct DecodeResult { - pub bytes: Vec, - pub diagnostics: Vec, // <-- We need this! - pub meta: StreamMeta, +pub struct ExtractionMetadata { + // ... other fields + /// Diagnostics emitted during extraction (coverage warnings, etc.) + #[serde(skip_serializing_if = "Vec::is_empty")] + pub diagnostics: Vec, + // ... other fields } ``` -### Option 2: Add diagnostic support to `StreamDecoder::decode()` trait -Modify the trait to return diagnostics alongside the decoded bytes. +**Key insight**: The diagnostics are stored in `extraction_result.metadata.diagnostics` as a `Vec`, not `Vec`. -## Recommended Assertion Implementation +### Diagnostic Code Format -After line 326 (byte comparison), add: +From `/home/coding/pdftract/crates/pdftract-core/src/diagnostics.rs`: +- **Enum variant**: `DiagCode::StreamDecodeError` +- **String representation**: `"STREAM_DECODE_ERROR"` (via `Display` impl) + +## Optimal Assertion Location + +The assertion should be added in the test function `test_truncated_flate_recovery()` at the following location: + +**After**: Extraction succeeds and we have the `ExtractionResult` +**Before**: Any other assertions that might depend on the result + +### Complete Test Pattern + +Based on the pattern from `TH-04-js-presence.rs` lines 31-94: ```rust -// Validate expected diagnostics -if !fixture.expected_diags.is_empty() { - // Check if each expected diagnostic code is present in the actual diagnostics - for expected_code in &fixture.expected_diags { - let found = result.diagnostics.iter().any(|d| d.code() == *expected_code); - assert!( - found, - "{}: expected diagnostic {} not found. Got: {:?}", - fixture.name, - expected_code, - result.diagnostics - ); +#[test] +fn test_truncated_flate_recovery() { + let fixture = PathBuf::from("tests/fixtures/malformed/truncated-flate.pdf"); + + // Skip if fixture doesn't exist + if !fixture.exists() { + eprintln!("Skipping test: fixture not found at {}", fixture.display()); + return; } + + // Extract with default options + let options = ExtractionOptions::default(); + let result = extract_pdf(&fixture, &options); + + // ASSERTION LOCATION 1: Assert extraction succeeds (non-fatal recovery) + assert!(result.is_ok(), "Extraction should succeed despite truncated stream"); + + let extraction_result = result.unwrap(); + + // ASSERTION LOCATION 2: Assert STREAM_DECODE_ERROR appears in diagnostics + let diagnostics = &extraction_result.metadata.diagnostics; + assert!( + diagnostics.iter().any(|d| d.contains("STREAM_DECODE_ERROR")), + "Expected STREAM_DECODE_ERROR diagnostic for truncated flate stream. Diagnostics: {:?}", + diagnostics + ); + + // Optional: Verify extraction produced some output despite error + assert!(extraction_result.pages.len() > 0, "Should extract at least one page"); } ``` -## Edge Cases and Considerations +## What the Assertion Should Validate -1. **Partial bytes vs error**: Truncated streams should return partial bytes AND emit a diagnostic (not `Err()`) -2. **Multiple diagnostics**: Some fixtures may emit multiple diagnostics - check all expected ones -3. **Bomb limit tests**: `flate_bomb_3gb` should emit `StreamBomb` diagnostic -4. **Unknown filter**: `unknown_filter` fixture should emit `StreamUnknownFilter` diagnostic +1. **Primary validation**: That `"STREAM_DECODE_ERROR"` is present in `metadata.diagnostics` +2. **Implicit validation**: That extraction succeeded (`result.is_ok()`) - confirms non-fatal error recovery per INV-8 +3. **Optional validation**: That partial output was produced (not zero pages) -## Files to Modify +## Edge Cases and Error Conditions -1. **Test scaffold** (`crates/pdftract-core/tests/stream_decoder_fixtures.rs`): - - Fix `flate_truncated.expected_diags` to `vec![DiagCode::StreamDecodeError]` - - Add diagnostic validation loop after byte comparison - - Change `decode_fixture()` to return `DecodeResult` instead of `Vec` +1. **Fixture not found**: Skip test with clear message (standard pattern) +2. **Multiple diagnostics**: Use `.contains()` to be flexible about exact string format +3. **Case sensitivity**: The diagnostic string is `"STREAM_DECODE_ERROR"` (uppercase) +4. **Alternative diagnostic codes**: Some code paths use `DecompressionFailed` internally but map to `StreamDecodeError` -2. **Stream decoder trait** (`crates/pdftract-core/src/parser/stream.rs`): - - Either: use `decode_stream()` which already returns `DecodeResult` with diagnostics - - Or: modify `StreamDecoder::decode()` trait to support diagnostics +## Test File Location + +Recommended: `/home/coding/pdftract/crates/pdftract-core/tests/test_truncated_flate_recovery.rs` + +**Reasoning**: +- Follows naming pattern: `test__.rs` +- Keeps malformed PDF tests isolated +- Dedicated file for this specific test case + +## Required Imports + +```rust +use pdftract_core::extract::extract_pdf; +use pdftract_core::options::ExtractionOptions; +use std::path::PathBuf; +``` + +## Key Implementation Details + +1. **Use `.contains()` not `.eq()`**: Diagnostic strings may have additional context +2. **Check for presence not count**: Use `.any()` not exact count matching +3. **Provide helpful failure message**: Include actual diagnostics in assert message +4. **Follow TH-04 pattern**: That test demonstrates the canonical pattern for diagnostic assertions + +## References + +- **Pattern source**: `/home/coding/pdftract/crates/pdftract-core/tests/TH-04-js-presence.rs` lines 86-93 +- **Diagnostic enum**: `/home/coding/pdftract/crates/pdftract-core/src/diagnostics.rs` +- **Extraction API**: `/home/coding/pdftract/crates/pdftract-core/src/extract.rs` +- **Fixture location**: `/home/coding/pdftract/tests/fixtures/malformed/truncated-flate.pdf` ## Acceptance Criteria Status -- [x] Assertion purpose and scope defined -- [x] Optimal assertion location identified (after line 326 in test loop) -- [x] Assertion requirements documented (validate `STREAM_DECODE_ERROR` for `flate_truncated`) +- [x] Assertion purpose and scope defined (validate STREAM_DECODE_ERROR in diagnostics) +- [x] Optimal assertion location identified (after extraction succeeds, in diagnostics check) +- [x] Assertion requirements documented (use .contains() on metadata.diagnostics Vec)