fix(data): strip only the key prefix when reading DynamoDB thread ids - #3049
Open
lihongyuan99 wants to merge 1 commit into
Open
lihongyuan99 wants to merge 1 commit into
lihongyuan99 wants to merge 1 commit into
Conversation
`str.strip("THREAD#")` treats its argument as a set of characters rather
than a prefix, so it also eats the identifier's own leading and trailing
letters: `THREAD#DEMO` came back as `MO`, `THREAD#CODE` as `CO`.
`list_threads` returned those truncated ids and `delete_feedback` built the
key it deletes from them, so a delete silently targeted a different row.
`str.removeprefix` strips the prefix only, which is what the same file
already uses for the thread-list branch at line 635.
Adds the first tests for the DynamoDB data layer.
Co-Authored-By: WorkBuddy AI <noreply@workbuddy.ai>
lihongyuan99
requested review from
asvishnyakov,
hayescode and
sandangel
as code owners
September 18, 2026 10:54
This branch has not been deployed
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
str.strip(chars)treats its argument as a set of characters rather than a prefix, so it also eats the identifier's own leading and trailing letters:The DynamoDB data layer uses
strip("THREAD#"),strip("STEP#")andstrip("TS#")in four places (lines 180, 181, 503, 504). This corrupts thread and step IDs that happen to start or end with any of those characters. Consequences:list_threadsreturns truncated IDs to the frontend.delete_feedbackbuilds the primary key from those truncated IDs, so a delete silently targets a different row.IDs generated by Chainlit are lowercase-hex UUIDs and are unaffected, so this is latent for the default case but real for custom IDs. The same file already uses
removeprefixcorrectly on line 635 (pk.removeprefix("THREAD#")); this PR applies that pattern consistently.Changes
backend/chainlit/data/dynamodb.py: replace fourstrip(...)calls withremoveprefix(...).backend/tests/data/test_dynamodb.py: add the first tests for the DynamoDB data layer, covering thread-list prefix stripping and feedback-delete key construction.Tests
cd backend python -m pytest tests/data/test_dynamodb.py -q{'PK': {'S': 'THREAD#CO'}}vs the expected{'PK': {'S': 'THREAD#CODE'}}).2 passed.Checklist
ruff check/ruff format --checkpass.Summary by cubic
Fixes DynamoDB thread ID handling where
str.strip()removed trailing characters from IDs likeTHREAD#DEMO→MO, causinglist_threadsto return truncated IDs anddelete_feedbackto build delete keys for the wrong rows. Switches toremoveprefix()so only the prefix is stripped, and adds the first tests for the DynamoDB data layer covering these paths.Written for commit 4acbc98. Summary will update on new commits.