feat(ai-chat): Codiff-style AI walkthroughs for generated SQL#1948
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![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: 72988d7bb1
ℹ️ 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".
| : String(localized: "Replace the editor content with the suggested SQL")) | ||
| } | ||
| .alert(String(localized: "Apply suggested SQL?"), isPresented: $showApplyConfirmation) { | ||
| Button(String(localized: "Apply")) { actions?.insertQueryFromAI(afterSQL) } |
There was a problem hiding this comment.
For the normal Optimize/Fix flow, the selected editor tab already contains the query being reviewed. insertQueryFromAI only mutates an empty selected query tab and otherwise opens a new query tab (MainContentCoordinator.swift:1017-1036), so confirming this action does not perform the replacement promised by the button and confirmation text; it leaves the original editor content unchanged.
Useful? React with 👍 / 👎.
| func resolveWalkthroughIfNeeded(id: UUID) { | ||
| guard let beforeSQL = pendingWalkthroughBeforeSQL else { return } | ||
| pendingWalkthroughBeforeSQL = nil |
There was a problem hiding this comment.
A successful walkthrough clears the only stored beforeSQL here, but regenerate() deletes that assistant turn and restarts streaming without restoring it; likewise the error path clears it before retry(). The hidden prompt still instructs the model to emit the envelope, but resolveWalkthroughIfNeeded then returns immediately, so regenerated/retried Explain, Optimize, and Fix responses display their raw JSON instead of the walkthrough UI.
Useful? React with 👍 / 👎.
| private func activateAnchor(_ anchor: SqlWalkthroughAnchor) { | ||
| activeAnchor = anchor | ||
| scrollTarget = unifiedScrollTarget(for: anchor) |
There was a problem hiding this comment.
When users select Split and press “Jump to lines,” this always calculates an ID from the unified diff. Split rows are identified by their DiffPair index, while unified IDs gain an extra entry for each changed pair, so after any prior replacement the target either scrolls to a different row or does not exist in the split view. The highlighted lines are correct, but the advertised jump action is not.
Useful? React with 👍 / 👎.
…parse it from any block
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22d7563d4e
ℹ️ 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".
| func sendWithWalkthroughContext(prompt: String, beforeSQL: String) { | ||
| pendingWalkthroughBeforeSQL = beforeSQL | ||
| sendWithContext(prompt: prompt) |
There was a problem hiding this comment.
When a walkthrough command cannot start—for example, no provider is configured, the connection policy is never, or preflight rejects an oversized prompt—this state remains set because none of those early exits clears it. The next ordinary chat message then receives the walkthrough system directive and is parsed against the stale SQL, so a normal response can be rendered as an unrelated SQL walkthrough.
Useful? React with 👍 / 👎.
| private func unifiedDiff(before: [String], after: [String]) -> some View { | ||
| let lines = Array(DiffComputer.computeUnified(before: before, after: after).prefix(Self.maxDiffLines)) |
There was a problem hiding this comment.
For a query with 500 changed lines, computeUnified emits 1,000 display rows (a removal and addition per change), but this prefixes the result at 500 while the footer compares against max(before.count, after.count), also 500. Consequently the entire added half of the diff is silently hidden with no truncation notice, preventing users from reviewing the proposed SQL before applying it.
Useful? React with 👍 / 👎.
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
…y, not opening a tab
# Conflicts: # CHANGELOG.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fdb88ae868
ℹ️ 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".
| guard case .text(let openText) = tail[0].kind else { return } | ||
| let prose = WalkthroughEnvelopeParser.stripFence(from: openText) | ||
| let consumedIDs = Set(tail.dropFirst().map(\.id)) | ||
| messages[idx].blocks.removeAll { consumedIDs.contains($0.id) } |
There was a problem hiding this comment.
When a model emits the opening marker but returns invalid or truncated JSON (a realistic streaming failure), this removes the opening block and every later text block before WalkthroughEnvelopeParser.parse is checked. The subsequent parse failure then leaves users with only the prose before the marker and discards the proposed SQL/steps entirely; parse first and mutate the blocks only on success so malformed model output remains visible as normal text.
Useful? React with 👍 / 👎.
Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b725cf34ca
ℹ️ 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".
| assistantID: assistantID, | ||
| settings: settings | ||
| settings: settings, | ||
| includeWalkthroughDirective: self.pendingWalkthroughBeforeSQL != nil |
There was a problem hiding this comment.
With GitHub Copilot, this walkthrough flag is dynamic per request, but CopilotChatProvider.streamChat only applies options.systemPrompt when conversationId == nil (CopilotChatProvider.swift:73-80). Consequently, a slash walkthrough issued in an existing Copilot conversation never receives the envelope directive; conversely, after a walkthrough creates a new conversation, later ordinary follow-ups remain governed by that directive and can expose raw walkthrough JSON because no walkthrough is pending for parsing. Reset or recreate the Copilot conversation when this flag changes.
Useful? React with 👍 / 👎.
| var transcriptText: String { | ||
| guard let afterSQL = envelope.afterSQL, !afterSQL.isEmpty else { return "" } | ||
| return "Proposed SQL:\n\(afterSQL)" |
There was a problem hiding this comment.
When a user types a normal follow-up such as “explain step 2,” this transcript retains only the proposed SQL and discards every step title, reason, importance, and anchor; Explain walkthroughs are omitted entirely because afterSQL is nil. Stateless providers therefore receive no assistant history describing the step being referenced, so follow-ups cannot reliably remain anchored to the walkthrough. Serialize the step narrative as well as the proposed SQL.
Useful? React with 👍 / 👎.
| sql | ||
| .replacingOccurrences(of: "\r\n", with: "\n") | ||
| .replacingOccurrences(of: "\r", with: "\n") | ||
| .trimmingCharacters(in: .whitespacesAndNewlines) |
There was a problem hiding this comment.
When the selected query begins with one or more blank lines, the model sees and numbers those lines in the prompt, but this normalization removes them before anchor validation, snippets, and highlighting. An otherwise valid anchor can therefore highlight a later, unrelated SQL line—for example, source line 2 becomes normalized line 1. Preserve boundary lines for anchor geometry, or normalize the query before sending it to the model.
Useful? React with 👍 / 👎.
1 participant
Closes #1945.
AI Explain, Optimize, and Fix Error now answer with a reviewable walkthrough in the chat panel instead of plain markdown, so a user can inspect an AI SQL change before trusting it.
What you get
Design
The AI writes only the narrative (the rewritten
afterSQLplus ordered steps). The app computes the diff itself from the original query, so the diff always reflects the real change and a model cannot show a diff that does not exist.Structured output travels as a fenced JSON envelope parsed out of the streamed text, not tool calling. This works on every provider including the local ones (Ollama, llama.cpp, MLX), and degrades to plain markdown when a model ignores the format: the fence is stripped so no raw JSON shows, and the response renders as normal text.
Native and HIG details: semantic red/green with
+/-gutter markers (never color alone),DisclosureGroupsteps, reduce-motion gated highlight, VoiceOver status on diff rows, confirm-before-apply per Apple's Generative AI guidance. Anchors that fall out of range render no highlight rather than pointing at the wrong line.Structure
Three commits:
refactor(datagrid)extracts the line-diff engine (DiffComputer/DiffPair) out ofFileConflictDiffSheetintoCore/Diff/SqlDiff.swift, adding unified-diff output and SQL normalization. The conflict sheet consumes it unchanged.feat(ai-chat)adds the.sqlWalkthroughchat block (additive wire Codable, old conversations still decode) andAIChatWalkthroughBlockView, and encodes the walkthrough as proposed-SQL text in every provider's history so follow-ups keep context.feat(ai-chat)adds the envelope parser, the three walkthrough prompt templates, and the view-model wiring, plus CHANGELOG and docs.Tests
New unit tests: envelope parsing (well-formed, code-fenced, no-close-fence, malformed, no-fence, null
afterSQL, unique step ids,stripFence), diff extraction parity and unified diff, SQL normalization, anchor validity (in-range / out-of-range / nil), and.sqlWalkthroughwire round-trip plus legacy decode. UI automation is deferred: the flow depends on non-deterministic AI responses and needs a mock provider injection point.Verified locally: build succeeds, and all four new suites pass.
Scope note
In-editor cross-highlighting (highlighting the anchored lines in the main editor, not just the in-chat diff) is intentionally left out of this first version to keep the panel decoupled from editor state. Worth a follow-up.