Trace server improvements

[nforro]

• Excluded orphan and no-issue traces from recent traces view to prevent visual noise
• Improved and optimized workflow filtering
• Fixed incorrect workflow naming in in-progress traces

[qodo-for-packit]

PR Summary by Qodo

Trace server: filter noisy traces and optimize workflow filtering

✨ Enhancement 🐞 Bug fix 🕐 40+ Minutes

AI Description

• Filter recent traces to show only traces linked to Jira issues.
• Add an indexed workflow_name column and partial-match workflow filtering to speed queries.
• Fix workflow display naming for in-progress traces in both UI and agent instrumentation.

Diagram

graph TD
  A["Ymir agents"] --> B["span_processor / OTLP"] --> C["Trace server ingest"] --> D[("SQLite spans/span_issues")]
  D --> E["query_recent_traces()"] --> F["Recent traces UI"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Keep JSON extraction with expression index

• ➕ Avoids schema migration/backfill complexity
• ➕ No need to store derived data twice
• ➖ SQLite expression indexes + json_extract portability/behavior can be tricky
• ➖ Still couples queries to JSON structure and can be slower than a plain indexed column

2. Normalize workflow names into a separate table

• ➕ Avoids duplicating workflow_name across many spans
• ➕ Enables stronger constraints/consistency for workflow naming
• ➖ More joins and more complex ingest/backfill logic
• ➖ Overkill if workflow_name is only needed for filtering/display

Recommendation: The chosen approach (denormalized spans.workflow_name with an index + one-time backfill) is the best fit here: it removes repeated json_extract usage from hot queries and enables simple case-insensitive partial matching. Keep an eye on backfill cost in init_db (currently guarded by workflow_name IS NULL) and ensure all new ingests populate workflow_name (already done in _extract_spans).

Files changed (8) +41 / -22

Enhancement (1) +29 / -10

server.pyAdd workflow_name column/index and tighten recent traces filtering +29/-10

Add workflow_name column/index and tighten recent traces filtering

• Adds an idempotent SQLite migration to introduce spans.workflow_name, indexes it, and backfills from attributes JSON. Populates workflow_name during ingest and updates recent-trace queries to (1) exclude traces without linked Jira issues, (2) use case-insensitive partial workflow matching, and (3) avoid json_extract in the in-progress path while fixing workflow suffix normalization.

trace_server/server.py

Bug fix (7) +12 / -12

app.jsFix in-progress placeholder workflow naming +1/-1

Fix in-progress placeholder workflow naming

• Prevents double-appending the "Workflow" suffix when naming placeholder root spans derived from workflow.name, keeping in-progress display names consistent.

trace_server/static/app.js

backport_agent.pyUse display workflow name for tracing transactions +2/-2

Use display workflow name for tracing transactions

• Updates start_transaction workflow parameter to the display name (BackportWorkflow) so in-progress traces show the correct workflow label.

ymir/agents/backport_agent.py

mr_consolidation_agent.pyUse display workflow name for tracing transactions +2/-2

Use display workflow name for tracing transactions

• Switches start_transaction workflow parameter to MRConsolidationWorkflow to align trace naming with UI/server expectations.

ymir/agents/mr_consolidation_agent.py

preliminary_testing_agent.pyUse display workflow name for tracing transactions +1/-1

Use display workflow name for tracing transactions

• Changes start_transaction workflow parameter to PreliminaryTestingWorkflow to fix incorrect workflow naming in traces.

ymir/agents/preliminary_testing_agent.py

rebase_agent.pyUse display workflow name for tracing transactions +2/-2

Use display workflow name for tracing transactions

• Updates start_transaction workflow parameter to RebaseWorkflow for consistent trace naming across in-progress and completed traces.

ymir/agents/rebase_agent.py

rebuild_agent.pyUse display workflow name for tracing transactions +2/-2

Use display workflow name for tracing transactions

• Updates start_transaction workflow parameter to RebuildWorkflow so trace workflow labels match the expected display format.

ymir/agents/rebuild_agent.py

triage_agent.pyUse display workflow name for tracing transactions +2/-2

Use display workflow name for tracing transactions

• Updates start_transaction workflow parameter to TriageWorkflow to correct workflow naming in in-progress traces.

ymir/agents/triage_agent.py

[qodo-for-packit]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 7 rules

1. endsWith on non-string workflow ✓ Resolved 🐞 Bug ≡ Correctness ⭐ New

Description

buildSpanTree() and traceWorkflowName() call .endsWith('Workflow') on values returned by
getVal(), but getVal() can return arrays for OTLP arrayValue attributes. For traces where
workflow.name is array-valued, this throws a TypeError and breaks rendering in the recent traces
and per-issue trace views.

Code

trace_server/static/app.js[680]

+        node.name = wfAttr.endsWith('Workflow') ? wfAttr : wfAttr[0].toUpperCase() + wfAttr.slice(1) + 'Workflow';

Relevance

⭐⭐⭐ High

Trace server handles OTLP arrayValue; team likely wants UI type-guards to prevent trace rendering
crashes.

PR-#665
PR-#699

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

getVal() explicitly returns arrays for OTLP arrayValue, but arrays do not implement
String.prototype.endsWith, so the new code will throw before it can compute a fallback label.

trace_server/static/app.js[7-16]
trace_server/static/app.js[676-683]
trace_server/static/app.js[1295-1302]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The UI now calls `wfAttr.endsWith('Workflow')` / `wf.endsWith('Workflow')` without ensuring the workflow value is a string. Because `getVal()` can return arrays for OTLP `arrayValue` attributes, this can throw a `TypeError` and prevent traces from rendering.

### Issue Context
- `getVal()` may return arrays (for `arrayValue`).
- The new `.endsWith(...)` logic is only valid for strings.

### Fix Focus Areas
- trace_server/static/app.js[7-16]
- trace_server/static/app.js[676-683]
- trace_server/static/app.js[1295-1302]

### Suggested fix
Before calling `.endsWith()`, normalize to a string value, e.g.:
- If the value is an array, pick the first string element (or join), otherwise
- If it’s not a string, coerce via `String(...)` (or skip naming).

Example approach:
```js
function workflowLabel(val) {
 if (Array.isArray(val)) val = val.find(v => typeof v === 'string');
 if (typeof val !== 'string' || !val) return null;
 return val.endsWith('Workflow') ? val : val[0].toUpperCase() + val.slice(1) + 'Workflow';
}
```
Then use `workflowLabel(wfAttr)` / `workflowLabel(wf)` in both call sites.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Legacy workflow filter mismatch ✓ Resolved 🐞 Bug ≡ Correctness

Description

query_recent_traces() filters in-progress traces with `LOWER(s.workflow_name) LIKE
%<workflow_param>%, but stored workflow_name` values can legitimately omit the "Workflow" suffix
(the response code conditionally appends it only for display). Filtering by a suffixed value like
"BackportWorkflow" will exclude in-progress traces whose stored workflow_name is the legacy
unsuffixed form (e.g. "backport").

Code

trace_server/server.py[R631-632]

+        inprog_filter = " AND LOWER(s.workflow_name) LIKE ?"
+        inprog_bindings.append(f"%{workflow.lower()}%")

Relevance

⭐⭐⭐ High

Earlier code handled legacy unsuffixed workflows via removesuffix('Workflow'); team likely accepts
avoiding filter regressions.

PR-#699

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The endpoint documentation suggests passing a suffixed workflow name, while the in-progress SQL
filter matches the user string directly against workflow_name. Separately, the response path still
treats missing suffix as normal by appending it only if absent, implying the DB may contain
unsuffixed values that won’t match a suffixed filter string.

trace_server/server.py[25-31]
trace_server/server.py[626-648]
trace_server/server.py[690-702]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `/traces/recent?workflow=...` filter now matches the full user-supplied string against `spans.workflow_name` using `LIKE`. However, the code also indicates `workflow_name` may be stored without the `Workflow` suffix (it appends the suffix only when formatting the response). This makes suffixed filter values fail to match legacy unsuffixed stored values.

## Issue Context
The endpoint docs encourage values like `BackportWorkflow`, and the server formats missing-suffix workflow names for display. The SQL filter should therefore normalize the query parameter (and/or match both forms) to preserve compatibility.

## Fix Focus Areas
- trace_server/server.py[597-648]
- trace_server/server.py[690-709]

## Suggested fix
Normalize the workflow filter before binding:
- Compute `wf = workflow.lower()` and `wf_base = wf.removesuffix('workflow')`.
- Use the base pattern for in-progress filtering (e.g. `LIKE %wf_base%`), or match both patterns with an OR.
- Keep bindings parameterized (no string interpolation of user input into SQL).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Workflow label double-suffix ✓ Resolved 🐞 Bug ≡ Correctness

Description

After this PR, agents emit workflow.name values already ending with "Workflow", but
traceWorkflowName() in the UI still appends "Workflow" unconditionally when it falls back to
workflow.name, producing labels like "TriageWorkflowWorkflow". This mislabels in-progress traces
(where the root workflow span is missing) in the per-issue trace list.

Code

ymir/agents/triage_agent.py[1000]

+        with span_processor.start_transaction(jira_issue, workflow="TriageWorkflow"):

Relevance

⭐⭐⭐ High

Team already added endsWith('Workflow') guard in UI; likely accept fixing remaining double-suffix
label cases.

PR-#699

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The PR changes agent transactions to pass already-suffixed workflow values, and the observability
span processor writes that string to the workflow.name attribute. The issue-detail UI uses
traceWorkflowName() for trace group labels, and that function appends 'Workflow' without
checking for an existing suffix.

ymir/agents/triage_agent.py[998-1004]
ymir/agents/observability.py[55-63]
trace_server/static/app.js[1295-1323]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Agents now set `workflow.name` to values like `TriageWorkflow`, but the UI helper `traceWorkflowName()` still appends `'Workflow'` unconditionally when using the attribute fallback. This can display `TriageWorkflowWorkflow` for traces missing a root workflow span.

## Issue Context
The PR already added an `endsWith('Workflow')` guard in `buildSpanTree()` for placeholder root naming, but the per-issue trace grouping label uses a different function.

## Fix Focus Areas
- trace_server/static/app.js[1295-1302]

## Suggested fix
Update `traceWorkflowName()` to mirror the `endsWith('Workflow')` logic:
- If `wf` is a string and already ends with `'Workflow'`, return it as-is.
- Otherwise, keep the existing capitalization + suffix behavior.
- (Optional hardening) If `wf` is not a string, coerce to string or skip.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

View more (1)

4. Null workflow hides traces ✗ Dismissed 🐞 Bug ◔ Observability

Description

The new HAVING workflow_name IS NOT NULL clause removes Jira-associated in-progress traces from
/traces/recent when none of the joined spans has a non-null workflow_name, even though the trace
is still linked to issues via span_issues. This can make valid active traces disappear from the
recent-traces view solely due to missing workflow metadata.

Code

trace_server/server.py[R644-645]

+            HAVING workflow_name IS NOT NULL
            ORDER BY first_start DESC

Relevance

⭐⭐ Medium

No historical evidence on NULL-workflow exclusion; could be intentional noise reduction for
/traces/recent.

PR-#665
PR-#699

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The in-progress recent-traces query now enforces non-null aggregated workflow_name via HAVING,
which excludes traces with missing workflow metadata. The codebase demonstrates that spans can be
created with jira.issue set but without workflow.name when using jira_issue_context, making
this exclusion possible in practice.

trace_server/server.py[626-648]
ymir/agents/observability.py[26-33]
ymir/agents/observability.py[60-62]
ymir/agents/tests/e2e/backport_agent/test_backport.py[63-75]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`/traces/recent` now requires `workflow_name IS NOT NULL` for in-progress traces, which drops traces that are issue-associated but lack workflow metadata. Previously these could still be returned (with a null workflow) and displayed with a fallback label.

## Issue Context
`jira.issue` and `workflow.name` are set independently (e.g., `jira_issue_context` sets only the issue). Even if most production traces include workflow metadata, dropping the trace entirely makes troubleshooting instrumentation gaps harder.

## Fix Focus Areas
- trace_server/server.py[626-648]
- trace_server/server.py[690-709]

## Suggested fix
Remove `HAVING workflow_name IS NOT NULL` (or only apply it when a workflow filter is provided), and keep the existing response fallback (`workflow: '(in progress)'` when workflow_name is falsy).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Previous review results

Review updated until commit 0aa7bc3

Results up to commit f8f104d ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Workflow label double-suffix ✓ Resolved 🐞 Bug ≡ Correctness

Description

After this PR, agents emit workflow.name values already ending with "Workflow", but
traceWorkflowName() in the UI still appends "Workflow" unconditionally when it falls back to
workflow.name, producing labels like "TriageWorkflowWorkflow". This mislabels in-progress traces
(where the root workflow span is missing) in the per-issue trace list.

Code

ymir/agents/triage_agent.py[1000]

+        with span_processor.start_transaction(jira_issue, workflow="TriageWorkflow"):

Relevance

⭐⭐⭐ High

Team already added endsWith('Workflow') guard in UI; likely accept fixing remaining double-suffix
label cases.

PR-#699

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The PR changes agent transactions to pass already-suffixed workflow values, and the observability
span processor writes that string to the workflow.name attribute. The issue-detail UI uses
traceWorkflowName() for trace group labels, and that function appends 'Workflow' without
checking for an existing suffix.

ymir/agents/triage_agent.py[998-1004]
ymir/agents/observability.py[55-63]
trace_server/static/app.js[1295-1323]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Agents now set `workflow.name` to values like `TriageWorkflow`, but the UI helper `traceWorkflowName()` still appends `'Workflow'` unconditionally when using the attribute fallback. This can display `TriageWorkflowWorkflow` for traces missing a root workflow span.

## Issue Context
The PR already added an `endsWith('Workflow')` guard in `buildSpanTree()` for placeholder root naming, but the per-issue trace grouping label uses a different function.

## Fix Focus Areas
- trace_server/static/app.js[1295-1302]

## Suggested fix
Update `traceWorkflowName()` to mirror the `endsWith('Workflow')` logic:
- If `wf` is a string and already ends with `'Workflow'`, return it as-is.
- Otherwise, keep the existing capitalization + suffix behavior.
- (Optional hardening) If `wf` is not a string, coerce to string or skip.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Legacy workflow filter mismatch ✓ Resolved 🐞 Bug ≡ Correctness

Description

query_recent_traces() filters in-progress traces with `LOWER(s.workflow_name) LIKE
%<workflow_param>%, but stored workflow_name` values can legitimately omit the "Workflow" suffix
(the response code conditionally appends it only for display). Filtering by a suffixed value like
"BackportWorkflow" will exclude in-progress traces whose stored workflow_name is the legacy
unsuffixed form (e.g. "backport").

Code

trace_server/server.py[R631-632]

+        inprog_filter = " AND LOWER(s.workflow_name) LIKE ?"
+        inprog_bindings.append(f"%{workflow.lower()}%")

Relevance

⭐⭐⭐ High

Earlier code handled legacy unsuffixed workflows via removesuffix('Workflow'); team likely accepts
avoiding filter regressions.

PR-#699

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The endpoint documentation suggests passing a suffixed workflow name, while the in-progress SQL
filter matches the user string directly against workflow_name. Separately, the response path still
treats missing suffix as normal by appending it only if absent, implying the DB may contain
unsuffixed values that won’t match a suffixed filter string.

trace_server/server.py[25-31]
trace_server/server.py[626-648]
trace_server/server.py[690-702]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `/traces/recent?workflow=...` filter now matches the full user-supplied string against `spans.workflow_name` using `LIKE`. However, the code also indicates `workflow_name` may be stored without the `Workflow` suffix (it appends the suffix only when formatting the response). This makes suffixed filter values fail to match legacy unsuffixed stored values.

## Issue Context
The endpoint docs encourage values like `BackportWorkflow`, and the server formats missing-suffix workflow names for display. The SQL filter should therefore normalize the query parameter (and/or match both forms) to preserve compatibility.

## Fix Focus Areas
- trace_server/server.py[597-648]
- trace_server/server.py[690-709]

## Suggested fix
Normalize the workflow filter before binding:
- Compute `wf = workflow.lower()` and `wf_base = wf.removesuffix('workflow')`.
- Use the base pattern for in-progress filtering (e.g. `LIKE %wf_base%`), or match both patterns with an OR.
- Keep bindings parameterized (no string interpolation of user input into SQL).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Null workflow hides traces ✗ Dismissed 🐞 Bug ◔ Observability

Description

The new HAVING workflow_name IS NOT NULL clause removes Jira-associated in-progress traces from
/traces/recent when none of the joined spans has a non-null workflow_name, even though the trace
is still linked to issues via span_issues. This can make valid active traces disappear from the
recent-traces view solely due to missing workflow metadata.

Code

trace_server/server.py[R644-645]

+            HAVING workflow_name IS NOT NULL
            ORDER BY first_start DESC

Relevance

⭐⭐ Medium

No historical evidence on NULL-workflow exclusion; could be intentional noise reduction for
/traces/recent.

PR-#665
PR-#699

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The in-progress recent-traces query now enforces non-null aggregated workflow_name via HAVING,
which excludes traces with missing workflow metadata. The codebase demonstrates that spans can be
created with jira.issue set but without workflow.name when using jira_issue_context, making
this exclusion possible in practice.

trace_server/server.py[626-648]
ymir/agents/observability.py[26-33]
ymir/agents/observability.py[60-62]
ymir/agents/tests/e2e/backport_agent/test_backport.py[63-75]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`/traces/recent` now requires `workflow_name IS NOT NULL` for in-progress traces, which drops traces that are issue-associated but lack workflow metadata. Previously these could still be returned (with a null workflow) and displayed with a fallback label.

## Issue Context
`jira.issue` and `workflow.name` are set independently (e.g., `jira_issue_context` sets only the issue). Even if most production traces include workflow metadata, dropping the trace entirely makes troubleshooting instrumentation gaps harder.

## Fix Focus Areas
- trace_server/server.py[626-648]
- trace_server/server.py[690-709]

## Suggested fix
Remove `HAVING workflow_name IS NOT NULL` (or only apply it when a workflow filter is provided), and keep the existing response fallback (`workflow: '(in progress)'` when workflow_name is falsy).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[nforro]

/agentic_review

[qodo-for-packit]

Code review by qodo was updated up to the latest commit 6d4a54d

[nforro]

/agentic_review

[qodo-for-packit]

Code review by qodo was updated up to the latest commit 19d004b

[opohorel]

LGTM