Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions actions/setup/js/run_evals.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -323,16 +323,19 @@ function extractAssistantTextFromJsonlLog(logContent) {
} catch {
continue;
}
// v3 schema: turn_end carries the complete assistant message
if (obj.type === "turn_end" && obj.message && Array.isArray(obj.message.content)) {
// v3 schema: turn_end carries the complete assistant message.
// Claude engine's native stream-json format also emits a top-level
// "assistant" event with the same nested message.content array shape
// (e.g. `{"type":"assistant","message":{"content":[{"type":"text","text":...}]}}`),
// so both are handled identically here.
if ((obj.type === "turn_end" || obj.type === "assistant") && obj.message && Array.isArray(obj.message.content)) {
for (const part of obj.message.content) {
if (part && typeof part.text === "string") {
texts.push(part.text);
}
}
}
// v1 legacy schema: assistant event carries raw text content
if (obj.type === "assistant" && typeof obj.content === "string" && obj.content) {
// v1 legacy schema: assistant event carries raw text content directly
} else if (obj.type === "assistant" && typeof obj.content === "string" && obj.content) {
texts.push(obj.content);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/diagnosing-bugs] The two if branches are independent, not else if — an assistant event with both message.content array and a flat obj.content string would push text twice. In practice this shape doesn't appear, but using else if makes the mutual-exclusivity explicit and defends against unexpected future payloads.

💡 Suggested fix
if ((obj.type === "turn_end" || obj.type === "assistant") && obj.message && Array.isArray(obj.message.content)) {
  for (const part of obj.message.content) {
    if (part && typeof part.text === "string") {
      texts.push(part.text);
    }
  }
} else if (obj.type === "assistant" && typeof obj.content === "string" && obj.content) {
  texts.push(obj.content);
}

Using else if documents the intent that these two shapes are mutually exclusive and prevents double-counting if a future schema change produces an object with both fields.

@copilot please address this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied in 7147347 — changed to else if so the two branches are mutually exclusive.

}
Expand Down
49 changes: 49 additions & 0 deletions actions/setup/js/run_evals.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,45 @@ describe("run_evals.cjs", () => {
expect(JSON.parse(line).answer).toBe("YES");
});

it("parses multiple ID-based answers from Claude engine's native assistant JSONL event", async () => {
// Regression test: Claude's stream-json format emits a top-level "assistant" event
// with a nested message.content array (not the v1 legacy plain-string content, nor
// the v3 turn_end wrapper). Previously this shape was not decoded, so the raw
// un-decoded JSON (with literal "\n" escape sequences) was searched instead,
// breaking \b word-boundary matching for any question ID following an embedded
// newline and causing spurious UNKNOWN answers.
vi.stubEnv(
"GH_AW_EVALS_QUESTIONS",
JSON.stringify([
{ id: "adr-check-performed", question: "Checked?" },
{ id: "action-taken", question: "Action taken?" },
{ id: "decision-justified", question: "Justified?" },
])
);
vi.stubEnv("GH_AW_EVALS_MODEL", "small");
vi.stubEnv("GITHUB_RUN_ID", "999");

const assistantEvent = JSON.stringify({
type: "assistant",
message: {
model: "claude-sonnet-4-6",
role: "assistant",
content: [{ type: "text", text: "adr-check-performed: NO\naction-taken: YES\ndecision-justified: YES" }],
},
});
fs.writeFileSync(EVALS_LOG_PATH, assistantEvent + "\n", "utf8");

await parseMain();

const lines = fs.readFileSync(EVALS_OUTPUT_PATH, "utf8").trim().split("\n");
const results = Object.fromEntries(lines.map(l => [JSON.parse(l).id, JSON.parse(l).answer]));
expect(results).toEqual({
"adr-check-performed": "NO",
"action-taken": "YES",
"decision-justified": "YES",
});
});

it('keeps missing answers as "UNKNOWN"', async () => {
vi.stubEnv("GH_AW_EVALS_QUESTIONS", JSON.stringify([{ id: "labels-applied", question: "Did labels get applied?" }]));
vi.stubEnv("GH_AW_EVALS_MODEL", "small");
Expand Down Expand Up @@ -209,6 +248,16 @@ describe("run_evals.cjs", () => {
expect(extractAssistantTextFromJsonlLog(log)).toBe("Q1: YES");
});

it("extracts text from Claude engine's native assistant events (message.content array)", () => {
// Claude's stream-json format emits `{"type":"assistant","message":{"content":[...]}}`,
// the same nested shape as turn_end but under the "assistant" type name.
const log = JSON.stringify({
type: "assistant",
message: { model: "claude-sonnet-4-6", role: "assistant", content: [{ type: "text", text: "adr-check-performed: NO\naction-taken: YES\ndecision-justified: YES" }] },
});
expect(extractAssistantTextFromJsonlLog(log)).toBe("adr-check-performed: NO\naction-taken: YES\ndecision-justified: YES");
});

it("joins multiple assistant messages with newlines", () => {
const lines = [JSON.stringify({ type: "assistant", content: "Q1: YES" }), JSON.stringify({ type: "assistant", content: "Q2: NO" })].join("\n");
expect(extractAssistantTextFromJsonlLog(lines)).toBe("Q1: YES\nQ2: NO");
Expand Down
Loading