Skip to content
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

added backlink to dataset in spans #178

Merged
merged 1 commit into from
Nov 8, 2024
Merged

added backlink to dataset in spans #178

merged 1 commit into from
Nov 8, 2024

Conversation

skull8888888
Copy link
Collaborator

@skull8888888 skull8888888 commented Nov 8, 2024

Important

This PR adds functionality to link datapoints to spans, updating the database schema, API, and frontend components accordingly.

  • Database:
    • Add datapoint_to_span table with foreign keys to dataset_datapoints and spans in 0004_worthless_kitty_pryde.sql.
    • Update relations.ts and schema.ts to include datapoint_to_span relations.
  • API:
    • Update route.ts in datapoints to handle POST requests linking datapoints to spans.
    • Add new route in spans to retrieve datapoints linked to a span.
  • Frontend:
    • Update dataset.tsx to handle datapoint selection and deletion.
    • Add span-datasets.tsx to display datasets linked to a span.
    • Update export-spans-dialog.tsx to include sourceSpanId when exporting spans to datasets.
    • Modify span-view-span.tsx to include SpanDatasets component.

This description was created by Ellipsis for 1630c4f. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Reviewed everything up to 1630c4f in 1 minute and 33 seconds

More details
  • Looked at 3568 lines of code in 15 files
  • Skipped 0 files when reviewing.
  • Skipped posting 4 drafted comments based on config settings.
1. frontend/app/api/projects/[projectId]/datasets/[datasetId]/datapoints/route.ts:55
  • Draft comment:
    Consider providing more detailed error messages in the response to aid in debugging when validation fails.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The use of safeParse in the POST method is good for validation, but the error message could be more descriptive for better debugging.
2. frontend/app/api/projects/[projectId]/spans/[spanId]/datapoints/route.ts:15
  • Draft comment:
    Consider using a consistent response status code for unauthorized access, such as 403, to align with the rest of the application.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    In the GET method of the new route.ts file, the response status for unauthorized access should be consistent with the rest of the application.
3. frontend/components/traces/traces-table.tsx:143
  • Draft comment:
    Ensure all dependencies used within the useEffect are included in the dependency array to prevent potential bugs.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The useEffect hook in the traces-table.tsx file should include all dependencies that are used within the effect to ensure it runs correctly.
4. frontend/components/traces/trace-view.tsx:70
  • Draft comment:
    Ensure all dependencies used within the useEffect are included in the dependency array to prevent potential bugs.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The useEffect hook in the trace-view.tsx file should include all dependencies that are used within the effect to ensure it runs correctly.

Workflow ID: wflow_X6awhqed3aDZpte3


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@@ -27,28 +28,66 @@ export async function GET(
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a check for the user's session to ensure that the user is authenticated before proceeding with the POST request.

@skull8888888 skull8888888 merged commit 497f343 into dev Nov 8, 2024
1 check passed
@skull8888888 skull8888888 deleted the backlink branch November 8, 2024 07:24
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