Skip to content

Commit 0617495

Browse files
authored
feat: expose quality_score on news_editor_review_signal (#604) (#605)
agent-news#864 added an optional quality_score override to PATCH /api/signals/:id/review, but the MCP wrapper never exposed it, so editors working through MCP could not correct the auto-scorer. Pass the field through unmodified. The guard is an explicit undefined check rather than a truthiness test, since 0 is a valid score — the #810 fabricated-sources case — and would otherwise be dropped silently. Adds tests/tools/news-editor-review-signal.test.ts covering 15 -> 15, 0 -> 0, and omitted -> absent.
1 parent 694c8ea commit 0617495

2 files changed

Lines changed: 170 additions & 1 deletion

File tree

src/tools/news.tools.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -877,6 +877,10 @@ be a registered editor for the signal's beat, or the beat's publisher.
877877
878878
When rejecting, feedback is required to explain why.
879879
880+
The auto-scorer judges each signal in isolation, so it cannot see duplication,
881+
template-bleed, or scraped boilerplate. Pass quality_score to correct it when the
882+
automated score does not match your editorial judgement.
883+
880884
When the daily approval cap has been reached and you want to approve a new signal,
881885
use displace_signal_id to swap it with a previously approved signal.
882886
@@ -896,9 +900,22 @@ Authenticated via BIP-322 signature.`,
896900
.string()
897901
.optional()
898902
.describe("ID of a previously approved signal to displace when at daily cap"),
903+
quality_score: z
904+
.number()
905+
.int()
906+
.min(0)
907+
.max(100)
908+
.optional()
909+
.describe(
910+
"Integer 0-100. Overrides the auto-scorer's quality_score on this signal. " +
911+
"Use when the automated score does not reflect the editorial judgement " +
912+
"(e.g. rejecting a well-sourced but duplicate or templated filing that " +
913+
"scored highly). The original score is preserved in score_breakdown.override " +
914+
"for audit."
915+
),
899916
},
900917
},
901-
async ({ signal_id, status, feedback, displace_signal_id }) => {
918+
async ({ signal_id, status, feedback, displace_signal_id, quality_score }) => {
902919
try {
903920
const account = await getAccount();
904921

@@ -925,6 +942,13 @@ Authenticated via BIP-322 signature.`,
925942
if (displace_signal_id) {
926943
payload.displace_signal_id = displace_signal_id;
927944
}
945+
// Explicit undefined check: 0 is a valid score and would be dropped by a
946+
// truthiness test. Omitting the field leaves the API's no-override path.
947+
// Covered by tests/tools/news-editor-review-signal.test.ts — the 0 case
948+
// fails if this is refactored to `if (quality_score)`.
949+
if (quality_score !== undefined) {
950+
payload.quality_score = quality_score;
951+
}
928952

929953
const res = await fetch(`${NEWS_BASE}/signals/${signal_id}/review`, {
930954
method: "PATCH",
Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
import { beforeEach, describe, expect, it, vi } from "vitest";
2+
3+
const mockGetAccount = vi.fn();
4+
5+
vi.mock("../../src/services/x402.service.js", () => ({
6+
getAccount: mockGetAccount,
7+
NETWORK: "mainnet",
8+
}));
9+
10+
vi.mock("../../src/services/sbtc.service.js", () => ({
11+
getSbtcService: vi.fn(() => ({ getBalance: vi.fn() })),
12+
}));
13+
14+
vi.mock("../../src/services/hiro-api.js", () => ({
15+
getHiroApi: vi.fn(() => ({
16+
getAccountInfo: vi.fn(),
17+
getMempoolTransactions: vi.fn(),
18+
})),
19+
}));
20+
21+
vi.mock("../../src/services/nonce-tracker.js", () => ({
22+
getTrackedNonce: vi.fn(),
23+
recordNonceUsed: vi.fn(),
24+
reconcileWithChain: vi.fn(),
25+
}));
26+
27+
// bip322Sign is real crypto over the mocked key material; stub it so the test
28+
// asserts payload construction rather than re-testing the signer.
29+
vi.mock("../../src/utils/bip322.js", () => ({
30+
bip322Sign: vi.fn(() => "mock-signature"),
31+
}));
32+
33+
vi.mock("@scure/btc-signer", () => ({
34+
p2wpkh: vi.fn(() => ({ script: new Uint8Array([0x00]) })),
35+
NETWORK: {},
36+
TEST_NETWORK: {},
37+
}));
38+
39+
const { registerNewsTools } = await import("../../src/tools/news.tools.js");
40+
41+
interface RegisteredTool {
42+
handler: (args: Record<string, unknown>) => Promise<unknown>;
43+
}
44+
45+
function createTrackingServer() {
46+
const tools = new Map<string, RegisteredTool>();
47+
const server = {
48+
registerTool: vi.fn(
49+
(
50+
name: string,
51+
_config: { description: string; inputSchema: unknown },
52+
handler: RegisteredTool["handler"]
53+
) => {
54+
tools.set(name, { handler });
55+
}
56+
),
57+
};
58+
return { server, tools };
59+
}
60+
61+
/**
62+
* Registers the news tools, invokes news_editor_review_signal with `args`, and
63+
* returns the JSON body that would have been PATCHed to /api/signals/:id/review.
64+
*/
65+
async function reviewPayload(
66+
args: Record<string, unknown>
67+
): Promise<Record<string, unknown>> {
68+
const { server, tools } = createTrackingServer();
69+
registerNewsTools(server as never);
70+
const tool = tools.get("news_editor_review_signal");
71+
if (!tool) throw new Error("news_editor_review_signal was not registered");
72+
73+
const fetchMock = vi.fn(async () => ({
74+
ok: true,
75+
status: 200,
76+
text: async () => JSON.stringify({ id: "sig_1", status: "rejected" }),
77+
}));
78+
vi.stubGlobal("fetch", fetchMock);
79+
80+
const result = (await tool.handler(args)) as { isError?: boolean };
81+
if (result?.isError) {
82+
throw new Error(
83+
`handler errored: ${JSON.stringify((result as Record<string, unknown>).content)}`
84+
);
85+
}
86+
if (fetchMock.mock.calls.length === 0) {
87+
throw new Error("handler did not issue a request");
88+
}
89+
90+
const [, init] = fetchMock.mock.calls[0] as unknown as [
91+
string,
92+
{ body: string },
93+
];
94+
return JSON.parse(init.body) as Record<string, unknown>;
95+
}
96+
97+
describe("news_editor_review_signal quality_score override", () => {
98+
beforeEach(() => {
99+
vi.clearAllMocks();
100+
vi.unstubAllGlobals();
101+
mockGetAccount.mockResolvedValue({
102+
address: "SP000000000000000000002Q6VF78",
103+
btcAddress: "bc1qeditor000000000000000000000000000000",
104+
btcPrivateKey: new Uint8Array(32).fill(1),
105+
btcPublicKey: new Uint8Array(33).fill(2),
106+
privateKey: "0".repeat(64),
107+
network: "mainnet",
108+
});
109+
});
110+
111+
it("forwards an editor-supplied quality_score", async () => {
112+
const payload = await reviewPayload({
113+
signal_id: "sig_1",
114+
status: "rejected",
115+
feedback: "duplicate refile",
116+
quality_score: 15,
117+
});
118+
119+
expect(payload.quality_score).toBe(15);
120+
});
121+
122+
// The #810 case: fabricated sources score 0. A truthiness guard
123+
// (`if (quality_score)`) drops it silently and the override becomes a no-op,
124+
// so this asserts the most severe correction an editor can make survives.
125+
it("forwards quality_score 0 rather than dropping it as falsy", async () => {
126+
const payload = await reviewPayload({
127+
signal_id: "sig_2",
128+
status: "rejected",
129+
feedback: "fabricated sources",
130+
quality_score: 0,
131+
});
132+
133+
expect(payload).toHaveProperty("quality_score");
134+
expect(payload.quality_score).toBe(0);
135+
});
136+
137+
it("omits quality_score entirely when not supplied", async () => {
138+
const payload = await reviewPayload({
139+
signal_id: "sig_3",
140+
status: "approved",
141+
});
142+
143+
expect(payload).not.toHaveProperty("quality_score");
144+
});
145+
});

0 commit comments

Comments
 (0)