Skip to content

Conversation

@ovitrif
Copy link
Collaborator

@ovitrif ovitrif commented Jan 9, 2026

This PR:

  1. Localizes most of the remaining hardcoded strings
    • Some toast messages aren't yet localized but the plan is to refactor the toast apis so we can pass either resourceId or string and localize the remaining texts in the PR for that work.
  2. Adds dynamic boost fee time confirmation estimates
    • Although on UI it will almost always still show the same estimate as the hardcoded one, that is because of the values of the fee rates.
  3. Implements weather widget API response caching
  4. Fixes WalletRepo state management by replacing uiState with individual repo states
  5. Cleans up deprecated code, warnings, and TODOs across the codebase
  6. Fixes all errors to use class instead of data class or data object
  7. Adds context to logs where it was missing
  8. Refactors most try/catch statements to runCatching
  9. Restructures Env constants
  10. Replaces combined main uiState with direct use of the separated node & wallet state
  11. Simplifies LightningService by removing redundant state management
  12. Removes deprecated migration test
  13. Remove completed TODOs and fixes or suppresses remaining ones
  14. Updates AGP & adds compose-stability-analyzer plugin
  15. Fixes or suppress all lint warnings
  16. Updates README and documentation adding recommended IDE plugins with setup guide
  17. Localizes articles timestamps using modern android APIs for relative time texts
  18. Captures and cancels job for running the LDK event listener(s)
  19. Fixes toast blur on wallet restore error screen
  20. Removes logs reset on wallet wipe (now only for debug builds)

Preview

N/A

QA Notes

Tests:

1. Boost Fee Estimates

  • Open a pending on-chain transaction
  • Tap boost/accelerate
  • Verify fee slider shows dynamic time estimates (e.g., "~10 min", "~30 min")

3. Weather Widget

  • Enable weather widget
  • Observe API calls are cached (check logs)
  • Widget should still update but without excessive network requests

4. Regression Testing

  • Verify wallet balance displays correctly
  • Test sending and receiving Lightning payments
  • Confirm background notifications work as expected

5️⃣ - Check timestamps of articles in widget using different languages - should be localized

@ovitrif ovitrif self-assigned this Jan 9, 2026
@ovitrif ovitrif changed the title chore: fix warnings and cleanup codebase chore: fix all warnings and cleanup codebase Jan 9, 2026
@ovitrif ovitrif changed the title chore: fix all warnings and cleanup codebase chore: fix all warnings and code cleanup Jan 9, 2026
@claude

This comment has been minimized.

@ovitrif
Copy link
Collaborator Author

ovitrif commented Jan 14, 2026

@claude what are the steps of the failing e2e tests and which tests are specifically failing, so we can test manually and fix the implementation on this PR as needed

@claude

This comment has been minimized.

@ovitrif ovitrif marked this pull request as draft January 14, 2026 12:01
auto-merge was automatically disabled January 14, 2026 12:01

Pull request was converted to draft

@ovitrif
Copy link
Collaborator Author

ovitrif commented Jan 14, 2026

Drafted to expand the work and address the e2e failures.

@ovitrif ovitrif requested a review from jvsena42 January 14, 2026 15:03
@ovitrif ovitrif marked this pull request as ready for review January 14, 2026 17:25
@claude

This comment has been minimized.

@claude
Copy link

claude bot commented Jan 14, 2026

Code Review

Found 4 CLAUDE.md compliance issues related to @Suppress annotation scoping. All issues involve using @file:Suppress when a more targeted suppression is possible.


1. app/src/main/java/to/bitkit/ui/components/Button.kt (Line 1)

CLAUDE.md Violation: Overly broad @Suppress scope

The @file:Suppress("MatchingDeclarationName") annotation is used at file level when a more targeted suppression is possible.

Issue: Only the ButtonSize enum (line 41) causes the MatchingDeclarationName violation. The suppression should be placed directly on that enum instead of at file level.

CLAUDE.md Rule:

ALWAYS place @Suppress(...) annotations at the narrowest possible scope (statement, property, or function level); NEVER use @file:Suppress(...) when a more targeted suppression is possible

CLAUDE.md reference

Suggested fix:
Remove the file-level suppression and add it directly to the enum at line 41:

@Suppress("MatchingDeclarationName")
enum class ButtonSize {
    Small, Large;
    // ...
}

2. app/src/main/java/to/bitkit/ui/components/LightningChannel.kt (Line 1)

CLAUDE.md Violation: Overly broad @Suppress scope

The @file:Suppress("MatchingDeclarationName") annotation is used at file level when a more targeted suppression is possible.

Issue: Only the ChannelStatusUi enum (line 34) causes the MatchingDeclarationName violation. The main LightningChannel composable matches the filename. The suppression should be placed directly on the enum instead of at file level.

CLAUDE.md Rule:

ALWAYS place @Suppress(...) annotations at the narrowest possible scope (statement, property, or function level); NEVER use @file:Suppress(...) when a more targeted suppression is possible

CLAUDE.md reference

Suggested fix:
Remove the file-level suppression and add it directly to the enum at line 34:

@Suppress("MatchingDeclarationName")
enum class ChannelStatusUi { PENDING, OPEN, CLOSED }

3. app/src/main/java/to/bitkit/ui/screens/wallets/receive/ReceiveConfirmScreen.kt (Line 1)

CLAUDE.md Violation: Overly broad @Suppress scope

The @file:Suppress("MatchingDeclarationName") annotation is used at file level when a more targeted suppression is possible.

Issue: Only the CjitEntryDetails data class (line 53) causes the MatchingDeclarationName violation. The suppression should be placed directly on that data class instead of at file level.

CLAUDE.md Rule:

ALWAYS place @Suppress(...) annotations at the narrowest possible scope (statement, property, or function level); NEVER use @file:Suppress(...) when a more targeted suppression is possible

CLAUDE.md reference

Suggested fix:
Remove the file-level suppression and add it directly to the data class at line 53:

@Suppress("MatchingDeclarationName")
@Serializable
data class CjitEntryDetails(
    val lspBaseUrl: String,
    val amount: USat,
)

4. app/src/main/java/to/bitkit/ui/settings/backups/ResetAndRestoreScreen.kt (Line 1)

CLAUDE.md Violation: Overly broad @Suppress scope

The @file:Suppress("MatchingDeclarationName") annotation is used at file level when a more targeted suppression is possible.

Issue: Only the ResetAndRestoreTestTags object (line 135) causes the MatchingDeclarationName violation. The main ResetAndRestoreScreen composable matches the filename. The suppression should be placed directly on the object instead of at file level.

CLAUDE.md Rule:

ALWAYS place @Suppress(...) annotations at the narrowest possible scope (statement, property, or function level); NEVER use @file:Suppress(...) when a more targeted suppression is possible

CLAUDE.md reference

Suggested fix:
Remove the file-level suppression and add it directly to the object at line 135:

@Suppress("MatchingDeclarationName")
object ResetAndRestoreTestTags {
    const val SCREEN = "restore_screen"
    const val BACKUP_BUTTON = "restore_backup_button"
    const val RESET_BUTTON = "restore_reset_button"
    const val RESET_DIALOG = "restore_reset_button"
}

Summary: All 4 issues can be fixed by moving the @Suppress annotation from file-level to the specific declaration that triggers the lint warning. No bugs or other issues found.

@ovitrif
Copy link
Collaborator Author

ovitrif commented Jan 14, 2026

@claude Is the review feedback about the matching declaration name suppress annotations correct?

In CLI you said it those SHOULD be at file level because that's the correct scope for such suppressions...

@claude
Copy link

claude bot commented Jan 14, 2026

Claude finished @ovitrif's task in 1m 4s —— View job


Answer: The review feedback is INCORRECT

You're correct to question the review feedback. The @file:Suppress("MatchingDeclarationName") annotations should remain at file level in these cases.


Why File-Level is Correct for MatchingDeclarationName

The MatchingDeclarationName lint rule (detekt.yml:355-368) checks that the primary declaration in a file matches the filename. This is inherently a file-level concern, not a declaration-level one.

Looking at the 4 files:

  1. Button.kt - Contains ButtonSize enum (line 41), but also PrimaryButton, SecondaryButton, TertiaryButton composables. The file is named after buttons, not ButtonSize.
  2. LightningChannel.kt - Contains ChannelStatusUi enum (line 34), but the main composable is LightningChannel which matches the filename.
  3. ReceiveConfirmScreen.kt - Contains CjitEntryDetails data class (line 53), but the main composable is ReceiveConfirmScreen which matches the filename.
  4. ResetAndRestoreScreen.kt - Contains ResetAndRestoreTestTags object (line 135), but the main composable is ResetAndRestoreScreen which matches the filename.

In all cases, the file is correctly named after the primary declaration. The secondary declarations (enums, data classes, objects) that don't match the filename are utility types that support the main declaration. This is the exact pattern mentioned in CLAUDE.md line 190:

PREFER declaring small dependant classes, constants, interfaces or top-level functions in the same file with the core class where these are used


Why Declaration-Level Suppression Won't Work

If you move @Suppress("MatchingDeclarationName") to just the ButtonSize enum, detekt will still flag the file because it sees a top-level declaration that doesn't match the filename. The rule operates at file scope, not declaration scope.


CLAUDE.md Rule Context

The CLAUDE.md rule (line 212):

ALWAYS place @Suppress(...) annotations at the narrowest possible scope (statement, property, or function level); NEVER use @file:Suppress(...) when a more targeted suppression is possible

This rule applies when a targeted suppression is actually possible. For file-level lint rules like MatchingDeclarationName, file-level is the correct and only scope that works.


Conclusion

The current @file:Suppress("MatchingDeclarationName") annotations are correct and should not be changed. The review feedback misapplied the CLAUDE.md rule to a case where file-level suppression is the appropriate solution.


@ovitrif
Copy link
Collaborator Author

ovitrif commented Jan 14, 2026

For PR #594 close comment:
Closed in favour of stacked PRs:

@ovitrif ovitrif closed this Jan 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants