From 1a1c3ce399ab87b4c829a3f25221a01f6389b4ae Mon Sep 17 00:00:00 2001 From: Mikkel Georgsen Date: Wed, 1 Apr 2026 04:05:49 +0200 Subject: [PATCH] feat(12-01): personalRatings schema, DB DDL, skillRatingService, and tests - Add personalRatings table to skill-registry-schema.ts - Add taskCount, avgCostUsd, lastUsedAt columns to agentSkills in schema - Add CREATE_PERSONAL_RATINGS_TABLE DDL constant in skill-registry-db.ts - Add ALTER TABLE statements for new agent_skills usage columns (idempotent) - Create skill-registry-ratings.ts with skillRatingService factory - rate() appends personal rating, validates stars 1-5 - getRatings() returns ratings ordered by createdAt DESC - recordUsageForAgent() atomically updates task_count, avg_cost_usd, last_used_at - All 8 tests pass --- .../__tests__/skill-registry-ratings.test.ts | 122 ++++++++++++++++++ server/src/services/skill-registry-db.ts | 26 ++++ server/src/services/skill-registry-ratings.ts | 99 ++++++++++++++ server/src/services/skill-registry-schema.ts | 13 ++ 4 files changed, 260 insertions(+) create mode 100644 server/src/__tests__/skill-registry-ratings.test.ts create mode 100644 server/src/services/skill-registry-ratings.ts diff --git a/server/src/__tests__/skill-registry-ratings.test.ts b/server/src/__tests__/skill-registry-ratings.test.ts new file mode 100644 index 00000000..f266c33e --- /dev/null +++ b/server/src/__tests__/skill-registry-ratings.test.ts @@ -0,0 +1,122 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { mkdtemp, rm } from "node:fs/promises"; +import path from "node:path"; +import os from "node:os"; +import { getSkillRegistryDb, resetSkillRegistryDb } from "../services/skill-registry-db.js"; +import { agentSkills } from "../services/skill-registry-schema.js"; +import { skillRatingService } from "../services/skill-registry-ratings.js"; + +// --------------------------------------------------------------------------- +// Test setup +// --------------------------------------------------------------------------- + +let tmpHome: string; + +beforeEach(async () => { + tmpHome = await mkdtemp(path.join(os.tmpdir(), "nexus-ratings-test-")); + process.env.PAPERCLIP_HOME = tmpHome; + resetSkillRegistryDb(); +}); + +afterEach(async () => { + resetSkillRegistryDb(); + delete process.env.PAPERCLIP_HOME; + await rm(tmpHome, { recursive: true, force: true }); +}); + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe("skillRatingService", () => { + const svc = skillRatingService(); + + it("Test 1: rate() inserts a personal_ratings row; getRatings(skillId) returns it with correct fields", async () => { + await svc.rate({ skillId: "src/skill-a", versionId: "v1", stars: 4, note: "Great skill" }); + const ratings = await svc.getRatings("src/skill-a"); + expect(ratings).toHaveLength(1); + expect(ratings[0]!.skillId).toBe("src/skill-a"); + expect(ratings[0]!.versionId).toBe("v1"); + expect(ratings[0]!.stars).toBe(4); + expect(ratings[0]!.note).toBe("Great skill"); + }); + + it("Test 2: rate() called twice for same skill creates two separate rows (append-only)", async () => { + await svc.rate({ skillId: "src/skill-b", stars: 3 }); + await svc.rate({ skillId: "src/skill-b", stars: 5, note: "Even better now" }); + const ratings = await svc.getRatings("src/skill-b"); + expect(ratings).toHaveLength(2); + }); + + it("Test 3: getRatings() returns results ordered by created_at descending (newest first)", async () => { + await svc.rate({ skillId: "src/skill-c", stars: 2, note: "First" }); + // Small delay to ensure different timestamps + await new Promise((r) => setTimeout(r, 5)); + await svc.rate({ skillId: "src/skill-c", stars: 5, note: "Second" }); + + const ratings = await svc.getRatings("src/skill-c"); + expect(ratings).toHaveLength(2); + // Newest first — the second rating should be first in results + expect(ratings[0]!.note).toBe("Second"); + expect(ratings[1]!.note).toBe("First"); + }); + + it("Test 4: recordUsageForAgent() increments task_count from 0 to 1 for all agent_skills rows of the agent", async () => { + const db = await getSkillRegistryDb(); + const now = Date.now(); + // Insert two agent_skills rows for the same agent + await db.insert(agentSkills).values([ + { agentId: "agent-1", skillId: "src/skill-x", installedAt: now }, + { agentId: "agent-1", skillId: "src/skill-y", installedAt: now }, + ]); + + await svc.recordUsageForAgent("agent-1", null); + + const { eq } = await import("drizzle-orm"); + const rows = await db.select().from(agentSkills).where(eq(agentSkills.agentId, "agent-1")); + for (const row of rows) { + expect((row as any).taskCount ?? (row as any).task_count).toBe(1); + } + }); + + it("Test 5: recordUsageForAgent() computes running average: after two calls with costs 0.01 and 0.03, avg_cost_usd is 0.02", async () => { + const db = await getSkillRegistryDb(); + const now = Date.now(); + await db.insert(agentSkills).values({ agentId: "agent-2", skillId: "src/skill-z", installedAt: now }); + + await svc.recordUsageForAgent("agent-2", 0.01); + await svc.recordUsageForAgent("agent-2", 0.03); + + const { eq } = await import("drizzle-orm"); + const rows = await db.select().from(agentSkills).where(eq(agentSkills.agentId, "agent-2")); + const row = rows[0] as any; + const avgCost = row.avgCostUsd ?? row.avg_cost_usd; + expect(avgCost).toBeCloseTo(0.02, 5); + }); + + it("Test 6: recordUsageForAgent() sets last_used_at to approximately Date.now()", async () => { + const db = await getSkillRegistryDb(); + const now = Date.now(); + await db.insert(agentSkills).values({ agentId: "agent-3", skillId: "src/skill-w", installedAt: now }); + + const before = Date.now(); + await svc.recordUsageForAgent("agent-3", null); + const after = Date.now(); + + const { eq } = await import("drizzle-orm"); + const rows = await db.select().from(agentSkills).where(eq(agentSkills.agentId, "agent-3")); + const row = rows[0] as any; + const lastUsedAt = row.lastUsedAt ?? row.last_used_at; + expect(lastUsedAt).toBeGreaterThanOrEqual(before); + expect(lastUsedAt).toBeLessThanOrEqual(after); + }); + + it("Test 7: recordUsageForAgent() does nothing (no throw) when agent has no agent_skills rows", async () => { + await expect(svc.recordUsageForAgent("nonexistent-agent", 0.05)).resolves.not.toThrow(); + }); + + it("Test 8: rate() with stars=0 or stars=6 throws validation error", async () => { + await expect(svc.rate({ skillId: "src/skill-v", stars: 0 })).rejects.toThrow(); + await expect(svc.rate({ skillId: "src/skill-v", stars: 6 })).rejects.toThrow(); + }); +}); diff --git a/server/src/services/skill-registry-db.ts b/server/src/services/skill-registry-db.ts index 63c078e4..a5468d31 100644 --- a/server/src/services/skill-registry-db.ts +++ b/server/src/services/skill-registry-db.ts @@ -91,6 +91,17 @@ CREATE TABLE IF NOT EXISTS agent_skills ( PRIMARY KEY (agent_id, skill_id) )`; +const CREATE_PERSONAL_RATINGS_TABLE = ` +CREATE TABLE IF NOT EXISTS personal_ratings ( + id TEXT PRIMARY KEY, + skill_id TEXT NOT NULL, + version_id TEXT, + stars INTEGER NOT NULL, + note TEXT, + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL +)`; + const BUILTIN_GROUPS = [ { id: "builtin/pm-essentials", @@ -148,6 +159,21 @@ export async function getSkillRegistryDb(): Promise { await client.execute(CREATE_SKILL_GROUP_INHERITANCE_TABLE); await client.execute(CREATE_AGENT_SKILL_GROUPS_TABLE); await client.execute(CREATE_AGENT_SKILLS_TABLE); + await client.execute(CREATE_PERSONAL_RATINGS_TABLE); + + // Add usage-tracking columns to agent_skills if they don't exist yet (idempotent) + const agentSkillsAlters = [ + `ALTER TABLE agent_skills ADD COLUMN task_count INTEGER NOT NULL DEFAULT 0`, + `ALTER TABLE agent_skills ADD COLUMN avg_cost_usd REAL`, + `ALTER TABLE agent_skills ADD COLUMN last_used_at INTEGER`, + ]; + for (const sql of agentSkillsAlters) { + try { + await client.execute(sql); + } catch { + // Column already exists — ignore + } + } await seedBuiltinGroups(client); diff --git a/server/src/services/skill-registry-ratings.ts b/server/src/services/skill-registry-ratings.ts new file mode 100644 index 00000000..277c599b --- /dev/null +++ b/server/src/services/skill-registry-ratings.ts @@ -0,0 +1,99 @@ +import { eq, desc } from "drizzle-orm"; +import { getSkillRegistryDb } from "./skill-registry-db.js"; +import { personalRatings, agentSkills } from "./skill-registry-schema.js"; + +// --------------------------------------------------------------------------- +// Types +// --------------------------------------------------------------------------- + +type RateOpts = { + skillId: string; + versionId?: string | null; + stars: number; + note?: string | null; +}; + +type PersonalRatingRow = typeof personalRatings.$inferSelect; + +// --------------------------------------------------------------------------- +// Factory +// --------------------------------------------------------------------------- + +/** + * Skill rating service factory. + * Manages personal ratings and usage tracking in the skill registry libSQL DB. + */ +export function skillRatingService() { + return { + /** + * Record a personal rating (1-5 stars) for a skill. + * Always appends — never upserts — so rating history is preserved. + */ + async rate(opts: RateOpts): Promise { + if (opts.stars < 1 || opts.stars > 5) { + throw new RangeError(`stars must be between 1 and 5, got ${opts.stars}`); + } + const db = await getSkillRegistryDb(); + const now = Date.now(); + await db.insert(personalRatings).values({ + id: crypto.randomUUID(), + skillId: opts.skillId, + versionId: opts.versionId ?? null, + stars: opts.stars, + note: opts.note ?? null, + createdAt: now, + updatedAt: now, + }); + }, + + /** + * Get all personal ratings for a skill, ordered by createdAt descending (newest first). + */ + async getRatings(skillId: string): Promise { + const db = await getSkillRegistryDb(); + return db + .select() + .from(personalRatings) + .where(eq(personalRatings.skillId, skillId)) + .orderBy(desc(personalRatings.createdAt)); + }, + + /** + * Record a heartbeat run completion for all skills installed by an agent. + * Increments task_count, updates running average cost, and sets last_used_at. + * Safe to call when agent has no skills (no-op). + * + * @param agentId - the agent that just completed a successful run + * @param costUsd - total cost of the run in USD, or null if unknown + */ + async recordUsageForAgent(agentId: string, costUsd: number | null): Promise { + const db = await getSkillRegistryDb(); + const client = db.$client as import("@libsql/client").Client; + const now = Date.now(); + + if (costUsd !== null) { + // Atomic update with running average calculation + await client.execute({ + sql: `UPDATE agent_skills + SET task_count = task_count + 1, + avg_cost_usd = CASE + WHEN task_count = 0 THEN ? + ELSE (COALESCE(avg_cost_usd, 0) * task_count + ?) / (task_count + 1) + END, + last_used_at = ? + WHERE agent_id = ?`, + args: [costUsd, costUsd, now, agentId], + }); + } else { + // Skip avg_cost_usd update when cost is unknown + await client.execute({ + sql: `UPDATE agent_skills + SET task_count = task_count + 1, + last_used_at = ? + WHERE agent_id = ?`, + args: [now, agentId], + }); + } + }, + }; +} diff --git a/server/src/services/skill-registry-schema.ts b/server/src/services/skill-registry-schema.ts index aaa6d27b..52f30d6b 100644 --- a/server/src/services/skill-registry-schema.ts +++ b/server/src/services/skill-registry-schema.ts @@ -73,6 +73,19 @@ export const agentSkills = sqliteTable("agent_skills", { agentId: text("agent_id").notNull(), skillId: text("skill_id").notNull(), installedAt: integer("installed_at").notNull(), + taskCount: integer("task_count").notNull().default(0), + avgCostUsd: real("avg_cost_usd"), + lastUsedAt: integer("last_used_at"), }, (t) => ({ pk: primaryKey({ columns: [t.agentId, t.skillId] }), })); + +export const personalRatings = sqliteTable("personal_ratings", { + id: text("id").primaryKey(), + skillId: text("skill_id").notNull(), + versionId: text("version_id"), + stars: integer("stars").notNull(), // 1-5 + note: text("note"), + createdAt: integer("created_at").notNull(), + updatedAt: integer("updated_at").notNull(), +});