[codex] Expand domain command params#294
Open
eviltester wants to merge 8 commits into
Open
Conversation
|
Important Review skippedToo many files! This PR contains 266 files, which is 116 over the limit of 150. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (266)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR aligns Faker-backed domain keyword parameter metadata with generated help docs, adds comparison tooling and tests, introduces BigInt bounds validation, and updates lorem/word parameter shapes and related expectations. ChangesDomain command parameter parity with Faker
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d304d32ae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses #286 by expanding/aligning domain keyword parameter metadata (especially for Faker-delegated commands), updating docs and tests accordingly, and adding an automated Domain-vs-Faker options-parameter comparison report plus a script to generate/check it.
Changes:
- Added a script + generated markdown report to compare Faker
options?: { ... }top-level keys against domainhelp.args. - Expanded multiple Faker-backed domain keyword definitions to include option parameters (with
argTransform: 'optionsFromHelpArgs') and added/updated related unit/UI tests. - Removed domain-only parameters that Faker doesn’t implement (e.g., legacy
word.*(max=...),lorem.word(min/max=...)) and updated docs/tests.
Reviewed changes
Copilot reviewed 48 out of 48 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/compare-domain-faker-params.mjs | New script to extract Faker option-param keys from .d.ts and compare vs domain keyword metadata (JSON/Markdown + optional check). |
| packages/core/src/tests/data_generation/unit/domain/domainKeywords.test.js | Updates sample-value helpers to handle bigint type tokens in test scaffolding. |
| packages/core/src/tests/data_generation/unit/domain/domain-param-invocation-coverage.test-helper.js | Updates type sampling and adds per-arg sample overrides for new option params. |
| packages/core/src/tests/data_generation/unit/domain/domain-keyword-parser.test.js | Updates parser expectations for Faker-unsupported lorem.word(min/max=...). |
| packages/core/src/tests/data_generation/unit/domain/domain-keyword-params-usage.test.js | Updates sampling for bigint and adds overrides/skip logic for specific params (e.g. zipCode state). |
| packages/core/src/tests/data_generation/unit/domain/domain-faker-param-comparison.test.js | New Jest coverage asserting no missing/domain-only params vs Faker options, plus a focused check for number.bigInt. |
| packages/core/src/tests/command-help/command-help-examples.test.js | Adjusts usage-example coverage logic to ignore params that are not usage-example supported. |
| packages/core/src/tests/command-help/command-help-examples.test-support.js | Adds isUsageExampleSupportedParam helper and exports it for usage-example tests. |
| packages/core/js/keywords/domain/word/words-keyword-definition.js | Removes unsupported max param/example for word.words. |
| packages/core/js/keywords/domain/word/verb-keyword-definition.js | Removes unsupported max param/example for word.verb. |
| packages/core/js/keywords/domain/word/sample-keyword-definition.js | Removes unsupported max param/example for word.sample. |
| packages/core/js/keywords/domain/word/preposition-keyword-definition.js | Removes unsupported max param/example for word.preposition. |
| packages/core/js/keywords/domain/word/noun-keyword-definition.js | Removes unsupported max param/example for word.noun. |
| packages/core/js/keywords/domain/word/interjection-keyword-definition.js | Removes unsupported max param/example for word.interjection. |
| packages/core/js/keywords/domain/word/conjunction-keyword-definition.js | Removes unsupported max param/example for word.conjunction. |
| packages/core/js/keywords/domain/word/adverb-keyword-definition.js | Removes unsupported max param/example for word.adverb. |
| packages/core/js/keywords/domain/word/adjective-keyword-definition.js | Removes unsupported max param/example for word.adjective. |
| packages/core/js/keywords/domain/system/network-interface-keyword-definition.js | Adds options args + optionsFromHelpArgs and expands usage examples. |
| packages/core/js/keywords/domain/system/file-name-keyword-definition.js | Adds extensionCount arg + optionsFromHelpArgs and updates examples. |
| packages/core/js/keywords/domain/person/sex-type-keyword-definition.js | Expands enum (generic) + adds includeGeneric option + optionsFromHelpArgs. |
| packages/core/js/keywords/domain/person/full-name-keyword-definition.js | Adds firstName/lastName/sex options + optionsFromHelpArgs and examples. |
| packages/core/js/keywords/domain/number/big-int-keyword-definition.js | Reworks params to min/max/multipleOf, adds args validation, and updates examples. |
| packages/core/js/keywords/domain/lorem/word-keyword-definition.js | Removes unsupported min/max params/examples; keeps length/strategy. |
| packages/core/js/keywords/domain/location/zip-code-keyword-definition.js | Adds state/format args + optionsFromHelpArgs; marks state as not usage-example supported. |
| packages/core/js/keywords/domain/internet/http-status-code-keyword-definition.js | Adds types option + optionsFromHelpArgs and updates examples. |
| packages/core/js/keywords/domain/internet/example-email-keyword-definition.js | Adds firstName/lastName/allowSpecialCharacters options + optionsFromHelpArgs. |
| packages/core/js/keywords/domain/internet/display-name-keyword-definition.js | Adds firstName/lastName options + optionsFromHelpArgs. |
| packages/core/js/keywords/domain/image/url-picsum-photos-keyword-definition.js | Adds width/height/grayscale/blur options + optionsFromHelpArgs and examples. |
| packages/core/js/keywords/domain/image/person-portrait-keyword-definition.js | Adds sex/size options + optionsFromHelpArgs and examples. |
| packages/core/js/keywords/domain/image/data-uri-keyword-definition.js | Adds width/height/color/type options + optionsFromHelpArgs and expanded examples. |
| packages/core/js/keywords/domain/git/commit-sha-keyword-definition.js | Adds length option + optionsFromHelpArgs and updates examples. |
| packages/core/js/keywords/domain/git/commit-entry-keyword-definition.js | Adds merge/eol/refDate options + optionsFromHelpArgs and expands examples. |
| packages/core/js/keywords/domain/git/commit-date-keyword-definition.js | Adds refDate option + optionsFromHelpArgs and updates examples. |
| packages/core/js/keywords/domain/airline/record-locator-keyword-definition.js | Adds option args + optionsFromHelpArgs and new examples. |
| packages/core/js/keywords/domain/airline/flight-number-keyword-definition.js | Adds length/addLeadingZeros options + optionsFromHelpArgs and expanded examples. |
| packages/core/js/domain/domain-keywords.js | Adds bigint support to type-matching. |
| packages/core-ui/src/tests/interaction/matrix/schema-interaction-scenario-builder.test.js | Adjusts scenario arg-coverage expectations to exclude location.zipCode.state. |
| docs/domain-faker-param-comparison.md | Generated report documenting current parity between domain args and Faker option keys. |
| docs-src/docs/040-test-data/domain/290-word.md | Updates domain docs to remove unsupported max params/examples for word.*. |
| docs-src/docs/040-test-data/domain/270-system.md | Updates docs for system.fileName/system.networkInterface to include args + () examples. |
| docs-src/docs/040-test-data/domain/230-person.md | Updates docs for person.fullName and person.sexType args + examples. |
| docs-src/docs/040-test-data/domain/220-number.md | Updates number.bigInt docs to min/max/multipleOf examples/args. |
| docs-src/docs/040-test-data/domain/200-lorem.md | Removes lorem.word(min/max) docs/examples. |
| docs-src/docs/040-test-data/domain/190-location.md | Adds args/docs/examples for location.zipCode. |
| docs-src/docs/040-test-data/domain/170-internet.md | Adds args/docs/examples for internet.displayName, internet.exampleEmail, internet.httpStatusCode. |
| docs-src/docs/040-test-data/domain/160-image.md | Adds args/docs/examples for image.dataUri, image.personPortrait, image.urlPicsumPhotos. |
| docs-src/docs/040-test-data/domain/140-git.md | Adds args/docs/examples for git.commitDate, git.commitEntry, git.commitSha. |
| docs-src/docs/040-test-data/domain/020-airline.md | Adds args/docs/examples for airline.flightNumber and airline.recordLocator. |
Greptile SummaryThis PR expands domain command parameter metadata across ~80 files, aligning documented args with what Faker actually implements — removing domain-only params that were silently ignored (e.g.
Confidence Score: 5/5All changes are metadata expansions, new validators, and test coverage. No data-loss paths, no auth changes, no migrations. The new validators are purely additive and defensive (pass-through on undefined/wrong type). The dry-run execution path is guarded correctly and only fires for faker-backed keywords with a resolvable target. The BigInt feasibility check uses correct modular arithmetic. The loremCountFromHelpArgs transform is tested end-to-end. No regressions were identified across the changed paths. No files require special attention. The most complex logic (validateBigIntMultipleOfCanMatchRange, applyLoremCountArgTransform) is well-covered by unit tests. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[DomainTestDataRuleValidator.validate] --> B[parseKeywordInvocation]
B --> C{Parse errors?}
C -- yes --> E[return false]
C -- no --> D[getDomainKeywordByAlias]
D --> F{Known keyword?}
F -- no --> E
F -- yes --> G[validateDomainKeywordArgs argsValidator chain]
G --> H{Args valid?}
H -- no --> E
H -- yes --> I{faker provided AND faker-backed keyword?}
I -- no --> J[return true]
I -- yes --> K[executeDomainKeyword dry-run]
K --> L{Faker threw?}
L -- yes --> E
L -- no --> J
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[DomainTestDataRuleValidator.validate] --> B[parseKeywordInvocation]
B --> C{Parse errors?}
C -- yes --> E[return false]
C -- no --> D[getDomainKeywordByAlias]
D --> F{Known keyword?}
F -- no --> E
F -- yes --> G[validateDomainKeywordArgs argsValidator chain]
G --> H{Args valid?}
H -- no --> E
H -- yes --> I{faker provided AND faker-backed keyword?}
I -- no --> J[return true]
I -- yes --> K[executeDomainKeyword dry-run]
K --> L{Faker threw?}
L -- yes --> E
L -- no --> J
Reviews (5): Last reviewed commit: "Fix domain command parameter validation" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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
🧹 Nitpick comments (6)
scripts/compare-domain-faker-params.mjs (2)
106-136: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffMethod overload collisions silently drop earlier signatures.
methods.set(methodName, ...)(Line 131) overwrites any prior entry for the samemethodNamewithin a module body. Faker frequently declares overloaded methods (e.g., a no-args overload plus an options-object overload); the regex-driven scan will only retain whichever overload the loop encounters last, which may not be the options-bearing one, potentially skewingfakerOptionParamsfor that command.🤖 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 `@scripts/compare-domain-faker-params.mjs` around lines 106 - 136, Method overloads in compare-domain-faker-params.mjs are being collapsed because methods.set(methodName, ...) overwrites earlier entries in the body scan. Update the parsing logic around the methods Map and the methodStartRegex loop to preserve overloads for the same methodName, and ensure the options-bearing overload is selected (or merged) rather than replaced by the last-seen signature. Use the existing findMatching, getObjectOptionKeys, and paramsText extraction flow to collect all overload signatures before deciding which signature should drive fakerOptionParams.
68-84: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid string-matching the Faker declaration bundle. This still couples the script to an internal
declare class NumberModuledetail; if it needs to keep working across faker upgrades, switch to a stable declaration path or export-based lookup instead.🤖 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 `@scripts/compare-domain-faker-params.mjs` around lines 68 - 84, The getFakerDeclarationPath() lookup is still brittle because it scans the dist bundle and matches the internal "declare class NumberModule" text. Replace that string-based bundle search with a stable resolution strategy, such as using the known exported declaration path from `@faker-js/faker/package.json` or another export-based lookup that does not depend on internal declaration contents, while keeping the fallback error in place if resolution fails.packages/core-ui/src/tests/interaction/matrix/schema-interaction-scenario-builder.test.js (1)
19-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded exclusion duplicates the definition-level
usageExampleSupportedflag.Same concern as in
command-help-examples.test-support.js:isScenarioArgCoverageRequiredre-encodes thelocation.zipCode/stateexception as a literal string comparison instead of deriving it fromarg.usageExampleSupported(already set tofalseinLOCATION_ZIP_CODE_KEYWORD_DEFINITION). Passing the fullargobject (instead of justargName) would let this check generalize automatically to any future param marked unsupported, rather than requiring another hardcoded exception here.♻️ Proposed simplification
-function isScenarioArgCoverageRequired(command, argName) { - return !(command === 'location.zipCode' && argName === 'state'); -} +function isScenarioArgCoverageRequired(arg) { + return arg?.usageExampleSupported !== false; +}(metadata.args || []) - .filter((arg) => isScenarioArgCoverageRequired(command, arg.name)) + .filter((arg) => isScenarioArgCoverageRequired(arg)) .forEach((arg) => { expect(bucket.coveredArgs.has(arg.name)).toBe(true); });Also applies to: 89-93
🤖 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/core-ui/src/tests/interaction/matrix/schema-interaction-scenario-builder.test.js` around lines 19 - 22, `isScenarioArgCoverageRequired` is hardcoding the `location.zipCode`/`state` exception instead of using the argument metadata. Update the helper in `schema-interaction-scenario-builder.test.js` to accept the full `arg` object (as in `command-help-examples.test-support.js`) and decide coverage from `arg.usageExampleSupported` rather than string-matching `command` and `argName`. Then update the related call sites and any duplicate logic around the referenced scenario coverage checks so unsupported args are excluded automatically.packages/core/src/tests/command-help/command-help-examples.test-support.js (1)
33-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRedundant hardcoded exclusion — already covered by
param.usageExampleSupported.
LOCATION_ZIP_CODE_KEYWORD_DEFINITIONalready setsusageExampleSupported: falseon thestatearg, so the fallbackparam?.usageExampleSupported !== falseon line 38 already excludes it. The explicitentry?.command === 'location.zipCode' && param?.name === 'state'branch is dead code that duplicates the single source of truth and must be manually kept in sync if similar exclusions are ever added elsewhere.♻️ Proposed simplification
function isUsageExampleSupportedParam(entry, param) { - if (entry?.command === 'location.zipCode' && param?.name === 'state') { - return false; - } - return param?.usageExampleSupported !== false; }🤖 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/core/src/tests/command-help/command-help-examples.test-support.js` around lines 33 - 40, Remove the hardcoded `entry?.command === 'location.zipCode' && param?.name === 'state'` բացառion from `isUsageExampleSupportedParam`; it duplicates the existing `param?.usageExampleSupported !== false` check and is already covered by `LOCATION_ZIP_CODE_KEYWORD_DEFINITION`. Keep the function relying only on `param.usageExampleSupported` so `isUsageExampleSupportedParam` stays the single source of truth.packages/core/src/tests/data_generation/unit/domain/domain-param-invocation-coverage.test-helper.js (1)
19-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider consolidating triplicated sample-value helpers.
sampleValueForType/sampleValueForKeywordArgare duplicated near-verbatim across this file,domain-keyword-params-usage.test.js, anddomainKeywords.test.js. This PR'sbiginthandling had to be patched in all three, and the implementations already diverged slightly (this file foldsbigintinto the same numeric branch asnumber/integeron line 35, while the other two files add a separateif (allowed.includes('bigint')) return 7;block) — evidence that keeping them in sync manually is fragile.Extracting a single shared test utility module would remove this maintenance burden going forward.
Also applies to: 56-92
🤖 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/core/src/tests/data_generation/unit/domain/domain-param-invocation-coverage.test-helper.js` around lines 19 - 54, The sample-value helpers are duplicated across this test helper and the related domain tests, and the bigint logic is already drifting between copies. Extract the shared logic into a single reusable test utility and have sampleValueForType/sampleValueForKeywordArg in this file and the matching helpers in domain-keyword-params-usage.test.js and domainKeywords.test.js call it, keeping bigint handling centralized in one place.packages/core/js/keywords/domain/internet/http-status-code-keyword-definition.js (1)
8-8: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueArgs added correctly match
faker.internet.httpStatusCodeoptions.Confirmed the
typescategories (informational, success, redirection, clientError, serverError) match faker'sHttpStatusCodeType. No enum-level validation is added fortypesvalues, so an invalid category will surface as whatever error faker itself throws, rather than a domain-level validation message. Given this mirrors the existing pattern for other array-typed args in this cohort, this is a minor nice-to-have rather than a blocker.Also applies to: 18-37
🤖 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/core/js/keywords/domain/internet/http-status-code-keyword-definition.js` at line 8, The `httpStatusCode` keyword definition uses `optionsFromHelpArgs` correctly, but `types` still lacks enum-level validation, so invalid values fall through to faker errors instead of a domain validation message. Update the `http-status-code-keyword-definition` setup to validate `types` against the allowed `HttpStatusCodeType` values in the same pattern used by other array-typed args in this cohort, keeping the existing `argTransform` behavior intact.
🤖 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/core/js/keywords/domain/number/big-int-keyword-definition.js`:
- Around line 8-16: The big-int keyword validation is comparing and
range-checking raw values without accounting for the declared
bigint/number/string types, so quoted numeric inputs can be misordered and
non-number multipleOf values bypass validation. Update the validation path in
validateBigIntBounds to normalize min/max before the createOrderedArgsValidator
comparison, and adjust createNumericArgRangeValidator usage so multipleOf is
coerced or validated across bigint/string/number inputs instead of skipping
non-number values. Keep the fix localized to the big-int keyword validators and
preserve the existing argument names and error behavior.
In `@scripts/compare-domain-faker-params.mjs`:
- Around line 220-234: The runCli check logic only fails when rows are missing
in the domain, so it can miss regressions where domainOnlyParams are
reintroduced. Update the --check path in runCli to also inspect the
domainOnlyParams data from getFakerOptionParamComparison() and set
process.exitCode = 1 whenever any row has non-empty domain-only params. Keep the
existing missingRows check, but expand the failure condition so the CLI enforces
both no missing Faker params and no domain-only params.
---
Nitpick comments:
In
`@packages/core-ui/src/tests/interaction/matrix/schema-interaction-scenario-builder.test.js`:
- Around line 19-22: `isScenarioArgCoverageRequired` is hardcoding the
`location.zipCode`/`state` exception instead of using the argument metadata.
Update the helper in `schema-interaction-scenario-builder.test.js` to accept the
full `arg` object (as in `command-help-examples.test-support.js`) and decide
coverage from `arg.usageExampleSupported` rather than string-matching `command`
and `argName`. Then update the related call sites and any duplicate logic around
the referenced scenario coverage checks so unsupported args are excluded
automatically.
In
`@packages/core/js/keywords/domain/internet/http-status-code-keyword-definition.js`:
- Line 8: The `httpStatusCode` keyword definition uses `optionsFromHelpArgs`
correctly, but `types` still lacks enum-level validation, so invalid values fall
through to faker errors instead of a domain validation message. Update the
`http-status-code-keyword-definition` setup to validate `types` against the
allowed `HttpStatusCodeType` values in the same pattern used by other
array-typed args in this cohort, keeping the existing `argTransform` behavior
intact.
In `@packages/core/src/tests/command-help/command-help-examples.test-support.js`:
- Around line 33-40: Remove the hardcoded `entry?.command === 'location.zipCode'
&& param?.name === 'state'` բացառion from `isUsageExampleSupportedParam`; it
duplicates the existing `param?.usageExampleSupported !== false` check and is
already covered by `LOCATION_ZIP_CODE_KEYWORD_DEFINITION`. Keep the function
relying only on `param.usageExampleSupported` so `isUsageExampleSupportedParam`
stays the single source of truth.
In
`@packages/core/src/tests/data_generation/unit/domain/domain-param-invocation-coverage.test-helper.js`:
- Around line 19-54: The sample-value helpers are duplicated across this test
helper and the related domain tests, and the bigint logic is already drifting
between copies. Extract the shared logic into a single reusable test utility and
have sampleValueForType/sampleValueForKeywordArg in this file and the matching
helpers in domain-keyword-params-usage.test.js and domainKeywords.test.js call
it, keeping bigint handling centralized in one place.
In `@scripts/compare-domain-faker-params.mjs`:
- Around line 106-136: Method overloads in compare-domain-faker-params.mjs are
being collapsed because methods.set(methodName, ...) overwrites earlier entries
in the body scan. Update the parsing logic around the methods Map and the
methodStartRegex loop to preserve overloads for the same methodName, and ensure
the options-bearing overload is selected (or merged) rather than replaced by the
last-seen signature. Use the existing findMatching, getObjectOptionKeys, and
paramsText extraction flow to collect all overload signatures before deciding
which signature should drive fakerOptionParams.
- Around line 68-84: The getFakerDeclarationPath() lookup is still brittle
because it scans the dist bundle and matches the internal "declare class
NumberModule" text. Replace that string-based bundle search with a stable
resolution strategy, such as using the known exported declaration path from
`@faker-js/faker/package.json` or another export-based lookup that does not depend
on internal declaration contents, while keeping the fallback error in place if
resolution fails.
🪄 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 Plus
Run ID: 6d6abd09-c118-4a9c-ad3b-34adbb8a1cea
📒 Files selected for processing (48)
docs-src/docs/040-test-data/domain/020-airline.mddocs-src/docs/040-test-data/domain/140-git.mddocs-src/docs/040-test-data/domain/160-image.mddocs-src/docs/040-test-data/domain/170-internet.mddocs-src/docs/040-test-data/domain/190-location.mddocs-src/docs/040-test-data/domain/200-lorem.mddocs-src/docs/040-test-data/domain/220-number.mddocs-src/docs/040-test-data/domain/230-person.mddocs-src/docs/040-test-data/domain/270-system.mddocs-src/docs/040-test-data/domain/290-word.mddocs/domain-faker-param-comparison.mdpackages/core-ui/src/tests/interaction/matrix/schema-interaction-scenario-builder.test.jspackages/core/js/domain/domain-keywords.jspackages/core/js/keywords/domain/airline/flight-number-keyword-definition.jspackages/core/js/keywords/domain/airline/record-locator-keyword-definition.jspackages/core/js/keywords/domain/git/commit-date-keyword-definition.jspackages/core/js/keywords/domain/git/commit-entry-keyword-definition.jspackages/core/js/keywords/domain/git/commit-sha-keyword-definition.jspackages/core/js/keywords/domain/image/data-uri-keyword-definition.jspackages/core/js/keywords/domain/image/person-portrait-keyword-definition.jspackages/core/js/keywords/domain/image/url-picsum-photos-keyword-definition.jspackages/core/js/keywords/domain/internet/display-name-keyword-definition.jspackages/core/js/keywords/domain/internet/example-email-keyword-definition.jspackages/core/js/keywords/domain/internet/http-status-code-keyword-definition.jspackages/core/js/keywords/domain/location/zip-code-keyword-definition.jspackages/core/js/keywords/domain/lorem/word-keyword-definition.jspackages/core/js/keywords/domain/number/big-int-keyword-definition.jspackages/core/js/keywords/domain/person/full-name-keyword-definition.jspackages/core/js/keywords/domain/person/sex-type-keyword-definition.jspackages/core/js/keywords/domain/system/file-name-keyword-definition.jspackages/core/js/keywords/domain/system/network-interface-keyword-definition.jspackages/core/js/keywords/domain/word/adjective-keyword-definition.jspackages/core/js/keywords/domain/word/adverb-keyword-definition.jspackages/core/js/keywords/domain/word/conjunction-keyword-definition.jspackages/core/js/keywords/domain/word/interjection-keyword-definition.jspackages/core/js/keywords/domain/word/noun-keyword-definition.jspackages/core/js/keywords/domain/word/preposition-keyword-definition.jspackages/core/js/keywords/domain/word/sample-keyword-definition.jspackages/core/js/keywords/domain/word/verb-keyword-definition.jspackages/core/js/keywords/domain/word/words-keyword-definition.jspackages/core/src/tests/command-help/command-help-examples.test-support.jspackages/core/src/tests/command-help/command-help-examples.test.jspackages/core/src/tests/data_generation/unit/domain/domain-faker-param-comparison.test.jspackages/core/src/tests/data_generation/unit/domain/domain-keyword-params-usage.test.jspackages/core/src/tests/data_generation/unit/domain/domain-keyword-parser.test.jspackages/core/src/tests/data_generation/unit/domain/domain-param-invocation-coverage.test-helper.jspackages/core/src/tests/data_generation/unit/domain/domainKeywords.test.jsscripts/compare-domain-faker-params.mjs
💤 Files with no reviewable changes (12)
- packages/core/js/keywords/domain/word/noun-keyword-definition.js
- packages/core/js/keywords/domain/word/sample-keyword-definition.js
- packages/core/js/keywords/domain/word/words-keyword-definition.js
- packages/core/js/keywords/domain/word/interjection-keyword-definition.js
- docs-src/docs/040-test-data/domain/200-lorem.md
- packages/core/js/keywords/domain/word/adverb-keyword-definition.js
- packages/core/js/keywords/domain/word/verb-keyword-definition.js
- packages/core/js/keywords/domain/word/conjunction-keyword-definition.js
- packages/core/js/keywords/domain/word/adjective-keyword-definition.js
- packages/core/js/keywords/domain/word/preposition-keyword-definition.js
- packages/core/js/keywords/domain/lorem/word-keyword-definition.js
- docs-src/docs/040-test-data/domain/290-word.md
This was referenced Jul 1, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/js/keywords/domain/lorem/lorem-arg-validators.js (1)
1-13: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReject mixed lorem count argument schemes
packages/core/js/keywords/domain/lorem/lorem-arg-validators.jsaccepts bothmin/maxand*CountMin/*CountMax, but the transform only uses one scheme and silently ignores the other. Reject mixed inputs or normalize them before validation.🤖 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/core/js/keywords/domain/lorem/lorem-arg-validators.js` around lines 1 - 13, The lorem count validator currently accepts both the generic min/max pair and the specific count names at the same time in createLoremCountArgsValidator, but the downstream transform only honors one scheme. Update the validation flow in createLoremCountArgsValidator to reject mixed argument schemes (or normalize to a single scheme before composing validators), and make sure the check uses the existing countName, minName, and maxName symbols so the behavior is enforced consistently.
🧹 Nitpick comments (1)
packages/core/js/keywords/domain/word/adverb-keyword-definition.js (1)
2-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
WORD_SELECTION_STRATEGY_TYPE/validateWordSelectionArgsacross many keyword files.The same two-line block is repeated verbatim in adverb, sample, preposition, interjection, and verb keyword definitions. Consider extracting a shared helper (e.g., in
common-arg-validators.js) such ascreateWordSelectionArgsValidator()returning both the type string and validator, so future changes to the strategy enum or length rules only need to happen once.♻️ Example consolidation
// shared/common-arg-validators.js export const WORD_SELECTION_STRATEGY_TYPE = 'fail|closest|shortest|longest|any-length'; export const validateWordSelectionArgs = createPositiveIntegerArgsValidator(['length']);-const WORD_SELECTION_STRATEGY_TYPE = 'fail|closest|shortest|longest|any-length'; -const validateWordSelectionArgs = createPositiveIntegerArgsValidator(['length']); +import { WORD_SELECTION_STRATEGY_TYPE, validateWordSelectionArgs } from '../shared/common-arg-validators.js';🤖 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/core/js/keywords/domain/word/adverb-keyword-definition.js` around lines 2 - 5, The duplicated WORD_SELECTION_STRATEGY_TYPE and validateWordSelectionArgs block in the keyword definition files should be centralized into a shared helper. Move this repeated setup into common-arg-validators.js as a reusable export or factory such as createWordSelectionArgsValidator(), then update adverb-keyword-definition.js and the other keyword definitions (sample, preposition, interjection, verb) to import the shared symbols instead of redefining them. Keep the exported strategy type string and length validator behavior identical while removing the repeated local declarations.
🤖 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/core/js/data_generation/domain/domainTestDataRuleValidator.js`:
- Around line 57-73: The validation path in domainTestDataRuleValidator.js is
mutating the shared Faker state because validate() passes this.faker into
executeDomainKeyword() for faker delegates. Update the validation logic around
executeDomainKeyword() to use an isolated Faker/randomizer instance or snapshot
and restore Faker state after the dry-run, so TestDataGenerator does not advance
the shared seed before real generation.
---
Outside diff comments:
In `@packages/core/js/keywords/domain/lorem/lorem-arg-validators.js`:
- Around line 1-13: The lorem count validator currently accepts both the generic
min/max pair and the specific count names at the same time in
createLoremCountArgsValidator, but the downstream transform only honors one
scheme. Update the validation flow in createLoremCountArgsValidator to reject
mixed argument schemes (or normalize to a single scheme before composing
validators), and make sure the check uses the existing countName, minName, and
maxName symbols so the behavior is enforced consistently.
---
Nitpick comments:
In `@packages/core/js/keywords/domain/word/adverb-keyword-definition.js`:
- Around line 2-5: The duplicated WORD_SELECTION_STRATEGY_TYPE and
validateWordSelectionArgs block in the keyword definition files should be
centralized into a shared helper. Move this repeated setup into
common-arg-validators.js as a reusable export or factory such as
createWordSelectionArgsValidator(), then update adverb-keyword-definition.js and
the other keyword definitions (sample, preposition, interjection, verb) to
import the shared symbols instead of redefining them. Keep the exported strategy
type string and length validator behavior identical while removing the repeated
local declarations.
🪄 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 Plus
Run ID: 821ac140-3fa3-4741-9c38-bd1f76447ed0
📒 Files selected for processing (53)
docs-src/docs/040-test-data/domain/020-airline.mddocs-src/docs/040-test-data/domain/190-location.mddocs-src/docs/040-test-data/domain/200-lorem.mddocs-src/docs/040-test-data/domain/250-science.mdpackages/core/js/data_generation/domain/domainTestDataRuleValidator.jspackages/core/js/data_generation/testDataRulesCompiler.jspackages/core/js/domain/domain-keyword-arg-validators.jspackages/core/js/domain/domain-keywords.jspackages/core/js/keywords/domain/airline/flight-number-keyword-definition.jspackages/core/js/keywords/domain/date/betweens-keyword-definition.jspackages/core/js/keywords/domain/finance/account-number-keyword-definition.jspackages/core/js/keywords/domain/finance/pin-keyword-definition.jspackages/core/js/keywords/domain/git/commit-sha-keyword-definition.jspackages/core/js/keywords/domain/image/data-uri-keyword-definition.jspackages/core/js/keywords/domain/image/url-keyword-definition.jspackages/core/js/keywords/domain/image/url-picsum-photos-keyword-definition.jspackages/core/js/keywords/domain/internet/http-status-code-keyword-definition.jspackages/core/js/keywords/domain/internet/password-keyword-definition.jspackages/core/js/keywords/domain/lorem/lines-keyword-definition.jspackages/core/js/keywords/domain/lorem/lorem-arg-validators.jspackages/core/js/keywords/domain/lorem/paragraph-keyword-definition.jspackages/core/js/keywords/domain/lorem/paragraphs-keyword-definition.jspackages/core/js/keywords/domain/lorem/sentence-keyword-definition.jspackages/core/js/keywords/domain/lorem/sentences-keyword-definition.jspackages/core/js/keywords/domain/lorem/slug-keyword-definition.jspackages/core/js/keywords/domain/lorem/word-keyword-definition.jspackages/core/js/keywords/domain/lorem/words-keyword-definition.jspackages/core/js/keywords/domain/number/big-int-keyword-definition.jspackages/core/js/keywords/domain/shared/common-arg-validators.jspackages/core/js/keywords/domain/string/alpha-keyword-definition.jspackages/core/js/keywords/domain/string/alphanumeric-keyword-definition.jspackages/core/js/keywords/domain/string/binary-keyword-definition.jspackages/core/js/keywords/domain/string/from-characters-keyword-definition.jspackages/core/js/keywords/domain/string/hexadecimal-keyword-definition.jspackages/core/js/keywords/domain/string/nanoid-keyword-definition.jspackages/core/js/keywords/domain/string/numeric-keyword-definition.jspackages/core/js/keywords/domain/string/octal-keyword-definition.jspackages/core/js/keywords/domain/string/sample-keyword-definition.jspackages/core/js/keywords/domain/string/symbol-keyword-definition.jspackages/core/js/keywords/domain/word/adjective-keyword-definition.jspackages/core/js/keywords/domain/word/adverb-keyword-definition.jspackages/core/js/keywords/domain/word/conjunction-keyword-definition.jspackages/core/js/keywords/domain/word/interjection-keyword-definition.jspackages/core/js/keywords/domain/word/noun-keyword-definition.jspackages/core/js/keywords/domain/word/preposition-keyword-definition.jspackages/core/js/keywords/domain/word/sample-keyword-definition.jspackages/core/js/keywords/domain/word/verb-keyword-definition.jspackages/core/js/keywords/domain/word/words-keyword-definition.jspackages/core/src/tests/data_generation/unit/domain/domain-doc-generator-output.test.jspackages/core/src/tests/data_generation/unit/domain/domain-keyword-params-usage.test.jspackages/core/src/tests/data_generation/unit/domain/domain-test-data-rule-validator.test.jspackages/core/src/tests/data_generation/unit/domain/domainKeywords.test.jsscripts/generate-domain-docs.mjs
✅ Files skipped from review due to trivial changes (4)
- docs-src/docs/040-test-data/domain/250-science.md
- docs-src/docs/040-test-data/domain/190-location.md
- docs-src/docs/040-test-data/domain/020-airline.md
- docs-src/docs/040-test-data/domain/200-lorem.md
🚧 Files skipped from review as they are similar to previous changes (11)
- packages/core/js/keywords/domain/git/commit-sha-keyword-definition.js
- packages/core/js/keywords/domain/airline/flight-number-keyword-definition.js
- packages/core/js/keywords/domain/image/data-uri-keyword-definition.js
- packages/core/js/keywords/domain/lorem/word-keyword-definition.js
- packages/core/js/keywords/domain/number/big-int-keyword-definition.js
- packages/core/js/keywords/domain/internet/http-status-code-keyword-definition.js
- packages/core/js/domain/domain-keywords.js
- packages/core/js/keywords/domain/word/conjunction-keyword-definition.js
- packages/core/js/keywords/domain/image/url-picsum-photos-keyword-definition.js
- packages/core/js/keywords/domain/word/noun-keyword-definition.js
- packages/core/src/tests/data_generation/unit/domain/domain-keyword-params-usage.test.js
|
Too many files changed for review. ( Bypass the limit by tagging |
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
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
Closes #286.
word.*(max=...)andlorem.word(min/max=...)casesValidation
pnpm run verify:localpnpm run verify:localsuccessfullySummary by CodeRabbit
Documentation / New Features
lorem.word/word.*option contracts (removedmax, clarifiedword.words(count)).Bug Fixes
number.bigIntbounds/multiples and for validated option sets likeinternet.httpStatusCode(types).Tests / Chores