Make Iroh transport diagnostics human-readable - #9485
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR replaces compact diagnostic exports with localized, human-readable timelines. It adds semantic event decoding, versioned report formatting, localized incident titles, richer Sentry fields, and updated CLI, terminal, iOS, and macOS integrations. ChangesDiagnostic reporting
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI_or_Settings
participant DiagnosticLog
participant DiagnosticReport
participant DiagnosticEventPresentation
CLI_or_Settings->>DiagnosticLog: request diagnostic export
DiagnosticLog->>DiagnosticReport: serialize snapshot
DiagnosticReport->>DiagnosticEventPresentation: describe events
DiagnosticEventPresentation-->>DiagnosticReport: localized titles and labeled values
DiagnosticReport-->>CLI_or_Settings: human-readable timeline
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (23 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@docs/transport-sentry-diagnostics.md`:
- Around line 13-15: Update the stable-code location statement in the transport
diagnostics documentation to include breadcrumb event_code and structured-log
transport.event_code and transport.role_code, while retaining the existing
Sentry tags and fingerprints references. Ensure it no longer claims stable case
names appear only in tags and fingerprints.
- Around line 58-65: Update the timestamp description in the transport
diagnostics documentation to state that humanReadableExport() uses UTC
timestamps when a wall date is available and +<seconds> relative
timestamps as the fallback when it is absent.
In
`@Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticReport.swift`:
- Around line 192-219: Update the diagnostic report generation around the report
header, empty-state branch, timeline timestamps, and DiagnosticEventPresentation
calls to use localized resources for every user-facing string, including
headings, labels, “No events recorded.”, and the elapsed-seconds suffix. Add
matching entries for all supported locales and preserve the existing report
structure and formatting behavior.
- Around line 207-220: Update humanReadableExport() to reserve capacity for the
output and replace the per-event String(format:) and Locale allocations in the
timeline loop with a bounded integer-based elapsed-time formatter. Preserve the
existing wall-clock timestamp formatting and relative-time output, while
appending each event through the reserved buffer.
In
`@Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/TransportIncidentPolicy.swift`:
- Around line 263-269: Update TransportIncidentPolicy.swift at lines 263-269 and
326-328 to use one localized incident-title formatter instead of direct English
string construction. Define ICU .one and .other templates for failure counts and
durations, ensuring singular wording for a count of one, and route the
occurrence suffix through the same formatter at lines 326-328.
In
`@Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticEventPresentationTests.swift`:
- Around line 44-48: Add assertions in describesRetryDelayAndCloseAttribution to
cover duration values representing exactly one second and a whole number of
seconds greater than one, verifying the singular “second” and plural “seconds”
outputs respectively. Keep the existing fractional-duration assertion 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c5f8b990-ab5f-41c4-904a-06a5d2767a4e
📒 Files selected for processing (18)
CLI/cmux.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticEvent.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticEventPresentation.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticLog.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticReport.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/InputResponderIdentity.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/TransportIncidentPolicy.swiftPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticEventPresentationTests.swiftPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticLogTests.swiftPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/TransportIncidentPolicyTests.swiftPackages/Shared/CmuxSentryTelemetry/Sources/CmuxSentryReporting/TransportSentryReporter.swiftPackages/Shared/CmuxSentryTelemetry/Tests/CmuxSentryReportingTests/TransportSentryReporterTests.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsModel.swiftPackages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/MobileIrohSettingsModelTests.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Models/IrohSettingsModel.swiftPackages/macOS/CmuxSettingsUI/Tests/CmuxSettingsUITests/IrohSettingsModelTests.swiftSources/TerminalController.swiftdocs/transport-sentry-diagnostics.md
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/TransportIncidentPolicyTests.swift (1)
32-35: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPin the locale for English title assertions.
TransportIncidentPolicy()capturesLocale.current. These English assertions will fail when the test host uses a non-English locale.Pass a fixed English locale to each policy used for title assertions. Alternatively, assert locale-independent presentation data.
- var policy = TransportIncidentPolicy() + var policy = TransportIncidentPolicy(locale: Locale(identifier: "en"))Also applies to: 51-56, 153-158, 172-191
🤖 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 `@Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/TransportIncidentPolicyTests.swift` around lines 32 - 35, Update each TransportIncidentPolicy instance used by the title assertions, including the cases around the referenced assertion ranges, to receive a fixed English locale instead of implicitly capturing Locale.current. Keep the existing English title expectations unchanged and apply the same locale pinning consistently across all affected tests.
🤖 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
`@Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticIncidentTitleFormatter.swift`:
- Around line 37-73: Update failureCount, secondCount, and occurrenceCountText
to select distinct ICU-style localization keys with .one for value == 1 and
.other for all other values, while preserving each existing fallback phrase. Add
matching .one and .other catalog entries for each key in every supported locale.
In `@Resources/Localizable.xcstrings`:
- Around line 36829-36845: Add localizations for cli.help.irohDiag covering
every supported catalog locale beyond en and ja, using accurate translations of
the existing CLI help text; if a translation is unavailable, add the
locale-specific fallback values required by the catalog rather than leaving
locales missing.
---
Outside diff comments:
In
`@Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/TransportIncidentPolicyTests.swift`:
- Around line 32-35: Update each TransportIncidentPolicy instance used by the
title assertions, including the cases around the referenced assertion ranges, to
receive a fixed English locale instead of implicitly capturing Locale.current.
Keep the existing English title expectations unchanged and apply the same locale
pinning consistently across all affected tests.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 92280c61-bc73-4a49-8cb6-4051e69562dc
📒 Files selected for processing (15)
CLI/cmux.swiftPackages/Shared/CMUXMobileCore/Package.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticEventPresentation.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticIncidentTitleFormatter.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticLocalization.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticReport.swiftPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/Resources/Localizable.xcstringsPackages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/TransportIncidentPolicy.swiftPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticEventPresentationTests.swiftPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticLogTests.swiftPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/TransportIncidentPolicyTests.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileIrohSettingsModel.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Models/IrohSettingsModel.swiftResources/Localizable.xcstringsdocs/transport-sentry-diagnostics.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticLocalization.swift`:
- Around line 25-38: The bundle(for:) locale lookup must canonicalize the locale
using BCP 47 and try script/region parent locales before falling back to the
bare language. Update bundle(for:) to use Bundle localization preferences or
explicit ordered fallbacks so locales such as zh-Hant-TW select zh-Hant.lproj
before zh.lproj, while preserving the final .module fallback.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 39e2ffa2-0577-4a8d-a8c6-dd28a56d01df
📒 Files selected for processing (1)
Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticLocalization.swift
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)
Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticEventPresentationTests.swift (1)
35-84: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake English localization explicit in both test suites.
Both suites use the default locale while comparing localized output with English literals. Use one explicit
Locale(identifier: "en")for every English presentation and export assertion.
Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticEventPresentationTests.swift#L35-L84: use an English presentation for title, field, and summary assertions.Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticEventPresentationTests.swift#L86-L106: use an English presentation for lifecycle and fallback assertions.Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticEventPresentationTests.swift#L108-L168: pass the English presentation to every title description.Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticEventPresentationTests.swift#L170-L236: use the English presentation for structured payload descriptions.Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticLogTests.swift#L52-L94: pass the English locale to export calls and export-equivalence checks.Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticLogTests.swift#L197-L216: pass the English locale to the empty-report export.Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticLogTests.swift#L218-L231: pass the English locale to relative-time export assertions.🤖 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 `@Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticEventPresentationTests.swift` around lines 35 - 84, Make English localization explicit across all affected tests by creating or reusing one Locale(identifier: "en") value. In Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticEventPresentationTests.swift ranges 35-84, 86-106, 108-168, and 170-236, pass the English presentation to title, field, summary, lifecycle, fallback, and structured-payload assertions. In Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticLogTests.swift ranges 52-94, 197-216, and 218-231, pass the same English locale to export, equivalence, empty-report, and relative-time assertions.
♻️ Duplicate comments (1)
Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticLocalization.swift (1)
38-50:⚠️ Potential issue | 🟠 MajorMatch script and region parent locales before falling back.
languageBundle(for:)checks only the raw locale identifier and the bare language code. Forzh-Hant-TW, it skipszh-Hant.lproj. Ifzh.lprojexists, the code selects the wrong script. Otherwise,bundle(for:)can fall back to.module. Build an ordered BCP 47 parent chain before the bare language. Apply the same lookup tohasCompiledLocalization(for:).This repeats the unresolved previous review finding.
Verification
#!/bin/bash set -euo pipefail root="Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/Resources" echo "Available localization bundles:" fd -a -t d '\.lproj$' "$root" | sort echo echo "Current locale lookup:" sed -n '38,50p' \ Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticLocalization.swift🤖 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 `@Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticLocalization.swift` around lines 38 - 50, Update languageBundle(for:) and hasCompiledLocalization(for:) to use the same ordered BCP 47 parent-locale chain: try the full locale, then progressively remove region while preserving script (for example, zh-Hant-TW → zh-Hant → zh), before any bare-language fallback. Reuse a shared helper if appropriate, and ensure lookup never selects zh before zh-Hant or falls through to Bundle.module when a more specific parent bundle exists.
🤖 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
`@Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticLocalization.swift`:
- Around line 13-20: Move hasCompiledLocalization(for:) out of the production
DiagnosticLocalization implementation and into shared test support used by
DiagnosticLogTests and DiagnosticEventPresentationTests. Update those tests to
call the test-support probe, preserving the existing locale-checking behavior
while removing the test-only API from the production target.
---
Outside diff comments:
In
`@Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticEventPresentationTests.swift`:
- Around line 35-84: Make English localization explicit across all affected
tests by creating or reusing one Locale(identifier: "en") value. In
Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticEventPresentationTests.swift
ranges 35-84, 86-106, 108-168, and 170-236, pass the English presentation to
title, field, summary, lifecycle, fallback, and structured-payload assertions.
In
Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticLogTests.swift
ranges 52-94, 197-216, and 218-231, pass the same English locale to export,
equivalence, empty-report, and relative-time assertions.
---
Duplicate comments:
In
`@Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticLocalization.swift`:
- Around line 38-50: Update languageBundle(for:) and
hasCompiledLocalization(for:) to use the same ordered BCP 47 parent-locale
chain: try the full locale, then progressively remove region while preserving
script (for example, zh-Hant-TW → zh-Hant → zh), before any bare-language
fallback. Reuse a shared helper if appropriate, and ensure lookup never selects
zh before zh-Hant or falls through to Bundle.module when a more specific parent
bundle exists.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0cd2a668-301e-454c-a6f4-d3db105d940a
📒 Files selected for processing (3)
Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticLocalization.swiftPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticEventPresentationTests.swiftPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticLogTests.swift
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticLogTests.swift`:
- Around line 72-73: Replace the lossy String(decoding:as:) conversions in
DiagnosticLogTests.swift at lines 72-73, 207-209, and 236-239 with failable
strict UTF-8 decoding, and require each decode to succeed before performing the
existing assertions.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8ae7db5c-c0d1-464e-b9c7-f126db1c91c1
📒 Files selected for processing (6)
Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticLocalization.swiftPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticEventPresentationTests.swiftPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticLogTests.swiftPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/LocalizationTestSupport.swiftPackages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/TransportIncidentPolicyTests.swiftResources/Localizable.xcstrings
This was referenced Aug 4, 2026
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
Testing
Implementation
DiagnosticEventPresentation is an instantiated, stateless formatter shared by every export owner. This is a principled change because one exhaustive schema now owns user-facing names and payload decoding while stable machine identifiers remain available for telemetry correlation.
Checklist
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Replaces compact
cmuxdiag v1rows with a UTC, localized plain‑language timeline for Iroh/transport diagnostics across the app, CLI, socket command, and Sentry. Honors explicit locales for all titles and texts while keeping stable machine codes in tags and grouping.New Features
CMUXMobileCore(format v2) decodes all transport event codes; addsResources/Localizable.xcstringsanddefaultLocalization: "en"with resources processed by the package.DiagnosticLocalizationresolves strings for an explicitLocalewith a runtime check for compiled catalogs;DiagnosticIncidentTitleFormatterproduces localized, pluralized incident titles;TransportIncidentPolicyaccepts and honors aLocale.DiagnosticLog.export()and sharing produce a localized timeline with UTC timestamps (or relative time if no wall‑clock anchor) and valid UTF‑8 text.cmux iroh-diagand theiroh_diagsocket command print the same readable report; CLI help text is localized viacli.help.irohDiag.transport.event_codeandtransport.role_code.Migration
DiagnosticReport.compactExport()now returns the same human‑readable report ashumanReadableExport()(format v2). Update any parser expecting numericcmuxdiag v1rows.transport.event_code,transport.role_code). Attachments are the plain‑language timeline.Written for commit d6ec333. Summary will update on new commits.
Summary by CodeRabbit