mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 07:12:00 +01:00
fix(opencode): save an answer once when the model restates it after the completion call (#21025)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Agents that use OpenCode models reply in chat through the Runner's
OpenCode driver. The driver settles one final response, and Paperclip
saves it as the reply comment
> - #20999 made the driver keep the prose written before the
`paperclip_finish` call together with the message written after it. That
stopped the loss of an answer when the later message only said "Answered
above"
> - In chat-eval cohort 4 (OpenCode with DeepSeek v4.1 flash), the model
often wrote its whole answer, called `paperclip_finish`, and then wrote
the same answer again in other words. The driver saved both. 13 cases
had one saved reply that held the answer twice
> - This pull request saves only the later message when it restates the
earlier prose. A short pointer line still keeps the earlier answer
> - The benefit is that people, deliveries and the next turn read the
answer once, and #20999's protection stays in place
## Linked Issues or Issue Description
Refs #20999
**What happened?**
An OpenCode chat turn wrote its answer, called `paperclip_finish`, and
then wrote the same answer again with small wording changes. The saved
reply comment held both copies, one after the other. Example
(budgets-costs.set-budget): "I can't set this from chat — changing an
agent's monthly budget is a board-only action ..." followed by "I can't
change agent budgets from chat — that's board-only ...", both with the
same two budget links.
**Expected behavior**
The saved reply holds the answer once. When the later message only
refers back to the earlier answer, the saved reply still keeps the
earlier answer.
**Steps to reproduce**
1. Use an OpenCode agent with
`openrouter/deepseek/deepseek-v4.1-flash-1017`.
2. Ask a question that the model answers before `paperclip_finish` and
again after it.
3. Read the saved reply with `GET /api/issues/{id}/comments`. It holds
the answer twice.
**Paperclip version or commit**
master at f1aef0cb58.
## What Changed
- New `restatesEarlierReply(earlier, later)` in
`packages/paperclip-runner/src/drivers/opencode/restated-reply.ts`. The
later message restates the earlier prose only when all of these are
true: it keeps every link target and fenced code block of the earlier
prose, it is at least half as long, and it repeats at least half of the
earlier prose's distinct words. Identical text is always a restatement.
- `OpenCodeServerDriver` uses it in the path that #20999 added. When the
post-completion message restates the pre-completion prose, the driver
saves the post-completion message alone. Otherwise it joins both, as
before.
- The fake OpenCode server gets an
`answer-before-finish-then-restatement` turn that uses the recorded
cohort-4 texts.
- Tests: one driver test for the recorded shape, and unit tests for the
helper (identical, reworded, pointer line, dropped link, dropped code
block, different content).
## Verification
- `cd packages/paperclip-runner && npx vitest run
src/drivers/opencode/restated-reply.test.ts
src/drivers/opencode/opencode-server-driver.test.ts`: 87 tests pass.
This includes the #20999 tests ("Answered above" keeps the earlier
answer; long commentary before a short post-tool answer keeps both).
- Red/green: with the driver file from `origin/master`, the new driver
test fails (the saved text holds both copies). With the change, it
passes.
- `npx tsc -p packages/paperclip-runner/tsconfig.json --noEmit` passes.
- I ran the helper over all 18 recorded cohort-4 turns that had prose
both before and after `paperclip_finish`. It classified the 13 reported
duplicate replies (and 4 turns whose earlier line was a short status
that the later answer covers) as restatements. It kept both texts for
`access.cli-authorization`, where the later message is "Commands above
are ready to paste."
- A paid chat-eval run on a branch with this change is linked in a
comment below.
## Risks
- The check reads text, so it can be wrong at the margin. If it wrongly
calls an earlier answer a restatement, the saved reply loses that
earlier text. The link, code-block and length conditions keep the
references and the content of a substantial earlier answer; the earlier
prose remains in the run transcript.
- No saved run state, session state or event shape changes. A run saved
by the previous release resumes without change; only the text of a newly
settled final response differs.
- During a live run, the chat can still show the earlier prose as
progress above the saved answer, as it did before #20999.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code
execution in Claude Code.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
940d681974
commit
f7d41ca5bf
5 files changed
+201
-4
No files matched your search
@@ -3062,6 +3062,45 @@ describe("OpenCodeServerDriver", () => {
|
||||
await session.close({ reason: "test" });
|
||||
});
|
||||
|
||||
it("saves an answer once when the model restates it after the completion call", async () => {
|
||||
await chmod(fixture, 0o755);
|
||||
const root = await mkdtemp(join(tmpdir(), "paperclip-opencode-driver-"));
|
||||
const workspace = await mkdtemp(
|
||||
join(tmpdir(), "paperclip-opencode-workspace-"),
|
||||
);
|
||||
roots.push(root, workspace);
|
||||
const driver = new OpenCodeServerDriver({
|
||||
model: "openrouter/deepseek/deepseek-v4-flash-0731",
|
||||
runtimeDirectory: root,
|
||||
command: fixture,
|
||||
environment: {
|
||||
PATH: process.env.PATH,
|
||||
OPENROUTER_API_KEY: "fixture-key",
|
||||
},
|
||||
});
|
||||
const session = await driver.openSession({
|
||||
runId: "run-answer-then-restatement",
|
||||
normalizedSessionId: "answer-then-restatement",
|
||||
workingDirectory: workspace,
|
||||
});
|
||||
await session.startTurn({
|
||||
message: { role: "user", text: "answer-before-finish-then-restatement" },
|
||||
});
|
||||
const events = await collectTurnEvents(session.events());
|
||||
const finalMessages = events.filter(
|
||||
(event) =>
|
||||
event.eventType === "item.completed" &&
|
||||
event.payload.kind === "agentMessage" &&
|
||||
event.payload.channel === "final",
|
||||
);
|
||||
expect(finalMessages).toHaveLength(1);
|
||||
expect(finalMessages[0]?.payload.text).toBe(
|
||||
"I can't change agent budgets from chat — that's board-only, and agent credentials get 403. Two agents also match \"Morgan,\" so choose the intended one and set $50/month on its budget tab: [Morgan budgets](/COM/agents/agent-morgan/budgets) or [Morgan Research budgets](/COM/agents/agent-morgan-research/budgets).",
|
||||
);
|
||||
expect(finalMessages[0]?.itemId).toBe("part-reference");
|
||||
await session.close({ reason: "test" });
|
||||
});
|
||||
|
||||
it.each(["session.idle", "session.status", "session.error", "aborted"])("settles accepted completion before a racing %s event", async terminalType => {
|
||||
await chmod(fixture, 0o755);
|
||||
const root = await mkdtemp(join(tmpdir(), "paperclip-opencode-completion-race-"));
|
||||
|
||||
@@ -23,6 +23,7 @@ import { getCACertificates } from "node:tls";
|
||||
import { pipeline } from "node:stream/promises";
|
||||
import { dirname, isAbsolute, join, relative, resolve, sep } from "node:path";
|
||||
|
||||
import { restatesEarlierReply } from "./restated-reply.js";
|
||||
import { parseOpenCodeReasoningMode, type OpenCodeReasoningMode } from "./reasoning-mode.js";
|
||||
import { attributeUnsupportedToolMedia } from "./tool-media-attribution.js";
|
||||
import { openCodeProviderFailure } from "./provider-failure.js";
|
||||
@@ -2301,12 +2302,17 @@ class OpenCodeHarnessSession implements HarnessSession {
|
||||
// before that call: the person saw both, and the later message often just
|
||||
// refers back ("Answered above"). Structure cannot tell an answer from
|
||||
// commentary, so keep every eligible part rather than drop the answer.
|
||||
const preface =
|
||||
const candidatePreface =
|
||||
this.#semanticResultProviderMessageId !== null &&
|
||||
correlatedAfterResult.length === 0 &&
|
||||
uncorrelatedAfterResult[0]?.part === selected
|
||||
? beforeResult.map(({ part }) => part)
|
||||
: [];
|
||||
const preface =
|
||||
candidatePreface.length &&
|
||||
restatesEarlierReply(candidatePreface.map((part) => part.text.trim()).join("\n\n"), selected.text)
|
||||
? []
|
||||
: candidatePreface;
|
||||
const text = preface.length
|
||||
? [...preface, selected].map((part) => part.text.trim()).filter(Boolean).join("\n\n")
|
||||
: selected.text;
|
||||
|
||||
@@ -0,0 +1,100 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { restatesEarlierReply } from "./restated-reply.js";
|
||||
|
||||
const recordedBefore =
|
||||
"I can't set this from chat — changing an agent's monthly budget is a board-only action (agent credentials get 403). There are also two matching agents, so pick the intended one in the web app and set its monthly limit to $50 on the budget tab: [Morgan budgets](/COM/agents/a1/budgets) or [Morgan Research budgets](/COM/agents/a2/budgets). Tell me which Morgan you meant and I'll track the right one.";
|
||||
const recordedAfter =
|
||||
"I can't change agent budgets from chat — that's board-only, and agent credentials get 403. Two agents also match \"Morgan,\" so choose the intended one and set $50/month on its budget tab: [Morgan budgets](/COM/agents/a1/budgets) or [Morgan Research budgets](/COM/agents/a2/budgets).";
|
||||
|
||||
describe("restatesEarlierReply", () => {
|
||||
it("treats an identical later message as a restatement", () => {
|
||||
expect(restatesEarlierReply(recordedBefore, `${recordedBefore}\n`)).toBe(true);
|
||||
});
|
||||
|
||||
it("treats the recorded reworded answer as a restatement", () => {
|
||||
expect(restatesEarlierReply(recordedBefore, recordedAfter)).toBe(true);
|
||||
});
|
||||
|
||||
it("keeps an answer whose later message only refers back to it", () => {
|
||||
expect(
|
||||
restatesEarlierReply(
|
||||
"Two inbox items need you: [COM-2](/COM/issues/COM-2) is blocked and [COM-1](/COM/issues/COM-1) is waiting for review.",
|
||||
"Answered above: two inbox items need your attention.",
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("keeps an earlier link the later message drops", () => {
|
||||
expect(
|
||||
restatesEarlierReply(
|
||||
recordedBefore,
|
||||
recordedAfter.replace(" or [Morgan Research budgets](/COM/agents/a2/budgets)", " or Morgan Research's budget tab"),
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("keeps an earlier code block the later message drops", () => {
|
||||
expect(
|
||||
restatesEarlierReply(
|
||||
"Run this in your terminal:\n\n```shell\nnpx paperclipai auth login\n```\n\nThen confirm with whoami.",
|
||||
"Run the sign-in command in your terminal, then confirm with whoami before any write.",
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
const reportBefore = "The full launch report is ready at https://example.com/reports/launch and covers the signup funnel, the pricing page and the onboarding emails for review.";
|
||||
const reportAfter = "The launch report is ready and covers the signup funnel, the pricing page and the onboarding emails, so you can review it.";
|
||||
|
||||
it("keeps an earlier bare URL the later message drops", () => {
|
||||
expect(restatesEarlierReply(reportBefore, reportAfter)).toBe(false);
|
||||
});
|
||||
|
||||
it("treats a later message that keeps the bare URL as a restatement", () => {
|
||||
expect(restatesEarlierReply(reportBefore, `${reportAfter} Link: https://example.com/reports/launch.`)).toBe(true);
|
||||
});
|
||||
|
||||
it("keeps an earlier autolink the later message drops", () => {
|
||||
expect(
|
||||
restatesEarlierReply(reportBefore.replace("https://example.com/reports/launch", "<https://example.com/reports/launch>"), reportAfter),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("keeps an earlier reference-style link the later message drops", () => {
|
||||
expect(
|
||||
restatesEarlierReply(
|
||||
"The full launch [report][launch] is ready and covers the signup funnel, the pricing page and the onboarding emails for review.\n\n[launch]: /COM/issues/COM-7#document-report",
|
||||
reportAfter,
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
const codeAnswer = (code: string) =>
|
||||
`Run this script to check the export before you send it to the team:\n\n\`\`\`python\n${code}\n\`\`\`\n\nIt prints the row count and exits.`;
|
||||
|
||||
it("keeps an earlier code block whose string contents differ only in spacing", () => {
|
||||
expect(restatesEarlierReply(codeAnswer('print("a b")'), codeAnswer('print("a b")'))).toBe(false);
|
||||
});
|
||||
|
||||
it("keeps an earlier code block whose indentation differs", () => {
|
||||
expect(
|
||||
restatesEarlierReply(
|
||||
codeAnswer("for row in rows:\n if row:\n print(row)"),
|
||||
codeAnswer("for row in rows:\n if row:\n print(row)"),
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("treats a later message with the same code block as a restatement", () => {
|
||||
const code = "for row in rows:\n print(row)";
|
||||
expect(restatesEarlierReply(codeAnswer(code), codeAnswer(code).replace("Run this script", "Use this script"))).toBe(true);
|
||||
});
|
||||
|
||||
it("keeps earlier prose that says something different", () => {
|
||||
expect(
|
||||
restatesEarlierReply(
|
||||
"The deploy failed because the database migration timed out after 30 seconds on the orders table.",
|
||||
"I opened a follow-up task for Morgan to split the migration and will report back once it lands.",
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,43 @@
|
||||
export function restatesEarlierReply(earlier: string, later: string): boolean {
|
||||
const before = earlier.trim();
|
||||
const after = later.trim();
|
||||
if (!before || !after) return false;
|
||||
if (before === after) return true;
|
||||
if (after.length * 2 < before.length) return false;
|
||||
const laterTargets = linkTargets(after);
|
||||
if (![...linkTargets(before)].every((target) => laterTargets.has(target))) return false;
|
||||
const laterBlocks = new Set(fencedBlocks(after));
|
||||
if (!fencedBlocks(before).every((block) => laterBlocks.has(block))) return false;
|
||||
const earlierWords = proseWords(before);
|
||||
if (earlierWords.size === 0) return true;
|
||||
const laterWords = proseWords(after);
|
||||
let repeated = 0;
|
||||
for (const word of earlierWords) if (laterWords.has(word)) repeated += 1;
|
||||
return repeated * 2 >= earlierWords.size;
|
||||
}
|
||||
|
||||
const INLINE_LINK_TARGET = /\]\(\s*<?([^)\s>]+)>?(?:\s+"[^"]*")?\s*\)/g;
|
||||
const REFERENCE_DEFINITION = /^[ \t]{0,3}\[[^\]\n]+\]:[ \t]*<?([^\s>]+)>?/gm;
|
||||
const AUTOLINK = /<([a-z][a-z\d+.-]*:[^\s<>]+)>/gi;
|
||||
const BARE_URL = /\b(?:https?|ftp|mailto):[^\s<>()[\]`"']+/gi;
|
||||
const FENCED_BLOCK = /```[^\n]*\n([\s\S]*?)```/g;
|
||||
|
||||
function linkTargets(text: string): Set<string> {
|
||||
const targets = new Set<string>();
|
||||
for (const pattern of [INLINE_LINK_TARGET, REFERENCE_DEFINITION, AUTOLINK]) {
|
||||
for (const match of text.matchAll(pattern)) targets.add(match[1]!);
|
||||
}
|
||||
for (const match of text.replace(FENCED_BLOCK, " ").matchAll(BARE_URL)) {
|
||||
targets.add(match[0].replace(/[.,;:!?]+$/, ""));
|
||||
}
|
||||
return targets;
|
||||
}
|
||||
|
||||
function fencedBlocks(text: string): string[] {
|
||||
return [...text.matchAll(FENCED_BLOCK)].map((match) => match[1]!.replace(/\n$/, ""));
|
||||
}
|
||||
|
||||
function proseWords(text: string): Set<string> {
|
||||
const prose = text.replace(FENCED_BLOCK, " ").replace(INLINE_LINK_TARGET, "]");
|
||||
return new Set(prose.toLowerCase().match(/[\p{L}\p{N}]+/gu) ?? []);
|
||||
}
|
||||
@@ -800,6 +800,15 @@ const server = createServer(async (request, response) => {
|
||||
const answerBeforeFinishThenReference = String(
|
||||
parsedPrompt.message ?? "",
|
||||
).includes("answer-before-finish-then-reference");
|
||||
const answerBeforeFinishThenRestatement = String(
|
||||
parsedPrompt.message ?? "",
|
||||
).includes("answer-before-finish-then-restatement");
|
||||
const answerBeforeFinishText = answerBeforeFinishThenRestatement
|
||||
? "I can't set this from chat — changing an agent's monthly budget is a board-only action (agent credentials get 403). There are also two matching agents, so pick the intended one in the web app and set its monthly limit to $50 on the budget tab: [Morgan budgets](/COM/agents/agent-morgan/budgets) or [Morgan Research budgets](/COM/agents/agent-morgan-research/budgets). Tell me which Morgan you meant and I'll track the right one."
|
||||
: "Two inbox items need you: [COM-2](/COM/issues/COM-2) is blocked and [COM-1](/COM/issues/COM-1) is waiting for review.";
|
||||
const answerAfterFinishText = answerBeforeFinishThenRestatement
|
||||
? "I can't change agent budgets from chat — that's board-only, and agent credentials get 403. Two agents also match \"Morgan,\" so choose the intended one and set $50/month on its budget tab: [Morgan budgets](/COM/agents/agent-morgan/budgets) or [Morgan Research budgets](/COM/agents/agent-morgan-research/budgets)."
|
||||
: "Answered above: two inbox items need your attention.";
|
||||
const finalAfterToolCommentary = String(
|
||||
parsedPrompt.message ?? "",
|
||||
).includes("final-after-tool-commentary");
|
||||
@@ -1001,7 +1010,7 @@ const server = createServer(async (request, response) => {
|
||||
},
|
||||
},
|
||||
});
|
||||
} else if (answerBeforeFinishThenReference) {
|
||||
} else if (answerBeforeFinishThenReference || answerBeforeFinishThenRestatement) {
|
||||
emit({
|
||||
type: "message.part.updated",
|
||||
id: "event-answer-before-finish",
|
||||
@@ -1011,7 +1020,7 @@ const server = createServer(async (request, response) => {
|
||||
id: "part-answer-before-finish",
|
||||
messageID: "message-assistant",
|
||||
type: "text",
|
||||
text: "Two inbox items need you: [COM-2](/COM/issues/COM-2) is blocked and [COM-1](/COM/issues/COM-1) is waiting for review.",
|
||||
text: answerBeforeFinishText,
|
||||
time: { start: 1, end: 2 },
|
||||
},
|
||||
},
|
||||
@@ -1052,7 +1061,7 @@ const server = createServer(async (request, response) => {
|
||||
id: "part-reference",
|
||||
messageID: "message-reference",
|
||||
type: "text",
|
||||
text: "Answered above: two inbox items need your attention.",
|
||||
text: answerAfterFinishText,
|
||||
time: { start: 3, end: 4 },
|
||||
},
|
||||
},
|
||||
|
||||
Reference in new issue
Block a user