Skip to content

Commit 33c331b

Browse files
kulvirgitclaude
andauthored
fix: three simple bugs — locale duration, dispatcher reset, impact test count (#529)
1. Fix Locale.duration for >=24h (#368): days and hours were swapped. `Math.floor(input / 3600000)` gave total hours, not days. Now correctly: days = input/86400000, hours = remainder/3600000. 2. Fix dispatcher.reset() not clearing lazy registration hook (#468): reset() only cleared handlers but left _ensureRegistered alive. Next call() would re-register all handlers via the stale hook, causing flaky test failures when test files share module state. 3. Fix impact_analysis showing project-wide test_count (#371): Was using manifest.test_count (all tests in project). Now collects individual test nodes with their depends_on refs and counts only tests that reference the target model or its downstream dependents. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 01f355c commit 33c331b

File tree

7 files changed

+42
-17
lines changed

7 files changed

+42
-17
lines changed

packages/opencode/src/altimate/native/dbt/manifest.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import type {
1010
DbtManifestResult,
1111
DbtModelInfo,
1212
DbtSourceInfo,
13+
DbtTestInfo,
1314
ModelColumn,
1415
} from "../types"
1516

@@ -30,6 +31,7 @@ export async function parseManifest(params: DbtManifestParams): Promise<DbtManif
3031
const emptyResult: DbtManifestResult = {
3132
models: [],
3233
sources: [],
34+
tests: [],
3335
source_count: 0,
3436
model_count: 0,
3537
test_count: 0,
@@ -67,6 +69,7 @@ export async function parseManifest(params: DbtManifestParams): Promise<DbtManif
6769
const sourcesDict = manifest.sources || {}
6870

6971
const models: DbtModelInfo[] = []
72+
const tests: DbtTestInfo[] = []
7073
let testCount = 0
7174
let snapshotCount = 0
7275
let seedCount = 0
@@ -88,6 +91,11 @@ export async function parseManifest(params: DbtManifestParams): Promise<DbtManif
8891
})
8992
} else if (resourceType === "test") {
9093
testCount++
94+
tests.push({
95+
unique_id: nodeId,
96+
name: node.name || "",
97+
depends_on: node.depends_on?.nodes || [],
98+
})
9199
} else if (resourceType === "snapshot") {
92100
snapshotCount++
93101
} else if (resourceType === "seed") {
@@ -111,6 +119,7 @@ export async function parseManifest(params: DbtManifestParams): Promise<DbtManif
111119
return {
112120
models,
113121
sources,
122+
tests,
114123
source_count: sources.length,
115124
model_count: models.length,
116125
test_count: testCount,

packages/opencode/src/altimate/native/dispatcher.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,15 @@ export function register(method: BridgeMethod, handler: NativeHandler): void {
1717
nativeHandlers.set(method, handler)
1818
}
1919

20-
/** Clear all registered handlers (for test isolation). */
20+
/** Lazy registration hook — set by native/index.ts */
21+
let _ensureRegistered: (() => Promise<void>) | null = null
22+
23+
/** Clear all registered handlers and lazy registration hook (for test isolation). */
2124
export function reset(): void {
2225
nativeHandlers.clear()
26+
_ensureRegistered = null
2327
}
2428

25-
/** Lazy registration hook — set by native/index.ts */
26-
let _ensureRegistered: (() => Promise<void>) | null = null
27-
2829
/** Called by native/index.ts to set the lazy registration function. */
2930
export function setRegistrationHook(fn: () => Promise<void>): void {
3031
_ensureRegistered = fn

packages/opencode/src/altimate/native/types.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,9 +188,16 @@ export interface DbtSourceInfo {
188188
columns: ModelColumn[]
189189
}
190190

191+
export interface DbtTestInfo {
192+
unique_id: string
193+
name: string
194+
depends_on: string[]
195+
}
196+
191197
export interface DbtManifestResult {
192198
models: DbtModelInfo[]
193199
sources: DbtSourceInfo[]
200+
tests: DbtTestInfo[]
194201
source_count: number
195202
model_count: number
196203
test_count: number

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

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,15 @@ export const ImpactAnalysisTool = Tool.define("impact_analysis", {
7070
const direct = downstream.filter((d) => d.depth === 1)
7171
const transitive = downstream.filter((d) => d.depth > 1)
7272

73-
// Step 4: Report test count (manifest has test_count but not individual tests)
74-
const affectedTestCount = manifest.test_count ?? 0
73+
// Step 4: Count only tests that reference the target model or its downstream models
74+
const affectedModelIds = new Set([
75+
targetModel.unique_id,
76+
...downstream.map((d) => modelsByName.get(d.name)?.unique_id).filter(Boolean),
77+
])
78+
const affectedTests = (manifest.tests ?? []).filter((t) =>
79+
t.depends_on?.some((dep) => affectedModelIds.has(dep)),
80+
)
81+
const affectedTestCount = affectedTests.length
7582

7683
// Step 5: If column specified, attempt column-level lineage
7784
let columnImpact: string[] = []
@@ -253,9 +260,13 @@ export function formatImpactReport(data: {
253260

254261
// Affected tests
255262
if (data.affectedTestCount > 0) {
256-
lines.push(`Tests in project: ${data.affectedTestCount}`)
263+
lines.push(`Affected tests: ${data.affectedTestCount}`)
257264
lines.push("".padEnd(40, "-"))
258-
lines.push(` Run \`dbt test\` to verify all ${data.affectedTestCount} tests still pass after this change.`)
265+
lines.push(
266+
data.affectedTestCount === 1
267+
? " Run `dbt test` to verify this test still passes after this change."
268+
: ` Run \`dbt test\` to verify these ${data.affectedTestCount} tests still pass after this change.`,
269+
)
259270
lines.push("")
260271
}
261272

packages/opencode/src/util/locale.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ export namespace Locale {
5454
const minutes = Math.floor((input % 3600000) / 60000)
5555
return `${hours}h ${minutes}m`
5656
}
57-
const hours = Math.floor(input / 3600000)
58-
const days = Math.floor((input % 3600000) / 86400000)
57+
const days = Math.floor(input / 86400000)
58+
const hours = Math.floor((input % 86400000) / 3600000)
5959
return `${days}d ${hours}h`
6060
}
6161

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ describe("formatImpactReport", () => {
151151
})
152152
expect(report).toContain("WARNING: Rename requires updating all downstream references.")
153153
expect(report).toContain("Update all downstream SQL references to new name")
154-
expect(report).toContain("Tests in project: 5")
154+
expect(report).toContain("Affected tests: 5")
155155
})
156156

157157
test("column-level impact shows affected columns", () => {

packages/opencode/test/util/locale.test.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,10 @@ describe("Locale.duration", () => {
4343
expect(Locale.duration(5400000)).toBe("1h 30m")
4444
})
4545

46-
// BUG: Locale.duration >=24h has swapped days/hours calculation.
47-
// hours = Math.floor(input / 3600000) gives total hours (25), not remainder.
48-
// days = Math.floor((input % 3600000) / 86400000) always yields 0.
49-
// Correct: days = Math.floor(input / 86400000), hours = Math.floor((input % 86400000) / 3600000)
50-
// 90000000ms = 25h = 1d 1h — should display "1d 1h"
46+
// Fixed in this PR: days and hours were swapped for >=24h durations.
47+
// 90000000ms = 25h = 1d 1h
5148
// See: https://github.com/AltimateAI/altimate-code/issues/368
52-
test.skip("FIXME: days and hours for >=24h are calculated correctly", () => {
49+
test("days and hours for >=24h are calculated correctly", () => {
5350
expect(Locale.duration(90000000)).toBe("1d 1h")
5451
})
5552
})

0 commit comments

Comments
 (0)