Skip to content

feat: add message pinning to sessions - #2534

Closed
Michaelyklam wants to merge 2 commits into
nesquena:masterfrom
Michaelyklam:feat/issue-2508-message-pins
Closed

Michaelyklam wants to merge 2 commits into
nesquena:masterfrom
Michaelyklam:feat/issue-2508-message-pins

Conversation

@Michaelyklam

@Michaelyklam Michaelyklam commented May 18, 2026 •

Copy link
Copy Markdown
Contributor

Thinking Path

Issue #2508 asks for a way to pin important conversation content inside a session. I kept the first slice session-scoped and bounded: no global bookmarks, no cross-session collections, and no broader transcript organization redesign.

What Changed

  • Added persisted pinned_messages metadata to WebUI session JSON/compact rows.
  • Added POST /api/session/message-pin to pin/unpin a message by session and message index, capped at three pins.
  • Added message footer pin buttons plus a right-click pin/unpin transcript action.
  • Added a chat-header pin button that opens a pinned-message popover with previews, click-to-jump, outside-click/Escape dismissal, and inline unpin controls.
  • Added regression coverage for session persistence, endpoint/source invariants, transcript actions, and panel rendering hooks.

Why It Matters

Users can keep a few important turns visible while working through a long conversation without copying them elsewhere or scrolling back repeatedly.

UI Media

Before — no pinned-message panel or per-message pin affordance:

Before: no pinned message panel

After — pinned message preview opens from the chat header pin button, leaving the workspace panel for files only:

After: header pin popover

Verification

  • env -u HERMES_CONFIG_PATH -u HERMES_WEBUI_HOST /home/michael/.hermes/hermes-agent/venv/bin/python -m pytest tests/test_issue2508_message_pins.py -q → 5 passed
  • node --check static/ui.js
  • env -u HERMES_CONFIG_PATH -u HERMES_WEBUI_HOST /home/michael/.hermes/hermes-agent/venv/bin/python -m py_compile api/models.py api/routes.py
  • git diff --check
  • Isolated local WebUI screenshot capture with demo session state; raw media URLs verified HTTP 200.
  • Follow-up: moved pinned-message previews from the workspace panel into a chat-header popover; after-header-pin-popover.png shows the revised placement.

Risks / Follow-ups

  • Pins are stored by message index plus a lightweight key and are intentionally session-local. If future transcript compaction/reordering changes visible coordinate spaces more aggressively, pins may need a richer durable message identity.
  • This first slice shows pinned previews in a header-anchored popover; broader pin search, labels, or cross-session saved snippets would be separate product work.

Closes #2508

Model Used

AI-assisted change with repository inspection, targeted editing, and shell-based test verification.

@Michaelyklam
Michaelyklam force-pushed the feat/issue-2508-message-pins branch 3 times, most recently from 1b4814b to 432e9e3 Compare May 18, 2026 15:22
@nesquena-hermes nesquena-hermes added the ux User experience / visual polish label May 18, 2026
@Michaelyklam

Copy link
Copy Markdown
Contributor Author

Rebased this on the latest master and kept the message-pinning changelog entry under [Unreleased].

Local verification after the rebase:

  • env -u HERMES_CONFIG_PATH -u HERMES_WEBUI_HOST /home/michael/.hermes/hermes-agent/venv/bin/python -m pytest tests/test_issue2508_message_pins.py -q — 5 passed
  • node --check static/ui.js
  • git diff --check
  • git merge-tree --write-tree origin/master HEAD

The PR now reads back as mergeable/clean; GitHub checks should attach to the new head shortly.

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Summary

Reading the diff (api/models.py, api/routes.py:5542-5575 for the new endpoint, static/ui.js:3722-3815, tests/test_issue2508_message_pins.py) plus context on origin/master, the slice is well-bounded and the data path is sound: server treats pinned_messages as a list of dicts on Session, persists it through compact() + METADATA_FIELDS, caps at 3, and the endpoint handles no-op unpin gracefully. The right-panel render and per-message pin button are scoped to existing transcript helpers. CI is green on 3.11/3.12/3.13.

One concrete bug to flag before merge plus a couple of smaller notes.

Bug: client and server compute different message_key digests

Server at api/routes.py:1974:

def _message_pin_key(message: dict, index: int) -> str:
    ...
    digest = hashlib.sha256(f"{role}\0{ts}\0{text}".encode("utf-8")).hexdigest()[:16]
    return f"msg:{index}:{role}:{digest}"

Client at static/ui.js:3728-3736 rolls its own djb2-style 32-bit hash:

let hash=0;
const seed=role+'\u0000'+ts+'\u0000'+text;
for(let i=0;i<seed.length;i++){hash=((hash<<5)-hash+seed.charCodeAt(i))|0;}
return {message_index:absIdx,message_key:'msg:'+absIdx+':'+role+':'+Math.abs(hash).toString(16)};

These will never produce the same digest. Today it's silently masked because the endpoint at api/routes.py:5559-5566 recomputes current_pin server-side and de-dupes on message_key OR message_index. But once a pin is persisted, the server's SHA digest is what lives in S.session.pinned_messages. The client's _isMessagePinned() then short-circuits through the index-only branch:

return _messagePins().some(p =>
  String(p.message_key||'')===key.message_key ||
  Number(p.message_index)===key.message_index);

Which works only until an earlier message is edited/compacted or _oldestIdx shifts (it gets reset on fork at sessions.js:1411, then bumped by _messages_offset on the next merge). At that point the index-only fallback either points at the wrong message or silently no-ops — defeating the whole purpose of the digested key field.

Suggested fix

Two clean options:

  1. Treat message_key as server-issued. Stop computing it on the client. _pinnedMessageButtonHtml writes only data-message-index and data-role; POST sends only message_index; _isMessagePinned matches by (absIdx, role) with a pin.preview.startsWith(msgContent(m).slice(0, 60)) sanity check. Server is the only source of truth for the digest. Simpler, no async needed.

  2. Mirror SHA-256 on the client. crypto.subtle.digest('SHA-256', ...) is available everywhere; cache results in a WeakMap<message,digest> to keep _pinnedMessageButtonHtml non-async after the first render.

(1) is the lower-risk path.

Smaller notes

  1. _oldestIdx cross-module dependency. _messagePinKeyFor reads _oldestIdx declared in static/sessions.js:1276. The typeof ... ==='number'?...:0 guard at ui.js:3729 handles early-init, but on a freshly forked-from-message session, _oldestIdx is 0 until the next merge populates data.session._messages_offset (sessions.js:1213). For one render window the absolute index encoded in pins is wrong. Probably tolerable given (1) above, but worth a comment.

  2. Pin previews are ambiguous on short repeats. _pinPreviewHtml shows role + first 220 chars. For short user turns (ok, templated replies), three pinned cards can look identical. Cheap fix: append #${pin.message_index} to the role badge in the pinned-messages-head.

  3. Endpoint defensiveness. existing[-3:] at routes.py:5572 runs after the len(existing) >= 3 guard rejects with 400. Belt-and-suspenders, harmless.

  4. Re-render double-call. renderPinnedMessages() runs both at the early-cache-hit branch (ui.js:5645) and at the end of renderMessages() (ui.js:6254). Negligible at 3 entries; the function early-returns on empty.

Test plan

After whichever key-strategy you adopt:

  • Pin a message, reload the page, confirm the same message renders pinned. (Currently broken on shifted indexes.)
  • Pin 3 messages, attempt a 4th → HTTP 400 with the cap message.
  • Pin a message in session A, fork from a later turn → confirm fork copy semantics for pinned_messages (Session.fork() doesn't appear to copy this field — worth either explicitly copying or asserting it doesn't, depending on intent).
  • Pin a tool-result message; verify the preview is the tool output, not "(empty)" — _message_pin_text walks content[].type in {text,thinking,reasoning} on the server but the client's msgContent() may compute differently for that shape.

The existing 5 tests are right for what they assert; consider adding a roundtrip test that POSTs with a deliberately-bad client-supplied message_key and verifies the persisted entry uses the server-computed one.

@Michaelyklam
Michaelyklam force-pushed the feat/issue-2508-message-pins branch from 6576ab8 to cc5c5c7 Compare May 18, 2026 23:23
@Michaelyklam

Copy link
Copy Markdown
Contributor Author

Rebased this branch on current master after the latest release batch and kept the message-pinning changelog entry under [Unreleased].

Verification on the rebased head cc5c5c71:

  • env -u HERMES_CONFIG_PATH -u HERMES_WEBUI_HOST /home/michael/.hermes/hermes-agent/venv/bin/python -m pytest tests/test_issue2508_message_pins.py -q — 5 passed
  • node --check static/ui.js
  • node --check static/icons.js
  • git diff --check
  • git diff --check origin/master...HEAD
  • git merge-tree --write-tree origin/master HEAD

GitHub Actions are green and the PR is mergeable CLEAN again.

@Michaelyklam
Michaelyklam force-pushed the feat/issue-2508-message-pins branch from cc5c5c7 to a283556 Compare May 19, 2026 05:02
@Michaelyklam

Michaelyklam commented May 19, 2026 •

Copy link
Copy Markdown
Contributor Author

Rebased this onto current master and resolved the CHANGELOG.md conflict, keeping the message-pinning entry under [Unreleased].

Verification on the updated head a2835568:

  • env -u HERMES_CONFIG_PATH -u HERMES_WEBUI_HOST /home/michael/.hermes/hermes-agent/venv/bin/python -m pytest tests/test_issue2508_message_pins.py -q → 5 passed
  • /home/michael/.hermes/hermes-agent/venv/bin/python -m py_compile api/models.py api/routes.py
  • git diff --check
  • git diff --check origin/master...HEAD
  • git merge-tree --write-tree origin/master HEAD
  • GitHub Actions: test (3.11), test (3.12), and test (3.13) all passed

GitHub now reports the PR as mergeable/clean.

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Holding for UX redesign — placement doesn't fit.

Thank you for the careful implementation. The functional pieces are all there (3-pin cap, per-message pin/unpin, persisted in session JSON, mobile-aware) and the tests cover the right invariants. But the maintainer flagged a placement concern that needs to be reworked before merge.

The issue

The pinned-messages section currently lives inside the right-hand workspace panel, above the file tree. The workspace panel is for workspace files — it's where users browse main.py, README.md, project folders. Putting message-level metadata there mixes two unrelated concepts: file navigation and conversation reference. A user who opens the workspace to browse files now has to scroll past pinned messages, and a user who wants to see pinned messages has to open a panel they otherwise associate with files.

This is the same reason we don't put profile settings inside the workspace panel — different concerns, different surfaces.

What we'd like instead (Discord is the right mental model here)

Discord handles pinned messages cleanly:

  • A pin icon lives in the channel header
  • Clicking it opens a popover/dropdown anchored to the header, showing the pinned messages
  • The popover has "jump to message" functionality on each pin
  • Closing the popover hides the pinned content — it never competes with the main UI for vertical space

Concrete suggestions for this PR:

  • Move the pinned-messages affordance to the chat header bar (next to the session title or the "8 messages" count), as a small pin icon with a badge count when ≥1 message is pinned
  • Clicking that icon opens a popover/dropdown showing the pinned messages, with click-to-jump behavior
  • The popover dismisses on outside-click or Escape
  • Do not place pinned content in any always-visible panel — keep it hidden behind the explicit click
  • Workspace panel returns to being purely for workspace files

What stays as-is:

  • Per-message pin button in the message footer ✅
  • Right-click pin/unpin context menu action ✅
  • 3-pin cap with polite error toast ✅
  • API endpoint shape ✅
  • Persistence model ✅
  • Tests ✅

The backend work and the per-message UI are great — only the presentation surface needs to be reworked.

What happens next

hold applied. The fix is moderate-sized (mostly removing the workspace-panel pinned section + adding a header-anchored popover) but contained — happy to either:

  • (A) Wait for an updated push from you (no time pressure, take it at your pace)
  • (B) Take a pass on the redesign ourselves if you'd prefer, then circle back for your review

Either way, the functional foundation here is solid — this is purely a presentation question. Just want to make sure pinned messages get the right home before they ship.

Thanks again for the careful work on this one. Looking forward to the next iteration.

@Michaelyklam

Copy link
Copy Markdown
Contributor Author

Addressed the placement feedback in follow-up commit fdd4fc00:

  • moved pinned-message previews out of the workspace panel
  • added a chat-header pin button with badge count
  • opens pins in a header-anchored popover with jump-to-message, inline unpin, outside-click, and Escape dismissal
  • updated the PR media with the revised placement screenshot

Verification:

  • env -u HERMES_CONFIG_PATH -u HERMES_WEBUI_HOST /home/michael/.hermes/hermes-agent/venv/bin/python -m pytest tests/test_issue2508_message_pins.py -q → 5 passed
  • node --check static/ui.js
  • node --check static/icons.js
  • /home/michael/.hermes/hermes-agent/venv/bin/python -m py_compile api/models.py api/routes.py
  • git diff --check
  • GitHub checks are green on the new head.

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Re-review: chat-header pin button looks like the right placement

Thanks for the rebuild @Michaelyklam. The follow-up commit fdd4fc00 addresses my earlier "placement doesn't fit" concern — moving pinned-message previews out of the workspace panel and using a chat-header pin button with badge count is the right shape.

I'll pull the updated branch, eyeball the popover behavior across desktop + mobile viewports, and either merge or post a focused redesign note. Removing the hold label and re-queueing as part of the next contributor batch.

In the meantime, two pre-merge questions on the popover surface, not blocking:

  1. Empty state — what does the popover show when a user clicks the pin button on a session with zero pinned messages? Should it appear at all, or stay hidden until there's at least one pin?
  2. Mobile drawer interaction — at narrow widths (≤768px) does the popover render inline as a header-anchored sheet, or does it expand the chat header height? A screenshot of the mobile state would be helpful in the PR body.

I'll set up an isolated test environment and screenshot both states myself if not provided — just easier to compare against your design intent if you confirm them now.

@nesquena-hermes nesquena-hermes added the size:L Large PR (>10 files or >250 LOC) label Jun 19, 2026
@cutter-sh

cutter-sh Bot commented Jun 20, 2026

Copy link
Copy Markdown

🎬 Cutter preview — PR #2534

Jump to a pinned message
Jump to a pinned message — Selecting a pinned message scrolls the chat to that message and highlights it.
Toggle message pin from footer
Toggle message pin from footer — Messages can be pinned from their footer, with a header pin counter and confirmation toast.

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Thanks for the careful work here @Michaelyklam — and apologies it sat after getting so close. Closing the core PR, but the feature isn't dead: we think message-pinning is a great fit for the extension system rather than core.

The reasoning: pinning individual messages inside a transcript isn't something core's reference points ship — ChatGPT/Claude pin whole conversations in the sidebar (a different feature), and it's a frequently-requested-but-unshipped idea elsewhere. That puts it squarely in "optional, self-contained enhancement of the chat transcript" territory — exactly what Hermes WebUI's extension system (now fully live: a registry + one-click install in Settings → Extensions) is for. A pin button per message + a header popover is the kind of thing an extension injects cleanly, without a new persistent core field.

We're planning to build a message-pinning extension (persisting pins client-side so it needs no core API), and your implementation here — the per-message pin UX, the 3-pin cap, the header popover, click-to-jump — is an excellent reference for it; you'll be credited. If you'd like to author the extension version yourself, we'd love that too.

Closing the core PR as a core-vs-extension scoping decision, not a quality one. Genuinely appreciate the work and the responsiveness through the review.

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

Labels

hold size:L Large PR (>10 files or >250 LOC) ux User experience / visual polish

Projects

None yet

Development

Successfully merging this pull request may close these issues.

会话里的对话内容 能提供置顶功能吗

2 participants