-
Notifications
You must be signed in to change notification settings - Fork 0
Feature/tl translator evals admehra #37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: genai-utils-e2e-dev
Are you sure you want to change the base?
Feature/tl translator evals admehra #37
Conversation
Signed-off-by: Pavan Sudheendra <[email protected]>
Signed-off-by: Pavan Sudheendra <[email protected]>
Signed-off-by: Pavan Sudheendra <[email protected]>
Signed-off-by: Pavan Sudheendra <[email protected]>
Signed-off-by: Pavan Sudheendra <[email protected]>
Signed-off-by: Pavan Sudheendra <[email protected]>
Signed-off-by: Pavan Sudheendra <[email protected]>
Signed-off-by: Pavan Sudheendra <[email protected]>
There was a problem hiding this comment.
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.
ℹ️ 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".
| # Set the sentinel attribute immediately on the new span to prevent recursion | ||
| if invocation.span and invocation.span.is_recording(): | ||
| invocation.span.set_attribute("_traceloop_translated", True) | ||
| # Store the mapping from original span_id to translated INVOCATION (we'll close it later) | ||
| if original_span_id: | ||
| self._original_to_translated_invocation[ | ||
| original_span_id | ||
| ] = invocation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LLM child spans lose parent links when translated
The span processor relies on _original_to_translated_invocation to locate a translated parent before calling start_llm, but entries are added to this map only after the synthetic span has already been started (lines shown). Because on_end processes LLM-like spans immediately while the real parent is still open, a child LLM span finishes before its parent span has been translated and inserted into the map. The lookup performed earlier in _process_span_translation therefore returns None, so the synthetic span is started as a root and the hierarchy between agent/task and child LLM spans is lost. This breaks parent–child relationships for most nested LLM invocations. Consider deferring translation until parents are processed or caching children until the parent mapping is available.
Useful? React with 👍 / 👎.
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.