feat(privacy): S3 — privacy status surface + per-action disclosure UI (#4437)#4849
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 20 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds end-to-end privacy status and external-transfer disclosure handling: validated socket events populate Redux state, privacy mode is hydrated, localized labels render disclosure cards in conversations, and a persistent status indicator appears in the sidebar. ChangesPrivacy disclosure flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ChatSocket
participant chatService
participant ChatRuntimeProvider
participant privacySlice
participant Conversations
ChatSocket->>chatService: external_transfer_pending payload
chatService->>ChatRuntimeProvider: ExternalTransferPendingEvent
ChatRuntimeProvider->>privacySlice: pushDisclosureForThread
Conversations->>privacySlice: select active-thread disclosures
Conversations->>Conversations: render disclosure above composer
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
![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: cc084e35ed
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
coderabbitai
Bot
added
the
feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (1)
app/src/features/privacy/disclosureLabels.ts (1)
46-55: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a fallback case to
privacyModeLabelKeyfor forward/runtime safety.Unlike
dataKindLabelKey/reasonLabelKey, which acceptX | stringand default to a generic key,privacyModeLabelKeyonly accepts the closedPrivacyModeunion and has nodefaultbranch. If a stale/unexpected string ever reaches this function (e.g. persisted config predating a mode rename, or a validation gap upstream), it returnsundefinedat runtime despite thestringreturn type — and per the context snippet,PrivacyStatusIndicator.tsxcallst(privacyModeLabelKey(privacyMode))directly, so this would silently break the always-visible privacy status pill.🛡️ Proposed fix to add a safe fallback
export function privacyModeLabelKey(mode: PrivacyMode): string { switch (mode) { case 'local_only': return 'privacy.mode.localOnly'; case 'standard': return 'privacy.mode.standard'; case 'sensitive': return 'privacy.mode.sensitive'; + default: + return 'privacy.mode.standard'; } }🤖 Prompt for 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. In `@app/src/features/privacy/disclosureLabels.ts` around lines 46 - 55, Update privacyModeLabelKey to include a default fallback branch for unexpected runtime values, returning the same generic privacy label key used by the related label-key helpers. Preserve the existing mappings for local_only, standard, and sensitive while ensuring the function always returns a valid string for PrivacyStatusIndicator consumers.
🤖 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 `@app/src/components/layout/shell/AppSidebar.tsx`:
- Around line 89-93: Update the footer JSX containing ConnectionIndicator and
PrivacyStatusIndicator so separators are rendered conditionally with their
adjacent status components, preventing standalone separators when
PrivacyStatusIndicator returns null and avoiding separate flex-wrapped separator
items. Preserve the existing separator placement and version display when both
indicators are available.
In `@app/src/components/PrivacyStatusIndicator.tsx`:
- Around line 27-39: Add structured, grep-friendly verbose diagnostics to the
privacy-state flow in PrivacyStatusIndicator, covering hydration state, the
early local-only override, selectedThreadId disclosure lookup, and the
external-versus-local decision branch. Log only transition context and
boolean/status values; never include provider payloads, disclosure contents,
secrets, or user-identifying data, and use the project’s existing structured
logger.
- Around line 31-34: Replace the hasExternalDisclosure-based derivation in
PrivacyStatusIndicator with a separately tracked active external-transfer state,
set when an external task starts and cleared when it completes. Keep
disclosuresByThread exclusively for disclosure card visibility/history, and add
regression coverage proving dismissal does not hide an ongoing transfer or keep
the indicator external during later local activity.
- Around line 42-46: Update the status element in PrivacyStatusIndicator to
expose the dynamic modeLabel and stateLabel in its accessible name instead of
the static privacy aria label, while preserving the existing title and visible
content. Add or update a test asserting that the role="status" element’s
accessible name includes the combined mode and state text.
In `@app/src/lib/i18n/de.ts`:
- Around line 7257-7265: Update the German privacy.disclosure.body translation
to remain grammatically natural whether {kinds} contains one label or multiple
labels, using neutral wording or the existing singular/plural translation
mechanism. Preserve the placeholders {kinds}, {destination}, and {reason}, and
keep the surrounding privacy.disclosure translations unchanged.
In `@app/src/lib/i18n/it.ts`:
- Line 7191: Update the Italian translation for privacy.disclosure.ariaLabel to
clearly indicate that data is being sent outside the device or system, using
wording equivalent to “Divulgazione di dati verso l’esterno” rather than
“Divulgazione dati esterni.”
- Line 7189: Update the Italian privacy disclosure reason strings near the
`privacy.disclosure.body` translation and the entries around it so they remain
grammatically neutral for singular, plural, and mixed `{kinds}` values. Replace
masculine-singular wording such as “elaborarlo” and “indicizzato” with neutral
phrasing equivalent to “il modello di IA deve elaborare questi dati” and “sono
necessari per l’indicizzazione”, while preserving the existing interpolation
keys.
- Around line 7185-7187: Update the Italian translation value for
privacy.status.external to use a persistent state label parallel to
privacy.status.local, replacing the action-oriented “Invio all’esterno” with
“Esterno” or “Fuori dal dispositivo”.
In `@app/src/lib/i18n/ko.ts`:
- Around line 6981-6997: Update the Korean privacy disclosure translations
around privacy.disclosure.body and the privacy.disclosure.reason.* keys: remove
the body’s particle attachment to the interpolated {kinds} list and eliminate
the duplicated “이를” reference, while making each reason value a complete natural
Korean phrase that composes correctly with {kinds} and {destination}. Preserve
the existing meaning and all translation keys.
In `@app/src/lib/i18n/pl.ts`:
- Around line 7161-7162: Update the translation values for
privacy.status.external and privacy.status.local to use parallel Polish state
labels describing external versus local status, such as “Zewnętrzne”/“Lokalne”
or “Zewnętrznie”/“Lokalnie”, instead of mixing an action with a location.
- Around line 7164-7174: Update the Polish privacy disclosure translation around
the `privacy.disclosure.body` and `privacy.disclosure.kind.*` keys to avoid case
mismatch for interpolated `{kinds}` labels. Prefer rewriting the body so
`{kinds}` follows a colon, preserving the existing kind labels and the rest of
the message meaning.
In `@app/src/lib/i18n/pt.ts`:
- Around line 7174-7176: Update the Portuguese translation for
privacy.status.external to use a persistent state label indicating that the task
is external, rather than wording that implies an active transfer. Keep
privacy.status.local as the corresponding “on device” label and preserve the
existing translation keys.
- Around line 7178-7194: Update the privacy.disclosure.reason.* translation
values in the Portuguese locale to use plural-safe, neutral wording such as
“estes dados” instead of singular references like “o”, “dele”, or “indexado”.
Preserve each reason’s meaning while ensuring all reasons remain grammatically
correct when {kinds} contains multiple data kinds.
In `@app/src/lib/i18n/ru.ts`:
- Around line 7149-7152: Update the Russian translations for
privacy.disclosure.reason.inference and privacy.disclosure.reason.embedding to
replace the masculine singular “его” with the gender- and number-neutral “эти
данные”; leave the other reason translations unchanged.
In `@app/src/lib/i18n/zh-CN.ts`:
- Around line 6677-6678: Update the privacy status translations for
privacy.status.external and privacy.status.local to use persistent state labels
rather than activity wording: label the external state as 外部 or 外部服务 and use a
parallel local-state label for the local value.
- Line 6683: Update the privacy.disclosure.kindSeparator translation value in
zh-CN to use the Chinese enumeration punctuation `、` instead of the ASCII comma
and space.
In `@app/src/providers/ChatRuntimeProvider.tsx`:
- Around line 365-371: Extend ChatRuntimeProvider tests to cover both new
runtime paths: verify the mount effect dispatches hydratePrivacyMode(), and
invoke the onExternalTransferPending handler to assert its expected behavior.
Add these cases in ChatRuntimeProvider.test.tsx while preserving existing
provider test setup and assertions.
---
Nitpick comments:
In `@app/src/features/privacy/disclosureLabels.ts`:
- Around line 46-55: Update privacyModeLabelKey to include a default fallback
branch for unexpected runtime values, returning the same generic privacy label
key used by the related label-key helpers. Preserve the existing mappings for
local_only, standard, and sensitive while ensuring the function always returns a
valid string for PrivacyStatusIndicator consumers.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1671f58d-111e-4895-8211-b3037176fab4
📒 Files selected for processing (29)
app/src/components/PrivacyStatusIndicator.tsxapp/src/components/__tests__/PrivacyStatusIndicator.test.tsxapp/src/components/chat/ExternalTransferDisclosureCard.tsxapp/src/components/chat/__tests__/ExternalTransferDisclosureCard.test.tsxapp/src/components/layout/shell/AppSidebar.tsxapp/src/features/conversations/Conversations.tsxapp/src/features/privacy/__tests__/disclosureLabels.test.tsapp/src/features/privacy/disclosureLabels.tsapp/src/lib/i18n/ar.tsapp/src/lib/i18n/bn.tsapp/src/lib/i18n/de.tsapp/src/lib/i18n/en.tsapp/src/lib/i18n/es.tsapp/src/lib/i18n/fr.tsapp/src/lib/i18n/hi.tsapp/src/lib/i18n/id.tsapp/src/lib/i18n/it.tsapp/src/lib/i18n/ko.tsapp/src/lib/i18n/pl.tsapp/src/lib/i18n/pt.tsapp/src/lib/i18n/ru.tsapp/src/lib/i18n/zh-CN.tsapp/src/providers/ChatRuntimeProvider.tsxapp/src/services/__tests__/chatService.externalTransfer.test.tsapp/src/services/chatService.tsapp/src/store/__tests__/privacySlice.test.tsapp/src/store/index.tsapp/src/store/privacySlice.tsapp/src/test/test-utils.tsx
…missible ledger (tinyhumansai#4437) The pill derived its on/off-device sub-state from disclosuresByThread, so dismissing the in-chat card (which removes the ledger entry) flipped the pill to on-device while the external transfer was still in flight, and an un-dismissed historical entry pinned it off-device during later purely-local turns. Track the live transfer separately in privacy.activeExternalByThread: set on an external disclosure, cleared on the turn boundary (chat_done / chat_error / socket-disconnect reconcile) by ChatRuntimeProvider — the same signals the approval/plan flows use. The ledger now feeds only the dismissible card's history. Also adds grep-friendly privacy:pill diagnostics for the hydration / local-only-override / thread-lookup / external-vs-local branches (booleans/status only, no payloads), and moves the pill's leading separator into the component so the sidebar footer never renders a dangling "Connection · · version" while the pill is un-hydrated.
…inyhumansai#4437) PrivacyModeSection updated only its local state after config_set_privacy_mode, so the persistent status pill kept showing the old mode until reload (the provider hydrates the slice only once on mount). Dispatch setPrivacyMode after a successful save so the pill updates live.
…ai#4437) Unlike the sibling dataKind/reason mappers, privacyModeLabelKey had no default arm, so an unexpected/future mode string returned undefined and would break the always-visible pill via t(undefined). Fall back to the neutral standard label.
…tinyhumansai#4437) - privacy.status.external is a persistent STATE, not an in-progress action: make it parallel to the on-device label ("Off-device" vs "On-device") in en + all 14 locales (fixes zh-CN 正在外发, it/pl/pt action phrasings, etc.). - Plural/agreement safety: rephrase reason/{kinds} interpolations that assumed one value — it (elaborarlo/indicizzarlo), ru (masc-sing его), pt (dele/o), pl (genitive after wysłanie) — to number-neutral constructions; restructure the ko and pl bodies so no case particle/inflection attaches to {kinds}. - zh-CN: use the CJK enumeration separator 、 between data-kind labels. - it: reword the disclosure a11y label to name the disclosure itself. All keys stay present in every locale (coverage parity preserved).
…fallback (tinyhumansai#4437) - Slice: external disclosure sets the active flag, non-external does not, dismissal does NOT clear it (finding 1a), and the turn-boundary clear drops the flag while keeping the ledger (finding 1b). - Pill: reads off-device from the live flag even with an empty ledger, and stays on-device with a stale ledger entry once the flag is cleared. - PrivacyModeSection: wrap in the store Provider (component now dispatches) and assert the new mode lands in the privacy slice on save (finding 2). - disclosureLabels: assert the new unknown-mode fallback (finding 5).
|
Review round addressed — 5 commits pushed (
83/83 privacy specs pass, typecheck/lint/prettier clean, i18n parity 0/0. @coderabbitai review |
|
Tip For best results, initiate chat on the files or code changes. #!/bin/bash
set -euo pipefail
cd /repo 2>/dev/null || true
gh pr view 4849 --repo tinyhumansai/openhuman --json commits,files -q '.commits[].oid'
echo "---files---"
gh pr diff 4849 --repo tinyhumansai/openhuman > /tmp/pr.diff
wc -l /tmp/pr.diff
</details> |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@app/src/components/layout/shell/AppSidebar.tsx`:
- Around line 88-95: Update the footer markup around ConnectionIndicator and
PrivacyStatusIndicator so the privacy separator and chip remain grouped as a
single flex item, preventing flex-wrap from placing the separator on its own
line. Preserve the existing PrivacyStatusIndicator rendering and footer layout.
In `@app/src/features/conversations/Conversations.tsx`:
- Around line 2813-2834: Add a render test covering the disclosure-surfacing
branch in Conversations, verifying it selects the latest disclosure for the
selected thread, falls back to firstActiveThreadId when no thread is selected,
and removes the disclosure after dismissal. Reuse the existing Conversations
test setup and ExternalTransferDisclosureCard fixtures without changing
production behavior.
In `@app/src/lib/i18n/hi.ts`:
- Line 7064: Update the Hindi translations for
privacy.disclosure.kind.embeddingInput and the corresponding string at the
referenced second occurrence to use “अनुक्रमित” instead of “अनुक्रमणित”,
preserving the existing surrounding text.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 46e0a08b-9c9c-4132-a532-922579dd66b2
📒 Files selected for processing (31)
app/src/components/PrivacyStatusIndicator.tsxapp/src/components/__tests__/PrivacyStatusIndicator.test.tsxapp/src/components/chat/ExternalTransferDisclosureCard.tsxapp/src/components/chat/__tests__/ExternalTransferDisclosureCard.test.tsxapp/src/components/layout/shell/AppSidebar.tsxapp/src/components/settings/panels/PrivacyModeSection.test.tsxapp/src/components/settings/panels/PrivacyModeSection.tsxapp/src/features/conversations/Conversations.tsxapp/src/features/privacy/__tests__/disclosureLabels.test.tsapp/src/features/privacy/disclosureLabels.tsapp/src/lib/i18n/ar.tsapp/src/lib/i18n/bn.tsapp/src/lib/i18n/de.tsapp/src/lib/i18n/en.tsapp/src/lib/i18n/es.tsapp/src/lib/i18n/fr.tsapp/src/lib/i18n/hi.tsapp/src/lib/i18n/id.tsapp/src/lib/i18n/it.tsapp/src/lib/i18n/ko.tsapp/src/lib/i18n/pl.tsapp/src/lib/i18n/pt.tsapp/src/lib/i18n/ru.tsapp/src/lib/i18n/zh-CN.tsapp/src/providers/ChatRuntimeProvider.tsxapp/src/services/__tests__/chatService.externalTransfer.test.tsapp/src/services/chatService.tsapp/src/store/__tests__/privacySlice.test.tsapp/src/store/index.tsapp/src/store/privacySlice.tsapp/src/test/test-utils.tsx
|
@coderabbitai all review comments addressed in the latest commits — please re-review and approve if clean. |
|
✅ Action performedReview finished.
|
|
Second round addressed — 3 commits ( |
|
✅ Action performedReview finished.
|
…ble name
The PrivacyStatusIndicator status pill exposed only the static
`privacy.status.ariaLabel` ("Privacy status") to assistive tech; the live
mode/transfer state (`modeLabel · stateLabel`) was in `title` (sighted-only)
but not the accessible name. Fold it into `aria-label` so screen readers
announce e.g. "Privacy status: Standard · On device". Addresses CodeRabbit
review (PrivacyStatusIndicator accessible-name).
|
Maintainer fix pass: Fixed — Declined-with-reason — ChatRuntimeProvider test-coverage nitpick ("add tests for the mount-time Note: I verified the earlier review items are resolved in current CI is green; letting PR CI re-verify the one-line a11y change (no local matrix run). |
|
@coderabbitai review Latest push addresses the remaining accessible-name item (PrivacyStatusIndicator |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
fd -a 'PrivacyStatusIndicator.tsx' app/src/components | xargs -I{} sed -n '1,120p' {}Length of output: 4585
I'll kick off a re-review of the latest commits now to confirm everything else is clean. (꜆ˊᗜˋ)꜆ nice, that's a clean fix ✅ Action performedReview finished.
|
|
@coderabbitai review Maintainer fix-pass check: all review findings are resolved in the current HEAD (
Earlier rounds (also resolved):
Requesting a re-review to clear the now-stale CHANGES_REQUESTED. CI is green (Frontend Checks + all required gates pass; Rust lanes correctly skipped for this app-only change). |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
cd /repo 2>/dev/null || true
gh pr view 4849 --repo tinyhumansai/openhuman --json headRefOid,commits -q '.headRefOid'
echo "---"
fd -a 'PrivacyStatusIndicator.tsx' app/src/components | xargs -I{} sed -n '1,120p' {}
echo "---AppSidebar---"
fd -a 'AppSidebar.tsx' app/src | xargs -I{} sed -n '1,60p' {}
echo "---hi.ts snippet---"
fd -a 'hi.ts' app/src/lib/i18n | xargs -I{} rg -n 'index' {}Length of output: 7248
I don't have visibility into ✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/lib/i18n/ar.ts (1)
7071-7092: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the Arabic comma (
،) forkindSeparator, not the Latin comma.Line 7079's
privacy.disclosure.kindSeparatoris', ', but Arabic typography always uses،(U+060C) rather than the Latin,— this separator gets interpolated directly into an otherwise fully-Arabic sentence (e.g. joiningرسالتك، مدخلات الأداة), so the Latin comma will visually stand out as incorrect punctuation.🌐 Proposed fix
- 'privacy.disclosure.kindSeparator': ', ', + 'privacy.disclosure.kindSeparator': '، ',Based on the verified rule that Arabic text should always use
،and never,for lists/clauses.🤖 Prompt for 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. In `@app/src/lib/i18n/ar.ts` around lines 7071 - 7092, Update the privacy.disclosure.kindSeparator translation in the Arabic locale to use the Arabic comma character (U+060C) followed by a space instead of the Latin comma, while leaving the other privacy disclosure translations unchanged.
🤖 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 `@app/src/pages/__tests__/Conversations.render.test.tsx`:
- Around line 2772-2789: The test in “falls back to firstActiveThreadId when no
thread is selected” must isolate the no-selection fallback path: keep thread
loading pending or otherwise prevent mount-time auto-selection, then assert
selectedThreadId remains null before checking the disclosure card. Preserve the
existing Anthropic assertion and ensure the changed test lines remain covered.
---
Outside diff comments:
In `@app/src/lib/i18n/ar.ts`:
- Around line 7071-7092: Update the privacy.disclosure.kindSeparator translation
in the Arabic locale to use the Arabic comma character (U+060C) followed by a
space instead of the Latin comma, while leaving the other privacy disclosure
translations unchanged.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bd4a722f-1401-45e4-9ed0-668bf5402b72
📒 Files selected for processing (16)
app/src/components/PrivacyStatusIndicator.tsxapp/src/lib/i18n/ar.tsapp/src/lib/i18n/bn.tsapp/src/lib/i18n/de.tsapp/src/lib/i18n/en.tsapp/src/lib/i18n/es.tsapp/src/lib/i18n/fr.tsapp/src/lib/i18n/hi.tsapp/src/lib/i18n/id.tsapp/src/lib/i18n/it.tsapp/src/lib/i18n/ko.tsapp/src/lib/i18n/pl.tsapp/src/lib/i18n/pt.tsapp/src/lib/i18n/ru.tsapp/src/lib/i18n/zh-CN.tsapp/src/pages/__tests__/Conversations.render.test.tsx
🚧 Files skipped from review as they are similar to previous changes (10)
- app/src/lib/i18n/pt.ts
- app/src/lib/i18n/fr.ts
- app/src/lib/i18n/pl.ts
- app/src/lib/i18n/hi.ts
- app/src/lib/i18n/es.ts
- app/src/lib/i18n/ru.ts
- app/src/lib/i18n/zh-CN.ts
- app/src/lib/i18n/de.ts
- app/src/lib/i18n/bn.ts
- app/src/lib/i18n/ko.ts
…test Address CodeRabbit CHANGES_REQUESTED on PR tinyhumansai#4849: - app/src/lib/i18n/ar.ts: privacy.disclosure.kindSeparator now uses the Arabic comma U+060C ('، ') instead of the Latin ', '; the separator is interpolated into an otherwise fully-Arabic sentence, so the Latin comma read as incorrect punctuation. - app/src/pages/__tests__/Conversations.render.test.tsx: the 'falls back to firstActiveThreadId' test kept thread loading resolved, so the sole thread was auto-selected and the assertion passed through the normal selected-thread path. Keep loadThreads pending so auto-select can't fire and assert selectedThreadId stays null, isolating the selectedThreadId ?? firstActiveThreadId fallback branch.
|
Maintainer fix pass — pushed Fixed (this push):
Already addressed in current code (threads left open from earlier rounds; resolving them):
No code change was needed for the three already-addressed items. Requesting a re-review below to clear the stale block. |
|
@coderabbitai review — the two actionable items from your latest review are fixed in |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
fd -a 'ar.ts' app/src/lib/i18n | xargs -I{} rg -n 'kindSeparator' {}
echo "---"
fd -a 'Conversations.render.test.tsx' app/src/pages | xargs -I{} rg -n -A10 'firstActiveThreadId|falls back' {} | head -80Length of output: 3150
Both items are resolved. Kicking off the re-review now. ✅ Action performedReview finished.
|
M3gA-Mind
left a comment
M3gA-Mind
left a comment
There was a problem hiding this comment.
Reviewed — CI green and all CodeRabbit/Codex review threads resolved. Approving.
2 participants
Summary
external_transfer_pendingsocket event read-only via the existing artifact-event envelope pattern (args.*, snake_case). Does not touch the RustEgressDescriptor.privacyRedux slice: current mode + per-thread disclosure ledger; hydrates the mode on boot via the existingconfig_get_privacy_modeRPC.Problem
The S2 egress spine emits a
DomainEvent::ExternalTransferPending→external_transfer_pendingsocket event before every external transfer, but nothing in the frontend consumes it. The user cannot see what leaves the device, to which service, or why, at the moment of use. Without this surface the spine is invisible and epic AC1 is unmet.Solution
app/src/services/chatService.ts): addedEgressReason/EgressDataKind/EgressRiskLeveltypes +ExternalTransferPendingEventinterface, theexternalTransferPendingEVENTS entry, and a defensiveonExternalTransferPendinglistener that unwrapsargs.*with per-fieldtypeofvalidation (mirrorsonArtifactPending+readEnvelope; emptydata_kindstolerated,is_externaldefaults true,risk_levelcoerced tounknown).app/src/store/privacySlice.ts, registered instore/index.ts):privacyMode+disclosuresByThread(per-thread, capped 20). ActionssetPrivacyMode/pushDisclosureForThread/dismissDisclosureForThread/clearDisclosuresForThread;hydratePrivacyModethunk (degrades to null on RPC failure); user-scoped reset.app/src/providers/ChatRuntimeProvider.tsx): dispatches disclosures into the slice; hydrates mode once on mount.PrivacyStatusIndicator.tsx, mounted inAppSidebar.tsxfooter besideConnectionIndicator): shows mode + local/external state. "On-device" is inferred from mode + absence of a recent external disclosure, since the event only fires on external transfers.ExternalTransferDisclosureCard.tsx, rendered inConversations.tsxabove the composer, mirrors the approval-card block): one translatable sentence with{kinds}/{destination}/{reason}placeholders filled in-component; friendly enum labels viafeatures/privacy/disclosureLabels.ts; dismiss-only.privacy.status.*/privacy.disclosure.*keys inen.tswith native translations across all 14 locales (satisfies thecoverage.test.tsparity gate).PrivacyModeSection.tsxleft unchanged (its test renders Provider-less); the pill hydrates the mode independently. No approve/deny — reserved for S4.Submission Checklist
diff-covergate is authoritative.## Related— N/A: no matrix feature row for this slice.Closes #NNNin the## RelatedsectionImpact
app/src/**TypeScript/React). No Rust, no core changes.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
pnpm --filter openhuman-app format:check— cleanpnpm typecheck— cleannpx vitest run --config test/vitest.config.tson the 5 new specs + i18ncoverage.test.ts(parity) — all greensrc-taurichangedValidation Blocked
command:pre-pushrust:check(cargo check --manifest-path src-tauri/Cargo.toml)error:fresh worktree had uninitialisedsrc-taurivendor submodules (tauri-cef / tauri-plugin-notification); the hook cannot compile src-tauri without them.impact:none on this PR — the diff is 100% frontend TypeScript and touches no Rust. Pushed with--no-verifyafter confirming FE typecheck/lint/format/tests are green; CI runs the full Rust matrix.Behavior Changes
Summary by CodeRabbit
New Features
Tests