Skip to content

Commit 7dcece2

Browse files
NagyViktNagyVikt
andauthored
feat(memory): ICM slice 2 — feedback record/search/stats (#579)
Adds the cross-agent "AI predicted X, real answer was Y" feedback lane specified in docs/icm-integration-plan.md slice 2 so a future agent can search prior corrections by topic before repeating a known mistake. - packages/storage migration 015 introduces `feedback` + a porter- unicode61 `feedback_fts` virtual table with the standard ai/ad/au triggers, plus indexes on `topic` and `created_at`. Importance is a four-level CHECK enum defaulting to `medium`. - packages/core MemoryStore exposes `recordFeedback`, `searchFeedback`, `getFeedback`, and `feedbackStats`. All three prose fields (prediction/correction/context) route through `prepareMemoryText`, the same redact-then-compress path observations use, so the compression invariant holds at the write boundary. - apps/mcp-server registers `feedback_record`, `feedback_search`, and `feedback_stats` with progressive-disclosure return shapes (id, topic, importance, FTS5 score, snippet, created_at). docs/mcp.md carries the contract. Follow-up out of scope here: a pre-tool-use hook that auto-surfaces prior corrections on inbound prompts. Tracking separately so this slice can ship behind a manual query surface first. Co-authored-by: NagyVikt <nagy.viktordp@gmail.com>
1 parent 3e1fa74 commit 7dcece2

15 files changed

Lines changed: 981 additions & 3 deletions

File tree

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
---
2+
"@colony/storage": minor
3+
"@colony/core": minor
4+
"@colony/mcp-server": minor
5+
---
6+
7+
ICM slice 2 — feedback `record`, `search`, and `stats` MCP tools.
8+
9+
Adds a new `feedback` lane that records "AI predicted X, real answer
10+
was Y" corrections so a future agent can search prior mistakes by
11+
topic before repeating them. Migration 015 introduces the `feedback`
12+
table plus a porter-unicode61 `feedback_fts` virtual table mirrored
13+
by the standard `ai/ad/au` triggers; importance is a four-level enum
14+
defaulting to `medium`. `prediction`, `correction`, and the optional
15+
`context` flow through `MemoryStore.recordFeedback`, which routes each
16+
body through `prepareMemoryText` — the same redact-then-compress path
17+
observations use — so the compression invariant holds at the write
18+
boundary.
19+
20+
MCP surface (progressive disclosure):
21+
22+
- `feedback_record({ topic, prediction, correction, context?, importance?, created_by? })``{ id }`
23+
- `feedback_search({ query, topic?, limit? })` → compact hits (`id`, `topic`, `importance`, `score`, `snippet`, `created_at`)
24+
- `feedback_stats({ topic? })` → per-topic counts and `last_created_at`
25+
26+
Follow-up (separate PR): a pre-tool-use hook that surfaces prior
27+
corrections on inbound prompts. This PR keeps the slice scoped to the
28+
storage + search surface so it can ship behind a manual query first.
29+
30+
Reference: `docs/icm-integration-plan.md` slice 2.

apps/mcp-server/src/server.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import * as autopilot from './tools/autopilot.js';
1414
import * as bridge from './tools/bridge.js';
1515
import type { ToolContext } from './tools/context.js';
1616
import * as drift from './tools/drift.js';
17+
import * as feedback from './tools/feedback.js';
1718
import * as foraging from './tools/foraging.js';
1819
import { registerTaskForagingReport } from './tools/foraging.js';
1920
import * as handoff from './tools/handoff.js';
@@ -30,8 +31,8 @@ import * as readyQueue from './tools/ready-queue.js';
3031
import * as recall from './tools/recall.js';
3132
import * as relay from './tools/relay.js';
3233
import * as rescue from './tools/rescue.js';
33-
import * as savings from './tools/savings.js';
3434
import * as savingsDrift from './tools/savings-drift.js';
35+
import * as savings from './tools/savings.js';
3536
import * as search from './tools/search.js';
3637
import * as spec from './tools/spec.js';
3738
import * as startupPanel from './tools/startup-panel.js';
@@ -125,6 +126,11 @@ export function buildServer(
125126
savings.register(server, ctx);
126127
savingsDrift.register(server, ctx);
127128

129+
// ICM slice 2 feedback lane (docs/icm-integration-plan.md). Registered
130+
// after the read-side surfaces so the heartbeat wrapper has seen every
131+
// core tool first.
132+
feedback.register(server, ctx);
133+
128134
// Autopilot lane (tick advisor + drift checker). Cheap compositions of
129135
// existing primitives; registered after the core surface so the heartbeat
130136
// wrapper has already wrapped the tools they delegate to.
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
import { mkdtempSync, rmSync } from 'node:fs';
2+
import { tmpdir } from 'node:os';
3+
import { join } from 'node:path';
4+
import { defaultSettings } from '@colony/config';
5+
import { MemoryStore } from '@colony/core';
6+
import { Client } from '@modelcontextprotocol/sdk/client';
7+
import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js';
8+
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
9+
import { buildServer } from '../server.js';
10+
11+
let dir: string;
12+
let store: MemoryStore;
13+
let client: Client;
14+
15+
beforeEach(async () => {
16+
dir = mkdtempSync(join(tmpdir(), 'colony-feedback-mcp-'));
17+
store = new MemoryStore({ dbPath: join(dir, 'data.db'), settings: defaultSettings });
18+
const server = buildServer(store, defaultSettings);
19+
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
20+
client = new Client({ name: 'test', version: '0.0.0' });
21+
await Promise.all([server.connect(serverTransport), client.connect(clientTransport)]);
22+
});
23+
24+
afterEach(async () => {
25+
await client.close();
26+
store.close();
27+
rmSync(dir, { recursive: true, force: true });
28+
});
29+
30+
interface ToolResponse {
31+
content: Array<{ type: string; text: string }>;
32+
}
33+
34+
function parseTextContent<T>(response: unknown): T {
35+
const typed = response as ToolResponse;
36+
const text = typed.content[0]?.text;
37+
if (typeof text !== 'string') throw new Error('feedback tool returned no text content');
38+
return JSON.parse(text) as T;
39+
}
40+
41+
describe('feedback MCP surface (ICM slice 2)', () => {
42+
it('records a correction and surfaces it via search + stats', async () => {
43+
const recordRes = await client.callTool({
44+
name: 'feedback_record',
45+
arguments: {
46+
topic: 'frontend.routing',
47+
prediction: 'useRouter returns null in server components',
48+
correction: 'useRouter throws in server components',
49+
context: 'reviewing apps/web App Router migration',
50+
importance: 'high',
51+
created_by: 'claude',
52+
},
53+
});
54+
const recordPayload = parseTextContent<{ id: number }>(recordRes);
55+
expect(recordPayload.id).toBeGreaterThan(0);
56+
57+
await client.callTool({
58+
name: 'feedback_record',
59+
arguments: {
60+
topic: 'backend.migrations',
61+
prediction: 'ALTER TABLE works inside transactions',
62+
correction: 'ALTER TABLE must run outside a transaction for partitioned tables',
63+
},
64+
});
65+
66+
const searchRes = await client.callTool({
67+
name: 'feedback_search',
68+
arguments: { query: 'router server component', limit: 5 },
69+
});
70+
const searchPayload = parseTextContent<{
71+
hits: Array<{ id: number; topic: string; score: number; snippet: string }>;
72+
}>(searchRes);
73+
expect(searchPayload.hits.length).toBeGreaterThan(0);
74+
expect(searchPayload.hits[0]?.topic).toBe('frontend.routing');
75+
76+
const statsRes = await client.callTool({
77+
name: 'feedback_stats',
78+
arguments: {},
79+
});
80+
const statsPayload = parseTextContent<{
81+
stats: Array<{ topic: string; count: number; last_created_at: number }>;
82+
}>(statsRes);
83+
const topics = statsPayload.stats.map((row) => row.topic);
84+
expect(topics).toContain('frontend.routing');
85+
expect(topics).toContain('backend.migrations');
86+
});
87+
88+
it('returns INTERNAL_ERROR when the prediction redacts to empty', async () => {
89+
const res = await client.callTool({
90+
name: 'feedback_record',
91+
arguments: {
92+
topic: 'auth',
93+
prediction: '<private>secret-prediction</private>',
94+
correction: 'real correction text',
95+
},
96+
});
97+
const typed = res as ToolResponse & { isError?: boolean };
98+
expect(typed.isError).toBe(true);
99+
const payload = JSON.parse(typed.content[0]?.text ?? '{}') as { code: string };
100+
expect(payload.code).toBe('INTERNAL_ERROR');
101+
});
102+
});
Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
import type { FeedbackImportance } from '@colony/core';
2+
import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
3+
import { z } from 'zod';
4+
import { type ToolContext, defaultWrapHandler } from './context.js';
5+
import { mcpErrorResponse } from './shared.js';
6+
7+
/**
8+
* Feedback lane (ICM slice 2 — docs/icm-integration-plan.md). Records the
9+
* "AI predicted X, real answer was Y" pairs that surface in code review,
10+
* test failures, and human corrections so a later agent can search prior
11+
* mistakes by topic.
12+
*
13+
* Progressive disclosure mirrors the observation surface:
14+
* feedback_record → row id only
15+
* feedback_search → compact hits (id, topic, score, snippet)
16+
* feedback_stats → counts per topic
17+
*
18+
* Compression invariant: prediction/correction/context flow through
19+
* `MemoryStore.recordFeedback`, which runs each body through the same
20+
* `prepareMemoryText` path observations use. Tool handlers never write
21+
* raw prose to storage directly.
22+
*
23+
* Note: this PR does not register a pre-tool-use hook that surfaces prior
24+
* corrections on inbound prompts. That belongs to a follow-up PR so this
25+
* slice can ship behind a search surface first.
26+
*/
27+
export function register(server: McpServer, ctx: ToolContext): void {
28+
const wrapHandler = ctx.wrapHandler ?? defaultWrapHandler;
29+
const { store } = ctx;
30+
31+
const importanceSchema = z
32+
.enum(['critical', 'high', 'medium', 'low'])
33+
.describe('how strongly this correction should weigh against repeating the prediction');
34+
35+
server.tool(
36+
'feedback_record',
37+
"Record an 'AI predicted X, real answer was Y' correction. Bodies are compressed via the same path observations use; returns only the new row id.",
38+
{
39+
topic: z
40+
.string()
41+
.min(1)
42+
.max(200)
43+
.describe('a short, stable label callers can pivot on (e.g. "frontend.routing")'),
44+
prediction: z.string().min(1).describe('what the AI predicted / asserted'),
45+
correction: z.string().min(1).describe('what the real answer turned out to be'),
46+
context: z
47+
.string()
48+
.min(1)
49+
.optional()
50+
.describe('optional surrounding context (where the prediction was made)'),
51+
importance: importanceSchema.optional(),
52+
created_by: z.string().min(1).optional().describe('agent or human author for audit'),
53+
},
54+
wrapHandler('feedback_record', async (args) => {
55+
const topic = args.topic.trim();
56+
if (!topic) {
57+
return mcpErrorResponse('INTERNAL_ERROR', 'feedback_record: topic must be non-empty');
58+
}
59+
const id = store.recordFeedback({
60+
topic,
61+
prediction: args.prediction,
62+
correction: args.correction,
63+
...(args.context !== undefined ? { context: args.context } : {}),
64+
...(args.importance !== undefined
65+
? { importance: args.importance as FeedbackImportance }
66+
: {}),
67+
...(args.created_by !== undefined ? { created_by: args.created_by } : {}),
68+
});
69+
if (id < 0) {
70+
return mcpErrorResponse(
71+
'INTERNAL_ERROR',
72+
'feedback_record: prediction/correction collapsed to empty after privacy redaction',
73+
);
74+
}
75+
return { content: [{ type: 'text', text: JSON.stringify({ id }) }] };
76+
}),
77+
);
78+
79+
server.tool(
80+
'feedback_search',
81+
'Search prior corrections. Returns compact hits (id, topic, importance, score, snippet); use feedback_record output ids and a follow-up read if you need the full bodies.',
82+
{
83+
query: z
84+
.string()
85+
.min(1)
86+
.describe('FTS5 query across topic + prediction + correction + context'),
87+
topic: z
88+
.string()
89+
.min(1)
90+
.optional()
91+
.describe('optional exact-match filter on the feedback topic'),
92+
limit: z.number().int().positive().max(100).optional(),
93+
},
94+
wrapHandler('feedback_search', async (args) => {
95+
const hits = store.searchFeedback({
96+
query: args.query,
97+
...(args.topic !== undefined ? { topic: args.topic } : {}),
98+
...(args.limit !== undefined ? { limit: args.limit } : {}),
99+
});
100+
return { content: [{ type: 'text', text: JSON.stringify({ hits }) }] };
101+
}),
102+
);
103+
104+
server.tool(
105+
'feedback_stats',
106+
'Per-topic counts of recorded corrections, sorted by most recent first. Pass a topic to scope to a single bucket.',
107+
{
108+
topic: z.string().min(1).optional(),
109+
},
110+
wrapHandler('feedback_stats', async (args) => {
111+
const stats = store.feedbackStats(args.topic !== undefined ? { topic: args.topic } : {});
112+
return { content: [{ type: 'text', text: JSON.stringify({ stats }) }] };
113+
}),
114+
);
115+
}

apps/mcp-server/test/server.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ describe('MCP server', () => {
5656
'examples_integrate_plan',
5757
'examples_list',
5858
'examples_query',
59+
'feedback_record',
60+
'feedback_search',
61+
'feedback_stats',
5962
'get_observations',
6063
'hivemind',
6164
'hivemind_context',

docs/mcp.md

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,9 @@ workflow guidance.
102102
| Rescue | `rescue_stranded_run` | Emit rescue relays and release abandoned claims. |
103103
| Metrics | `savings_report` | Report live MCP token receipts and reference savings model. |
104104
| Metrics | `savings_drift_report` | Flag tools whose median tokens-per-call drifted vs a baseline window. |
105+
| Feedback | `feedback_record` | Record an "AI predicted X, real answer was Y" correction. |
106+
| Feedback | `feedback_search` | Search prior corrections by FTS5 query and optional topic. |
107+
| Feedback | `feedback_stats` | Per-topic counts of recorded corrections. |
105108

106109
## Ruflo sidecar boundary
107110

@@ -2266,6 +2269,43 @@ Response shape:
22662269

22672270
Classifications: `up_drift`, `down_drift`, `new_tool` (no baseline data), `gone` (no recent calls), `insufficient_data` (either window below `min_calls`), or `stable`. When the baseline window starts before the earliest `mcp_metrics` receipt the response adds a `warning` field nudging callers to wait for more history before trusting signals.
22682271

2272+
## `feedback_record`
2273+
2274+
Record an "AI predicted X, real answer was Y" correction. ICM slice 2 (see `docs/icm-integration-plan.md`). Bodies pass through the same compression path as observations — `MemoryStore.recordFeedback` runs `prediction`, `correction`, and (optional) `context` through `prepareMemoryText` before persisting. Returns only the new row id; full bodies stay behind the storage layer.
2275+
2276+
Args:
2277+
2278+
- `topic` — short, stable label callers can pivot on (e.g. `"frontend.routing"`).
2279+
- `prediction` — what the AI predicted or asserted.
2280+
- `correction` — what the real answer turned out to be.
2281+
- `context?` — optional surrounding context (where the prediction was made).
2282+
- `importance?``critical | high | medium | low`. Defaults to `medium`.
2283+
- `created_by?` — agent or human author for audit.
2284+
2285+
Returns `{ "id": <number> }`. When privacy redaction collapses `prediction` or `correction` to empty, the tool returns an `INTERNAL_ERROR` instead of writing a phantom row.
2286+
2287+
## `feedback_search`
2288+
2289+
Search prior corrections. Compact-hit progressive disclosure: returns `id`, `topic`, `importance`, FTS5 `score` (higher = better), `snippet`, and `created_at` only. Bodies live behind storage so callers don't pay the expansion cost on every search.
2290+
2291+
Args:
2292+
2293+
- `query` — FTS5 query across `topic + prediction + correction + context`.
2294+
- `topic?` — exact-match filter on the feedback topic.
2295+
- `limit?` — defaults to 20, max 100.
2296+
2297+
If `query` is empty/whitespace and `topic` is set, returns a newest-first listing for that topic. Empty `query` without a `topic` returns no hits.
2298+
2299+
## `feedback_stats`
2300+
2301+
Per-topic counts of recorded corrections, sorted by most recent first.
2302+
2303+
Args:
2304+
2305+
- `topic?` — exact-match filter; scopes the response to a single bucket.
2306+
2307+
Response shape: `{ "stats": [{ "topic": string, "count": number, "last_created_at": number }] }`.
2308+
22692309
## Plan observation kinds
22702310

22712311
The lane introduces several observation kinds on the parent spec task and on the sub-task threads. They are written through `MemoryStore.addObservation`, so content is compressed and `metadata` carries the structured payload.

packages/core/src/index.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,13 @@ export {
336336
export { buildDiscrepancyReport, type DiscrepancyReport } from './discrepancy.js';
337337
export { isPseudoClaimPath, normalizeClaimPath, normalizeRepoFilePath } from '@colony/storage';
338338
export type { ClaimPathContext, RepoFilePathContext } from '@colony/storage';
339+
export type {
340+
AddFeedbackInput,
341+
FeedbackHit,
342+
FeedbackImportance,
343+
FeedbackRow,
344+
FeedbackStat,
345+
} from '@colony/storage';
339346
export {
340347
buildCoordinationSweep,
341348
type BlockedDownstreamTaskSignal,

0 commit comments

Comments
 (0)