Skip to content

Conversation

louis-menlo
Copy link
Contributor

@louis-menlo louis-menlo commented Jul 19, 2025

Summary

• Fixed timestamp handling for legacy threads that were using millisecond timestamps instead of
seconds
• Threads are now properly sorted by creation time with newer threads appearing first• Added
comprehensive test coverage for the timestamp migration logic

The root cause is that Jan's legacy code creates threads with timestamps in milliseconds. The latest UI revamp persists these threads with timestamps in seconds.

Threads 1 and 2 created from Jan 0.5.6, they all on top before the fix.

CleanShot 2025-07-19 at 16 58 08@2x

Closes #5696

Changes

  • web-app/src/services/threads.ts: Added timestamp normalization to convert millisecond
    timestamps to seconds for proper sorting
  • web-app/src/services/tests/threads.test.ts: Added test case to verify legacy thread
    migration and updated existing tests

Test plan

  • Verify legacy threads with millisecond timestamps are properly converted
  • Confirm thread sorting displays newest threads first
  • Run existing test suite to ensure no regressions

Important

Fixes thread sorting by normalizing legacy timestamps in fetchThreads and adds test coverage for timestamp migration.

  • Behavior:
    • Fixes sorting of threads by normalizing timestamps in fetchThreads in threads.ts.
    • Legacy threads with millisecond timestamps are converted to seconds.
  • Tests:
    • Adds test case for legacy thread migration in threads.test.ts.
    • Updates existing tests to cover new timestamp logic.
  • Misc:

This description was created by Ellipsis for 6d6d525. You can customize this summary. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Caution

Changes requested ❌

Reviewed everything up to 5335962 in 2 minutes and 0 seconds. Click for details.
  • Reviewed 210 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. web-app/src/services/threads.ts:20
  • Draft comment:
    Legacy timestamp conversion is correct, but there is no sorting applied. To ensure newer threads appear first, consider sorting by the 'updated' field in descending order.
  • Reason this comment was not posted:
    Comment was on unchanged code.
2. web-app/src/services/threads.ts:115
  • Draft comment:
    Consider using Math.floor(Date.now() / 1000) in updateThread for 'created' and 'updated' to ensure integer timestamps, consistent with the conversion logic.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_DI6q3mzDoPdlorFS

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed 6d6d525 in 2 minutes and 6 seconds. Click for details.
  • Reviewed 41 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. web-app/src/services/__tests__/threads.test.ts:91
  • Draft comment:
    Sort order issue: After normalizing, the thread with updated=1234567890 (id '2') is newer and should appear first if newer threads appear first, but the test expects it second. Please verify the sorting order.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment points out that Thread 2 has a newer timestamp (1234567890) than Thread 1 (1234567880), suggesting they should be sorted differently. However, I don't see any explicit sorting requirements in the test name or context. The test is about migrating old threads, not about sorting. Without seeing the actual implementation or requirements, I can't be certain that newer threads should appear first. I might be missing documentation or requirements elsewhere that specify the sorting order. The comment author may have knowledge of the intended behavior that isn't visible here. Even if there are hidden requirements, a test about thread migration is not the right place to enforce sorting rules. If sorting is important, it should have its own dedicated test case. The comment should be deleted because it makes assumptions about sorting requirements that aren't evident in this test case, which is specifically about thread migration.
2. web-app/src/services/__tests__/threads.test.ts:296
  • Draft comment:
    Expected thread count change: The test now expects 1 thread instead of 2. Confirm that the input and migration logic yield a single thread in this scenario.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.

Workflow ID: wflow_g0MPllNxtteORLpO

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link
Contributor

github-actions bot commented Jul 19, 2025

Barecheck - Code coverage report

Total: 35.01%

Your code coverage diff: 0.01% ▴

✅ All code changes are covered

@louis-menlo louis-menlo requested a review from qnixsynapse July 19, 2025 14:46
@louis-menlo louis-menlo force-pushed the fix/5696-legacy-threads-show-on-top-of-new-thread branch from 6d6d525 to 7892f0c Compare July 19, 2025 15:58
@louis-menlo louis-menlo merged commit 5696e95 into release/v0.6.6 Jul 20, 2025
18 of 20 checks passed
@louis-menlo louis-menlo deleted the fix/5696-legacy-threads-show-on-top-of-new-thread branch July 20, 2025 09:58
@github-project-automation github-project-automation bot moved this to QA in Jan Jul 20, 2025
@github-actions github-actions bot added this to the v0.6.6 milestone Jul 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants