Skip to content

fix: bound oversized paginated message content - #6815

Open
Andre-4711 wants to merge 3 commits into
nesquena:masterfrom
Andre-4711:fix/wkwebview-oversized-message-display
Open

Andre-4711 wants to merge 3 commits into
nesquena:masterfrom
Andre-4711:fix/wkwebview-oversized-message-display

Conversation

@Andre-4711

@Andre-4711 Andre-4711 commented Aug 7, 2026

Copy link
Copy Markdown

Thinking Path

  • Paginated GET /api/session responses already bound oversized hidden role:"tool" rows, but equally large user or assistant rows still passed through unchanged.
  • A real restored session contained one historical user string of roughly 5.38 million characters. The API returned successfully, but native WKWebView remained stuck on Loading conversation... while its WebContent process consumed sustained CPU.
  • The narrow boundary is the existing paginated-display payload helper: bound only the copied response rows after the visible window is selected, without changing storage, full-history retrieval, export, or model context.
  • Structured content must retain the frontend's current visibility semantics: only type:"text" blocks count toward visible text, while hidden reasoning/tool/raw blocks are never projected into a new preview.

What Changed

  • Added a 64 KiB visible-content ceiling for oversized user and assistant rows in paginated session responses.
  • Reused the existing _content_truncated and _content_original_chars response metadata.
  • Kept the existing tool-result limiter intact and composed both guards at the existing _messages_for_limited_payload(...) chokepoint.
  • Preserved source messages through shallow copied rows and copied structured text blocks.
  • Left bare/no-msg_limit full-session retrieval exact and unchanged.
  • Made full-history loading also recognize clipped-content markers, so edit and regenerate actions recover authoritative text before mutating durable session state.
  • Added regression coverage for user and assistant strings, structured text-block lists, the limit + 1 boundary, source immutability, hidden-block privacy, top-level object semantics, full-load preservation, and edit/regenerate safety.

Why It Matters

One oversized historical renderable row could turn an otherwise bounded tail request into a multi-megabyte initial render and permanently stall WKWebView. The guard keeps paginated display payloads renderable while preserving the complete transcript as the source of truth.

Verification

  • RED on current origin/master: the new regressions failed because paginated user/assistant content remained unbounded.
  • GREEN on this branch after rebasing to current origin/master:
    • Backend payload, frontend action/race, branching, and neighboring regression suite: 219 passed.
  • .venv/bin/python -m py_compile api/routes.py tests/test_session_tail_payload.py passed.
  • git diff --check passed.
  • Ruff passes for the changed test files; pre-existing api/routes.py findings are outside this patch.
  • Static added-line scan found no real credential, shell-injection, eval/exec, or unsafe-deserialization issue. The only secret-pattern match is the synthetic DICT_SECRET privacy-test sentinel.
  • Real patched-response check, without printing message bodies:
    • 54 returned rows, approximately 508 KB serialized response.
    • Largest display row reduced from 5,378,015 to exactly 65,536 characters.
    • Full source retrieval remained exact.
  • Native Hermes Agent 1.7.3 on macOS 15.7.7 loaded the previously blocked 302-message session instead of remaining on Loading conversation....

Risks / Follow-ups

Contract Routing

Task type: focused paginated-session display safety fix.

Touched areas:

  • api/routes.py: copied response shaping for GET /api/session when msg_limit is present.
  • static/sessions.js: authoritative full-history reload when a display row carries _content_truncated.
  • static/ui.js: edit/regenerate actions resolve full text before destructive session operations.
  • tests/test_session_tail_payload.py: observable response, privacy, immutability, and full-load invariants.
  • tests/test_limited_session_content_actions.py: clipped-preview action safety.

Relevant public docs:

  • AGENTS.md
  • CONTRIBUTING.md
  • docs/GUIDELINES.md
  • docs/CONTRACTS.md

Scope boundaries:

  • No persisted session mutation.
  • No export or model-context change.
  • No visual-layout or control-surface change.
  • No new dependency or build step.

Evidence needed before claiming done:

  • RED/GREEN regression proof, neighboring tests, real response-size verification, native-app loading verification, and explicit residual-CPU disclosure.

Release Note

Prevent oversized user or assistant messages from blocking paginated session rendering while preserving complete session history.

Model Used

AI-assisted with OpenAI GPT-5.6 SOL through Hermes Agent. Notable tools: repository tests and Git inspection, native macOS/WebKit process measurement, live API shape checks without message-body disclosure, and independent fail-closed code review.

@Andre-4711
Andre-4711 marked this pull request as draft August 7, 2026 01:33
@Andre-4711
Andre-4711 marked this pull request as ready for review August 7, 2026 01:36
@greptile-apps

greptile-apps Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR bounds oversized renderable content in paginated session responses while preserving authoritative full-history retrieval.

  • Adds a 64 KiB preview ceiling for oversized user and assistant messages.
  • Reloads full message content before edit and regenerate operations.
  • Extends synthesized CLI-session responses to use the same pagination and payload guards.
  • Adds backend and frontend regression coverage for truncation, privacy, immutability, and action safety.

Confidence Score: 4/5

The PR is not yet safe to merge because editing a clipped message can still silently fail when its row is excluded from the virtualized render window after full-history reload.

The previously reported edit failure remains reachable: renderMessages() remounts only the current virtual window, and editMessage() returns without opening an editor when the reloaded target row is not present.

Files Needing Attention: static/ui.js

Important Files Changed

Filename Overview
api/routes.py Adds bounded paginated previews and applies existing display-window semantics to synthesized sessions.
static/sessions.js Extends authoritative full-history loading to detect clipped-content markers.
static/ui.js Reloads authoritative content before edit and regenerate operations.
tests/test_limited_session_content_actions.py Adds focused Node-based regression coverage for full-content reloads before mutations.
tests/test_session_tail_payload.py Covers payload bounds, structured content semantics, source immutability, and synthesized-session behavior.

Sequence Diagram

sequenceDiagram
  participant UI as Browser UI
  participant API as GET /api/session
  participant Store as Session storage
  UI->>API: Paginated request with msg_limit
  API->>Store: Load authoritative transcript
  Store-->>API: Full stored messages
  API->>API: Select display window
  API->>API: Bound oversized renderable rows
  API-->>UI: Paginated previews with truncation markers
  UI->>API: Full-history request before mutation
  API->>Store: Load authoritative transcript
  Store-->>API: Complete messages
  API-->>UI: Exact full history
Loading

Reviews (2): Last reviewed commit: "fix: preserve visible structured preview..." | Re-trigger Greptile

Comment thread static/ui.js
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Thanks @Andre-4711 — this is a real and valuable fix (a ~5.38M-char row bricking native WKWebView on "Loading conversation…" is a genuine brick), and the backend bounding is well-built: Codex verified exact-limit passthrough, limit+1 landing at exactly 65,536, safe handling of empty/None/dict/int, preserved tool/image parts, source immutability, and byte-identical normal rows. The core idea is right.

I gated the current head (Codex adversarial review + full suite). Holding for a revision — there are 4 issues, two verified by Codex on the frontend and two real regressions the suite caught:

1. CORE regression — Claude-Code profile-agnostic detail-load returns 0 messages (api/routes.py)

tests/test_claude_code_profile_agnostic_detail_load.py::test_claude_code_detail_load_survives_named_active_profile now fails: it expects 2 messages, gets 0 (assert 0 == 2). This passes on clean master and fails only on this branch, so the payload restructuring dropped the messages on the Claude-Code detail-load path. This is the same path the PR reshapes, so it needs to preserve that path's output.

2. Regression — session-payload helper signature drift (api/routes.py)

tests/test_session_todo_state_route.py::test_routes_attach_todo_state_from_webui_and_cli_session_paths fails: it asserts the helper signature ('sess','msgs') but the branch now exposes ('raw','_all_msgs') / ('sess','all_msgs'). Either keep the existing signature or update this test to match the new shape (the test encodes the WebUI+CLI attach-path contract, so make sure the rename is intentional and the CLI path still gets its messages).

(Both #1 and #2 pass on clean master, same box — verified regressions, not flakes.)

3. SILENT — editing a truncated message can silently do nothing (static/ui.js:18533)

Reproduced with a 60-row tail at offset 140: after _ensureAllMessagesLoaded() the full 200-row render mounts rows 0–10 and 150–199, excluding target 140, and line 18536 returns → the edit no-ops with no feedback. Fix: retain the clicked row after the reload and use absoluteMsgIdx; don't rerender/requery before opening the editor, or explicitly remount the target.

4. SILENT — oversized messages lose their full read/copy behavior + copy silently lies (static/ui.js:16304)

_content_original_chars is returned by the backend but never consumed by the frontend — no "load full" control is rendered, and copyMsg() (:8510) copies the 64-KiB preview + truncation notice while reporting a successful full copy. So a user copying a truncated message silently gets the preview, not their content. Fix: render a safe "Truncated — N chars / load full / copy full" action and resolve the absolute message from the bare/full-detail endpoint (no wholesale transcript rerender).

Net: the backend cap is solid; the gaps are (a) the detail-load path must still return its messages (#1/#2), and (b) the frontend needs to give the user a way back to the full content and not silently truncate edit/copy (#3/#4). Since a bounded message is only safe if the full content is still reachable, #4 is the load-bearing one. Ping me when it's re-pushed and I'll re-gate the exact head.

Gate: Codex (adversarial frontend/boundary reproduction) + full suite (13,954 passed, 2 failed — both verified as branch regressions against clean master on the same box). 26 focused PR tests pass; the failures are in existing session-route tests the restructuring touched.

@nesquena-hermes nesquena-hermes added changes-requested Maintainer left detailed feedback requesting changes; PR is waiting on author to address size:L Large PR (>10 files or >250 LOC) labels Aug 7, 2026

@nesquena-hermes nesquena-hermes left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @Andre-4711 — bounding oversized paginated message content is a real need, and the core byte-math is correct (persisted messages are never mutated, no-msg_limit retrieval stays canonical, exactly 65,536 chars unbounded while 65,537 reduces to 65,536, ordinary/synthesized pagination agree, 26 focused tests pass). But the gate found this is the same defect class as #7040 — the bounded preview leaks into the canonical-action paths, plus a render-safety cut bug. Seven to fix:

Must-fix (CORE) — oversized messages aren't recoverable in the transcript UI

static/ui.js:16935. The renderer creates no expand/full-content control for _content_truncated; the only marker consumers are the loader and edit path, and session-jump buttons are disabled by default. Add a per-message full-content action that fetches the no-msg_limit canonical value and displays/downloads it.

Must-fix (SILENT) — Copy copies the 64 KiB preview + truncation notice

static/ui.js:8831. copyMsg() reads only stale data-raw-text and never triggers authoritative loading. Capture session ID + absolute message index, load canonical content when marked (guard session switches), and copy the full normalized message.

Must-fix (SILENT) — Listen/TTS speaks only the preview + truncation notice

static/ui.js:9009. speakMessage() has the same stale data-raw-text dependency. Resolve the full message via the same race-safe absolute-index helper before stripping Markdown and starting TTS.

Must-fix (SILENT) — Markdown download exports bounded previews

static/boot.js:2101. transcript() serializes current S.messages (unlike JSON/HTML exports, which use the canonical server endpoint), so the Markdown contains the preview and omits the full value. Await authoritative loading with a session guard before building the blob, or add a canonical server-side Markdown export.

Must-fix (CORE render-safety) — slicing can create DOM-breaking bounded content

api/routes.py:8895. Both string and structured-text paths cut without respecting HTML comments, safe tags, fences, or tables. Verified: a valid cutoff-crossing <!-- … --> becomes an unclosed <!-- in actual renderMd() output → the HTML tokenizer eats the footer/actions. Build previews at safe structural boundaries, or neutralize unmatched comments/tags before rendering. Cover a cutoff-crossing actual-renderMd() case.

Also

  • static/ui.js:19165 — preserve normalized edit text for ordinary messages and normalize authoritative text after a truncated-message reload.
  • static/sessions.js:3864 — remove the global "any truncated row" reload condition; scope content recovery to the specific message the caller requested.

The pattern (same as #7040): keep the persisted/canonical path authoritative, never let the bounded preview reach Copy/TTS/export/edit, resolve those by message identity, and cut only at safe structural boundaries. Re-push and I'll re-gate.

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

Labels

changes-requested Maintainer left detailed feedback requesting changes; PR is waiting on author to address size:L Large PR (>10 files or >250 LOC)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants