UN-2646 [FEAT] OSS half of the VLM image-answer feature (path contract, loader, vision policy, bridge) - #2218
Merged
chandrasekharan-zipstack merged 12 commits intoAug 5, 2026
Conversation
…surface
Foundation for the VLM consumer (MUNS-206): the writer (adapter) and the
upcoming page-image reader must agree on the storage layout without
metadata persistence or a manifest sidecar.
- Promote build_page_store_dir ({extract_dir}/{stem}/pages) from the
LLMWhisperer helper to unstract.sdk1.adapters.x2text.constants; the
helper now binds the shared function directly (identity, not a copy)
- Add page-naming constants to ImageOutputConstants: PAGES_SUBFOLDER,
PAGE_IMAGE_PREFIX/EXTENSION/PADDING, PAGE_GLOB_PATTERN and
PAGE_NUMBER_REGEX (integer capture for natural sort — lexicographic
ordering misorders past page 999)
- ImageOutputConfig now aliases the shared layout constants; the
tolerant ZIP-member pattern moves to a named ZIP_PAGE_MEMBER_REGEX
(service wire contract, distinct from the storage contract)
- New writer/reader agreement tests: path determinism across stem
shapes, glob/regex contract incl. >999-page fixtures, and the
writer's filename builder round-tripping through the reader's
glob + regex
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…licy Reader-side sdk1 utilities for the VLM consumer (MUNS-207 + the sdk1 half of MUNS-208): - page_image_loader: discovers persisted page_NNN.png files through the FileStorage abstraction using the shared naming constants, orders by integer page index, enforces a fail-fast page cap before any bytes are read, base64-encodes, and shapes complete_vision content blocks with 'Page N' labels before each image. Typed failures keep empty (never-extracted/purged), incomplete (post-write loss, with found vs missing pages), and over-cap cases distinct — remediation copy steers to cache-bypass re-extraction with the per-page re-billing warning. - vision_capability: classifies a model as SUPPORTED / UNSUPPORTED / UNKNOWN from litellm's local model_cost registry only — never get_model_info, which can make network calls for self-hosted providers, and never bare supports_vision, which returns False for both non-vision and unknown models and would wrongly hard-block custom vision models (Ollama, proxies). Policy: hard-block only a definitive UNSUPPORTED; UNKNOWN warns and allows. Tests: 33 new (in-memory S3-like double + real local FileStorage backend, >999-page ordering, cap-before-read assertion, registry fakes + real-registry smoke checks). Full sdk1 image suite: 104 pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Image-mode prompts are answered by a vision LLM via the cloud-only 'vlm-image-answer' executor plugin. This adds the OSS half (mirrors the lookup_enrichment bridge, with the opposite error policy — lookups degrade, image mode fails loudly): - workers vlm_image_answer bridge: detects image mode by resolving the x2text adapter config through the platform service (the payload carries only the instance id), cached per (execution, adapter); dispatches to the plugin with the deterministic pages dir from the shared path helper; raises structured, code-prefixed errors (IMAGE_OUTPUT_REQUIRES_CLOUD / IMAGE_OUTPUT_MISSING / IMAGE_PAGE_CAP_EXCEEDED / VISION_LLM_REQUIRED) that survive the string-only error propagation to PS and deployment responses — never a silent fallthrough to the text path - legacy_executor: image-mode branch replaces retrieval + completion only; type conversion, lookups, webhooks and challenge run unchanged. Single-pass extraction is rejected for image-mode profiles before delegating to the cloud single-pass plugin - backend vlm_utils no-op bridge (lookup_utils pattern) wired into: profile serialization (non-blocking vision_warning), API deployment creation (deploy-time guard), and the extraction choke point (VLM answer invalidation after a fresh extraction rewrites pages/) Tests: 13 bridge (detection/cache/dispatch/error mapping/single-pass guard) + 8 vlm_utils (OSS no-ops + cloud delegation policies). Full sdk1 image suite (104) and gating suite (12) stay green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Profile save responses may carry a backend-computed vision_warning (image output mode selected with an LLM that is not verifiably vision-capable — populated by the cloud vlm_image_answer hooks, never present in OSS responses). Show it as a non-blocking warning toast so the user learns about the mismatch at config time instead of at run time. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pk-zipstack
changed the title
Live-testing regression: with pages purged from object storage, the loader raised a raw FileNotFoundError instead of the typed incomplete-set error — fsspec's directory cache served discovery a stale listing in the long-lived worker, so the contiguity check passed and the purge only surfaced at read time, bypassing the error mapping. - discover_page_images now refreshes the backend's listing cache first (duck-typed invalidate_cache; no-op for backends without one) - load_page_images maps a read-time FileNotFoundError (TOCTOU: page vanished after discovery) onto PageImageSetIncompleteError with the cache-bypass + re-billing remediation - in-memory test double now raises FileNotFoundError like real backends Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Live deployment-path regression: workflow executions store the source file under an extension-less name (e.g. SOURCE), so the extension-only PDF guard false-rejected every API-deployment input — a real .pdf upload failed with the PDF-only error. _validate_pdf_only now checks the extension first and falls back to content sniffing (%PDF- magic bytes via FileStorage) when the name has no .pdf suffix; unverifiable content still rejects (fail-closed). No extra read on the common .pdf-named path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…re string Live-testing regression: validate_image_output_allowed raises from inside AdapterInstanceSerializer.to_internal_value, where DRF folds the detail into its per-field error mapping — a bare-string detail crashed error collection (ValueError: dictionary update sequence) and surfaced as a 500 'Something went wrong' instead of the clean 400 with the 'available only on Unstract Cloud' message. Use a field-keyed dict detail, and log the rejected adapter/mode for diagnosability. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pk-zipstack
marked this pull request as ready for review
July 29, 2026 10:09
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
|
| Filename | Overview |
|---|---|
| workers/executor/executors/vlm_image_answer.py | Adds image-mode detection, run-scoped caching, cloud-plugin dispatch, and structured error mapping; the previously reported stale-cache issue is fixed by run scoping and no-scope cache bypass. |
| workers/executor/executors/legacy_executor.py | Integrates VLM answers into the existing prompt execution path while retaining normal conversion, enrichment, webhook, and persistence behavior. |
| unstract/sdk1/src/unstract/sdk1/adapters/x2text/page_image_loader.py | Adds ordered page discovery, completeness and count checks, bounded aggregate reads, base64 encoding, and typed loader failures; the previously reported unbounded-byte issue is addressed. |
| unstract/sdk1/src/unstract/sdk1/adapters/x2text/constants.py | Defines the shared page-path and naming contract used by both image writers and readers. |
| unstract/sdk1/src/unstract/sdk1/utils/vision_capability.py | Classifies vision support using LiteLLM's local registry and blocks only definitively unsupported models. |
| backend/prompt_studio/vlm_utils.py | Adds optional cloud-hook bridges for compatibility warnings, deployment validation, and answer invalidation. |
| backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py | Stamps image-mode metadata, preserves the original extraction path, and invokes answer invalidation after successful re-extraction. |
| frontend/src/components/custom-tools/add-llm-profile/AddLlmProfile.jsx | Displays the backend-provided vision compatibility warning after a successful profile save. |
Sequence Diagram
sequenceDiagram
participant UI as Prompt Studio / Deployment
participant Backend
participant Worker as Executor Worker
participant Bridge as VLM Bridge
participant Plugin as Cloud VLM Plugin
participant Store as FileStorage
participant LLM as Vision LLM
UI->>Backend: Run image-mode prompt
Backend->>Worker: Dispatch answer_prompt
Worker->>Bridge: Resolve image output mode
Bridge->>Plugin: run_with_metrics(...)
Plugin->>Store: Discover and bounded-read page images
Store-->>Plugin: Ordered page PNGs
Plugin->>LLM: Prompt and page-image blocks
LLM-->>Plugin: Answer
Plugin-->>Worker: VLM answer and usage
Worker-->>Backend: Persist and return normal prompt result
Backend-->>UI: Answer or structured image-mode error
Reviews (5): Last reviewed commit: "UN-2646 [FIX] Bound page-image reads to ..." | Re-trigger Greptile
… budget
Two review findings on the consumer path:
- Run-scoped detection cache (P1): IDE payloads carry no execution_id,
so every IDE run shared one ('', adapter) cache entry — an adapter
switched between text and image output kept serving the stale mode
until worker restart. The cache key now scopes to execution_id or,
for IDE runs, the run_id (deduplicating the N per-prompt resolutions
within one run — the cache's actual purpose — with no cross-run
reuse), and skips caching entirely when no scope id exists.
- Aggregate byte budget on image loading (P2): the page cap bounds the
COUNT of images, not their size. load_page_images now enforces a
50MB (default, disable-able) raw-byte budget while reading, raising
the typed PageImageSetTooLargeError with page-range remediation; the
bridge maps it to a distinct IMAGE_PAGES_TOO_LARGE error code.
Tests: run-scoped vs no-scope cache behavior, byte-budget stop-at-page
accounting, and the new error-code mapping.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…y read Greptile follow-up: the read-time budget still pulled each full object into worker memory before checking cumulative size, so one pathological page could spike RAM before rejection. The budget is now enforced twice: first from storage size metadata (object HEAD — zero bytes transferred) before any read, so an oversized set is rejected with no image bytes in memory; the read-time accounting stays as a belt-and-braces guard for backends without size metadata and for stat/read races. Tests: metadata pre-check rejects with zero reads; no-size() backends still enforced at read time; unreadable metadata falls back cleanly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
Re: Confidence 3/5 — "an oversized individual page can still cause an uncontrolled worker-memory allocation before the new byte limit rejects it" Addressed in 46c34f2. The byte budget is now enforced from storage size metadata (object HEAD) before any read — an oversized page set is rejected with zero image bytes entering worker memory. The read-time accounting remains as a belt-and-braces guard for backends without a |
chandrasekharan-zipstack
left a comment
chandrasekharan-zipstack
left a comment
Contributor
There was a problem hiding this comment.
Reviewed both halves (#2218 + Zipstack/unstract-cloud#1690) against the live code paths.
The architecture holds up well. Binding writer and reader to one build_page_store_dir (with a test asserting is identity, not equality) is the right way to make a two-sided storage contract undriftable, and the fail-loud-never-fall-through error policy is applied consistently across the bridge, the loader and the gating. Gate-before-read with a test asserting fs.read_calls == [], and the error-code prefixing to survive string-only ExecutionResult.error propagation, are both nicely done.
Verified and holding: fs.ls / fs.size / fs.read(length=) all exist on FileStorage; complete_vision / get_model_name / get_metrics match the SDK1 signatures; the TOCTOU except FileNotFoundError works because skip_local_cache re-raises FileNotFoundError after its invalidate-and-retry, and the new invalidate_cache in discovery covers the stale-positive listing case that decorator does not; the DRF dict-detail fix is the correct shape for a raise inside to_internal_value.
Six comments inline plus one NIT roll-up. My ranking: #1 is a blocker — user-reachable, misleading, and costs money to discover. #2 and #3 are design decisions I would like stated rather than left as defaults. Everything else is cleanup.
… image-mode scoping Addresses Chandrasekharan's review on the consumer path: 1. Pages directory keys on a never-rewritten extract path (blocker): summarize-as-source and smart-table runs rewrite the payload file_path before the answer step, so the reader looked in the wrong directory and surfaced IMAGE_OUTPUT_MISSING with a paid remediation that could not help. The payload builders (IDE single/bulk and the structure tool task) now stamp an explicit extract_file_path, captured before any rewrite; the bridge derives pages/ from it, with the old file_path as fallback for older payloads. 2. Text-mode prompts no longer depend on the platform service: the backend stamps the x2text adapter's output_mode per prompt (it already holds the decrypted metadata), so detection is payload-only for stamped runs; the run-scoped platform resolution remains as the fallback for unstamped payloads (API deployments, older payloads). 3. Challenge and evaluation are skipped in image mode with a visible log line — both verify an answer against retrieval context, which image mode does not have; running them billed a doomed second LLM call. A vision-aware challenge is a later-phase decision. 4. Retrieval adapters are no longer constructed for image-mode prompts: detection now runs before adapter init, which proceeds LLM-only (embedding/vector DB skipped) when image mode is detected. 5. Profile-save toasts no longer swallow each other: one alert renders — 'Saved — check LLM compatibility' with the warning when present, plain success otherwise (the alert store holds a single entry, so two synchronous calls batched into showing only the last). 6. vlm_utils logs a distinct warning when plugins.vlm_image_answer is present but backend_hooks fails to import — 'cloud hooks broken' is no longer indistinguishable from 'running OSS'. Bridge API split accordingly: detect_image_mode_config (stamp fast path + platform fallback) and run_vlm_image_answer (dispatch only, keyed on extract_file_path). Tests: 23 bridge (stamped/unstamped detection, cache scoping, extract-path keying) all green; sdk1 130; backend 20. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
for more information, see https://pre-commit.ci
Greptile's follow-up was right: the metadata pre-pass fell back to an unbounded full-object read when the size lookup failed, so one pathological object could still spike worker memory before the budget check. Replaced both layers with a single stronger mechanism — bounded reads: every page is read with length = remaining budget + 1 via FileStorage.read, giving a hard allocation ceiling of budget + 1 bytes for the whole loop, independent of object sizes or the backend's size metadata. The metadata pre-pass is gone (also Chandrasekharan's call — one mechanism, less code). Also folds in the review nits: dead PAGE_GLOB_PATTERN removed (readers list + regex, never glob) and the duplicated not-found message extracted to one helper. Tests: bounded-read guarantees pinned (single 10MB object → exactly 51 bytes read; aggregate reads <= budget + 1) — 127 sdk1 + 23 bridge pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
chandrasekharan-zipstack
approved these changes
Aug 5, 2026
chandrasekharan-zipstack
merged commit Aug 5, 2026
a92f347
into
feat/llmwhisperer-image-output-adapter
6 checks passed
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
2 participants
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.
What
The complete OSS half of the VLM image-answer feature (the cloud-only consumer that answers prompts against LLMWhisperer image-output documents by sending the persisted page images to a vision LLM). Four commits, reviewable independently:
build_page_store_dir({extract_dir}/{stem}/pages) and the page-naming constants (PAGE_GLOB_PATTERN,PAGE_NUMBER_REGEX) from the LLMWhisperer helper to the shared x2text surface; the adapter binds the shared function by identity. The tolerant ZIP-ingest pattern stays adapter-local as a namedZIP_PAGE_MEMBER_REGEX(service wire contract ≠ storage contract).complete_visioncontent blocks with "Page N" labels, typed empty/partial/over-cap errors with cache-bypass + re-billing remediation copy) and a vision-capability policy (LiteLLM local-registry classification: SUPPORTED / UNSUPPORTED / UNKNOWN; hard-block only definitive UNSUPPORTED, warn-and-allow unknown/self-hosted — neverget_model_info, which can make network calls for Ollama).vlm_image_answer, mirroring thelookup_enrichmentbridge with the opposite error policy): detects image mode in theanswer_promptflow by resolving the x2text adapter config via the platform service (payload carries only the instance id), dispatches to the cloudvlm-image-answerexecutor plugin, and raises structured code-prefixed errors (IMAGE_OUTPUT_REQUIRES_CLOUD,IMAGE_OUTPUT_MISSING,IMAGE_PAGE_CAP_EXCEEDED,VISION_LLM_REQUIRED) when it cannot serve — never a silent fallthrough to the text path. Single-pass extraction is rejected for image-mode profiles. Backendvlm_utilsno-op bridge wires: profilevision_warning, deploy-time guard, and re-extraction invalidation.vision_warningtoast (field never present in OSS responses).Why
feat/vlm-image-answer).How
_execute_single_prompt, the image-mode branch replaces only retrieval + completion; type conversion, lookup enrichment, webhooks, challenge, and output persistence run unchanged, so image-mode answers behave identically downstream.(execution_id, adapter_instance_id), bounded — one platform-service call per adapter per run, and adapter edits are picked up on the next execution.ExecutionResult.erroris string-only; the code survives verbatim to the Prompt Studio socket events and deployment API responses.Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)
vlm_answer is Nonebranch and behave byte-identically (the bridge returnsNonewithout a platform call when no x2text adapter id is present, and after one cached resolution otherwise). The path-contract refactor is behavior-preserving — all 57 pre-existing sdk1 image tests pass unmodified. The backend hooks are no-ops without the cloud package. The FE change keys off a response field OSS never emits.Database Migrations
Env Config
VLM_IMAGE_ANSWER_PAGE_CAPis read by the cloud plugin, not OSS.)Relevant Docs
Related Issues or PRs
mainwhen UN-2646 [FEAT] LLMWhisperer image output mode adapter #2210 merges.unstract-cloudfeat/vlm-image-answer(executor plugin + backend hooks; the two halves meet only at runtime via theunstract.executor.pluginsentry point and theplugins.vlm_image_answerpackage presence).Dependencies Versions
Notes on Testing
Unit suites
executor/tests/test_vlm_image_answer_bridge.py— detection, per-execution caching, plugin-absent structured error, error-code mapping, single-pass guard).vlm_utilsbridge + 12 image-output gating).Live end-to-end validation (merged-tree Docker stack, MinIO object storage)
Run against a local rig with the companion cloud plugin installed, then removed — every path below verified in the running product:
POST /deployment/api/...)pages/IMAGE_OUTPUT_MISSINGwith cache-bypass + per-page re-billing remediation[6, 9]; zero VLM cost on failureIMAGE_PAGE_CAP_EXCEEDED; zero image reads (fail-fast before any bytes)nova-micro, non-vision)VISION_LLM_REQUIREDnaming the modelvision_warningtoast on profile saveIMAGE_OUTPUT_UNSUPPORTED_OPERATIONbefore the executor is invokederrorstatusimagestripped from the served schema (enum + enumNames + conditional block); existing image-mode profile →IMAGE_OUTPUT_REQUIRES_CLOUD(never a silent answer from the summary); adapter save backstop → clean 400Bugs found and fixed by the live testing (each with regression tests)
ff7a4774): fsspec's directory cache in the long-lived worker served a stale page listing, so a purged page passed discovery and blew up at read time as a rawFileNotFoundError. Discovery now refreshes the backend listing cache, and a read-time miss maps to the typedPageImageSetIncompleteError.801ce759): workflow executions store the source file asSOURCE(no extension), so the extension-only PDF guard false-rejected every API-deployment input. The guard now falls back to%PDF-magic-byte content sniffing (fail-closed; no extra read on.pdf-named paths).4d40c473): the rejection was raised with a bare-stringValidationErrorfrom insideto_internal_value, which crashes DRF's error collection into a 500. Now a field-keyed dict detail → clean 400 with the "available only on Unstract Cloud" message.Known accepted edge
An adapter saved in image mode on cloud and later opened on a plugin-less deployment renders a misleading dropdown state (RJSF displays the schema default while
formDatastill holds the removedimagevalue); submit is correctly rejected with the clean 400, and actively re-picking any mode recovers. Cloud→OSS downgrade only; documented rather than fixed (a fix would mean mutating stored data on read or custom widget work for a rare corner).Checklist
I have read and understood the Contribution Guidelines.
🤖 Generated with Claude Code