fix(cli): Fix composite JSON

[brian-lou]

Greptile Summary

This PR fixes a bug in the composite JSON pipeline (parseJson → mergeJson → extractJson) where locale-keyed values that are plain strings (e.g. { "en": "Hello", "fr": "Bonjour" }) were not handled correctly — the code previously tried to flatten or object-merge them via JSONPath, which silently produced empty results. The fix adds a type guard at each stage of the pipeline to short-circuit into a string-direct path when the source/target item is a string.

Key changes:

• parseJson.ts: stores a string sourceItem directly in sourceObjectsToTranslate instead of calling flattenJson; type signature updated to Record<string, Record<string, Record<string, string>> | string>.
• mergeJson.ts: when defaultLocaleSourceItem is a string, uses the translated string directly instead of cloning and applying object-level JSONPointer merges. mutateSourceItemKey declaration moved before the new guard.
• extractJson.ts: stores a string sourceItem directly in compositeResult instead of calling flattenJsonWithStringFilter.

Issues found:

• In mergeJson.ts, the existing !targetItems → {} coercion (line 331–333) runs before the new string check. Because !"" is true, an empty-string translation is silently replaced with {}, causing the new guard to fall back to the default-locale string rather than the (intentionally empty) translation.
• No test cases were added for the string-valued composite path across any of the three files, leaving the bug fix without a regression test.

Confidence Score: 3/5

• Safe to merge for the common case; one edge-case with empty-string translations and missing test coverage lower confidence.
• The core fix is logically correct and consistent across all three pipeline stages. However, the upstream !targetItems → {} coercion in mergeJson.ts quietly swallows empty-string translated values before the new string guard can inspect them, which is a subtle correctness gap. Additionally, there are no new tests covering the string-valued composite path, so regressions in this flow would not be caught automatically.
• packages/cli/src/formats/json/mergeJson.ts — the !targetItems null-coercion on lines 331–333 interacts poorly with the new string path.

Important Files Changed

Filename	Overview
packages/cli/src/formats/json/parseJson.ts	Adds string shortcut in composite object-type path: if sourceItem is a plain string, it's stored directly in sourceObjectsToTranslate instead of being flattened. Type of sourceObjectsToTranslate updated accordingly. No new tests cover this path.
packages/cli/src/formats/json/mergeJson.ts	Adds string shortcut in composite object-type path: if defaultLocaleSourceItem is a string, the translated string is used directly instead of object-merging. Has a subtle edge-case: the upstream !targetItems → {} coercion silently discards empty-string translations before the new string check runs.
packages/cli/src/formats/json/extractJson.ts	Adds string shortcut in composite object-type path: if the matched target item's sourceItem is a plain string, it's stored directly in compositeResult instead of being flattened. Logic is straightforward and consistent with the other two files.
.changeset/smart-bobcats-cheat.md	Patch-level changeset for the gt package. Description is minimal ("Fix string behavior") but package version bump and changeset type are appropriate.
packages/cli/src/generated/version.ts	Auto-generated version bump from 2.10.0 to 2.10.1, consistent with the patch changeset.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Composite JSON file\ne.g. {en: 'Hello', fr: 'Bonjour'}"] --> B["parseJson()"]

    B --> C{sourceItem\ntype?}
    C -- "string (NEW)" --> D["Store string directly\nsourceObjectsToTranslate[ptr] = sourceItem"]
    C -- "object" --> E["Flatten via JSONPath\nsourceObjectsToTranslate[ptr] = flattenJson(...)"]

    D --> F["Serialised payload\nsent to translation API"]
    E --> F

    F --> G["mergeJson()"]

    G --> H{defaultLocaleSourceItem\ntype?}
    H -- "string (NEW)" --> I{"targetItems\ntype?"}
    I -- "string" --> J["Use translated string\nsourceObjectValue[key] = targetItems"]
    I -- "other (or empty string coerced to {})" --> K["Fallback to default\nsourceObjectValue[key] = defaultLocaleSourceItem"]
    H -- "object" --> L["Object merge path\n(clone + JSONPointer.set)"]

    J --> M["JSONPointer.set(mergedJson, ptr, sourceObjectValue)"]
    K --> M
    L --> M

    M --> N["extractJson()"]

    N --> O{matchingTargetItem\n.sourceItem type?}
    O -- "string (NEW)" --> P["Store string directly\ncompositeResult[ptr] = sourceItem"]
    O -- "object" --> Q["Flatten via flattenJsonWithStringFilter"]

    P --> R["Final composite JSON output"]
    Q --> R

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: packages/cli/src/formats/json/mergeJson.ts
Line: 347-354

Comment:
**`targetItems` replaced before string check**

The `!targetItems` guard on line 331–333 replaces a falsy `targetItems` with `{}` before the new string path is reached. An empty-string translation (`""`) is falsy, so it gets silently overwritten with `{}`, causing `typeof targetItems === 'string'` to be `false` and the code to fall back to `defaultLocaleSourceItem`. This means an empty translated string in the source map (intentional or otherwise) is silently discarded and replaced with the default-locale value.

```ts
let targetItems = targetJson[sourceObjectPointer];
if (!targetItems) {
  targetItems = {};
}
```

Consider tightening the guard to only replace `null`/`undefined`, not empty strings:

```suggestion
        // If the source item is a string, use the translated string directly
        if (typeof defaultLocaleSourceItem === 'string') {
          const translatedValue =
            typeof targetItems === 'string'
              ? targetItems
              : defaultLocaleSourceItem;
          sourceObjectValue[mutateSourceItemKey] = translatedValue;
          continue;
        }
```

The real fix belongs at the `!targetItems` initialisation (line 331):
```ts
// Before (replaces "" with {}):
if (!targetItems) { targetItems = {}; }

// After (only replaces null/undefined):
if (targetItems == null) { targetItems = {}; }
```
This keeps `""` as a string so the downstream `typeof targetItems === 'string'` check works correctly.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: packages/cli/src/formats/json/parseJson.ts
Line: 63-66

Comment:
**No tests for new string handling**

There are no test cases in `parseJson.test.ts`, `mergeJson.test.ts`, or `extractJson.test.ts` covering the new string-value composite path (e.g. `{ translations: { en: "Hello", fr: "Bonjour" } }`). Since this PR is specifically fixing a bug in this code path, a regression test covering the full parse → merge → extract round-trip for string-valued composite schemas would significantly improve confidence in the fix.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: 9925731}

Greptile also left 2 inline comments on this PR.

[greptile-apps]

targetItems replaced before string check

The !targetItems guard on line 331–333 replaces a falsy targetItems with {} before the new string path is reached. An empty-string translation ("") is falsy, so it gets silently overwritten with {}, causing typeof targetItems === 'string' to be false and the code to fall back to defaultLocaleSourceItem. This means an empty translated string in the source map (intentional or otherwise) is silently discarded and replaced with the default-locale value.

let targetItems = targetJson[sourceObjectPointer];
if (!targetItems) {
  targetItems = {};
}

Consider tightening the guard to only replace null/undefined, not empty strings:

Suggested change

	if (typeof defaultLocaleSourceItem === 'string') {
	const translatedValue =
	typeof targetItems === 'string'
	? targetItems
	: defaultLocaleSourceItem;
	sourceObjectValue[mutateSourceItemKey] = translatedValue;
	continue;
	}
	// If the source item is a string, use the translated string directly
	if (typeof defaultLocaleSourceItem === 'string') {
	const translatedValue =
	typeof targetItems === 'string'
	? targetItems
	: defaultLocaleSourceItem;
	sourceObjectValue[mutateSourceItemKey] = translatedValue;
	continue;
	}

The real fix belongs at the !targetItems initialisation (line 331):

// Before (replaces "" with {}):
if (!targetItems) { targetItems = {}; }

// After (only replaces null/undefined):
if (targetItems == null) { targetItems = {}; }

This keeps "" as a string so the downstream typeof targetItems === 'string' check works correctly.

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/cli/src/formats/json/mergeJson.ts
Line: 347-354

Comment:
**`targetItems` replaced before string check**

The `!targetItems` guard on line 331–333 replaces a falsy `targetItems` with `{}` before the new string path is reached. An empty-string translation (`""`) is falsy, so it gets silently overwritten with `{}`, causing `typeof targetItems === 'string'` to be `false` and the code to fall back to `defaultLocaleSourceItem`. This means an empty translated string in the source map (intentional or otherwise) is silently discarded and replaced with the default-locale value.

```ts
let targetItems = targetJson[sourceObjectPointer];
if (!targetItems) {
  targetItems = {};
}
```

Consider tightening the guard to only replace `null`/`undefined`, not empty strings:

```suggestion
        // If the source item is a string, use the translated string directly
        if (typeof defaultLocaleSourceItem === 'string') {
          const translatedValue =
            typeof targetItems === 'string'
              ? targetItems
              : defaultLocaleSourceItem;
          sourceObjectValue[mutateSourceItemKey] = translatedValue;
          continue;
        }
```

The real fix belongs at the `!targetItems` initialisation (line 331):
```ts
// Before (replaces "" with {}):
if (!targetItems) { targetItems = {}; }

// After (only replaces null/undefined):
if (targetItems == null) { targetItems = {}; }
```
This keeps `""` as a string so the downstream `typeof targetItems === 'string'` check works correctly.

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

No tests for new string handling

There are no test cases in parseJson.test.ts, mergeJson.test.ts, or extractJson.test.ts covering the new string-value composite path (e.g. { translations: { en: "Hello", fr: "Bonjour" } }). Since this PR is specifically fixing a bug in this code path, a regression test covering the full parse → merge → extract round-trip for string-valued composite schemas would significantly improve confidence in the fix.

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/cli/src/formats/json/parseJson.ts
Line: 63-66

Comment:
**No tests for new string handling**

There are no test cases in `parseJson.test.ts`, `mergeJson.test.ts`, or `extractJson.test.ts` covering the new string-value composite path (e.g. `{ translations: { en: "Hello", fr: "Bonjour" } }`). Since this PR is specifically fixing a bug in this code path, a regression test covering the full parse → merge → extract round-trip for string-valued composite schemas would significantly improve confidence in the fix.

How can I resolve this? If you propose a fix, please make it concise.

[fernando-aviles]

lgtm