Repository navigation
feat(editor): add "Fix with AI" button to query errors - #1327
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Incremental review of
The earlier checks still hold: both Generate entry points reset Files Reviewed (4 files)
Note: the raw Previous Review Summaries (3 snapshots, latest commit 61b9259)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 61b9259)Status: No Issues Found | Recommendation: Merge No changes since the last review (HEAD Files Reviewed (18 files)
Previous review (commit e899df8)Status: No Issues Found | Recommendation: Merge The only change since the last review is Files Reviewed (18 files)
Previous review (commit cf778ad)Status: No Issues Found | Recommendation: Merge Files Reviewed (17 files)
Reviewed by claude-sonnet-5.5 · Input: 256 · Output: 1.7K · Cached: 312.5K Review guidance: REVIEW.md from base branch |
e899df8 to
61b9259
Compare
debba
left a comment
debba
left a comment
There was a problem hiding this comment.
Thanks for the work on connecting query errors to a Fix with AI action. We are evaluating moving AI assistant integrations into plugins, so I am holding off on reviewing and merging AI assistant changes for now. The draft with that work is #1337.
1-DARK
left a comment
1-DARK
left a comment
There was a problem hiding this comment.
Thanks for the clean implementation! This follows the issue's pointers closely - the helper lives in utils/ with mirrored tests, the prefill uses the adjust-state-during-render pattern rather than an effect, and I verified all acceptance criteria of #1305 on your branch: new tests pass (8/8),the full web-ui suite passes (7669 tests), and pnpm lint / pnpm typecheck are clean.
Two things are needed before merge:
- Rebase / merge conflict: main removed the web UI API inventory in #1342, deleting docs/web/web-ui-parity.json. Since this PR modifies that file,GitHub reports it as conflicting. Please rebase onto current main and drop the web-ui-parity.json hunk entirely.
- Missing locale key: vi.json is missing editor.fixWithAi (the other 11 locales have it), so Vietnamese would fall back to English. Please add it next to showErrorDetails (~line 1677).
One optional suggestion (not blocking, and not a reachable bug today): aiFixPrompt in Editor.tsx is only reset by the two "Generate" entry points, never on close. Resetting it in onClose would make fix mode less fragile if a new open path is added later, and would also avoid a cancelled fix leaving its prompt as the modal's draft the next time "Generate" is opened.
|
The architecture hold now has a concrete interface: #1337 is merged, with built-in providers and installable declarative AI assistants. The existing typed Please update this branch against current main and keep Fix with AI on that shared client path, using the selected provider/model and the relevant schema context rather than adding provider-specific dispatch. The new package guide is https://github.com/TabularisDB/tabularis/blob/main/plugins/ai-assistants/README.md . I have not revalidated this older UI revision against the merged runtime, so this status update is not a merge approval. The previous wait for the AI interface itself is over. |
|
A follow-up on the AI runtime/registry prerequisites: Tabularium #35 is now merged and the The eight existing providers remain embedded in the app, so the earlier replacement-package rollout is unnecessary. The remaining code work can use the current provider/context interfaces on main and the independent external-package flow where applicable. |
…tton # Conflicts: # docs/web/web-ui-parity.json
|
@NewtTheWolf @debba Can you please review this PR now |
debba
left a comment
debba
left a comment
There was a problem hiding this comment.
Re-reviewed 929ea7ed50fce87f820e9f8505c2c275d6908015. The architecture hold is resolved: Fix with AI uses the existing typed generate_ai_query client call, selected provider/model and schema context. The removed web API inventory is no longer in the diff. Two issues still need correction before merge:
-
P2 — A cancelled fix leaks into ordinary Generate. In
AiQueryModal.tsx:49-53, reopening only resetspromptwheninitialPromptis defined. Reproduce: open Fix with AI, cancel/close it, then open ordinary Generate from the dropdown or command. Editor clearsaiFixPromptto null, but the persistent modal still shows the previous fix prompt because its newinitialPromptis undefined. Submitting this stale prompt now also takes the ordinary Generate path (Editor.tsx:6950-6955), which callsrunQuery, rather than the insert-only fix path. Please clear/separate the fix draft on the transition back to ordinary Generate while preserving the intended ordinary draft behavior, and add a cancel/reopen regression. I reproduced this with a temporary component regression test using the same close/reopen props as Editor: expected an empty ordinary prompt, received the cancelled fix prompt. This is reachable with the current UI, not only a possible future entry point. -
P2 — Missing locale keys.
editor.fixWithAiis absent in bothpackages/web-ui/src/i18n/locales/vi.jsonandzh-TW.json. The earlier Vietnamese locale finding is still outstanding, and Traditional Chinese is missing too. Please add the key to both, as required by the issue/repository rules.
The existing focused tests pass: 13 tests across aiFixPrompt, ErrorDisplay and AiQueryModal. The added temporary reproduction fails as described and was removed after verification. Typecheck/lint results are recorded below. GitNexus impact analysis remains unavailable because this checkout's copied index points to another directory; I traced the affected paths directly (the shared AI modal, Editor Generate/Fix actions and ErrorDisplay callers). No production source edits were made.
Manual fixture: SQLite users(id INTEGER PRIMARY KEY, name TEXT, active INTEGER) with (1, 'Alice', 1) and (2, 'Bob', 0). Running SELECT nme FROM users gives the real error no such column: nme; SELECT name FROM users ORDER BY id returns Alice/Bob. Test Fix -> generate inserts only; Fix -> cancel -> Generate must not retain the cancelled fix; with AI disabled the button is hidden. Test the same client path with an installed AI assistant when configured. No paid/live AI request was made in this review.
Frontend typecheck and full lint (ESLint, Prettier and theme tokens) passed on this head.
|
@debba Thanks for the review. Both P2s are fixed. Stale fix prompt: Locales: I added The typecheck, the modal and i18n tests, and Prettier on the touched files pass. I'll wait for your next review. |
debba
left a comment
debba
left a comment
There was a problem hiding this comment.
Re-reviewed cfe8a81b4f484a78fda25906a731edac24138648. Both blockers from my previous review are resolved. promptIsFixDraft clears a cancelled Fix prompt when ordinary Generate reopens, while keeping ordinary Generate drafts; successful fixes also clear the marker. The new regression covers the cancel/reopen and ordinary-draft cases. editor.fixWithAi is now present in all 13 locales, including vi and zh-TW. The shared AI client/provider/model/schema path and insert-only Fix behavior remain unchanged.
Local validation on this exact head: 277 tests across 19 files passed (modal, error display, prompt helper and i18n), frontend typecheck passed. Lint results recorded below. No source edits made. GitNexus remains unavailable for this checkout as documented in the preceding review; the incremental change is limited to the modal, its test and the two missing locale keys. No remaining code blocker found.
Full lint passed: ESLint, Prettier and theme-token checks.
Superseded after maintainer re-review of cfe8a81: the removed web API inventory is absent from the PR diff, the branch is mergeable, the Vietnamese locale key is present (as is zh-TW), and the cancelled-fix lifecycle has a passing regression. All requested corrections verified.
Summary
Adds a "Fix with AI" button next to query errors in the editor results area. It opens the existing AI query dialog prefilled with a prompt containing the failed SQL and the error message. The user can review and edit the prompt, then generate a corrected query. The generated SQL is inserted into the editor and is never run automatically.
Closes #1305
Changes
utils/aiFixPrompt.ts:buildFixQueryPrompt(sql, error)buildsFix this query. It failed with: <error>\n\n<sql>. Only the brief error message is used; details after the first blank line (stack traces etc.) are dropped.ErrorDisplay.tsx: new optionalonFixWithAiprop that renders the button. Other callers are unchanged.AiQueryModal.tsx: new optionalinitialPromptprop. The prompt is applied each time the dialog opens and cleared after a successful fix, so a later "Generate" doesn't show stale text.Editor.tsx: the button is shown only whensettings.aiEnabled, there is an active connection and the query is non-empty (same availability rule as the AI generate action). In fix mode,onInsertonly updates the editor and skipsrunQuery. The regular AI generate flow still runs the query as before.editor.fixWithAiadded to all locales.generate_ai_queryis reused.Tests
tests/utils/aiFixPrompt.test.ts: prompt format, error details dropped, whitespace trimmed.tests/components/ui/ErrorDisplay.test.tsx: button hidden without a handler; handler called on click.Manual testing
SELECT nme FROM users. The error shows a "Fix with AI" button next to Copy.Notes
The failed SQL is taken from the tab's editor text, so if only a selection was run, the prompt contains the whole editor text.