Skip to content

Trace improvements#2502

Merged
shagun-singh-inkeep merged 4 commits intomainfrom
TI-26
Mar 3, 2026
Merged

Trace improvements#2502
shagun-singh-inkeep merged 4 commits intomainfrom
TI-26

Conversation

@shagun-singh-inkeep
Copy link
Copy Markdown
Collaborator

  • filter conversations with errors

  • message contents should render new lines

  • agent name discrepancy

@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 3, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agents-manage-ui Ready Ready Preview, Comment Mar 3, 2026 10:53pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
agents-api Ignored Ignored Preview Mar 3, 2026 10:53pm
agents-docs Ignored Ignored Preview Mar 3, 2026 10:53pm

Request Review

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Mar 3, 2026

🦋 Changeset detected

Latest commit: 18adcb0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 10 packages
Name Type
@inkeep/agents-manage-ui Patch
@inkeep/agents-api Patch
@inkeep/agents-cli Patch
@inkeep/agents-core Patch
@inkeep/agents-email Patch
@inkeep/agents-mcp Patch
@inkeep/agents-sdk Patch
@inkeep/agents-work-apps Patch
@inkeep/ai-sdk-provider Patch
@inkeep/create-agents Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pullfrog
Copy link
Copy Markdown
Contributor

pullfrog Bot commented Mar 3, 2026

Improves trace observability by fixing the agent.name span attribute and adding an "Errors only" filter to the traces UI.

API (agents-api)

  • Adds agentName to AgentConfig and passes it from generateTaskHandler.ts so agent.name in trace spans uses the actual agent name instead of the (missing) config.name
  • Sets agent.name attribute on the chat route's message span (chat.ts)

Traces UI (agents-manage-ui)

  • Adds an "Errors only" toggle button to the traces page filter bar
  • Threads hasErrors through query state (use-traces-query-state.ts), the stats hook (use-traces.ts), and the SigNoz API client (signoz-stats.ts) to filter on hasError = true
  • Renders message content with whitespace-pre-wrap and unescaped newlines in both timeline items and the detail panel

Pullfrog  | View workflow run | Using Claude Code | Triggered by Pullfrogpullfrog.com𝕏

Copy link
Copy Markdown
Contributor

@pullfrog pullfrog Bot left a comment

Choose a reason for hiding this comment

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

Clean PR. Backend changes correctly fix the agent name discrepancy in span attributes, and the error filter + newline rendering changes are well-plumbed. One accessibility nit on the toggle button.

Pullfrog  | View workflow runpullfrog.com𝕏

Comment on lines +248 to +260
<Button
variant="gray-outline"
size="sm"
onClick={() => setHasErrorsFilter(!hasErrors)}
className={
hasErrors
? 'border-destructive/40 text-destructive hover:bg-destructive/5 hover:text-destructive'
: ''
}
>
<CircleAlert className="h-3.5 w-3.5" />
Errors only
</Button>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This button acts as a toggle — add aria-pressed={hasErrors} so screen readers announce the on/off state.

Suggested change
<Button
variant="gray-outline"
size="sm"
onClick={() => setHasErrorsFilter(!hasErrors)}
className={
hasErrors
? 'border-destructive/40 text-destructive hover:bg-destructive/5 hover:text-destructive'
: ''
}
>
<CircleAlert className="h-3.5 w-3.5" />
Errors only
</Button>
<Button
variant="gray-outline"
size="sm"
onClick={() => setHasErrorsFilter(!hasErrors)}
aria-pressed={hasErrors}
className={
hasErrors
? 'border-destructive/40 text-destructive hover:bg-destructive/5 hover:text-destructive'
: ''
}
>
<CircleAlert className="h-3.5 w-3.5" />
Errors only
</Button>

Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

PR Review Summary

(2) Total Issues | Risk: Low

🟡 Minor (2) 🟡

Inline Comments:

  • 🟡 Minor: traces/page.tsx:248-260 Missing aria-pressed for toggle button accessibility
  • 🟡 Minor: .changeset/moody-deer-own.md:6 Changeset message too vague per AGENTS.md guidelines

💭 Consider (1) 💭

Inline Comments:

  • 💭 Consider: Agent.ts:207 agentName optionality may cause undefined span attributes for nested sub-agents

💡 APPROVE WITH SUGGESTIONS

Summary: This PR cleanly implements three trace improvements: an error filter for conversations, newline rendering in message content, and an agent name span attribute fix. The changes follow established patterns (e.g., hasErrors filter mirrors the existing agentId filter). The two minor issues are: (1) missing aria-pressed on the toggle button for accessibility, and (2) the changeset message being too vague. The optional agentName field may cause undefined span attributes for nested sub-agents, but this is a consideration rather than a blocker since the primary use case (top-level agent spans) is correctly handled.

Discarded (8)
Location Issue Reason Discarded
signoz-stats.ts Missing unit tests for hasErrors filter logic Pre-existing condition — no unit tests exist for this module, not introduced by this PR
generateTaskHandler.ts:200 Missing test for agentName propagation Valid concern but test infrastructure setup is out of PR scope; flagged in Consider section instead
Agent.ts:2412 Missing test for span attribute change Pre-existing test gap, not introduced by this PR
timeline-item.tsx:86 Newline transformation runs on every render Negligible performance impact for typical list sizes; pattern is acceptable
render-panel-content.tsx:264 String transformation at render time Panel only renders when selected; acceptable pattern
signoz-stats.ts filter INFO: follows established patterns Positive finding, not an issue
use-traces-query-state.ts INFO: follows established patterns Positive finding, not an issue
chat.ts:326 INFO: span attribute naming consistent Positive finding, not an issue
Reviewers (5)
Reviewer Returned Main Findings Consider While You're Here Inline Comments Pending Recs Discarded
pr-review-standards 1 0 0 0 1 0 0
pr-review-frontend 4 0 0 0 1 0 3
pr-review-consistency 6 0 0 0 0 0 6
pr-review-tests 4 0 0 0 0 0 4
pr-review-types 1 0 0 0 1 0 0
Total 16 0 0 0 3 0 13

Note: Many consistency and test findings were positive confirmations ("follows existing patterns") or pre-existing gaps not introduced by this PR.

Comment thread .changeset/moody-deer-own.md Outdated
tenantId: string;
projectId: string;
agentId: string;
agentName?: string;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💭 Consider: Consider whether agentName should have a fallback

Issue: agentName is optional, but nested sub-agent relations (e.g., subAgentRelations, delegateRelations in generateTaskHandler.ts lines 210-262) don't propagate this field. When a sub-agent creates its span, agent.name will be undefined.

Why: While the top-level agent correctly sets agentName: currentAgent.name at line 200, nested agents constructed from relations don't include agentName. This could result in missing agent.name span attributes for sub-agent traces, affecting SigNoz filtering/grouping.

Fix: Consider adding a fallback at the span attribute level:

'agent.name': this.config.agentName ?? this.config.name,

Or ensure agentName is propagated in nested relation configs.

Refs:

@github-actions github-actions Bot deleted a comment from claude Bot Mar 3, 2026
…s/page.tsx

Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
@vercel vercel Bot temporarily deployed to Preview – agents-docs March 3, 2026 22:51 Inactive
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
@shagun-singh-inkeep shagun-singh-inkeep merged commit a175379 into main Mar 3, 2026
9 checks passed
@shagun-singh-inkeep shagun-singh-inkeep deleted the TI-26 branch March 3, 2026 22:54
Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

PR Review Summary

(0) Total Issues | Risk: Low

✅ Delta Review — Prior Feedback Addressed

This is a re-review scoped to the 2 commits since the last automated review. Both issues raised in the prior review have been correctly addressed:

Prior Issue Status Fix
🟡 Changeset message too vague ✅ Fixed Updated to "Add hasErrors filter to traces page and fix agent.name span attribute" — follows AGENTS.md guidelines (sentence case, action verb, specific)
🟡 Missing aria-pressed for accessibility ✅ Fixed Added aria-pressed={hasErrors} and aria-hidden="true" on the icon

No new issues were introduced in the delta.


✅ APPROVE

Summary: All prior review feedback has been addressed. The changeset message now clearly describes the changes, and the error filter toggle button is accessible to screen reader users. Ship it! 🚀

Reviewers (0)
Reviewer Returned Main Findings Consider While You're Here Inline Comments Pending Recs Discarded
N/A — delta-only re-review

Note: Sub-reviewers were not dispatched for this re-review because the delta contained only fixes for prior review feedback with no new logic changes.

@github-actions github-actions Bot deleted a comment from claude Bot Mar 3, 2026
@inkeep
Copy link
Copy Markdown
Contributor

inkeep Bot commented Mar 3, 2026

No documentation updates required for this PR.

The changes are:

  • Error filter (hasErrors): UI enhancement for filtering conversations with errors
  • Newline rendering: Visual improvement for message content display
  • Agent name fix: Internal tracing attribute correction

These are UI/internal improvements that don't require user-facing documentation updates.

@robert-inkeep
Copy link
Copy Markdown
Collaborator

@claude
a.messageContent?.replaceAll('\n', '\n')

check this
i think this means we are double encoding it somewhere, or not decoding it right
and it doesnt cover text serialization generally
like even \t or \r would not work here

@inkeep-internal-ci
Copy link
Copy Markdown
Contributor

inkeep-internal-ci Bot commented Mar 3, 2026

Claude finished @robert-inkeep's task in 4m 22s —— View job


Investigating double-encoding of message content in traces

  • Gather context — read the UI files using replaceAll('\\n', '\n')
  • Trace the data flow upstream — find where message content is serialized/stored
  • Identify the root cause of double encoding
  • Provide findings and recommendation

You're right — the .replaceAll('\\n', '\n') is a band-aid that only handles \n and misses \t, \r, \", etc. Here's the full data flow and where the issue lives:

Data flow

  1. API sets span attributechat.ts:324, chatDataStream.ts:348, mcp.ts:213, TriggerService.ts:813 all set 'message.content' as a plain string with actual newline characters (extracted via extractTextFromParts() in message-parts.ts:79-84)
  2. OTEL exports to SigNoz — the OTEL SDK serializes the attribute and sends it to the collector, which stores it in ClickHouse
  3. SigNoz API returns data — the conversation route (agents-manage-ui/src/app/api/signoz/conversations/[conversationId]/route.ts) queries it back with STRING_TAG field config and reads it via getString() (line 57-60), which just returns the raw value
  4. UI renderstimeline-item.tsx:86 and render-panel-content.tsx:264 apply the workaround

Root cause

The escaping happens in the ClickHouse/SigNoz storage layer. ClickHouse stores string values with C-style escape sequences — actual newlines (0x0A) become the literal two-character string \n, tabs become \t, etc. When queried back via the SigNoz API, these come back as literal escape sequences, not as the original characters. This is a known ClickHouse behavior with its string handling/output formats.

This is not a double-encoding bug in our code — our API correctly sets plain text attributes. The escaping is introduced by the storage/query layer we don't control.

Recommended fix

Instead of scattered .replaceAll('\\n', '\n') calls at render time, fix it once at the data ingestion boundary:

Option A — Fix in getString() helper (line 57-60 of the conversation route):

function unescapeCStyleString(s: string): string {
  return s.replace(/\\([nrtv0'"\\])/g, (_, ch) => {
    const map: Record<string, string> = {
      n: '\n', r: '\r', t: '\t', v: '\v',
      '0': '\0', "'": "'", '"': '"', '\\': '\\',
    };
    return map[ch] ?? ch;
  });
}

function getString(span: SigNozListItem, key: string, fallback = ''): string {
  const v = getField(span, key);
  const raw = typeof v === 'string' ? v : v == null ? fallback : String(v);
  return unescapeCStyleString(raw);
}

This is the cleanest approach because it fixes the problem at the boundary where SigNoz data enters our system. All downstream consumers (timeline items, panel content, any future components) would get properly unescaped strings. Then the two .replaceAll('\\n', '\n') workarounds in timeline-item.tsx:86 and render-panel-content.tsx:264 can be removed.

Risk: This would unescape all string attributes, not just message content. For most attributes (span IDs, agent names, timestamps) this is a no-op since they don't contain escape sequences. But worth verifying that no attribute intentionally stores literal \n text.

Option B — If you want to be more targeted, apply unescapeCStyleString only when building the messageContent field in the ActivityItem mapping (line 1668):

messageContent: unescapeCStyleString(getString(span, SPAN_KEYS.MESSAGE_CONTENT, '')),

This is safer but means you'd need to add it to every string field that could contain user-authored text.

@robert-inkeep
Copy link
Copy Markdown
Collaborator

@claude the content though for my spans for ai assistant response look fine though, the text looks right with all the right line breaks

@inkeep-internal-ci
Copy link
Copy Markdown
Contributor

inkeep-internal-ci Bot commented Mar 3, 2026

Claude finished @robert-inkeep's task in 3m 26s —— View job


Investigating span content encoding for AI assistant responses

  • Gather context — read the relevant code files
  • Analyze the data flow from span attributes to UI rendering
  • Provide findings

Good observation — that's actually useful for narrowing down where the escaping happens. Here's what I found:

Both message.content (user messages) and ai.response.content (AI responses) follow the exact same path:

  • Both are set as plain text strings on span attributes in the API (chat.ts:324, executionHandler.ts:509)
  • Both are queried from SigNoz with the same STRING_TAG field config (lines 517 and 603 in the conversation route)
  • Both are read back via the same getString() helper (lines 1668 and 1699)

If your spans show the right line breaks in SigNoz's own UI, that confirms the escaping is not in ClickHouse storage — it's happening somewhere in the SigNoz API response → our conversation route pipeline. The previous analysis attributing it to ClickHouse was likely wrong.

Why AI assistant responses look fine in the traces UI despite the same escaping: they're rendered through <Streamdown> (a markdown component) in both timeline-item.tsx:317 and render-panel-content.tsx:104, which likely handles the literal \n sequences. User messages render as raw text ({messageContent?.replaceAll('\\n', '\n')} at timeline-item.tsx:86), hence the band-aid.

The recommendation from the prior analysis still holds — fix it once at the getString() boundary with a proper C-style unescape function, and remove the scattered .replaceAll('\\n', '\n') calls. This would correctly handle \t, \r, etc. too.

The remaining question is whether the escaping is:

  1. In the SigNoz API response format (likely — ClickHouse JSON output format escapes string values)
  2. In the axios/JSON parsing layer

You could verify by logging the raw axios response for a conversation that has newlines in message.content to see exactly what comes back from the SigNoz API.


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.

2 participants