Skip to content

fix(state): deserialize display_metadata in get_messages and get_messages_around - #70829

Closed
DavidMetcalfe wants to merge 1 commit into
NousResearch:mainfrom
DavidMetcalfe:fix/display-metadata-deserialize-get-messages
Closed

DavidMetcalfe wants to merge 1 commit into
NousResearch:mainfrom
DavidMetcalfe:fix/display-metadata-deserialize-get-messages

Conversation

@DavidMetcalfe

@DavidMetcalfe DavidMetcalfe commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

What

get_messages() and get_messages_around() in hermes_state.py use SELECT * but don't deserialize the display_metadata column (added in #69771) from JSON. The frontend receives a raw JSON string, and the 'task_count' in message.display_metadata expression in chat-messages.ts throws TypeError because JavaScript's in operator requires an object, not a primitive.

How to reproduce

  1. Have a session with an async_delegation_complete display event (any session that used delegate_task)
  2. Resume it in Desktop or load messages via GET /api/sessions/{id}/messages
  3. "Resume failed" popup with Cannot use 'in' operator to search for 'task_count' in {"delegation_id":...}

Fix

Add json.loads() for display_metadata in both get_messages() and get_messages_around(), matching the existing pattern for tool_calls and the pattern already in _rows_to_conversation().

Testing

  • tests/test_hermes_state.py: 413 passed
  • tests/hermes_cli/test_web_server.py -k session: 49 passed

Closes #70835

@isak-ialogics

Copy link
Copy Markdown
Contributor

The fix covers both public retrieval paths, but the diff currently has no regression test exercising either one. Existing display_metadata coverage on current main only round-trips through conversation loading (get_conversation / _rows_to_conversation), which is why this omission could recur. Could you add a behavior test that stores non-empty metadata and asserts it is a dict with the original value from both get_messages() and get_messages_around()?

@DavidMetcalfe
DavidMetcalfe force-pushed the fix/display-metadata-deserialize-get-messages branch from ff9d278 to 34c5897 Compare July 24, 2026 17:06
…ages_around

Commit a4bc1ca added display_metadata as a new messages column and properly
deserialized it in _rows_to_conversation(), but missed adding json.loads() to
get_messages() and get_messages_around(). Both use SELECT * which now returns
the raw JSON string.

When the Desktop frontend renders a message with
display_kind='async_delegation_complete', chat-messages.ts:321 does
'task_count' in message.display_metadata. Since display_metadata was a JSON
string (not a parsed dict), the 'in' operator threw TypeError ('Cannot use
in operator to search for task_count in {...}'), causing a 'Resume failed'
popup when resuming sessions containing delegation completion events.
@DavidMetcalfe
DavidMetcalfe force-pushed the fix/display-metadata-deserialize-get-messages branch from 34c5897 to a7364f9 Compare July 24, 2026 17:08
@DavidMetcalfe

DavidMetcalfe commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

@isak-ialogics Thanks for the review! Added regression tests for both get_messages() and get_messages_around() — asserting that display_metadata is returned as a parsed dict (not a raw JSON string), plus a corrupt-JSON parse-failure test asserting the key is safely absent. All three fail pre-fix and pass with the deserialization in place.

Test suite: 416 passed (tests/test_hermes_state.py).

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 24, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused report and regression tests.

Automated hermes-sweeper review found that current main already contains a broader implementation in 3399bf28a5ad268ad806378346f8be0e90cf3fdd (fix(state): decode display_metadata at every message read path):

  • hermes_state.py:6158 decodes display_metadata in get_messages().
  • hermes_state.py:6229 does the same in get_messages_around().
  • hermes_state.py:5428 centralizes decoding, including invalid/non-object data and legacy double-encoded rows; it is also used by the conversation and anchored-view paths.
  • tests/test_hermes_state.py:2751 covers all four read paths and malformed metadata.

The requested behavioral guarantee is therefore already implemented on main.

@teknium1 teknium1 closed this Jul 30, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) P2 Medium — degraded but workaround exists sweeper:implemented-on-main Sweeper: behavior already present on current main sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Desktop 'Resume failed' TypeError — display_metadata not deserialized in get_messages / get_messages_around

4 participants