mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-12 13:52:15 +00:00
tests: pin ink engine in _make_tui_argv npm-bootstrap tests (post-merge semantic fix)
Main's rewritten test_tui_npm_install.py tests call _make_tui_argv expecting the Ink/npm flow unconditionally; with the dual-engine dispatch merged in, _resolve_tui_engine() auto-selects opentui whenever ui-opentui/dist is built in the repo, routing the call away from the path under test (first subprocess became 'node --version' instead of 'npm run build'). Pin the engine to ink via an autouse fixture, mirroring the existing pinning precedent in test_tui_resume_flow.py.
This commit is contained in:
alt-glitch
parent
ab37440ce6
commit
e1067dbbe5
756 changed files with 79874 additions and 19585 deletions
144
docs/design/profile-builder.md
Normal file
144
docs/design/profile-builder.md
Normal file
|
|
@ -0,0 +1,144 @@
|
|||
# Profile Builder — Dashboard-Native, Full-Featured Profile Creation
|
||||
|
||||
Status: design proposal (not yet implemented)
|
||||
Author: drafted for Teknium
|
||||
Supersedes: PR #31781 (prompt_toolkit `hermes profile wizard`)
|
||||
|
||||
## Why this, not the CLI wizard
|
||||
|
||||
PR #31781 added a keyboard-driven `hermes profile wizard` in the terminal.
|
||||
The decision is to **not** build the profile-creation experience in the CLI.
|
||||
The dashboard already owns mature, separate pages for every element a profile
|
||||
needs, and a profile is just a HERMES_HOME directory — so the dashboard is the
|
||||
right home for a full-featured builder, and it can reuse everything that
|
||||
already exists.
|
||||
|
||||
A profile = a full `~/.hermes/profiles/<name>/` directory with its own:
|
||||
- `config.yaml` — holds `model`/`provider`, `mcp_servers`, enabled skills
|
||||
- `skills/` — physical SKILL.md files (built-in seed + optional + hub installs)
|
||||
- `.env` — secrets
|
||||
- `SOUL.md` / `USER.md` — identity
|
||||
|
||||
So per-profile scoping of Model, MCPs, and Skills is **native** — no data-model
|
||||
change needed. The gap is purely UX: creation today is a thin modal
|
||||
(name + clone + model + description), and you can only compose skills/MCPs
|
||||
*after* the profile exists, by visiting other pages and remembering to scope
|
||||
them.
|
||||
|
||||
## What already exists (reuse, don't rebuild)
|
||||
|
||||
| Element | Existing page | Existing API | Profile-scopable? |
|
||||
|---|---|---|---|
|
||||
| Name / Description | ProfilesPage create modal | `POST /api/profiles` (`create_profile`) | yes (args) |
|
||||
| Model + Provider | ModelsPage | `_write_profile_model(profile_dir, …)` | yes — HERMES_HOME override, already wired into create endpoint |
|
||||
| MCPs | McpPage | `mcp_config._save_mcp_server` + `/api/mcp/catalog` | yes — wrap with HERMES_HOME override |
|
||||
| Skills (built-in/optional) | SkillsPage | `GET /api/skills`, `/api/skills/toggle` | yes — config write |
|
||||
| Skills (hub) | SkillsPage | `/api/skills/hub/search`, `/api/skills/hub/install` | **only via subprocess** — see seam #1 |
|
||||
|
||||
## Two architectural seams found while grounding this design
|
||||
|
||||
These are load-bearing — they change the implementation, not just the polish.
|
||||
|
||||
### Seam #1 — hub-skill install cannot use the HERMES_HOME override
|
||||
|
||||
`tools/skills_hub.py` binds `SKILLS_DIR = HERMES_HOME / "skills"` at **module
|
||||
import time**. The context-local `set_hermes_home_override()` swap (which makes
|
||||
`_write_profile_model` and the MCP write land in the target profile) does NOT
|
||||
retroactively rebind that already-imported module global. So a data-layer wrap
|
||||
of hub install would write into the dashboard's *own* active profile, not the
|
||||
new one.
|
||||
|
||||
The correct mechanism is the existing subprocess path: `_spawn_hermes_action`
|
||||
runs `python -m hermes_cli.main <subcommand>`, and `_apply_profile_override()`
|
||||
re-reads `sys.argv` at import in the fresh child. Prepend `-p <profile>`:
|
||||
|
||||
```python
|
||||
_spawn_hermes_action(["-p", profile, "skills", "install", identifier], "skills-install")
|
||||
```
|
||||
|
||||
A fresh subprocess re-imports `skills_hub` with the profile's HERMES_HOME bound
|
||||
from the start, so `SKILLS_DIR` resolves to `<profile>/skills/`. Correct by
|
||||
construction.
|
||||
|
||||
### Seam #2 — hub installs are async, so create cannot be fully atomic
|
||||
|
||||
Built-in/optional skill enabling and MCP writes are **synchronous config ops**
|
||||
and can be part of the create call. Hub installs are long-running git fetches
|
||||
spawned detached (`_spawn_hermes_action` returns a PID immediately). So the
|
||||
create flow is:
|
||||
|
||||
1. `create_profile()` — make the dir (synchronous)
|
||||
2. write model (synchronous, HERMES_HOME override)
|
||||
3. write selected MCP servers (synchronous, HERMES_HOME override)
|
||||
4. seed/enable selected built-in + optional skills (synchronous)
|
||||
5. spawn `hermes -p <profile> skills install <id>` per hub skill (async, returns PIDs)
|
||||
|
||||
Steps 1–4 commit before the response; step 5 returns a list of action PIDs the
|
||||
UI polls (same pattern as today's SkillsPage hub install). The builder's
|
||||
"Review → Create" returns `{ok, name, path, hub_installs: [{id, pid}]}` and the
|
||||
final screen shows live install progress for the hub skills.
|
||||
|
||||
## Proposed backend change (small, follows existing patterns)
|
||||
|
||||
Extend `ProfileCreate` and the create endpoint — no new endpoints, no rewrite:
|
||||
|
||||
```python
|
||||
class ProfileCreate(BaseModel):
|
||||
name: str
|
||||
clone_from_default: bool = False
|
||||
clone_all: bool = False
|
||||
no_skills: bool = False
|
||||
description: Optional[str] = None
|
||||
provider: Optional[str] = None
|
||||
model: Optional[str] = None
|
||||
# NEW — all optional, all best-effort post-create (profile already exists)
|
||||
mcp_servers: List[MCPServerCreate] = [] # synchronous, HERMES_HOME override
|
||||
builtin_skills: List[str] = [] # synchronous enable/seed
|
||||
hub_skills: List[str] = [] # async spawn, returns PIDs
|
||||
```
|
||||
|
||||
The endpoint already does best-effort post-create steps (`seed_profile_skills`,
|
||||
`_write_profile_model`). Add two more best-effort blocks (MCP write, hub-skill
|
||||
spawn) in the same style — a failure in any of them must not 500 the create,
|
||||
since the profile dir already exists and the user can fix it from the relevant
|
||||
page afterward. Mirror `_write_profile_model`'s HERMES_HOME-override helper for
|
||||
the MCP write (`_write_profile_mcp_servers(profile_dir, servers)`).
|
||||
|
||||
## Proposed frontend — dedicated builder page `/profiles/new`
|
||||
|
||||
A full page (not the cramped modal), stepped, each step reusing the existing
|
||||
page's component + API, targeted at the new profile:
|
||||
|
||||
```
|
||||
① Identity Name + Description (+ optional clone-from existing profile)
|
||||
② Model Provider + model picker (reuse ModelsPage picker)
|
||||
③ Skills Tabs: Built-in · Optional · Hub-search
|
||||
multi-select; "Start from default bundle" preset button
|
||||
④ MCPs Tabs: Catalog browse · Manual add (reuse McpPage form)
|
||||
⑤ Review Blueprint preview → Create
|
||||
→ progress screen for async hub installs
|
||||
```
|
||||
|
||||
Nothing writes to disk until ⑤.
|
||||
|
||||
## Open product decisions (need Teknium)
|
||||
|
||||
1. **Skills seeding default.** Fresh profiles auto-seed the default bundle
|
||||
today. In the builder, should the skill step **replace** the bundle (pick
|
||||
exactly what you want; offer a "start from default bundle" preset) or
|
||||
**augment** it? Recommendation: replace + preset button.
|
||||
|
||||
2. **Page vs richer modal.** Dedicated `/profiles/new` page (room to grow:
|
||||
SOUL editing, multi-agent fleets later) vs a bigger create modal on
|
||||
ProfilesPage. Recommendation: dedicated page — matches "full-featured / way
|
||||
more options."
|
||||
|
||||
## Verification plan (when built)
|
||||
|
||||
- Backend E2E with isolated HERMES_HOME: POST a full create body
|
||||
(name + model + 2 MCPs + 3 builtin skills + 1 hub skill), assert the new
|
||||
profile dir has the model in config.yaml, both MCP servers in config.yaml,
|
||||
the builtin skills enabled, and a spawned PID for the hub skill. Negative:
|
||||
a bad MCP entry must not 500 the create.
|
||||
- `cd web && npm run build` (no JS test suite in web/).
|
||||
- Targeted: `pytest tests/<web_server profile tests> -k profile_create`.
|
||||
|
|
@ -1,57 +0,0 @@
|
|||
# OpenTUI env flags — the consolidated ledger
|
||||
|
||||
Every environment variable the OpenTUI TUI reads (grep-verified 2026-06-12),
|
||||
classified by who should ever touch it. The design rule shipped with this doc:
|
||||
**regular users see zero diagnostic surface by default; one master switch
|
||||
(`HERMES_TUI_DIAGNOSTICS=1`) turns all of it on when needed.**
|
||||
|
||||
## 1. The master switch
|
||||
|
||||
| var | default | effect |
|
||||
|---|---|---|
|
||||
| `HERMES_TUI_DIAGNOSTICS` | **off** | Enables the diagnostic slash commands (`/mem`, `/heapdump`). While off they're hidden from `/help` (client-side filter) and invoking them prints the enable hint rather than executing. They never appear in slash *completion* in either state — completion is gateway-driven and these are client-only commands the gateway doesn't know (an adversarial review confirmed there's no bypass path; if a SERVER command named `mem`/`heapdump` is ever added it must be gated gateway-side too — the client gate would shadow but not hide it). Also flips the *default* of `HERMES_TUI_WINDOW_STATS` to on. Not a secret — support flows are "relaunch with `HERMES_TUI_DIAGNOSTICS=1`". |
|
||||
|
||||
## 2. User-facing configuration (fine to document publicly)
|
||||
|
||||
| var | default | effect |
|
||||
|---|---|---|
|
||||
| `HERMES_TUI_ENGINE` | auto (`opentui` if Node≥26.3 + built, else `ink`) | Engine pick; also `display.tui_engine` in config.yaml. |
|
||||
| `HERMES_TUI_MOUSE` | on (launcher sets it) | Mouse support (wheel scroll, selection, click-to-expand). **Glitch verdict 2026-06-12: leave as-is — always on, no realistic reason to disable; treat as plumbing, don't document it user-facing.** |
|
||||
| `HERMES_TUI_MAX_MESSAGES` | ceiling | Scrollback rows kept in the TUI. Can LOWER the ceiling, never raise: 3000 with windowing, 1000 with windowing off (handle-table safety). |
|
||||
| `HERMES_TUI_TOOL_OUTPUT_LINES` | unlimited | Cap expanded tool-output lines (set a number to restore a cap). |
|
||||
| `HERMES_TUI_COMPOSER_ROWS` | default rows | Composer height. |
|
||||
|
||||
## 3. Escape hatches & tuning (dev-facing, individually settable)
|
||||
|
||||
| var | default | effect |
|
||||
|---|---|---|
|
||||
| `HERMES_TUI_WINDOWING` | **on** | `0` = bit-exact pre-windowing renderer (every row mounts; cap clamps back to 1000). The A/B + regression escape hatch. |
|
||||
| `HERMES_TUI_WINDOW_IDLE_MS` | ~1000 | Idle-measure pulse cadence (the spacer-exactness march). Test knob. |
|
||||
| `HERMES_TUI_WINDOW_STATS` | = `HERMES_TUI_DIAGNOSTICS` | Exposes live/peak mounted-row counters (`globalThis.__hermesTuiWindowStats`) for bench/live-attach reads. |
|
||||
| `HERMES_TUI_LOG_LEVEL` / `HERMES_TUI_LOG_FILE` | engine defaults | Logging verbosity/destination (`/logs` reads the ring buffer regardless). Deliberately independent of the master switch — support often wants logs without the full diag surface. |
|
||||
|
||||
## 4. Internal plumbing (set by the launcher/bench/tests — humans never set these)
|
||||
|
||||
| var | set by | effect |
|
||||
|---|---|---|
|
||||
| `HERMES_PYTHON`, `HERMES_PYTHON_SRC_ROOT`, `HERMES_CWD` | launcher / bench | Which gateway python + repo root + cwd the TUI spawns against (the bench's fake-gateway seam). |
|
||||
| `HERMES_TUI_ACTIVE_SESSION_FILE` | launcher/bench | Session handoff file. |
|
||||
| `HERMES_TUI_RESUME`, `HERMES_TUI_PROMPT`, `HERMES_TUI_FAKE` | launcher/tests | Resume-at-boot, seeded prompt, fake-mode. |
|
||||
| `HERMES_TUI_RPC_TIMEOUT_MS`, `HERMES_TUI_STARTUP_TIMEOUT_MS` | tests/CI | Protocol timeouts. |
|
||||
| (`ui-tui` only) `HERMES_TUI_MEMSAMPLE_FD/MS` | bench | Ink fd-3 node sampler. |
|
||||
|
||||
## How the pieces compose (the support script)
|
||||
|
||||
- Regular user, normal day: zero flags, zero diagnostic commands visible.
|
||||
- "My TUI feels heavy" support flow: `HERMES_TUI_DIAGNOSTICS=1 hermes` → `/mem`
|
||||
for the live numbers, `/heapdump` for a snapshot to attach, window stats
|
||||
exposed for `bench/live-attach.sh <pid>` to read.
|
||||
- Developer profiling: same master switch + the individual knobs
|
||||
(`HERMES_TUI_WINDOWING=0` A/B, `WINDOW_IDLE_MS` tuning) as needed.
|
||||
- Anything in section 4 appearing in a user-facing doc is a bug.
|
||||
|
||||
Gating implementation: `logic/env.ts` (`diagnosticsEnabled()`),
|
||||
`logic/slash.ts` (`DIAGNOSTIC_COMMANDS` — dispatch hint, help + completion
|
||||
filtering), `view/transcript.tsx` (stats default). Tests:
|
||||
`slash.test.ts` (gating both states), `utilityCommands.test.ts` (commands
|
||||
themselves, gate enabled suite-wide).
|
||||
|
|
@ -1,207 +0,0 @@
|
|||
# How the OpenTUI transcript got from 686MB to ~300MB — the full story
|
||||
|
||||
*For: glitch. Branch: `feat/opentui-memory-window`. Everything here is measured,
|
||||
not vibes; every number has a result JSON in `bench/results/`.*
|
||||
|
||||
---
|
||||
|
||||
## 1. The cast of characters (the primitives, bottom-up)
|
||||
|
||||
To understand where the memory went, you need to know who's holding it. Six
|
||||
layers, from the screen up:
|
||||
|
||||
**The terminal grid.** Your terminal is a spreadsheet of character cells.
|
||||
Nobody pays per-message here — tmux holds ~5MB flat no matter how long the
|
||||
session is (we measured). The terminal is never the problem.
|
||||
|
||||
**The OpenTUI native renderer (Zig).** A compiled library that owns the
|
||||
"frame buffer" — the grid of cells about to be painted. Every piece of text the
|
||||
TUI shows lives in a native **TextBuffer** (the characters + their colors),
|
||||
viewed through a **TextBufferView**, styled by a **SyntaxStyle**. Each of those
|
||||
is a **native handle** — a ticket into one global table that has only **65,535
|
||||
slots, total, ever** (16-bit indices — like a coat check with 65k hooks).
|
||||
Destroying a renderable returns its tickets, so the constraint is not "how much
|
||||
have you ever created" but **"how much is alive right now."**
|
||||
|
||||
**Renderables.** OpenTUI's UI objects — `<text>`, `<box>`, `<markdown>`,
|
||||
`<code>`, `<scrollbox>`. One transcript row (a message with its tool calls,
|
||||
markdown, code blocks, copy chips) is a *tree* of these: **~16 text renderables
|
||||
≈ 47 native handles ≈ ~250–340KB of RSS, per row.** This is the number that
|
||||
drives everything. 1,400 mounted rows × 47 handles = table full = the crash we
|
||||
root-caused last week.
|
||||
|
||||
**Yoga (the layout engine, WASM).** Every renderable also has a Yoga node —
|
||||
Yoga is the flexbox calculator that decides where boxes go. OpenTUI ships it
|
||||
compiled to **WebAssembly**, and WASM has a brutal property: its memory can
|
||||
**grow but never shrink** back to the OS. So the peak number of
|
||||
*simultaneously-mounted* renderables sets a high-water mark you pay **forever**,
|
||||
even after everything is destroyed. (Fun fact from this week's forensics: we
|
||||
spent two days believing Ink had this disease. It doesn't — our Ink fork swapped
|
||||
Yoga-WASM for a plain TypeScript port at fork creation. **We** are the ones
|
||||
running layout in WASM. The accusation was true; we just had the defendant
|
||||
wrong.)
|
||||
|
||||
**Solid (the view framework).** Renders each store message into a row via
|
||||
`<For>`. The property we exploit: Solid mounts/unmounts *surgically* — remove a
|
||||
row from what the component returns and Solid destroys exactly that row's
|
||||
renderables (returning its handles and freeing its Yoga nodes), touching
|
||||
nothing else. No virtual-DOM diffing, no collateral re-renders.
|
||||
|
||||
**V8 (the JavaScript engine) + the store.** The store keeps every message as JS
|
||||
strings/objects. V8's garbage collector is *lazy by design*: with the default
|
||||
8GB ceiling we launch with, it sees no reason to clean up aggressively, so RSS
|
||||
includes a lot of "collectible but not yet collected" garbage. Cheap to fix,
|
||||
worth real MB (measured below).
|
||||
|
||||
**The scrollbox.** One detail that fooled everyone at some point:
|
||||
`viewportCulling` (on by default) skips *drawing* offscreen rows — but they stay
|
||||
fully **mounted**: handles held, Yoga nodes alive, memory paid. Culling saves
|
||||
paint time, not memory. That misunderstanding is half the reason the "rolling
|
||||
store cap" was expected to be enough, and wasn't.
|
||||
|
||||
## 2. Why it was 686MB
|
||||
|
||||
Simple arithmetic. The old TUI mounted **every message in the store** as a full
|
||||
renderable tree. 2,000 messages × ~16 renderables × (handles + Yoga nodes +
|
||||
text buffers + V8 objects) ≈ 670–690MB, growing ~300MB per 1,000 messages. And
|
||||
at ~1,400 rows the handle table filled: first a hard crash (exit 7), then —
|
||||
after our containment fix — survival with **unstyled text** past that point,
|
||||
plus a cap clamped from 3,000 rows down to 1,000 as the price of not crashing.
|
||||
|
||||
Ink, meanwhile, sat at ~234MB at the same workload, because Ink only ever
|
||||
mounts the rows near your viewport (~84–400 live nodes). Its memory is the
|
||||
*data* plus some caches — not the *view*.
|
||||
|
||||
## 3. The decisions, in order
|
||||
|
||||
### Decision 1: virtualize the view, don't starve the store
|
||||
|
||||
Two ways to cut view memory: keep fewer messages (opencode's answer — they keep
|
||||
100 and delete the rest from memory; transcript truth lives on their server), or
|
||||
keep all messages but only *materialize* the ones near the viewport. You vetoed
|
||||
the first (your p90 session is 182 messages — a 100-row store truncates normal
|
||||
sessions), so: **windowing**. Notably the OpenTUI devs confirmed this week that
|
||||
framework-level virtualization is the intended path — the engine doesn't ship
|
||||
it out of the box, and opencode never built it. We did.
|
||||
|
||||
### Decision 2: exact heights, recorded at unmount — never estimates in your face
|
||||
|
||||
This is the load-bearing idea, and it's where we beat Ink at its own game.
|
||||
|
||||
The hard problem of any virtualized list: an unmounted row still needs to
|
||||
occupy its correct *height*, or the scrollbar lies and content jumps. Ink
|
||||
solves it by **guessing** heights and correcting after measurement — those
|
||||
corrections are precisely the 83–101ms scroll stutters you hate. You explicitly
|
||||
vetoed "estimate-correction jank" as a model.
|
||||
|
||||
Our advantage: OpenTUI lays out with real, queryable heights. So when a row
|
||||
scrolls out of the window, we record its **exact laid-out height** (an
|
||||
`onSizeChange` hook fires inside layout, pre-paint) and replace the row with an
|
||||
empty `<box height={exactly-that}/>` — a **spacer**: one Yoga node, zero text
|
||||
buffers, zero native handles. Think of a bookshelf where books you're not
|
||||
reading are swapped for cardboard sleeves cut to *exactly* the book's
|
||||
thickness: the shelf never shifts, and you can't tell from across the room.
|
||||
|
||||
The window is your viewport ± one viewport of margin (plus hysteresis so it
|
||||
doesn't thrash at the edges). Scroll near a spacer and the real row remounts —
|
||||
at the recorded height, so nothing moves.
|
||||
|
||||
And one **law**, written into the code as `correctionIsLegal`: a spacer's
|
||||
height may only ever be corrected where you *cannot see it* — fully above the
|
||||
viewport (with the scroll position compensated in the same frame, so the world
|
||||
doesn't move) or fully below it. A correction that would shift visible content
|
||||
is forbidden, structurally. Jank isn't tuned down; it's outlawed.
|
||||
|
||||
### Decision 3 (the S2 insight): adjudicate on *append*, not just on scroll
|
||||
|
||||
S1 alone got 686 → 518MB. Why not more? Because of *when* windowing decided.
|
||||
S1 re-decided the window when you **scrolled**. But during a streaming burst —
|
||||
an agent turn dumping hundreds of rows — you don't scroll; rows arrive, each
|
||||
mounting fully, and only get demoted later. That transient pile-up is mostly
|
||||
invisible in steady-state numbers… except for Yoga-WASM, where **the transient
|
||||
peak is permanent** (memory never shrinks). The burst was quietly ratcheting
|
||||
the floor.
|
||||
|
||||
S2 makes the window recompute on **transcript growth**: while you're pinned at
|
||||
the bottom, the window anchors to the content *bottom*, so a row that falls
|
||||
more than a margin behind the live edge becomes a spacer the moment it's
|
||||
measured — not whenever you next scroll. Measured result: across a 1,500-row
|
||||
burst, the peak number of simultaneously-mounted rows is **31**.
|
||||
|
||||
Same trick for **resume**: opening a 2,000-message session used to mount all of
|
||||
it (transient peak again — paid forever). Now resume mounts only the bottom
|
||||
window; everything above starts as spacers using a line-count estimate, and an
|
||||
idle-time "measure march" quietly mounts ten rows at a time near the window
|
||||
edge, records their true heights, and swaps them back — all outside the
|
||||
viewport, all invisible by the law above.
|
||||
|
||||
### Decision 4: rows that must never be windowed
|
||||
|
||||
Windowing has to know what it's not allowed to touch:
|
||||
- **Streaming rows** — the native markdown renderer streams incrementally;
|
||||
unmounting mid-stream would restart it visibly.
|
||||
- **The bottom 30 rows** — the region you actually live in.
|
||||
- **Rows under a mouse selection** — the review caught that a lingering
|
||||
highlight originally froze windowing *forever* (memory regrowing silently).
|
||||
Fixed: only an active drag pauses swaps, and selected rows get pinned, so
|
||||
copy is byte-exact while everything else keeps windowing.
|
||||
|
||||
### Decision 5: give back the scrollback (cap 1,000 → 3,000)
|
||||
|
||||
The 1,000-row clamp existed only because mounted-rows == stored-rows and the
|
||||
handle table dies at ~1,400. With windowing, mounted ≈ 31 regardless of store
|
||||
size — so the cap went back to the originally-shipped 3,000. It's
|
||||
windowing-aware: the `HERMES_TUI_WINDOWING=0` escape hatch (which mounts
|
||||
everything again) keeps the safe 1,000.
|
||||
|
||||
### Decision 6 (measured, not yet shipped as default): right-size the V8 heap
|
||||
|
||||
Running the windowed TUI with a 512MB heap ceiling instead of 8GB forced V8 to
|
||||
actually collect: another −90MB with zero latency cost. That's queued as a
|
||||
launcher default change (~1GB), for both engines.
|
||||
|
||||
## 4. The scoreboard
|
||||
|
||||
At 2,000 messages (your real p99 session size — yes, we checked your DB:
|
||||
median session is 20 messages, p99 is 1,941):
|
||||
|
||||
| | peak memory | scroll p99 (slowest 1-in-100) |
|
||||
|---|---|---|
|
||||
| OpenTUI before | 686MB | 16ms |
|
||||
| + S1 windowing | 518MB | 16ms |
|
||||
| + S2 append/resume windowing | **300–375MB** | **6ms** |
|
||||
| Ink (reference) | 229–246MB | ~100ms |
|
||||
|
||||
At the **3,000-message stress** with the restored triple-size scrollback:
|
||||
**360MB, fully styled, scroll p99 8ms** — a workload that six days ago crashed
|
||||
the process, and three days ago survived only by dropping syntax colors.
|
||||
|
||||
Scroll got *faster* because there are simply fewer live renderables to walk.
|
||||
The determinism gate stayed **byte-identical** — the windowed TUI's settled
|
||||
frame is provably the same pixels as before. And the live smoke (2,000-message
|
||||
session: full sweep to the top, resize storm, back to bottom) returned a frame
|
||||
pixel-identical to boot, with deep history fully syntax-highlighted — something
|
||||
the pre-windowing TUI literally could not do.
|
||||
|
||||
## 5. What's honestly still open
|
||||
|
||||
- The remaining ~60–120MB over Ink is mostly the **store's JS strings** and
|
||||
process baseline — the view is no longer the problem. The structural fix is
|
||||
the **thin renderer** (W1): bodies live in the Python gateway (which already
|
||||
has them in SQLite); the TUI keeps ~300-byte stubs and fetches bodies only
|
||||
for the window. That also fixes the class of problem neither engine handles
|
||||
today: a single 10MB tool output.
|
||||
- Two accepted, documented limits: scrollbar-*jumping* deep into a freshly
|
||||
resumed session can land on estimate-height rows that snap to true height as
|
||||
they enter view (normal scrolling doesn't — the margin pre-measures; the idle
|
||||
march erodes the exposure over time), and a tool you expanded, scrolled far
|
||||
away from, then returned to will have re-collapsed (state is component-local;
|
||||
hoisting it to the store is queued).
|
||||
- Everything is behind `HERMES_TUI_WINDOWING` (default on, `0` = bit-exact old
|
||||
behavior) — a one-env escape hatch if anything feels off in real use.
|
||||
|
||||
*Where to verify: `bench/results/` (every number above), the design+gates doc
|
||||
`docs/plans/opentui-transcript-windowing.md`, tests in
|
||||
`ui-opentui/src/test/window.test.ts` and `transcriptWindow.test.tsx` (the
|
||||
zero-jank invariants are literal assertions: identical scrollHeight windowed
|
||||
vs not, byte-stable frames across corrections).*
|
||||
|
|
@ -1,73 +0,0 @@
|
|||
# Upstream alignment — how we inherit OpenTUI's performance work for free
|
||||
|
||||
Context (maintainer, 2026-06-11): opencode's 100-message cap was a November-era
|
||||
performance workaround, since obsoleted; the **next OpenTUI version ships
|
||||
native yoga** (≥2× layout performance, more improvements building on it);
|
||||
opencode does not use virtualization.
|
||||
|
||||
## The invariant that makes alignment free
|
||||
|
||||
**We are forkless and public-API-only.** The windowing layer (S1+S2) drives the
|
||||
STOCK `<scrollbox>` through documented surface only — `onSizeChange`,
|
||||
`setFrameCallback`, `scrollTop`/`viewport`/`scrollHeight`, Solid `<Show>`
|
||||
mount/unmount. Zero patches to `@opentui/core`. Every upstream release
|
||||
therefore drops in by bumping three pinned versions in `ui-opentui/package.json`
|
||||
(`@opentui/{core,keymap,solid}`, currently 0.4.0). Keep it that way: any new
|
||||
code that needs core behavior goes through a `boundary/` wrapper, never a
|
||||
patched dependency.
|
||||
|
||||
## What native yoga changes for us (and what it doesn't)
|
||||
|
||||
- **Kills the WASM ratchet** (grow-only linear memory → freeable native
|
||||
allocations). This retro-justifies S2 less, but S2's append-time windowing
|
||||
remains correct: transient mounted peaks still cost handles and RSS.
|
||||
- **Does NOT obsolete windowing.** The binding constraint is the 65,535-slot
|
||||
native handle table: ~47 handles/row × 3,000 stored rows ≈ 141k handles —
|
||||
over the table at ANY layout speed. Windowing is what makes the 3,000-row
|
||||
scrollback possible; yoga's backend is irrelevant to that math.
|
||||
- **Makes windowing feel even better**: 2× layout = cheaper margin remounts =
|
||||
smaller window margins viable and less exposure for the one accepted limit
|
||||
(estimate-height snap under scrollbar jumps). After the bump, re-tune margin/
|
||||
hysteresis against the scroll cell.
|
||||
|
||||
## The shim ledger (delete-on-upstream-fix; all in `ui-opentui/src/boundary/`)
|
||||
|
||||
| shim | what it papers over | delete when |
|
||||
|---|---|---|
|
||||
| `ffiSafe.ts` | u32 draw coords go negative under Node FFI (Bun silently wraps) — ERR_INVALID_ARG_VALUE loop | upstream clamps, or Node FFI path is officially supported |
|
||||
| `nativeHandles.ts` | SyntaxStyle exhaustion crashes mid-mount; degrade-to-unstyled | handle table widened (INDEX_BITS>16) or per-kind tables |
|
||||
| `renderer.ts` exit-signal guard | core 0.4.0 treats SIGPIPE (clipboard spawn) as an exit signal; its own uncaughtException handler allocates a handle and dies (exit-7 masking) | both fixed upstream |
|
||||
| `clipboard.ts` hardening | same SIGPIPE incident class | with the above |
|
||||
|
||||
Each is (a) isolated, (b) inert if upstream fixes the behavior, (c) worth
|
||||
reporting upstream — four concrete, reproduced, root-caused issues. Filing them
|
||||
is the cheapest alignment lever we have: it converts our workarounds into
|
||||
upstream regression tests. (Needs glitch's go-ahead — public repo activity.)
|
||||
|
||||
## The upgrade playbook (per upstream release)
|
||||
|
||||
1. Branch `chore/opentui-X.Y.Z`, bump the three pins, `npm ci`.
|
||||
2. `npm run check` (648 tests; the windowing invariants — identical
|
||||
scrollHeight ON/OFF, byte-stable frames across corrections — are literal
|
||||
assertions and will catch behavioral drift).
|
||||
3. Bench acceptance, sequential: `--cell gate` (determinism digest; EXPECT a
|
||||
new digest if upstream changed rendering — eyeball the frame, re-bless),
|
||||
`--cell mem3000 --msgs 2000` + `--cell scroll --msgs 3000` vs current
|
||||
numbers (300–375MB / p99 6–8ms), `--cell pipeline` (frame pacing ≥22fps).
|
||||
4. Shim audit: try each boundary shim OFF; delete the ones upstream fixed.
|
||||
5. Live tmux smoke (scroll sweep / resize / selection-copy), screenshots.
|
||||
6. Windowing re-tune if layout got faster: margins up or hysteresis down,
|
||||
re-run scroll cell, keep p99 ≤ 17ms gate.
|
||||
|
||||
The bench suite IS the upgrade contract — it's exactly the harness that lets
|
||||
us take every upstream improvement within a day of release, with proof.
|
||||
|
||||
## Questions worth relaying to the maintainer
|
||||
|
||||
1. Any plan to widen the 16-bit native handle table (or split per-kind)?
|
||||
That's our hard ceiling, independent of yoga.
|
||||
2. Is the Node `--experimental-ffi` path on their support radar, or Bun-only?
|
||||
(Native yoga adds new FFI surface; we run Node.)
|
||||
3. Would they take the windowing layer's core-agnostic pieces (exact-height
|
||||
spacer pattern, correction-legality rule) as a documented recipe or
|
||||
framework-level utility? We have it production-shaped with tests.
|
||||
|
|
@ -0,0 +1,240 @@
|
|||
---
|
||||
title: "fix: Prevent Telegram streamed replies from ending after first overflow chunk"
|
||||
status: active
|
||||
date: 2026-06-09
|
||||
type: fix
|
||||
target_repo: hermes-agent
|
||||
origin: user-reported Telegram topic screenshot
|
||||
---
|
||||
|
||||
# fix: Prevent Telegram streamed replies from ending after first overflow chunk
|
||||
|
||||
## Summary
|
||||
|
||||
Fix a Telegram gateway bug where a long streamed assistant reply can appear to stop mid-answer in a topic after the first overflow chunk. The reported screenshot shows a long Hermes response in the `Nehemiah - Coding` Telegram topic ending at `- The visible tool-call summary`, followed by the user noting that the previous message did not finish streaming to that Telegram topic.
|
||||
|
||||
The plan targets the streamed edit overflow path, not general model generation. A completed assistant response must either reach Telegram in full across all continuation messages or leave enough state for the gateway fallback path to deliver the remaining content instead of marking the turn complete after a partial delivery.
|
||||
|
||||
---
|
||||
|
||||
## Problem Frame
|
||||
|
||||
Telegram limits message text to 4096 UTF-16 code units. Hermes streams gateway responses by editing a message and, when a streamed message grows past the limit, splitting the overflow into additional Telegram messages. The adapter already has a split-and-deliver path for oversized edits, but the partial-continuation failure contract is weak: if chunk 1 is edited successfully and a later continuation fails, the adapter can still report success for the operation. The stream consumer may then mark the final response delivered even though the visible topic only contains the first part.
|
||||
|
||||
This is especially visible in Telegram forum topics because a long final response can be split below tool-progress bubbles, and a missing continuation looks exactly like the stream stopped mid-answer.
|
||||
|
||||
---
|
||||
|
||||
## Requirements
|
||||
|
||||
- R1. Long streamed Telegram replies must preserve all final content across overflow chunks.
|
||||
- R2. If any continuation chunk fails after the first overflow edit lands, the gateway must not mark the final response as fully delivered.
|
||||
- R3. Continuation chunks must remain routed to the same Telegram topic/thread as the original response.
|
||||
- R4. The fix must avoid duplicate full-answer sends when all overflow chunks were delivered successfully.
|
||||
- R5. Tests must cover the reported failure shape: a final streamed reply that exceeds Telegram's limit, succeeds on the first edit, fails on a continuation, and must not be treated as complete.
|
||||
|
||||
---
|
||||
|
||||
## Key Technical Decisions
|
||||
|
||||
- Treat overflow delivery as all-or-not-complete. `_edit_overflow_split` should only return a successful final-delivery result when every planned chunk reaches Telegram. Partial delivery is a distinct outcome that downstream code can recover from.
|
||||
- Carry partial-overflow metadata through `SendResult.raw_response` rather than adding a new public dataclass field unless implementation proves the existing result shape is insufficient. The stream consumer already inspects `SendResult` after adapter edits, so a small raw response contract can keep the change contained.
|
||||
- Make the stream consumer responsible for final-delivery truth. The adapter knows which chunks landed, but the consumer owns `_final_response_sent`, `_final_content_delivered`, `_fallback_prefix`, and fallback final-send behaviour.
|
||||
- Keep routing inside Telegram adapter helpers. Continuation sends should continue to use `_thread_kwargs_for_send(...)` with metadata-derived `message_thread_id` and reply anchors so forum topic behaviour stays consistent.
|
||||
|
||||
---
|
||||
|
||||
## High-Level Technical Design
|
||||
|
||||
```mermaid
|
||||
sequenceDiagram
|
||||
participant C as GatewayStreamConsumer
|
||||
participant T as TelegramAdapter.edit_message
|
||||
participant B as Telegram Bot API
|
||||
|
||||
C->>T: finalize/edit long accumulated response
|
||||
T->>B: edit original message with chunk 1
|
||||
loop remaining chunks
|
||||
T->>B: send continuation in same topic/thread
|
||||
end
|
||||
alt all chunks delivered
|
||||
T-->>C: success, last message id, continuation ids
|
||||
C->>C: mark final response delivered
|
||||
else any continuation failed
|
||||
T-->>C: partial overflow failure with delivered prefix metadata
|
||||
C->>C: do not mark final delivered
|
||||
C->>B: fallback sends missing tail or full final response safely
|
||||
end
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Implementation Units
|
||||
|
||||
### U1. Add a partial-overflow contract for Telegram edit splits
|
||||
|
||||
**Goal:** Make `TelegramAdapter._edit_overflow_split` distinguish complete overflow delivery from partial delivery.
|
||||
|
||||
**Requirements:** R1, R2, R4
|
||||
|
||||
**Dependencies:** None
|
||||
|
||||
**Files:**
|
||||
- `gateway/platforms/telegram.py`
|
||||
- `tests/gateway/test_telegram_send.py` or the existing Telegram adapter test module that already covers `edit_message` overflow behaviour
|
||||
|
||||
**Approach:**
|
||||
- Keep the successful path unchanged when every chunk is delivered: return `SendResult(success=True, message_id=<last chunk>, continuation_message_ids=(...))`.
|
||||
- When a continuation fails after the first edit, return a result that clearly indicates partial delivery instead of plain success. Prefer `success=False`, `retryable=True`, and `raw_response` metadata such as delivered chunk count, total chunk count, last delivered message id, and the visible delivered prefix.
|
||||
- Preserve logging, but do not rely on logs as the only signal. The caller must be able to tell partial delivery happened.
|
||||
- Ensure the first edited chunk and all successful continuation chunks still include the existing Markdown/plain-text fallback behaviour.
|
||||
|
||||
**Patterns to follow:**
|
||||
- Existing overflow handling in `TelegramAdapter.edit_message` and `_edit_overflow_split`.
|
||||
- Existing `SendResult` semantics in `gateway/platforms/base.py`, especially `retryable`, `raw_response`, and `continuation_message_ids`.
|
||||
|
||||
**Test scenarios:**
|
||||
- Oversized finalized edit where all continuations succeed returns success, the last continuation id, and all continuation ids.
|
||||
- Oversized finalized edit where the first continuation send fails returns a partial-overflow failure and does not report success.
|
||||
- Oversized finalized edit where one continuation succeeds and a later continuation fails reports the last delivered continuation id and delivered count in raw metadata.
|
||||
- A continuation MarkdownV2 formatting failure still retries plain text before being treated as a delivery failure.
|
||||
|
||||
**Verification:** Adapter tests prove complete overflow remains successful and partial overflow is observable by the caller.
|
||||
|
||||
### U2. Teach the stream consumer to recover from partial overflow
|
||||
|
||||
**Goal:** Ensure a partial Telegram overflow does not set `_final_response_sent` or `_final_content_delivered` unless the full response reached the user.
|
||||
|
||||
**Requirements:** R1, R2, R4, R5
|
||||
|
||||
**Dependencies:** U1
|
||||
|
||||
**Files:**
|
||||
- `gateway/stream_consumer.py`
|
||||
- `tests/gateway/test_stream_consumer.py` or a focused new `tests/gateway/test_stream_consumer_telegram_overflow.py`
|
||||
|
||||
**Approach:**
|
||||
- In `_send_or_edit`, when `adapter.edit_message(...)` returns a partial-overflow failure, update consumer state to reflect the last visible prefix/message and enter fallback delivery for the missing content.
|
||||
- Avoid treating `_already_sent` as final delivery. A partial visible message can be true while final delivery is false.
|
||||
- Use the delivered-prefix metadata if available so `_send_fallback_final(...)` sends only the missing tail. If implementation finds the prefix is unreliable after Markdown formatting, prefer sending the complete final response as a fresh fallback message rather than silently dropping the tail.
|
||||
- Keep the existing success handling for `continuation_message_ids` when the adapter delivered all chunks.
|
||||
|
||||
**Patterns to follow:**
|
||||
- Existing fallback mode in `GatewayStreamConsumer._send_or_edit` and `_send_fallback_final`.
|
||||
- Existing comments around `_final_response_sent`, `_final_content_delivered`, and `_fallback_prefix` for prior partial-delivery regressions.
|
||||
|
||||
**Test scenarios:**
|
||||
- A final streamed response that overflows and receives a complete-success edit split sets final-delivery flags and does not invoke fallback.
|
||||
- A final streamed response whose adapter reports partial overflow does not set final-delivery flags immediately.
|
||||
- After partial overflow, fallback delivery sends the remaining tail and then marks final content delivered only if the fallback send succeeds.
|
||||
- If fallback delivery also fails, the consumer leaves final-delivery false so the gateway's non-streaming final-send safety path can still run.
|
||||
|
||||
**Verification:** Stream consumer tests reproduce the screenshot shape by simulating first chunk visible and continuation failure, then assert the final answer is not suppressed.
|
||||
|
||||
### U3. Preserve Telegram topic/thread routing for overflow and fallback continuations
|
||||
|
||||
**Goal:** Ensure overflow recovery messages land in the same Telegram forum topic or DM topic fallback context.
|
||||
|
||||
**Requirements:** R3
|
||||
|
||||
**Dependencies:** U1, U2
|
||||
|
||||
**Files:**
|
||||
- `gateway/platforms/telegram.py`
|
||||
- `gateway/stream_consumer.py`
|
||||
- `tests/gateway/test_stream_consumer_thread_routing.py`
|
||||
- Relevant Telegram adapter routing tests, if existing coverage is closer there
|
||||
|
||||
**Approach:**
|
||||
- Keep passing `metadata` through every overflow continuation and fallback send.
|
||||
- Keep reply anchors where valid, but do not let a missing reply anchor drop the `message_thread_id` for normal forum topics.
|
||||
- For private DM topic fallback metadata, preserve the existing stricter anchor behaviour documented in the adapter comments.
|
||||
|
||||
**Patterns to follow:**
|
||||
- `TelegramAdapter._thread_kwargs_for_send(...)`.
|
||||
- Existing tests around Telegram topic recovery and stream consumer thread routing.
|
||||
|
||||
**Test scenarios:**
|
||||
- Overflow continuations include `message_thread_id` for a forum topic.
|
||||
- A continuation retry after `reply message not found` keeps forum topic routing when allowed.
|
||||
- Partial-overflow fallback sends receive the same metadata passed to the original stream consumer.
|
||||
|
||||
**Verification:** Thread-routing assertions inspect fake bot calls and confirm all continuation/fallback messages carry the expected topic metadata.
|
||||
|
||||
### U4. Add issue evidence and PR body traceability
|
||||
|
||||
**Goal:** Make the upstream issue and PR clearly trace the user-visible bug and verification evidence.
|
||||
|
||||
**Requirements:** R5
|
||||
|
||||
**Dependencies:** U1, U2, U3
|
||||
|
||||
**Files:**
|
||||
- GitHub issue body created via `gh issue create`
|
||||
- PR body using `.github/PULL_REQUEST_TEMPLATE.md`
|
||||
|
||||
**Approach:**
|
||||
- Create a GitHub issue with the screenshot evidence: the long message in the `Nehemiah - Coding` Telegram topic stops at `- The visible tool-call summary`, and the user's reply says the previous message did not finish streaming to that Telegram topic.
|
||||
- Reference affected component as Gateway and platform as Telegram.
|
||||
- In the PR body, link the issue with `Fixes #...`, describe the split-delivery contract change, and include the screenshot or attach it if GitHub upload is available.
|
||||
- Follow `CONTRIBUTING.md` and the repository PR template exactly.
|
||||
|
||||
**Patterns to follow:**
|
||||
- `.github/ISSUE_TEMPLATE/bug_report.yml`
|
||||
- `.github/PULL_REQUEST_TEMPLATE.md`
|
||||
|
||||
**Test scenarios:**
|
||||
- Test expectation: none, this is tracker and PR documentation work.
|
||||
|
||||
**Verification:** The GitHub issue exists with screenshot evidence or an explicit screenshot reference, and the PR body links the issue and lists the tests run.
|
||||
|
||||
---
|
||||
|
||||
## Scope Boundaries
|
||||
|
||||
### In Scope
|
||||
|
||||
- Telegram streamed response overflow splitting and recovery.
|
||||
- Stream consumer final-delivery truth for partial overflow delivery.
|
||||
- Topic/thread metadata preservation for overflow and fallback continuation sends.
|
||||
- Focused unit tests around adapter and stream consumer behaviour.
|
||||
|
||||
### Out of Scope
|
||||
|
||||
- Changing model streaming semantics in `run_agent.py`.
|
||||
- Reworking Telegram draft streaming, which is DM-only and not the forum-topic path in the screenshot.
|
||||
- Changing general platform message splitting for Discord, Slack, WhatsApp, or Matrix unless a shared helper must be corrected for the Telegram fix.
|
||||
- Altering tool-progress display settings or terminal progress rendering.
|
||||
|
||||
### Deferred to Follow-Up Work
|
||||
|
||||
- Broader observability for gateway delivery completeness across all messaging platforms.
|
||||
- A user-facing resend/recover command for a previous truncated response.
|
||||
|
||||
---
|
||||
|
||||
## Risks & Mitigations
|
||||
|
||||
- Risk: fallback recovery duplicates already-visible first chunks. Mitigation: use delivered-prefix metadata where reliable and add tests for no-duplicate complete-success behaviour.
|
||||
- Risk: preserving forum topic routing while dropping invalid reply anchors is easy to regress. Mitigation: include fake bot call assertions for `message_thread_id` and reply behaviour.
|
||||
- Risk: MarkdownV2 formatting can alter visible/raw prefix comparisons. Mitigation: keep fallback conservative; duplicate content is preferable to silently missing content, but tests should keep the common path tail-only.
|
||||
|
||||
---
|
||||
|
||||
## Sources & Research
|
||||
|
||||
- User-provided screenshot at `/root/.hermes/image_cache/img_f664e68f6ddf.jpg`.
|
||||
- `gateway/stream_consumer.py` streamed edit, overflow, fallback, and final-delivery state handling.
|
||||
- `gateway/platforms/telegram.py` Telegram send/edit overflow splitting and topic routing helpers.
|
||||
- `gateway/platforms/base.py` `SendResult` contract and shared message chunking helper.
|
||||
- `tests/gateway/test_stream_consumer.py`, `tests/gateway/test_stream_consumer_thread_routing.py`, and Telegram adapter tests for focused regression coverage.
|
||||
|
||||
---
|
||||
|
||||
## Verification Strategy
|
||||
|
||||
- Run focused Telegram adapter overflow tests.
|
||||
- Run focused stream consumer overflow/fallback tests.
|
||||
- Run topic-routing tests affected by metadata changes.
|
||||
- Run the gateway test subset around Telegram send/edit, stream consumer, and run progress if touched.
|
||||
- Before PR creation, ensure `git diff` contains only the plan, implementation, tests, and PR/issue-relevant documentation for this bug.
|
||||
Loading…
Add table
Add a link
Reference in a new issue