Skip to content

chore: bump version_id changeset from patch to minor#29

Merged
ErnestM1234 merged 1 commit intogeneraltranslation:mainfrom
moss-bryophyta:fix/minor-version-bump-version-id
Mar 19, 2026
Merged

chore: bump version_id changeset from patch to minor#29
ErnestM1234 merged 1 commit intogeneraltranslation:mainfrom
moss-bryophyta:fix/minor-version-bump-version-id

Conversation

@moss-bryophyta
Copy link
Copy Markdown
Contributor

@moss-bryophyta moss-bryophyta commented Mar 19, 2026

The version_id feature (get_version_id() + version_id param for initialize_gt()) was mistakenly tagged as a patch bump. This updates the changeset to minor for gt-i18n, gt-fastapi, and gt-flask.

Greptile Summary

This PR introduces version_id support across gt-i18n, gt-fastapi, and gt-flask — adding a version_id parameter to initialize_gt(), storing it in I18nManager, exposing it via a new get_version_id() helper, and mapping it from _versionId in gt.config.json — then corrects the changeset bump from patch to minor to match the scope of the feature addition.

  • version_id stored but not used in CDN translation loading: The parameter is accepted, resolved from config/args, and stored in I18nManager, but create_remote_translation_loader() is never given version_id, so the built-in CDN URL remains {cache_url}/{project_id}/{locale}. Users expecting the flag to "pin translations" (as the docstring states) will observe no difference in behaviour when using the default CDN loader.
  • Unconventional _versionId key: The gt.config.json key uses a leading underscore, unlike every other key in _KEY_MAP. If this is intentional (marking it as an internal/private field) a comment would clarify intent; otherwise it should be "versionId" for consistency.
  • The plumbing across gt-fastapi and gt-flask (param → resolve → pass to manager) is symmetric and correct.
  • The new helpers/_version_id.py follows the same pattern as the existing helpers/_locales.py.

Confidence Score: 3/5

  • The PR is mergeable but the core feature does not work as documented — version_id is stored but has no effect on translation loading.
  • The changeset bump (patch → minor) is correct and the plumbing code is well-structured. However, the docstring explicitly promises "pinning translations" and the version_id is never forwarded to the remote CDN loader, meaning the feature silently does nothing when using the default loader. This is a functional gap that should be resolved or clearly documented before shipping.
  • packages/gt-i18n/src/gt_i18n/i18n_manager/_i18n_manager.py and packages/gt-i18n/src/gt_i18n/i18n_manager/_remote_loader.py need attention to wire version_id into the CDN URL.

Important Files Changed

Filename Overview
packages/gt-i18n/src/gt_i18n/i18n_manager/_i18n_manager.py Adds version_id param to I18nManager.__init__ and a get_version_id() getter, but the stored value is never forwarded to the CDN remote loader, so it has no actual effect on which translations are fetched.
packages/gt-i18n/src/gt_i18n/helpers/_version_id.py New helper that delegates get_version_id() to the manager singleton; follows the same pattern as _locales.py.
packages/gt-i18n/src/gt_i18n/internal/_load_gt_config.py Maps _versionId (note: leading underscore, unlike every other key) to version_id in the config; also adds version_id to the GTConfig TypedDict.
packages/gt-fastapi/src/gt_fastapi/_setup.py Adds version_id param to initialize_gt(), resolves it against the file config, and passes it to I18nManager. Implementation is consistent and correct at the setup level.
packages/gt-flask/src/gt_flask/_setup.py Same version_id wiring as gt-fastapi. Pre-existing: eager loading uses unresolved locales instead of resolved_locales, but that is outside the scope of this PR.

Sequence Diagram

sequenceDiagram
    participant App
    participant initialize_gt as initialize_gt() (fastapi/flask)
    participant I18nManager
    participant RemoteLoader as create_remote_translation_loader
    participant CDN

    App->>initialize_gt: version_id, project_id, cache_url, ...
    initialize_gt->>initialize_gt: resolve version_id (arg || config)
    initialize_gt->>I18nManager: __init__(version_id=resolved_version_id, project_id, cache_url, ...)
    Note over I18nManager: self._version_id = version_id ✓
    I18nManager->>RemoteLoader: create_remote_translation_loader(project_id, cache_url)
    Note over RemoteLoader: version_id NOT forwarded ✗
    RemoteLoader-->>I18nManager: loader(locale) → {cache_url}/{project_id}/{locale}
    App->>I18nManager: get_version_id()
    I18nManager-->>App: self._version_id (readable only)
    App->>I18nManager: get_translations(locale)
    I18nManager->>CDN: GET {cache_url}/{project_id}/{locale}
    Note over CDN: version_id has no effect on URL
Loading

Comments Outside Diff (1)

  1. packages/gt-i18n/src/gt_i18n/i18n_manager/_i18n_manager.py, line 63-67 (link)

    P1 version_id not forwarded to the remote loader

    version_id is stored on the manager and exposed via get_version_id(), but when the built-in CDN loader is constructed (lines 63–67), it only receives project_id and cache_url. The resulting URL is {cache_url}/{project_id}/{locale} — the version_id is never incorporated.

    The docstrings in every initialize_gt() say "Version ID for pinning translations", but with the current implementation the flag has zero effect on which translations are fetched from the CDN. Users who set version_id expecting deterministic/pinned translations will silently get the latest (un-pinned) version instead.

    If the CDN URL is expected to include the version, create_remote_translation_loader needs to accept and embed version_id:

    elif project_id:
        from gt_i18n.i18n_manager._remote_loader import (
            create_remote_translation_loader,
        )
        loader = create_remote_translation_loader(project_id, cache_url or "", version_id)

    And _remote_loader.py would need to build the URL as:

    url = f"{cache_url}/{project_id}/{version_id}/{locale}" if version_id else f"{cache_url}/{project_id}/{locale}"

    If passing version_id to the CDN URL is intentionally deferred, the docstring should be updated to clarify that the parameter is currently only accessible via get_version_id() for use in custom load_translations callbacks.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: packages/gt-i18n/src/gt_i18n/i18n_manager/_i18n_manager.py
    Line: 63-67
    
    Comment:
    **`version_id` not forwarded to the remote loader**
    
    `version_id` is stored on the manager and exposed via `get_version_id()`, but when the built-in CDN loader is constructed (lines 63–67), it only receives `project_id` and `cache_url`. The resulting URL is `{cache_url}/{project_id}/{locale}` — the `version_id` is never incorporated.
    
    The docstrings in every `initialize_gt()` say _"Version ID for pinning translations"_, but with the current implementation the flag has zero effect on which translations are fetched from the CDN. Users who set `version_id` expecting deterministic/pinned translations will silently get the latest (un-pinned) version instead.
    
    If the CDN URL is expected to include the version, `create_remote_translation_loader` needs to accept and embed `version_id`:
    
    ```python
    elif project_id:
        from gt_i18n.i18n_manager._remote_loader import (
            create_remote_translation_loader,
        )
        loader = create_remote_translation_loader(project_id, cache_url or "", version_id)
    ```
    
    And `_remote_loader.py` would need to build the URL as:
    ```python
    url = f"{cache_url}/{project_id}/{version_id}/{locale}" if version_id else f"{cache_url}/{project_id}/{locale}"
    ```
    
    If passing `version_id` to the CDN URL is intentionally deferred, the docstring should be updated to clarify that the parameter is currently only accessible via `get_version_id()` for use in custom `load_translations` callbacks.
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: packages/gt-i18n/src/gt_i18n/i18n_manager/_i18n_manager.py
Line: 63-67

Comment:
**`version_id` not forwarded to the remote loader**

`version_id` is stored on the manager and exposed via `get_version_id()`, but when the built-in CDN loader is constructed (lines 63–67), it only receives `project_id` and `cache_url`. The resulting URL is `{cache_url}/{project_id}/{locale}` — the `version_id` is never incorporated.

The docstrings in every `initialize_gt()` say _"Version ID for pinning translations"_, but with the current implementation the flag has zero effect on which translations are fetched from the CDN. Users who set `version_id` expecting deterministic/pinned translations will silently get the latest (un-pinned) version instead.

If the CDN URL is expected to include the version, `create_remote_translation_loader` needs to accept and embed `version_id`:

```python
elif project_id:
    from gt_i18n.i18n_manager._remote_loader import (
        create_remote_translation_loader,
    )
    loader = create_remote_translation_loader(project_id, cache_url or "", version_id)
```

And `_remote_loader.py` would need to build the URL as:
```python
url = f"{cache_url}/{project_id}/{version_id}/{locale}" if version_id else f"{cache_url}/{project_id}/{locale}"
```

If passing `version_id` to the CDN URL is intentionally deferred, the docstring should be updated to clarify that the parameter is currently only accessible via `get_version_id()` for use in custom `load_translations` callbacks.

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/gt-i18n/src/gt_i18n/internal/_load_gt_config.py
Line: 19

Comment:
**Unconventional leading-underscore config key**

The JSON config key is mapped as `"_versionId"` (with a leading underscore). All other keys in `_KEY_MAP` follow standard camelCase without a leading underscore (`"projectId"`, `"cacheUrl"`, `"customMapping"`, etc.).

If this leading underscore is intentional (e.g. to signal that it is an internal/private field in `gt.config.json`), a brief comment would help future readers understand the convention. If it is unintentional, it should be renamed to `"versionId"` for consistency.

```suggestion
    "_versionId": "version_id",  # leading underscore intentional — marks as internal-use field
```

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

Last reviewed commit: "chore: bump version_..."

Greptile also left 1 inline comment on this PR.

"cacheUrl": "cache_url",
"runtimeUrl": "runtime_url",
"customMapping": "custom_mapping",
"_versionId": "version_id",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Unconventional leading-underscore config key

The JSON config key is mapped as "_versionId" (with a leading underscore). All other keys in _KEY_MAP follow standard camelCase without a leading underscore ("projectId", "cacheUrl", "customMapping", etc.).

If this leading underscore is intentional (e.g. to signal that it is an internal/private field in gt.config.json), a brief comment would help future readers understand the convention. If it is unintentional, it should be renamed to "versionId" for consistency.

Suggested change
"_versionId": "version_id",
"_versionId": "version_id", # leading underscore intentional — marks as internal-use field
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/gt-i18n/src/gt_i18n/internal/_load_gt_config.py
Line: 19

Comment:
**Unconventional leading-underscore config key**

The JSON config key is mapped as `"_versionId"` (with a leading underscore). All other keys in `_KEY_MAP` follow standard camelCase without a leading underscore (`"projectId"`, `"cacheUrl"`, `"customMapping"`, etc.).

If this leading underscore is intentional (e.g. to signal that it is an internal/private field in `gt.config.json`), a brief comment would help future readers understand the convention. If it is unintentional, it should be renamed to `"versionId"` for consistency.

```suggestion
    "_versionId": "version_id",  # leading underscore intentional — marks as internal-use field
```

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!

@moss-bryophyta moss-bryophyta force-pushed the fix/minor-version-bump-version-id branch from abac13e to 3532138 Compare March 19, 2026 18:55
@ErnestM1234 ErnestM1234 merged commit dc2d696 into generaltranslation:main Mar 19, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants