Feat/i18n tooling#163
Open
agualdron wants to merge 4 commits into
Open
Conversation
Add an *i18n:catalog* CLI that scans route or full-app entry graphs, resolves route ownership, and reports used keys, missing keys, orphaned keys, locale gaps, and hardcoded copy candidates. Cover the scanner with fixture-heavy tests for entry discovery, route resolution, namespace ownership, dynamic copy access, and hardcoded metadata detection. Strengthen *public-copy* tests to preserve landing array shapes and keep locale catalog structures aligned. Update *type-check* to run *next typegen* first and use the dedicated *tsconfig.typecheck.json* so generated Next route types are included in type validation.
Contributor
|
| Filename | Overview |
|---|---|
| apps/tradinggoose/scripts/i18n-catalog/catalog.ts | Report builder: loads all locale catalogs, indexes keys, computes missing/orphaned keys and target-locale gaps. Orphan detection uses Array.includes (O(n²)) and the default-locale catalog is accessed via hardcoded .en instead of catalogs[defaultLocale]. |
| apps/tradinggoose/scripts/i18n-catalog/scan.ts | ~3500-line TypeScript AST scanner: collects import/export bindings, propagates descriptor semantics across files, and emits coverage and hardcoded-candidate records for each entry invocation. Logic is dense but well-structured; no direct correctness issues found. |
| apps/tradinggoose/scripts/i18n-catalog/entries.ts | Next.js entry-point discovery: walks the app directory, discovers localized page/layout files, and resolves route-boundary entries. Logic correctly distinguishes localized vs. non-localized boundary entries. |
| apps/tradinggoose/scripts/i18n-catalog/ownership.ts | Route-ownership rules table and namespace derivation helpers. Static table is straightforward; route pattern matching and specificity comparison look correct. |
| apps/tradinggoose/scripts/i18n-catalog/index.ts | CLI façade: parses flags, delegates to scan/report builders, and formats output. Guard logic for flag combinations is correct; error paths set process.exitCode appropriately. |
| apps/tradinggoose/package.json | Adds i18n:catalog script and updates type-check to run next typegen then point tsc at tsconfig.typecheck.json. Changes are targeted and correct. |
| apps/tradinggoose/tsconfig.typecheck.json | Extends base tsconfig and adds .next/types/**/*.ts and explicit entry files so tsc sees generated Next route types. Minimal and correct. |
| apps/tradinggoose/i18n/public-copy.test.ts | Adds two regression guards: landing array shape checks and cross-locale catalog shape alignment via normalizeShape. Both are straightforward additions to the existing test suite. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["bun run i18n:catalog\n--route /path | --all\n[--with-orphans] [--json]"] --> B{Flag validation}
B -->|invalid combo| ERR["throw Error"]
B -->|--route| C["scanCatalogProjectWithContext\nmode: 'route'"]
B -->|--all| D["scanCatalogProjectWithContext\nmode: 'all'"]
C --> E["resolveRouteEntries\n(entries.ts)"]
D --> F["discoverAllModeEntries\n(entries.ts)"]
E --> G["collectReachableFiles\n(BFS over runtime imports)"]
F --> G
G --> H["buildAnalysisProjectFiles\n(load TS AST per file)"]
H --> I["populateProjectFileTypeRoots\n(propagate i18n type descriptors)"]
I --> J["buildGlobalFunctionSemantics\n(infer translator/root descriptors)"]
J --> K["scanFile per entry\n(scan.ts AST walker)"]
K --> L["CatalogScanResult\ncoverage + hardcoded candidates"]
L --> M{withOrphans?}
M -->|yes| N["second all-mode scan\nfor globalScanResult"]
M -->|no| O["buildCatalogReport\n(catalog.ts)"]
N --> O
O --> P["loadLocaleCatalogs\nen.json / es.json / zh.json"]
P --> Q["buildCatalogIndex\n(leaf map + objectPaths)"]
Q --> R["buildCoverageSummary\nmissing keys + used keys"]
R --> S["buildTargetLocaleGaps\nper non-default locale"]
S --> T["CatalogReport\nusedKeys / missingKeys / gaps\norphanedKeys / hardcodedCandidates"]
T -->|--json| U["JSON stdout"]
T -->|text| V["Formatted text stdout"]
%%{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["bun run i18n:catalog\n--route /path | --all\n[--with-orphans] [--json]"] --> B{Flag validation}
B -->|invalid combo| ERR["throw Error"]
B -->|--route| C["scanCatalogProjectWithContext\nmode: 'route'"]
B -->|--all| D["scanCatalogProjectWithContext\nmode: 'all'"]
C --> E["resolveRouteEntries\n(entries.ts)"]
D --> F["discoverAllModeEntries\n(entries.ts)"]
E --> G["collectReachableFiles\n(BFS over runtime imports)"]
F --> G
G --> H["buildAnalysisProjectFiles\n(load TS AST per file)"]
H --> I["populateProjectFileTypeRoots\n(propagate i18n type descriptors)"]
I --> J["buildGlobalFunctionSemantics\n(infer translator/root descriptors)"]
J --> K["scanFile per entry\n(scan.ts AST walker)"]
K --> L["CatalogScanResult\ncoverage + hardcoded candidates"]
L --> M{withOrphans?}
M -->|yes| N["second all-mode scan\nfor globalScanResult"]
M -->|no| O["buildCatalogReport\n(catalog.ts)"]
N --> O
O --> P["loadLocaleCatalogs\nen.json / es.json / zh.json"]
P --> Q["buildCatalogIndex\n(leaf map + objectPaths)"]
Q --> R["buildCoverageSummary\nmissing keys + used keys"]
R --> S["buildTargetLocaleGaps\nper non-default locale"]
S --> T["CatalogReport\nusedKeys / missingKeys / gaps\norphanedKeys / hardcodedCandidates"]
T -->|--json| U["JSON stdout"]
T -->|text| V["Formatted text stdout"]
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
apps/tradinggoose/package.json:15
**Missing changelog entry for staging target**
This PR targets `staging` but adds no dated markdown file under `changelog/`. Per repo convention, every PR that lands on `staging` must include exactly one new dated changelog file. Please add a file such as `changelog/July-06-2026.md` documenting this branch before merging.
### Issue 2 of 3
apps/tradinggoose/scripts/i18n-catalog/catalog.ts:289-294
**Quadratic orphan-key check** — `usedKeys` is a sorted array, so `.includes()` scans it linearly for every leaf in the catalog. With large catalogs (thousands of keys) this becomes O(keys × usedKeys). Convert `usedKeys` to a `Set` once before the filter.
```suggestion
const usedKeySet = globalCoverageSummary ? new Set(globalCoverageSummary.usedKeys) : null
const orphanedKeys = hasGlobalScan
? [...englishCatalogIndex.leafMap.keys()]
.filter((pathKey) => !usedKeySet!.has(pathKey))
.filter((pathKey) => isOwnedPath(pathKey, ownedNamespaces))
.sort()
: null
```
### Issue 3 of 3
apps/tradinggoose/scripts/i18n-catalog/catalog.ts:274
**Hardcoded `.en` instead of `defaultLocale`** — `buildTargetLocaleGaps` correctly derives target locales dynamically via `defaultLocale`, but the main catalog index is built from `catalogs.en`. If `defaultLocale` ever changes, missing-key and orphan reports would silently compare against the wrong locale's catalog. Use `catalogs[defaultLocale]` to keep both paths aligned.
```suggestion
const englishCatalogIndex = buildCatalogIndex(catalogs[defaultLocale])
```
Reviews (1): Last reviewed commit: "style(i18n): align catalog tooling with ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -15,12 +15,13 @@ | |||
| "start": "next start", | |||
Contributor
There was a problem hiding this comment.
This PR targets staging but adds no dated markdown file under changelog/. Per repo convention, every PR that lands on staging must include exactly one new dated changelog file. Please add a file such as changelog/July-06-2026.md documenting this branch before merging.
Rule Used: Any PR targeting staging must add exactly one ne... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/tradinggoose/package.json
Line: 15
Comment:
**Missing changelog entry for staging target**
This PR targets `staging` but adds no dated markdown file under `changelog/`. Per repo convention, every PR that lands on `staging` must include exactly one new dated changelog file. Please add a file such as `changelog/July-06-2026.md` documenting this branch before merging.
**Rule Used:** Any PR targeting `staging` must add exactly one ne... ([source](greptile.json))
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Comment on lines
+289
to
+294
| const orphanedKeys = hasGlobalScan | ||
| ? [...englishCatalogIndex.leafMap.keys()] | ||
| .filter((pathKey) => !globalCoverageSummary!.usedKeys.includes(pathKey)) | ||
| .filter((pathKey) => isOwnedPath(pathKey, ownedNamespaces)) | ||
| .sort() | ||
| : null |
Contributor
There was a problem hiding this comment.
usedKeys is a sorted array, so .includes() scans it linearly for every leaf in the catalog. With large catalogs (thousands of keys) this becomes O(keys × usedKeys). Convert usedKeys to a Set once before the filter.
Suggested change
| const orphanedKeys = hasGlobalScan | |
| ? [...englishCatalogIndex.leafMap.keys()] | |
| .filter((pathKey) => !globalCoverageSummary!.usedKeys.includes(pathKey)) | |
| .filter((pathKey) => isOwnedPath(pathKey, ownedNamespaces)) | |
| .sort() | |
| : null | |
| const usedKeySet = globalCoverageSummary ? new Set(globalCoverageSummary.usedKeys) : null | |
| const orphanedKeys = hasGlobalScan | |
| ? [...englishCatalogIndex.leafMap.keys()] | |
| .filter((pathKey) => !usedKeySet!.has(pathKey)) | |
| .filter((pathKey) => isOwnedPath(pathKey, ownedNamespaces)) | |
| .sort() | |
| : null |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/tradinggoose/scripts/i18n-catalog/catalog.ts
Line: 289-294
Comment:
**Quadratic orphan-key check** — `usedKeys` is a sorted array, so `.includes()` scans it linearly for every leaf in the catalog. With large catalogs (thousands of keys) this becomes O(keys × usedKeys). Convert `usedKeys` to a `Set` once before the filter.
```suggestion
const usedKeySet = globalCoverageSummary ? new Set(globalCoverageSummary.usedKeys) : null
const orphanedKeys = hasGlobalScan
? [...englishCatalogIndex.leafMap.keys()]
.filter((pathKey) => !usedKeySet!.has(pathKey))
.filter((pathKey) => isOwnedPath(pathKey, ownedNamespaces))
.sort()
: null
```
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| export function buildCatalogReport(options: BuildReportOptions): CatalogReport { | ||
| const catalogs = loadLocaleCatalogs(options.projectRoot) | ||
| const englishCatalogIndex = buildCatalogIndex(catalogs.en) |
Contributor
There was a problem hiding this comment.
.en instead of defaultLocale — buildTargetLocaleGaps correctly derives target locales dynamically via defaultLocale, but the main catalog index is built from catalogs.en. If defaultLocale ever changes, missing-key and orphan reports would silently compare against the wrong locale's catalog. Use catalogs[defaultLocale] to keep both paths aligned.
Suggested change
| const englishCatalogIndex = buildCatalogIndex(catalogs.en) | |
| const englishCatalogIndex = buildCatalogIndex(catalogs[defaultLocale]) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/tradinggoose/scripts/i18n-catalog/catalog.ts
Line: 274
Comment:
**Hardcoded `.en` instead of `defaultLocale`** — `buildTargetLocaleGaps` correctly derives target locales dynamically via `defaultLocale`, but the main catalog index is built from `catalogs.en`. If `defaultLocale` ever changes, missing-key and orphan reports would silently compare against the wrong locale's catalog. Use `catalogs[defaultLocale]` to keep both paths aligned.
```suggestion
const englishCatalogIndex = buildCatalogIndex(catalogs[defaultLocale])
```
How can I resolve this? If you propose a fix, please make it concise.…163) Extract graph reachability, runtime scanning, semantic analysis, and shared descriptor logic from *scan.ts* into dedicated modules to reduce scanner complexity and isolate responsibilities. Move shared catalog test fixtures into *test-utils.ts* and replace the monolithic *index.test.ts* suite with focused graph, runtime, semantics, scan, and report test files.
Document the feat/i18n_tooling branch in the dated staging changelog. Summarize the new i18n catalog tooling, type-check updates, and test coverage noted for the branch diff against origin/staging.
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
bun run i18n:catalogdeveloper CLI underapps/tradinggoose/scripts/i18n-catalogto scan either a single localized route or the full app and report used keys, missing keys, target-locale gaps, optional orphaned keys, dynamic protected roots, and hardcoded copy candidates.apps/tradinggoose/i18n/public-copy.test.tsto guard landing array shapes and cross-locale catalog shape alignment, and updates app type-checking to runnext typegenthrough a dedicatedtsconfig.typecheck.json.Why
next typegenbeforetsckeeps type-checking aligned with generated Next route types instead of relying on incomplete compile inputs.Affected Areas
apps/tradinggooseapps/docspackages/*Validation