Skip to content

Conversation

@shellmayr
Copy link
Member

@shellmayr shellmayr commented Nov 7, 2025

  • Realized in this discussion that we don't handle the case of the single large message well at the moment

Truncate the message list to

  • the last message, with its content truncated by characters, if the last message's serialized size exceeds a byte threshold; otherwise,
  • the maximum number of messages, starting from the end of the list, whose total serialized size does not exceed a byte threshold.

Contributes to TET-1208

@shellmayr shellmayr marked this pull request as ready for review November 7, 2025 08:40
@shellmayr shellmayr requested a review from a team as a code owner November 7, 2025 08:40
@shellmayr shellmayr requested review from a team and ArthurKnaus November 7, 2025 08:40
@linear
Copy link

linear bot commented Nov 7, 2025

Copy link
Contributor

@alexander-alderman-webb alexander-alderman-webb left a comment

Choose a reason for hiding this comment

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

Looks good in terms of the general approach.

The message mutation also to be addressed; we should not edit the return value of another library and return the edited value to the user.

@alexander-alderman-webb alexander-alderman-webb marked this pull request as draft November 12, 2025 09:32
@codecov
Copy link

codecov bot commented Nov 12, 2025

Codecov Report

❌ Patch coverage is 88.88889% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.92%. Comparing base (659bd84) to head (768a5b7).
⚠️ Report is 7 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
sentry_sdk/ai/utils.py 88.88% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5079   +/-   ##
=======================================
  Coverage   83.92%   83.92%           
=======================================
  Files         179      179           
  Lines       17948    17967   +19     
  Branches     3193     3198    +5     
=======================================
+ Hits        15062    15079   +17     
- Misses       1913     1914    +1     
- Partials      973      974    +1     
Files with missing lines Coverage Δ
sentry_sdk/ai/utils.py 91.20% <88.88%> (-0.80%) ⬇️

... and 3 files with indirect coverage changes

@alexander-alderman-webb alexander-alderman-webb marked this pull request as ready for review November 12, 2025 11:41
Comment on lines -229 to +230
if result:
assert result[-1] == large_messages[-1]
assert result[-1] == large_messages[-1]
Copy link
Contributor

@alexander-alderman-webb alexander-alderman-webb Nov 12, 2025

Choose a reason for hiding this comment

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

Removed the if result because result always has at least one element due to this PR.

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.

3 participants