-
Notifications
You must be signed in to change notification settings - Fork 48
fix(spans): Get spans params #121
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: main
Are you sure you want to change the base?
Conversation
WalkthroughStandardized Spans API docs: renamed multiple query parameters and a response field from camelCase to snake_case, added Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🔇 Additional comments (4)
Comment |
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.
Important
Looks good to me! 👍
Reviewed everything up to 0b9d72e in 1 minute and 47 seconds. Click for details.
- Reviewed
47lines of code in1files - Skipped
0files when reviewing. - Skipped posting
5draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. api-reference/warehouse/get_spans.mdx:22
- Draft comment:
Changed query param to snake_case: ensure consistency with other time params. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. api-reference/warehouse/get_spans.mdx:26
- Draft comment:
Renamed 'toTimestampSec' to 'to_timestamp_sec' for snake_case consistency. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, as it only states that a function was renamed for consistency. It does not provide any actionable feedback or suggestions for improvement.
3. api-reference/warehouse/get_spans.mdx:42
- Draft comment:
Updated 'sortOrder' to 'sort_order' to match snake_case naming conventions. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, as it only states what was changed without providing any suggestion, question, or request for confirmation. It doesn't align with the rules for useful comments.
4. api-reference/warehouse/get_spans.mdx:63
- Draft comment:
Enhanced filter parameter description—ensure that 'URL-encoded' note and supported operators match the backend implementation. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking the author to ensure that the description matches the backend implementation, which is a form of asking for confirmation or verification. This violates the rule against asking the author to confirm or ensure things.
5. api-reference/warehouse/get_spans.mdx:106
- Draft comment:
Renamed response field 'spanName' to 'span_name'. Update example responses accordingly for consistency. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, as it is just stating that a field was renamed and suggesting to update example responses for consistency. It does not provide a specific code suggestion or ask for a test to be written.
Workflow ID: wflow_g7M8f35RiMUAJm9T
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
api-reference/warehouse/get_spans.mdx(4 hunks)
🔇 Additional comments (2)
api-reference/warehouse/get_spans.mdx (2)
22-22: Verify parameter naming reflects actual backend API expectations.The documentation does show mixed parameter naming conventions (snake_case:
from_timestamp_sec,to_timestamp_sec,sort_orderand camelCase:envProjectIDs,workflowName,sortBy,compositeCursor,logicalOperator). However, without access to the backend API implementation in this repository, it cannot be confirmed whether this mixed naming is intentional (reflecting actual API parameter names) or a documentation oversight.Clarify with the backend team whether the API actually expects these exact parameter names with mixed conventions, or if they should be standardized in both the API and documentation.
78-172: Correct the documentation field namespan_nametospanNameto match the example response.The review comment's premise is incorrect. Query parameters are not standardized to snake_case—they use mixed naming conventions (
from_timestamp_sec,sort_orderalongsideworkflowName,spanName,sortBy).However, there is a real documentation error: the response field is documented as
"span_name"but the example response shows"spanName"in camelCase. Update line 78-172 to change thespan_namefield documentation tospanNameto align with the actual example response and the camelCase convention used throughout the response object.Likely an incorrect or invalid review comment.
| <ResponseField name="span_name" type="string"> | ||
| The name of the span. | ||
| </ResponseField> |
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.
Critical: Example response doesn't match updated field name documentation.
Line 106 documents the response field as span_name (snake_case), but the example response on line 186 still has "spanName" (camelCase). This creates a breaking inconsistency for API consumers.
Update the example response to match the new field name:
"parentSpanId": "span_parent_789",
"traceState": "",
- "spanName": "llm.completion",
+ "span_name": "llm.completion",
"spanKind": "CLIENT",Also applies to: 186-186
🤖 Prompt for AI Agents
In api-reference/warehouse/get_spans.mdx around lines 106-108 (and example at
line 186), the docs define the response field as "span_name" (snake_case) but
the example JSON still uses "spanName" (camelCase); update the example response
at line 186 (and any other examples) to use "span_name" instead of "spanName" so
the example matches the documented field name, and run a quick search in this
file to replace any remaining camelCase occurrences of this field.
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.
Important
Looks good to me! 👍
Reviewed 5693a0e in 1 minute and 53 seconds. Click for details.
- Reviewed
33lines of code in1files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. api-reference/warehouse/get_spans.mdx:27
- Draft comment:
Update query parameter to snake_case: 'workflowName' → 'workflow'. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. api-reference/warehouse/get_spans.mdx:38
- Draft comment:
Convert parameter 'spanName' to snake_case ('span_name'). - Reason this comment was not posted:
Comment looked like it was already resolved.
3. api-reference/warehouse/get_spans.mdx:66
- Draft comment:
Remove deprecated 'logicalOperator' parameter; ensure clients are aware. - Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_HxNiDHqnz9DwrBsV
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Important
Updated Spans API documentation to use snake_case for parameters and response fields, and enhanced filters documentation.
get_spans.mdxto snake_case:from_timestamp_sec,to_timestamp_sec,workflow,span_name,sort_order.spanNametospan_nameinget_spans.mdx.get_spans.mdxwith structure, supported operators, and URL-encoding info.This description was created by
for 5693a0e. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit