Skip to content

feat: preserve last chat order after sync#10

Open
songsongshuo785-art wants to merge 2 commits intoDailin521:mainfrom
songsongshuo785-art:feat/preserve-last-chat-order-after-sync
Open

feat: preserve last chat order after sync#10
songsongshuo785-art wants to merge 2 commits intoDailin521:mainfrom
songsongshuo785-art:feat/preserve-last-chat-order-after-sync

Conversation

@songsongshuo785-art
Copy link
Copy Markdown

Summary

  • normalize rewritten rollout file mtimes to each thread's latest activity timestamp
  • persist the normalized timestamp in backup manifests so restore keeps the same ordering behavior
  • add JS and .NET integration tests covering the latest-activity ordering case

Why

After provider sync, Codex session visibility comes back, but the visible ordering can drift because the rollout file mtime changes during rewrite. Preserving the latest activity timestamp keeps transferred sessions sorted by the most recent chat instead of the sync time.

Validation

  • npm test
  • dotnet restore desktop/CodexProviderSync.Core.Tests/CodexProviderSync.Core.Tests.csproj --configfile ./NuGet.Config
  • dotnet test desktop/CodexProviderSync.Core.Tests/CodexProviderSync.Core.Tests.csproj --no-restore

Copy link
Copy Markdown
Owner

@Dailin521 Dailin521 left a comment

Choose a reason for hiding this comment

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

感谢提交,这个方向有价值,但当前实现里有一个我认为需要先处理的阻塞点。

阻塞问题:

  • 这次改动把 provider sync 的热路径重新变成了按文件扫描完整 rollout 内容。JS 侧在 src/session-files.jsreadLastActivityTimestampMs() 中直接 readFile() 整个文件并逐行 JSON.parse,而 collectSessionChanges() 会在每个需要改写 provider 的 rollout 上调用它。.NET 侧的 ReadLastActivityTimeUtcTicksAsync() 也在同一个收集阶段逐行扫描完整文件。
  • 这意味着原本只读首行 session_meta 的收集路径,重新退化成了按变更文件扫描整份 rollout 内容。对于首次全量 sync 或大量历史会话场景,这会直接落回我们之前刚处理过的性能敏感区,尤其 JS 路径还会把整文件读入内存。

建议先把“排序归一化时间”的来源改成不会在 sync 热路径里全量读取 rollout 的方式,或者至少把这条路径和已有性能基线一起验证清楚,再考虑合入。

@songsongshuo785-art
Copy link
Copy Markdown
Author

Thanks for calling this out. I agreed this needed to be fixed before merge, so I updated the branch to remove the rollout full-scan from the sync hot path.

What changed:

  • rollout collection still reads only the first session_meta line plus file stat
  • sort-normalization time now comes from state_5.sqlite thread metadata instead of scanning rollout content
  • it prefers hreads.updated_at_ms, falls back to updated_at, and finally falls back to the rollout's original mtime when SQLite activity is unavailable
  • both the JS and .NET paths now follow the same approach

So the sync path no longer reintroduces per-rollout full-content reads, and the JS path no longer loads the whole rollout into memory just to derive last activity time.

Validation I ran locally:

pm test

  • dotnet test desktop/CodexProviderSync.Core.Tests/CodexProviderSync.Core.Tests.csproj
  • dotnet build CodexProviderSync.sln

I also added coverage for:

  • normalizing mtime from SQLite updated_at_ms
  • preserving original rollout mtime when SQLite activity is unavailable

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.

2 participants