fix: route smolagents OpenInference spans to OpenInferenceSessionMapper

[ybdarrenwang]

Description

Problem

SmolagentsInstrumentor sets its OTel scope to "openinference.instrumentation.smolagents", but OpenInferenceSessionMapper only accepted "openinference.instrumentation.langchain". Smolagents spans were silently filtered out, producing empty Sessions with no tool data.

Fix

Add SCOPES_OPENINFERENCE_FAMILY — a set containing both the langchain and smolagents scope variants. detect_otel_mapper() and OpenInferenceSessionMapper's span filter now check membership in this set. The mapper's existing attribute parsing already handles smolagents' bare-string output.value format correctly.

Related Issues

Documentation PR

Type of Change

Bug fix

Testing

How have you tested the change? Verify that the changes do not break functionality or introduce new warnings.

• I ran hatch run prepare

Checklist

• I have read the CONTRIBUTING document
• I have reviewed and understand every line of code in this PR, including any generated by AI tools, and I can explain why it works
• My change is focused and reasonably small; I have split unrelated work into separate PRs
• I have added any necessary tests that prove my fix is effective or my feature works
• I have updated the documentation accordingly
• I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
• My changes generate no new warnings
• Any dependent changes have been merged and published

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[github-actions]

Assessment: Comment / minor changes requested

Clean, well-scoped bugfix: routing smolagents' OpenInference scope through the existing mapper is the right call, and the SCOPES_OPENINFERENCE_FAMILY frozenset keeps the three touched call sites consistent. Test coverage for scope routing (mapper + detect_otel_mapper) is solid. My one substantive concern is a robustness gap in output.value parsing that this change makes more likely to surface.

Review Categories

• Correctness (Important): Bare-string output.value that parses as valid non-object JSON (numbers, lists, quoted strings) triggers an uncaught AttributeError and the span is silently dropped — reproducing the empty-trace symptom this PR fixes. smolagents tools commonly return such values. See inline comment with a repro and suggested guard.
• Testing: New regression tests are focused and readable. Suggest one more case covering non-dict JSON output.value to close the gap above.
• Naming (minor): SCOPE_OPENINFERENCE is now ambiguous next to SCOPE_OPENINFERENCE_SMOLAGENTS; optional rename for symmetry.

Nice, targeted fix — addressing the non-dict output edge case would make it fully robust for real smolagents traces.

[github-actions]

🔴 Follow-up on the "Important / Correctness" item from my review (re-posting as a general comment — the original inline note targeted src/strands_evals/mappers/openinference_session_mapper.py:300, which is outside the diff hunks so it didn't render as a thread).

Issue: This PR routes smolagents spans into _convert_tool_execution_span, and the PR description states the existing output.value parsing "already handles smolagents' bare-string output format correctly." That's true only for strings that fail JSON parsing. When output.value is a bare string that happens to be valid non-object JSON (a number, list, or quoted string), the current block crashes:

# openinference_session_mapper.py ~L296
if isinstance(output_value, str):
    parsed = json.loads(output_value)          # "42" -> 42, "[1,2,3]" -> list, '"x"' -> str
    tool_output_content = parsed.get("content", str(parsed))  # AttributeError on non-dict
    ...
except json.JSONDecodeError:                    # does NOT catch AttributeError
    tool_output_content = str(output_value)

The AttributeError propagates to the outer handler, which logs Failed to convert span ... and drops the span — producing an empty trace, i.e. the exact symptom this PR is meant to fix. smolagents tools commonly return non-dict values (numbers, lists), so this is likely to be hit in practice.

Verified against the current branch (e16bf0d):

'Temperature: 15C' -> OK -> 'Temperature: 15C'
'42'                -> span dropped ('int' object has no attribute 'get')
'[1, 2, 3]'         -> span dropped ('list' object has no attribute 'get')
'"quoted"'          -> span dropped ('str' object has no attribute 'get')

Suggestion: Only treat parsed as structured when it's a dict, otherwise fall back to the raw string:

if isinstance(output_value, str):
    try:
        parsed = json.loads(output_value)
    except json.JSONDecodeError:
        parsed = None
    if isinstance(parsed, dict):
        tool_output_content = parsed.get("content", str(parsed))
        tool_call_id = parsed.get("tool_call_id")
        tool_status = parsed.get("status", "success")
    else:
        tool_output_content = output_value
elif isinstance(output_value, dict):
    tool_output_content = output_value.get("content", str(output_value))

And add a regression test with a numeric/list/JSON-string output.value to lock it in. Happy to be corrected if smolagents guarantees dict-or-unparseable outputs, but the repro above suggests it doesn't.

[github-actions]

✅ Re-reviewed at dd911a8 — the correctness concern is resolved.

The output.value parsing now isolates json.loads in its own try/except and guards on isinstance(parsed, dict) before calling .get(), falling back to the raw string otherwise. I re-ran my repro against the new commit and all previously-dropped cases now map cleanly:

'Temperature: 15C'   -> 'Temperature: 15C'
'42'                 -> '42'
'[1, 2, 3]'          -> '[1, 2, 3]'
'"quoted"'           -> '"quoted"'
'{"content":"hi"}'   -> 'hi'     (dict path still extracts content)

Thanks also for adding test_smolagents_tool_span_non_dict_json_output — the parametrized coverage across number/list/quoted-string/boolean/null is exactly the kind of regression guard that locks this in, and it goes a bit beyond what I suggested. Full tests/strands_evals/mappers/ passes locally (208 passed; the one failure is an unrelated missing pytest-asyncio plugin in my env).

Minor / optional (non-blocking): the outer fallback was widened to except Exception:. With the inner dict guard now in place that outer catch is essentially defensive, but a bare except Exception will silently swallow any future error here and stringify it, which can mask regressions. If you want to keep it defensive, consider narrowing it (e.g. except (TypeError, ValueError)) or at least logging at debug level. Entirely your call.

Assessment: Approve — the substantive issue from my earlier review is fixed and well-tested. The SCOPE_OPENINFERENCE naming note remains optional. Nice work turning this around.

[github-actions]

🔁 Re-reviewed at 6f3897f4 — this commit meaningfully addresses the open threads from @jjbuck's review. I checked out the branch, verified behavior empirically, and ran the suite (tests/strands_evals/mappers/test_openinference_session_mapper.py: 75 passed). Summary for faster re-review:

Thread-by-thread verification

• Broad except Exception — Resolved. The outer catch is gone; only the inner try/except json.JSONDecodeError + isinstance(parsed, dict) guard remains, so genuine mapper defects now surface via _build_trace() logging instead of being swallowed.
• LLM plural contents path — Resolved. New _normalize_smolagents_llm_attrs rewrites llm.*.message.contents.0.message_content.text → singular .content. The real fixture exercises this and produces InferenceSpans with populated user+assistant content.
• Producer-specific normalization — Resolved. _normalize_smolagents_span is scope-selected in map_to_session() and canonicalizes before the shared conversion runs — matches the "select producer path, feed canonical representation" suggestion.
• Module/class docs + false AGENT claim — Resolved. Docstrings now describe the multi-producer family, and AGENT spans are genuinely supported (_is_agent_invocation_span accepts them when both input & output are present, preserving LangChain rejection).
• Root AGENT span in "full trace" — Resolved. test_agent_span_produces_agent_invocation_span asserts exactly one AgentInvocationSpan from the real CodeAgent.run span.
• Real telemetry fixture — smolagents_live_spans.json (7 spans, instrumentor v0.1.31, correct scope, plural contents path, and the {"args","kwargs","sanitize_inputs_outputs"} wrapper) now backs TestSmolagentsFixtureIntegration, so tests validate real encoding rather than routing alone.

Two minor residual notes on tool-argument normalization (_normalize_smolagents_tool_attrs) — non-blocking, and I verified them against the current code:

1. When both args and kwargs are present, the positional args are silently dropped:

   {"args":[4], "kwargs":{"unit":"C"}, ...}  ->  {"unit": "C"}   # the [4] is lost
   

2. Positional-only calls fall back to {"args":[4]} rather than mapping to named parameters via tool.parameters (the mapping @jjbuck suggested). Instrumentation-only keys are correctly stripped, so this is a smaller concern.

Both are edge cases — the common smolagents kwargs-only path normalizes cleanly to {"query": "..."} and is well tested. Worth a short comment noting the positional-arg limitation, or a focused test if mixed args/kwargs tools are in scope.

Assessment: Approve (pending @jjbuck resolving their own threads). Nice turnaround — the normalization layer plus a real captured fixture is a solid, durable approach.

[github-actions]

🔁 Re-reviewed at 9a29d5f0 — this commit resolves the two follow-up threads @jjbuck reopened on 07-20. I checked out the branch, verified behavior empirically, and ran the suite (test_openinference_session_mapper.py: 79 passed, up from 75).

Thread-by-thread verification

• Positional arg-mapping (test_openinference_session_mapper.py:1409) — Resolved. _normalize_smolagents_tool_attrs now maps positional args to named parameters using tool.parameters order, then merges kwargs. Verified empirically against the fixture:

  • final_answer: {"args": ["The population..."], "kwargs": {}} + tool.parameters {"answer": ...} → {"answer": "The population..."}
  • mixed: {"args": ["weather in Tokyo", 5], "kwargs": {"offset": 10}} → {"query": "weather in Tokyo", "limit": 5, "offset": 10} — positional values no longer dropped
  • fallback to {"args": [...]} only when tool.parameters is absent
    New exact-assertion tests cover all three: test_final_answer_tool_positional_args_normalized (asserts arguments == {"answer": ...} and arguments["answer"] == tool_result.content) and test_mixed_positional_and_kwargs_normalization.

• Task extraction from CodeAgent.run input (test_openinference_session_mapper.py:1444) — Resolved. The AGENT branch now parses input.value and extracts the task field, so user_prompt == "What is the population of Tokyo?" rather than the full {"task": ..., "stream": ..., ...} wrapper. Covered by test_agent_span_extracts_task_from_smolagents_json with an exact assertion.

One minor, non-blocking note: positional→named mapping relies on tool.parameters key order matching the tool's positional-parameter order. This holds for smolagents (parameters are derived from the signature in order, and JSON/dict preserve insertion order), so it's reliable in practice — just worth a one-line comment for future readers.

Assessment: Approve. Both residual threads from my earlier pass are now fully addressed with exact assertions against the real captured fixture — clean, durable resolution.

[github-actions]

🔁 Re-checked at 098d208e (amend of 9a29d5f0) — this is a test-only refactor; mapper source behavior is unchanged (empirical normalization + task-extraction checks produce identical results).

The suite count moved 79 → 70 passed, which I verified is consolidation, not a coverage regression: the previously standalone/inline tests were merged into TestSmolagentsFixtureIntegration using a shared make_span helper and a module-scoped smolagents_session fixture. All the exact assertions from my prior pass are retained:

• test_tool_spans_positional_args_mapped → arguments == {"answer": ...} and == tool_result.content
• test_mixed_positional_and_kwargs_merged → {"query": "tokyo", "limit": 5, "offset": 10}
• test_agent_span_extracts_task_field → user_prompt == "What is the population of Tokyo?"
• new test_langchain_agent_span_rejected explicitly locks in the LangChain AGENT rejection

Assessment: Approve (unchanged). Cleaner test organization with equivalent coverage — no further concerns from my side.

[github-actions]

🔁 Re-checked at 95fd070d — the one delta over the previous commit is an additive defensive test: test_non_dict_json_output_not_crash (parametrized: number / list / quoted-string), verifying a non-dict JSON output.value doesn't crash the TOOL path. Nice edge-case coverage. Source normalization behavior is unchanged (empirical checks identical); suite now 71 passed.

Assessment: Approve (unchanged). The only remaining item is my earlier minor, non-blocking suggestion to add a one-line comment noting the positional→named mapping relies on tool.parameters key order — purely optional. Nothing blocking.