fix: candidates carry runnable T-SQL; placeholder sql is dropped, not rendered
Measured on a 2.7 MB production plan (plan-cxpaket.sqlplan, kept out of git): - rewrite_sql becomes option_type + sql_to_run. The old name told the model 'query rewrite only', so an index option carried no CREATE INDEX anywhere in the pipeline and the report had zero SqlBlocks. It now requires paste-ready T-SQL per option, and the report prompt makes a SqlBlock per non-null sql_to_run a hard requirement. - verify_semantics keys on option_type === 'rewrite' rather than 'has sql', so index and statistics options are not flagged just for carrying DDL. - dropPlaceholderSql: 'SELECT ... FROM ... OPTION (MAXDOP 1)' is not runnable and was rendered as if it were. Ellipsis SQL is now nulled, and digestForModel reports sql_truncated so the model is told not to offer a rewrite for a statement it never received. - max_tokens 2048 -> 4096 for both calls: the larger JSON outgrew the old ceiling and the user waited 38s for 'malformed JSON'. A truncated response now says how many characters it got, and extra params still override the ceiling. - Real plan, live: 52s, 17 cards, 3 SqlBlocks with real table names and no placeholders, 3 ScoreBars, headline 'Query Plan Timeout on Parallel Join'. 43 unit, 23 e2e. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
3fa436103a
commit
6248540857
+58
-2
@@ -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);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user