-
Notifications
You must be signed in to change notification settings - Fork 528
[trajectory-grader] Implement exploitation-error #57152
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,113 @@ | ||
| --- | ||
| graders: | ||
| # For runs that left one or more declared objectives unsatisfied but had | ||
| # gathered enough evidence to succeed (observations >= distinctStatesVisited), | ||
| # measures how much of that evidence was never used: unusedObservations / | ||
| # observations, clamped to [0, 1]. An observation is used when it declares a | ||
| # non-empty consumedByActionIds. distinctStatesVisited comes from distinct | ||
| # state_change event refs, falling back to the declared states[] count when | ||
| # no state_change events are recorded. Runs with all objectives satisfied | ||
| # score 0 (no exploitation error to attribute); runs whose exploration was | ||
| # insufficient are not applicable and defer to exploration-error. Lower is | ||
| # better: fewer unmet objectives attributable to unused evidence. | ||
| exploitation-error: | ||
| name: Exploitation Error | ||
| unit: ratio | ||
| direction: lower_is_better | ||
| min: 0.0 | ||
| max: 1.0 | ||
| script: | | ||
| const isRecord = value => value !== null && typeof value === "object" && !Array.isArray(value); | ||
| const candidates = [ | ||
| trace.trajectoryIR, | ||
| trace.trajectoryIr, | ||
| trace.ir, | ||
| isRecord(trace.agentOutput) ? trace.agentOutput.trajectoryIR : null, | ||
| isRecord(trace.agentOutput) ? trace.agentOutput.trajectoryIr : null, | ||
| isRecord(trace.agentOutput) ? trace.agentOutput.trajectory : null, | ||
| isRecord(trace.agentOutput) ? trace.agentOutput : null, | ||
| ].filter(isRecord); | ||
|
|
||
| const candidate = | ||
| candidates.find(value => Array.isArray(value.objectives) && value.objectives.some(isRecord)) ?? | ||
| candidates.find(value => | ||
| Array.isArray(value.objectives) || | ||
| Array.isArray(value.events) || | ||
| Array.isArray(value.states) || | ||
| Array.isArray(value.observations) | ||
| ) ?? | ||
| null; | ||
| const objectives = (candidate && Array.isArray(candidate.objectives) ? candidate.objectives : []).filter(isRecord); | ||
| if (objectives.length === 0) { | ||
| return { value: null, unit: "ratio", passed: null, message: "not applicable: no declared objectives in the trace" }; | ||
| } | ||
|
|
||
| const unmet = objectives.filter(objective => objective.satisfiedAtEventIndex === null || objective.satisfiedAtEventIndex === undefined); | ||
| if (unmet.length === 0) { | ||
| return { value: 0, unit: "ratio", details: `objectives=${objectives.length} unmet=0; all objectives satisfied` }; | ||
| } | ||
|
|
||
| const events = (candidate && Array.isArray(candidate.events) ? candidate.events : []).filter(isRecord); | ||
| const states = (candidate && Array.isArray(candidate.states) ? candidate.states : []).filter(isRecord); | ||
| const observations = (candidate && Array.isArray(candidate.observations) ? candidate.observations : []).filter(isRecord); | ||
|
|
||
| const stateChangeEvents = events.filter(event => event.kind === "state_change"); | ||
| let distinctStatesVisited = 0; | ||
| let source = ""; | ||
| if (stateChangeEvents.length > 0) { | ||
| const visited = new Set(stateChangeEvents.map(event => (typeof event.ref === "string" ? event.ref : JSON.stringify(event.ref)))); | ||
| distinctStatesVisited = visited.size; | ||
| source = "state_change events"; | ||
| } else if (states.length > 0) { | ||
| distinctStatesVisited = states.length; | ||
| source = "declared states[]"; | ||
| } else { | ||
| return { value: null, unit: "ratio", passed: null, message: "not applicable: no state_change events or declared states in the trace" }; | ||
| } | ||
|
|
||
| if (observations.length === 0) { | ||
| return { value: null, unit: "ratio", passed: null, message: "not applicable: no observations in the trace" }; | ||
| } | ||
|
|
||
| if (observations.length < distinctStatesVisited) { | ||
| return { | ||
| value: null, | ||
| unit: "ratio", | ||
| passed: null, | ||
| message: `not applicable: exploration was insufficient (observations=${observations.length} < distinctStatesVisited=${distinctStatesVisited}); see exploration-error`, | ||
| }; | ||
| } | ||
|
|
||
| const unused = observations.filter( | ||
| observation => !Array.isArray(observation.consumedByActionIds) || observation.consumedByActionIds.length === 0 | ||
| ); | ||
| const value = helpers.clamp(unused.length / observations.length, 0, 1); | ||
| const unmetDescriptions = unmet.slice(0, 5).map(objective => (typeof objective.id === "string" && objective.id !== "" ? objective.id : objective.description)); | ||
|
|
||
| return { | ||
|
Contributor
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. [/tdd] 💡 Suggested fixconst unmetDescriptions = unmet.slice(0, 5).map(objective =>
(typeof objective.id === "string" && objective.id !== "" ? objective.id :
typeof objective.description === "string" && objective.description !== "" ? objective.description :
"(unnamed)")
);No test currently exercises this path. A companion test with @copilot please address this. |
||
| value, | ||
| unit: "ratio", | ||
| details: `objectives=${objectives.length} unmet=${unmet.length} observations=${observations.length} unused=${unused.length} distinctStatesVisited=${distinctStatesVisited} (from ${source})${unmetDescriptions.length === 0 ? "" : `; unmet objectives: ${unmetDescriptions.join(", ")}`}`, | ||
|
Contributor
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.
If an objective object has neither a truthy string Consider a string-safe fallback: const unmetDescriptions = unmet.slice(0, 5).map(objective => {
if (typeof objective.id === 'string' && objective.id !== '') return objective.id;
if (typeof objective.description === 'string' && objective.description !== '') return objective.description;
return '(unnamed)';
});@copilot please address this. |
||
| }; | ||
| --- | ||
|
|
||
| <!-- | ||
| exploitation-error attributes objective failure to unused evidence rather | ||
| than insufficient search. For runs that left one or more declared objectives | ||
| unsatisfied (satisfiedAtEventIndex null/undefined) *and* gathered at least as | ||
| many observations as the number of distinct states visited, it computes | ||
| unusedObservations / observations, clamped to [0, 1]. An observation counts as | ||
| used when it declares a non-empty consumedByActionIds -- i.e. some later action | ||
| consumed it. A high score means the run had the evidence it needed but never | ||
| acted on most of it. distinctStatesVisited is the count of distinct refs across | ||
| events[] of kind "state_change"; when no such events are recorded it falls back | ||
| to the declared states[] count. Runs with all objectives satisfied score 0 -- | ||
| there is no exploitation error to attribute, since exploitation failures only | ||
| apply to failed runs. This is the complement of exploration-error: when | ||
| observations < distinctStatesVisited the failure is an insufficient-search | ||
| failure, so this grader reports not-applicable (passed: null) and defers to | ||
| exploration-error, making the two mutually exclusive on the same trace. | ||
| Not-applicable is also reported when no objectives are declared, when neither | ||
| state_change events nor declared states are present, and when the trace records | ||
| no observations. | ||
| --> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -104,6 +104,24 @@ function runExplorationError(trace) { | |
| }); | ||
| } | ||
|
|
||
| const exploitationErrorScriptMatch = fs.readFileSync(path.join(__dirname, "../../../.github/workflows/shared/graders/exploitation-error.md"), "utf8").match(/script: \|\n([\s\S]*?)\n^---\s*$/m); | ||
| if (!exploitationErrorScriptMatch?.[1]) { | ||
| throw new Error("unable to extract exploitation-error grader script"); | ||
| } | ||
| const exploitationErrorScript = exploitationErrorScriptMatch[1] | ||
| .split("\n") | ||
| .map(line => line.slice(6)) | ||
| .join("\n"); | ||
|
|
||
| function runExploitationError(trace) { | ||
| return runCustomGrader("exploitation-error", exploitationErrorScript, makeTrace(trace), { | ||
| name: "Exploitation Error", | ||
| unit: "ratio", | ||
| direction: "lower_is_better", | ||
| source: "inline", | ||
| }); | ||
| } | ||
|
|
||
| const skillConstraintCoverageScriptMatch = fs.readFileSync(path.join(__dirname, "../../../.github/workflows/shared/graders/skill-constraint-coverage.md"), "utf8").match(/script: \|\n([\s\S]*?)\n^---\s*$/m); | ||
| if (!skillConstraintCoverageScriptMatch?.[1]) { | ||
| throw new Error("unable to extract skill-constraint-coverage grader script"); | ||
|
|
@@ -757,6 +775,121 @@ describe("trace_graders", () => { | |
| }); | ||
|
Contributor
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. L775-890: shrink: eight near-duplicate cases for one grader. Two smoke tests plus a table-driven case would cover the same branches with less noise. |
||
| }); | ||
|
|
||
| describe("exploitation-error custom grader", () => { | ||
| it("reports unavailable when no objectives are declared", () => { | ||
| const result = runExploitationError({ trajectoryIR: { observations: [{ id: "obs-1" }] } }); | ||
|
|
||
| expect(result.value).toBeNull(); | ||
| expect(result.passed).toBeNull(); | ||
| expect(result.message).toContain("no declared objectives"); | ||
| }); | ||
|
|
||
| it("returns zero when all objectives are already satisfied", () => { | ||
| const result = runExploitationError({ | ||
| trajectoryIR: { | ||
| objectives: [{ id: "goal", description: "Read all files", satisfiedAtEventIndex: 0 }], | ||
| }, | ||
| }); | ||
|
|
||
| expect(result.value).toBe(0); | ||
| expect(result.details).toContain("all objectives satisfied"); | ||
| }); | ||
|
|
||
| it("is unavailable without state_change events or declared states", () => { | ||
| const result = runExploitationError({ | ||
| trajectoryIR: { | ||
| objectives: [{ id: "goal", description: "Inspect repo", satisfiedAtEventIndex: null }], | ||
| observations: [{ id: "obs-1" }], | ||
| }, | ||
| }); | ||
|
|
||
| expect(result.value).toBeNull(); | ||
| expect(result.passed).toBeNull(); | ||
| expect(result.message).toContain("no state_change events or declared states"); | ||
| }); | ||
|
|
||
| it("is unavailable when the trace records no observations", () => { | ||
| const result = runExploitationError({ | ||
| trajectoryIR: { | ||
| objectives: [{ id: "goal", description: "Inspect repo", satisfiedAtEventIndex: null }], | ||
| states: [{ id: "a" }], | ||
| }, | ||
| }); | ||
|
|
||
| expect(result.value).toBeNull(); | ||
| expect(result.passed).toBeNull(); | ||
| expect(result.message).toContain("no observations"); | ||
| }); | ||
|
|
||
| it("defers to exploration-error when exploration was insufficient", () => { | ||
| const result = runExploitationError({ | ||
| trajectoryIR: { | ||
| objectives: [{ id: "goal", description: "Inspect repo", satisfiedAtEventIndex: null }], | ||
| events: [ | ||
| { kind: "state_change", ref: "repo-root" }, | ||
| { kind: "state_change", ref: "repo-readme" }, | ||
|
Contributor
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. [/tdd] Missing boundary test: 💡 Suggested testit("scores when observations exactly equal distinctStatesVisited (boundary)", () => {
const result = runExploitationError({
trajectoryIR: {
objectives: [{ id: "goal", description: "Inspect repo", satisfiedAtEventIndex: null }],
events: [{ kind: "state_change", ref: "s1" }],
observations: [{ id: "obs-1", consumedByActionIds: [] }],
},
});
// observations (1) === distinctStatesVisited (1): should score, not defer
expect(result.value).toBe(1);
expect(result.details).toContain("observations=1 unused=1");
});The @copilot please address this. |
||
| ], | ||
| observations: [{ id: "obs-1", consumedByActionIds: ["act-1"] }], | ||
| }, | ||
| }); | ||
|
|
||
| expect(result.value).toBeNull(); | ||
| expect(result.passed).toBeNull(); | ||
| expect(result.message).toContain("exploration was insufficient"); | ||
| expect(result.message).toContain("exploration-error"); | ||
| }); | ||
|
|
||
| it("scores the unused fraction of observations when exploration was sufficient", () => { | ||
| const result = runExploitationError({ | ||
|
Contributor
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. [/tdd] The duplicate 💡 SuggestionAdd an inline comment explaining the intent: // Two events with the same ref → Set deduplication → distinctStatesVisited=1
// so observations(2) > distinctStatesVisited(1): exploitation-error applies.
events: [
{ kind: "state_change", ref: "repo-root" },
{ kind: "state_change", ref: "repo-root" },
],@copilot please address this. |
||
| trajectoryIR: { | ||
| objectives: [{ id: "goal", description: "Inspect repo", satisfiedAtEventIndex: null }], | ||
| events: [ | ||
| { kind: "state_change", ref: "repo-root" }, | ||
| { kind: "state_change", ref: "repo-root" }, | ||
| ], | ||
| observations: [ | ||
| { id: "obs-1", consumedByActionIds: ["act-1"] }, | ||
| { id: "obs-2", consumedByActionIds: [] }, | ||
| ], | ||
| }, | ||
| }); | ||
|
|
||
| expect(result.value).toBeCloseTo(0.5); | ||
| expect(result.details).toContain("observations=2 unused=1"); | ||
| expect(result.details).toContain("distinctStatesVisited=1"); | ||
| expect(result.details).toContain("unmet objectives: goal"); | ||
| }); | ||
|
|
||
| it("falls back to declared states[] when no state_change events exist", () => { | ||
| const result = runExploitationError({ | ||
| trajectoryIR: { | ||
| objectives: [{ id: "goal", description: "Inspect repo", satisfiedAtEventIndex: null }], | ||
| states: [{ id: "a" }, { id: "b" }], | ||
| observations: [{ id: "obs-1" }, { id: "obs-2" }], | ||
| }, | ||
| }); | ||
|
|
||
| expect(result.value).toBe(1); | ||
| expect(result.details).toContain("from declared states[]"); | ||
| }); | ||
|
|
||
| it("prefers the objective-bearing IR candidate over unrelated agentOutput observations", () => { | ||
| const result = runExploitationError({ | ||
| trajectoryIR: { | ||
| objectives: [{ id: "goal", description: "Inspect repo", satisfiedAtEventIndex: null }], | ||
| states: [{ id: "a" }], | ||
| observations: [{ id: "obs-1", consumedByActionIds: ["act-1"] }], | ||
| }, | ||
| agentOutput: { | ||
| observations: [{ id: "obs-x" }], | ||
| }, | ||
| }); | ||
|
|
||
| expect(result.value).toBe(0); | ||
| expect(result.details).toContain("observations=1 unused=0"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("skill-constraint-coverage custom grader", () => { | ||
| it("reports full coverage when every constraint is exercised and succeeds", () => { | ||
| const result = runSkillConstraintCoverage( | ||
|
|
||
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.
L20-39: yagni: multi-branch trace-shape fallback for
trajectoryIR,trajectoryIr,ir, andagentOutputvariants. Use one canonicaltrajectoryIRpayload and let the producer emit it.