feat(reedy): Appendix A · Phase 5.1 — SkillRegistry + 3 seed skills (#4306)
This commit is contained in:
Huang Xin
@@ -0,0 +1,188 @@
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import { NodeDatabaseService } from '@/services/database/nodeDatabaseService';
|
||||
import { DatabaseService } from '@/types/database';
|
||||
import { migrate } from '@/services/database/migrate';
|
||||
import { getMigrations } from '@/services/database/migrations';
|
||||
import { SkillRegistry } from '@/services/reedy/skills/SkillRegistry';
|
||||
import { BUILTIN_SKILLS } from '@/services/reedy/skills/builtins';
|
||||
|
||||
describe('SkillRegistry', () => {
|
||||
let svc: DatabaseService;
|
||||
let registry: SkillRegistry;
|
||||
|
||||
beforeEach(async () => {
|
||||
svc = await NodeDatabaseService.open(':memory:', { experimental: ['index_method'] });
|
||||
await migrate(svc, getMigrations('reedy'));
|
||||
registry = new SkillRegistry(svc);
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await svc.close();
|
||||
});
|
||||
|
||||
describe('migration', () => {
|
||||
it('creates reedy_skills table', async () => {
|
||||
const tables = await svc.select(
|
||||
"SELECT name FROM sqlite_master WHERE type='table' AND name='reedy_skills'",
|
||||
);
|
||||
expect(tables).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('init()', () => {
|
||||
it('seeds every built-in skill on first boot and reports the inserted count', async () => {
|
||||
const inserted = await registry.init();
|
||||
expect(inserted).toBe(BUILTIN_SKILLS.length);
|
||||
const ids = (await registry.list()).map((s) => s.id);
|
||||
for (const builtin of BUILTIN_SKILLS) {
|
||||
expect(ids).toContain(builtin.id);
|
||||
}
|
||||
});
|
||||
|
||||
it('is idempotent — re-running init() on a populated DB inserts nothing', async () => {
|
||||
await registry.init();
|
||||
const inserted = await registry.init();
|
||||
expect(inserted).toBe(0);
|
||||
const skills = await registry.list();
|
||||
expect(skills.length).toBe(BUILTIN_SKILLS.length);
|
||||
});
|
||||
|
||||
it('leaves user-edited built-in instructions untouched on re-seed', async () => {
|
||||
await registry.init();
|
||||
await registry.upsert({
|
||||
id: 'spoiler-free',
|
||||
name: 'Spoiler-free (custom)',
|
||||
description: 'edited',
|
||||
instructions: 'EDITED INSTRUCTIONS',
|
||||
builtin: true,
|
||||
enabled: true,
|
||||
});
|
||||
await registry.init();
|
||||
const after = await registry.getById('spoiler-free');
|
||||
expect(after?.instructions).toBe('EDITED INSTRUCTIONS');
|
||||
});
|
||||
});
|
||||
|
||||
describe('list / listEnabled / getById', () => {
|
||||
it('list returns built-in skills first, then alphabetical', async () => {
|
||||
await registry.init();
|
||||
const out = await registry.list();
|
||||
// Built-ins are ordered by name within their group (all built-in
|
||||
// for now): chapter-summary < quote-finder < spoiler-free
|
||||
expect(out.slice(0, BUILTIN_SKILLS.length).every((s) => s.builtin)).toBe(true);
|
||||
const names = out.slice(0, 3).map((s) => s.name);
|
||||
expect(names).toEqual([...names].sort());
|
||||
});
|
||||
|
||||
it('listEnabled hides disabled skills', async () => {
|
||||
await registry.init();
|
||||
await registry.setEnabled('quote-finder', false);
|
||||
const out = await registry.listEnabled();
|
||||
expect(out.map((s) => s.id)).not.toContain('quote-finder');
|
||||
const all = await registry.list();
|
||||
expect(all.map((s) => s.id)).toContain('quote-finder');
|
||||
});
|
||||
|
||||
it('getById returns null for unknown ids', async () => {
|
||||
expect(await registry.getById('missing')).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('upsert / setEnabled / delete', () => {
|
||||
it('upsert creates a user skill with builtin=false by default', async () => {
|
||||
const out = await registry.upsert({
|
||||
id: 'my-skill',
|
||||
name: 'My Skill',
|
||||
description: 'whatever',
|
||||
instructions: 'do whatever',
|
||||
builtin: false,
|
||||
});
|
||||
expect(out.builtin).toBe(false);
|
||||
expect(out.enabled).toBe(true);
|
||||
});
|
||||
|
||||
it('upsert round-trips toolAllowlist as a string array', async () => {
|
||||
await registry.upsert({
|
||||
id: 'restricted',
|
||||
name: 'Restricted',
|
||||
description: 'd',
|
||||
instructions: 'i',
|
||||
toolAllowlist: ['lookupPassage', 'getReadingContext'],
|
||||
builtin: false,
|
||||
});
|
||||
const got = await registry.getById('restricted');
|
||||
expect(got?.toolAllowlist).toEqual(['lookupPassage', 'getReadingContext']);
|
||||
});
|
||||
|
||||
it('upsert replaces an existing row in place', async () => {
|
||||
await registry.init();
|
||||
await registry.upsert({
|
||||
id: 'spoiler-free',
|
||||
name: 'Replaced name',
|
||||
description: 'replaced',
|
||||
instructions: 'replaced instructions',
|
||||
});
|
||||
const after = await registry.getById('spoiler-free');
|
||||
expect(after?.name).toBe('Replaced name');
|
||||
expect(after?.instructions).toBe('replaced instructions');
|
||||
});
|
||||
|
||||
it('setEnabled flips the enabled flag and reports whether the row existed', async () => {
|
||||
await registry.init();
|
||||
expect(await registry.setEnabled('chapter-summary', false)).toBe(true);
|
||||
expect((await registry.getById('chapter-summary'))?.enabled).toBe(false);
|
||||
expect(await registry.setEnabled('does-not-exist', true)).toBe(false);
|
||||
});
|
||||
|
||||
it('delete removes the row and returns whether anything matched', async () => {
|
||||
await registry.init();
|
||||
expect(await registry.delete('spoiler-free')).toBe(true);
|
||||
expect(await registry.getById('spoiler-free')).toBeNull();
|
||||
expect(await registry.delete('spoiler-free')).toBe(false);
|
||||
});
|
||||
|
||||
it('init() re-plants a built-in skill that was deleted', async () => {
|
||||
await registry.init();
|
||||
await registry.delete('spoiler-free');
|
||||
const inserted = await registry.init();
|
||||
expect(inserted).toBe(1);
|
||||
expect(await registry.getById('spoiler-free')).not.toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolveActiveSkill', () => {
|
||||
it('returns null for null/undefined ids', async () => {
|
||||
expect(await registry.resolveActiveSkill(null)).toBeNull();
|
||||
expect(await registry.resolveActiveSkill(undefined)).toBeNull();
|
||||
});
|
||||
|
||||
it('returns null when the skill is disabled or unknown', async () => {
|
||||
await registry.init();
|
||||
await registry.setEnabled('quote-finder', false);
|
||||
expect(await registry.resolveActiveSkill('quote-finder')).toBeNull();
|
||||
expect(await registry.resolveActiveSkill('missing')).toBeNull();
|
||||
});
|
||||
|
||||
it('returns { id, instructions } for active skills (shape SkillLayer wants)', async () => {
|
||||
await registry.init();
|
||||
const out = await registry.resolveActiveSkill('spoiler-free');
|
||||
expect(out?.id).toBe('spoiler-free');
|
||||
expect(typeof out?.instructions).toBe('string');
|
||||
expect(out!.instructions.length).toBeGreaterThan(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('toolAllowlist corruption handling', () => {
|
||||
it('falls back to null when the stored JSON is malformed', async () => {
|
||||
// Bypass upsert to plant a row with bad JSON directly.
|
||||
await svc.execute(
|
||||
`INSERT INTO reedy_skills
|
||||
(id, name, description, instructions, tool_allowlist, builtin, enabled)
|
||||
VALUES (?, ?, ?, ?, ?, 0, 1)`,
|
||||
['bad-json', 'Bad', 'd', 'i', '{not valid json'],
|
||||
);
|
||||
const got = await registry.getById('bad-json');
|
||||
expect(got?.toolAllowlist).toBeNull();
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -121,6 +121,25 @@ const migrations: Record<SchemaType, MigrationEntry[]> = {
|
||||
ON reedy_memory (scope, scope_key, updated_at DESC);
|
||||
`,
|
||||
},
|
||||
{
|
||||
// Skill catalog for the agent runtime (Phase 5.1). Built-in skills
|
||||
// are seeded on first SkillRegistry boot; user-defined skills
|
||||
// (post-MVP) live in the same table with builtin=0. tool_allowlist
|
||||
// is a JSON-encoded string array applied by the runtime when
|
||||
// building the per-turn ToolSet.
|
||||
name: '2026052604_reedy_skills',
|
||||
sql: `
|
||||
CREATE TABLE IF NOT EXISTS reedy_skills (
|
||||
id TEXT PRIMARY KEY,
|
||||
name TEXT NOT NULL,
|
||||
description TEXT NOT NULL,
|
||||
instructions TEXT NOT NULL,
|
||||
tool_allowlist TEXT,
|
||||
builtin INTEGER NOT NULL DEFAULT 1,
|
||||
enabled INTEGER NOT NULL DEFAULT 1
|
||||
);
|
||||
`,
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
|
||||
@@ -0,0 +1,175 @@
|
||||
import type { DatabaseService } from '@/types/database';
|
||||
import type { SkillInstructions } from '../context/layers/SkillLayer';
|
||||
import { BUILTIN_SKILLS } from './builtins';
|
||||
import type { Skill, UpsertSkillArgs } from './types';
|
||||
|
||||
/**
|
||||
* DB-backed skill catalog (Phase 5.1).
|
||||
*
|
||||
* On first init() call the registry seeds the three built-in skills
|
||||
* (spoiler-free, chapter-summary, quote-finder) so a fresh reedy.db
|
||||
* always has them. Re-seeding is idempotent: builtin rows that already
|
||||
* exist are left alone (so user-edited instructions survive across
|
||||
* upgrades). User-defined skills (post-MVP) live in the same table with
|
||||
* builtin=0; the seeding pass only touches builtin=1 rows.
|
||||
*
|
||||
* The registry doesn't enforce the toolAllowlist itself — that's the
|
||||
* runtime's job when constructing the per-turn ToolSet (Phase 2.6 +
|
||||
* Phase 5 follow-up). This file just stores and retrieves.
|
||||
*/
|
||||
export class SkillRegistry {
|
||||
constructor(private readonly db: DatabaseService) {}
|
||||
|
||||
/**
|
||||
* Seed the built-in skills if they aren't present yet. Safe to call on
|
||||
* every app boot. Returns the count of newly inserted rows.
|
||||
*/
|
||||
async init(): Promise<number> {
|
||||
let inserted = 0;
|
||||
for (const skill of BUILTIN_SKILLS) {
|
||||
const existing = await this.getById(skill.id);
|
||||
if (existing) continue;
|
||||
await this.insert(skill);
|
||||
inserted++;
|
||||
}
|
||||
return inserted;
|
||||
}
|
||||
|
||||
async list(): Promise<Skill[]> {
|
||||
const rows = await this.db.select<SkillRowSql>(
|
||||
'SELECT * FROM reedy_skills ORDER BY builtin DESC, name ASC',
|
||||
);
|
||||
return rows.map(toSkill);
|
||||
}
|
||||
|
||||
async listEnabled(): Promise<Skill[]> {
|
||||
const rows = await this.db.select<SkillRowSql>(
|
||||
'SELECT * FROM reedy_skills WHERE enabled = 1 ORDER BY builtin DESC, name ASC',
|
||||
);
|
||||
return rows.map(toSkill);
|
||||
}
|
||||
|
||||
async getById(id: string): Promise<Skill | null> {
|
||||
const rows = await this.db.select<SkillRowSql>('SELECT * FROM reedy_skills WHERE id = ?', [id]);
|
||||
return rows[0] ? toSkill(rows[0]) : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Upsert a skill. Built-in skills always overwrite; user skills replace
|
||||
* the prior row when the id matches.
|
||||
*/
|
||||
async upsert(args: UpsertSkillArgs): Promise<Skill> {
|
||||
await this.db.execute(
|
||||
`INSERT INTO reedy_skills (id, name, description, instructions, tool_allowlist, builtin, enabled)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?)
|
||||
ON CONFLICT(id) DO UPDATE SET
|
||||
name = excluded.name,
|
||||
description = excluded.description,
|
||||
instructions = excluded.instructions,
|
||||
tool_allowlist = excluded.tool_allowlist,
|
||||
builtin = excluded.builtin,
|
||||
enabled = excluded.enabled`,
|
||||
[
|
||||
args.id,
|
||||
args.name,
|
||||
args.description,
|
||||
args.instructions,
|
||||
args.toolAllowlist ? JSON.stringify(args.toolAllowlist) : null,
|
||||
args.builtin === false ? 0 : 1,
|
||||
args.enabled === false ? 0 : 1,
|
||||
],
|
||||
);
|
||||
const row = await this.getById(args.id);
|
||||
if (!row) throw new Error(`SkillRegistry: row vanished after upsert id=${args.id}`);
|
||||
return row;
|
||||
}
|
||||
|
||||
/** Toggle the `enabled` flag without touching anything else. */
|
||||
async setEnabled(id: string, enabled: boolean): Promise<boolean> {
|
||||
const exists = await this.getById(id);
|
||||
if (!exists) return false;
|
||||
await this.db.execute('UPDATE reedy_skills SET enabled = ? WHERE id = ?', [
|
||||
enabled ? 1 : 0,
|
||||
id,
|
||||
]);
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Delete a skill. Built-in skills can be deleted too — `init()` will
|
||||
* re-seed them on next boot. Returns true if a row was removed.
|
||||
*/
|
||||
async delete(id: string): Promise<boolean> {
|
||||
const exists = await this.getById(id);
|
||||
if (!exists) return false;
|
||||
await this.db.execute('DELETE FROM reedy_skills WHERE id = ?', [id]);
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the active skill into the shape the Phase 2.5 SkillLayer
|
||||
* consumes. Returns null when the skill id is unknown, disabled, or
|
||||
* null/undefined — callers can then pass `null` to createSkillLayer.
|
||||
*/
|
||||
async resolveActiveSkill(id: string | null | undefined): Promise<SkillInstructions | null> {
|
||||
if (!id) return null;
|
||||
const skill = await this.getById(id);
|
||||
if (!skill || !skill.enabled) return null;
|
||||
return { id: skill.id, instructions: skill.instructions };
|
||||
}
|
||||
|
||||
private async insert(skill: Skill): Promise<void> {
|
||||
await this.db.execute(
|
||||
`INSERT INTO reedy_skills (id, name, description, instructions, tool_allowlist, builtin, enabled)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?)`,
|
||||
[
|
||||
skill.id,
|
||||
skill.name,
|
||||
skill.description,
|
||||
skill.instructions,
|
||||
skill.toolAllowlist ? JSON.stringify(skill.toolAllowlist) : null,
|
||||
skill.builtin ? 1 : 0,
|
||||
skill.enabled ? 1 : 0,
|
||||
],
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// helpers
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
interface SkillRowSql {
|
||||
id: string;
|
||||
name: string;
|
||||
description: string;
|
||||
instructions: string;
|
||||
tool_allowlist: string | null;
|
||||
builtin: number;
|
||||
enabled: number;
|
||||
[key: string]: unknown;
|
||||
}
|
||||
|
||||
function toSkill(row: SkillRowSql): Skill {
|
||||
let allowlist: string[] | null = null;
|
||||
if (row.tool_allowlist) {
|
||||
try {
|
||||
const parsed = JSON.parse(row.tool_allowlist);
|
||||
if (Array.isArray(parsed) && parsed.every((s) => typeof s === 'string')) {
|
||||
allowlist = parsed;
|
||||
}
|
||||
} catch {
|
||||
// Corrupted JSON — fall back to null (no allowlist).
|
||||
allowlist = null;
|
||||
}
|
||||
}
|
||||
return {
|
||||
id: row.id,
|
||||
name: row.name,
|
||||
description: row.description,
|
||||
instructions: row.instructions,
|
||||
toolAllowlist: allowlist,
|
||||
builtin: row.builtin === 1,
|
||||
enabled: row.enabled === 1,
|
||||
};
|
||||
}
|
||||
@@ -0,0 +1,19 @@
|
||||
import type { Skill } from '../types';
|
||||
|
||||
export const chapterSummarySkill: Skill = {
|
||||
id: 'chapter-summary',
|
||||
name: 'Chapter summary',
|
||||
description: 'Concise summary of the chapter the user is currently reading.',
|
||||
instructions: `You are in chapter-summary mode. Your job is to give the user a concise summary of the chapter they are currently reading.
|
||||
|
||||
Workflow:
|
||||
1. Call getReadingContext to find the current chapter title + section index.
|
||||
2. Call lookupPassage with a few queries targeting the chapter's key beats (e.g. its title; "what happens at the start of <chapter>"; "main themes of <chapter>"). Limit to top 5 per call.
|
||||
3. Synthesize a 3-5 sentence summary that covers what happened in the chapter, what the chapter introduced or resolved, and the tone/mood. Cite each major point by CFI.
|
||||
|
||||
Stay grounded in retrieved passages — never invent plot. If retrieval comes back empty (status != 'ok'), repeat the hint the tool returned.`,
|
||||
// Read-only — this skill never writes or navigates.
|
||||
toolAllowlist: ['getReadingContext', 'lookupPassage', 'addCitation'],
|
||||
builtin: true,
|
||||
enabled: true,
|
||||
};
|
||||
@@ -0,0 +1,9 @@
|
||||
import { chapterSummarySkill } from './chapterSummary';
|
||||
import { quoteFinderSkill } from './quoteFinder';
|
||||
import { spoilerFreeSkill } from './spoilerFree';
|
||||
import type { Skill } from '../types';
|
||||
|
||||
/** The three v1 seed skills SkillRegistry plants on first boot. */
|
||||
export const BUILTIN_SKILLS: Skill[] = [spoilerFreeSkill, chapterSummarySkill, quoteFinderSkill];
|
||||
|
||||
export { spoilerFreeSkill, chapterSummarySkill, quoteFinderSkill };
|
||||
@@ -0,0 +1,19 @@
|
||||
import type { Skill } from '../types';
|
||||
|
||||
export const quoteFinderSkill: Skill = {
|
||||
id: 'quote-finder',
|
||||
name: 'Quote finder',
|
||||
description: 'Finds passages matching a description and surfaces them with CFI citations.',
|
||||
instructions: `You are in quote-finder mode. The user is looking for a specific passage in the book — exact quote, paraphrase, or thematic match.
|
||||
|
||||
Workflow:
|
||||
1. If the user already selected text (via getSelection), use it as the seed and search for similar/related passages.
|
||||
2. Call lookupPassage with the user's query (or the selection) using topK=5.
|
||||
3. Return EVERY useful passage as a citation, with a one-line note on why each is relevant. Don't filter aggressively — the user is browsing, not asking for a single answer.
|
||||
4. If status='not_indexed' or 'stale_index', repeat the hint verbatim.
|
||||
|
||||
Never paraphrase the book content — keep quotes literal. Use addCitation only if you remember a passage with a CFI from earlier in the session that didn't show up in the search.`,
|
||||
toolAllowlist: ['getReadingContext', 'getSelection', 'lookupPassage', 'addCitation'],
|
||||
builtin: true,
|
||||
enabled: true,
|
||||
};
|
||||
@@ -0,0 +1,18 @@
|
||||
import type { Skill } from '../types';
|
||||
|
||||
export const spoilerFreeSkill: Skill = {
|
||||
id: 'spoiler-free',
|
||||
name: 'Spoiler-free',
|
||||
description:
|
||||
'Answers only from material the user has already read, never from later parts of the book.',
|
||||
instructions: `You are answering in spoiler-free mode. The user's current reading position is in the reading context (look for "Page" and "CFI"). Never reference plot points, character developments, or themes from later in the book than the user's current position.
|
||||
|
||||
When you call lookupPassage, set spoilerBoundPosition to the user's current page. When you call any other tool that returns book content, you must self-filter: drop anything that mentions events past the user's current location.
|
||||
|
||||
If the user asks about something that would necessarily spoil future content, tell them you can't answer without spoiling and ask if they'd like to disable spoiler protection for this question.`,
|
||||
// No allowlist — the skill works with the full tool catalog; it just
|
||||
// changes how those tools are used.
|
||||
toolAllowlist: null,
|
||||
builtin: true,
|
||||
enabled: true,
|
||||
};
|
||||
@@ -0,0 +1,31 @@
|
||||
/**
|
||||
* A skill is a persisted bundle of instructions + optional tool allowlist
|
||||
* the user picks from the Composer chip row. The active skill's
|
||||
* instructions are pushed through the Phase 2.5 SkillLayer; the
|
||||
* allowlist (if any) filters the per-turn ToolSet so the model only
|
||||
* sees the relevant subset.
|
||||
*/
|
||||
export interface Skill {
|
||||
id: string;
|
||||
name: string;
|
||||
description: string;
|
||||
instructions: string;
|
||||
/**
|
||||
* When non-null, the runtime restricts the ToolSet to these tool names.
|
||||
* `null` means every registered tool is available.
|
||||
*/
|
||||
toolAllowlist: string[] | null;
|
||||
/** Seeded by SkillRegistry on first boot; user-defined skills set false. */
|
||||
builtin: boolean;
|
||||
enabled: boolean;
|
||||
}
|
||||
|
||||
export interface UpsertSkillArgs {
|
||||
id: string;
|
||||
name: string;
|
||||
description: string;
|
||||
instructions: string;
|
||||
toolAllowlist?: string[] | null;
|
||||
builtin?: boolean;
|
||||
enabled?: boolean;
|
||||
}
|
||||
Reference in New Issue
Block a user