From aad97d93fed01dae178eb9f1805f345a4d43e690 Mon Sep 17 00:00:00 2001 From: Harjoth Khara Date: Tue, 18 Aug 2026 12:19:04 -0700 Subject: [PATCH] fix(hermes): surface real reasoning text from reasoning.available events (#9237) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When an agent runs through the Hermes gateway adapter, its stdout is parsed line-by-line into transcript entries that the issue chat renders (the UI fetches the adapter's `./ui-parser` from `/api/adapters/:type/ui-parser.js` and runs `parseStdoutLine` client-side) > - Reasoning-capable models emit a `reasoning.available` gateway event carrying the model's reasoning text, and the chat renders `thinking` parts as expandable chain-of-thought > - The gateway parser mapped `reasoning.available` to a hardcoded `"Hermes reasoning available"` string and discarded the event payload, so the "thinking" part had no real content — the indicator looked static and expanding it revealed nothing (#9209) > - This pull request extracts the actual reasoning text from the event payload and uses it as the `thinking` part's text, keeping the old string only as a fallback for payloads that carry no text > - The benefit is that the "Hermes reasoning available" indicator now surfaces the model's real reasoning, which the existing expandable-thinking UI can display ## Linked Issues or Issue Description Fixes: #9209 ## What Changed - `packages/adapters/hermes/src/gateway/ui/parse-stdout.ts`: the `reasoning.available` handler now extracts the reasoning text from the event `data` via a small helper (`extractReasoningText`), checking the plausible field names (`reasoning`, `reasoning_text`, `thinking`, `text`, `summary`, `content`) and recursing one level into nested `data` / `payload` records, with ANSI stripped. The prior `"Hermes reasoning available"` string is kept only as a fallback when no text field is present. - `packages/adapters/hermes/gateway-ui-parser.cjs`: applied the identical logical change to the committed CommonJS mirror (exported as `./gateway/ui-parser`), keeping the two files in sync. - `packages/adapters/hermes/src/gateway/ui/parse-stdout.test.ts` (new): unit tests for the gateway parser (there were none) covering direct-field, `summary`, nested `data`/`payload` extraction, the no-text fallback, and regression guards for `message.delta` and plain stdout. ## Verification Ran from `packages/adapters/hermes`: - `node_modules/.bin/vitest run src/gateway/ui/parse-stdout.test.ts` → **8/8 passed**. - Negative control: stashed the source changes and re-ran the same test file against the current (pre-patch) parser → **4/8 failed** (exactly the reasoning-extraction assertions), then restored — confirming the tests are discriminating, not vacuous. - `npx tsc --noEmit -p .` → clean. Real-behavior proof (driving the actual shipped `gateway-ui-parser.cjs` `parseStdoutLine`) is in the block below. ## Risks - **Low risk.** Behavior is unchanged for events that carry no recognizable text field — the `"Hermes reasoning available"` fallback is preserved (verified). Only the `reasoning.available` branch changed; `message.delta`, `run.failed`/`run.error`, and the generic/system/stdout branches are untouched. - The exact field name in a real `reasoning.available` payload is defined by the external Hermes gateway and is not present anywhere in this repo, so the extraction is intentionally defensive across several plausible field names rather than pinned to one. If the real event nests the text differently than `data` / `payload`, it will fall back to the existing placeholder (i.e. no regression vs. today). Happy to tighten the field list against real gateway traffic if a maintainer can share a sample. ## Model Used Claude Sonnet 5 (`claude-sonnet-5`) via Claude Code, with tool use and local test execution (ran vitest/tsc against the change). Planning, code review, and the real-behavior proof were done with Claude (Opus 4.8) in the same session. ## Real behavior proof **Behavior addressed:** A `reasoning.available` Hermes gateway event now produces a `thinking` transcript part containing the model's real reasoning text, instead of a static `"Hermes reasoning available"` placeholder with no content behind it (#9209). **Real environment tested:** Drove the actual shipped production artifact — `packages/adapters/hermes/gateway-ui-parser.cjs`, the exact module the UI loads via `/api/adapters/hermes-gateway/ui-parser.js` and runs to parse gateway stdout — on Node v24.16.0, macOS. The input is a raw stdout line in the exact format emitted by `packages/adapters/hermes/src/gateway/server/execute.ts` (`[hermes-gateway:event] run=… event=reasoning.available data=…`). Only the external gateway boundary (the raw line) is synthesized; the parser code path is the real one. **Exact steps or command run after this patch:** ``` # BEFORE = git show HEAD:…/gateway-ui-parser.cjs ; AFTER = patched artifact node proof.cjs # requires each parser build and calls parseStdoutLine(line, ts) # line = [hermes-gateway:event] run=run-abc123 event=reasoning.available \ # data={"text":"Checking whether the cache key includes the tenant id before I refactor the lookup."} ``` **Evidence after fix:** ``` ===== BEFORE (master / old code) ===== [ { "kind": "thinking", "ts": "…", "text": "Hermes reasoning available" } ] thinking part carries real reasoning text? -> NO (static placeholder, nothing for the UI to expand) ===== AFTER (this patch) ===== [ { "kind": "thinking", "ts": "…", "text": "Checking whether the cache key includes the tenant id before I refactor the lookup." } ] thinking part carries real reasoning text? -> YES ``` Additional cases through the same shipped artifact after the patch: ``` -- nested payload (data.payload.reasoning) -- {"kind":"thinking","ts":"…","text":"Weighing two migration orders."} -- bare signal, no text field (regression guard) -- {"kind":"thinking","ts":"…","text":"Hermes reasoning available"} # fallback preserved -- message.delta still works (regression guard) -- {"kind":"assistant","ts":"…","text":"Hello","delta":true} ``` **Observed result after fix:** The `reasoning.available` event yields a `thinking` part carrying the model's real reasoning text (top-level or nested), which the existing expandable-thinking rendering in the chat can display. Events with no text field still yield the original placeholder, and unrelated events are unaffected. **What was not tested:** I did not run against a live Hermes gateway — Paperclip's Hermes gateway binary and its credentials aren't available on this machine, and no captured real `reasoning.available` payload exists in the repo, so the exact wire field name is inferred (hence the defensive multi-field extraction + safe fallback). I also did not render the full React chat component in jsdom; the change is confined to the parser, and the chat's expandable `thinking` rendering already exists (`ui/src/components/IssueChatThread.tsx`). CI / unit tests here are supplemental to the runtime proof above. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above (searched `9209 in:body` and keyword variants — none found) - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (`fix/hermes-reasoning-available-payload`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] I have updated relevant documentation to reflect my changes (no user-facing docs describe this behavior; none needed) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green (will confirm once CI runs on the PR) - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups (will address on review) - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Claude Opus 4.8 --- .../adapters/hermes/gateway-ui-parser.cjs | 28 ++++- .../src/gateway/ui/parse-stdout.test.ts | 100 ++++++++++++++++++ .../hermes/src/gateway/ui/parse-stdout.ts | 30 +++++- 3 files changed, 156 insertions(+), 2 deletions(-) create mode 100644 packages/adapters/hermes/src/gateway/ui/parse-stdout.test.ts diff --git a/packages/adapters/hermes/gateway-ui-parser.cjs b/packages/adapters/hermes/gateway-ui-parser.cjs index 4654dbdef8..5d67ff4ce0 100644 --- a/packages/adapters/hermes/gateway-ui-parser.cjs +++ b/packages/adapters/hermes/gateway-ui-parser.cjs @@ -23,6 +23,31 @@ function asString(value) { return typeof value === "string" ? value : ""; } +// Pull the reasoning text out of a `reasoning.available` event payload. +// The exact field name is defined by the external Hermes gateway, so this +// checks the plausible field names (mirroring the fallback chain used by +// extractOutput() in gateway/server/execute.ts) and recurses one level into +// nested `data` / `payload` records. +function extractDirectReasoningText(data) { + return ( + asString(data.reasoning).trim() || + asString(data.reasoning_text).trim() || + asString(data.thinking).trim() || + asString(data.text).trim() || + asString(data.summary).trim() || + asString(data.content).trim() + ); +} + +function extractReasoningText(data) { + if (!data) return ""; + const direct = extractDirectReasoningText(data); + if (direct) return stripAnsi(direct); + const nested = asRecord(data.data) || asRecord(data.payload); + const nestedDirect = nested ? extractDirectReasoningText(nested) : ""; + return nestedDirect ? stripAnsi(nestedDirect) : ""; +} + function parseStdoutLine(line, ts) { const cleaned = stripAnsi(line); const trimmed = cleaned.trim(); @@ -41,7 +66,8 @@ function parseStdoutLine(line, ts) { return [{ kind: "stderr", ts, text: message }]; } if (eventName === "reasoning.available") { - return [{ kind: "thinking", ts, text: "Hermes reasoning available" }]; + const reasoning = extractReasoningText(data); + return [{ kind: "thinking", ts, text: reasoning || "Hermes reasoning available" }]; } return [{ kind: "system", ts, text: `Hermes event: ${eventName}` }]; } diff --git a/packages/adapters/hermes/src/gateway/ui/parse-stdout.test.ts b/packages/adapters/hermes/src/gateway/ui/parse-stdout.test.ts new file mode 100644 index 0000000000..aa0b354166 --- /dev/null +++ b/packages/adapters/hermes/src/gateway/ui/parse-stdout.test.ts @@ -0,0 +1,100 @@ +import { describe, expect, it } from "vitest"; +import { parseHermesGatewayStdoutLine } from "./parse-stdout.js"; + +const TS = "2026-07-08T12:00:00.000Z"; + +function eventLine(eventName: string, data: unknown): string { + return `[hermes-gateway:event] run=run-123 event=${eventName} data=${JSON.stringify(data)}`; +} + +describe("parseHermesGatewayStdoutLine — reasoning.available payload extraction", () => { + // This is the assertion that FAILS on the old hardcoded-placeholder code + // and PASSES once the real reasoning text is extracted from `data`. + it("uses the real reasoning text from data.text instead of the hardcoded placeholder", () => { + const result = parseHermesGatewayStdoutLine( + eventLine("reasoning.available", { text: "Considering three approaches to the cache invalidation bug." }), + TS, + ); + expect(result).toHaveLength(1); + expect(result[0]).toEqual({ + kind: "thinking", + ts: TS, + text: "Considering three approaches to the cache invalidation bug.", + }); + expect(result[0]).not.toHaveProperty("text", "Hermes reasoning available"); + }); + + it("uses the real reasoning text from data.summary", () => { + const result = parseHermesGatewayStdoutLine( + eventLine("reasoning.available", { summary: "Weighing tradeoffs between two refactor strategies." }), + TS, + ); + expect(result).toHaveLength(1); + expect(result[0]).toEqual({ + kind: "thinking", + ts: TS, + text: "Weighing tradeoffs between two refactor strategies.", + }); + }); + + it("recurses one level into a nested data.data record to find the reasoning text", () => { + const result = parseHermesGatewayStdoutLine( + eventLine("reasoning.available", { data: { text: "Nested reasoning payload from gateway wrapper." } }), + TS, + ); + expect(result).toHaveLength(1); + expect(result[0]).toEqual({ + kind: "thinking", + ts: TS, + text: "Nested reasoning payload from gateway wrapper.", + }); + }); + + it("recurses one level into a nested data.payload record to find the reasoning text", () => { + const result = parseHermesGatewayStdoutLine( + eventLine("reasoning.available", { payload: { reasoning: "Nested via payload wrapper instead of data." } }), + TS, + ); + expect(result).toHaveLength(1); + expect(result[0]).toEqual({ + kind: "thinking", + ts: TS, + text: "Nested via payload wrapper instead of data.", + }); + }); + + it("falls back to the placeholder when data has no recognizable text field (preserves bare-signal behavior)", () => { + const result = parseHermesGatewayStdoutLine( + eventLine("reasoning.available", { unrelatedField: 42 }), + TS, + ); + expect(result).toHaveLength(1); + expect(result[0]).toEqual({ kind: "thinking", ts: TS, text: "Hermes reasoning available" }); + }); + + it("falls back to the placeholder when data is entirely absent/unparseable", () => { + const result = parseHermesGatewayStdoutLine( + "[hermes-gateway:event] run=run-123 event=reasoning.available data=not-json", + TS, + ); + expect(result).toHaveLength(1); + expect(result[0]).toEqual({ kind: "thinking", ts: TS, text: "Hermes reasoning available" }); + }); +}); + +describe("parseHermesGatewayStdoutLine — regression guards for unrelated handlers", () => { + it("still yields an assistant delta part for message.delta", () => { + const result = parseHermesGatewayStdoutLine( + eventLine("message.delta", { delta: "Hello there" }), + TS, + ); + expect(result).toHaveLength(1); + expect(result[0]).toEqual({ kind: "assistant", ts: TS, text: "Hello there", delta: true }); + }); + + it("still yields a stdout part for a plain non-event line", () => { + const result = parseHermesGatewayStdoutLine("just a plain line of output", TS); + expect(result).toHaveLength(1); + expect(result[0]).toEqual({ kind: "stdout", ts: TS, text: "just a plain line of output" }); + }); +}); diff --git a/packages/adapters/hermes/src/gateway/ui/parse-stdout.ts b/packages/adapters/hermes/src/gateway/ui/parse-stdout.ts index 2837bea15e..894f923515 100644 --- a/packages/adapters/hermes/src/gateway/ui/parse-stdout.ts +++ b/packages/adapters/hermes/src/gateway/ui/parse-stdout.ts @@ -27,6 +27,33 @@ function asString(value: unknown): string { return typeof value === "string" ? value : ""; } +/** + * Pull the reasoning text out of a `reasoning.available` event payload. + * The exact field name is defined by the external Hermes gateway, so this + * checks the plausible field names (mirroring the fallback chain used by + * extractOutput() in gateway/server/execute.ts) and recurses one level into + * nested `data` / `payload` records. + */ +function extractDirectReasoningText(data: Record): string { + return ( + asString(data.reasoning).trim() || + asString(data.reasoning_text).trim() || + asString(data.thinking).trim() || + asString(data.text).trim() || + asString(data.summary).trim() || + asString(data.content).trim() + ); +} + +function extractReasoningText(data: Record | null): string { + if (!data) return ""; + const direct = extractDirectReasoningText(data); + if (direct) return stripAnsi(direct); + const nested = asRecord(data.data) ?? asRecord(data.payload); + const nestedDirect = nested ? extractDirectReasoningText(nested) : ""; + return nestedDirect ? stripAnsi(nestedDirect) : ""; +} + export function parseHermesGatewayStdoutLine(line: string, ts: string): TranscriptEntry[] { const cleaned = stripAnsi(line); const trimmed = cleaned.trim(); @@ -45,7 +72,8 @@ export function parseHermesGatewayStdoutLine(line: string, ts: string): Transcri return [{ kind: "stderr", ts, text: message }]; } if (eventName === "reasoning.available") { - return [{ kind: "thinking", ts, text: "Hermes reasoning available" }]; + const reasoning = extractReasoningText(data); + return [{ kind: "thinking", ts, text: reasoning || "Hermes reasoning available" }]; } return [{ kind: "system", ts, text: `Hermes event: ${eventName}` }]; }