Skip to content

Commit 7c77d26

Browse files
anandgupta42claude
andcommitted
fix: address CodeRabbit review comments
1. Replace `process.exit()` with `process.exitCode` + `return` so the outer `finally` block in `index.ts` can run `Telemetry.shutdown()`. 2. Preserve A-F grade value from `altimate_core.grade` response in `CheckCategoryResult` metadata (`grade`, `score` fields). 3. Skip DB migration for `check` command — it only needs `Dispatcher` and has zero database dependencies. Prevents startup failure on read-only CI environments and removes multi-minute overhead. 4. Remove unused `--dialect`, `--dbt-project`, `--manifest` from docs options table (code already removed in prior commit). 5. Add `text` language specifier to fenced code block in `check.md`. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 34f91df commit 7c77d26

4 files changed

Lines changed: 58 additions & 23 deletions

File tree

docs/docs/usage/check.md

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,8 @@ altimate-code check --checks lint
6767
| `--checks` | string | `lint,safety` | Comma-separated list of checks to run |
6868
| `--schema` | string | - | Path to schema file for validation context |
6969
| `--policy` | string | - | Path to policy JSON file (required for `policy` check) |
70-
| `--dialect` | string | - | SQL dialect (`snowflake`, `bigquery`, `postgres`, etc.) |
7170
| `--severity` | string | `info` | Minimum severity level to report: `info`, `warning`, `error` |
7271
| `--fail-on` | string | `none` | Exit 1 if findings at this level or above: `none`, `warning`, `error` |
73-
| `--dbt-project` | string | - | Path to dbt project directory |
74-
| `--manifest` | string | - | Path to dbt `manifest.json` |
7572

7673
---
7774

@@ -151,7 +148,7 @@ When using `--format json`, the command writes structured JSON to stdout (diagno
151148

152149
The default `text` format is designed for human consumption:
153150

154-
```
151+
```text
155152
Checked 3 file(s) with [lint, safety]
156153
157154
--- LINT ---

packages/opencode/src/cli/cmd/check.ts

Lines changed: 39 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,11 @@ async function runSemantic(sql: string, file: string, schemaPath?: string): Prom
233233
}
234234
}
235235

236-
async function runGrade(sql: string, file: string, schemaPath?: string): Promise<Finding[]> {
236+
async function runGrade(
237+
sql: string,
238+
file: string,
239+
schemaPath?: string,
240+
): Promise<{ findings: Finding[]; grade?: string; score?: number }> {
237241
try {
238242
const result = await Dispatcher.call("altimate_core.grade", {
239243
sql,
@@ -243,7 +247,7 @@ async function runGrade(sql: string, file: string, schemaPath?: string): Promise
243247
const issues = (result.data.issues ?? result.data.findings ?? result.data.recommendations ?? []) as Array<
244248
Record<string, unknown>
245249
>
246-
return issues.map((f) => ({
250+
const findings = issues.map((f) => ({
247251
file,
248252
line: f.line as number | undefined,
249253
column: f.column as number | undefined,
@@ -253,9 +257,13 @@ async function runGrade(sql: string, file: string, schemaPath?: string): Promise
253257
message: (f.message ?? f.description ?? "") as string,
254258
suggestion: f.suggestion as string | undefined,
255259
}))
260+
// Preserve the primary A-F grade value from the backend
261+
const grade = (result.data.grade ?? result.data.letter_grade) as string | undefined
262+
const score = (result.data.score ?? result.data.numeric_score) as number | undefined
263+
return { findings, grade, score }
256264
} catch (e) {
257265
console.error(`[grade] error processing ${file}: ${e instanceof Error ? e.message : String(e)}`)
258-
return [dispatcherErrorFinding("grade", file, e)]
266+
return { findings: [dispatcherErrorFinding("grade", file, e)] }
259267
}
260268
}
261269

@@ -321,18 +329,21 @@ export const CheckCommand = cmd({
321329
})
322330
if (checks.length === 0) {
323331
console.error("Error: no valid checks specified.")
324-
process.exit(1)
332+
process.exitCode = 1
333+
return
325334
}
326335

327336
// 2. Validate policy requirement
328337
if (checks.includes("policy")) {
329338
if (!args.policy) {
330339
console.error("Error: --policy is required when running the policy check.")
331-
process.exit(1)
340+
process.exitCode = 1
341+
return
332342
}
333343
if (!existsSync(args.policy)) {
334344
console.error(`Error: policy file not found: ${args.policy}`)
335-
process.exit(1)
345+
process.exitCode = 1
346+
return
336347
}
337348
}
338349

@@ -366,7 +377,7 @@ export const CheckCommand = cmd({
366377

367378
if (files.length === 0) {
368379
console.error("No SQL files found to check.")
369-
process.exit(0)
380+
return
370381
}
371382

372383
console.error(`Found ${files.length} SQL file(s) to check with [${checks.join(", ")}]`)
@@ -379,13 +390,16 @@ export const CheckCommand = cmd({
379390
policyJson = readFileSync(args.policy, "utf-8")
380391
} catch (e) {
381392
console.error(`Error reading policy file: ${e instanceof Error ? e.message : String(e)}`)
382-
process.exit(1)
393+
process.exitCode = 1
394+
return
383395
}
384396
}
385397

386398
// 5. Run checks on all files in batches of 10
387399
const BATCH_SIZE = 10
388400
const allResults: Record<string, Finding[]> = {}
401+
let gradeValue: string | undefined
402+
let gradeScore: number | undefined
389403
for (const check of checks) {
390404
allResults[check] = []
391405
}
@@ -425,9 +439,13 @@ export const CheckCommand = cmd({
425439
case "semantic":
426440
findings = await runSemantic(sql, relFile, schemaPath)
427441
break
428-
case "grade":
429-
findings = await runGrade(sql, relFile, schemaPath)
442+
case "grade": {
443+
const gradeResult = await runGrade(sql, relFile, schemaPath)
444+
findings = gradeResult.findings
445+
if (gradeResult.grade) gradeValue = gradeResult.grade
446+
if (gradeResult.score != null) gradeScore = gradeResult.score
430447
break
448+
}
431449
}
432450
allResults[check].push(...findings)
433451
}
@@ -452,7 +470,13 @@ export const CheckCommand = cmd({
452470
results[check] = toCategoryResult(filterBySeverity(findings, minSeverity))
453471
}
454472

455-
// 8. Build output using the helper
473+
// 8. Attach grade metadata if available
474+
if (results.grade) {
475+
if (gradeValue) results.grade.grade = gradeValue
476+
if (gradeScore != null) results.grade.score = gradeScore
477+
}
478+
479+
// 9. Build output using the helper
456480
const output = buildCheckOutput({
457481
filesChecked: files.length,
458482
checksRun: checks,
@@ -463,7 +487,7 @@ export const CheckCommand = cmd({
463487
// Override pass with our pre-computed value from unfiltered findings
464488
output.summary.pass = pass
465489

466-
// 9. Output
490+
// 10. Output
467491
const duration = Date.now() - startTime
468492
if (args.format === "json") {
469493
process.stdout.write(JSON.stringify(output, null, 2) + "\n")
@@ -472,9 +496,10 @@ export const CheckCommand = cmd({
472496
}
473497
console.error(`Completed in ${duration}ms`)
474498

475-
// 10. Exit code
499+
// 11. Exit code — use process.exitCode instead of process.exit() to allow
500+
// the outer finally block in index.ts to run Telemetry.shutdown().
476501
if (!pass) {
477-
process.exit(1)
502+
process.exitCode = 1
478503
}
479504
},
480505
})

packages/opencode/src/index.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,10 +134,13 @@ let cli = yargs(hideBin(process.argv))
134134
args: process.argv.slice(2),
135135
})
136136

137+
// altimate_change start — check: skip DB migration for stateless commands (check only needs Dispatcher)
138+
const isStatelessCommand = process.argv[2] === "check"
139+
// altimate_change end
137140
// altimate_change start - db marker name
138141
const marker = path.join(Global.Path.data, "altimate-code.db")
139142
// altimate_change end
140-
if (!(await Filesystem.exists(marker))) {
143+
if (!isStatelessCommand && !(await Filesystem.exists(marker))) {
141144
const tty = process.stderr.isTTY
142145
process.stderr.write("Performing one time database migration, may take a few minutes..." + EOL)
143146
const width = 36

packages/opencode/test/cli/check-e2e.test.ts

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,10 +130,12 @@ const origExit = process.exit
130130
beforeEach(async () => {
131131
resetDispatcherMocks()
132132
exitCode = undefined
133+
process.exitCode = 0
133134
stdoutData = ""
134135
stderrData = ""
135136
tmpDir = await mktmp()
136137

138+
// Keep process.exit mock as safety net
137139
process.exit = ((code?: number) => {
138140
exitCode = code ?? 0
139141
throw new Error(`__EXIT_${code ?? 0}__`)
@@ -154,13 +156,16 @@ beforeEach(async () => {
154156

155157
afterEach(async () => {
156158
process.exit = origExit
159+
process.exitCode = 0
157160
mock.restore()
158161
await tmpDir.cleanup()
159162
})
160163

161164
async function runHandler(
162165
args: HandlerArgs,
163166
): Promise<{ exitCode: number | undefined; stdout: string; stderr: string }> {
167+
exitCode = undefined
168+
process.exitCode = 0
164169
try {
165170
const savedCwd = process.cwd
166171
;(process as any).cwd = () => tmpDir.dir
@@ -171,12 +176,16 @@ async function runHandler(
171176
}
172177
} catch (e) {
173178
if (e instanceof Error && e.message.startsWith("__EXIT_")) {
174-
// expected
179+
// expected — from legacy process.exit() mock
175180
} else {
176181
throw e
177182
}
178183
}
179-
return { exitCode, stdout: stdoutData, stderr: stderrData }
184+
// Handler uses process.exitCode (preferred) or process.exit() (mocked to set exitCode)
185+
// Note: Bun doesn't support process.exitCode = undefined, so we use 0 as "no error"
186+
const code = exitCode ?? (process.exitCode === 0 ? undefined : (process.exitCode as number))
187+
process.exitCode = 0
188+
return { exitCode: code, stdout: stdoutData, stderr: stderrData }
180189
}
181190

182191
function parseJson(stdout: string): any {
@@ -475,9 +484,10 @@ describe("check command E2E", () => {
475484
expect(j.files_checked).toBe(1)
476485
})
477486

478-
test("exits 0 when no SQL files found", async () => {
487+
test("returns cleanly when no SQL files found (exit 0)", async () => {
479488
const r = await runHandler(baseArgs({ files: ["/nonexistent.sql"], checks: "lint" }))
480-
expect(r.exitCode).toBe(0)
489+
// No exitCode set — handler just returns without error
490+
expect(r.exitCode).toBeUndefined()
481491
expect(r.stderr).toContain("No SQL files found")
482492
})
483493

0 commit comments

Comments
 (0)