Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughCodex discovery now parses session metadata directly, derives importability from payload fields, and uses cached per-directory indexes for project-scoped rollouts. Discovery processing and tests cover indexed scope filtering and exclusion of object-shaped subagent sources. ChangesCodex discovery
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/main/services/externalSessions/discoverCodex.ts (1)
605-611: 🚀 Performance & Scalability | 🔵 TrivialFull-index rewrite on any single-file change may amplify writes on large Codex dirs.
indexChangedflips whenever one rollout'smtimeMs/sizediffers — which happens on every discovery while the current-day session is still being appended to. Each such run re-serializes and rewrites the entire index (all entries), so on a directory with thousands of rollouts this becomes a large JSON write on nearly every scoped discovery. Consider debouncing writes, batching, or only rewriting past a change threshold if this path runs frequently.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/main/services/externalSessions/discoverCodex.ts` around lines 605 - 611, Reduce full-index rewrites in the discovery flow around indexChanged and writeCodexScopeIndex: avoid serializing and writing the entire index for every single rollout mtimeMs/size update during active sessions. Implement a scoped batching, debounce, or change-threshold policy while preserving index updates and ensuring accumulated changes are eventually persisted.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/desktop/src/main/services/externalSessions/discoverCodex.ts`:
- Around line 605-611: Reduce full-index rewrites in the discovery flow around
indexChanged and writeCodexScopeIndex: avoid serializing and writing the entire
index for every single rollout mtimeMs/size update during active sessions.
Implement a scoped batching, debounce, or change-threshold policy while
preserving index updates and ensuring accumulated changes are eventually
persisted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8c51085d-2750-434b-b0d6-3129a0e8c1ce
⛔ Files ignored due to path filters (1)
docs/features/terminals-and-sessions/external-session-import.mdis excluded by!docs/**
📒 Files selected for processing (2)
apps/desktop/src/main/services/externalSessions/discoverCodex.tsapps/desktop/src/main/services/externalSessions/discoverProviders.test.ts
|
@codex review |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Fixes ADE-104
Linked Linear issues
Summary by CodeRabbit
Bug Fixes
Performance
Greptile Summary
This PR improves Codex external session discovery for project-scoped imports. The main changes are:
Confidence Score: 5/5
Safe to merge with low risk.
The changes are localized to Codex external-session discovery, add targeted tests for the previous truncation failure and cache behavior, and preserve existing non-scoped discovery paths.
No files require special attention.
What T-Rex did
Important Files Changed
Sequence Diagram
sequenceDiagram participant Caller as External session list participant Discover as discoverCodexSessions participant FS as Codex sessions dir participant Index as ADE cwd/importability index Caller->>Discover: request Codex sessions with scopeRoots Discover->>Index: load cached scope index Discover->>FS: stat rollout inventory loop each rollout file alt missing or changed index entry Discover->>FS: read session_meta prefix Discover->>Index: refresh mtime/size/meta entry else unchanged Index-->>Discover: reuse cached meta end end Discover->>Discover: filter importable sessions in scope Discover->>Index: write immediately or schedule batched rewrite Discover-->>Caller: sorted Codex session summariesReviews (2): Last reviewed commit: "Refs ADE-104: ship: iteration 1 — batch ..." | Re-trigger Greptile