fix(slack): collect thread attachments across recent messages instead of only the newest#706
Open
shiminshen wants to merge 2 commits into
Open
fix(slack): collect thread attachments across recent messages instead of only the newest#706shiminshen wants to merge 2 commits into
shiminshen wants to merge 2 commits into
Conversation
…ewest message collectInboundFileParts scans thread.recentMessages backwards but returns unconditionally on the first non-bot message, so files posted earlier in the thread are silently dropped whenever the newest user message has no attachments. Signed-off-by: shiminshen <16914659+shiminshen@users.noreply.github.com>
… of only the newest collectInboundFileParts scanned thread.recentMessages backwards but returned unconditionally on the first non-bot message, so files posted earlier in a thread were silently dropped whenever the newest user message had no attachments — the exact follow-up-mention case the lookback exists to cover. Collect across non-bot messages newest first, capped at 10 file parts to keep model input bounded. Signed-off-by: shiminshen <16914659+shiminshen@users.noreply.github.com>
Contributor
|
@shiminshen is attempting to deploy a commit to the Vercel Team on Vercel. A member of the Team first needs to authorize it. |
Author
|
One note on the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #705.
collectInboundFilePartsscansthread.recentMessagesbackwards, but the loop body ends with an unconditional return on the first non-bot message it inspects — the loop never advances. When a user posts an image in a thread and then mentions the bot in a text-only follow-up, the newest non-bot message has no files, so everything earlier is silently dropped and the model sees zero file parts. This is the exact case the function's own doc comment says the lookback exists to cover ("a user dropped a file in the thread first, then mentioned the bot in a follow-up").The fix collects attachments across non-bot messages, newest first, capped at 10 file parts to keep model input bounded on long threads. If you'd rather keep single-message semantics ("most recent message that has files") instead of bounded accumulation, I'm happy to adjust — flagged the same question in #705.
We've been running this change in production for an internal Slack knowledge agent (as a patch-package patch of the dist) since eve 0.17.
How did you test your changes?
pnpm run test:unitinpackages/eve— 445 files, 4683 passed (adds a regression test that fails onmain, plus accumulation and cap cases)pnpm run typecheck— cleanpnpm lint— cleanPR Checklist
CONTRIBUTING.mdevepackagegit commit --signoff)