Repository navigation
Conversation
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: 7db8077e43
ℹ️ 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".
| const sessionFilesReady = ((!data && isNew) | ||
| || (data?.sessionId === sessionIdRef.current && filesContextLeafId === activeLeafId)) | ||
| && (!session || session.id === sessionIdRef.current); |
There was a problem hiding this comment.
On a fresh composer's first prompt, promoteNewSession() causes AppShell to pass a non-null session and clear newSessionCwd without remounting this hook or populating data. Both readiness branches then become false, so confirmed SSE write results are collapsed to [] for the entire first run and only reappear when agent_end calls loadSession(). Keep the new-session live state ready through that promotion so the panel reports writes as they happen.
Useful? React with 👍 / 👎.
| stats: cached.stats as SessionData["stats"], | ||
| }); | ||
| setActiveLeafId(cached.leafId); | ||
| setFilesContextLeafId(cached.leafId); |
There was a problem hiding this comment.
On a session-view cache hit, the cached SessionData constructed just above contains no writtenFileManifest, yet setting this leaf marks sessionFilesReady; the hook consequently reports sessionFilesFailed until the forced detail request finishes. With a slow or large session, especially while the Files panel remains open during a switch, a routine load is shown as “Unable to load” with Retry and can launch a duplicate request. Leave the manifest not-ready while that fetch is pending, or cache its state separately.
Useful? React with 👍 / 👎.
| const key = `${write.toolCallId}\0${write.filePath}`; | ||
| if (seenCalls.has(key)) continue; | ||
| seenCalls.add(key); | ||
| const current = byPath.get(write.filePath) ?? { writeCount: 0 }; |
There was a problem hiding this comment.
On Windows, tool calls can refer to the same file with casing variants such as C:/Repo/a.ts and c:/repo/a.ts, and resolveLocalFilePath() does not case-fold them. Using raw filePath as the map key therefore renders duplicate rows and splits write counts, even though the repository's samePath() treats them as one filesystem location. Group with Windows path equivalence while retaining one spelling for display.
Useful? React with 👍 / 👎.
| for (const block of message.content ?? []) { | ||
| if (block.type !== "toolCall") continue; | ||
| const written = extractTurnWrittenFiles([block], toolResults, cwd); | ||
| const at = toolResults?.get(block.toolCallId)?.timestamp ?? message.timestamp; |
There was a problem hiding this comment.
When a successful tool result lacks its optional message timestamp, as older or extension-authored session entries can, this falls back to the assistant timestamp, which AssistantMessage defines as the model request start. A slow response can therefore label and sort a write minutes before it occurred even though the manifest caller still has the enclosing result entry's append timestamp; use that entry/result time or omit the time rather than reporting request start.
Useful? React with 👍 / 👎.
| <span style={{ minWidth: 0 }}> | ||
| <span className="name">{location.name}</span> | ||
| {location.directory && <span className="dir">{location.directory}</span>} |
There was a problem hiding this comment.
For every file below the session cwd, these two spans are inline and have no whitespace or separator between them, so src/App.tsx is rendered as App.tsxsrc; the overflow and ellipsis rules on .name and .dir also do not apply to ordinary inline spans. Make them block or flex rows, or add an explicit separator, so the displayed path is intelligible.
Useful? React with 👍 / 👎.
|
Thanks @uvforce for the careful work and the thorough tests. I'm going to pass on this one, though, because it runs against the UI rules in CONTRIBUTING.md:
In case any of this comes back, a few things I hit while testing:
If a write is ever missing from the per-turn list, a fix there would be very welcome. For a feature like this, please open an issue first; it can save you the build. Closing for now. |
2 participants
Summary
Add a session-wide Files written in this session panel while keeping the project file explorer and original file viewer unchanged.
The panel lists successful writes on the selected branch, not every path mentioned by a message. It does not query disk modification times, display disk-status badges, or add a file-metadata endpoint. Missing or inaccessible files are handled by the original viewer when opened.
Evidence
After: The same route test passes with successful writes before compaction and the last-50 boundary, explicit branch selection, and deduplication across loaded pages.
Merge Danger
Door: Two-way
This is an additive, read-only session response and UI feature. It requires no session-file migration or destructive operation and can be reverted without changing existing conversations. The filter preference is stored locally.
Blast Radius: Sessions
The affected paths are session loading, branch/history reconciliation and the Files panel. FileExplorer, FileViewer, file-access policies and tool execution remain unchanged. No dependencies are added.