feat(di): full Hilt migration — delete ServiceLocator, all 12 ViewModels injected by fabiodalez-dev · Pull Request #14 · fabiodalez-dev/Pinakes-Android

fabiodalez-dev · 2026-06-30T14:11:15Z

Full migration of the app's DI from the manual ServiceLocator to Hilt. Stacked on the Sentry branch.

What

ServiceLocator + LocalServices deleted. Hilt owns the whole graph.
AppModule: real @Provides @Singleton for every store / network / DB / DAO / repository.
All 12 ViewModels are @HiltViewModel @Inject; their hand-written ViewModelProvider.Factory classes are gone; every screen builds its VM with hiltViewModel(). BookDetailViewModel reads its bookId nav-arg from an injected SavedStateHandle.
App-wide state (auth state, feature flags, theme) → new AppViewModel (@HiltViewModel), replacing LocalServices. NavHost/MainScaffold use hiltViewModel(); MainActivity field-injects the stores.
Can't-be-injected code (the WorkManager CatalogSyncWorker, the Application foreground-refresh observer) reaches Hilt via an @EntryPoint.

Why it matters

Repositories/ViewModels are now unit-testable by swapping fakes via @TestInstallIn — the original goal that unblocks the CatalogRepository network tests.

Verified

assembleDebug builds — kapt validates the entire Hilt graph (every binding across all 12 ViewModels + the EntryPoints resolves).
testDebugUnitTest 37/37 green.

Caveat

Hilt graph correctness is compile-checked; runtime wiring is not. Needs one device smoke-test (launch → each tab renders, theme switch, open a book) before merge.

Note for the test branch (#13)

#13's CatalogSyncWorkerTest asserts the worker is a no-op under a non-PinakesApplication context. This branch switches the worker to a Hilt @EntryPoint, so that test must be updated to a Hilt test (@HiltAndroidTest + HiltTestApplication) when the two branches reconcile.

@androidentrypoint

…idges the rest) A working proof-of-concept of the "root-cause" DI fix (the testability lever), done incrementally so it's safe to land before the full migration. - Hilt wired: @HiltAndroidApp (PinakesApplication), @androidentrypoint (MainActivity), hilt-android + hilt-android-compiler (kapt) + hilt-navigation-compose. - AppModule: an incremental-migration BRIDGE — it exposes to Hilt the SAME singletons the manual ServiceLocator already owns (session/stores/network/repositories), so a Hilt-injected ViewModel and a ServiceLocator-built one share one instance each (no auth-state / cache desync while the migration is half-done). - Home slice migrated end-to-end: HomeViewModel is now @hiltviewmodel @Inject (its hand-written ViewModelProvider.Factory is deleted); HomeScreen builds it with hiltViewModel(). The other 9 screens are untouched and keep using LocalServices. Why this matters: once every screen is on Hilt, the AppModule bridges become real @provides (or @Inject constructors), ServiceLocator is deleted, and repositories become unit-testable by swapping fakes via @TestInstallIn — which unblocks the CatalogRepository network tests (the original goal). Verified: assembleDebug builds (kapt validates the full Hilt graph — every binding resolves); testDebugUnitTest 37/37 green. NOTE: needs a device smoke-test (launch → Home renders) before merge — Hilt graph correctness is compile-checked, runtime wiring is not.

coderabbitai · 2026-06-30T14:11:22Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4423f6a7-fda6-4036-8711-282cdbf727c9

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feat/hilt-di

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

@provides

… ViewModels injected Finishes the Hilt trial into a full migration. The manual ServiceLocator + LocalServices CompositionLocal are gone; Hilt owns the whole dependency graph. - AppModule: real @provides @singleton for every store/network/db/dao/repository (no longer a bridge to ServiceLocator). - All 12 ViewModels are @hiltviewmodel @Inject and their hand-written ViewModelProvider .Factory classes are deleted; every screen builds its VM with hiltViewModel(). BookDetailViewModel reads its bookId nav-arg from an injected SavedStateHandle. - App-wide state (auth state, feature flags, theme) moves to a new AppViewModel (@hiltviewmodel) — replaces LocalServices. NavHost/MainScaffold use hiltViewModel(); MainActivity field-injects SessionStore/ThemeStore/AuthRepository. - Code that can't be constructor-injected reaches Hilt via an EntryPoint: the CatalogSyncWorker (WorkManager-created) and the Application's foreground-refresh observer. - Deleted di/ServiceLocator.kt and ui/common/LocalServices.kt. Why it matters: repositories/ViewModels are now unit-testable by swapping fakes via @TestInstallIn (the original goal — unblocks the CatalogRepository network tests). Verified: assembleDebug builds — kapt validates the ENTIRE Hilt graph (every binding across all 12 ViewModels + EntryPoints resolves); testDebugUnitTest 37/37 green. NOTE: still needs a device smoke-test (launch → each tab/screen renders, theme switch, book detail) before merge — Hilt graph correctness is compile-checked, runtime is not.

fabiodalez-dev · 2026-06-30T16:00:15Z

Auto-recommendation acceptance

rev_01KWCHVEBKR4QK6RKXPR75TC63 · run_id=fixrun_01KWCM039QSSGGATC11AZF3QRS · choice=apply_all · threshold=60 · reviewer=auto-rec/fabiodalezbackup@gmail.com · ts=2026-06-30T16:00:14Z

1 auto-rec finding eligible at threshold ≥ 60. 1 auto-promoted via batch, 0 edited, 0 skipped.

Promoted (batch)

F004 — The BOOK_DETAIL composable still extracts the nav arg and forwards it to BookDetailScreen(bookId=...), but that parameter is ignored downstream (the VM reads the same arg via SavedStateHandle). Dead/duplicated work; the two paths reading the same key could silently diverge if one default is later changed.
- Hint: Remove the bookId: Int parameter from BookDetailScreen's signature (declared on line 89 but never read inside the function body — the ViewModel acquires the ID itself via savedStateHandle.get<Int>(Routes.ARG_BOOK_ID)). In PinakesNavHost.kt collapse lines 112-113 to a single call with no bookId arg: BookDetailScreen(onNavigateUp = { navController.popBackStack() }), dropping the now-unused val bookId local. Leave the navArgument(Routes.ARG_BOOK_ID) declaration on line 108 untouched — it populates Hilt's SavedStateHandle for the ViewModel.

Auto-recommendation acceptance: append-only audit. Promoted findings are now confirmed_mechanical with human_confirmation set; Phase 8 dispatches them next.

…F004) Fix groups (committed): - [FG-1] F004 — BookDetailScreen.kt, PinakesNavHost.kt: verified The Hilt migration moved book-id reading into BookDetailViewModel via SavedStateHandle, leaving the bookId parameter on BookDetailScreen and its nav-host extraction as dead pass-through. Removed both; kept the navArgument(ARG_BOOK_ID) declaration that feeds SavedStateHandle. Post-fix review: 1/1 groups verified complete; 0 partial; 0 reverted.

fabiodalez-dev · 2026-06-30T16:10:14Z

Code review

Branch: feat/hilt-di → feat/sentry-crash-reporting
PR: #14 (open)
Review ID: rev_01KWCHVEBKR4QK6RKXPR75TC63
Sub-agent tokens: 893,729 across 13 invocations

Found 8 findings across all lanes:

Pre-existing (high-confidence origin, report-only): 2

Deep lane — correctness & security

Pre-existing — report-only (2)

Shown only when origin_confidence: high. Never auto-fixed in v1 (§13.1 pre-existing override).

#	Score	File	Finding	Follow-up
F008		`app/src/main/java/com/pinakes/app/PinakesApplication.kt:37`	Sentry DSN is hardcoded in plaintext in version-controlled source. A public DSN allows any actor who reads the source to flood the Sentry project with forged events, exhausting the event quota.
F009		`app/src/main/java/com/pinakes/app/PinakesApplication.kt:35-43`	Sentry Gradle plugin OkHttp auto-instrumentation may capture HTTP breadcrumbs including the Authorization: Bearer header; isSendDefaultPii=false suppresses user/IP but does not explicitly strip custom request headers from OkHttp breadcrumbs.

Fix runs

Run `fixrun_01KWCM039QSSGGATC11AZF3QRS` — 2026-06-30T16:09:47Z

Outcomes: 1 fixed and verified
Commits: 61e287b

Finding	Group	Outcome	phase_9_finding
F004	FG-1	✓ fixed and verified

🤖 Generated with Adam's Claude Code Review Command

New release including the Hilt DI migration and the F004 dead-parameter cleanup.

- #11 CatalogSyncWorker: retry only TRANSIENT failures; give up (success) on permanent ones (401/403/app_disabled/validation/not_found) so the periodic chain isn't stuck retrying an unrecoverable condition. Classification is a pure isPermanentFailure() so it's unit-testable without Android/Hilt. - #13 CatalogSyncWorkerTest: rewritten — the old 'no-op on plain context' test was written for the pre-Hilt (ServiceLocator) worker and broke under #14's EntryPointAccessors migration. Now covers the real branch (permanent vs transient) via the pure function. - #9 HomeViewModel: don't flip loading=false on an empty cache emission — keep the skeleton until the first refresh resolves, so a cold start no longer flashes the 'empty library' state while the network is in flight. - #10 CI: build the release variant too (assembleRelease) so R8/minify is exercised in CI, not only at runtime. Merged the 7-PR stack (#8 navbar/reserve, #9 offline cache, #10 CI/R8, #11 WorkManager sync, #12 Sentry, #13 tests, #14 Hilt) into main; the libs.versions.toml conflict (#13 work-testing vs #14 Hilt libs) resolved as a union. Full build green: 67 unit tests, assembleDebug + assembleRelease.

fabiodalez-dev · 2026-07-02T14:43:21Z

Landed in main via the integrated stack merge (main @ 754d25e). Every commit of this branch is now reachable from main; the review fixes were applied on top in the same integration (see commit 754d25e). Closing as merged.

fabiodalez-dev changed the title ~~feat(di): Hilt trial — infra + Home slice (ServiceLocator bridges the rest)~~ feat(di): full Hilt migration — delete ServiceLocator, all 12 ViewModels injected Jun 30, 2026

chore: bump version to 1.2.1 (versionCode 5)

5a6486f

New release including the Hilt DI migration and the F004 dead-parameter cleanup.

fabiodalez-dev closed this Jul 2, 2026

fabiodalez-dev deleted the feat/hilt-di branch July 2, 2026 14:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(di): full Hilt migration — delete ServiceLocator, all 12 ViewModels injected#14

feat(di): full Hilt migration — delete ServiceLocator, all 12 ViewModels injected#14
fabiodalez-dev wants to merge 4 commits into
feat/sentry-crash-reportingfrom
feat/hilt-di

fabiodalez-dev commented Jun 30, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jun 30, 2026 •

edited

Loading

Review skipped

Uh oh!

fabiodalez-dev commented Jun 30, 2026

Uh oh!

fabiodalez-dev commented Jun 30, 2026

Uh oh!

fabiodalez-dev commented Jul 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

fabiodalez-dev commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why it matters

Verified

Caveat

Note for the test branch (#13)

Uh oh!

coderabbitai Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

fabiodalez-dev commented Jun 30, 2026

Auto-recommendation acceptance

Promoted (batch)

Uh oh!

fabiodalez-dev commented Jun 30, 2026

Code review

Deep lane — correctness & security

Pre-existing — report-only (2)

Fix runs

Run fixrun_01KWCM039QSSGGATC11AZF3QRS — 2026-06-30T16:09:47Z

Uh oh!

fabiodalez-dev commented Jul 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

fabiodalez-dev commented Jun 30, 2026 •

edited

Loading

coderabbitai Bot commented Jun 30, 2026 •

edited

Loading

Run `fixrun_01KWCM039QSSGGATC11AZF3QRS` — 2026-06-30T16:09:47Z