From dd0148d8e92e4854979eec1db4c441c3d17df29d Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Wed, 7 Oct 2026 11:13:03 -0700 Subject: [PATCH 01/10] fix(validators): stop reading backticked columns and variables as required models `extractRequiredDeliverables` accepted every inline code span on a requirement line as a model. Spans the line itself introduces as a column, field, variable or macro ("column called `x`", "`x` column", "should have `a`, `b`", "variable ... called `v`", "rename column `a` to `b`") are now dropped; a bare name with no such wording is still required. Closes #1421 Co-Authored-By: Claude Opus 5.5 (1M context) --- .../altimate/validators/validator-utils.ts | 56 +++++++++++- .../validators/dbt-deliverable-names.test.ts | 86 +++++++++++++++++++ 2 files changed, 141 insertions(+), 1 deletion(-) diff --git a/packages/opencode/src/altimate/validators/validator-utils.ts b/packages/opencode/src/altimate/validators/validator-utils.ts index e1d85c476e..afa008085e 100644 --- a/packages/opencode/src/altimate/validators/validator-utils.ts +++ b/packages/opencode/src/altimate/validators/validator-utils.ts @@ -694,7 +694,7 @@ export function extractRequiredDeliverables(text: string): RequiredDeliverables // required makes the deliverable gate reject the correct implementation // forever, so a negated verb disqualifies the whole line. if (verbIsNegated(line, verb.index)) continue - let spans = inlineCodeSpans(requirementHead(line, verb.index)) + let spans = modelCandidateSpans(requirementHead(line, verb.index)) // "Rename `old_orders` to `new_orders`" names two artifacts, but only the // destination is required to exist once the rename is done — the source // is expected to be GONE. `to` is not a qualifier `requirementHead` cuts @@ -772,6 +772,60 @@ function inlineCodeSpans(line: string): string[] { return out } +/** + * Words that mark the code span they introduce as something other than a + * model: a column, a dbt variable, a macro. "Add the column `order_id`" and + * "add a variable called `use_x`" name real things, but not relations, so + * requiring a model by that name blocks a correct implementation forever. + */ +const NON_MODEL_KIND = "columns?|fields?|attributes?|variables?|vars?|macros?|settings?|parameters?" +/** Kind word, then at most a short phrase, then "called"/"named" or a colon, ending the gap before a span. */ +const INTRODUCED_AS_NON_MODEL_RE = new RegExp( + `\\b(?:${NON_MODEL_KIND})\\b(?:(?:[^.;:!?]|\\.(?=\\w)){0,60}\\b(?:called|named)|\\s*:|\\s*-|\\s*\\()?\\s*$`, + "i", +) +/** "`x` column", "`x` variable": the kind word follows the span. */ +const FOLLOWED_BY_NON_MODEL_RE = new RegExp(`^\\s*(?:${NON_MODEL_KIND})\\b`, "i") +/** "should have `a`, `b`": what a model has is its columns. */ +const HAS_LIST_RE = /\b(?:have|has|having)\s+(?:the\s+)?(?:following\s+)?(?:(?:new|extra|additional)\s+)?$/i +/** Separator between items of a list of spans, allowing a short parenthetical note after an item. */ +const LIST_SEPARATOR_RE = /^\s*(?:\([^)`]*\))?\s*(?:,|;|,?\s*(?:and|or|&))?\s*$/i +/** "rename column `a` to `b`": the target of a column rename is a column. */ +const RENAME_TARGET_RE = new RegExp(`^\\s*(?:(?:${NON_MODEL_KIND})\\s+)?(?:to|into|as)\\s*$`, "i") + +/** + * Code spans of a requirement line that can name a model: every span except + * those the line itself introduces as a column, field, variable or macro. + * A span is dropped only on explicit wording around it ("column called `x`", + * "`x` column", "should have `a`, `b`", a list continuing such a span); with no + * such wording it stays, so a bare name is still required. + */ +function modelCandidateSpans(line: string): string[] { + const out: string[] = [] + CODE_SPAN_RE.lastIndex = 0 + let prevEnd = 0 + let prevNonModel = false + let m: RegExpExecArray | null + const matches: { text: string; start: number; end: number }[] = [] + while ((m = CODE_SPAN_RE.exec(line)) !== null) { + if (m[1]) matches.push({ text: m[1], start: m.index, end: m.index + m[0].length }) + } + for (let i = 0; i < matches.length; i++) { + const cur = matches[i]! + const before = line.slice(prevEnd, cur.start) + const after = line.slice(cur.end, matches[i + 1]?.start ?? line.length) + const nonModel: boolean = + INTRODUCED_AS_NON_MODEL_RE.test(before) || + FOLLOWED_BY_NON_MODEL_RE.test(after) || + HAS_LIST_RE.test(before) || + (prevNonModel && (LIST_SEPARATOR_RE.test(before) || RENAME_TARGET_RE.test(before))) + if (!nonModel) out.push(cur.text) + prevNonModel = nonModel + prevEnd = cur.end + } + return out +} + /** * Top-level path segments that never produce a dbt relation, even for a * `.sql`/`.csv` file within them. A required `macros/generate_schema_name.sql` diff --git a/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts b/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts index 07dd49118a..82c7e6c175 100644 --- a/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts +++ b/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts @@ -168,4 +168,90 @@ describe("DbtDeliverableNamesValidator — check", () => { expect(r.ok).toBe(true) }) }) + +// A code span in a requirement line is only a model when nothing marks it as +// something else. These cases are the shapes found in real benchmark requests +// (column lists, "column called", dbt variables) plus the shapes that must +// keep speaking. +describe("DbtDeliverableNamesValidator — code spans that are not models", () => { + async function requiredModels(task: string): Promise { + await makeProject() + await writeTask(task) + const r = await DbtDeliverableNamesValidator.check(ctx()) + return r.details!["required_models"] as string[] + } + + test("a column list after 'should have' is not required as models", async () => { + const models = await requiredModels( + "Create a `dim_superhost_evolution` model. It should have `is_currently_superhost`, `status_change_count` and `last_status_change_at`.\n", + ) + expect(models).toEqual(["dim_superhost_evolution"]) + }) + + test("a column list with parenthetical notes is still all columns", async () => { + const models = await requiredModels( + "Create a `dim_superhost_evolution` model. It should have `is_currently_superhost`, `acct_age_before_achieving_superhost` (for first time), `status_change_count`, `last_status_change_at`.\n", + ) + expect(models).toEqual(["dim_superhost_evolution"]) + }) + + test("passes when the model exists and only its columns are in backticks", async () => { + await makeProject() + await writeTask("Create a model `dim_superhost_evolution` that has `is_currently_superhost`, `status_change_count`.\n") + await writeModel("dim_superhost_evolution.sql") + expect((await DbtDeliverableNamesValidator.check(ctx())).ok).toBe(true) + }) + + test("'a `x` column to the model `m`' requires the model and not the column", async () => { + expect(await requiredModels("Add a `department` column to the model `int_workspace_roster`.\n")).toEqual([ + "int_workspace_roster", + ]) + }) + + test("'column called `x`' is not a model", async () => { + expect( + await requiredModels("Create a dbt model called `analysis__answer` that has one column called `answer`.\n"), + ).toEqual(["analysis__answer"]) + }) + + test("'columns `a`, `b` and `c`' are all columns", async () => { + expect( + await requiredModels("Add the columns `order_id`, `order_total` and `placed_at` to the model `fct_orders`.\n"), + ).toEqual(["fct_orders"]) + }) + + test("a renamed column is not a model, the model it lives in still is", async () => { + expect( + await requiredModels("Rename the `old_status` column to `new_status` in the model `stg_accounts`.\n"), + ).toEqual(["stg_accounts"]) + }) + + test("a dbt variable is not a model", async () => { + expect( + await requiredModels( + "Fix the model: add a global variable in dbt_project.yml called `surrogate_key_treat_nulls_as_empty_strings` to the `monthly_agg_reviews` model.\n", + ), + ).toEqual(["monthly_agg_reviews"]) + }) + + test("still requires every model in a list of models", async () => { + expect(await requiredModels("Create the models `stg_orders` and `stg_customers`.\n")).toEqual([ + "stg_orders", + "stg_customers", + ]) + }) + + test("a model that merely follows the word 'include' is still required", async () => { + expect(await requiredModels("Build the marts to include the model `fct_orders`.\n")).toEqual(["fct_orders"]) + }) + + test("still fails when the model is missing even though its columns are listed", async () => { + await makeProject() + await writeTask("Create a `dim_superhost_evolution` model. It should have `is_currently_superhost`.\n") + const r = await DbtDeliverableNamesValidator.check(ctx()) + expect(r.ok).toBe(false) + expect(r.details!["missing_models"]).toEqual(["dim_superhost_evolution"]) + expect(r.reason).not.toContain("is_currently_superhost") + }) +}) // altimate_change end From 679021b42ec024f4a51e406d983d3fef1978bc3e Mon Sep 17 00:00:00 2001 From: Anand Gupta Date: Wed, 7 Oct 2026 11:17:48 -0700 Subject: [PATCH 02/10] fix(validators): keep the model required when a column span sits beside it Independent review showed the first version of the span filter could drop the only model name on a line ("Add a `x` column to `m` model"), which makes the contract read as absent and silences `dbt-deliverable-names` and `dbt-nothing-built`. A span introduced or followed by a deliverable noun is now never dropped, "called" only binds to a column word within the same clause and without an intervening noun, "have/contain/expose" lists drop only after an earlier kept span, the rename target rule applies only to rename lines, and "`x` column names" is no longer read as a column. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../altimate/validators/validator-utils.ts | 58 ++++++++++++++----- .../validators/dbt-deliverable-names.test.ts | 39 +++++++++++++ 2 files changed, 84 insertions(+), 13 deletions(-) diff --git a/packages/opencode/src/altimate/validators/validator-utils.ts b/packages/opencode/src/altimate/validators/validator-utils.ts index afa008085e..79f847500d 100644 --- a/packages/opencode/src/altimate/validators/validator-utils.ts +++ b/packages/opencode/src/altimate/validators/validator-utils.ts @@ -694,7 +694,7 @@ export function extractRequiredDeliverables(text: string): RequiredDeliverables // required makes the deliverable gate reject the correct implementation // forever, so a negated verb disqualifies the whole line. if (verbIsNegated(line, verb.index)) continue - let spans = modelCandidateSpans(requirementHead(line, verb.index)) + let spans = modelCandidateSpans(requirementHead(line, verb.index), RENAME_VERB_RE.test(verb[0])) // "Rename `old_orders` to `new_orders`" names two artifacts, but only the // destination is required to exist once the rename is done — the source // is expected to be GONE. `to` is not a qualifier `requirementHead` cuts @@ -779,15 +779,34 @@ function inlineCodeSpans(line: string): string[] { * requiring a model by that name blocks a correct implementation forever. */ const NON_MODEL_KIND = "columns?|fields?|attributes?|variables?|vars?|macros?|settings?|parameters?" -/** Kind word, then at most a short phrase, then "called"/"named" or a colon, ending the gap before a span. */ -const INTRODUCED_AS_NON_MODEL_RE = new RegExp( - `\\b(?:${NON_MODEL_KIND})\\b(?:(?:[^.;:!?]|\\.(?=\\w)){0,60}\\b(?:called|named)|\\s*:|\\s*-|\\s*\\()?\\s*$`, +/** + * Kind word, a short phrase, then "called"/"named"; or a kind word and a colon, + * dash or opening parenthesis — ending the gap before a span. Applied to one + * clause of the gap that holds no deliverable noun, so a later "called" that + * belongs to a model ("a settings model called `app_settings`") does not hand + * its name to the kind word. + */ +const INTRODUCED_CALLED_RE = new RegExp( + `\\b(?:${NON_MODEL_KIND})\\b(?:[^.;:,!?]|\\.(?=\\w)){0,60}?\\b(?:called|named)\\s*$`, "i", ) -/** "`x` column", "`x` variable": the kind word follows the span. */ -const FOLLOWED_BY_NON_MODEL_RE = new RegExp(`^\\s*(?:${NON_MODEL_KIND})\\b`, "i") -/** "should have `a`, `b`": what a model has is its columns. */ -const HAS_LIST_RE = /\b(?:have|has|having)\s+(?:the\s+)?(?:following\s+)?(?:(?:new|extra|additional)\s+)?$/i +const INTRODUCED_COLON_RE = new RegExp(`\\b(?:${NON_MODEL_KIND})\\b\\s*[:\\-(]?\\s*$`, "i") +/** A span directly introduced by a deliverable noun ("the model `x`", "table called `x`") is a model. */ +const INTRODUCED_AS_MODEL_RE = + /\b(?:models?|tables?|views?|seeds?|snapshots?|marts?|files?)\s*(?:(?:called|named)\s*)?:?\s*$/i +/** ... and so is a span followed by one ("`x` model"). */ +const FOLLOWED_BY_MODEL_NOUN_RE = /^\s*(?:models?|tables?|views?|seeds?|snapshots?|files?)\b/i +/** + * "`x` column", "`x` variable": the kind word follows the span and ends the + * noun phrase. "`x` column names" is attributive and does not qualify. + */ +const FOLLOWED_BY_NON_MODEL_RE = new RegExp( + `^\\s*(?:${NON_MODEL_KIND})\\b(?=\\s*(?:$|[,.;:)]|(?:to|in|of|on|from|that|which|with|for|as|and|or|is|are|should|must|so|called|named|by)\\b))`, + "i", +) +/** "should have `a`, `b`": what a model has or exposes is its columns. */ +const HAS_LIST_RE = + /\b(?:have|has|having|contains?|containing|exposes?|exposing)\s+(?:the\s+)?(?:following\s+)?(?:(?:new|extra|additional)\s+)?$/i /** Separator between items of a list of spans, allowing a short parenthetical note after an item. */ const LIST_SEPARATOR_RE = /^\s*(?:\([^)`]*\))?\s*(?:,|;|,?\s*(?:and|or|&))?\s*$/i /** "rename column `a` to `b`": the target of a column rename is a column. */ @@ -799,8 +818,15 @@ const RENAME_TARGET_RE = new RegExp(`^\\s*(?:(?:${NON_MODEL_KIND})\\s+)?(?:to|in * A span is dropped only on explicit wording around it ("column called `x`", * "`x` column", "should have `a`, `b`", a list continuing such a span); with no * such wording it stays, so a bare name is still required. + * + * Dropping errs toward keeping: a span introduced or followed by a deliverable + * noun is never dropped, "have `x`" drops only when an earlier span of the line + * is already kept (so "the project should have `stg_a` and `stg_b`" keeps both), + * and the target of "to"/"as" is dropped only on a rename line. Dropping a real + * model could leave the line with no name at all, which makes the whole + * contract read as absent and silences both completion gates. */ -function modelCandidateSpans(line: string): string[] { +function modelCandidateSpans(line: string, renameLine: boolean): string[] { const out: string[] = [] CODE_SPAN_RE.lastIndex = 0 let prevEnd = 0 @@ -814,11 +840,17 @@ function modelCandidateSpans(line: string): string[] { const cur = matches[i]! const before = line.slice(prevEnd, cur.start) const after = line.slice(cur.end, matches[i + 1]?.start ?? line.length) + // "column ... called `x`" counts only inside one clause that names no deliverable noun. + const clause = before.split(/[;:!?,]|\.(?!\w)/).pop() ?? "" + const introducedAsNonModel = + (INTRODUCED_CALLED_RE.test(clause) && !DELIVERABLE_NOUN_RE.test(clause)) || INTRODUCED_COLON_RE.test(before) const nonModel: boolean = - INTRODUCED_AS_NON_MODEL_RE.test(before) || - FOLLOWED_BY_NON_MODEL_RE.test(after) || - HAS_LIST_RE.test(before) || - (prevNonModel && (LIST_SEPARATOR_RE.test(before) || RENAME_TARGET_RE.test(before))) + !INTRODUCED_AS_MODEL_RE.test(before) && + !FOLLOWED_BY_MODEL_NOUN_RE.test(after) && + (introducedAsNonModel || + FOLLOWED_BY_NON_MODEL_RE.test(after) || + (out.length > 0 && HAS_LIST_RE.test(before)) || + (prevNonModel && (LIST_SEPARATOR_RE.test(before) || (renameLine && RENAME_TARGET_RE.test(before))))) if (!nonModel) out.push(cur.text) prevNonModel = nonModel prevEnd = cur.end diff --git a/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts b/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts index 82c7e6c175..3c8e315912 100644 --- a/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts +++ b/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts @@ -245,6 +245,45 @@ describe("DbtDeliverableNamesValidator — code spans that are not models", () = expect(await requiredModels("Build the marts to include the model `fct_orders`.\n")).toEqual(["fct_orders"]) }) + test("a rename's column target is not a model even when it is the last span", async () => { + expect( + await requiredModels("In the model `stg_accounts`, rename column `old_status` to `new_status`.\n"), + ).toEqual(["stg_accounts"]) + }) + + // The model is the only name on these lines. Dropping it would make the + // contract read as absent and silence both completion gates, so each of + // these must keep requiring it. + test.each([ + ["Add a `department` column to `int_workspace_roster` model.", "int_workspace_roster"], + ["Add the column `order_id` to `fct_orders` table.", "fct_orders"], + ["Add the columns `order_id`, `order_total` and `placed_at` to `fct_orders` model.", "fct_orders"], + ["Add a `department` column to the model called `int_workspace_roster`.", "int_workspace_roster"], + ["Add a new column in the model named `fct_orders`.", "fct_orders"], + ["Create a settings model called `app_settings`.", "app_settings"], + ["Create a table of customer attributes called `dim_customer_attributes`.", "dim_customer_attributes"], + ["Create a model that aggregates the `amount` column, called `fct_amounts`.", "fct_amounts"], + ["Update the model `stg_orders` columns so they are snake_case.", "stg_orders"], + ])("the model stays required: %s", async (task, model) => { + expect(await requiredModels(task + "\n")).toEqual([model]) + // The contract must still exist, or the gate is never consulted. + expect(await DbtDeliverableNamesValidator.appliesTo(ctx())).toBe(true) + }) + + test("models a project should have are still required", async () => { + expect( + await requiredModels("Create it so the project should have `stg_orders` and `stg_customers` models.\n"), + ).toEqual(["stg_orders", "stg_customers"]) + expect(await requiredModels("Make sure the marts folder has `fct_orders` built as a table.\n")).toEqual([ + "fct_orders", + ]) + }) + + test("'contains' and 'exposes' lists are columns", async () => { + expect(await requiredModels("Create a `dim_x` model. It should contain `col_a`, `col_b`.\n")).toEqual(["dim_x"]) + expect(await requiredModels("Create a `dim_x` model. It must expose `col_a` and `col_b`.\n")).toEqual(["dim_x"]) + }) + test("still fails when the model is missing even though its columns are listed", async () => { await makeProject() await writeTask("Create a `dim_superhost_evolution` model. It should have `is_currently_superhost`.\n") From 040f83fdf6a97d0f34b1cbfda45ab34ddd5dfec4 Mon Sep 17 00:00:00 2001 From: Anand Gupta Date: Wed, 7 Oct 2026 11:32:53 -0700 Subject: [PATCH 03/10] fix(validators): address review of the span filter - a trailing "columns" now applies to the whole list it ends - a path-shaped span is never dropped, so the file check still sees it - "a column called `x`" binds on the determiner, a later model noun no longer vetoes it - "have `x`" in a new sentence drops only when the subject is the model just described - rename context is read from the clause, not the first requirement verb - "add a column to a model" is recorded as a modification of that model, so the nothing-built gate needs session evidence for it - tests no longer leave a temp project behind when a case builds two Co-Authored-By: Claude Opus 5.5 (1M context) --- .../altimate/validators/validator-utils.ts | 105 ++++++++++++------ .../validators/dbt-deliverable-names.test.ts | 45 ++++++++ 2 files changed, 116 insertions(+), 34 deletions(-) diff --git a/packages/opencode/src/altimate/validators/validator-utils.ts b/packages/opencode/src/altimate/validators/validator-utils.ts index 79f847500d..9c1a1145f3 100644 --- a/packages/opencode/src/altimate/validators/validator-utils.ts +++ b/packages/opencode/src/altimate/validators/validator-utils.ts @@ -610,6 +610,8 @@ const MODIFICATION_VERB_RE = * `requirementHead`'s caller in `extractRequiredDeliverables`. */ const RENAME_VERB_RE = /^renam/i +/** "add" — creation of a column on an existing model when a column span was filtered from the line. */ +const ADD_VERB_RE = /^add/i /** Noun that makes the requirement about a data artifact. */ const DELIVERABLE_NOUN_RE = /\b(?:model|models|table|tables|view|views|seed|seeds|snapshot|snapshots|mart|marts|file|files)\b/i @@ -694,7 +696,9 @@ export function extractRequiredDeliverables(text: string): RequiredDeliverables // required makes the deliverable gate reject the correct implementation // forever, so a negated verb disqualifies the whole line. if (verbIsNegated(line, verb.index)) continue - let spans = modelCandidateSpans(requirementHead(line, verb.index), RENAME_VERB_RE.test(verb[0])) + const head = requirementHead(line, verb.index) + const candidates = modelCandidateSpans(head, /\brenam(?:e|es|ed|ing)\b/i.test(head)) + let spans = candidates.spans // "Rename `old_orders` to `new_orders`" names two artifacts, but only the // destination is required to exist once the rename is done — the source // is expected to be GONE. `to` is not a qualifier `requirementHead` cuts @@ -706,7 +710,13 @@ export function extractRequiredDeliverables(text: string): RequiredDeliverables spans = [spans[spans.length - 1]!] } proseTokens.push(...spans) - if (MODIFICATION_VERB_RE.test(verb[0])) modificationTokens.push(...spans) + // "Add a `department` column to the model `m`" asks for a change to `m` + // even though "add" is not a modification verb: the filtered column span is + // the evidence. Without this, `m` merely existing before the session would + // satisfy the nothing-built gate. + if (MODIFICATION_VERB_RE.test(verb[0]) || (candidates.dropped && ADD_VERB_RE.test(verb[0]))) { + modificationTokens.push(...spans) + } } const prose = collectDeliverableTokens(proseTokens) if (prose.models.length > 0 || prose.files.length > 0) { @@ -780,13 +790,17 @@ function inlineCodeSpans(line: string): string[] { */ const NON_MODEL_KIND = "columns?|fields?|attributes?|variables?|vars?|macros?|settings?|parameters?" /** - * Kind word, a short phrase, then "called"/"named"; or a kind word and a colon, - * dash or opening parenthesis — ending the gap before a span. Applied to one - * clause of the gap that holds no deliverable noun, so a later "called" that - * belongs to a model ("a settings model called `app_settings`") does not hand - * its name to the kind word. + * "a column called", "the variable named": a kind word led by a determiner, + * a short phrase without a deliverable noun, then "called"/"named". Also + * accepted without a determiner when the whole clause holds no deliverable + * noun ("has columns called"). A deliverable noun before an undetermined kind + * word ("a table of customer attributes called `x`") leaves the span a model. */ const INTRODUCED_CALLED_RE = new RegExp( + `\\b(?:a|an|the|one|another|each|every|this|that)\\s+(?:(?:new|extra|additional)\\s+)?(?:${NON_MODEL_KIND})\\b((?:[^.;:,!?]|\\.(?=\\w)){0,60}?)\\b(?:called|named)\\s*$`, + "i", +) +const INTRODUCED_BARE_CALLED_RE = new RegExp( `\\b(?:${NON_MODEL_KIND})\\b(?:[^.;:,!?]|\\.(?=\\w)){0,60}?\\b(?:called|named)\\s*$`, "i", ) @@ -807,10 +821,16 @@ const FOLLOWED_BY_NON_MODEL_RE = new RegExp( /** "should have `a`, `b`": what a model has or exposes is its columns. */ const HAS_LIST_RE = /\b(?:have|has|having|contains?|containing|exposes?|exposing)\s+(?:the\s+)?(?:following\s+)?(?:(?:new|extra|additional)\s+)?$/i +/** Subject of a "have" sentence that is the model just described rather than a new requirement. */ +const MODEL_PRONOUN_SUBJECT_RE = /^\s*(?:it|they|this|that|each|the\s+(?:model|table|view|snapshot|seed))\b/i /** Separator between items of a list of spans, allowing a short parenthetical note after an item. */ const LIST_SEPARATOR_RE = /^\s*(?:\([^)`]*\))?\s*(?:,|;|,?\s*(?:and|or|&))?\s*$/i /** "rename column `a` to `b`": the target of a column rename is a column. */ const RENAME_TARGET_RE = new RegExp(`^\\s*(?:(?:${NON_MODEL_KIND})\\s+)?(?:to|into|as)\\s*$`, "i") +/** A literal path or file name stays eligible for the file check even when it is not a relation. */ +const PATH_SHAPED_RE = /[\\/]|\.(?:sql|csv|ya?ml)$/i +/** End of a sentence inside a gap between spans. */ +const SENTENCE_END_RE = /[.!?](?=\s|$)/ /** * Code spans of a requirement line that can name a model: every span except @@ -820,42 +840,59 @@ const RENAME_TARGET_RE = new RegExp(`^\\s*(?:(?:${NON_MODEL_KIND})\\s+)?(?:to|in * such wording it stays, so a bare name is still required. * * Dropping errs toward keeping: a span introduced or followed by a deliverable - * noun is never dropped, "have `x`" drops only when an earlier span of the line - * is already kept (so "the project should have `stg_a` and `stg_b`" keeps both), - * and the target of "to"/"as" is dropped only on a rename line. Dropping a real - * model could leave the line with no name at all, which makes the whole - * contract read as absent and silences both completion gates. - */ -function modelCandidateSpans(line: string, renameLine: boolean): string[] { + * noun is never dropped, a path-shaped span is never dropped (the file check + * still needs it), "have `x`" drops only when an earlier span of the sentence + * is kept or the sentence continues the description of the model ("It should + * have ..."), and the target of "to"/"as" is dropped only on a rename line. + * Dropping a real model could leave the line with no name at all, which makes + * the whole contract read as absent and silences both completion gates. + * `dropped` tells the caller a column-like span was removed from the line. + */ +function modelCandidateSpans(line: string, renameLine: boolean): { spans: string[]; dropped: boolean } { const out: string[] = [] CODE_SPAN_RE.lastIndex = 0 - let prevEnd = 0 - let prevNonModel = false let m: RegExpExecArray | null const matches: { text: string; start: number; end: number }[] = [] while ((m = CODE_SPAN_RE.exec(line)) !== null) { if (m[1]) matches.push({ text: m[1], start: m.index, end: m.index + m[0].length }) } + const gapBefore = (i: number) => line.slice(i === 0 ? 0 : matches[i - 1]!.end, matches[i]!.start) + const gapAfter = (i: number) => line.slice(matches[i]!.end, matches[i + 1]?.start ?? line.length) + const isModelLike = (i: number) => + PATH_SHAPED_RE.test(matches[i]!.text) || + INTRODUCED_AS_MODEL_RE.test(gapBefore(i)) || + FOLLOWED_BY_MODEL_NOUN_RE.test(gapAfter(i)) + const nonModel: boolean[] = matches.map(() => false) + + // Trailing kind word ("`a` and `b` columns"): applies to the whole list it ends. + for (let i = matches.length - 1; i >= 0; i--) { + if (isModelLike(i)) continue + if (FOLLOWED_BY_NON_MODEL_RE.test(gapAfter(i))) nonModel[i] = true + else if (nonModel[i + 1] && LIST_SEPARATOR_RE.test(gapAfter(i))) nonModel[i] = true + } for (let i = 0; i < matches.length; i++) { - const cur = matches[i]! - const before = line.slice(prevEnd, cur.start) - const after = line.slice(cur.end, matches[i + 1]?.start ?? line.length) - // "column ... called `x`" counts only inside one clause that names no deliverable noun. - const clause = before.split(/[;:!?,]|\.(?!\w)/).pop() ?? "" - const introducedAsNonModel = - (INTRODUCED_CALLED_RE.test(clause) && !DELIVERABLE_NOUN_RE.test(clause)) || INTRODUCED_COLON_RE.test(before) - const nonModel: boolean = - !INTRODUCED_AS_MODEL_RE.test(before) && - !FOLLOWED_BY_MODEL_NOUN_RE.test(after) && - (introducedAsNonModel || - FOLLOWED_BY_NON_MODEL_RE.test(after) || - (out.length > 0 && HAS_LIST_RE.test(before)) || - (prevNonModel && (LIST_SEPARATOR_RE.test(before) || (renameLine && RENAME_TARGET_RE.test(before))))) - if (!nonModel) out.push(cur.text) - prevNonModel = nonModel - prevEnd = cur.end + const before = gapBefore(i) + const afterBoundary = before.split(SENTENCE_END_RE).pop() ?? "" + const sentenceBroke = SENTENCE_END_RE.test(before) + if (!isModelLike(i) && !nonModel[i]) { + // "column ... called `x`" counts only inside one clause that holds no deliverable noun + // before the kind word. + const clause = before.split(/[;:!?,]|\.(?!\w)/).pop() ?? "" + const called = INTRODUCED_CALLED_RE.exec(clause) + nonModel[i] = + (called !== null && !DELIVERABLE_NOUN_RE.test(called[1] ?? "")) || + (INTRODUCED_BARE_CALLED_RE.test(clause) && !DELIVERABLE_NOUN_RE.test(clause)) || + INTRODUCED_COLON_RE.test(before) || + (HAS_LIST_RE.test(before) && + out.length > 0 && + (!sentenceBroke || MODEL_PRONOUN_SUBJECT_RE.test(afterBoundary))) || + (i > 0 && + nonModel[i - 1] && + (LIST_SEPARATOR_RE.test(before) || (renameLine && RENAME_TARGET_RE.test(before)))) + } + if (!nonModel[i]) out.push(matches[i]!.text) } - return out + return { spans: out, dropped: nonModel.some(Boolean) } } /** diff --git a/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts b/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts index 3c8e315912..994758d3d8 100644 --- a/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts +++ b/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts @@ -4,6 +4,7 @@ import { promises as fs } from "fs" import { tmpdir } from "os" import { join } from "path" import { DbtDeliverableNamesValidator } from "../../../src/altimate/validators/dbt-deliverable-names" +import { extractRequiredDeliverables } from "../../../src/altimate/validators/validator-utils" import type { ValidatorContext } from "../../../src/session/validators/types" let dir = "" @@ -175,6 +176,8 @@ describe("DbtDeliverableNamesValidator — check", () => { // keep speaking. describe("DbtDeliverableNamesValidator — code spans that are not models", () => { async function requiredModels(task: string): Promise { + // Some tests call this twice; drop the previous project so none is left behind. + if (dir) await fs.rm(dir, { recursive: true, force: true }) await makeProject() await writeTask(task) const r = await DbtDeliverableNamesValidator.check(ctx()) @@ -284,6 +287,48 @@ describe("DbtDeliverableNamesValidator — code spans that are not models", () = expect(await requiredModels("Create a `dim_x` model. It must expose `col_a` and `col_b`.\n")).toEqual(["dim_x"]) }) + test("a trailing 'columns' applies to the whole list before it", async () => { + expect(await requiredModels("Add the `first_name` and `last_name` columns to the model `customers`.\n")).toEqual([ + "customers", + ]) + }) + + test("a path-shaped span is still checked as a file when it follows a kind word", async () => { + await makeProject() + await writeTask("Create macro `macros/helper.sql` for the model `orders`.\n") + await writeModel("orders.sql") + const r = await DbtDeliverableNamesValidator.check(ctx()) + expect(r.ok).toBe(false) + expect(r.details!["missing_files"]).toEqual(["macros/helper.sql"]) + }) + + test("a column called `x` is a column even when 'model' appears earlier in the clause", () => { + expect(extractRequiredDeliverables("Update the model to add a column called `status_flag`.\n")).toBeNull() + expect( + extractRequiredDeliverables("Update the model `orders` to add a column called `status_flag`.\n")!.models, + ).toEqual(["orders"]) + }) + + test("a second model after 'have' in a new sentence is still required", async () => { + expect( + await requiredModels("Create model `stg_orders`. The project should also have `fct_orders`.\n"), + ).toEqual(["stg_orders", "fct_orders"]) + }) + + test("a rename after another verb still drops the column target", async () => { + expect( + await requiredModels("Update the model `accounts` and rename column `old_status` to `new_status`.\n"), + ).toEqual(["accounts"]) + }) + + test("adding a column to an existing model is a modification of that model", () => { + const r = extractRequiredDeliverables("Add a `department` column to the model `int_workspace_roster`.\n") + expect(r!.models).toEqual(["int_workspace_roster"]) + expect(r!.modificationModels).toEqual(["int_workspace_roster"]) + // A plain creation stays a creation. + expect(extractRequiredDeliverables("Create the model `fct_orders`.\n")!.modificationModels).toEqual([]) + }) + test("still fails when the model is missing even though its columns are listed", async () => { await makeProject() await writeTask("Create a `dim_superhost_evolution` model. It should have `is_currently_superhost`.\n") From fbdea405e9e07ead183accef08d945c735fbf4e2 Mon Sep 17 00:00:00 2001 From: Anand Gupta Date: Wed, 7 Oct 2026 11:33:22 -0700 Subject: [PATCH 04/10] test(validators): cover a column called x beside a model noun Co-Authored-By: Claude Opus 5.5 (1M context) --- .../test/altimate/validators/dbt-deliverable-names.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts b/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts index 994758d3d8..899351eb5d 100644 --- a/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts +++ b/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts @@ -307,6 +307,9 @@ describe("DbtDeliverableNamesValidator — code spans that are not models", () = expect( extractRequiredDeliverables("Update the model `orders` to add a column called `status_flag`.\n")!.models, ).toEqual(["orders"]) + expect( + extractRequiredDeliverables("Create the model `orders` and a column called `status`.\n")!.models, + ).toEqual(["orders"]) }) test("a second model after 'have' in a new sentence is still required", async () => { From 08cabb79c48b5db30bab23574aa45184ffb4e0bb Mon Sep 17 00:00:00 2001 From: Anand Gupta Date: Wed, 7 Oct 2026 12:00:25 -0700 Subject: [PATCH 05/10] fix(validators): narrow the span filter further after second review round - drop the "add a column is a modification" marking: three distinct counterexamples (create verbs, variables edited outside the model, mixed create and add lines) showed it can ask for work the task did not require - the kind-word colon form no longer drops a span when the sentence holds a deliverable noun - a semicolon ends the clause for the "have" and trailing-kind rules - only a plural trailing kind ("columns") reaches back over a list - a kept file path does not count as the model a "have" list describes - "the resulting table should have x" counts as describing the model - a negated rename is not a rename Co-Authored-By: Claude Opus 5.5 (1M context) --- .../altimate/validators/validator-utils.ts | 47 ++++++++++--------- .../validators/dbt-deliverable-names.test.ts | 45 +++++++++++++++--- 2 files changed, 64 insertions(+), 28 deletions(-) diff --git a/packages/opencode/src/altimate/validators/validator-utils.ts b/packages/opencode/src/altimate/validators/validator-utils.ts index 9c1a1145f3..65b33b26ca 100644 --- a/packages/opencode/src/altimate/validators/validator-utils.ts +++ b/packages/opencode/src/altimate/validators/validator-utils.ts @@ -610,8 +610,6 @@ const MODIFICATION_VERB_RE = * `requirementHead`'s caller in `extractRequiredDeliverables`. */ const RENAME_VERB_RE = /^renam/i -/** "add" — creation of a column on an existing model when a column span was filtered from the line. */ -const ADD_VERB_RE = /^add/i /** Noun that makes the requirement about a data artifact. */ const DELIVERABLE_NOUN_RE = /\b(?:model|models|table|tables|view|views|seed|seeds|snapshot|snapshots|mart|marts|file|files)\b/i @@ -697,8 +695,7 @@ export function extractRequiredDeliverables(text: string): RequiredDeliverables // forever, so a negated verb disqualifies the whole line. if (verbIsNegated(line, verb.index)) continue const head = requirementHead(line, verb.index) - const candidates = modelCandidateSpans(head, /\brenam(?:e|es|ed|ing)\b/i.test(head)) - let spans = candidates.spans + let spans = modelCandidateSpans(head, hasAffirmativeRename(head)) // "Rename `old_orders` to `new_orders`" names two artifacts, but only the // destination is required to exist once the rename is done — the source // is expected to be GONE. `to` is not a qualifier `requirementHead` cuts @@ -710,13 +707,7 @@ export function extractRequiredDeliverables(text: string): RequiredDeliverables spans = [spans[spans.length - 1]!] } proseTokens.push(...spans) - // "Add a `department` column to the model `m`" asks for a change to `m` - // even though "add" is not a modification verb: the filtered column span is - // the evidence. Without this, `m` merely existing before the session would - // satisfy the nothing-built gate. - if (MODIFICATION_VERB_RE.test(verb[0]) || (candidates.dropped && ADD_VERB_RE.test(verb[0]))) { - modificationTokens.push(...spans) - } + if (MODIFICATION_VERB_RE.test(verb[0])) modificationTokens.push(...spans) } const prose = collectDeliverableTokens(proseTokens) if (prose.models.length > 0 || prose.files.length > 0) { @@ -822,15 +813,22 @@ const FOLLOWED_BY_NON_MODEL_RE = new RegExp( const HAS_LIST_RE = /\b(?:have|has|having|contains?|containing|exposes?|exposing)\s+(?:the\s+)?(?:following\s+)?(?:(?:new|extra|additional)\s+)?$/i /** Subject of a "have" sentence that is the model just described rather than a new requirement. */ -const MODEL_PRONOUN_SUBJECT_RE = /^\s*(?:it|they|this|that|each|the\s+(?:model|table|view|snapshot|seed))\b/i +const MODEL_PRONOUN_SUBJECT_RE = /^\s*(?:it|they|this|that|each|the\s+(?:\w+\s+){0,2}(?:model|table|view|snapshot|seed))\b/i /** Separator between items of a list of spans, allowing a short parenthetical note after an item. */ -const LIST_SEPARATOR_RE = /^\s*(?:\([^)`]*\))?\s*(?:,|;|,?\s*(?:and|or|&))?\s*$/i +const LIST_SEPARATOR_RE = /^\s*(?:\([^)`]*\))?\s*(?:,|,?\s*(?:and|or|&))?\s*$/i /** "rename column `a` to `b`": the target of a column rename is a column. */ const RENAME_TARGET_RE = new RegExp(`^\\s*(?:(?:${NON_MODEL_KIND})\\s+)?(?:to|into|as)\\s*$`, "i") /** A literal path or file name stays eligible for the file check even when it is not a relation. */ const PATH_SHAPED_RE = /[\\/]|\.(?:sql|csv|ya?ml)$/i -/** End of a sentence inside a gap between spans. */ -const SENTENCE_END_RE = /[.!?](?=\s|$)/ +/** End of a sentence or independent clause inside a gap between spans. */ +const SENTENCE_END_RE = /[.!?;](?=\s|$)/ +/** A rename verb that is not negated ("do not rename the model" is not a rename). */ +function hasAffirmativeRename(head: string): boolean { + const re = /\brenam(?:e|es|ed|ing)\b/gi + let m: RegExpExecArray | null + while ((m = re.exec(head)) !== null) if (!verbIsNegated(head, m.index)) return true + return false +} /** * Code spans of a requirement line that can name a model: every span except @@ -846,9 +844,8 @@ const SENTENCE_END_RE = /[.!?](?=\s|$)/ * have ..."), and the target of "to"/"as" is dropped only on a rename line. * Dropping a real model could leave the line with no name at all, which makes * the whole contract read as absent and silences both completion gates. - * `dropped` tells the caller a column-like span was removed from the line. */ -function modelCandidateSpans(line: string, renameLine: boolean): { spans: string[]; dropped: boolean } { +function modelCandidateSpans(line: string, renameLine: boolean): string[] { const out: string[] = [] CODE_SPAN_RE.lastIndex = 0 let m: RegExpExecArray | null @@ -862,13 +859,16 @@ function modelCandidateSpans(line: string, renameLine: boolean): { spans: string PATH_SHAPED_RE.test(matches[i]!.text) || INTRODUCED_AS_MODEL_RE.test(gapBefore(i)) || FOLLOWED_BY_MODEL_NOUN_RE.test(gapAfter(i)) + let keptModels = 0 const nonModel: boolean[] = matches.map(() => false) + // Only a plural kind ("`a` and `b` columns") reaches back over a list; "`x` column" is just `x`. + const pluralKind: boolean[] = matches.map((_, i) => /^\s*\w+s\b/i.test(gapAfter(i)) && FOLLOWED_BY_NON_MODEL_RE.test(gapAfter(i))) // Trailing kind word ("`a` and `b` columns"): applies to the whole list it ends. for (let i = matches.length - 1; i >= 0; i--) { if (isModelLike(i)) continue if (FOLLOWED_BY_NON_MODEL_RE.test(gapAfter(i))) nonModel[i] = true - else if (nonModel[i + 1] && LIST_SEPARATOR_RE.test(gapAfter(i))) nonModel[i] = true + else if (pluralKind[i + 1] && nonModel[i + 1] && LIST_SEPARATOR_RE.test(gapAfter(i))) nonModel[i] = true } for (let i = 0; i < matches.length; i++) { const before = gapBefore(i) @@ -882,17 +882,20 @@ function modelCandidateSpans(line: string, renameLine: boolean): { spans: string nonModel[i] = (called !== null && !DELIVERABLE_NOUN_RE.test(called[1] ?? "")) || (INTRODUCED_BARE_CALLED_RE.test(clause) && !DELIVERABLE_NOUN_RE.test(clause)) || - INTRODUCED_COLON_RE.test(before) || + (INTRODUCED_COLON_RE.test(before) && !DELIVERABLE_NOUN_RE.test(afterBoundary)) || (HAS_LIST_RE.test(before) && - out.length > 0 && + keptModels > 0 && (!sentenceBroke || MODEL_PRONOUN_SUBJECT_RE.test(afterBoundary))) || (i > 0 && nonModel[i - 1] && (LIST_SEPARATOR_RE.test(before) || (renameLine && RENAME_TARGET_RE.test(before)))) } - if (!nonModel[i]) out.push(matches[i]!.text) + if (!nonModel[i]) { + out.push(matches[i]!.text) + if (!PATH_SHAPED_RE.test(matches[i]!.text)) keptModels++ + } } - return { spans: out, dropped: nonModel.some(Boolean) } + return out } /** diff --git a/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts b/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts index 899351eb5d..74614cfbcc 100644 --- a/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts +++ b/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts @@ -324,12 +324,45 @@ describe("DbtDeliverableNamesValidator — code spans that are not models", () = ).toEqual(["accounts"]) }) - test("adding a column to an existing model is a modification of that model", () => { - const r = extractRequiredDeliverables("Add a `department` column to the model `int_workspace_roster`.\n") - expect(r!.models).toEqual(["int_workspace_roster"]) - expect(r!.modificationModels).toEqual(["int_workspace_roster"]) - // A plain creation stays a creation. - expect(extractRequiredDeliverables("Create the model `fct_orders`.\n")!.modificationModels).toEqual([]) + test("a model named after a colon keeps being required", async () => { + expect(await requiredModels("Create a model for storing application settings: `app_settings`.\n")).toEqual([ + "app_settings", + ]) + }) + + test("a new clause after a semicolon is not a description of the previous model", async () => { + expect( + await requiredModels("Create model `stg_orders`; the warehouse should have `fct_orders`.\n"), + ).toEqual(["stg_orders", "fct_orders"]) + expect( + await requiredModels("Build `stg_orders`; `order_id` column must be present in the resulting model.\n"), + ).toEqual(["stg_orders"]) + }) + + test("a file earlier on the line does not make a later 'has `x`' a column", async () => { + await makeProject() + await writeTask("Create the macro `macros/key.sql` and ensure the project has `dim_accounts` as a table.\n") + const r = await DbtDeliverableNamesValidator.check(ctx()) + expect(r.details!["required_models"]).toEqual(["dim_accounts"]) + expect(r.details!["required_files"]).toEqual(["macros/key.sql"]) + }) + + test("'the resulting table should have `x`' describes the model just named", async () => { + expect( + await requiredModels("Create the model `fct_orders`. The resulting table should have `order_id`.\n"), + ).toEqual(["fct_orders"]) + }) + + test("a negated rename does not make the model a rename target", async () => { + expect( + await requiredModels("Add a column `department` to `int_workspace_roster` (do not rename the model).\n"), + ).toEqual(["int_workspace_roster"]) + }) + + test("a singular trailing 'column' does not reach back over other spans", async () => { + expect( + await requiredModels("Build `stg_orders` and `order_id` column must be present in the model.\n"), + ).toEqual(["stg_orders"]) }) test("still fails when the model is missing even though its columns are listed", async () => { From b76eed6aa366fcc96148661bcc9c9303ebe9eef0 Mon Sep 17 00:00:00 2001 From: Anand Gupta Date: Wed, 7 Oct 2026 12:24:01 -0700 Subject: [PATCH 06/10] fix(validators): close remaining fail-open cases in the span filter - a postfix "columns" covers a list of any length - "add column x" after a model clause drops x (colon form judged on its own clause) - rename context is scoped to the sentence before the span - a model file path counts as the model a "have" list describes - "as a table" after a span marks it as a model - a closing quote after the sentence end still ends the sentence Co-Authored-By: Claude Opus 5.5 (1M context) --- .../altimate/validators/validator-utils.ts | 42 +++++++++++++++---- .../validators/dbt-deliverable-names.test.ts | 37 ++++++++++++++++ 2 files changed, 70 insertions(+), 9 deletions(-) diff --git a/packages/opencode/src/altimate/validators/validator-utils.ts b/packages/opencode/src/altimate/validators/validator-utils.ts index 65b33b26ca..d3b92cca01 100644 --- a/packages/opencode/src/altimate/validators/validator-utils.ts +++ b/packages/opencode/src/altimate/validators/validator-utils.ts @@ -695,7 +695,7 @@ export function extractRequiredDeliverables(text: string): RequiredDeliverables // forever, so a negated verb disqualifies the whole line. if (verbIsNegated(line, verb.index)) continue const head = requirementHead(line, verb.index) - let spans = modelCandidateSpans(head, hasAffirmativeRename(head)) + let spans = modelCandidateSpans(head) // "Rename `old_orders` to `new_orders`" names two artifacts, but only the // destination is required to exist once the rename is done — the source // is expected to be GONE. `to` is not a qualifier `requirementHead` cuts @@ -800,7 +800,8 @@ const INTRODUCED_COLON_RE = new RegExp(`\\b(?:${NON_MODEL_KIND})\\b\\s*[:\\-(]?\ const INTRODUCED_AS_MODEL_RE = /\b(?:models?|tables?|views?|seeds?|snapshots?|marts?|files?)\s*(?:(?:called|named)\s*)?:?\s*$/i /** ... and so is a span followed by one ("`x` model"). */ -const FOLLOWED_BY_MODEL_NOUN_RE = /^\s*(?:models?|tables?|views?|seeds?|snapshots?|files?)\b/i +const FOLLOWED_BY_MODEL_NOUN_RE = + /^\s*(?:as\s+(?:an?\s+|the\s+)?)?(?:models?|tables?|views?|seeds?|snapshots?|files?)\b/i /** * "`x` column", "`x` variable": the kind word follows the span and ends the * noun phrase. "`x` column names" is attributive and does not qualify. @@ -820,8 +821,16 @@ const LIST_SEPARATOR_RE = /^\s*(?:\([^)`]*\))?\s*(?:,|,?\s*(?:and|or|&))?\s*$/i const RENAME_TARGET_RE = new RegExp(`^\\s*(?:(?:${NON_MODEL_KIND})\\s+)?(?:to|into|as)\\s*$`, "i") /** A literal path or file name stays eligible for the file check even when it is not a relation. */ const PATH_SHAPED_RE = /[\\/]|\.(?:sql|csv|ya?ml)$/i +/** A `.sql`/`.csv` path outside the macro/analysis directories names a relation. */ +function producesRelation(path: string): boolean { + const normalised = path.replace(/\\/g, "/") + return ( + /\.(?:sql|csv)$/i.test(normalised) && + !NON_RELATION_TOP_SEGMENTS.has(normalised.split("/")[0]?.toLowerCase() ?? "") + ) +} /** End of a sentence or independent clause inside a gap between spans. */ -const SENTENCE_END_RE = /[.!?;](?=\s|$)/ +const SENTENCE_END_RE = /[.!?;]["')\]]*(?=\s|$)/ /** A rename verb that is not negated ("do not rename the model" is not a rename). */ function hasAffirmativeRename(head: string): boolean { const re = /\brenam(?:e|es|ed|ing)\b/gi @@ -845,7 +854,7 @@ function hasAffirmativeRename(head: string): boolean { * Dropping a real model could leave the line with no name at all, which makes * the whole contract read as absent and silences both completion gates. */ -function modelCandidateSpans(line: string, renameLine: boolean): string[] { +function modelCandidateSpans(line: string): string[] { const out: string[] = [] CODE_SPAN_RE.lastIndex = 0 let m: RegExpExecArray | null @@ -865,24 +874,39 @@ function modelCandidateSpans(line: string, renameLine: boolean): string[] { const pluralKind: boolean[] = matches.map((_, i) => /^\s*\w+s\b/i.test(gapAfter(i)) && FOLLOWED_BY_NON_MODEL_RE.test(gapAfter(i))) // Trailing kind word ("`a` and `b` columns"): applies to the whole list it ends. + let pluralList = false for (let i = matches.length - 1; i >= 0; i--) { - if (isModelLike(i)) continue - if (FOLLOWED_BY_NON_MODEL_RE.test(gapAfter(i))) nonModel[i] = true - else if (pluralKind[i + 1] && nonModel[i + 1] && LIST_SEPARATOR_RE.test(gapAfter(i))) nonModel[i] = true + if (isModelLike(i)) { + pluralList = false + continue + } + if (FOLLOWED_BY_NON_MODEL_RE.test(gapAfter(i))) { + nonModel[i] = true + pluralList = pluralKind[i]! + } else if (pluralList && nonModel[i + 1] && LIST_SEPARATOR_RE.test(gapAfter(i))) { + nonModel[i] = true + } else { + pluralList = false + } } for (let i = 0; i < matches.length; i++) { const before = gapBefore(i) const afterBoundary = before.split(SENTENCE_END_RE).pop() ?? "" const sentenceBroke = SENTENCE_END_RE.test(before) + const renameLine = hasAffirmativeRename( + line.slice(0, matches[i]!.start).split(SENTENCE_END_RE).pop() ?? "", + ) if (!isModelLike(i) && !nonModel[i]) { // "column ... called `x`" counts only inside one clause that holds no deliverable noun // before the kind word. const clause = before.split(/[;:!?,]|\.(?!\w)/).pop() ?? "" + // The colon form ("settings: `x`") is judged on the clause up to the colon. + const colonClause = before.split(/[;!?,]|\.(?!\w)/).pop() ?? "" const called = INTRODUCED_CALLED_RE.exec(clause) nonModel[i] = (called !== null && !DELIVERABLE_NOUN_RE.test(called[1] ?? "")) || (INTRODUCED_BARE_CALLED_RE.test(clause) && !DELIVERABLE_NOUN_RE.test(clause)) || - (INTRODUCED_COLON_RE.test(before) && !DELIVERABLE_NOUN_RE.test(afterBoundary)) || + (INTRODUCED_COLON_RE.test(before) && !DELIVERABLE_NOUN_RE.test(colonClause)) || (HAS_LIST_RE.test(before) && keptModels > 0 && (!sentenceBroke || MODEL_PRONOUN_SUBJECT_RE.test(afterBoundary))) || @@ -892,7 +916,7 @@ function modelCandidateSpans(line: string, renameLine: boolean): string[] { } if (!nonModel[i]) { out.push(matches[i]!.text) - if (!PATH_SHAPED_RE.test(matches[i]!.text)) keptModels++ + if (!PATH_SHAPED_RE.test(matches[i]!.text) || producesRelation(matches[i]!.text)) keptModels++ } } return out diff --git a/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts b/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts index 74614cfbcc..992e5186d4 100644 --- a/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts +++ b/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts @@ -365,6 +365,43 @@ describe("DbtDeliverableNamesValidator — code spans that are not models", () = ).toEqual(["stg_orders"]) }) + test("a postfix 'columns' covers a list of three or more", async () => { + expect( + await requiredModels("Add the `first_name`, `middle_name`, and `last_name` columns to the model `customers`.\n"), + ).toEqual(["customers"]) + }) + + test("'add column x' after a model clause drops x", async () => { + expect(await requiredModels("In the model `orders`, add column `foo`.\n")).toEqual(["orders"]) + }) + + test("a rename later in the line does not hide an earlier model", async () => { + expect( + await requiredModels( + "Add the column `foo` to the model `orders` and rename the column `old` to `new` in this model.\n", + ), + ).toEqual(["orders"]) + }) + + test("a model file path counts as the model a have list describes", async () => { + await makeProject() + await writeTask("Create model `models/orders.sql`. It should have `order_id`.\n") + const r = await DbtDeliverableNamesValidator.check(ctx()) + expect(r.details!["required_models"]).toEqual(["orders"]) + }) + + test("'as a table' after a span marks it as a model", async () => { + expect( + await requiredModels("Create model `stg_orders` and ensure the project has `fct_orders` as a table.\n"), + ).toEqual(["stg_orders", "fct_orders"]) + }) + + test("a closing quote after the sentence end still ends the sentence", async () => { + expect( + await requiredModels('"Build the model `stg_orders`." The project should also have `fct_orders`.\n'), + ).toEqual(["stg_orders", "fct_orders"]) + }) + test("still fails when the model is missing even though its columns are listed", async () => { await makeProject() await writeTask("Create a `dim_superhost_evolution` model. It should have `is_currently_superhost`.\n") From af6cdb5bbfc9b5a1435f76bf338294aa8a0cea6a Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Wed, 7 Oct 2026 12:50:56 -0700 Subject: [PATCH 07/10] fix(validators): undo the relation-path count and finish the remaining filter cases - a relation path no longer counts as the model a "have" list describes (it hid a second required model); only bare identifiers count - "includes", "these/those models", "as columns" and "as a model column" - rename context is read per sentence instead of rescanning the prefix Co-Authored-By: Claude Opus 5.5 (1M context) --- .../altimate/validators/validator-utils.ts | 36 +++++++++---------- .../validators/dbt-deliverable-names.test.ts | 29 ++++++++++++--- 2 files changed, 42 insertions(+), 23 deletions(-) diff --git a/packages/opencode/src/altimate/validators/validator-utils.ts b/packages/opencode/src/altimate/validators/validator-utils.ts index d3b92cca01..e8166b9abb 100644 --- a/packages/opencode/src/altimate/validators/validator-utils.ts +++ b/packages/opencode/src/altimate/validators/validator-utils.ts @@ -800,35 +800,31 @@ const INTRODUCED_COLON_RE = new RegExp(`\\b(?:${NON_MODEL_KIND})\\b\\s*[:\\-(]?\ const INTRODUCED_AS_MODEL_RE = /\b(?:models?|tables?|views?|seeds?|snapshots?|marts?|files?)\s*(?:(?:called|named)\s*)?:?\s*$/i /** ... and so is a span followed by one ("`x` model"). */ -const FOLLOWED_BY_MODEL_NOUN_RE = - /^\s*(?:as\s+(?:an?\s+|the\s+)?)?(?:models?|tables?|views?|seeds?|snapshots?|files?)\b/i +const FOLLOWED_BY_MODEL_NOUN_RE = new RegExp( + `^\\s*(?:as\\s+(?:an?\\s+|the\\s+)?)?(?:models?|tables?|views?|seeds?|snapshots?|files?)\\b(?!\\s+(?:${NON_MODEL_KIND})\\b)`, + "i", +) /** * "`x` column", "`x` variable": the kind word follows the span and ends the * noun phrase. "`x` column names" is attributive and does not qualify. */ const FOLLOWED_BY_NON_MODEL_RE = new RegExp( - `^\\s*(?:${NON_MODEL_KIND})\\b(?=\\s*(?:$|[,.;:)]|(?:to|in|of|on|from|that|which|with|for|as|and|or|is|are|should|must|so|called|named|by)\\b))`, + `^\\s*(?:as\\s+(?:an?\\s+|the\\s+)?(?:(?:model|table|view)\\s+)?)?(${NON_MODEL_KIND})\\b(?=\\s*(?:$|[,.;:)]|(?:to|in|of|on|from|that|which|with|for|as|and|or|is|are|should|must|so|called|named|by)\\b))`, "i", ) /** "should have `a`, `b`": what a model has or exposes is its columns. */ const HAS_LIST_RE = - /\b(?:have|has|having|contains?|containing|exposes?|exposing)\s+(?:the\s+)?(?:following\s+)?(?:(?:new|extra|additional)\s+)?$/i + /\b(?:have|has|having|includes?|contains?|containing|exposes?|exposing)\s+(?:the\s+)?(?:following\s+)?(?:(?:new|extra|additional)\s+)?$/i /** Subject of a "have" sentence that is the model just described rather than a new requirement. */ -const MODEL_PRONOUN_SUBJECT_RE = /^\s*(?:it|they|this|that|each|the\s+(?:\w+\s+){0,2}(?:model|table|view|snapshot|seed))\b/i +const MODEL_PRONOUN_SUBJECT_RE = /^\s*(?:it|they|this|that|these|those|each|the\s+(?:\w+\s+){0,2}(?:models?|tables?|views?|snapshots?|seeds?))\b/i /** Separator between items of a list of spans, allowing a short parenthetical note after an item. */ const LIST_SEPARATOR_RE = /^\s*(?:\([^)`]*\))?\s*(?:,|,?\s*(?:and|or|&))?\s*$/i /** "rename column `a` to `b`": the target of a column rename is a column. */ const RENAME_TARGET_RE = new RegExp(`^\\s*(?:(?:${NON_MODEL_KIND})\\s+)?(?:to|into|as)\\s*$`, "i") +/** A bare relation-style name; only these count as the model a "have" list describes. */ +const BARE_IDENTIFIER_RE = /^[A-Za-z_][A-Za-z0-9_]*$/ /** A literal path or file name stays eligible for the file check even when it is not a relation. */ const PATH_SHAPED_RE = /[\\/]|\.(?:sql|csv|ya?ml)$/i -/** A `.sql`/`.csv` path outside the macro/analysis directories names a relation. */ -function producesRelation(path: string): boolean { - const normalised = path.replace(/\\/g, "/") - return ( - /\.(?:sql|csv)$/i.test(normalised) && - !NON_RELATION_TOP_SEGMENTS.has(normalised.split("/")[0]?.toLowerCase() ?? "") - ) -} /** End of a sentence or independent clause inside a gap between spans. */ const SENTENCE_END_RE = /[.!?;]["')\]]*(?=\s|$)/ /** A rename verb that is not negated ("do not rename the model" is not a rename). */ @@ -869,9 +865,14 @@ function modelCandidateSpans(line: string): string[] { INTRODUCED_AS_MODEL_RE.test(gapBefore(i)) || FOLLOWED_BY_MODEL_NOUN_RE.test(gapAfter(i)) let keptModels = 0 + // Rename context is read once per sentence, not by rescanning the growing prefix for every span. + const hasRenameWord = /\brenam/i.test(line) + let sentenceStart = 0 const nonModel: boolean[] = matches.map(() => false) // Only a plural kind ("`a` and `b` columns") reaches back over a list; "`x` column" is just `x`. - const pluralKind: boolean[] = matches.map((_, i) => /^\s*\w+s\b/i.test(gapAfter(i)) && FOLLOWED_BY_NON_MODEL_RE.test(gapAfter(i))) + const pluralKind: boolean[] = matches.map((_, i) => + /s$/i.test(FOLLOWED_BY_NON_MODEL_RE.exec(gapAfter(i))?.[1] ?? ""), + ) // Trailing kind word ("`a` and `b` columns"): applies to the whole list it ends. let pluralList = false @@ -893,9 +894,8 @@ function modelCandidateSpans(line: string): string[] { const before = gapBefore(i) const afterBoundary = before.split(SENTENCE_END_RE).pop() ?? "" const sentenceBroke = SENTENCE_END_RE.test(before) - const renameLine = hasAffirmativeRename( - line.slice(0, matches[i]!.start).split(SENTENCE_END_RE).pop() ?? "", - ) + if (sentenceBroke) sentenceStart = matches[i]!.start - (afterBoundary.length) + const renameLine = hasRenameWord && hasAffirmativeRename(line.slice(sentenceStart, matches[i]!.start)) if (!isModelLike(i) && !nonModel[i]) { // "column ... called `x`" counts only inside one clause that holds no deliverable noun // before the kind word. @@ -916,7 +916,7 @@ function modelCandidateSpans(line: string): string[] { } if (!nonModel[i]) { out.push(matches[i]!.text) - if (!PATH_SHAPED_RE.test(matches[i]!.text) || producesRelation(matches[i]!.text)) keptModels++ + if (BARE_IDENTIFIER_RE.test(matches[i]!.text)) keptModels++ } } return out diff --git a/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts b/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts index 992e5186d4..6ca64ac578 100644 --- a/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts +++ b/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts @@ -383,11 +383,30 @@ describe("DbtDeliverableNamesValidator — code spans that are not models", () = ).toEqual(["orders"]) }) - test("a model file path counts as the model a have list describes", async () => { - await makeProject() - await writeTask("Create model `models/orders.sql`. It should have `order_id`.\n") - const r = await DbtDeliverableNamesValidator.check(ctx()) - expect(r.details!["required_models"]).toEqual(["orders"]) + test("a relation path does not hide a second model named after 'has'", async () => { + expect( + await requiredModels("Create model `models/stg_orders.sql` and ensure the project has `fct_orders`.\n"), + ).toEqual(["stg_orders", "fct_orders"]) + }) + + test("a non-identifier span does not count as the model a have list describes", async () => { + expect( + await requiredModels("Create a model that computes `COUNT(*)` and ensure the project has `fct_orders`.\n"), + ).toEqual(["fct_orders"]) + }) + + test("'as columns' and 'as a model column' mark columns", async () => { + expect( + await requiredModels("Add `order_id` and `customer_id` as columns to the model `orders`.\n"), + ).toEqual(["orders"]) + expect(await requiredModels("Add `order_id` as a model column to `fct_orders`.\n")).toEqual(["fct_orders"]) + }) + + test("'include' and plural subjects describe the model just named", async () => { + expect(await requiredModels("Create a `dim_x` model. It should include `col_a` and `col_b`.\n")).toEqual(["dim_x"]) + expect( + await requiredModels("Create models `orders` and `customers`. These models should have `created_at`.\n"), + ).toEqual(["orders", "customers"]) }) test("'as a table' after a span marks it as a model", async () => { From ad6c9e29ba6f5ad3b35746fe958d275af1a3f17c Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Wed, 7 Oct 2026 13:03:04 -0700 Subject: [PATCH 08/10] fix(validators): keep plainly named models, drop spans the text calls columns - "This project should have X", "the project includes/contains X" and stopword spans such as `date` no longer make a plainly named model disappear: the have/include rule needs the model itself as subject, and only spans that collection would keep count as the model being described - "as a new/additional/extra column", "model column" and "table column" mark columns - state the preference for ambiguous wording in the code and add a test that the required set is unchanged from origin/main over plainly worded requests Co-Authored-By: Claude Opus 5.5 (1M context) --- .../altimate/validators/validator-utils.ts | 45 ++++++++++++--- .../validators/dbt-deliverable-names.test.ts | 55 +++++++++++++++++++ 2 files changed, 91 insertions(+), 9 deletions(-) diff --git a/packages/opencode/src/altimate/validators/validator-utils.ts b/packages/opencode/src/altimate/validators/validator-utils.ts index e8166b9abb..ef16e2558a 100644 --- a/packages/opencode/src/altimate/validators/validator-utils.ts +++ b/packages/opencode/src/altimate/validators/validator-utils.ts @@ -809,22 +809,40 @@ const FOLLOWED_BY_MODEL_NOUN_RE = new RegExp( * noun phrase. "`x` column names" is attributive and does not qualify. */ const FOLLOWED_BY_NON_MODEL_RE = new RegExp( - `^\\s*(?:as\\s+(?:an?\\s+|the\\s+)?(?:(?:model|table|view)\\s+)?)?(${NON_MODEL_KIND})\\b(?=\\s*(?:$|[,.;:)]|(?:to|in|of|on|from|that|which|with|for|as|and|or|is|are|should|must|so|called|named|by)\\b))`, + `^\\s*(?:as\\s+(?:an?\\s+|the\\s+)?(?:(?:new|extra|additional|separate)\\s+)?(?:(?:model|table|view)\\s+)?)?(${NON_MODEL_KIND})\\b(?=\\s*(?:$|[,.;:)]|(?:to|in|of|on|from|that|which|with|for|as|and|or|is|are|should|must|so|called|named|by)\\b))`, "i", ) /** "should have `a`, `b`": what a model has or exposes is its columns. */ const HAS_LIST_RE = /\b(?:have|has|having|includes?|contains?|containing|exposes?|exposing)\s+(?:the\s+)?(?:following\s+)?(?:(?:new|extra|additional)\s+)?$/i -/** Subject of a "have" sentence that is the model just described rather than a new requirement. */ -const MODEL_PRONOUN_SUBJECT_RE = /^\s*(?:it|they|this|that|these|those|each|the\s+(?:\w+\s+){0,2}(?:models?|tables?|views?|snapshots?|seeds?))\b/i +/** + * The words just before a "have/include/contain" verb must name the model being + * described ("It should", "that has", "the resulting table should", "These models + * must"). "This project should have `x`" or "ensure the project includes `x`" names + * a different subject, so `x` stays a requirement. + */ +const MODEL_SUBJECT_BEFORE_VERB_RE = + /(?:\b(?:it|they|that|which|these|those|this|each)|\b(?:models?|tables?|views?|snapshots?|seeds?))\s+(?:(?:also|should|must|will|can|then)\s+)*$/i /** Separator between items of a list of spans, allowing a short parenthetical note after an item. */ const LIST_SEPARATOR_RE = /^\s*(?:\([^)`]*\))?\s*(?:,|,?\s*(?:and|or|&))?\s*$/i /** "rename column `a` to `b`": the target of a column rename is a column. */ const RENAME_TARGET_RE = new RegExp(`^\\s*(?:(?:${NON_MODEL_KIND})\\s+)?(?:to|into|as)\\s*$`, "i") /** A bare relation-style name; only these count as the model a "have" list describes. */ const BARE_IDENTIFIER_RE = /^[A-Za-z_][A-Za-z0-9_]*$/ +/** A span that collection will keep as a model name (not a stopword such as `date`). */ +function countsAsModel(text: string): boolean { + const name = text.toLowerCase() + return BARE_IDENTIFIER_RE.test(text) && IDENTIFIER_RE.test(name) && !DELIVERABLE_STOPWORDS.has(name) +} +/** Compound noun ("model column"): the model noun belongs to the kind word and introduces no model. */ +const COMPOUND_KIND_RE = new RegExp(`\\b(?:models?|tables?|views?)\\s+(?=(?:${NON_MODEL_KIND})\\b)`, "gi") /** A literal path or file name stays eligible for the file check even when it is not a relation. */ const PATH_SHAPED_RE = /[\\/]|\.(?:sql|csv|ya?ml)$/i +/** True when `before` ends in a have/include/contain verb whose subject is the model just named. */ +function hasModelDescriptionVerb(before: string): boolean { + const verb = HAS_LIST_RE.exec(before) + return verb !== null && MODEL_SUBJECT_BEFORE_VERB_RE.test(before.slice(0, verb.index)) +} /** End of a sentence or independent clause inside a gap between spans. */ const SENTENCE_END_RE = /[.!?;]["')\]]*(?=\s|$)/ /** A rename verb that is not negated ("do not rename the model" is not a rename). */ @@ -842,6 +860,17 @@ function hasAffirmativeRename(head: string): boolean { * "`x` column", "should have `a`, `b`", a list continuing such a span); with no * such wording it stays, so a bare name is still required. * + * Preference when the wording is ambiguous: the costlier error for a finish-time + * validator is the false alarm (a retry turn spent on a requirement nobody made), + * so a span that the text marks as a column, field, variable or macro by any + * explicit cue ("column", "as a new column", "model column", "should have" said of + * the model just named) is not required. A span with NO such cue is not ambiguous, + * it is named like a deliverable ("create model X", "the project should have X"), + * and is required exactly as before this filter existed: losing it would empty the + * contract and silence both completion gates. This is pattern matching on English + * phrasing, not understanding; wording it does not recognise falls on the "required" + * side because that is what the validator did before. + * * Dropping errs toward keeping: a span introduced or followed by a deliverable * noun is never dropped, a path-shaped span is never dropped (the file check * still needs it), "have `x`" drops only when an earlier span of the sentence @@ -899,24 +928,22 @@ function modelCandidateSpans(line: string): string[] { if (!isModelLike(i) && !nonModel[i]) { // "column ... called `x`" counts only inside one clause that holds no deliverable noun // before the kind word. - const clause = before.split(/[;:!?,]|\.(?!\w)/).pop() ?? "" + const clause = (before.split(/[;:!?,]|\.(?!\w)/).pop() ?? "").replace(COMPOUND_KIND_RE, "") // The colon form ("settings: `x`") is judged on the clause up to the colon. - const colonClause = before.split(/[;!?,]|\.(?!\w)/).pop() ?? "" + const colonClause = (before.split(/[;!?,]|\.(?!\w)/).pop() ?? "").replace(COMPOUND_KIND_RE, "") const called = INTRODUCED_CALLED_RE.exec(clause) nonModel[i] = (called !== null && !DELIVERABLE_NOUN_RE.test(called[1] ?? "")) || (INTRODUCED_BARE_CALLED_RE.test(clause) && !DELIVERABLE_NOUN_RE.test(clause)) || (INTRODUCED_COLON_RE.test(before) && !DELIVERABLE_NOUN_RE.test(colonClause)) || - (HAS_LIST_RE.test(before) && - keptModels > 0 && - (!sentenceBroke || MODEL_PRONOUN_SUBJECT_RE.test(afterBoundary))) || + (keptModels > 0 && hasModelDescriptionVerb(before)) || (i > 0 && nonModel[i - 1] && (LIST_SEPARATOR_RE.test(before) || (renameLine && RENAME_TARGET_RE.test(before)))) } if (!nonModel[i]) { out.push(matches[i]!.text) - if (BARE_IDENTIFIER_RE.test(matches[i]!.text)) keptModels++ + if (countsAsModel(matches[i]!.text)) keptModels++ } } return out diff --git a/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts b/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts index 6ca64ac578..702dc68a44 100644 --- a/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts +++ b/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts @@ -421,6 +421,61 @@ describe("DbtDeliverableNamesValidator — code spans that are not models", () = ).toEqual(["stg_orders", "fct_orders"]) }) + // Direction of the rule: when the wording leaves a span's role open, it is NOT + // dropped (a model that is plainly named stays required); a span is dropped only + // when the text calls it a column, field, variable or macro. The cost of a wrong + // drop is a silenced gate; the cost of a wrong keep is one retry turn, and the + // sibling cases below pin both sides. + test.each([ + ["Create model `stg_orders`. This project should have `fct_orders`.", ["stg_orders", "fct_orders"]], + ["Create model `stg_orders`. That warehouse should have `fct_orders`.", ["stg_orders", "fct_orders"]], + ["Create a model that computes `date` and ensure the project has `fct_orders`.", ["fct_orders"]], + ["Create a model that uses `count` and ensure the project has `fct_orders`.", ["count", "fct_orders"]], + ["Create model `stg_orders` and ensure the project includes `fct_orders`.", ["stg_orders", "fct_orders"]], + ["Create model `stg_orders` and ensure the project contains `fct_orders`.", ["stg_orders", "fct_orders"]], + ["Create model `stg_orders` and ensure the project has `fct_orders` with `id` as key.", ["stg_orders", "fct_orders"]], + ])("a plainly named model is not lost: %s", async (task, models) => { + expect(await requiredModels(task + "\n")).toEqual(models) + }) + + test.each([ + ["Add `status_flag` as a new column to the model `orders`.", ["orders"]], + ["Add `status_flag` as an additional column to the model `orders`.", ["orders"]], + ["Add `status_flag` as an extra field to the model `orders`.", ["orders"]], + ["Add a model column `status_flag` to `orders`.", ["orders"]], + ["Add a table column `status_flag` to the model `orders`.", ["orders"]], + ])("a span the text calls a column is not a model: %s", async (task, models) => { + expect(await requiredModels(task + "\n")).toEqual(models) + }) + + // For plainly worded requests the required set must equal what origin/main + // returned before the span filter existed; the filter only differs where a span + // is explicitly called a column, variable or macro. + test.each([ + ["Create the model `fct_orders`.", ["fct_orders"]], + ["Create the models `stg_orders` and `stg_customers`.", ["stg_orders", "stg_customers"]], + ["Create model `stg_orders`. This project should have `fct_orders`.", ["stg_orders", "fct_orders"]], + ["Create a model that computes `date` and ensure the project has `fct_orders`.", ["fct_orders"]], + ["Create model `stg_orders` and ensure the project includes `fct_orders`.", ["stg_orders", "fct_orders"]], + ["Create model `stg_orders` and ensure the project contains `fct_orders`.", ["stg_orders", "fct_orders"]], + ["Create model `stg_orders`. That warehouse should have `fct_orders`.", ["stg_orders", "fct_orders"]], + ["Create model `stg_orders`. Those teams should include `fct_orders`.", ["stg_orders", "fct_orders"]], + ["Create model `stg_orders` and ensure the project has `fct_orders` with `id` as key.", ["stg_orders", "fct_orders"]], + ["Rename the model `old_orders` to `new_orders`.", ["new_orders"]], + ["Update the model `orders` so it excludes cancelled rows.", ["orders"]], + ["Create the model `dim_customer` and the table `dim_product`.", ["dim_customer", "dim_product"]], + ["Build `stg_a` and `stg_b` as views.", ["stg_a", "stg_b"]], + ["Add the seed `country_codes` and the snapshot `snap_orders`.", ["country_codes", "snap_orders"]], + ["Do not create the model `legacy_orders`; create `fct_orders` instead.", null], + ["Create `models/marts/fct_orders.sql` and the model `dim_dates`.", ["fct_orders", "dim_dates"]], + ["Create a model that uses `count` and ensure the project has `fct_orders`.", ["count", "fct_orders"]], + ["Fix the model `stg_orders`; the project should also have `fct_payments` as a table.", ["stg_orders", "fct_payments"]], + ["Create the model `a_model`. The mart must include `fct_sales`.", ["a_model", "fct_sales"]], + ] as [string, string[] | null][])("unchanged from origin/main: %s", async (task, expected) => { + const r = extractRequiredDeliverables(task + "\n") + expect(r === null ? null : r.models).toEqual(expected) + }) + test("still fails when the model is missing even though its columns are listed", async () => { await makeProject() await writeTask("Create a `dim_superhost_evolution` model. It should have `is_currently_superhost`.\n") From 8fde7082cda9f9558c07fe9072229188bf5f2c59 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Wed, 7 Oct 2026 13:25:58 -0700 Subject: [PATCH 09/10] fix(validators): plural model nouns are not subjects, a kept span can be - "The required models should include X" keeps X required - "`orders` should have `order_id`" drops order_id Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/altimate/validators/validator-utils.ts | 12 ++++++++---- .../validators/dbt-deliverable-names.test.ts | 5 +++++ 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/opencode/src/altimate/validators/validator-utils.ts b/packages/opencode/src/altimate/validators/validator-utils.ts index ef16e2558a..91d6c723e6 100644 --- a/packages/opencode/src/altimate/validators/validator-utils.ts +++ b/packages/opencode/src/altimate/validators/validator-utils.ts @@ -822,7 +822,7 @@ const HAS_LIST_RE = * a different subject, so `x` stays a requirement. */ const MODEL_SUBJECT_BEFORE_VERB_RE = - /(?:\b(?:it|they|that|which|these|those|this|each)|\b(?:models?|tables?|views?|snapshots?|seeds?))\s+(?:(?:also|should|must|will|can|then)\s+)*$/i + /(?:\b(?:it|they|that|which|these|those|this|each)|\b(?:these|those)\s+(?:models|tables|views|snapshots|seeds)|\b(?:model|table|view|snapshot|seed))\s+(?:(?:also|should|must|will|can|then)\s+)*$/i /** Separator between items of a list of spans, allowing a short parenthetical note after an item. */ const LIST_SEPARATOR_RE = /^\s*(?:\([^)`]*\))?\s*(?:,|,?\s*(?:and|or|&))?\s*$/i /** "rename column `a` to `b`": the target of a column rename is a column. */ @@ -839,9 +839,13 @@ const COMPOUND_KIND_RE = new RegExp(`\\b(?:models?|tables?|views?)\\s+(?=(?:${NO /** A literal path or file name stays eligible for the file check even when it is not a relation. */ const PATH_SHAPED_RE = /[\\/]|\.(?:sql|csv|ya?ml)$/i /** True when `before` ends in a have/include/contain verb whose subject is the model just named. */ -function hasModelDescriptionVerb(before: string): boolean { +function hasModelDescriptionVerb(before: string, previousSpanIsModel: boolean): boolean { const verb = HAS_LIST_RE.exec(before) - return verb !== null && MODEL_SUBJECT_BEFORE_VERB_RE.test(before.slice(0, verb.index)) + if (verb === null) return false + const subject = before.slice(0, verb.index) + // "`orders` should have `order_id`": the subject is the kept span just before the verb. + if (previousSpanIsModel && /^\s*(?:(?:also|should|must|will|can|then)\s+)*$/i.test(subject)) return true + return MODEL_SUBJECT_BEFORE_VERB_RE.test(subject) } /** End of a sentence or independent clause inside a gap between spans. */ const SENTENCE_END_RE = /[.!?;]["')\]]*(?=\s|$)/ @@ -936,7 +940,7 @@ function modelCandidateSpans(line: string): string[] { (called !== null && !DELIVERABLE_NOUN_RE.test(called[1] ?? "")) || (INTRODUCED_BARE_CALLED_RE.test(clause) && !DELIVERABLE_NOUN_RE.test(clause)) || (INTRODUCED_COLON_RE.test(before) && !DELIVERABLE_NOUN_RE.test(colonClause)) || - (keptModels > 0 && hasModelDescriptionVerb(before)) || + (keptModels > 0 && hasModelDescriptionVerb(before, i > 0 && !nonModel[i - 1])) || (i > 0 && nonModel[i - 1] && (LIST_SEPARATOR_RE.test(before) || (renameLine && RENAME_TARGET_RE.test(before)))) diff --git a/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts b/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts index 702dc68a44..aa2698057c 100644 --- a/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts +++ b/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts @@ -434,10 +434,15 @@ describe("DbtDeliverableNamesValidator — code spans that are not models", () = ["Create model `stg_orders` and ensure the project includes `fct_orders`.", ["stg_orders", "fct_orders"]], ["Create model `stg_orders` and ensure the project contains `fct_orders`.", ["stg_orders", "fct_orders"]], ["Create model `stg_orders` and ensure the project has `fct_orders` with `id` as key.", ["stg_orders", "fct_orders"]], + ["Create model `stg_orders`. The required models should include `fct_orders`.", ["stg_orders", "fct_orders"]], ])("a plainly named model is not lost: %s", async (task, models) => { expect(await requiredModels(task + "\n")).toEqual(models) }) + test("a kept span that is the subject of 'should have' describes itself", async () => { + expect(await requiredModels("Build the model `orders`. `orders` should have `order_id`.\n")).toEqual(["orders"]) + }) + test.each([ ["Add `status_flag` as a new column to the model `orders`.", ["orders"]], ["Add `status_flag` as an additional column to the model `orders`.", ["orders"]], From 086cefa8f91ccc195c316c7f78cb094517f93b2e Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Wed, 7 Oct 2026 13:48:15 -0700 Subject: [PATCH 10/10] fix(validators): stopword subjects, "both models", model lists with a trailing noun, clause starts Co-Authored-By: Claude Opus 5.5 (1M context) --- .../altimate/validators/validator-utils.ts | 17 +++++++++---- .../validators/dbt-deliverable-names.test.ts | 24 +++++++++++++++++++ 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/packages/opencode/src/altimate/validators/validator-utils.ts b/packages/opencode/src/altimate/validators/validator-utils.ts index 91d6c723e6..2c667a2a4b 100644 --- a/packages/opencode/src/altimate/validators/validator-utils.ts +++ b/packages/opencode/src/altimate/validators/validator-utils.ts @@ -822,7 +822,7 @@ const HAS_LIST_RE = * a different subject, so `x` stays a requirement. */ const MODEL_SUBJECT_BEFORE_VERB_RE = - /(?:\b(?:it|they|that|which|these|those|this|each)|\b(?:these|those)\s+(?:models|tables|views|snapshots|seeds)|\b(?:model|table|view|snapshot|seed))\s+(?:(?:also|should|must|will|can|then)\s+)*$/i + /(?:\b(?:it|they|that|which|these|those|this|each)|\b(?:these|those|both)\s+(?:models|tables|views|snapshots|seeds)|\b(?:model|table|view|snapshot|seed))\s+(?:(?:also|should|must|will|can|then)\s+)*$/i /** Separator between items of a list of spans, allowing a short parenthetical note after an item. */ const LIST_SEPARATOR_RE = /^\s*(?:\([^)`]*\))?\s*(?:,|,?\s*(?:and|or|&))?\s*$/i /** "rename column `a` to `b`": the target of a column rename is a column. */ @@ -836,6 +836,8 @@ function countsAsModel(text: string): boolean { } /** Compound noun ("model column"): the model noun belongs to the kind word and introduces no model. */ const COMPOUND_KIND_RE = new RegExp(`\\b(?:models?|tables?|views?)\\s+(?=(?:${NON_MODEL_KIND})\\b)`, "gi") +/** A span followed by "should have" etc. starts its own clause; it is not another item of the previous list. */ +const CLAUSE_VERB_AFTER_RE = /^\s*(?:should|must|will|needs?|has|have)\b/i /** A literal path or file name stays eligible for the file check even when it is not a relation. */ const PATH_SHAPED_RE = /[\\/]|\.(?:sql|csv|ya?ml)$/i /** True when `before` ends in a have/include/contain verb whose subject is the model just named. */ @@ -893,10 +895,14 @@ function modelCandidateSpans(line: string): string[] { } const gapBefore = (i: number) => line.slice(i === 0 ? 0 : matches[i - 1]!.end, matches[i]!.start) const gapAfter = (i: number) => line.slice(matches[i]!.end, matches[i + 1]?.start ?? line.length) + // "`dim_a` and `dim_b` models": a trailing model noun covers the whole list before it. + const modelList: boolean[] = matches.map(() => false) + for (let i = matches.length - 1; i >= 0; i--) { + modelList[i] = + FOLLOWED_BY_MODEL_NOUN_RE.test(gapAfter(i)) || (!!modelList[i + 1] && LIST_SEPARATOR_RE.test(gapAfter(i))) + } const isModelLike = (i: number) => - PATH_SHAPED_RE.test(matches[i]!.text) || - INTRODUCED_AS_MODEL_RE.test(gapBefore(i)) || - FOLLOWED_BY_MODEL_NOUN_RE.test(gapAfter(i)) + PATH_SHAPED_RE.test(matches[i]!.text) || INTRODUCED_AS_MODEL_RE.test(gapBefore(i)) || modelList[i]! let keptModels = 0 // Rename context is read once per sentence, not by rescanning the growing prefix for every span. const hasRenameWord = /\brenam/i.test(line) @@ -940,9 +946,10 @@ function modelCandidateSpans(line: string): string[] { (called !== null && !DELIVERABLE_NOUN_RE.test(called[1] ?? "")) || (INTRODUCED_BARE_CALLED_RE.test(clause) && !DELIVERABLE_NOUN_RE.test(clause)) || (INTRODUCED_COLON_RE.test(before) && !DELIVERABLE_NOUN_RE.test(colonClause)) || - (keptModels > 0 && hasModelDescriptionVerb(before, i > 0 && !nonModel[i - 1])) || + (keptModels > 0 && hasModelDescriptionVerb(before, i > 0 && !nonModel[i - 1] && countsAsModel(matches[i - 1]!.text))) || (i > 0 && nonModel[i - 1] && + !CLAUSE_VERB_AFTER_RE.test(gapAfter(i)) && (LIST_SEPARATOR_RE.test(before) || (renameLine && RENAME_TARGET_RE.test(before)))) } if (!nonModel[i]) { diff --git a/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts b/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts index aa2698057c..66f55057fe 100644 --- a/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts +++ b/packages/opencode/test/altimate/validators/dbt-deliverable-names.test.ts @@ -443,6 +443,30 @@ describe("DbtDeliverableNamesValidator — code spans that are not models", () = expect(await requiredModels("Build the model `orders`. `orders` should have `order_id`.\n")).toEqual(["orders"]) }) + test("a new clause is not another item of the previous column list", async () => { + expect( + await requiredModels("Build the models `orders` and `dim_dates`. `orders` should have `order_id`, and `dim_dates` should have `d`.\n"), + ).toEqual(["orders", "dim_dates"]) + }) + + test("a stopword span is not the subject of 'should have'", async () => { + expect(await requiredModels("Build the model `stg_orders`. The `project` should have `fct_orders`.\n")).toEqual([ + "stg_orders", + "fct_orders", + ]) + }) + + test("'both models' describes the models just named, and a trailing 'models' covers a list", async () => { + expect( + await requiredModels("Create models `orders` and `customers`. Both models should have `created_at`.\n"), + ).toEqual(["orders", "customers"]) + expect(await requiredModels("Create model `bundle`. It should contain `dim_a` and `dim_b` models.\n")).toEqual([ + "bundle", + "dim_a", + "dim_b", + ]) + }) + test.each([ ["Add `status_flag` as a new column to the model `orders`.", ["orders"]], ["Add `status_flag` as an additional column to the model `orders`.", ["orders"]],