Connected image chunks silently dropped in assemble_retrieval_results

Problem

In packages/shared-python/shared/services/retrieval/hydration/result_assembly.py, image chunks referenced by text chunks via connect_to metadata are silently dropped from retrieval results.

Root Cause

Line 46-50: All chunks referenced via connect_to are marked as embedded_targets and excluded from the final results list (line 54: if row.get('chunk_id') in embedded_targets: continue)

Line 71: When assembling text chunk content, only table type targets are inlined:

if normalize_chunk_type(target_row.get('chunk_type')) != 'table':
    continue  # images are skipped here

This means connected images are:

Excluded from the results array (treated as "embedded")
Never actually embedded/inlined into the parent text content

Impact

Affected paths: _run_classic_topk_route (new) and _try_run_small_corpus_route (existing, runs in production daily)
Not affected: Agentic route — the evidence renderer (agentic/evidence/renderer.py) has its own image handling that correctly inlines [Image: {asset_url}]\n{description}
Standalone image chunks (not referenced by any text via connect_to) are unaffected — they appear in results with correct asset_url

Suggested Fix

In assemble_retrieval_results, when processing text chunks' connected targets, add handling for image type alongside table:

if normalize_chunk_type(target_row.get('chunk_type')) == 'table':
    # existing table summary logic
elif normalize_chunk_type(target_row.get('chunk_type')) == 'image':
    # inline image reference: [Image: {asset_url or file_path}]\n{description}

Alternatively, exclude image chunks from embedded_targets so they appear as standalone results with their own asset_url.

I'd like to take this. Confirmed the drop against current main:

result_assembly.py:54 excludes every connect_to target from the results list (embedded_targets).
result_assembly.py:71 then inlines only table targets into the parent text; the != 'table': continue means image targets fall through and are never inlined. So a connected image is dropped from both places, exactly as described.

Of the two fixes in the issue, I'd go with option 1 (inline the image into the parent text) rather than option 2 (surface as standalone), because it keeps the classic/small-corpus routes consistent with the agentic route. The agentic renderer already inlines images as [Image: {display_ref}]\n{image_description} with display_ref = asset_url or file_path (agentic/evidence/renderer.py:196). Matching that format avoids two divergent image representations across routes.

Concrete shape:

Add an _image_inline_content(row) helper mirroring _table_summary_content / _table_display_ref, producing [Image: {asset_url|file_path}] followed by the image description (the image chunk's content, stripped of any self-referential ref the agentic path already strips at line 191).
In the text branch, replace the table-only gate with a dispatch: table → existing summary, image → the new helper. Keep the sort_order ordering so multiple connected assets stay stable.
Leave embedded_targets semantics unchanged so connected images still don't appear as duplicate standalone rows (they're now represented inline instead).

Tests: extend the retrieval assembly coverage (currently exercised via apps/worker/tests/contract/test_page_memory_retrieval_contract.py) with a text chunk that connects to one image target, asserting the image ref + description land in the parent content and the image is not emitted as a separate result. I'll add a focused unit test for _image_inline_content too.

One scope check before I open a draft: for a connected image with no asset_url/file_path and no description, should the parent still get an empty [Image] marker (parallels the table path's [Table] fallback at line 109), or should that degenerate case be skipped entirely? I lean toward skipping so we don't inject empty markers, but the table path keeps them, so I want to match whatever you prefer for consistency.

If you would rather surface images as standalone results instead, say so and I will take that route.

This looks fixed on main by #221 (cb807d18). _connected_media_parts now dispatches image targets to _image_display_content inside the text branch, so connected images are inlined as [Image: {asset_url|file_path}] plus the description instead of being dropped, and _compose_table_content covers the table-parent case via _connected_image_parts. The degenerate case (no asset_url/file_path, no description) returns empty and is filtered out, so no bare markers get injected.

Both routes named in the report (_run_classic_topk_route, _try_run_small_corpus_route) go through assemble_retrieval_results, so they pick this up.

Can probably be closed unless there's a remaining case you want covered.

Independently verified on current main (cd2ea36e).

The drop described here is fixed by #221 (cb807d18):

text → image connect_to targets are inlined via _connected_media_parts → _image_display_content as [Image: {asset_url|file_path}] plus the description.
text → table targets still compose through _compose_table_content (summary/keywords/caption; HTML is not leaked).
embedded_targets is unchanged, so connected image/table chunks are not also emitted as standalone results.
Classic top-k and small-corpus both go through assemble_retrieval_results, so they pick this up.

There was no regression test for the image path, so I am opening a test-only PR that locks the cases above (synthetic fixtures only). Related to #206 — maintainers can close this once that lands.

Out of scope for this issue (flagging only): chunk_type=page parents still use the summary path and do not inline connect_to images. Happy to file a follow-up if that case should be covered too.

Follow-up: the regression tests are now in #331.

Independent check on main: #221 (cb807d18) fixed the silent drop. _connected_media_parts now dispatches image connect_to targets to _image_display_content ([Image: {asset_url|file_path}] plus description) instead of skipping them. Table targets still compose correctly, and connected chunks stay out of the standalone result list.

Maintainers can close this issue against #331.

Verified again on current main (85d5b2ba).

The drop described here is already fixed by #221:

result_assembly._connected_display_by_target inlines both image (via _image_display_content) and table.
inline_assets_at_placeholders inserts at [images/...] / [tables/...] or appends when the parent has connect_to but no placeholder.
Connected targets stay in embedded_targets and are not also emitted as standalone rows.
Classic top-k and small-corpus both go through assemble_retrieval_results.

I closed the duplicate test PR #331 (parallel module). Assemble-level image regressions are in #409, added to the existing test_asset_inline.py rather than a new file.

Please close this issue against #221; #409 only locks the behavior.

Closing: production fix is #221; remaining test lock is #409. The original silent-drop path is gone on main.

test: lock connected image inline in assemble_retrieval_results

Navigation Menu

Repository navigation

Connected image chunks silently dropped in assemble_retrieval_results #206

Problem

Root Cause

Impact

Suggested Fix

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Connected image chunks silently dropped in assemble_retrieval_results #206

Description

Problem

Root Cause

Impact

Suggested Fix

Activity

nuemaan commented on Jul 9, 2026

nuemaan commented on Aug 5, 2026

Frankie-Xu commented on Aug 25, 2026

Frankie-Xu commented on Aug 25, 2026

Frankie-Xu commented on Sep 11, 2026

Frankie-Xu commented on Sep 11, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions