Skip to content

refactor(ui): extract GitSyncCoordinator from StelekitViewModel - #379

Merged
tstapler merged 1 commit into
mainfrom
refactor/stelekit-viewmodel-git-sync-coordinator
Oct 1, 2026
Merged

tstapler merged 1 commit into
mainfrom
refactor/stelekit-viewmodel-git-sync-coordinator

Conversation

@tstapler

@tstapler tstapler commented Oct 1, 2026

Copy link
Copy Markdown
Owner

Summary

Phase 3 of the StelekitViewModel decomposition plan — 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 StateFlows (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

Moves the git sync status machinery (syncState/gitLastSyncAt derived
StateFlows) and the git setup/conflict/journal-merge dialog surfaces into a
new GitSyncCoordinator collaborator, following the SectionManagementCoordinator
(PR #375) and LlmSuggestionCoordinator (PR #378) pattern. StelekitViewModel
keeps one-line forwarders for every public method and the two public
StateFlow properties that Compose call sites and dedicated tests read
directly.

Phase 3 of project_plans/stelekit-viewmodel-decomposition/plan.md.
@tstapler
tstapler marked this pull request as ready for review October 1, 2026 05:48
Copilot AI balanced review requested due to automatic review settings October 1, 2026 05:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The extraction preserves existing behavior and lifecycle ownership while retaining all public entry points.

Review effort: Balanced
Findings: None

What changed in this PR

Extracts Git sync state and dialog coordination from StelekitViewModel into a focused collaborator while preserving its public API.

Changes:

  • Adds GitSyncCoordinator for sync flows, actions, and dialog state.
  • Replaces ViewModel implementations with forwarding methods and properties.
File Description
StelekitViewModel.kt Delegates Git sync responsibilities to the coordinator.
GitSyncCoordinator.kt Contains the extracted Git sync behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

JVM Load Benchmark (Desktop)

Synthetic in-memory benchmark measuring load performance for the desktop (JVM) app.
Comparing 1fde1f01 (this PR) vs c8cf89ac (baseline)
Graph config: xlarge — 230 pages

Metric This PR Baseline Delta
Phase 1 TTI ↓ 1ms 1ms 0 (0%)
Phase 2 background ↓ 0ms 0ms 0 (0%)
Phase 3 index ↓ 1ms 1ms 0 (0%)
Total ↓ 2ms 2ms 0 (0%)
Write p95 (baseline) ↓ 18ms 20ms -2ms (-10%) ✅
Write p95 (under load) ↓ n/a n/a
Jank factor ↓ n/a n/a
↓ lower is better
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) `36.5%` byte[]_[k] `7.7%` java.lang.String_[k] `7.2%` java.util.LinkedHashMap$Entry_[k] `6.3%` int[]_[k] `3.4%` java.lang.Object[]_[k]
Top CPU hotspots (this PR) `96.3%` /usr/lib/x86_64-linux-gnu/libc.so.6 `1.4%` /tmp/sqlite-3.51.3.0-d4b50008-d0fa-44ff-a32c-a1f86b1231bc-libsqlitejdbc.so `0.5%` __libc_pwrite `0.2%` fsync `0.2%` pthread_cond_signal

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Android Load Benchmark

Instrumented benchmark on an API 30 x86_64 emulator — 500-page synthetic graph.

Comparing 1fde1f01 (this PR) vs 7115ce11 (baseline)
Device: API 30 x86_64 emulator — 530 pages loaded

Graph Load

Metric This PR Baseline Delta
Phase 1 TTI ↓ 47ms 37ms +10ms (+27%) ⚠️
Phase 3 index ↓ 5125ms 4834ms +291ms (+6%) ⚠️

Interactive Write Latency (during Phase 3)

Metric This PR Baseline Delta
Write p95 (baseline) ↓ 14ms 11ms +3ms (+27%) ⚠️
Write p95 (during phase 3) ↓ 19ms 18ms +1ms (+6%) ⚠️
Jank factor ↓ 1.36x 1.64x -0.28x (-17%) ✅
Concurrent writes ↑ 24 23 +1ms (+4%) ✅

SAF I/O Overhead (ContentProvider vs direct File read)

Measures Binder IPC cost added by ContentResolver per readFile() call.
Real SAF via ExternalStorageProvider will be higher on device; this is a lower bound.

Metric This PR Baseline Delta
Direct read / file ↓ 0.0ms 0.0ms 0 (0%)
Provider read / file ↓ 0.2ms 0.2ms 0 (0%)
IPC overhead ratio ↓ 7x 7x 0 (0%)
↓ lower is better · ↑ higher is better

@tstapler
tstapler merged commit 639ab56 into main Oct 1, 2026
21 of 25 checks passed
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.
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