Skip to content

codex: route thread/read persistence through thread store#18232

Closed
wiltzius-openai wants to merge 2 commits intowiltzius/codex/thread-read-view-refactorfrom
wiltzius/codex/thread-read-store-migration
Closed

codex: route thread/read persistence through thread store#18232
wiltzius-openai wants to merge 2 commits intowiltzius/codex/thread-read-view-refactorfrom
wiltzius/codex/thread-read-store-migration

Conversation

@wiltzius-openai
Copy link
Copy Markdown
Contributor

@wiltzius-openai wiltzius-openai commented Apr 16, 2026

Summary

  • replace the thread/read persisted-load helper with ThreadStore::read_thread
  • move SQLite/rollout summary, name, fork metadata, and history loading for persisted reads into LocalThreadStore
  • leave getConversationSummary unchanged for a later PR

Stack

Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6df72b6a61

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

{
let mut thread = stored_thread_from_sqlite_metadata(store, metadata).await;
if params.include_history {
let path = resolve_rollout_path(store, thread_id, /*include_archived*/ false)
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.

P1 Badge Load archived rollout for history requests

When SQLite metadata is found, read_thread loads history via resolve_rollout_path(..., /*include_archived*/ false). For archived threads (metadata.archived_at.is_some()) with params.include_archived=true, this skips the archived directory and returns an internal "failed to locate rollout" error. As a result, archived thread/read calls with history/includeTurns fail even though metadata exists.

Useful? React with 👍 / 👎.

{
let mut thread = stored_thread_from_sqlite_metadata(store, metadata).await;
if params.include_history {
let path = resolve_rollout_path(store, thread_id, /*include_archived*/ false)
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.

P1 Badge Read archived rollout when loading history

When SQLite metadata exists, history loading hard-codes include_archived to false. Archived threads with include_archived=true and include_history=true therefore fail with an internal "failed to locate rollout" error even though the archived rollout exists.

Useful? React with 👍 / 👎.

@wiltzius-openai wiltzius-openai force-pushed the wiltzius/codex/thread-read-store-migration branch from b83c5de to db1b134 Compare April 16, 2026 23:57
@wiltzius-openai wiltzius-openai deleted the branch wiltzius/codex/thread-read-view-refactor April 17, 2026 04:06
@wiltzius-openai
Copy link
Copy Markdown
Contributor Author

replaced by #18352

wiltzius-openai added a commit that referenced this pull request Apr 17, 2026
Summary
- replace the thread/read persisted-load helper with
ThreadStore::read_thread
- move SQLite/rollout summary, name, fork metadata, and history loading
for persisted reads into LocalThreadStore
- leave getConversationSummary unchanged for a later PR

Context
- Replaces closed stacked PR #18232 after PR #18231 merged and its base
branch was deleted.
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