dfsgdhn#52
Open
alfred-murray wants to merge 15 commits intojakemor:mainfrom
Open
Conversation
Replaces the default Changes view with a side-by-side collapsible three-column layout showing traditional diffs, a prototype LLM-reordered diff story, and natural language summaries. Sidebar expands dynamically based on visible panel count. Made-with: Cursor
…eaders, auto-fit grid - Moved Raw/AI/Summary toggle buttons into the parent Changes/History header to avoid z-index overlap issues with file cards - Made panel headers sticky so they stay visible while scrolling - Switched panels to auto-fit CSS grid that respects available sidebar width - Removed DiffFileCard sticky header to prevent overlap with control bar Made-with: Cursor
…ion for Raw, Reordered, Natural Language, and Summary views. Implement automatic analysis on entering the diff view and streamline stale analysis handling. Adjust commit form placement for improved layout and usability.
Greptile SummaryThis PR introduces a Codex-powered diff analysis feature: a
Confidence Score: 4/5Safe to merge after fixing the One P1 logic bug (non-atomic concurrency guard in src/server/diff-analysis-store.ts — move Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant WsRouter
participant DiffAnalysisStore
participant DiffStore
participant CodexAppServer
Client->>WsRouter: chat.analyzeDiff { paths }
WsRouter->>DiffAnalysisStore: startAnalysis(projectId, paths)
DiffAnalysisStore->>DiffStore: refreshSnapshot()
DiffStore-->>DiffAnalysisStore: ChatDiffSnapshot
DiffAnalysisStore->>DiffStore: readPatchesForAnalysis()
DiffStore-->>DiffAnalysisStore: raw diff patch
DiffAnalysisStore->>DiffAnalysisStore: parseUnifiedDiffHunks() → sourceBlocks
DiffAnalysisStore->>DiffAnalysisStore: computeDiffStats()
DiffAnalysisStore-->>WsRouter: (resolves, status=running)
WsRouter-->>Client: ack
DiffAnalysisStore->>CodexAppServer: startSession + startTurn(prompt)
loop streaming
CodexAppServer-->>DiffAnalysisStore: onAgentMessageDelta
DiffAnalysisStore->>DiffAnalysisStore: parseAgentResponse() → partial hunks
DiffAnalysisStore->>WsRouter: onChange(projectId)
WsRouter-->>Client: project-diff-analysis snapshot
end
CodexAppServer-->>DiffAnalysisStore: result (completed/cancelled/error)
DiffAnalysisStore->>WsRouter: onChange(projectId) status=completed
WsRouter-->>Client: project-diff-analysis snapshot (final)
Reviews (1): Last reviewed commit: "Revise diff panel UX: replace independen..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+57
to
+98
| if (this.active.has(args.projectId)) { | ||
| throw new Error("A diff analysis is already running.") | ||
| } | ||
|
|
||
| const selectedPaths = [...new Set(args.paths)].sort((left, right) => left.localeCompare(right)) | ||
| if (selectedPaths.length === 0) { | ||
| throw new Error("Select at least one file to analyze") | ||
| } | ||
|
|
||
| await this.diffStore.refreshSnapshot(args.projectId, args.projectPath) | ||
| const diffSnapshot = this.diffStore.getProjectSnapshot(args.projectId) | ||
| if (diffSnapshot.status !== "ready") { | ||
| throw new Error(diffSnapshot.status === "no_repo" | ||
| ? "Project is not in a git repository" | ||
| : "Diffs are not ready yet") | ||
| } | ||
|
|
||
| const fileByPath = new Map(diffSnapshot.files.map((file) => [file.path, file])) | ||
| const missingPath = selectedPaths.find((path) => !fileByPath.has(path)) | ||
| if (missingPath) { | ||
| throw new Error(`File is no longer changed: ${missingPath}`) | ||
| } | ||
|
|
||
| const runId = randomUUID() | ||
| const chatId = `diff-analysis-${args.projectId}` | ||
| const requestKey = createDiffAnalysisRequestKey(diffSnapshot.files, selectedPaths) | ||
| this.active.set(args.projectId, { runId, chatId }) | ||
| this.itemBuffersByRun.set(runId, new Map()) | ||
| this.patchState(args.projectId, { | ||
| status: "starting", | ||
| statusText: "Preparing diff analysis", | ||
| startedAt: new Date().toISOString(), | ||
| completedAt: null, | ||
| error: null, | ||
| selectedPaths, | ||
| requestKey, | ||
| diffStats: null, | ||
| sourceBlocks: [], | ||
| parsed: createEmptyParsedDiffAnalysis(), | ||
| plan: [], | ||
| }) | ||
|
|
There was a problem hiding this comment.
startAnalysis guard check
The concurrency guard at line 57 and the active.set(...) at line 83 are separated by await this.diffStore.refreshSnapshot(...) at line 66. A second startAnalysis call arriving while the first is suspended at that await will see active.has(projectId) === false and also pass the guard, launching two concurrent runAnalysis loops for the same project. Because both share the same chatId (diff-analysis-${projectId}), they will contend on the same Codex session.
Fix: move the sentinel into active (or a separate starting set) before the first await, and clean it up if the pre-flight checks fail:
// Fail fast before any async work
if (this.active.has(args.projectId)) {
throw new Error("A diff analysis is already running.")
}
// Reserve the slot synchronously so a concurrent call fails immediately
const runId = randomUUID()
const chatId = `diff-analysis-${args.projectId}`
this.active.set(args.projectId, { runId, chatId })
try {
await this.diffStore.refreshSnapshot(args.projectId, args.projectPath)
// ... remaining validation ...
this.patchState(args.projectId, { status: "starting", ... })
void this.runAnalysis({ ... })
} catch (error) {
// Remove the slot so a future call can succeed
if (this.active.get(args.projectId)?.runId === runId) {
this.active.delete(args.projectId)
this.itemBuffersByRun.delete(runId)
}
throw error
}
Comment on lines
+252
to
+263
|
|
||
| function classifyDiffLine(line: string) { | ||
| if (line.startsWith("diff --git ") || line.startsWith("---") || line.startsWith("+++")) { | ||
| return "bg-muted/60 text-muted-foreground font-medium" | ||
| } | ||
| if (line.startsWith("@@")) { | ||
| return "bg-accent text-muted-foreground" | ||
| } | ||
| if (line.startsWith("+")) { | ||
| return "bg-emerald-500/10 text-emerald-700 dark:text-emerald-300" | ||
| } | ||
| if (line.startsWith("-")) { |
There was a problem hiding this comment.
The left gutter <span> shows line.slice(0, 1) (the +/-/space prefix), and the <code> tag shows the full line — so the prefix character appears in both columns. For a line like +const x = 1, viewers see + in the gutter and +const x = 1 in the content area. The content should strip the leading prefix character:
Suggested change
| function classifyDiffLine(line: string) { | |
| if (line.startsWith("diff --git ") || line.startsWith("---") || line.startsWith("+++")) { | |
| return "bg-muted/60 text-muted-foreground font-medium" | |
| } | |
| if (line.startsWith("@@")) { | |
| return "bg-accent text-muted-foreground" | |
| } | |
| if (line.startsWith("+")) { | |
| return "bg-emerald-500/10 text-emerald-700 dark:text-emerald-300" | |
| } | |
| if (line.startsWith("-")) { | |
| <code className="whitespace-pre py-1 pr-3">{line.slice(1)}</code> |
…son mode, allowing users to analyze differences between the current branch and the default branch. Update UI components to support new comparison mode, including adjustments to diff loading and analysis commands. Refactor related functions and tests to ensure compatibility with the new feature.
…ine the default comparison mode based on available diffs and enhance the RightSidebar component to utilize these functions. Update tests to cover new comparison mode scenarios and ensure proper behavior when switching between modes.
Standalone React+Vite site showcasing better-diff concept. Chalkboard theme with SVG doodles (diff blocks, magnifying glass, braces, plus/minus signs) scattered as background illustrations. Lenis smooth scrolling, sketch arrows, and typographic effects (zoom-in/out, marker highlights, SHOUT caps). Made-with: Cursor
Made-with: Cursor
…diff blocks - Introduced a new utility function to filter and return only the hunk body lines from diff text. - Updated RightSidebar component to utilize this function for rendering diff lines. - Added a test case to verify the correct output of the new function for reordered diff blocks.
Preserve original typos, remove paraphrased descriptions, add missing paragraphs (jump in and out, cant this be just a skill, dev flow section). Made-with: Cursor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.