Skip to content

fix(native-compaction): retain image-only user content - #91557

Open
andrexibiza wants to merge 1 commit into
NousResearch:mainfrom
andrexibiza:fix/native-compaction-image-only-retention
Open

fix(native-compaction): retain image-only user content#91557
andrexibiza wants to merge 1 commit into
NousResearch:mainfrom
andrexibiza:fix/native-compaction-image-only-retention

Conversation

@andrexibiza

@andrexibiza andrexibiza commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Problem

#91477 repaired native-compaction summary provenance, but pre-checkpoint pruning still dropped an image-only user message. The normalized Responses item has a valid input_image part and no measurable text, so the text-only extraction path returned None and pruning discarded the message before retention accounting.

Repair

  • Keep _extract_item_text() text-only.
  • Grant retention authority only to the adapter-owned normalized input_image shape with a non-empty string image_url.
  • Retain the original user-message object verbatim at one-token cost and preserve the explicit retention budget.
  • Reject empty, malformed, and unknown multipart placeholders instead of treating any non-empty list as durable history.
  • Do not claim file-only support: the current chat-to-Responses adapter does not emit input_file.

Proof

The regression suite proves text extraction remains text-only, valid image-only user content survives a replayed compaction checkpoint verbatim, zero budget still drops it, malformed or unknown multipart content is not promoted, and the production _chat_messages_to_responses_input() path carries the image-only message across the checkpoint.

Exact green object

The branch contains one commit only. Superseded red objects are no longer part of the PR history.

The final object republishes the identical source tree after an unrelated focus-redraw timing failure on the superseded object; there is no source delta between the two objects.

Interlocks

#90976 → merged #91477#91557. This is a distinct post-merge regression repair, not a reopening or repeat review of #90975.

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint area/compression Context compression and continuation sessions P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 21, 2026
@andrexibiza andrexibiza changed the title fix(native-compaction): retain image/file-only user content fix(native-compaction): retain image-only user content Aug 21, 2026

@andrexibiza andrexibiza left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-head review complete at fca470be7cf50d3ff4f6f0dc9a873f5f6b8e4609: no remaining blocker.

This head closes the residual post-#91477 non-text loss without broadening arbitrary multipart content into durable history:

  • _extract_item_text() remains text-only;
  • only the adapter-owned normalized input_image shape with a non-empty image_url receives attachment retention authority;
  • the original user message is retained verbatim and still consumes the explicit retention budget;
  • empty, malformed, and unknown multipart placeholders remain non-retainable;
  • the production _chat_messages_to_responses_input() conversion path is exercised across an actual compaction checkpoint, rather than fabricating only a pruner-local shape.

The earlier unsupported file-only claim is correctly removed because the current adapter does not emit input_file.

Fresh exact-head hosted evidence is green: CI 32499632201, Docker 32499631402, and Nix 32499631392. The PR is open, non-draft, and mergeable against current main. No source, review, exact-head CI, or mergeability blocker remains.

@andrexibiza
andrexibiza force-pushed the fix/native-compaction-image-only-retention branch 5 times, most recently from 8223fda to 4efadbb Compare August 21, 2026 17:24
Preserve valid normalized input_image user messages across native-compaction checkpoints at bounded one-token retention cost. Keep text extraction text-only, reject malformed or unknown multipart placeholders, and prove the production adapter path without claiming unsupported input_file behavior.

Republish the identical source tree after an unrelated nondeterministic focus-redraw test failure; this commit contains no source delta from the previously verified object.

Refs NousResearch#90976 and NousResearch#91477.
@andrexibiza
andrexibiza force-pushed the fix/native-compaction-image-only-retention branch from 4efadbb to 039e269 Compare August 21, 2026 17:31

Copy link
Copy Markdown
Contributor Author

Final status

This is the authoritative current-state receipt after the branch cleanup.

Exact-object verification

The repair is deliberately narrow: valid normalized input_image user content survives native-compaction checkpoint pruning verbatim; text extraction remains text-only; malformed or unknown multipart placeholders are not promoted; zero retention budget still drops the item; and no unsupported input_file claim is made.

The final object republishes the identical source tree after an unrelated timing failure on the superseded object. No source delta was introduced during the history cleanup. No source, exact-head CI, base-alignment, or mergeability blocker remains. The remaining repository transition is upstream acceptance and merge.

Interlock: #90976 → merged #91477#91557.

@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

Good catch on what looks like a comment-vs-code regression: the old inline comment promised "Image-only user messages … Don't skip them just because text is falsy", but the guard above it (if text is None: continue) ran first — an image-only message has no text parts, _extract_item_text returns None, and the item was dropped before the sympathetic branch could ever fire. This restores the documented contract, and doing it via a positive has_retainable_image signal instead of loosening the None-check is the right structure: junk stays dropped by default and images earn retention explicitly.

Points:

  1. The strict predicate matches reality — pin that coupling. I checked agent/codex_responses_adapter.py: every image emit site produces exactly {"type": "input_image", "image_url": <str>} (no file_id variants), so _has_retainable_image_content's narrow shape test covers all producible inputs today, and its fail-closed stance against malformed/empty parts is correct — "non-empty list" was far too weak to justify durable history. Since correctness depends on the adapter never growing a new image shape without updating this predicate, consider a one-line comment on _has_retainable_image_content pointing at the adapter's emit sites (or vice versa) so the two evolve together.

  2. Test suite is exemplary — verbatim retention asserted with identity (pruned[1] is image_only, not just equality), budget obedience at zero (so images can't smuggle past a tight budget), the malformed-matrix ({}, empty URL, unknown type), and an end-to-end conversion test proving the chat-completions image_url → Responses input_image handoff survives across a checkpoint. All behavior contracts, nothing snapshot-shaped.

  3. Two scope notes worth a docstring line each: (a) retention is user-only (is_user and …) — assistant/tool items carrying images are still pruned; presumably intentional (matches main's accounting) but stating it prevents "why not symmetric?" follow-ups. (b) A mixed [input_text(""), input_image] message falls through with text="" rather than the image path — fine either way, just confirming it's considered.

  4. Nit: the new if text is None: text = "" coercion reads slightly odd directly under a continue that already excluded None-without-image; folding it into the first condition (if text is None and not has_retainable_image: continue / text = text or "") would be marginally tighter — purely stylistic.

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

Labels

area/compression Context compression and continuation sessions comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants