Use goal preview metadata for goal-first threads#21981
Use goal preview metadata for goal-first threads#21981etraut-openai wants to merge 3 commits intomainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7d33f1fac
ℹ️ 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".
jif-oai
left a comment
There was a problem hiding this comment.
This will probably break all goal-threads but good for now I guess as it still experimental
I wonder if we should communicate though
| ALTER TABLE threads ADD COLUMN preview TEXT NOT NULL DEFAULT ''; | ||
|
|
||
| UPDATE threads | ||
| SET preview = first_user_message |
There was a problem hiding this comment.
this won't be 100% backward compatible right? If the thread later got a normal prompt, this stores that later prompt instead of the original goal; if the goal was replaced, the goal backfill below stores the latest objective rather than the first one
| } | ||
|
|
||
| if summary.saw_session_meta && summary.saw_user_event { | ||
| if summary.saw_session_meta && summary.preview.is_some() { |
There was a problem hiding this comment.
This now stops the scan as soon as a goal gives us a preview, so a /goal-first thread that later gets a normal user turn comes back from the rollout path with first_user_message = None. That sounds the opposite of the objectif no ?
| cwd = excluded.cwd, | ||
| cli_version = excluded.cli_version, | ||
| title = excluded.title, | ||
| preview = COALESCE(NULLIF(excluded.preview, ''), threads.preview), |
There was a problem hiding this comment.
I'm not convinced by the backward compatibility of this PR but as goal is still experimental, I guess that's fine
Fixes #20792
Why
/goal-first threads are valid resumable threads, but they can be missing fromcodex resumeand app recents because discovery depends on metadata derived from a normal first user message.PR #21489 attempted to fix this by using the goal objective as
first_user_message. Review feedback pointed out thatfirst_user_messagedoes more than provide visible text today: it gates listing, supplies preview text, and participates in deciding whether a later title should surface as a distinct thread name. Reusing it for the goal objective could leave a/goal-first thread withfirst_user_message=<goal>andtitle=<later prompt>, even though the goal should only provide the initial visible preview.This PR follows that feedback by and keeps the
first_user_messageas is but introduces a newpreviewfield to separate concerns. Thepreviewfield is populated from the first user message or the goal objective. We can extend it in the future to include other sources.What Changed
previewmetadata incodex-state, including a SQLite migration that backfills fromfirst_user_messageand from existingthread_goalsobjectives when needed.ThreadGoalUpdatedas preview-bearing metadata so goal-first threads can be listed and searched without mutatingfirst_user_message.previewfield.first_user_message, so a later normal prompt after/goaldoes not surface as a separate name just because the goal supplied the initial preview.first_user_messagewhen explicit preview metadata is absent.Verification
/goal <objective>shows up in the resume picker.