UI: localize instances hardcoded labels#61073
Conversation
![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: d4148133e5
ℹ️ 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".
Greptile SummaryThis PR localizes the previously hardcoded English labels on the Instances screen by extracting them into
Confidence Score: 3/5Mostly safe but has one visible UI regression where a null/undefined reason renders the raw placeholder string 'Reason {value}'. The change is well-scoped and the locale keys are correctly structured. One logic bug is introduced: passing an empty string param through the t() helper exposes a pre-existing falsy-check flaw in translate.ts, causing 'Reason {value}' to appear in the UI when no reason is set. This is user-visible and worth fixing before merge. ui/src/ui/views/instances.ts line 99 — the Prompt To Fix All With AIThis is a comment left during a code review.
Path: ui/src/ui/views/instances.ts
Line: 99
Comment:
**Empty reason leaks `{value}` placeholder into UI**
When `entry.reason` is `null` or `undefined`, the expression `entry.reason ?? ""` produces `""`. The `t()` interpolation in `translate.ts` uses `params[k] || \`{${k}}\`` (line 142 of `ui/src/i18n/lib/translate.ts`), which treats an empty string as falsy — so instead of substituting `""`, it leaves the literal `{value}` in the output. Users will see **"Reason {value}"** rendered in the UI instead of the previous "Reason " or nothing.
A safe fix is to guard with a conditional so an empty param is never passed:
```suggestion
<div class="muted">${entry.reason ? t("instances.reason", { value: entry.reason }) : ""}</div>
```
Alternatively, the `t()` helper itself could be hardened to use `params[k] !== undefined ? params[k] : \`{${k}}\`` — but that is a separate change with wider blast radius.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "UI: localize instances hardcoded labels" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Good catch — I updated the Instances screen to skip the localized reason line when |
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Keep this PR open: current main and v2026.6.6 already cover much of the static Instances localization, but the branch still contains useful unique cleanup for fallback/scope strings and the empty reason placeholder. It needs a rebase onto the current generated i18n workflow, regenerated locale artifacts, and real behavior proof before it is mergeable. Canonical path: Close this stale PR. The latest review rated it F, the branch still lacks merge-ready proof, and there has been no human follow-up after the durable review. So I’m closing this here because the remaining work is already tracked in the canonical issue. Review detailsBest possible solution: Close this stale PR. The latest review rated it F, the branch still lacks merge-ready proof, and there has been no human follow-up after the durable review. Do we have a high-confidence way to reproduce the issue? Yes from source inspection, but not from a live browser run: current main still hits hardcoded Instances fallback/scope strings and an empty-reason translation path when the relevant entry fields are missing or scoped. The PR body provides steps, but no screenshot, recording, or runtime output for the rendered UI. Is this the best way to solve the issue? No, not as submitted. The idea is sound and narrow, but the best current solution is to rebase onto main's generated i18n pipeline and regenerate all locale artifacts instead of hand-maintaining pt-BR only. Security review: Security review cleared: The diff is limited to Control UI rendering text and locale strings; I found no concrete security or supply-chain concern. AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against 6c88811b4bba. |
clawsweeper
Bot
added
rating: 🧂 unranked krab
openclaw-barnacle
Bot
added
the
triage: needs-real-behavior-proof
clawsweeper
Bot
added
the
merge-risk: 🚨 compatibility
|
ClawSweeper PR egg 🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat. Where did the egg go?
|
|
This pull request has been automatically marked as stale due to inactivity. |
clawsweeper
Bot
added
rating: 🌊 off-meta tidepool
clawsweeper
Bot
removed
the
rating: 🌊 off-meta tidepool
clawsweeper
Bot
added
rating: 🧂 unranked krab
|
ClawSweeper applied the proposed close for this PR.
|
1 participant
Summary
instances.tsthrought(...).Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
ui/src/ui/views/instances.tsinstead of using locale files.Regression Test Plan (if applicable)
ui/src/i18n/test/translate.test.tsui/src/i18n/test/translate.test.tsUser-visible / Behavior Changes
Diagram (if applicable)
N/A
Security Impact (required)
Yes/No): NoYes/No): NoYes/No): NoYes/No): NoYes/No): NoYes, explain risk + mitigation:Repro + Verification
Environment
Steps
Expected
Actual
Evidence
Validation:
Human Verification (required)
Review Conversations
Compatibility / Migration
Yes/No): YesYes/No): NoYes/No): NoRisks and Mitigations