feat: localize args validation errors - #613
Conversation
|
Warning Review limit reached
Next review available in: 20 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds ChangesArgs validation error localization
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant renderValidationErrors
participant I18nPluginExtension
participant formatResource
CLI->>renderValidationErrors: AggregateError of ArgsValidationError
loop each error
renderValidationErrors->>renderValidationErrors: resolveValidationValues(error)
renderValidationErrors->>I18nPluginExtension: localize(error.code, values)
I18nPluginExtension->>formatResource: formatResource(template, values)
formatResource-->>I18nPluginExtension: formatted message
I18nPluginExtension-->>renderValidationErrors: localized string or key
end
renderValidationErrors-->>CLI: joined localized messages
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
@gunshi/bone
@gunshi/combinators
@gunshi/definition
@gunshi/docs
gunshi
@gunshi/plugin
@gunshi/plugin-completion
@gunshi/plugin-dryrun
@gunshi/plugin-global
@gunshi/plugin-i18n
@gunshi/plugin-renderer
@gunshi/resources
@gunshi/shared
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/plugin-i18n/src/index.ts (1)
158-180: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPartial locale overrides shadow sibling built-in translations.
setResourcestores the locale map as-is, andtranslate()only falls back toDEFAULT_LOCALEwhen the locale entry is missing entirely. Supplying just onebuiltinResources['ja-JP']key makes every other built-in key forja-JPresolve to the raw key. Merge missing keys from the default resource, or fall back per key, before saving the locale map.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugin-i18n/src/index.ts` around lines 158 - 180, The built-in locale handling in translate() and setResource() does not preserve default translations when a locale only overrides some keys. Update the locale resource flow so partial overrides are merged with DEFAULT_LOCALE (or fall back per key) instead of storing the override map unchanged. Use the existing translate(), setResource(), and mapResourceWithBuiltinKey() logic to ensure missing built-in keys for a locale still resolve to the default translated value rather than the raw key.
🧹 Nitpick comments (4)
packages/gunshi/src/index.ts (1)
1-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate module doc comment to reflect new exports.
The module-level JSDoc (Line 12) still only mentions
parseArgsandresolveArgsfromargs-tokens, but the export block now also re-exportsArgsValidationError,ArgsValidationErrorKeys,isArgsValidationError, and theArgsValidationErrorCodetype.📝 Proposed doc update
- * - `args-tokens` utilities, `parseArgs` and `resolveArgs` for parsing command line arguments. + * - `args-tokens` utilities: `parseArgs`, `resolveArgs`, `ArgsValidationError`, `ArgsValidationErrorKeys`, `ArgsValidationErrorCode`, and `isArgsValidationError` for parsing and validating command line arguments.Also applies to: 29-43
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/gunshi/src/index.ts` around lines 1 - 21, The module-level JSDoc in the `gunshi` entry point is out of date and should be updated to list the newly re-exported `args-tokens` symbols. Adjust the export summary in `index.ts` so it mentions `ArgsValidationError`, `ArgsValidationErrorKeys`, `isArgsValidationError`, and `ArgsValidationErrorCode` alongside `parseArgs` and `resolveArgs`, keeping the rest of the documented API list in sync with the actual exports.packages/shared/src/localization.ts (2)
58-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider consolidating the near-duplicate built-in/error resource branches.
Both branches perform the same
formatResource(...) || keypattern, differing only in whether the prefix is stripped before lookup. Extracting a small shared helper would reduce duplication as more prefixed-key namespaces are added.♻️ Proposed consolidation
+function resolveAndFormat( + resourceKey: string, + originalKey: string, + values?: Record<string, unknown> +): string { + return ( + formatResource(DefaultResource[resourceKey as keyof typeof DefaultResource], values) || + originalKey + ) +} + if ((key as string).startsWith(BUILD_IN_PREFIX_AND_KEY_SEPARATOR)) { const resKey = (key as string).slice(BUILD_IN_PREFIX_AND_KEY_SEPARATOR.length) - return ( - formatResource(DefaultResource[resKey as keyof typeof DefaultResource], values) || - (key as string) - ) + return resolveAndFormat(resKey, key as string, values) } if ((key as string).startsWith(ERROR_PREFIX_AND_KEY_SEPARATOR)) { - return ( - formatResource(DefaultResource[key as keyof typeof DefaultResource], values) || - (key as string) - ) + return resolveAndFormat(key as string, key as string, values) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/shared/src/localization.ts` around lines 58 - 71, Consolidate the duplicated prefixed-resource handling in localization.ts by extracting a shared helper used by the built-in and error branches. The helper should encapsulate the common formatResource(... ) || key fallback, while accepting the lookup key and an option to strip BUILD_IN_PREFIX_AND_KEY_SEPARATOR when needed. Update the logic around the existing key prefix checks to call this helper so the behavior stays the same and future prefixed namespaces can reuse it.
102-110: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
stringfor the replacer callback’s first parameter.replaceAllpasses the matched substring here, so(_: string | RegExp, name: string)is misleading.🧹 Proposed fix
- return resource?.replaceAll(/\{\$(\w+)\}/g, (_: string | RegExp, name: string): string => { + return resource?.replaceAll(/\{\$(\w+)\}/g, (_: string, name: string): string => { return values[name] == null ? '' : String(values[name]) })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/shared/src/localization.ts` around lines 102 - 110, The replacer callback in formatResource uses an incorrect type for the first parameter of replaceAll, since it receives the matched substring rather than string | RegExp. Update the callback signature in formatResource to use string for the first argument, keeping the existing name parameter and replacement logic unchanged.packages/plugin-renderer/src/validation.ts (1)
47-70: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant fallback:
localizedReasonis already guaranteed truthy here.The early return on Line 62-64 (
if (!localizedReason || localizedReason === key) return error.values) means execution never reaches Line 68 unlesslocalizedReasonis truthy, making|| reasondead code.🧹 Simplify
return { ...error.values, - reason: localizedReason || reason + reason: localizedReason }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/plugin-renderer/src/validation.ts` around lines 47 - 70, In resolveValidationValues, the fallback in the returned reason object is redundant because the prior guard already ensures localizedReason is truthy, so simplify the customParse branch by returning the localized value directly when it differs from the key. Keep the existing checks around error.code, reasonKey, and localize intact, but remove the dead fallback expression in the final object spread so the logic in resolveValidationValues stays consistent and easier to read.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/gunshi/src/index.ts`:
- Around line 29-43: The public re-exports in the package entrypoint are using
names that are not exposed by the current args-tokens API, so update the symbols
exported from the top-level barrel to match the dependency’s actual error types.
In index.ts, replace the invalid
ArgsValidationError/ArgsValidationErrorKeys/isArgsValidationError/ArgsValidationErrorCode
exports with the corresponding ArgResolveError/ArgResolveErrorType exports (and
any related helpers that actually exist in args-tokens), keeping parseArgs and
resolveArgs as-is.
In `@packages/plugin-i18n/src/index.ts`:
- Around line 352-359: The local formatResource in
packages/plugin-i18n/src/index.ts should not reimplement placeholder
interpolation; it duplicates the shared localization helper added in
packages/shared/src/localization.ts. Update the code path that uses
formatResource to import and reuse the shared helper instead of the local
function, and remove the duplicate implementation so the plugin stays aligned
with the shared behavior.
---
Outside diff comments:
In `@packages/plugin-i18n/src/index.ts`:
- Around line 158-180: The built-in locale handling in translate() and
setResource() does not preserve default translations when a locale only
overrides some keys. Update the locale resource flow so partial overrides are
merged with DEFAULT_LOCALE (or fall back per key) instead of storing the
override map unchanged. Use the existing translate(), setResource(), and
mapResourceWithBuiltinKey() logic to ensure missing built-in keys for a locale
still resolve to the default translated value rather than the raw key.
---
Nitpick comments:
In `@packages/gunshi/src/index.ts`:
- Around line 1-21: The module-level JSDoc in the `gunshi` entry point is out of
date and should be updated to list the newly re-exported `args-tokens` symbols.
Adjust the export summary in `index.ts` so it mentions `ArgsValidationError`,
`ArgsValidationErrorKeys`, `isArgsValidationError`, and
`ArgsValidationErrorCode` alongside `parseArgs` and `resolveArgs`, keeping the
rest of the documented API list in sync with the actual exports.
In `@packages/plugin-renderer/src/validation.ts`:
- Around line 47-70: In resolveValidationValues, the fallback in the returned
reason object is redundant because the prior guard already ensures
localizedReason is truthy, so simplify the customParse branch by returning the
localized value directly when it differs from the key. Keep the existing checks
around error.code, reasonKey, and localize intact, but remove the dead fallback
expression in the final object spread so the logic in resolveValidationValues
stays consistent and easier to read.
In `@packages/shared/src/localization.ts`:
- Around line 58-71: Consolidate the duplicated prefixed-resource handling in
localization.ts by extracting a shared helper used by the built-in and error
branches. The helper should encapsulate the common formatResource(... ) || key
fallback, while accepting the lookup key and an option to strip
BUILD_IN_PREFIX_AND_KEY_SEPARATOR when needed. Update the logic around the
existing key prefix checks to call this helper so the behavior stays the same
and future prefixed namespaces can reuse it.
- Around line 102-110: The replacer callback in formatResource uses an incorrect
type for the first parameter of replaceAll, since it receives the matched
substring rather than string | RegExp. Update the callback signature in
formatResource to use string for the first argument, keeping the existing name
parameter and replacement logic unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6733e099-6f0f-401f-b16d-b2c27916ad3f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
.gitignorepackages/gunshi/src/cli.test.tspackages/gunshi/src/index.test-d.tspackages/gunshi/src/index.test.tspackages/gunshi/src/index.tspackages/plugin-i18n/src/index.test.tspackages/plugin-i18n/src/index.tspackages/plugin-i18n/src/types.test-d.tspackages/plugin-i18n/src/types.tspackages/plugin-renderer/package.jsonpackages/plugin-renderer/src/validation.test.tspackages/plugin-renderer/src/validation.tspackages/resources/locales/en-US.jsonpackages/resources/locales/ja-JP.jsonpackages/shared/src/constants.tspackages/shared/src/localization.test.tspackages/shared/src/localization.tspackages/shared/src/types.test-d.tspackages/shared/src/types.tspnpm-workspace.yaml
| export { | ||
| ArgsValidationError, | ||
| ArgsValidationErrorKeys, | ||
| isArgsValidationError, | ||
| parseArgs, | ||
| resolveArgs | ||
| } from 'args-tokens' | ||
| export * from './cli.ts' | ||
| export { ANONYMOUS_COMMAND_NAME } from './constants.ts' | ||
| export { createCommandContext } from './context.ts' | ||
| export { define, defineWithTypes, lazy, lazyWithTypes } from './definition.ts' | ||
| export { plugin } from './plugin/core.ts' | ||
|
|
||
| export type { CommandContextParams } from './context.ts' | ||
| export type { ArgsValidationErrorCode } from 'args-tokens' |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate args-tokens declaration files to confirm exported symbols
fd -HI 'args-tokens' -t d node_modules 2>/dev/null | head -20
rg -n "ArgsValidationError|ArgsValidationErrorKeys|isArgsValidationError|ArgsValidationErrorCode" -g '*.d.ts' node_modules/args-tokens 2>/dev/nullRepository: kazupon/gunshi
Length of output: 152
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect repository structure and locate relevant package metadata
git ls-files 'packages/gunshi/package.json' 'pnpm-lock.yaml' 'package.json' 'pnpm-workspace.yaml' | cat
echo
echo "=== packages/gunshi/package.json ==="
cat -n packages/gunshi/package.json
echo
echo "=== search args-tokens references in lockfile and workspace files ==="
rg -n '"args-tokens"|args-tokens@|args-tokens' package.json pnpm-lock.yaml packages/gunshi/package.json pnpm-workspace.yaml 2>/dev/null || trueRepository: kazupon/gunshi
Length of output: 5358
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== package importer entries around args-tokens ==="
sed -n '1,80p' pnpm-lock.yaml
echo
sed -n '7000,7160p' pnpm-lock.yaml
echo
echo "=== any local source references to args-tokens symbols ==="
rg -n "ArgsValidationError|ArgsValidationErrorKeys|isArgsValidationError|ArgsValidationErrorCode|from 'args-tokens'|from \"args-tokens\"" packages . 2>/dev/null || true
echo
echo "=== package-level build/type tooling references ==="
rg -n "dts\.resolve|args-tokens" packages/gunshi tsdown.config.* packages/**/tsdown.config.* 2>/dev/null || trueRepository: kazupon/gunshi
Length of output: 15674
🌐 Web query:
args-tokens 0.28.0 ArgsValidationError ArgsValidationErrorKeys isArgsValidationError ArgsValidationErrorCode
💡 Result:
The terms ArgsValidationError, ArgsValidationErrorKeys, isArgsValidationError, and ArgsValidationErrorCode are not associated with the args-tokens JavaScript/TypeScript library (kazupon/args-tokens) [1][2][3]. The args-tokens library is a high-performance command-line argument parser focused on util.parseArgs compatibility [1][4]. These specific terms appear in other disparate technical contexts, typically related to argument or configuration validation in other programming languages or projects: 1. ArgsValidationError (in Zig): This appears in the documentation for the devnw/zig/cli project as an error type returned by ArgsValidator.validate [5]. 2. ArgsValidationError / ArgsValidationErrorKind (in Rust): This appears in the tokyodoves crate documentation, related to error handling for game analysis [6][7][8]. 3. ArgsValidationError (in JavaScript/TypeScript): This also appears in other unrelated projects, such as the runsheet-js library, where it is used as a subclass of a custom error class [9]. If you are encountering these specific identifiers, they are likely from a different library or project than args-tokens.
Citations:
- 1: https://github.com/kazupon/args-tokens
- 2: https://jsr.io/@kazupon/args-tokens/doc
- 3: https://github.com/kazupon/args-tokens/blob/HEAD/docs/index.md
- 4: https://jsr.io/@kazupon/args-tokens
- 5: https://gitlab.com/devnw/zig/cli/-/blob/main/docs/API.md?ref_type=heads
- 6: https://docs.rs/tokyodoves/latest/tokyodoves/error/enum.AnalysisError.html
- 7: https://docs.rs/tokyodoves/latest/src/tokyodoves/error.rs.html
- 8: https://docs.rs/tokyodoves/latest/tokyodoves/error/index.html
- 9: shaug/runsheet-js@4dfaee6
🌐 Web query:
site:github.com/kazupon/args-tokens ArgsValidationErrorKeys args-tokens
💡 Result:
ArgsValidationErrorKeys is a type definition associated with the args-tokens library, which is a high-performance command-line argument parser for Node.js [1]. The library utilizes a validation system to handle errors that occur during the resolution of command-line arguments against an ArgSchema [1][2]. Specifically, the library supports aggregated validation errors, allowing it to collect and report multiple issues [2]. While the exact usage of ArgsValidationErrorKeys is not detailed in the provided documentation modules, it functions within the context of the library's error handling mechanism, often associated with the ArgResolveError class thrown when schema requirements or conflict constraints are violated [1].
Citations:
- 1: https://github.com/kazupon/args-tokens
- 2: https://github.com/kazupon/args-tokens/releases/tag/v0.9.0
🌐 Web query:
site:jsr.io/@kazupon/args-tokens/doc ArgsValidationError Keys isArgsValidationError ArgsValidationErrorCode
💡 Result:
The library @kazupon/args-tokens does not appear to contain symbols named ArgsValidationError, Keys, isArgsValidationError, or ArgsValidationErrorCode [1][2][3][4]. Instead, the library uses ArgResolveError to handle errors that occur when resolving arguments [1][2][4]. The associated error types for ArgResolveError are defined by ArgResolveErrorType, which includes the following values: "type", "required", and "conflict" [1][2][4]. Validation errors in this library are typically returned as an AggregateError object within the results of functions such as resolveArgs or parse [2][3][4].
Citations:
- 1: https://jsr.io/@kazupon/args-tokens/doc/resolver
- 2: https://jsr.io/@kazupon/args-tokens/doc
- 3: https://jsr.io/@kazupon/args-tokens/doc/~/ParsedArgs
- 4: https://jsr.io/@kazupon/args-tokens/doc/all_symbols
Replace the args-tokens re-exports
ArgsValidationError, ArgsValidationErrorKeys, isArgsValidationError, and ArgsValidationErrorCode aren't part of the published args-tokens API; the docs list ArgResolveError/ArgResolveErrorType instead, so this entrypoint will fail type-check/build until the exported names match the dependency.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/gunshi/src/index.ts` around lines 29 - 43, The public re-exports in
the package entrypoint are using names that are not exposed by the current
args-tokens API, so update the symbols exported from the top-level barrel to
match the dependency’s actual error types. In index.ts, replace the invalid
ArgsValidationError/ArgsValidationErrorKeys/isArgsValidationError/ArgsValidationErrorCode
exports with the corresponding ArgResolveError/ArgResolveErrorType exports (and
any related helpers that actually exist in args-tokens), keeping parseArgs and
resolveArgs as-is.
| "dependencies": { | ||
| "@gunshi/plugin": "workspace:*" | ||
| "@gunshi/plugin": "workspace:*", | ||
| "args-tokens": "catalog:" |
There was a problem hiding this comment.
Instead of adding args-tokens to the dependencies to use the API, could you please export it to @gunshil/plugin and use it from there? As in this case, if you export it to @gunshil/plugin, you can use args-tokens without having to add it to the dependencies, which is convenient.
Deploying gunshi with
|
| Latest commit: |
22e2ae0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://15fc32ed.gunshi.pages.dev |
| Branch Preview URL: | https://feat-args-validation-i18n.gunshi.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/plugin-i18n/src/index.test.ts`:
- Around line 141-146: The err:arg:invalid-type test is using the wrong
placeholder key, so the localized message does not render the expected value.
Update the test in the extension.translate assertion to pass expected: 'number'
instead of type, and change the expectation to match the en-US locale string
rendered by the i18n lookup for this case. Use the err:arg:invalid-type
translation key and the translate call in packages/plugin-i18n/src/index.test.ts
to locate it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5d749181-c179-4287-813d-0dfb0ccd2c88
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
packages/gunshi/src/index.tspackages/gunshi/src/plugin.tspackages/plugin-i18n/src/index.test.tspackages/plugin-i18n/src/index.tspackages/plugin-renderer/src/validation.test.tspackages/plugin-renderer/src/validation.tspackages/shared/src/localization.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/plugin-renderer/src/validation.test.ts
- packages/shared/src/localization.ts
- packages/gunshi/src/index.ts
- packages/plugin-renderer/src/validation.ts
- packages/plugin-i18n/src/index.ts
1 participant
Summary
This PR wires args-token validation metadata into Gunshi so built-in argument validation errors carry stable err:arg:* codes and interpolation values.
The i18n plugin can now resolve those keys from user resources, while the renderer localizes validation output only when the i18n extension is active and otherwise preserves existing fallback messages.
It also re-exports validation error helpers from gunshi, adds CLI integration coverage for required, type, choice, and custom parse errors, and ignores local planning notes.
Validation: pnpm test, pnpm lint:typecheck, pnpm build, pnpm lint.
Summary by CodeRabbit
New Features
Bug Fixes