Repository navigation
Find in the open thread and diffs with ⌘F (#1524) - #1596
Conversation
Solenta-Message-Id: 81296e12-f3df-45c4-9068-72ec657d373b
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f8c011d01
ℹ️ 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".
| messages: detail.messages, | ||
| plan: detail.thread.plan, | ||
| planSteps: detail.thread.planSteps, |
There was a problem hiding this comment.
When a plan approval prompt is open, the visible plan comes from detail.pendingPermission.plan, while the regular PlanCard is deliberately suppressed. Searching text unique to that visible plan therefore reports "No matches" because only detail.thread.plan and its steps are passed to the matcher; include the pending plan and make its rendered prompt a reveal target.
Useful? React with 👍 / 👎.
| for (const message of input.messages) { | ||
| if (unitMatches(message.text, query)) { | ||
| hits.push({ kind: "message", id: message.id }); |
There was a problem hiding this comment.
In the normal transcript view, settled thinking-only groups are removed by collapseTimeline, but this loop still counts every thinking message whose text matches. For such a hit, revealTranscriptHit cannot find a group to expand and useScrollFindHit cannot find a DOM target, so the bar can show a valid count while Enter produces no highlight or scroll; either omit these hidden messages from the hit list or provide a way to mount and target them.
Useful? React with 👍 / 👎.
| if (planParts.some((t) => unitMatches(t, query))) { | ||
| hits.push({ kind: "plan", id: "plan" }); |
There was a problem hiding this comment.
When a query matches both the plan and transcript messages, inserting the plan hit first makes the initial result jump to the bottom-mounted PlanCard, after which Next jumps back to the first message near the top. This reverses the document order users expect when stepping through matches; append the plan hit after the chronologically rendered messages.
Useful? React with 👍 / 👎.
⌘F now searches the open thread (messages, tool summaries, plan cards) and loaded Changes/turn diffs. Long threads were unsearchable because the transcript is a tail window and collapsed diffs are not in the DOM, so this searches the data and scrolls the hit into view instead of using Electron
findInPage. Fixes #1524.Changes
transcript.findanddiff.findare remappable and listed in the Keyboard sheet (shared default ⌘F is intentional).Verification
node --import=./test/support/disable-grok-mcp.mjs --import=./test/support/render.mjs --experimental-strip-types --test --test-timeout=20000 test/findInView.test.ts test/findInThread.test.tsx test/keybindings.test.tsx— 24 passed.Notes for reviewers
cursor-grok-4.6 via Cursor
Made with Cursor