Skip to content

Commit 44f3174

Browse files
DylanMerigaudclaude
andcommitted
Fix the edit model emitting a bad integration op + harden the edit eval
Two real bugs the edit eval surfaced (it was crashing before, so they were hidden): 1. add-integration: the prompt described the action but never named the field, so the model emitted `{op:"add-integration", kind:"slack"}` — the schema wants `integration`, so parseEditOp threw and killed the whole run (and would throw in the live app too). Name the field explicitly in the prompt ("integration", NOT "kind"). Verified: 6/6 slack/jira replies now carry `integration`. 2. The eval aborted on the first un-parseable reply, hiding the other 9 cases. Catch per case: a bad op is a FAILURE for that case, not a crash. Also: add the user's exact phrasing "Require CFO approval for all invoices over $50k" as a case (must map to add-approval, a NEW gate, not set-threshold on the existing director), and fix the remove-co-approver case, its fixture had no co-approver to remove (the model rightly returned "none"), so the director gate now seeds Jordan Ellis and the add case adds a different person. Live eval: 11/11. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent e02e050 commit 44f3174

3 files changed

Lines changed: 35 additions & 10 deletions

File tree

‎eval/edit-cases.ts‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ export const EDIT_FIXTURE: ApprovalWorkflow = {
3131
when: { kind: "leaf", field: "amount", op: ">", value: 10000 },
3232
approverTitle: "Director",
3333
approverName: "Cameron Diaz",
34+
// Jordan Ellis is a co-approver here so the remove-co-approver case has a real
35+
// one to drop; the add-co-approver case adds a DIFFERENT person (Taylor Nguyen).
36+
approvers: ["Jordan Ellis"],
3437
next: ["post-netsuite"],
3538
},
3639
{
@@ -74,6 +77,16 @@ export const EDIT_CASES: EditCase[] = [
7477
/cfo|chief financial/i.test(op.approverTitle),
7578
why: "a new gate above a NEW threshold (50k ≠ the existing 10k director)",
7679
},
80+
{
81+
id: "require-cfo-over",
82+
instruction: "Require CFO approval for all invoices over $50k",
83+
expectedOp: "add-approval",
84+
check: (op) =>
85+
op.op === "add-approval" &&
86+
op.amountOver === 50000 &&
87+
/cfo|chief financial/i.test(op.approverTitle),
88+
why: "a NEW CFO gate, not raising the existing director's threshold (the phrasing without 'also' must still add, not mutate)",
89+
},
7790
{
7891
id: "add-slack",
7992
instruction: "Send a Slack message whenever an invoice is posted",
@@ -108,13 +121,13 @@ export const EDIT_CASES: EditCase[] = [
108121
{
109122
id: "add-director-co-approver",
110123
instruction:
111-
"The director review should also require Jordan Ellis to sign off",
124+
"The director review should also require Taylor Nguyen to sign off",
112125
expectedOp: "add-approver",
113126
check: (op) =>
114127
op.op === "add-approver" &&
115128
op.stepId === "director-review" &&
116-
/jordan ellis/i.test(op.approverName),
117-
why: "ADD a co-approver to the director gate (keep Cameron Diaz), not replace the approver",
129+
/taylor nguyen/i.test(op.approverName),
130+
why: "ADD a co-approver to the director gate (keep Cameron Diaz + Jordan Ellis), not replace the approver",
118131
},
119132
{
120133
id: "remove-director-co-approver",

‎eval/edit-run.ts‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,23 @@ const main = async (): Promise<void> => {
6262

6363
const results: Result[] = [];
6464
for (const c of EDIT_CASES) {
65-
const op =
66-
dryRun || !planEdit
67-
? stubOp(c)
68-
: await planEdit(EDIT_FIXTURE, c.instruction);
69-
results.push(scoreOne(c, op));
65+
// A model reply that fails to parse (bad JSON, or an op the schema rejects) is a
66+
// FAILURE for that case, not a crash of the whole run, so one flaky reply doesn't
67+
// hide the other nine cases' results.
68+
try {
69+
const op =
70+
dryRun || !planEdit
71+
? stubOp(c)
72+
: await planEdit(EDIT_FIXTURE, c.instruction);
73+
results.push(scoreOne(c, op));
74+
} catch (err) {
75+
results.push({
76+
id: c.id,
77+
pass: false,
78+
got: "error",
79+
why: `${c.why}: ${err instanceof Error ? err.message : String(err)}`,
80+
});
81+
}
7082
}
7183

7284
const idW = Math.max(8, ...results.map((r) => r.id.length));
@@ -127,7 +139,7 @@ const stubOp = (c: EditCase): WorkflowEditOp => {
127139
return {
128140
op: "add-approver",
129141
stepId: "director-review",
130-
approverName: "Jordan Ellis",
142+
approverName: "Taylor Nguyen",
131143
};
132144
case "remove-approver":
133145
return {

‎lib/workflow-edit.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -651,7 +651,7 @@ export const proposeEdit = async (
651651
export const WORKFLOW_EDIT_SYSTEM_PROMPT = `You translate a plain-language instruction into ONE structured edit for a procure-to-pay approval workflow. You are given the current workflow's steps (id, label, kind, approver) and the instruction. Return a single edit op:
652652
653653
- add-approval: a new human approval gate. Set "label" to a SHORT title only (e.g. "CFO review" or "VP sign-off"), do NOT put the threshold or department in the label, they're shown separately. Set "approverTitle" (the role, e.g. "CFO"), "amountOver" (the dollar threshold it applies above, or null for every invoice), and "department" (scope to one department like "Product", or null for any).
654-
- add-integration: a system action, "slack", "jira", or "netsuite", that runs after the bill posts. Set "label" to a short title (e.g. "Notify on Slack", "Open Jira ticket").
654+
- add-integration: a system action that runs after the bill posts. Set "integration" to exactly one of "slack" | "jira" | "netsuite" (the field is named "integration", NOT "kind"), and "label" to a short title (e.g. "Notify on Slack", "Open Jira ticket").
655655
- set-threshold: change an existing approval step's amount threshold. Use the step's "stepId" from the current workflow.
656656
- set-approver: set the PRIMARY person on an existing approval step (by "stepId"). Use for "make X the approver" / "assign X".
657657
- add-approver: add ANOTHER required approver to a gate that already has one (by "stepId" + "approverName"). Use for "also require X" / "add X as a co-approver" / "X should sign off too". The gate keeps its existing approver and also routes to this person.

0 commit comments

Comments
 (0)