refactor(ui): extract LlmSuggestionCoordinator from StelekitViewModel - #378
Merged
tstapler merged 4 commits intoOct 1, 2026
Merged
Conversation
Phase 2 of 5 from project_plans/stelekit-viewmodel-decomposition/plan.md: moves the LLM approval-gated edit workflow (llmSuggestions, observeLlmSuggestions, proposeLlmSuggestion, dismissLlmSuggestionReview, rejectLlmSuggestion, acceptLlmSuggestion, openLlmProviderSettings, dismissLlmProviderSettings) out of StelekitViewModel into a new LlmSuggestionCoordinator, mirroring Phase 1's SectionManagementCoordinator pattern (shares _uiState directly, reuses the ViewModel's scope). StelekitViewModel keeps one-line forwarders for every call site.
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The extraction preserves existing behavior and lifecycle handling; the remaining documentation correction is non-blocking.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Extracts the LLM suggestion workflow from StelekitViewModel into a dedicated coordinator while preserving its public API and shared lifecycle.
Changes:
- Adds
LlmSuggestionCoordinatorfor suggestion review, acceptance, rejection, and provider settings. - Retains ViewModel forwarding methods and initializes observation through the coordinator.
| File | Description |
|---|---|
StelekitViewModel.kt |
Delegates LLM workflow operations to the coordinator. |
LlmSuggestionCoordinator.kt |
Implements the extracted workflow and state orchestration. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+61
to
+63
| * parallel to `StelekitViewModel.observeSyncState`'s `syncState.collect` — does NOT | ||
| * auto-dismiss when the inbox becomes empty via accept/reject (those explicitly set | ||
| * visibility, same "do NOT auto-dismiss" rule as journal-merge review). |
Contributor
JVM Load Benchmark (Desktop)Synthetic in-memory benchmark measuring load performance for the desktop (JVM) app.
Flamegraphs (this PR)**Allocation** — object allocation pressure (JDBC/SQLite churn)Alloc flamegraph not available CPU — method-level hotspots by on-CPU time CPU flamegraph not available Top allocation hotspots (this PR)`37.4%` byte[]_[k] `7.4%` java.lang.String_[k] `7.2%` int[]_[k] `6.7%` java.util.LinkedHashMap$Entry_[k] `3.8%` java.lang.StringBuilder_[k]Top CPU hotspots (this PR)`96.5%` /usr/lib/x86_64-linux-gnu/libc.so.6 `1.4%` /tmp/sqlite-3.51.3.0-18cd4715-8427-4742-a479-2d534ce09ee6-libsqlitejdbc.so `0.5%` __libc_pwrite `0.2%` fsync `0.2%` SR_handler |
Contributor
Android Load BenchmarkInstrumented benchmark on an API 30 x86_64 emulator — 500-page synthetic graph. Comparing Graph Load
Interactive Write Latency (during Phase 3)
SAF I/O Overhead (ContentProvider vs direct File read)Measures Binder IPC cost added by ContentResolver per readFile() call.
|
This was referenced Oct 1, 2026
tstapler
added a commit
that referenced
this pull request
Oct 1, 2026
## Summary Phase 3 of [the StelekitViewModel decomposition plan](https://github.com/tstapler/stelekit/blob/main/project_plans/stelekit-viewmodel-decomposition/plan.md) — extracts the git-sync status machinery and the git setup/conflict/journal-merge dialog group into a new `GitSyncCoordinator` collaborator, following the `SectionManagementCoordinator` (#375) and `LlmSuggestionCoordinator` (#378) pattern. - New `GitSyncCoordinator` owns the `syncState`/`gitLastSyncAt` derived `StateFlow`s (built from the active `GitSyncService`) and the 16 moved functions (`triggerSync`, `triggerFetchOnly`, `setGitConfig`, git setup wizard open/dismiss variants, conflict-resolution/journal-merge-review dialog methods, git-detection/browser-only-sync banner dismissal). - `StelekitViewModel` keeps one-line forwarders for every public method plus the two public `StateFlow` properties (`syncState`, `gitLastSyncAt`), both read directly by Compose call sites (`GraphContentActiveShell`, `GraphDialogLayer`, `GraphContentLeftSidebar`) and by dedicated tests — verified by grep before removal. - Pure structural move, no behavior change. ## Verification - `./gradlew :kmp:compileKotlinJvm` — BUILD SUCCESSFUL - `./gradlew :kmp:jvmTest --tests StelekitViewModelSyncStateTest --tests StelekitViewModelSyncStateIntegrationTest` — all 4 tests PASSED - `scripts/jvm-display-check.sh -- ./gradlew :kmp:jvmTest` (full suite) — 5072 tests completed, 5 failed, 39 skipped; all 5 failures are the known pre-existing flakes (`JournalViewFanoutBenchmarkTest`, `CapturePopupWindowTest`, `CapturePopupWindowUxTest`, `BlockItemGestureTest.shiftClick_extendsSelection_notEditMode`), unrelated to this change
tstapler
added a commit
that referenced
this pull request
Oct 1, 2026
… from breaking CI (#381) ## Why `downloads.sourceforge.net` had intermittent Cloudflare 522 ("origin unreachable") outages on 2026-10-01 that: - Failed the Bazel Android build/test jobs on PRs #370, #373, #378, #379 - Caused `v0.89.0`'s release safety gate to exhaust its 3 retries, skipping every downstream build/publish job — the release shipped with **zero assets** Root cause: `MODULE.bazel`'s `unzip_src`/`zip_src` `http_archive` rules had a single hardcoded SourceForge URL each, with no fallback. ## Fix Both tarballs (InfoZip `unzip60.tar.gz` / `zip30.tar.gz`) are now hosted as assets on a dedicated, non-app [GitHub release](https://github.com/tstapler/stelekit/releases/tag/ci-vendored-deps-infozip-v1) in this repo — verified `sha256`-identical to the hashes already pinned in `MODULE.bazel` (confirmed against the original SourceForge files before the outage, and cross-checked against MacPorts/OSUOSL mirrors). `MODULE.bazel`'s `urls` lists now try, in order: our own GitHub release → MacPorts/OSUOSL mirrors → SourceForge (demoted to last, kept only as a final fallback). `http_archive` tries each URL until one succeeds, so this requires no other code changes. ## Verification - `bazel build --repository_cache=<empty> @unzip_src//:unzip @zip_src//:zip` after deleting the previously-extracted external repos — forces a true cold fetch, confirms the new URL list resolves and the sha256 check passes. - `bazel build //:stelekit_android_toolchain_impl` — the consumer of these two targets — builds clean.
…wmodel-llm-suggestion-coordinator # Conflicts: # kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/StelekitViewModel.kt
…wmodel-llm-suggestion-coordinator
…wmodel-llm-suggestion-coordinator # Conflicts: # kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/StelekitViewModel.kt
tstapler
deleted the
refactor/stelekit-viewmodel-llm-suggestion-coordinator
branch
October 1, 2026 09:05
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
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
project_plans/stelekit-viewmodel-decomposition/plan.md(PR docs(plan): StelekitViewModel.kt decomposition plan #373): extracts the 8-function LLM Suggestion Workflow group (llmSuggestions,observeLlmSuggestions,proposeLlmSuggestion,dismissLlmSuggestionReview,rejectLlmSuggestion,acceptLlmSuggestion,openLlmProviderSettings,dismissLlmProviderSettings) out ofStelekitViewModel.ktinto a newLlmSuggestionCoordinator, mirroring Phase 1'sSectionManagementCoordinator(PR refactor(ui): extract SectionManagementCoordinator from StelekitViewModel #375) — shares_uiState: MutableStateFlow<AppState>directly (the 2AppStatefields it owns are read byGraphDialogLayer.kt), reuses the ViewModel's ownscope.StelekitViewModel.kt: 2,667 → 2,619 lines. New file is 138 lines.llmSuggestionsval + 6 funs) keep one-line forwarders — verified by grep against every Compose UI and test call site (GraphDialogLayer.kt,GraphContentTagVoiceSetup.kt,screens/PageView.kt,StelekitViewModelLlmSettingsTest.kt,StelekitViewModelLlmSuggestionTest.kt).observeLlmSuggestionsis private/init-only, so it's called directly on the coordinator frominit {}with no VM-level forwarder, matching Phase 1'sloadSectionManifestprecedent.Test plan
./gradlew :kmp:compileKotlinJvm— BUILD SUCCESSFUL.StelekitViewModelLlmSettingsTest(jvmTest) — 1/1 passed;StelekitViewModelLlmSuggestionTest(businessTest) — 6/6 passed../gradlew :kmp:jvmTest(5,072 tests): 5 pre-existing failures (JournalViewFanoutBenchmarkTest,CapturePopupWindowTest,CapturePopupWindowUxTest×2,BlockItemGestureTest) — same count as PR refactor(ui): extract SectionManagementCoordinator from StelekitViewModel #375's baseline, confirmed via content-grep to have zero relation to LLM suggestion code.