Skip to content

fix(data): strip only the key prefix when reading DynamoDB thread ids - #3049

Open
lihongyuan99 wants to merge 1 commit into
Chainlit:mainfrom
lihongyuan99:fix/dynamodb-removeprefix
Open

lihongyuan99 wants to merge 1 commit into
Chainlit:mainfrom
lihongyuan99:fix/dynamodb-removeprefix

Conversation

@lihongyuan99

@lihongyuan99 lihongyuan99 commented Sep 18, 2026

Copy link
Copy Markdown

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:

>>> "THREAD#DEMO".strip("THREAD#")
'MO'
>>> "THREAD#CODE".strip("THREAD#")
'CO'

The DynamoDB data layer uses strip("THREAD#"), strip("STEP#") and strip("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_threads returns truncated IDs to the frontend.
  • delete_feedback builds 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 removeprefix correctly on line 635 (pk.removeprefix("THREAD#")); this PR applies that pattern consistently.

Changes

  • backend/chainlit/data/dynamodb.py: replace four strip(...) calls with removeprefix(...).
  • 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
  • Before: 2 failures, with the assertion diff showing the truncation ({'PK': {'S': 'THREAD#CO'}} vs the expected {'PK': {'S': 'THREAD#CODE'}}).
  • After: 2 passed.

Checklist

  • Conventional Commits format used.
  • Change is backward-compatible.
  • Tests pass locally, and ruff check / ruff format --check pass.

Summary by cubic

Fixes DynamoDB thread ID handling where str.strip() removed trailing characters from IDs like THREAD#DEMOMO, causing list_threads to return truncated IDs and delete_feedback to build delete keys for the wrong rows. Switches to removeprefix() 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.

Review in cubic

`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>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 2 files

Re-trigger cubic

This branch has not been deployed

No deployments
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.

1 participant