upload large payloads to storage

[dinmukhamedm]

Important

This PR adds functionality to store large payloads in S3 and updates the database and frontend to handle these changes.

• Behavior:
  • Store large payloads in S3 if they exceed PAYLOAD_SIZE_THRESHOLD in store_payloads() in spans.rs.
  • Add input_url and output_url to Span in spans.rs and schema.ts.
  • Fetch payloads from S3 in SpanViewSpan in span-view-span.tsx.
• Database:
  • Add input_url and output_url columns to spans table in 0017_groovy_senator_kelly.sql.
• Frontend:
  • Update route.ts to handle raw payload type.
  • Fetch and display input/output from URLs in span-view-span.tsx.
• Misc:
  • Update drizzle-kit version in package.json.

^{This description was created by for 2f926ad. It will automatically update as commits are pushed.}

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to d4cc761 in 2 minutes and 4 seconds

More details

• Looked at 3233 lines of code in 13 files
• Skipped 1 files when reviewing.
• Skipped posting 3 drafted comments based on config settings.

1. frontend/lib/db/migrations/0017_groovy_senator_kelly.sql:1

• Draft comment:
  Consider adding a migration step to handle existing data for input_url and output_url fields to avoid potential null-related issues.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The columns are being added as nullable text fields, which is a valid and common pattern in database migrations. There's no inherent problem with having nullable URL fields - they might be intentionally optional. Without knowing the business logic or requirements, we can't assume null values are problematic. The comment is speculative about "potential" issues.
  Maybe there are indeed requirements that these fields should never be null, and the comment is trying to prevent a real issue?
  If null values were not allowed, the migration would have included NOT NULL constraints. The author clearly intended these to be nullable fields. We shouldn't speculate about requirements we don't know about.
  This comment should be deleted as it's speculative and assumes requirements we don't have evidence for. Adding nullable columns is a valid pattern.

2. frontend/lib/db/migrations/0017_groovy_senator_kelly.sql:2

• Draft comment:
  Consider adding a migration step to handle existing data for input_url and output_url fields to avoid potential null-related issues.
• Reason this comment was not posted:
  Marked as duplicate.

3. app-server/src/traces/spans.rs:600

• Draft comment:
  Consider using streaming or chunked uploads for better performance when handling large payloads in store_payloads.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  While streaming could potentially improve performance for very large payloads, the current implementation is reasonable for the expected payload sizes. The PAYLOAD_SIZE_THRESHOLD is set to ~448KB which is a moderate size. The comment doesn't provide specific details on how to implement streaming or what the benefits would be. The current implementation using a Vec buffer is simple and works well for these payload sizes.
  The suggestion could be valid for extremely large payloads. If the system regularly handles multi-megabyte payloads, streaming could reduce memory usage.
  Given the moderate threshold size and lack of evidence of performance issues, this optimization seems premature. The current implementation is clean and handles the expected use case well.
  Delete the comment as it suggests a premature optimization without clear evidence of need or specific implementation guidance.

Workflow ID: wflow_S8reCMJ7fPYE4qCm

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

❌ Changes requested. Incremental review on 2f926ad in 58 seconds

More details

• Looked at 155 lines of code in 4 files
• Skipped 0 files when reviewing.
• Skipped posting 4 drafted comments based on config settings.

1. frontend/components/traces/span-view-span.tsx:34

• Draft comment:
  The dependency array for useEffect should be [scrollAreaRef] instead of [scrollAreaRef.current] to avoid unexpected behavior.
• Reason this comment was not posted:
  Comment was on unchanged code.

2. frontend/components/traces/span-view-span.tsx:45

• Draft comment:
  Consider adding error handling for the fetch calls to handle potential network or parsing errors.
• Reason this comment was not posted:
  Marked as duplicate.

3. frontend/app/api/projects/[projectId]/payloads/[payloadId]/route.ts:29

• Draft comment:
  Consider adding validation to ensure AWS credentials are set in environment variables to prevent potential runtime errors.
• Reason this comment was not posted:
  Comment was not on a valid diff hunk.

4. app-server/src/db/spans.rs:103

• Draft comment:
  Consider using named parameters in the SQL query for better readability and maintainability.
• Reason this comment was not posted:
  Confidence changes required: 50%
  In the spans.rs file, the SQL query uses positional parameters which can be error-prone and hard to maintain. Named parameters would improve readability and maintainability.

Workflow ID: wflow_X5rg6DEggo7jjQRc

---

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.

[ellipsis-dev]

Consider adding error handling for the fetch calls to handle potential network or parsing errors. This applies to both inputUrl and outputUrl fetch calls.