Skip to content

feat: payload visualization dashboard - #1

Merged
heyalchang merged 2 commits into
local/dashboard-and-trackingfrom
feat/payload-viz
Mar 12, 2026
Merged

feat: payload visualization dashboard#1
heyalchang merged 2 commits into
local/dashboard-and-trackingfrom
feat/payload-viz

Conversation

@heyalchang

Copy link
Copy Markdown
Owner

Summary

  • Tiktoken-based per-component token counting after _build_api_kwargs() in the main agent loop — counts system prompt, tool definitions, user messages, assistant messages, and tool results separately
  • breakdown dict added to JSONL token log entries (old entries without it handled gracefully)
  • New /api/payload-breakdown?session_id=<id> endpoint in dashboard
  • New Payload tab: canvas stacked area chart showing context growth over a session, horizontal stacked bars for per-turn proportions, click-to-expand detail with actual vs estimated vs cached stats
  • Version display (v0.2.0) in dashboard topbar
  • DEVJOURNAL entry documenting the feature

Files changed

  • run_agent.py_compute_payload_breakdown() method + instrumentation at main loop + breakdown in JSONL entry
  • dashboard/data.pyget_payload_breakdown() function
  • dashboard/server.py/api/payload-breakdown route
  • dashboard/static/index.html — Payload tab UI (area chart, stacked bars, detail view, legend, CSS)
  • DEVJOURNAL.md — feature entry

Design decisions

  • tiktoken gpt-4o encoding for exact counts (~3ms overhead), not the rough estimator
  • Instrument only main agent loop, not memory flush or compression side tasks
  • Extend existing JSONL format rather than new file or DB table
  • Color scheme: blue (system), purple (tool defs), green (user), orange (assistant), red (tool results)

Not included (deferred)

  • Reasoning chevron in transcript view — requires message_extras extension table (DB doesn't store reasoning content today)
  • Live testing — needs gateway restart + CLI messages to generate breakdown data

Test plan

  • Restart gateway + dashboard with new code
  • Send 2-3 CLI messages to generate JSONL entries with breakdown field
  • Verify ~/.hermes/token_usage.jsonl has breakdown data
  • Open dashboard Payload tab, select a session, verify area chart + bars render
  • Click a turn in the chart, verify detail drill-down shows correct component breakdown
  • Verify old sessions (pre-instrumentation) show graceful "no data" message

🤖 Generated with Claude Code

Add tiktoken-based instrumentation to count tokens per payload component
(system prompt, tool definitions, user/assistant/tool messages) after each
API call. Extend JSONL token log with breakdown field. New Payload tab in
dashboard with stacked area chart (context growth over session) and
per-turn stacked bar detail with cached token overlay.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ccreview

ccreview Bot commented Mar 12, 2026

Copy link
Copy Markdown

prbot Review (Claude)


Code Review: PR #1 — Payload Visualization Dashboard

PR: feat: payload visualization dashboard
+497 / -0 across 5 files


Overview

This PR adds a payload visualization feature to the Hermes dashboard that shows what goes into each API call: system prompt, tool definitions, user messages, assistant messages, and tool results. It instruments the main agent loop with tiktoken-based token counting, extends the existing JSONL log format, adds a new API endpoint, and builds a canvas-based visualization with stacked area charts and per-turn breakdowns.

The design is well-considered — instrumenting at the right point in the pipeline, extending existing formats rather than introducing new storage, and handling backward compatibility with pre-instrumentation data.


Strengths

  • Minimal overhead, good placement: Instrumenting after _build_api_kwargs() + preflight but only in the main loop (not side tasks) is the right call. ~3ms tiktoken overhead is negligible.
  • Graceful degradation: Lazy import tiktoken with ImportError fallback means the feature is silently disabled if tiktoken isn't installed. Old JSONL entries without breakdown are handled cleanly.
  • Consistent patterns: The new endpoint/route follows existing conventions in server.py and data.py.
  • Self-contained visualization: The canvas-based charts avoid adding a charting library dependency.
  • Good dev journal documentation: The DEVJOURNAL entry clearly documents design decisions and deferred work.

Issues & Suggestions

🔴 High Priority

  1. tiktoken is not a declared dependency

    tiktoken is imported in run_agent.py but isn't listed in pyproject.toml or requirements.txt. It currently works because it's a transitive dependency (likely via litellm), but this is fragile — if litellm drops it or users install with --no-deps, the feature silently breaks with no warning.

    Suggestion: Add tiktoken as an explicit dependency, or at minimum log a one-time warning when the import fails:

    except ImportError:
        logger.debug("tiktoken not installed; payload breakdown disabled")
        return {}
  2. Encoding is re-created on every call

    _compute_payload_breakdown calls tiktoken.encoding_for_model("gpt-4o") on every invocation. While tiktoken may cache internally, this should be made explicit:

    # Cache at instance or class level
    if not hasattr(self, '_tiktoken_enc'):
        self._tiktoken_enc = enc
  3. Instance attribute _last_payload_breakdown set outside __init__

    self._last_payload_breakdown is first set in run_conversation() and later accessed with getattr(self, '_last_payload_breakdown', None). This is a code smell — the attribute should be initialized in __init__ to make the class contract clear and avoid the getattr dance:

    # In __init__:
    self._last_payload_breakdown = {}

🟡 Medium Priority

  1. _count() helper double-serializes dict messages

    The _count function does json.dumps(obj) for non-string objects, then tokenizes the JSON string. But API messages are dicts, and the actual token count the model sees is based on the chat-ml encoding, not a JSON serialization. This means the "estimated" counts will consistently diverge from "actual" — particularly for structured tool definitions. The DEVJOURNAL calls it "exact counts" but it's actually an approximation.

    Suggestion: Clarify in the DEVJOURNAL/comments that these are estimated counts, not exact chat-ml counts. The summary view already labels "Estimated" vs "Actual" which is good.

  2. No input sanitization on session_id

    get_payload_breakdown(session_id) takes the raw query parameter and compares it against JSONL entries. While this isn't an injection vector (it's just a string comparison), the function reads the entire JSONL file into memory for every request. For large JSONL files (months of usage), this could be slow.

    Suggestion: Consider adding a maxResults parameter or reading the file in reverse (tail) to limit scan scope, consistent with how other endpoints cap results.

  3. Bare except Exception in JSONL reader

    except Exception:
        return []

    This silently swallows all errors including PermissionError, MemoryError, etc. A logged warning would help debugging.

  4. Reasoning chevron CSS included but no JS implementation

    The CSS for .msg-reasoning-toggle and .msg-reasoning-body is added (lines 338-365 of the HTML diff) but there's no corresponding JavaScript to render or toggle these elements. This is dead CSS that should either be deferred to a separate PR or removed.

🟢 Low Priority / Nits

  1. Hardcoded version string

    <span ...>v0.2.0</span>

    This will inevitably drift. Consider pulling from a single source of truth (e.g., pyproject.toml version, or a JS constant).

  2. fmtTokens helper used but not defined in the diff

    The JS code calls fmtTokens() and fmtNum() — these presumably exist already, but it's worth confirming they handle edge cases (0, negative, very large numbers).

  3. Canvas click handler doesn't account for DPR scaling

    const mx = e.clientX - rect.left;

    The xScale function operates in CSS pixels, and clientX - rect.left also gives CSS pixels, so this should be fine. But the DPR-scaled canvas could cause issues on some browsers — worth testing on a high-DPI display.

  4. Global window.payloadData

    drawPayloadBars assigns to window.payloadData for the click handler. This works but pollutes the global namespace. Consider using a closure or a module-scoped variable within the script block.

  5. esc() function used in template literals

    Good practice for XSS prevention in the session selector. Confirm esc() properly handles all HTML entities (not just <, >, & but also " and ').


Security

  • No obvious injection vectors — session_id is used only for string comparison, not in SQL or file paths.
  • The JSONL file is read-only from the dashboard side.
  • HTML escaping is applied to session labels in the dropdown — good.

Test Coverage

  • No automated tests are included. The PR relies on manual testing per the test plan.
  • Suggestion: At minimum, add a unit test for _compute_payload_breakdown() with sample api_kwargs for both codex_responses and chat completions modes, and a test for get_payload_breakdown() with a sample JSONL file.

Summary

This is a well-architected feature addition with thoughtful design decisions. The main concerns are: (1) tiktoken should be a declared dependency, (2) the encoding should be cached, (3) dead reasoning-chevron CSS should be removed or deferred, and (4) unit tests should be added for the new Python functions. The visualization code is clean and self-contained. After addressing the high-priority items, this is ready to merge.

@heyalchang

Copy link
Copy Markdown
Owner Author

@codex review

@ccreview

ccreview Bot commented Mar 12, 2026

Copy link
Copy Markdown

prbot Review (Codex)

Summary

Nice addition overall—the instrumentation plus dashboard tab ties log data back to a visual story. Unfortunately, the current payload accounting isn’t accurate for several common scenarios, which makes the new UI misleading. Fixing the counting bugs below should get the feature over the line.

P0/P1 Findings

  • P1: run_agent.py:2466-2492 – Codex Responses payloads mis-classify everything that isn’t a plain {role: ...} message as “user” tokens. _chat_messages_to_responses_input() emits {"type": "function_call"} and {"type": "function_call_output"} entries (assistant tool calls and tool results) with no role, so _compute_payload_breakdown() drops into the default else branch and charges them to the user bucket. As a result, Sessions that went through the Responses API will always show zero tool usage and wildly inflated user counts, defeating the whole point of the new dashboard. Inspect item.get("type") and attribute function_call rows to the assistant, function_call_output rows to tool_results, etc., instead of treating them as user content.

  • P1: run_agent.py:2457-2462 – Non‑ASCII conversations are grossly miscounted because _count() serializes message dicts with json.dumps(obj, default=str) (which defaults to ensure_ascii=True). That turns every kanji/emoji/etc. into \uXXXX escape sequences before tokenization, so the “estimated” tokens skyrocket relative to the actual payload whenever someone writes in another language. The dashboard claims to show what truly went into the payload, but for any non‑English session it produces nonsense. Serialize with ensure_ascii=False (or directly encode message content strings) so the encoder sees the same Unicode text the API sends.

Test Gaps

  • No unit tests cover _compute_payload_breakdown (chat vs Responses mode) or dashboard.data.get_payload_breakdown; a simple fixture with a fake Responses payload would have exposed the mis-bucketed tool calls and the Unicode escaping problem before landing.

Overall Verdict

Request changes.

…d CSS

- Fix Codex Responses mode: classify function_call items as assistant,
  function_call_output as tool_results (was falling through to user)
- Fix ensure_ascii=False in json.dumps to avoid inflated token counts
  for non-ASCII text
- Cache tiktoken encoding on self._tiktoken_enc (was re-created per call)
- Initialize _last_payload_breakdown in __init__
- Add logger.debug on tiktoken ImportError
- Remove dead reasoning chevron CSS (deferred to message_extras work)
- Clarify "estimated" vs "exact" in docstring and DEVJOURNAL

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@heyalchang

Copy link
Copy Markdown
Owner Author

Review Response

Addressed (P1)

Codex Responses mis-bucketing (Codex review)
Fixed. _compute_payload_breakdown now checks item.get("type") first — function_call → assistant, function_call_output → tool_results. Only falls through to role-based classification for regular messages.

Unicode inflation (Codex review)
Fixed. json.dumps now uses ensure_ascii=False so non-ASCII text is serialized as-is, matching what the API actually sends.

Encoding re-created every call (Claude review #2)
Fixed. Cached on self._tiktoken_enc at first use.

_last_payload_breakdown not in __init__ (Claude review NousResearch#3)
Fixed. Both _last_payload_breakdown and _tiktoken_enc initialized in __init__.

Addressed (P2 clean wins)

Dead reasoning chevron CSS (Claude review NousResearch#7)
Removed. Will be added back when message_extras table is implemented.

tiktoken ImportError silent (Claude review #1)
Added logger.debug("tiktoken not installed; payload breakdown disabled") on import failure.

"Exact" vs "estimated" language (Claude review NousResearch#4)
Updated docstring and DEVJOURNAL to say "estimated counts based on JSON serialization" — the summary view already labels "Estimated" vs "Actual" which is the right framing.

Deferred (with rationale)

  • tiktoken as explicit dependency: It's a transitive dep via litellm, and the ImportError path logs and degrades gracefully. Adding it to pyproject.toml would be an upstream file change. Not worth it for a local fork feature.
  • JSONL scan optimization: Single user, ~250 entries, reads in <1ms. Will revisit if the file grows to 10K+ entries.
  • Unit tests: Agree these would be valuable. Deferring to a follow-up since the feature needs live testing first anyway.
  • Hardcoded version: True, but pulling from pyproject.toml at runtime would require a build-time or server-side injection. Acceptable for now.
  • Global window.payloadData: Dashboard is a single vanilla JS file with no module system. Closure scoping would add complexity for no real benefit here.

@ccreview

ccreview Bot commented Mar 12, 2026

Copy link
Copy Markdown

prbot Review (Claude)


Code Review: PR #1 — Payload Visualization Dashboard

Overview

This PR adds a payload visualization feature to the Hermes dashboard. It instruments the main agent loop with tiktoken-based token counting, breaks down API payloads into 5 components (system, tool defs, user, assistant, tool results), logs breakdowns to the existing JSONL file, adds a new API endpoint, and renders an interactive area chart + stacked bar visualization in a new dashboard tab.

Verdict: Well-structured feature with clean separation of concerns. A few issues worth addressing before merge.


👍 Strengths

  • Clean architecture: Instrumentation in run_agent.py → data access in data.py → route in server.py → UI in index.html — follows existing patterns perfectly
  • Graceful degradation: Old JSONL entries without breakdown are handled cleanly; tiktoken import failure is caught; the bare except Exception around breakdown computation prevents it from ever crashing the main loop
  • Low overhead: Lazy tiktoken initialization, ~3ms per call, only in the main loop
  • Good UX: Click-to-drill-down on turns, cached token overlay line, percentage breakdowns

⚠️ Issues & Suggestions

1. Security: No input sanitization on session_id query parameter (Medium)

dashboard/server.py line 116:

session_id = request.query.get("session_id", "")

This value is passed directly to get_payload_breakdown() where it's compared against JSONL entries. While this is a string equality check (not SQL), consider validating the format (e.g., UUID pattern) to prevent unexpected behavior or abuse if the dashboard is ever exposed beyond localhost.

2. Performance: Full JSONL scan on every request (Medium)

dashboard/data.py get_payload_breakdown():

The function reads the entire token_usage.jsonl file line-by-line on every call. As usage accumulates, this file can grow significantly.

Suggestions:

  • Add a maxResults / limit parameter to stop early once enough turns are found
  • Consider reading the file in reverse (most recent entries first) since users typically care about recent sessions
  • At minimum, document the O(n) cost so future contributors are aware

3. Silent exception swallowing (Low-Medium)

dashboard/data.py line 260:

except Exception:
    return []

A bare except Exception: return [] around the entire file read is very broad. If the JSONL file is corrupted or has permission issues, the user gets an empty result with no feedback. Consider logging the exception:

except Exception as e:
    logger.warning("Failed to read payload breakdown: %s", e)
    return []

Similarly in run_agent.py line 3656:

except Exception:
    self._last_payload_breakdown = {}

This is more defensible (don't crash the agent loop), but a logger.debug would help with troubleshooting.

4. _count() double-serializes strings (Low)

run_agent.py _compute_payload_breakdown():

def _count(obj):
    if isinstance(obj, str):
        return len(enc.encode(obj))
    return len(enc.encode(json.dumps(obj, default=str, ensure_ascii=False)))

When _count() is called with a message dict like {"role": "user", "content": "hello"}, it serializes the entire dict to JSON first. This means it counts JSON syntax tokens ({, "role", :, etc.) that aren't part of the actual payload — inflating estimates. This is acknowledged in the docstring as "approximations based on JSON serialization", but worth noting the estimates could be 10-20% higher than actual for small messages.

5. getattr check is fragile (Low)

run_agent.py line 3957:

if getattr(self, '_last_payload_breakdown', None):

_last_payload_breakdown is already initialized in __init__ to {}, so getattr with a default is unnecessary. Also, an empty dict {} is falsy, which is the intended behavior, but using hasattr + explicit check would be clearer:

if self._last_payload_breakdown:

6. Canvas rendering doesn't handle DPR for click coordinates (Low)

index.html drawPayloadAreaChart():

The chart uses dpr scaling for rendering but the onclick handler uses e.clientX - rect.left without accounting for the DPR-to-CSS coordinate mapping. Since xScale operates in CSS pixels and getBoundingClientRect() also returns CSS pixels, this actually works correctly — but only because the canvas CSS width:100% matches. If the canvas is ever resized differently, this would break. A comment noting the assumption would help.

7. Global variable window.payloadData (Low)

index.html line drawPayloadBars():

window.payloadData = data;

Storing data on window works but pollutes the global scope. Consider using a module-scoped variable or a data attribute on the container element. That said, I see the rest of the dashboard uses similar patterns (e.g., inline onclick handlers), so this is consistent with existing conventions.

8. Version hardcoded in HTML (Nit)

<span style="font-size:12px;color:var(--text-dim);margin-left:-8px">v0.2.0</span>

The version is hardcoded inline. If there's a version constant elsewhere, it would be better to serve it from the API. If this is the first version indicator, consider at least adding a comment or TODO noting it should be kept in sync.


🧪 Test Coverage

  • No automated tests are included for the new get_payload_breakdown() function, the _compute_payload_breakdown() method, or the new API endpoint. The PR includes a manual test plan which is reasonable for a dashboard feature, but unit tests for the data parsing logic (especially edge cases like malformed JSONL lines, missing fields, mixed API modes) would add confidence.

📋 Summary

Category Rating
Correctness ✅ Good — logic is sound, edge cases handled
Code Style ✅ Consistent with existing codebase
Performance ⚠️ JSONL full-scan could be slow at scale
Security ⚠️ Minor — session_id not validated
Test Coverage ⚠️ No automated tests
Documentation ✅ Thorough DEVJOURNAL entry and PR description

Recommendation: Approve with minor changes — address the logging for swallowed exceptions (NousResearch#3) and consider the JSONL scan performance (#2) before merge. The rest are optional improvements.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 10267b6766

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread run_agent.py Outdated
Comment on lines +2486 to +2489
elif role in ("tool", "function"):
tool_results_tokens += _count(item)
else:
user_tokens += _count(item)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Classify Codex payload items by type before defaulting to user

In codex_responses mode, many normalized input items are shaped as {"type": "function_call"} or {"type": "function_call_output"} (without a role), but this classifier only branches on role and then falls through to the user bucket. As a result, tool-call and tool-result tokens are recorded as user, which makes the new payload breakdown endpoint/dashboard materially inaccurate for Codex sessions even when API usage succeeds.

Useful? React with 👍 / 👎.

@ccreview

ccreview Bot commented Mar 12, 2026

Copy link
Copy Markdown

prbot Review (Codex)

Summary

Nice addition: the agent now logs a per-component token estimate per API call and the dashboard exposes a new payload tab with stacked and per-turn views. The implementation is sensible overall and the UI looks polished. I only spotted a few maintainability/perf gaps worth addressing.

P0/P1 Findings

None.

P2 Findings

  • P2: run_agent.py:2442-2460 and pyproject.toml:13-38 — _compute_payload_breakdown now imports and uses tiktoken, but tiktoken is not declared as a direct dependency anywhere. Relying on litellm’s transitive dependency is brittle; if litellm drops it or someone installs the agent with --no-deps, the new feature silently disables itself. Please add tiktoken (and pin it alongside the other core deps) in pyproject.toml/requirements.txt so the runtime requirement is explicit.
  • P2: run_agent.py:2450-2455, dashboard/static/index.html:1315-1326 — When tiktoken isn’t available we try to import it on every API call (raising an ImportError each time) and the dashboard tab just reports “pre-instrumentation data” forever with no hint that instrumentation is disabled. This wastes CPU and makes the new tab unusable without spelunking logs. Cache the failure (e.g., set _tiktoken_enc = False so we don’t re-import) and persist a flag in token_usage.jsonl/API responses so the UI can warn the user that token breakdowns are unavailable because tiktoken isn’t installed rather than implying their data is “old”.
  • P2: dashboard/data.py:225-261 — get_payload_breakdown scans the entire token_usage.jsonl file for every request, even though the new tab encourages repeated ad-hoc queries. That file grows linearly with every model call, so once it reaches tens of thousands of lines each dashboard click will block the aiohttp event loop for noticeable time. Consider indexing the JSONL by session_id (e.g., maintain an in-memory map or move the per-turn payload data into SQLite) or at least caching the parsed file so subsequent requests don’t re-read the whole thing.

Clean Wins

None.

Test Gaps

  • No automated tests exercise _compute_payload_breakdown (run_agent.py:2442-2542) or verify that token_usage.jsonl gains the new breakdown field, so regressions in the instrumentation will go unnoticed.
  • The new data pathway (dashboard/data.py:225-261 and dashboard/server.py:112-156) and UI code aren’t covered, so we don’t have guardrails that the /api/payload-breakdown route returns correctly ordered data or that the dashboard handles empty/error cases.

Overall Verdict

Approve.

@heyalchang
heyalchang merged commit 74b27b8 into local/dashboard-and-tracking Mar 12, 2026
2 checks passed
heyalchang pushed a commit that referenced this pull request Mar 20, 2026
…ult (NousResearch#1922)

SOUL.md now loads in slot #1 of the system prompt, replacing the
hardcoded DEFAULT_AGENT_IDENTITY. This lets users fully customize
the agent's identity and personality by editing ~/.hermes/SOUL.md
without it conflicting with the built-in identity text.

When SOUL.md is loaded as identity, it's excluded from the context
files section to avoid appearing twice. When SOUL.md is missing,
empty, unreadable, or skip_context_files is set, the hardcoded
DEFAULT_AGENT_IDENTITY is used as a fallback.

The default SOUL.md (seeded on first run) already contains the full
Hermes personality, so existing installs are unaffected.

Co-authored-by: Test <test@test.com>
heyalchang pushed a commit that referenced this pull request Mar 20, 2026
Update all SOUL.md documentation to reflect that it now occupies
slot #1 in the system prompt, replacing the hardcoded default identity.

Updated pages:
- user-guide/features/personality.md — SOUL.md is primary identity, not just a layer
- developer-guide/prompt-assembly.md — updated prompt layer order, context files list
- guides/use-soul-with-hermes.md — SOUL.md replaces built-in identity
- user-guide/configuration.md — updated context files table and directory tree

Co-authored-by: Test <test@test.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant