Skip to content

fix(tests): test_toc_extractor encodes page-1 coverage gap as expected correct behavior #273

Closed

Description

@piyushmakhija28

Summary

ingestion/tests/test_toc_extractor.py at line 163 asserts covered == list(range(2, 9)), explicitly encoding a page-1 coverage gap as the expected correct result. The test name claims 'no gap' but content on page 1 is silently excluded. In production, page 1 is never covered by any section, making its content permanently unreachable via routing.

Root Cause

ingestion/tests/test_toc_extractor.py around line 163:

def test_out_of_order_raw_toc_keeps_content_and_leaves_no_gap():
    raw = [(1, 'A', 5), (1, 'B', 2)]  # B on page 2, A on page 5
    total_pages = 8
    entries = _derive_page_ranges(raw, total_pages)
    covered = sorted({p for e in entries for p in range(e.page_start, e.page_end + 1)})
    assert covered == list(range(2, 9))   # ← page 1 is excluded
    # The test name says 'no gap' but range(2,9) starts at page 2, not page 1

After sorting and deriving ranges: B=[2,4], A=[5,8]. Page 1 is never assigned to any section. The first entry starts at page 2 because that's where the earliest section begins — but page 1 content is abandoned.

Failure Scenario

  1. A legal PDF has its executive summary on page 1, followed by a table of contents starting page 2.
  2. TOC extraction produces raw = [(level, 'Section 1', 2), (level, 'Section 2', 5)].
  3. _derive_page_ranges correctly gives B=[2,4], A=[5,N] — page 1 unassigned.
  4. Chunker processes only pages 2–N. Page 1 executive summary produces zero chunks.
  5. Routing queries about the executive summary return no results.

Proposed Fix

The fix depends on intended behavior:

Option A (cover page 1 via first section extension):

# In _derive_page_ranges: extend the first section back to page 1
if entries and entries[0].page_start > 1:
    entries[0] = replace(entries[0], page_start=1)

Option B (document the gap explicitly):

  • Rename test to test_section_before_first_toc_entry_is_not_covered
  • Add a separate test test_pre_toc_content_is_covered_when_enabled for the Option A behavior

Update the assertion accordingly:

# Option A:
assert covered == list(range(1, 9))   # page 1 now included

Activity

  1. piyushmakhija28 commented on Jun 21, 2026

    @piyushmakhija28
    ContributorAuthor

    Fixed in PR #280 (Sprint R3/R4: fix(sprint-r3): root-cause fixes for all 39 bugs from code review round 3 + Sprint R4 round 4)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingcode-reviewFound via automated code reviewpriority: mediumFunctional error with workaround

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions