Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 10eb74f6a4
ℹ️ 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".
4bcea08 to
f184c3c
Compare
samholmes
left a comment
There was a problem hiding this comment.
Well-structured debug scene that follows project conventions nicely. A few suggestions for reducing duplication and improving consistency — see inline comments.
|
Addressed all inline suggestions (dedupe wallet labels, remove unnecessary as-const, add Nodes empty state, document shared cache, and update effect deps/order). |
238c52f to
337d49a
Compare
4cbe75b to
7d40e47
Compare
abfaff7 to
45c9997
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| ...prev, | ||
| [dumpKey]: !(prev[dumpKey] ?? false) | ||
| } | ||
| }) |
There was a problem hiding this comment.
Side effect inside React state updater function
Low Severity
handleDumpWalletPress sets the external variable shouldLoadDump as a side effect inside the setWalletExpandedMap state updater. React state updaters are expected to be pure functions. In Strict Mode, React may invoke updaters twice, causing shouldLoadDump to be written twice. While the current implementation happens to work (both invocations receive the same prev), this couples async dispatch logic to a state transition in a fragile way. Computing shouldLoadDump from refs before the setState call would be safer and clearer.


CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
Description
Asana task
Context
Asana task
Add debug scene to settingsrequests a Developer Mode-only debugscene under Settings to inspect:
dataDumpoutput,Intentionally violating 1rem margin rules because this is a debug scene and we want all the space we can get
Changes
debugSettingsnavigation route and registered the new scene.DebugScenewith:dumpData()values,dumpDataloading + display,infoandactivitylog viewing viareadLogs().Unreleased (develop)changelog entry.Requirements
If you have made any visual changes to the GUI. Make sure you have:
Note
Medium Risk
Although gated behind Developer Mode, this introduces new UI paths that surface and copy potentially sensitive wallet dump and log data; correctness and performance depend on
dumpData()calls across wallets and log file reads.Overview
Adds a new Developer Mode-only
Debugsettings screen (debugSettings) and wires it into navigation and the Settings developer section.The new
DebugScenelets developers expand wallets to inspect node/server configuration derived from walletdumpData(), view full enginedataDumpoutput, and view/refresh/copyinfoandactivitylogs viareadLogs()(including long-press-to-copy for sections/rows). Localized strings and the Unreleased changelog entry are updated accordingly.Written by Cursor Bugbot for commit 45c9997. This will update automatically on new commits. Configure here.