Skip to content

UN-4042 [FIX] Bill the pages LLMWhisperer extracted, not every page in the file - #2266

Open
athul-rs wants to merge 1 commit into
mainfrom
athul/UN-4042-bill-extracted-pages
Open

UN-4042 [FIX] Bill the pages LLMWhisperer extracted, not every page in the file#2266
athul-rs wants to merge 1 commit into
mainfrom
athul/UN-4042-bill-extracted-pages

Conversation

@athul-rs

@athul-rs athul-rs commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

What

push_usage_details opened the input file and counted its pages, so an adapter configured with pages_to_extract = "1-5" billed all 199 pages of a 199-page PDF while LLMWhisperer billed the 5 it actually read.

Real case — CoverForce, 27 July 2026: a 199-page PDF, 5 pages requested, 5 processed, completed successfully. LLMWhisperer billed 5. Unstract billed 199.

Across July for that org alone: 1,622 documents, 24,078 pages charged and never extracted.

Why this approach

LLMWhisperer already returns the number it bills, and it was reaching the adapter and being thrown away.

assemble_document() computes processed_page_count once (unstract-llm-whisperer/backend/tasks_v2.py:1834) and uses that same local in both places:

line use
tasks_v2.py:2032 into whisper_metadata — what this PR reads
tasks_v2.py:2095 page_count_total= — what LLMWhisperer bills

There is no second derivation, so the two meters cannot drift. The whole assembled_document already travels to the SDK via /whisper-retrieveclient_v2.py:621helper.make_request → the V2 adapter, which kept only whisper_hash and line_metadata.

No LLMWhisperer-side change is required. Re-implementing the page-range rule inside the SDK was considered and rejected — it would be a second implementation to keep in sync, which is the bug we are fixing.

Change

  • TextExtractionResult carries an optional page_count. Additive with a default, so the five adapters that report nothing are unaffected.
  • LLMWhispererHelper.get_processed_page_count() reads and validates it off the response. bool is rejected explicitly, since isinstance(True, int) is True and would silently bill 1 page.
  • push_usage_details() prefers it and falls back to counting the file when it is absent or unusable.

Side effect: PDFs on the LLMWhisperer path are no longer re-read and parsed by pdfplumber. One fewer full-file read and parse per document.

Scope

PDFs only. LLMWhisperer also applies its 3,000-words-per-page rule to spreadsheets and text (tasks_v2.py:1836-1857) where Unstract hardcodes 1 page. Adopting that here would fix a second gap but would raise those bills the day it ships, so it stays in UN-4043 where account owners can be warned first.

input before after
PDF, pages_to_extract=1-5, 199 pages 199 5
PDF, no range 199 199
text file, 60k words 1 1
xlsx, 12k words 1 1

Invariant: the billed count can only go down or stay equal. Never up.

Reviewer notes

A second behaviour change beyond pages_to_extract. processed_page_count = len(page_nos) counts pages actually assembled. If a single page fails inside an otherwise-successful document, processed < total even with no range configured, so that document now bills less than before. This is correct — we bill what we extracted, matching LLMWhisperer — and it only ever reduces, but it is worth knowing about.

Revenue impact is real and intended. CoverForce July would move 55,721 → ~32,189, a 42% reduction for that account. Every customer using the "Page number(s) or range to extract" setting is affected.

LLMWhisperer V1 is not fixed. It has the same bug and the same setting, but its API returns raw text with no page count — the response carries nothing to read. It is deprecated in its own schema (llm_whisperer/src/static/json_schema.json:9). Left alone deliberately.

Not in this PR: the dead MimeType.TEXT branch at x2txt.py:110-114 reads the whole file into memory, assigns text_extraction_result, and is overwritten unconditionally by the next line. Real and confirmed inert by coverage, but unrelated — kept out to keep this diff reviewable.

Import churn: the two import-block edits are from the pinned pre-commit ruff; those files were already failing lint on main.

Testing

New unstract/sdk1/tests/test_x2txt_page_usage.py — 20 tests, all passing. Every changed line is covered.

  • adapter reports 5 on a 199-page PDF → bills 5 (fails before this change)
  • adapter reports nothing → bills 199 via pdfplumber, unchanged
  • adapter reports 0, -3, True, "5", 5.0 → all rejected, falls back
  • non-PDF with a reported count of 20 → still bills 1, gate holds
  • X2Text.process() forwards the adapter's count end to end
  • LLMWhispererV2.process() lifts it from whisper_metadata, and returns None when absent

Full SDK suite: 494 passed. The 4 failures in test_llm_compat.py are pre-existing — pytest-asyncio is not installed in the dev venv — and reproduce identically on a stashed tree.

Jira: UN-4042 · follow-up: UN-4043

🤖 Generated with Claude Code

https://claude.ai/code/session_019zLkLjqzwoN5Lm8thPqV9p

…n the file

`push_usage_details` opened the input file and counted its pages, so an adapter
configured with `pages_to_extract = "1-5"` billed all 199 pages of a 199-page
PDF while LLMWhisperer billed the 5 it actually read.

LLMWhisperer already returns the number it bills. `assemble_document`
(`tasks_v2.py:1834`) computes `processed_page_count` once and uses that same
value for both `whisper_metadata` (line 2032) and `page_count_total` (line 2095),
and the whole `assembled_document` reaches the adapter through `/whisper-retrieve`.
The V2 adapter was discarding it.

- `TextExtractionResult` carries an optional `page_count`
- `LLMWhispererHelper.get_processed_page_count` reads and validates it off the
  response
- `push_usage_details` prefers it, falling back to counting the file when an
  adapter reports nothing

Scoped to PDFs. LLMWhisperer also applies its 3000-words rule to sheets and text
where Unstract hardcodes 1 page; adopting that raises those bills and is tracked
separately in UN-4043.

The billed count can now only go down or stay equal, never up.

CoverForce, July 2026: 1,622 documents, 24,078 pages charged and never extracted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zLkLjqzwoN5Lm8thPqV9p
@sonarqubecloud

Copy link
Copy Markdown

@greptile-apps

greptile-apps Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR propagates LLMWhisperer V2’s processed-page count through the extraction result and uses it when recording PDF usage, retaining full-file counting as the compatibility fallback.

  • Adds an optional processed-page count to TextExtractionResult.
  • Validates and extracts the count from LLMWhisperer V2 metadata.
  • Bills PDFs using the reported count while preserving existing non-PDF behavior.
  • Adds focused tests for validation, fallback behavior, and end-to-end propagation.

Confidence Score: 5/5

The PR appears safe to merge with no actionable correctness, compatibility, or security issue identified.

The new field is additive, all existing adapters return the updated result type, LLMWhisperer values are validated before use, and missing metadata preserves the previous PDF-counting behavior.

Important Files Changed

Filename Overview
unstract/sdk1/src/unstract/sdk1/adapters/x2text/dto.py Adds an optional, backward-compatible page-count field to extraction results.
unstract/sdk1/src/unstract/sdk1/adapters/x2text/llm_whisperer_v2/src/helper.py Safely reads a positive non-boolean integer from LLMWhisperer response metadata.
unstract/sdk1/src/unstract/sdk1/adapters/x2text/llm_whisperer_v2/src/llm_whisperer_v2.py Propagates the validated processed-page count into the adapter result.
unstract/sdk1/src/unstract/sdk1/x2txt.py Uses reported counts for PDF usage and preserves full-file and non-PDF fallback behavior.
unstract/sdk1/tests/test_x2txt_page_usage.py Covers metadata validation, PDF fallback accounting, non-PDF behavior, and end-to-end propagation.

Sequence Diagram

sequenceDiagram
    participant LW as LLMWhisperer V2
    participant A as X2Text Adapter
    participant X as X2Text
    participant U as Usage Audit
    LW-->>A: whisper_metadata.processed_page_count
    A-->>X: TextExtractionResult(page_count)
    alt Valid count and PDF
        X->>U: Report processed page count
    else Missing count and PDF
        X->>X: Count pages with pdfplumber
        X->>U: Report full PDF page count
    else Non-PDF
        X->>U: Report one page
    end
Loading

Reviews (1): Last reviewed commit: "UN-4042 [FIX] Bill the pages LLMWhispere..." | Re-trigger Greptile

@github-actions

Copy link
Copy Markdown
Contributor

Unstract test results

Per-group results

Status Group Tier Passed Failed Errors Skipped Duration (s)
e2e-api-deployment e2e 3 0 0 0 16.3
e2e-coowners e2e 1 0 0 0 1.3
e2e-etl e2e 1 0 0 0 8.1
e2e-login e2e 2 0 0 0 0.9
e2e-prompt-studio e2e 1 0 0 0 4.1
e2e-smoke e2e 2 0 0 0 0.9
e2e-workflow e2e 1 0 0 0 27.9
integration-backend integration 310 0 0 26 48.0
integration-connectors integration 1 0 0 7 8.5
integration-workers integration 157 0 0 1 51.9
unit-backend unit 1158 0 0 1 46.6
unit-connectors unit 63 0 0 0 10.8
unit-core unit 33 0 0 0 1.5
unit-platform-service unit 15 0 0 0 2.9
unit-rig unit 117 0 0 0 5.8
unit-runner unit 5 0 0 0 3.2
unit-sdk1 unit 583 0 0 0 33.3
unit-workers unit 1397 0 0 1 135.4
TOTAL 3850 0 0 36 407.3

Critical paths

⚠️ Critical paths not yet covered

  • workflow-execution-fan-out — Multi-file workflow execution fans out to file-processing workers and rejoins. (declared coverage: no groups declared)
✅ Covered critical paths
  • auth-login — covered by e2e-login
  • adapter-register-llm — covered by integration-backend
  • workflow-author — covered by integration-backend
  • co-owner-manage — covered by integration-backend, e2e-coowners
  • workflow-create-execute — covered by e2e-workflow
  • api-deployment-provision — covered by integration-backend
  • api-deployment-auth — covered by integration-backend
  • api-deployment-run — covered by e2e-api-deployment
  • mcp-server-auth — covered by integration-backend
  • mcp-platform-auth — covered by integration-backend
  • prompt-studio-author — covered by integration-backend
  • prompt-studio-fetch-response — covered by e2e-prompt-studio
  • connector-register-test — covered by integration-backend
  • pipeline-etl-execute — covered by e2e-etl
  • usage-aggregate-read — covered by integration-backend
  • usage-token-tracking — covered by e2e-api-deployment
  • callback-result-delivery — covered by e2e-api-deployment

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant