Repository navigation
feat: reject an under-asked answer_commit once, naming the missing item
#1553
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
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
215b068
feat: reject an under-asked `answer_commit` once, naming the missing …
anandgupta42 e507249
refactor: consume `answer-commit-conjunction` through its namespace p…
anandgupta42 2dfb483
test: isolate `warehouse_list` hint tests from the shared global conn…
anandgupta42 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
135 changes: 135 additions & 0 deletions
135
packages/opencode/src/altimate/tools/answer-commit-conjunction.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,135 @@ | ||
| import type { Tool } from "../../tool/tool" | ||
| import { RunBudget } from "../run-budget" | ||
| import { effortOverride, toolsDisabled } from "../session/run-budget-effort" | ||
|
|
||
| /** Identifies this check's rejections in the current turn's history. */ | ||
| export const CONJUNCTION_REJECTION_MARKER = "answer_commit conjunction check:" | ||
|
|
||
| // Narrow connectives strongly suggest another requested item. | ||
| const NARROW = [ | ||
| /\band (?:its|their|his|her|the corresponding|the associated)\b/gi, | ||
| /\balong with\b/gi, | ||
| /\bas well as\b/gi, | ||
| /\btogether with\b/gi, | ||
| /\band also\b/gi, | ||
| /\bin addition to\b/gi, | ||
| /\band (?:report|list|show|include|provide|return|give|compute|calculate|identify|find)\b/gi, | ||
| ] | ||
| // Articles and comma-separated conjunctions catch less explicit item lists. | ||
| const BROAD_EXTRA = [/,\s+and\s+(?!then\b)/gi, /\band (?:the|a|an|which|what|how|whether)\b/gi] | ||
| // Ranges and paired expressions do not introduce requested items. | ||
| const EXCL = /(?:between|both|either|from)\s+[^.,;]{0,60}?\band\b/gi | ||
| // Constraints qualify an item rather than request another one. | ||
| const CONSTRAINT = | ||
| /\band (?:only|also consider|consider|excluding|exclude|limit|using|ensure|ensuring|where|for|in|on|at|from|with|by|to|if)\b/gi | ||
| /** Words of the request quoted after each connective, to name the item it introduces. */ | ||
| const JOIN_WORDS = 8 | ||
|
|
||
| /** | ||
| * Lexical estimate of the items a request asks for: 1 plus the most item connectives in any sentence | ||
| * of its first paragraph, ignoring ranges, constraint clauses and rounding instructions. `joins` quotes | ||
| * each counted connective with the words that follow it, naming the extra items. | ||
| */ | ||
| export function requestedItems(request: string): { count: number; joins: string[] } { | ||
| const para = request | ||
| .trim() | ||
| .split(/\n\s*\n|\n#/)[0]! | ||
| .replace(EXCL, " ") | ||
| let joins: string[] = [] | ||
| for (const sentence of para.split(/(?<=[.?!])\s+/)) { | ||
| if (/\b(decimal|precision|round(?:ed|ing)?)\b/i.test(sentence) && !/\band\b/i.test(sentence)) continue | ||
|
anandgupta42 marked this conversation as resolved.
|
||
| const text = sentence.replace(CONSTRAINT, " ") | ||
| const spans: number[] = [] | ||
| for (const pattern of [...NARROW, ...BROAD_EXTRA]) { | ||
| for (const match of text.matchAll(pattern)) { | ||
| if (!spans.some((start) => Math.abs(match.index - start) < 4)) spans.push(match.index) | ||
|
anandgupta42 marked this conversation as resolved.
|
||
| } | ||
| } | ||
| if (spans.length > joins.length) | ||
| joins = spans | ||
| .sort((a, b) => a - b) | ||
| .map((start) => | ||
| text | ||
| .slice(start) | ||
| .replace(/^,\s*/, "") | ||
| .split(/[.,;?!]/)[0]! | ||
| .split(/\s+/) | ||
| .filter(Boolean) | ||
| .slice(0, JOIN_WORDS) | ||
| .join(" "), | ||
| ) | ||
| } | ||
| return { count: 1 + joins.length, joins } | ||
| } | ||
|
|
||
| type Part = { | ||
| type?: string | ||
| tool?: string | ||
| text?: string | ||
| synthetic?: boolean | ||
| state?: { status?: string; error?: unknown } | ||
| } | ||
|
|
||
| /** Text of the latest user message, without synthetic parts such as time-budget reminders. */ | ||
| function latestRequest(messages: Tool.Context["messages"]): string { | ||
| const latest = messages.findLast((message) => message.info.role === "user") | ||
| return ((latest?.parts ?? []) as Part[]) | ||
| .filter((part) => part.type === "text" && !part.synthetic) | ||
|
anandgupta42 marked this conversation as resolved.
anandgupta42 marked this conversation as resolved.
anandgupta42 marked this conversation as resolved.
|
||
| .map((part) => part.text ?? "") | ||
| .join("\n\n") | ||
|
anandgupta42 marked this conversation as resolved.
|
||
| } | ||
|
|
||
| /** Bounds the in-process record of advised requests; the oldest entries are dropped first. */ | ||
| const MAX_ADVISED_REQUESTS = 1_000 | ||
|
anandgupta42 marked this conversation as resolved.
|
||
| /** | ||
| * Session and request text already advised in this process. Reserved synchronously before the rejection | ||
| * returns, so parallel commits in one step and a compaction replay of the same request are not rejected again. | ||
| */ | ||
| const advised = new Set<string>() | ||
|
|
||
| /** Near the run's time budget a rejected commit may not get another turn, so it is never rejected. */ | ||
| function budgetEnding(sessionID: string): boolean { | ||
| const deadline = RunBudget.queryDeadline(sessionID) | ||
| return toolsDisabled(sessionID) || effortOverride(sessionID) || (deadline !== undefined && Date.now() >= deadline) | ||
| } | ||
|
|
||
| /** | ||
| * Rejects a fields commit with fewer fields than the items the latest user request joins. Fires at most | ||
| * once per session and request, so the next commit, changed or not, is accepted, and never once the run | ||
| * nears its time budget. Purely lexical: no queries or model calls. | ||
| */ | ||
| export function conjunctionRejection( | ||
| args: { fields?: { name: string }[] }, | ||
| messages: Tool.Context["messages"], | ||
| sessionID: string, | ||
| ): string | undefined { | ||
| if (!args.fields || budgetEnding(sessionID)) return undefined | ||
|
anandgupta42 marked this conversation as resolved.
|
||
| const text = latestRequest(messages) | ||
| const { count, joins } = requestedItems(text) | ||
| if (count <= args.fields.length) return undefined | ||
| const key = `${sessionID}\u0000${text}` | ||
|
anandgupta42 marked this conversation as resolved.
|
||
| if (advised.has(key)) return undefined | ||
|
anandgupta42 marked this conversation as resolved.
|
||
| const latest = messages.findLastIndex((message) => message.info.role === "user") | ||
| // History covers a resumed session whose in-process record is gone. | ||
| if ( | ||
| messages | ||
| .slice(latest) | ||
| .some((message) => | ||
| (message.parts as Part[]).some( | ||
| (part) => | ||
| part.type === "tool" && | ||
| part.tool === "answer_commit" && | ||
| part.state?.status === "error" && | ||
| String(part.state.error).includes(CONJUNCTION_REJECTION_MARKER), | ||
| ), | ||
| ) | ||
| ) | ||
| return undefined | ||
| advised.add(key) | ||
| if (advised.size > MAX_ADVISED_REQUESTS) advised.delete(advised.values().next().value!) | ||
| const names = args.fields.map((field) => field.name) | ||
| const fields = `${names.length} field${names.length === 1 ? "" : "s"}` | ||
| return `${CONJUNCTION_REJECTION_MARKER} the request appears to ask for ${count} items, but this commit has ${fields} (${JSON.stringify(names)}). The request also asks for ${joins.map((join) => `"${join}"`).join(", ")}. Call answer_commit again with each requested item as its own field, named after it. If the request asks for fewer items, recommit unchanged; the next commit is accepted.` | ||
|
anandgupta42 marked this conversation as resolved.
anandgupta42 marked this conversation as resolved.
|
||
| } | ||
|
|
||
| export * as AnswerCommitConjunction from "./answer-commit-conjunction" | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
185 changes: 185 additions & 0 deletions
185
packages/opencode/test/altimate/tools/answer-commit-conjunction.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,185 @@ | ||
| import { describe, expect, test } from "bun:test" | ||
| import { RunBudget } from "../../../src/altimate/run-budget" | ||
| import { begin, downgradeEffort } from "../../../src/altimate/session/run-budget-effort" | ||
| import { AnswerCommitTool, renderAnswer } from "../../../src/altimate/tools/answer-commit" | ||
| import { AnswerCommitConjunction } from "../../../src/altimate/tools/answer-commit-conjunction" | ||
| const { CONJUNCTION_REJECTION_MARKER, requestedItems } = AnswerCommitConjunction | ||
| import type { Tool } from "../../../src/tool/tool" | ||
| import { initTool } from "../tool-fixture" | ||
|
|
||
| const singles = [ | ||
| "Which repository has the most stars?", | ||
| "How many films were released between 2010 and 2015?", | ||
| "Which artist sold the most Rock and Roll albums?", | ||
| "Which artist sold the most rock and roll albums?", | ||
| "Which customer from Trinidad and Tobago spent the most?", | ||
| "List the top product, and only consider active stores.", | ||
| ] | ||
| const compound = "Which repository and its most copied file?" | ||
| const basis = "Selected repositories at repository grain from repositories.name." | ||
| const payload = { basis, fields: [{ name: "repository", value: "repo-a" }] } | ||
| const complete = { basis, fields: [...payload.fields, { name: "most copied file", value: "main.swift" }] } | ||
| function user(text: string) { | ||
| return { info: { role: "user" }, parts: [{ type: "text", text }] } | ||
| } | ||
| function rejectedCommit(error: string) { | ||
| return { | ||
| info: { role: "assistant" }, | ||
| parts: [{ type: "tool", tool: "answer_commit", state: { status: "error", error } }], | ||
| } | ||
| } | ||
| // The rejection is recorded per session, so each call gets a fresh session unless one is shared. | ||
| function context(messages: unknown[] = [], sessionID: string = crypto.randomUUID()) { | ||
| return { | ||
| sessionID, | ||
| messageID: "test-message", | ||
| agent: "analyst", | ||
| abort: new AbortController().signal, | ||
| messages: messages as Tool.Context["messages"], | ||
| ask: () => {}, | ||
| } | ||
| } | ||
|
|
||
| describe("requestedItems", () => { | ||
| test.each(singles)("counts one item: %s", (request) => { | ||
| expect(requestedItems(request)).toEqual({ count: 1, joins: [] }) | ||
| }) | ||
| test("counts and quotes the connective that introduces each extra item", () => { | ||
| expect(requestedItems(compound)).toEqual({ count: 2, joins: ["and its most copied file"] }) | ||
| expect(requestedItems("Identify the repository and the most copied file.").joins).toEqual([ | ||
| "and the most copied file", | ||
| ]) | ||
| expect(requestedItems("Which repository contains Swift code and its most copied file?").count).toBe(2) | ||
| }) | ||
| test("takes the sentence with the most items and ignores later paragraphs", () => { | ||
| const request = | ||
| "Find the store with the most sales, along with its city, and the manager's name. Round to 2 decimals.\n\nThe schema has a stores table and a sales table." | ||
| expect(requestedItems(request)).toEqual({ count: 3, joins: ["along with its city", "and the manager's name"] }) | ||
| }) | ||
| }) | ||
|
|
||
| describe("answer_commit conjunction check", () => { | ||
| test("rejects an under-asked fields commit once, naming the missing item; an unchanged recommit is accepted", async () => { | ||
| const tool = await initTool(AnswerCommitTool) | ||
| const session = crypto.randomUUID() | ||
| const first = await tool.execute(payload, context([user(compound)], session)) | ||
| expect(first.status).toBe("error") | ||
| expect(first.title).toBe("Answer not committed") | ||
| expect(first.output.startsWith(CONJUNCTION_REJECTION_MARKER)).toBe(true) | ||
| expect(first.output).toContain('"and its most copied file"') | ||
| expect(first.output).toContain('["repository"]') | ||
| expect(first.metadata).not.toHaveProperty("committed_text") | ||
| expect(first.metadata).toHaveProperty("failure_kind", "conjunction_check") | ||
| const second = await tool.execute(payload, context([user(compound), rejectedCommit(first.output)], session)) | ||
| expect(second.status).not.toBe("error") | ||
| expect(second.output).toBe(renderAnswer(payload)) | ||
| expect(second.metadata).toHaveProperty("committed_text", renderAnswer(payload)) | ||
| }) | ||
| test("the rejection in history alone allows the recommit, as after a restart", async () => { | ||
| const tool = await initTool(AnswerCommitTool) | ||
| const first = await tool.execute(payload, context([user(compound)])) | ||
| const resumed = await tool.execute(payload, context([user(compound), rejectedCommit(first.output)])) | ||
| expect(resumed.output).toBe(renderAnswer(payload)) | ||
| // A rejection before the latest request does not cover the new request. | ||
| const next = "Which author and their most cited paper?" | ||
| const later = await tool.execute(payload, context([user(compound), rejectedCommit(first.output), user(next)])) | ||
| expect(later.status).toBe("error") | ||
| }) | ||
| test("rejects once per session and request without history of the rejection", async () => { | ||
| const tool = await initTool(AnswerCommitTool) | ||
| const session = crypto.randomUUID() | ||
| // Parallel commits in one step share the same history snapshot. | ||
| const [first, second] = await Promise.all([ | ||
| tool.execute(payload, context([user(compound)], session)), | ||
| tool.execute(payload, context([user(compound)], session)), | ||
| ]) | ||
| expect([first.status, second.status].filter((status) => status === "error")).toHaveLength(1) | ||
| // A compaction replay repeats the request as a new user message without the earlier rejection. | ||
| expect((await tool.execute(payload, context([user(compound)], session))).output).toBe(renderAnswer(payload)) | ||
| expect((await tool.execute(payload, context([user(compound)]))).status).toBe("error") | ||
| }) | ||
| test("answer_only commits are checked too", async () => { | ||
| const tool = await initTool(AnswerCommitTool) | ||
| const result = await tool.execute( | ||
| { ...payload, answer_only: true }, | ||
| context([user("Identify the repository and the most copied file.")]), | ||
| ) | ||
| expect(result.status).toBe("error") | ||
| }) | ||
| test("two fields for a three-item request are rejected; three are accepted", async () => { | ||
| const tool = await initTool(AnswerCommitTool) | ||
| const request = "Find the store with the most sales, along with its city, and the manager's name." | ||
| expect((await tool.execute(complete, context([user(request)]))).status).toBe("error") | ||
| const three = { basis, fields: [...complete.fields, { name: "manager", value: "Ana" }] } | ||
| expect((await tool.execute(three, context([user(request)]))).output).toBe(renderAnswer(three)) | ||
| }) | ||
| test.each(singles)("accepts a single-item request: %s", async (request) => { | ||
| const tool = await initTool(AnswerCommitTool) | ||
| expect((await tool.execute(payload, context([user(request)]))).output).toBe(renderAnswer(payload)) | ||
| }) | ||
| test("accepts complete fields, rows, unresolved and commits without a user message", async () => { | ||
| const tool = await initTool(AnswerCommitTool) | ||
| expect((await tool.execute(complete, context([user(compound)]))).output).toBe(renderAnswer(complete)) | ||
| const rows = { basis, rows: { columns: ["repository"], values: [["repo-a"]] } } | ||
| expect((await tool.execute(rows, context([user(compound)]))).output).toBe(renderAnswer(rows)) | ||
| const unresolved = { basis, unresolved: { reason: "No matching records." } } | ||
| expect((await tool.execute(unresolved, context([user(compound)]))).output).toBe(renderAnswer(unresolved)) | ||
| expect((await tool.execute(payload, context())).output).toBe(renderAnswer(payload)) | ||
| }) | ||
| }) | ||
|
|
||
| describe("near the time budget a commit is never rejected", () => { | ||
| test("after the query deadline", async () => { | ||
| const tool = await initTool(AnswerCommitTool) | ||
| const session = crypto.randomUUID() | ||
| const release = RunBudget.registerQueryDeadline(session, Date.now() - 1) | ||
| try { | ||
| expect((await tool.execute(payload, context([user(compound)], session))).output).toBe(renderAnswer(payload)) | ||
| } finally { | ||
| release() | ||
| } | ||
| // The same session is checked again once no deadline applies. | ||
| expect((await tool.execute(payload, context([user(compound)], session))).status).toBe("error") | ||
| }) | ||
| test("once effort is downgraded at the soft deadline", async () => { | ||
| const tool = await initTool(AnswerCommitTool) | ||
| const session = crypto.randomUUID() | ||
| const release = begin(session) | ||
| downgradeEffort(session) | ||
| try { | ||
| expect((await tool.execute(payload, context([user(compound)], session))).output).toBe(renderAnswer(payload)) | ||
| } finally { | ||
| release() | ||
| } | ||
| }) | ||
| test("after the synthetic time-budget reminder", async () => { | ||
| const tool = await initTool(AnswerCommitTool) | ||
| // The reminder joins two items, so it would be rejected if synthetic text were read as the request. | ||
| const text = "Answer now, and the basis in one sentence." | ||
| expect(requestedItems(text).count).toBe(2) | ||
| const reminder = { info: { role: "user" }, parts: [{ type: "text", text, synthetic: true }] } | ||
| expect((await tool.execute(payload, context([user(compound), reminder]))).output).toBe(renderAnswer(payload)) | ||
| }) | ||
| }) | ||
|
|
||
| // Native replies echo the committed text: the basis paragraph sits above the `**Answer:**` line. | ||
| describe("committed answer layout", () => { | ||
| const answerLine = (text: string) => text.slice(text.indexOf("**Answer:**")) | ||
| test("a complete fields commit keeps the basis off the answer line", () => { | ||
| const text = renderAnswer(complete) | ||
| expect(text).toBe(`${basis}\n\n**Answer:** repository: repo-a; most copied file: main.swift`) | ||
| expect(answerLine(text)).not.toContain(basis) | ||
| }) | ||
| test("a rows commit keeps the basis off the answer", () => { | ||
| const text = renderAnswer({ basis, rows: { columns: ["repository"], values: [["repo-a"]] } }) | ||
| expect(text.startsWith(`${basis}\n\n**Answer:**\n`)).toBe(true) | ||
| expect(answerLine(text)).not.toContain(basis) | ||
| }) | ||
| test("an accepted under-asked commit keeps the basis in the reply", async () => { | ||
| const tool = await initTool(AnswerCommitTool) | ||
| const withItem = { ...payload, basis: `${basis} Its most copied file is main.swift.` } | ||
| const first = await tool.execute(withItem, context([user(compound)])) | ||
|
anandgupta42 marked this conversation as resolved.
|
||
| const accepted = await tool.execute(withItem, context([user(compound), rejectedCommit(first.output)])) | ||
| expect(accepted.metadata.committed_text).toBe(`${withItem.basis}\n\n**Answer:** repository: repo-a`) | ||
| }) | ||
| }) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.