Skip to content

fix(ui): prevent browser OOM during long agent interactions#4661

Open
holden093 wants to merge 6 commits into
odysseus-dev:devfrom
holden093:fix/browser-memory-leak
Open

fix(ui): prevent browser OOM during long agent interactions#4661
holden093 wants to merge 6 commits into
odysseus-dev:devfrom
holden093:fix/browser-memory-leak

Conversation

@holden093

@holden093 holden093 commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Long agent interactions (many tool calls, large thinking blocks, document generation) caused the browser tab to consume multiple GB of RAM and eventually OOM-crash or freeze. This PR addresses the root causes with lightweight, non-destructive fixes that keep all content fully visible.

Target branch

  • This PR targets dev, not main. All PRs land in dev; main is curated by the maintainer at each release.

Linked Issue

Fixes #4644

Type of Change

  • Bug fix (non-breaking — fixes a confirmed issue)

Checklist

  • I searched open issues and open PRs — this is not a duplicate.
  • This PR targets dev
  • My changes are limited to the scope described above — no unrelated refactors or whitespace changes mixed in.
  • I actually ran the app and verified the change works end-to-end.

How to Test

  1. Start an agent interaction with many tool calls. The thinking block streams as plain text without freezing, then renders as rich markdown when it closes — fully visible throughout.
  2. Let the agent run 30+ tool calls. A "Show N older messages" bar appears at the top of the chat. Click it — older messages load from the server inline. Click again until all are loaded.
  3. Switch to another session and back — long sessions also get the load-older bar after history render.
  4. Ask the agent to create a large document. The editor fills in real-time without lag. Line numbers and syntax highlighting apply once on completion.
  5. Check DevTools → Network — /api/history/:id includes ?limit=400 and returns a "total" field.

Visual / UI changes — REQUIRED if you touched anything that renders

  • Screenshot or short clip of the change in the running app, attached below.
  • Style match: the change uses Odysseus's existing visual language:
    • Reuses existing CSS variables (--accent, --fg, --border) and classes (.msg, .msg-ai, .agent-thread).
    • No Unicode emoji in UI or code.
    • Dark theme is the default; light-mode inherits through the existing theme system.
  • No new component patterns. The load-older bar is a single <div> with dashed accent border. No parallel styling or new widgets.
  • I am not an LLM agent submitting a bulk PR.

Screenshots / clips

Schermata del 2026-06-20 22-16-49

@github-actions github-actions Bot added the ready for review Description complete — ready for maintainer review label Jun 20, 2026
@holden093
holden093 force-pushed the fix/browser-memory-leak branch 2 times, most recently from f926124 to ce096a7 Compare June 20, 2026 21:20
jdmanring pushed a commit to jdmanring/odysseus that referenced this pull request Jun 21, 2026
Three confirmed root causes of 14–18 GB RSS in long agent sessions.
Upstream PR odysseus-dev#4661 addresses two. Research at
docs/fork/memory-explosion-research.md. Test branches:
test/upstream-pr-4661 and test/pr-4661.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jdmanring pushed a commit to jdmanring/odysseus that referenced this pull request Jun 21, 2026
…and complete adaptation plan

_trimChatHistoryDOM() destroys chatHistory.js control elements (sentinel,
histSep, spacer) and bypasses _all[]. _loadOlderMessages() breaks multi-round
agent messages. Safe parts taken as-is; DOM cap replaced with Phase 2
_evictLive() design that routes evicted live messages back through _all[].

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jdmanring pushed a commit to jdmanring/odysseus that referenced this pull request Jun 21, 2026
…e branch plan table

- Branch plan: correct branch names (fix/dom-oom-phase2-guard, not
  fix/dom-oom-live-eviction) and mark both as built/pushed
- Add Attribution section: which parts of our implementation came from
  upstream PR odysseus-dev#4661 (textContent fix, background stream cleanup pattern,
  _evictLive teardown) and what we did not take (trimChatHistoryDOM,
  loadOlderMessages, pagination, doc streaming throttle, CSS)
- Inline attribution in Fix A section (source: upstream PR odysseus-dev#4661)
- Inline attribution in Fix B teardown code block

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jdmanring pushed a commit to jdmanring/odysseus that referenced this pull request Jun 21, 2026
…hread; revert --expose-gc

content-visibility: auto tells Blink to skip layout and paint for off-screen
elements. For a session with 100+ messages, this means only the visible window
triggers full layout work — off-screen messages are deferred until scrolled into
view. Reduces Oilpan memory pressure for the rendered-but-off-screen DOM without
requiring any V8 internals.

contain-intrinsic-size gives the browser a placeholder size estimate so scroll
geometry remains stable while content is deferred:
  .msg              → auto 120px (typical message height)
  .thinking-content → auto 300px (collapsed by default; rarely visible off-screen)
  .agent-thread     → auto 80px  (compact tool timeline entry)

Adapted from upstream PR odysseus-dev#4661 (holden093).

Also reverts --expose-gc + explicit gc() calls (commits e3efb70, cf8c955).
gc() is a V8 debugging API not intended for production; content-visibility is the
correct mechanism to reduce Oilpan pressure for off-screen DOM content.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jdmanring pushed a commit to jdmanring/odysseus that referenced this pull request Jun 21, 2026
Three independent fixes from holden093's PR odysseus-dev#4661 ("fix(ui): prevent browser
OOM during long agent interactions"), applied to develop while the PR is in
review. Will arrive again via the ingest pipeline when odysseus-dev#4661 merges; these
are pre-emptive copies so the app benefits now.

_purgeStaleBackgroundStreams() (chat.js):
  Removes completed/error entries from _backgroundStreams Map at the start
  of each handleChatSubmit(). Without this, accumulated text from finished
  background streams is retained indefinitely in the Map.

History pagination (routes/history_routes.py + sessions.js):
  GET /api/history/:id now accepts ?limit= and ?offset= and returns a
  "total" field. Session load requests ?limit=400 — caps the initial JSON
  payload and DOM render for very long sessions.

streamDocDelta() rAF throttle (document.js):
  Document editor previously updated textarea + code element + line numbers
  on every SSE delta. Now throttled to one DOM write per animation frame.
  streamDocFinalize() cancels any pending frame and flushes the final
  content synchronously before triggering highlighting.

Not taken from PR odysseus-dev#4661: _trimChatHistoryDOM() and _loadOlderMessages() —
incompatible with the chatHistory.js three-phase virtualization system
(destroys sentinel, histSep, and spacer control nodes).

Adapted from upstream PR odysseus-dev#4661 by holden093.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jdmanring pushed a commit to jdmanring/odysseus that referenced this pull request Jun 21, 2026
…t; note pre-applied pieces and ingest exclusions

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@holden093
holden093 force-pushed the fix/browser-memory-leak branch from ce096a7 to ada564f Compare June 22, 2026 19:56

@vdmkenny vdmkenny 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.

This is a strong, well-profiled set of fixes and I want it in. The DOM cap with proper teardown (intervals, spinners, data: images) before removal, the plain-text-during-stream thinking block that kills the per-token O(n^2) markdown re-render, the rAF-throttled doc streaming, and freeing accumulated on bg-stream completion are all real wins. One blocking bug in the pagination, then this is good.

The initial/reload pagination returns the oldest messages, not the most-recent. session.history is chronological (oldest first), and the slice is:

if limit is not None:
    history_dict = history_dict[offset:offset + limit]   # offset defaults to 0

So sessions.js calling /api/history/{id}?limit=400 (offset 0) gets history_dict[0:400] = the oldest 400 messages, even though the comment there says "400 most-recent" and the code then scrolls to bottom. On a long session you reload and see ancient history, and since there is only a "load older" path (no "load newer"), the recent conversation is unreachable.

Fix: for the initial load, slice from the end, e.g.

if limit is not None:
    start = offset if offset else max(0, total_count - limit)
    history_dict = history_dict[start:start + limit]

(or return history_dict[-limit:] when no offset is given), and make the "load older" offset math (offset = _unloadedMsgCount - 50 in _loadOlderMessages) consistent with that orientation so paging backward from the most-recent window lines up.

A quick test on the backend slice would lock it: a session with N messages and ?limit=k returns the last k (and offset pages backward correctly).

Everything else looks good; just the slice direction.

jdmanring pushed a commit to jdmanring/odysseus that referenced this pull request Jun 25, 2026
- perf-agent-gc-catchup PR draft: position idle-GC against upstream odysseus-dev#4644
  (OOM issue) and odysseus-dev#4661 (DOM-windowing PR) — complementary, not overlapping;
  fix the empty 'Linked Issue' to 'Relates to odysseus-dev#4644'.
- perf-audit A1: correct the overstated 'headline gallery win' framing —
  upstream odysseus-dev#4852 (server-side thumbnails, cf. Immich) is the real gallery-grid
  fix; client-side lazy/async doesn't help the grid. #98's value is the
  document page-stack.
- ai-policy: add mandatory 'search upstream prior art before staging' rule so
  contributions reference/coordinate with existing issues/PRs/ROADMAP instead
  of landing as duplicates or next to better proposals.

Fork issues #97/#98/#99/#100/#101/#102 commented with their upstream
relationships (odysseus-dev#4644/odysseus-dev#4661, odysseus-dev#4852, odysseus-dev#2125, ROADMAP).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jdmanring pushed a commit to jdmanring/odysseus that referenced this pull request Jun 25, 2026
… drop AI tells

- Correct stale counts: '14 tests' -> 20; '1 commit' -> the 4 commits actually
  on the branch (verified against git log + the test file).
- Remove all em-dashes; reduce bold/italic; plainer senior-developer prose.
- Keep the upstream odysseus-dev#4644/odysseus-dev#4661 reconciliation and the corrected 'Relates to'
  linkage from the prior commit.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jdmanring pushed a commit to jdmanring/odysseus that referenced this pull request Jun 25, 2026
…dysseus-dev#4661

- fix-dom-oom-virtualization: state plainly that it COMPETES with odysseus-dev#4661 on the
  DOM-bounding mechanism (virtualization vs pagination) and needs maintainer
  coordination before filing, rather than implying it is independent.
- fix-dom-oom-streaming-throttle: link odysseus-dev#4644; remove 'our virtualization' and a
  fork-internal 'during ingest' instruction that should not appear in an upstream
  PR body; keep the holden093/odysseus-dev#4661 attribution.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jdmanring pushed a commit to jdmanring/odysseus that referenced this pull request Jun 25, 2026
…dev#4661); fix Branch scaffolding

- fix/dom-oom-virtualization marked HOLD across draft, pr-status, and active-work:
  it competes with open upstream PR odysseus-dev#4661 (full virtualization vs odysseus-dev#4661's pagination
  for the same DOM-bounding problem). Recommend letting odysseus-dev#4661 land, then proposing
  virtualization as an enhancement only if it adds value over pagination.
- Removed the stray 'jdmanring/odysseus:' prefix from 31 drafts' Branch/Base header
  lines (the branch is e.g. 'feat/qt-native-linux-app', not a namespaced path).
- Corrected active-work's stale '94 + 572 tests' to 109 + 11.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jdmanring pushed a commit to jdmanring/odysseus that referenced this pull request Jun 25, 2026
…te alternative to odysseus-dev#4661

Provenance verified (not a takeover): our first commit (31d0bbb, 2026-06-20
01:42 UTC) predates odysseus-dev#4661 being opened (2026-06-20 21:07 UTC) by ~19h, and our
chatHistory.js code does not reference or reuse theirs. Teardown comparison shows
ours additionally releases _streamRenderer, IntersectionObserver (_sObs), and
hljs-defer refs that odysseus-dev#4661's trim leaves attached, plus bidirectional scroll-up;
odysseus-dev#4661 is the smaller patch. Reframed from HOLD/competes to an independently-
developed alternative offered on merits, with the timeline + teardown evidence and
respect for odysseus-dev#4661. Flagged the fork-doc contamination on the branch to remove
before filing.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jdmanring pushed a commit to jdmanring/odysseus that referenced this pull request Jun 25, 2026
…ndependent, teardown adapted from odysseus-dev#4661)

Critical accuracy fix. My earlier 'does not reference or reuse odysseus-dev#4661's code'
claim contradicted the draft's own line 58 and memory-explosion-research.md
(lines 365, 527): the per-node teardown pattern in _evictLive() WAS adapted from
odysseus-dev#4661's _trimChatHistoryDOM(). Reframed across draft, pr-status, and active-work
to the precise truth: the MessageWindow ARCHITECTURE is independent and predates
odysseus-dev#4661 (timeline), but the teardown cleanup was adapted from odysseus-dev#4661 and extended
(StreamRenderer, IntersectionObserver, hljs-defer). Credit odysseus-dev#4661; do not claim
full code-independence.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jdmanring pushed a commit to jdmanring/odysseus that referenced this pull request Jun 25, 2026
…pstream contribution

Six-part plan: (1) precise provenance + attribution (architecture independent,
teardown adapted from odysseus-dev#4661, credited); (2) comparative superiority proven with
a reproducible CDP benchmark (ours vs test/pr-4661 vs baseline) and verified
cited sources; (3) behaviour verification including the known bugs — scroll-down
inverse windowing is wrong, snap-to-bottom breaks across the Thinking-box
grow/shrink/grow transition — plus the Thinking-as-overlay improvement and a
usability sweep; (4) code/test/cleanliness audit; (5) sequencing + exit criteria.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@alteixeira20 alteixeira20 added the bug Something isn't working label Jun 26, 2026
@holden093
holden093 force-pushed the fix/browser-memory-leak branch from ada564f to 3401f0d Compare June 26, 2026 16:27
@holden093

Copy link
Copy Markdown
Contributor Author

Good catch on the pagination direction, @vdmkenny — fixed:

Backend (routes/history_routes.py): when no explicit offset query param is given, the endpoint now slices from the end of the chronological array, so ?limit=400 returns the most-recent 400 messages (matching the comment in sessions.js). Explicit offsets (e.g. from the "load older" pager) are used as-is.

Frontend (chat.js + sessions.js): the initial load now stores data.total so the "load older" pager can compute the correct backward-walking offset: offset = total - 400 + _unloadedMsgCount - 50. Each "load older" click walks 50 messages further back from the most-recent window, and the bar disappears once all offloaded messages are restored.

@alteixeira20 alteixeira20 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.

The pagination direction fix is now described in the follow-up, but this branch still has no regression test for the new routes/history_routes.py contract.

Please add a test proving an initial ?limit=k request returns the latest k chronological messages and that explicit offsets page backward without gaps/duplicates. That pins the requested-change blocker before the broader UI work is re-audited.

@holden093
holden093 force-pushed the fix/browser-memory-leak branch from 3401f0d to 29f8095 Compare June 29, 2026 12:03
holden093 added a commit to holden093/odysseus that referenced this pull request Jun 29, 2026
Add 6 tests for the ?limit / ?offset pagination in routes/history_routes.py
to address the requested-changes blocker from odysseus-dev#4661 review:

- No-offset ?limit=k returns most-recent k messages, not oldest
- Explicit offset returns exact slice from chronological array
- Adjacent explicit-offset windows are disjoint and contiguous
- Most-recent window + explicit-offset backward page are adjacent, no gap
- total excludes hidden messages (compaction summaries)
- No ?limit returns all non-hidden messages
@holden093
holden093 force-pushed the fix/browser-memory-leak branch from 29f8095 to db0946a Compare June 29, 2026 13:37
@holden093

Copy link
Copy Markdown
Contributor Author

@alteixeira20 thanks — regression test added in db0946a.

tests/test_history_pagination.py pins the pagination contract with 6 tests:

  • ?limit=k (no offset) returns the most-recent k messages, not the oldest
  • ?limit=k&offset=j returns the exact slice messages[j:j+k]
  • Adjacent explicit-offset windows (0→3→6→9) are disjoint and contiguous — no gaps or duplicates
  • Most-recent window + explicit backward offset are adjacent (no gap, no overlap)
  • total excludes hidden messages so offset math stays consistent
  • No ?limit returns all non-hidden messages

All 6 pass alongside the existing 9 history tests. Rebased on current upstream/dev.

holden093 added a commit to holden093/odysseus that referenced this pull request Jul 5, 2026
Add 6 tests for the ?limit / ?offset pagination in routes/history_routes.py
to address the requested-changes blocker from odysseus-dev#4661 review:

- No-offset ?limit=k returns most-recent k messages, not oldest
- Explicit offset returns exact slice from chronological array
- Adjacent explicit-offset windows are disjoint and contiguous
- Most-recent window + explicit-offset backward page are adjacent, no gap
- total excludes hidden messages (compaction summaries)
- No ?limit returns all non-hidden messages
@holden093
holden093 force-pushed the fix/browser-memory-leak branch from db0946a to 31e0b4e Compare July 5, 2026 18:41
jdmanring pushed a commit to jdmanring/odysseus that referenced this pull request Jul 7, 2026
7 parallel maintainer reviews + mechanical pass. Findings: 1 provenance failure
(dom-oom-virtualization borrows from odysseus-dev#4661 undisclosed), 3 commit-msg-vs-diff
mismatches (basicsr/css-render-perf/nvidia-nim), real bugs (aria2c remote+unquoted
repo_id, spinner offsetParent, skill IDF-cache staleness, longcat over-broad
except), a mutually-exclusive skill pair, several collision/ordering pairs, and a
systemic reliance on source-grep tests. Large clean senior-grade core remains.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
holden093 added 3 commits July 7, 2026 21:32
Five root causes addressed:

1. Unbounded DOM growth: _trimChatHistoryDOM caps #chat-history at 150
   children. Old messages are offloaded and replaced with a clickable
   'Show N older messages' bar that fetches pages from the server via
   the paginated /api/history endpoint.

2. Thinking-block O(n^2) rendering: during streaming, thinking text
   renders as plain textContent (O(1) per token). A single mdToHtml
   pass happens when the block closes. Full content always visible.

3. Document streaming layout thrash: textarea/code updates throttled
   to requestAnimationFrame. updateLineNumbers (which forces layout
   per line and creates DOM per line) skipped entirely during
   streaming — only runs once on finalize via syncHighlighting.

4. No server-side pagination: GET /api/history/:id accepts ?limit=&offset=
   and returns 'total'. Frontend requests ?limit=400.

5. Background stream memory leak: _purgeStaleBackgroundStreams frees
   accumulated text and abort controllers from completed/error entries.

Bonus: content-visibility:auto on .msg, .agent-thread, .thinking-content
so the browser skips paint/layout for off-screen elements.
…lder paging

The /api/history/{id} endpoint returns messages in chronological order
(oldest first), but the frontend's ?limit=400 call with offset=0 was
returning the oldest 400, not the most-recent 400 as the comment claimed.

- Backend: when no explicit offset query param is given, slice from the
  end so the initial load returns the most-recent messages.
- Frontend: store data.total on initial load and update the "load older"
  offset formula to count backward from the end of the array:
  offset = total - 400 + _unloadedMsgCount - 50.
Add 6 tests for the ?limit / ?offset pagination in routes/history_routes.py
to address the requested-changes blocker from odysseus-dev#4661 review:

- No-offset ?limit=k returns most-recent k messages, not oldest
- Explicit offset returns exact slice from chronological array
- Adjacent explicit-offset windows are disjoint and contiguous
- Most-recent window + explicit-offset backward page are adjacent, no gap
- total excludes hidden messages (compaction summaries)
- No ?limit returns all non-hidden messages
@holden093
holden093 force-pushed the fix/browser-memory-leak branch from 31e0b4e to cc34e5a Compare July 7, 2026 19:32
@touzenesmy

Copy link
Copy Markdown
Contributor

I have been waiting on this PR since I've added a chat virtualizer as a stopgap and I can't wait for it to be finished. Thanks for the hard work guys

jdmanring pushed a commit to jdmanring/odysseus that referenced this pull request Jul 7, 2026
…rdination at odysseus-dev#5090

Verified against primary sources: eviction teardown was written independently;
the only lines resembling odysseus-dev#4661 clear the app's own timer fields (forced by the
shared codebase). odysseus-dev#4661 was superseded by odysseus-dev#5090 (the merged _installHistoryPager
the graft composes with). Retract 'adapted from odysseus-dev#4661'; no in-code attribution.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jdmanring pushed a commit to jdmanring/odysseus that referenced this pull request Jul 7, 2026
…dev#5090) across trackers + audit

Verified via gh + git:
- The history pager is maintainer direct commit 45ee5a7 ('Polish mobile UI...'),
  NOT odysseus-dev#5090. odysseus-dev#5090 (merged, Tal.Yuan) is a route-subpackage refactor only.
- odysseus-dev#4661 is OPEN/unmerged (separate _trimChatHistoryDOM OOM PR) — not superseded.
- The pager is inert on current upstream/dev (== upstream-mirror) via legacy-route
  shadowing; proven by route analysis + empirical endpoint test.

Corrected: recon §2/§7/§8, pr-status.md, active-work.md. Added issue+PR drafts for
the route-shadowing fix (with the no-limit-caller behaviour note) and the eviction
graft. Flagged stale fix-dom-oom-virtualization drafts (MessageWindow-port abandoned)
and #2 closed-prematurely (upstream-candidate, no PR filed).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jdmanring pushed a commit to jdmanring/odysseus that referenced this pull request Jul 8, 2026
…ct tests

Gold-standard audit of the fork's MessageWindow virtualizer against upstream
(odysseus-dev#4644/odysseus-dev#4661/odysseus-dev#4998) and current best practice. Mechanical core (scroll
anchoring, eviction geometry, teardown, race safety) already matched reference;
the systematic weakness was accessibility. Fixes (each guard-tested):

- aria-busy brackets prepend/prune batches (_loadOlder/_loadNewer) so AT
  coalesces churn instead of announcing scroll-up history as new; _clearBusy
  restores the PRIOR value so it composes with the streaming aria-busy chat.js
  toggles on the same log element rather than clearing it mid-stream.
- focus inside an evicted node is moved to the log container, not dumped to
  <body> (_pruneTop/_evictLive/_loadOlder/_loadNewer).
- interactive bottom sentinel gets role=button/tabindex/Enter+Space; decorative
  top sentinel and spacer get aria-hidden.
- corrected the misleading "overflow-anchor:none globally" comment to the real
  invariant (only sentinels/spacer set none; Chromium target).

Removed the dead upstream pager cluster orphaned on the fork by the olderLoader
path (_installHistoryPager/_renderHistoryMessage/_addHistoryMessageWithFullRenderer
+ 3 now-unused helpers) — zero call sites, zero test refs.

Tests: test_history_pagination_contract.py (pagination math incl. limit cap /
offset clamp / has_more_after, hidden-row + inline-base64 stripping through the
paginated branch, and a direct route-shadowing guard); test_chat_history_a11y_js.py
(7 real-Chromium a11y guards); extended the real render-path Playwright test to
seed markdown/fenced-code/image content, closing the markdownModule-regression
class beyond plain strings. 161 chat/history tests green together.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jdmanring pushed a commit to jdmanring/odysseus that referenced this pull request Jul 8, 2026
…scape + audit

Update the decision record and active-work to one current story: the merged
upstream pager still has no bounded-DOM path (supersession holds), but two OPEN
upstream PRs now circle this area — odysseus-dev#4661 (paging-only) and odysseus-dev#4998 (chatVirtualizer
windowing that bounds lag, not RSS) — plus issue odysseus-dev#4644 requesting DOM removal.
Records the honest two-axis verdict (ours wins on memory, odysseus-dev#4998 on simplicity/
state-preservation), the convergence path if odysseus-dev#4998 merges, CONTRIBUTING
constraints (issue-first agent PRs, attach eviction to odysseus-dev#4644), and the gold-
standard audit outcome (a11y fixes, dead-code removal, new tests).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fixes from 3-model review (Deepseek + Gemini 3.1 Pro + GPT-5.6 Sol):

BLOCKERS:
- Remove 666-line dead shim from routes/history_routes.py
  (sys.modules replacement bypassed the entire backend change)
- Fix 6/6 failing pagination tests via temp SQLite DB fixture

CRITICAL:
- Enforce chat token scope in _verify_session_owner
  (non-chat tokens could access session history + mutations)

HIGH:
- Remove duplicate load-older pager, consolidate to scroll pager
- Make document stream identity-safe (capturedId + rAF re-check)
- Bound _backgroundStreams Map (MAX=10 + oldest-eviction)
- Debounce thinking regex processing (200ms, avoids residual O(n^2))
- Configurable MAX_CHAT_DOM_NODES via data-max-dom-nodes attribute
- Clean background-image data URIs in DOM trimmer

Tests: 26/26 pass (6 pagination + 8 scope + 12 existing)
…tests

The new scope check in _verify_session_owner requires API tokens
to carry 'chat' or '*' scope. Two tests in test_session_owner_attribution.py
created api_token=True requests without api_token_scopes, causing 403.

Add api_token_scopes=['chat'] to:
- test_bearer_owner_A_cannot_verify_owner_B_session
- test_owner_can_verify_their_own_session
@holden093

Copy link
Copy Markdown
Contributor Author

Ran a 3-model review (Deepseek → Gemini 3.1 Pro → GPT-5.6 Sol) on this branch. Here's what was fixed:

BLOCKER — dead shim. routes/history_routes.py:17 replaced itself in sys.modules, so all 600+ lines of backend changes were never executed. Removed dead code — canonical module at routes/history/history_routes.py was already correct.

CRITICAL — API token scope bypass. _verify_session_owner checked ownership but not api_token_scopes. A documents:read token could access chat history. Added a chat/* scope gate matching codex_routes.py.

HIGH — duplicate pager, document stream race, unbounded background streams. Removed the conflict between the new 400-message pager and the existing 8/24 scroll pager. Added capturedId re-checks in streamDocDelta/streamDocFinalize to prevent wrong-document writes. Capped _backgroundStreams at 10 with oldest-eviction. Debounced thinking regex to 200ms to eliminate residual O(n²).

Tests: 62/62 pass (6 pagination, 8 token scope, 48 existing). Pagination tests now hit a real temp SQLite DB instead of crashing on missing tables.

Pre-existing issues found in files this PR doesn't touch (duplicate compact route, mutation serialization) are already tracked in #2644 and #2654.

jdmanring pushed a commit to jdmanring/odysseus that referenced this pull request Jul 18, 2026
…ysseus-dev#4661

Adds an in-code credit at the borrow site so a reviewer reading the diff (not
just the PR body) sees the provenance. The teardown timer-clearing idiom is
adapted from odysseus-dev#4661 (holden093)'s _trimChatHistoryDOM() and extended here.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jdmanring pushed a commit to jdmanring/odysseus that referenced this pull request Jul 18, 2026
…ce evidence

Part 1 exit-check re-run found the plan mandating 'adapted from odysseus-dev#4661,
credited' while the draft header retracted exactly that claim. Resolved
with git author-date evidence: the architecture was AUTHORED from
2026-06-11 -- nine days before odysseus-dev#4661 opened, not the recorded 19 hours
(that figure was a rebase commit-date) -- and the _evictLive teardown
was authored ~5h after odysseus-dev#4661 opened, so timeline proves nothing either
way; its only overlap is clearing the app's own timer handles (forced
idiom) and odysseus-dev#4661's mechanism was never used. The resolved framing
claims neither adaptation nor proven teardown-independence: facts only,
odysseus-dev#4661 acknowledged as parallel work. Plan 1.1-1.3, draft, pr-status,
active-work, and the research doc (history-preserving correction
banner) now carry one consistent story.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jdmanring pushed a commit to jdmanring/odysseus that referenced this pull request Jul 18, 2026
… Part 2's missing comparison)

The bench compared naive/odysseus-dev#4998/evict/hybrid but never odysseus-dev#4661 itself --
the PR this contribution will be filed next to. Vendored faithfully
from 27f35e1 (constants, removal loop, teardown, offset arithmetic all
the PR's own; renderer/fetch/session adapted for the harness, marked).

The arm's reload is CLICK-driven, so the scroll excursions legitimately
stall for it (cells withheld, self-describing); its own click path is
measured via loadOlderAll (wall time + clicks + post-reload DOM state).
First smoke (n=500): steady-state DOM IS bounded (~150 children /
nodes_loaded 1188) -- falsifying the record's 'odysseus-dev#4661 misses the memory
bound' characterization, to be corrected with the full run -- while the
click-reload path restores everything with nothing evicting behind it
(nodes_peak 3991) until the next message re-trims. 5 guard tests pin
vendor faithfulness.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jdmanring pushed a commit to jdmanring/odysseus that referenced this pull request Jul 18, 2026
…ory, stated as fact, everywhere

Plan Part 1.2 is the canonical framing: independent architecture
authored nine days before odysseus-dev#4661 opened; the per-node teardown described
by what it does; odysseus-dev#4661 acknowledged as parallel work on the same
problem. No adaptation claim, no attribution, no provenance argument --
in the draft, pr-status, active-work, or the research doc. The
streaming-throttle branch's separate adaptation credits stand.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jdmanring pushed a commit to jdmanring/odysseus that referenced this pull request Jul 18, 2026
Adds the vendored odysseus-dev#4661 arm to the published artifact (naive, detach,
evict, hybrid, trim4661 x n=250/1000/2000/5000, 5 kept repeats).
trim4661 steady-state DOM is bounded (~1.19k nodes at every n); its
click-reload path restores the entire history with no re-trim until the
next message (nodes_peak 39k, USS 122.9 MB at n=5000, within spread of
naive). Deep scroll-back cells for hybrid/trim4661 remain withheld with
self-describing reasons (excursion shortfall).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jdmanring pushed a commit to jdmanring/odysseus that referenced this pull request Jul 18, 2026
…ed truth; add 2.1 matrix

The decision record and active-work described odysseus-dev#4661 as paging-only /
missing the memory bound. The vendored-arm measurement corrects this:
its _trimChatHistoryDOM bounds the steady state (~1.19k nodes at every
n), but the click-reload path restores the full history with no re-trim
until the next message (n=5000: 39k nodes / 122.9 MB USS, within spread
of unpatched) and loses scroll position. The unmet-requirement claim is
now stated precisely: no upstream work holds the bound *while reading
old history*.

Adds the plan 2.1 comparison matrix (ours vs odysseus-dev#4661 vs baseline, from
the published artifact) to the PR draft, and marks plan 2.1/2.2 done
with the disclosed unfavourable measurements noted.

Suite: pytest-exit:0 (5457 passed, 7 skipped).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jdmanring pushed a commit to jdmanring/odysseus that referenced this pull request Jul 18, 2026
…rrected to measured

The 1.3 grep found one nonconforming phrase — the plan's own closing line
said 'and credit'; corrected to the Part-1.2 framing. The draft's review-size
claim (~873 lines) was stale and understated the branch: measured
~1,432 source insertions across 5 files plus ~2,202 test lines
(git diff --shortstat upstream-mirror...fix/dom-oom-virtualization);
odysseus-dev#4661's vendored trim is 142 lines. Matrix row and prose updated.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@gprocunier

Copy link
Copy Markdown

Digging into this more deeply myself after confirming the bug on my system, I tested a selective adaptation against a real long-running agent workload and a synthetic browser case with 52 agent rounds, 156 tool nodes, and 100,000 characters of thinking content. The adapted version settled at approximately 9,200 DOM nodes and 18.6 MB of JavaScript heap with no page errors.

The root-cause analysis here is directionally correct, particularly around repeated Markdown rendering, document-stream layout work, retained background payloads, and off-screen rendering. That testing did identify several correctness issues worth addressing before merge.

1. Flush the current thinking text synchronously

The 200 ms debounce reduces Markdown work, but the current close path clears the pending timer and finalizes from the existing DOM text. Any text received since the previous timer tick can therefore be lost.

This affects more than the normal closing-tag path. Thinking may terminate through:

  • a closing </think> boundary
  • [DONE]
  • tool_start
  • agent_step
  • an error or interrupted stream

Keep the latest raw roundText independently of the DOM and use one synchronous flush helper on every terminal transition:

function flushLiveThinking(finalRender) {
  latestThinkingRoundText = roundText;

  if (thinkingRenderTimer) {
    clearTimeout(thinkingRenderTimer);
    thinkingRenderTimer = null;
  }

  renderLiveThinking(latestThinkingRoundText, { finalRender });
}

During streaming, renderLiveThinking should use textContent. During finalization, it should calculate the final token count and perform exactly one mdToHtml render.

Useful regressions would cover:

  • a closing tag arriving less than 200 ms after the final text delta
  • [DONE] closing an unclosed thinking block
  • tool_start closing thinking
  • agent_step beginning a new round while a debounce is pending

This changes the operation from per-delta Markdown parsing to bounded-frequency linear text updates. It is a substantial improvement, but not strictly O(1), since assigning the growing string still scales with its length.

2. Do not abort active background streams to enforce the cap

The current cardinality guard evicts the oldest map entry and calls its AbortController. If more than ten legitimate runs are active, this converts a browser-memory guard into cancellation of user work.

The safe distinction is terminal versus running entries:

if (entry.status === 'completed' || entry.status === 'error') {
  entry.accumulated = '';
  entry.sourcesHtml = '';
  entry.findingsData = null;
  entry.metrics = null;
  entry.docContent = '';
  entry.abortCtrl = null;
}

A cardinality cap may remove old terminal entries after releasing their payloads, but it should never abort or delete a running entry. If concurrent running streams need a limit, enforce it before starting or detaching another run and surface that limit explicitly.

A regression should create more than ten running entries and assert that none of their abort controllers are invoked.

3. Make document finalization unconditional

The final document flush currently depends on both the textarea and highlighted-code element existing. These elements can be independently absent while switching document modes or rebuilding the panel.

Update them independently:

if (textarea) textarea.value = finalContent;
if (codeEl) codeEl.textContent = finalContent + '\n';

Then perform one final highlighting and line-number pass. Cancel the pending animation frame first and verify the captured document ID before writing.

4. Reconsider raw-child DOM trimming

The existing history pager already limits restored history. _trimChatHistoryDOM() additionally removes raw #chat-history children, but those children do not map cleanly to persisted messages. Agent threads, continuation bubbles, tool timelines, and live status elements may be separate siblings.

In its current form, the trimmer can remove part of a live or partially persisted agent timeline. Re-entering the session may reconstruct it, but the active view has already lost information and associated lifecycle state.

I would either omit this part initially and rely on history paging plus content-visibility, or trim only complete persisted conversation groups while explicitly protecting:

  • .streaming messages
  • .agent-thread.streaming
  • the current holder and round holder
  • pending tool nodes
  • document-writing indicators
  • unpersisted messages

A browser test should exercise trimming during a multi-round tool run and verify that the active timeline remains complete and interactive.

The main optimization approach is sound. With synchronous thinking flushes, terminal-only background cleanup, independent document finalization, and safer message-group trimming, this should address the browser OOM without introducing lost reasoning text, cancelled work, or damaged tool timelines.

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

Labels

bug Something isn't working ready for review Description complete — ready for maintainer review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(ui): browser tab OOM and freeze during long agent interactions

5 participants