Repository navigation
feat(run): enforce output schemas with bounded corrective retries (aictrl#5958) #129
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
62fbed9
51b19bc
7525fab
c6b306c
52b94b7
20df10f
286c329
368a386
ff1d6d1
cef40a5
487f616
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,63 @@ | ||||||
| import path from "path" | ||||||
| import fs from "fs/promises" | ||||||
| import { OutputSchema } from "@/session/output-schema" | ||||||
|
|
||||||
| export const OUTPUT_SCHEMA_CONFIG = "OUTPUT_SCHEMA_CONFIG" | ||||||
| export const OUTPUT_CONFIG_EXIT = 2 | ||||||
| export const OUTPUT_FAILED_EXIT = 3 | ||||||
|
|
||||||
| export async function outputSchema(input: { schema?: string; retries?: number; result?: string }) { | ||||||
| if (!input.schema) { | ||||||
| if (input.retries !== undefined || input.result !== undefined) | ||||||
| throw new Error("--output-schema-retries and --output-result require --output-schema") | ||||||
| return | ||||||
| } | ||||||
| if (input.retries !== undefined && (!Number.isSafeInteger(input.retries) || input.retries < 0)) | ||||||
| throw new Error("--output-schema-retries must be an integer >= 0") | ||||||
| const text = await Bun.file(input.schema) | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ⚪ Module mixes Bun and node fs APIs, skips Filesystem util.
Suggested change
🤖 Fix with your agentWhy this mattersThe new module reads the schema via import path from "path"
import fs from "fs/promises"
import { OutputSchema } from "@/session/output-schema" |
||||||
| .text() | ||||||
| .catch(() => { | ||||||
| throw new Error(`${input.schema}: cannot read schema file`) | ||||||
| }) | ||||||
| const schema: unknown = (() => { | ||||||
| try { | ||||||
| return JSON.parse(text) | ||||||
| } catch { | ||||||
| throw new Error(`${input.schema}: invalid JSON`) | ||||||
| } | ||||||
| })() | ||||||
| if (!schema || typeof schema !== "object" || Array.isArray(schema)) | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Two distinct config errors share identical message. --- a/packages/cli/src/cli/cmd/run.output.ts
+++ b/packages/cli/src/cli/cmd/run.output.ts
@@ -29,2 +29,2 @@
- throw new Error(`${input.schema}: schema root must have type "object"`)
+ throw new Error(`${input.schema}: schema root must be a JSON object`)
🤖 Fix with your agentWhy this mattersrun.output.ts:29-30 throws const schema: unknown = (() => {
try {
return JSON.parse(text)
} catch {
throw new Error(`${input.schema}: invalid JSON`)
}
})()
if (!schema || typeof schema !== "object" || Array.isArray(schema))
throw new Error(`${input.schema}: schema root must have type "object"`) |
||||||
| throw new Error(`${input.schema}: schema root must have type "object"`) | ||||||
| const canonical = schema as Record<string, unknown> | ||||||
| const validate = (() => { | ||||||
| try { | ||||||
| return OutputSchema.compile(canonical) | ||||||
| } catch (error) { | ||||||
| throw new Error(`${input.schema}: ${error instanceof Error ? error.message : "cannot compile schema"}`) | ||||||
| } | ||||||
| })() | ||||||
| if (input.result) { | ||||||
| const directory = path.dirname(input.result) | ||||||
| const writable = await fs | ||||||
| .stat(directory) | ||||||
| .then(async (stat) => { | ||||||
| if (!stat.isDirectory()) return false | ||||||
| await fs.access(directory, fs.constants.W_OK | fs.constants.X_OK) | ||||||
| return true | ||||||
| }) | ||||||
| .catch(() => false) | ||||||
| if (!writable) throw new Error(`${input.result}: output-result parent directory must exist and be writable`) | ||||||
| } | ||||||
| return { format: { type: "json_schema" as const, schema: canonical, retryCount: input.retries ?? 2 }, validate } | ||||||
| } | ||||||
|
|
||||||
| export async function outputResult(file: string, value: unknown, aborted = () => false) { | ||||||
| const temp = path.join(path.dirname(file), `.${path.basename(file)}.${crypto.randomUUID()}.tmp`) | ||||||
| try { | ||||||
| await Bun.write(temp, JSON.stringify(value, null, 2) + "\n", { createPath: false }) | ||||||
| if (aborted()) throw new Error("Structured output cancelled") | ||||||
| await fs.rename(temp, file) | ||||||
| } finally { | ||||||
| await fs.rm(temp, { force: true }) | ||||||
| } | ||||||
| } | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Result section duplicated verbatim in two READMEs.
🤖 Fix with your agent
Why this matters
The new 34-line "Validated Final Results" section is duplicated word-for-word between README.md and packages/cli/README.md (only the EVENTS.md link path differs). The two READMEs do not verbatim-mirror each other today — neighbouring sections are tailored per audience ("For programmatic consumption..." with jq '.type' in the root vs "For programmatic pipes:" with jq '.properties.part.text' in packages/cli). Verbatim duplication breaks that pattern and creates dual-maintenance drift risk for exit codes, defaults and bounds that must stay in sync with EVENTS.md and the code.