feat(validation): accept Spanish normative keywords in validator#806
Open
santif wants to merge 3 commits intoFission-AI:mainfrom
Open
feat(validation): accept Spanish normative keywords in validator#806santif wants to merge 3 commits intoFission-AI:mainfrom
santif wants to merge 3 commits intoFission-AI:mainfrom
Conversation
… validator Add multi-language support for RFC 2119 normative keywords. Introduce a centralized NORMATIVE_KEYWORDS registry in constants.ts with English (SHALL, MUST) and Spanish (DEBE, DEBERA/DEBERÁ) keywords, replacing hardcoded checks in base.schema.ts and validator.ts. Update error messages to list all accepted keywords, add tests for all keyword variants including substring rejection, and document accepted keywords in multi-language.md. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughAdds language-aware RFC 2119 keyword validation: introduces a centralized Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis PR adds multi-language normative keyword support to the validator, centralizing the English ( Key changes:
Issues to address before merge:
Confidence Score: 2/5
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Requirement text input] --> B{containsNormativeKeyword}
B --> C[Iterate MATCH_KEYWORDS<br/>SHALL, MUST, DEBE, DEBERA, DEBERÁ]
C --> D{"(?<=\\s|^)KW(?=\\s|$)<br/>matches?"}
D -- yes --> E[return true ✓]
D -- no, next keyword --> C
C -- exhausted --> F[return false ✗]
E --> G1[base.schema.ts<br/>RequirementSchema<br/>.refine passes]
E --> G2[validator.ts<br/>containsShallOrMust<br/>returns true]
F --> H1[Zod error:<br/>Requirement must contain<br/>a normative keyword<br/>SHALL, MUST, DEBE, DEBERA]
F --> H2[ValidationIssue ERROR:<br/>ADDED/MODIFIED block<br/>must contain normative keyword]
Last reviewed commit: 812592d |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
openspec/specs/i18n-validator-keywords/spec.md (1)
34-36: Avoid baking a private method name into the spec.
containsShallOrMustis just an internal helper name, and it's already misleading now that Spanish keywords are supported. Keeping this requirement at the shared-matcher behavior level will let the implementation rename or inline that method without forcing a spec change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/specs/i18n-validator-keywords/spec.md` around lines 34 - 36, The spec currently references a private helper name (containsShallOrMust); update the requirement to avoid naming internal methods: reword the scenario so that when a new language keyword is added to NORMATIVE_KEYWORDS both the RequirementSchema refinement and the validator (or shared matcher) that checks for "shall"/"must"-style keywords rely on that constant and therefore recognize the new keyword without additional code changes—do not mention or mandate the internal method name containsShallOrMust so implementations can rename/inline it freely.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openspec/changes/archive/2026-03-06-i18n-validator-keywords/design.md`:
- Around line 39-63: The design and code disagree: update the code to match the
design by centralizing keyword matching into a single helper and using
word-boundary regex with unaccented canonical Spanish forms. Add a
containsNormativeKeyword(text: string): boolean in constants.ts (or keywords.ts)
that builds allKeywords = Object.values(NORMATIVE_KEYWORDS).flat() and returns
allKeywords.some(kw => new RegExp(`\\b${kw}\\b`, 'i').test(text)); replace the
existing hard-coded Spanish "DEBERÁ" and the (?<=\s|^)…(?=\s|$) lookaround
usages with calls to containsNormativeKeyword from both base.schema.ts and
validator.ts, and ensure NORMATIVE_KEYWORDS lists unaccented Spanish variants
(e.g., DEBERA) so accented variants match via word boundaries.
In `@openspec/changes/archive/2026-03-06-i18n-validator-keywords/proposal.md`:
- Around line 7-10: The archive entry overstates functionality that wasn't
implemented: rather than claiming config-driven language selection and localized
error output, update the proposal text to accurately reflect the shipped
behavior — that a fixed English/Spanish RFC2119 keyword map was added and that
RequirementSchema (base.schema.ts) and containsShallOrMust (validator.ts) accept
those keywords, but validation messages remain English; remove or reword the
bullets about configurable language selection and bilingual error messages so
the entry only documents the actual changes implemented.
In `@src/core/validation/constants.ts`:
- Line 35: Update the shared error message constant REQUIREMENT_NO_SHALL to
append an uppercase remediation hint so callers (RequirementSchema and the delta
validation used by cli-validate) clearly state that lowercase normative keywords
are rejected; modify the message built from NORMATIVE_KEYWORDS
(Object.values(NORMATIVE_KEYWORDS).flat().join(', ')) to include a short clause
like "use UPPERCASE" or equivalent guidance so both RequirementSchema and delta
validation show the remediation to use uppercase keywords.
- Around line 26-27: The containsNormativeKeyword function currently uses
whitespace-only lookarounds; change its regex to use word boundaries (e.g., new
RegExp(`\\b${kw}\\b`)) so keywords like "MUST:" or "**DEBE**" match correctly
(reference MATCH_KEYWORDS and containsNormativeKeyword). Also update the
validation/remediation error message that lists the accepted keywords to
explicitly state that keywords must be in UPPERCASE (use the MATCH_KEYWORDS
array to build the list so the message stays in sync).
In `@src/core/validation/validator.ts`:
- Around line 434-435: containsShallOrMust calls containsNormativeKeyword which
currently only matches keywords with ASCII word boundaries, so tokens like
"SHALL." or "DEBE," are missed; update the regex in containsNormativeKeyword (in
constants.ts) to use Unicode-aware boundaries by using lookarounds (e.g.,
(?<!\p{L})KEYWORD(?!\p{L})) with the Unicode flag so keywords adjacent to
punctuation still match, and add unit tests covering punctuation-delimited cases
for containsShallOrMust/containsNormativeKeyword to validate ADDED/MODIFIED
delta detection.
---
Nitpick comments:
In `@openspec/specs/i18n-validator-keywords/spec.md`:
- Around line 34-36: The spec currently references a private helper name
(containsShallOrMust); update the requirement to avoid naming internal methods:
reword the scenario so that when a new language keyword is added to
NORMATIVE_KEYWORDS both the RequirementSchema refinement and the validator (or
shared matcher) that checks for "shall"/"must"-style keywords rely on that
constant and therefore recognize the new keyword without additional code
changes—do not mention or mandate the internal method name containsShallOrMust
so implementations can rename/inline it freely.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 006cd6b6-7ad1-49aa-b947-c8e15cadbb32
📒 Files selected for processing (13)
docs/multi-language.mdopenspec/changes/archive/2026-03-06-i18n-validator-keywords/.openspec.yamlopenspec/changes/archive/2026-03-06-i18n-validator-keywords/design.mdopenspec/changes/archive/2026-03-06-i18n-validator-keywords/proposal.mdopenspec/changes/archive/2026-03-06-i18n-validator-keywords/specs/cli-validate/spec.mdopenspec/changes/archive/2026-03-06-i18n-validator-keywords/specs/i18n-validator-keywords/spec.mdopenspec/changes/archive/2026-03-06-i18n-validator-keywords/tasks.mdopenspec/specs/cli-validate/spec.mdopenspec/specs/i18n-validator-keywords/spec.mdsrc/core/schemas/base.schema.tssrc/core/validation/constants.tssrc/core/validation/validator.tstest/core/validation.test.ts
openspec/changes/archive/2026-03-06-i18n-validator-keywords/proposal.md
Outdated
Show resolved
Hide resolved
Switch containsNormativeKeyword regex from whitespace-only lookarounds to \b…(?=\W|$) so keywords adjacent to punctuation (MUST:, DEBE,) and markdown formatting (**SHALL**) match correctly while still supporting accented DEBERÁ. Update REQUIREMENT_NO_SHALL to list all MATCH_KEYWORDS including DEBERÁ and state that keywords must be UPPERCASE. Align archived proposal and design with shipped behavior. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
openspec/changes/archive/2026-03-06-i18n-validator-keywords/design.md (1)
39-41:⚠️ Potential issue | 🟡 MinorAlign this rationale with the matcher that actually shipped.
Lines 39-41 still say the solution avoids regex and relies on
Array.includes(), but the implementation insrc/core/validation/constants.tsand the very next section both describe a regex-based helper. Keeping the archive internally consistent will make future changes much easier to reason about.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/archive/2026-03-06-i18n-validator-keywords/design.md` around lines 39 - 41, Update the design note to match the shipped implementation: replace the statement that validation uses Array.includes() with an explanation of the regex-based matcher actually implemented (referencing the regex helper described in src/core/validation/constants.ts and the "matcher" utility in the next section), explain why the team chose a regex helper over Array.includes() for this case, and remove or reframe the “no regex patterns” rationale so the archive is internally consistent with the shipped regex-based helper.openspec/changes/archive/2026-03-06-i18n-validator-keywords/proposal.md (1)
25-27:⚠️ Potential issue | 🟡 MinorMake the Impact section describe shipped behavior, not a future possibility.
Line 27 says
constants.ts"may need" a keyword map, but this change already addsNORMATIVE_KEYWORDSandcontainsNormativeKeyword(). Rewording this in present tense will keep the archived proposal aligned with the implementation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/archive/2026-03-06-i18n-validator-keywords/proposal.md` around lines 25 - 27, The Impact section is written as future-possibility but the implementation already added the new constants and function; update the wording to present tense and state the shipped behavior: replace "may need new keyword constants or a keyword map" with a definitive statement that src/core/validation/constants.ts now exposes NORMATIVE_KEYWORDS and src/core/validation/validator.ts provides containsNormativeKeyword() (and that RequirementSchema now uses multi-language keyword checks) so the archived proposal matches the implemented changes.
🧹 Nitpick comments (1)
test/core/validation.test.ts (1)
84-114: Add regression cases for punctuation- and Markdown-adjacent keywords.The matcher change was made to accept forms like
MUST:and**DEBE**, but this suite still only proves whitespace-delimited matches. A couple of positive cases here would lock in the exact behavior that motivated the regex update.Suggested test addition
it('should accept accented Spanish keyword', () => { expect(containsNormativeKeyword('El sistema DEBERÁ hacer X')).toBe(true); }); + + it('should accept keywords adjacent to punctuation and Markdown formatting', () => { + expect(containsNormativeKeyword('The system MUST: do X')).toBe(true); + expect(containsNormativeKeyword('**DEBE** es obligatorio')).toBe(true); + expect(containsNormativeKeyword('El sistema DEBERÁ, si falla, reintenta')).toBe(true); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/core/validation.test.ts` around lines 84 - 114, Add regression tests to cover punctuation- and Markdown-adjacent matches for containsNormativeKeyword: update the containsNormativeKeyword test suite to include positive cases such as a keyword followed by punctuation ("The system MUST: do X" or "MUST: do X") and a Markdown-emphasized keyword ("**DEBE** hacer X" or "El sistema **DEBERÁ** hacer X") to ensure the matcher accepts those forms; add these expectations alongside the existing positive cases referencing the containsNormativeKeyword function so the test suite locks in the regex behavior for punctuation- and Markdown-adjacent keywords.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@openspec/changes/archive/2026-03-06-i18n-validator-keywords/design.md`:
- Around line 39-41: Update the design note to match the shipped implementation:
replace the statement that validation uses Array.includes() with an explanation
of the regex-based matcher actually implemented (referencing the regex helper
described in src/core/validation/constants.ts and the "matcher" utility in the
next section), explain why the team chose a regex helper over Array.includes()
for this case, and remove or reframe the “no regex patterns” rationale so the
archive is internally consistent with the shipped regex-based helper.
In `@openspec/changes/archive/2026-03-06-i18n-validator-keywords/proposal.md`:
- Around line 25-27: The Impact section is written as future-possibility but the
implementation already added the new constants and function; update the wording
to present tense and state the shipped behavior: replace "may need new keyword
constants or a keyword map" with a definitive statement that
src/core/validation/constants.ts now exposes NORMATIVE_KEYWORDS and
src/core/validation/validator.ts provides containsNormativeKeyword() (and that
RequirementSchema now uses multi-language keyword checks) so the archived
proposal matches the implemented changes.
---
Nitpick comments:
In `@test/core/validation.test.ts`:
- Around line 84-114: Add regression tests to cover punctuation- and
Markdown-adjacent matches for containsNormativeKeyword: update the
containsNormativeKeyword test suite to include positive cases such as a keyword
followed by punctuation ("The system MUST: do X" or "MUST: do X") and a
Markdown-emphasized keyword ("**DEBE** hacer X" or "El sistema **DEBERÁ** hacer
X") to ensure the matcher accepts those forms; add these expectations alongside
the existing positive cases referencing the containsNormativeKeyword function so
the test suite locks in the regex behavior for punctuation- and
Markdown-adjacent keywords.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c7471d63-6f86-4ed6-9602-8b127d6629cd
📒 Files selected for processing (4)
openspec/changes/archive/2026-03-06-i18n-validator-keywords/design.mdopenspec/changes/archive/2026-03-06-i18n-validator-keywords/proposal.mdsrc/core/validation/constants.tstest/core/validation.test.ts
…e load Avoid creating new RegExp objects on every containsNormativeKeyword call by compiling patterns once. Switch from (?=\W|$) to equivalent (?!\w) lookahead. Add test coverage for keywords adjacent to punctuation and update spec/archive docs accordingly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Author
Summary of recent changes1. Hybrid word-boundary regex (
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
NORMATIVE_KEYWORDSregistry supporting English (SHALL,MUST) and Spanish (DEBE,DEBERA/DEBERÁ) RFC 2119 keywordsbase.schema.tsandvalidator.tswith sharedcontainsNormativeKeyword()functionChanges
src/core/validation/constants.ts— NewNORMATIVE_KEYWORDSconstant andcontainsNormativeKeyword()function with word-boundary matchingsrc/core/schemas/base.schema.ts—RequirementSchemarefinement now usescontainsNormativeKeyword()src/core/validation/validator.ts—containsShallOrMust()delegates tocontainsNormativeKeyword(); delta validation errors reference the shared messagetest/core/validation.test.ts— Unit tests for English, Spanish, accented variants, lowercase rejection, substring rejection, and delta spec validation with Spanish keywordsdocs/multi-language.md— Document accepted normative keywords per languageopenspec/specs/— Newi18n-validator-keywordsspec and updatedcli-validatespec with multi-language hint scenarioopenspec/changes/archive/— Archived completed changeTest plan
containsNormativeKeyword()acceptsSHALL,MUST,DEBE,DEBERA,DEBERÁINDEBTED,MUSTERING) are rejectedRequirementSchemapasses with Spanish keywordsSummary by CodeRabbit
New Features
Documentation
Tests