refactor(groomer): schema-driven enum validation with structured alias resolution

[itsmiso-ai]

Why

PR #475 was closed because it fixed the immediate symptom (lane id normal rejected) but didn't address why it happened. The hand-rolled validateGroomerOutput used brittle Array.includes checks that only worked for specific enum fields and silently swallowed alias resolution.

This PR replaces it with a schema-driven validator that generalizes across all enum fields.

What changed

New files

• src/lib/groomer/enum-config.ts — Generic EnumConfig<T> interface + resolveEnumConfig() helper. Defines the contract for declaring enum fields: valid values, aliases (static or lazy), and dynamic resolvers.
• src/lib/groomer/enum-configs.ts — GROOMER_ENUM_CONFIGS registry covering all groomer output enum fields (lane.id, lane.confidence, confidence, actionability, nextGroomingAction). Status/priority/type labels excluded (prefix-validated, not closed enums).

Modified files

• src/lib/groomer/schema.ts — Rewrote validateGroomerOutput to:

  • Use zod for structural validation (required fields, types)
  • Walk the enum registry for each enum field with alias resolution
  • Return ValidationResult with new resolutions?: ResolutionEvent[] array
  • Mutate parsed output to canonical values when aliases fire (same as before, now observable)
  • Preserve null tolerance for optional string fields and isAllowedLabel prefix logic

• src/lib/groomer/run.ts — After validation, appends structured warnings to contextWarnings:

  enum:lane.id: resolved 'normal' -> 'local' via alias
  

  Grep-friendly and machine-parseable without changing the contextWarnings: string[] type.

• src/lib/groomer/schema.test.ts — 34 → 44 tests (+10):

  • Lane alias resolution (normal → local, escalated → frontier)
  • Canonical lane (no spurious resolution event)
  • Unknown lane (validation error, no resolution event)
  • Multiple enum fields with aliases in one output
  • Empty aliases map behavior
  • resolutions array presence on valid/invalid/early-return paths

Design decisions

1. EnumConfig registry pattern — Anyone adding a new enum field just appends to GROOMER_ENUM_CONFIGS. No validator code changes needed. Aliases can be static or lazy (functions), supporting env-driven configs like lane topology.

2. resolutions is informational, not an error — Alias resolution succeeds; it's just worth observing. The ResolutionEvent struct records field, rawValue, resolvedValue, and source: "alias" for downstream tooling.

3. Zod for structure, custom logic for enums — Zod validates the shape (required fields, types). Enum validation with alias resolution uses a targeted helper (validateEnumValue) that returns { error?, resolvedValue? }. This keeps zod clean while getting structured alias handling.

4. Preserved all existing behavior — Null tolerance, isAllowedLabel prefix validation, error messages, and the public API shape ({ valid, parsed?, errors? }) are maintained. Only grew with resolutions?.

Checklist

• All groomer tests pass (164 across 8 files)
• npm run typecheck passes
• npm run lint passes
• No new dependencies (uses existing zod)
• System prompt in llm.ts untouched

[its-saffron]

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)

Review: PR PR 476 — Schema-Driven Enum Validation with Structured Alias Resolution

Recommendation: Approve

This is a pure TypeScript refactor of the groomer's validation layer. It does not modify the Prisma schema, add migrations, or alter database operations in any way. The db_or_migration_changes classification appears to be a false positive for this PR.

---

Change-by-Change Findings

New files — src/lib/groomer/enum-config.ts and enum-configs.ts

• enum-config.ts introduces EnumConfig<T>, ResolvedEnumConfig<T>, ResolutionEvent, and resolveEnumConfig(). Clean generic interface; aliases support both static objects and lazy getters.
• enum-configs.ts registers configs for all groomer enum fields (lane.id, lane.confidence, confidence, actionability, nextGroomingAction). Status/priority/type labels are correctly excluded since they use prefix-based isAllowedLabel validation.
• lane.id uses lazy resolution (resolveValidValues + aliases as functions) so it reads from getConfiguredLanes() / getLaneAliases() at validation time, making it lane-config-aware.

Modified — src/lib/groomer/schema.ts

• Replaces the hand-rolled Array.includes validation with a validateEnumValue() helper that checks validValues first, then aliases, then errors. This fixes the root cause identified in PR PR 475.
• Uses a minimal Zod baseSchema for structural validation (required fields, types) while keeping enum logic in custom code — a reasonable split.
• Returns ValidationResult with new resolutions?: ResolutionEvent[] array; resolutions is always present on the return object (empty array on success, non-array only on early-return for null/non-object input — correctly tested).
• All existing behavior preserved: null tolerance on optional string fields, isAllowedLabel prefix logic, error message shapes, public API surface.

Modified — src/lib/groomer/run.ts

• After validation, iterates validation.resolutions and appends structured warning strings (enum:lane.id: resolved 'normal' -> 'local' via alias) to contextWarnings. Grep-friendly, machine-parseable, no type changes needed.

Modified — src/lib/groomer/schema.test.ts

• +10 new tests covering: alias resolution (normal→local, escalated→frontier), canonical lane (no spurious resolution event), unknown lane (validation error, no resolution event), multiple enum fields, empty aliases map, resolutions array presence on all return paths.

---

Required Checks — Explicitly Addressed

1. Review migration for data loss risk — Does not apply. This PR contains zero database schema changes, migrations, or data mutations. It is a pure TypeScript refactor of in-memory validation logic. No prisma/schema.prisma modifications, no .sql migration files, no Prisma model changes.

2. Test migration on a copy of production schema — Does not apply. Same rationale as above: no migration exists to test. The validation runs entirely in application code (validateGroomerOutput) with no persistent state changes.

---

Standards Compliance

• No new dependencies: Uses existing zod — confirmed in PR checklist and not added to package.json.
• Code standards: Error handling uses instanceof-compatible patterns; input validation occurs before database operations (though no DB operations are involved here); no secrets committed.
• Label conventions: status/ / priority/ / type/ prefix validation is preserved exactly via isAllowedLabel; agent/* labels remain blocked.
• Null tolerance: Optional string fields (summary, githubComment, needsInfoReason, blockedReason) continue to treat null as absent.
• API shape: validateGroomerOutput returns { valid, parsed?, errors?, resolutions? } — backward-compatible extension with new optional resolutions field.

---

Linked Issue Fit

No linked issues. The PR body references PR PR 475 as context for motivation, but no issue acceptance criteria are defined to verify against.

---

Evidence Provider Findings

No evidence providers configured.

---

Tool Harness Findings

No tool calls were issued; reviewing the diff directly.

---

CI Results

All checks passed:

• Tests: success
• Lint: success
• Typecheck: success
• Build: success
• Docker Build: success

---

Unknowns / Needs Verification

None. The diff is self-contained, tests are comprehensive, and all CI checks pass.