Repository navigation
fix(hub): read check verdicts from the tool result's detail #163
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
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| import { type } from "arktype"; | ||
|
|
||
| // A check's verdict is read once from its whole output and kept in the result's detail, which | ||
| // history trimming leaves alone: a trimmed result loses the "check passed" line the model read. | ||
| const CheckDetail = type({ checkPassed: "boolean", "filed?": "boolean" }); | ||
| export type CheckDetail = typeof CheckDetail.infer; | ||
|
|
||
| /** The detail a check tool records, or undefined for any other result. */ | ||
| export function readCheckDetail(detail: unknown): CheckDetail | undefined { | ||
| const parsed = CheckDetail(detail); | ||
| return parsed instanceof type.errors ? undefined : parsed; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| import { expect, test } from "bun:test"; | ||
| import { appCheckDetail } from "./sidecar-tools.ts"; | ||
|
|
||
| const PASSED = | ||
| "check passed: built, started, and opened / at 1280 and 390 px with no errors"; | ||
| const MOCKUP_PASSED = | ||
| "check passed: served .builder/designs and opened / at 1280 and 390 px with no errors"; | ||
|
|
||
| const shell = (command: string) => ({ | ||
| id: "call", | ||
| name: "run_shell", | ||
| arguments: { command }, | ||
| }); | ||
| const output = (content: string) => ({ callId: "call", content }); | ||
|
|
||
| test("a check-app run records its verdict however the command runs it", () => { | ||
| for (const command of [ | ||
| "check-app", | ||
| "cd app && check-app", | ||
| "timeout 300 check-app", | ||
| "CI=1 check-app", | ||
| "./bin/check-app", | ||
| "bash -lc 'check-app'", | ||
| "timeout 600 check-app 2>&1 | tail -40", | ||
| ]) | ||
| expect(appCheckDetail(shell(command), output(PASSED))).toEqual({ | ||
| checkPassed: true, | ||
| }); | ||
| }); | ||
|
|
||
| test("only an app check-app pass of a command that succeeded counts as passed", () => { | ||
| expect( | ||
| appCheckDetail(shell("check-app"), output("check failed: 1 problem(s)")), | ||
| ).toEqual({ checkPassed: false }); | ||
| expect( | ||
| appCheckDetail(shell("check-app .builder/designs"), output(MOCKUP_PASSED)), | ||
| ).toEqual({ checkPassed: false }); | ||
| expect( | ||
| appCheckDetail(shell("check-app"), { | ||
| callId: "call", | ||
| content: PASSED, | ||
| isError: true, | ||
| }), | ||
| ).toEqual({ checkPassed: false }); | ||
| expect( | ||
| appCheckDetail( | ||
| shell("check-app && bun test"), | ||
| output(`exit code 1\n${PASSED}`), | ||
| ), | ||
| ).toEqual({ checkPassed: false }); | ||
| }); | ||
|
|
||
| test("a command that only names check-app records no verdict", () => { | ||
| expect( | ||
| appCheckDetail( | ||
| shell("grep -rn check-app ."), | ||
| output(`src/check.ts:12: ${PASSED}`), | ||
| ), | ||
| ).toBeUndefined(); | ||
| expect( | ||
| appCheckDetail( | ||
| shell("check-app --help"), | ||
| output("check-app check the app here"), | ||
| ), | ||
| ).toBeUndefined(); | ||
| expect( | ||
| appCheckDetail( | ||
| { id: "call", name: "read_file", arguments: { path: "check-app" } }, | ||
| output(PASSED), | ||
| ), | ||
| ).toBeUndefined(); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,7 @@ import type { | |
| RetryPolicy, | ||
| } from "@intx/types/runtime"; | ||
| import { REPETITION_ABORT } from "../inference/repetition.ts"; | ||
| import { CHECK_DESIGN_TOOL } from "../builds/design-check.ts"; | ||
| import { readCheckDetail, type CheckDetail } from "../builds/check-detail.ts"; | ||
| import { COMPACTOR, DROP_DEGENERATE } from "./compaction.ts"; | ||
|
|
||
| // About 1 coder turn in 100 skipped reasoning and repeated one sentence ("Ok to write. Let me | ||
|
|
@@ -144,11 +144,11 @@ const SHELL_WRITE = | |
| const WRITE_TOOLS = new Set(["write_file", "edit_file"]); | ||
| // The current task's calls, newest first: everything after the last mail the agent was handed. | ||
| function taskCalls(turns: readonly ConversationTurn[]) { | ||
| const results = new Map<string, { text: string; isError: boolean }>(); | ||
| const results = new Map<string, { detail: unknown; isError: boolean }>(); | ||
| const calls: { | ||
| name: string; | ||
| command: unknown; | ||
| result: string | undefined; | ||
| check: CheckDetail | undefined; | ||
| isError: boolean; | ||
| }[] = []; | ||
| for (let index = turns.length - 1; index >= 0; index--) { | ||
|
|
@@ -161,15 +161,15 @@ function taskCalls(turns: readonly ConversationTurn[]) { | |
| for (const block of [...turn.content].reverse()) { | ||
| if (block.type === "tool_result") | ||
| results.set(block.callId, { | ||
| text: textOf(block.content), | ||
| detail: block.detail, | ||
| isError: block.isError === true, | ||
| }); | ||
| if (block.type === "tool_call") { | ||
| const result = results.get(block.id); | ||
| calls.push({ | ||
| name: block.name, | ||
| command: (block.arguments as Record<string, unknown>)["command"], | ||
| result: result?.text, | ||
| check: readCheckDetail(result?.detail), | ||
| isError: result?.isError ?? true, | ||
| }); | ||
| } | ||
|
|
@@ -185,18 +185,12 @@ const ranShell = (call: TaskCall, pattern: RegExp) => | |
| typeof call.command === "string" && | ||
| pattern.test(call.command); | ||
|
|
||
| const says = (call: TaskCall, pattern: RegExp) => | ||
| call.result !== undefined && pattern.test(call.result); | ||
|
|
||
| // Run 36's coder passed check-app, kept editing, broke the app and would have replied done; a reply | ||
| // counts as done only when no file changed after the last passing check. check_design is the | ||
| // designer's check-app; file_design_option files only a design it passed as it stands. | ||
| function unchecked(turns: readonly ConversationTurn[]): boolean { | ||
| for (const call of taskCalls(turns)) { | ||
| if (ranShell(call, /check-app/)) | ||
| return call.isError || !says(call, /check passed/); | ||
| if (call.name === CHECK_DESIGN_TOOL) | ||
| return call.isError || !says(call, /check passed/); | ||
| if (call.check) return call.isError || !call.check.checkPassed; | ||
|
TheGreatAxios marked this conversation as resolved.
Resolved
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. P2 · A check-app run that prints no verdict no longer counts as a failed check, so the gate falls back to an older passing check The old gate stopped at any run_shell command that named check-app and treated it as failed unless its output said it passed. Newest first: Resolve this comment. Triage will not finish while it is open. |
||
| if (call.name === "file_design_option") return call.isError; | ||
| if (WRITE_TOOLS.has(call.name) || ranShell(call, SHELL_WRITE)) return true; | ||
| } | ||
|
|
@@ -208,7 +202,7 @@ const filed = (call: TaskCall) => | |
| (call.name === "write_artifact" || | ||
| call.name === "file_package" || | ||
| call.name === "file_design_option" || | ||
| (call.name === CHECK_DESIGN_TOOL && says(call, /Filed design_option:/))); | ||
| call.check?.filed === true); | ||
|
|
||
| // A writer on 2026-10-30 replied with the whole approaches document as text and never filed it; | ||
| // the coordinator waited on a writer that had already answered. A designer's reply also waits for a | ||
|
|
||
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.
P2 · A passing mockup check-app run is recorded as a failed check and blocks the done gate
Any run_shell output with a verdict line gets a CheckDetail. A passing mockup check (
check passed: served .builder/designs ...) therefore gets{checkPassed:false}. The director'suncheckedgate stops at the newest call that has a check detail and returns!checkPassed. So a passing mockup check that only reads files now counts as a newest failing check-app. It hides an earlier passing app check and holds the reply. The old text gate did not treat a passing mockup run as a failure.A coder passes
check-app, then runscheck-app .builder/designsto look at the mockup and replies done. Or a designer checks the mockup with check-app from the shell. The reply is held even though nothing changed and both checks passed.Resolve this comment. Triage will not finish while it is open.