fix(search): prevent Space key from dismissing the search dialog by tstapler · Pull Request #99 · tstapler/stelekit

tstapler · 2026-05-27T00:48:17Z

Summary

Bug fix: Typing a space in the search dialog no longer closes it
Root cause: Compose's clickable modifier responds to the Space key (accessibility activation). Unhandled Space KeyEvents were bubbling from the focused TextField up to the backdrop Box (which has clickable(onClick = onDismiss)), triggering onDismiss() on every space typed
Fix: Consume Key.Spacebar in the dialog panel's onKeyEvent handler so it never reaches the backdrop. The TextField's text-input system adds the space character independently of the key-event pipeline

This PR also includes preceding architectural refactoring commits (port interfaces, ISP split, scope ownership, watcher correctness, and atomic write actor improvements) that were already reviewed.

Test plan

New regression test searchDialog_spaceKeyDoesNotDismiss — presses Space via performKeyInput, asserts onDismiss is NOT called
All 4 SearchDialogTest tests pass locally

🤖 Generated with Claude Code

Fast typing followed by Enter caused new blocks to disappear or revert because splitBlock/mergeBlock/handleBackspace called blockRepository directly, bypassing the actor queue. A pending updateBlockContentOnly enqueued in the actor would execute after the structural op, overwriting the split/merge result with stale content. Fix: add typed splitBlock/mergeBlocks/deleteBlockStructural methods to DatabaseWriteActor and route all structural call sites in BlockStateManager through the actor via private writeSplitBlock/writeMergeBlocks/writeDeleteBlockStructural helpers (writeActor?.xxx() ?: blockRepository.xxx() fallback for tests). Also adds hasPendingWrites (AtomicInt counter) to DatabaseWriteActor and exposes hasActorPendingWrites on BlockStateManager so conflict detection in StelekitViewModel catches external file changes that arrive while a split/merge is in-flight. Adds 4 race-condition tests that verify ordering under concurrent content-write + structural-op scenarios. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…ased hasPendingWrites Two issues flagged in PR review: 1. `_activeOps` used `@Volatile Int` with `++`/`--` — not atomic; concurrent `execute()` callers could corrupt the count or make it go negative. 2. Counter was incremented before `send()` but decremented only after `await()`. If `send()` threw (closed channel, cancellation), the counter leaked indefinitely. Fix: drop the explicit counter entirely. `hasPendingWrites` now derives state from the channels themselves (`!highPriority.isEmpty || !lowPriority.isEmpty`) plus an actor-owned `@Volatile Boolean` flag set inside the `processRequest` try/finally. Channel.isEmpty is concurrency-safe; the flag is single-writer (actor coroutine), so @volatile gives correct multi-reader visibility without atomics or new deps. Also adds the ClassLevelDirectRepositoryWriteOptIn detekt rule (post-mortem enforcement) and moves class-level @OptIn to function-level across BlockStateManager, StelekitViewModel, AnnotationEditorViewModel, BacklinkRenamer, and GlobalUnlinkedReferencesViewModel. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

… scope ownership, watcher correctness Addresses all 31 findings from the post-architecture code review. Key changes: - GraphLoaderPort / GraphWriterPort: replace mutable var properties with explicit setter functions; add renamePage/savePage/deletePage to port so callers never need unsafe casts to the concrete type - BlockRepository split into BlockReadRepository, BlockWriteRepository, BlockSearchRepository, BlockStructureRepository (ISP) - ImportViewModel / ScreenRouter: class now owns its CoroutineScope; removed rememberCoroutineScope() leaking out of composition (C18 fix) - GraphFileWatcher: fix SharedFlow suppression (Channel.RENDEZVOUS replaces broken yield() pattern), add close() to cancel owned scope, add pollIntervalMs parameter, log dropped events (C13/C14/C15/A3) - MarkdownPageParser: extract to own file, replace Pair/Triple returns with typed PageBuildResult/PageMetadata, deduplicate mergedProperties, remove dead title alias - FlashcardScheduler: extract SM-2 logic from @composable into pure stateless object - GraphEvents.kt: move ExternalFileChange and WriteError out of GraphLoader - AllPagesViewModel: depend on BlockSearchRepository, not full BlockRepository - BlockWriteRepository.clear(): return Either<DomainError, Unit> - Remove unused count field from DuplicateGroup - StelekitViewModelDependencies: remove default scope value, require explicit supply - GraphLoaderWatcherTest: replace vacuous assertTrue(count >= 0) with real assertion - GraphFileWatcherTest: new suite covering suppression, close(), git-merge paths; fix virtual-time/real-time timeout mismatch and registry priming Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Compose's `clickable` modifier responds to Space as an accessibility activation. Unhandled Space key events were bubbling from the focused TextField up to the backdrop Box (which has `clickable(onClick = onDismiss)`), causing the dialog to close whenever the user typed a space. Fix: consume `Key.Spacebar` in the dialog panel's `onKeyEvent` handler so it never reaches the backdrop. The TextField's text-input system handles adding the space character independently of the key-event pipeline. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

github-actions · 2026-05-27T00:51:06Z

JVM Load Benchmark (Desktop)

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

Metric	This PR	Baseline	Delta
Phase 1 TTI ↓	10ms	9ms	+1ms (+11%) ⚠️
Phase 2 background ↓	4ms	3ms	+1ms (+33%) ⚠️
Phase 3 index ↓	15ms	15ms	0 (0%)
Total ↓	28ms	26ms	+2ms (+8%) ⚠️
Write p95 (baseline) ↓	28ms	35ms	-7ms (-20%) ✅
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 SQL queries by total time (this PR)

| table:operation | calls | p50 | p99 | max | total | |-----------------|-------|-----|-----|-----|-------| | `pages:select` | 2 | 1ms | 1ms | 1ms | 1ms |

Top allocation hotspots (this PR)

`67.9%` byte[]_[k] `3.7%` java.lang.String_[k] `3.2%` java.lang.Object[]_[k] `2.1%` jdk.internal.org.objectweb.asm.SymbolTable$Entry_[k] `1.6%` java.lang.Integer_[k]

Top CPU hotspots (this PR)

`99.2%` /usr/lib/x86_64-linux-gnu/libc.so.6 `0%` kotlin/coroutines/CoroutineContext$DefaultImpls.plus$lambda$0_[0] `0%` g1_post_barrier_slow `0%` __libc_pwrite `0%` jdk/proxy3/$Proxy39._[0]

Copilot

Pull request overview

This PR fixes the search dialog Space-key dismissal bug and includes broader repository/view-model refactors around dependency injection, repository interface separation, watcher behavior, write-actor serialization, and flashcard scheduling.

Changes:

Consumes Key.Spacebar in SearchDialog and adds a regression test.
Splits repository/graph ports, introduces StelekitViewModelDependencies, and migrates call sites.
Adds/refactors watcher, parser, flashcard, and write-actor tests and supporting production code.

Reviewed changes

Copilot reviewed 64 out of 64 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/components/SearchDialog.kt`	Consumes Space key in search dialog panel.
`kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/components/SearchDialogTest.kt`	Adds Space-key regression test.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/StelekitViewModelDependencies.kt`	Adds ViewModel dependency parameter object.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/StelekitViewModel.kt`	Migrates to ports/dependencies and narrower opt-ins.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/state/BlockStateManager.kt`	Routes more writes through actor helpers.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/GraphFileWatcher.kt`	Extracts file watcher implementation.
`kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphFileWatcherTest.kt`	Adds watcher suppression/lifecycle tests.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/GraphLoader.kt`	Adopts watcher/parser/port abstractions.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/GraphLoaderPort.kt`	Adds loader port interface.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/GraphWriter.kt`	Adopts writer port and owned scope.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/GraphWriterPort.kt`	Adds writer port interface.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/MarkdownPageParser.kt`	Extracts pure markdown page parsing/model building.
`kmp/src/commonTest/kotlin/dev/stapler/stelekit/db/MarkdownPageParserTest.kt`	Adds parser unit tests.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/GraphEvents.kt`	Moves graph event data classes.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/DatabaseWriteActor.kt`	Adds pending-write tracking and structural helpers.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/BacklinkRenamer.kt`	Uses writer port and narrows opt-in.
`kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderWatcherTest.kt`	Tightens watcher suppression assertion.
`kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderTest.kt`	Renames test repository type.
`kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/DemoGraphIntegrationTest.kt`	Renames test repository type.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/BlockRepository.kt`	Adds composite block repository interface.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/BlockReadRepository.kt`	Adds block read interface.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/BlockWriteRepository.kt`	Adds block write interface.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/BlockStructureRepository.kt`	Adds block structure interface.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/BlockSearchRepository.kt`	Adds block search interface.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/PageRepository.kt`	Splits page repository interface into own file.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/PropertyRepository.kt`	Splits property repository interface.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/ReferenceRepository.kt`	Splits reference repository interface.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/SearchRepository.kt`	Splits search repository interface.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/RepositoryFactory.kt`	Moves factory/set types and renames Datalog impls.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/GraphRepository.kt`	Removes monolithic repository definitions.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/DatalogBlockRepository.kt`	Renames Datascript block repository and updates clear return.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/DatalogPageRepository.kt`	Renames Datascript page repository.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/DatalogPropertyRepository.kt`	Renames Datascript property repository.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/DatalogReferenceRepository.kt`	Renames Datascript reference repository.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/InMemoryRepositories.kt`	Updates clear and duplicate group construction.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/SqlDelightBlockRepository.kt`	Updates clear and duplicate group construction.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/screens/AllPagesViewModel.kt`	Narrows dependency to block search interface.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/screens/ImportViewModel.kt`	Uses writer port and owned scope.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/screens/GlobalUnlinkedReferencesViewModel.kt`	Narrows direct-write opt-in.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/annotate/AnnotationEditorViewModel.kt`	Narrows direct-write opt-ins.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/ScreenRouter.kt`	Extracts screen routing composable.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/GraphDialogLayer.kt`	Extracts dialog/overlay layer.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/FlashcardsScreen.kt`	Adds flashcards UI.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/flashcard/FlashcardScheduler.kt`	Adds pure flashcard scheduling logic.
`kmp/src/businessTest/kotlin/dev/stapler/stelekit/flashcard/FlashcardReviewTest.kt`	Updates flashcard tests to use scheduler.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/editor/persistence/PersistenceIntegration.kt`	Renames repository implementation usage.
`kmp/src/commonMain/kotlin/dev/stapler/stelekit/data/repositories/IBlockRepository.kt`	Removes legacy block repository interface.
`kmp/src/commonTest/kotlin/dev/stapler/stelekit/ui/state/BlockStateManagerTest.kt`	Adds actor race-condition tests.
`kmp/src/commonTest/kotlin/dev/stapler/stelekit/ui/screens/JournalsViewModelTest.kt`	Updates fake clear signature.
`kmp/src/commonTest/kotlin/dev/stapler/stelekit/ui/screens/JournalsViewModelEditorTest.kt`	Updates fake clear signature.
`kmp/src/commonTest/kotlin/dev/stapler/stelekit/repository/DatalogBlockRepositoryTest.kt`	Renames repository test class/type.
`kmp/src/commonTest/kotlin/dev/stapler/stelekit/repository/BlockOperationsEdgeCaseTest.kt`	Renames repository type.
`kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/StelekitViewModelLoadingTest.kt`	Migrates ViewModel construction.
`kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/screens/PageViewUITest.kt`	Migrates ViewModel construction.
`kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/RecentPagesTest.kt`	Migrates ViewModel construction.
`kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/DiskConflictResolutionTest.kt`	Migrates ViewModel construction.
`kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/ComposeUITestBase.kt`	Migrates ViewModel construction.
`kmp/src/jvmTest/kotlin/dev/stapler/stelekit/ui/fixtures/FakeRepositories.kt`	Updates fake block clear signature.
`kmp/src/androidUnitTest/kotlin/dev/stapler/stelekit/ui/LoadingStateTest.kt`	Migrates ViewModel construction.
`kmp/config/detekt/detekt.yml`	Enables new detekt rule.
`buildSrc/src/main/kotlin/dev/stapler/detekt/ClassLevelDirectRepositoryWriteOptInRule.kt`	Adds detekt rule for class-level opt-ins.
`buildSrc/src/main/kotlin/dev/stapler/detekt/SteleKitRuleSetProvider.kt`	Registers new detekt rule.
`buildSrc/src/test/kotlin/dev/stapler/detekt/ClassLevelDirectRepositoryWriteOptInRuleTest.kt`	Adds rule tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

github-actions · 2026-05-27T00:55:26Z

Android Load Benchmark

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

Comparing 0c4f762 (this PR) vs 33a1f18 (baseline)
Device: API 30 x86_64 emulator — 530 pages loaded

Graph Load

Metric	This PR	Baseline	Delta
Phase 1 TTI ↓	87ms	99ms	-12ms (-12%) ✅
Phase 3 index ↓	3999ms	4052ms	-53ms (-1%) ✅

Interactive Write Latency (during Phase 3)

Metric	This PR	Baseline	Delta
Write p95 (baseline) ↓	5ms	8ms	-3ms (-37%) ✅
Write p95 (during phase 3) ↓	14ms	17ms	-3ms (-18%) ✅
Jank factor ↓	2.8x	2.13x	+0.67x (+31%) ⚠️
Concurrent writes ↑	20	19	+1ms (+5%) ✅

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 ↓	5x	5x	0 (0%)
↓ lower is better · ↑ higher is better

… nested depth, unused param - GraphDialogLayer: move frameMetric (no default) before params with defaults - FlashcardsScreen: use mutableIntStateOf/mutableFloatStateOf for primitive perf - DatabaseWriteActor: extract processDeleteBlocksForPages + logDeletesForChunk to reduce NestedBlockDepth - GraphWriter: remove unused debounceMs param from test-only startAutoSave(scope) overload Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…g leak, test timing - Use Channel<Boolean>(1) instead of RENDEZVOUS for suppress channel so trySend succeeds even if receive() hasn't started waiting yet (e.g. Unconfined collectors or test harnesses) - Add fileSystem.stopExternalChangeDetection() to stopWatching() to prevent native detector (ContentObserver/FileObserver) from leaking after the watcher is stopped without closing the scope - Pass watcherPollIntervalMs=100L in the suppression test and add a 300ms delay so the watcher actually cycles before the assertion runs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

TylerStaplerAtFanatics · 2026-05-27T03:53:49Z

@claude fix the merge conflicts

Resolved conflict in DatabaseWriteActor.kt: removed duplicate processDeleteBlocksForPages/logDeletedBlocksForChunk from main's version, keeping our null-safe logDeletesForChunk variant from the refactor. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

tstapler and others added 4 commits May 26, 2026 09:46

Copilot AI review requested due to automatic review settings May 27, 2026 00:48

Copilot started reviewing on behalf of tstapler May 27, 2026 00:48 View session

Copilot AI reviewed May 27, 2026

View reviewed changes

Comment thread kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/GraphFileWatcher.kt Outdated

Comment thread kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/GraphFileWatcher.kt

Comment thread kmp/src/jvmTest/kotlin/dev/stapler/stelekit/db/GraphLoaderWatcherTest.kt

tstapler and others added 3 commits May 26, 2026 18:04

chore: merge main into stelekit-spoace-in-search

a636633

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

tstapler merged commit d83d343 into main May 27, 2026
12 checks passed

github-actions Bot mentioned this pull request May 27, 2026

chore(main): release 0.29.0 #101

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(search): prevent Space key from dismissing the search dialog#99

fix(search): prevent Space key from dismissing the search dialog#99
tstapler merged 8 commits into
mainfrom
stelekit-spoace-in-search

tstapler commented May 27, 2026

Uh oh!

github-actions Bot commented May 27, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented May 27, 2026 •

edited

Loading

Uh oh!

TylerStaplerAtFanatics commented May 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

tstapler commented May 27, 2026

Summary

Test plan

Uh oh!

github-actions Bot commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

JVM Load Benchmark (Desktop)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Android Load Benchmark

Graph Load

Interactive Write Latency (during Phase 3)

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

Uh oh!

TylerStaplerAtFanatics commented May 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented May 27, 2026 •

edited

Loading

github-actions Bot commented May 27, 2026 •

edited

Loading