diff --git a/.gitignore b/.gitignore index 3efe2a3..c9acc77 100644 --- a/.gitignore +++ b/.gitignore @@ -22,3 +22,6 @@ next-env.d.ts # e2e fixture input is committed; scratch output is not .DS_Store .playwright-data + +# real production plans dropped in for debugging stay out of git +/plan-*.sqlplan diff --git a/lib/analyst.ts b/lib/analyst.ts index 6fa5ef8..b88cec2 100644 --- a/lib/analyst.ts +++ b/lib/analyst.ts @@ -8,7 +8,9 @@ export class AnalystError extends Error {} const Action = z.object({ title: z.string().min(3), detail: z.string().min(3), effort: z.enum(["low", "medium", "high"]), requires_change_control: z.boolean() }); const RawCandidate = z.object({ key: z.string().min(2), title: z.string().min(3), diagnosis: z.string().min(15), - actions: z.array(Action).min(1), rewrite_sql: z.string().nullable(), expected: z.string().min(5) }); + actions: z.array(Action).min(1), + option_type: z.enum(["index", "statistics", "rewrite", "schema", "app", "ops"]), + sql_to_run: z.string().nullable(), expected: z.string().min(5) }); const Proposal = z.object({ candidates: z.array(RawCandidate).min(2).max(4) }); export type Action = z.infer; export type Candidate = z.infer; @@ -22,18 +24,41 @@ lowest-risk one. Each option needs a snake_case key, a diagnosis naming WHY the optional rewritten query, and an expected effect stated as a concrete claim about the plan (logical reads, seek vs scan, tempdb spill, degree of parallelism). -A rewrite must return the same rows as the original. If you cannot rewrite safely, set rewrite_sql to null. +A rewrite must return the same rows as the original. If you cannot rewrite safely, do not offer one. +When the digest says sql_truncated is true, you were not given the whole statement: never offer option_type +"rewrite", because you cannot reproduce text you never received. Offer an index, statistics, or query-hint +change instead, and say in expected that the statement itself was not rewritten here. + +option_type is what the option actually is: index | statistics | rewrite | schema | app | ops. +sql_to_run is the exact T-SQL the user must execute for that option — CREATE INDEX, CREATE STATISTICS, +UPDATE STATISTICS, ALTER TABLE, or the rewritten query — with the real table, column and index names taken from +the digest. Null only when the option needs no SQL at all (query-store verification, app-side batching, an ops +change). An option whose key says index or statistics and whose sql_to_run is null is a wrong answer. Output ONE JSON object and nothing else: {"candidates":[{"key":"idx_covering_orders","title":"...","diagnosis":"...","actions":[{"title":"...","detail":"...", -"effort":"low|medium|high","requires_change_control":false}],"rewrite_sql":null,"expected":"..."}]}`; +"effort":"low|medium|high","requires_change_control":false}],"option_type":"index","sql_to_run":"CREATE INDEX ...","expected":"..."}]}`; -export function extractJson(text: string): unknown { - const fenced = /```(?:json)?\s*([\s\S]*?)```/.exec(text); +export function extractJson(text: string): unknown { const fenced = /```(?:json)?\s*([\s\S]*?)```/.exec(text); const s = (fenced ? fenced[1] : text).trim(); const start = s.indexOf("{"); const end = s.lastIndexOf("}"); if (start === -1 || end <= start) throw new AnalystError("The analyst model returned prose instead of JSON."); try { return JSON.parse(s.slice(start, end + 1)); } - catch { throw new AnalystError("The analyst model returned malformed JSON."); } + catch { + // "malformed JSON" after 38 seconds blamed the model for what was usually our own output ceiling: + // a truncated response has no closing brace, so say the length out loud and let the user raise it + // through the extra-params field (which overrides max_tokens). + throw new AnalystError(`The analyst model's JSON stopped after ${s.length} characters without closing. ` + + `Raise the ceiling in settings extra params, e.g. {"max_tokens":8192}.`); + } +} + +// A SqlBlock the user cannot paste is worse than no SqlBlock: it reads as an instruction and breaks on run. +// Models reach for "SELECT ... FROM ..." when the original statement is too long to reproduce verbatim. +// ponytail: "..." is not valid T-SQL, so the ellipsis is a safe tell; a literal '...' inside a string +// literal would be dropped too, which costs a block and never invents one. +export function dropPlaceholderSql(candidates: T[]): T[] { + return candidates.map((c) => (c.sql_to_run && /[\u2026]|\.\.\.\s*(FROM|WHERE|OPTION|;|$)|(FROM|SELECT)\s+\.\.\./i.test(c.sql_to_run) + ? { ...c, sql_to_run: null } : c)); } export async function proposeCandidates(cfg: AnalystConfig, digest: Digest, note: string, @@ -41,7 +66,7 @@ export async function proposeCandidates(cfg: AnalystConfig, digest: Digest, note const res = await fetchImpl(`${cfg.baseUrl}/chat/completions`, { method: "POST", signal, headers: { "content-type": "application/json", authorization: `Bearer ${cfg.apiKey}` }, - body: JSON.stringify({ model: cfg.model, temperature: 0.6, max_tokens: 2048, ...cfg.extra, stream: false, + body: JSON.stringify({ model: cfg.model, temperature: 0.6, max_tokens: 4096, ...cfg.extra, stream: false, messages: [{ role: "system", content: CANDIDATE_SYSTEM_PROMPT }, { role: "user", content: JSON.stringify({ digest: digestForModel(digest), user_note: note || null }) }] }), }).catch((e) => { throw new AnalystError(`Could not reach ${cfg.baseUrl}: ${(e as Error).message}`); }); @@ -55,7 +80,7 @@ export async function proposeCandidates(cfg: AnalystConfig, digest: Digest, note ? `The analyst model proposed ${n} option(s); at least 2 are needed before Jev can rank anything.` : `Analyst output did not match the schema at ${parsed.error.issues[0].path.join(".")}: ${parsed.error.issues[0].message}`); } - return parsed.data.candidates; + return dropPlaceholderSql(parsed.data.candidates); } export const REPORT_SYSTEM_PROMPT = (openuiPrompt: string) => `${openuiPrompt} @@ -63,14 +88,22 @@ export const REPORT_SYSTEM_PROMPT = (openuiPrompt: string) => `${openuiPrompt} You are writing the final answer for a DBA holding one SQL Server execution plan. The ranking you are given was computed by Jev from typed judgments. It is not your opinion: do not re-rank it, soften it, or hide it. Quote each option's scores as given. A rewrite must return the same rows as the original — when in doubt keep the original text -and say why. Never invent a number that is not in the input.`; +and say why. Never invent a number that is not in the input. + +Hard requirements, in order: +1. Every option whose sql_to_run is not null gets its own SqlBlock containing exactly that T-SQL, unaltered and + runnable as pasted. Never write an ellipsis or a placeholder inside a SqlBlock: if the option's sql_to_run is + null, that option gets no SqlBlock at all. A report whose first move needs something executed and contains no + SqlBlock is wrong. +2. Every option gets a ScoreBar for its Jev composite score, carrying the confidence the ranking gave it. +3. VerdictBlock names the first move and states what to run first.`; export async function* streamReport(cfg: AnalystConfig, payload: unknown, openuiPrompt: string, fetchImpl: typeof fetch = fetch, signal?: AbortSignal): AsyncGenerator { const res = await fetchImpl(`${cfg.baseUrl}/chat/completions`, { method: "POST", signal, headers: { "content-type": "application/json", authorization: `Bearer ${cfg.apiKey}` }, - body: JSON.stringify({ model: cfg.model, temperature: 0.3, max_tokens: 2048, ...cfg.extra, stream: true, + body: JSON.stringify({ model: cfg.model, temperature: 0.3, max_tokens: 4096, ...cfg.extra, stream: true, messages: [{ role: "system", content: REPORT_SYSTEM_PROMPT(openuiPrompt) }, { role: "user", content: JSON.stringify(payload) }] }), }).catch((e) => { throw new AnalystError(`Could not reach ${cfg.baseUrl}: ${(e as Error).message}`); }); diff --git a/lib/digest.ts b/lib/digest.ts index fbda422..797a4d7 100644 --- a/lib/digest.ts +++ b/lib/digest.ts @@ -132,7 +132,8 @@ export function digestPlan(raw: string): Digest { // Tokens are the constraint: models and Jev read this, never the raw XML. export const digestForModel = (d: Digest) => ({ - sql: d.sql.slice(0, 4000), subtreeCost: d.subtreeCost, parallel: d.parallel, + sql: d.sql.slice(0, 4000), sql_truncated: d.sql.length > 4000, + subtreeCost: d.subtreeCost, parallel: d.parallel, nonParallelReason: d.nonParallelReason, earlyAbort: d.earlyAbort, tables: d.tables.slice(0, 20), topOperators: d.topOperators, rowGuessErrors: d.rowGuessErrors, missingIndexes: d.missingIndexes.slice(0, 5), warnings: d.warnings.slice(0, 20), waits: d.waits, diff --git a/lib/jev.ts b/lib/jev.ts index 59e1eab..a9299b5 100644 --- a/lib/jev.ts +++ b/lib/jev.ts @@ -50,11 +50,11 @@ function rank(c: Candidate, a: any): Ranked { const composite = (Object.keys(WEIGHTS) as Dim[]).reduce((s, k) => s + WEIGHTS[k] * dims[k].value, 0); const confs = Object.values(dims).map((d) => d.confidence).filter((x): x is number => x !== null); const flags: string[] = []; - // Live Jev scores an index create around 0.57 ("probably equivalent, no proof stated"). A blanket floor - // flagged every index option, so the warning would be noise. Flag where semantics can actually move: - // any rewrite below the soft floor, or anything at all below the hard floor. + // Live Jev scores an index create around 0.57 ("probably equivalent, no proof stated"), so a blanket floor + // flags every index option and the warning becomes noise. Semantics only move when the query text changes: + // flag a rewrite below the soft floor, or anything at all below the hard floor. const safety = dims.semantic_safety.value; - if (safety < SAFETY_HARD_FLOOR || (c.rewrite_sql !== null && safety < SAFETY_FLOOR)) flags.push("verify_semantics"); + if (safety < SAFETY_HARD_FLOOR || (c.option_type === "rewrite" && safety < SAFETY_FLOOR)) flags.push("verify_semantics"); if (confs.length && Math.min(...confs) < CONFIDENCE_FLOOR) flags.push("low_confidence"); if (c.actions.some((x) => x.requires_change_control)) flags.push("change_control"); return { key: c.key, dims, composite: Number(composite.toFixed(4)), flags }; diff --git a/probe-tmp.mjs b/probe-tmp.mjs deleted file mode 100644 index 929b612..0000000 --- a/probe-tmp.mjs +++ /dev/null @@ -1,40 +0,0 @@ -import { chromium } from "@playwright/test"; -import { readFileSync } from "node:fs"; - -const BASE = "http://localhost:3000"; -const b = await chromium.launch(); -const p = await (await b.newContext({ baseURL: BASE, viewport: { width: 1400, height: 1000 } })).newPage(); - -await p.goto("/signup"); -await p.fill("#email", `bigplan${Date.now()}@qai.test`); -await p.fill("#password", "0123456789ab"); -await p.getByRole("button", { name: "Create account" }).click(); -await p.waitForURL(/setup/, { timeout: 20000 }); -await p.getByRole("link", { name: "Next" }).click(); -await p.waitForURL(/step=2/, { timeout: 20000 }); -await p.fill("#analyst_base_url", "http://192.168.88.131:8888/v1"); -await p.fill("#analyst_key", "EMPTY"); -await p.fill("#analyst_model", "qwen3.8-flash-next"); -await p.fill("#analyst_extra", '{"chat_template_kwargs":{"enable_thinking":false}}'); -await p.fill("#jev_key", process.env.JEV_API_KEY || ""); -await p.getByRole("button", { name: "Save" }).click(); -await p.waitForURL(/step=3/, { timeout: 20000 }); - -await p.goto("/app"); -const t0 = Date.now(); -await p.setInputFiles('input[type="file"]', "plan-cxpaket.sqlplan"); -console.log("UPLIFT chars in box:", (await p.locator("#xml").inputValue()).length); -await p.getByRole("button", { name: "Analyse" }).click(); -await p.waitForFunction(() => { - const btn = [...document.querySelectorAll("button")].find((x) => x.textContent === "Analyse"); - return !!btn && !btn.disabled; -}, null, { timeout: 420000 }); -const secs = Math.round((Date.now() - t0) / 1000); -const alert = await p.getByRole("alert").allInnerTexts(); -console.log("RUN", secs + "s", "| alert:", JSON.stringify(alert).slice(0, 200)); -console.log("REPORT headline:", JSON.stringify(await p.locator("#report h2").first().innerText().catch(() => "none"))); -console.log("REPORT cards:", await p.locator("#report .card").count(), - "| sql blocks:", await p.locator("#report pre.sql").count(), - "| verdict:", JSON.stringify((await p.locator("[data-verdict] .label").first().innerText().catch(() => "none")))); -console.log("TABLES MENTIONED:", await p.getByText(/OSUSR_MSQ|dbo\./).count()); -await b.close(); diff --git a/tests/analyst.test.ts b/tests/analyst.test.ts index 6d44d40..8b08b1a 100644 --- a/tests/analyst.test.ts +++ b/tests/analyst.test.ts @@ -10,10 +10,10 @@ const ok = (content: string) => ({ ok: true, status: 200, const CANDS = { candidates: [ { key: "idx_customer", title: "Covering index", diagnosis: "Scan of 1.2M rows on Sales.Orders, no key on CustomerId.", actions: [{ title: "Create index", detail: "CREATE INDEX ...", effort: "medium", requires_change_control: true }], - rewrite_sql: null, expected: "seek instead of scan" }, + option_type: "index", sql_to_run: "CREATE NONCLUSTERED INDEX [IX_Orders_CustomerId] ON [dbo].[Orders] ([CustomerId]);", expected: "seek instead of scan" }, { key: "rewrite_split", title: "Split the statement", diagnosis: "One statement scans everything then filters.", actions: [{ title: "Deploy rewrite", detail: "Two statements", effort: "high", requires_change_control: true }], - rewrite_sql: "SELECT ...", expected: "far fewer logical reads" }] }; + option_type: "rewrite", sql_to_run: "SELECT ...", expected: "far fewer logical reads" }] }; test("returns schema-valid candidates", async () => { const got = await proposeCandidates(cfg, digest, "", async () => ok(JSON.stringify(CANDS)) as any); @@ -43,3 +43,59 @@ test("extractJson copes with fences and chatter", () => { assert.deepEqual(extractJson('Sure!\n```json\n{"a":1}\n```'), { a: 1 }); assert.deepEqual(extractJson('noise {"a":1} noise'), { a: 1 }); }); + +test("a response cut off mid-JSON says it was cut off", async () => { + // With every option required to carry runnable T-SQL, the candidates payload outgrew the old 2048-token + // budget and the user was told "malformed JSON" after 38 seconds of nothing useful. + const { extractJson, AnalystError } = await import("../lib/analyst.ts"); + const truncated = '{"candidates":[{"key":"a","title":"x","diagnosis":"y","actions":[{"title":"t","detail":"d","effort":"low","requires_change_control":false}],"option_type":"index","sql_to_run":"CREATE INDEX [IX_a] ON [dbo].[a] ([b]);","expected":"see'; + let err: unknown = null; + try { extractJson(truncated); } catch (e) { err = e; } + assert.ok(err instanceof AnalystError, String(err)); + assert.match((err as Error).message, /stopped after \d+ characters/); + assert.doesNotThrow(() => extractJson('{"a":1}')); +}); + +test("the candidate call leaves the model room to finish the JSON", async () => { + const { proposeCandidates } = await import("../lib/analyst.ts"); + const calls: any[] = []; + const fetchImpl = async (_u: string, init: any) => { + calls.push(JSON.parse(init.body)); + const two = { candidates: [0, 1].map((i) => ({ key: "k" + i, title: "Covering index " + i, diagnosis: "full scan of dbo.Orders", + actions: [{ title: "Create the index", detail: "in the reporting DB off-peak", effort: "low", requires_change_control: false }], + option_type: "index", sql_to_run: "CREATE INDEX [IX_a] ON [dbo].[a] ([b]);", expected: "fewer reads" })) }; + return { ok: true, json: async () => ({ choices: [{ message: { content: JSON.stringify(two) } }] }) } as any; + }; + const digest = { sql: "SELECT 1", bytes: 100, subtreeCost: 1, parallel: false, nonParallelReason: null, + earlyAbort: null, tables: ["dbo.Orders"], topOperators: [], rowGuessErrors: [], missingIndexes: [], + warnings: [], waits: [] }; + const cfg = { baseUrl: "http://x/v1", apiKey: "k", model: "m", extra: {} }; + await proposeCandidates(cfg as any, digest as any, "", fetchImpl as any); + assert.ok(calls[0].max_tokens >= 4096, `max_tokens was ${calls[0].max_tokens}`); + // extra params still win: that is how a user raises the ceiling without a rebuild. + await proposeCandidates({ ...cfg, extra: { max_tokens: 8192 } } as any, digest as any, "", fetchImpl as any); + assert.equal(calls[1].max_tokens, 8192); +}); + +test("placeholder SQL never reaches the report as if it were runnable", async () => { + // On a plan whose query text is 100 KB the model cannot reproduce the statement, and it offered + // "SELECT ... FROM ... OPTION (MAXDOP 1);" as a rewrite. A block that cannot be pasted is worse than no block. + const { dropPlaceholderSql } = await import("../lib/analyst.ts"); + const cands = [ + { key: "hint_maxdop", option_type: "rewrite", sql_to_run: "SELECT ... FROM dbo.T OPTION (MAXDOP 1);" }, + { key: "idx", option_type: "index", sql_to_run: "CREATE INDEX [IX_T_A] ON [dbo].[T] ([A]);" }, + { key: "ops", option_type: "ops", sql_to_run: null }, + ]; + const out = dropPlaceholderSql(cands as any); + assert.equal(out[0].sql_to_run, null, "the ellipsis rewrite should not survive"); + assert.equal(out[1].sql_to_run, "CREATE INDEX [IX_T_A] ON [dbo].[T] ([A]);"); + assert.equal(out[2].sql_to_run, null); +}); + +test("the digest tells the model when the query text was cut", async () => { + const { digestForModel } = await import("../lib/digest.ts"); + const base = { bytes: 1, subtreeCost: null, parallel: false, nonParallelReason: null, earlyAbort: null, + tables: [], topOperators: [], rowGuessErrors: [], missingIndexes: [], warnings: [], waits: [] }; + assert.equal(digestForModel({ ...base, sql: "SELECT 1" } as any).sql_truncated, false); + assert.equal(digestForModel({ ...base, sql: "SELECT 1" + " ".repeat(5000) } as any).sql_truncated, true); +}); diff --git a/tests/jev.test.ts b/tests/jev.test.ts index 741538b..9692fd8 100644 --- a/tests/jev.test.ts +++ b/tests/jev.test.ts @@ -10,10 +10,10 @@ const digest = { sql: "SELECT", bytes: 1, subtreeCost: 1, parallel: false, nonPa const candidates = [ { key: "a", title: "Covering index", diagnosis: "Scan of a million rows; no key exists on CustomerId.", actions: [{ title: "Create index", detail: "CREATE INDEX", effort: "medium", requires_change_control: true }], - rewrite_sql: null, expected: "seek instead of scan" }, + option_type: "index", sql_to_run: "CREATE NONCLUSTERED INDEX [IX_Orders_CustomerId] ON [dbo].[Orders] ([CustomerId]) INCLUDE ([TotalDue]);", expected: "seek instead of scan" }, { key: "b", title: "Add TOP 100", diagnosis: "Too many rows leave the scan, so cut them off.", actions: [{ title: "Add TOP 100", detail: "TOP 100", effort: "low", requires_change_control: false }], - rewrite_sql: "SELECT TOP 100", expected: "less I/O" }, + option_type: "rewrite", sql_to_run: "SELECT TOP 100 * FROM dbo.Orders ORDER BY TotalDue DESC;", expected: "less I/O" }, ] as any; const scored = (fit: number, safe: number, ease: number, cause: number) => ({ @@ -81,7 +81,7 @@ test("a silent jev does not accuse every option of being unsafe", () => { test("verify_semantics fires where semantics can actually move", async () => { // Real Jev scores an index create ~0.57 ("probably fine, no stated proof"), which a blanket <0.6 floor // flagged as unsafe on every run. Index-only options must not shout; rewrites and bad scores must. - const idxOnly = [{ ...candidates[0], rewrite_sql: null }, candidates[1]]; + const idxOnly = [{ ...candidates[0], option_type: "statistics", sql_to_run: "UPDATE STATISTICS dbo.Orders WITH FULLSCAN;" }, candidates[1]]; const mid = { byKey: { a: scored(4, 2, 2, 1), b: scored(4, 2, 2, 1) }, cross: GOOD.cross }; const v = await judgeCandidates(digest, idxOnly, fakeJev(mid)); assert.ok(!v.order.find((r) => r.key === "a")!.flags.includes("verify_semantics"), "index create flagged");