Conversation
Wire MAC-based ghost suppression through PacketRouter and node side effects, and only store parseable BLE MACs so Linux Web BT UUIDs cannot pretend to be radios for Meshtastic last_heard bumps.
AppPanel is lazy, so Chat and other panels could keep a stale localStorage clock until the App tab was opened. Reconcile early from app_settings like locale.
Align hub→backbone picker/CTA copy, Nomad countdown “left”, anatomy backbone labels, zh tabAll, ja I2P spacing, and related path/guidance strings; gate regressions in check-i18n-quality.
|
Warning Review limit reached
Next review available in: 47 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: Path: .coderabbit.yaml Review profile: QUIET Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds pre-release locale checks, hydrates the time-format store from SQLite at startup, and derives valid MeshCore BLE MACs for Meshtastic ghost-node suppression across runtime, packet routing, and node side effects. ChangesLocale quality validation
SQLite time-format hydration
MeshCore BLE ghost-node suppression
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (2)
scripts/check-i18n-quality.mjs-3828-3834 (1)
3828-3834: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd regression coverage for the Spanish
Pathrule.This branch is independent from the four added test cases. Add a Spanish
networkPanel.reticulumStackSettings.pathMediumPreferencecase that contains EnglishPath. This prevents removal or breakage of this validation.Proposed test
+ it('flags untranslated Path in Spanish path preference', () => { + const issues = localeStringQualityIssues({ + locale: 'es', + flatKey: 'networkPanel.reticulumStackSettings.pathMediumPreference', + enVal: 'Path preference', + val: 'Preferencia de Path', + }); + expectIssue(issues, 'must not leave English "Path"'); + });As per path instructions, “include regression tests for ... localization checks.”
🤖 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 `@scripts/check-i18n-quality.mjs` around lines 3828 - 3834, Add an independent regression fixture in the i18n quality checks for Spanish key networkPanel.reticulumStackSettings.pathMediumPreference whose value contains the English word “Path”, and assert that the validator reports an issue. Keep the existing validation branch unchanged and ensure the test specifically exercises the locale/key/value condition.Source: Path instructions
src/renderer/lib/drivers/PacketRouter.test.ts-411-455 (1)
411-455: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMove BLE MAC cleanup into
afterEachto avoid test pollution on failure.
resetConnectedMeshcoreBleMacForTests()runs only as the last statement in the test body, after theexpectcalls. If anexpectthrows, cleanup is skipped, and the suppression MAC set at line 436 stays applied for later tests. Move the reset toafterEachso it always runs.🧹 Proposed fix
it('skips Meshtastic node_info store writes for MeshCore BLE MAC ghost nodes', async () => { const { setConnectedMeshcoreBleMac, resetConnectedMeshcoreBleMacForTests } = await import('../connectedMeshcoreBleMac'); + afterEach(() => { + resetConnectedMeshcoreBleMacForTests(); + }); addIdentity({ ... }); ... expect(useNodeStore.getState().nodes[ID_MT][ghostId].lastHeardAt).toBe(priorHeard); expect(listener).not.toHaveBeenCalled(); detach(); - resetConnectedMeshcoreBleMacForTests(); });Note:
afterEachmust be registered outside theitcallback (e.g., in a wrappingdescribeor at file scope) for it to apply to this test; the snippet above illustrates intent only.🤖 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 `@src/renderer/lib/drivers/PacketRouter.test.ts` around lines 411 - 455, Move resetConnectedMeshcoreBleMacForTests cleanup out of the individual test and register it in an afterEach hook at describe or file scope so it runs even when assertions fail. Remove the inline reset call from the test while preserving the existing setConnectedMeshcoreBleMac setup and test behavior.Source: Path instructions
🤖 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.
Other comments:
In `@scripts/check-i18n-quality.mjs`:
- Around line 3828-3834: Add an independent regression fixture in the i18n
quality checks for Spanish key
networkPanel.reticulumStackSettings.pathMediumPreference whose value contains
the English word “Path”, and assert that the validator reports an issue. Keep
the existing validation branch unchanged and ensure the test specifically
exercises the locale/key/value condition.
In `@src/renderer/lib/drivers/PacketRouter.test.ts`:
- Around line 411-455: Move resetConnectedMeshcoreBleMacForTests cleanup out of
the individual test and register it in an afterEach hook at describe or file
scope so it runs even when assertions fail. Remove the inline reset call from
the test while preserving the existing setConnectedMeshcoreBleMac setup and test
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Pro Plus
Run ID: b5468d29-f993-49c7-8b62-c01bf42fe16f
⛔ Files ignored due to path filters (16)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/pnpm-lock.yamlsrc/renderer/locales/cs/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/de/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/es/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/fr/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/id/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/it/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/ja/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/ko/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/nl/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/pl/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/pt-BR/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/ru/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/tr/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/uk/translation.jsonis excluded by!src/renderer/locales/**src/renderer/locales/zh/translation.jsonis excluded by!src/renderer/locales/**
📒 Files selected for processing (12)
scripts/check-i18n-quality.mjsscripts/check-i18n-quality.test.mjssrc/renderer/App.tsxsrc/renderer/lib/connectedMeshcoreBleMac.test.tssrc/renderer/lib/connectedMeshcoreBleMac.tssrc/renderer/lib/drivers/PacketRouter.test.tssrc/renderer/lib/drivers/PacketRouter.tssrc/renderer/lib/meshtastic/meshtasticNodeSideEffects.test.tssrc/renderer/lib/meshtastic/meshtasticNodeSideEffects.tssrc/renderer/runtime/useMeshcoreRuntime.reconnect.test.tssrc/renderer/runtime/useMeshcoreRuntime.tssrc/renderer/stores/timeFormatStore.test.ts
Add an independent i18n-quality fixture for es pathMediumPreference with English "Path", and move MeshCore BLE MAC test cleanup into afterEach so it still runs when assertions fail.
1 participant
Summary
last_heardbumps.use24HourTimefrom SQLite on App mount so Chat/Diagnostics do not keep a stale localStorage clock until the App tab opens.tabAll, ja I2P spacing, path/guidance copy) and addcheck-i18n-qualityguards.baseline-browser-mapping).Test plan
pnpm exec vitest run src/renderer/lib/meshtastic/meshtasticNodeSideEffects.test.ts src/renderer/lib/drivers/PacketRouter.test.ts src/renderer/lib/connectedMeshcoreBleMac.test.ts src/renderer/runtime/useMeshcoreRuntime.reconnect.test.ts src/renderer/stores/timeFormatStore.test.ts scripts/check-i18n-quality.test.mjspnpm run check:i18nSummary by CodeRabbit