Skip to content

fix(ingest): rank gen_ai.conversation.id before session.id for OpenInference-style vendors - #1177

Merged
JeremyFunk merged 1 commit into
mainfrom
fix/ingest-conversation-id-before-session-id
Sep 30, 2026
Merged

JeremyFunk merged 1 commit into
mainfrom
fix/ingest-conversation-id-before-session-id

Conversation

@JeremyFunk

@JeremyFunk JeremyFunk commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

Why

Several vendors read bare session.id before gen_ai.conversation.id when grouping spans into agent sessions. The two keys usually carry the same value, because OpenInference's dual-write copies it. When they differ, the GenAI-standard conversation id should win. session.id is a generic key: a customer's own middleware can stamp a web-login session on every span, and Maple's browser SDK stamps its replay session there. Either one can span several conversations and merge them into one Maple session.

Change

apps/ingest/src/ai_session.rs: a new CONVERSATION_THEN_SESSION_ID key list (gen_ai.conversation.id, then session.id), used by the vendors below. A span with only session.id still groups by it. Because these lists now include the conversation id, run_predicates no longer appends it as a trailing fallback for them (the fallback added in #1139).

Vendor Before After
dspy, agno, openinference-openai, crewai, smolagents, strands session.id, then the fallback conversation id, session.id
haystack (OpenInference scope), llamaindex, openai_agents_sdk session.id, conversation id conversation id, session.id
langchain LangSmith thread, session.id, conversation id LangSmith thread (still first), conversation id, session.id
unknown:openinference already conversation id first same, now through the shared list

Unchanged:

  • claude_agent_sdk: its session.id is Claude Code's own session id, which is the correct session key.
  • openrouter: its session.id echoes the caller's OpenRouter session_id request field. That is OpenRouter's own session key, and it is how a call joins the framework session that tagged it.
  • google_adk, pydantic_ai: these already read the conversation id before session.id.
  • Vendors whose native key is not session.id (maple, eve, spring_ai, vercel_ai_sdk, the conversation-id-only dialects).

Tests

openinference_session_id_ranks_below_the_conversation_id checks every affected vendor two ways: with both keys set, the conversation id wins; with only session.id, the span still groups by it. It also checks that claude_agent_sdk and openrouter keep their session.id when both keys are set. It replaces vendors_with_their_own_key_fall_back_to_the_conversation_id, which asserted the old order. The OpenInference Haystack case in session_key_order_follows_the_emitting_dialect is dropped because the new test covers it. cargo test --lib in apps/ingest: 204 passed.

Note

This is an ingest stamp, so it applies only to newly ingested spans. Rows already stamped keep their session id.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Summary by CodeRabbit

  • Bug Fixes
    • Session identification now prefers conversation IDs over generic session IDs for most supported formats, while retaining dedicated vendor-specific keys and existing precedence for Claude Code and OpenRouter.

@maple-review-bot

maple-review-bot Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 4/5 · likely safe to merge
A data-list reorder confined to apps/ingest/src/ai_session.rs, with the new test exercising both-keys and session-only inputs for every affected vendor.
quality 100/100 · no findings · tests covered · risk medium

Ranks gen_ai.conversation.id ahead of bare session.id in the session-key list of the OpenInference-style vendors, keeping claude_agent_sdk and openrouter on session.id and LangSmith's thread first for langchain. The reorder is confined to the key tables and their test; safe to merge.

  • CONVERSATION_THEN_SESSION_ID puts gen_ai.conversation.id before session.id
  • dspy, agno, crewai, smolagents, strands, openinference-openai, llamaindex, haystack-OI and openai_agents_sdk use it
  • langchain keeps langsmith.metadata.thread_id first, then the conversation id, then session.id
What was checked
  • run_predicates fallback (ai_session.rs:1110) skips a list that already ranks the conversation id, so no key is looked up twice
  • No session_keys list now repeats a key, and langchain's list carries the conversation id once (ai_session.rs:954)
  • claude_agent_sdk and openrouter still list session.id alone (ai_session.rs:906, ai_session.rs:968), so they keep their own key when both are set

5ffc5df · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 80ad188a-e9e1-4951-86d9-1030ff0008c9

📥 Commits

Reviewing files that changed from the base of the PR and between 2e91012 and 5ffc5df.

📒 Files selected for processing (1)
  • apps/ingest/src/ai_session.rs

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Session-key lookup precedence changes across several OpenInference and native dialects. The updated tests check when both identifiers exist, when only session.id exists, and the retained precedence for Claude Code and OpenRouter.

Changes

Session key precedence

Layer / File(s) Summary
Configure session-key precedence
apps/ingest/src/ai_session.rs
Several dialects now check gen_ai.conversation.id before session.id. LangChain still checks its LangSmith thread key first.
Test session-key selection
apps/ingest/src/ai_session.rs
Tests cover conversation-ID selection, session.id fallback, and retained session.id precedence for Claude Code and OpenRouter.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: makisuo

Merge Risk: ⚪ Minimal · up to 5ffc5

This change only alters which session key is preferred for newly ingested spans. Tests cover the new ordering and the fallback, and no merge-blocking risk was found.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 5ffc5

Conversation identifiers now take precedence for affected vendors without replacing tenant access controls. The main compatibility effect is that historical data and mixed-version ingestion can retain different session groupings.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The inspected effect is conversation grouping within the separately supplied organization context. Selecting a conversation identifier does not replace the organization used for native acceptance or session-read query compilation.

Trust Boundaries and Controls

  • observed — Dashboard session handlers obtain CurrentTenant context, and the read service supplies tenant.orgId separately from requested session identifiers. The inspected schema also represents OrgId and SessionId as distinct columns.

Resilience and Maintainability Implications

  • observed — Native commitment reserves capacity before WAL append. Uncommitted organization-byte reservations release on cancellation or error; export advances the WAL cursor after delivery or an intentional terminal drop. Recovery reads stored organization and payload bytes without rerunning session selection, preserving their association across that recovery path.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: prioritizing gen_ai.conversation.id over session.id for OpenInference-style vendors.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Warning

Some tools did not complete. Review the errors below.

🔧 Clippy (1.98.1)

Clippy execution failed


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@JeremyFunk
JeremyFunk merged commit 314e260 into main Sep 30, 2026
38 checks passed
@JeremyFunk
JeremyFunk deleted the fix/ingest-conversation-id-before-session-id branch September 30, 2026 13:29
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