Skip to content

feat(validate): use ajv-errors plugin for schema error messages - #83

Open
alltheseas wants to merge 1 commit into
mainfrom
feat/ajv-errors-plugin
Open

feat(validate): use ajv-errors plugin for schema error messages#83
alltheseas wants to merge 1 commit into
mainfrom
feat/ajv-errors-plugin

Conversation

@alltheseas

@alltheseas alltheseas commented Apr 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Schemata schemas already embed errorMessage keywords (592 occurrences across 245 schemas)
  • Previously Sherlock stripped these, re-extracted them via codegen into error-messages.ts, then matched AJV schema paths back at runtime — a workaround for not having the ajv-errors plugin
  • Now ajv-errors handles errorMessage natively: ~40 lines of workaround removed, ~6 lines added
  • Original AJV keywords preserved via unwrapErrorKeywords() so DB grouping/filtering is unaffected

What changed in src/validate/engine.ts

Removed Added
stripErrorMessages() — recursive delete of errorMessage from schemas ajvErrors(ajv) — one-line plugin init
enrichErrors() — manual codegen-based schemaPath matching unwrapErrorKeywords() — restore original keyword from ajv-errors wrapper
BASE_ERROR_MESSAGES / KIND_ERROR_MESSAGES import

Why

User feedback: improving error messages with errorMessage should be the canonical approach. The schemas already have the data — this PR lets AJV use it directly instead of stripping and rebuilding it.

error-messages.ts is now dead code (no imports). Can be cleaned up separately along with the codegen --errors flag.

Test plan

  • npm run build — compiles cleanly
  • npm run scan — validate events and confirm error messages appear correctly
  • Compare violation messages in DB before/after — should be identical (messages come from the same errorMessage source)
  • Check sherlock report --by-error output for human-readable messages

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Improvements
    • Enhanced validation error messages with more descriptive and user-friendly formatting to help users better understand validation failures.

Schemata schemas already embed errorMessage keywords (592 occurrences
across 245 schemas). Previously Sherlock stripped these, re-extracted
them via codegen into error-messages.ts, then matched AJV schema paths
back to those messages at runtime. This was a workaround for not having
the ajv-errors plugin.

Now ajv-errors handles errorMessage natively:
- Remove stripErrorMessages() — schemas keep their errorMessage fields
- Remove enrichErrors() — no more manual path matching
- Remove BASE_ERROR_MESSAGES/KIND_ERROR_MESSAGES import
- Add unwrapErrorKeywords() to preserve original AJV keyword for DB grouping

Net: ~40 lines of workaround removed, ~6 lines added.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Apr 4, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Added ajv-errors runtime dependency to package.json and integrated the ajv-errors plugin into the AJV validation engine. Replaced the previous error enrichment logic with a plugin-based approach that provides human-friendly error messages while preserving the original validation keywords.

Changes

Cohort / File(s) Summary
Dependency Addition
package.json
Added ajv-errors@^3.0.0 as a runtime dependency.
Validation Engine Update
src/validate/engine.ts
Initialized ajv-errors plugin on the AJV instance, replaced enrichErrors logic with unwrapErrorKeywords, and removed the stripErrorMessages helper and related error message imports.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Poem

🐰 Errors dressed in human words so fine,
ajv-errors makes them shine,
Keywords wrapped, messages clear,
Validation just became more dear!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: integrating the ajv-errors plugin for schema error message handling, which is the central objective of the PR.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/ajv-errors-plugin

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/validate/engine.ts (1)

96-102: Add defensive check when accessing nested keyword.

If err.params.errors[0] exists but lacks a keyword property, err.keyword would be set to undefined, which could cause unexpected behavior in downstream DB grouping/filtering.

🛡️ Proposed defensive check
 function unwrapErrorKeywords(errors: ErrorObject[]): void {
   for (const err of errors) {
-    if (err.keyword === 'errorMessage' && err.params?.errors?.length) {
-      err.keyword = err.params.errors[0].keyword;
+    if (err.keyword === 'errorMessage' && err.params?.errors?.length) {
+      const originalKeyword = err.params.errors[0].keyword;
+      if (originalKeyword) {
+        err.keyword = originalKeyword;
+      }
     }
   }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/validate/engine.ts` around lines 96 - 102, In unwrapErrorKeywords, avoid
setting err.keyword to undefined by adding a defensive check that verifies
err.params?.errors?.[0]?.keyword is defined before assigning; if the nested
keyword is missing, skip the assignment (or preserve the original err.keyword /
set a safe fallback) so downstream grouping/filtering that relies on keyword
won't receive undefined. Reference: function unwrapErrorKeywords and the
err.params.errors[0] access.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/validate/engine.ts`:
- Around line 96-102: In unwrapErrorKeywords, avoid setting err.keyword to
undefined by adding a defensive check that verifies
err.params?.errors?.[0]?.keyword is defined before assigning; if the nested
keyword is missing, skip the assignment (or preserve the original err.keyword /
set a safe fallback) so downstream grouping/filtering that relies on keyword
won't receive undefined. Reference: function unwrapErrorKeywords and the
err.params.errors[0] access.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9f2d8516-6ea0-4c92-9e7e-46eebcb0d055

📥 Commits

Reviewing files that changed from the base of the PR and between 73ecb32 and 04cc26d.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (2)
  • package.json
  • src/validate/engine.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant