Skip to content

Commit 167d4a7

Browse files
anandgupta42claude
anandgupta42
and
claude
authored
test: consolidate 12 hourly test PRs + fix slugify and hints sort (#439)
* test: consolidate 12 hourly test PRs into single coverage PR Combines test coverage from 12 redundant `test:` PRs (each containing duplicate `hints.test.ts`) into one cohesive PR with unique tests only. **New test files (10):** - `command/hints.test.ts` — `Command.hints()` placeholder extraction - `skill/fmt.test.ts` — `Skill.fmt()` formatting for TUI and system prompt - `altimate/tools/sql-analyze-tool.test.ts` — AI-5975 success semantics - `patch/seek-sequence.test.ts` — `seekSequence` 4-pass matching - `cli/error-format.test.ts` — `FormatError` all 8 NamedError branches - `cli/is-tool-on-path.test.ts` — tool discovery paths - `tool/batch.test.ts` — `BatchTool` schema + `formatValidationError` - `command/altimate-commands.test.ts` — builtin command registration - `altimate/tools/impact-analysis.test.ts` — DAG traversal + report - `altimate/tools/training-import.test.ts` — markdown parser + `slugify` **Enhanced existing tests (4):** - `finops-formatting.test.ts` — TB/PB units, negative values - `wildcard.test.ts` — star crosses `/`, regex escaping, tail matching - `config/markdown.test.ts` — shell extraction, frontmatter sanitization - `util/filesystem.test.ts` — `findUp`, `globUp`, `overlaps` **Bug fixes (2):** - `slugify`: NFKD normalization for accented chars (café→cafe not caf), `"untitled"` fallback for empty results, truncate-before-trim order - `Command.hints`: numeric sort for multi-digit placeholders ($1,$2,$10 not $1,$10,$2) 269 new tests, 5046 total pass, 0 fail. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address PR review comments — export functions, use tmpdir fixture, fix portability - Export `findDownstream`, `formatImpactReport`, `DownstreamModel` from `impact-analysis.ts` and import in tests instead of copying - Export `parseMarkdownSections`, `slugify`, `MarkdownSection` from `training-import.ts` and import in tests instead of copying - Replace manual `beforeEach`/`afterEach` temp dir lifecycle in `seek-sequence.test.ts` with shared `tmpdir()` fixture pattern - Replace `require("fs").writeFileSync` with async `fs.writeFile` - Replace hardcoded `/tmp/` path with `path.join(tmp.path, ...)` - Use cross-platform PATH separator in `is-tool-on-path.test.ts` Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 544903f commit 167d4a7

17 files changed

Lines changed: 1680 additions & 10 deletions

‎packages/opencode/src/altimate/tools/impact-analysis.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,14 +152,14 @@ export const ImpactAnalysisTool = Tool.define("impact_analysis", {
152152
},
153153
})
154154

155-
interface DownstreamModel {
155+
export interface DownstreamModel {
156156
name: string
157157
depth: number
158158
materialized?: string
159159
path: string[]
160160
}
161161

162-
function findDownstream(
162+
export function findDownstream(
163163
targetName: string,
164164
models: Array<{ name: string; depends_on: string[]; materialized?: string }>,
165165
): DownstreamModel[] {
@@ -188,7 +188,7 @@ function findDownstream(
188188
return results
189189
}
190190

191-
function formatImpactReport(data: {
191+
export function formatImpactReport(data: {
192192
model: string
193193
column?: string
194194
changeType: string

‎packages/opencode/src/altimate/tools/training-import.ts‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -159,12 +159,12 @@ export const TrainingImportTool = Tool.define("training_import", {
159159
},
160160
})
161161

162-
interface MarkdownSection {
162+
export interface MarkdownSection {
163163
name: string
164164
content: string
165165
}
166166

167-
function parseMarkdownSections(markdown: string): MarkdownSection[] {
167+
export function parseMarkdownSections(markdown: string): MarkdownSection[] {
168168
const sections: MarkdownSection[] = []
169169
const lines = markdown.split("\n")
170170
let currentH1 = ""
@@ -222,11 +222,14 @@ function parseMarkdownSections(markdown: string): MarkdownSection[] {
222222
return sections
223223
}
224224

225-
function slugify(text: string): string {
226-
return text
225+
export function slugify(text: string): string {
226+
const result = text
227+
.normalize("NFKD")
228+
.replace(/[\u0300-\u036f]/g, "")
227229
.toLowerCase()
228230
.replace(/[^a-z0-9\s-]/g, "")
229231
.replace(/\s+/g, "-")
230-
.replace(/^-+|-+$/g, "")
231232
.slice(0, 64)
233+
.replace(/^-+|-+$/g, "")
234+
return result || "untitled"
232235
}

‎packages/opencode/src/command/index.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,9 @@ export namespace Command {
5454
const result: string[] = []
5555
const numbered = template.match(/\$\d+/g)
5656
if (numbered) {
57-
for (const match of [...new Set(numbered)].sort()) result.push(match)
57+
// altimate_change start — fix lexicographic sort of multi-digit placeholders ($10 before $2)
58+
for (const match of [...new Set(numbered)].sort((a, b) => parseInt(a.slice(1), 10) - parseInt(b.slice(1), 10))) result.push(match)
59+
// altimate_change end
5860
}
5961
if (template.includes("$ARGUMENTS")) result.push("$ARGUMENTS")
6062
return result

‎packages/opencode/test/altimate/tools/finops-formatting.test.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,34 @@ describe("formatBytes: normal cases", () => {
1919
})
2020
})
2121

22+
describe("formatBytes: higher units (TB, PB)", () => {
23+
test("TB boundary", () => {
24+
expect(formatBytes(1024 ** 4)).toBe("1.00 TB")
25+
})
26+
27+
test("PB boundary", () => {
28+
expect(formatBytes(1024 ** 5)).toBe("1.00 PB")
29+
})
30+
31+
test("values beyond PB stay at PB (no EB unit)", () => {
32+
expect(formatBytes(1024 ** 6)).toBe("1024.00 PB")
33+
})
34+
35+
test("multi-PB value", () => {
36+
expect(formatBytes(2 * 1024 ** 5)).toBe("2.00 PB")
37+
})
38+
})
39+
2240
describe("formatBytes: edge cases", () => {
2341
test("negative bytes displays with sign", () => {
2442
expect(formatBytes(-100)).toBe("-100 B")
2543
expect(formatBytes(-1536)).toBe("-1.50 KB")
2644
})
2745

46+
test("negative KB", () => {
47+
expect(formatBytes(-1024)).toBe("-1.00 KB")
48+
})
49+
2850
test("fractional bytes clamps to B unit", () => {
2951
expect(formatBytes(0.5)).toBe("1 B")
3052
})

‎packages/opencode/test/altimate/tools/impact-analysis.test.ts‎

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
import { describe, test, expect } from "bun:test"
2+
import { findDownstream, formatImpactReport } from "../../../src/altimate/tools/impact-analysis"
3+
import type { DownstreamModel } from "../../../src/altimate/tools/impact-analysis"
4+
5+
describe("findDownstream: DAG traversal", () => {
6+
test("returns empty for leaf model with no dependents", () => {
7+
const models = [
8+
{ name: "stg_orders", depends_on: ["source.raw_orders"], materialized: "view" },
9+
{ name: "stg_customers", depends_on: ["source.raw_customers"], materialized: "view" },
10+
]
11+
const result = findDownstream("stg_orders", models)
12+
expect(result).toHaveLength(0)
13+
})
14+
15+
test("finds direct dependents (depth 1)", () => {
16+
const models = [
17+
{ name: "stg_orders", depends_on: ["source.raw_orders"] },
18+
{ name: "fct_orders", depends_on: ["project.stg_orders", "project.stg_customers"] },
19+
{ name: "stg_customers", depends_on: ["source.raw_customers"] },
20+
]
21+
const result = findDownstream("stg_orders", models)
22+
expect(result).toHaveLength(1)
23+
expect(result[0].name).toBe("fct_orders")
24+
expect(result[0].depth).toBe(1)
25+
})
26+
27+
test("finds transitive dependents across multiple depths", () => {
28+
const models = [
29+
{ name: "stg_orders", depends_on: ["source.raw_orders"] },
30+
{ name: "fct_orders", depends_on: ["project.stg_orders"] },
31+
{ name: "dim_orders", depends_on: ["project.fct_orders"] },
32+
{ name: "report_orders", depends_on: ["project.dim_orders"] },
33+
]
34+
const result = findDownstream("stg_orders", models)
35+
expect(result).toHaveLength(3)
36+
expect(result[0]).toMatchObject({ name: "fct_orders", depth: 1 })
37+
expect(result[1]).toMatchObject({ name: "dim_orders", depth: 2 })
38+
expect(result[2]).toMatchObject({ name: "report_orders", depth: 3 })
39+
})
40+
41+
test("tracks dependency paths correctly", () => {
42+
const models = [
43+
{ name: "stg_orders", depends_on: [] as string[] },
44+
{ name: "fct_orders", depends_on: ["project.stg_orders"] },
45+
{ name: "report", depends_on: ["project.fct_orders"] },
46+
]
47+
const result = findDownstream("stg_orders", models)
48+
expect(result[0].path).toEqual(["stg_orders", "fct_orders"])
49+
expect(result[1].path).toEqual(["stg_orders", "fct_orders", "report"])
50+
})
51+
52+
test("handles diamond dependency (A\u2192B, A\u2192C, B\u2192D, C\u2192D)", () => {
53+
const models = [
54+
{ name: "A", depends_on: [] as string[] },
55+
{ name: "B", depends_on: ["project.A"] },
56+
{ name: "C", depends_on: ["project.A"] },
57+
{ name: "D", depends_on: ["project.B", "project.C"] },
58+
]
59+
const result = findDownstream("A", models)
60+
// D should appear only once (visited set prevents duplicates)
61+
const names = result.map((r) => r.name)
62+
expect(names.filter((n) => n === "D")).toHaveLength(1)
63+
expect(result).toHaveLength(3) // B, C, D
64+
})
65+
66+
test("self-referencing model \u2014 behavior documentation only, not a valid dbt graph", () => {
67+
const models = [
68+
{ name: "stg_orders", depends_on: ["project.stg_orders"] },
69+
]
70+
// This assertion exists only to document current behavior, not to endorse it.
71+
// Self-referencing dbt models are invalid and cannot compile, so this edge case
72+
// is not reachable in practice. The visited set prevents infinite recursion.
73+
const result = findDownstream("stg_orders", models)
74+
expect(result).toHaveLength(1)
75+
expect(result[0].name).toBe("stg_orders")
76+
})
77+
78+
test("parses qualified names (strips prefix before last dot)", () => {
79+
const models = [
80+
{ name: "stg_orders", depends_on: [] as string[] },
81+
{ name: "fct_orders", depends_on: ["my_project.stg_orders", "other_project.stg_customers"] },
82+
]
83+
const result = findDownstream("stg_orders", models)
84+
expect(result).toHaveLength(1)
85+
expect(result[0].name).toBe("fct_orders")
86+
})
87+
88+
test("preserves materialization metadata", () => {
89+
const models = [
90+
{ name: "stg_orders", depends_on: [] as string[], materialized: "view" },
91+
{ name: "fct_orders", depends_on: ["project.stg_orders"], materialized: "table" },
92+
{ name: "report", depends_on: ["project.fct_orders"], materialized: "incremental" },
93+
]
94+
const result = findDownstream("stg_orders", models)
95+
expect(result[0].materialized).toBe("table")
96+
expect(result[1].materialized).toBe("incremental")
97+
})
98+
99+
test("model not in graph returns empty", () => {
100+
const models = [
101+
{ name: "stg_orders", depends_on: ["source.raw_orders"] },
102+
{ name: "fct_orders", depends_on: ["project.stg_orders"] },
103+
]
104+
const result = findDownstream("nonexistent_model", models)
105+
expect(result).toHaveLength(0)
106+
})
107+
})
108+
109+
describe("formatImpactReport", () => {
110+
test("safe change with zero downstream", () => {
111+
const report = formatImpactReport({
112+
model: "stg_temp",
113+
changeType: "remove",
114+
direct: [],
115+
transitive: [],
116+
affectedTestCount: 0,
117+
columnImpact: [],
118+
totalModels: 10,
119+
})
120+
expect(report).toContain("REMOVE stg_temp")
121+
expect(report).toContain("Blast radius: 0/10 models (0.0%)")
122+
expect(report).toContain("No downstream models depend on this. Change is safe to make.")
123+
expect(report).not.toContain("WARNING")
124+
})
125+
126+
test("remove with downstream shows BREAKING warning", () => {
127+
const report = formatImpactReport({
128+
model: "stg_orders",
129+
changeType: "remove",
130+
direct: [{ name: "fct_orders", depth: 1, path: ["stg_orders", "fct_orders"] }],
131+
transitive: [],
132+
affectedTestCount: 0,
133+
columnImpact: [],
134+
totalModels: 20,
135+
})
136+
expect(report).toContain("WARNING: This is a BREAKING change")
137+
expect(report).toContain("Blast radius: 1/20 models (5.0%)")
138+
expect(report).toContain("Direct Dependents (1)")
139+
expect(report).toContain("Consider deprecation period before removal")
140+
})
141+
142+
test("rename shows rename-specific warning and actions", () => {
143+
const report = formatImpactReport({
144+
model: "stg_orders",
145+
changeType: "rename",
146+
direct: [{ name: "fct_orders", depth: 1, path: ["stg_orders", "fct_orders"] }],
147+
transitive: [],
148+
affectedTestCount: 5,
149+
columnImpact: [],
150+
totalModels: 10,
151+
})
152+
expect(report).toContain("WARNING: Rename requires updating all downstream references.")
153+
expect(report).toContain("Update all downstream SQL references to new name")
154+
expect(report).toContain("Tests in project: 5")
155+
})
156+
157+
test("column-level impact shows affected columns", () => {
158+
const report = formatImpactReport({
159+
model: "stg_orders",
160+
column: "order_id",
161+
changeType: "retype",
162+
direct: [{ name: "fct_orders", depth: 1, path: ["stg_orders", "fct_orders"] }],
163+
transitive: [],
164+
affectedTestCount: 0,
165+
columnImpact: ["total_amount", "order_count"],
166+
totalModels: 10,
167+
})
168+
expect(report).toContain("RETYPE stg_orders.order_id")
169+
expect(report).toContain("CAUTION: Type change may cause implicit casts")
170+
expect(report).toContain("Affected Output Columns (2)")
171+
expect(report).toContain("total_amount")
172+
expect(report).toContain("order_count")
173+
})
174+
175+
test("percentage calculation with 0 total models does not produce NaN or Infinity", () => {
176+
const report = formatImpactReport({
177+
model: "stg_orders",
178+
changeType: "add",
179+
direct: [],
180+
transitive: [],
181+
affectedTestCount: 0,
182+
columnImpact: [],
183+
totalModels: 0,
184+
})
185+
expect(report).not.toContain("NaN")
186+
expect(report).not.toContain("Infinity")
187+
expect(report).toContain("Blast radius: 0/0 models")
188+
})
189+
190+
test("transitive dependents show dependency path", () => {
191+
const report = formatImpactReport({
192+
model: "stg_orders",
193+
changeType: "modify",
194+
direct: [{ name: "fct_orders", depth: 1, path: ["stg_orders", "fct_orders"] }],
195+
transitive: [{ name: "report", depth: 2, materialized: "table", path: ["stg_orders", "fct_orders", "report"] }],
196+
affectedTestCount: 0,
197+
columnImpact: [],
198+
totalModels: 50,
199+
})
200+
expect(report).toContain("Direct Dependents (1)")
201+
expect(report).toContain("Transitive Dependents (1)")
202+
expect(report).toContain("report [table] (via: stg_orders \u2192 fct_orders \u2192 report)")
203+
expect(report).toContain("Blast radius: 2/50 models (4.0%)")
204+
})
205+
})

0 commit comments

Comments
 (0)