fix(groomer): keep ready issues in a claimable lane (#492)#493
Conversation
The groomer LLM sometimes returns actionability=ready with lane=backlog, conflating low priority with the non-claimable backlog lane. Ready issues then sit in a lane no worker queue can see. Enforce the invariant deterministically: when actionability is 'ready', coerce a non-claimable lane to the default claimable lane (recorded as an 'invariant' resolution), and tighten the groomer prompt to say ready => claimable worker lane, never backlog. Lane-agnostic via lane-config helpers (respects DISPATCH_LANE_CONFIG_JSON). Fixes #492
There was a problem hiding this comment.
AI Automated Review
Full PR review.
Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic) — routed smart (risk match: db_or_migration_changes)
PR PR 493 Review: fix(groomer): keep ready issues in a claimable lane
Recommendation: APPROVE
Summary
This PR fixes issue PR 492 where the groomer was incorrectly assigning actionability: "ready" issues to the non-claimable backlog lane, making them invisible to worker queues.
Required Checks
1. review migration for data loss risk
✅ NOT APPLICABLE — This PR contains zero database migrations or schema changes. The modified files are:
src/lib/groomer/enum-config.ts— adds"invariant"to a TypeScript union typesrc/lib/groomer/llm.ts— updates an LLM system prompt stringsrc/lib/groomer/schema.ts— adds validation logic that operates on in-memory groomer outputsrc/lib/groomer/schema.test.ts— adds unit tests
No Prisma schema was touched, no migration files exist in the diff, and no database writes occur. There is zero data loss risk.
2. test migration on a copy of production schema
✅ NOT APPLICABLE — No migrations exist in this PR to test. This is a pure TypeScript logic change to groomer validation, not a database change.
Change-by-Change Review
| File | Change | Assessment |
|---|---|---|
enum-config.ts |
Adds "invariant" to ResolutionEvent.source union type |
✅ Correct — documents the new resolution source |
llm.ts |
Adds prompt rule: when actionability: "ready", lane must be claimable, never backlog |
✅ Correct — attempts to prevent the bug upstream in LLM output |
schema.ts |
Adds cross-field invariant: coerces ready+non-claimable-lane → default claimable lane, records invariant resolution |
✅ Correct — deterministic safety net that catches any remaining LLM misassignments |
schema.test.ts |
3 new test cases covering the coercion logic | ✅ Correct — covers happy path (ready→claimable stays), fix path (ready→backlog coerced), and guard path (non-ready→backlog untouched) |
Standards Compliance
- ✅ Uses
lane-confighelpers (isClaimableLane,getDefaultClaimableLane,getBacklogLane) — respectsDISPATCH_LANE_CONFIG_JSON, no hardcoded lane names - ✅ Follows existing
ResolutionEventpattern for structured resolution recording - ✅ TypeScript types updated consistently
- ✅ No secrets or environment variables modified
Linked Issue Fit
| Issue PR 492 Requirement | How PR Satisfies It |
|---|---|
Ready issues should be in local, cloud, or frontier — never backlog |
✅ Prompt explicitly tells LLM this; invariant coercion enforces it |
| Backlog lane only for non-ready issues | ✅ Invariant preserves needs_info/blocked/backlog/already_done+backlog combinations |
| Low priority (P2/P3) ≠ backlog | ✅ Prompt explicitly states: "low-priority but ready issue still goes to a claimable lane" |
The fix is layered correctly: prompt guidance (prevention) + validation coercion (detection/correction).
Evidence Provider Findings
No evidence providers configured for this review.
Tool Harness Findings
No tool calls issued; reviewing corpus directly.
CI Check Results
All CI checks passed: ✅ Tests, ✅ Typecheck, ✅ Build, ✅ Lint, ✅ Docker Build
Unknowns / Needs Verification
None — the PR is self-contained TypeScript logic with comprehensive test coverage, no external dependencies, and no database impact.
Summary
Fixes #492 — the groomer stranding
status/readyissues in the non-claimablebackloglane, invisible to worker queues.schema.ts— deterministic cross-field invariant: whenactionability === "ready"but the lane is non-claimable, coerce to the default claimable lane and record aninvariantresolution. This is the single source of truthrun.tsconsumes, so the fix applies everywhere the groom result is used.llm.ts— prompt now states: when actionability isready,lane.idmust be a claimable worker lane, never the backlog lane; low priority (P2/P3) ≠ backlog.enum-config.ts—ResolutionEvent.sourcegains"invariant".lane-confighelpers (isClaimableLane,getDefaultClaimableLane), so it respectsDISPATCH_LANE_CONFIG_JSON— no hardcoded lane names.Verification
vitest run src/lib/groomer→ 187 passed (3 new: coerce ready+backlog→claimable, leave ready+claimable untouched, leave non-ready+backlog untouched).tscerrors (groomingRunmissing on PrismaClient) are unrelated to this change —prisma generategap in local checkout, untouched files.