Skip to content

fix(tui): session.history withholds the row_id message.react needs - #74701

Open
Drexuxux wants to merge 1 commit into
NousResearch:mainfrom
Drexuxux:drex/session-history-row-ids
Open

fix(tui): session.history withholds the row_id message.react needs#74701
Drexuxux wants to merge 1 commit into
NousResearch:mainfrom
Drexuxux:drex/session-history-row-ids

Conversation

@Drexuxux

Copy link
Copy Markdown
Contributor

What

session.history hands the renderer a transcript with no durable message ids, so the new reactions feature can only ever react to the newest message.

session.history is a renderer-facing projection — it returns _history_to_messages(history), and that helper forwards row_id only when the history dicts carry _row_id:

# tui_gateway/server.py
if m.get("_row_id") is not None:
    msg["row_id"] = m["_row_id"]

1af883913 made _row_id opt-in (include_row_ids=True) so model-facing consumers — ACP restore, export, inspection — keep the historical transcript shape. That was the right call, and every other display projection was updated to opt in (server.py resume/display paths, session.resume in methods_session.py). session.history was missed.

message.react addresses messages by exactly that id, and its docstring says so: "row_id is the durable messages.id forwarded by _history_to_messages — the renderer's own message ids are ephemeral." Its only fallback, newest_role, resolves to the newest row of a role — so with the ids gone, every message except the latest becomes unreactable.

Measured on a 4-message transcript:

session.history  : 0/4 messages addressable   (row_id missing)
sibling projections : 4/4
newest_role fallback reaches user row 3; user row 1 is unreachable

The write path is fine — set_message_reaction accepts the older row once you can name it. Only the projection was withholding the id.

Fix

Pass include_row_ids=True in session.history, matching the other display projections. The post-undo live-replay reload (methods_tools.py) deliberately stays opt-out so replay history keeps its historical shape.

Tests

tests/tui_gateway/test_session_history_row_ids.py — 4 tests; the 3 behavioral ones fail without the fix (KeyError: 'row_id'), the 4th guards the other half of the split:

  • every rendered message carries row_id
  • the ids are the real messages.id values, in order
  • older messages are addressable, not just the one newest_role can reach
  • model-facing replay history still has no _row_id
tests/tui_gateway/test_session_history_row_ids.py ....  4 passed

Suite unaffected: 794 passed across tests/tui_gateway/ + tests/test_tui_gateway_server.py (the 2 failures there — test_entry_import_off_main_thread, test_append_log_record_single_write_lines — reproduce identically on main without this change).

session.history is a renderer-facing projection - it returns
_history_to_messages(history), and that helper forwards `row_id` only when
the history dicts carry `_row_id` (tui_gateway/server.py).

1af8839 made `_row_id` opt-in (include_row_ids=True) so model-facing
consumers - ACP restore, export, inspection - keep the historical transcript
shape. Every other display projection was updated to opt in
(server.py resume/display paths, methods_session.py session.resume);
session.history was missed, so it now hands the renderer a transcript with
no durable ids at all.

message.react addresses messages by exactly that id. Its only fallback,
newest_role, resolves to the newest row of a role - so with the ids gone,
every message except the latest becomes unreactable.

Measured on a 4-message transcript: 0/4 messages addressable via
session.history, 4/4 via the sibling projections.

Opt in, matching the other display projections. The post-undo live-replay
reload deliberately stays opt-out.
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/tui Terminal UI (ui-tui/ + tui_gateway/) labels Jul 30, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks—this is a narrow, verified correction.

Current main's session.history path calls get_messages_as_conversation() without include_row_ids=True at tui_gateway/methods_session.py:2268-2270. That API defaults the option to false at hermes_state.py:6332-6339, while _history_to_messages only emits row_id when _row_id is present at tui_gateway/server.py:6657-6664. message.react accepts a durable row ID, with its fallback restricted to the newest row of a role at tui_gateway/methods_session.py:935-964.

The proposed opt-in targets that exact display projection. The added tests exercise persisted rows and verify both durable-ID ordering and addressing an older row, while retaining the default no-row-ID model-facing projection.

Automated hermes-sweeper review.

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

Labels

area/sessions Session lifecycle, resume, persistence, history comp/tui Terminal UI (ui-tui/ + tui_gateway/) P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants