Skip to content

Commit 61a3ff1

Browse files
anandgupta42claude
andcommitted
fix: address code review — guard --downstream without --model, remove dead code
- Error when `--downstream` used without `--model` instead of silently triggering a full project build - Remove unused `condition` field from `Suggestion` interface - Rename misleading integration test names to match actual behavior - Add test for `--downstream` without `--model` error case Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 77408e2 commit 61a3ff1

4 files changed

Lines changed: 14 additions & 4 deletions

File tree

packages/dbt-tools/src/commands/build.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@ import type { DBTProjectIntegrationAdapter, CommandProcessResult } from "@altima
22

33
export async function build(adapter: DBTProjectIntegrationAdapter, args: string[]) {
44
const model = flag(args, "model")
5-
if (!model) return project(adapter)
65
const downstream = args.includes("--downstream")
6+
if (!model) {
7+
if (downstream) return { error: "--downstream requires --model" }
8+
return project(adapter)
9+
}
710
const result = await adapter.unsafeBuildModelImmediately({
811
plusOperatorLeft: "",
912
modelName: model,

packages/dbt-tools/test/build.test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,14 @@ describe("build command", () => {
4545
})
4646
})
4747

48+
test("build --downstream without --model returns error", async () => {
49+
const adapter = makeAdapter()
50+
const result = await build(adapter, ["--downstream"])
51+
expect(result).toEqual({ error: "--downstream requires --model" })
52+
expect(adapter.unsafeBuildProjectImmediately).not.toHaveBeenCalled()
53+
expect(adapter.unsafeBuildModelImmediately).not.toHaveBeenCalled()
54+
})
55+
4856
test("build surfaces stderr as error", async () => {
4957
const adapter = makeAdapter({
5058
unsafeBuildProjectImmediately: mock(() =>

packages/opencode/src/skill/followups.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ export namespace SkillFollowups {
44
skill: string // skill name to suggest
55
label: string // short display label
66
description: string // why this is a good next step
7-
condition?: string // optional: when this suggestion applies
87
}
98

109
// Map from skill name to follow-up suggestions

packages/opencode/test/skill/skill-followups-integration.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { Instance } from "../../src/project/instance"
55
import { tmpdir } from "../fixture/fixture"
66
import path from "path"
77

8-
test("skill tool output includes follow-up suggestions for skills with followups", async () => {
8+
test("SkillFollowups.format returns follow-up suggestions for skills with followups", async () => {
99
await using tmp = await tmpdir({
1010
git: true,
1111
init: async (dir) => {
@@ -40,7 +40,7 @@ Instructions here.
4040
})
4141
})
4242

43-
test("skill tool output has no follow-ups for skills without followups defined", async () => {
43+
test("SkillFollowups.format returns empty for skills without followups defined", async () => {
4444
await using tmp = await tmpdir({
4545
git: true,
4646
init: async (dir) => {

0 commit comments

Comments
 (0)