Skip to content

Commit efef758

Browse files
suryaiyer95claude
andcommitted
fix: [AI-5975] address code review findings for tool wrappers
- Add contract documentation to `ok()` explaining `success` semantics - Make `extractEquivalenceErrors` and `extractSemanticsErrors` defensive against object-type `validation_errors` entries - Apply `isRealFailure` pattern to validate tool for consistent error handling Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent f123470 commit efef758

4 files changed

Lines changed: 17 additions & 5 deletions

File tree

packages/opencode/src/altimate/native/altimate-core.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,14 @@ function toData(obj: unknown): Record<string, unknown> {
2525
return JSON.parse(JSON.stringify(obj)) as Record<string, unknown>
2626
}
2727

28-
/** Wrap a handler body into the standard AltimateCoreResult envelope. */
28+
/**
29+
* Wrap a handler body into the standard AltimateCoreResult envelope.
30+
*
31+
* Contract: ok(true, data) means "the operation completed." Semantic results
32+
* (e.g., SQL is invalid, queries are not equivalent) live in the data fields,
33+
* NOT in the success flag. success=false only when the handler throws (fail()).
34+
* This prevents semantic findings from being misreported as tool crashes.
35+
*/
2936
function ok(
3037
success: boolean,
3138
data: Record<string, unknown>,

packages/opencode/src/altimate/tools/altimate-core-equivalence.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,9 @@ export const AltimateCoreEquivalenceTool = Tool.define("altimate_core_equivalenc
4242

4343
function extractEquivalenceErrors(data: Record<string, any>): string | undefined {
4444
if (Array.isArray(data.validation_errors) && data.validation_errors.length > 0) {
45-
const msgs = data.validation_errors.filter(Boolean)
45+
const msgs = data.validation_errors
46+
.map((e: any) => (typeof e === "string" ? e : e.message ?? String(e)))
47+
.filter(Boolean)
4648
return msgs.length > 0 ? msgs.join("; ") : undefined
4749
}
4850
return undefined

packages/opencode/src/altimate/tools/altimate-core-semantics.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ export const AltimateCoreSemanticsTool = Tool.define("altimate_core_semantics",
3939

4040
function extractSemanticsErrors(data: Record<string, any>): string | undefined {
4141
if (Array.isArray(data.validation_errors) && data.validation_errors.length > 0) {
42-
const msgs = data.validation_errors.filter(Boolean)
42+
const msgs = data.validation_errors
43+
.map((e: any) => (typeof e === "string" ? e : e.message ?? String(e)))
44+
.filter(Boolean)
4345
return msgs.length > 0 ? msgs.join("; ") : undefined
4446
}
4547
return undefined

packages/opencode/src/altimate/tools/altimate-core-validate.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,10 @@ export const AltimateCoreValidateTool = Tool.define("altimate_core_validate", {
2424
})
2525
const data = (result.data ?? {}) as Record<string, any>
2626
const error = result.error ?? data.error ?? extractValidationErrors(data)
27+
const isRealFailure = !!error
2728
return {
28-
title: `Validate: ${data.valid ? "VALID" : "INVALID"}`,
29-
metadata: { success: result.success, valid: data.valid, ...(error && { error }) },
29+
title: isRealFailure ? "Validate: ERROR" : `Validate: ${data.valid ? "VALID" : "INVALID"}`,
30+
metadata: { success: !isRealFailure, valid: data.valid, ...(error && { error }) },
3031
output: formatValidate(data),
3132
}
3233
} catch (e) {

0 commit comments

Comments
 (0)