feat: Trust Score System — Phase 1 (feedback, fetch tracking, skill stats)

[ComeOnOliver]

Summary

Phase 1 of the SkillsHub trust/rating system, inspired by Netflix (implicit+explicit feedback), Stack Overflow (reputation), and npm (usage counts).

What's New

New Endpoints

• POST /api/v1/skills/{id}/feedback — agents submit helpful/not-helpful feedback (auth required, deduplicated per agent per skill per day)
• GET /api/v1/skills/{id}/stats — public trust stats (fetchCount, helpfulRate, feedbackCount)

Implicit Tracking

• Markdown fetch endpoint now tracks fetch counts (fire-and-forget, doesn't block response)
• skill_events table logs all events for future analysis

Schema Changes

• New tables: skill_events, skill_feedback
• New columns on skills: trust_score, fetch_count, helpful_rate, feedback_count
• New column on users: reputation
• Migration script: packages/db/src/migrate-trust.ts

Resolve/Detail Enhancement

• trustScore and helpfulRate included in resolve and skill detail responses
• Resolve scoring gives bonus to skills with high helpfulRate (when feedbackCount >= 5)

Anti-Gaming

• Auth required for feedback
• One feedback per agent per skill per 24h (upsert on duplicate)
• Rate limited via existing API rate limiter

Design Doc

Full design: https://github.com/ComeOnOliver/skillshub/blob/feat/trust-score-phase1/packages/db/src/migrate-trust.ts

Testing

• TypeScript compiles clean
• Schema changes are additive (no breaking changes)
• Migration is idempotent (safe to run multiple times)

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
skillshub	Ready	Preview, Comment	Mar 17, 2026 7:04pm

[ComeOnOliver]

🔍 Code Review — PR #16: Trust Score System Phase 1

Verdict: 🟡 Request Changes — Solid foundation, but a few issues need fixing before merge.

---

✅ What looks good

• Clean separation of concerns: feedback, stats, and events as distinct endpoints
• Zod validation on POST input with proper error formatting
• UUID regex validation before hitting the DB
• Dedupe logic (one feedback per agent per skill per day) is smart
• Fire-and-forget pattern for non-critical writes is appropriate
• Schema design is reasonable — denormalized feedbackCount/helpfulRate on skills table for fast reads is the right call
• Migration script is idempotent (IF NOT EXISTS everywhere)
• CORS properly exported on new endpoints

---

🔴 Critical Issues (must fix)

1. skill_events table will grow unbounded with no cleanup strategy

Every single raw-skill fetch inserts a row into skill_events. This is an append-only audit log with no TTL, no partitioning, and no retention policy. At scale this table will become massive and degrade query performance.

Recommendation: At minimum, add a comment/TODO about a cleanup cron. Better: add a created_at index that supports range deletes, or use time-based partitioning.

2. helpful_rate stored as NUMERIC(3,2) — max value is 9.99, not 1.00

helpful_rate NUMERIC(3,2)

NUMERIC(3,2) means 3 total digits, 2 after decimal → max 9.99. This works for a 0.00–1.00 rate, but the precision declaration is misleading. If anyone ever stores a percentage (0–100) here by mistake, it will silently truncate. The column name + type should make intent obvious.

Recommendation: Either use NUMERIC(4,3) for 0.000–1.000 range, or document clearly that this is a ratio not a percentage. Also validate in application code before writing.

3. Race condition in feedback POST: read-then-write on skillFeedback

const [existing] = await db.select(...)  // check if exists
if (existing) { await db.update(...) }   // update
else { await db.insert(...) }            // insert

Two concurrent requests from the same agent on the same day will both see existing = null and both try to insert. The unique index skill_feedback_daily_idx will catch one, but the second will throw an unhandled DB error that returns a 500.

Recommendation: Use INSERT ... ON CONFLICT (skill_id, agent_id, (created_at::date)) DO UPDATE SET ... (upsert). Drizzle supports this via .onConflictDoUpdate(). This eliminates the race and the extra SELECT.

4. Fire-and-forget error swallowing in raw-skill route

db.update(skills).set({ fetchCount: ... }).execute().catch(() => {});
db.insert(skillEvents).values(...).execute().catch(() => {});

Silently swallowing errors is fine for non-critical ops, but you should at minimum log them. A persistent DB connection issue would be completely invisible.

Recommendation: .catch((err) => console.error("fetch tracking failed:", err)) at minimum.

5. trustScore on skills table is added but never computed anywhere

The column exists, it's returned in API responses, and it's used in the resolve endpoint's output — but nothing in this PR ever writes to it. The resolve endpoint uses feedbackBonus in its in-memory scoring but never persists it to trustScore.

Recommendation: Either compute and persist trustScore (even if just = feedbackBonus for Phase 1), or don't expose it in the API yet. Returning a field that's always 0 is confusing for consumers.

---

🟡 Suggestions (nice to have)

6. Feedback recomputation does a full table scan per POST

const [stats] = await db.select({ totalFeedback: sql`count(*)::int`, ... })
  .from(skillFeedback).where(eq(skillFeedback.skillId, id));

After every feedback submission, you count ALL feedback for that skill. This is fine at small scale but will degrade. Consider incrementing/decrementing counters instead, or computing asynchronously.

7. context field on feedback is an open TEXT column in the DB but an enum in the API

The Zod schema validates context as "resolve" | "search" | "direct", but the DB column is plain TEXT. Anyone with direct DB access (admin, migration, other service) could insert arbitrary values. Consider a CHECK constraint or text("context", { enum: [...] }) in Drizzle.

8. reputation column on users is added but unused

The migration adds users.reputation (integer, default 0) and the schema declares it, but nothing reads or writes it. Dead code in schema.

Recommendation: Remove until Phase 2 needs it, or add a TODO comment linking to the Phase 2 issue.

9. The resolve endpoint now returns trustScore and helpfulRate in results but doesn't document the change

The /api/v1/skills/resolve response shape changed (new fields). The API guide in route.ts should document these new response fields.

10. feedbackBonus threshold of 5 feedbacks is a magic number

if (skill.helpfulRate !== null && skill.feedbackCount >= 5) {
  feedbackBonus = Number(skill.helpfulRate) * 10;
}

Extract 5 to a named constant like MIN_FEEDBACK_FOR_BONUS. Also, this means a skill with 5 feedback entries that are all helpful gets +10 score, same as one with 500 — consider a log-scale or confidence interval.

11. No rate limiting on feedback POST

The daily dedupe limits to 1 per agent per skill per day, but a malicious agent could submit feedback to every skill in the registry in a loop. Consider per-agent rate limiting (e.g., max 50 feedbacks per day total).

12. Migration script uses raw pg.Client instead of Drizzle migrations

This works, but it's a parallel migration system. If the rest of the project uses Drizzle's migration tooling, this will cause drift. If not, that's fine — but document the convention.

---

❓ Questions

1. Why denormalize fetchCount/feedbackCount/helpfulRate on the skills table AND keep skillEvents/skillFeedback as source-of-truth tables? The denormalized fields can drift from the source tables if a bug skips the recomputation step. Is there a reconciliation plan?

2. Is trustScore on skills intended to be computed by a separate cron/worker in Phase 2? If so, it would help to document that intent in a code comment.

3. Why no ON DELETE CASCADE on skillEvents.skillId and skillFeedback.skillId? If a skill is deleted, orphaned feedback/events will linger. The stars table uses onDelete: "cascade" — should be consistent.

4. The raw-skill route now does 3 fire-and-forget writes per request (downloadCount, fetchCount, skillEvents insert). At high traffic, this is 3x write amplification. Any plans to batch or queue these?

---

Summary

The architecture is sound — denormalized stats for reads, event sourcing for audit, feedback with dedupe. But the race condition (#3), the never-computed trustScore (#5), and the silent error swallowing (#4) need to be addressed before merge. The upsert fix (#3) is a one-line change that eliminates both the race and an unnecessary SELECT.

Good work overall. Fix the criticals and this is ready to ship. 🚢