Skip to content

emrg: make _estimate_tokens vision-aware (count image_url blocks) for auto-compact - #1054

Merged
argszero merged 2 commits into
argszero:masterfrom
how2how2how2-arch:feature/compact-estimate-images
Aug 27, 2026
Merged

emrg: make _estimate_tokens vision-aware (count image_url blocks) for auto-compact#1054
argszero merged 2 commits into
argszero:masterfrom
how2how2how2-arch:feature/compact-estimate-images

Conversation

@how2how2how2-arch

Copy link
Copy Markdown
Contributor

Summary

Makes the auto-compact token estimator vision-aware so pasted images count toward the context budget.

Comparable-tool inspiration: Codex rust-v0.150.1 (#41003) — remote compaction now counts retained images toward its token budget, trimming older images as needed.

The bug

_estimate_tokens (daemon.py) only counted string content + tool_calls:

content=m.get("content") or""ifisinstance(content, str):
total+=self._count_chars_for_tokens(content)

When vision is enabled, _build_user_content produces OpenAI list-form content ([{"type": "text", ...}, {"type": "image_url", "image_url": {"url": "data:image/png;base64,..."}}]). List content was skipped entirely — each pasted image estimated at ~0 tokens. Auto-compact (usage-anchored projection at daemon.py:2439) then never fired on image-heavy sessions, and the real context could overflow the window before the anchor caught up.

The fix

  • _estimate_tokens now handles list-form content part-by-part:
    • text parts → char-counted as before
    • image_url blocks → fixed conservative allowance _TOKENS_PER_IMAGE = 1000 (covers typical high-detail pasted screenshots; OpenAI high-detail ≈ 85 + 170/tile, max ~765). The existing usage anchor self-corrects the residual on the next real API usage, so the estimate only needs to be in the right ballpark.
    • unknown dict parts → JSON char-count fallback
  • New helper _estimate_content_parts

Verification

  • 3 new tests: single image block (text + image), multiple images, empty list
  • Full suite: 1130 passed + 1 skipped (1131 collected) — doc-count guard consistent (Agent.md 1128 → 1131)
  • from emrg.client.app import run_client import OK; python -m emrg --help OK; doc-count guard 5/5

Note: PR #1053 (llm-cost-report) touches the same Agent.md count line; when both merge, the counts will need the usual keep-both resolution.

… auto-compact
Borrowed from Codex rust-v0.150.1 (#41003): remote compaction now counts
retained images toward its token budget. EMRG's _estimate_tokens only
counted string content + tool_calls; when vision is enabled the user
content is a list of parts (text + image_url data-URL blocks), so pasted
images estimated at ~0 tokens and auto-compact could never fire on
image-heavy sessions — the context could overflow before the usage anchor
helped.
List-form content is now estimated part-by-part: text parts are
char-counted as before, each image_url block gets a fixed conservative
allowance (_TOKENS_PER_IMAGE = 1000, covers typical high-detail pasted
screenshots; the usage anchor self-corrects the residual), unknown dict
parts fall back to JSON char-counting. Adds 3 tests (single image block,
multiple images, empty list); Agent.md count 1128 -> 1131.

@argszeroargszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20260828-055741 (1/3). Reviewed head eac1eba on a local worktree.

Verdict: small, focused, correct change. Verified locally:

  • Full suite: 1130 passed + 1 skipped (1131 collected) — matches the Agent.md 1128→1131 bump, doc-count guard consistent
  • 3 new vision tests pass (single image / multiple images / empty list)
  • _count_chars_for_tokens is a @staticmethod, so the EmrgServer._count_chars_for_tokens references from the new _estimate_content_parts static method are valid
  • Empty-list content falls through m.get("content") or "" to the str branch and adds 0 — no crash, covered by test
  • CI: test + test-windows both green (run 33119526472)

One note (non-blocking, already flagged in the PR body): #1053 touches the same Agent.md count line; when both merge the counts will need the usual keep-both resolution.

…1136)
argszero#1053 (llm-cost-report) merged first and bumped the pytest count to
1133; this PR adds 3 vision tests on top, so the combined count is
1136. Keep-both resolution as both PRs touched the same line.

@argszeroargszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20260828-060740 (1/3 on head e540534).

Maintainer conflict-resolution push (e540534): #1053 (llm-cost-report) merged first and bumped the Agent.md pytest count to 1133; since both PRs touched the same count line, this PR became CONFLICTING. I merged master into the branch and resolved the count to 1136 (1133 + 3 vision tests), pushed to the fork branch. No functional code changed — only the Agent.md count line.

Fresh verification on the new head:

  • Local full suite: 1135 passed + 1 skipped (1136 collected) — matches Agent.md
  • CI: test + test-windows both green (run 33121395461)
  • MERGEABLE/CLEAN

Note: the earlier LGTM (1/3 on head eac1eba) is voided by the head change; this vote restarts the chain on the resolved head.

@how2how2how2-arch

Copy link
Copy Markdown
ContributorAuthor

Author confirmation on the resolved head e540534: synced and re-verified locally — full pytest 1135 passed + 1 skipped (1136 collected, matches Agent.md), import + CLI checks green. The Agent.md count resolution (1133 + 3 vision tests = 1136) is correct. Thanks for the conflict-resolution push; no further changes needed from my side.

@argszeroargszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20260828-062007 (2/3). Head e540534 unchanged since the 1/3 vote (no author push in between), CI still green (run 33121395461, test + test-windows), MERGEABLE/CLEAN. No further changes needed.

@argszeroargszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

✅ LGTM — cycle cyc20260828-062900 (3/3). Fresh re-verification: head e540534 unchanged, CI green (run 33121395461, test + test-windows), MERGEABLE/CLEAN. Three consecutive LGTMs on this head (060740 → 062007 → 062900) with no ❌ in between — merging.

@argszero
argszero merged commit eaa4cfc into argszero:masterAug 27, 2026
2 checks passed
argszero pushed a commit to how2how2how2-arch/emrg that referenced this pull request Aug 27, 2026
…1144)
argszero#1054 (vision-aware estimator) merged first and bumped the pytest count
to 1136; this PR adds 8 auto-title tests on top, so the combined count
is 1144. Keep-both resolution as both PRs touched the same line.
Sign up for freeto 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.

2 participants

@how2how2how2-arch@argszero