Complete Translation correctness and worker lifecycle handling - #483
Merged
Conversation
Use one numeric grammar for extracting and stripping plural conditions so paired delimiters and supported range forms cannot drift apart. Preserve malformed conditions as literal text instead of selecting them through PHP coercion.\n\nKeep floating-point values for exact and range comparisons while casting only modulo operands required by PHP 8.4. Expand the selector matrix to cover decimals, signed ranges, wildcard endpoints, mismatched delimiters, and the real modulo path.
Reject path separators and exact dot segments before any translation filesystem access while retaining valid dot-bearing locale names. Validate JSON roots and top-level values with file and key context, preserving null as the untranslated sentinel and arrays as supported values.\n\nComplete loader namespace-map metadata, remove redundant promoted-property assignment, and add direct coverage for invalid locales, JSON diagnostics, numeric array keys, and null values.
Expose string, array, and fallback accessors through the public contract and generated Lang facade. Make get return its real array-or-string domain, route choice and translated strings through the typed boundary, and keep request locale validation eager.\n\nCentralize replacements for nested array translations without coercing mixed leaves. Preserve falsey values and null fallback behavior, reject unsupported stringable registrations immediately, and document worker-shared mutators at their public boundaries. Add regression coverage for typed values, mixed arrays, fallback state, callable registration, locale paths, and FileLoader-backed null sentinels.
Route framework call sites that promise text through the Translator string accessor so array-valued configuration fails at the translation boundary with a key-specific error. Keep bulk validation metadata and notification lines on the array-capable get path where arrays remain supported.\n\nCover FormRequest, validation messages, password-reset mail, and verification mail without duplicating the Translator wrong-type matrix at every consumer.
Resolve the canonical Translator and event dispatcher contracts once when reading or changing locale state. Keep current locale request-local and fallback locale on the shared Translator instead of mutating process-global configuration.\n\nGuard LocaleUpdated construction and dispatch behind targeted listener detection while preserving mutation order for active listeners. Add ordered listener and no-listener coverage together with assertions that configuration remains the worker boot seed.
Run locale and missing-key interleavings through the repository coroutine test harness with deliberate yields and propagated task results. Prove concurrent locale changes remain isolated and missing-key suppression cannot hide a sibling callback.\n\nReplace process-global integration fixture communication with local captured state, remove redundant shared cleanup, and complete current test typing and Hypervel-owned fixture prose.
Resolve configuration, filesystem, and loader dependencies through their canonical typed services during provider construction. Read configuration once, rely on the shipped defaults, and avoid duplicate promoted-property assignment.\n\nRemove Translation's false direct Container requirement while retaining the dependency through its provider parent. This keeps split-package metadata aligned with the classes the package uses directly.
Describe the conditional results of translation, redirect, session, and view helpers so static analysis narrows supported calls to their real return types. Replace the session helper's information-free Store annotation with the SessionManager, value, and null branches it actually returns.\n\nNormalize the touched redirect types and the existing CookieJar annotation to imported short names. These changes improve caller inference without changing any executable helper branch.
Document typed translation retrieval with direct string and array examples, including the wrong-type failure behavior. Explain that application locale configuration seeds each worker, current locale changes apply to the active request, and fallback changes belong at application boot.\n\nKeep the package READMEs thin while recording the public Laravel differences for coroutine-local locale state, worker-shared fallback state, and JSON value validation.
Raise the root and dogfood ParaTest floor to the first released version that supports and requires PHPUnit 13.3. This keeps unlocked package resolution on a compatible runner pair without narrowing PHPUnit or carrying a temporary vendor patch.\n\nThe change affects development and test execution only; runtime package dependencies are unchanged.
State the PHP language rule directly: constructors and destructors cannot declare return types. This prevents the repository's full-typing convention from being applied to syntax PHP rejects while leaving every other typing requirement unchanged.
Capture the final Translation architecture, verified findings, public API decisions, worker and coroutine ownership, implementation boundaries, and performance accounting. Record the exact regression strategy and checkpoint gates so future maintenance can distinguish intentional Hypervel behavior from stale parity drift.\n\nThe plan also documents the rejected machinery: no request-scoped translator, loader lock, watcher, compatibility wrapper, parser object, or duplicate replacement path.
Close the Translation package checklist and route its completed cross-package findings to Contracts, Foundation, Auth, Validation, Support, Routing, Session, and View. Record the final implementation, verification, public differences, hot-path accounting, rejected complexity, and review result in the companion ledger.\n\nUpdate the durable Pagination owner route now that the translated helper type is complete, while preserving dated detail plans as historical design records.
|
Warning Review limit reached
Next review available in: 18 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughTranslation now has typed accessors, stricter locale and JSON validation, updated pluralization, worker and coroutine lifecycle rules, contract-based framework wiring, migrated string consumers, expanded tests, and updated documentation and audit records. ChangesTranslation correctness
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Application
participant Translator
participant FileLoader
participant Dispatcher
Application->>Translator: set current or fallback locale
Translator->>FileLoader: load validated translation data
FileLoader-->>Translator: return validated values
Application->>Dispatcher: check and dispatch LocaleUpdated
Translator-->>Application: return typed translation value
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
Greptile SummaryThe PR expands Translation’s typed API and corrects translation selection, loading, coroutine-local locale ownership, framework consumers, documentation, and regression coverage.
Confidence Score: 4/5The PR is not yet safe to merge because the intended root ParaTest update is absent from the lockfile used by reproducible installs. The root manifest requests ParaTest Files Needing Attention: composer.json and composer.lock
|
| Filename | Overview |
|---|---|
| src/translation/src/Translator.php | Adds typed translation retrieval, preserves valid falsey and mixed translation values, and isolates the effective locale through coroutine context. |
| src/translation/src/FileLoader.php | Adds locale-path validation and enforces the declared structure of decoded JSON translation files. |
| src/translation/src/MessageSelector.php | Consolidates plural-condition parsing and stripping while preserving float comparisons on PHP 8.4. |
| src/foundation/src/Application.php | Resolves canonical translation and event contracts and avoids constructing locale events when no targeted listener exists. |
| src/contracts/src/Translation/Translator.php | Expands the public translation contract with typed value and fallback accessors. |
| src/foundation/src/helpers.php | Restores conditional helper return metadata for translation and other framework helper boundaries. |
| composer.json | Raises the root ParaTest development constraint to the line intended for PHPUnit 13.3 compatibility. |
Reviews (2): Last reviewed commit: "fix(translation): preserve keyed empty a..." | 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
🤖 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 `@src/foundation/src/helpers.php`:
- Around line 1021-1022: The conditional return annotation for view() does not
match its func_num_args() === 0 behavior. Update the view() annotation to
distinguish omitted arguments from an explicitly passed null, or change the
implementation’s branch to use is_null($view) if explicit null should return
ViewFactory; ensure the declared return types match the actual behavior.
In `@src/translation/src/Translator.php`:
- Around line 250-254: Update the value check in Translator::get so an empty
array is preserved as a valid translation and passed to makeReplacements, rather
than treated as missing. Keep the existing handling for strings, non-empty
arrays, and null or unsupported values unchanged.
🪄 Autofix
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: 2269f49c-f2b1-452a-84ce-3cf591a80a0e
📒 Files selected for processing (35)
AGENTS.mdcomposer.jsondocs/plans/2026-07-12-0900-framework-coroutine-state-lifecycle-audit.mddocs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.mddocs/plans/2026-08-06-0925-translation-correctness-current-parity-and-worker-lifecycles.mddogfood/testbench-package/composer.jsonsrc/auth/src/Notifications/ResetPassword.phpsrc/auth/src/Notifications/VerifyEmail.phpsrc/boost/docs/localization.mdsrc/contracts/src/Translation/Loader.phpsrc/contracts/src/Translation/Translator.phpsrc/foundation/README.mdsrc/foundation/src/Application.phpsrc/foundation/src/Http/FormRequest.phpsrc/foundation/src/helpers.phpsrc/support/src/Facades/Lang.phpsrc/translation/README.mdsrc/translation/composer.jsonsrc/translation/src/ArrayLoader.phpsrc/translation/src/FileLoader.phpsrc/translation/src/MessageSelector.phpsrc/translation/src/PotentiallyTranslatedString.phpsrc/translation/src/TranslationServiceProvider.phpsrc/translation/src/Translator.phpsrc/validation/src/Concerns/FormatsMessages.phpsrc/validation/src/ValidationException.phptests/Auth/VerifyEmailNotificationTest.phptests/Foundation/FoundationApplicationTest.phptests/Foundation/FoundationFormRequestTest.phptests/Integration/Translation/TranslatorTest.phptests/Translation/CoroutineIsolationTest.phptests/Translation/TranslationFileLoaderTest.phptests/Translation/TranslationMessageSelectorTest.phptests/Translation/TranslationTranslatorTest.phptests/Validation/ValidationValidatorTest.php
Treat an explicitly keyed empty PHP array as a real translation value instead of falling through to another locale. Keep empty loader results for whole-group lookups as missing so sentence-style keys and absent groups retain their existing behavior. Add a two-sided regression covering get, has, the typed array accessor, fallback suppression, and missing groups. Update the detailed plan and completed audit ledger so the array contract and its string-only consumer consequence are recorded accurately.
Collaborator
Author
|
@coderabbitai @greptile-apps Follow-up complete.
The focused Translation coverage is green, the changed source passes formatting and static-analysis checks, and the final diff is clean. |
|
Thanks for the thorough follow-up — all four points are well-reasoned and the explanations are sound.
Nothing outstanding from my side. The PR looks good to merge. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '=== PR diff: relevant files ==='
git diff --unified=40 HEAD^ HEAD -- \
src/translation/src/Translator.php \
tests/Translation/TranslationTranslatorTest.php \
src/foundation/src/helpers.php \
composer.json \
dogfood/testbench-package/composer.json \
dogfood/testbench-package/composer.lock || true
printf '%s\n' '=== Translator outline ==='
ast-grep outline src/translation/src/Translator.php --items all --view expanded || true
printf '%s\n' '=== Empty-array / loader-result handling ==='
rg -n -C 8 -P '\b(empty|is_array|load|fallback|getLine|parseKey)\b' \
src/translation/src/Translator.php \
tests/Translation/TranslationTranslatorTest.php || true
printf '%s\n' '=== view helper documentation and implementation ==='
rg -n -C 10 -P 'function\s+view\s*\(|`@return`.*(View|Factory|mixed)' \
src/foundation/src/helpers.php || true
printf '%s\n' '=== ParaTest constraints and available local lockfile entries ==='
rg -n -C 2 -P '"brianium/paratest"|"phpunit/phpunit"' \
composer.json \
dogfood/testbench-package/composer.json \
dogfood/testbench-package/composer.lock || trueLength of output: 91098
The new condition preserves explicitly keyed empty arrays. It still treats an empty group as missing. The regression test covers both cases and prevents fallback for the keyed empty array. The Both Composer manifests now require I also withdraw the generic docstring concern. The changed code follows the repository documentation convention. ✏️ Learnings added
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! You are interacting with an AI system. |
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
This pull request completes the Translation package's current Laravel API surface and makes its state ownership explicit for long-lived workers.
The main changes are:
Translation values
Translator::get now declares the array-or-string result it already provides. Translator::string and Translator::array offer explicit boundaries for call sites that require one shape, with key-specific errors when configuration contains the wrong type. Fallback accessors are also part of the public contract, so the helper and facade expose the same usable API as the concrete translator.
Array replacements now have one implementation. Strings are replaced recursively, while integer, float, boolean, null, and object leaves remain unchanged. Empty strings, string zero, and empty arrays remain valid translations. A JSON null value keeps its established meaning: the key is untranslated and normal group fallback is attempted.
Framework-owned consumers use the typed string accessor only where their own API requires text. Validation metadata and notification lines remain on the array-capable path.
Selectors and file loading
Plural selectors now use one grammar for extraction and stripping. Supported numeric, signed, decimal, and wildcard ranges remain available. Invalid labels and mismatched delimiters remain literal text instead of matching through PHP coercion. Modulo branches cast only their operands, preserving float exact and range comparisons under PHP 8.4.
FileLoader rejects path separators and exact dot segments before filesystem access while allowing locale names such as en.UTF-8. JSON files must decode to arrays, and each non-null top-level value must be a string or array. Errors name the file and offending key.
Worker and request ownership
Translator and FileLoader remain worker-lifetime services. Loaded definitions are shared immutable data. Current locale remains in coroutine context, so one request cannot change another request's locale. Fallback locale and registration callbacks remain shared boot-time configuration and are documented at their public mutators.
Application locale methods resolve the canonical contracts once. Locale changes no longer imply process-global configuration changes. LocaleUpdated retains its ordering for active listeners, while the no-listener path avoids event allocation and dispatch.
No request-scoped translator, loader lock, watcher, cache invalidation registry, retry path, or compatibility wrapper is introduced.
Package and documentation cleanup
The Translation provider resolves typed dependencies once and uses configuration as the construction seed. The split package no longer declares Container as a direct dependency when it is inherited through the provider parent.
The localization guide documents typed values, effective locale behavior, and the worker-shared fallback boundary in the same task-oriented style as the rest of the framework documentation. Translation and Foundation READMEs contain only the public differences developers need when moving from Laravel.
The root and dogfood test environments require the released ParaTest line compatible with PHPUnit 13.3.
Compatibility
Supported Laravel method names, signatures, named arguments, helper entry points, facade methods, and protected extension points remain compatible. The new accessors are additive.
Behavior changes are limited to invalid or previously inconsistent cases: malformed plural conditions, unsafe locale paths, non-null scalar JSON values outside the declared return contract, and callable forms that could be accepted by the old type but could not register successfully.
Performance
Cached translation reads keep the same filesystem and cache path. Locale validation runs on assignment or load. JSON validation walks a decoded file once. Array recursion runs only for array translations with replacements. Typed accessors add one result check, and the optional event guard removes work when no listener exists.
The change adds no query, network call, lock, retry, polling loop, serialization layer, or unbounded worker state.
Verification
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests