feat(i18n): add normative keyword internationalization#840
Open
santif wants to merge 9 commits intoFission-AI:mainfrom
Open
feat(i18n): add normative keyword internationalization#840santif wants to merge 9 commits intoFission-AI:mainfrom
santif wants to merge 9 commits intoFission-AI:mainfrom
Conversation
…ation Introduce OpenSpec change artifacts (proposal, specs, design, tasks) for supporting language-specific normative keywords in validation (e.g., DEBE/DEBERÁ for Spanish instead of MUST/SHALL). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… refactor
- Add src/core/i18n/keywords.ts with NORMATIVE_KEYWORDS registry (en/es),
buildKeywordRegex() using Unicode \p{L} boundaries, formatKeywordMessage()
- Add language field to ProjectConfigSchema with resilient parsing
- Refactor base.schema.ts, spec.schema.ts, change.schema.ts to factory
functions (createRequirementSchema, createSpecSchema, createChangeSchema)
with backward-compatible default exports
- Update Validator to accept language param, precompile keyword regex,
and use language-aware schema validation
- Add 22 new tests (18 keyword + 4 config)
Remaining: thread language into Validator call sites, integration tests,
README docs (tasks 3.4-3.6, 4.1)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ests, and docs - Convert REQUIREMENT_NO_SHALL, GUIDE_MISSING_SPEC_SECTIONS, and GUIDE_SCENARIO_FORMAT to keyword-aware functions in constants.ts - Thread language from project config into Validator in validate.ts - Add 10 i18n validation tests covering English, Spanish, unknown language fallback, error messages, and English-keyword-in-Spanish edge cases - Document language config in README.md and docs/multi-language.md - Mark all 13/13 tasks complete Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Archive add-i18n-normative-keywords to openspec/changes/archive/. Sync delta specs to main: add i18n-normative-keywords spec, update config-loading and cli-validate specs with language-aware requirements. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Contributor
📝 WalkthroughWalkthroughAdds i18n support for normative keywords: a keyword registry and utilities, optional project-level language parsing, language-aware regex/message formatting, schema factory functions that accept language, and propagation of language through the Validator and CLI validate command. Tests, docs, and spec files updated. Changes
Sequence Diagram(s)sequenceDiagram
participant Config as "openspec/config.yaml"
participant Reader as "readProjectConfig()"
participant CLI as "validate command"
participant Validator as "Validator"
participant i18n as "keywords module"
participant Schema as "schema factory"
participant Out as "Validation Output"
Config->>Reader: read config (language?)
Reader-->>CLI: ProjectConfig (language?)
CLI->>Validator: instantiate(strict, language?)
Validator->>i18n: resolveKeywords(language?)
i18n-->>Validator: keywords list
Validator->>i18n: buildKeywordRegex(keywords)
Validator->>Schema: createSpecSchema(language)
Schema-->>Validator: spec/change schemas (language-aware)
Validator->>Schema: validate(requirement text)
Schema-->>Out: result with localized messages
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
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)
📝 Coding Plan
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 Tip CodeRabbit can use your project's `biome` configuration to improve the quality of JS/TS/CSS/JSON code reviews.Add a configuration file to your project to customize how CodeRabbit runs |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
test/core/i18n-keywords.test.ts (1)
76-79: Verify: Comma before "or" with two keywords is intentional?The expected format
"MUST", or "SHALL"includes a comma before "or" when there are only two keywords. Standard English typically uses "X or Y" without a comma for two items, reserving the serial/Oxford comma for three or more items (e.g., "X, Y, or Z").If this is intentional for consistency with the 3-keyword case, consider adding a brief comment in the implementation explaining the style choice. Otherwise, the 2-keyword case might read more naturally as
"MUST" or "SHALL".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/core/i18n-keywords.test.ts` around lines 76 - 79, The test expects a comma before "or" for two keywords but standard style omits that comma; update the implementation in formatKeywordMessage to special-case when keywords.length === 2 and join as '"X" or "Y"' (no comma), and then update the corresponding test case "should format two keywords" to expect 'Requirement must contain "MUST" or "SHALL"'; alternatively, if the comma is intentional, add a clarifying comment in formatKeywordMessage explaining the style choice and keep the test as-is—reference formatKeywordMessage and the "should format two keywords" test to locate the changes.src/core/schemas/base.schema.ts (1)
3-3: Keep the keyword error formatter single-sourced.
formatKeywordMessage()here duplicatesVALIDATION_MESSAGES.REQUIREMENT_NO_SHALL()insrc/core/validation/constants.ts. If one copy changes, schema errors and validator errors will drift. Reusing the existing validation-message helper would avoid that split brain.♻️ Small cleanup
-import { buildKeywordRegex, formatKeywordMessage, resolveKeywords } from '../i18n/keywords.js'; +import { buildKeywordRegex, resolveKeywords } from '../i18n/keywords.js'; ... - const message = formatKeywordMessage(keywords); + const message = VALIDATION_MESSAGES.REQUIREMENT_NO_SHALL(keywords);Also applies to: 13-15
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/schemas/base.schema.ts` at line 3, The duplicated keyword error formatting in base.schema (using formatKeywordMessage) should be replaced with the single canonical helper VALIDATION_MESSAGES.REQUIREMENT_NO_SHALL to avoid divergence; locate the call sites that currently invoke formatKeywordMessage (and any associated buildKeywordRegex/resolveKeywords usage) and refactor them to call VALIDATION_MESSAGES.REQUIREMENT_NO_SHALL(...) instead, passing the same keyword/context parameters so the output remains identical, and remove the local duplicate formatter import to ensure all schema and validator messages come from the single source of truth.
🤖 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-13-add-i18n-normative-keywords/tasks.md`:
- Line 5: The regex in the task description is malformed: replace the incorrect
(?\p{L}) token with a negative lookahead (?!\p{L}) so the intended boundary
becomes (?<!\p{L})... (?!\p{L}) and ensure the `u` flag is specified; update the
`buildKeywordRegex()` description (and any examples) in the task to use this
corrected pattern and keep references to NORMATIVE_KEYWORDS, DEFAULT_LANGUAGE,
and formatKeywordMessage() intact.
In `@openspec/specs/cli-validate/spec.md`:
- Line 29: Remove the inline edit artifact "← (was: hardcoded SHALL)" from the
requirement text so the line reads only "Show compliant example using the
normative keywords from the configured language (e.g., `DEBE` for Spanish,
`SHALL` for English)"; update the string that currently contains that trailing
note (search for the exact sentence containing "Show compliant example using the
normative keywords" or the tokens `DEBE`/`SHALL`) to eliminate the parenthetical
edit note and leave the clean requirement text.
In `@openspec/specs/i18n-normative-keywords/spec.md`:
- Around line 23-27: The spec and implementation disagree: when an unknown
language code is given the spec requires logging a warning listing available
language codes but resolveKeywords (src/core/i18n/keywords.ts) silently falls
back to English and the Validator constructor that calls it doesn't warn; fix by
modifying resolveKeywords to detect an unsupported language and emit a warning
via the project's logging facility (e.g., logger.warn or the existing app
logger) that includes the requested language and the list of available language
codes, or alternatively add that same warning in the Validator constructor
immediately after resolveKeywords is called so the fallback to 'en' is logged.
In `@src/core/i18n/keywords.ts`:
- Around line 50-54: In resolveKeywords, normalize the incoming language (e.g.,
language = language.trim().toLowerCase()) before lookup and change the fallback
behavior so that when a language string is provided but not found in
NORMATIVE_KEYWORDS the function returns undefined (instead of silently returning
NORMATIVE_KEYWORDS[DEFAULT_LANGUAGE]); keep the existing behavior of returning
NORMATIVE_KEYWORDS[DEFAULT_LANGUAGE] only when language is omitted/undefined.
This uses resolveKeywords, NORMATIVE_KEYWORDS and DEFAULT_LANGUAGE to allow
callers to detect unsupported languages and warn.
- Around line 25-28: The current buildKeywordRegex in function buildKeywordRegex
uses (?<!\p{L})... (?!\p{L}) which only blocks adjacent letters and still
matches inside tokens like MUST-API or MUST2; update the lookarounds around
pattern (the pattern variable) to assert that the preceding and following
character are NOT any Unicode letter, number, connector punctuation or dash
punctuation (e.g. use (?<![\p{L}\p{N}\p{Pc}\p{Pd}]) and
(?![\p{L}\p{N}\p{Pc}\p{Pd}]) ) so keywords will only match as standalone tokens,
keeping the existing escaping logic and 'u' flag in new RegExp.
---
Nitpick comments:
In `@src/core/schemas/base.schema.ts`:
- Line 3: The duplicated keyword error formatting in base.schema (using
formatKeywordMessage) should be replaced with the single canonical helper
VALIDATION_MESSAGES.REQUIREMENT_NO_SHALL to avoid divergence; locate the call
sites that currently invoke formatKeywordMessage (and any associated
buildKeywordRegex/resolveKeywords usage) and refactor them to call
VALIDATION_MESSAGES.REQUIREMENT_NO_SHALL(...) instead, passing the same
keyword/context parameters so the output remains identical, and remove the local
duplicate formatter import to ensure all schema and validator messages come from
the single source of truth.
In `@test/core/i18n-keywords.test.ts`:
- Around line 76-79: The test expects a comma before "or" for two keywords but
standard style omits that comma; update the implementation in
formatKeywordMessage to special-case when keywords.length === 2 and join as '"X"
or "Y"' (no comma), and then update the corresponding test case "should format
two keywords" to expect 'Requirement must contain "MUST" or "SHALL"';
alternatively, if the comma is intentional, add a clarifying comment in
formatKeywordMessage explaining the style choice and keep the test
as-is—reference formatKeywordMessage and the "should format two keywords" test
to locate the changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cbda8364-7403-40cd-9fc3-513c6dc294d5
📒 Files selected for processing (24)
README.mddocs/multi-language.mdopenspec/changes/archive/2026-03-13-add-i18n-normative-keywords/.openspec.yamlopenspec/changes/archive/2026-03-13-add-i18n-normative-keywords/design.mdopenspec/changes/archive/2026-03-13-add-i18n-normative-keywords/proposal.mdopenspec/changes/archive/2026-03-13-add-i18n-normative-keywords/specs/cli-validate/spec.mdopenspec/changes/archive/2026-03-13-add-i18n-normative-keywords/specs/config-loading/spec.mdopenspec/changes/archive/2026-03-13-add-i18n-normative-keywords/specs/i18n-normative-keywords/spec.mdopenspec/changes/archive/2026-03-13-add-i18n-normative-keywords/tasks.mdopenspec/specs/cli-validate/spec.mdopenspec/specs/config-loading/spec.mdopenspec/specs/i18n-normative-keywords/spec.mdsrc/commands/validate.tssrc/core/i18n/keywords.tssrc/core/project-config.tssrc/core/schemas/base.schema.tssrc/core/schemas/change.schema.tssrc/core/schemas/index.tssrc/core/schemas/spec.schema.tssrc/core/validation/constants.tssrc/core/validation/validator.tstest/core/i18n-keywords.test.tstest/core/project-config.test.tstest/core/validation.test.ts
openspec/changes/archive/2026-03-13-add-i18n-normative-keywords/tasks.md
Outdated
Show resolved
Hide resolved
- Fix malformed regex typo in tasks.md: (?\p{L}) → (?!\p{L})
- Remove editing artifact '← (was: hardcoded SHALL)' from cli-validate spec
- Add console.warn for unknown language codes in resolveKeywords() per spec
- Fix comma grammar in formatKeywordMessage() for two-keyword case
- Update REQUIREMENT_NO_SHALL in constants.ts to match
- Update tests for warning assertion and corrected message format
https://claude.ai/code/session_011nfbaV82D5YRVT1jQbWDC8
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/core/i18n/keywords.ts (1)
25-28:⚠️ Potential issue | 🟠 MajorBroaden token boundaries so identifier fragments don’t satisfy normative-keyword validation.
Line 27 only blocks adjacent letters, so keywords still match inside tokens like
MUST-API,MUST2, orMUST_v2, which can incorrectly pass validation.Suggested fix
export function buildKeywordRegex(keywords: string[]): RegExp { const escaped = keywords.map(k => k.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')); - const pattern = `(?<!\\p{L})(${escaped.join('|')})(?!\\p{L})`; + const boundary = `[\\p{L}\\p{N}\\p{M}\\p{Pc}\\p{Pd}]`; + const pattern = `(?<!${boundary})(?:${escaped.join('|')})(?!${boundary})`; return new RegExp(pattern, 'u'); }In ECMAScript RegExp with Unicode property escapes, does `(?<!\p{L})MUST(?!\p{L})` match inside `MUST-API`, `MUST2`, or `MUST_v2`?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/i18n/keywords.ts` around lines 25 - 28, The regex in buildKeywordRegex only prevents adjacent letters but still allows matches inside tokens like MUST-API, MUST2, or MUST_v2; update the lookbehind/lookahead to block letters, numbers and token connector chars instead of only \p{L}. Replace the pattern variable (pattern = `(?<!\\p{L})(${escaped.join('|')})(?!\\p{L})`) with one that uses a character class such as [\p{L}\p{N}\p{Pc}\p{Pd}_-] in both lookarounds (e.g. `(?<![\\p{L}\\p{N}\\p{Pc}\\p{Pd}_-])(...)(?![\\p{L}\\p{N}\\p{Pc}\\p{Pd}_-])`) so keywords won’t match when adjacent to letters, digits, connector punctuation or dashes.
🧹 Nitpick comments (1)
test/core/validation.test.ts (1)
676-733: Add a regression case for configured keywords embedded in identifiers.Please add explicit cases like
DEBE-API,DEBE2, andDEBE_v2in Spanish mode to ensure token-boundary behavior stays correct over time.🤖 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 676 - 733, Add regression tests in the same suite that verify token-boundary behavior for configured keywords embedded inside identifiers by creating a new changeDir (e.g., "test-change-es-keyword-embedded") and writing a spec with requirement text containing DEBE-API, DEBE2, and DEBE_v2 but no standalone "DEBE", then call Validator(false, 'es').validateChangeDeltaSpecs(changeDir) and assert the report is invalid (report.valid === false and report.summary.errors > 0); also add a companion test that includes a standalone "DEBE" alongside those embedded variants and assert the report is valid (report.valid === true and report.summary.errors === 0) to ensure the validator (Validator and its validateChangeDeltaSpecs method) treats only token-boundary matches as normative.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/core/i18n/keywords.ts`:
- Around line 25-28: The regex in buildKeywordRegex only prevents adjacent
letters but still allows matches inside tokens like MUST-API, MUST2, or MUST_v2;
update the lookbehind/lookahead to block letters, numbers and token connector
chars instead of only \p{L}. Replace the pattern variable (pattern =
`(?<!\\p{L})(${escaped.join('|')})(?!\\p{L})`) with one that uses a character
class such as [\p{L}\p{N}\p{Pc}\p{Pd}_-] in both lookarounds (e.g.
`(?<![\\p{L}\\p{N}\\p{Pc}\\p{Pd}_-])(...)(?![\\p{L}\\p{N}\\p{Pc}\\p{Pd}_-])`) so
keywords won’t match when adjacent to letters, digits, connector punctuation or
dashes.
---
Nitpick comments:
In `@test/core/validation.test.ts`:
- Around line 676-733: Add regression tests in the same suite that verify
token-boundary behavior for configured keywords embedded inside identifiers by
creating a new changeDir (e.g., "test-change-es-keyword-embedded") and writing a
spec with requirement text containing DEBE-API, DEBE2, and DEBE_v2 but no
standalone "DEBE", then call Validator(false,
'es').validateChangeDeltaSpecs(changeDir) and assert the report is invalid
(report.valid === false and report.summary.errors > 0); also add a companion
test that includes a standalone "DEBE" alongside those embedded variants and
assert the report is valid (report.valid === true and report.summary.errors ===
0) to ensure the validator (Validator and its validateChangeDeltaSpecs method)
treats only token-boundary matches as normative.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 869f1825-b1e6-4bd4-82a7-e8427a821772
📒 Files selected for processing (6)
openspec/changes/archive/2026-03-13-add-i18n-normative-keywords/tasks.mdopenspec/specs/cli-validate/spec.mdsrc/core/i18n/keywords.tssrc/core/validation/constants.tstest/core/i18n-keywords.test.tstest/core/validation.test.ts
✅ Files skipped from review due to trivial changes (1)
- test/core/i18n-keywords.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/validation/constants.ts
…ors, and dashes
Addresses review comment: keywords like MUST/SHALL were matching inside
compound tokens such as "MUST-API", "SHALL_v2", or "MUST2". The regex
lookarounds now use [\p{L}\p{N}\p{Pc}\p{Pd}] instead of just \p{L}.
https://claude.ai/code/session_015vFGVW2seFXEMpFNu6G5yq
Addresses review comment: language codes like "ES", " es ", or whitespace-only strings are now trimmed and lowercased before lookup. This prevents silent misconfiguration where users think i18n is active but the code silently falls back to English. https://claude.ai/code/session_015vFGVW2seFXEMpFNu6G5yq
Claude/analyze pr 840 yy8 fb
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/core/i18n/keywords.ts (1)
25-29: Guard against empty keyword arrays in regex builder.Line 27 can produce an empty alternation when
keywordsis empty, which makes matching behavior too permissive. Add a defensive guard and switch to non-capturing alternation.Suggested patch
export function buildKeywordRegex(keywords: string[]): RegExp { + if (keywords.length === 0) { + throw new Error('buildKeywordRegex requires at least one keyword'); + } const escaped = keywords.map(k => k.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')); - const pattern = `(?<![\\p{L}\\p{N}\\p{Pc}\\p{Pd}])(${escaped.join('|')})(?![\\p{L}\\p{N}\\p{Pc}\\p{Pd}])`; + const pattern = `(?<![\\p{L}\\p{N}\\p{Pc}\\p{Pd}])(?:${escaped.join('|')})(?![\\p{L}\\p{N}\\p{Pc}\\p{Pd}])`; return new RegExp(pattern, 'u'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/i18n/keywords.ts` around lines 25 - 29, buildKeywordRegex currently constructs a pattern that yields an empty alternation when keywords is empty and uses a capturing group; update buildKeywordRegex to first guard for an empty keywords array and return a regex that never matches (e.g., new RegExp('^(?!)')) in that case, and change the group in the produced pattern from capturing to non-capturing (use (?:...)) so replace the current "(${escaped.join('|')})" with "(?:${escaped.join('|')})"; ensure you still escape keywords (escaped) and compile with the 'u' flag.
🤖 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/core/i18n/keywords.ts`:
- Around line 25-29: buildKeywordRegex currently constructs a pattern that
yields an empty alternation when keywords is empty and uses a capturing group;
update buildKeywordRegex to first guard for an empty keywords array and return a
regex that never matches (e.g., new RegExp('^(?!)')) in that case, and change
the group in the produced pattern from capturing to non-capturing (use (?:...))
so replace the current "(${escaped.join('|')})" with "(?:${escaped.join('|')})";
ensure you still escape keywords (escaped) and compile with the 'u' flag.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c95f8d05-c77e-4cd1-86ec-a6bae4e0547e
📒 Files selected for processing (3)
openspec/changes/archive/2026-03-13-add-i18n-normative-keywords/tasks.mdsrc/core/i18n/keywords.tstest/core/i18n-keywords.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/core/i18n-keywords.test.ts
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
2 participants
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
languagefield toopenspec/config.yamlfor localized normative keyword validation (e.g.,esfor Spanish:DEBE,DEBERA,DEBERÁ)\p{L}boundaries), and dynamic validation messagesValidatoracross single-item and bulk validation pathslanguageconfig in README.md anddocs/multi-language.mdTest plan
npx vitest run test/core/validation.test.ts test/core/i18n-keywords.test.ts)test/core/validation.enriched-messages.test.ts)test/commands/validate.test.ts)language: esin config.yaml and validate a Spanish spec withDEBE/DEBERÁ🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests