Skip to content

Revert "feat: add tier tags in s3"#4730

Merged
tomharmon merged 1 commit intomainfrom
tom/eng-3086
Sep 12, 2025
Merged

Revert "feat: add tier tags in s3"#4730
tomharmon merged 1 commit intomainfrom
tom/eng-3086

Conversation

@tomharmon
Copy link
Contributor

This reverts commit 0e481d6.

@vercel
Copy link

vercel bot commented Sep 11, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
helicone Error Error Sep 11, 2025 8:20pm
helicone-bifrost Ready Ready Preview Comment Sep 11, 2025 8:20pm
helicone-eu Ready Ready Preview Comment Sep 11, 2025 8:20pm

@claude
Copy link
Contributor

claude bot commented Sep 11, 2025

Claude encountered an error —— View job


I'll analyze this and get back to you.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Summary

This PR reverts a previously implemented tier-based tagging feature for S3 storage operations across the Helicone logging system. The revert removes tier functionality from three key files: LoggingHandler.ts, s3Client.ts, and completely deletes tiers.ts.

The original feature added the ability to tag S3 objects with tier information (like 'pro', 'enterprise', etc.) for organizational and potentially billing purposes. The normalizeTier function in tiers.ts was designed to extract base tier names from complex tier strings (e.g., 'pro-variant' → 'pro'). The tier information was being passed through the entire upload pipeline, from the logging handler down to the S3 client methods, where it was applied as object metadata/tags.

This revert simplifies the S3 storage workflow by removing all tier-related parameters, method signatures, and tagging logic. The logging handler no longer extracts or passes tier information, and the S3 client methods have been simplified to their original form without tagging capabilities. This affects how S3 objects are organized and may impact any downstream processes that were relying on tier-based object categorization.

Confidence score: 1/5

  • This PR has critical bugs that will cause runtime failures and should not be merged
  • Score reflects incomplete revert that leaves broken code - the store method still references undefined tags parameter
  • Pay close attention to valhalla/jawn/src/lib/shared/db/s3Client.ts lines 310 where Metadata: tags will fail

3 files reviewed, 1 comment

Edit Code Review Bot Settings | Greptile

}

command = new PutObjectCommand(commandOptions);
Metadata: tags,
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Critical bug: tags parameter may be undefined here since calling methods no longer pass it, but it's still being used in Metadata

Suggested change
Metadata: tags,
Metadata: tags || {},

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is just what it was before i'd changed anything so its fine, https://github.com/Helicone/helicone/blob/21b649fdb1465d3b766e9b3ff851242f2cfd6a7b/valhalla/jawn/src/lib/shared/db/s3Client.ts

i also tested locally and this still logs fine (and made sure it was going into this else)

@tomharmon tomharmon merged commit 717d570 into main Sep 12, 2025
12 of 14 checks passed
@tomharmon tomharmon deleted the tom/eng-3086 branch September 12, 2025 17:46
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.

2 participants