Repository navigation
fix(validators): stop reading backticked columns and variables as required models - #1422
anandgupta42 wants to merge 10 commits into
Conversation
…uired 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) <noreply@anthropic.com>
…de 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) <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 1395534d-08a4-4416-9167-b66e99a99d87) |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughRequirement extraction now filters inline-code spans to identify model names and exclude columns, variables, and other non-model entities. Tests cover varied wording, clause boundaries, file paths, and missing-model results. ChangesDeliverable Name Filtering
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The change narrows which backticked names count as required models, and no merge-blocking issue was found. It is ready to merge after normal checks. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit checks each backticked name, Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 679021b42e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: 5 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous Review Summaries (7 snapshots, latest commit 8fde708)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 8fde708)Status: 7 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit ad6c9e2)Status: 6 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit af6cdb5)Status: 9 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit b76eed6)Status: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit 08cabb7)Status: 7 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit fbdea40)Status: 8 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit 679021b)Status: 7 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (2 files)
Reviewed by gpt-6-sol · Input: 18 · Output: 11.9K · Cached: 916K Review guidance: REVIEW.md from base branch |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
- 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) <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 22e19300-23fe-477b-8d1e-90097697d61f) |
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 77cf6567-2d9a-483a-982e-45ba8d961679) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fbdea405e9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
…ound
- 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) <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 442c725c-e331-4f3f-8b9e-5517d9e13870) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08cabb79c4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- 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) <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 9f1dc2aa-16a8-42ec-8478-fb5a1c0c87b5) |
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b76eed6aa3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…g 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) <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: ead244a4-1c85-4fae-825c-00072c7098a5) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af6cdb5bbf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… 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) <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 915e08af-0906-4dfa-9bf9-5f15323eec70) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad6c9e29ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- "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) <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: c7f6a424-9732-43a7-92e8-fb4543641405) |
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8fde7082cd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… trailing noun, clause starts Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: ce575917-3887-4768-9a3b-cf589f6a3a2d) |
| (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)) && |
There was a problem hiding this comment.
WARNING: Keep column-list continuations with their own predicates
For Create the model orders. It should have order_id, and status_flag should be nullable., order_id is correctly dropped as a column, but the new guard sees should after status_flag and prevents the list-continuation rule from dropping it. status_flag is then required as a model, so an otherwise correct orders implementation fails the deliverable-name gate. Distinguish a new model-subject clause from a column-list item whose value simply has another property.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
The counterexample is real and is not fixed in this PR. Classifying a backticked name as a model, column or variable is done here by matching English phrasing, and several rounds of review have each produced further phrasings that the matcher gets wrong; adding one more rule per phrasing is not converging. The PR body's "Known limits" paragraph states this. Leaving this thread open for the human reviewer to weigh: the choice is between accepting the documented limit and replacing the phrase matcher with a different classifier in a follow-up.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 086cefa8f9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /\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 = new RegExp( | ||
| `^\\s*(?:as\\s+(?:an?\\s+|the\\s+)?)?(?:models?|tables?|views?|seeds?|snapshots?|files?)\\b(?!\\s+(?:${NON_MODEL_KIND})\\b)`, |
There was a problem hiding this comment.
Fresh evidence after the prior trailing-noun fix is Create model bundle. It should contain dim_aanddim_b marts.: mart is accepted by the general deliverable and leading-model matchers but is absent from this postfix matcher, so the contain heuristic discards both explicitly requested marts and the contract requires only bundle. A submission omitting dim_a and dim_b can therefore pass.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The counterexample is real and is not fixed in this PR. Classifying a backticked name as a model, column or variable is done here by matching English phrasing, and several rounds of review have each produced further phrasings that the matcher gets wrong; adding one more rule per phrasing is not converging. The PR body's "Known limits" paragraph states this. Leaving this thread open for the human reviewer to weigh: the choice is between accepting the documented limit and replacing the phrase matcher with a different classifier in a follow-up.
| const HAS_LIST_RE = | ||
| /\b(?:have|has|having|includes?|contains?|containing|exposes?|exposing)\s+(?:the\s+)?(?:following\s+)?(?:(?:new|extra|additional)\s+)?$/i |
There was a problem hiding this comment.
When a requirement explicitly says Create model orders. Its columns are order_idandcustomer_id., this matcher does not recognize the columns are construction, and neither span has a kind immediately adjacent to it. Both column names therefore survive as required models, causing the validator to reject a correct orders implementation for missing two nonexistent relations.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The counterexample is real and is not fixed in this PR. Classifying a backticked name as a model, column or variable is done here by matching English phrasing, and several rounds of review have each produced further phrasings that the matcher gets wrong; adding one more rule per phrasing is not converging. The PR body's "Known limits" paragraph states this. Leaving this thread open for the human reviewer to weigh: the choice is between accepting the documented limit and replacing the phrase matcher with a different classifier in a follow-up.
| (i > 0 && | ||
| nonModel[i - 1] && | ||
| !CLAUSE_VERB_AFTER_RE.test(gapAfter(i)) && | ||
| (LIST_SEPARATOR_RE.test(before) || (renameLine && RENAME_TARGET_RE.test(before)))) |
There was a problem hiding this comment.
Fresh evidence beyond the handled adjacent source-to-target form is Rename the old_statuscolumn in the modelorderstonew_status.: because orders sits between the non-model source and its target, this propagation does not mark new_status as a column. The candidate list becomes orders, new_status, after which the outer rename handling keeps only the final span, so the validator requires a model named new_status and discards the actual orders requirement.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The counterexample is real and is not fixed in this PR. Classifying a backticked name as a model, column or variable is done here by matching English phrasing, and several rounds of review have each produced further phrasings that the matcher gets wrong; adding one more rule per phrasing is not converging. The PR body's "Known limits" paragraph states this. Leaving this thread open for the human reviewer to weigh: the choice is between accepting the documented limit and replacing the phrase matcher with a different classifier in a follow-up.
This branch has not been deployed
1 participant
Issue for this PR
Closes #1421
Type of change
What does this PR do?
Why. I was asked whether a deterministic finish-time check that "the models and columns the request names explicitly exist" is worth adding to the validator lane. The offline evidence says no new blocking check is worth adding (numbers below), so this PR is the smaller deliverable: fix the known bug in
dbt-deliverable-namesthat the investigation touched.Root cause.
extractRequiredDeliverablesaccepts every inline code span on a line that has a requirement verb and a data-artifact noun as a required model. A column or a dbt variable written in backticks on such a line was therefore required as a model, and the validator reported it as a missing deliverable. Run on the 73 benchmark requests that the earlier replay used, the unfixed extractor requires, besides real models:is_currently_superhost,acct_age_before_achieving_superhost,status_change_count,last_status_change_at(a column list),answer("one column calledanswer") andsurrogate_key_treat_nulls_as_empty_strings(a dbt variable).What changed. A new
modelCandidateSpansinvalidator-utils.tsdrops a span only on explicit wording that the line itself uses for something that is not a model:x" or "column:x" (one clause, no deliverable noun in it);xcolumn" / "xvariable" ending the noun phrase ("xcolumn names" does not count);a,b" and the list that continues it, only after an earlier span of the line was kept;aandbcolumns").It errs toward keeping the name; path-shaped spans (
macros/x.sql) are never dropped. A span introduced or followed by a deliverable noun ("the modelx", "xtable") is never dropped, because dropping the only model name on a line would make the contract read as absent and silence bothdbt-deliverable-namesanddbt-nothing-built. The independent review found exactly that regression in my first version; the second commit fixes it, and the third and fourth address two rounds of automated review (file paths, trailing kinds, sentence and clause boundaries, rename context).Before and after, same input,
TASK.md=Create a \dim_superhost_evolution` model. It should have `is_currently_superhost`, `status_change_count` and `last_status_change_at`.withmodels/dim_superhost_evolution.sql` present:dim_superhost_evolution, the check passes; with the model absent it still fails and names only that model.Why not the broader check (the investigation). Offline, on the 818 rebuilt cases of
requirement-check(74 gold, 242 passing and 138 failing faithful rebuilds, plus unfaithful and unverifiable ones), a strictly conservative rule (only identifiers the request names explicitly as a model or column to create, matched exactly, no model call, request text and built facts only) was tried in four variants:Xmodel")analytics_engineering008are flagged under C and D (ingestion_timestamp); their rebuild failed (dbtexit 2, nothing built), so the facts are wrong, not the submission.geo_segment, 4 runs) and one run whose build also failed.Conclusion: no rule variant both has zero false alarms and catches a meaningful number of failures that could be reached, so no new check is added. Numbers are tiny (2 tasks behind all finished column hits). The effect on live task success was not measured; no model calls were made.
Behaviour a user could notice, and a known limit. For
Add adepartmentcolumn to the modelm``, the contract is nowmonly (before: `m` plus a bogus model named `department`, so the gate failed regardless). `m` merely existing before the session satisfies the name check, as it does for any model named in a modification line that uses `add`; this validator checks names, not whether the column was added. I tried marking such lines as modifications so `dbt-nothing-built` would ask for session work, and removed it: reviewers produced three counterexamples where it would demand edits the task did not require (variables edited in `dbt_project.yml`, `create` verbs, mixed create and add lines).Which error this validator prefers, and known limits. The costlier error for a finish-time validator is the false alarm: a live comparison showed sessions spending all three retries on false validator messages. So a span that the text marks as a column, field, variable or macro by an explicit cue is not required as a model. A span with no such cue is not ambiguous; it is named like a deliverable ("create model X", "the project should have X") and stays required exactly as before this change, because losing it would empty the contract and silence both completion gates. A test asserts that, over 19 plainly worded requests, the required set equals what
origin/mainreturned.Known limits. This is pattern matching on English phrasing, not understanding. It does not understand: other languages; pronouns or references across sentences beyond "it/they/that/these/the ... table"; a column named with no cue word at all; phrasing such as "the table now carries
x"; negations other than the verb-adjacent ones; lists broken by parentheses, quotes or sub-clauses beyond the ones tested; and column or variable wording outside the kind words column, field, attribute, variable, macro, setting, parameter. Such wording falls on the "required" side. Reviewers' round after round of counterexamples are of this kind; I fixed those that lose a plainly named model or raise a false alarm on plain wording and stop there.Deployment readiness. TypeScript change in the validator lane only; no migration, flag or config. Rolls back by revert.
User impact. A task document that mentions a column or variable in backticks no longer produces a "missing deliverable" retry turn (up to three) for it. Cost: a name the extractor used to require may now be unrequired when the line also describes it as a column, a variable or a "have/contain/expose" item. A required model on a line that names no earlier model and that follows "have" is still required. Task documents with a missing real model are reported as before.
How did you verify your code works?
bun test test/altimate/validators(1021 tests, 889 pass, 132 skipped, 0 fail; the 2 files changed arevalidator-utils.tsanddbt-deliverable-names.test.ts). about 85 new test cases through the realDbtDeliverableNamesValidatorwith an on-disk project andTASK.md: the issue's three shapes, a column list with parenthetical notes, "columnsa,bandc", a rename whose column target is the last span, and the "must stay silent / must stay required" cases from the review (model beside a column span,model called,settings model called, attributive "columns", "project should haveaandbmodels"). Run against the unmodifiedorigin/mainsource, 22 of the 49 tests in the file fail; the others pin behaviour that must not change.bun run typecheckclean. Marker check (script/upstream/analyze.ts --markers --base main --strict) clean.snap__hosts,dim_superhost_evolution,analysis__answer,fct_reviews) still are.contain/exposesiblings.e2e-real-dbttests in the same directory ran as part of the suite.validator-utils.tsnear line 242 only (concurrency); this change is near line 780, so no overlap.Screenshots / recordings
Not applicable (no UI change).
Checklist
🤖 Generated with Claude Code
Note
Medium Risk
Changes finish-time contract parsing for all requirement-line tasks; incorrect filtering could silence the gate or miss a real model, though the implementation biases toward keeping requirements and is heavily regression-tested.
Overview
Fixes false “missing deliverable” failures in
dbt-deliverable-nameswhen task prose lists columns, fields, dbt variables, or rename targets in backticks alongside real model names.extractRequiredDeliverables(tier-3 requirement lines) no longer treats every inline code span as a required model. It calls newmodelCandidateSpans, which keeps spans that look like models/tables/files and drops spans only when surrounding English explicitly marks them as columns, variables/macros, “should have/contain/expose” column lists, or rename targets—while erring toward keeping ambiguous names so plainly required models are not stripped (path-shaped spans still count as files).Adds a large
dbt-deliverable-names.test.tssuite (~85 cases) covering benchmark-style phrasing and asserting unchanged required-model sets for plainly worded tasks vs.origin/main.Reviewed by Cursor Bugbot for commit 086cefa. Bugbot is set up for automated code reviews on this repo. Configure here.