test(editor): fix two more Phase 3 spec defects found on a second pass

Both are repeats of failure modes already hit in this workstream, which is why
they are worth naming rather than just fixing.

1. SELF-CONTRADICTORY CONTRACT — parseCallContext was specified to return null
   for "WHERE (a > 1 AND " and {sum, 0} for "SELECT sum (". Those are the same
   shape: <identifier><space>(. Given only text, nothing can tell a keyword from
   a function, so no implementation satisfies both. Identical in kind to the
   Phase 2 spec where resolveKeywords was required to equal SQL_KEYWORDS and to
   contain SELECT.

   Resolved by giving the parser one job: report whatever identifier precedes
   the paren, keyword or not. Deciding what is a function belongs to the
   catalog, which is also what stops a column named like a keyword from
   breaking the feature. Added an end-to-end test showing the two halves
   compose: "WHERE (" parses, finds no function, yields no signature.

2. UNSATISFIABLE-AFTER-FIX ASSERTION — the C5 test captured the provider count
   mid-file and required three editors to add exactly one. But the fix moves
   registration to module scope, so an earlier describe in the same file will
   already have registered it and three more editors add ZERO. The test could
   only ever pass before the fix, never after. Same shape as the Phase 1 tests
   that read wrapper.vm.suggestions, which was a prop and could never hold the
   computed.

   Now asserts the order-independent invariant: at most one added here, and
   exactly one SQL provider registered across the whole file.

Still 8 failing for their intended reasons, 1 suite blocked on the unwritten
module, nothing else affected.
This commit is contained in:
Prabhat Sharma 2026-08-02 14:36:18 -07:00
parent 7413edbf76
commit e55e9a2284
2 changed files with 37 additions and 6 deletions

View File

@ -495,10 +495,17 @@ describe("Phase 3 — C5: one provider per language, not per editor", () => {
}
const added = reg.mock.calls.filter((c) => c[0] === "sql").length - providersBefore;
// Monaco aggregates every provider registered for a language, so N editors
// meant N providers answering on every keystroke — N-1 of them only to
// return an empty list for a model that did not ask.
expect(added, `three editors registered ${added} SQL completion providers`).toBe(1);
const total = reg.mock.calls.filter((c) => c[0] === "sql").length;
// Asserting `added === 1` would be order-dependent AND unsatisfiable once
// the fix lands: registration moves to module scope, so an earlier
// describe in this file has already done it and three more editors add
// ZERO. The order-independent invariant is that the module ever registers
// one SQL provider. Monaco aggregates every provider registered for a
// language, so today each editor adds another that answers every
// keystroke only to return an empty list for a model that did not ask.
expect(added, `three editors added ${added} SQL completion providers`).toBeLessThanOrEqual(1);
expect(total, `${total} SQL completion providers registered in this file`).toBe(1);
},
);
});

View File

@ -114,13 +114,37 @@ describe("parseCallContext — locating the enclosing call", () => {
});
});
it("ignores a bare parenthesised group with no function name", () => {
expect(parseCallContext("WHERE (a > 1 AND ")).toBeNull();
it("returns null when the paren has no identifier before it at all", () => {
expect(parseCallContext("WHERE (")).toBeNull();
expect(parseCallContext("(")).toBeNull();
});
it("tolerates whitespace between the name and the paren", () => {
expect(parseCallContext("SELECT sum (")).toEqual({ name: "sum", activeParameter: 0 });
});
// This parser is purely syntactic: it reports whatever identifier precedes
// the open paren, including a SQL keyword. It cannot do otherwise — `WHERE (`
// and `sum (` are the same shape, and the text alone does not say which is a
// function. Rejecting non-functions is the catalog's job (see below), which
// is also what keeps a column named like a keyword from breaking anything.
it("reports a preceding keyword rather than trying to judge it", () => {
expect(parseCallContext("WHERE (a > 1 AND ")).toEqual({
name: "WHERE",
activeParameter: 0,
});
});
});
describe("keyword-shaped call sites produce no signature", () => {
it("the catalog rejects what the parser cannot", () => {
// End to end: `WHERE (` parses, then finds no function, so the provider has
// nothing to show. Neither half can make that decision alone.
const ctx = parseCallContext("WHERE (a > 1 AND ")!;
expect(ctx.name).toBe("WHERE");
expect(findFunctionEntry(ctx.name, [], SQL_FUNCTIONS)).toBeNull();
expect(buildSignatureHelp(findFunctionEntry(ctx.name, [], SQL_FUNCTIONS), 0)).toBeNull();
});
});
// ───────────────────────────────────────────────────────────────────────────